1. Patchew
  2. linux
  3. [PATCH v5 0/3] Support for Avago APDS9306 Ambient Light Sensor
  4. View patch

[PATCH v5 2/3] dt-bindings: iio: light: Avago APDS9306

Subhajit Ghosh posted 3 patches 1 year, 11 months ago
There is a newer version of this series
[PATCH v5 2/3] dt-bindings: iio: light: Avago APDS9306
Posted by Subhajit Ghosh 1 year, 11 months ago 
Adding device tree support for APDS9306 Ambient Light Sensor.
Updating datasheet hyperlinks.
Adding interrupt definition macro and header file.
Adding vdd-supply property.

Signed-off-by: Subhajit Ghosh <subhajit.ghosh@tweaklogic.com>
---
v2 -> v5:
 - Implemented changes as per previous reviews:
   Link: https://lore.kernel.org/lkml/20231028142944.7e210eb6@jic23-huawei/
   Link: https://lore.kernel.org/lkml/22e9e5e9-d26a-46e9-8986-5062bbfd72ec@linaro.org/
---
 .../bindings/iio/light/avago,apds9300.yaml        | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/light/avago,apds9300.yaml b/Documentation/devicetree/bindings/iio/light/avago,apds9300.yaml
index c610780346e8..bee73a590424 100644
--- a/Documentation/devicetree/bindings/iio/light/avago,apds9300.yaml
+++ b/Documentation/devicetree/bindings/iio/light/avago,apds9300.yaml
@@ -4,19 +4,21 @@
 $id: http://devicetree.org/schemas/iio/light/avago,apds9300.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Avago Gesture/RGB/ALS/Proximity sensors
+title: Avago (Broadcom) Gesture/RGB/ALS/Proximity sensors
 
 maintainers:
   - Subhajit Ghosh <subhajit.ghosh@tweaklogic.com>
 
 description: |
-  Datasheet: https://www.avagotech.com/docs/AV02-1077EN
-  Datasheet: https://www.avagotech.com/docs/AV02-4191EN
+  Datasheet: https://docs.broadcom.com/doc/AV02-1077EN
+  Datasheet: https://docs.broadcom.com/doc/AV02-4191EN
+  Datasheet: https://docs.broadcom.com/doc/AV02-4755EN
 
 properties:
   compatible:
     enum:
       - avago,apds9300
+      - avago,apds9306
       - avago,apds9960
 
   reg:
@@ -25,6 +27,8 @@ properties:
   interrupts:
     maxItems: 1
 
+  vdd-supply: true
+
 additionalProperties: false
 
 required:
@@ -33,6 +37,8 @@ required:
 
 examples:
   - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
     i2c {
         #address-cells = <1>;
         #size-cells = <0>;
@@ -41,7 +47,8 @@ examples:
             compatible = "avago,apds9300";
             reg = <0x39>;
             interrupt-parent = <&gpio2>;
-            interrupts = <29 8>;
+            interrupts = <29 IRQ_TYPE_LEVEL_LOW>;
+            vdd-supply = <&regulator_3v3>;
         };
     };
 ...
-- 
2.34.1
Re: [PATCH v5 2/3] dt-bindings: iio: light: Avago APDS9306
Posted by Krzysztof Kozlowski 1 year, 11 months ago 
On 21/01/2024 06:17, Subhajit Ghosh wrote:
> Adding device tree support for APDS9306 Ambient Light Sensor.
> Updating datasheet hyperlinks.
> Adding interrupt definition macro and header file.
> Adding vdd-supply property.

Why? Do other devices have it?


>  required:
> @@ -33,6 +37,8 @@ required:
>  
>  examples:
>    - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
>      i2c {
>          #address-cells = <1>;
>          #size-cells = <0>;
> @@ -41,7 +47,8 @@ examples:
>              compatible = "avago,apds9300";
>              reg = <0x39>;
>              interrupt-parent = <&gpio2>;
> -            interrupts = <29 8>;
> +            interrupts = <29 IRQ_TYPE_LEVEL_LOW>;

Separate change.

Best regards,
Krzysztof
Re: [PATCH v5 2/3] dt-bindings: iio: light: Avago APDS9306
Posted by Subhajit Ghosh 1 year, 11 months ago 
On 22/1/24 20:21, Krzysztof Kozlowski wrote:
> On 21/01/2024 06:17, Subhajit Ghosh wrote:
>> Adding device tree support for APDS9306 Ambient Light Sensor.
>> Updating datasheet hyperlinks.
>> Adding interrupt definition macro and header file.
>> Adding vdd-supply property.
> 
> Why? Do other devices have it?
Are you referring to vdd-supply? No, I guess, haven't checked actually.
If other devices don't have, can you suggest the best way to handle that?
> 
> 
>>   required:
>> @@ -33,6 +37,8 @@ required:
>>   
>>   examples:
>>     - |
>> +    #include <dt-bindings/interrupt-controller/irq.h>
>> +
>>       i2c {
>>           #address-cells = <1>;
>>           #size-cells = <0>;
>> @@ -41,7 +47,8 @@ examples:
>>               compatible = "avago,apds9300";
>>               reg = <0x39>;
>>               interrupt-parent = <&gpio2>;
>> -            interrupts = <29 8>;
>> +            interrupts = <29 IRQ_TYPE_LEVEL_LOW>;
> 
> Separate change.
Understood.
> 
> Best regards,
> Krzysztof
> 
Regards,
Subhajit Ghosh
Re: [PATCH v5 2/3] dt-bindings: iio: light: Avago APDS9306
Posted by Krzysztof Kozlowski 1 year, 11 months ago 
On 22/01/2024 11:07, Subhajit Ghosh wrote:
> On 22/1/24 20:21, Krzysztof Kozlowski wrote:
>> On 21/01/2024 06:17, Subhajit Ghosh wrote:
>>> Adding device tree support for APDS9306 Ambient Light Sensor.
>>> Updating datasheet hyperlinks.
>>> Adding interrupt definition macro and header file.
>>> Adding vdd-supply property.
>>
>> Why? Do other devices have it?
> Are you referring to vdd-supply? No, I guess, haven't checked actually.
> If other devices don't have, can you suggest the best way to handle that?

Commit msg should explain that. I assume all devices need it, so just
like Jonathan suggested: this might be separate patch.

Best regards,
Krzysztof
Re: [PATCH v5 2/3] dt-bindings: iio: light: Avago APDS9306
Posted by Jonathan Cameron 1 year, 11 months ago 
On Sun, 21 Jan 2024 15:47:33 +1030
Subhajit Ghosh <subhajit.ghosh@tweaklogic.com> wrote:

> Adding device tree support for APDS9306 Ambient Light Sensor.
> Updating datasheet hyperlinks.
> Adding interrupt definition macro and header file.

This is an unrelated change, so should probably be in a separate patch.

> Adding vdd-supply property.

This one is reasonable to have in same patch as the new device addition
as, whilst I assume it's valid for the existing devices, you are adding it
to incorporate something that device also has.
Could also be a separate precursor patch.

> 
> Signed-off-by: Subhajit Ghosh <subhajit.ghosh@tweaklogic.com>
> ---
> v2 -> v5:
>  - Implemented changes as per previous reviews:
>    Link: https://lore.kernel.org/lkml/20231028142944.7e210eb6@jic23-huawei/
>    Link: https://lore.kernel.org/lkml/22e9e5e9-d26a-46e9-8986-5062bbfd72ec@linaro.org/
> ---
>  .../bindings/iio/light/avago,apds9300.yaml        | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/light/avago,apds9300.yaml b/Documentation/devicetree/bindings/iio/light/avago,apds9300.yaml
> index c610780346e8..bee73a590424 100644
> --- a/Documentation/devicetree/bindings/iio/light/avago,apds9300.yaml
> +++ b/Documentation/devicetree/bindings/iio/light/avago,apds9300.yaml
> @@ -4,19 +4,21 @@
>  $id: http://devicetree.org/schemas/iio/light/avago,apds9300.yaml#
>  $schema: http://devicetree.org/meta-schemas/core.yaml#
>  
> -title: Avago Gesture/RGB/ALS/Proximity sensors
> +title: Avago (Broadcom) Gesture/RGB/ALS/Proximity sensors
>  
>  maintainers:
>    - Subhajit Ghosh <subhajit.ghosh@tweaklogic.com>
>  
>  description: |
> -  Datasheet: https://www.avagotech.com/docs/AV02-1077EN
> -  Datasheet: https://www.avagotech.com/docs/AV02-4191EN
> +  Datasheet: https://docs.broadcom.com/doc/AV02-1077EN
> +  Datasheet: https://docs.broadcom.com/doc/AV02-4191EN
> +  Datasheet: https://docs.broadcom.com/doc/AV02-4755EN

Old links seem to still work, so why the change?

>  
>  properties:
>    compatible:
>      enum:
>        - avago,apds9300
> +      - avago,apds9306
>        - avago,apds9960
>  
>    reg:
> @@ -25,6 +27,8 @@ properties:
>    interrupts:
>      maxItems: 1
>  
> +  vdd-supply: true
> +
>  additionalProperties: false
>  
>  required:
> @@ -33,6 +37,8 @@ required:
>  
>  examples:
>    - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
>      i2c {
>          #address-cells = <1>;
>          #size-cells = <0>;
> @@ -41,7 +47,8 @@ examples:
>              compatible = "avago,apds9300";
>              reg = <0x39>;
>              interrupt-parent = <&gpio2>;
> -            interrupts = <29 8>;
> +            interrupts = <29 IRQ_TYPE_LEVEL_LOW>;
> +            vdd-supply = <&regulator_3v3>;
>          };
>      };
>  ...
Re: [PATCH v5 2/3] dt-bindings: iio: light: Avago APDS9306
Posted by Subhajit Ghosh 1 year, 11 months ago 
On 22/1/24 02:06, Jonathan Cameron wrote:
> On Sun, 21 Jan 2024 15:47:33 +1030
> Subhajit Ghosh <subhajit.ghosh@tweaklogic.com> wrote:
> 
>> Adding device tree support for APDS9306 Ambient Light Sensor.
>> Updating datasheet hyperlinks.
>> Adding interrupt definition macro and header file.
> 
> This is an unrelated change, so should probably be in a separate patch.
Understood.
> 
>> Adding vdd-supply property.
> 
> This one is reasonable to have in same patch as the new device addition
> as, whilst I assume it's valid for the existing devices, you are adding it
> to incorporate something that device also has.
> Could also be a separate precursor patch.
> 
>>

>>   
>>   description: |
>> -  Datasheet: https://www.avagotech.com/docs/AV02-1077EN
>> -  Datasheet: https://www.avagotech.com/docs/AV02-4191EN
>> +  Datasheet: https://docs.broadcom.com/doc/AV02-1077EN
>> +  Datasheet: https://docs.broadcom.com/doc/AV02-4191EN
>> +  Datasheet: https://docs.broadcom.com/doc/AV02-4755EN
> 
> Old links seem to still work, so why the change?
My bad. I will revert to keep things simple. No reason, just thought of updating.

Thanks for the review.

Regards,
Subhajit Ghosh

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.