diff mbox series

[v2,9/9] dt-bindings: iio: imu: st_lsm6dsx: add support to i2c pullup resistors

Message ID 97543627e6e0a26d7ea6b4daa160f10813c05335.1541945612.git.lorenzo.bianconi@redhat.com (mailing list archive)
State New, archived
Headers show
Series add i2c controller support to st_lsm6dsx driver | expand

Commit Message

Lorenzo Bianconi Nov. 11, 2018, 2:15 p.m. UTC
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
---
 Documentation/devicetree/bindings/iio/imu/st_lsm6dsx.txt | 1 +
 1 file changed, 1 insertion(+)

Comments

Jonathan Cameron Nov. 11, 2018, 4:45 p.m. UTC | #1
On Sun, 11 Nov 2018 15:15:36 +0100
Lorenzo Bianconi <lorenzo.bianconi@redhat.com> wrote:

> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
There seems to be some precedents in tree for this, so fair enough.
It's one of those things that almost seems worth standardising, but
it's not totally apparent where it would be done as it can apply
to any bus or gpio.

Anyhow, applied this as is (Rob / Mark input of course welcome) to
my togreg branch which will be pushed out in a few mins as testing
for the autobuilders to play with it.

Thanks,

Jonathan
> ---
>  Documentation/devicetree/bindings/iio/imu/st_lsm6dsx.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/imu/st_lsm6dsx.txt b/Documentation/devicetree/bindings/iio/imu/st_lsm6dsx.txt
> index 879322ad50fd..69d53d98d0f0 100644
> --- a/Documentation/devicetree/bindings/iio/imu/st_lsm6dsx.txt
> +++ b/Documentation/devicetree/bindings/iio/imu/st_lsm6dsx.txt
> @@ -13,6 +13,7 @@ Required properties:
>  Optional properties:
>  - st,drdy-int-pin: the pin on the package that will be used to signal
>    "data ready" (valid values: 1 or 2).
> +- st,pullups : enable/disable internal i2c controller pullup resistors.
>  - drive-open-drain: the interrupt/data ready line will be configured
>    as open drain, which is useful if several sensors share the same
>    interrupt line. This is a boolean property.
Rob Herring Nov. 17, 2018, 3:37 p.m. UTC | #2
On Sun, Nov 11, 2018 at 03:15:36PM +0100, Lorenzo Bianconi wrote:
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
> ---
>  Documentation/devicetree/bindings/iio/imu/st_lsm6dsx.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/imu/st_lsm6dsx.txt b/Documentation/devicetree/bindings/iio/imu/st_lsm6dsx.txt
> index 879322ad50fd..69d53d98d0f0 100644
> --- a/Documentation/devicetree/bindings/iio/imu/st_lsm6dsx.txt
> +++ b/Documentation/devicetree/bindings/iio/imu/st_lsm6dsx.txt
> @@ -13,6 +13,7 @@ Required properties:
>  Optional properties:
>  - st,drdy-int-pin: the pin on the package that will be used to signal
>    "data ready" (valid values: 1 or 2).
> +- st,pullups : enable/disable internal i2c controller pullup resistors.

bias-pull-up is the standard property for this.

>  - drive-open-drain: the interrupt/data ready line will be configured
>    as open drain, which is useful if several sensors share the same
>    interrupt line. This is a boolean property.
> -- 
> 2.19.1
>
Jonathan Cameron Nov. 17, 2018, 4:26 p.m. UTC | #3
On Sat, 17 Nov 2018 09:37:54 -0600
Rob Herring <robh@kernel.org> wrote:

> On Sun, Nov 11, 2018 at 03:15:36PM +0100, Lorenzo Bianconi wrote:
> > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
> > ---
> >  Documentation/devicetree/bindings/iio/imu/st_lsm6dsx.txt | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/imu/st_lsm6dsx.txt b/Documentation/devicetree/bindings/iio/imu/st_lsm6dsx.txt
> > index 879322ad50fd..69d53d98d0f0 100644
> > --- a/Documentation/devicetree/bindings/iio/imu/st_lsm6dsx.txt
> > +++ b/Documentation/devicetree/bindings/iio/imu/st_lsm6dsx.txt
> > @@ -13,6 +13,7 @@ Required properties:
> >  Optional properties:
> >  - st,drdy-int-pin: the pin on the package that will be used to signal
> >    "data ready" (valid values: 1 or 2).
> > +- st,pullups : enable/disable internal i2c controller pullup resistors.  
> 
> bias-pull-up is the standard property for this.

The problem here is 'of what?'.  Perhaps it wasn't clear in the current
binding either, but we are talking the pull ups on the auxilliary i2c bus
(which doesn't really have a direct representation in DT at all).
If we just go with bias-pull-up my instinct would be that it was applying
to the drdy pin for example.

Lorenzo, could you send a follow up patch fixing this once we
have it pinned down?

We have plenty of time to get it in before the end of the cycle, so I would
rather do that than revert this series over it.  Conversely I don't
want a binding we are still discussing getting into a release.  I should have
given Rob time to get to this.  Lots of precedence doesn't mean there isn't
a better way!  Sorry All.

Thanks,

Jonathan

> 
> >  - drive-open-drain: the interrupt/data ready line will be configured
> >    as open drain, which is useful if several sensors share the same
> >    interrupt line. This is a boolean property.
> > -- 
> > 2.19.1
> >
Lorenzo Bianconi Nov. 17, 2018, 5:24 p.m. UTC | #4
> On Sat, 17 Nov 2018 09:37:54 -0600
> Rob Herring <robh@kernel.org> wrote:
>

[...]

> > >  Optional properties:
> > >  - st,drdy-int-pin: the pin on the package that will be used to signal
> > >    "data ready" (valid values: 1 or 2).
> > > +- st,pullups : enable/disable internal i2c controller pullup resistors.
> >
> > bias-pull-up is the standard property for this.
>
> The problem here is 'of what?'.  Perhaps it wasn't clear in the current
> binding either, but we are talking the pull ups on the auxilliary i2c bus
> (which doesn't really have a direct representation in DT at all).
> If we just go with bias-pull-up my instinct would be that it was applying
> to the drdy pin for example.
>
> Lorenzo, could you send a follow up patch fixing this once we
> have it pinned down?
>

Sure, it sounds good. IIUC we should use 'bias-pull-up' here, without
vendor info, right?

Regards,
Lorenzo

> We have plenty of time to get it in before the end of the cycle, so I would
> rather do that than revert this series over it.  Conversely I don't
> want a binding we are still discussing getting into a release.  I should have
> given Rob time to get to this.  Lots of precedence doesn't mean there isn't
> a better way!  Sorry All.
>
> Thanks,
>
> Jonathan
>
> >
> > >  - drive-open-drain: the interrupt/data ready line will be configured
> > >    as open drain, which is useful if several sensors share the same
> > >    interrupt line. This is a boolean property.
> > > --
> > > 2.19.1
> > >
>
Jonathan Cameron Nov. 21, 2018, 6:57 p.m. UTC | #5
On Sat, 17 Nov 2018 18:24:03 +0100
Lorenzo Bianconi <lorenzo.bianconi@redhat.com> wrote:

> > On Sat, 17 Nov 2018 09:37:54 -0600
> > Rob Herring <robh@kernel.org> wrote:
> >  
> 
> [...]
> 
> > > >  Optional properties:
> > > >  - st,drdy-int-pin: the pin on the package that will be used to signal
> > > >    "data ready" (valid values: 1 or 2).
> > > > +- st,pullups : enable/disable internal i2c controller pullup resistors.  
> > >
> > > bias-pull-up is the standard property for this.  
> >
> > The problem here is 'of what?'.  Perhaps it wasn't clear in the current
> > binding either, but we are talking the pull ups on the auxilliary i2c bus
> > (which doesn't really have a direct representation in DT at all).
> > If we just go with bias-pull-up my instinct would be that it was applying
> > to the drdy pin for example.
> >
> > Lorenzo, could you send a follow up patch fixing this once we
> > have it pinned down?
> >  
> 
> Sure, it sounds good. IIUC we should use 'bias-pull-up' here, without
> vendor info, right?
Yes, though my concern about it being on it's own without any reference
points.  Makes it unobvious what we are applying the pull up on.

Jonathan

> 
> Regards,
> Lorenzo
> 
> > We have plenty of time to get it in before the end of the cycle, so I would
> > rather do that than revert this series over it.  Conversely I don't
> > want a binding we are still discussing getting into a release.  I should have
> > given Rob time to get to this.  Lots of precedence doesn't mean there isn't
> > a better way!  Sorry All.
> >
> > Thanks,
> >
> > Jonathan
> >  
> > >  
> > > >  - drive-open-drain: the interrupt/data ready line will be configured
> > > >    as open drain, which is useful if several sensors share the same
> > > >    interrupt line. This is a boolean property.
> > > > --
> > > > 2.19.1
> > > >  
> >
Lorenzo Bianconi Nov. 25, 2018, 9:13 a.m. UTC | #6
>
> On Sat, 17 Nov 2018 18:24:03 +0100
> Lorenzo Bianconi <lorenzo.bianconi@redhat.com> wrote:
>
> > > On Sat, 17 Nov 2018 09:37:54 -0600
> > > Rob Herring <robh@kernel.org> wrote:
> > >
> >
> > [...]
> >
> > > > >  Optional properties:
> > > > >  - st,drdy-int-pin: the pin on the package that will be used to signal
> > > > >    "data ready" (valid values: 1 or 2).
> > > > > +- st,pullups : enable/disable internal i2c controller pullup resistors.
> > > >
> > > > bias-pull-up is the standard property for this.
> > >
> > > The problem here is 'of what?'.  Perhaps it wasn't clear in the current
> > > binding either, but we are talking the pull ups on the auxilliary i2c bus
> > > (which doesn't really have a direct representation in DT at all).
> > > If we just go with bias-pull-up my instinct would be that it was applying
> > > to the drdy pin for example.
> > >
> > > Lorenzo, could you send a follow up patch fixing this once we
> > > have it pinned down?
> > >
> >
> > Sure, it sounds good. IIUC we should use 'bias-pull-up' here, without
> > vendor info, right?
> Yes, though my concern about it being on it's own without any reference
> points.  Makes it unobvious what we are applying the pull up on.
>
> Jonathan
>

@Rob: do you agree to use 'bias-pull-up' also in this particular case?
If so I will send a follow-up patch to fix it

Regards,
Lorenzo
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/iio/imu/st_lsm6dsx.txt b/Documentation/devicetree/bindings/iio/imu/st_lsm6dsx.txt
index 879322ad50fd..69d53d98d0f0 100644
--- a/Documentation/devicetree/bindings/iio/imu/st_lsm6dsx.txt
+++ b/Documentation/devicetree/bindings/iio/imu/st_lsm6dsx.txt
@@ -13,6 +13,7 @@  Required properties:
 Optional properties:
 - st,drdy-int-pin: the pin on the package that will be used to signal
   "data ready" (valid values: 1 or 2).
+- st,pullups : enable/disable internal i2c controller pullup resistors.
 - drive-open-drain: the interrupt/data ready line will be configured
   as open drain, which is useful if several sensors share the same
   interrupt line. This is a boolean property.