diff mbox series

[v6,2/5] dt-bindings: iio: light: adps9300: Add property vdd-supply

Message ID 20240206130017.7839-3-subhajit.ghosh@tweaklogic.com
State New
Headers
Series Support for Avago APDS9306 Ambient Light Sensor |

Commit Message

Subhajit Ghosh Feb. 6, 2024, 1 p.m. UTC 
  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(+)

Comments

Krzysztof Kozlowski Feb. 8, 2024, 8:17 a.m. UTC | #1 
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
Subhajit Ghosh Feb. 8, 2024, 10:40 a.m. UTC | #2 
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
Krzysztof Kozlowski Feb. 9, 2024, 7:33 a.m. UTC | #3 
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
Jonathan Cameron Feb. 10, 2024, 5:01 p.m. UTC | #4 
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 mbox series

Patch

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 = <&regulator_3v3>;
         };
     };
 ...