* [PATCH 0/1] iio: accel: bma400: add support for bma400 spi @ 2020-05-22 1:46 Dan Robertson 2020-05-22 1:46 ` [PATCH 1/1] " Dan Robertson 0 siblings, 1 reply; 4+ messages in thread From: Dan Robertson @ 2020-05-22 1:46 UTC (permalink / raw) To: Jonathan Cameron Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, Linus Walleij, Andy Shevchenko, linux-iio, Dan Robertson The Bosch Sensortec BMA400 3-axes ultra-low power supports a 4 wire SPI ditital interface. This patch adds support for the device when configured for SPI instead of I2C. I was originally hoping to avoid using a regmap_bus definition, but the register reads from the device are padded by a byte while register writes are not padded. As a result, a regmap_config like the following does not work. const struct regmap_config bma400_regmap_spi_config = { reg_bits = 8, pad_bits = 8, val_bits = 8, read_flag_mask = BIT(7), max_register = BMA400_CMD_REG, }; I used a regmap_bus structure with read and write implementations to work around this, but would appreciate feedback on this approach and my implementation. Cheers, - Dan Dan Robertson (1): iio: accel: bma400: add support for bma400 spi drivers/iio/accel/Kconfig | 8 ++- drivers/iio/accel/Makefile | 1 + drivers/iio/accel/bma400_spi.c | 126 +++++++++++++++++++++++++++++++++ 3 files changed, 134 insertions(+), 1 deletion(-) create mode 100644 drivers/iio/accel/bma400_spi.c ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/1] iio: accel: bma400: add support for bma400 spi 2020-05-22 1:46 [PATCH 0/1] iio: accel: bma400: add support for bma400 spi Dan Robertson @ 2020-05-22 1:46 ` Dan Robertson 2020-05-22 8:50 ` Andy Shevchenko 0 siblings, 1 reply; 4+ messages in thread From: Dan Robertson @ 2020-05-22 1:46 UTC (permalink / raw) To: Jonathan Cameron Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, Linus Walleij, Andy Shevchenko, linux-iio, Dan Robertson Add basic support for the Bosch Sensortec BMA400 3-axes ultra-low power accelerometer when configured to use SPI. Signed-off-by: Dan Robertson <dan@dlrobertson.com> --- drivers/iio/accel/Kconfig | 8 ++- drivers/iio/accel/Makefile | 1 + drivers/iio/accel/bma400_spi.c | 126 +++++++++++++++++++++++++++++++++ 3 files changed, 134 insertions(+), 1 deletion(-) create mode 100644 drivers/iio/accel/bma400_spi.c diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig index 24ebe9e76915..cb4ef367e5fa 100644 --- a/drivers/iio/accel/Kconfig +++ b/drivers/iio/accel/Kconfig @@ -116,18 +116,24 @@ config BMA400 tristate "Bosch BMA400 3-Axis Accelerometer Driver" select REGMAP select BMA400_I2C if I2C + select BMA400_SPI if I2C help Say Y here if you want to build a driver for the Bosch BMA400 triaxial acceleration sensor. To compile this driver as a module, choose M here: the module will be called bma400_core and you will also get - bma400_i2c if I2C is enabled. + bma400_i2c if I2C is enabled and bma400_spi if SPI is + enabled. config BMA400_I2C tristate depends on BMA400 +config BMA400_SPI + tristate + depends on BMA400 + config BMC150_ACCEL tristate "Bosch BMC150 Accelerometer Driver" select IIO_BUFFER diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile index 3a051cf37f40..4f6c1ebe13b0 100644 --- a/drivers/iio/accel/Makefile +++ b/drivers/iio/accel/Makefile @@ -16,6 +16,7 @@ obj-$(CONFIG_BMA180) += bma180.o obj-$(CONFIG_BMA220) += bma220_spi.o obj-$(CONFIG_BMA400) += bma400_core.o obj-$(CONFIG_BMA400_I2C) += bma400_i2c.o +obj-$(CONFIG_BMA400_SPI) += bma400_spi.o obj-$(CONFIG_BMC150_ACCEL) += bmc150-accel-core.o obj-$(CONFIG_BMC150_ACCEL_I2C) += bmc150-accel-i2c.o obj-$(CONFIG_BMC150_ACCEL_SPI) += bmc150-accel-spi.o diff --git a/drivers/iio/accel/bma400_spi.c b/drivers/iio/accel/bma400_spi.c new file mode 100644 index 000000000000..d92b37e1a657 --- /dev/null +++ b/drivers/iio/accel/bma400_spi.c @@ -0,0 +1,126 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * SPI IIO driver for Bosch BMA400 triaxial acceleration sensor. + * + * Copyright 2020 Dan Robertson <dan@dlrobertson.com> + * + */ +#include <linux/init.h> +#include <linux/module.h> +#include <linux/spi/spi.h> + +#include <linux/spi/spi.h> +#include <linux/mod_devicetable.h> +#include <linux/module.h> +#include <linux/regmap.h> + +#include "bma400.h" + +#define BMA400_MAX_SPI_READ 2 +#define BMA400_SPI_READ_BUFFER_SIZE (BMA400_MAX_SPI_READ + 1) + +static int bma400_regmap_spi_read(void *context, + const void *reg, size_t reg_size, + void *val, size_t val_size) +{ + struct device *dev = context; + struct spi_device *spi = to_spi_device(dev); + ssize_t status; + /* + * TODO(dlrobertson): What is a reasonable length to cap + * this at. + */ + u8 result[BMA400_SPI_READ_BUFFER_SIZE]; + + if (val_size > BMA400_MAX_SPI_READ) + return -EINVAL; + + status = spi_write_then_read(spi, reg, 1, result, val_size + 1); + if (status) + return status; + + /* + * From the BMA400 datasheet: + * + * > For a basic read operation two bytes have to be read and the first + * > has to be dropped and the second byte must be interpreted. + */ + memcpy(val, result + 1, val_size); + + return 0; +} + +static int bma400_regmap_spi_write(void *context, const void *data, + size_t count) +{ + struct device *dev = context; + struct spi_device *spi = to_spi_device(dev); + + return spi_write(spi, data, count); +} + +static struct regmap_bus bma400_regmap_bus = { + .read = bma400_regmap_spi_read, + .write = bma400_regmap_spi_write, + .read_flag_mask = BIT(7), + .max_raw_read = BMA400_MAX_SPI_READ, +}; + +static int bma400_spi_probe(struct spi_device *spi) +{ + struct regmap *regmap; + unsigned int val; + int ret; + + regmap = devm_regmap_init(&spi->dev, &bma400_regmap_bus, + &spi->dev, &bma400_regmap_config); + if (IS_ERR(regmap)) { + dev_err(&spi->dev, "failed to create regmap\n"); + return PTR_ERR(regmap); + } + + /* + * Per the bma400 datasheet, the first SPI read may + * return garbage on the first read. The chip ID + * register will be read and checked again in the + * following probe. + */ + ret = regmap_read(regmap, BMA400_CHIP_ID_REG, &val); + if (ret) + dev_err(&spi->dev, "Failed to read chip id register\n"); + + return bma400_probe(&spi->dev, regmap, "bma400"); +} + +static int bma400_spi_remove(struct spi_device *spi) +{ + return bma400_remove(&spi->dev); +} + +static const struct spi_device_id bma400_spi_ids[] = { + { "bma400", 0 }, + { } +}; +MODULE_DEVICE_TABLE(spi, bma400_spi_ids); + +static const struct of_device_id bma400_of_spi_match[] = { + { .compatible = "bosch,bma400" }, + { } +}; +MODULE_DEVICE_TABLE(of, bma400_of_spi_match); + +static struct spi_driver bma400_spi_driver = { + .driver = { + .name = "bma400", + .of_match_table = bma400_of_spi_match, + }, + .probe = bma400_spi_probe, + .remove = bma400_spi_remove, + .id_table = bma400_spi_ids, +}; + +module_spi_driver(bma400_spi_driver); + +MODULE_AUTHOR("Dan Robertson <dan@dlrobertson.com>"); +MODULE_DESCRIPTION("Bosch BMA400 triaxial acceleration sensor (SPI)"); +MODULE_LICENSE("GPL"); ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] iio: accel: bma400: add support for bma400 spi 2020-05-22 1:46 ` [PATCH 1/1] " Dan Robertson @ 2020-05-22 8:50 ` Andy Shevchenko 2020-05-22 12:36 ` Dan Robertson 0 siblings, 1 reply; 4+ messages in thread From: Andy Shevchenko @ 2020-05-22 8:50 UTC (permalink / raw) To: Dan Robertson Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, Linus Walleij, linux-iio On Fri, May 22, 2020 at 4:48 AM Dan Robertson <dan@dlrobertson.com> wrote: > > Add basic support for the Bosch Sensortec BMA400 3-axes ultra-low power > accelerometer when configured to use SPI. ... > tristate "Bosch BMA400 3-Axis Accelerometer Driver" > select REGMAP > select BMA400_I2C if I2C > + select BMA400_SPI if I2C This is not right. ... > +#include <linux/module.h> > +#include <linux/spi/spi.h> What's the point of dups (see below)? > +#include <linux/spi/spi.h> > +#include <linux/mod_devicetable.h> > +#include <linux/module.h> > +#include <linux/regmap.h> Keep them ordered? ... > +#define BMA400_SPI_READ_BUFFER_SIZE (BMA400_MAX_SPI_READ + 1) Do wee need separate macro? It seems longer than explicit use. Do we need the original macro either? ... > + /* > + * TODO(dlrobertson): What is a reasonable length to cap > + * this at. > + */ Either drop this or fulfill. There is no way to leave such in the non-staging code. ... > + .read_flag_mask = BIT(7), #include <linux/bits.h> ... > +static struct spi_driver bma400_spi_driver = { > + .driver = { > + .name = "bma400", > + .of_match_table = bma400_of_spi_match, > + }, > + .probe = bma400_spi_probe, > + .remove = bma400_spi_remove, > + .id_table = bma400_spi_ids, > +}; > + This blank line is not needed. > +module_spi_driver(bma400_spi_driver); -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] iio: accel: bma400: add support for bma400 spi 2020-05-22 8:50 ` Andy Shevchenko @ 2020-05-22 12:36 ` Dan Robertson 0 siblings, 0 replies; 4+ messages in thread From: Dan Robertson @ 2020-05-22 12:36 UTC (permalink / raw) To: Andy Shevchenko Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, Linus Walleij, linux-iio [-- Attachment #1: Type: text/plain, Size: 1513 bytes --] On Fri, May 22, 2020 at 11:50:37AM +0300, Andy Shevchenko wrote: > On Fri, May 22, 2020 at 4:48 AM Dan Robertson <dan@dlrobertson.com> wrote: > > > > Add basic support for the Bosch Sensortec BMA400 3-axes ultra-low power > > accelerometer when configured to use SPI. > > ... > > > tristate "Bosch BMA400 3-Axis Accelerometer Driver" > > select REGMAP > > select BMA400_I2C if I2C > > > + select BMA400_SPI if I2C > > This is not right. Will fix in the second version. > > +#include <linux/module.h> > > +#include <linux/spi/spi.h> > > What's the point of dups (see below)? An error on my part. > > +#define BMA400_SPI_READ_BUFFER_SIZE (BMA400_MAX_SPI_READ + 1) > > Do wee need separate macro? It seems longer than explicit use. > Do we need the original macro either? I was just trying to avoid magic values. I have no problem with removing this though. > > + /* > > + * TODO(dlrobertson): What is a reasonable length to cap > > + * this at. > > + */ > > Either drop this or fulfill. There is no way to leave such in the > non-staging code. I'll drop this in the next patchset version if we stick with regmap_bus implementation. We never read more than two bytes in bma400_core, so this size should be fine. > > + .read_flag_mask = BIT(7), > > #include <linux/bits.h> Good catch. Thanks for the review. I will make the changes for the next patchset version. Cheers, - Dan [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 902 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-05-25 17:33 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-05-22 1:46 [PATCH 0/1] iio: accel: bma400: add support for bma400 spi Dan Robertson 2020-05-22 1:46 ` [PATCH 1/1] " Dan Robertson 2020-05-22 8:50 ` Andy Shevchenko 2020-05-22 12:36 ` Dan Robertson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).