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

Convert the driver to use regmap instead of I2C-specific functions. Also, introduce the header file "adxl345.h" and export the probe and remove 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]>

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

-static const struct i2c_device_id adxl345_i2c_id[] = {
-       { "adxl345", 0 },
-       { }
-};
-
-MODULE_DEVICE_TABLE(i2c, adxl345_i2c_id);
-
-static struct i2c_driver adxl345_driver = {
-       .driver = {
-               .name   = "adxl345",
-       },
-       .probe          = adxl345_probe,
-       .remove         = adxl345_remove,
-       .id_table       = adxl345_i2c_id,
-};
-
-module_i2c_driver(adxl345_driver);
+EXPORT_SYMBOL_GPL(adxl345_common_remove);

Quoted 6 lines · Eva Rachel Retuya

Convert the driver to use regmap instead of I2C-specific functions. Also, introduce the header file "adxl345.h" and export the probe and remove 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]>

Is it possible to use device just after this very patch? For me it seems there is bisectability issue.

Quoted 18 lines

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

-static const struct i2c_device_id adxl345_i2c_id[] = {
-       { "adxl345", 0 },
-       { }
-};
-
-MODULE_DEVICE_TABLE(i2c, adxl345_i2c_id);
-
-static struct i2c_driver adxl345_driver = {
-       .driver = {
-               .name   = "adxl345",
-       },
-       .probe          = adxl345_probe,
-       .remove         = adxl345_remove,
-       .id_table       = adxl345_i2c_id,
-};
-
-module_i2c_driver(adxl345_driver);
+EXPORT_SYMBOL_GPL(adxl345_common_remove);

Signature · 2 lines

With Best Regards,
Andy Shevchenko

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.

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

+static const struct i2c_device_id adxl345_i2c_id[] = {
+       { "adxl345", 0 },
+       { }
+};
+
+MODULE_DEVICE_TABLE(i2c, adxl345_i2c_id);
+
+#ifdef CONFIG_OF
+static const struct of_device_id adxl345_of_match[] = {
+       { .compatible = "adi,adxl345" },
+       { },
+};
+
+MODULE_DEVICE_TABLE(of, adxl345_of_match);
+#endif
+
+static struct i2c_driver adxl345_i2c_driver = {
+       .driver = {
+               .name   = "adxl345_i2c",
+               .of_match_table = of_match_ptr(adxl345_of_match),
+       },
+       .probe          = adxl345_i2c_probe,
+       .remove         = adxl345_i2c_remove,
+       .id_table       = adxl345_i2c_id,
+};
+
+module_i2c_driver(adxl345_i2c_driver);
+
+MODULE_AUTHOR("Eva Rachel Retuya <[email protected]>");
+MODULE_DESCRIPTION("ADXL345 3-Axis Digital Accelerometer I2C driver");
+MODULE_LICENSE("GPL v2");

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.

Like Lars already pointed to possibility of use of this in ACPI based platforms, I would add a reference to
commit 01427fe7c4b9 ("Input: adxl34x - make it enumerable in ACPI environment").

Please fix your patches accordingly.

Quoted 32 lines

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

+static const struct i2c_device_id adxl345_i2c_id[] = {
+       { "adxl345", 0 },
+       { }
+};
+
+MODULE_DEVICE_TABLE(i2c, adxl345_i2c_id);
+
+#ifdef CONFIG_OF
+static const struct of_device_id adxl345_of_match[] = {
+       { .compatible = "adi,adxl345" },
+       { },
+};
+
+MODULE_DEVICE_TABLE(of, adxl345_of_match);
+#endif
+
+static struct i2c_driver adxl345_i2c_driver = {
+       .driver = {
+               .name   = "adxl345_i2c",
+               .of_match_table = of_match_ptr(adxl345_of_match),
+       },
+       .probe          = adxl345_i2c_probe,
+       .remove         = adxl345_i2c_remove,
+       .id_table       = adxl345_i2c_id,
+};
+
+module_i2c_driver(adxl345_i2c_driver);
+
+MODULE_AUTHOR("Eva Rachel Retuya <[email protected]>");
+MODULE_DESCRIPTION("ADXL345 3-Axis Digital Accelerometer I2C driver");
+MODULE_LICENSE("GPL v2");

Signature · 2 lines

With Best Regards,
Andy Shevchenko

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.

drivers/iio/accel/Kconfig +2 -0

@@ -8,20 +8,26 @@ menu "Accelerometers"
 config ADXL345
        tristate "Analog Devices ADXL345 3-Axis Digital Accelerometer Driver"
        depends on !(INPUT_ADXL34X=y || INPUT_ADXL34X=m)
+       depends on (I2C || SPI)
        select REGMAP
        select ADXL345_I2C if I2C
+       select ADXL345_SPI if 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 core
          module will be called adxl345_core and you will also get

Quoted 20 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.

drivers/iio/accel/Kconfig +2 -0

@@ -8,20 +8,26 @@ menu "Accelerometers"
 config ADXL345
        tristate "Analog Devices ADXL345 3-Axis Digital Accelerometer Driver"
        depends on !(INPUT_ADXL34X=y || INPUT_ADXL34X=m)
+       depends on (I2C || SPI)
        select REGMAP
        select ADXL345_I2C if I2C
+       select ADXL345_SPI if 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 core
          module will be called adxl345_core and you will also get

drivers/iio/accel/Kconfig menu "Accelerometers" · line 29

Quoted 10 lines

drivers/iio/accel/Kconfig +4 -1

-         adxl345_i2c for I2C.
+         adxl345_i2c for I2C and/or adxl345_spi for SPI.
 
 config ADXL345_I2C
        tristate
        select REGMAP_I2C
 
+config ADXL345_SPI
+       tristate
+       select REGMAP_SPI

Hmm...
I saw another pattern

Library / core part is non-visible to user, while
SPI and I2C parts are selectable by user.

Why do you use inverted pattern? What did I miss?

Quoted 16 lines · Andy Shevchenko

drivers/iio/accel/Kconfig menu "Accelerometers" · line 29

Quoted 11 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.

config ADXL345_I2C
tristate
select REGMAP_I2C

drivers/iio/accel/Kconfig +3 -0

+config ADXL345_SPI
+       tristate
+       select REGMAP_SPI

Hmm...
I saw another pattern

Quoted 4 lines

Library / core part is non-visible to user, while
SPI and I2C parts are selectable by user.

Why do you use inverted pattern? What did I miss?

The first version of the patch used the other pattern SPI/I2C visible. Jonathan suggested this other pattern. I prefer the explicit SPI/I2C visible pattern, but in the end it doesn't really matter as long as both work.

Quoted 17 lines · Lars-Peter Clausen

Quoted 16 lines · Andy Shevchenko

drivers/iio/accel/Kconfig menu "Accelerometers" · line 29

Quoted 11 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.

config ADXL345_I2C
tristate
select REGMAP_I2C

drivers/iio/accel/Kconfig +3 -0

+config ADXL345_SPI
+       tristate
+       select REGMAP_SPI

Hmm...
I saw another pattern

Quoted 8 lines

Quoted 4 lines

Library / core part is non-visible to user, while
SPI and I2C parts are selectable by user.

Why do you use inverted pattern? What did I miss?

The first version of the patch used the other pattern SPI/I2C visible. Jonathan suggested this other pattern. I prefer the explicit SPI/I2C visible pattern, but in the end it doesn't really matter as long as both work.

Yes, but this pattern makes extra footprint of the kernel and basically dead code when I would like, for example, to have SPI bus enabled, I2C module available, but SPI module not compiled.

Other one is when I want to have one compiled in, one as a module by whatever reason.

At the end I have no strong opinion, though rationale for the opposite is above.

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Quoted 34 lines · Andy Shevchenko

Quoted 17 lines · Lars-Peter Clausen

Quoted 16 lines · Andy Shevchenko

drivers/iio/accel/Kconfig menu "Accelerometers" · line 29

Quoted 11 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.

config ADXL345_I2C
tristate
select REGMAP_I2C

drivers/iio/accel/Kconfig +3 -0

+config ADXL345_SPI
+       tristate
+       select REGMAP_SPI

Hmm...
I saw another pattern

Quoted 8 lines

Quoted 4 lines

Library / core part is non-visible to user, while
SPI and I2C parts are selectable by user.

Why do you use inverted pattern? What did I miss?

The first version of the patch used the other pattern SPI/I2C visible. Jonathan suggested this other pattern. I prefer the explicit SPI/I2C visible pattern, but in the end it doesn't really matter as long as both work.

Yes, but this pattern makes extra footprint of the kernel and basically dead code when I would like, for example, to have SPI bus enabled, I2C module available, but SPI module not compiled.

Other one is when I want to have one compiled in, one as a module by whatever reason.

Quoted 2 lines

At the end I have no strong opinion, though rationale for the opposite is above.

Hello Lars and Andy,

I'll revert to the explicit SPI/I2C pattern in order to give more freedom in configuring as per the scenarios previously stated.

Quoted 7 lines · Eva Rachel Retuya

Quoted 3 lines · Andy Shevchenko

Quoted 2 lines · Lars-Peter Clausen

Quoted 1 line · Andy Shevchenko

On Wed, Feb 22, 2017 at 12:23 PM, Eva Rachel Retuya <[email protected]> wrote:

I'll revert to the explicit SPI/I2C pattern in order to give more freedom in configuring as per the scenarios previously stated.

Please check with Jonathan before you do, in the end he'll have to apply the patch.

Quoted 5 lines · Lars-Peter Clausen

Quoted 4 lines · Eva Rachel Retuya

Quoted 3 lines · Andy Shevchenko

Quoted 2 lines · Lars-Peter Clausen

Quoted 1 line · Andy Shevchenko

On Wed, Feb 22, 2017 at 12:23 PM, Eva Rachel Retuya <[email protected]> wrote:

Quoted 5 lines

Quoted 2 lines

I'll revert to the explicit SPI/I2C pattern in order to give more freedom in configuring as per the scenarios previously stated.

Please check with Jonathan before you do, in the end he'll have to apply the patch.

+1. I will be fine with either just need to make sure maintainer understands pros and cons.

Signature · 6 lines

With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Quoted 1 line · Andy Shevchenko

On Fri, Feb 24, 2017 at 11:22 AM, Lars-Peter Clausen

Quoted 3 lines · <[email protected]> wrote:

Quoted 3 lines · Eva Rachel Retuya

Quoted 2 lines · Andy Shevchenko

Quoted 1 line

On Thu, Feb 23, 2017 at 6:47 PM, Lars-Peter Clausen

Quoted 2 lines · <[email protected]> wrote:

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines · Andy Shevchenko

Quoted 1 line

On Wed, Feb 22, 2017 at 12:23 PM, Eva Rachel Retuya

Quoted 5 lines · <[email protected]> wrote:

Quoted 5 lines

Quoted 3 lines

I'll revert to the explicit SPI/I2C pattern in order to give more freedom in configuring as per the scenarios previously stated.

Please check with Jonathan before you do, in the end he'll have to

apply the

Quoted 5 lines

Quoted 1 line

patch.

+1. I will be fine with either just need to make sure maintainer understands pros and cons.

OK.

Jonathan, please let me know whether to continue with your preferred configuration or go with the "explicit" pattern that was discussed here.

Thanks,
Eva

Quoted 5 lines

Signature · 4 lines

With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-iio"

in

Quoted 2 lines

the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

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

Quoted 22 lines · Eva Rachel Retuya

Quoted 1 line · Andy Shevchenko

On Fri, Feb 24, 2017 at 11:22 AM, Lars-Peter Clausen

Quoted 3 lines · <lars-Qo5EllUWu/[email protected]> wrote:

Quoted 3 lines · Eva Rachel Retuya

Quoted 2 lines · Andy Shevchenko

Quoted 1 line

On Thu, Feb 23, 2017 at 6:47 PM, Lars-Peter Clausen

Quoted 2 lines · <lars-Qo5EllUWu/[email protected]> wrote:

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines · Andy Shevchenko

Quoted 1 line

On Wed, Feb 22, 2017 at 12:23 PM, Eva Rachel Retuya

Quoted 5 lines · <[email protected]> wrote:

Quoted 5 lines

Quoted 3 lines

I'll revert to the explicit SPI/I2C pattern in order to give more freedom in configuring as per the scenarios previously stated.

Please check with Jonathan before you do, in the end he'll have to

apply the

Quoted 5 lines

Quoted 1 line

patch.

+1. I will be fine with either just need to make sure maintainer understands pros and cons.

Quoted 6 lines

OK.

Jonathan, please let me know whether to continue with your preferred configuration or go with the "explicit" pattern that was discussed here.

Have lost track. Will catch up with thread tomorrow and get back to you.

J

Quoted 15 lines

Thanks,
Eva

Quoted 5 lines

Signature · 4 lines

With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-iio"

in

Quoted 2 lines

the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

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