iio: accel: adxl345: Split driver into core and I2C then add SPI support

PATCH Superseded Eva Rachel Retuya 4 participants 16 messages Feb 28, 2017 – Mar 4, 2017

Subsystems (3)

Device tree bindings IIO Analog Devices IIO drivers

lore

Share & tools

lore

Eva Rachel Retuya

author

2017-02-28 02:37 UTC #0 lore raw

v5 cover iio: accel: adxl345: Split driver into core and I2C then add SPI support

This patchset modifies the adxl345 to use regmap. In doing so, we can easily introduce SPI support and let regmap handle the rest.

Recap of basic features: read_raw for x, y and z axes, scale. After applying this series, driver now supports the SPI protocol and enumeration of device via device tree.

Changes from v4:
[PATCH 1/4]
* Update subject-prefix
* Update node name from "adxl345@unit-address" to "accelerometer@unit-address" Patches 2-4
* Add Andy's Reviewed-by tag

Changes from v3:
[PATCH 1/4]
* None
[PATCH 2/4]
* Keep intact I2C client structure which was deleted from v3
* Make use of regmap_get_device to retrieve struct device, use these for debugging prints instead of &client->dev.
[PATCH 3/4] and [PATCH 4/4]
* Revert to explicit and separate I2C and SPI configuration
* Add OF match table, make it enumerable in ACPI environment (Andy's suggestion)

Changes from v2:
* Drop PATCH 4 iio: accel: adxl345: Add ACPI support
* Add OF match table on both I2C and SPI files and document them

Changes from v1:
[PATCH 1/4]
* Move other deletions from patch 2 in here -- make it clear what got deleted and/or modified that is hard to see previously
* Introduce the driver header file "adxl345.h" here instead of doing it in the next patch
* Completely omit traces of i2c_client and let this file (adxl345.c) mirror the core file on the next patch.
* Improve debugging print about invalid device ID in probe.
[PATCH 2/4]
* Update Kconfig to Jonathan's preferred style
* Improve similarity index from 78% to 100% (rename detection) [PATCH 4/4]
* Correct acpi_device_id: ADX0345 -> ADS0345

Eva Rachel Retuya (4):
dt-bindings: iio: accel: Document ADXL345 accelerometer binding
iio: accel: adxl345: Use I2C regmap instead of direct I2C access
iio: accel: adxl345: Split driver into core and I2C
iio: accel: adxl345: Add SPI support

 .../devicetree/bindings/iio/accel/adxl345.txt      | 38 +++++++++
 drivers/iio/accel/Kconfig                          | 26 ++++++-
 drivers/iio/accel/Makefile                         |  4 +-
 drivers/iio/accel/adxl345.h                        | 18 +++++
 drivers/iio/accel/{adxl345.c => adxl345_core.c}    | 89 +++++++++-------------
 drivers/iio/accel/adxl345_i2c.c                    | 78 +++++++++++++++++++
 drivers/iio/accel/adxl345_spi.c                    | 83 ++++++++++++++++++++
 7 files changed, 281 insertions(+), 55 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/accel/adxl345.txt
 create mode 100644 drivers/iio/accel/adxl345.h
 rename drivers/iio/accel/{adxl345.c => adxl345_core.c} (62%)
 create mode 100644 drivers/iio/accel/adxl345_i2c.c
 create mode 100644 drivers/iio/accel/adxl345_spi.c

Signature · 6 lines

2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Eva Rachel Retuya

author

2017-02-28 02:37 UTC #1 lore raw

PATCH v5 1/4 dt-bindings: iio: accel: Document ADXL345 accelerometer binding

Add the device tree binding documentation for the ADXL345 3-axis digital accelerometer.

Signed-off-by Eva Rachel Retuya <[email protected]>

---
Changes from v4:
* Update subject-prefix
* Update node name from "adxl345@unit-address" to "accelerometer@unit-address"

 .../devicetree/bindings/iio/accel/adxl345.txt      | 38 ++++++++++++++++++++++
 1 file changed, 38 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/accel/adxl345.txt

Documentation/devicetree/bindings/iio/accel/adxl345.txt new file +38 -0

new file mode 100644
index 0000000..e7111b0
@@ -0,0 +1,38 @@
+Analog Devices ADXL345 3-Axis, +/-(2g/4g/8g/16g) Digital Accelerometer
+
+http://www.analog.com/en/products/mems/accelerometers/adxl345.html
+
+Required properties:
+ - compatible : should be "adi,adxl345"
+ - reg : the I2C address or SPI chip select number of the sensor
+
+Required properties for SPI bus usage:
+ - spi-max-frequency : set maximum clock frequency, must be 5000000
+ - spi-cpol and spi-cpha : must be defined for adxl345 to enable SPI mode 3
+
+Optional properties:
+ - interrupt-parent : phandle to the parent interrupt controller as documented
+   in Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
+ - interrupts: interrupt mapping for IRQ as documented in
+   Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
+
+Example for a I2C device node:
+
+	accelerometer@2a {
+		compatible = "adi,adxl345";
+		reg = <0x53>;
+		interrupt-parent = <&gpio1>;
+		interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
+	};
+
+Example for a SPI device node:
+
+	accelerometer@0 {
+		compatible = "adi,adxl345";
+		reg = <0>;
+		spi-max-frequency = <5000000>;
+		spi-cpol;
+		spi-cpha;
+		interrupt-parent = <&gpio1>;
+		interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
+	};

Signature · 1 line

2.7.4

Rob Herring

maintainer Acked-by on v5 1/4

2017-03-03 06:20 UTC #2 lore raw

Re: [PATCH v5 1/4] dt-bindings: iio: accel: Document ADXL345 accelerometer binding

Quoted 4 lines · Eva Rachel Retuya

Add the device tree binding documentation for the ADXL345 3-axis digital accelerometer.
Signed-off-by Eva Rachel Retuya <[email protected]>

Quoted 8 lines

---
Changes from v4:
* Update subject-prefix
* Update node name from "adxl345@unit-address" to "accelerometer@unit-address"
 .../devicetree/bindings/iio/accel/adxl345.txt      | 38 ++++++++++++++++++++++
 1 file changed, 38 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/accel/adxl345.txt

Acked-by Rob Herring <[email protected]>

Eva Rachel Retuya

author

2017-02-28 02:37 UTC #3 lore raw

PATCH v5 2/4 iio: accel: adxl345: Use I2C regmap instead of direct I2C access

In reply to Eva Rachel Retuya

Convert the driver to use regmap instead of I2C-specific functions. This is done in preparation for splitting this driver into core and I2C-specific code as well as introduction of SPI driver.

Signed-off-by Eva Rachel Retuya <[email protected]>

Reviewed-by Andy Shevchenko <[email protected]>

---
Changes from v4:
* Add Andy's Reviewed-by tag

Changes from v3:
* Keep intact I2C client structure which was deleted from v3
* Make use of regmap_get_device to retrieve struct device, use these for debugging prints instead of &client->dev.

 drivers/iio/accel/Kconfig   |  1 +
 drivers/iio/accel/adxl345.c | 66 +++++++++++++++++++++++++++++----------------
 2 files changed, 44 insertions(+), 23 deletions(-)

drivers/iio/accel/Kconfig +1 -0

index 2308bac..26b8614 100644
@@ -9,6 +9,7 @@ config ADXL345
 	tristate "Analog Devices ADXL345 3-Axis Digital Accelerometer Driver"
 	depends on !(INPUT_ADXL34X=y || INPUT_ADXL34X=m)
 	depends on I2C
+	select REGMAP_I2C
 	help
 	  Say Y here if you want to build support for the Analog Devices
 	  ADXL345 3-axis digital accelerometer.

drivers/iio/accel/adxl345.c +43 ~~-23~~

140 lines not on this page · raw message

Signature · 1 line

2.7.4

Andy Shevchenko

2017-03-03 16:44 UTC #4 lore raw

Re: [PATCH v5 2/4] iio: accel: adxl345: Use I2C regmap instead of direct I2C access

Quoted 6 lines · Eva Rachel Retuya

Convert the driver to use regmap instead of I2C-specific functions. This is done in preparation for splitting this driver into core and I2C-specific code as well as introduction of SPI driver.
Signed-off-by Eva Rachel Retuya <[email protected]>
Reviewed-by Andy Shevchenko <[email protected]>

Okay, my another set of comments.

drivers/iio/accel/adxl345.c static int adxl345_read_raw(struct iio_dev *indio_dev, · line 80

Quoted 6 lines

drivers/iio/accel/adxl345.c +1 -0

@@ -70,6 +76,7 @@ static int adxl345_read_raw(struct iio_dev *indio_dev,
 {
        struct adxl345_data *data = iio_priv(indio_dev);
 
        int ret;
+       __le16 regval;

Reverse tree order, please.

     __le16 regval;
     int ret;

drivers/iio/accel/adxl345.c static int adxl345_probe(struct i2c_client *client, · line 119

Quoted 9 lines

drivers/iio/accel/adxl345.c +3 -0

@@ -104,37 +112,50 @@ static int adxl345_probe(struct i2c_client *client,
 {
        struct adxl345_data *data;
        struct iio_dev *indio_dev;
+       struct device *dev;
+       struct regmap *regmap;
 
        int ret;
+       u32 regval;

Ditto.

drivers/iio/accel/adxl345.c static int adxl345_probe(struct i2c_client *client, · line 123

Quoted 5 lines

drivers/iio/accel/adxl345.c +5 -0

+
+       regmap = devm_regmap_init_i2c(client, &adxl345_regmap_config);
+       if (IS_ERR(regmap)) {
+               dev_err(&client->dev, "Error initializing regmap: %d\n",
+                       (int)PTR_ERR(regmap));

I don't like explicit casting in such cases at all. Why not to use %ld? Same for the similar places in the series.

drivers/iio/accel/adxl345.c static int adxl345_probe(struct i2c_client *client, · line 137

Quoted 5 lines

drivers/iio/accel/adxl345.c +3 -2

-       if (ret != ADXL345_DEVID) {
-               dev_err(&client->dev, "Invalid device ID: %d\n", ret);
+       if (regval != ADXL345_DEVID) {
+               dev_err(dev, "Invalid device ID: %x, expected %x\n",
+                       regval, ADXL345_DEVID);

So, originally it was in decimal form. Does hex looks better or what is behind the change?

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Eva Rachel Retuya

author

2017-03-04 05:52 UTC #5 lore raw

Re: [PATCH v5 2/4] iio: accel: adxl345: Use I2C regmap instead of direct I2C access

Quoted 13 lines · Andy Shevchenko

Quoted 6 lines · Eva Rachel Retuya
Convert the driver to use regmap instead of I2C-specific functions. This is done in preparation for splitting this driver into core and I2C-specific code as well as introduction of SPI driver.
Signed-off-by Eva Rachel Retuya <[email protected]>
Reviewed-by Andy Shevchenko <[email protected]>
Okay, my another set of comments.
Quoted 3 lines
drivers/iio/accel/adxl345.c +0 -0
@@ -70,6 +76,7 @@ static int adxl345_read_raw(struct iio_dev *indio_dev,
 {
        struct adxl345_data *data = iio_priv(indio_dev);

Quoted 8 lines

drivers/iio/accel/adxl345.c static int adxl345_read_raw(struct iio_dev *indio_dev, · line 79
Quoted 2 lines
drivers/iio/accel/adxl345.c +1 -0
        int ret;
+       __le16 regval;
Reverse tree order, please.
     __le16 regval;
     int ret;

Ack.

Quoted 12 lines

drivers/iio/accel/adxl345.c static int adxl345_probe(struct i2c_client *client, · line 119

Quoted 9 lines

drivers/iio/accel/adxl345.c +3 -0

@@ -104,37 +112,50 @@ static int adxl345_probe(struct i2c_client *client,
 {
        struct adxl345_data *data;
        struct iio_dev *indio_dev;
+       struct device *dev;
+       struct regmap *regmap;
 
        int ret;
+       u32 regval;

Ditto.

Quoted 9 lines

drivers/iio/accel/adxl345.c static int adxl345_probe(struct i2c_client *client, · line 123
Quoted 5 lines
drivers/iio/accel/adxl345.c +5 -0
+
+       regmap = devm_regmap_init_i2c(client, &adxl345_regmap_config);
+       if (IS_ERR(regmap)) {
+               dev_err(&client->dev, "Error initializing regmap: %d\n",
+                       (int)PTR_ERR(regmap));
I don't like explicit casting in such cases at all. Why not to use %ld? Same for the similar places in the series.

OK. Will do that instead.

Quoted 8 lines

drivers/iio/accel/adxl345.c static int adxl345_probe(struct i2c_client *client, · line 137
Quoted 5 lines
drivers/iio/accel/adxl345.c +3 -2
-       if (ret != ADXL345_DEVID) {
-               dev_err(&client->dev, "Invalid device ID: %d\n", ret);
+       if (regval != ADXL345_DEVID) {
+               dev_err(dev, "Invalid device ID: %x, expected %x\n",
+                       regval, ADXL345_DEVID);
So, originally it was in decimal form. Does hex looks better or what is behind the change?

Yes it looks better. I made the change to make it easily seen what it read in hex compared to the known DEVID.

Thanks,
Eva

Quoted 4 lines

Signature · 3 lines
-- 
With Best Regards,
Andy Shevchenko

Eva Rachel Retuya

author

2017-02-28 02:37 UTC #6 lore raw

PATCH v5 3/4 iio: accel: adxl345: Split driver into core and I2C

In reply to Eva Rachel Retuya

Move I2C-specific code into its own file and rely on regmap to access registers. The core code provides access to x, y, z and scale readings.

Signed-off-by Eva Rachel Retuya <[email protected]>

Reviewed-by Andy Shevchenko <[email protected]>

---
Changes from v4:
* Add Andy's Reviewed-by tag

Changes from v3:
* Revert to explicit and separate I2C and SPI configuration
* Add OF match table, make it enumerable in ACPI environment (Andy's suggestion)

 drivers/iio/accel/Kconfig                       | 11 +++-
 drivers/iio/accel/Makefile                      |  3 +-
 drivers/iio/accel/adxl345.h                     | 18 ++++++
 drivers/iio/accel/{adxl345.c => adxl345_core.c} | 55 ++++-------------
 drivers/iio/accel/adxl345_i2c.c                 | 78 +++++++++++++++++++++++++
 5 files changed, 117 insertions(+), 48 deletions(-)
 create mode 100644 drivers/iio/accel/adxl345.h
 rename drivers/iio/accel/{adxl345.c => adxl345_core.c} (78%)
 create mode 100644 drivers/iio/accel/adxl345_i2c.c

drivers/iio/accel/Kconfig +8 -3

index 26b8614..9f5a889 100644
@@ -6,16 +6,21 @@
 menu "Accelerometers"
 
 config ADXL345
-	tristate "Analog Devices ADXL345 3-Axis Digital Accelerometer Driver"
+	tristate
+
+config ADXL345_I2C
+	tristate "Analog Devices ADXL345 3-Axis Digital Accelerometer I2C Driver"
 	depends on !(INPUT_ADXL34X=y || INPUT_ADXL34X=m)
 	depends on I2C
+	select ADXL345
 	select REGMAP_I2C
 	help
 	  Say Y here if you want to build support for the Analog Devices
 	  ADXL345 3-axis digital accelerometer.
 
-	  To compile this driver as a module, choose M here: the
-	  module will be called adxl345.
+	  To compile this driver as a module, choose M here: the module
+	  will be called adxl345_i2c and you will also get adxl345_core
+	  for the core module.
 
 config BMA180
 	tristate "Bosch BMA180/BMA250 3-Axis Accelerometer Driver"

drivers/iio/accel/Makefile +2 -1

index 618488d..3f4a6d6 100644
@@ -3,7 +3,8 @@
 #
 
 # When adding new entries keep the list in alphabetical order
-obj-$(CONFIG_ADXL345) += adxl345.o
+obj-$(CONFIG_ADXL345) += adxl345_core.o
+obj-$(CONFIG_ADXL345_I2C) += adxl345_i2c.o
 obj-$(CONFIG_BMA180) += bma180.o
 obj-$(CONFIG_BMA220) += bma220_spi.o
 obj-$(CONFIG_BMC150_ACCEL) += bmc150-accel-core.o

drivers/iio/accel/adxl345.h new file +18 -0

new file mode 100644
index 0000000..fca3e25
@@ -0,0 +1,18 @@
+/*
+ * ADXL345 3-Axis Digital Accelerometer
+ *
+ * Copyright (c) 2017 Eva Rachel Retuya <[email protected]>
+ *
+ * This file is subject to the terms and conditions of version 2 of
+ * the GNU General Public License. See the file COPYING in the main
+ * directory of this archive for more details.
+ */
+
+#ifndef _ADXL345_H_
+#define _ADXL345_H_
+
+int adxl345_common_probe(struct device *dev, struct regmap *regmap,
+			 const char *name);
+int adxl345_common_remove(struct device *dev);
+
+#endif /* _ADXL345_H_ */

drivers/iio/accel/adxl345_core.c renamed from drivers/iio/accel/adxl345.c +11 ~~-44~~

106 lines not on this page · raw message

drivers/iio/accel/adxl345_i2c.c new file +78 -0

78 lines not on this page · raw message

Signature · 1 line

2.7.4

Andy Shevchenko

2017-03-03 16:51 UTC #7 lore raw

Re: [PATCH v5 3/4] iio: accel: adxl345: Split driver into core and I2C

Quoted 2 lines · Eva Rachel Retuya

Move I2C-specific code into its own file and rely on regmap to access registers. The core code provides access to x, y, z and scale readings.

Portion of minor comments.

drivers/iio/accel/Kconfig

Quoted 4 lines

drivers/iio/accel/Kconfig +2 -0

+config ADXL345_I2C
+       tristate "Analog Devices ADXL345 3-Axis Digital Accelerometer I2C Driver"
 
        depends on !(INPUT_ADXL34X=y || INPUT_ADXL34X=m)

I'm wondering if it works in a form of

depends on INPUT_ADXL34X=n

drivers/iio/accel/adxl345.h line 16

Quoted 3 lines

drivers/iio/accel/adxl345.h new file +3 -0

+int adxl345_common_probe(struct device *dev, struct regmap *regmap,
+                        const char *name);
+int adxl345_common_remove(struct device *dev);

I think a "common" word is redundant.

Quoted 5 lines

- * IIO driver for ADXL345
- * 7-bit I2C slave address: 0x1D (ALT ADDRESS pin tied to VDDIO) or
- * 0x53 (ALT ADDRESS pin grounded)
+ * IIO core driver for ADXL345

Should not it be at the beginning of header comment?

drivers/iio/accel/adxl345_i2c.c line 30

Quoted 6 lines

drivers/iio/accel/adxl345_i2c.c new file +5 -0

+static int adxl345_i2c_probe(struct i2c_client *client,
+                            const struct i2c_device_id *id)
+{
 
+       struct regmap *regmap;
+       const char *name = NULL;

Reverse tree order, please.

drivers/iio/accel/adxl345_i2c.c line 42

Quoted 13 lines

drivers/iio/accel/adxl345_i2c.c new file +12 -0

+
+       regmap = devm_regmap_init_i2c(client, &adxl345_i2c_regmap_config);
+       if (IS_ERR(regmap)) {
+               dev_err(&client->dev, "Error initializing i2c regmap: %d\n",
+                       (int)PTR_ERR(regmap));
+               return PTR_ERR(regmap);
+       }
+
 
+       if (id)
+               name = id->name;
+
+       return adxl345_common_probe(&client->dev, regmap, name);

Do you need temporary variable?

return adxl345_probe(&client->dev, regmap, id ? id->name : NULL);

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Eva Rachel Retuya

author

2017-03-04 05:56 UTC #8 lore raw

Re: [PATCH v5 3/4] iio: accel: adxl345: Split driver into core and I2C

Quoted 6 lines · Andy Shevchenko

Quoted 2 lines · Eva Rachel Retuya
Move I2C-specific code into its own file and rely on regmap to access registers. The core code provides access to x, y, z and scale readings.
Portion of minor comments.

Quoted 9 lines

drivers/iio/accel/Kconfig
Quoted 4 lines
drivers/iio/accel/Kconfig +2 -0
+config ADXL345_I2C
+       tristate "Analog Devices ADXL345 3-Axis Digital Accelerometer I2C Driver"
 
        depends on !(INPUT_ADXL34X=y || INPUT_ADXL34X=m)
I'm wondering if it works in a form of
depends on INPUT_ADXL34X=n

Yes it works. Will change it into this form.

Quoted 6 lines

drivers/iio/accel/adxl345.h line 16
Quoted 3 lines
drivers/iio/accel/adxl345.h new file +3 -0
+int adxl345_common_probe(struct device *dev, struct regmap *regmap,
+                        const char *name);
+int adxl345_common_remove(struct device *dev);
I think a "common" word is redundant.

OK. I will use "core" instead.

Quoted 8 lines

Quoted 5 lines
- * IIO driver for ADXL345
- * 7-bit I2C slave address: 0x1D (ALT ADDRESS pin tied to VDDIO) or
- * 0x53 (ALT ADDRESS pin grounded)
+ * IIO core driver for ADXL345
Should not it be at the beginning of header comment?

Ack.

Quoted 18 lines

drivers/iio/accel/adxl345_i2c.c line 30

Quoted 6 lines

drivers/iio/accel/adxl345_i2c.c new file +5 -0

+static int adxl345_i2c_probe(struct i2c_client *client,
+                            const struct i2c_device_id *id)
+{
 
+       struct regmap *regmap;
+       const char *name = NULL;

Reverse tree order, please.

Quoted 8 lines

drivers/iio/accel/adxl345_i2c.c new file +8 -0

+
+       regmap = devm_regmap_init_i2c(client, &adxl345_i2c_regmap_config);
+       if (IS_ERR(regmap)) {
+               dev_err(&client->dev, "Error initializing i2c regmap: %d\n",
+                       (int)PTR_ERR(regmap));
+               return PTR_ERR(regmap);
+       }
+

Quoted 9 lines

drivers/iio/accel/adxl345_i2c.c line 42
Quoted 4 lines
drivers/iio/accel/adxl345_i2c.c new file +4 -0
+       if (id)
+               name = id->name;
+
+       return adxl345_common_probe(&client->dev, regmap, name);
Do you need temporary variable?
return adxl345_probe(&client->dev, regmap, id ? id->name : NULL);

Will remove it and use the form you suggest.

Thanks,
Eva

Quoted 3 lines

Signature · 2 lines
With Best Regards,
Andy Shevchenko

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Eva Rachel Retuya

author

2017-02-28 02:37 UTC #9 lore raw

PATCH v5 4/4 iio: accel: adxl345: Add SPI support

In reply to Eva Rachel Retuya

Add SPI driver that initializes SPI regmap for the adxl345 core driver. The driver supports the same functionality as I2C namely the x, y, z and scale readings.

Signed-off-by Eva Rachel Retuya <[email protected]>

Reviewed-by Andy Shevchenko <[email protected]>

---
Changes from v4:
* Add Andy's Reviewed-by tag

Changes from v3:
* Revert to explicit and separate I2C and SPI configuration
* Add OF match table, make it enumerable in ACPI environment (Andy's suggestion)

 drivers/iio/accel/Kconfig       | 14 +++++++
 drivers/iio/accel/Makefile      |  1 +
 drivers/iio/accel/adxl345_spi.c | 83 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 98 insertions(+)
 create mode 100644 drivers/iio/accel/adxl345_spi.c

drivers/iio/accel/Kconfig +14 -0

index 9f5a889..8994175 100644
@@ -22,6 +22,20 @@ config ADXL345_I2C
 	  will be called adxl345_i2c and you will also get adxl345_core
 	  for the core module.
 
+config ADXL345_SPI
+	tristate "Analog Devices ADXL345 3-Axis Digital Accelerometer SPI Driver"
+	depends on !(INPUT_ADXL34X=y || INPUT_ADXL34X=m)
+	depends on SPI
+	select ADXL345
+	select REGMAP_SPI
+	help
+	  Say Y here if you want to build support for the Analog Devices
+	  ADXL345 3-axis digital accelerometer.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called adxl345_spi and you will also get adxl345_core
+	  for the core module.
+
 config BMA180
 	tristate "Bosch BMA180/BMA250 3-Axis Accelerometer Driver"
 	depends on I2C

drivers/iio/accel/Makefile +1 -0

index 3f4a6d6..31fba19 100644
@@ -5,6 +5,7 @@
 # When adding new entries keep the list in alphabetical order
 obj-$(CONFIG_ADXL345) += adxl345_core.o
 obj-$(CONFIG_ADXL345_I2C) += adxl345_i2c.o
+obj-$(CONFIG_ADXL345_SPI) += adxl345_spi.o
 obj-$(CONFIG_BMA180) += bma180.o
 obj-$(CONFIG_BMA220) += bma220_spi.o
 obj-$(CONFIG_BMC150_ACCEL) += bmc150-accel-core.o

drivers/iio/accel/adxl345_spi.c new file +83 -0

83 lines not on this page · raw message

Signature · 6 lines

2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Andy Shevchenko

2017-03-03 16:55 UTC #10 lore raw

Re: [PATCH v5 4/4] iio: accel: adxl345: Add SPI support

Quoted 3 lines · Eva Rachel Retuya

Add SPI driver that initializes SPI regmap for the adxl345 core driver. The driver supports the same functionality as I2C namely the x, y, z and scale readings.

Portion of minor comments.

drivers/iio/accel/Kconfig config ADXL345_I2C · line 27

Quoted 4 lines

drivers/iio/accel/Kconfig +3 -0

+config ADXL345_SPI
+       tristate "Analog Devices ADXL345 3-Axis Digital Accelerometer SPI Driver"
 
+       depends on !(INPUT_ADXL34X=y || INPUT_ADXL34X=m)

Same question. Would it be just

depends on INPUT_ADXL34X=n

drivers/iio/accel/adxl345_spi.c line 25

Quoted 2 lines

drivers/iio/accel/adxl345_spi.c new file +2 -0

+        /* Setting bits 7 and 6 enables multiple-byte read */
+       .read_flag_mask = BIT(7) | BIT(6),

GENMASK(7, 6) ?

drivers/iio/accel/adxl345_spi.c line 31

Quoted 5 lines

drivers/iio/accel/adxl345_spi.c new file +4 -0

+static int adxl345_spi_probe(struct spi_device *spi)
+{
 
+       struct regmap *regmap;
+       const struct spi_device_id *id = spi_get_device_id(spi);

Reverse order.

And usually we do assignments from function parameters first.

drivers/iio/accel/adxl345_spi.c line 43

Quoted 2 lines

drivers/iio/accel/adxl345_spi.c new file +2 -0

+               dev_err(&spi->dev, "Error initializing spi regmap: %d\n",
+                       (int)PTR_ERR(regmap));

Ugly casting!

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Eva Rachel Retuya

author

2017-03-04 06:00 UTC #11 lore raw

Re: [PATCH v5 4/4] iio: accel: adxl345: Add SPI support

Quoted 14 lines · Andy Shevchenko

Quoted 3 lines · Eva Rachel Retuya
Add SPI driver that initializes SPI regmap for the adxl345 core driver. The driver supports the same functionality as I2C namely the x, y, z and scale readings.
Portion of minor comments.
drivers/iio/accel/Kconfig config ADXL345_I2C · line 27
Quoted 4 lines
drivers/iio/accel/Kconfig +3 -0
+config ADXL345_SPI
+       tristate "Analog Devices ADXL345 3-Axis Digital Accelerometer SPI Driver"
 
+       depends on !(INPUT_ADXL34X=y || INPUT_ADXL34X=m)
Same question. Would it be just

Quoted 9 lines

depends on INPUT_ADXL34X=n
?
drivers/iio/accel/adxl345_spi.c line 25
Quoted 2 lines
drivers/iio/accel/adxl345_spi.c new file +2 -0
+        /* Setting bits 7 and 6 enables multiple-byte read */
+       .read_flag_mask = BIT(7) | BIT(6),
GENMASK(7, 6) ?

True, but I would like to keep this as is. It is more readable and obvious than GENMASK().

Quoted 9 lines

drivers/iio/accel/adxl345_spi.c line 31
Quoted 5 lines
drivers/iio/accel/adxl345_spi.c new file +4 -0
+static int adxl345_spi_probe(struct spi_device *spi)
+{
 
+       struct regmap *regmap;
+       const struct spi_device_id *id = spi_get_device_id(spi);
Reverse order.
And usually we do assignments from function parameters first.

Ack.

Thanks,
Eva

Quoted 9 lines

drivers/iio/accel/adxl345_spi.c line 43
Quoted 3 lines
drivers/iio/accel/adxl345_spi.c new file +2 -0
+               dev_err(&spi->dev, "Error initializing spi regmap: %d\n",
+                       (int)PTR_ERR(regmap));
Ugly casting!
Signature · 2 lines
With Best Regards,
Andy Shevchenko

Andy Shevchenko

2017-03-01 23:22 UTC #12 lore raw

In reply to Eva Rachel Retuya

Quoted 6 lines · Eva Rachel Retuya

This patchset modifies the adxl345 to use regmap. In doing so, we can easily introduce SPI support and let regmap handle the rest.
Recap of basic features: read_raw for x, y and z axes, scale. After applying this series, driver now supports the SPI protocol and enumeration of device via device tree.

If I knew you are going to send new version I would have commented on v4...

Quoted 61 lines

Changes from v4:
[PATCH 1/4]
* Update subject-prefix
* Update node name from "adxl345@unit-address" to "accelerometer@unit-address" Patches 2-4
* Add Andy's Reviewed-by tag
Changes from v3:
[PATCH 1/4]
* None
[PATCH 2/4]
* Keep intact I2C client structure which was deleted from v3
* Make use of regmap_get_device to retrieve struct device, use these for debugging prints instead of &client->dev.
[PATCH 3/4] and [PATCH 4/4]
* Revert to explicit and separate I2C and SPI configuration
* Add OF match table, make it enumerable in ACPI environment (Andy's suggestion)
Changes from v2:
* Drop PATCH 4 iio: accel: adxl345: Add ACPI support
* Add OF match table on both I2C and SPI files and document them
Changes from v1:
[PATCH 1/4]
* Move other deletions from patch 2 in here -- make it clear what got deleted and/or modified that is hard to see previously
* Introduce the driver header file "adxl345.h" here instead of doing it in the next patch
* Completely omit traces of i2c_client and let this file (adxl345.c) mirror the core file on the next patch.
* Improve debugging print about invalid device ID in probe.
[PATCH 2/4]
* Update Kconfig to Jonathan's preferred style
* Improve similarity index from 78% to 100% (rename detection) [PATCH 4/4]
* Correct acpi_device_id: ADX0345 -> ADS0345
Eva Rachel Retuya (4):
dt-bindings: iio: accel: Document ADXL345 accelerometer binding
iio: accel: adxl345: Use I2C regmap instead of direct I2C access
iio: accel: adxl345: Split driver into core and I2C
iio: accel: adxl345: Add SPI support
 .../devicetree/bindings/iio/accel/adxl345.txt      | 38 +++++++++
 drivers/iio/accel/Kconfig                          | 26 ++++++-
 drivers/iio/accel/Makefile                         |  4 +-
 drivers/iio/accel/adxl345.h                        | 18 +++++
 drivers/iio/accel/{adxl345.c => adxl345_core.c}    | 89 +++++++++-------------
 drivers/iio/accel/adxl345_i2c.c                    | 78 +++++++++++++++++++
 drivers/iio/accel/adxl345_spi.c                    | 83 ++++++++++++++++++++
 7 files changed, 281 insertions(+), 55 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/accel/adxl345.txt
 create mode 100644 drivers/iio/accel/adxl345.h
 rename drivers/iio/accel/{adxl345.c => adxl345_core.c} (62%)
 create mode 100644 drivers/iio/accel/adxl345_i2c.c
 create mode 100644 drivers/iio/accel/adxl345_spi.c
--
2.7.4

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Eva Rachel Retuya

author

2017-03-02 03:02 UTC #13 lore raw

Quoted 10 lines · Andy Shevchenko

Quoted 6 lines · Eva Rachel Retuya
This patchset modifies the adxl345 to use regmap. In doing so, we can easily introduce SPI support and let regmap handle the rest.
Recap of basic features: read_raw for x, y and z axes, scale. After applying this series, driver now supports the SPI protocol and enumeration of device via device tree.
If I knew you are going to send new version I would have commented on v4...

Hi Andy,

Please state your comments and I will submit a new revision.

Thanks,
Eva

Quoted 58 lines

Quoted 58 lines
Changes from v4:
[PATCH 1/4]
* Update subject-prefix
* Update node name from "adxl345@unit-address" to "accelerometer@unit-address" Patches 2-4
* Add Andy's Reviewed-by tag
Changes from v3:
[PATCH 1/4]
* None
[PATCH 2/4]
* Keep intact I2C client structure which was deleted from v3
* Make use of regmap_get_device to retrieve struct device, use these for debugging prints instead of &client->dev.
[PATCH 3/4] and [PATCH 4/4]
* Revert to explicit and separate I2C and SPI configuration
* Add OF match table, make it enumerable in ACPI environment (Andy's suggestion)
Changes from v2:
* Drop PATCH 4 iio: accel: adxl345: Add ACPI support
* Add OF match table on both I2C and SPI files and document them
Changes from v1:
[PATCH 1/4]
* Move other deletions from patch 2 in here -- make it clear what got deleted and/or modified that is hard to see previously
* Introduce the driver header file "adxl345.h" here instead of doing it in the next patch
* Completely omit traces of i2c_client and let this file (adxl345.c) mirror the core file on the next patch.
* Improve debugging print about invalid device ID in probe.
[PATCH 2/4]
* Update Kconfig to Jonathan's preferred style
* Improve similarity index from 78% to 100% (rename detection) [PATCH 4/4]
* Correct acpi_device_id: ADX0345 -> ADS0345
Eva Rachel Retuya (4):
dt-bindings: iio: accel: Document ADXL345 accelerometer binding
iio: accel: adxl345: Use I2C regmap instead of direct I2C access
iio: accel: adxl345: Split driver into core and I2C
iio: accel: adxl345: Add SPI support
 .../devicetree/bindings/iio/accel/adxl345.txt      | 38 +++++++++
 drivers/iio/accel/Kconfig                          | 26 ++++++-
 drivers/iio/accel/Makefile                         |  4 +-
 drivers/iio/accel/adxl345.h                        | 18 +++++
 drivers/iio/accel/{adxl345.c => adxl345_core.c}    | 89 +++++++++-------------
 drivers/iio/accel/adxl345_i2c.c                    | 78 +++++++++++++++++++
 drivers/iio/accel/adxl345_spi.c                    | 83 ++++++++++++++++++++
 7 files changed, 281 insertions(+), 55 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/accel/adxl345.txt
 create mode 100644 drivers/iio/accel/adxl345.h
 rename drivers/iio/accel/{adxl345.c => adxl345_core.c} (62%)
 create mode 100644 drivers/iio/accel/adxl345_i2c.c
 create mode 100644 drivers/iio/accel/adxl345_spi.c

Quoted 9 lines

Quoted 3 lines
--
2.7.4
Signature · 2 lines
With Best Regards,
Andy Shevchenko

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Jonathan Cameron

maintainer

2017-03-02 19:59 UTC #14 lore raw

Quoted 12 lines · Eva Rachel Retuya

Quoted 10 lines · Andy Shevchenko
Quoted 6 lines · Eva Rachel Retuya
This patchset modifies the adxl345 to use regmap. In doing so, we can easily introduce SPI support and let regmap handle the rest.
Recap of basic features: read_raw for x, y and z axes, scale. After applying this series, driver now supports the SPI protocol and enumeration of device via device tree.
If I knew you are going to send new version I would have commented on v4...

Quoted 6 lines

Hi Andy,
Please state your comments and I will submit a new revision.
Thanks,
Eva

Looks good to me so I'll probably pick this up over the weekend unless you want to make a V6 with anything from Andy. Can always send follow up patches later for any issues highlighted.

Ideally we 'might' have devicetree ack by then, but given you have fixed the one issue raised in the review, I may not wait for that (can add it in the week it'll inevitably sit in my testing branch for!)

Jonathan

Quoted 68 lines

Quoted 68 lines
Quoted 62 lines
Changes from v4:
[PATCH 1/4]
* Update subject-prefix
* Update node name from "adxl345@unit-address" to "accelerometer@unit-address" Patches 2-4
* Add Andy's Reviewed-by tag
Changes from v3:
[PATCH 1/4]
* None
[PATCH 2/4]
* Keep intact I2C client structure which was deleted from v3
* Make use of regmap_get_device to retrieve struct device, use these for debugging prints instead of &client->dev.
[PATCH 3/4] and [PATCH 4/4]
* Revert to explicit and separate I2C and SPI configuration
* Add OF match table, make it enumerable in ACPI environment (Andy's suggestion)
Changes from v2:
* Drop PATCH 4 iio: accel: adxl345: Add ACPI support
* Add OF match table on both I2C and SPI files and document them
Changes from v1:
[PATCH 1/4]
* Move other deletions from patch 2 in here -- make it clear what got deleted and/or modified that is hard to see previously
* Introduce the driver header file "adxl345.h" here instead of doing it in the next patch
* Completely omit traces of i2c_client and let this file (adxl345.c) mirror the core file on the next patch.
* Improve debugging print about invalid device ID in probe.
[PATCH 2/4]
* Update Kconfig to Jonathan's preferred style
* Improve similarity index from 78% to 100% (rename detection) [PATCH 4/4]
* Correct acpi_device_id: ADX0345 -> ADS0345
Eva Rachel Retuya (4):
dt-bindings: iio: accel: Document ADXL345 accelerometer binding
iio: accel: adxl345: Use I2C regmap instead of direct I2C access
iio: accel: adxl345: Split driver into core and I2C
iio: accel: adxl345: Add SPI support
 .../devicetree/bindings/iio/accel/adxl345.txt      | 38 +++++++++
 drivers/iio/accel/Kconfig                          | 26 ++++++-
 drivers/iio/accel/Makefile                         |  4 +-
 drivers/iio/accel/adxl345.h                        | 18 +++++
 drivers/iio/accel/{adxl345.c => adxl345_core.c}    | 89 +++++++++-------------
 drivers/iio/accel/adxl345_i2c.c                    | 78 +++++++++++++++++++
 drivers/iio/accel/adxl345_spi.c                    | 83 ++++++++++++++++++++
 7 files changed, 281 insertions(+), 55 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/accel/adxl345.txt
 create mode 100644 drivers/iio/accel/adxl345.h
 rename drivers/iio/accel/{adxl345.c => adxl345_core.c} (62%)
 create mode 100644 drivers/iio/accel/adxl345_i2c.c
 create mode 100644 drivers/iio/accel/adxl345_spi.c
--
2.7.4
Signature · 2 lines
With Best Regards,
Andy Shevchenko

Andy Shevchenko

2017-03-03 16:57 UTC #15 lore raw

Quoted 7 lines · Jonathan Cameron

Quoted 6 lines · Eva Rachel Retuya
Quoted 3 lines · Andy Shevchenko
Quoted 2 lines · Eva Rachel Retuya
This patchset modifies the adxl345 to use regmap. In doing so, we can easily introduce SPI support and let regmap handle the rest.
Please state your comments and I will submit a new revision.

Just did.

Quoted 3 lines

Looks good to me so I'll probably pick this up over the weekend unless you want to make a V6 with anything from Andy. Can always send follow up patches later for any issues highlighted.

Yep, feel free incorporate now, or fix later (though I prefer now :-) )

Signature · 2 lines

With Best Regards,
Andy Shevchenko