[PATCH 00/17] KCSD9 driver cleanup and modernization

[PATCH 00/17] KCSD9 driver cleanup and modernization

[Linus Walleij]

This patch series:

- Fixes a bunch of bugs in the KXSD9 driver.

- Make the KXSD9 driver actually report m/s^2 and removes
  a fat offset.

- Adds I2C support.

- Adds triggered buffer handling.

- Adds system and runtime PM.

- Adds support for the mounting matrix.

- Adds device tree bindings.

- Misc cleanup.

Some of it should arguably go into stable but I am pretty well
convinced that this driver is in use by one person in the world:
me.

Jonathan: if you wanna try it on your SPI-based KXSD9 (if it
doesn't work I am pretty sure all fixing just need to happen in
kxsd9-spi.c) go ahead, but unless there is some horrific problem
with any of the patches and you don't have time to test it on
SPI I would just take them and run ;)

Linus Walleij (17):
  iio: accel: kxsd9: Add device tree bindings
  iio: accel: kxsd9: Fix raw read return
  iio: accel: kxsd9: Split out transport mechanism
  iio: accel: kxsd9: Use devm_iio_device_register()
  iio: accel: kxsd9: Split out SPI transport
  iio: accel: kxsd9: Do away with the write2 helper
  iio: accel: kxsd9: Convert to use regmap for transport
  iio: accel: kxsd9: Add I2C transport
  iio: accel: kxsd9: Drop the buffer lock
  iio: accel: kxsd9: Fix scaling bug
  iio: accel: kxsd9: Fix up offset and scaling
  iio: accel: kxsd9: Add triggered buffer handling
  iio: accel: kxsd9: Deploy proper register bit defines
  iio: accel: kxsd9: Fetch and handle regulators
  iio: accel: kxsd9: Replace "parent" with "dev"
  iio: accel: kxsd9: Deploy system and runtime PM
  iio: accel: kxsd9: Support reading a mounting matrix

 .../devicetree/bindings/iio/accel/kionix,kxsd9.txt |  22 ++
 drivers/iio/accel/Kconfig                          |  25 +-
 drivers/iio/accel/Makefile                         |   2 +
 drivers/iio/accel/kxsd9-i2c.c                      |  64 +++
 drivers/iio/accel/kxsd9-spi.c                      |  56 +++
 drivers/iio/accel/kxsd9.c                          | 438 ++++++++++++++++-----
 drivers/iio/accel/kxsd9.h                          |  12 +
 7 files changed, 522 insertions(+), 97 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/accel/kionix,kxsd9.txt
 create mode 100644 drivers/iio/accel/kxsd9-i2c.c
 create mode 100644 drivers/iio/accel/kxsd9-spi.c
 create mode 100644 drivers/iio/accel/kxsd9.h

-- 
2.7.4

[PATCH 01/17] iio: accel: kxsd9: Add device tree bindings

[Linus Walleij]

This accelerometer can be probed from the device tree, so it needs
to have proper documentation of it's device tree bindings.

Cc: devicetree@vger.kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 .../devicetree/bindings/iio/accel/kionix,kxsd9.txt | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/accel/kionix,kxsd9.txt

diff --git a/Documentation/devicetree/bindings/iio/accel/kionix,kxsd9.txt b/Documentation/devicetree/bindings/iio/accel/kionix,kxsd9.txt
new file mode 100644
index 000000000000..b25bf3a77e0f
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/accel/kionix,kxsd9.txt
@@ -0,0 +1,22 @@
+Kionix KXSD9 Accelerometer device tree bindings
+
+Required properties:
+ - compatible: 		should be set to "kionix,kxsd9"
+ - reg:			i2c slave address
+
+Optional properties:
+ - vdd-supply:		The input supply for VDD
+ - iovdd-supply:	The input supply for IOVDD
+ - interrupts:		The movement detection interrupt
+ - mount-matrix:	See mount-matrix.txt
+
+Example:
+
+kxsd9@18 {
+	compatible = "kionix,kxsd9";
+	reg = <0x18>;
+	interrupt-parent = <&foo>;
+	interrupts = <57 IRQ_TYPE_EDGE_FALLING>;
+	iovdd-supply = <&bar>;
+	vdd-supply = <&baz>;
+};
-- 
2.7.4

Re: [PATCH 01/17] iio: accel: kxsd9: Add device tree bindings

[Rob Herring]

On Tue, Aug 16, 2016 at 03:33:27PM +0200, Linus Walleij wrote:
> This accelerometer can be probed from the device tree, so it needs
> to have proper documentation of it's device tree bindings.
> 
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  .../devicetree/bindings/iio/accel/kionix,kxsd9.txt | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/accel/kionix,kxsd9.txt

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH 01/17] iio: accel: kxsd9: Add device tree bindings

[Jonathan Cameron]

On 18/08/16 20:21, Rob Herring wrote:
> On Tue, Aug 16, 2016 at 03:33:27PM +0200, Linus Walleij wrote:
>> This accelerometer can be probed from the device tree, so it needs
>> to have proper documentation of it's device tree bindings.
>>
>> Cc: devicetree@vger.kernel.org
>> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>> ---
>>  .../devicetree/bindings/iio/accel/kionix,kxsd9.txt | 22 ++++++++++++++++++++++
>>  1 file changed, 22 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/iio/accel/kionix,kxsd9.txt
> 
> Acked-by: Rob Herring <robh@kernel.org>
Applied to the togreg branch of iio.git - pushed out as
testing.  Thanks,

Jonathan
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[PATCH 02/17] iio: accel: kxsd9: Fix raw read return

[Linus Walleij]

Any readings from the raw interface of the KXSD9 driver will
return an empty string, because it does not return
IIO_VAL_INT but rather some random value from the accelerometer
to the caller.

Cc: stable@vger.kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/iio/accel/kxsd9.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/iio/accel/kxsd9.c b/drivers/iio/accel/kxsd9.c
index 3a9f106787d2..da5fb67ecb34 100644
--- a/drivers/iio/accel/kxsd9.c
+++ b/drivers/iio/accel/kxsd9.c
@@ -160,6 +160,7 @@ static int kxsd9_read_raw(struct iio_dev *indio_dev,
 		if (ret < 0)
 			goto error_ret;
 		*val = ret;
+		ret = IIO_VAL_INT;
 		break;
 	case IIO_CHAN_INFO_SCALE:
 		ret = spi_w8r8(st->us, KXSD9_READ(KXSD9_REG_CTRL_C));
-- 
2.7.4

Re: [PATCH 02/17] iio: accel: kxsd9: Fix raw read return

[Linus Walleij]

On Tue, Aug 16, 2016 at 6:24 PM, Jonathan Cameron <jic23@kernel.org> wrote:

> *whistles whilst pretending this driver has nothing to do with me*
>
> What's really odd is I ran this not long ago and didn't notice this little
> quirk...

Yeah I remember you mentioned it... there must be something that
made it work and tick anyways. Maybe core changes?

If you can get the SPI-based system up we can get the driver in
really nice shape though! It's working like a charm for me at the
end of the series.

Yours,
Linus Walleij

Re: [PATCH 02/17] iio: accel: kxsd9: Fix raw read return

[Jonathan Cameron]

On 17/08/16 08:15, Linus Walleij wrote:
> On Tue, Aug 16, 2016 at 6:24 PM, Jonathan Cameron <jic23@kernel.org> wrote:
> 
>> *whistles whilst pretending this driver has nothing to do with me*
>>
>> What's really odd is I ran this not long ago and didn't notice this little
>> quirk...
> 
> Yeah I remember you mentioned it... there must be something that
> made it work and tick anyways. Maybe core changes?
> 
> If you can get the SPI-based system up we can get the driver in
> really nice shape though! It's working like a charm for me at the
> end of the series.
> 
It may take me a while (couple of weeks possibly) to spin up that hardware
as I'm not quite on top of my back queue of patches.

I'll do a visual review in meantime but not apply anything until I can
test.

Jonathan
> Yours,
> Linus Walleij
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 02/17] iio: accel: kxsd9: Fix raw read return

[Jonathan Cameron]

On 16/08/16 14:33, Linus Walleij wrote:
> Any readings from the raw interface of the KXSD9 driver will
> return an empty string, because it does not return
> IIO_VAL_INT but rather some random value from the accelerometer
> to the caller.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Applied to the fixes-togreg branch of iio.git.

Thanks,

Jonathan
> ---
>  drivers/iio/accel/kxsd9.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/iio/accel/kxsd9.c b/drivers/iio/accel/kxsd9.c
> index 3a9f106787d2..da5fb67ecb34 100644
> --- a/drivers/iio/accel/kxsd9.c
> +++ b/drivers/iio/accel/kxsd9.c
> @@ -160,6 +160,7 @@ static int kxsd9_read_raw(struct iio_dev *indio_dev,
>  		if (ret < 0)
>  			goto error_ret;
>  		*val = ret;
> +		ret = IIO_VAL_INT;
>  		break;
>  	case IIO_CHAN_INFO_SCALE:
>  		ret = spi_w8r8(st->us, KXSD9_READ(KXSD9_REG_CTRL_C));
> 

[PATCH 03/17] iio: accel: kxsd9: Split out transport mechanism

[Linus Walleij]

Split off a transport mechanism struct that will deal with the SPI
traffic in preparation for adding I2C support.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/iio/accel/kxsd9.c | 172 ++++++++++++++++++++++++++++++++--------------
 1 file changed, 122 insertions(+), 50 deletions(-)

diff --git a/drivers/iio/accel/kxsd9.c b/drivers/iio/accel/kxsd9.c
index da5fb67ecb34..094071306e42 100644
--- a/drivers/iio/accel/kxsd9.c
+++ b/drivers/iio/accel/kxsd9.c
@@ -43,6 +43,27 @@
 
 #define KXSD9_STATE_RX_SIZE 2
 #define KXSD9_STATE_TX_SIZE 2
+
+struct kxsd9_transport;
+
+/**
+ * struct kxsd9_transport - transport adapter for SPI or I2C
+ * @trdev: transport device such as SPI or I2C
+ * @write1(): function to write a byte to the device
+ * @write2(): function to write two consecutive bytes to the device
+ * @readval(): function to read a 16bit value from the device
+ * @rx: cache aligned read buffer
+ * @tx: cache aligned write buffer
+ */
+struct kxsd9_transport {
+	void *trdev;
+	int (*write1) (struct kxsd9_transport *tr, u8 byte);
+	int (*write2) (struct kxsd9_transport *tr, u8 b1, u8 b2);
+	int (*readval) (struct kxsd9_transport *tr, u8 address);
+	u8 rx[KXSD9_STATE_RX_SIZE] ____cacheline_aligned;
+	u8 tx[KXSD9_STATE_TX_SIZE];
+};
+
 /**
  * struct kxsd9_state - device related storage
  * @buf_lock:	protect the rx and tx buffers.
@@ -51,10 +72,8 @@
  * @tx:		single tx buffer storage
  **/
 struct kxsd9_state {
+	struct kxsd9_transport *transport;
 	struct mutex buf_lock;
-	struct spi_device *us;
-	u8 rx[KXSD9_STATE_RX_SIZE] ____cacheline_aligned;
-	u8 tx[KXSD9_STATE_TX_SIZE];
 };
 
 #define KXSD9_SCALE_2G "0.011978"
@@ -80,13 +99,12 @@ static int kxsd9_write_scale(struct iio_dev *indio_dev, int micro)
 		return -EINVAL;
 
 	mutex_lock(&st->buf_lock);
-	ret = spi_w8r8(st->us, KXSD9_READ(KXSD9_REG_CTRL_C));
-	if (ret < 0)
+	ret = st->transport->write1(st->transport, KXSD9_READ(KXSD9_REG_CTRL_C));
+	if (ret)
 		goto error_ret;
-	st->tx[0] = KXSD9_WRITE(KXSD9_REG_CTRL_C);
-	st->tx[1] = (ret & ~KXSD9_FS_MASK) | i;
-
-	ret = spi_write(st->us, st->tx, 2);
+	ret = st->transport->write2(st->transport,
+				    KXSD9_WRITE(KXSD9_REG_CTRL_C),
+				    (ret & ~KXSD9_FS_MASK) | i);
 error_ret:
 	mutex_unlock(&st->buf_lock);
 	return ret;
@@ -96,24 +114,9 @@ static int kxsd9_read(struct iio_dev *indio_dev, u8 address)
 {
 	int ret;
 	struct kxsd9_state *st = iio_priv(indio_dev);
-	struct spi_transfer xfers[] = {
-		{
-			.bits_per_word = 8,
-			.len = 1,
-			.delay_usecs = 200,
-			.tx_buf = st->tx,
-		}, {
-			.bits_per_word = 8,
-			.len = 2,
-			.rx_buf = st->rx,
-		},
-	};
 
 	mutex_lock(&st->buf_lock);
-	st->tx[0] = KXSD9_READ(address);
-	ret = spi_sync_transfer(st->us, xfers, ARRAY_SIZE(xfers));
-	if (!ret)
-		ret = (((u16)(st->rx[0])) << 8) | (st->rx[1] & 0xF0);
+	ret = st->transport->readval(st->transport, KXSD9_READ(address));
 	mutex_unlock(&st->buf_lock);
 	return ret;
 }
@@ -163,8 +166,8 @@ static int kxsd9_read_raw(struct iio_dev *indio_dev,
 		ret = IIO_VAL_INT;
 		break;
 	case IIO_CHAN_INFO_SCALE:
-		ret = spi_w8r8(st->us, KXSD9_READ(KXSD9_REG_CTRL_C));
-		if (ret < 0)
+		ret = st->transport->write1(st->transport, KXSD9_READ(KXSD9_REG_CTRL_C));
+		if (ret)
 			goto error_ret;
 		*val2 = kxsd9_micro_scales[ret & KXSD9_FS_MASK];
 		ret = IIO_VAL_INT_PLUS_MICRO;
@@ -202,15 +205,10 @@ static int kxsd9_power_up(struct kxsd9_state *st)
 {
 	int ret;
 
-	st->tx[0] = 0x0d;
-	st->tx[1] = 0x40;
-	ret = spi_write(st->us, st->tx, 2);
+	ret = st->transport->write2(st->transport, 0x0d, 0x40);
 	if (ret)
 		return ret;
-
-	st->tx[0] = 0x0c;
-	st->tx[1] = 0x9b;
-	return spi_write(st->us, st->tx, 2);
+	return st->transport->write2(st->transport, 0x0c, 0x9b);
 };
 
 static const struct iio_info kxsd9_info = {
@@ -220,56 +218,130 @@ static const struct iio_info kxsd9_info = {
 	.driver_module = THIS_MODULE,
 };
 
-static int kxsd9_probe(struct spi_device *spi)
+static int kxsd9_common_probe(struct device *parent,
+			      struct kxsd9_transport *transport,
+			      const char *name,
+			      struct iio_dev **retdev)
 {
 	struct iio_dev *indio_dev;
 	struct kxsd9_state *st;
+	int ret;
 
-	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+	indio_dev = devm_iio_device_alloc(parent, sizeof(*st));
 	if (!indio_dev)
 		return -ENOMEM;
 
 	st = iio_priv(indio_dev);
-	spi_set_drvdata(spi, indio_dev);
+	st->transport = transport;
 
-	st->us = spi;
 	mutex_init(&st->buf_lock);
 	indio_dev->channels = kxsd9_channels;
 	indio_dev->num_channels = ARRAY_SIZE(kxsd9_channels);
-	indio_dev->name = spi_get_device_id(spi)->name;
-	indio_dev->dev.parent = &spi->dev;
+	indio_dev->name = name;
+	indio_dev->dev.parent = parent;
 	indio_dev->info = &kxsd9_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 
+	kxsd9_power_up(st);
+
+	ret = iio_device_register(indio_dev);
+	if (ret)
+		return ret;
+
+	*retdev = indio_dev;
+	return 0;
+}
+
+int kxsd9_spi_write1(struct kxsd9_transport *tr, u8 byte)
+{
+	struct spi_device *spi = tr->trdev;
+
+	return spi_w8r8(spi, byte);
+}
+
+int kxsd9_spi_write2(struct kxsd9_transport *tr, u8 b1, u8 b2)
+{
+	struct spi_device *spi = tr->trdev;
+
+	tr->tx[0] = b1;
+	tr->tx[1] = b2;
+	return spi_write(spi, tr->tx, 2);
+}
+
+int kxsd9_spi_readval(struct kxsd9_transport *tr, u8 address)
+{
+	struct spi_device *spi = tr->trdev;
+	struct spi_transfer xfers[] = {
+		{
+			.bits_per_word = 8,
+			.len = 1,
+			.delay_usecs = 200,
+			.tx_buf = tr->tx,
+		}, {
+			.bits_per_word = 8,
+			.len = 2,
+			.rx_buf = tr->rx,
+		},
+	};
+	int ret;
+
+	tr->tx[0] = address;
+	ret = spi_sync_transfer(spi, xfers, ARRAY_SIZE(xfers));
+	if (!ret)
+		ret = (((u16)(tr->rx[0])) << 8) | (tr->rx[1] & 0xF0);
+	return ret;
+}
+
+static int kxsd9_spi_probe(struct spi_device *spi)
+{
+	struct kxsd9_transport *transport;
+	struct iio_dev *indio_dev;
+	int ret;
+
+	transport = devm_kzalloc(&spi->dev, sizeof(*transport), GFP_KERNEL);
+	if (!transport)
+		return -ENOMEM;
+
+	transport->trdev = spi;
+	transport->write1 = kxsd9_spi_write1;
+	transport->write2 = kxsd9_spi_write2;
+	transport->readval = kxsd9_spi_readval;
 	spi->mode = SPI_MODE_0;
 	spi_setup(spi);
-	kxsd9_power_up(st);
 
-	return iio_device_register(indio_dev);
+	ret = kxsd9_common_probe(&spi->dev,
+				 transport,
+				 spi_get_device_id(spi)->name,
+				 &indio_dev);
+	if (ret)
+		return ret;
+
+	spi_set_drvdata(spi, indio_dev);
+	return 0;
 }
 
-static int kxsd9_remove(struct spi_device *spi)
+static int kxsd9_spi_remove(struct spi_device *spi)
 {
 	iio_device_unregister(spi_get_drvdata(spi));
 
 	return 0;
 }
 
-static const struct spi_device_id kxsd9_id[] = {
+static const struct spi_device_id kxsd9_spi_id[] = {
 	{"kxsd9", 0},
 	{ },
 };
-MODULE_DEVICE_TABLE(spi, kxsd9_id);
+MODULE_DEVICE_TABLE(spi, kxsd9_spi_id);
 
-static struct spi_driver kxsd9_driver = {
+static struct spi_driver kxsd9_spi_driver = {
 	.driver = {
 		.name = "kxsd9",
 	},
-	.probe = kxsd9_probe,
-	.remove = kxsd9_remove,
-	.id_table = kxsd9_id,
+	.probe = kxsd9_spi_probe,
+	.remove = kxsd9_spi_remove,
+	.id_table = kxsd9_spi_id,
 };
-module_spi_driver(kxsd9_driver);
+module_spi_driver(kxsd9_spi_driver);
 
 MODULE_AUTHOR("Jonathan Cameron <jic23@kernel.org>");
 MODULE_DESCRIPTION("Kionix KXSD9 SPI driver");
-- 
2.7.4

Re: [PATCH 03/17] iio: accel: kxsd9: Split out transport mechanism

[Peter Meerwald-Stadler]

> Split off a transport mechanism struct that will deal with the SPI
> traffic in preparation for adding I2C support.

comment below
 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/iio/accel/kxsd9.c | 172 ++++++++++++++++++++++++++++++++--------------
>  1 file changed, 122 insertions(+), 50 deletions(-)
> 
> diff --git a/drivers/iio/accel/kxsd9.c b/drivers/iio/accel/kxsd9.c
> index da5fb67ecb34..094071306e42 100644
> --- a/drivers/iio/accel/kxsd9.c
> +++ b/drivers/iio/accel/kxsd9.c
> @@ -43,6 +43,27 @@
>  
>  #define KXSD9_STATE_RX_SIZE 2
>  #define KXSD9_STATE_TX_SIZE 2
> +
> +struct kxsd9_transport;
> +
> +/**
> + * struct kxsd9_transport - transport adapter for SPI or I2C
> + * @trdev: transport device such as SPI or I2C
> + * @write1(): function to write a byte to the device
> + * @write2(): function to write two consecutive bytes to the device
> + * @readval(): function to read a 16bit value from the device
> + * @rx: cache aligned read buffer
> + * @tx: cache aligned write buffer

tx doesn't seem to be cachline_aligned?

> + */
> +struct kxsd9_transport {
> +	void *trdev;
> +	int (*write1) (struct kxsd9_transport *tr, u8 byte);
> +	int (*write2) (struct kxsd9_transport *tr, u8 b1, u8 b2);
> +	int (*readval) (struct kxsd9_transport *tr, u8 address);
> +	u8 rx[KXSD9_STATE_RX_SIZE] ____cacheline_aligned;
> +	u8 tx[KXSD9_STATE_TX_SIZE];
> +};
> +
>  /**
>   * struct kxsd9_state - device related storage

should add transport and remove us, rx, tx

>   * @buf_lock:	protect the rx and tx buffers.
> @@ -51,10 +72,8 @@
>   * @tx:		single tx buffer storage
>   **/
>  struct kxsd9_state {
> +	struct kxsd9_transport *transport;
>  	struct mutex buf_lock;
> -	struct spi_device *us;
> -	u8 rx[KXSD9_STATE_RX_SIZE] ____cacheline_aligned;
> -	u8 tx[KXSD9_STATE_TX_SIZE];
>  };
>  
>  #define KXSD9_SCALE_2G "0.011978"
> @@ -80,13 +99,12 @@ static int kxsd9_write_scale(struct iio_dev *indio_dev, int micro)
>  		return -EINVAL;
>  
>  	mutex_lock(&st->buf_lock);
> -	ret = spi_w8r8(st->us, KXSD9_READ(KXSD9_REG_CTRL_C));
> -	if (ret < 0)
> +	ret = st->transport->write1(st->transport, KXSD9_READ(KXSD9_REG_CTRL_C));
> +	if (ret)
>  		goto error_ret;
> -	st->tx[0] = KXSD9_WRITE(KXSD9_REG_CTRL_C);
> -	st->tx[1] = (ret & ~KXSD9_FS_MASK) | i;
> -
> -	ret = spi_write(st->us, st->tx, 2);
> +	ret = st->transport->write2(st->transport,
> +				    KXSD9_WRITE(KXSD9_REG_CTRL_C),
> +				    (ret & ~KXSD9_FS_MASK) | i);
>  error_ret:
>  	mutex_unlock(&st->buf_lock);
>  	return ret;
> @@ -96,24 +114,9 @@ static int kxsd9_read(struct iio_dev *indio_dev, u8 address)
>  {
>  	int ret;
>  	struct kxsd9_state *st = iio_priv(indio_dev);
> -	struct spi_transfer xfers[] = {
> -		{
> -			.bits_per_word = 8,
> -			.len = 1,
> -			.delay_usecs = 200,
> -			.tx_buf = st->tx,
> -		}, {
> -			.bits_per_word = 8,
> -			.len = 2,
> -			.rx_buf = st->rx,
> -		},
> -	};
>  
>  	mutex_lock(&st->buf_lock);
> -	st->tx[0] = KXSD9_READ(address);
> -	ret = spi_sync_transfer(st->us, xfers, ARRAY_SIZE(xfers));
> -	if (!ret)
> -		ret = (((u16)(st->rx[0])) << 8) | (st->rx[1] & 0xF0);
> +	ret = st->transport->readval(st->transport, KXSD9_READ(address));
>  	mutex_unlock(&st->buf_lock);
>  	return ret;
>  }
> @@ -163,8 +166,8 @@ static int kxsd9_read_raw(struct iio_dev *indio_dev,
>  		ret = IIO_VAL_INT;
>  		break;
>  	case IIO_CHAN_INFO_SCALE:
> -		ret = spi_w8r8(st->us, KXSD9_READ(KXSD9_REG_CTRL_C));
> -		if (ret < 0)
> +		ret = st->transport->write1(st->transport, KXSD9_READ(KXSD9_REG_CTRL_C));
> +		if (ret)
>  			goto error_ret;
>  		*val2 = kxsd9_micro_scales[ret & KXSD9_FS_MASK];
>  		ret = IIO_VAL_INT_PLUS_MICRO;
> @@ -202,15 +205,10 @@ static int kxsd9_power_up(struct kxsd9_state *st)
>  {
>  	int ret;
>  
> -	st->tx[0] = 0x0d;
> -	st->tx[1] = 0x40;
> -	ret = spi_write(st->us, st->tx, 2);
> +	ret = st->transport->write2(st->transport, 0x0d, 0x40);
>  	if (ret)
>  		return ret;
> -
> -	st->tx[0] = 0x0c;
> -	st->tx[1] = 0x9b;
> -	return spi_write(st->us, st->tx, 2);
> +	return st->transport->write2(st->transport, 0x0c, 0x9b);
>  };
>  
>  static const struct iio_info kxsd9_info = {
> @@ -220,56 +218,130 @@ static const struct iio_info kxsd9_info = {
>  	.driver_module = THIS_MODULE,
>  };
>  
> -static int kxsd9_probe(struct spi_device *spi)
> +static int kxsd9_common_probe(struct device *parent,
> +			      struct kxsd9_transport *transport,
> +			      const char *name,
> +			      struct iio_dev **retdev)
>  {
>  	struct iio_dev *indio_dev;
>  	struct kxsd9_state *st;
> +	int ret;
>  
> -	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> +	indio_dev = devm_iio_device_alloc(parent, sizeof(*st));
>  	if (!indio_dev)
>  		return -ENOMEM;
>  
>  	st = iio_priv(indio_dev);
> -	spi_set_drvdata(spi, indio_dev);
> +	st->transport = transport;
>  
> -	st->us = spi;
>  	mutex_init(&st->buf_lock);
>  	indio_dev->channels = kxsd9_channels;
>  	indio_dev->num_channels = ARRAY_SIZE(kxsd9_channels);
> -	indio_dev->name = spi_get_device_id(spi)->name;
> -	indio_dev->dev.parent = &spi->dev;
> +	indio_dev->name = name;
> +	indio_dev->dev.parent = parent;
>  	indio_dev->info = &kxsd9_info;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
>  
> +	kxsd9_power_up(st);
> +
> +	ret = iio_device_register(indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	*retdev = indio_dev;
> +	return 0;
> +}
> +
> +int kxsd9_spi_write1(struct kxsd9_transport *tr, u8 byte)
> +{
> +	struct spi_device *spi = tr->trdev;
> +
> +	return spi_w8r8(spi, byte);
> +}
> +
> +int kxsd9_spi_write2(struct kxsd9_transport *tr, u8 b1, u8 b2)
> +{
> +	struct spi_device *spi = tr->trdev;
> +
> +	tr->tx[0] = b1;
> +	tr->tx[1] = b2;
> +	return spi_write(spi, tr->tx, 2);
> +}
> +
> +int kxsd9_spi_readval(struct kxsd9_transport *tr, u8 address)
> +{
> +	struct spi_device *spi = tr->trdev;
> +	struct spi_transfer xfers[] = {
> +		{
> +			.bits_per_word = 8,
> +			.len = 1,
> +			.delay_usecs = 200,
> +			.tx_buf = tr->tx,
> +		}, {
> +			.bits_per_word = 8,
> +			.len = 2,
> +			.rx_buf = tr->rx,
> +		},
> +	};
> +	int ret;
> +
> +	tr->tx[0] = address;
> +	ret = spi_sync_transfer(spi, xfers, ARRAY_SIZE(xfers));
> +	if (!ret)
> +		ret = (((u16)(tr->rx[0])) << 8) | (tr->rx[1] & 0xF0);
> +	return ret;
> +}
> +
> +static int kxsd9_spi_probe(struct spi_device *spi)
> +{
> +	struct kxsd9_transport *transport;
> +	struct iio_dev *indio_dev;
> +	int ret;
> +
> +	transport = devm_kzalloc(&spi->dev, sizeof(*transport), GFP_KERNEL);
> +	if (!transport)
> +		return -ENOMEM;
> +
> +	transport->trdev = spi;
> +	transport->write1 = kxsd9_spi_write1;
> +	transport->write2 = kxsd9_spi_write2;
> +	transport->readval = kxsd9_spi_readval;
>  	spi->mode = SPI_MODE_0;
>  	spi_setup(spi);
> -	kxsd9_power_up(st);
>  
> -	return iio_device_register(indio_dev);
> +	ret = kxsd9_common_probe(&spi->dev,
> +				 transport,
> +				 spi_get_device_id(spi)->name,
> +				 &indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	spi_set_drvdata(spi, indio_dev);
> +	return 0;
>  }
>  
> -static int kxsd9_remove(struct spi_device *spi)
> +static int kxsd9_spi_remove(struct spi_device *spi)
>  {
>  	iio_device_unregister(spi_get_drvdata(spi));
>  
>  	return 0;
>  }
>  
> -static const struct spi_device_id kxsd9_id[] = {
> +static const struct spi_device_id kxsd9_spi_id[] = {
>  	{"kxsd9", 0},
>  	{ },
>  };
> -MODULE_DEVICE_TABLE(spi, kxsd9_id);
> +MODULE_DEVICE_TABLE(spi, kxsd9_spi_id);
>  
> -static struct spi_driver kxsd9_driver = {
> +static struct spi_driver kxsd9_spi_driver = {
>  	.driver = {
>  		.name = "kxsd9",
>  	},
> -	.probe = kxsd9_probe,
> -	.remove = kxsd9_remove,
> -	.id_table = kxsd9_id,
> +	.probe = kxsd9_spi_probe,
> +	.remove = kxsd9_spi_remove,
> +	.id_table = kxsd9_spi_id,
>  };
> -module_spi_driver(kxsd9_driver);
> +module_spi_driver(kxsd9_spi_driver);
>  
>  MODULE_AUTHOR("Jonathan Cameron <jic23@kernel.org>");
>  MODULE_DESCRIPTION("Kionix KXSD9 SPI driver");
> 

-- 

Peter Meerwald-Stadler
+43-664-2444418 (mobile)

Re: [PATCH 03/17] iio: accel: kxsd9: Split out transport mechanism

[Linus Walleij]

On Tue, Aug 16, 2016 at 3:53 PM, Peter Meerwald-Stadler
<pmeerw@pmeerw.net> wrote:

>> +/**
>> + * struct kxsd9_transport - transport adapter for SPI or I2C
>> + * @trdev: transport device such as SPI or I2C
>> + * @write1(): function to write a byte to the device
>> + * @write2(): function to write two consecutive bytes to the device
>> + * @readval(): function to read a 16bit value from the device
>> + * @rx: cache aligned read buffer
>> + * @tx: cache aligned write buffer
>
> tx doesn't seem to be cachline_aligned?

Haha no, comment carried over when moving the code.
I remove it later in the patch series.

>> +     void *trdev;
>> +     int (*write1) (struct kxsd9_transport *tr, u8 byte);
>> +     int (*write2) (struct kxsd9_transport *tr, u8 b1, u8 b2);
>> +     int (*readval) (struct kxsd9_transport *tr, u8 address);
>> +     u8 rx[KXSD9_STATE_RX_SIZE] ____cacheline_aligned;
>> +     u8 tx[KXSD9_STATE_TX_SIZE];
>> +};
>> +
>>  /**
>>   * struct kxsd9_state - device related storage
>
> should add transport and remove us, rx, tx

Yeah. I remove  it later in the patch series when switching the
transport to regmap (IIRC) and this doesn't really cause any issues
to have left until that point.

But if you think it's worth it, I can respin it to make a cleaner
patch.

Yours,
Linus Walleij

Re: [PATCH 03/17] iio: accel: kxsd9: Split out transport mechanism

[Jonathan Cameron]

On 17/08/16 08:18, Linus Walleij wrote:
> On Tue, Aug 16, 2016 at 3:53 PM, Peter Meerwald-Stadler
> <pmeerw@pmeerw.net> wrote:
> 
>>> +/**
>>> + * struct kxsd9_transport - transport adapter for SPI or I2C
>>> + * @trdev: transport device such as SPI or I2C
>>> + * @write1(): function to write a byte to the device
>>> + * @write2(): function to write two consecutive bytes to the device
>>> + * @readval(): function to read a 16bit value from the device
>>> + * @rx: cache aligned read buffer
>>> + * @tx: cache aligned write buffer
>>
>> tx doesn't seem to be cachline_aligned?
> 
> Haha no, comment carried over when moving the code.
> I remove it later in the patch series.
> 
>>> +     void *trdev;
>>> +     int (*write1) (struct kxsd9_transport *tr, u8 byte);
>>> +     int (*write2) (struct kxsd9_transport *tr, u8 b1, u8 b2);
>>> +     int (*readval) (struct kxsd9_transport *tr, u8 address);
>>> +     u8 rx[KXSD9_STATE_RX_SIZE] ____cacheline_aligned;
>>> +     u8 tx[KXSD9_STATE_TX_SIZE];
>>> +};
>>> +
>>>  /**
>>>   * struct kxsd9_state - device related storage
>>
>> should add transport and remove us, rx, tx
> 
> Yeah. I remove  it later in the patch series when switching the
> transport to regmap (IIRC) and this doesn't really cause any issues
> to have left until that point.
> 
> But if you think it's worth it, I can respin it to make a cleaner
> patch.
If we are being fussy:
drivers/iio/accel/kxsd9.c:255:5: warning: symbol 'kxsd9_spi_write1' was not declared. Should it be static?
drivers/iio/accel/kxsd9.c:262:5: warning: symbol 'kxsd9_spi_write2' was not declared. Should it be static?
drivers/iio/accel/kxsd9.c:271:5: warning: symbol 'kxsd9_spi_readval' was not declared. Should it be static?

On the plus side:
Tested-by: Jonathan Cameron <jic23@kernel.org>

Amazing how many ways one can miss specify how a device is connected to an spi bus..
Took me rather longer to get this working that it should have done!

Jonathan
> 
> Yours,
> Linus Walleij
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[PATCH 04/17] iio: accel: kxsd9: Use devm_iio_device_register()

[Linus Walleij]

This gives us less to keep track of when removing the driver
and simplifies the code.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/iio/accel/kxsd9.c | 19 +++----------------
 1 file changed, 3 insertions(+), 16 deletions(-)

diff --git a/drivers/iio/accel/kxsd9.c b/drivers/iio/accel/kxsd9.c
index 094071306e42..d3e87eef2594 100644
--- a/drivers/iio/accel/kxsd9.c
+++ b/drivers/iio/accel/kxsd9.c
@@ -220,8 +220,7 @@ static const struct iio_info kxsd9_info = {
 
 static int kxsd9_common_probe(struct device *parent,
 			      struct kxsd9_transport *transport,
-			      const char *name,
-			      struct iio_dev **retdev)
+			      const char *name)
 {
 	struct iio_dev *indio_dev;
 	struct kxsd9_state *st;
@@ -244,11 +243,10 @@ static int kxsd9_common_probe(struct device *parent,
 
 	kxsd9_power_up(st);
 
-	ret = iio_device_register(indio_dev);
+	ret = devm_iio_device_register(parent, indio_dev);
 	if (ret)
 		return ret;
 
-	*retdev = indio_dev;
 	return 0;
 }
 
@@ -295,7 +293,6 @@ int kxsd9_spi_readval(struct kxsd9_transport *tr, u8 address)
 static int kxsd9_spi_probe(struct spi_device *spi)
 {
 	struct kxsd9_transport *transport;
-	struct iio_dev *indio_dev;
 	int ret;
 
 	transport = devm_kzalloc(&spi->dev, sizeof(*transport), GFP_KERNEL);
@@ -311,19 +308,10 @@ static int kxsd9_spi_probe(struct spi_device *spi)
 
 	ret = kxsd9_common_probe(&spi->dev,
 				 transport,
-				 spi_get_device_id(spi)->name,
-				 &indio_dev);
+				 spi_get_device_id(spi)->name);
 	if (ret)
 		return ret;
 
-	spi_set_drvdata(spi, indio_dev);
-	return 0;
-}
-
-static int kxsd9_spi_remove(struct spi_device *spi)
-{
-	iio_device_unregister(spi_get_drvdata(spi));
-
 	return 0;
 }
 
@@ -338,7 +326,6 @@ static struct spi_driver kxsd9_spi_driver = {
 		.name = "kxsd9",
 	},
 	.probe = kxsd9_spi_probe,
-	.remove = kxsd9_spi_remove,
 	.id_table = kxsd9_spi_id,
 };
 module_spi_driver(kxsd9_spi_driver);
-- 
2.7.4

[PATCH 05/17] iio: accel: kxsd9: Split out SPI transport

[Linus Walleij]

This moves the KXSD9 SPI transport out to its own file and Kconfig
entry, so that we will be able to add another transport method.
We export the common probe and add a local header file for the
functionality shared between the main driver and the transport
driver.

We make the SPI transport the default for the driver if SPI is
available and the KXSD9 driver was selected, so the oldconfig
upgrade path will be clear.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/iio/accel/Kconfig     |  10 ++-
 drivers/iio/accel/Makefile    |   1 +
 drivers/iio/accel/kxsd9-spi.c | 104 +++++++++++++++++++++++++++++++
 drivers/iio/accel/kxsd9.c     | 140 ++++++------------------------------------
 drivers/iio/accel/kxsd9.h     |  31 ++++++++++
 5 files changed, 163 insertions(+), 123 deletions(-)
 create mode 100644 drivers/iio/accel/kxsd9-spi.c
 create mode 100644 drivers/iio/accel/kxsd9.h

diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index 89d78208de3f..95e3fc09f640 100644
--- a/drivers/iio/accel/Kconfig
+++ b/drivers/iio/accel/Kconfig
@@ -96,7 +96,6 @@ config IIO_ST_ACCEL_SPI_3AXIS
 
 config KXSD9
 	tristate "Kionix KXSD9 Accelerometer Driver"
-	depends on SPI
 	help
 	  Say yes here to build support for the Kionix KXSD9 accelerometer.
 	  Currently this only supports the device via an SPI interface.
@@ -104,6 +103,15 @@ config KXSD9
 	  To compile this driver as a module, choose M here: the module
 	  will be called kxsd9.
 
+config KXSD9_SPI
+	tristate "Kionix KXSD9 SPI transport"
+	depends on KXSD9
+	depends on SPI
+	default KXSD9
+	help
+	  Say yes here to enable the Kionix KXSD9 accelerometer
+	  SPI transport channel.
+
 config KXCJK1013
 	tristate "Kionix 3-Axis Accelerometer Driver"
 	depends on I2C
diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
index 6cedbecca2ee..22a5770f62a9 100644
--- a/drivers/iio/accel/Makefile
+++ b/drivers/iio/accel/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_BMC150_ACCEL_SPI) += bmc150-accel-spi.o
 obj-$(CONFIG_HID_SENSOR_ACCEL_3D) += hid-sensor-accel-3d.o
 obj-$(CONFIG_KXCJK1013) += kxcjk-1013.o
 obj-$(CONFIG_KXSD9)	+= kxsd9.o
+obj-$(CONFIG_KXSD9_SPI)	+= kxsd9-spi.o
 
 obj-$(CONFIG_MMA7455)		+= mma7455_core.o
 obj-$(CONFIG_MMA7455_I2C)	+= mma7455_i2c.o
diff --git a/drivers/iio/accel/kxsd9-spi.c b/drivers/iio/accel/kxsd9-spi.c
new file mode 100644
index 000000000000..0789695c89e1
--- /dev/null
+++ b/drivers/iio/accel/kxsd9-spi.c
@@ -0,0 +1,104 @@
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/spi/spi.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+
+#include "kxsd9.h"
+
+#define KXSD9_READ(a) (0x80 | (a))
+#define KXSD9_WRITE(a) (a)
+
+static int kxsd9_spi_readreg(struct kxsd9_transport *tr, u8 address)
+{
+	struct spi_device *spi = tr->trdev;
+
+	return spi_w8r8(spi, KXSD9_READ(address));
+}
+
+static int kxsd9_spi_writereg(struct kxsd9_transport *tr, u8 address, u8 val)
+{
+	struct spi_device *spi = tr->trdev;
+
+	tr->tx[0] = KXSD9_WRITE(address),
+	tr->tx[1] = val;
+	return spi_write(spi, tr->tx, 2);
+}
+
+static int kxsd9_spi_write2(struct kxsd9_transport *tr, u8 b1, u8 b2)
+{
+	struct spi_device *spi = tr->trdev;
+
+	tr->tx[0] = b1;
+	tr->tx[1] = b2;
+	return spi_write(spi, tr->tx, 2);
+}
+
+static int kxsd9_spi_readval(struct kxsd9_transport *tr, u8 address)
+{
+	struct spi_device *spi = tr->trdev;
+	struct spi_transfer xfers[] = {
+		{
+			.bits_per_word = 8,
+			.len = 1,
+			.delay_usecs = 200,
+			.tx_buf = tr->tx,
+		}, {
+			.bits_per_word = 8,
+			.len = 2,
+			.rx_buf = tr->rx,
+		},
+	};
+	int ret;
+
+	tr->tx[0] = KXSD9_READ(address);
+	ret = spi_sync_transfer(spi, xfers, ARRAY_SIZE(xfers));
+	if (!ret)
+		ret = (((u16)(tr->rx[0])) << 8) | (tr->rx[1]);
+	return ret;
+}
+
+static int kxsd9_spi_probe(struct spi_device *spi)
+{
+	struct kxsd9_transport *transport;
+	int ret;
+
+	transport = devm_kzalloc(&spi->dev, sizeof(*transport), GFP_KERNEL);
+	if (!transport)
+		return -ENOMEM;
+
+	transport->trdev = spi;
+	transport->readreg = kxsd9_spi_readreg;
+	transport->writereg = kxsd9_spi_writereg;
+	transport->write2 = kxsd9_spi_write2;
+	transport->readval = kxsd9_spi_readval;
+	spi->mode = SPI_MODE_0;
+	spi_setup(spi);
+
+	ret = kxsd9_common_probe(&spi->dev,
+				 transport,
+				 spi_get_device_id(spi)->name);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static const struct spi_device_id kxsd9_spi_id[] = {
+	{"kxsd9", 0},
+	{ },
+};
+MODULE_DEVICE_TABLE(spi, kxsd9_spi_id);
+
+static struct spi_driver kxsd9_spi_driver = {
+	.driver = {
+		.name = "kxsd9",
+	},
+	.probe = kxsd9_spi_probe,
+	.id_table = kxsd9_spi_id,
+};
+module_spi_driver(kxsd9_spi_driver);
+
+MODULE_AUTHOR("Jonathan Cameron <jic23@kernel.org>");
+MODULE_DESCRIPTION("Kionix KXSD9 SPI driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/iio/accel/kxsd9.c b/drivers/iio/accel/kxsd9.c
index d3e87eef2594..efa9ba665f6e 100644
--- a/drivers/iio/accel/kxsd9.c
+++ b/drivers/iio/accel/kxsd9.c
@@ -18,7 +18,6 @@
 
 #include <linux/device.h>
 #include <linux/kernel.h>
-#include <linux/spi/spi.h>
 #include <linux/sysfs.h>
 #include <linux/slab.h>
 #include <linux/module.h>
@@ -26,6 +25,8 @@
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
 
+#include "kxsd9.h"
+
 #define KXSD9_REG_X		0x00
 #define KXSD9_REG_Y		0x02
 #define KXSD9_REG_Z		0x04
@@ -38,32 +39,6 @@
 #define KXSD9_REG_CTRL_B	0x0d
 #define KXSD9_REG_CTRL_A	0x0e
 
-#define KXSD9_READ(a) (0x80 | (a))
-#define KXSD9_WRITE(a) (a)
-
-#define KXSD9_STATE_RX_SIZE 2
-#define KXSD9_STATE_TX_SIZE 2
-
-struct kxsd9_transport;
-
-/**
- * struct kxsd9_transport - transport adapter for SPI or I2C
- * @trdev: transport device such as SPI or I2C
- * @write1(): function to write a byte to the device
- * @write2(): function to write two consecutive bytes to the device
- * @readval(): function to read a 16bit value from the device
- * @rx: cache aligned read buffer
- * @tx: cache aligned write buffer
- */
-struct kxsd9_transport {
-	void *trdev;
-	int (*write1) (struct kxsd9_transport *tr, u8 byte);
-	int (*write2) (struct kxsd9_transport *tr, u8 b1, u8 b2);
-	int (*readval) (struct kxsd9_transport *tr, u8 address);
-	u8 rx[KXSD9_STATE_RX_SIZE] ____cacheline_aligned;
-	u8 tx[KXSD9_STATE_TX_SIZE];
-};
-
 /**
  * struct kxsd9_state - device related storage
  * @buf_lock:	protect the rx and tx buffers.
@@ -99,12 +74,13 @@ static int kxsd9_write_scale(struct iio_dev *indio_dev, int micro)
 		return -EINVAL;
 
 	mutex_lock(&st->buf_lock);
-	ret = st->transport->write1(st->transport, KXSD9_READ(KXSD9_REG_CTRL_C));
-	if (ret)
+	ret = st->transport->readreg(st->transport,
+				     KXSD9_REG_CTRL_C);
+	if (ret < 0)
 		goto error_ret;
-	ret = st->transport->write2(st->transport,
-				    KXSD9_WRITE(KXSD9_REG_CTRL_C),
-				    (ret & ~KXSD9_FS_MASK) | i);
+	ret = st->transport->writereg(st->transport,
+				      KXSD9_REG_CTRL_C,
+				      (ret & ~KXSD9_FS_MASK) | i);
 error_ret:
 	mutex_unlock(&st->buf_lock);
 	return ret;
@@ -116,7 +92,9 @@ static int kxsd9_read(struct iio_dev *indio_dev, u8 address)
 	struct kxsd9_state *st = iio_priv(indio_dev);
 
 	mutex_lock(&st->buf_lock);
-	ret = st->transport->readval(st->transport, KXSD9_READ(address));
+	ret = st->transport->readval(st->transport, address);
+	/* Only 12 bits are valid */
+	ret &= 0xfff0;
 	mutex_unlock(&st->buf_lock);
 	return ret;
 }
@@ -166,8 +144,9 @@ static int kxsd9_read_raw(struct iio_dev *indio_dev,
 		ret = IIO_VAL_INT;
 		break;
 	case IIO_CHAN_INFO_SCALE:
-		ret = st->transport->write1(st->transport, KXSD9_READ(KXSD9_REG_CTRL_C));
-		if (ret)
+		ret = st->transport->readreg(st->transport,
+					     KXSD9_REG_CTRL_C);
+		if (ret < 0)
 			goto error_ret;
 		*val2 = kxsd9_micro_scales[ret & KXSD9_FS_MASK];
 		ret = IIO_VAL_INT_PLUS_MICRO;
@@ -218,9 +197,9 @@ static const struct iio_info kxsd9_info = {
 	.driver_module = THIS_MODULE,
 };
 
-static int kxsd9_common_probe(struct device *parent,
-			      struct kxsd9_transport *transport,
-			      const char *name)
+int kxsd9_common_probe(struct device *parent,
+		       struct kxsd9_transport *transport,
+		       const char *name)
 {
 	struct iio_dev *indio_dev;
 	struct kxsd9_state *st;
@@ -249,87 +228,4 @@ static int kxsd9_common_probe(struct device *parent,
 
 	return 0;
 }
-
-int kxsd9_spi_write1(struct kxsd9_transport *tr, u8 byte)
-{
-	struct spi_device *spi = tr->trdev;
-
-	return spi_w8r8(spi, byte);
-}
-
-int kxsd9_spi_write2(struct kxsd9_transport *tr, u8 b1, u8 b2)
-{
-	struct spi_device *spi = tr->trdev;
-
-	tr->tx[0] = b1;
-	tr->tx[1] = b2;
-	return spi_write(spi, tr->tx, 2);
-}
-
-int kxsd9_spi_readval(struct kxsd9_transport *tr, u8 address)
-{
-	struct spi_device *spi = tr->trdev;
-	struct spi_transfer xfers[] = {
-		{
-			.bits_per_word = 8,
-			.len = 1,
-			.delay_usecs = 200,
-			.tx_buf = tr->tx,
-		}, {
-			.bits_per_word = 8,
-			.len = 2,
-			.rx_buf = tr->rx,
-		},
-	};
-	int ret;
-
-	tr->tx[0] = address;
-	ret = spi_sync_transfer(spi, xfers, ARRAY_SIZE(xfers));
-	if (!ret)
-		ret = (((u16)(tr->rx[0])) << 8) | (tr->rx[1] & 0xF0);
-	return ret;
-}
-
-static int kxsd9_spi_probe(struct spi_device *spi)
-{
-	struct kxsd9_transport *transport;
-	int ret;
-
-	transport = devm_kzalloc(&spi->dev, sizeof(*transport), GFP_KERNEL);
-	if (!transport)
-		return -ENOMEM;
-
-	transport->trdev = spi;
-	transport->write1 = kxsd9_spi_write1;
-	transport->write2 = kxsd9_spi_write2;
-	transport->readval = kxsd9_spi_readval;
-	spi->mode = SPI_MODE_0;
-	spi_setup(spi);
-
-	ret = kxsd9_common_probe(&spi->dev,
-				 transport,
-				 spi_get_device_id(spi)->name);
-	if (ret)
-		return ret;
-
-	return 0;
-}
-
-static const struct spi_device_id kxsd9_spi_id[] = {
-	{"kxsd9", 0},
-	{ },
-};
-MODULE_DEVICE_TABLE(spi, kxsd9_spi_id);
-
-static struct spi_driver kxsd9_spi_driver = {
-	.driver = {
-		.name = "kxsd9",
-	},
-	.probe = kxsd9_spi_probe,
-	.id_table = kxsd9_spi_id,
-};
-module_spi_driver(kxsd9_spi_driver);
-
-MODULE_AUTHOR("Jonathan Cameron <jic23@kernel.org>");
-MODULE_DESCRIPTION("Kionix KXSD9 SPI driver");
-MODULE_LICENSE("GPL v2");
+EXPORT_SYMBOL(kxsd9_common_probe);
diff --git a/drivers/iio/accel/kxsd9.h b/drivers/iio/accel/kxsd9.h
new file mode 100644
index 000000000000..c260a54e00fe
--- /dev/null
+++ b/drivers/iio/accel/kxsd9.h
@@ -0,0 +1,31 @@
+#include <linux/device.h>
+#include <linux/kernel.h>
+
+#define KXSD9_STATE_RX_SIZE 2
+#define KXSD9_STATE_TX_SIZE 2
+
+struct kxsd9_transport;
+
+/**
+ * struct kxsd9_transport - transport adapter for SPI or I2C
+ * @trdev: transport device such as SPI or I2C
+ * @readreg(): function to read a byte from an address in the device
+ * @writereg(): function to write a byte to an address in the device
+ * @write2(): function to write two consecutive bytes to the device
+ * @readval(): function to read a 16bit value from the device
+ * @rx: cache aligned read buffer
+ * @tx: cache aligned write buffer
+ */
+struct kxsd9_transport {
+	void *trdev;
+	int (*readreg) (struct kxsd9_transport *tr, u8 address);
+	int (*writereg) (struct kxsd9_transport *tr, u8 address, u8 val);
+	int (*write2) (struct kxsd9_transport *tr, u8 b1, u8 b2);
+	int (*readval) (struct kxsd9_transport *tr, u8 address);
+	u8 rx[KXSD9_STATE_RX_SIZE] ____cacheline_aligned;
+	u8 tx[KXSD9_STATE_TX_SIZE];
+};
+
+int kxsd9_common_probe(struct device *parent,
+		       struct kxsd9_transport *transport,
+		       const char *name);
-- 
2.7.4

Re: [PATCH 05/17] iio: accel: kxsd9: Split out SPI transport

[Jonathan Cameron]

On 16/08/16 14:33, Linus Walleij wrote:
> This moves the KXSD9 SPI transport out to its own file and Kconfig
> entry, so that we will be able to add another transport method.
> We export the common probe and add a local header file for the
> functionality shared between the main driver and the transport
> driver.
> 
> We make the SPI transport the default for the driver if SPI is
> available and the KXSD9 driver was selected, so the oldconfig
> upgrade path will be clear.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

insmod kxsd9.ko

[  612.188999] kxsd9: module license 'unspecified' taints kernel.
[  612.197563] kxsd9: Unknown symbol mutex_lock_nested (err 0)
[  612.210046] kxsd9: Unknown symbol devm_iio_device_alloc (err 0)
[  612.220192] kxsd9: Unknown symbol devm_iio_device_register (err 0)
insmod: cannot insert 'kxsd9.ko': unknown symbol in module, or unknown parameter

Otherwise works.

Jonathan
> ---
>  drivers/iio/accel/Kconfig     |  10 ++-
>  drivers/iio/accel/Makefile    |   1 +
>  drivers/iio/accel/kxsd9-spi.c | 104 +++++++++++++++++++++++++++++++
>  drivers/iio/accel/kxsd9.c     | 140 ++++++------------------------------------
>  drivers/iio/accel/kxsd9.h     |  31 ++++++++++
>  5 files changed, 163 insertions(+), 123 deletions(-)
>  create mode 100644 drivers/iio/accel/kxsd9-spi.c
>  create mode 100644 drivers/iio/accel/kxsd9.h
> 
> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> index 89d78208de3f..95e3fc09f640 100644
> --- a/drivers/iio/accel/Kconfig
> +++ b/drivers/iio/accel/Kconfig
> @@ -96,7 +96,6 @@ config IIO_ST_ACCEL_SPI_3AXIS
>  
>  config KXSD9
>  	tristate "Kionix KXSD9 Accelerometer Driver"
> -	depends on SPI
>  	help
>  	  Say yes here to build support for the Kionix KXSD9 accelerometer.
>  	  Currently this only supports the device via an SPI interface.
> @@ -104,6 +103,15 @@ config KXSD9
>  	  To compile this driver as a module, choose M here: the module
>  	  will be called kxsd9.
>  
> +config KXSD9_SPI
> +	tristate "Kionix KXSD9 SPI transport"
> +	depends on KXSD9
> +	depends on SPI
> +	default KXSD9
> +	help
> +	  Say yes here to enable the Kionix KXSD9 accelerometer
> +	  SPI transport channel.
> +
>  config KXCJK1013
>  	tristate "Kionix 3-Axis Accelerometer Driver"
>  	depends on I2C
> diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
> index 6cedbecca2ee..22a5770f62a9 100644
> --- a/drivers/iio/accel/Makefile
> +++ b/drivers/iio/accel/Makefile
> @@ -11,6 +11,7 @@ obj-$(CONFIG_BMC150_ACCEL_SPI) += bmc150-accel-spi.o
>  obj-$(CONFIG_HID_SENSOR_ACCEL_3D) += hid-sensor-accel-3d.o
>  obj-$(CONFIG_KXCJK1013) += kxcjk-1013.o
>  obj-$(CONFIG_KXSD9)	+= kxsd9.o
> +obj-$(CONFIG_KXSD9_SPI)	+= kxsd9-spi.o
>  
>  obj-$(CONFIG_MMA7455)		+= mma7455_core.o
>  obj-$(CONFIG_MMA7455_I2C)	+= mma7455_i2c.o
> diff --git a/drivers/iio/accel/kxsd9-spi.c b/drivers/iio/accel/kxsd9-spi.c
> new file mode 100644
> index 000000000000..0789695c89e1
> --- /dev/null
> +++ b/drivers/iio/accel/kxsd9-spi.c
> @@ -0,0 +1,104 @@
> +#include <linux/device.h>
> +#include <linux/kernel.h>
> +#include <linux/spi/spi.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +
> +#include "kxsd9.h"
> +
> +#define KXSD9_READ(a) (0x80 | (a))
> +#define KXSD9_WRITE(a) (a)
> +
> +static int kxsd9_spi_readreg(struct kxsd9_transport *tr, u8 address)
> +{
> +	struct spi_device *spi = tr->trdev;
> +
> +	return spi_w8r8(spi, KXSD9_READ(address));
> +}
> +
> +static int kxsd9_spi_writereg(struct kxsd9_transport *tr, u8 address, u8 val)
> +{
> +	struct spi_device *spi = tr->trdev;
> +
> +	tr->tx[0] = KXSD9_WRITE(address),
> +	tr->tx[1] = val;
> +	return spi_write(spi, tr->tx, 2);
> +}
> +
> +static int kxsd9_spi_write2(struct kxsd9_transport *tr, u8 b1, u8 b2)
> +{
> +	struct spi_device *spi = tr->trdev;
> +
> +	tr->tx[0] = b1;
> +	tr->tx[1] = b2;
> +	return spi_write(spi, tr->tx, 2);
> +}
> +
> +static int kxsd9_spi_readval(struct kxsd9_transport *tr, u8 address)
> +{
> +	struct spi_device *spi = tr->trdev;
> +	struct spi_transfer xfers[] = {
> +		{
> +			.bits_per_word = 8,
> +			.len = 1,
> +			.delay_usecs = 200,
> +			.tx_buf = tr->tx,
> +		}, {
> +			.bits_per_word = 8,
> +			.len = 2,
> +			.rx_buf = tr->rx,
> +		},
> +	};
> +	int ret;
> +
> +	tr->tx[0] = KXSD9_READ(address);
> +	ret = spi_sync_transfer(spi, xfers, ARRAY_SIZE(xfers));
> +	if (!ret)
> +		ret = (((u16)(tr->rx[0])) << 8) | (tr->rx[1]);
> +	return ret;
> +}
> +
> +static int kxsd9_spi_probe(struct spi_device *spi)
> +{
> +	struct kxsd9_transport *transport;
> +	int ret;
> +
> +	transport = devm_kzalloc(&spi->dev, sizeof(*transport), GFP_KERNEL);
> +	if (!transport)
> +		return -ENOMEM;
> +
> +	transport->trdev = spi;
> +	transport->readreg = kxsd9_spi_readreg;
> +	transport->writereg = kxsd9_spi_writereg;
> +	transport->write2 = kxsd9_spi_write2;
> +	transport->readval = kxsd9_spi_readval;
> +	spi->mode = SPI_MODE_0;
> +	spi_setup(spi);
> +
> +	ret = kxsd9_common_probe(&spi->dev,
> +				 transport,
> +				 spi_get_device_id(spi)->name);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static const struct spi_device_id kxsd9_spi_id[] = {
> +	{"kxsd9", 0},
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(spi, kxsd9_spi_id);
> +
> +static struct spi_driver kxsd9_spi_driver = {
> +	.driver = {
> +		.name = "kxsd9",
> +	},
> +	.probe = kxsd9_spi_probe,
> +	.id_table = kxsd9_spi_id,
> +};
> +module_spi_driver(kxsd9_spi_driver);
> +
> +MODULE_AUTHOR("Jonathan Cameron <jic23@kernel.org>");
> +MODULE_DESCRIPTION("Kionix KXSD9 SPI driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iio/accel/kxsd9.c b/drivers/iio/accel/kxsd9.c
> index d3e87eef2594..efa9ba665f6e 100644
> --- a/drivers/iio/accel/kxsd9.c
> +++ b/drivers/iio/accel/kxsd9.c
> @@ -18,7 +18,6 @@
>  
>  #include <linux/device.h>
>  #include <linux/kernel.h>
> -#include <linux/spi/spi.h>
>  #include <linux/sysfs.h>
>  #include <linux/slab.h>
>  #include <linux/module.h>
> @@ -26,6 +25,8 @@
>  #include <linux/iio/iio.h>
>  #include <linux/iio/sysfs.h>
>  
> +#include "kxsd9.h"
> +
>  #define KXSD9_REG_X		0x00
>  #define KXSD9_REG_Y		0x02
>  #define KXSD9_REG_Z		0x04
> @@ -38,32 +39,6 @@
>  #define KXSD9_REG_CTRL_B	0x0d
>  #define KXSD9_REG_CTRL_A	0x0e
>  
> -#define KXSD9_READ(a) (0x80 | (a))
> -#define KXSD9_WRITE(a) (a)
> -
> -#define KXSD9_STATE_RX_SIZE 2
> -#define KXSD9_STATE_TX_SIZE 2
> -
> -struct kxsd9_transport;
> -
> -/**
> - * struct kxsd9_transport - transport adapter for SPI or I2C
> - * @trdev: transport device such as SPI or I2C
> - * @write1(): function to write a byte to the device
> - * @write2(): function to write two consecutive bytes to the device
> - * @readval(): function to read a 16bit value from the device
> - * @rx: cache aligned read buffer
> - * @tx: cache aligned write buffer
> - */
> -struct kxsd9_transport {
> -	void *trdev;
> -	int (*write1) (struct kxsd9_transport *tr, u8 byte);
> -	int (*write2) (struct kxsd9_transport *tr, u8 b1, u8 b2);
> -	int (*readval) (struct kxsd9_transport *tr, u8 address);
> -	u8 rx[KXSD9_STATE_RX_SIZE] ____cacheline_aligned;
> -	u8 tx[KXSD9_STATE_TX_SIZE];
> -};
> -
>  /**
>   * struct kxsd9_state - device related storage
>   * @buf_lock:	protect the rx and tx buffers.
> @@ -99,12 +74,13 @@ static int kxsd9_write_scale(struct iio_dev *indio_dev, int micro)
>  		return -EINVAL;
>  
>  	mutex_lock(&st->buf_lock);
> -	ret = st->transport->write1(st->transport, KXSD9_READ(KXSD9_REG_CTRL_C));
> -	if (ret)
> +	ret = st->transport->readreg(st->transport,
> +				     KXSD9_REG_CTRL_C);
> +	if (ret < 0)
>  		goto error_ret;
> -	ret = st->transport->write2(st->transport,
> -				    KXSD9_WRITE(KXSD9_REG_CTRL_C),
> -				    (ret & ~KXSD9_FS_MASK) | i);
> +	ret = st->transport->writereg(st->transport,
> +				      KXSD9_REG_CTRL_C,
> +				      (ret & ~KXSD9_FS_MASK) | i);
>  error_ret:
>  	mutex_unlock(&st->buf_lock);
>  	return ret;
> @@ -116,7 +92,9 @@ static int kxsd9_read(struct iio_dev *indio_dev, u8 address)
>  	struct kxsd9_state *st = iio_priv(indio_dev);
>  
>  	mutex_lock(&st->buf_lock);
> -	ret = st->transport->readval(st->transport, KXSD9_READ(address));
> +	ret = st->transport->readval(st->transport, address);
> +	/* Only 12 bits are valid */
> +	ret &= 0xfff0;
>  	mutex_unlock(&st->buf_lock);
>  	return ret;
>  }
> @@ -166,8 +144,9 @@ static int kxsd9_read_raw(struct iio_dev *indio_dev,
>  		ret = IIO_VAL_INT;
>  		break;
>  	case IIO_CHAN_INFO_SCALE:
> -		ret = st->transport->write1(st->transport, KXSD9_READ(KXSD9_REG_CTRL_C));
> -		if (ret)
> +		ret = st->transport->readreg(st->transport,
> +					     KXSD9_REG_CTRL_C);
> +		if (ret < 0)
>  			goto error_ret;
>  		*val2 = kxsd9_micro_scales[ret & KXSD9_FS_MASK];
>  		ret = IIO_VAL_INT_PLUS_MICRO;
> @@ -218,9 +197,9 @@ static const struct iio_info kxsd9_info = {
>  	.driver_module = THIS_MODULE,
>  };
>  
> -static int kxsd9_common_probe(struct device *parent,
> -			      struct kxsd9_transport *transport,
> -			      const char *name)
> +int kxsd9_common_probe(struct device *parent,
> +		       struct kxsd9_transport *transport,
> +		       const char *name)
>  {
>  	struct iio_dev *indio_dev;
>  	struct kxsd9_state *st;
> @@ -249,87 +228,4 @@ static int kxsd9_common_probe(struct device *parent,
>  
>  	return 0;
>  }
> -
> -int kxsd9_spi_write1(struct kxsd9_transport *tr, u8 byte)
> -{
> -	struct spi_device *spi = tr->trdev;
> -
> -	return spi_w8r8(spi, byte);
> -}
> -
> -int kxsd9_spi_write2(struct kxsd9_transport *tr, u8 b1, u8 b2)
> -{
> -	struct spi_device *spi = tr->trdev;
> -
> -	tr->tx[0] = b1;
> -	tr->tx[1] = b2;
> -	return spi_write(spi, tr->tx, 2);
> -}
> -
> -int kxsd9_spi_readval(struct kxsd9_transport *tr, u8 address)
> -{
> -	struct spi_device *spi = tr->trdev;
> -	struct spi_transfer xfers[] = {
> -		{
> -			.bits_per_word = 8,
> -			.len = 1,
> -			.delay_usecs = 200,
> -			.tx_buf = tr->tx,
> -		}, {
> -			.bits_per_word = 8,
> -			.len = 2,
> -			.rx_buf = tr->rx,
> -		},
> -	};
> -	int ret;
> -
> -	tr->tx[0] = address;
> -	ret = spi_sync_transfer(spi, xfers, ARRAY_SIZE(xfers));
> -	if (!ret)
> -		ret = (((u16)(tr->rx[0])) << 8) | (tr->rx[1] & 0xF0);
> -	return ret;
> -}
> -
> -static int kxsd9_spi_probe(struct spi_device *spi)
> -{
> -	struct kxsd9_transport *transport;
> -	int ret;
> -
> -	transport = devm_kzalloc(&spi->dev, sizeof(*transport), GFP_KERNEL);
> -	if (!transport)
> -		return -ENOMEM;
> -
> -	transport->trdev = spi;
> -	transport->write1 = kxsd9_spi_write1;
> -	transport->write2 = kxsd9_spi_write2;
> -	transport->readval = kxsd9_spi_readval;
> -	spi->mode = SPI_MODE_0;
> -	spi_setup(spi);
> -
> -	ret = kxsd9_common_probe(&spi->dev,
> -				 transport,
> -				 spi_get_device_id(spi)->name);
> -	if (ret)
> -		return ret;
> -
> -	return 0;
> -}
> -
> -static const struct spi_device_id kxsd9_spi_id[] = {
> -	{"kxsd9", 0},
> -	{ },
> -};
> -MODULE_DEVICE_TABLE(spi, kxsd9_spi_id);
> -
> -static struct spi_driver kxsd9_spi_driver = {
> -	.driver = {
> -		.name = "kxsd9",
> -	},
> -	.probe = kxsd9_spi_probe,
> -	.id_table = kxsd9_spi_id,
> -};
> -module_spi_driver(kxsd9_spi_driver);
> -
> -MODULE_AUTHOR("Jonathan Cameron <jic23@kernel.org>");
> -MODULE_DESCRIPTION("Kionix KXSD9 SPI driver");
> -MODULE_LICENSE("GPL v2");
> +EXPORT_SYMBOL(kxsd9_common_probe);
> diff --git a/drivers/iio/accel/kxsd9.h b/drivers/iio/accel/kxsd9.h
> new file mode 100644
> index 000000000000..c260a54e00fe
> --- /dev/null
> +++ b/drivers/iio/accel/kxsd9.h
> @@ -0,0 +1,31 @@
> +#include <linux/device.h>
> +#include <linux/kernel.h>
> +
> +#define KXSD9_STATE_RX_SIZE 2
> +#define KXSD9_STATE_TX_SIZE 2
> +
> +struct kxsd9_transport;
> +
> +/**
> + * struct kxsd9_transport - transport adapter for SPI or I2C
> + * @trdev: transport device such as SPI or I2C
> + * @readreg(): function to read a byte from an address in the device
> + * @writereg(): function to write a byte to an address in the device
> + * @write2(): function to write two consecutive bytes to the device
> + * @readval(): function to read a 16bit value from the device
> + * @rx: cache aligned read buffer
> + * @tx: cache aligned write buffer
> + */
> +struct kxsd9_transport {
> +	void *trdev;
> +	int (*readreg) (struct kxsd9_transport *tr, u8 address);
> +	int (*writereg) (struct kxsd9_transport *tr, u8 address, u8 val);
> +	int (*write2) (struct kxsd9_transport *tr, u8 b1, u8 b2);
> +	int (*readval) (struct kxsd9_transport *tr, u8 address);
> +	u8 rx[KXSD9_STATE_RX_SIZE] ____cacheline_aligned;
> +	u8 tx[KXSD9_STATE_TX_SIZE];
> +};
> +
> +int kxsd9_common_probe(struct device *parent,
> +		       struct kxsd9_transport *transport,
> +		       const char *name);
> 

[PATCH 06/17] iio: accel: kxsd9: Do away with the write2 helper

[Linus Walleij]

This is just a masquerading register write function, so use the
register write function instead.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/iio/accel/kxsd9-spi.c | 10 ----------
 drivers/iio/accel/kxsd9.c     |  4 ++--
 drivers/iio/accel/kxsd9.h     |  2 --
 3 files changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/iio/accel/kxsd9-spi.c b/drivers/iio/accel/kxsd9-spi.c
index 0789695c89e1..332b7664233e 100644
--- a/drivers/iio/accel/kxsd9-spi.c
+++ b/drivers/iio/accel/kxsd9-spi.c
@@ -25,15 +25,6 @@ static int kxsd9_spi_writereg(struct kxsd9_transport *tr, u8 address, u8 val)
 	return spi_write(spi, tr->tx, 2);
 }
 
-static int kxsd9_spi_write2(struct kxsd9_transport *tr, u8 b1, u8 b2)
-{
-	struct spi_device *spi = tr->trdev;
-
-	tr->tx[0] = b1;
-	tr->tx[1] = b2;
-	return spi_write(spi, tr->tx, 2);
-}
-
 static int kxsd9_spi_readval(struct kxsd9_transport *tr, u8 address)
 {
 	struct spi_device *spi = tr->trdev;
@@ -70,7 +61,6 @@ static int kxsd9_spi_probe(struct spi_device *spi)
 	transport->trdev = spi;
 	transport->readreg = kxsd9_spi_readreg;
 	transport->writereg = kxsd9_spi_writereg;
-	transport->write2 = kxsd9_spi_write2;
 	transport->readval = kxsd9_spi_readval;
 	spi->mode = SPI_MODE_0;
 	spi_setup(spi);
diff --git a/drivers/iio/accel/kxsd9.c b/drivers/iio/accel/kxsd9.c
index efa9ba665f6e..7c94a0d3464a 100644
--- a/drivers/iio/accel/kxsd9.c
+++ b/drivers/iio/accel/kxsd9.c
@@ -184,10 +184,10 @@ static int kxsd9_power_up(struct kxsd9_state *st)
 {
 	int ret;
 
-	ret = st->transport->write2(st->transport, 0x0d, 0x40);
+	ret = st->transport->writereg(st->transport, KXSD9_REG_CTRL_B, 0x40);
 	if (ret)
 		return ret;
-	return st->transport->write2(st->transport, 0x0c, 0x9b);
+	return st->transport->writereg(st->transport, KXSD9_REG_CTRL_C, 0x9b);
 };
 
 static const struct iio_info kxsd9_info = {
diff --git a/drivers/iio/accel/kxsd9.h b/drivers/iio/accel/kxsd9.h
index c260a54e00fe..32f86c9b33c7 100644
--- a/drivers/iio/accel/kxsd9.h
+++ b/drivers/iio/accel/kxsd9.h
@@ -11,7 +11,6 @@ struct kxsd9_transport;
  * @trdev: transport device such as SPI or I2C
  * @readreg(): function to read a byte from an address in the device
  * @writereg(): function to write a byte to an address in the device
- * @write2(): function to write two consecutive bytes to the device
  * @readval(): function to read a 16bit value from the device
  * @rx: cache aligned read buffer
  * @tx: cache aligned write buffer
@@ -20,7 +19,6 @@ struct kxsd9_transport {
 	void *trdev;
 	int (*readreg) (struct kxsd9_transport *tr, u8 address);
 	int (*writereg) (struct kxsd9_transport *tr, u8 address, u8 val);
-	int (*write2) (struct kxsd9_transport *tr, u8 b1, u8 b2);
 	int (*readval) (struct kxsd9_transport *tr, u8 address);
 	u8 rx[KXSD9_STATE_RX_SIZE] ____cacheline_aligned;
 	u8 tx[KXSD9_STATE_TX_SIZE];
-- 
2.7.4

Re: [PATCH 06/17] iio: accel: kxsd9: Do away with the write2 helper

[Jonathan Cameron]

On 16/08/16 14:33, Linus Walleij wrote:
> This is just a masquerading register write function, so use the
> register write function instead.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Tested-by: Jonathan Cameron <jic23@kernel.org>
> ---
>  drivers/iio/accel/kxsd9-spi.c | 10 ----------
>  drivers/iio/accel/kxsd9.c     |  4 ++--
>  drivers/iio/accel/kxsd9.h     |  2 --
>  3 files changed, 2 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/iio/accel/kxsd9-spi.c b/drivers/iio/accel/kxsd9-spi.c
> index 0789695c89e1..332b7664233e 100644
> --- a/drivers/iio/accel/kxsd9-spi.c
> +++ b/drivers/iio/accel/kxsd9-spi.c
> @@ -25,15 +25,6 @@ static int kxsd9_spi_writereg(struct kxsd9_transport *tr, u8 address, u8 val)
>  	return spi_write(spi, tr->tx, 2);
>  }
>  
> -static int kxsd9_spi_write2(struct kxsd9_transport *tr, u8 b1, u8 b2)
> -{
> -	struct spi_device *spi = tr->trdev;
> -
> -	tr->tx[0] = b1;
> -	tr->tx[1] = b2;
> -	return spi_write(spi, tr->tx, 2);
> -}
> -
>  static int kxsd9_spi_readval(struct kxsd9_transport *tr, u8 address)
>  {
>  	struct spi_device *spi = tr->trdev;
> @@ -70,7 +61,6 @@ static int kxsd9_spi_probe(struct spi_device *spi)
>  	transport->trdev = spi;
>  	transport->readreg = kxsd9_spi_readreg;
>  	transport->writereg = kxsd9_spi_writereg;
> -	transport->write2 = kxsd9_spi_write2;
>  	transport->readval = kxsd9_spi_readval;
>  	spi->mode = SPI_MODE_0;
>  	spi_setup(spi);
> diff --git a/drivers/iio/accel/kxsd9.c b/drivers/iio/accel/kxsd9.c
> index efa9ba665f6e..7c94a0d3464a 100644
> --- a/drivers/iio/accel/kxsd9.c
> +++ b/drivers/iio/accel/kxsd9.c
> @@ -184,10 +184,10 @@ static int kxsd9_power_up(struct kxsd9_state *st)
>  {
>  	int ret;
>  
> -	ret = st->transport->write2(st->transport, 0x0d, 0x40);
> +	ret = st->transport->writereg(st->transport, KXSD9_REG_CTRL_B, 0x40);
>  	if (ret)
>  		return ret;
> -	return st->transport->write2(st->transport, 0x0c, 0x9b);
> +	return st->transport->writereg(st->transport, KXSD9_REG_CTRL_C, 0x9b);
>  };
>  
>  static const struct iio_info kxsd9_info = {
> diff --git a/drivers/iio/accel/kxsd9.h b/drivers/iio/accel/kxsd9.h
> index c260a54e00fe..32f86c9b33c7 100644
> --- a/drivers/iio/accel/kxsd9.h
> +++ b/drivers/iio/accel/kxsd9.h
> @@ -11,7 +11,6 @@ struct kxsd9_transport;
>   * @trdev: transport device such as SPI or I2C
>   * @readreg(): function to read a byte from an address in the device
>   * @writereg(): function to write a byte to an address in the device
> - * @write2(): function to write two consecutive bytes to the device
>   * @readval(): function to read a 16bit value from the device
>   * @rx: cache aligned read buffer
>   * @tx: cache aligned write buffer
> @@ -20,7 +19,6 @@ struct kxsd9_transport {
>  	void *trdev;
>  	int (*readreg) (struct kxsd9_transport *tr, u8 address);
>  	int (*writereg) (struct kxsd9_transport *tr, u8 address, u8 val);
> -	int (*write2) (struct kxsd9_transport *tr, u8 b1, u8 b2);
>  	int (*readval) (struct kxsd9_transport *tr, u8 address);
>  	u8 rx[KXSD9_STATE_RX_SIZE] ____cacheline_aligned;
>  	u8 tx[KXSD9_STATE_TX_SIZE];
> 

[PATCH 07/17] iio: accel: kxsd9: Convert to use regmap for transport

[Linus Walleij]

This converts the KXSD9 driver to drop the custom transport
mechanism and just use regmap like everything else.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/iio/accel/Kconfig     |  1 +
 drivers/iio/accel/kxsd9-spi.c | 79 ++++++++++---------------------------------
 drivers/iio/accel/kxsd9.c     | 40 +++++++++++++---------
 drivers/iio/accel/kxsd9.h     | 22 +-----------
 4 files changed, 43 insertions(+), 99 deletions(-)

diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index 95e3fc09f640..0ac316fbba38 100644
--- a/drivers/iio/accel/Kconfig
+++ b/drivers/iio/accel/Kconfig
@@ -108,6 +108,7 @@ config KXSD9_SPI
 	depends on KXSD9
 	depends on SPI
 	default KXSD9
+	select REGMAP_SPI
 	help
 	  Say yes here to enable the Kionix KXSD9 accelerometer
 	  SPI transport channel.
diff --git a/drivers/iio/accel/kxsd9-spi.c b/drivers/iio/accel/kxsd9-spi.c
index 332b7664233e..04dbc6f94e7d 100644
--- a/drivers/iio/accel/kxsd9-spi.c
+++ b/drivers/iio/accel/kxsd9-spi.c
@@ -3,75 +3,30 @@
 #include <linux/spi/spi.h>
 #include <linux/module.h>
 #include <linux/slab.h>
+#include <linux/regmap.h>
 
 #include "kxsd9.h"
 
-#define KXSD9_READ(a) (0x80 | (a))
-#define KXSD9_WRITE(a) (a)
-
-static int kxsd9_spi_readreg(struct kxsd9_transport *tr, u8 address)
-{
-	struct spi_device *spi = tr->trdev;
-
-	return spi_w8r8(spi, KXSD9_READ(address));
-}
-
-static int kxsd9_spi_writereg(struct kxsd9_transport *tr, u8 address, u8 val)
-{
-	struct spi_device *spi = tr->trdev;
-
-	tr->tx[0] = KXSD9_WRITE(address),
-	tr->tx[1] = val;
-	return spi_write(spi, tr->tx, 2);
-}
-
-static int kxsd9_spi_readval(struct kxsd9_transport *tr, u8 address)
-{
-	struct spi_device *spi = tr->trdev;
-	struct spi_transfer xfers[] = {
-		{
-			.bits_per_word = 8,
-			.len = 1,
-			.delay_usecs = 200,
-			.tx_buf = tr->tx,
-		}, {
-			.bits_per_word = 8,
-			.len = 2,
-			.rx_buf = tr->rx,
-		},
-	};
-	int ret;
-
-	tr->tx[0] = KXSD9_READ(address);
-	ret = spi_sync_transfer(spi, xfers, ARRAY_SIZE(xfers));
-	if (!ret)
-		ret = (((u16)(tr->rx[0])) << 8) | (tr->rx[1]);
-	return ret;
-}
-
 static int kxsd9_spi_probe(struct spi_device *spi)
 {
-	struct kxsd9_transport *transport;
-	int ret;
-
-	transport = devm_kzalloc(&spi->dev, sizeof(*transport), GFP_KERNEL);
-	if (!transport)
-		return -ENOMEM;
+	static const struct regmap_config config = {
+		.reg_bits = 8,
+		.val_bits = 8,
+		.max_register = 0x0e,
+	};
+	struct regmap *regmap;
 
-	transport->trdev = spi;
-	transport->readreg = kxsd9_spi_readreg;
-	transport->writereg = kxsd9_spi_writereg;
-	transport->readval = kxsd9_spi_readval;
 	spi->mode = SPI_MODE_0;
-	spi_setup(spi);
-
-	ret = kxsd9_common_probe(&spi->dev,
-				 transport,
-				 spi_get_device_id(spi)->name);
-	if (ret)
-		return ret;
-
-	return 0;
+	regmap = devm_regmap_init_spi(spi, &config);
+	if (IS_ERR(regmap)) {
+		dev_err(&spi->dev, "%s: regmap allocation failed: %ld\n",
+			__func__, PTR_ERR(regmap));
+		return PTR_ERR(regmap);
+	}
+
+	return kxsd9_common_probe(&spi->dev,
+				  regmap,
+				  spi_get_device_id(spi)->name);
 }
 
 static const struct spi_device_id kxsd9_spi_id[] = {
diff --git a/drivers/iio/accel/kxsd9.c b/drivers/iio/accel/kxsd9.c
index 7c94a0d3464a..fe85f61cfa9d 100644
--- a/drivers/iio/accel/kxsd9.c
+++ b/drivers/iio/accel/kxsd9.c
@@ -21,7 +21,7 @@
 #include <linux/sysfs.h>
 #include <linux/slab.h>
 #include <linux/module.h>
-
+#include <linux/regmap.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
 
@@ -47,7 +47,7 @@
  * @tx:		single tx buffer storage
  **/
 struct kxsd9_state {
-	struct kxsd9_transport *transport;
+	struct regmap *map;
 	struct mutex buf_lock;
 };
 
@@ -64,6 +64,7 @@ static int kxsd9_write_scale(struct iio_dev *indio_dev, int micro)
 	int ret, i;
 	struct kxsd9_state *st = iio_priv(indio_dev);
 	bool foundit = false;
+	unsigned int val;
 
 	for (i = 0; i < 4; i++)
 		if (micro == kxsd9_micro_scales[i]) {
@@ -74,13 +75,14 @@ static int kxsd9_write_scale(struct iio_dev *indio_dev, int micro)
 		return -EINVAL;
 
 	mutex_lock(&st->buf_lock);
-	ret = st->transport->readreg(st->transport,
-				     KXSD9_REG_CTRL_C);
+	ret = regmap_read(st->map,
+			  KXSD9_REG_CTRL_C,
+			  &val);
 	if (ret < 0)
 		goto error_ret;
-	ret = st->transport->writereg(st->transport,
-				      KXSD9_REG_CTRL_C,
-				      (ret & ~KXSD9_FS_MASK) | i);
+	ret = regmap_write(st->map,
+			   KXSD9_REG_CTRL_C,
+			   (val & ~KXSD9_FS_MASK) | i);
 error_ret:
 	mutex_unlock(&st->buf_lock);
 	return ret;
@@ -90,11 +92,15 @@ static int kxsd9_read(struct iio_dev *indio_dev, u8 address)
 {
 	int ret;
 	struct kxsd9_state *st = iio_priv(indio_dev);
+	__be16 raw_val;
 
 	mutex_lock(&st->buf_lock);
-	ret = st->transport->readval(st->transport, address);
+	ret = regmap_bulk_read(st->map, address, &raw_val, sizeof(raw_val));
+	if (ret)
+		goto out_fail_read;
 	/* Only 12 bits are valid */
-	ret &= 0xfff0;
+	ret = be16_to_cpu(raw_val) & 0xfff0;
+out_fail_read:
 	mutex_unlock(&st->buf_lock);
 	return ret;
 }
@@ -134,6 +140,7 @@ static int kxsd9_read_raw(struct iio_dev *indio_dev,
 {
 	int ret = -EINVAL;
 	struct kxsd9_state *st = iio_priv(indio_dev);
+	unsigned int regval;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
@@ -144,11 +151,12 @@ static int kxsd9_read_raw(struct iio_dev *indio_dev,
 		ret = IIO_VAL_INT;
 		break;
 	case IIO_CHAN_INFO_SCALE:
-		ret = st->transport->readreg(st->transport,
-					     KXSD9_REG_CTRL_C);
+		ret = regmap_read(st->map,
+				  KXSD9_REG_CTRL_C,
+				  &regval);
 		if (ret < 0)
 			goto error_ret;
-		*val2 = kxsd9_micro_scales[ret & KXSD9_FS_MASK];
+		*val2 = kxsd9_micro_scales[regval & KXSD9_FS_MASK];
 		ret = IIO_VAL_INT_PLUS_MICRO;
 		break;
 	}
@@ -184,10 +192,10 @@ static int kxsd9_power_up(struct kxsd9_state *st)
 {
 	int ret;
 
-	ret = st->transport->writereg(st->transport, KXSD9_REG_CTRL_B, 0x40);
+	ret = regmap_write(st->map, KXSD9_REG_CTRL_B, 0x40);
 	if (ret)
 		return ret;
-	return st->transport->writereg(st->transport, KXSD9_REG_CTRL_C, 0x9b);
+	return regmap_write(st->map, KXSD9_REG_CTRL_C, 0x9b);
 };
 
 static const struct iio_info kxsd9_info = {
@@ -198,7 +206,7 @@ static const struct iio_info kxsd9_info = {
 };
 
 int kxsd9_common_probe(struct device *parent,
-		       struct kxsd9_transport *transport,
+		       struct regmap *map,
 		       const char *name)
 {
 	struct iio_dev *indio_dev;
@@ -210,7 +218,7 @@ int kxsd9_common_probe(struct device *parent,
 		return -ENOMEM;
 
 	st = iio_priv(indio_dev);
-	st->transport = transport;
+	st->map = map;
 
 	mutex_init(&st->buf_lock);
 	indio_dev->channels = kxsd9_channels;
diff --git a/drivers/iio/accel/kxsd9.h b/drivers/iio/accel/kxsd9.h
index 32f86c9b33c7..ef3dbbf70b98 100644
--- a/drivers/iio/accel/kxsd9.h
+++ b/drivers/iio/accel/kxsd9.h
@@ -4,26 +4,6 @@
 #define KXSD9_STATE_RX_SIZE 2
 #define KXSD9_STATE_TX_SIZE 2
 
-struct kxsd9_transport;
-
-/**
- * struct kxsd9_transport - transport adapter for SPI or I2C
- * @trdev: transport device such as SPI or I2C
- * @readreg(): function to read a byte from an address in the device
- * @writereg(): function to write a byte to an address in the device
- * @readval(): function to read a 16bit value from the device
- * @rx: cache aligned read buffer
- * @tx: cache aligned write buffer
- */
-struct kxsd9_transport {
-	void *trdev;
-	int (*readreg) (struct kxsd9_transport *tr, u8 address);
-	int (*writereg) (struct kxsd9_transport *tr, u8 address, u8 val);
-	int (*readval) (struct kxsd9_transport *tr, u8 address);
-	u8 rx[KXSD9_STATE_RX_SIZE] ____cacheline_aligned;
-	u8 tx[KXSD9_STATE_TX_SIZE];
-};
-
 int kxsd9_common_probe(struct device *parent,
-		       struct kxsd9_transport *transport,
+		       struct regmap *map,
 		       const char *name);
-- 
2.7.4

Re: [PATCH 07/17] iio: accel: kxsd9: Convert to use regmap for transport

[Jonathan Cameron]

On 16/08/16 14:33, Linus Walleij wrote:
> This converts the KXSD9 driver to drop the custom transport
> mechanism and just use regmap like everything else.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Mostly fine, but we are dropping an explicit delay in the
read function. Which bothers me.  Even if I test this on
my board and it's fine (as the natural delay may well be
big enough) that's not say the device isn't on a quicker
spi bus somewhere else....

Jonathan
> ---
>  drivers/iio/accel/Kconfig     |  1 +
>  drivers/iio/accel/kxsd9-spi.c | 79 ++++++++++---------------------------------
>  drivers/iio/accel/kxsd9.c     | 40 +++++++++++++---------
>  drivers/iio/accel/kxsd9.h     | 22 +-----------
>  4 files changed, 43 insertions(+), 99 deletions(-)
> 
> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> index 95e3fc09f640..0ac316fbba38 100644
> --- a/drivers/iio/accel/Kconfig
> +++ b/drivers/iio/accel/Kconfig
> @@ -108,6 +108,7 @@ config KXSD9_SPI
>  	depends on KXSD9
>  	depends on SPI
>  	default KXSD9
> +	select REGMAP_SPI
>  	help
>  	  Say yes here to enable the Kionix KXSD9 accelerometer
>  	  SPI transport channel.
> diff --git a/drivers/iio/accel/kxsd9-spi.c b/drivers/iio/accel/kxsd9-spi.c
> index 332b7664233e..04dbc6f94e7d 100644
> --- a/drivers/iio/accel/kxsd9-spi.c
> +++ b/drivers/iio/accel/kxsd9-spi.c
> @@ -3,75 +3,30 @@
>  #include <linux/spi/spi.h>
>  #include <linux/module.h>
>  #include <linux/slab.h>
> +#include <linux/regmap.h>
>  
>  #include "kxsd9.h"
>  
> -#define KXSD9_READ(a) (0x80 | (a))
> -#define KXSD9_WRITE(a) (a)
> -
> -static int kxsd9_spi_readreg(struct kxsd9_transport *tr, u8 address)
> -{
> -	struct spi_device *spi = tr->trdev;
> -
> -	return spi_w8r8(spi, KXSD9_READ(address));
> -}
> -
> -static int kxsd9_spi_writereg(struct kxsd9_transport *tr, u8 address, u8 val)
> -{
> -	struct spi_device *spi = tr->trdev;
> -
> -	tr->tx[0] = KXSD9_WRITE(address),
> -	tr->tx[1] = val;
> -	return spi_write(spi, tr->tx, 2);
> -}
> -
> -static int kxsd9_spi_readval(struct kxsd9_transport *tr, u8 address)
> -{
> -	struct spi_device *spi = tr->trdev;
> -	struct spi_transfer xfers[] = {
> -		{
> -			.bits_per_word = 8,
> -			.len = 1,
> -			.delay_usecs = 200,
Dropping this explicit delay? 
> -			.tx_buf = tr->tx,
> -		}, {
> -			.bits_per_word = 8,
> -			.len = 2,
> -			.rx_buf = tr->rx,
> -		},
> -	};
> -	int ret;
> -
> -	tr->tx[0] = KXSD9_READ(address);
> -	ret = spi_sync_transfer(spi, xfers, ARRAY_SIZE(xfers));
> -	if (!ret)
> -		ret = (((u16)(tr->rx[0])) << 8) | (tr->rx[1]);
> -	return ret;
> -}
> -
>  static int kxsd9_spi_probe(struct spi_device *spi)
>  {
> -	struct kxsd9_transport *transport;
> -	int ret;
> -
> -	transport = devm_kzalloc(&spi->dev, sizeof(*transport), GFP_KERNEL);
> -	if (!transport)
> -		return -ENOMEM;
> +	static const struct regmap_config config = {
> +		.reg_bits = 8,
> +		.val_bits = 8,
> +		.max_register = 0x0e,
> +	};
> +	struct regmap *regmap;
>  
> -	transport->trdev = spi;
> -	transport->readreg = kxsd9_spi_readreg;
> -	transport->writereg = kxsd9_spi_writereg;
> -	transport->readval = kxsd9_spi_readval;
>  	spi->mode = SPI_MODE_0;
> -	spi_setup(spi);
> -
> -	ret = kxsd9_common_probe(&spi->dev,
> -				 transport,
> -				 spi_get_device_id(spi)->name);
> -	if (ret)
> -		return ret;
> -
> -	return 0;
> +	regmap = devm_regmap_init_spi(spi, &config);
> +	if (IS_ERR(regmap)) {
> +		dev_err(&spi->dev, "%s: regmap allocation failed: %ld\n",
> +			__func__, PTR_ERR(regmap));
> +		return PTR_ERR(regmap);
> +	}
> +
> +	return kxsd9_common_probe(&spi->dev,
> +				  regmap,
> +				  spi_get_device_id(spi)->name);
>  }
>  
>  static const struct spi_device_id kxsd9_spi_id[] = {
> diff --git a/drivers/iio/accel/kxsd9.c b/drivers/iio/accel/kxsd9.c
> index 7c94a0d3464a..fe85f61cfa9d 100644
> --- a/drivers/iio/accel/kxsd9.c
> +++ b/drivers/iio/accel/kxsd9.c
> @@ -21,7 +21,7 @@
>  #include <linux/sysfs.h>
>  #include <linux/slab.h>
>  #include <linux/module.h>
> -
> +#include <linux/regmap.h>
>  #include <linux/iio/iio.h>
>  #include <linux/iio/sysfs.h>
>  
> @@ -47,7 +47,7 @@
>   * @tx:		single tx buffer storage
>   **/
>  struct kxsd9_state {
> -	struct kxsd9_transport *transport;
> +	struct regmap *map;
>  	struct mutex buf_lock;
>  };
>  
> @@ -64,6 +64,7 @@ static int kxsd9_write_scale(struct iio_dev *indio_dev, int micro)
>  	int ret, i;
>  	struct kxsd9_state *st = iio_priv(indio_dev);
>  	bool foundit = false;
> +	unsigned int val;
>  
>  	for (i = 0; i < 4; i++)
>  		if (micro == kxsd9_micro_scales[i]) {
> @@ -74,13 +75,14 @@ static int kxsd9_write_scale(struct iio_dev *indio_dev, int micro)
>  		return -EINVAL;
>  
>  	mutex_lock(&st->buf_lock);
> -	ret = st->transport->readreg(st->transport,
> -				     KXSD9_REG_CTRL_C);
> +	ret = regmap_read(st->map,
> +			  KXSD9_REG_CTRL_C,
> +			  &val);
>  	if (ret < 0)
>  		goto error_ret;
> -	ret = st->transport->writereg(st->transport,
> -				      KXSD9_REG_CTRL_C,
> -				      (ret & ~KXSD9_FS_MASK) | i);
> +	ret = regmap_write(st->map,
> +			   KXSD9_REG_CTRL_C,
> +			   (val & ~KXSD9_FS_MASK) | i);
>  error_ret:
>  	mutex_unlock(&st->buf_lock);
>  	return ret;
> @@ -90,11 +92,15 @@ static int kxsd9_read(struct iio_dev *indio_dev, u8 address)
>  {
>  	int ret;
>  	struct kxsd9_state *st = iio_priv(indio_dev);
> +	__be16 raw_val;
>  
>  	mutex_lock(&st->buf_lock);
> -	ret = st->transport->readval(st->transport, address);
> +	ret = regmap_bulk_read(st->map, address, &raw_val, sizeof(raw_val));
> +	if (ret)
> +		goto out_fail_read;
>  	/* Only 12 bits are valid */
> -	ret &= 0xfff0;
> +	ret = be16_to_cpu(raw_val) & 0xfff0;
> +out_fail_read:
>  	mutex_unlock(&st->buf_lock);
>  	return ret;
>  }
> @@ -134,6 +140,7 @@ static int kxsd9_read_raw(struct iio_dev *indio_dev,
>  {
>  	int ret = -EINVAL;
>  	struct kxsd9_state *st = iio_priv(indio_dev);
> +	unsigned int regval;
>  
>  	switch (mask) {
>  	case IIO_CHAN_INFO_RAW:
> @@ -144,11 +151,12 @@ static int kxsd9_read_raw(struct iio_dev *indio_dev,
>  		ret = IIO_VAL_INT;
>  		break;
>  	case IIO_CHAN_INFO_SCALE:
> -		ret = st->transport->readreg(st->transport,
> -					     KXSD9_REG_CTRL_C);
> +		ret = regmap_read(st->map,
> +				  KXSD9_REG_CTRL_C,
> +				  &regval);
>  		if (ret < 0)
>  			goto error_ret;
> -		*val2 = kxsd9_micro_scales[ret & KXSD9_FS_MASK];
> +		*val2 = kxsd9_micro_scales[regval & KXSD9_FS_MASK];
>  		ret = IIO_VAL_INT_PLUS_MICRO;
>  		break;
>  	}
> @@ -184,10 +192,10 @@ static int kxsd9_power_up(struct kxsd9_state *st)
>  {
>  	int ret;
>  
> -	ret = st->transport->writereg(st->transport, KXSD9_REG_CTRL_B, 0x40);
> +	ret = regmap_write(st->map, KXSD9_REG_CTRL_B, 0x40);
>  	if (ret)
>  		return ret;
> -	return st->transport->writereg(st->transport, KXSD9_REG_CTRL_C, 0x9b);
> +	return regmap_write(st->map, KXSD9_REG_CTRL_C, 0x9b);
>  };
>  
>  static const struct iio_info kxsd9_info = {
> @@ -198,7 +206,7 @@ static const struct iio_info kxsd9_info = {
>  };
>  
>  int kxsd9_common_probe(struct device *parent,
> -		       struct kxsd9_transport *transport,
> +		       struct regmap *map,
>  		       const char *name)
>  {
>  	struct iio_dev *indio_dev;
> @@ -210,7 +218,7 @@ int kxsd9_common_probe(struct device *parent,
>  		return -ENOMEM;
>  
>  	st = iio_priv(indio_dev);
> -	st->transport = transport;
> +	st->map = map;
>  
>  	mutex_init(&st->buf_lock);
>  	indio_dev->channels = kxsd9_channels;
> diff --git a/drivers/iio/accel/kxsd9.h b/drivers/iio/accel/kxsd9.h
> index 32f86c9b33c7..ef3dbbf70b98 100644
> --- a/drivers/iio/accel/kxsd9.h
> +++ b/drivers/iio/accel/kxsd9.h
> @@ -4,26 +4,6 @@
>  #define KXSD9_STATE_RX_SIZE 2
>  #define KXSD9_STATE_TX_SIZE 2
>  
> -struct kxsd9_transport;
> -
> -/**
> - * struct kxsd9_transport - transport adapter for SPI or I2C
> - * @trdev: transport device such as SPI or I2C
> - * @readreg(): function to read a byte from an address in the device
> - * @writereg(): function to write a byte to an address in the device
> - * @readval(): function to read a 16bit value from the device
> - * @rx: cache aligned read buffer
> - * @tx: cache aligned write buffer
> - */
> -struct kxsd9_transport {
> -	void *trdev;
> -	int (*readreg) (struct kxsd9_transport *tr, u8 address);
> -	int (*writereg) (struct kxsd9_transport *tr, u8 address, u8 val);
> -	int (*readval) (struct kxsd9_transport *tr, u8 address);
> -	u8 rx[KXSD9_STATE_RX_SIZE] ____cacheline_aligned;
> -	u8 tx[KXSD9_STATE_TX_SIZE];
> -};
> -
>  int kxsd9_common_probe(struct device *parent,
> -		       struct kxsd9_transport *transport,
> +		       struct regmap *map,
>  		       const char *name);
> 

Re: [PATCH 07/17] iio: accel: kxsd9: Convert to use regmap for transport

[Jonathan Cameron]

On 16/08/16 14:33, Linus Walleij wrote:
> This converts the KXSD9 driver to drop the custom transport
> mechanism and just use regmap like everything else.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Looks good.

Tested-by: Jonathan Cameron <jic23@kernel.org>

> ---
>  drivers/iio/accel/Kconfig     |  1 +
>  drivers/iio/accel/kxsd9-spi.c | 79 ++++++++++---------------------------------
>  drivers/iio/accel/kxsd9.c     | 40 +++++++++++++---------
>  drivers/iio/accel/kxsd9.h     | 22 +-----------
>  4 files changed, 43 insertions(+), 99 deletions(-)
> 
> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> index 95e3fc09f640..0ac316fbba38 100644
> --- a/drivers/iio/accel/Kconfig
> +++ b/drivers/iio/accel/Kconfig
> @@ -108,6 +108,7 @@ config KXSD9_SPI
>  	depends on KXSD9
>  	depends on SPI
>  	default KXSD9
> +	select REGMAP_SPI
>  	help
>  	  Say yes here to enable the Kionix KXSD9 accelerometer
>  	  SPI transport channel.
> diff --git a/drivers/iio/accel/kxsd9-spi.c b/drivers/iio/accel/kxsd9-spi.c
> index 332b7664233e..04dbc6f94e7d 100644
> --- a/drivers/iio/accel/kxsd9-spi.c
> +++ b/drivers/iio/accel/kxsd9-spi.c
> @@ -3,75 +3,30 @@
>  #include <linux/spi/spi.h>
>  #include <linux/module.h>
>  #include <linux/slab.h>
> +#include <linux/regmap.h>
>  
>  #include "kxsd9.h"
>  
> -#define KXSD9_READ(a) (0x80 | (a))
> -#define KXSD9_WRITE(a) (a)
> -
> -static int kxsd9_spi_readreg(struct kxsd9_transport *tr, u8 address)
> -{
> -	struct spi_device *spi = tr->trdev;
> -
> -	return spi_w8r8(spi, KXSD9_READ(address));
> -}
> -
> -static int kxsd9_spi_writereg(struct kxsd9_transport *tr, u8 address, u8 val)
> -{
> -	struct spi_device *spi = tr->trdev;
> -
> -	tr->tx[0] = KXSD9_WRITE(address),
> -	tr->tx[1] = val;
> -	return spi_write(spi, tr->tx, 2);
> -}
> -
> -static int kxsd9_spi_readval(struct kxsd9_transport *tr, u8 address)
> -{
> -	struct spi_device *spi = tr->trdev;
> -	struct spi_transfer xfers[] = {
> -		{
> -			.bits_per_word = 8,
> -			.len = 1,
> -			.delay_usecs = 200,
> -			.tx_buf = tr->tx,
> -		}, {
> -			.bits_per_word = 8,
> -			.len = 2,
> -			.rx_buf = tr->rx,
> -		},
> -	};
> -	int ret;
> -
> -	tr->tx[0] = KXSD9_READ(address);
> -	ret = spi_sync_transfer(spi, xfers, ARRAY_SIZE(xfers));
> -	if (!ret)
> -		ret = (((u16)(tr->rx[0])) << 8) | (tr->rx[1]);
> -	return ret;
> -}
> -
>  static int kxsd9_spi_probe(struct spi_device *spi)
>  {
> -	struct kxsd9_transport *transport;
> -	int ret;
> -
> -	transport = devm_kzalloc(&spi->dev, sizeof(*transport), GFP_KERNEL);
> -	if (!transport)
> -		return -ENOMEM;
> +	static const struct regmap_config config = {
> +		.reg_bits = 8,
> +		.val_bits = 8,
> +		.max_register = 0x0e,
> +	};
> +	struct regmap *regmap;
>  
> -	transport->trdev = spi;
> -	transport->readreg = kxsd9_spi_readreg;
> -	transport->writereg = kxsd9_spi_writereg;
> -	transport->readval = kxsd9_spi_readval;
>  	spi->mode = SPI_MODE_0;
> -	spi_setup(spi);
> -
> -	ret = kxsd9_common_probe(&spi->dev,
> -				 transport,
> -				 spi_get_device_id(spi)->name);
> -	if (ret)
> -		return ret;
> -
> -	return 0;
> +	regmap = devm_regmap_init_spi(spi, &config);
> +	if (IS_ERR(regmap)) {
> +		dev_err(&spi->dev, "%s: regmap allocation failed: %ld\n",
> +			__func__, PTR_ERR(regmap));
> +		return PTR_ERR(regmap);
> +	}
> +
> +	return kxsd9_common_probe(&spi->dev,
> +				  regmap,
> +				  spi_get_device_id(spi)->name);
>  }
>  
>  static const struct spi_device_id kxsd9_spi_id[] = {
> diff --git a/drivers/iio/accel/kxsd9.c b/drivers/iio/accel/kxsd9.c
> index 7c94a0d3464a..fe85f61cfa9d 100644
> --- a/drivers/iio/accel/kxsd9.c
> +++ b/drivers/iio/accel/kxsd9.c
> @@ -21,7 +21,7 @@
>  #include <linux/sysfs.h>
>  #include <linux/slab.h>
>  #include <linux/module.h>
> -
> +#include <linux/regmap.h>
>  #include <linux/iio/iio.h>
>  #include <linux/iio/sysfs.h>
>  
> @@ -47,7 +47,7 @@
>   * @tx:		single tx buffer storage
>   **/
>  struct kxsd9_state {
> -	struct kxsd9_transport *transport;
> +	struct regmap *map;
>  	struct mutex buf_lock;
>  };
>  
> @@ -64,6 +64,7 @@ static int kxsd9_write_scale(struct iio_dev *indio_dev, int micro)
>  	int ret, i;
>  	struct kxsd9_state *st = iio_priv(indio_dev);
>  	bool foundit = false;
> +	unsigned int val;
>  
>  	for (i = 0; i < 4; i++)
>  		if (micro == kxsd9_micro_scales[i]) {
> @@ -74,13 +75,14 @@ static int kxsd9_write_scale(struct iio_dev *indio_dev, int micro)
>  		return -EINVAL;
>  
>  	mutex_lock(&st->buf_lock);
> -	ret = st->transport->readreg(st->transport,
> -				     KXSD9_REG_CTRL_C);
> +	ret = regmap_read(st->map,
> +			  KXSD9_REG_CTRL_C,
> +			  &val);
>  	if (ret < 0)
>  		goto error_ret;
> -	ret = st->transport->writereg(st->transport,
> -				      KXSD9_REG_CTRL_C,
> -				      (ret & ~KXSD9_FS_MASK) | i);
> +	ret = regmap_write(st->map,
> +			   KXSD9_REG_CTRL_C,
> +			   (val & ~KXSD9_FS_MASK) | i);
>  error_ret:
>  	mutex_unlock(&st->buf_lock);
>  	return ret;
> @@ -90,11 +92,15 @@ static int kxsd9_read(struct iio_dev *indio_dev, u8 address)
>  {
>  	int ret;
>  	struct kxsd9_state *st = iio_priv(indio_dev);
> +	__be16 raw_val;
>  
>  	mutex_lock(&st->buf_lock);
> -	ret = st->transport->readval(st->transport, address);
> +	ret = regmap_bulk_read(st->map, address, &raw_val, sizeof(raw_val));
> +	if (ret)
> +		goto out_fail_read;
>  	/* Only 12 bits are valid */
> -	ret &= 0xfff0;
> +	ret = be16_to_cpu(raw_val) & 0xfff0;
> +out_fail_read:
>  	mutex_unlock(&st->buf_lock);
>  	return ret;
>  }
> @@ -134,6 +140,7 @@ static int kxsd9_read_raw(struct iio_dev *indio_dev,
>  {
>  	int ret = -EINVAL;
>  	struct kxsd9_state *st = iio_priv(indio_dev);
> +	unsigned int regval;
>  
>  	switch (mask) {
>  	case IIO_CHAN_INFO_RAW:
> @@ -144,11 +151,12 @@ static int kxsd9_read_raw(struct iio_dev *indio_dev,
>  		ret = IIO_VAL_INT;
>  		break;
>  	case IIO_CHAN_INFO_SCALE:
> -		ret = st->transport->readreg(st->transport,
> -					     KXSD9_REG_CTRL_C);
> +		ret = regmap_read(st->map,
> +				  KXSD9_REG_CTRL_C,
> +				  &regval);
>  		if (ret < 0)
>  			goto error_ret;
> -		*val2 = kxsd9_micro_scales[ret & KXSD9_FS_MASK];
> +		*val2 = kxsd9_micro_scales[regval & KXSD9_FS_MASK];
>  		ret = IIO_VAL_INT_PLUS_MICRO;
>  		break;
>  	}
> @@ -184,10 +192,10 @@ static int kxsd9_power_up(struct kxsd9_state *st)
>  {
>  	int ret;
>  
> -	ret = st->transport->writereg(st->transport, KXSD9_REG_CTRL_B, 0x40);
> +	ret = regmap_write(st->map, KXSD9_REG_CTRL_B, 0x40);
>  	if (ret)
>  		return ret;
> -	return st->transport->writereg(st->transport, KXSD9_REG_CTRL_C, 0x9b);
> +	return regmap_write(st->map, KXSD9_REG_CTRL_C, 0x9b);
>  };
>  
>  static const struct iio_info kxsd9_info = {
> @@ -198,7 +206,7 @@ static const struct iio_info kxsd9_info = {
>  };
>  
>  int kxsd9_common_probe(struct device *parent,
> -		       struct kxsd9_transport *transport,
> +		       struct regmap *map,
>  		       const char *name)
>  {
>  	struct iio_dev *indio_dev;
> @@ -210,7 +218,7 @@ int kxsd9_common_probe(struct device *parent,
>  		return -ENOMEM;
>  
>  	st = iio_priv(indio_dev);
> -	st->transport = transport;
> +	st->map = map;
>  
>  	mutex_init(&st->buf_lock);
>  	indio_dev->channels = kxsd9_channels;
> diff --git a/drivers/iio/accel/kxsd9.h b/drivers/iio/accel/kxsd9.h
> index 32f86c9b33c7..ef3dbbf70b98 100644
> --- a/drivers/iio/accel/kxsd9.h
> +++ b/drivers/iio/accel/kxsd9.h
> @@ -4,26 +4,6 @@
>  #define KXSD9_STATE_RX_SIZE 2
>  #define KXSD9_STATE_TX_SIZE 2
>  
> -struct kxsd9_transport;
> -
> -/**
> - * struct kxsd9_transport - transport adapter for SPI or I2C
> - * @trdev: transport device such as SPI or I2C
> - * @readreg(): function to read a byte from an address in the device
> - * @writereg(): function to write a byte to an address in the device
> - * @readval(): function to read a 16bit value from the device
> - * @rx: cache aligned read buffer
> - * @tx: cache aligned write buffer
> - */
> -struct kxsd9_transport {
> -	void *trdev;
> -	int (*readreg) (struct kxsd9_transport *tr, u8 address);
> -	int (*writereg) (struct kxsd9_transport *tr, u8 address, u8 val);
> -	int (*readval) (struct kxsd9_transport *tr, u8 address);
> -	u8 rx[KXSD9_STATE_RX_SIZE] ____cacheline_aligned;
> -	u8 tx[KXSD9_STATE_TX_SIZE];
> -};
> -
>  int kxsd9_common_probe(struct device *parent,
> -		       struct kxsd9_transport *transport,
> +		       struct regmap *map,
>  		       const char *name);
> 

[PATCH 08/17] iio: accel: kxsd9: Add I2C transport

[Linus Walleij]

This adds I2C regmap transport for the KXSD9 driver.
Tested on the KXSD9 sensor on the APQ8060 Dragonboard.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/iio/accel/Kconfig     | 12 ++++++++-
 drivers/iio/accel/Makefile    |  1 +
 drivers/iio/accel/kxsd9-i2c.c | 57 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 69 insertions(+), 1 deletion(-)
 create mode 100644 drivers/iio/accel/kxsd9-i2c.c

diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index 0ac316fbba38..ab1c87ce07ed 100644
--- a/drivers/iio/accel/Kconfig
+++ b/drivers/iio/accel/Kconfig
@@ -98,7 +98,7 @@ config KXSD9
 	tristate "Kionix KXSD9 Accelerometer Driver"
 	help
 	  Say yes here to build support for the Kionix KXSD9 accelerometer.
-	  Currently this only supports the device via an SPI interface.
+	  It can be accessed using an (optional) SPI or I2C interface.
 
 	  To compile this driver as a module, choose M here: the module
 	  will be called kxsd9.
@@ -113,6 +113,16 @@ config KXSD9_SPI
 	  Say yes here to enable the Kionix KXSD9 accelerometer
 	  SPI transport channel.
 
+config KXSD9_I2C
+	tristate "Kionix KXSD9 I2C transport"
+	depends on KXSD9
+	depends on I2C
+	default KXSD9
+	select REGMAP_I2C
+	help
+	  Say yes here to enable the Kionix KXSD9 accelerometer
+	  I2C transport channel.
+
 config KXCJK1013
 	tristate "Kionix 3-Axis Accelerometer Driver"
 	depends on I2C
diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
index 22a5770f62a9..7031e842c2cb 100644
--- a/drivers/iio/accel/Makefile
+++ b/drivers/iio/accel/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_HID_SENSOR_ACCEL_3D) += hid-sensor-accel-3d.o
 obj-$(CONFIG_KXCJK1013) += kxcjk-1013.o
 obj-$(CONFIG_KXSD9)	+= kxsd9.o
 obj-$(CONFIG_KXSD9_SPI)	+= kxsd9-spi.o
+obj-$(CONFIG_KXSD9_I2C)	+= kxsd9-i2c.o
 
 obj-$(CONFIG_MMA7455)		+= mma7455_core.o
 obj-$(CONFIG_MMA7455_I2C)	+= mma7455_i2c.o
diff --git a/drivers/iio/accel/kxsd9-i2c.c b/drivers/iio/accel/kxsd9-i2c.c
new file mode 100644
index 000000000000..2c910755288d
--- /dev/null
+++ b/drivers/iio/accel/kxsd9-i2c.c
@@ -0,0 +1,57 @@
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/delay.h>
+#include <linux/regmap.h>
+
+#include "kxsd9.h"
+
+static int kxsd9_i2c_probe(struct i2c_client *i2c,
+			   const struct i2c_device_id *id)
+{
+	static const struct regmap_config config = {
+		.reg_bits = 8,
+		.val_bits = 8,
+		.max_register = 0x0e,
+	};
+	struct regmap *regmap;
+
+	regmap = devm_regmap_init_i2c(i2c, &config);
+	if (IS_ERR(regmap)) {
+		dev_err(&i2c->dev, "Failed to register i2c regmap %d\n",
+			(int)PTR_ERR(regmap));
+		return PTR_ERR(regmap);
+	}
+
+	return kxsd9_common_probe(&i2c->dev,
+				  regmap,
+				  i2c->name);
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id kxsd9_of_match[] = {
+	{ .compatible = "kionix,kxsd9", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, kxsd9_of_match);
+#else
+#define kxsd9_of_match NULL
+#endif
+
+static const struct i2c_device_id kxsd9_i2c_id[] = {
+	{"kxsd9", 0},
+	{ },
+};
+MODULE_DEVICE_TABLE(i2c, kxsd9_i2c_id);
+
+static struct i2c_driver kxsd9_i2c_driver = {
+	.driver = {
+		.name	= "kxsd9",
+		.of_match_table = of_match_ptr(kxsd9_of_match),
+	},
+	.probe		= kxsd9_i2c_probe,
+	.id_table	= kxsd9_i2c_id,
+};
+module_i2c_driver(kxsd9_i2c_driver);
-- 
2.7.4

[PATCH 09/17] iio: accel: kxsd9: Drop the buffer lock

[Linus Walleij]

The RX/TX buffers are gone so drop the lock (it should have been
in the transport struct anyway).

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/iio/accel/kxsd9.c | 19 ++++---------------
 1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/drivers/iio/accel/kxsd9.c b/drivers/iio/accel/kxsd9.c
index fe85f61cfa9d..a8ee041dd4e4 100644
--- a/drivers/iio/accel/kxsd9.c
+++ b/drivers/iio/accel/kxsd9.c
@@ -41,14 +41,10 @@
 
 /**
  * struct kxsd9_state - device related storage
- * @buf_lock:	protect the rx and tx buffers.
- * @us:		spi device
- * @rx:		single rx buffer storage
- * @tx:		single tx buffer storage
- **/
+ * @map: regmap to the device
+ */
 struct kxsd9_state {
 	struct regmap *map;
-	struct mutex buf_lock;
 };
 
 #define KXSD9_SCALE_2G "0.011978"
@@ -74,7 +70,6 @@ static int kxsd9_write_scale(struct iio_dev *indio_dev, int micro)
 	if (!foundit)
 		return -EINVAL;
 
-	mutex_lock(&st->buf_lock);
 	ret = regmap_read(st->map,
 			  KXSD9_REG_CTRL_C,
 			  &val);
@@ -84,7 +79,6 @@ static int kxsd9_write_scale(struct iio_dev *indio_dev, int micro)
 			   KXSD9_REG_CTRL_C,
 			   (val & ~KXSD9_FS_MASK) | i);
 error_ret:
-	mutex_unlock(&st->buf_lock);
 	return ret;
 }
 
@@ -94,15 +88,11 @@ static int kxsd9_read(struct iio_dev *indio_dev, u8 address)
 	struct kxsd9_state *st = iio_priv(indio_dev);
 	__be16 raw_val;
 
-	mutex_lock(&st->buf_lock);
 	ret = regmap_bulk_read(st->map, address, &raw_val, sizeof(raw_val));
 	if (ret)
-		goto out_fail_read;
+		return ret;
 	/* Only 12 bits are valid */
-	ret = be16_to_cpu(raw_val) & 0xfff0;
-out_fail_read:
-	mutex_unlock(&st->buf_lock);
-	return ret;
+	return be16_to_cpu(raw_val) & 0xfff0;
 }
 
 static IIO_CONST_ATTR(accel_scale_available,
@@ -220,7 +210,6 @@ int kxsd9_common_probe(struct device *parent,
 	st = iio_priv(indio_dev);
 	st->map = map;
 
-	mutex_init(&st->buf_lock);
 	indio_dev->channels = kxsd9_channels;
 	indio_dev->num_channels = ARRAY_SIZE(kxsd9_channels);
 	indio_dev->name = name;
-- 
2.7.4

Re: [PATCH 09/17] iio: accel: kxsd9: Drop the buffer lock

[Jonathan Cameron]

On 16/08/16 14:33, Linus Walleij wrote:
> The RX/TX buffers are gone so drop the lock (it should have been
> in the transport struct anyway).
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Tested-by: Jonathan Cameron <jic23@kernel.org>
> ---
>  drivers/iio/accel/kxsd9.c | 19 ++++---------------
>  1 file changed, 4 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/iio/accel/kxsd9.c b/drivers/iio/accel/kxsd9.c
> index fe85f61cfa9d..a8ee041dd4e4 100644
> --- a/drivers/iio/accel/kxsd9.c
> +++ b/drivers/iio/accel/kxsd9.c
> @@ -41,14 +41,10 @@
>  
>  /**
>   * struct kxsd9_state - device related storage
> - * @buf_lock:	protect the rx and tx buffers.
> - * @us:		spi device
> - * @rx:		single rx buffer storage
> - * @tx:		single tx buffer storage
> - **/
> + * @map: regmap to the device
> + */
>  struct kxsd9_state {
>  	struct regmap *map;
> -	struct mutex buf_lock;
>  };
>  
>  #define KXSD9_SCALE_2G "0.011978"
> @@ -74,7 +70,6 @@ static int kxsd9_write_scale(struct iio_dev *indio_dev, int micro)
>  	if (!foundit)
>  		return -EINVAL;
>  
> -	mutex_lock(&st->buf_lock);
>  	ret = regmap_read(st->map,
>  			  KXSD9_REG_CTRL_C,
>  			  &val);
> @@ -84,7 +79,6 @@ static int kxsd9_write_scale(struct iio_dev *indio_dev, int micro)
>  			   KXSD9_REG_CTRL_C,
>  			   (val & ~KXSD9_FS_MASK) | i);
>  error_ret:
> -	mutex_unlock(&st->buf_lock);
>  	return ret;
>  }
>  
> @@ -94,15 +88,11 @@ static int kxsd9_read(struct iio_dev *indio_dev, u8 address)
>  	struct kxsd9_state *st = iio_priv(indio_dev);
>  	__be16 raw_val;
>  
> -	mutex_lock(&st->buf_lock);
>  	ret = regmap_bulk_read(st->map, address, &raw_val, sizeof(raw_val));
>  	if (ret)
> -		goto out_fail_read;
> +		return ret;
>  	/* Only 12 bits are valid */
> -	ret = be16_to_cpu(raw_val) & 0xfff0;
> -out_fail_read:
> -	mutex_unlock(&st->buf_lock);
> -	return ret;
> +	return be16_to_cpu(raw_val) & 0xfff0;
>  }
>  
>  static IIO_CONST_ATTR(accel_scale_available,
> @@ -220,7 +210,6 @@ int kxsd9_common_probe(struct device *parent,
>  	st = iio_priv(indio_dev);
>  	st->map = map;
>  
> -	mutex_init(&st->buf_lock);
>  	indio_dev->channels = kxsd9_channels;
>  	indio_dev->num_channels = ARRAY_SIZE(kxsd9_channels);
>  	indio_dev->name = name;
> 

[PATCH 10/17] iio: accel: kxsd9: Fix scaling bug

[Linus Walleij]

All the scaling of the KXSD9 involves multiplication with a
fraction number < 1.

However the scaling value returned from IIO_INFO_SCALE was
unpredictable as only the micros of the value was assigned, and
not the integer part, resulting in scaling like this:

$cat in_accel_scale
-1057462640.011978

Fix this by assigning zero to the integer part.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/iio/accel/kxsd9.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/iio/accel/kxsd9.c b/drivers/iio/accel/kxsd9.c
index a8ee041dd4e4..14d949a4caf9 100644
--- a/drivers/iio/accel/kxsd9.c
+++ b/drivers/iio/accel/kxsd9.c
@@ -146,6 +146,7 @@ static int kxsd9_read_raw(struct iio_dev *indio_dev,
 				  &regval);
 		if (ret < 0)
 			goto error_ret;
+		*val = 0;
 		*val2 = kxsd9_micro_scales[regval & KXSD9_FS_MASK];
 		ret = IIO_VAL_INT_PLUS_MICRO;
 		break;
-- 
2.7.4

Re: [PATCH 10/17] iio: accel: kxsd9: Fix scaling bug

[Jonathan Cameron]

On 16/08/16 14:33, Linus Walleij wrote:
> All the scaling of the KXSD9 involves multiplication with a
> fraction number < 1.
> 
> However the scaling value returned from IIO_INFO_SCALE was
> unpredictable as only the micros of the value was assigned, and
> not the integer part, resulting in scaling like this:
> 
> $cat in_accel_scale
> -1057462640.011978
> 
> Fix this by assigning zero to the integer part.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
I'll pick this up for stable.  If you want to reorder so it's at the top
of the series of the remaining patches that would be cool.
If not I'll wiggle it till it fits.

Tested-by: Jonathan Cameron <jic23@kernel.org>

oops.
> ---
>  drivers/iio/accel/kxsd9.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/iio/accel/kxsd9.c b/drivers/iio/accel/kxsd9.c
> index a8ee041dd4e4..14d949a4caf9 100644
> --- a/drivers/iio/accel/kxsd9.c
> +++ b/drivers/iio/accel/kxsd9.c
> @@ -146,6 +146,7 @@ static int kxsd9_read_raw(struct iio_dev *indio_dev,
>  				  &regval);
>  		if (ret < 0)
>  			goto error_ret;
> +		*val = 0;
>  		*val2 = kxsd9_micro_scales[regval & KXSD9_FS_MASK];
>  		ret = IIO_VAL_INT_PLUS_MICRO;
>  		break;
> 

Re: [PATCH 10/17] iio: accel: kxsd9: Fix scaling bug

[Linus Walleij]

On Wed, Aug 31, 2016 at 9:47 PM, Jonathan Cameron <jic23@kernel.org> wrote:
> On 16/08/16 14:33, Linus Walleij wrote:
>> All the scaling of the KXSD9 involves multiplication with a
>> fraction number < 1.
>>
>> However the scaling value returned from IIO_INFO_SCALE was
>> unpredictable as only the micros of the value was assigned, and
>> not the integer part, resulting in scaling like this:
>>
>> $cat in_accel_scale
>> -1057462640.011978
>>
>> Fix this by assigning zero to the integer part.
>>
>> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>
> I'll pick this up for stable.  If you want to reorder so it's at the top
> of the series of the remaining patches that would be cool.
> If not I'll wiggle it till it fits.
>
> Tested-by: Jonathan Cameron <jic23@kernel.org>

OK rebasing it to front and tagging for stable, no
problem. Should've thought about it myself.

Yours,
Linus Walleij

[PATCH 11/17] iio: accel: kxsd9: Fix up offset and scaling

[Linus Walleij]

This fixes several errors in the offset and scaling of the raw
values from the KXSD9 sensor:

- The code did not convert the big endian value from the sensor
  into the endianness of the host CPU. Fix this with
  be16_to_cpu() on the raw obtained value.

- The code did not regard the fact that only the upper 12 bits of
  the accelerometer values are valid. Shift these
  down four bits to yield the real raw value.

- Further the sensor provides 2048 at zero g. This means that an
  offset of 2048 must be subtracted from the raw value before
  scaling. This was not taken into account by the driver,
  yielding a weird value. Fix this by providing this offset in
  sysfs.

To house the scaling code better, the value reading code was
factored into the raw reading function.

This proper scaling and offseting is necessary to get proper
values out of triggered buffer by offsetting, shifting and scaling
them.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/iio/accel/kxsd9.c | 35 ++++++++++++++++++-----------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/drivers/iio/accel/kxsd9.c b/drivers/iio/accel/kxsd9.c
index 14d949a4caf9..152042ef3e3c 100644
--- a/drivers/iio/accel/kxsd9.c
+++ b/drivers/iio/accel/kxsd9.c
@@ -55,6 +55,8 @@ struct kxsd9_state {
 /* reverse order */
 static const int kxsd9_micro_scales[4] = { 47853, 35934, 23927, 11978 };
 
+#define KXSD9_ZERO_G_OFFSET -2048
+
 static int kxsd9_write_scale(struct iio_dev *indio_dev, int micro)
 {
 	int ret, i;
@@ -82,19 +84,6 @@ error_ret:
 	return ret;
 }
 
-static int kxsd9_read(struct iio_dev *indio_dev, u8 address)
-{
-	int ret;
-	struct kxsd9_state *st = iio_priv(indio_dev);
-	__be16 raw_val;
-
-	ret = regmap_bulk_read(st->map, address, &raw_val, sizeof(raw_val));
-	if (ret)
-		return ret;
-	/* Only 12 bits are valid */
-	return be16_to_cpu(raw_val) & 0xfff0;
-}
-
 static IIO_CONST_ATTR(accel_scale_available,
 		KXSD9_SCALE_2G " "
 		KXSD9_SCALE_4G " "
@@ -131,13 +120,24 @@ static int kxsd9_read_raw(struct iio_dev *indio_dev,
 	int ret = -EINVAL;
 	struct kxsd9_state *st = iio_priv(indio_dev);
 	unsigned int regval;
+	__be16 raw_val;
+	u16 nval;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
-		ret = kxsd9_read(indio_dev, chan->address);
-		if (ret < 0)
+		ret = regmap_bulk_read(st->map, chan->address, &raw_val,
+				       sizeof(raw_val));
+		if (ret)
 			goto error_ret;
-		*val = ret;
+		nval = be16_to_cpu(raw_val);
+		/* Only 12 bits are valid */
+		nval >>= 4;
+		*val = nval;
+		ret = IIO_VAL_INT;
+		break;
+	case IIO_CHAN_INFO_OFFSET:
+		/* This has a bias of -2048 */
+		*val = KXSD9_ZERO_G_OFFSET;
 		ret = IIO_VAL_INT;
 		break;
 	case IIO_CHAN_INFO_SCALE:
@@ -161,7 +161,8 @@ error_ret:
 		.modified = 1,						\
 		.channel2 = IIO_MOD_##axis,				\
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
-		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),	\
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
+					BIT(IIO_CHAN_INFO_OFFSET),	\
 		.address = KXSD9_REG_##axis,				\
 	}
 
-- 
2.7.4

Re: [PATCH 11/17] iio: accel: kxsd9: Fix up offset and scaling

[Peter Meerwald-Stadler]

> - The code did not regard the fact that only the upper 12 bits of
>   the accelerometer values are valid. Shift these
>   down four bits to yield the real raw value.

maybe can do without nval temporary variable

> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/iio/accel/kxsd9.c | 35 ++++++++++++++++++-----------------
>  1 file changed, 18 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/iio/accel/kxsd9.c b/drivers/iio/accel/kxsd9.c
> index 14d949a4caf9..152042ef3e3c 100644
> --- a/drivers/iio/accel/kxsd9.c
> +++ b/drivers/iio/accel/kxsd9.c
> @@ -55,6 +55,8 @@ struct kxsd9_state {
>  /* reverse order */
>  static const int kxsd9_micro_scales[4] = { 47853, 35934, 23927, 11978 };
>  
> +#define KXSD9_ZERO_G_OFFSET -2048
> +
>  static int kxsd9_write_scale(struct iio_dev *indio_dev, int micro)
>  {
>  	int ret, i;
> @@ -82,19 +84,6 @@ error_ret:
>  	return ret;
>  }
>  
> -static int kxsd9_read(struct iio_dev *indio_dev, u8 address)
> -{
> -	int ret;
> -	struct kxsd9_state *st = iio_priv(indio_dev);
> -	__be16 raw_val;
> -
> -	ret = regmap_bulk_read(st->map, address, &raw_val, sizeof(raw_val));
> -	if (ret)
> -		return ret;
> -	/* Only 12 bits are valid */
> -	return be16_to_cpu(raw_val) & 0xfff0;
> -}
> -
>  static IIO_CONST_ATTR(accel_scale_available,
>  		KXSD9_SCALE_2G " "
>  		KXSD9_SCALE_4G " "
> @@ -131,13 +120,24 @@ static int kxsd9_read_raw(struct iio_dev *indio_dev,
>  	int ret = -EINVAL;
>  	struct kxsd9_state *st = iio_priv(indio_dev);
>  	unsigned int regval;
> +	__be16 raw_val;
> +	u16 nval;
>  
>  	switch (mask) {
>  	case IIO_CHAN_INFO_RAW:
> -		ret = kxsd9_read(indio_dev, chan->address);
> -		if (ret < 0)
> +		ret = regmap_bulk_read(st->map, chan->address, &raw_val,
> +				       sizeof(raw_val));
> +		if (ret)
>  			goto error_ret;
> -		*val = ret;
> +		nval = be16_to_cpu(raw_val);
> +		/* Only 12 bits are valid */
> +		nval >>= 4;
> +		*val = nval;
> +		ret = IIO_VAL_INT;
> +		break;
> +	case IIO_CHAN_INFO_OFFSET:
> +		/* This has a bias of -2048 */
> +		*val = KXSD9_ZERO_G_OFFSET;
>  		ret = IIO_VAL_INT;
>  		break;
>  	case IIO_CHAN_INFO_SCALE:
> @@ -161,7 +161,8 @@ error_ret:
>  		.modified = 1,						\
>  		.channel2 = IIO_MOD_##axis,				\
>  		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
> -		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),	\
> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
> +					BIT(IIO_CHAN_INFO_OFFSET),	\
>  		.address = KXSD9_REG_##axis,				\
>  	}
>  
> 

-- 

Peter Meerwald-Stadler
+43-664-2444418 (mobile)

Re: [PATCH 11/17] iio: accel: kxsd9: Fix up offset and scaling

[Linus Walleij]

On Tue, Aug 16, 2016 at 3:58 PM, Peter Meerwald-Stadler
<pmeerw@pmeerw.net> wrote:

>> - The code did not regard the fact that only the upper 12 bits of
>>   the accelerometer values are valid. Shift these
>>   down four bits to yield the real raw value.
>
> maybe can do without nval temporary variable

I could but I thought this code looks neater than the alternatives,
I tried the inlining way but thought it was hard to read.
Well let's see what Jonathan prefers, I can rewrite it for sure.

Yours,
Linus Walleij

[PATCH 12/17] iio: accel: kxsd9: Add triggered buffer handling

[Linus Walleij]

As is custom with all modern sensors, add a clever burst mode
that will just stream out values from the sensor and provide it
to userspace to do the proper offsetting and scaling.

This is the result when tested with an HRTimer trigger:

$ generic_buffer -a -c 10 -n kxsd9 -t foo
/sys/bus/iio/devices/iio:device1 foo
0.371318 0.718680 9.869872 1795.000000 97545896129
-0.586922 0.179670 9.378775 2398.000000 97555864721
-0.299450 0.179670 10.348992 2672.000000 97565874055
0.371318 0.335384 11.103606 2816.000000 97575883240
0.179670 0.574944 10.540640 2847.000000 97585862351
0.335384 0.754614 9.953718 2840.000000 97595872425
0.179670 0.754614 10.732288 2879.000000 97605882351
0.000000 0.754614 10.348992 2872.000000 97615891832
-0.730658 0.574944 9.570422 2831.000000 97625871536
0.000000 1.137910 10.732288 2872.000000 97635881610

Columns shown are x, y, z acceleration, so a positive acceleration
of ~9.81 (shaky due to bad calibration) along the z axis. The
fourth column is the AUX IN which is floating on this system,
it seems to float up to the 2.85V VDD voltage.

To be able to cleanup the triggered buffer, we need to add .remove()
callbacks to the I2C and SPI subdrivers and call back into an
exported .remove() callback in the core.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/iio/accel/Kconfig     |  2 +
 drivers/iio/accel/kxsd9-i2c.c |  6 +++
 drivers/iio/accel/kxsd9-spi.c |  6 +++
 drivers/iio/accel/kxsd9.c     | 93 ++++++++++++++++++++++++++++++++++++++++---
 drivers/iio/accel/kxsd9.h     |  1 +
 5 files changed, 102 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index ab1c87ce07ed..cd69353989cf 100644
--- a/drivers/iio/accel/Kconfig
+++ b/drivers/iio/accel/Kconfig
@@ -96,6 +96,8 @@ config IIO_ST_ACCEL_SPI_3AXIS
 
 config KXSD9
 	tristate "Kionix KXSD9 Accelerometer Driver"
+	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
 	help
 	  Say yes here to build support for the Kionix KXSD9 accelerometer.
 	  It can be accessed using an (optional) SPI or I2C interface.
diff --git a/drivers/iio/accel/kxsd9-i2c.c b/drivers/iio/accel/kxsd9-i2c.c
index 2c910755288d..4aaa27d0aa32 100644
--- a/drivers/iio/accel/kxsd9-i2c.c
+++ b/drivers/iio/accel/kxsd9-i2c.c
@@ -30,6 +30,11 @@ static int kxsd9_i2c_probe(struct i2c_client *i2c,
 				  i2c->name);
 }
 
+static int kxsd9_i2c_remove(struct i2c_client *client)
+{
+	return kxsd9_common_remove(&client->dev);
+}
+
 #ifdef CONFIG_OF
 static const struct of_device_id kxsd9_of_match[] = {
 	{ .compatible = "kionix,kxsd9", },
@@ -52,6 +57,7 @@ static struct i2c_driver kxsd9_i2c_driver = {
 		.of_match_table = of_match_ptr(kxsd9_of_match),
 	},
 	.probe		= kxsd9_i2c_probe,
+	.remove		= kxsd9_i2c_remove,
 	.id_table	= kxsd9_i2c_id,
 };
 module_i2c_driver(kxsd9_i2c_driver);
diff --git a/drivers/iio/accel/kxsd9-spi.c b/drivers/iio/accel/kxsd9-spi.c
index 04dbc6f94e7d..c5af51b7dd7e 100644
--- a/drivers/iio/accel/kxsd9-spi.c
+++ b/drivers/iio/accel/kxsd9-spi.c
@@ -29,6 +29,11 @@ static int kxsd9_spi_probe(struct spi_device *spi)
 				  spi_get_device_id(spi)->name);
 }
 
+static int kxsd9_spi_remove(struct spi_device *spi)
+{
+	return kxsd9_common_remove(&spi->dev);
+}
+
 static const struct spi_device_id kxsd9_spi_id[] = {
 	{"kxsd9", 0},
 	{ },
@@ -40,6 +45,7 @@ static struct spi_driver kxsd9_spi_driver = {
 		.name = "kxsd9",
 	},
 	.probe = kxsd9_spi_probe,
+	.remove = kxsd9_spi_remove,
 	.id_table = kxsd9_spi_id,
 };
 module_spi_driver(kxsd9_spi_driver);
diff --git a/drivers/iio/accel/kxsd9.c b/drivers/iio/accel/kxsd9.c
index 152042ef3e3c..78afbe05710e 100644
--- a/drivers/iio/accel/kxsd9.c
+++ b/drivers/iio/accel/kxsd9.c
@@ -12,8 +12,6 @@
  * I have a suitable wire made up.
  *
  * TODO:	Support the motion detector
- *		Uses register address incrementing so could have a
- *		heavily optimized ring buffer access function.
  */
 
 #include <linux/device.h>
@@ -24,6 +22,9 @@
 #include <linux/regmap.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/iio/trigger_consumer.h>
 
 #include "kxsd9.h"
 
@@ -41,9 +42,11 @@
 
 /**
  * struct kxsd9_state - device related storage
+ * @dev: pointer to the parent device
  * @map: regmap to the device
  */
 struct kxsd9_state {
+	struct device *dev;
 	struct regmap *map;
 };
 
@@ -155,7 +158,35 @@ static int kxsd9_read_raw(struct iio_dev *indio_dev,
 error_ret:
 	return ret;
 };
-#define KXSD9_ACCEL_CHAN(axis)						\
+
+static irqreturn_t kxsd9_trigger_handler(int irq, void *p)
+{
+	const struct iio_poll_func *pf = p;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct kxsd9_state *st = iio_priv(indio_dev);
+	int ret;
+	/* 4 * 16bit values AND timestamp */
+	__be16 hw_values[8];
+
+	ret = regmap_bulk_read(st->map,
+			       KXSD9_REG_X,
+			       &hw_values,
+			       8);
+	if (ret) {
+		dev_err(st->dev,
+			"error reading data\n");
+		return ret;
+	}
+
+	iio_push_to_buffers_with_timestamp(indio_dev,
+					   hw_values,
+					   iio_get_time_ns(indio_dev));
+	iio_trigger_notify_done(indio_dev->trig);
+
+	return IRQ_HANDLED;
+}
+
+#define KXSD9_ACCEL_CHAN(axis, index)						\
 	{								\
 		.type = IIO_ACCEL,					\
 		.modified = 1,						\
@@ -164,16 +195,35 @@ error_ret:
 		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
 					BIT(IIO_CHAN_INFO_OFFSET),	\
 		.address = KXSD9_REG_##axis,				\
+		.scan_index = index,					\
+		.scan_type = {                                          \
+			.sign = 'u',					\
+			.realbits = 12,					\
+			.storagebits = 16,				\
+			.shift = 4,					\
+			.endianness = IIO_BE,				\
+		},							\
 	}
 
 static const struct iio_chan_spec kxsd9_channels[] = {
-	KXSD9_ACCEL_CHAN(X), KXSD9_ACCEL_CHAN(Y), KXSD9_ACCEL_CHAN(Z),
+	KXSD9_ACCEL_CHAN(X, 0),
+	KXSD9_ACCEL_CHAN(Y, 1),
+	KXSD9_ACCEL_CHAN(Z, 2),
 	{
 		.type = IIO_VOLTAGE,
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
 		.indexed = 1,
 		.address = KXSD9_REG_AUX,
-	}
+		.scan_index = 3,
+		.scan_type = {
+			.sign = 'u',
+			.realbits = 12,
+			.storagebits = 16,
+			.shift = 4,
+			.endianness = IIO_BE,
+		},
+	},
+	IIO_CHAN_SOFT_TIMESTAMP(4),
 };
 
 static const struct attribute_group kxsd9_attribute_group = {
@@ -197,6 +247,9 @@ static const struct iio_info kxsd9_info = {
 	.driver_module = THIS_MODULE,
 };
 
+/* Four channels apart from timestamp, scan mask = 0x0f */
+static const unsigned long kxsd9_scan_masks[] = { 0xf, 0 };
+
 int kxsd9_common_probe(struct device *parent,
 		       struct regmap *map,
 		       const char *name)
@@ -210,6 +263,7 @@ int kxsd9_common_probe(struct device *parent,
 		return -ENOMEM;
 
 	st = iio_priv(indio_dev);
+	st->dev = parent;
 	st->map = map;
 
 	indio_dev->channels = kxsd9_channels;
@@ -218,13 +272,40 @@ int kxsd9_common_probe(struct device *parent,
 	indio_dev->dev.parent = parent;
 	indio_dev->info = &kxsd9_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->available_scan_masks = kxsd9_scan_masks;
 
 	kxsd9_power_up(st);
 
+	ret = iio_triggered_buffer_setup(indio_dev,
+					 iio_pollfunc_store_time,
+					 kxsd9_trigger_handler,
+					 NULL);
+	if (ret) {
+		dev_err(parent, "triggered buffer setup failed\n");
+		return ret;
+	}
+
 	ret = devm_iio_device_register(parent, indio_dev);
 	if (ret)
-		return ret;
+		goto err_cleanup_buffer;
+
+	dev_set_drvdata(parent, indio_dev);
 
 	return 0;
+
+err_cleanup_buffer:
+	iio_triggered_buffer_cleanup(indio_dev);
+
+	return ret;
 }
 EXPORT_SYMBOL(kxsd9_common_probe);
+
+int kxsd9_common_remove(struct device *parent)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(parent);
+
+	iio_triggered_buffer_cleanup(indio_dev);
+
+	return 0;
+}
+EXPORT_SYMBOL(kxsd9_common_remove);
diff --git a/drivers/iio/accel/kxsd9.h b/drivers/iio/accel/kxsd9.h
index ef3dbbf70b98..19131a7a692c 100644
--- a/drivers/iio/accel/kxsd9.h
+++ b/drivers/iio/accel/kxsd9.h
@@ -7,3 +7,4 @@
 int kxsd9_common_probe(struct device *parent,
 		       struct regmap *map,
 		       const char *name);
+int kxsd9_common_remove(struct device *parent);
-- 
2.7.4

Re: [PATCH 12/17] iio: accel: kxsd9: Add triggered buffer handling

[Jonathan Cameron]

On 16/08/16 14:33, Linus Walleij wrote:
> As is custom with all modern sensors, add a clever burst mode
> that will just stream out values from the sensor and provide it
> to userspace to do the proper offsetting and scaling.
> 
> This is the result when tested with an HRTimer trigger:
> 
> $ generic_buffer -a -c 10 -n kxsd9 -t foo
> /sys/bus/iio/devices/iio:device1 foo
> 0.371318 0.718680 9.869872 1795.000000 97545896129
> -0.586922 0.179670 9.378775 2398.000000 97555864721
> -0.299450 0.179670 10.348992 2672.000000 97565874055
> 0.371318 0.335384 11.103606 2816.000000 97575883240
> 0.179670 0.574944 10.540640 2847.000000 97585862351
> 0.335384 0.754614 9.953718 2840.000000 97595872425
> 0.179670 0.754614 10.732288 2879.000000 97605882351
> 0.000000 0.754614 10.348992 2872.000000 97615891832
> -0.730658 0.574944 9.570422 2831.000000 97625871536
> 0.000000 1.137910 10.732288 2872.000000 97635881610
> 
> Columns shown are x, y, z acceleration, so a positive acceleration
> of ~9.81 (shaky due to bad calibration) along the z axis. The
> fourth column is the AUX IN which is floating on this system,
> it seems to float up to the 2.85V VDD voltage.
> 
> To be able to cleanup the triggered buffer, we need to add .remove()
> callbacks to the I2C and SPI subdrivers and call back into an
> exported .remove() callback in the core.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
One issue inline. Otherwise:

Tested-by: Jonathan Cameron <jic23@kernel.org>

Jonathan
> ---
>  drivers/iio/accel/Kconfig     |  2 +
>  drivers/iio/accel/kxsd9-i2c.c |  6 +++
>  drivers/iio/accel/kxsd9-spi.c |  6 +++
>  drivers/iio/accel/kxsd9.c     | 93 ++++++++++++++++++++++++++++++++++++++++---
>  drivers/iio/accel/kxsd9.h     |  1 +
>  5 files changed, 102 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> index ab1c87ce07ed..cd69353989cf 100644
> --- a/drivers/iio/accel/Kconfig
> +++ b/drivers/iio/accel/Kconfig
> @@ -96,6 +96,8 @@ config IIO_ST_ACCEL_SPI_3AXIS
>  
>  config KXSD9
>  	tristate "Kionix KXSD9 Accelerometer Driver"
> +	select IIO_BUFFER
> +	select IIO_TRIGGERED_BUFFER
>  	help
>  	  Say yes here to build support for the Kionix KXSD9 accelerometer.
>  	  It can be accessed using an (optional) SPI or I2C interface.
> diff --git a/drivers/iio/accel/kxsd9-i2c.c b/drivers/iio/accel/kxsd9-i2c.c
> index 2c910755288d..4aaa27d0aa32 100644
> --- a/drivers/iio/accel/kxsd9-i2c.c
> +++ b/drivers/iio/accel/kxsd9-i2c.c
> @@ -30,6 +30,11 @@ static int kxsd9_i2c_probe(struct i2c_client *i2c,
>  				  i2c->name);
>  }
>  
> +static int kxsd9_i2c_remove(struct i2c_client *client)
> +{
> +	return kxsd9_common_remove(&client->dev);
> +}
> +
>  #ifdef CONFIG_OF
>  static const struct of_device_id kxsd9_of_match[] = {
>  	{ .compatible = "kionix,kxsd9", },
> @@ -52,6 +57,7 @@ static struct i2c_driver kxsd9_i2c_driver = {
>  		.of_match_table = of_match_ptr(kxsd9_of_match),
>  	},
>  	.probe		= kxsd9_i2c_probe,
> +	.remove		= kxsd9_i2c_remove,
>  	.id_table	= kxsd9_i2c_id,
>  };
>  module_i2c_driver(kxsd9_i2c_driver);
> diff --git a/drivers/iio/accel/kxsd9-spi.c b/drivers/iio/accel/kxsd9-spi.c
> index 04dbc6f94e7d..c5af51b7dd7e 100644
> --- a/drivers/iio/accel/kxsd9-spi.c
> +++ b/drivers/iio/accel/kxsd9-spi.c
> @@ -29,6 +29,11 @@ static int kxsd9_spi_probe(struct spi_device *spi)
>  				  spi_get_device_id(spi)->name);
>  }
>  
> +static int kxsd9_spi_remove(struct spi_device *spi)
> +{
> +	return kxsd9_common_remove(&spi->dev);
> +}
> +
>  static const struct spi_device_id kxsd9_spi_id[] = {
>  	{"kxsd9", 0},
>  	{ },
> @@ -40,6 +45,7 @@ static struct spi_driver kxsd9_spi_driver = {
>  		.name = "kxsd9",
>  	},
>  	.probe = kxsd9_spi_probe,
> +	.remove = kxsd9_spi_remove,
>  	.id_table = kxsd9_spi_id,
>  };
>  module_spi_driver(kxsd9_spi_driver);
> diff --git a/drivers/iio/accel/kxsd9.c b/drivers/iio/accel/kxsd9.c
> index 152042ef3e3c..78afbe05710e 100644
> --- a/drivers/iio/accel/kxsd9.c
> +++ b/drivers/iio/accel/kxsd9.c
> @@ -12,8 +12,6 @@
>   * I have a suitable wire made up.
>   *
>   * TODO:	Support the motion detector
> - *		Uses register address incrementing so could have a
> - *		heavily optimized ring buffer access function.
>   */
>  
>  #include <linux/device.h>
> @@ -24,6 +22,9 @@
>  #include <linux/regmap.h>
>  #include <linux/iio/iio.h>
>  #include <linux/iio/sysfs.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/trigger_consumer.h>
>  
>  #include "kxsd9.h"
>  
> @@ -41,9 +42,11 @@
>  
>  /**
>   * struct kxsd9_state - device related storage
> + * @dev: pointer to the parent device
>   * @map: regmap to the device
>   */
>  struct kxsd9_state {
> +	struct device *dev;
>  	struct regmap *map;
>  };
>  
> @@ -155,7 +158,35 @@ static int kxsd9_read_raw(struct iio_dev *indio_dev,
>  error_ret:
>  	return ret;
>  };
> -#define KXSD9_ACCEL_CHAN(axis)						\
> +
> +static irqreturn_t kxsd9_trigger_handler(int irq, void *p)
> +{
> +	const struct iio_poll_func *pf = p;
> +	struct iio_dev *indio_dev = pf->indio_dev;
> +	struct kxsd9_state *st = iio_priv(indio_dev);
> +	int ret;
> +	/* 4 * 16bit values AND timestamp */
> +	__be16 hw_values[8];
> +
> +	ret = regmap_bulk_read(st->map,
> +			       KXSD9_REG_X,
> +			       &hw_values,
> +			       8);
> +	if (ret) {
> +		dev_err(st->dev,
> +			"error reading data\n");
> +		return ret;
> +	}
> +
> +	iio_push_to_buffers_with_timestamp(indio_dev,
> +					   hw_values,
> +					   iio_get_time_ns(indio_dev));
> +	iio_trigger_notify_done(indio_dev->trig);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +#define KXSD9_ACCEL_CHAN(axis, index)						\
>  	{								\
>  		.type = IIO_ACCEL,					\
>  		.modified = 1,						\
> @@ -164,16 +195,35 @@ error_ret:
>  		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
>  					BIT(IIO_CHAN_INFO_OFFSET),	\
>  		.address = KXSD9_REG_##axis,				\
> +		.scan_index = index,					\
> +		.scan_type = {                                          \
> +			.sign = 'u',					\
> +			.realbits = 12,					\
> +			.storagebits = 16,				\
> +			.shift = 4,					\
> +			.endianness = IIO_BE,				\
> +		},							\
>  	}
>  
>  static const struct iio_chan_spec kxsd9_channels[] = {
> -	KXSD9_ACCEL_CHAN(X), KXSD9_ACCEL_CHAN(Y), KXSD9_ACCEL_CHAN(Z),
> +	KXSD9_ACCEL_CHAN(X, 0),
> +	KXSD9_ACCEL_CHAN(Y, 1),
> +	KXSD9_ACCEL_CHAN(Z, 2),
>  	{
>  		.type = IIO_VOLTAGE,
>  		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
>  		.indexed = 1,
>  		.address = KXSD9_REG_AUX,
> -	}
> +		.scan_index = 3,
> +		.scan_type = {
> +			.sign = 'u',
> +			.realbits = 12,
> +			.storagebits = 16,
> +			.shift = 4,
> +			.endianness = IIO_BE,
> +		},
> +	},
> +	IIO_CHAN_SOFT_TIMESTAMP(4),
>  };
>  
>  static const struct attribute_group kxsd9_attribute_group = {
> @@ -197,6 +247,9 @@ static const struct iio_info kxsd9_info = {
>  	.driver_module = THIS_MODULE,
>  };
>  
> +/* Four channels apart from timestamp, scan mask = 0x0f */
> +static const unsigned long kxsd9_scan_masks[] = { 0xf, 0 };
> +
>  int kxsd9_common_probe(struct device *parent,
>  		       struct regmap *map,
>  		       const char *name)
> @@ -210,6 +263,7 @@ int kxsd9_common_probe(struct device *parent,
>  		return -ENOMEM;
>  
>  	st = iio_priv(indio_dev);
> +	st->dev = parent;
>  	st->map = map;
>  
>  	indio_dev->channels = kxsd9_channels;
> @@ -218,13 +272,40 @@ int kxsd9_common_probe(struct device *parent,
>  	indio_dev->dev.parent = parent;
>  	indio_dev->info = &kxsd9_info;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->available_scan_masks = kxsd9_scan_masks;
>  
>  	kxsd9_power_up(st);
>  
> +	ret = iio_triggered_buffer_setup(indio_dev,
> +					 iio_pollfunc_store_time,
> +					 kxsd9_trigger_handler,
> +					 NULL);
> +	if (ret) {
> +		dev_err(parent, "triggered buffer setup failed\n");
> +		return ret;
> +	}
> +
>  	ret = devm_iio_device_register(parent, indio_dev);
>  	if (ret)
> -		return ret;
> +		goto err_cleanup_buffer;
> +
> +	dev_set_drvdata(parent, indio_dev);
>  
>  	return 0;
> +
> +err_cleanup_buffer:
> +	iio_triggered_buffer_cleanup(indio_dev);
> +
> +	return ret;
>  }
>  EXPORT_SYMBOL(kxsd9_common_probe);
> +
> +int kxsd9_common_remove(struct device *parent)
> +{
> +	struct iio_dev *indio_dev = dev_get_drvdata(parent);
> +
> +	iio_triggered_buffer_cleanup(indio_dev);
Race as you are removing the buffer whilst the userspaces
are still exposed.
Don't use devm_iio_device_register...

Jonathan
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(kxsd9_common_remove);
> diff --git a/drivers/iio/accel/kxsd9.h b/drivers/iio/accel/kxsd9.h
> index ef3dbbf70b98..19131a7a692c 100644
> --- a/drivers/iio/accel/kxsd9.h
> +++ b/drivers/iio/accel/kxsd9.h
> @@ -7,3 +7,4 @@
>  int kxsd9_common_probe(struct device *parent,
>  		       struct regmap *map,
>  		       const char *name);
> +int kxsd9_common_remove(struct device *parent);
> 

Re: [PATCH 12/17] iio: accel: kxsd9: Add triggered buffer handling

[Jonathan Cameron]

On 31/08/16 20:58, Jonathan Cameron wrote:
> On 16/08/16 14:33, Linus Walleij wrote:
>> As is custom with all modern sensors, add a clever burst mode
>> that will just stream out values from the sensor and provide it
>> to userspace to do the proper offsetting and scaling.
>>
>> This is the result when tested with an HRTimer trigger:
>>
>> $ generic_buffer -a -c 10 -n kxsd9 -t foo
>> /sys/bus/iio/devices/iio:device1 foo
>> 0.371318 0.718680 9.869872 1795.000000 97545896129
>> -0.586922 0.179670 9.378775 2398.000000 97555864721
>> -0.299450 0.179670 10.348992 2672.000000 97565874055
>> 0.371318 0.335384 11.103606 2816.000000 97575883240
>> 0.179670 0.574944 10.540640 2847.000000 97585862351
>> 0.335384 0.754614 9.953718 2840.000000 97595872425
>> 0.179670 0.754614 10.732288 2879.000000 97605882351
>> 0.000000 0.754614 10.348992 2872.000000 97615891832
>> -0.730658 0.574944 9.570422 2831.000000 97625871536
>> 0.000000 1.137910 10.732288 2872.000000 97635881610
>>
>> Columns shown are x, y, z acceleration, so a positive acceleration
>> of ~9.81 (shaky due to bad calibration) along the z axis. The
>> fourth column is the AUX IN which is floating on this system,
>> it seems to float up to the 2.85V VDD voltage.
>>
>> To be able to cleanup the triggered buffer, we need to add .remove()
>> callbacks to the I2C and SPI subdrivers and call back into an
>> exported .remove() callback in the core.
>>
>> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> One issue inline. Otherwise:

Hmm. bigger issue that I thought...

segfault on remove. Trace is:

[ 1173.843747] [<bf062008>] (iio_triggered_buffer_cleanup [industrialio_triggered_buffer]) from [<bf06600c>] (kxsd9_common_remove+0xc/0x14 [kxsd9])
[ 1173.856749] [<bf06600c>] (kxsd9_common_remove [kxsd9]) from [<c0203704>] (spi_drv_remove+0x1c/0x34)
[ 1173.865830] [<c0203704>] (spi_drv_remove) from [<c01dd6b0>] (__device_release_driver+0x94/0x10c)
[ 1173.874597] [<c01dd6b0>] (__device_release_driver) from [<c01dd89c>] (driver_detach+0x94/0xbc)
[ 1173.883188] [<c01dd89c>] (driver_detach) from [<c01dcbac>] (bus_remove_driver+0x64/0x90)
[ 1173.891277] [<c01dcbac>] (bus_remove_driver) from [<c007b39c>] (SyS_delete_module+0x164/0x1d8)
[ 1173.899921] [<c007b39c>] (SyS_delete_module) from [<c000f400>] (ret_fast_syscall+0x0/0x1c)
[ 1173.908237] Code: bad PC value
[ 1173.917616] ---[ end trace 2edcdd6d5c33ebd0 ]---

Not immediately sure why and running out of time this evening.

Bed time stories don't read themselves.

Jonathan
> 
> Tested-by: Jonathan Cameron <jic23@kernel.org>
> 
> Jonathan
>> ---
>>  drivers/iio/accel/Kconfig     |  2 +
>>  drivers/iio/accel/kxsd9-i2c.c |  6 +++
>>  drivers/iio/accel/kxsd9-spi.c |  6 +++
>>  drivers/iio/accel/kxsd9.c     | 93 ++++++++++++++++++++++++++++++++++++++++---
>>  drivers/iio/accel/kxsd9.h     |  1 +
>>  5 files changed, 102 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
>> index ab1c87ce07ed..cd69353989cf 100644
>> --- a/drivers/iio/accel/Kconfig
>> +++ b/drivers/iio/accel/Kconfig
>> @@ -96,6 +96,8 @@ config IIO_ST_ACCEL_SPI_3AXIS
>>  
>>  config KXSD9
>>  	tristate "Kionix KXSD9 Accelerometer Driver"
>> +	select IIO_BUFFER
>> +	select IIO_TRIGGERED_BUFFER
>>  	help
>>  	  Say yes here to build support for the Kionix KXSD9 accelerometer.
>>  	  It can be accessed using an (optional) SPI or I2C interface.
>> diff --git a/drivers/iio/accel/kxsd9-i2c.c b/drivers/iio/accel/kxsd9-i2c.c
>> index 2c910755288d..4aaa27d0aa32 100644
>> --- a/drivers/iio/accel/kxsd9-i2c.c
>> +++ b/drivers/iio/accel/kxsd9-i2c.c
>> @@ -30,6 +30,11 @@ static int kxsd9_i2c_probe(struct i2c_client *i2c,
>>  				  i2c->name);
>>  }
>>  
>> +static int kxsd9_i2c_remove(struct i2c_client *client)
>> +{
>> +	return kxsd9_common_remove(&client->dev);
>> +}
>> +
>>  #ifdef CONFIG_OF
>>  static const struct of_device_id kxsd9_of_match[] = {
>>  	{ .compatible = "kionix,kxsd9", },
>> @@ -52,6 +57,7 @@ static struct i2c_driver kxsd9_i2c_driver = {
>>  		.of_match_table = of_match_ptr(kxsd9_of_match),
>>  	},
>>  	.probe		= kxsd9_i2c_probe,
>> +	.remove		= kxsd9_i2c_remove,
>>  	.id_table	= kxsd9_i2c_id,
>>  };
>>  module_i2c_driver(kxsd9_i2c_driver);
>> diff --git a/drivers/iio/accel/kxsd9-spi.c b/drivers/iio/accel/kxsd9-spi.c
>> index 04dbc6f94e7d..c5af51b7dd7e 100644
>> --- a/drivers/iio/accel/kxsd9-spi.c
>> +++ b/drivers/iio/accel/kxsd9-spi.c
>> @@ -29,6 +29,11 @@ static int kxsd9_spi_probe(struct spi_device *spi)
>>  				  spi_get_device_id(spi)->name);
>>  }
>>  
>> +static int kxsd9_spi_remove(struct spi_device *spi)
>> +{
>> +	return kxsd9_common_remove(&spi->dev);
>> +}
>> +
>>  static const struct spi_device_id kxsd9_spi_id[] = {
>>  	{"kxsd9", 0},
>>  	{ },
>> @@ -40,6 +45,7 @@ static struct spi_driver kxsd9_spi_driver = {
>>  		.name = "kxsd9",
>>  	},
>>  	.probe = kxsd9_spi_probe,
>> +	.remove = kxsd9_spi_remove,
>>  	.id_table = kxsd9_spi_id,
>>  };
>>  module_spi_driver(kxsd9_spi_driver);
>> diff --git a/drivers/iio/accel/kxsd9.c b/drivers/iio/accel/kxsd9.c
>> index 152042ef3e3c..78afbe05710e 100644
>> --- a/drivers/iio/accel/kxsd9.c
>> +++ b/drivers/iio/accel/kxsd9.c
>> @@ -12,8 +12,6 @@
>>   * I have a suitable wire made up.
>>   *
>>   * TODO:	Support the motion detector
>> - *		Uses register address incrementing so could have a
>> - *		heavily optimized ring buffer access function.
>>   */
>>  
>>  #include <linux/device.h>
>> @@ -24,6 +22,9 @@
>>  #include <linux/regmap.h>
>>  #include <linux/iio/iio.h>
>>  #include <linux/iio/sysfs.h>
>> +#include <linux/iio/buffer.h>
>> +#include <linux/iio/triggered_buffer.h>
>> +#include <linux/iio/trigger_consumer.h>
>>  
>>  #include "kxsd9.h"
>>  
>> @@ -41,9 +42,11 @@
>>  
>>  /**
>>   * struct kxsd9_state - device related storage
>> + * @dev: pointer to the parent device
>>   * @map: regmap to the device
>>   */
>>  struct kxsd9_state {
>> +	struct device *dev;
>>  	struct regmap *map;
>>  };
>>  
>> @@ -155,7 +158,35 @@ static int kxsd9_read_raw(struct iio_dev *indio_dev,
>>  error_ret:
>>  	return ret;
>>  };
>> -#define KXSD9_ACCEL_CHAN(axis)						\
>> +
>> +static irqreturn_t kxsd9_trigger_handler(int irq, void *p)
>> +{
>> +	const struct iio_poll_func *pf = p;
>> +	struct iio_dev *indio_dev = pf->indio_dev;
>> +	struct kxsd9_state *st = iio_priv(indio_dev);
>> +	int ret;
>> +	/* 4 * 16bit values AND timestamp */
>> +	__be16 hw_values[8];
>> +
>> +	ret = regmap_bulk_read(st->map,
>> +			       KXSD9_REG_X,
>> +			       &hw_values,
>> +			       8);
>> +	if (ret) {
>> +		dev_err(st->dev,
>> +			"error reading data\n");
>> +		return ret;
>> +	}
>> +
>> +	iio_push_to_buffers_with_timestamp(indio_dev,
>> +					   hw_values,
>> +					   iio_get_time_ns(indio_dev));
>> +	iio_trigger_notify_done(indio_dev->trig);
>> +
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +#define KXSD9_ACCEL_CHAN(axis, index)						\
>>  	{								\
>>  		.type = IIO_ACCEL,					\
>>  		.modified = 1,						\
>> @@ -164,16 +195,35 @@ error_ret:
>>  		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
>>  					BIT(IIO_CHAN_INFO_OFFSET),	\
>>  		.address = KXSD9_REG_##axis,				\
>> +		.scan_index = index,					\
>> +		.scan_type = {                                          \
>> +			.sign = 'u',					\
>> +			.realbits = 12,					\
>> +			.storagebits = 16,				\
>> +			.shift = 4,					\
>> +			.endianness = IIO_BE,				\
>> +		},							\
>>  	}
>>  
>>  static const struct iio_chan_spec kxsd9_channels[] = {
>> -	KXSD9_ACCEL_CHAN(X), KXSD9_ACCEL_CHAN(Y), KXSD9_ACCEL_CHAN(Z),
>> +	KXSD9_ACCEL_CHAN(X, 0),
>> +	KXSD9_ACCEL_CHAN(Y, 1),
>> +	KXSD9_ACCEL_CHAN(Z, 2),
>>  	{
>>  		.type = IIO_VOLTAGE,
>>  		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
>>  		.indexed = 1,
>>  		.address = KXSD9_REG_AUX,
>> -	}
>> +		.scan_index = 3,
>> +		.scan_type = {
>> +			.sign = 'u',
>> +			.realbits = 12,
>> +			.storagebits = 16,
>> +			.shift = 4,
>> +			.endianness = IIO_BE,
>> +		},
>> +	},
>> +	IIO_CHAN_SOFT_TIMESTAMP(4),
>>  };
>>  
>>  static const struct attribute_group kxsd9_attribute_group = {
>> @@ -197,6 +247,9 @@ static const struct iio_info kxsd9_info = {
>>  	.driver_module = THIS_MODULE,
>>  };
>>  
>> +/* Four channels apart from timestamp, scan mask = 0x0f */
>> +static const unsigned long kxsd9_scan_masks[] = { 0xf, 0 };
>> +
>>  int kxsd9_common_probe(struct device *parent,
>>  		       struct regmap *map,
>>  		       const char *name)
>> @@ -210,6 +263,7 @@ int kxsd9_common_probe(struct device *parent,
>>  		return -ENOMEM;
>>  
>>  	st = iio_priv(indio_dev);
>> +	st->dev = parent;
>>  	st->map = map;
>>  
>>  	indio_dev->channels = kxsd9_channels;
>> @@ -218,13 +272,40 @@ int kxsd9_common_probe(struct device *parent,
>>  	indio_dev->dev.parent = parent;
>>  	indio_dev->info = &kxsd9_info;
>>  	indio_dev->modes = INDIO_DIRECT_MODE;
>> +	indio_dev->available_scan_masks = kxsd9_scan_masks;
>>  
>>  	kxsd9_power_up(st);
>>  
>> +	ret = iio_triggered_buffer_setup(indio_dev,
>> +					 iio_pollfunc_store_time,
>> +					 kxsd9_trigger_handler,
>> +					 NULL);
>> +	if (ret) {
>> +		dev_err(parent, "triggered buffer setup failed\n");
>> +		return ret;
>> +	}
>> +
>>  	ret = devm_iio_device_register(parent, indio_dev);
>>  	if (ret)
>> -		return ret;
>> +		goto err_cleanup_buffer;
>> +
>> +	dev_set_drvdata(parent, indio_dev);
>>  
>>  	return 0;
>> +
>> +err_cleanup_buffer:
>> +	iio_triggered_buffer_cleanup(indio_dev);
>> +
>> +	return ret;
>>  }
>>  EXPORT_SYMBOL(kxsd9_common_probe);
>> +
>> +int kxsd9_common_remove(struct device *parent)
>> +{
>> +	struct iio_dev *indio_dev = dev_get_drvdata(parent);
>> +
>> +	iio_triggered_buffer_cleanup(indio_dev);
> Race as you are removing the buffer whilst the userspaces
> are still exposed.
> Don't use devm_iio_device_register...
> 
> Jonathan
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL(kxsd9_common_remove);
>> diff --git a/drivers/iio/accel/kxsd9.h b/drivers/iio/accel/kxsd9.h
>> index ef3dbbf70b98..19131a7a692c 100644
>> --- a/drivers/iio/accel/kxsd9.h
>> +++ b/drivers/iio/accel/kxsd9.h
>> @@ -7,3 +7,4 @@
>>  int kxsd9_common_probe(struct device *parent,
>>  		       struct regmap *map,
>>  		       const char *name);
>> +int kxsd9_common_remove(struct device *parent);
>>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[PATCH 13/17] iio: accel: kxsd9: Deploy proper register bit defines

[Linus Walleij]

There are some hardcoded register values etc in the code, define
proper bitfield definitions, and use them when getting and setting
the scale. Optimize a read/modify/write to use regmap_update_bits()
at the same time.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/iio/accel/kxsd9.c | 36 +++++++++++++++++++++++++++---------
 1 file changed, 27 insertions(+), 9 deletions(-)

diff --git a/drivers/iio/accel/kxsd9.c b/drivers/iio/accel/kxsd9.c
index 78afbe05710e..93be5e1f8968 100644
--- a/drivers/iio/accel/kxsd9.c
+++ b/drivers/iio/accel/kxsd9.c
@@ -20,6 +20,7 @@
 #include <linux/slab.h>
 #include <linux/module.h>
 #include <linux/regmap.h>
+#include <linux/bitops.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
 #include <linux/iio/buffer.h>
@@ -35,9 +36,29 @@
 #define KXSD9_REG_RESET		0x0a
 #define KXSD9_REG_CTRL_C	0x0c
 
-#define KXSD9_FS_MASK		0x03
+#define KXSD9_CTRL_C_FS_MASK	0x03
+#define KXSD9_CTRL_C_FS_8G	0x00
+#define KXSD9_CTRL_C_FS_6G	0x01
+#define KXSD9_CTRL_C_FS_4G	0x02
+#define KXSD9_CTRL_C_FS_2G	0x03
+#define KXSD9_CTRL_C_MOT_LAT	BIT(3)
+#define KXSD9_CTRL_C_MOT_LEV	BIT(4)
+#define KXSD9_CTRL_C_LP_MASK	0xe0
+#define KXSD9_CTRL_C_LP_NONE	0x00
+#define KXSD9_CTRL_C_LP_2000HZC	BIT(5)
+#define KXSD9_CTRL_C_LP_2000HZB	BIT(6)
+#define KXSD9_CTRL_C_LP_2000HZA	(BIT(5)|BIT(6))
+#define KXSD9_CTRL_C_LP_1000HZ	BIT(7)
+#define KXSD9_CTRL_C_LP_500HZ	(BIT(7)|BIT(5))
+#define KXSD9_CTRL_C_LP_100HZ	(BIT(7)|BIT(6))
+#define KXSD9_CTRL_C_LP_50HZ	(BIT(7)|BIT(6)|BIT(5))
 
 #define KXSD9_REG_CTRL_B	0x0d
+
+#define KXSD9_CTRL_B_CLK_HLD	BIT(7)
+#define KXSD9_CTRL_B_ENABLE	BIT(6)
+#define KXSD9_CTRL_B_ST		BIT(5) /* Self-test */
+
 #define KXSD9_REG_CTRL_A	0x0e
 
 /**
@@ -65,7 +86,6 @@ static int kxsd9_write_scale(struct iio_dev *indio_dev, int micro)
 	int ret, i;
 	struct kxsd9_state *st = iio_priv(indio_dev);
 	bool foundit = false;
-	unsigned int val;
 
 	for (i = 0; i < 4; i++)
 		if (micro == kxsd9_micro_scales[i]) {
@@ -75,14 +95,12 @@ static int kxsd9_write_scale(struct iio_dev *indio_dev, int micro)
 	if (!foundit)
 		return -EINVAL;
 
-	ret = regmap_read(st->map,
-			  KXSD9_REG_CTRL_C,
-			  &val);
+	ret = regmap_update_bits(st->map,
+				 KXSD9_REG_CTRL_C,
+				 KXSD9_CTRL_C_FS_MASK,
+				 i);
 	if (ret < 0)
 		goto error_ret;
-	ret = regmap_write(st->map,
-			   KXSD9_REG_CTRL_C,
-			   (val & ~KXSD9_FS_MASK) | i);
 error_ret:
 	return ret;
 }
@@ -150,7 +168,7 @@ static int kxsd9_read_raw(struct iio_dev *indio_dev,
 		if (ret < 0)
 			goto error_ret;
 		*val = 0;
-		*val2 = kxsd9_micro_scales[regval & KXSD9_FS_MASK];
+		*val2 = kxsd9_micro_scales[regval & KXSD9_CTRL_C_FS_MASK];
 		ret = IIO_VAL_INT_PLUS_MICRO;
 		break;
 	}
-- 
2.7.4

Re: [PATCH 13/17] iio: accel: kxsd9: Deploy proper register bit defines

[Jonathan Cameron]

On 16/08/16 14:33, Linus Walleij wrote:
> There are some hardcoded register values etc in the code, define
> proper bitfield definitions, and use them when getting and setting
> the scale. Optimize a read/modify/write to use regmap_update_bits()
> at the same time.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Tested this one, but the rest are trivial and with that crash on rmmod
skulking around the reboot cycle on this board means I'm not going
to try them until that's fixed up.

Thanks Linus, mostly a pretty good set (though you 'could' have
done regmap at the start and probably made it a shorter series ;)

Jonathan
> ---
>  drivers/iio/accel/kxsd9.c | 36 +++++++++++++++++++++++++++---------
>  1 file changed, 27 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/iio/accel/kxsd9.c b/drivers/iio/accel/kxsd9.c
> index 78afbe05710e..93be5e1f8968 100644
> --- a/drivers/iio/accel/kxsd9.c
> +++ b/drivers/iio/accel/kxsd9.c
> @@ -20,6 +20,7 @@
>  #include <linux/slab.h>
>  #include <linux/module.h>
>  #include <linux/regmap.h>
> +#include <linux/bitops.h>
>  #include <linux/iio/iio.h>
>  #include <linux/iio/sysfs.h>
>  #include <linux/iio/buffer.h>
> @@ -35,9 +36,29 @@
>  #define KXSD9_REG_RESET		0x0a
>  #define KXSD9_REG_CTRL_C	0x0c
>  
> -#define KXSD9_FS_MASK		0x03
> +#define KXSD9_CTRL_C_FS_MASK	0x03
> +#define KXSD9_CTRL_C_FS_8G	0x00
> +#define KXSD9_CTRL_C_FS_6G	0x01
> +#define KXSD9_CTRL_C_FS_4G	0x02
> +#define KXSD9_CTRL_C_FS_2G	0x03
> +#define KXSD9_CTRL_C_MOT_LAT	BIT(3)
> +#define KXSD9_CTRL_C_MOT_LEV	BIT(4)
> +#define KXSD9_CTRL_C_LP_MASK	0xe0
> +#define KXSD9_CTRL_C_LP_NONE	0x00
> +#define KXSD9_CTRL_C_LP_2000HZC	BIT(5)
> +#define KXSD9_CTRL_C_LP_2000HZB	BIT(6)
> +#define KXSD9_CTRL_C_LP_2000HZA	(BIT(5)|BIT(6))
> +#define KXSD9_CTRL_C_LP_1000HZ	BIT(7)
> +#define KXSD9_CTRL_C_LP_500HZ	(BIT(7)|BIT(5))
> +#define KXSD9_CTRL_C_LP_100HZ	(BIT(7)|BIT(6))
> +#define KXSD9_CTRL_C_LP_50HZ	(BIT(7)|BIT(6)|BIT(5))
>  
>  #define KXSD9_REG_CTRL_B	0x0d
> +
> +#define KXSD9_CTRL_B_CLK_HLD	BIT(7)
> +#define KXSD9_CTRL_B_ENABLE	BIT(6)
> +#define KXSD9_CTRL_B_ST		BIT(5) /* Self-test */
> +
>  #define KXSD9_REG_CTRL_A	0x0e
>  
>  /**
> @@ -65,7 +86,6 @@ static int kxsd9_write_scale(struct iio_dev *indio_dev, int micro)
>  	int ret, i;
>  	struct kxsd9_state *st = iio_priv(indio_dev);
>  	bool foundit = false;
> -	unsigned int val;
>  
>  	for (i = 0; i < 4; i++)
>  		if (micro == kxsd9_micro_scales[i]) {
> @@ -75,14 +95,12 @@ static int kxsd9_write_scale(struct iio_dev *indio_dev, int micro)
>  	if (!foundit)
>  		return -EINVAL;
>  
> -	ret = regmap_read(st->map,
> -			  KXSD9_REG_CTRL_C,
> -			  &val);
> +	ret = regmap_update_bits(st->map,
> +				 KXSD9_REG_CTRL_C,
> +				 KXSD9_CTRL_C_FS_MASK,
> +				 i);
>  	if (ret < 0)
>  		goto error_ret;
> -	ret = regmap_write(st->map,
> -			   KXSD9_REG_CTRL_C,
> -			   (val & ~KXSD9_FS_MASK) | i);
>  error_ret:
>  	return ret;
>  }
> @@ -150,7 +168,7 @@ static int kxsd9_read_raw(struct iio_dev *indio_dev,
>  		if (ret < 0)
>  			goto error_ret;
>  		*val = 0;
> -		*val2 = kxsd9_micro_scales[regval & KXSD9_FS_MASK];
> +		*val2 = kxsd9_micro_scales[regval & KXSD9_CTRL_C_FS_MASK];
>  		ret = IIO_VAL_INT_PLUS_MICRO;
>  		break;
>  	}
> 

[PATCH 14/17] iio: accel: kxsd9: Fetch and handle regulators

[Linus Walleij]

This adds supply regulator handling for the VDD and IOVDD inputs
on the KXSD9 component, makes sure to bring the regulators online
during probe and disable them on remove or the errorpath.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/iio/accel/kxsd9.c | 88 +++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 85 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/accel/kxsd9.c b/drivers/iio/accel/kxsd9.c
index 93be5e1f8968..e169f40311af 100644
--- a/drivers/iio/accel/kxsd9.c
+++ b/drivers/iio/accel/kxsd9.c
@@ -21,6 +21,8 @@
 #include <linux/module.h>
 #include <linux/regmap.h>
 #include <linux/bitops.h>
+#include <linux/delay.h>
+#include <linux/regulator/consumer.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
 #include <linux/iio/buffer.h>
@@ -65,10 +67,12 @@
  * struct kxsd9_state - device related storage
  * @dev: pointer to the parent device
  * @map: regmap to the device
+ * @regs: regulators for this device, VDD and IOVDD
  */
 struct kxsd9_state {
 	struct device *dev;
 	struct regmap *map;
+	struct regulator_bulk_data regs[2];
 };
 
 #define KXSD9_SCALE_2G "0.011978"
@@ -81,6 +85,12 @@ static const int kxsd9_micro_scales[4] = { 47853, 35934, 23927, 11978 };
 
 #define KXSD9_ZERO_G_OFFSET -2048
 
+/*
+ * Regulator names
+ */
+static const char kxsd9_reg_vdd[] = "vdd";
+static const char kxsd9_reg_iovdd[] = "iovdd";
+
 static int kxsd9_write_scale(struct iio_dev *indio_dev, int micro)
 {
 	int ret, i;
@@ -252,12 +262,69 @@ static int kxsd9_power_up(struct kxsd9_state *st)
 {
 	int ret;
 
-	ret = regmap_write(st->map, KXSD9_REG_CTRL_B, 0x40);
+	/* Enable the regulators */
+	ret = regulator_bulk_enable(ARRAY_SIZE(st->regs), st->regs);
+	if (ret) {
+		dev_err(st->dev, "Cannot enable regulators\n");
+		return ret;
+	}
+
+	/* Power up */
+	ret = regmap_write(st->map,
+			   KXSD9_REG_CTRL_B,
+			   KXSD9_CTRL_B_ENABLE);
 	if (ret)
 		return ret;
-	return regmap_write(st->map, KXSD9_REG_CTRL_C, 0x9b);
+
+	/*
+	 * Set 1000Hz LPF, 2g fullscale, motion wakeup threshold 1g,
+	 * latched wakeup
+	 */
+	ret = regmap_write(st->map,
+			   KXSD9_REG_CTRL_C,
+			   KXSD9_CTRL_C_LP_1000HZ |
+			   KXSD9_CTRL_C_MOT_LEV	|
+			   KXSD9_CTRL_C_MOT_LAT |
+			   KXSD9_CTRL_C_FS_2G);
+	if (ret)
+		return ret;
+
+	/*
+	 * Power-up time depends on the LPF setting, but typ 15.9 ms, let's
+	 * set 20 ms to allow for some slack.
+	 */
+	msleep(20);
+
+	return 0;
 };
 
+static int kxsd9_power_down(struct kxsd9_state *st)
+{
+	int ret;
+
+	/*
+	 * Set into low power mode - since there may be more users of the
+	 * regulators this is the first step of the power saving: it will
+	 * make sure we conserve power even if there are others users on the
+	 * regulators.
+	 */
+	ret = regmap_update_bits(st->map,
+				 KXSD9_REG_CTRL_B,
+				 KXSD9_CTRL_B_ENABLE,
+				 0);
+	if (ret)
+		return ret;
+
+	/* Disable the regulators */
+	ret = regulator_bulk_disable(ARRAY_SIZE(st->regs), st->regs);
+	if (ret) {
+		dev_err(st->dev, "Cannot disable regulators\n");
+		return ret;
+	}
+
+	return 0;
+}
+
 static const struct iio_info kxsd9_info = {
 	.read_raw = &kxsd9_read_raw,
 	.write_raw = &kxsd9_write_raw,
@@ -292,6 +359,17 @@ int kxsd9_common_probe(struct device *parent,
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->available_scan_masks = kxsd9_scan_masks;
 
+	/* Fetch and turn on regulators */
+	st->regs[0].supply = kxsd9_reg_vdd;
+	st->regs[1].supply = kxsd9_reg_iovdd;
+	ret = devm_regulator_bulk_get(parent,
+				      ARRAY_SIZE(st->regs),
+				      st->regs);
+	if (ret) {
+		dev_err(parent, "Cannot get regulators\n");
+		return ret;
+	}
+
 	kxsd9_power_up(st);
 
 	ret = iio_triggered_buffer_setup(indio_dev,
@@ -300,7 +378,7 @@ int kxsd9_common_probe(struct device *parent,
 					 NULL);
 	if (ret) {
 		dev_err(parent, "triggered buffer setup failed\n");
-		return ret;
+		goto err_power_down;
 	}
 
 	ret = devm_iio_device_register(parent, indio_dev);
@@ -313,6 +391,8 @@ int kxsd9_common_probe(struct device *parent,
 
 err_cleanup_buffer:
 	iio_triggered_buffer_cleanup(indio_dev);
+err_power_down:
+	kxsd9_power_down(st);
 
 	return ret;
 }
@@ -321,8 +401,10 @@ EXPORT_SYMBOL(kxsd9_common_probe);
 int kxsd9_common_remove(struct device *parent)
 {
 	struct iio_dev *indio_dev = dev_get_drvdata(parent);
+	struct kxsd9_state *st = iio_priv(indio_dev);
 
 	iio_triggered_buffer_cleanup(indio_dev);
+	kxsd9_power_down(st);
 
 	return 0;
 }
-- 
2.7.4

[PATCH 15/17] iio: accel: kxsd9: Replace "parent" with "dev"

[Linus Walleij]

What is passed to the .probe() and .remove() functions is
technically the parent of the created IIO device but it becomes
a big confusion for the head to have it named like this since
it is usually clear from context the "dev" refers to the physical
device, and when next adding PM callbacks a clean
"struct device *dev" pointer is passed to these and that makes
it even more confused. Rename "parent" to "dev" like in most
other drivers.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/iio/accel/kxsd9.c | 22 +++++++++++-----------
 drivers/iio/accel/kxsd9.h |  4 ++--
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/iio/accel/kxsd9.c b/drivers/iio/accel/kxsd9.c
index e169f40311af..1b1b7cdbe627 100644
--- a/drivers/iio/accel/kxsd9.c
+++ b/drivers/iio/accel/kxsd9.c
@@ -335,7 +335,7 @@ static const struct iio_info kxsd9_info = {
 /* Four channels apart from timestamp, scan mask = 0x0f */
 static const unsigned long kxsd9_scan_masks[] = { 0xf, 0 };
 
-int kxsd9_common_probe(struct device *parent,
+int kxsd9_common_probe(struct device *dev,
 		       struct regmap *map,
 		       const char *name)
 {
@@ -343,18 +343,18 @@ int kxsd9_common_probe(struct device *parent,
 	struct kxsd9_state *st;
 	int ret;
 
-	indio_dev = devm_iio_device_alloc(parent, sizeof(*st));
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
 	if (!indio_dev)
 		return -ENOMEM;
 
 	st = iio_priv(indio_dev);
-	st->dev = parent;
+	st->dev = dev;
 	st->map = map;
 
 	indio_dev->channels = kxsd9_channels;
 	indio_dev->num_channels = ARRAY_SIZE(kxsd9_channels);
 	indio_dev->name = name;
-	indio_dev->dev.parent = parent;
+	indio_dev->dev.parent = dev;
 	indio_dev->info = &kxsd9_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->available_scan_masks = kxsd9_scan_masks;
@@ -362,11 +362,11 @@ int kxsd9_common_probe(struct device *parent,
 	/* Fetch and turn on regulators */
 	st->regs[0].supply = kxsd9_reg_vdd;
 	st->regs[1].supply = kxsd9_reg_iovdd;
-	ret = devm_regulator_bulk_get(parent,
+	ret = devm_regulator_bulk_get(dev,
 				      ARRAY_SIZE(st->regs),
 				      st->regs);
 	if (ret) {
-		dev_err(parent, "Cannot get regulators\n");
+		dev_err(dev, "Cannot get regulators\n");
 		return ret;
 	}
 
@@ -377,15 +377,15 @@ int kxsd9_common_probe(struct device *parent,
 					 kxsd9_trigger_handler,
 					 NULL);
 	if (ret) {
-		dev_err(parent, "triggered buffer setup failed\n");
+		dev_err(dev, "triggered buffer setup failed\n");
 		goto err_power_down;
 	}
 
-	ret = devm_iio_device_register(parent, indio_dev);
+	ret = devm_iio_device_register(dev, indio_dev);
 	if (ret)
 		goto err_cleanup_buffer;
 
-	dev_set_drvdata(parent, indio_dev);
+	dev_set_drvdata(dev, indio_dev);
 
 	return 0;
 
@@ -398,9 +398,9 @@ err_power_down:
 }
 EXPORT_SYMBOL(kxsd9_common_probe);
 
-int kxsd9_common_remove(struct device *parent)
+int kxsd9_common_remove(struct device *dev)
 {
-	struct iio_dev *indio_dev = dev_get_drvdata(parent);
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct kxsd9_state *st = iio_priv(indio_dev);
 
 	iio_triggered_buffer_cleanup(indio_dev);
diff --git a/drivers/iio/accel/kxsd9.h b/drivers/iio/accel/kxsd9.h
index 19131a7a692c..9c0861f6b838 100644
--- a/drivers/iio/accel/kxsd9.h
+++ b/drivers/iio/accel/kxsd9.h
@@ -4,7 +4,7 @@
 #define KXSD9_STATE_RX_SIZE 2
 #define KXSD9_STATE_TX_SIZE 2
 
-int kxsd9_common_probe(struct device *parent,
+int kxsd9_common_probe(struct device *dev,
 		       struct regmap *map,
 		       const char *name);
-int kxsd9_common_remove(struct device *parent);
+int kxsd9_common_remove(struct device *dev);
-- 
2.7.4

[PATCH 16/17] iio: accel: kxsd9: Deploy system and runtime PM

[Linus Walleij]

This deploys runtime and system PM in the KXSD9 driver:

- Use the force_runtime_suspend/resume callbacks as system PM
  operations.

- Add buffer prepare/unprepare callbacks to grab the runtime
  PM while we're using buffered reads and put get/put_autosuspend
  in these.

- Insert get/put_autosuspend calls anywhere the IO is used from
  the raw read/write callbacks.

- Move the fullscale setting to be cached in the state container
  so we can restore it properly when coming back from
  system/runtime suspend.

- Set the autosuspend delay to two orders of magnitude that of
  the sensor start-up time (20ms) so we will autosuspend after
  2s.

- Register the callbacks in both the SPI and I2C subdrivers.

Tested with the I2C KXSD9 on the Qualcomm APQ8060 Dragonboard.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/iio/accel/kxsd9-i2c.c |  1 +
 drivers/iio/accel/kxsd9-spi.c |  1 +
 drivers/iio/accel/kxsd9.c     | 92 ++++++++++++++++++++++++++++++++++++++++++-
 drivers/iio/accel/kxsd9.h     |  2 +
 4 files changed, 94 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/accel/kxsd9-i2c.c b/drivers/iio/accel/kxsd9-i2c.c
index 4aaa27d0aa32..95e20855d2ef 100644
--- a/drivers/iio/accel/kxsd9-i2c.c
+++ b/drivers/iio/accel/kxsd9-i2c.c
@@ -55,6 +55,7 @@ static struct i2c_driver kxsd9_i2c_driver = {
 	.driver = {
 		.name	= "kxsd9",
 		.of_match_table = of_match_ptr(kxsd9_of_match),
+		.pm = &kxsd9_dev_pm_ops,
 	},
 	.probe		= kxsd9_i2c_probe,
 	.remove		= kxsd9_i2c_remove,
diff --git a/drivers/iio/accel/kxsd9-spi.c b/drivers/iio/accel/kxsd9-spi.c
index c5af51b7dd7e..b7d0078fd00e 100644
--- a/drivers/iio/accel/kxsd9-spi.c
+++ b/drivers/iio/accel/kxsd9-spi.c
@@ -43,6 +43,7 @@ MODULE_DEVICE_TABLE(spi, kxsd9_spi_id);
 static struct spi_driver kxsd9_spi_driver = {
 	.driver = {
 		.name = "kxsd9",
+		.pm = &kxsd9_dev_pm_ops,
 	},
 	.probe = kxsd9_spi_probe,
 	.remove = kxsd9_spi_remove,
diff --git a/drivers/iio/accel/kxsd9.c b/drivers/iio/accel/kxsd9.c
index 1b1b7cdbe627..ce866ef1ab70 100644
--- a/drivers/iio/accel/kxsd9.c
+++ b/drivers/iio/accel/kxsd9.c
@@ -23,6 +23,7 @@
 #include <linux/bitops.h>
 #include <linux/delay.h>
 #include <linux/regulator/consumer.h>
+#include <linux/pm_runtime.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
 #include <linux/iio/buffer.h>
@@ -68,11 +69,13 @@
  * @dev: pointer to the parent device
  * @map: regmap to the device
  * @regs: regulators for this device, VDD and IOVDD
+ * @scale: the current scaling setting
  */
 struct kxsd9_state {
 	struct device *dev;
 	struct regmap *map;
 	struct regulator_bulk_data regs[2];
+	u8 scale;
 };
 
 #define KXSD9_SCALE_2G "0.011978"
@@ -111,6 +114,10 @@ static int kxsd9_write_scale(struct iio_dev *indio_dev, int micro)
 				 i);
 	if (ret < 0)
 		goto error_ret;
+
+	/* Cached scale when the sensor is powered down */
+	st->scale = i;
+
 error_ret:
 	return ret;
 }
@@ -133,6 +140,9 @@ static int kxsd9_write_raw(struct iio_dev *indio_dev,
 			   long mask)
 {
 	int ret = -EINVAL;
+	struct kxsd9_state *st = iio_priv(indio_dev);
+
+	pm_runtime_get_sync(st->dev);
 
 	if (mask == IIO_CHAN_INFO_SCALE) {
 		/* Check no integer component */
@@ -141,6 +151,9 @@ static int kxsd9_write_raw(struct iio_dev *indio_dev,
 		ret = kxsd9_write_scale(indio_dev, val2);
 	}
 
+	pm_runtime_mark_last_busy(st->dev);
+	pm_runtime_put_autosuspend(st->dev);
+
 	return ret;
 }
 
@@ -154,6 +167,8 @@ static int kxsd9_read_raw(struct iio_dev *indio_dev,
 	__be16 raw_val;
 	u16 nval;
 
+	pm_runtime_get_sync(st->dev);
+
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
 		ret = regmap_bulk_read(st->map, chan->address, &raw_val,
@@ -184,6 +199,9 @@ static int kxsd9_read_raw(struct iio_dev *indio_dev,
 	}
 
 error_ret:
+	pm_runtime_mark_last_busy(st->dev);
+	pm_runtime_put_autosuspend(st->dev);
+
 	return ret;
 };
 
@@ -214,6 +232,32 @@ static irqreturn_t kxsd9_trigger_handler(int irq, void *p)
 	return IRQ_HANDLED;
 }
 
+static int kxsd9_buffer_preenable(struct iio_dev *indio_dev)
+{
+	struct kxsd9_state *st = iio_priv(indio_dev);
+
+	pm_runtime_get_sync(st->dev);
+
+	return 0;
+}
+
+static int kxsd9_buffer_postdisable(struct iio_dev *indio_dev)
+{
+	struct kxsd9_state *st = iio_priv(indio_dev);
+
+	pm_runtime_mark_last_busy(st->dev);
+	pm_runtime_put_autosuspend(st->dev);
+
+	return 0;
+}
+
+static const struct iio_buffer_setup_ops kxsd9_buffer_setup_ops = {
+	.preenable = kxsd9_buffer_preenable,
+	.postenable = iio_triggered_buffer_postenable,
+	.predisable = iio_triggered_buffer_predisable,
+	.postdisable = kxsd9_buffer_postdisable,
+};
+
 #define KXSD9_ACCEL_CHAN(axis, index)						\
 	{								\
 		.type = IIO_ACCEL,					\
@@ -285,7 +329,7 @@ static int kxsd9_power_up(struct kxsd9_state *st)
 			   KXSD9_CTRL_C_LP_1000HZ |
 			   KXSD9_CTRL_C_MOT_LEV	|
 			   KXSD9_CTRL_C_MOT_LAT |
-			   KXSD9_CTRL_C_FS_2G);
+			   st->scale);
 	if (ret)
 		return ret;
 
@@ -369,13 +413,15 @@ int kxsd9_common_probe(struct device *dev,
 		dev_err(dev, "Cannot get regulators\n");
 		return ret;
 	}
+	/* Default scaling */
+	st->scale = KXSD9_CTRL_C_FS_2G;
 
 	kxsd9_power_up(st);
 
 	ret = iio_triggered_buffer_setup(indio_dev,
 					 iio_pollfunc_store_time,
 					 kxsd9_trigger_handler,
-					 NULL);
+					 &kxsd9_buffer_setup_ops);
 	if (ret) {
 		dev_err(dev, "triggered buffer setup failed\n");
 		goto err_power_down;
@@ -387,6 +433,19 @@ int kxsd9_common_probe(struct device *dev,
 
 	dev_set_drvdata(dev, indio_dev);
 
+	/* Enable runtime PM */
+	pm_runtime_get_noresume(dev);
+	pm_runtime_set_active(dev);
+	pm_runtime_enable(dev);
+	/*
+	 * Set autosuspend to two orders of magnitude larger than the
+	 * start-up time. 20ms start-up time means 2000ms autosuspend,
+	 * i.e. 2 seconds.
+	 */
+	pm_runtime_set_autosuspend_delay(dev, 2000);
+	pm_runtime_use_autosuspend(dev);
+	pm_runtime_put(dev);
+
 	return 0;
 
 err_cleanup_buffer:
@@ -404,8 +463,37 @@ int kxsd9_common_remove(struct device *dev)
 	struct kxsd9_state *st = iio_priv(indio_dev);
 
 	iio_triggered_buffer_cleanup(indio_dev);
+	pm_runtime_get_sync(dev);
+	pm_runtime_put_noidle(dev);
+	pm_runtime_disable(dev);
 	kxsd9_power_down(st);
 
 	return 0;
 }
 EXPORT_SYMBOL(kxsd9_common_remove);
+
+#ifdef CONFIG_PM
+static int kxsd9_runtime_suspend(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct kxsd9_state *st = iio_priv(indio_dev);
+
+	return kxsd9_power_down(st);
+}
+
+static int kxsd9_runtime_resume(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct kxsd9_state *st = iio_priv(indio_dev);
+
+	return kxsd9_power_up(st);
+}
+#endif /* CONFIG_PM */
+
+const struct dev_pm_ops kxsd9_dev_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+				pm_runtime_force_resume)
+	SET_RUNTIME_PM_OPS(kxsd9_runtime_suspend,
+			   kxsd9_runtime_resume, NULL)
+};
+EXPORT_SYMBOL(kxsd9_dev_pm_ops);
diff --git a/drivers/iio/accel/kxsd9.h b/drivers/iio/accel/kxsd9.h
index 9c0861f6b838..7e8a28168310 100644
--- a/drivers/iio/accel/kxsd9.h
+++ b/drivers/iio/accel/kxsd9.h
@@ -8,3 +8,5 @@ int kxsd9_common_probe(struct device *dev,
 		       struct regmap *map,
 		       const char *name);
 int kxsd9_common_remove(struct device *dev);
+
+extern const struct dev_pm_ops kxsd9_dev_pm_ops;
-- 
2.7.4

[PATCH 17/17] iio: accel: kxsd9: Support reading a mounting matrix

[Linus Walleij]

This adds support for the mounting matrix to the KXSD9 driver.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/iio/accel/kxsd9.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/iio/accel/kxsd9.c b/drivers/iio/accel/kxsd9.c
index ce866ef1ab70..0c0d456cff8a 100644
--- a/drivers/iio/accel/kxsd9.c
+++ b/drivers/iio/accel/kxsd9.c
@@ -68,12 +68,14 @@
  * struct kxsd9_state - device related storage
  * @dev: pointer to the parent device
  * @map: regmap to the device
+ * @orientation: mounting matrix, flipped axis etc
  * @regs: regulators for this device, VDD and IOVDD
  * @scale: the current scaling setting
  */
 struct kxsd9_state {
 	struct device *dev;
 	struct regmap *map;
+	struct iio_mount_matrix orientation;
 	struct regulator_bulk_data regs[2];
 	u8 scale;
 };
@@ -258,6 +260,20 @@ static const struct iio_buffer_setup_ops kxsd9_buffer_setup_ops = {
 	.postdisable = kxsd9_buffer_postdisable,
 };
 
+static const struct iio_mount_matrix *
+kxsd9_get_mount_matrix(const struct iio_dev *indio_dev,
+		       const struct iio_chan_spec *chan)
+{
+	struct kxsd9_state *st = iio_priv(indio_dev);
+
+	return &st->orientation;
+}
+
+static const struct iio_chan_spec_ext_info kxsd9_ext_info[] = {
+	IIO_MOUNT_MATRIX(IIO_SHARED_BY_TYPE, kxsd9_get_mount_matrix),
+	{ },
+};
+
 #define KXSD9_ACCEL_CHAN(axis, index)						\
 	{								\
 		.type = IIO_ACCEL,					\
@@ -266,6 +282,7 @@ static const struct iio_buffer_setup_ops kxsd9_buffer_setup_ops = {
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
 		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
 					BIT(IIO_CHAN_INFO_OFFSET),	\
+		.ext_info = kxsd9_ext_info,				\
 		.address = KXSD9_REG_##axis,				\
 		.scan_index = index,					\
 		.scan_type = {                                          \
@@ -403,6 +420,13 @@ int kxsd9_common_probe(struct device *dev,
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->available_scan_masks = kxsd9_scan_masks;
 
+	/* Read the mounting matrix, if present */
+	ret = of_iio_read_mount_matrix(dev,
+				       "mount-matrix",
+				       &st->orientation);
+	if (ret)
+		return ret;
+
 	/* Fetch and turn on regulators */
 	st->regs[0].supply = kxsd9_reg_vdd;
 	st->regs[1].supply = kxsd9_reg_iovdd;
-- 
2.7.4