Re: [PATCH] Documentation: dt: i2c: Add LPS001WP to the Trivial Devices list

Re: [PATCH] Documentation: dt: i2c: Add LPS001WP to the Trivial Devices list

[Mark Rutland]

Hi Lee,

Cheers for producing a binding.

On Wed, Sep 04, 2013 at 02:50:55PM +0100, Lee Jones wrote:
> LPS001WP is a Pressure and Temperature sensor.
> 
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  Documentation/devicetree/bindings/i2c/trivial-devices.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> index ad6a738..6038807e6 100644
> --- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> +++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> @@ -57,6 +57,7 @@ st-micro,24c256		i2c serial eeprom  (24cxx)
>  stm,m41t00		Serial Access TIMEKEEPER
>  stm,m41t62		Serial real-time clock (RTC) with alarm
>  stm,m41t80		M41T80 - SERIAL ACCESS RTC WITH ALARMS
> +stm,lps001wp		Pressure and Temperature sensor

As I hinted at in the other thread [1], I don't think this can be
documented as a trivial binding -- the driver expects a couple of
regulators which should be described in the binding (or the driver
shouldn't be using them...).

Also, in the other thread the "st," prefix was used [2], while "stm," is
listed at the prefix in the binding. As far as I can see, "st," should
be used, as it's in vendor-prefixes and "stm," is not.

I'm happy to cook up a patch adding deprecation notes and corrected
strings for the existing incorrect "stm," bindings.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2013-September/196561.html
[2] http://lists.infradead.org/pipermail/linux-arm-kernel/2013-September/196558.html

Thanks,
Mark.

>  taos,tsl2550		Ambient Light Sensor with SMBUS/Two Wire Serial Interface
>  ti,tsc2003		I2C Touch-Screen Controller
>  ti,tmp102		Low Power Digital Temperature Sensor with SMBUS/Two Wire Serial Interface
> -- 
> 1.8.1.2
> 
> 

Re: [PATCH] Documentation: dt: i2c: Add LPS001WP to the Trivial Devices list

[Mark Rutland]

On Wed, Sep 04, 2013 at 04:11:59PM +0100, Lee Jones wrote:
> > Cheers for producing a binding.
> > 
> > On Wed, Sep 04, 2013 at 02:50:55PM +0100, Lee Jones wrote:
> > > LPS001WP is a Pressure and Temperature sensor.
> > > 
> > > Signed-off-by: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > > ---
> > >  Documentation/devicetree/bindings/i2c/trivial-devices.txt | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/i2c/trivial-devices.txt b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> > > index ad6a738..6038807e6 100644
> > > --- a/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> > > +++ b/Documentation/devicetree/bindings/i2c/trivial-devices.txt
> > > @@ -57,6 +57,7 @@ st-micro,24c256		i2c serial eeprom  (24cxx)
> > >  stm,m41t00		Serial Access TIMEKEEPER
> > >  stm,m41t62		Serial real-time clock (RTC) with alarm
> > >  stm,m41t80		M41T80 - SERIAL ACCESS RTC WITH ALARMS
> > > +stm,lps001wp		Pressure and Temperature sensor
> > 
> > As I hinted at in the other thread [1], I don't think this can be
> > documented as a trivial binding -- the driver expects a couple of
> > regulators which should be described in the binding (or the driver
> > shouldn't be using them...).
> 
> Surely all devices need to be powered and would subsequently require a
> regulator of one description or other? We don't really *need* to
> manually enable it on my platform but that's because it's 'always
> on', but we do anyway for completeness.

They may or may not, that's a grey area. Regardless of whether you
actually need it, the driver is currently looking for two undocumented
entries in the DT. If you want to be able to use them, they must be
documented so that people know they exist, what format they are expected
to be, and what they actually represent.

We should not allow for undocumented ABIs.

> 
> Would a (regulator: vdd) note in this patch be suitable? It seems a
> shame to have to produce a boilerplate binding entry for the sake of a
> regulator.

I thought this could have two regulators (vdd and vms)? 

I also note that the device can also be attached to SPI. Do we have any
other devices which may be attached to either? Do we handle that, and if
so, how (do we have the same compatible string for both interfaces?)?

Thanks,
Mark.

Re: [PATCH] Documentation: dt: i2c: Add LPS001WP to the Trivial Devices list

[Pawel Moll]

On Thu, 2013-09-05 at 12:30 +0100, Mark Rutland wrote:
> I also note that the device can also be attached to SPI. Do we have any
> other devices which may be attached to either? Do we handle that, and if
> so, how (do we have the same compatible string for both interfaces?)?

Theoretically you don't need anything to make it work - nodes being a
children of a I2C master will instantiate i2c_clients, offspring of a
SPI master will become spi_devices. Then you register two drivers - I2C
and SPI ones, probably sharing most of the code - both matching the same
compatible value. The I2C one will be bound only with the I2C nodes, the
same applies to SPI.

I hope :-)

Paweł