linux-kernel - Re: [PATCH 1/6] media: dt-bindings: media: camss: Add qcom,sc7180-camss binding

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Sat, 22 Jun 2024 23:24:43 +0800
From: george chan <gchan9527@...il.com>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: Robert Foss <rfoss@...nel.org>, Todor Tomov <todor.too@...il.com>, 
	"Bryan O'Donoghue" <bryan.odonoghue@...aro.org>, Mauro Carvalho Chehab <mchehab@...nel.org>, 
	Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	cros-qcom-dts-watchers@...omium.org, Bjorn Andersson <andersson@...nel.org>, 
	Konrad Dybcio <konrad.dybcio@...aro.org>, linux-media@...r.kernel.org, 
	linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/6] media: dt-bindings: media: camss: Add
 qcom,sc7180-camss binding

On Fri, Jun 21, 2024 at 6:02 PM Krzysztof Kozlowski <krzk@...nel.org> wrote:
>
> On 21/06/2024 11:40, George Chan via B4 Relay wrote:
> > From: George Chan <gchan9527@...il.com>
> >
> > Add bindings for qcom,sc7180-camss in order to support the camera
> > subsystem for sm7125 as found in the Xiaomi Redmi 9 Pro cellphone.
> >
> > Signed-off-by: George Chan <gchan9527@...il.com>
>
> Subject: just one media (first). No need to write media: media: ...
>
>
> A nit, subject: drop second/last, redundant "binding". The "dt-bindings"
> prefix is already stating that these are bindings.
> See also:
> https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18

I found the cause of this, see if I can fix it next round.

> > +title: Qualcomm CAMSS ISP
>
> What is CAMSS?
>

No idea from an outsider, i can suggest one like "title: Qualcomm
Camera SubSystem"

> > +
> > +maintainers:
> > +  - Robert Foss <robert.foss@...aro.org>
>
> For sure this is not true. Robert does not work in Linaro and I doubt he
> cares that much about camss.
>
Well, i might suggest to be like below if no objection

 maintainers:
-  - Robert Foss <robert.foss@...aro.org>
+  - Bryan O'Donoghue <bryan.odonoghue@...aro.org>
...
> > +
> > +description: |
>
> Do not need '|' unless you need to preserve formatting.

How about this? I have no idea what this is, just modify it blindly below.
-description: |
+description:

Then drop all "minItems"
...
> > +required:
> > +  - clock-names
> > +  - clocks
> > +  - compatible
>
> Keep the list ordered, the same as list properties.

Sure for this "required" list
...
>
> Missed alignment with previous <.
>
Sure
...
> > +        reg = <0 0xacb3000 0 0x1000>,
>
> reg is always the second property. See DTS coding style.
>
...
> > +        reg-names = "csid0",
>
> So this will be the third property.
>
>
>
> Best regards,
> Krzysztof
>
Best regards,
George