Re: [PATCH v3 9/9] staging: iio: ad2s1200: Move driver out of staging

[David Julian Veenstra]

On 28, April 2018 17:46, Jonathan Cameron wrote:

> On Mon, 23 Apr 2018 00:04:42 +0200
> David Veenstra  wrote:
>
>> Move the iio driver for the ad2s1200 and ad2s1205 resolver-to-digital
>> converter out of staging, into mainline iio subsystems.
>> 
>> Signed-off-by: David Veenstra 
> A few more minor bits and bobs + suggestions.
>
> Some little things that I missed in earlier patches.  If I haven't
> taken the relevant patch, please roll the fixes in for V4.  If I have
> just do a little follow up patch.
>
> Very nearly there!
>

Thanks, I'll add all the recommendations in V4.

Best regards,
David Veenstra

> thanks,
>
> Jonathan
>
>> ---
>> Changes in v3:
>>  - Add mention of ad2s1205 in commit message.
>> 
>>  drivers/iio/Kconfig |   1 +
>>  drivers/iio/Makefile|   1 +
>>  drivers/iio/resolver/Kconfig|  17 +++
>>  drivers/iio/resolver/Makefile   |   5 +
>>  drivers/iio/resolver/ad2s1200.c | 201 
>> 
>>  drivers/staging/iio/resolver/Kconfig|  12 --
>>  drivers/staging/iio/resolver/Makefile   |   1 -
>>  drivers/staging/iio/resolver/ad2s1200.c | 201 
>> 
>>  8 files changed, 225 insertions(+), 214 deletions(-)
>>  create mode 100644 drivers/iio/resolver/Kconfig
>>  create mode 100644 drivers/iio/resolver/Makefile
>>  create mode 100644 drivers/iio/resolver/ad2s1200.c
>>  delete mode 100644 drivers/staging/iio/resolver/ad2s1200.c
>> 
>> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
>> index b3c8c6ef0dff..4bec3ccbf4a1 100644
>> --- a/drivers/iio/Kconfig
>> +++ b/drivers/iio/Kconfig
>> @@ -92,6 +92,7 @@ source "drivers/iio/potentiometer/Kconfig"
>>  source "drivers/iio/potentiostat/Kconfig"
>>  source "drivers/iio/pressure/Kconfig"
>>  source "drivers/iio/proximity/Kconfig"
>> +source "drivers/iio/resolver/Kconfig"
>>  source "drivers/iio/temperature/Kconfig"
>>  
>>  endif # IIO
>> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
>> index b16b2e9ddc40..1865361b8714 100644
>> --- a/drivers/iio/Makefile
>> +++ b/drivers/iio/Makefile
>> @@ -35,5 +35,6 @@ obj-y += potentiometer/
>>  obj-y += potentiostat/
>>  obj-y += pressure/
>>  obj-y += proximity/
>> +obj-y += resolver/
>>  obj-y += temperature/
>>  obj-y += trigger/
>> diff --git a/drivers/iio/resolver/Kconfig b/drivers/iio/resolver/Kconfig
>> new file mode 100644
>> index ..2ced9f22aa70
>> --- /dev/null
>> +++ b/drivers/iio/resolver/Kconfig
>> @@ -0,0 +1,17 @@
>> +#
>> +# Resolver/Synchro drivers
>> +#
>> +menu "Resolver to digital converters"
>> +
>> +config AD2S1200
>> +tristate "Analog Devices ad2s1200/ad2s1205 driver"
>> +depends on SPI
>> +depends on GPIOLIB || COMPILE_TEST
>> +help
>> +  Say yes here to build support for Analog Devices spi resolver
>> +  to digital converters, ad2s1200 and ad2s1205, provides direct access
>> +  via sysfs.
>> +
>> +  To compile this driver as a module, choose M here: the
>> +  module will be called ad2s1200.
>> +endmenu
>> diff --git a/drivers/iio/resolver/Makefile b/drivers/iio/resolver/Makefile
>> new file mode 100644
>> index ..4e1dccae07e7
>> --- /dev/null
>> +++ b/drivers/iio/resolver/Makefile
>> @@ -0,0 +1,5 @@
>> +#
>> +# Makefile for Resolver/Synchro drivers
>> +#
>> +
>> +obj-$(CONFIG_AD2S1200) += ad2s1200.o
>> diff --git a/drivers/iio/resolver/ad2s1200.c 
>> b/drivers/iio/resolver/ad2s1200.c
>> new file mode 100644
>> index ..d2b62308b31d
>> --- /dev/null
>> +++ b/drivers/iio/resolver/ad2s1200.c
>> @@ -0,0 +1,201 @@
>> +/*
>> + * ad2s1200.c simple support for the ADI Resolver to Digital Converters:
>> + * AD2S1200/1205
>> + *
>> + * Copyright (c) 2010-2010 Analog Devices Inc.
>
> I think you have done enough changes in here that if you want to add your
> own copyright I think it would be justified.
>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>
> One of my pet hates if you want to clean it up :)
> No point in having a blank line above here.
>
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include 
>> +#include 
>> +
>> +#define DRV_NAME "ad2s1200"
>> +
>> +/* input clock on serial interface */
>> +#define AD2S1200_HZ 8192000
>> +/* clock period in nano second */
>> +#define AD2S1200_TSCLK  (10 / AD2S1200_HZ)
>> +
>> +/**
>> + * struct ad2s1200_state - driver instance specific data.
>> + * @lock:   protects both the GPIO pins and the rx buffer, to prevent
>> + *  concurrent spi transactions.
>> + * @sdev:   spi device.
>> + * @sample: GPIO pin SAMPLE.
>> + * @rdvel:  GPIO pin RDVEL.
>> + * @rx: buffer for spi 

Re: [PATCH v3 9/9] staging: iio: ad2s1200: Move driver out of staging

[Jonathan Cameron]

On Mon, 23 Apr 2018 00:04:42 +0200
David Veenstra  wrote:

> Move the iio driver for the ad2s1200 and ad2s1205 resolver-to-digital
> converter out of staging, into mainline iio subsystems.
> 
> Signed-off-by: David Veenstra 
A few more minor bits and bobs + suggestions.

Some little things that I missed in earlier patches.  If I haven't
taken the relevant patch, please roll the fixes in for V4.  If I have
just do a little follow up patch.

Very nearly there!

thanks,

Jonathan

> ---
> Changes in v3:
>  - Add mention of ad2s1205 in commit message.
> 
>  drivers/iio/Kconfig |   1 +
>  drivers/iio/Makefile|   1 +
>  drivers/iio/resolver/Kconfig|  17 +++
>  drivers/iio/resolver/Makefile   |   5 +
>  drivers/iio/resolver/ad2s1200.c | 201 
> 
>  drivers/staging/iio/resolver/Kconfig|  12 --
>  drivers/staging/iio/resolver/Makefile   |   1 -
>  drivers/staging/iio/resolver/ad2s1200.c | 201 
> 
>  8 files changed, 225 insertions(+), 214 deletions(-)
>  create mode 100644 drivers/iio/resolver/Kconfig
>  create mode 100644 drivers/iio/resolver/Makefile
>  create mode 100644 drivers/iio/resolver/ad2s1200.c
>  delete mode 100644 drivers/staging/iio/resolver/ad2s1200.c
> 
> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
> index b3c8c6ef0dff..4bec3ccbf4a1 100644
> --- a/drivers/iio/Kconfig
> +++ b/drivers/iio/Kconfig
> @@ -92,6 +92,7 @@ source "drivers/iio/potentiometer/Kconfig"
>  source "drivers/iio/potentiostat/Kconfig"
>  source "drivers/iio/pressure/Kconfig"
>  source "drivers/iio/proximity/Kconfig"
> +source "drivers/iio/resolver/Kconfig"
>  source "drivers/iio/temperature/Kconfig"
>  
>  endif # IIO
> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
> index b16b2e9ddc40..1865361b8714 100644
> --- a/drivers/iio/Makefile
> +++ b/drivers/iio/Makefile
> @@ -35,5 +35,6 @@ obj-y += potentiometer/
>  obj-y += potentiostat/
>  obj-y += pressure/
>  obj-y += proximity/
> +obj-y += resolver/
>  obj-y += temperature/
>  obj-y += trigger/
> diff --git a/drivers/iio/resolver/Kconfig b/drivers/iio/resolver/Kconfig
> new file mode 100644
> index ..2ced9f22aa70
> --- /dev/null
> +++ b/drivers/iio/resolver/Kconfig
> @@ -0,0 +1,17 @@
> +#
> +# Resolver/Synchro drivers
> +#
> +menu "Resolver to digital converters"
> +
> +config AD2S1200
> + tristate "Analog Devices ad2s1200/ad2s1205 driver"
> + depends on SPI
> + depends on GPIOLIB || COMPILE_TEST
> + help
> +   Say yes here to build support for Analog Devices spi resolver
> +   to digital converters, ad2s1200 and ad2s1205, provides direct access
> +   via sysfs.
> +
> +   To compile this driver as a module, choose M here: the
> +   module will be called ad2s1200.
> +endmenu
> diff --git a/drivers/iio/resolver/Makefile b/drivers/iio/resolver/Makefile
> new file mode 100644
> index ..4e1dccae07e7
> --- /dev/null
> +++ b/drivers/iio/resolver/Makefile
> @@ -0,0 +1,5 @@
> +#
> +# Makefile for Resolver/Synchro drivers
> +#
> +
> +obj-$(CONFIG_AD2S1200) += ad2s1200.o
> diff --git a/drivers/iio/resolver/ad2s1200.c b/drivers/iio/resolver/ad2s1200.c
> new file mode 100644
> index ..d2b62308b31d
> --- /dev/null
> +++ b/drivers/iio/resolver/ad2s1200.c
> @@ -0,0 +1,201 @@
> +/*
> + * ad2s1200.c simple support for the ADI Resolver to Digital Converters:
> + * AD2S1200/1205
> + *
> + * Copyright (c) 2010-2010 Analog Devices Inc.

I think you have done enough changes in here that if you want to add your
own copyright I think it would be justified.

> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *

One of my pet hates if you want to clean it up :)
No point in having a blank line above here.

> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +
> +#define DRV_NAME "ad2s1200"
> +
> +/* input clock on serial interface */
> +#define AD2S1200_HZ  8192000
> +/* clock period in nano second */
> +#define AD2S1200_TSCLK   (10 / AD2S1200_HZ)
> +
> +/**
> + * struct ad2s1200_state - driver instance specific data.
> + * @lock:protects both the GPIO pins and the rx buffer, to prevent
> + *   concurrent spi transactions.
> + * @sdev:spi device.
> + * @sample:  GPIO pin SAMPLE.
> + * @rdvel:   GPIO pin RDVEL.
> + * @rx:  buffer for spi transfers.
> + */
> +struct ad2s1200_state {
> + struct mutex lock;
> + struct spi_device *sdev;
> + struct gpio_desc *sample;
> + struct gpio_desc *rdvel;
> + __be16 rx cacheline_aligned;
> +};
> +
> +static int ad2s1200_read_raw(struct iio_dev *indio_dev,
> + 

[PATCH v3 9/9] staging: iio: ad2s1200: Move driver out of staging

[David Veenstra]

Move the iio driver for the ad2s1200 and ad2s1205 resolver-to-digital
converter out of staging, into mainline iio subsystems.

Signed-off-by: David Veenstra 
---
Changes in v3:
 - Add mention of ad2s1205 in commit message.

 drivers/iio/Kconfig |   1 +
 drivers/iio/Makefile|   1 +
 drivers/iio/resolver/Kconfig|  17 +++
 drivers/iio/resolver/Makefile   |   5 +
 drivers/iio/resolver/ad2s1200.c | 201 
 drivers/staging/iio/resolver/Kconfig|  12 --
 drivers/staging/iio/resolver/Makefile   |   1 -
 drivers/staging/iio/resolver/ad2s1200.c | 201 
 8 files changed, 225 insertions(+), 214 deletions(-)
 create mode 100644 drivers/iio/resolver/Kconfig
 create mode 100644 drivers/iio/resolver/Makefile
 create mode 100644 drivers/iio/resolver/ad2s1200.c
 delete mode 100644 drivers/staging/iio/resolver/ad2s1200.c

diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
index b3c8c6ef0dff..4bec3ccbf4a1 100644
--- a/drivers/iio/Kconfig
+++ b/drivers/iio/Kconfig
@@ -92,6 +92,7 @@ source "drivers/iio/potentiometer/Kconfig"
 source "drivers/iio/potentiostat/Kconfig"
 source "drivers/iio/pressure/Kconfig"
 source "drivers/iio/proximity/Kconfig"
+source "drivers/iio/resolver/Kconfig"
 source "drivers/iio/temperature/Kconfig"
 
 endif # IIO
diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
index b16b2e9ddc40..1865361b8714 100644
--- a/drivers/iio/Makefile
+++ b/drivers/iio/Makefile
@@ -35,5 +35,6 @@ obj-y += potentiometer/
 obj-y += potentiostat/
 obj-y += pressure/
 obj-y += proximity/
+obj-y += resolver/
 obj-y += temperature/
 obj-y += trigger/
diff --git a/drivers/iio/resolver/Kconfig b/drivers/iio/resolver/Kconfig
new file mode 100644
index ..2ced9f22aa70
--- /dev/null
+++ b/drivers/iio/resolver/Kconfig
@@ -0,0 +1,17 @@
+#
+# Resolver/Synchro drivers
+#
+menu "Resolver to digital converters"
+
+config AD2S1200
+   tristate "Analog Devices ad2s1200/ad2s1205 driver"
+   depends on SPI
+   depends on GPIOLIB || COMPILE_TEST
+   help
+ Say yes here to build support for Analog Devices spi resolver
+ to digital converters, ad2s1200 and ad2s1205, provides direct access
+ via sysfs.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ad2s1200.
+endmenu
diff --git a/drivers/iio/resolver/Makefile b/drivers/iio/resolver/Makefile
new file mode 100644
index ..4e1dccae07e7
--- /dev/null
+++ b/drivers/iio/resolver/Makefile
@@ -0,0 +1,5 @@
+#
+# Makefile for Resolver/Synchro drivers
+#
+
+obj-$(CONFIG_AD2S1200) += ad2s1200.o
diff --git a/drivers/iio/resolver/ad2s1200.c b/drivers/iio/resolver/ad2s1200.c
new file mode 100644
index ..d2b62308b31d
--- /dev/null
+++ b/drivers/iio/resolver/ad2s1200.c
@@ -0,0 +1,201 @@
+/*
+ * ad2s1200.c simple support for the ADI Resolver to Digital Converters:
+ * AD2S1200/1205
+ *
+ * Copyright (c) 2010-2010 Analog Devices Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+#define DRV_NAME "ad2s1200"
+
+/* input clock on serial interface */
+#define AD2S1200_HZ8192000
+/* clock period in nano second */
+#define AD2S1200_TSCLK (10 / AD2S1200_HZ)
+
+/**
+ * struct ad2s1200_state - driver instance specific data.
+ * @lock:  protects both the GPIO pins and the rx buffer, to prevent
+ * concurrent spi transactions.
+ * @sdev:  spi device.
+ * @sample:GPIO pin SAMPLE.
+ * @rdvel: GPIO pin RDVEL.
+ * @rx:buffer for spi transfers.
+ */
+struct ad2s1200_state {
+   struct mutex lock;
+   struct spi_device *sdev;
+   struct gpio_desc *sample;
+   struct gpio_desc *rdvel;
+   __be16 rx cacheline_aligned;
+};
+
+static int ad2s1200_read_raw(struct iio_dev *indio_dev,
+struct iio_chan_spec const *chan,
+int *val,
+int *val2,
+long m)
+{
+   struct ad2s1200_state *st = iio_priv(indio_dev);
+   int ret = 0;
+
+   switch (m) {
+   case IIO_CHAN_INFO_SCALE:
+   switch (chan->type) {
+   case IIO_ANGL:
+   /* 2 * Pi / (2^12 - 1) ~= 0.001534355 */
+   *val = 0;
+   *val2 = 1534355;
+   return IIO_VAL_INT_PLUS_NANO;
+   case IIO_ANGL_VEL:
+   /* 2 * Pi ~= 6.283185 */
+   *val = 6;
+   *val2 = 283185;
+   return IIO_VAL_INT_PLUS_MICRO;
+