Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1586046 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v3 4/4] iio: accel: adxl345: Add SPI support Eva Rachel Retuya <eraretuya@gmail.com> - 2017-02-22 11:30 +0100
    Re: [PATCH v3 4/4] iio: accel: adxl345: Add SPI support Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-02-23 17:50 +0100
      Re: [PATCH v3 4/4] iio: accel: adxl345: Add SPI support Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-02-23 18:00 +0100
        Re: [PATCH v3 4/4] iio: accel: adxl345: Add SPI support Eva Rachel Retuya <eraretuya@gmail.com> - 2017-02-24 10:20 +0100
          Re: [PATCH v3 4/4] iio: accel: adxl345: Add SPI support Lars-Peter Clausen <lars@metafoo.de> - 2017-02-24 10:50 +0100
            Re: [PATCH v3 4/4] iio: accel: adxl345: Add SPI support Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-02-24 12:50 +0100
              Re: [PATCH v3 4/4] iio: accel: adxl345: Add SPI support Eva Rachel Retuya <eraretuya@gmail.com> - 2017-02-24 15:40 +0100
                Re: [PATCH v3 4/4] iio: accel: adxl345: Add SPI support Jonathan Cameron <jic23@jic23.retrosnub.co.uk> - 2017-02-24 20:50 +0100
                  Re: [PATCH v3 4/4] iio: accel: adxl345: Add SPI support Jonathan Cameron <jic23@kernel.org> - 2017-02-25 16:20 +0100
      Re: [PATCH v3 4/4] iio: accel: adxl345: Add SPI support Lars-Peter Clausen <lars@metafoo.de> - 2017-02-23 18:10 +0100

#1586046 — [PATCH v3 4/4] iio: accel: adxl345: Add SPI support

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 <eraretuya@gmail.com>
---
Change from v2:
* Add OF match table on both I2C and SPI files and document them

 drivers/iio/accel/Kconfig       |  8 +++-
 drivers/iio/accel/Makefile      |  1 +
 drivers/iio/accel/adxl345_spi.c | 86 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 94 insertions(+), 1 deletion(-)
 create mode 100644 drivers/iio/accel/adxl345_spi.c

diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index ffb0a63..454aeff 100644
--- a/drivers/iio/accel/Kconfig
+++ b/drivers/iio/accel/Kconfig
@@ -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
-	  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
+
 config BMA180
 	tristate "Bosch BMA180/BMA250 3-Axis Accelerometer Driver"
 	depends on I2C
diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
index 3f4a6d6..31fba19 100644
--- a/drivers/iio/accel/Makefile
+++ b/drivers/iio/accel/Makefile
@@ -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
diff --git a/drivers/iio/accel/adxl345_spi.c b/drivers/iio/accel/adxl345_spi.c
new file mode 100644
index 0000000..5b6f01c
--- /dev/null
+++ b/drivers/iio/accel/adxl345_spi.c
@@ -0,0 +1,86 @@
+/*
+ * ADXL345 3-Axis Digital Accelerometer
+ *
+ * Copyright (c) 2017 Eva Rachel Retuya <eraretuya@gmail.com>
+ *
+ * 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.
+ *
+ * SPI driver for ADXL345
+ */
+
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/regmap.h>
+#include <linux/spi/spi.h>
+
+#include "adxl345.h"
+
+#define ADXL345_MAX_SPI_FREQ_HZ		5000000
+
+static const struct regmap_config adxl345_spi_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	 /* Setting bits 7 and 6 enables multiple-byte read */
+	.read_flag_mask = BIT(7) | BIT(6),
+};
+
+static int adxl345_spi_probe(struct spi_device *spi)
+{
+	struct regmap *regmap;
+	const struct spi_device_id *id = spi_get_device_id(spi);
+
+	/* Bail out if max_speed_hz exceeds 5 MHz */
+	if (spi->max_speed_hz > ADXL345_MAX_SPI_FREQ_HZ) {
+		dev_err(&spi->dev, "SPI CLK, %d Hz exceeds 5 MHz\n",
+			spi->max_speed_hz);
+		return -EINVAL;
+	}
+
+	regmap = devm_regmap_init_spi(spi, &adxl345_spi_regmap_config);
+	if (IS_ERR(regmap)) {
+		dev_err(&spi->dev, "Error initializing spi regmap: %d\n",
+			(int)PTR_ERR(regmap));
+		return PTR_ERR(regmap);
+	}
+
+	return adxl345_common_probe(&spi->dev, regmap, id->name);
+}
+
+static int adxl345_spi_remove(struct spi_device *spi)
+{
+	return adxl345_common_remove(&spi->dev);
+}
+
+static const struct spi_device_id adxl345_spi_id[] = {
+	{ "adxl345", 0 },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(spi, adxl345_spi_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 spi_driver adxl345_spi_driver = {
+	.driver = {
+		.name	= "adxl345_spi",
+		.of_match_table = of_match_ptr(adxl345_of_match),
+	},
+	.probe		= adxl345_spi_probe,
+	.remove		= adxl345_spi_remove,
+	.id_table	= adxl345_spi_id,
+};
+
+module_spi_driver(adxl345_spi_driver);
+
+MODULE_AUTHOR("Eva Rachel Retuya <eraretuya@gmail.com>");
+MODULE_DESCRIPTION("ADXL345 3-Axis Digital Accelerometer SPI driver");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

[toc] | [next] | [standalone]

#1587013

On Wed, Feb 22, 2017 at 12:23 PM, Eva Rachel Retuya <eraretuya@gmail.com> wrote:
> 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.

> --- a/drivers/iio/accel/Kconfig
> +++ b/drivers/iio/accel/Kconfig
> @@ -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
> -         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?

-- 
With Best Regards,
Andy Shevchenko

[toc] | [prev] | [next] | [standalone]

#1587026

On Thu, Feb 23, 2017 at 6:47 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
> On 02/23/2017 05:43 PM, Andy Shevchenko wrote:
>> On Wed, Feb 22, 2017 at 12:23 PM, Eva Rachel Retuya <eraretuya@gmail.com> wrote:
>>> 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
>>>
>>> +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?
>
> 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.

-- 
With Best Regards,
Andy Shevchenko

[toc] | [prev] | [next] | [standalone]

#1587448

On Thu, Feb 23, 2017 at 06:58:12PM +0200, Andy Shevchenko wrote:
> On Thu, Feb 23, 2017 at 6:47 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
> > On 02/23/2017 05:43 PM, Andy Shevchenko wrote:
> >> On Wed, Feb 22, 2017 at 12:23 PM, Eva Rachel Retuya <eraretuya@gmail.com> wrote:
> >>> 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
> >>>
> >>> +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?
> >
> > 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.
> 

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.

Thanks,
Eva

> -- 
> With Best Regards,
> Andy Shevchenko

[toc] | [prev] | [next] | [standalone]

#1587471

On 02/24/2017 10:12 AM, Eva Rachel Retuya wrote:
> On Thu, Feb 23, 2017 at 06:58:12PM +0200, Andy Shevchenko wrote:
>> On Thu, Feb 23, 2017 at 6:47 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
>>> On 02/23/2017 05:43 PM, Andy Shevchenko wrote:
>>>> On Wed, Feb 22, 2017 at 12:23 PM, Eva Rachel Retuya <eraretuya@gmail.com> wrote:
>>>>> 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
>>>>>
>>>>> +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?
>>>
>>> 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.
>>
> 
> 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.

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

[toc] | [prev] | [next] | [standalone]

#1587541

On Fri, Feb 24, 2017 at 11:22 AM, Lars-Peter Clausen <lars@metafoo.de> wrote:
> On 02/24/2017 10:12 AM, Eva Rachel Retuya wrote:
>> On Thu, Feb 23, 2017 at 06:58:12PM +0200, Andy Shevchenko wrote:
>>> On Thu, Feb 23, 2017 at 6:47 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
>>>> On 02/23/2017 05:43 PM, Andy Shevchenko wrote:
>>>>> On Wed, Feb 22, 2017 at 12:23 PM, Eva Rachel Retuya <eraretuya@gmail.com> 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.

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

-- 
With Best Regards,
Andy Shevchenko

[toc] | [prev] | [next] | [standalone]

#1587691

On Fri, Feb 24, 2017 at 01:48:39PM +0200, Andy Shevchenko wrote:
> On Fri, Feb 24, 2017 at 11:22 AM, Lars-Peter Clausen <lars@metafoo.de> wrote:
> > On 02/24/2017 10:12 AM, Eva Rachel Retuya wrote:
> >> On Thu, Feb 23, 2017 at 06:58:12PM +0200, Andy Shevchenko wrote:
> >>> On Thu, Feb 23, 2017 at 6:47 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
> >>>> On 02/23/2017 05:43 PM, Andy Shevchenko wrote:
> >>>>> On Wed, Feb 22, 2017 at 12:23 PM, Eva Rachel Retuya <eraretuya@gmail.com> 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.
> 
> +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

> -- 
> With Best Regards,
> Andy Shevchenko
> --
> 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

[toc] | [prev] | [next] | [standalone]

#1587895

On 24 February 2017 14:30:35 GMT+00:00, Eva Rachel Retuya <eraretuya@gmail.com> wrote:
>On Fri, Feb 24, 2017 at 01:48:39PM +0200, Andy Shevchenko wrote:
>> On Fri, Feb 24, 2017 at 11:22 AM, Lars-Peter Clausen
><lars@metafoo.de> wrote:
>> > On 02/24/2017 10:12 AM, Eva Rachel Retuya wrote:
>> >> On Thu, Feb 23, 2017 at 06:58:12PM +0200, Andy Shevchenko wrote:
>> >>> On Thu, Feb 23, 2017 at 6:47 PM, Lars-Peter Clausen
><lars@metafoo.de> wrote:
>> >>>> On 02/23/2017 05:43 PM, Andy Shevchenko wrote:
>> >>>>> On Wed, Feb 22, 2017 at 12:23 PM, Eva Rachel Retuya
><eraretuya@gmail.com> 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.
>> 
>> +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.
>
Have lost track. Will catch up with thread tomorrow and get back to you.

J
>Thanks,
>Eva
>
>> -- 
>> With Best Regards,
>> Andy Shevchenko
>> --
>> 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
>--
>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

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

[toc] | [prev] | [next] | [standalone]

#1588148

On 24/02/17 19:46, Jonathan Cameron wrote:
> 
> 
> On 24 February 2017 14:30:35 GMT+00:00, Eva Rachel Retuya <eraretuya@gmail.com> wrote:
>> On Fri, Feb 24, 2017 at 01:48:39PM +0200, Andy Shevchenko wrote:
>>> On Fri, Feb 24, 2017 at 11:22 AM, Lars-Peter Clausen
>> <lars@metafoo.de> wrote:
>>>> On 02/24/2017 10:12 AM, Eva Rachel Retuya wrote:
>>>>> On Thu, Feb 23, 2017 at 06:58:12PM +0200, Andy Shevchenko wrote:
>>>>>> On Thu, Feb 23, 2017 at 6:47 PM, Lars-Peter Clausen
>> <lars@metafoo.de> wrote:
>>>>>>> On 02/23/2017 05:43 PM, Andy Shevchenko wrote:
>>>>>>>> On Wed, Feb 22, 2017 at 12:23 PM, Eva Rachel Retuya
>> <eraretuya@gmail.com> 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.
>>>
>>> +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.
>>
> Have lost track. Will catch up with thread tomorrow and get back to you.
> 
Ah, now I have caught up, I'll go with the same no strong opinions.
Looks like it is 2 to one in favour of the explicit i2c / spi bits so
lets go with the original form.

Sorry for wasting your time on this Eva!

Divide is always between simplifying configurability of the kernel
vs minimising the extra code built that isn't needed.  Never entirely
obvious where that boundary falls and it really doesn't make that
much difference on either side.

Anyhow, at least you know the options and arguments either way now
so not completely wasted time.

Thanks,

Jonathan
> J
>> Thanks,
>> Eva
>>
>>> -- 
>>> With Best Regards,
>>> Andy Shevchenko
>>> --
>>> 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
>> --
>> 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
> 

[toc] | [prev] | [next] | [standalone]

#1587028

On 02/23/2017 05:43 PM, Andy Shevchenko wrote:
> On Wed, Feb 22, 2017 at 12:23 PM, Eva Rachel Retuya <eraretuya@gmail.com> wrote:
>> 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.
> 
>> --- a/drivers/iio/accel/Kconfig
>> +++ b/drivers/iio/accel/Kconfig
>> @@ -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
>> -         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?

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.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web