[v3,02/14] dt-bindings: media: rkisp1: Add i.MX8MP ISP example

Message ID 20221118093931.1284465-3-paul.elder@ideasonboard.com
State New
Headers
Series media: rkisp1: Add support for i.MX8MP |

Commit Message

Paul Elder Nov. 18, 2022, 9:39 a.m. UTC
  From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Add an example to the rockchip-isp1 DT binding that showcases usage of
the parallel input of the ISP, connected to the CSI-2 receiver internal
to the i.MX8MP.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 .../bindings/media/rockchip-isp1.yaml         | 72 +++++++++++++++++++
 1 file changed, 72 insertions(+)
  

Comments

Krzysztof Kozlowski Nov. 18, 2022, 1:06 p.m. UTC | #1
On 18/11/2022 10:39, Paul Elder wrote:
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Add an example to the rockchip-isp1 DT binding that showcases usage of
> the parallel input of the ISP, connected to the CSI-2 receiver internal
> to the i.MX8MP.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Missing SoB.

> ---
>  .../bindings/media/rockchip-isp1.yaml         | 72 +++++++++++++++++++
>  1 file changed, 72 insertions(+)
> 

I don't know what do you demonstrate there... usage of endpoints? That's
the only difference. Such usage is the same everywhere, nothing specific
to this example. You already have two examples, so I don't think this
brings anything more.

Best regards,
Krzysztof
  
Rob Herring Nov. 18, 2022, 1:31 p.m. UTC | #2
On Fri, 18 Nov 2022 18:39:19 +0900, Paul Elder wrote:
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Add an example to the rockchip-isp1 DT binding that showcases usage of
> the parallel input of the ISP, connected to the CSI-2 receiver internal
> to the i.MX8MP.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  .../bindings/media/rockchip-isp1.yaml         | 72 +++++++++++++++++++
>  1 file changed, 72 insertions(+)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/media/rockchip-isp1.example.dts:199:18: fatal error: dt-bindings/media/video-interfaces.h: No such file or directory
  199 |         #include <dt-bindings/media/video-interfaces.h>
      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[1]: *** [scripts/Makefile.lib:406: Documentation/devicetree/bindings/media/rockchip-isp1.example.dtb] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1492: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20221118093931.1284465-3-paul.elder@ideasonboard.com

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command.
  
Paul Elder Nov. 19, 2022, 6:33 a.m. UTC | #3
On Fri, Nov 18, 2022 at 07:31:19AM -0600, Rob Herring wrote:
> 
> On Fri, 18 Nov 2022 18:39:19 +0900, Paul Elder wrote:
> > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > 
> > Add an example to the rockchip-isp1 DT binding that showcases usage of
> > the parallel input of the ISP, connected to the CSI-2 receiver internal
> > to the i.MX8MP.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  .../bindings/media/rockchip-isp1.yaml         | 72 +++++++++++++++++++
> >  1 file changed, 72 insertions(+)
> > 
> 
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> Documentation/devicetree/bindings/media/rockchip-isp1.example.dts:199:18: fatal error: dt-bindings/media/video-interfaces.h: No such file or directory
>   199 |         #include <dt-bindings/media/video-interfaces.h>
>       |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> compilation terminated.
> make[1]: *** [scripts/Makefile.lib:406: Documentation/devicetree/bindings/media/rockchip-isp1.example.dtb] Error 1
> make[1]: *** Waiting for unfinished jobs....
> make: *** [Makefile:1492: dt_binding_check] Error 2
> 
> doc reference errors (make refcheckdocs):
> 
> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20221118093931.1284465-3-paul.elder@ideasonboard.com
> 
> This check can fail if there are any dependencies. The base for a patch
> series is generally the most recent rc1.

In the cover letter I mention:

"This series depends on v3 of "dt-bindings: media: Add macros for video
interface bus types" [1]."

[1] https://lore.kernel.org/linux-media/20220615221410.27459-2-laurent.pinchart@ideasonboard.com/


Paul

> 
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
> 
> pip3 install dtschema --upgrade
> 
> Please check and re-submit after running the above command.
>
  
Paul Elder Nov. 19, 2022, 6:55 a.m. UTC | #4
On Fri, Nov 18, 2022 at 02:06:14PM +0100, Krzysztof Kozlowski wrote:
> On 18/11/2022 10:39, Paul Elder wrote:
> > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > 
> > Add an example to the rockchip-isp1 DT binding that showcases usage of
> > the parallel input of the ISP, connected to the CSI-2 receiver internal
> > to the i.MX8MP.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Missing SoB.

I don't quite understand. I see an SoB right there.

> 
> > ---
> >  .../bindings/media/rockchip-isp1.yaml         | 72 +++++++++++++++++++
> >  1 file changed, 72 insertions(+)
> > 
> 
> I don't know what do you demonstrate there... usage of endpoints? That's
> the only difference. Such usage is the same everywhere, nothing specific

I guess...? Doesn't the same argument apply against the px30 example too
then?

> to this example. You already have two examples, so I don't think this
> brings anything more.

We do have usage of this in imx8mp.dtsi and overlays for the ISP, but
those patches haven't been sent/merged yet, so in the meantime I think
there is value in providing an example here for the imx8mp.


Thanks,

Paul
  
Laurent Pinchart Nov. 19, 2022, 4:59 p.m. UTC | #5
Hi Krzysztof,

On Fri, Nov 18, 2022 at 02:06:14PM +0100, Krzysztof Kozlowski wrote:
> On 18/11/2022 10:39, Paul Elder wrote:
> > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > 
> > Add an example to the rockchip-isp1 DT binding that showcases usage of
> > the parallel input of the ISP, connected to the CSI-2 receiver internal
> > to the i.MX8MP.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Missing SoB.
> 
> > ---
> >  .../bindings/media/rockchip-isp1.yaml         | 72 +++++++++++++++++++
> >  1 file changed, 72 insertions(+)
> > 
> 
> I don't know what do you demonstrate there... usage of endpoints? That's
> the only difference. Such usage is the same everywhere, nothing specific
> to this example. You already have two examples, so I don't think this
> brings anything more.

The i.MX8MP is the only SoC integrating this ISP (and supported in
mainlineĆ  that has an external CSI-2 receiver, as opposed to using the
CSI-2 receiver from the ISP. This patch this showcases the DT
integration for that use case. If you think it's not worth it, I'm fine
dropping it.
  
Krzysztof Kozlowski Nov. 20, 2022, 10:34 a.m. UTC | #6
On 19/11/2022 17:59, Laurent Pinchart wrote:
> Hi Krzysztof,
> 
> On Fri, Nov 18, 2022 at 02:06:14PM +0100, Krzysztof Kozlowski wrote:
>> On 18/11/2022 10:39, Paul Elder wrote:
>>> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>
>>> Add an example to the rockchip-isp1 DT binding that showcases usage of
>>> the parallel input of the ISP, connected to the CSI-2 receiver internal
>>> to the i.MX8MP.
>>>
>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>
>> Missing SoB.
>>
>>> ---
>>>  .../bindings/media/rockchip-isp1.yaml         | 72 +++++++++++++++++++
>>>  1 file changed, 72 insertions(+)
>>>
>>
>> I don't know what do you demonstrate there... usage of endpoints? That's
>> the only difference. Such usage is the same everywhere, nothing specific
>> to this example. You already have two examples, so I don't think this
>> brings anything more.
> 
> The i.MX8MP is the only SoC integrating this ISP (and supported in
> mainlineĆ  that has an external CSI-2 receiver, as opposed to using the
> CSI-2 receiver from the ISP. This patch this showcases the DT
> integration for that use case. If you think it's not worth it, I'm fine
> dropping it.

The purpose of examples are not to demonstrate the SoC, but only this
given binding.
> 

Best regards,
Krzysztof
  
Krzysztof Kozlowski Nov. 20, 2022, 10:36 a.m. UTC | #7
On 19/11/2022 07:55, Paul Elder wrote:
> On Fri, Nov 18, 2022 at 02:06:14PM +0100, Krzysztof Kozlowski wrote:
>> On 18/11/2022 10:39, Paul Elder wrote:
>>> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>
>>> Add an example to the rockchip-isp1 DT binding that showcases usage of
>>> the parallel input of the ISP, connected to the CSI-2 receiver internal
>>> to the i.MX8MP.
>>>
>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>
>> Missing SoB.
> 
> I don't quite understand. I see an SoB right there.

Laurent did not sent it. Did you run checkpatch before sending?

> 
>>
>>> ---
>>>  .../bindings/media/rockchip-isp1.yaml         | 72 +++++++++++++++++++
>>>  1 file changed, 72 insertions(+)
>>>
>>
>> I don't know what do you demonstrate there... usage of endpoints? That's
>> the only difference. Such usage is the same everywhere, nothing specific
> 
> I guess...? Doesn't the same argument apply against the px30 example too
> then?
> 
>> to this example. You already have two examples, so I don't think this
>> brings anything more.
> 
> We do have usage of this in imx8mp.dtsi and overlays for the ISP, but
> those patches haven't been sent/merged yet, so in the meantime I think
> there is value in providing an example here for the imx8mp.

The examples are not for demonstrating imx8mp or any other soc, but this
one given binding. Changing compatibles and few properties is not a
different example - from "exampleness" point of view it is very similar.

Best regards,
Krzysztof
  
Paul Elder Nov. 21, 2022, 5:09 a.m. UTC | #8
On Sun, Nov 20, 2022 at 11:36:31AM +0100, Krzysztof Kozlowski wrote:
> On 19/11/2022 07:55, Paul Elder wrote:
> > On Fri, Nov 18, 2022 at 02:06:14PM +0100, Krzysztof Kozlowski wrote:
> >> On 18/11/2022 10:39, Paul Elder wrote:
> >>> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>>
> >>> Add an example to the rockchip-isp1 DT binding that showcases usage of
> >>> the parallel input of the ISP, connected to the CSI-2 receiver internal
> >>> to the i.MX8MP.
> >>>
> >>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>
> >> Missing SoB.
> > 
> > I don't quite understand. I see an SoB right there.
> 
> Laurent did not sent it. Did you run checkpatch before sending?

That's why he's on the "From:" in the beginning. checkpatch says it's
fine.

> 
> > 
> >>
> >>> ---
> >>>  .../bindings/media/rockchip-isp1.yaml         | 72 +++++++++++++++++++
> >>>  1 file changed, 72 insertions(+)
> >>>
> >>
> >> I don't know what do you demonstrate there... usage of endpoints? That's
> >> the only difference. Such usage is the same everywhere, nothing specific
> > 
> > I guess...? Doesn't the same argument apply against the px30 example too
> > then?
> > 
> >> to this example. You already have two examples, so I don't think this
> >> brings anything more.
> > 
> > We do have usage of this in imx8mp.dtsi and overlays for the ISP, but
> > those patches haven't been sent/merged yet, so in the meantime I think
> > there is value in providing an example here for the imx8mp.
> 
> The examples are not for demonstrating imx8mp or any other soc, but this
> one given binding. Changing compatibles and few properties is not a
> different example - from "exampleness" point of view it is very similar.

Ah okay, I see.


Paul
  
Krzysztof Kozlowski Nov. 21, 2022, 8:04 a.m. UTC | #9
On 21/11/2022 06:09, Paul Elder wrote:
> On Sun, Nov 20, 2022 at 11:36:31AM +0100, Krzysztof Kozlowski wrote:
>> On 19/11/2022 07:55, Paul Elder wrote:
>>> On Fri, Nov 18, 2022 at 02:06:14PM +0100, Krzysztof Kozlowski wrote:
>>>> On 18/11/2022 10:39, Paul Elder wrote:
>>>>> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>>>
>>>>> Add an example to the rockchip-isp1 DT binding that showcases usage of
>>>>> the parallel input of the ISP, connected to the CSI-2 receiver internal
>>>>> to the i.MX8MP.
>>>>>
>>>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>>
>>>> Missing SoB.
>>>
>>> I don't quite understand. I see an SoB right there.
>>
>> Laurent did not sent it. Did you run checkpatch before sending?
> 
> That's why he's on the "From:" in the beginning. checkpatch says it's
> fine.

Ah, indeed, checkpatch misses that feature (it's part of Greg's
verify_signedoff.sh). Anyway, your SoB is missing, as Laurent did not
send the patch.

Best regards,
Krzysztof
  
Laurent Pinchart Nov. 21, 2022, 10:38 a.m. UTC | #10
On Mon, Nov 21, 2022 at 09:04:29AM +0100, Krzysztof Kozlowski wrote:
> On 21/11/2022 06:09, Paul Elder wrote:
> > On Sun, Nov 20, 2022 at 11:36:31AM +0100, Krzysztof Kozlowski wrote:
> >> On 19/11/2022 07:55, Paul Elder wrote:
> >>> On Fri, Nov 18, 2022 at 02:06:14PM +0100, Krzysztof Kozlowski wrote:
> >>>> On 18/11/2022 10:39, Paul Elder wrote:
> >>>>> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>>>>
> >>>>> Add an example to the rockchip-isp1 DT binding that showcases usage of
> >>>>> the parallel input of the ISP, connected to the CSI-2 receiver internal
> >>>>> to the i.MX8MP.
> >>>>>
> >>>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>>>
> >>>> Missing SoB.
> >>>
> >>> I don't quite understand. I see an SoB right there.
> >>
> >> Laurent did not sent it. Did you run checkpatch before sending?
> > 
> > That's why he's on the "From:" in the beginning. checkpatch says it's
> > fine.
> 
> Ah, indeed, checkpatch misses that feature (it's part of Greg's
> verify_signedoff.sh). Anyway, your SoB is missing, as Laurent did not
> send the patch.

I thought adding an SoB was only required either when making changes or
when pushing commits through git, not when forwarding patches on mailing
lists ?
  
Krzysztof Kozlowski Nov. 21, 2022, 11:16 a.m. UTC | #11
On 21/11/2022 11:38, Laurent Pinchart wrote:
> On Mon, Nov 21, 2022 at 09:04:29AM +0100, Krzysztof Kozlowski wrote:
>> On 21/11/2022 06:09, Paul Elder wrote:
>>> On Sun, Nov 20, 2022 at 11:36:31AM +0100, Krzysztof Kozlowski wrote:
>>>> On 19/11/2022 07:55, Paul Elder wrote:
>>>>> On Fri, Nov 18, 2022 at 02:06:14PM +0100, Krzysztof Kozlowski wrote:
>>>>>> On 18/11/2022 10:39, Paul Elder wrote:
>>>>>>> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>>>>>
>>>>>>> Add an example to the rockchip-isp1 DT binding that showcases usage of
>>>>>>> the parallel input of the ISP, connected to the CSI-2 receiver internal
>>>>>>> to the i.MX8MP.
>>>>>>>
>>>>>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>>>>
>>>>>> Missing SoB.
>>>>>
>>>>> I don't quite understand. I see an SoB right there.
>>>>
>>>> Laurent did not sent it. Did you run checkpatch before sending?
>>>
>>> That's why he's on the "From:" in the beginning. checkpatch says it's
>>> fine.
>>
>> Ah, indeed, checkpatch misses that feature (it's part of Greg's
>> verify_signedoff.sh). Anyway, your SoB is missing, as Laurent did not
>> send the patch.
> 
> I thought adding an SoB was only required either when making changes or
> when pushing commits through git, not when forwarding patches on mailing
> lists ?

Anyone touching the file should signed it off. You cannot send it
without touching (e.g. git format-patch).

https://elixir.bootlin.com/linux/v5.19-rc5/source/Documentation/process/submitting-patches.rst#L397

https://elixir.bootlin.com/linux/v5.19-rc5/source/Documentation/process/submitting-patches.rst#L420

Best regards,
Krzysztof
  
Laurent Pinchart Nov. 21, 2022, 1:50 p.m. UTC | #12
On Mon, Nov 21, 2022 at 12:16:41PM +0100, Krzysztof Kozlowski wrote:
> On 21/11/2022 11:38, Laurent Pinchart wrote:
> > On Mon, Nov 21, 2022 at 09:04:29AM +0100, Krzysztof Kozlowski wrote:
> >> On 21/11/2022 06:09, Paul Elder wrote:
> >>> On Sun, Nov 20, 2022 at 11:36:31AM +0100, Krzysztof Kozlowski wrote:
> >>>> On 19/11/2022 07:55, Paul Elder wrote:
> >>>>> On Fri, Nov 18, 2022 at 02:06:14PM +0100, Krzysztof Kozlowski wrote:
> >>>>>> On 18/11/2022 10:39, Paul Elder wrote:
> >>>>>>> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>>>>>>
> >>>>>>> Add an example to the rockchip-isp1 DT binding that showcases usage of
> >>>>>>> the parallel input of the ISP, connected to the CSI-2 receiver internal
> >>>>>>> to the i.MX8MP.
> >>>>>>>
> >>>>>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>>>>>
> >>>>>> Missing SoB.
> >>>>>
> >>>>> I don't quite understand. I see an SoB right there.
> >>>>
> >>>> Laurent did not sent it. Did you run checkpatch before sending?
> >>>
> >>> That's why he's on the "From:" in the beginning. checkpatch says it's
> >>> fine.
> >>
> >> Ah, indeed, checkpatch misses that feature (it's part of Greg's
> >> verify_signedoff.sh). Anyway, your SoB is missing, as Laurent did not
> >> send the patch.
> > 
> > I thought adding an SoB was only required either when making changes or
> > when pushing commits through git, not when forwarding patches on mailing
> > lists ?
> 
> Anyone touching the file should signed it off. You cannot send it
> without touching (e.g. git format-patch).
> 
> https://elixir.bootlin.com/linux/v5.19-rc5/source/Documentation/process/submitting-patches.rst#L397
> 
> https://elixir.bootlin.com/linux/v5.19-rc5/source/Documentation/process/submitting-patches.rst#L420

The second link states

  SoB chains should reflect the **real** route a patch took as it was
  propagated to the maintainers and ultimately to Linus, with the first
  SoB entry signalling primary authorship of a single author.

This series will (eventually) be upstreamed by me through a pull request
to Mauro. Paul's SoB will thus not be needed. Of course you have no way
to know this when reviewing the series on the list.

Adding a SoB line when taking a patch in a git tree is standard
practice, but when posting unmodified patches to a mailing list, there's
more of a grey area. Look at
https://lore.kernel.org/all/20221024113058.096628238@linuxfoundation.org/
for instance, posted by Greg, but without his SoB.
  
Krzysztof Kozlowski Nov. 21, 2022, 4:37 p.m. UTC | #13
On 21/11/2022 14:50, Laurent Pinchart wrote:
> On Mon, Nov 21, 2022 at 12:16:41PM +0100, Krzysztof Kozlowski wrote:
>> On 21/11/2022 11:38, Laurent Pinchart wrote:
>>> On Mon, Nov 21, 2022 at 09:04:29AM +0100, Krzysztof Kozlowski wrote:
>>>> On 21/11/2022 06:09, Paul Elder wrote:
>>>>> On Sun, Nov 20, 2022 at 11:36:31AM +0100, Krzysztof Kozlowski wrote:
>>>>>> On 19/11/2022 07:55, Paul Elder wrote:
>>>>>>> On Fri, Nov 18, 2022 at 02:06:14PM +0100, Krzysztof Kozlowski wrote:
>>>>>>>> On 18/11/2022 10:39, Paul Elder wrote:
>>>>>>>>> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>>>>>>>
>>>>>>>>> Add an example to the rockchip-isp1 DT binding that showcases usage of
>>>>>>>>> the parallel input of the ISP, connected to the CSI-2 receiver internal
>>>>>>>>> to the i.MX8MP.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>>>>>>
>>>>>>>> Missing SoB.
>>>>>>>
>>>>>>> I don't quite understand. I see an SoB right there.
>>>>>>
>>>>>> Laurent did not sent it. Did you run checkpatch before sending?
>>>>>
>>>>> That's why he's on the "From:" in the beginning. checkpatch says it's
>>>>> fine.
>>>>
>>>> Ah, indeed, checkpatch misses that feature (it's part of Greg's
>>>> verify_signedoff.sh). Anyway, your SoB is missing, as Laurent did not
>>>> send the patch.
>>>
>>> I thought adding an SoB was only required either when making changes or
>>> when pushing commits through git, not when forwarding patches on mailing
>>> lists ?
>>
>> Anyone touching the file should signed it off. You cannot send it
>> without touching (e.g. git format-patch).
>>
>> https://elixir.bootlin.com/linux/v5.19-rc5/source/Documentation/process/submitting-patches.rst#L397
>>
>> https://elixir.bootlin.com/linux/v5.19-rc5/source/Documentation/process/submitting-patches.rst#L420
> 
> The second link states
> 
>   SoB chains should reflect the **real** route a patch took as it was
>   propagated to the maintainers and ultimately to Linus, with the first
>   SoB entry signalling primary authorship of a single author.
> 
> This series will (eventually) be upstreamed by me through a pull request
> to Mauro. Paul's SoB will thus not be needed. Of course you have no way
> to know this when reviewing the series on the list.
> 
> Adding a SoB line when taking a patch in a git tree is standard
> practice, but when posting unmodified patches to a mailing list, there's
> more of a grey area. Look at
> https://lore.kernel.org/all/20221024113058.096628238@linuxfoundation.org/
> for instance, posted by Greg, but without his SoB.

I have no clue what Paul modified here what not. I am not going to
investigate and I have no way to actually perform such investigation. I
cannot verify the source.

The case with Greg, is indeed surprising, but I could perform the
verification, because the work is both public and in known place.

It's expected for submitter to certify (c) from the list which was BTW
expressed also many times during many reviews by many people.

Best regards,
Krzysztof
  
Krzysztof Kozlowski Nov. 21, 2022, 4:39 p.m. UTC | #14
On 21/11/2022 17:37, Krzysztof Kozlowski wrote:
> On 21/11/2022 14:50, Laurent Pinchart wrote:
>> On Mon, Nov 21, 2022 at 12:16:41PM +0100, Krzysztof Kozlowski wrote:
>>> On 21/11/2022 11:38, Laurent Pinchart wrote:
>>>> On Mon, Nov 21, 2022 at 09:04:29AM +0100, Krzysztof Kozlowski wrote:
>>>>> On 21/11/2022 06:09, Paul Elder wrote:
>>>>>> On Sun, Nov 20, 2022 at 11:36:31AM +0100, Krzysztof Kozlowski wrote:
>>>>>>> On 19/11/2022 07:55, Paul Elder wrote:
>>>>>>>> On Fri, Nov 18, 2022 at 02:06:14PM +0100, Krzysztof Kozlowski wrote:
>>>>>>>>> On 18/11/2022 10:39, Paul Elder wrote:
>>>>>>>>>> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>>>>>>>>
>>>>>>>>>> Add an example to the rockchip-isp1 DT binding that showcases usage of
>>>>>>>>>> the parallel input of the ISP, connected to the CSI-2 receiver internal
>>>>>>>>>> to the i.MX8MP.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>>>>>>>
>>>>>>>>> Missing SoB.
>>>>>>>>
>>>>>>>> I don't quite understand. I see an SoB right there.
>>>>>>>
>>>>>>> Laurent did not sent it. Did you run checkpatch before sending?
>>>>>>
>>>>>> That's why he's on the "From:" in the beginning. checkpatch says it's
>>>>>> fine.
>>>>>
>>>>> Ah, indeed, checkpatch misses that feature (it's part of Greg's
>>>>> verify_signedoff.sh). Anyway, your SoB is missing, as Laurent did not
>>>>> send the patch.
>>>>
>>>> I thought adding an SoB was only required either when making changes or
>>>> when pushing commits through git, not when forwarding patches on mailing
>>>> lists ?
>>>
>>> Anyone touching the file should signed it off. You cannot send it
>>> without touching (e.g. git format-patch).
>>>
>>> https://elixir.bootlin.com/linux/v5.19-rc5/source/Documentation/process/submitting-patches.rst#L397
>>>
>>> https://elixir.bootlin.com/linux/v5.19-rc5/source/Documentation/process/submitting-patches.rst#L420
>>
>> The second link states
>>
>>   SoB chains should reflect the **real** route a patch took as it was
>>   propagated to the maintainers and ultimately to Linus, with the first
>>   SoB entry signalling primary authorship of a single author.
>>
>> This series will (eventually) be upstreamed by me through a pull request
>> to Mauro. Paul's SoB will thus not be needed. Of course you have no way
>> to know this when reviewing the series on the list.
>>
>> Adding a SoB line when taking a patch in a git tree is standard
>> practice, but when posting unmodified patches to a mailing list, there's
>> more of a grey area. Look at
>> https://lore.kernel.org/all/20221024113058.096628238@linuxfoundation.org/
>> for instance, posted by Greg, but without his SoB.
> 
> I have no clue what Paul modified here what not. I am not going to
> investigate and I have no way to actually perform such investigation. I
> cannot verify the source.

BTW, rebasing is modifying and Paul probably did it (or is likely that
will rebase in the future). Greg did not perform rebases on these, I think.

> 
> The case with Greg, is indeed surprising, but I could perform the
> verification, because the work is both public and in known place.
> 
> It's expected for submitter to certify (c) from the list which was BTW
> expressed also many times during many reviews by many people.
> 
> Best regards,
> Krzysztof
> 

Best regards,
Krzysztof
  
Laurent Pinchart Nov. 21, 2022, 4:48 p.m. UTC | #15
Hi Krzysztof,

(CC'ing Greg)

On Mon, Nov 21, 2022 at 05:37:33PM +0100, Krzysztof Kozlowski wrote:
> On 21/11/2022 14:50, Laurent Pinchart wrote:
> > On Mon, Nov 21, 2022 at 12:16:41PM +0100, Krzysztof Kozlowski wrote:
> >> On 21/11/2022 11:38, Laurent Pinchart wrote:
> >>> On Mon, Nov 21, 2022 at 09:04:29AM +0100, Krzysztof Kozlowski wrote:
> >>>> On 21/11/2022 06:09, Paul Elder wrote:
> >>>>> On Sun, Nov 20, 2022 at 11:36:31AM +0100, Krzysztof Kozlowski wrote:
> >>>>>> On 19/11/2022 07:55, Paul Elder wrote:
> >>>>>>> On Fri, Nov 18, 2022 at 02:06:14PM +0100, Krzysztof Kozlowski wrote:
> >>>>>>>> On 18/11/2022 10:39, Paul Elder wrote:
> >>>>>>>>> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>>>>>>>>
> >>>>>>>>> Add an example to the rockchip-isp1 DT binding that showcases usage of
> >>>>>>>>> the parallel input of the ISP, connected to the CSI-2 receiver internal
> >>>>>>>>> to the i.MX8MP.
> >>>>>>>>>
> >>>>>>>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>>>>>>>
> >>>>>>>> Missing SoB.
> >>>>>>>
> >>>>>>> I don't quite understand. I see an SoB right there.
> >>>>>>
> >>>>>> Laurent did not sent it. Did you run checkpatch before sending?
> >>>>>
> >>>>> That's why he's on the "From:" in the beginning. checkpatch says it's
> >>>>> fine.
> >>>>
> >>>> Ah, indeed, checkpatch misses that feature (it's part of Greg's
> >>>> verify_signedoff.sh). Anyway, your SoB is missing, as Laurent did not
> >>>> send the patch.
> >>>
> >>> I thought adding an SoB was only required either when making changes or
> >>> when pushing commits through git, not when forwarding patches on mailing
> >>> lists ?
> >>
> >> Anyone touching the file should signed it off. You cannot send it
> >> without touching (e.g. git format-patch).
> >>
> >> https://elixir.bootlin.com/linux/v5.19-rc5/source/Documentation/process/submitting-patches.rst#L397
> >>
> >> https://elixir.bootlin.com/linux/v5.19-rc5/source/Documentation/process/submitting-patches.rst#L420
> > 
> > The second link states
> > 
> >   SoB chains should reflect the **real** route a patch took as it was
> >   propagated to the maintainers and ultimately to Linus, with the first
> >   SoB entry signalling primary authorship of a single author.
> > 
> > This series will (eventually) be upstreamed by me through a pull request
> > to Mauro. Paul's SoB will thus not be needed. Of course you have no way
> > to know this when reviewing the series on the list.
> > 
> > Adding a SoB line when taking a patch in a git tree is standard
> > practice, but when posting unmodified patches to a mailing list, there's
> > more of a grey area. Look at
> > https://lore.kernel.org/all/20221024113058.096628238@linuxfoundation.org/
> > for instance, posted by Greg, but without his SoB.
> 
> I have no clue what Paul modified here what not. I am not going to
> investigate and I have no way to actually perform such investigation. I
> cannot verify the source.
> 
> The case with Greg, is indeed surprising, but I could perform the
> verification, because the work is both public and in known place.
> 
> It's expected for submitter to certify (c) from the list which was BTW
> expressed also many times during many reviews by many people.

Given that this patch will be dropped anyway, it doesn't matter much in
this specific case, but for future reference, I've CC'ed Greg to get his
opinion on the matter.
  

Patch

diff --git a/Documentation/devicetree/bindings/media/rockchip-isp1.yaml b/Documentation/devicetree/bindings/media/rockchip-isp1.yaml
index 95cf945f7ac5..88d9bc378f79 100644
--- a/Documentation/devicetree/bindings/media/rockchip-isp1.yaml
+++ b/Documentation/devicetree/bindings/media/rockchip-isp1.yaml
@@ -285,3 +285,75 @@  examples:
             };
         };
     };
+
+  - |
+    #include <dt-bindings/clock/imx8mp-clock.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/media/video-interfaces.h>
+    #include <dt-bindings/power/imx8mp-power.h>
+
+    parent2: parent {
+        #address-cells = <1>;
+        #size-cells = <1>;
+
+        isp@32e10000 {
+            compatible = "fsl,imx8mp-isp";
+            reg = <0x32e10000 0x10000>;
+            interrupts = <GIC_SPI 74 IRQ_TYPE_LEVEL_HIGH>;
+            clocks = <&clk IMX8MP_CLK_MEDIA_ISP_ROOT>,
+                     <&clk IMX8MP_CLK_MEDIA_AXI_ROOT>,
+                     <&clk IMX8MP_CLK_MEDIA_APB_ROOT>;
+            clock-names = "isp", "hclk", "aclk";
+            assigned-clocks = <&clk IMX8MP_CLK_MEDIA_ISP>;
+            assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_500M>;
+            assigned-clock-rates = <500000000>;
+            power-domains = <&media_blk_ctrl IMX8MP_MEDIABLK_PD_ISP>;
+            fsl,blk-ctrl = <&media_blk_ctrl 0>;
+
+            ports {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                port@1 {
+                    reg = <1>;
+                    isp0_in: endpoint {
+                        bus-type = <MEDIA_BUS_TYPE_PARALLEL>;
+                        remote-endpoint = <&mipi_csi_0_out>;
+                    };
+                };
+            };
+        };
+
+        csi@32e40000 {
+            compatible = "fsl,imx8mp-mipi-csi2", "fsl,imx8mm-mipi-csi2";
+            reg = <0x32e40000 0x10000>;
+            interrupts = <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>;
+            clock-frequency = <500000000>;
+            clocks = <&clk IMX8MP_CLK_MEDIA_APB_ROOT>,
+                     <&clk IMX8MP_CLK_MEDIA_CAM1_PIX_ROOT>,
+                     <&clk IMX8MP_CLK_MEDIA_MIPI_PHY1_REF_ROOT>,
+                     <&clk IMX8MP_CLK_MEDIA_AXI_ROOT>;
+            clock-names = "pclk", "wrap", "phy", "axi";
+            assigned-clocks = <&clk IMX8MP_CLK_MEDIA_CAM1_PIX>;
+            assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_1000M>;
+            assigned-clock-rates = <500000000>;
+            power-domains = <&media_blk_ctrl IMX8MP_MEDIABLK_PD_MIPI_CSI2_1>;
+
+            ports {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                port@0 {
+                    reg = <0>;
+                };
+
+                port@1 {
+                    reg = <1>;
+                    mipi_csi_0_out: endpoint {
+                        remote-endpoint = <&isp0_in>;
+                    };
+                };
+            };
+        };
+    };
+...