| Message ID | 20231008154857.24162-2-subhajit.ghosh@tweaklogic.com |
|---|---|
| State | New |
| Headers | show |
| Series | Support for Avago APDS9306 Ambient Light Sensor | expand |
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
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 + + 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>; + }; + }; +...
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