| Message ID | 20240206130017.7839-3-subhajit.ghosh@tweaklogic.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Support for Avago APDS9306 Ambient Light Sensor | expand |
On 06/02/2024 14:00, Subhajit Ghosh wrote: > Add vdd-supply property which is valid and useful for all the > devices in this schema. Why is it useful? How is it useful? DT describes the hardware, not because something is "useful". > > this patch depends on patch: > "dt-bindings: iio: light: Merge APDS9300 and APDS9960 schemas" This is unrelated and does not make any sense in commit msg. Drop. Best regards, Krzysztof
Hi Krzysztof, On 8/2/24 18:47, Krzysztof Kozlowski wrote: > On 06/02/2024 14:00, Subhajit Ghosh wrote: >> Add vdd-supply property which is valid and useful for all the >> devices in this schema. > > Why is it useful? How is it useful? DT describes the hardware, not > because something is "useful". I am adding this property based on a previous review: https://lore.kernel.org/all/20240121153655.5f734180@jic23-huawei/ Does the below commit message in this context make sense to you? "Add vdd-supply property for all the devices in this schema." > >> >> this patch depends on patch: >> "dt-bindings: iio: light: Merge APDS9300 and APDS9960 schemas" > > This is unrelated and does not make any sense in commit msg. Drop. Apologies for the silly questions: What does the "Drop" signify? Are you asking me to drop/delete the above "...patch depends..." message or does it have any other meaning? In the next version, do I include the ack tag by Conor for this commit? > > > Best regards, > Krzysztof > Thank you for reviewing. Regards, Subhajit Ghosh
On 08/02/2024 11:40, Subhajit Ghosh wrote: > Hi Krzysztof, > > On 8/2/24 18:47, Krzysztof Kozlowski wrote: >> On 06/02/2024 14:00, Subhajit Ghosh wrote: >>> Add vdd-supply property which is valid and useful for all the >>> devices in this schema. >> >> Why is it useful? How is it useful? DT describes the hardware, not >> because something is "useful". > I am adding this property based on a previous review: > https://lore.kernel.org/all/20240121153655.5f734180@jic23-huawei/ The property was there already. > > Does the below commit message in this context make sense to you? > "Add vdd-supply property for all the devices in this schema." It's still poor. You should say why, e.g. because devices have it. >> >>> >>> this patch depends on patch: >>> "dt-bindings: iio: light: Merge APDS9300 and APDS9960 schemas" >> >> This is unrelated and does not make any sense in commit msg. Drop. > Apologies for the silly questions: > What does the "Drop" signify? Are you asking me to drop/delete the above > "...patch depends..." message or does it have any other meaning? Drop entire paragraph. Best regards, Krzysztof
On Fri, 9 Feb 2024 08:33:11 +0100 Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > On 08/02/2024 11:40, Subhajit Ghosh wrote: > > Hi Krzysztof, > > > > On 8/2/24 18:47, Krzysztof Kozlowski wrote: > >> On 06/02/2024 14:00, Subhajit Ghosh wrote: > >>> Add vdd-supply property which is valid and useful for all the > >>> devices in this schema. > >> > >> Why is it useful? How is it useful? DT describes the hardware, not > >> because something is "useful". > > I am adding this property based on a previous review: > > https://lore.kernel.org/all/20240121153655.5f734180@jic23-huawei/ > > The property was there already. > > > > > Does the below commit message in this context make sense to you? > > "Add vdd-supply property for all the devices in this schema." > > It's still poor. You should say why, e.g. because devices have it. I'd change the patch title to: dt-bindings: iio: light: adps9300: Add missing vdd-supply For description something simple like: All devices covered by the binding have a vdd supply. > > >> > >>> > >>> this patch depends on patch: > >>> "dt-bindings: iio: light: Merge APDS9300 and APDS9960 schemas" > >> > >> This is unrelated and does not make any sense in commit msg. Drop. > > Apologies for the silly questions: > > What does the "Drop" signify? Are you asking me to drop/delete the above > > "...patch depends..." message or does it have any other meaning? > > Drop entire paragraph. > > > Best regards, > Krzysztof >
diff --git a/Documentation/devicetree/bindings/iio/light/avago,apds9300.yaml b/Documentation/devicetree/bindings/iio/light/avago,apds9300.yaml index c610780346e8..a328c8a1daef 100644 --- a/Documentation/devicetree/bindings/iio/light/avago,apds9300.yaml +++ b/Documentation/devicetree/bindings/iio/light/avago,apds9300.yaml @@ -25,6 +25,8 @@ properties: interrupts: maxItems: 1 + vdd-supply: true + additionalProperties: false required: @@ -42,6 +44,7 @@ examples: reg = <0x39>; interrupt-parent = <&gpio2>; interrupts = <29 8>; + vdd-supply = <®ulator_3v3>; }; }; ...
Add vdd-supply property which is valid and useful for all the devices in this schema. this patch depends on patch: "dt-bindings: iio: light: Merge APDS9300 and APDS9960 schemas" Signed-off-by: Subhajit Ghosh <subhajit.ghosh@tweaklogic.com> --- v5 -> v6: - Separate commit for individual change as per below review: Link: https://lore.kernel.org/all/20240121153655.5f734180@jic23-huawei/ --- .../devicetree/bindings/iio/light/avago,apds9300.yaml | 3 +++ 1 file changed, 3 insertions(+)