| Message ID | 20200731104555.v3.1.I0925046377211b8b6f06764857f03b4ab592bddb@changeid (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | sx9310 iio driver updates | expand |
On Fri, 31 Jul 2020 10:48:38 -0600, Daniel Campello wrote: > Adds device tree bandings for sx9310 sensor. > > Signed-off-by: Daniel Campello <campello@chromium.org> > Cc: Hartmut Knaack <knaack.h@gmx.de> > Cc: Lars-Peter Clausen <lars@metafoo.de> > Cc: Peter Meerwald-Stadler <pmeerw@pmeerw.net> > Cc: Rob Herring <robh+dt@kernel.org> > Reviewed-by: Douglas Anderson <dianders@chromium.org> > [swboyd@chromium.org: Add both regulators and make them optional] > Signed-off-by: Stephen Boyd <swboyd@chromium.org> > --- > > Changes in v3: None > Changes in v2: > - Added #io-channel-cells as a required property > > .../iio/proximity/semtech,sx9310.yaml | 65 +++++++++++++++++++ > 1 file changed, 65 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/proximity/semtech,sx9310.yaml > Reviewed-by: Rob Herring <robh@kernel.org>
On Fri, 31 Jul 2020 10:48:38 -0600 Daniel Campello <campello@chromium.org> wrote: > Adds device tree bandings for sx9310 sensor. > > Signed-off-by: Daniel Campello <campello@chromium.org> > Cc: Hartmut Knaack <knaack.h@gmx.de> > Cc: Lars-Peter Clausen <lars@metafoo.de> > Cc: Peter Meerwald-Stadler <pmeerw@pmeerw.net> > Cc: Rob Herring <robh+dt@kernel.org> > Reviewed-by: Douglas Anderson <dianders@chromium.org> > [swboyd@chromium.org: Add both regulators and make them optional] > Signed-off-by: Stephen Boyd <swboyd@chromium.org> > --- > > Changes in v3: None > Changes in v2: > - Added #io-channel-cells as a required property > > .../iio/proximity/semtech,sx9310.yaml | 65 +++++++++++++++++++ > 1 file changed, 65 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/proximity/semtech,sx9310.yaml > > diff --git a/Documentation/devicetree/bindings/iio/proximity/semtech,sx9310.yaml b/Documentation/devicetree/bindings/iio/proximity/semtech,sx9310.yaml > new file mode 100644 > index 00000000000000..5739074d3592fe > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/proximity/semtech,sx9310.yaml > @@ -0,0 +1,65 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/iio/proximity/semtech,sx9310.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Semtech's SX9310 capacitive proximity sensor > + > +maintainers: > + - Daniel Campello <campello@chromium.org> > + > +description: | > + Semtech's SX9310/SX9311 capacitive proximity/button solution. > + > + Specifications about the devices can be found at: > + https://www.semtech.com/products/smart-sensing/sar-sensors/sx9310 > + > +properties: > + compatible: > + enum: > + - semtech,sx9310 > + - semtech,sx9311 > + > + reg: > + maxItems: 1 > + > + interrupts: > + description: > + The sole interrupt generated by the device used to announce the > + preceding reading request has finished and that data is > + available or that a close/far proximity event has happened. > + maxItems: 1 > + > + vdd-supply: > + description: Main power supply > + > + svdd-supply: > + description: Host interface power supply > + > + "#io-channel-cells": > + const: 1 > + > +required: > + - compatible > + - reg > + - "#io-channel-cells" Missed this in earlier review (only noticed when I saw whilst santity checking earlier versions. Fairly sure we should only need #io-channel-cells if we have a consumer of a channel somewhere else in DT. So it's not required as far as I can see. > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/interrupt-controller/irq.h> > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + proximity@28 { > + compatible = "semtech,sx9310"; > + reg = <0x28>; > + interrupt-parent = <&pio>; > + interrupts = <5 IRQ_TYPE_LEVEL_LOW 5>; > + vdd-supply = <&pp3300_a>; > + svdd-supply = <&pp1800_prox>; > + #io-channel-cells = <1>; > + }; > + };
Quoting Jonathan Cameron (2020-08-01 08:06:39) > On Fri, 31 Jul 2020 10:48:38 -0600 > Daniel Campello <campello@chromium.org> wrote: > > diff --git a/Documentation/devicetree/bindings/iio/proximity/semtech,sx9310.yaml b/Documentation/devicetree/bindings/iio/proximity/semtech,sx9310.yaml > > new file mode 100644 > > index 00000000000000..5739074d3592fe > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/iio/proximity/semtech,sx9310.yaml > > @@ -0,0 +1,65 @@ [...] > > + > > + "#io-channel-cells": > > + const: 1 > > + > > +required: > > + - compatible > > + - reg > > + - "#io-channel-cells" > > Missed this in earlier review (only noticed when I saw whilst santity > checking earlier versions. > > Fairly sure we should only need #io-channel-cells if we have > a consumer of a channel somewhere else in DT. So it's not > required as far as I can see. > This is mostly a decision for Rob to make, but I would make it required because the device is always an io channel provider. It may be that it isn't providing anything in the DT to something else in the DT but it is providing this information somewhere so always having to spell that out is simple and doesn't hurt.
On Mon, Aug 3, 2020 at 1:00 PM Stephen Boyd <swboyd@chromium.org> wrote: > > Quoting Jonathan Cameron (2020-08-01 08:06:39) > > On Fri, 31 Jul 2020 10:48:38 -0600 > > Daniel Campello <campello@chromium.org> wrote: > > > diff --git a/Documentation/devicetree/bindings/iio/proximity/semtech,sx9310.yaml b/Documentation/devicetree/bindings/iio/proximity/semtech,sx9310.yaml > > > new file mode 100644 > > > index 00000000000000..5739074d3592fe > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/iio/proximity/semtech,sx9310.yaml > > > @@ -0,0 +1,65 @@ > [...] > > > + > > > + "#io-channel-cells": > > > + const: 1 > > > + > > > +required: > > > + - compatible > > > + - reg > > > + - "#io-channel-cells" > > > > Missed this in earlier review (only noticed when I saw whilst santity > > checking earlier versions. > > > > Fairly sure we should only need #io-channel-cells if we have > > a consumer of a channel somewhere else in DT. So it's not > > required as far as I can see. > > > > This is mostly a decision for Rob to make, but I would make it required > because the device is always an io channel provider. It may be that it > isn't providing anything in the DT to something else in the DT but it is > providing this information somewhere so always having to spell that out > is simple and doesn't hurt. I agree. If the user is split in a board file or overlay, we don't want to have to be adding it to the provider at that time. Rob
On Mon, 3 Aug 2020 20:01:06 -0600 Rob Herring <robh+dt@kernel.org> wrote: > On Mon, Aug 3, 2020 at 1:00 PM Stephen Boyd <swboyd@chromium.org> wrote: > > > > Quoting Jonathan Cameron (2020-08-01 08:06:39) > > > On Fri, 31 Jul 2020 10:48:38 -0600 > > > Daniel Campello <campello@chromium.org> wrote: > > > > diff --git a/Documentation/devicetree/bindings/iio/proximity/semtech,sx9310.yaml b/Documentation/devicetree/bindings/iio/proximity/semtech,sx9310.yaml > > > > new file mode 100644 > > > > index 00000000000000..5739074d3592fe > > > > --- /dev/null > > > > +++ b/Documentation/devicetree/bindings/iio/proximity/semtech,sx9310.yaml > > > > @@ -0,0 +1,65 @@ > > [...] > > > > + > > > > + "#io-channel-cells": > > > > + const: 1 > > > > + > > > > +required: > > > > + - compatible > > > > + - reg > > > > + - "#io-channel-cells" > > > > > > Missed this in earlier review (only noticed when I saw whilst santity > > > checking earlier versions. > > > > > > Fairly sure we should only need #io-channel-cells if we have > > > a consumer of a channel somewhere else in DT. So it's not > > > required as far as I can see. > > > > > > > This is mostly a decision for Rob to make, but I would make it required > > because the device is always an io channel provider. It may be that it > > isn't providing anything in the DT to something else in the DT but it is > > providing this information somewhere so always having to spell that out > > is simple and doesn't hurt. > > I agree. If the user is split in a board file or overlay, we don't > want to have to be adding it to the provider at that time. That is perhaps a reasonable view point for devices with channels that are likely to be used by consumer drivers, but in this particular case we are talking about a proximity sensor. So far I don't think we have any consumer drivers for this type of sensor (I might have forgotten one of course!) I'm not that fussed though, so will leave it in. The argument is a lot stronger for ADCs and such like, so we can start encouraging it for those. Jonathan > > Rob
On Thu, Aug 6, 2020 at 12:14 PM Jonathan Cameron <jic23@kernel.org> wrote: > > On Mon, 3 Aug 2020 20:01:06 -0600 > Rob Herring <robh+dt@kernel.org> wrote: > > > On Mon, Aug 3, 2020 at 1:00 PM Stephen Boyd <swboyd@chromium.org> wrote: > > > > > > Quoting Jonathan Cameron (2020-08-01 08:06:39) > > > > On Fri, 31 Jul 2020 10:48:38 -0600 > > > > Daniel Campello <campello@chromium.org> wrote: > > > > > diff --git a/Documentation/devicetree/bindings/iio/proximity/semtech,sx9310.yaml b/Documentation/devicetree/bindings/iio/proximity/semtech,sx9310.yaml > > > > > new file mode 100644 > > > > > index 00000000000000..5739074d3592fe > > > > > --- /dev/null > > > > > +++ b/Documentation/devicetree/bindings/iio/proximity/semtech,sx9310.yaml > > > > > @@ -0,0 +1,65 @@ > > > [...] > > > > > + > > > > > + "#io-channel-cells": > > > > > + const: 1 > > > > > + > > > > > +required: > > > > > + - compatible > > > > > + - reg > > > > > + - "#io-channel-cells" > > > > > > > > Missed this in earlier review (only noticed when I saw whilst santity > > > > checking earlier versions. > > > > > > > > Fairly sure we should only need #io-channel-cells if we have > > > > a consumer of a channel somewhere else in DT. So it's not > > > > required as far as I can see. > > > > > > > > > > This is mostly a decision for Rob to make, but I would make it required > > > because the device is always an io channel provider. It may be that it > > > isn't providing anything in the DT to something else in the DT but it is > > > providing this information somewhere so always having to spell that out > > > is simple and doesn't hurt. > > > > I agree. If the user is split in a board file or overlay, we don't > > want to have to be adding it to the provider at that time. > > That is perhaps a reasonable view point for devices with channels that > are likely to be used by consumer drivers, but in this particular case we > are talking about a proximity sensor. So far I don't think we > have any consumer drivers for this type of sensor (I might have forgotten > one of course!) Indeed, I didn't consider whether it made sense in the first place. So should it just not be specified at all in this case? I can't really picture what the usecase for a consumer node would be. Rob
Quoting Rob Herring (2020-08-06 15:12:34) > On Thu, Aug 6, 2020 at 12:14 PM Jonathan Cameron <jic23@kernel.org> wrote: > > > > On Mon, 3 Aug 2020 20:01:06 -0600 > > Rob Herring <robh+dt@kernel.org> wrote: > > > > > On Mon, Aug 3, 2020 at 1:00 PM Stephen Boyd <swboyd@chromium.org> wrote: > > > > > > > > This is mostly a decision for Rob to make, but I would make it required > > > > because the device is always an io channel provider. It may be that it > > > > isn't providing anything in the DT to something else in the DT but it is > > > > providing this information somewhere so always having to spell that out > > > > is simple and doesn't hurt. > > > > > > I agree. If the user is split in a board file or overlay, we don't > > > want to have to be adding it to the provider at that time. > > > > That is perhaps a reasonable view point for devices with channels that > > are likely to be used by consumer drivers, but in this particular case we > > are talking about a proximity sensor. So far I don't think we > > have any consumer drivers for this type of sensor (I might have forgotten > > one of course!) > > Indeed, I didn't consider whether it made sense in the first place. So > should it just not be specified at all in this case? I can't really > picture what the usecase for a consumer node would be. > I was thinking that a WiFi DT node may directly grab the channel from this device and use this for SAR power changes. That would avoid going all the way to userspace to figure out that something is close proximity and then tell WiFi to reduce power.
diff --git a/Documentation/devicetree/bindings/iio/proximity/semtech,sx9310.yaml b/Documentation/devicetree/bindings/iio/proximity/semtech,sx9310.yaml new file mode 100644 index 00000000000000..5739074d3592fe --- /dev/null +++ b/Documentation/devicetree/bindings/iio/proximity/semtech,sx9310.yaml @@ -0,0 +1,65 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iio/proximity/semtech,sx9310.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Semtech's SX9310 capacitive proximity sensor + +maintainers: + - Daniel Campello <campello@chromium.org> + +description: | + Semtech's SX9310/SX9311 capacitive proximity/button solution. + + Specifications about the devices can be found at: + https://www.semtech.com/products/smart-sensing/sar-sensors/sx9310 + +properties: + compatible: + enum: + - semtech,sx9310 + - semtech,sx9311 + + reg: + maxItems: 1 + + interrupts: + description: + The sole interrupt generated by the device used to announce the + preceding reading request has finished and that data is + available or that a close/far proximity event has happened. + maxItems: 1 + + vdd-supply: + description: Main power supply + + svdd-supply: + description: Host interface power supply + + "#io-channel-cells": + const: 1 + +required: + - compatible + - reg + - "#io-channel-cells" + +additionalProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/irq.h> + i2c { + #address-cells = <1>; + #size-cells = <0>; + proximity@28 { + compatible = "semtech,sx9310"; + reg = <0x28>; + interrupt-parent = <&pio>; + interrupts = <5 IRQ_TYPE_LEVEL_LOW 5>; + vdd-supply = <&pp3300_a>; + svdd-supply = <&pp1800_prox>; + #io-channel-cells = <1>; + }; + };