On Fri, Jun 21, 2024 at 6:02 PM Krzysztof Kozlowski <krzk@xxxxxxxxxx> wrote: > > On 21/06/2024 11:40, George Chan via B4 Relay wrote: > > From: George Chan <gchan9527@xxxxxxxxx> > > > > 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@xxxxxxxxx> > > 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@xxxxxxxxxx> > > 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@xxxxxxxxxx> + - Bryan O'Donoghue <bryan.odonoghue@xxxxxxxxxx> ... > > + > > +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