Re: [PATCH v2 2/2] iio: distance: srf08: add IIO driver for us ranger

[Andreas Klinger]

Hi Jonathan,

[...]

> >> For the range, it's an interesting one.  Again the term range could
> >> mean too many things within the wider ABI. We need to make it more
> >> specific.
> >>
> >> Actually reading the datasheet, I think this is fundamentally about the
> >> maximum sampling frequency rather than directly about the range.
> >> The only reason you'd reduce the range is to speed that up. It doesn't
> >> improve the resolution, the device simply answers quicker.
> >>
> >> So I'd support this as sampling_frequency.  You could then use
> >> the the iio_info_mask_*_available and relevant callback to provide
> >> info on what it then restricts the possible output values to
> >> (rather than controlling it directly).
> >>
> > 
> > By changing the range one cannot influence the sampling frequency directly. 
> > I
> > have seen on the oszilloscope that the telegrams arrive almost at the same 
> > time
> > with different settings of range and the same gain.
> > 
> > Only if the gain is also adjusted the sensor works faster and a higher 
> > frequency
> > can be used. But the gain is also used to adjust the sensitivity of the 
> > sensor. 
> That's rather weird and not what the datasheet suggests. Ah well.
> > 
> > What about calling it "sensor_domain" or "sensor_max_range"?
> hmm. Not sure - propose that with appropriate Docs and we can think more on 
> it.
> > 
> > 

I made a mistake in the earlier mentioned measurement which i have to correct:

The telegrams arrive faster with a smaller range and the same gain. 

But we cannot use the sensor at a higher frequency if we are not adjusting the
gain of the sensor as well. This is because of echos of the former cycle which
are still around. And this setting of the gain depends on the surrounding where
the sensor is used. So in the driver we cannot estimate it.

Therefore i'll send out the next version with the attribute name
"sensor_max_range" together with documentation as proposal.

Andreas

-- 

Re: [PATCH v2 2/2] iio: distance: srf08: add IIO driver for us ranger

[Andreas Klinger]

Hi Jonathan,

[...]

> >> For the range, it's an interesting one.  Again the term range could
> >> mean too many things within the wider ABI. We need to make it more
> >> specific.
> >>
> >> Actually reading the datasheet, I think this is fundamentally about the
> >> maximum sampling frequency rather than directly about the range.
> >> The only reason you'd reduce the range is to speed that up. It doesn't
> >> improve the resolution, the device simply answers quicker.
> >>
> >> So I'd support this as sampling_frequency.  You could then use
> >> the the iio_info_mask_*_available and relevant callback to provide
> >> info on what it then restricts the possible output values to
> >> (rather than controlling it directly).
> >>
> > 
> > By changing the range one cannot influence the sampling frequency directly. 
> > I
> > have seen on the oszilloscope that the telegrams arrive almost at the same 
> > time
> > with different settings of range and the same gain.
> > 
> > Only if the gain is also adjusted the sensor works faster and a higher 
> > frequency
> > can be used. But the gain is also used to adjust the sensitivity of the 
> > sensor. 
> That's rather weird and not what the datasheet suggests. Ah well.
> > 
> > What about calling it "sensor_domain" or "sensor_max_range"?
> hmm. Not sure - propose that with appropriate Docs and we can think more on 
> it.
> > 
> > 

I made a mistake in the earlier mentioned measurement which i have to correct:

The telegrams arrive faster with a smaller range and the same gain. 

But we cannot use the sensor at a higher frequency if we are not adjusting the
gain of the sensor as well. This is because of echos of the former cycle which
are still around. And this setting of the gain depends on the surrounding where
the sensor is used. So in the driver we cannot estimate it.

Therefore i'll send out the next version with the attribute name
"sensor_max_range" together with documentation as proposal.

Andreas

-- 

Re: [PATCH v2 2/2] iio: distance: srf08: add IIO driver for us ranger

[Jonathan Cameron]

On 14/01/17 17:48, Andreas Klinger wrote:
> Hi Jonathan,
> 
> see comments below.
> 
> Andreas
> 
> 
> Jonathan Cameron  schrieb am Sat, 14. Jan 12:17:
>> On 10/01/17 18:48, Andreas Klinger wrote:
>>> This is the IIO driver for devantech srf08 ultrasonic ranger which can be
>>> used to measure the distances to an object.
>>>
>>> The sensor supports I2C with some registers.
>>>
>>> Supported Features include:
>>> - read the distance in ranging mode in centimeters
>>> - output of the driver is directly the read value
>>> - together with the scale the driver delivers the distance in meters
>>> - only the first echo of the nearest object is delivered
>>> - set max gain register; userspace enters analogue value
>>> - set range registers; userspace enters range in millimeters in 43 mm steps
>>>
>>> Features not supported by this driver:
>>> - ranging mode in inches or in microseconds
>>> - ANN mode
>>> - change I2C address through this driver
>>> - light sensor
>>>
>>> The driver was added in the directory "proximity" of the iio subsystem
>>> in absence of another directory named "distance".
>>> There is also a new submenu "distance"
>> Hi Andreas,
>>
>> Sorry it took me a while to get to this!
>>
>> I'd not bother with the new submenu.  Perhaps we should rename the
>> proximity menu to proximity/distance.
>>
>> We already the lightening detector in there which is definitely not
>> measuring proximity in the convetional sense!
>>
>> Anyhow, the actual code is fine, but we need to think about how the
>> userspace ABI fits within the wider IIO ABI.  Naming and approaches
>> that make sense in a single class of drivers can end up meaining
>> very different things for other drivers.  Various suggestions inline.
>>
>> Jonathan
>>>
>>> Signed-off-by: Andreas Klinger 
>>> ---
>>>  drivers/iio/proximity/Kconfig  |  15 ++
>>>  drivers/iio/proximity/Makefile |   1 +
>>>  drivers/iio/proximity/srf08.c  | 362 
>>> +
>>>  3 files changed, 378 insertions(+)
>>>  create mode 100644 drivers/iio/proximity/srf08.c
>>>
>>> diff --git a/drivers/iio/proximity/Kconfig b/drivers/iio/proximity/Kconfig
>>> index ef4c73db5b53..7b10a137702b 100644
>>> --- a/drivers/iio/proximity/Kconfig
>>> +++ b/drivers/iio/proximity/Kconfig
>>> @@ -46,3 +46,18 @@ config SX9500
>>>   module will be called sx9500.
>>>  
>>>  endmenu
>>> +
>>> +menu "Distance sensors"
>>> +
>>> +config SRF08
>>> +   tristate "Devantech SRF08 ultrasonic ranger sensor"
>>> +   depends on I2C
>>> +   help
>>> + Say Y here to build a driver for Devantech SRF08 ultrasonic
>>> + ranger sensor. This driver can be used to measure the distance
>>> + of objects.
>>> +
>>> + To compile this driver as a module, choose M here: the
>>> + module will be called srf08.
>>> +
>>> +endmenu
>>> diff --git a/drivers/iio/proximity/Makefile b/drivers/iio/proximity/Makefile
>>> index 9aadd9a8ee99..e914c2a5dd49 100644
>>> --- a/drivers/iio/proximity/Makefile
>>> +++ b/drivers/iio/proximity/Makefile
>>> @@ -5,4 +5,5 @@
>>>  # When adding new entries keep the list in alphabetical order
>>>  obj-$(CONFIG_AS3935)   += as3935.o
>>>  obj-$(CONFIG_LIDAR_LITE_V2)+= pulsedlight-lidar-lite-v2.o
>>> +obj-$(CONFIG_SRF08)+= srf08.o
>>>  obj-$(CONFIG_SX9500)   += sx9500.o
>>> diff --git a/drivers/iio/proximity/srf08.c b/drivers/iio/proximity/srf08.c
>>> new file mode 100644
>>> index ..f38c74ed0933
>>> --- /dev/null
>>> +++ b/drivers/iio/proximity/srf08.c
>>> @@ -0,0 +1,362 @@
>>> +/*
>>> + * srf08.c - Support for Devantech SRF08 ultrasonic ranger
>>> + *
>>> + * Copyright (c) 2016 Andreas Klinger 
>>> + *
>>> + * This file is subject to the terms and conditions of version 2 of
>>> + * the GNU General Public License.  See the file COPYING in the main
>>> + * directory of this archive for more details.
>>> + *
>>> + * For details about the device see:
>>> + * http://www.robot-electronics.co.uk/htm/srf08tech.html
>>> + */
>>> +
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +
>>> +/* registers of SRF08 device */
>>> +#define SRF08_WRITE_COMMAND0x00/* Command Register */
>>> +#define SRF08_WRITE_MAX_GAIN   0x01/* Max Gain Register: 0 .. 31 */
>>> +#define SRF08_WRITE_RANGE  0x02/* Range Register: 0 .. 255 */
>>> +#define SRF08_READ_SW_REVISION 0x00/* Software Revision */
>>> +#define SRF08_READ_LIGHT   0x01/* Light Sensor during last echo */
>>> +#define SRF08_READ_ECHO_1_HIGH 0x02/* Range of first echo received 
>>> */
>>> +#define SRF08_READ_ECHO_1_LOW  0x03/* Range of first echo received 
>>> */
>>> +
>>> +#define SRF08_CMD_RANGING_CM   0x51/* Ranging Mode - Result in cm 
>>> */
>>> +
>>> +#define SRF08_DEFAULT_GAIN 1025/* max. analogue value of Gain */
>>> +#define SRF08_DEFAULT_RANGE11008   /* max. 

Re: [PATCH v2 2/2] iio: distance: srf08: add IIO driver for us ranger

[Jonathan Cameron]

On 14/01/17 17:48, Andreas Klinger wrote:
> Hi Jonathan,
> 
> see comments below.
> 
> Andreas
> 
> 
> Jonathan Cameron  schrieb am Sat, 14. Jan 12:17:
>> On 10/01/17 18:48, Andreas Klinger wrote:
>>> This is the IIO driver for devantech srf08 ultrasonic ranger which can be
>>> used to measure the distances to an object.
>>>
>>> The sensor supports I2C with some registers.
>>>
>>> Supported Features include:
>>> - read the distance in ranging mode in centimeters
>>> - output of the driver is directly the read value
>>> - together with the scale the driver delivers the distance in meters
>>> - only the first echo of the nearest object is delivered
>>> - set max gain register; userspace enters analogue value
>>> - set range registers; userspace enters range in millimeters in 43 mm steps
>>>
>>> Features not supported by this driver:
>>> - ranging mode in inches or in microseconds
>>> - ANN mode
>>> - change I2C address through this driver
>>> - light sensor
>>>
>>> The driver was added in the directory "proximity" of the iio subsystem
>>> in absence of another directory named "distance".
>>> There is also a new submenu "distance"
>> Hi Andreas,
>>
>> Sorry it took me a while to get to this!
>>
>> I'd not bother with the new submenu.  Perhaps we should rename the
>> proximity menu to proximity/distance.
>>
>> We already the lightening detector in there which is definitely not
>> measuring proximity in the convetional sense!
>>
>> Anyhow, the actual code is fine, but we need to think about how the
>> userspace ABI fits within the wider IIO ABI.  Naming and approaches
>> that make sense in a single class of drivers can end up meaining
>> very different things for other drivers.  Various suggestions inline.
>>
>> Jonathan
>>>
>>> Signed-off-by: Andreas Klinger 
>>> ---
>>>  drivers/iio/proximity/Kconfig  |  15 ++
>>>  drivers/iio/proximity/Makefile |   1 +
>>>  drivers/iio/proximity/srf08.c  | 362 
>>> +
>>>  3 files changed, 378 insertions(+)
>>>  create mode 100644 drivers/iio/proximity/srf08.c
>>>
>>> diff --git a/drivers/iio/proximity/Kconfig b/drivers/iio/proximity/Kconfig
>>> index ef4c73db5b53..7b10a137702b 100644
>>> --- a/drivers/iio/proximity/Kconfig
>>> +++ b/drivers/iio/proximity/Kconfig
>>> @@ -46,3 +46,18 @@ config SX9500
>>>   module will be called sx9500.
>>>  
>>>  endmenu
>>> +
>>> +menu "Distance sensors"
>>> +
>>> +config SRF08
>>> +   tristate "Devantech SRF08 ultrasonic ranger sensor"
>>> +   depends on I2C
>>> +   help
>>> + Say Y here to build a driver for Devantech SRF08 ultrasonic
>>> + ranger sensor. This driver can be used to measure the distance
>>> + of objects.
>>> +
>>> + To compile this driver as a module, choose M here: the
>>> + module will be called srf08.
>>> +
>>> +endmenu
>>> diff --git a/drivers/iio/proximity/Makefile b/drivers/iio/proximity/Makefile
>>> index 9aadd9a8ee99..e914c2a5dd49 100644
>>> --- a/drivers/iio/proximity/Makefile
>>> +++ b/drivers/iio/proximity/Makefile
>>> @@ -5,4 +5,5 @@
>>>  # When adding new entries keep the list in alphabetical order
>>>  obj-$(CONFIG_AS3935)   += as3935.o
>>>  obj-$(CONFIG_LIDAR_LITE_V2)+= pulsedlight-lidar-lite-v2.o
>>> +obj-$(CONFIG_SRF08)+= srf08.o
>>>  obj-$(CONFIG_SX9500)   += sx9500.o
>>> diff --git a/drivers/iio/proximity/srf08.c b/drivers/iio/proximity/srf08.c
>>> new file mode 100644
>>> index ..f38c74ed0933
>>> --- /dev/null
>>> +++ b/drivers/iio/proximity/srf08.c
>>> @@ -0,0 +1,362 @@
>>> +/*
>>> + * srf08.c - Support for Devantech SRF08 ultrasonic ranger
>>> + *
>>> + * Copyright (c) 2016 Andreas Klinger 
>>> + *
>>> + * This file is subject to the terms and conditions of version 2 of
>>> + * the GNU General Public License.  See the file COPYING in the main
>>> + * directory of this archive for more details.
>>> + *
>>> + * For details about the device see:
>>> + * http://www.robot-electronics.co.uk/htm/srf08tech.html
>>> + */
>>> +
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +
>>> +/* registers of SRF08 device */
>>> +#define SRF08_WRITE_COMMAND0x00/* Command Register */
>>> +#define SRF08_WRITE_MAX_GAIN   0x01/* Max Gain Register: 0 .. 31 */
>>> +#define SRF08_WRITE_RANGE  0x02/* Range Register: 0 .. 255 */
>>> +#define SRF08_READ_SW_REVISION 0x00/* Software Revision */
>>> +#define SRF08_READ_LIGHT   0x01/* Light Sensor during last echo */
>>> +#define SRF08_READ_ECHO_1_HIGH 0x02/* Range of first echo received 
>>> */
>>> +#define SRF08_READ_ECHO_1_LOW  0x03/* Range of first echo received 
>>> */
>>> +
>>> +#define SRF08_CMD_RANGING_CM   0x51/* Ranging Mode - Result in cm 
>>> */
>>> +
>>> +#define SRF08_DEFAULT_GAIN 1025/* max. analogue value of Gain */
>>> +#define SRF08_DEFAULT_RANGE11008   /* max. value of Range in mm */
>>> +
>>> +struct srf08_data {

Re: [PATCH v2 2/2] iio: distance: srf08: add IIO driver for us ranger

[Lars-Peter Clausen]

On 01/10/2017 07:48 PM, Andreas Klinger wrote:
[...]
> + indio_dev->name = dev_name(>dev);

The name is supposed to be the type of the device, e.g. part name, not the
name of parent device instance. E.g. "srf08" in this case.

Re: [PATCH v2 2/2] iio: distance: srf08: add IIO driver for us ranger

[Lars-Peter Clausen]

On 01/10/2017 07:48 PM, Andreas Klinger wrote:
[...]
> + indio_dev->name = dev_name(>dev);

The name is supposed to be the type of the device, e.g. part name, not the
name of parent device instance. E.g. "srf08" in this case.

Re: [PATCH v2 2/2] iio: distance: srf08: add IIO driver for us ranger

[Andreas Klinger]

Hi Jonathan,

see comments below.

Andreas


Jonathan Cameron  schrieb am Sat, 14. Jan 12:17:
> On 10/01/17 18:48, Andreas Klinger wrote:
> > This is the IIO driver for devantech srf08 ultrasonic ranger which can be
> > used to measure the distances to an object.
> > 
> > The sensor supports I2C with some registers.
> > 
> > Supported Features include:
> > - read the distance in ranging mode in centimeters
> > - output of the driver is directly the read value
> > - together with the scale the driver delivers the distance in meters
> > - only the first echo of the nearest object is delivered
> > - set max gain register; userspace enters analogue value
> > - set range registers; userspace enters range in millimeters in 43 mm steps
> > 
> > Features not supported by this driver:
> > - ranging mode in inches or in microseconds
> > - ANN mode
> > - change I2C address through this driver
> > - light sensor
> > 
> > The driver was added in the directory "proximity" of the iio subsystem
> > in absence of another directory named "distance".
> > There is also a new submenu "distance"
> Hi Andreas,
> 
> Sorry it took me a while to get to this!
> 
> I'd not bother with the new submenu.  Perhaps we should rename the
> proximity menu to proximity/distance.
> 
> We already the lightening detector in there which is definitely not
> measuring proximity in the convetional sense!
> 
> Anyhow, the actual code is fine, but we need to think about how the
> userspace ABI fits within the wider IIO ABI.  Naming and approaches
> that make sense in a single class of drivers can end up meaining
> very different things for other drivers.  Various suggestions inline.
> 
> Jonathan
> > 
> > Signed-off-by: Andreas Klinger 
> > ---
> >  drivers/iio/proximity/Kconfig  |  15 ++
> >  drivers/iio/proximity/Makefile |   1 +
> >  drivers/iio/proximity/srf08.c  | 362 
> > +
> >  3 files changed, 378 insertions(+)
> >  create mode 100644 drivers/iio/proximity/srf08.c
> > 
> > diff --git a/drivers/iio/proximity/Kconfig b/drivers/iio/proximity/Kconfig
> > index ef4c73db5b53..7b10a137702b 100644
> > --- a/drivers/iio/proximity/Kconfig
> > +++ b/drivers/iio/proximity/Kconfig
> > @@ -46,3 +46,18 @@ config SX9500
> >   module will be called sx9500.
> >  
> >  endmenu
> > +
> > +menu "Distance sensors"
> > +
> > +config SRF08
> > +   tristate "Devantech SRF08 ultrasonic ranger sensor"
> > +   depends on I2C
> > +   help
> > + Say Y here to build a driver for Devantech SRF08 ultrasonic
> > + ranger sensor. This driver can be used to measure the distance
> > + of objects.
> > +
> > + To compile this driver as a module, choose M here: the
> > + module will be called srf08.
> > +
> > +endmenu
> > diff --git a/drivers/iio/proximity/Makefile b/drivers/iio/proximity/Makefile
> > index 9aadd9a8ee99..e914c2a5dd49 100644
> > --- a/drivers/iio/proximity/Makefile
> > +++ b/drivers/iio/proximity/Makefile
> > @@ -5,4 +5,5 @@
> >  # When adding new entries keep the list in alphabetical order
> >  obj-$(CONFIG_AS3935)   += as3935.o
> >  obj-$(CONFIG_LIDAR_LITE_V2)+= pulsedlight-lidar-lite-v2.o
> > +obj-$(CONFIG_SRF08)+= srf08.o
> >  obj-$(CONFIG_SX9500)   += sx9500.o
> > diff --git a/drivers/iio/proximity/srf08.c b/drivers/iio/proximity/srf08.c
> > new file mode 100644
> > index ..f38c74ed0933
> > --- /dev/null
> > +++ b/drivers/iio/proximity/srf08.c
> > @@ -0,0 +1,362 @@
> > +/*
> > + * srf08.c - Support for Devantech SRF08 ultrasonic ranger
> > + *
> > + * Copyright (c) 2016 Andreas Klinger 
> > + *
> > + * This file is subject to the terms and conditions of version 2 of
> > + * the GNU General Public License.  See the file COPYING in the main
> > + * directory of this archive for more details.
> > + *
> > + * For details about the device see:
> > + * http://www.robot-electronics.co.uk/htm/srf08tech.html
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +/* registers of SRF08 device */
> > +#define SRF08_WRITE_COMMAND0x00/* Command Register */
> > +#define SRF08_WRITE_MAX_GAIN   0x01/* Max Gain Register: 0 .. 31 */
> > +#define SRF08_WRITE_RANGE  0x02/* Range Register: 0 .. 255 */
> > +#define SRF08_READ_SW_REVISION 0x00/* Software Revision */
> > +#define SRF08_READ_LIGHT   0x01/* Light Sensor during last echo */
> > +#define SRF08_READ_ECHO_1_HIGH 0x02/* Range of first echo received 
> > */
> > +#define SRF08_READ_ECHO_1_LOW  0x03/* Range of first echo received 
> > */
> > +
> > +#define SRF08_CMD_RANGING_CM   0x51/* Ranging Mode - Result in cm 
> > */
> > +
> > +#define SRF08_DEFAULT_GAIN 1025/* max. analogue value of Gain */
> > +#define SRF08_DEFAULT_RANGE11008   /* max. value of Range in mm */
> > +
> > +struct srf08_data {
> > +   

Re: [PATCH v2 2/2] iio: distance: srf08: add IIO driver for us ranger

[Andreas Klinger]

Hi Jonathan,

see comments below.

Andreas


Jonathan Cameron  schrieb am Sat, 14. Jan 12:17:
> On 10/01/17 18:48, Andreas Klinger wrote:
> > This is the IIO driver for devantech srf08 ultrasonic ranger which can be
> > used to measure the distances to an object.
> > 
> > The sensor supports I2C with some registers.
> > 
> > Supported Features include:
> > - read the distance in ranging mode in centimeters
> > - output of the driver is directly the read value
> > - together with the scale the driver delivers the distance in meters
> > - only the first echo of the nearest object is delivered
> > - set max gain register; userspace enters analogue value
> > - set range registers; userspace enters range in millimeters in 43 mm steps
> > 
> > Features not supported by this driver:
> > - ranging mode in inches or in microseconds
> > - ANN mode
> > - change I2C address through this driver
> > - light sensor
> > 
> > The driver was added in the directory "proximity" of the iio subsystem
> > in absence of another directory named "distance".
> > There is also a new submenu "distance"
> Hi Andreas,
> 
> Sorry it took me a while to get to this!
> 
> I'd not bother with the new submenu.  Perhaps we should rename the
> proximity menu to proximity/distance.
> 
> We already the lightening detector in there which is definitely not
> measuring proximity in the convetional sense!
> 
> Anyhow, the actual code is fine, but we need to think about how the
> userspace ABI fits within the wider IIO ABI.  Naming and approaches
> that make sense in a single class of drivers can end up meaining
> very different things for other drivers.  Various suggestions inline.
> 
> Jonathan
> > 
> > Signed-off-by: Andreas Klinger 
> > ---
> >  drivers/iio/proximity/Kconfig  |  15 ++
> >  drivers/iio/proximity/Makefile |   1 +
> >  drivers/iio/proximity/srf08.c  | 362 
> > +
> >  3 files changed, 378 insertions(+)
> >  create mode 100644 drivers/iio/proximity/srf08.c
> > 
> > diff --git a/drivers/iio/proximity/Kconfig b/drivers/iio/proximity/Kconfig
> > index ef4c73db5b53..7b10a137702b 100644
> > --- a/drivers/iio/proximity/Kconfig
> > +++ b/drivers/iio/proximity/Kconfig
> > @@ -46,3 +46,18 @@ config SX9500
> >   module will be called sx9500.
> >  
> >  endmenu
> > +
> > +menu "Distance sensors"
> > +
> > +config SRF08
> > +   tristate "Devantech SRF08 ultrasonic ranger sensor"
> > +   depends on I2C
> > +   help
> > + Say Y here to build a driver for Devantech SRF08 ultrasonic
> > + ranger sensor. This driver can be used to measure the distance
> > + of objects.
> > +
> > + To compile this driver as a module, choose M here: the
> > + module will be called srf08.
> > +
> > +endmenu
> > diff --git a/drivers/iio/proximity/Makefile b/drivers/iio/proximity/Makefile
> > index 9aadd9a8ee99..e914c2a5dd49 100644
> > --- a/drivers/iio/proximity/Makefile
> > +++ b/drivers/iio/proximity/Makefile
> > @@ -5,4 +5,5 @@
> >  # When adding new entries keep the list in alphabetical order
> >  obj-$(CONFIG_AS3935)   += as3935.o
> >  obj-$(CONFIG_LIDAR_LITE_V2)+= pulsedlight-lidar-lite-v2.o
> > +obj-$(CONFIG_SRF08)+= srf08.o
> >  obj-$(CONFIG_SX9500)   += sx9500.o
> > diff --git a/drivers/iio/proximity/srf08.c b/drivers/iio/proximity/srf08.c
> > new file mode 100644
> > index ..f38c74ed0933
> > --- /dev/null
> > +++ b/drivers/iio/proximity/srf08.c
> > @@ -0,0 +1,362 @@
> > +/*
> > + * srf08.c - Support for Devantech SRF08 ultrasonic ranger
> > + *
> > + * Copyright (c) 2016 Andreas Klinger 
> > + *
> > + * This file is subject to the terms and conditions of version 2 of
> > + * the GNU General Public License.  See the file COPYING in the main
> > + * directory of this archive for more details.
> > + *
> > + * For details about the device see:
> > + * http://www.robot-electronics.co.uk/htm/srf08tech.html
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +/* registers of SRF08 device */
> > +#define SRF08_WRITE_COMMAND0x00/* Command Register */
> > +#define SRF08_WRITE_MAX_GAIN   0x01/* Max Gain Register: 0 .. 31 */
> > +#define SRF08_WRITE_RANGE  0x02/* Range Register: 0 .. 255 */
> > +#define SRF08_READ_SW_REVISION 0x00/* Software Revision */
> > +#define SRF08_READ_LIGHT   0x01/* Light Sensor during last echo */
> > +#define SRF08_READ_ECHO_1_HIGH 0x02/* Range of first echo received 
> > */
> > +#define SRF08_READ_ECHO_1_LOW  0x03/* Range of first echo received 
> > */
> > +
> > +#define SRF08_CMD_RANGING_CM   0x51/* Ranging Mode - Result in cm 
> > */
> > +
> > +#define SRF08_DEFAULT_GAIN 1025/* max. analogue value of Gain */
> > +#define SRF08_DEFAULT_RANGE11008   /* max. value of Range in mm */
> > +
> > +struct srf08_data {
> > +   struct i2c_client   *client;
> > +   int   

Re: [PATCH v2 2/2] iio: distance: srf08: add IIO driver for us ranger

[Jonathan Cameron]

On 10/01/17 18:48, Andreas Klinger wrote:
> This is the IIO driver for devantech srf08 ultrasonic ranger which can be
> used to measure the distances to an object.
> 
> The sensor supports I2C with some registers.
> 
> Supported Features include:
> - read the distance in ranging mode in centimeters
> - output of the driver is directly the read value
> - together with the scale the driver delivers the distance in meters
> - only the first echo of the nearest object is delivered
> - set max gain register; userspace enters analogue value
> - set range registers; userspace enters range in millimeters in 43 mm steps
> 
> Features not supported by this driver:
> - ranging mode in inches or in microseconds
> - ANN mode
> - change I2C address through this driver
> - light sensor
> 
> The driver was added in the directory "proximity" of the iio subsystem
> in absence of another directory named "distance".
> There is also a new submenu "distance"
Hi Andreas,

Sorry it took me a while to get to this!

I'd not bother with the new submenu.  Perhaps we should rename the
proximity menu to proximity/distance.

We already the lightening detector in there which is definitely not
measuring proximity in the convetional sense!

Anyhow, the actual code is fine, but we need to think about how the
userspace ABI fits within the wider IIO ABI.  Naming and approaches
that make sense in a single class of drivers can end up meaining
very different things for other drivers.  Various suggestions inline.

Jonathan
> 
> Signed-off-by: Andreas Klinger 
> ---
>  drivers/iio/proximity/Kconfig  |  15 ++
>  drivers/iio/proximity/Makefile |   1 +
>  drivers/iio/proximity/srf08.c  | 362 
> +
>  3 files changed, 378 insertions(+)
>  create mode 100644 drivers/iio/proximity/srf08.c
> 
> diff --git a/drivers/iio/proximity/Kconfig b/drivers/iio/proximity/Kconfig
> index ef4c73db5b53..7b10a137702b 100644
> --- a/drivers/iio/proximity/Kconfig
> +++ b/drivers/iio/proximity/Kconfig
> @@ -46,3 +46,18 @@ config SX9500
> module will be called sx9500.
>  
>  endmenu
> +
> +menu "Distance sensors"
> +
> +config SRF08
> + tristate "Devantech SRF08 ultrasonic ranger sensor"
> + depends on I2C
> + help
> +   Say Y here to build a driver for Devantech SRF08 ultrasonic
> +   ranger sensor. This driver can be used to measure the distance
> +   of objects.
> +
> +   To compile this driver as a module, choose M here: the
> +   module will be called srf08.
> +
> +endmenu
> diff --git a/drivers/iio/proximity/Makefile b/drivers/iio/proximity/Makefile
> index 9aadd9a8ee99..e914c2a5dd49 100644
> --- a/drivers/iio/proximity/Makefile
> +++ b/drivers/iio/proximity/Makefile
> @@ -5,4 +5,5 @@
>  # When adding new entries keep the list in alphabetical order
>  obj-$(CONFIG_AS3935) += as3935.o
>  obj-$(CONFIG_LIDAR_LITE_V2)  += pulsedlight-lidar-lite-v2.o
> +obj-$(CONFIG_SRF08)  += srf08.o
>  obj-$(CONFIG_SX9500) += sx9500.o
> diff --git a/drivers/iio/proximity/srf08.c b/drivers/iio/proximity/srf08.c
> new file mode 100644
> index ..f38c74ed0933
> --- /dev/null
> +++ b/drivers/iio/proximity/srf08.c
> @@ -0,0 +1,362 @@
> +/*
> + * srf08.c - Support for Devantech SRF08 ultrasonic ranger
> + *
> + * Copyright (c) 2016 Andreas Klinger 
> + *
> + * This file is subject to the terms and conditions of version 2 of
> + * the GNU General Public License.  See the file COPYING in the main
> + * directory of this archive for more details.
> + *
> + * For details about the device see:
> + * http://www.robot-electronics.co.uk/htm/srf08tech.html
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/* registers of SRF08 device */
> +#define SRF08_WRITE_COMMAND  0x00/* Command Register */
> +#define SRF08_WRITE_MAX_GAIN 0x01/* Max Gain Register: 0 .. 31 */
> +#define SRF08_WRITE_RANGE0x02/* Range Register: 0 .. 255 */
> +#define SRF08_READ_SW_REVISION   0x00/* Software Revision */
> +#define SRF08_READ_LIGHT 0x01/* Light Sensor during last echo */
> +#define SRF08_READ_ECHO_1_HIGH   0x02/* Range of first echo received 
> */
> +#define SRF08_READ_ECHO_1_LOW0x03/* Range of first echo received 
> */
> +
> +#define SRF08_CMD_RANGING_CM 0x51/* Ranging Mode - Result in cm */
> +
> +#define SRF08_DEFAULT_GAIN   1025/* max. analogue value of Gain */
> +#define SRF08_DEFAULT_RANGE  11008   /* max. value of Range in mm */
> +
> +struct srf08_data {
> + struct i2c_client   *client;
> + int gain;   /* Max Gain */
> + int range_mm;   /* Range in mm */
> + struct mutexlock;
> +};
> +
> +static const int srf08_gain[] = {
> +  94,  97, 100, 103, 107, 110, 114, 118,
> + 123, 128, 133, 139, 145, 152, 159, 168,
> + 177, 187, 199, 212, 

Re: [PATCH v2 2/2] iio: distance: srf08: add IIO driver for us ranger

[Jonathan Cameron]

On 10/01/17 18:48, Andreas Klinger wrote:
> This is the IIO driver for devantech srf08 ultrasonic ranger which can be
> used to measure the distances to an object.
> 
> The sensor supports I2C with some registers.
> 
> Supported Features include:
> - read the distance in ranging mode in centimeters
> - output of the driver is directly the read value
> - together with the scale the driver delivers the distance in meters
> - only the first echo of the nearest object is delivered
> - set max gain register; userspace enters analogue value
> - set range registers; userspace enters range in millimeters in 43 mm steps
> 
> Features not supported by this driver:
> - ranging mode in inches or in microseconds
> - ANN mode
> - change I2C address through this driver
> - light sensor
> 
> The driver was added in the directory "proximity" of the iio subsystem
> in absence of another directory named "distance".
> There is also a new submenu "distance"
Hi Andreas,

Sorry it took me a while to get to this!

I'd not bother with the new submenu.  Perhaps we should rename the
proximity menu to proximity/distance.

We already the lightening detector in there which is definitely not
measuring proximity in the convetional sense!

Anyhow, the actual code is fine, but we need to think about how the
userspace ABI fits within the wider IIO ABI.  Naming and approaches
that make sense in a single class of drivers can end up meaining
very different things for other drivers.  Various suggestions inline.

Jonathan
> 
> Signed-off-by: Andreas Klinger 
> ---
>  drivers/iio/proximity/Kconfig  |  15 ++
>  drivers/iio/proximity/Makefile |   1 +
>  drivers/iio/proximity/srf08.c  | 362 
> +
>  3 files changed, 378 insertions(+)
>  create mode 100644 drivers/iio/proximity/srf08.c
> 
> diff --git a/drivers/iio/proximity/Kconfig b/drivers/iio/proximity/Kconfig
> index ef4c73db5b53..7b10a137702b 100644
> --- a/drivers/iio/proximity/Kconfig
> +++ b/drivers/iio/proximity/Kconfig
> @@ -46,3 +46,18 @@ config SX9500
> module will be called sx9500.
>  
>  endmenu
> +
> +menu "Distance sensors"
> +
> +config SRF08
> + tristate "Devantech SRF08 ultrasonic ranger sensor"
> + depends on I2C
> + help
> +   Say Y here to build a driver for Devantech SRF08 ultrasonic
> +   ranger sensor. This driver can be used to measure the distance
> +   of objects.
> +
> +   To compile this driver as a module, choose M here: the
> +   module will be called srf08.
> +
> +endmenu
> diff --git a/drivers/iio/proximity/Makefile b/drivers/iio/proximity/Makefile
> index 9aadd9a8ee99..e914c2a5dd49 100644
> --- a/drivers/iio/proximity/Makefile
> +++ b/drivers/iio/proximity/Makefile
> @@ -5,4 +5,5 @@
>  # When adding new entries keep the list in alphabetical order
>  obj-$(CONFIG_AS3935) += as3935.o
>  obj-$(CONFIG_LIDAR_LITE_V2)  += pulsedlight-lidar-lite-v2.o
> +obj-$(CONFIG_SRF08)  += srf08.o
>  obj-$(CONFIG_SX9500) += sx9500.o
> diff --git a/drivers/iio/proximity/srf08.c b/drivers/iio/proximity/srf08.c
> new file mode 100644
> index ..f38c74ed0933
> --- /dev/null
> +++ b/drivers/iio/proximity/srf08.c
> @@ -0,0 +1,362 @@
> +/*
> + * srf08.c - Support for Devantech SRF08 ultrasonic ranger
> + *
> + * Copyright (c) 2016 Andreas Klinger 
> + *
> + * This file is subject to the terms and conditions of version 2 of
> + * the GNU General Public License.  See the file COPYING in the main
> + * directory of this archive for more details.
> + *
> + * For details about the device see:
> + * http://www.robot-electronics.co.uk/htm/srf08tech.html
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/* registers of SRF08 device */
> +#define SRF08_WRITE_COMMAND  0x00/* Command Register */
> +#define SRF08_WRITE_MAX_GAIN 0x01/* Max Gain Register: 0 .. 31 */
> +#define SRF08_WRITE_RANGE0x02/* Range Register: 0 .. 255 */
> +#define SRF08_READ_SW_REVISION   0x00/* Software Revision */
> +#define SRF08_READ_LIGHT 0x01/* Light Sensor during last echo */
> +#define SRF08_READ_ECHO_1_HIGH   0x02/* Range of first echo received 
> */
> +#define SRF08_READ_ECHO_1_LOW0x03/* Range of first echo received 
> */
> +
> +#define SRF08_CMD_RANGING_CM 0x51/* Ranging Mode - Result in cm */
> +
> +#define SRF08_DEFAULT_GAIN   1025/* max. analogue value of Gain */
> +#define SRF08_DEFAULT_RANGE  11008   /* max. value of Range in mm */
> +
> +struct srf08_data {
> + struct i2c_client   *client;
> + int gain;   /* Max Gain */
> + int range_mm;   /* Range in mm */
> + struct mutexlock;
> +};
> +
> +static const int srf08_gain[] = {
> +  94,  97, 100, 103, 107, 110, 114, 118,
> + 123, 128, 133, 139, 145, 152, 159, 168,
> + 177, 187, 199, 212, 227, 245, 265, 288,
> + 317, 352,