[1/2] dt-bindings: iio: light: Avago APDS9306

  Add devicetree bindings for Avago APDS9306 Ambient Light Sensor.

Signed-off-by: Subhajit Ghosh <subhajit.ghosh@tweaklogic.com>
---
 .../bindings/iio/light/avago,apds9306.yaml    | 49 +++++++++++++++++++
 1 file changed, 49 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/light/avago,apds9306.yaml
  

On 08/10/2023 17:48, Subhajit Ghosh wrote:
> Add devicetree bindings for Avago APDS9306 Ambient Light Sensor.
> 
> Signed-off-by: Subhajit Ghosh <subhajit.ghosh@tweaklogic.com>
> ---
>  .../bindings/iio/light/avago,apds9306.yaml    | 49 +++++++++++++++++++
>  1 file changed, 49 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/light/avago,apds9306.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/light/avago,apds9306.yaml b/Documentation/devicetree/bindings/iio/light/avago,apds9306.yaml
> new file mode 100644
> index 000000000000..e8bb897782fc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/light/avago,apds9306.yaml
> @@ -0,0 +1,49 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/light/avago,apds9306.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Avago APDS9306 Ambient Light Sensor
> +
> +maintainers:
> +  - Subhajit Ghosh <subhajit.ghosh@tweaklogic.com>
> +
> +description:
> +  Datasheet at https://docs.broadcom.com/doc/AV02-4755EN

This is exactly the same as two other Avago devices. It should be
squashed - first two device schemas squashed, then add new device support.

Also, the supply is not vin, but vdd.

Best regards,
Krzysztof
  

>> +description:
>> +  Datasheet at https://docs.broadcom.com/doc/AV02-4755EN
> 
> This is exactly the same as two other Avago devices. It should be
> squashed - first two device schemas squashed, then add new device support.
> 
> Also, the supply is not vin, but vdd.
>

Yes, they look similar. I will combine them all in a single yaml file in
the next revision. Thank you Krzysztof.

Regards,
Subhajit Ghosh
  

On 10/8/23 18:48, Subhajit Ghosh wrote:
> Add devicetree bindings for Avago APDS9306 Ambient Light Sensor.
> 
> Signed-off-by: Subhajit Ghosh <subhajit.ghosh@tweaklogic.com>
> ---
>   .../bindings/iio/light/avago,apds9306.yaml    | 49 +++++++++++++++++++
>   1 file changed, 49 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/iio/light/avago,apds9306.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/light/avago,apds9306.yaml b/Documentation/devicetree/bindings/iio/light/avago,apds9306.yaml
> new file mode 100644
> index 000000000000..e8bb897782fc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/light/avago,apds9306.yaml
> @@ -0,0 +1,49 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/light/avago,apds9306.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Avago APDS9306 Ambient Light Sensor
> +
> +maintainers:
> +  - Subhajit Ghosh <subhajit.ghosh@tweaklogic.com>
> +
> +description:
> +  Datasheet at https://docs.broadcom.com/doc/AV02-4755EN
> +
> +properties:
> +  compatible:
> +    const: avago,apds9306

I see the driver supports two different variants of this IC, 
differentiated by the part-ID register. Variants are named as apds9306 
and apds9306-065. I wonder if we could/should have different compatibles 
for them?

Yours,
	-- Matti
  

On 10/10/23 19:22, Matti Vaittinen wrote:

>> +properties:
>> +  compatible:
>> +    const: avago,apds9306
> 
> I see the driver supports two different variants of this IC, differentiated by the part-ID register. Variants are named as apds9306 and apds9306-065. I wonder if we could/should have different compatibles for them?
> 

Yes, we can. It makes sense. I'll implement that.

Regards,
Subhajit Ghosh
  

On Mon,  9 Oct 2023 02:18:56 +1030
Subhajit Ghosh <subhajit.ghosh@tweaklogic.com> wrote:

> Add devicetree bindings for Avago APDS9306 Ambient Light Sensor.
> 
> Signed-off-by: Subhajit Ghosh <subhajit.ghosh@tweaklogic.com>
> ---
>  .../bindings/iio/light/avago,apds9306.yaml    | 49 +++++++++++++++++++
>  1 file changed, 49 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/light/avago,apds9306.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/light/avago,apds9306.yaml b/Documentation/devicetree/bindings/iio/light/avago,apds9306.yaml
> new file mode 100644
> index 000000000000..e8bb897782fc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/light/avago,apds9306.yaml
> @@ -0,0 +1,49 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/light/avago,apds9306.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Avago APDS9306 Ambient Light Sensor
> +
> +maintainers:
> +  - Subhajit Ghosh <subhajit.ghosh@tweaklogic.com>
> +
> +description:
> +  Datasheet at https://docs.broadcom.com/doc/AV02-4755EN
> +
> +properties:
> +  compatible:
> +    const: avago,apds9306
> +
> +  reg:
> +    maxItems: 1
> +
> +  vin-supply:
> +    description: Regulator supply to the sensor

Why vin?  It seems to be vdd on the datasheet.
We tend to match the datasheet naming for power supplies as that is normally
what is seen on circuit board schematics.


> +
> +  interrupts:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        light-sensor@52 {
> +            compatible = "avago,apds9306";
> +            reg = <0x52>;
> +            interrupt-parent = <&gpiof>;
> +            interrupts = <6 IRQ_TYPE_LEVEL_LOW>;
> +        };
> +    };
> +...
  

On Tue, 10 Oct 2023 22:48:43 +1030
Subhajit Ghosh <subhajit.ghosh@tweaklogic.com> wrote:

> On 10/10/23 19:22, Matti Vaittinen wrote:
> 
> >> +properties:
> >> +  compatible:
> >> +    const: avago,apds9306  
> > 
> > I see the driver supports two different variants of this IC, differentiated by the part-ID register. Variants are named as apds9306 and apds9306-065. I wonder if we could/should have different compatibles for them?
> >   
> 
> Yes, we can. It makes sense. I'll implement that.
We could.  The reason to do so is that we might in future want to use
fallback compatibles.  So we want to allow a new DT to work with older
kernel by saying - I have a new device, but it is fully compatible with
this earlier one.  In those cases we check the ID as your driver current
does, but just print a warning that we aren't sure what the device is so
are going with what the DT told us to fall back to.

Jonathan

> 
> Regards,
> Subhajit Ghosh
>
  

On Tue, Oct 10, 2023 at 11:52:28AM +0300, Matti Vaittinen wrote:
> On 10/8/23 18:48, Subhajit Ghosh wrote:
> > Add devicetree bindings for Avago APDS9306 Ambient Light Sensor.
> > 
> > Signed-off-by: Subhajit Ghosh <subhajit.ghosh@tweaklogic.com>
> > ---
> >   .../bindings/iio/light/avago,apds9306.yaml    | 49 +++++++++++++++++++
> >   1 file changed, 49 insertions(+)
> >   create mode 100644 Documentation/devicetree/bindings/iio/light/avago,apds9306.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/light/avago,apds9306.yaml b/Documentation/devicetree/bindings/iio/light/avago,apds9306.yaml
> > new file mode 100644
> > index 000000000000..e8bb897782fc
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/light/avago,apds9306.yaml
> > @@ -0,0 +1,49 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/light/avago,apds9306.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Avago APDS9306 Ambient Light Sensor
> > +
> > +maintainers:
> > +  - Subhajit Ghosh <subhajit.ghosh@tweaklogic.com>
> > +
> > +description:
> > +  Datasheet at https://docs.broadcom.com/doc/AV02-4755EN
> > +
> > +properties:
> > +  compatible:
> > +    const: avago,apds9306
> 
> I see the driver supports two different variants of this IC, differentiated
> by the part-ID register. Variants are named as apds9306 and apds9306-065. I
> wonder if we could/should have different compatibles for them?

If 1 compatible is sufficient to know how to power on both devices and 
read the part-ID register, then no need for different compatibles.

Rob
  

On 11/10/23 02:49, Rob Herring wrote:

>>> +
>>> +properties:
>>> +  compatible:
>>> +    const: avago,apds9306
>>
>> I see the driver supports two different variants of this IC, differentiated
>> by the part-ID register. Variants are named as apds9306 and apds9306-065. I
>> wonder if we could/should have different compatibles for them?
> 
> If 1 compatible is sufficient to know how to power on both devices and
> read the part-ID register, then no need for different compatibles.
> 
> Rob
Understood. Thanks Rob.

Regards,
Subhajit Ghosh
  

On 11/10/23 00:21, Jonathan Cameron wrote:

>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  vin-supply:
>> +    description: Regulator supply to the sensor
> 
> Why vin?  It seems to be vdd on the datasheet.
> We tend to match the datasheet naming for power supplies as that is normally
> what is seen on circuit board schematics.
Got it, I'll fix it. Thanks for looking into it.

Regards,
Subhajit Ghosh