Re: [PATCH v4 3/4] media: dt-bindings: Add qcom,msm8939-camss

[Vladimir Zapolskiy]

Hi Vincent.

On 6/8/25 00:42, Vincent Knecht wrote:
> Le vendredi 06 juin 2025 à 13:46 +0300, Vladimir Zapolskiy a écrit :
> > Hello Vincent.
>
> Hi Vladimir,
> thank you for the review.
>
> > On 6/2/25 20:27, Vincent Knecht via B4 Relay wrote:
> > > From: Vincent Knecht <vincent.knecht@xxxxxxxxxx>
> > >
> > > Add bindings for qcom,msm8939-camss in order to support the camera
> > > subsystem for MSM8939.
> > >
> > > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>
> > > Signed-off-by: Vincent Knecht <vincent.knecht@xxxxxxxxxx>
> > > ---
> > >    .../bindings/media/qcom,msm8939-camss.yaml         | 254 +++++++++++++++++++++
> > >    1 file changed, 254 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/media/qcom,msm8939-camss.yaml
> > > b/Documentation/devicetree/bindings/media/qcom,msm8939-camss.yaml
> > > new file mode 100644
> > > index 0000000000000000000000000000000000000000..59bf16888a8235495a2080e512ce179583bcd25d
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/media/qcom,msm8939-camss.yaml
> > > @@ -0,0 +1,254 @@
> > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/media/qcom,msm8939-camss.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Qualcomm MSM8939 Camera Subsystem (CAMSS)
> > > +
> > > +maintainers:
> > > +  - Vincent Knecht <vincent.knecht@xxxxxxxxxx>
> > > +
> > > +description:
> > > +  The CAMSS IP is a CSI decoder and ISP present on Qualcomm platforms
> > > +
> > > +properties:
> > > +  compatible:
> > > +    const: qcom,msm8939-camss
> > > +
> > > +  reg:
> > > +    maxItems: 11
> > > +
> > > +  reg-names:
> > > +    items:
> > > +      - const: csid0
> > > +      - const: csid1
> > > +      - const: csid2
> > > +      - const: csiphy0
> > > +      - const: csiphy0_clk_mux
> > > +      - const: csiphy1
> > > +      - const: csiphy1_clk_mux
> > > +      - const: csi_clk_mux
> > > +      - const: ispif
> > > +      - const: vfe0
> > > +      - const: vfe0_vbif
> >
> > Please sort the list alphanumerically, accorting to the ASCII character set
> > the underscore symbol precedes lower case letters.
>
> I followed "latest and greatest" qcom,x1e80100-camss bindings which
> were largely reviewed and accepted, but if I must, so be it...

Well, this particular dt binding is known to be imperfect, for instance
see one of the series with a number of fix-ups:

https://lore.kernel.org/all/20250502204142.2064496-1-vladimir.zapolskiy@xxxxxxxxxx/

So, apparently the qcom,x1e80100-camss bindings shall be fixed in the given
aspect as well before the first .dtsi/.dts user of it appears.

> I guess this means I should also mod the DTSI from patch 4 ?
> Should I also drop R-b tags ?

I'm not sure about any granted Reviewed-by tags, this question should be
addressed to the persons who gave the tags...

The formal process says this [1]:

  Both Tested-by and Reviewed-by tags, once received on mailing list from tester
  or reviewer, should be added by author to the applicable patches when sending
  next versions.  However if the patch has changed substantially in following
  version, these tags might not be applicable anymore and thus should be removed.
  Usually removal of someone's Tested-by or Reviewed-by tags should be mentioned
  in the patch changelog (after the '---' separator).

I would say the fixes should not be considered as substantial changes.

[1] Documentation/process/submitting-patches.rst

--
Best wishes,
Vladimir