| Message ID | f239834a6b8bd179094cdc19a3ac5ecaf807cee3.1562931742.git.vitor.soares@synopsys.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add ST lsm6dso i3c support | expand |
On Fri, 12 Jul 2019 13:53:30 +0200 Vitor Soares <Vitor.Soares@synopsys.com> wrote: > For today the st_lsm6dsx driver support LSM6DSO and LSM6DSR sensor only in > spi and i2c mode. > > The LSM6DSO and LSM6DSR are also i3c capable so lets give i3c support to > them. > > Signed-off-by: Vitor Soares <vitor.soares@synopsys.com> > Acked-by: Lorenzo Bianconi <lorenzo@kernel.org> > --- > Changes in v4: > Remove hw_id variable > > Changes in v3: > Remove unnecessary st_lsm6dsx_i3c_data table used to hold device name > Use st_lsm6dsx_probe new form > > Changes in v2: > Add support for LSM6DSR > Set pm_ops to st_lsm6dsx_pm_ops > > drivers/iio/imu/st_lsm6dsx/Kconfig | 8 +++- > drivers/iio/imu/st_lsm6dsx/Makefile | 1 + > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c | 58 +++++++++++++++++++++++++++++ > 3 files changed, 66 insertions(+), 1 deletion(-) > create mode 100644 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c > > diff --git a/drivers/iio/imu/st_lsm6dsx/Kconfig b/drivers/iio/imu/st_lsm6dsx/Kconfig > index 9e59297..6b5a73c 100644 > --- a/drivers/iio/imu/st_lsm6dsx/Kconfig > +++ b/drivers/iio/imu/st_lsm6dsx/Kconfig > @@ -1,11 +1,12 @@ > > config IIO_ST_LSM6DSX > tristate "ST_LSM6DSx driver for STM 6-axis IMU MEMS sensors" > - depends on (I2C || SPI) > + depends on (I2C || SPI || I3C) > select IIO_BUFFER > select IIO_KFIFO_BUF > select IIO_ST_LSM6DSX_I2C if (I2C) > select IIO_ST_LSM6DSX_SPI if (SPI_MASTER) > + select IIO_ST_LSM6DSX_I3C if (I3C) > help > Say yes here to build support for STMicroelectronics LSM6DSx imu > sensor. Supported devices: lsm6ds3, lsm6ds3h, lsm6dsl, lsm6dsm, > @@ -23,3 +24,8 @@ config IIO_ST_LSM6DSX_SPI > tristate > depends on IIO_ST_LSM6DSX > select REGMAP_SPI > + > +config IIO_ST_LSM6DSX_I3C > + tristate > + depends on IIO_ST_LSM6DSX > + select REGMAP_I3C > diff --git a/drivers/iio/imu/st_lsm6dsx/Makefile b/drivers/iio/imu/st_lsm6dsx/Makefile > index e5f733c..c676965 100644 > --- a/drivers/iio/imu/st_lsm6dsx/Makefile > +++ b/drivers/iio/imu/st_lsm6dsx/Makefile > @@ -4,3 +4,4 @@ st_lsm6dsx-y := st_lsm6dsx_core.o st_lsm6dsx_buffer.o \ > obj-$(CONFIG_IIO_ST_LSM6DSX) += st_lsm6dsx.o > obj-$(CONFIG_IIO_ST_LSM6DSX_I2C) += st_lsm6dsx_i2c.o > obj-$(CONFIG_IIO_ST_LSM6DSX_SPI) += st_lsm6dsx_spi.o > +obj-$(CONFIG_IIO_ST_LSM6DSX_I3C) += st_lsm6dsx_i3c.o > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c > new file mode 100644 > index 0000000..2e89524 > --- /dev/null > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c > @@ -0,0 +1,58 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2018 Synopsys, Inc. and/or its affiliates. > + * > + * Author: Vitor Soares <vitor.soares@synopsys.com> > + */ > + > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/i3c/device.h> > +#include <linux/i3c/master.h> > +#include <linux/slab.h> > +#include <linux/of.h> > +#include <linux/regmap.h> > + > +#include "st_lsm6dsx.h" > + > +static const struct i3c_device_id st_lsm6dsx_i3c_ids[] = { > + I3C_DEVICE(0x0104, 0x006C, (void *)ST_LSM6DSO_ID), > + I3C_DEVICE(0x0104, 0x006B, (void *)ST_LSM6DSR_ID), I think you need an uintptr_t cast here: I3C_DEVICE(0x0104, 0x006C, (void *)(uintptr_t)ST_LSM6DSO_ID), I3C_DEVICE(0x0104, 0x006B, (void *)(uintptr_t)ST_LSM6DSR_ID), otherwise gcc might complain that the integer and pointer do not have the same size (on 64-bit architectures). > + { /* sentinel */ }, > +}; > +MODULE_DEVICE_TABLE(i3c, st_lsm6dsx_i3c_ids); > + > +static const struct regmap_config st_lsm6dsx_i3c_regmap_config = { > + .reg_bits = 8, > + .val_bits = 8, > +}; This can be moved ... > + > +static int st_lsm6dsx_i3c_probe(struct i3c_device *i3cdev) > +{ ... here without the static and const qualifiers: struct regmap_config regmap_config = { .reg_bits = 8, .val_bits = 8, }; > + const struct i3c_device_id *id = i3c_device_match_id(i3cdev, > + st_lsm6dsx_i3c_ids); > + struct regmap *regmap; > + > + regmap = devm_regmap_init_i3c(i3cdev, &st_lsm6dsx_i3c_regmap_config); > + if (IS_ERR(regmap)) { > + dev_err(&i3cdev->dev, "Failed to register i3c regmap %d\n", > + (int)PTR_ERR(regmap)); > + return PTR_ERR(regmap); > + } > + > + return st_lsm6dsx_probe(&i3cdev->dev, 0, (int)id->data, regmap); uintptr_t cast here. > +} > + > +static struct i3c_driver st_lsm6dsx_driver = { > + .driver = { > + .name = "st_lsm6dsx_i3c", > + .pm = &st_lsm6dsx_pm_ops, > + }, > + .probe = st_lsm6dsx_i3c_probe, > + .id_table = st_lsm6dsx_i3c_ids, > +}; > +module_i3c_driver(st_lsm6dsx_driver); > + > +MODULE_AUTHOR("Vitor Soares <vitor.soares@synopsys.com>"); > +MODULE_DESCRIPTION("STMicroelectronics st_lsm6dsx i3c driver"); > +MODULE_LICENSE("GPL v2");
From: Boris Brezillon <boris.brezillon@collabora.com> Date: Fri, Jul 12, 2019 at 17:14:29 > On Fri, 12 Jul 2019 13:53:30 +0200 > Vitor Soares <Vitor.Soares@synopsys.com> wrote: > > > For today the st_lsm6dsx driver support LSM6DSO and LSM6DSR sensor only in > > spi and i2c mode. > > > > The LSM6DSO and LSM6DSR are also i3c capable so lets give i3c support to > > them. > > > > Signed-off-by: Vitor Soares <vitor.soares@synopsys.com> > > Acked-by: Lorenzo Bianconi <lorenzo@kernel.org> > > --- > > Changes in v4: > > Remove hw_id variable > > > > Changes in v3: > > Remove unnecessary st_lsm6dsx_i3c_data table used to hold device name > > Use st_lsm6dsx_probe new form > > > > Changes in v2: > > Add support for LSM6DSR > > Set pm_ops to st_lsm6dsx_pm_ops > > > > drivers/iio/imu/st_lsm6dsx/Kconfig | 8 +++- > > drivers/iio/imu/st_lsm6dsx/Makefile | 1 + > > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c | 58 +++++++++++++++++++++++++++++ > > 3 files changed, 66 insertions(+), 1 deletion(-) > > create mode 100644 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c > > > > diff --git a/drivers/iio/imu/st_lsm6dsx/Kconfig b/drivers/iio/imu/st_lsm6dsx/Kconfig > > index 9e59297..6b5a73c 100644 > > --- a/drivers/iio/imu/st_lsm6dsx/Kconfig > > +++ b/drivers/iio/imu/st_lsm6dsx/Kconfig > > @@ -1,11 +1,12 @@ > > > > config IIO_ST_LSM6DSX > > tristate "ST_LSM6DSx driver for STM 6-axis IMU MEMS sensors" > > - depends on (I2C || SPI) > > + depends on (I2C || SPI || I3C) > > select IIO_BUFFER > > select IIO_KFIFO_BUF > > select IIO_ST_LSM6DSX_I2C if (I2C) > > select IIO_ST_LSM6DSX_SPI if (SPI_MASTER) > > + select IIO_ST_LSM6DSX_I3C if (I3C) > > help > > Say yes here to build support for STMicroelectronics LSM6DSx imu > > sensor. Supported devices: lsm6ds3, lsm6ds3h, lsm6dsl, lsm6dsm, > > @@ -23,3 +24,8 @@ config IIO_ST_LSM6DSX_SPI > > tristate > > depends on IIO_ST_LSM6DSX > > select REGMAP_SPI > > + > > +config IIO_ST_LSM6DSX_I3C > > + tristate > > + depends on IIO_ST_LSM6DSX > > + select REGMAP_I3C > > diff --git a/drivers/iio/imu/st_lsm6dsx/Makefile b/drivers/iio/imu/st_lsm6dsx/Makefile > > index e5f733c..c676965 100644 > > --- a/drivers/iio/imu/st_lsm6dsx/Makefile > > +++ b/drivers/iio/imu/st_lsm6dsx/Makefile > > @@ -4,3 +4,4 @@ st_lsm6dsx-y := st_lsm6dsx_core.o st_lsm6dsx_buffer.o \ > > obj-$(CONFIG_IIO_ST_LSM6DSX) += st_lsm6dsx.o > > obj-$(CONFIG_IIO_ST_LSM6DSX_I2C) += st_lsm6dsx_i2c.o > > obj-$(CONFIG_IIO_ST_LSM6DSX_SPI) += st_lsm6dsx_spi.o > > +obj-$(CONFIG_IIO_ST_LSM6DSX_I3C) += st_lsm6dsx_i3c.o > > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c > > new file mode 100644 > > index 0000000..2e89524 > > --- /dev/null > > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c > > @@ -0,0 +1,58 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Copyright (c) 2018 Synopsys, Inc. and/or its affiliates. > > + * > > + * Author: Vitor Soares <vitor.soares@synopsys.com> > > + */ > > + > > +#include <linux/kernel.h> > > +#include <linux/module.h> > > +#include <linux/i3c/device.h> > > +#include <linux/i3c/master.h> > > +#include <linux/slab.h> > > +#include <linux/of.h> > > +#include <linux/regmap.h> > > + > > +#include "st_lsm6dsx.h" > > + > > +static const struct i3c_device_id st_lsm6dsx_i3c_ids[] = { > > + I3C_DEVICE(0x0104, 0x006C, (void *)ST_LSM6DSO_ID), > > + I3C_DEVICE(0x0104, 0x006B, (void *)ST_LSM6DSR_ID), > > I think you need an uintptr_t cast here: > > I3C_DEVICE(0x0104, 0x006C, (void *)(uintptr_t)ST_LSM6DSO_ID), > I3C_DEVICE(0x0104, 0x006B, (void *)(uintptr_t)ST_LSM6DSR_ID), > > otherwise gcc might complain that the integer and pointer do not have > the same size (on 64-bit architectures). I don't understand this part. Can you provide or point some background? > > > + { /* sentinel */ }, > > +}; > > +MODULE_DEVICE_TABLE(i3c, st_lsm6dsx_i3c_ids); > > + > > +static const struct regmap_config st_lsm6dsx_i3c_regmap_config = { > > + .reg_bits = 8, > > + .val_bits = 8, > > +}; > > This can be moved ... > > > + > > +static int st_lsm6dsx_i3c_probe(struct i3c_device *i3cdev) > > +{ > > ... here without the static and const qualifiers: I understand that can be move to here, but I don't understand the advantages. Can you explain? > > struct regmap_config regmap_config = { > .reg_bits = 8, > .val_bits = 8, > }; > > > + const struct i3c_device_id *id = i3c_device_match_id(i3cdev, > > + st_lsm6dsx_i3c_ids); > > + struct regmap *regmap; > > + > > + regmap = devm_regmap_init_i3c(i3cdev, &st_lsm6dsx_i3c_regmap_config); > > + if (IS_ERR(regmap)) { > > + dev_err(&i3cdev->dev, "Failed to register i3c regmap %d\n", > > + (int)PTR_ERR(regmap)); > > + return PTR_ERR(regmap); > > + } > > + > > + return st_lsm6dsx_probe(&i3cdev->dev, 0, (int)id->data, regmap); > > uintptr_t cast here. > > > +} > > + > > +static struct i3c_driver st_lsm6dsx_driver = { > > + .driver = { > > + .name = "st_lsm6dsx_i3c", > > + .pm = &st_lsm6dsx_pm_ops, > > + }, > > + .probe = st_lsm6dsx_i3c_probe, > > + .id_table = st_lsm6dsx_i3c_ids, > > +}; > > +module_i3c_driver(st_lsm6dsx_driver); > > + > > +MODULE_AUTHOR("Vitor Soares <vitor.soares@synopsys.com>"); > > +MODULE_DESCRIPTION("STMicroelectronics st_lsm6dsx i3c driver"); > > +MODULE_LICENSE("GPL v2"); Best regards, Vitor Soares
On Fri, 12 Jul 2019 16:28:02 +0000 Vitor Soares <Vitor.Soares@synopsys.com> wrote: > From: Boris Brezillon <boris.brezillon@collabora.com> > Date: Fri, Jul 12, 2019 at 17:14:29 > > > On Fri, 12 Jul 2019 13:53:30 +0200 > > Vitor Soares <Vitor.Soares@synopsys.com> wrote: > > > > > For today the st_lsm6dsx driver support LSM6DSO and LSM6DSR sensor only in > > > spi and i2c mode. > > > > > > The LSM6DSO and LSM6DSR are also i3c capable so lets give i3c support to > > > them. > > > > > > Signed-off-by: Vitor Soares <vitor.soares@synopsys.com> > > > Acked-by: Lorenzo Bianconi <lorenzo@kernel.org> > > > --- > > > Changes in v4: > > > Remove hw_id variable > > > > > > Changes in v3: > > > Remove unnecessary st_lsm6dsx_i3c_data table used to hold device name > > > Use st_lsm6dsx_probe new form > > > > > > Changes in v2: > > > Add support for LSM6DSR > > > Set pm_ops to st_lsm6dsx_pm_ops > > > > > > drivers/iio/imu/st_lsm6dsx/Kconfig | 8 +++- > > > drivers/iio/imu/st_lsm6dsx/Makefile | 1 + > > > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c | 58 +++++++++++++++++++++++++++++ > > > 3 files changed, 66 insertions(+), 1 deletion(-) > > > create mode 100644 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c > > > > > > diff --git a/drivers/iio/imu/st_lsm6dsx/Kconfig b/drivers/iio/imu/st_lsm6dsx/Kconfig > > > index 9e59297..6b5a73c 100644 > > > --- a/drivers/iio/imu/st_lsm6dsx/Kconfig > > > +++ b/drivers/iio/imu/st_lsm6dsx/Kconfig > > > @@ -1,11 +1,12 @@ > > > > > > config IIO_ST_LSM6DSX > > > tristate "ST_LSM6DSx driver for STM 6-axis IMU MEMS sensors" > > > - depends on (I2C || SPI) > > > + depends on (I2C || SPI || I3C) > > > select IIO_BUFFER > > > select IIO_KFIFO_BUF > > > select IIO_ST_LSM6DSX_I2C if (I2C) > > > select IIO_ST_LSM6DSX_SPI if (SPI_MASTER) > > > + select IIO_ST_LSM6DSX_I3C if (I3C) > > > help > > > Say yes here to build support for STMicroelectronics LSM6DSx imu > > > sensor. Supported devices: lsm6ds3, lsm6ds3h, lsm6dsl, lsm6dsm, > > > @@ -23,3 +24,8 @@ config IIO_ST_LSM6DSX_SPI > > > tristate > > > depends on IIO_ST_LSM6DSX > > > select REGMAP_SPI > > > + > > > +config IIO_ST_LSM6DSX_I3C > > > + tristate > > > + depends on IIO_ST_LSM6DSX > > > + select REGMAP_I3C > > > diff --git a/drivers/iio/imu/st_lsm6dsx/Makefile b/drivers/iio/imu/st_lsm6dsx/Makefile > > > index e5f733c..c676965 100644 > > > --- a/drivers/iio/imu/st_lsm6dsx/Makefile > > > +++ b/drivers/iio/imu/st_lsm6dsx/Makefile > > > @@ -4,3 +4,4 @@ st_lsm6dsx-y := st_lsm6dsx_core.o st_lsm6dsx_buffer.o \ > > > obj-$(CONFIG_IIO_ST_LSM6DSX) += st_lsm6dsx.o > > > obj-$(CONFIG_IIO_ST_LSM6DSX_I2C) += st_lsm6dsx_i2c.o > > > obj-$(CONFIG_IIO_ST_LSM6DSX_SPI) += st_lsm6dsx_spi.o > > > +obj-$(CONFIG_IIO_ST_LSM6DSX_I3C) += st_lsm6dsx_i3c.o > > > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c > > > new file mode 100644 > > > index 0000000..2e89524 > > > --- /dev/null > > > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c > > > @@ -0,0 +1,58 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +/* > > > + * Copyright (c) 2018 Synopsys, Inc. and/or its affiliates. > > > + * > > > + * Author: Vitor Soares <vitor.soares@synopsys.com> > > > + */ > > > + > > > +#include <linux/kernel.h> > > > +#include <linux/module.h> > > > +#include <linux/i3c/device.h> > > > +#include <linux/i3c/master.h> > > > +#include <linux/slab.h> > > > +#include <linux/of.h> > > > +#include <linux/regmap.h> > > > + > > > +#include "st_lsm6dsx.h" > > > + > > > +static const struct i3c_device_id st_lsm6dsx_i3c_ids[] = { > > > + I3C_DEVICE(0x0104, 0x006C, (void *)ST_LSM6DSO_ID), > > > + I3C_DEVICE(0x0104, 0x006B, (void *)ST_LSM6DSR_ID), > > > > I think you need an uintptr_t cast here: > > > > I3C_DEVICE(0x0104, 0x006C, (void *)(uintptr_t)ST_LSM6DSO_ID), > > I3C_DEVICE(0x0104, 0x006B, (void *)(uintptr_t)ST_LSM6DSR_ID), > > > > otherwise gcc might complain that the integer and pointer do not have > > the same size (on 64-bit architectures). > > I don't understand this part. Can you provide or point some background? If you don't do that you'll get the following warning: warning: cast to 'void *' from smaller integer type 'int' [-Wint-to-void-pointer-cast] > > > > > > + { /* sentinel */ }, > > > +}; > > > +MODULE_DEVICE_TABLE(i3c, st_lsm6dsx_i3c_ids); > > > + > > > +static const struct regmap_config st_lsm6dsx_i3c_regmap_config = { > > > + .reg_bits = 8, > > > + .val_bits = 8, > > > +}; > > > > This can be moved ... > > > > > + > > > +static int st_lsm6dsx_i3c_probe(struct i3c_device *i3cdev) > > > +{ > > > > ... here without the static and const qualifiers: > > I understand that can be move to here, but I don't understand the > advantages. Can you explain? It reduces the variable scope (this variable is only needed in the probe path) and avoids consuming space in the .bss section, though the second point is not so important.
From: Boris Brezillon <boris.brezillon@collabora.com> Date: Fri, Jul 12, 2019 at 17:43:23 > On Fri, 12 Jul 2019 16:28:02 +0000 > Vitor Soares <Vitor.Soares@synopsys.com> wrote: > > > From: Boris Brezillon <boris.brezillon@collabora.com> > > Date: Fri, Jul 12, 2019 at 17:14:29 > > > > > On Fri, 12 Jul 2019 13:53:30 +0200 > > > Vitor Soares <Vitor.Soares@synopsys.com> wrote: > > > > > > > For today the st_lsm6dsx driver support LSM6DSO and LSM6DSR sensor only in > > > > spi and i2c mode. > > > > > > > > The LSM6DSO and LSM6DSR are also i3c capable so lets give i3c support to > > > > them. > > > > > > > > Signed-off-by: Vitor Soares <vitor.soares@synopsys.com> > > > > Acked-by: Lorenzo Bianconi <lorenzo@kernel.org> > > > > --- > > > > Changes in v4: > > > > Remove hw_id variable > > > > > > > > Changes in v3: > > > > Remove unnecessary st_lsm6dsx_i3c_data table used to hold device name > > > > Use st_lsm6dsx_probe new form > > > > > > > > Changes in v2: > > > > Add support for LSM6DSR > > > > Set pm_ops to st_lsm6dsx_pm_ops > > > > > > > > drivers/iio/imu/st_lsm6dsx/Kconfig | 8 +++- > > > > drivers/iio/imu/st_lsm6dsx/Makefile | 1 + > > > > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c | 58 +++++++++++++++++++++++++++++ > > > > 3 files changed, 66 insertions(+), 1 deletion(-) > > > > create mode 100644 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c > > > > > > > > diff --git a/drivers/iio/imu/st_lsm6dsx/Kconfig b/drivers/iio/imu/st_lsm6dsx/Kconfig > > > > index 9e59297..6b5a73c 100644 > > > > --- a/drivers/iio/imu/st_lsm6dsx/Kconfig > > > > +++ b/drivers/iio/imu/st_lsm6dsx/Kconfig > > > > @@ -1,11 +1,12 @@ > > > > > > > > config IIO_ST_LSM6DSX > > > > tristate "ST_LSM6DSx driver for STM 6-axis IMU MEMS sensors" > > > > - depends on (I2C || SPI) > > > > + depends on (I2C || SPI || I3C) > > > > select IIO_BUFFER > > > > select IIO_KFIFO_BUF > > > > select IIO_ST_LSM6DSX_I2C if (I2C) > > > > select IIO_ST_LSM6DSX_SPI if (SPI_MASTER) > > > > + select IIO_ST_LSM6DSX_I3C if (I3C) > > > > help > > > > Say yes here to build support for STMicroelectronics LSM6DSx imu > > > > sensor. Supported devices: lsm6ds3, lsm6ds3h, lsm6dsl, lsm6dsm, > > > > @@ -23,3 +24,8 @@ config IIO_ST_LSM6DSX_SPI > > > > tristate > > > > depends on IIO_ST_LSM6DSX > > > > select REGMAP_SPI > > > > + > > > > +config IIO_ST_LSM6DSX_I3C > > > > + tristate > > > > + depends on IIO_ST_LSM6DSX > > > > + select REGMAP_I3C > > > > diff --git a/drivers/iio/imu/st_lsm6dsx/Makefile b/drivers/iio/imu/st_lsm6dsx/Makefile > > > > index e5f733c..c676965 100644 > > > > --- a/drivers/iio/imu/st_lsm6dsx/Makefile > > > > +++ b/drivers/iio/imu/st_lsm6dsx/Makefile > > > > @@ -4,3 +4,4 @@ st_lsm6dsx-y := st_lsm6dsx_core.o st_lsm6dsx_buffer.o \ > > > > obj-$(CONFIG_IIO_ST_LSM6DSX) += st_lsm6dsx.o > > > > obj-$(CONFIG_IIO_ST_LSM6DSX_I2C) += st_lsm6dsx_i2c.o > > > > obj-$(CONFIG_IIO_ST_LSM6DSX_SPI) += st_lsm6dsx_spi.o > > > > +obj-$(CONFIG_IIO_ST_LSM6DSX_I3C) += st_lsm6dsx_i3c.o > > > > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c > > > > new file mode 100644 > > > > index 0000000..2e89524 > > > > --- /dev/null > > > > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c > > > > @@ -0,0 +1,58 @@ > > > > +// SPDX-License-Identifier: GPL-2.0 > > > > +/* > > > > + * Copyright (c) 2018 Synopsys, Inc. and/or its affiliates. > > > > + * > > > > + * Author: Vitor Soares <vitor.soares@synopsys.com> > > > > + */ > > > > + > > > > +#include <linux/kernel.h> > > > > +#include <linux/module.h> > > > > +#include <linux/i3c/device.h> > > > > +#include <linux/i3c/master.h> > > > > +#include <linux/slab.h> > > > > +#include <linux/of.h> > > > > +#include <linux/regmap.h> > > > > + > > > > +#include "st_lsm6dsx.h" > > > > + > > > > +static const struct i3c_device_id st_lsm6dsx_i3c_ids[] = { > > > > + I3C_DEVICE(0x0104, 0x006C, (void *)ST_LSM6DSO_ID), > > > > + I3C_DEVICE(0x0104, 0x006B, (void *)ST_LSM6DSR_ID), > > > > > > I think you need an uintptr_t cast here: > > > > > > I3C_DEVICE(0x0104, 0x006C, (void *)(uintptr_t)ST_LSM6DSO_ID), > > > I3C_DEVICE(0x0104, 0x006B, (void *)(uintptr_t)ST_LSM6DSR_ID), > > > > > > otherwise gcc might complain that the integer and pointer do not have > > > the same size (on 64-bit architectures). > > > > I don't understand this part. Can you provide or point some background? > > If you don't do that you'll get the following warning: > > warning: cast to 'void *' from smaller integer type 'int' [-Wint-to-void-pointer-cast] I don't get the warning during compilation. Is there any flag to enable or so? > > > > > > > > > > + { /* sentinel */ }, > > > > +}; > > > > +MODULE_DEVICE_TABLE(i3c, st_lsm6dsx_i3c_ids); > > > > + > > > > +static const struct regmap_config st_lsm6dsx_i3c_regmap_config = { > > > > + .reg_bits = 8, > > > > + .val_bits = 8, > > > > +}; > > > > > > This can be moved ... > > > > > > > + > > > > +static int st_lsm6dsx_i3c_probe(struct i3c_device *i3cdev) > > > > +{ > > > > > > ... here without the static and const qualifiers: > > > > I understand that can be move to here, but I don't understand the > > advantages. Can you explain? > > It reduces the variable scope (this variable is only needed in the > probe path) and avoids consuming space in the .bss section, though the > second point is not so important. Thanks, Vitor Soares
On Fri, 12 Jul 2019 18:40:14 +0000 Vitor Soares <Vitor.Soares@synopsys.com> wrote: > From: Boris Brezillon <boris.brezillon@collabora.com> > Date: Fri, Jul 12, 2019 at 17:43:23 > > > On Fri, 12 Jul 2019 16:28:02 +0000 > > Vitor Soares <Vitor.Soares@synopsys.com> wrote: > > > > > From: Boris Brezillon <boris.brezillon@collabora.com> > > > Date: Fri, Jul 12, 2019 at 17:14:29 > > > > > > > On Fri, 12 Jul 2019 13:53:30 +0200 > > > > Vitor Soares <Vitor.Soares@synopsys.com> wrote: > > > > > > > > > For today the st_lsm6dsx driver support LSM6DSO and LSM6DSR sensor only in > > > > > spi and i2c mode. > > > > > > > > > > The LSM6DSO and LSM6DSR are also i3c capable so lets give i3c support to > > > > > them. > > > > > > > > > > Signed-off-by: Vitor Soares <vitor.soares@synopsys.com> > > > > > Acked-by: Lorenzo Bianconi <lorenzo@kernel.org> > > > > > --- > > > > > Changes in v4: > > > > > Remove hw_id variable > > > > > > > > > > Changes in v3: > > > > > Remove unnecessary st_lsm6dsx_i3c_data table used to hold device name > > > > > Use st_lsm6dsx_probe new form > > > > > > > > > > Changes in v2: > > > > > Add support for LSM6DSR > > > > > Set pm_ops to st_lsm6dsx_pm_ops > > > > > > > > > > drivers/iio/imu/st_lsm6dsx/Kconfig | 8 +++- > > > > > drivers/iio/imu/st_lsm6dsx/Makefile | 1 + > > > > > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c | 58 +++++++++++++++++++++++++++++ > > > > > 3 files changed, 66 insertions(+), 1 deletion(-) > > > > > create mode 100644 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c > > > > > > > > > > diff --git a/drivers/iio/imu/st_lsm6dsx/Kconfig b/drivers/iio/imu/st_lsm6dsx/Kconfig > > > > > index 9e59297..6b5a73c 100644 > > > > > --- a/drivers/iio/imu/st_lsm6dsx/Kconfig > > > > > +++ b/drivers/iio/imu/st_lsm6dsx/Kconfig > > > > > @@ -1,11 +1,12 @@ > > > > > > > > > > config IIO_ST_LSM6DSX > > > > > tristate "ST_LSM6DSx driver for STM 6-axis IMU MEMS sensors" > > > > > - depends on (I2C || SPI) > > > > > + depends on (I2C || SPI || I3C) > > > > > select IIO_BUFFER > > > > > select IIO_KFIFO_BUF > > > > > select IIO_ST_LSM6DSX_I2C if (I2C) > > > > > select IIO_ST_LSM6DSX_SPI if (SPI_MASTER) > > > > > + select IIO_ST_LSM6DSX_I3C if (I3C) > > > > > help > > > > > Say yes here to build support for STMicroelectronics LSM6DSx imu > > > > > sensor. Supported devices: lsm6ds3, lsm6ds3h, lsm6dsl, lsm6dsm, > > > > > @@ -23,3 +24,8 @@ config IIO_ST_LSM6DSX_SPI > > > > > tristate > > > > > depends on IIO_ST_LSM6DSX > > > > > select REGMAP_SPI > > > > > + > > > > > +config IIO_ST_LSM6DSX_I3C > > > > > + tristate > > > > > + depends on IIO_ST_LSM6DSX > > > > > + select REGMAP_I3C > > > > > diff --git a/drivers/iio/imu/st_lsm6dsx/Makefile b/drivers/iio/imu/st_lsm6dsx/Makefile > > > > > index e5f733c..c676965 100644 > > > > > --- a/drivers/iio/imu/st_lsm6dsx/Makefile > > > > > +++ b/drivers/iio/imu/st_lsm6dsx/Makefile > > > > > @@ -4,3 +4,4 @@ st_lsm6dsx-y := st_lsm6dsx_core.o st_lsm6dsx_buffer.o \ > > > > > obj-$(CONFIG_IIO_ST_LSM6DSX) += st_lsm6dsx.o > > > > > obj-$(CONFIG_IIO_ST_LSM6DSX_I2C) += st_lsm6dsx_i2c.o > > > > > obj-$(CONFIG_IIO_ST_LSM6DSX_SPI) += st_lsm6dsx_spi.o > > > > > +obj-$(CONFIG_IIO_ST_LSM6DSX_I3C) += st_lsm6dsx_i3c.o > > > > > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c > > > > > new file mode 100644 > > > > > index 0000000..2e89524 > > > > > --- /dev/null > > > > > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c > > > > > @@ -0,0 +1,58 @@ > > > > > +// SPDX-License-Identifier: GPL-2.0 > > > > > +/* > > > > > + * Copyright (c) 2018 Synopsys, Inc. and/or its affiliates. > > > > > + * > > > > > + * Author: Vitor Soares <vitor.soares@synopsys.com> > > > > > + */ > > > > > + > > > > > +#include <linux/kernel.h> > > > > > +#include <linux/module.h> > > > > > +#include <linux/i3c/device.h> > > > > > +#include <linux/i3c/master.h> > > > > > +#include <linux/slab.h> > > > > > +#include <linux/of.h> > > > > > +#include <linux/regmap.h> > > > > > + > > > > > +#include "st_lsm6dsx.h" > > > > > + > > > > > +static const struct i3c_device_id st_lsm6dsx_i3c_ids[] = { > > > > > + I3C_DEVICE(0x0104, 0x006C, (void *)ST_LSM6DSO_ID), > > > > > + I3C_DEVICE(0x0104, 0x006B, (void *)ST_LSM6DSR_ID), > > > > > > > > I think you need an uintptr_t cast here: > > > > > > > > I3C_DEVICE(0x0104, 0x006C, (void *)(uintptr_t)ST_LSM6DSO_ID), > > > > I3C_DEVICE(0x0104, 0x006B, (void *)(uintptr_t)ST_LSM6DSR_ID), > > > > > > > > otherwise gcc might complain that the integer and pointer do not have > > > > the same size (on 64-bit architectures). > > > > > > I don't understand this part. Can you provide or point some background? > > > > If you don't do that you'll get the following warning: > > > > warning: cast to 'void *' from smaller integer type 'int' [-Wint-to-void-pointer-cast] > > I don't get the warning during compilation. Is there any flag to enable > or so? Nope, nothing specific to enable, just enable this driver on an arm64 config. Note that that gcc doesn't seem to complain about this int -> void * cast (there's probably some kind of auto-promotion to pointer size), but it does complains about the following void * -> int cast: drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c: In function ‘st_lsm6dsx_i3c_probe’: drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c:43:43: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] 43 | return st_lsm6dsx_probe(&i3cdev->dev, 0, (int)id->data, regmap); |
Hi Boris, From: Boris Brezillon <boris.brezillon@collabora.com> Date: Fri, Jul 12, 2019 at 21:03:20 > On Fri, 12 Jul 2019 18:40:14 +0000 > Vitor Soares <Vitor.Soares@synopsys.com> wrote: > > > From: Boris Brezillon <boris.brezillon@collabora.com> > > Date: Fri, Jul 12, 2019 at 17:43:23 > > > > > On Fri, 12 Jul 2019 16:28:02 +0000 > > > Vitor Soares <Vitor.Soares@synopsys.com> wrote: > > > > > > > From: Boris Brezillon <boris.brezillon@collabora.com> > > > > Date: Fri, Jul 12, 2019 at 17:14:29 > > > > > > > > > On Fri, 12 Jul 2019 13:53:30 +0200 > > > > > Vitor Soares <Vitor.Soares@synopsys.com> wrote: > > > > > > > > > > > For today the st_lsm6dsx driver support LSM6DSO and LSM6DSR sensor only in > > > > > > spi and i2c mode. > > > > > > > > > > > > The LSM6DSO and LSM6DSR are also i3c capable so lets give i3c support to > > > > > > them. > > > > > > > > > > > > Signed-off-by: Vitor Soares <vitor.soares@synopsys.com> > > > > > > Acked-by: Lorenzo Bianconi <lorenzo@kernel.org> > > > > > > --- > > > > > > Changes in v4: > > > > > > Remove hw_id variable > > > > > > > > > > > > Changes in v3: > > > > > > Remove unnecessary st_lsm6dsx_i3c_data table used to hold device name > > > > > > Use st_lsm6dsx_probe new form > > > > > > > > > > > > Changes in v2: > > > > > > Add support for LSM6DSR > > > > > > Set pm_ops to st_lsm6dsx_pm_ops > > > > > > > > > > > > drivers/iio/imu/st_lsm6dsx/Kconfig | 8 +++- > > > > > > drivers/iio/imu/st_lsm6dsx/Makefile | 1 + > > > > > > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c | 58 +++++++++++++++++++++++++++++ > > > > > > 3 files changed, 66 insertions(+), 1 deletion(-) > > > > > > create mode 100644 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c > > > > > > > > > > > > diff --git a/drivers/iio/imu/st_lsm6dsx/Kconfig b/drivers/iio/imu/st_lsm6dsx/Kconfig > > > > > > index 9e59297..6b5a73c 100644 > > > > > > --- a/drivers/iio/imu/st_lsm6dsx/Kconfig > > > > > > +++ b/drivers/iio/imu/st_lsm6dsx/Kconfig > > > > > > @@ -1,11 +1,12 @@ > > > > > > > > > > > > config IIO_ST_LSM6DSX > > > > > > tristate "ST_LSM6DSx driver for STM 6-axis IMU MEMS sensors" > > > > > > - depends on (I2C || SPI) > > > > > > + depends on (I2C || SPI || I3C) > > > > > > select IIO_BUFFER > > > > > > select IIO_KFIFO_BUF > > > > > > select IIO_ST_LSM6DSX_I2C if (I2C) > > > > > > select IIO_ST_LSM6DSX_SPI if (SPI_MASTER) > > > > > > + select IIO_ST_LSM6DSX_I3C if (I3C) > > > > > > help > > > > > > Say yes here to build support for STMicroelectronics LSM6DSx imu > > > > > > sensor. Supported devices: lsm6ds3, lsm6ds3h, lsm6dsl, lsm6dsm, > > > > > > @@ -23,3 +24,8 @@ config IIO_ST_LSM6DSX_SPI > > > > > > tristate > > > > > > depends on IIO_ST_LSM6DSX > > > > > > select REGMAP_SPI > > > > > > + > > > > > > +config IIO_ST_LSM6DSX_I3C > > > > > > + tristate > > > > > > + depends on IIO_ST_LSM6DSX > > > > > > + select REGMAP_I3C > > > > > > diff --git a/drivers/iio/imu/st_lsm6dsx/Makefile b/drivers/iio/imu/st_lsm6dsx/Makefile > > > > > > index e5f733c..c676965 100644 > > > > > > --- a/drivers/iio/imu/st_lsm6dsx/Makefile > > > > > > +++ b/drivers/iio/imu/st_lsm6dsx/Makefile > > > > > > @@ -4,3 +4,4 @@ st_lsm6dsx-y := st_lsm6dsx_core.o st_lsm6dsx_buffer.o \ > > > > > > obj-$(CONFIG_IIO_ST_LSM6DSX) += st_lsm6dsx.o > > > > > > obj-$(CONFIG_IIO_ST_LSM6DSX_I2C) += st_lsm6dsx_i2c.o > > > > > > obj-$(CONFIG_IIO_ST_LSM6DSX_SPI) += st_lsm6dsx_spi.o > > > > > > +obj-$(CONFIG_IIO_ST_LSM6DSX_I3C) += st_lsm6dsx_i3c.o > > > > > > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c > > > > > > new file mode 100644 > > > > > > index 0000000..2e89524 > > > > > > --- /dev/null > > > > > > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c > > > > > > @@ -0,0 +1,58 @@ > > > > > > +// SPDX-License-Identifier: GPL-2.0 > > > > > > +/* > > > > > > + * Copyright (c) 2018 Synopsys, Inc. and/or its affiliates. > > > > > > + * > > > > > > + * Author: Vitor Soares <vitor.soares@synopsys.com> > > > > > > + */ > > > > > > + > > > > > > +#include <linux/kernel.h> > > > > > > +#include <linux/module.h> > > > > > > +#include <linux/i3c/device.h> > > > > > > +#include <linux/i3c/master.h> > > > > > > +#include <linux/slab.h> > > > > > > +#include <linux/of.h> > > > > > > +#include <linux/regmap.h> > > > > > > + > > > > > > +#include "st_lsm6dsx.h" > > > > > > + > > > > > > +static const struct i3c_device_id st_lsm6dsx_i3c_ids[] = { > > > > > > + I3C_DEVICE(0x0104, 0x006C, (void *)ST_LSM6DSO_ID), > > > > > > + I3C_DEVICE(0x0104, 0x006B, (void *)ST_LSM6DSR_ID), > > > > > > > > > > I think you need an uintptr_t cast here: > > > > > > > > > > I3C_DEVICE(0x0104, 0x006C, (void *)(uintptr_t)ST_LSM6DSO_ID), > > > > > I3C_DEVICE(0x0104, 0x006B, (void *)(uintptr_t)ST_LSM6DSR_ID), > > > > > > > > > > otherwise gcc might complain that the integer and pointer do not have > > > > > the same size (on 64-bit architectures). > > > > > > > > I don't understand this part. Can you provide or point some background? > > > > > > If you don't do that you'll get the following warning: > > > > > > warning: cast to 'void *' from smaller integer type 'int' [-Wint-to-void-pointer-cast] > > > > I don't get the warning during compilation. Is there any flag to enable > > or so? > > Nope, nothing specific to enable, just enable this driver on an arm64 > config. Note that that gcc doesn't seem to complain about this > int -> void * cast (there's probably some kind of auto-promotion to > pointer size), but it does complains about the following void * -> int > cast: > > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c: In function ‘st_lsm6dsx_i3c_probe’: > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c:43:43: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] > 43 | return st_lsm6dsx_probe(&i3cdev->dev, 0, (int)id->data, regmap); > | I fixed the warning by changing: st_lsm6dsx_probe(&i3cdev->dev, 0, (int)id->data, regmap); to st_lsm6dsx_probe(&i3cdev->dev, 0, (int)(uintptr_t)id->data, regmap); But I wonder if it isn't more save to change the following too: I3C_DEVICE(0x0104, 0x006C, (void *)(uintptr_t)ST_LSM6DSO_ID), I3C_DEVICE(0x0104, 0x006B, (void *)(uintptr_t)ST_LSM6DSR_ID), What do you think? Best regards, Vitor Soares
On Tue, 16 Jul 2019 13:22:25 +0000 Vitor Soares <Vitor.Soares@synopsys.com> wrote: > Hi Boris, > > From: Boris Brezillon <boris.brezillon@collabora.com> > Date: Fri, Jul 12, 2019 at 21:03:20 > > > On Fri, 12 Jul 2019 18:40:14 +0000 > > Vitor Soares <Vitor.Soares@synopsys.com> wrote: > > > > > From: Boris Brezillon <boris.brezillon@collabora.com> > > > Date: Fri, Jul 12, 2019 at 17:43:23 > > > > > > > On Fri, 12 Jul 2019 16:28:02 +0000 > > > > Vitor Soares <Vitor.Soares@synopsys.com> wrote: > > > > > > > > > From: Boris Brezillon <boris.brezillon@collabora.com> > > > > > Date: Fri, Jul 12, 2019 at 17:14:29 > > > > > > > > > > > On Fri, 12 Jul 2019 13:53:30 +0200 > > > > > > Vitor Soares <Vitor.Soares@synopsys.com> wrote: > > > > > > > > > > > > > For today the st_lsm6dsx driver support LSM6DSO and LSM6DSR sensor only in > > > > > > > spi and i2c mode. > > > > > > > > > > > > > > The LSM6DSO and LSM6DSR are also i3c capable so lets give i3c support to > > > > > > > them. > > > > > > > > > > > > > > Signed-off-by: Vitor Soares <vitor.soares@synopsys.com> > > > > > > > Acked-by: Lorenzo Bianconi <lorenzo@kernel.org> > > > > > > > --- > > > > > > > Changes in v4: > > > > > > > Remove hw_id variable > > > > > > > > > > > > > > Changes in v3: > > > > > > > Remove unnecessary st_lsm6dsx_i3c_data table used to hold device name > > > > > > > Use st_lsm6dsx_probe new form > > > > > > > > > > > > > > Changes in v2: > > > > > > > Add support for LSM6DSR > > > > > > > Set pm_ops to st_lsm6dsx_pm_ops > > > > > > > > > > > > > > drivers/iio/imu/st_lsm6dsx/Kconfig | 8 +++- > > > > > > > drivers/iio/imu/st_lsm6dsx/Makefile | 1 + > > > > > > > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c | 58 +++++++++++++++++++++++++++++ > > > > > > > 3 files changed, 66 insertions(+), 1 deletion(-) > > > > > > > create mode 100644 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c > > > > > > > > > > > > > > diff --git a/drivers/iio/imu/st_lsm6dsx/Kconfig b/drivers/iio/imu/st_lsm6dsx/Kconfig > > > > > > > index 9e59297..6b5a73c 100644 > > > > > > > --- a/drivers/iio/imu/st_lsm6dsx/Kconfig > > > > > > > +++ b/drivers/iio/imu/st_lsm6dsx/Kconfig > > > > > > > @@ -1,11 +1,12 @@ > > > > > > > > > > > > > > config IIO_ST_LSM6DSX > > > > > > > tristate "ST_LSM6DSx driver for STM 6-axis IMU MEMS sensors" > > > > > > > - depends on (I2C || SPI) > > > > > > > + depends on (I2C || SPI || I3C) > > > > > > > select IIO_BUFFER > > > > > > > select IIO_KFIFO_BUF > > > > > > > select IIO_ST_LSM6DSX_I2C if (I2C) > > > > > > > select IIO_ST_LSM6DSX_SPI if (SPI_MASTER) > > > > > > > + select IIO_ST_LSM6DSX_I3C if (I3C) > > > > > > > help > > > > > > > Say yes here to build support for STMicroelectronics LSM6DSx imu > > > > > > > sensor. Supported devices: lsm6ds3, lsm6ds3h, lsm6dsl, lsm6dsm, > > > > > > > @@ -23,3 +24,8 @@ config IIO_ST_LSM6DSX_SPI > > > > > > > tristate > > > > > > > depends on IIO_ST_LSM6DSX > > > > > > > select REGMAP_SPI > > > > > > > + > > > > > > > +config IIO_ST_LSM6DSX_I3C > > > > > > > + tristate > > > > > > > + depends on IIO_ST_LSM6DSX > > > > > > > + select REGMAP_I3C > > > > > > > diff --git a/drivers/iio/imu/st_lsm6dsx/Makefile b/drivers/iio/imu/st_lsm6dsx/Makefile > > > > > > > index e5f733c..c676965 100644 > > > > > > > --- a/drivers/iio/imu/st_lsm6dsx/Makefile > > > > > > > +++ b/drivers/iio/imu/st_lsm6dsx/Makefile > > > > > > > @@ -4,3 +4,4 @@ st_lsm6dsx-y := st_lsm6dsx_core.o st_lsm6dsx_buffer.o \ > > > > > > > obj-$(CONFIG_IIO_ST_LSM6DSX) += st_lsm6dsx.o > > > > > > > obj-$(CONFIG_IIO_ST_LSM6DSX_I2C) += st_lsm6dsx_i2c.o > > > > > > > obj-$(CONFIG_IIO_ST_LSM6DSX_SPI) += st_lsm6dsx_spi.o > > > > > > > +obj-$(CONFIG_IIO_ST_LSM6DSX_I3C) += st_lsm6dsx_i3c.o > > > > > > > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c > > > > > > > new file mode 100644 > > > > > > > index 0000000..2e89524 > > > > > > > --- /dev/null > > > > > > > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c > > > > > > > @@ -0,0 +1,58 @@ > > > > > > > +// SPDX-License-Identifier: GPL-2.0 > > > > > > > +/* > > > > > > > + * Copyright (c) 2018 Synopsys, Inc. and/or its affiliates. > > > > > > > + * > > > > > > > + * Author: Vitor Soares <vitor.soares@synopsys.com> > > > > > > > + */ > > > > > > > + > > > > > > > +#include <linux/kernel.h> > > > > > > > +#include <linux/module.h> > > > > > > > +#include <linux/i3c/device.h> > > > > > > > +#include <linux/i3c/master.h> > > > > > > > +#include <linux/slab.h> > > > > > > > +#include <linux/of.h> > > > > > > > +#include <linux/regmap.h> > > > > > > > + > > > > > > > +#include "st_lsm6dsx.h" > > > > > > > + > > > > > > > +static const struct i3c_device_id st_lsm6dsx_i3c_ids[] = { > > > > > > > + I3C_DEVICE(0x0104, 0x006C, (void *)ST_LSM6DSO_ID), > > > > > > > + I3C_DEVICE(0x0104, 0x006B, (void *)ST_LSM6DSR_ID), > > > > > > > > > > > > I think you need an uintptr_t cast here: > > > > > > > > > > > > I3C_DEVICE(0x0104, 0x006C, (void *)(uintptr_t)ST_LSM6DSO_ID), > > > > > > I3C_DEVICE(0x0104, 0x006B, (void *)(uintptr_t)ST_LSM6DSR_ID), > > > > > > > > > > > > otherwise gcc might complain that the integer and pointer do not have > > > > > > the same size (on 64-bit architectures). > > > > > > > > > > I don't understand this part. Can you provide or point some background? > > > > > > > > If you don't do that you'll get the following warning: > > > > > > > > warning: cast to 'void *' from smaller integer type 'int' [-Wint-to-void-pointer-cast] > > > > > > I don't get the warning during compilation. Is there any flag to enable > > > or so? > > > > Nope, nothing specific to enable, just enable this driver on an arm64 > > config. Note that that gcc doesn't seem to complain about this > > int -> void * cast (there's probably some kind of auto-promotion to > > pointer size), but it does complains about the following void * -> int > > cast: > > > > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c: In function ‘st_lsm6dsx_i3c_probe’: > > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c:43:43: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] > > 43 | return st_lsm6dsx_probe(&i3cdev->dev, 0, (int)id->data, regmap); > > | > > I fixed the warning by changing: > > st_lsm6dsx_probe(&i3cdev->dev, 0, (int)id->data, regmap); > > to > > st_lsm6dsx_probe(&i3cdev->dev, 0, (int)(uintptr_t)id->data, regmap); The (int) cast is implicit, no need to add it here. > > But I wonder if it isn't more save to change the following too: > I3C_DEVICE(0x0104, 0x006C, (void *)(uintptr_t)ST_LSM6DSO_ID), > I3C_DEVICE(0x0104, 0x006B, (void *)(uintptr_t)ST_LSM6DSR_ID), > > What do you think? I think we're good, we would have a problem if you were defining ST_LSM6DSO_ID as an ULL (unsigned long long, AKA u64) and trying to cast that value to a void pointer on a 32-bit arch.
diff --git a/drivers/iio/imu/st_lsm6dsx/Kconfig b/drivers/iio/imu/st_lsm6dsx/Kconfig index 9e59297..6b5a73c 100644 --- a/drivers/iio/imu/st_lsm6dsx/Kconfig +++ b/drivers/iio/imu/st_lsm6dsx/Kconfig @@ -1,11 +1,12 @@ config IIO_ST_LSM6DSX tristate "ST_LSM6DSx driver for STM 6-axis IMU MEMS sensors" - depends on (I2C || SPI) + depends on (I2C || SPI || I3C) select IIO_BUFFER select IIO_KFIFO_BUF select IIO_ST_LSM6DSX_I2C if (I2C) select IIO_ST_LSM6DSX_SPI if (SPI_MASTER) + select IIO_ST_LSM6DSX_I3C if (I3C) help Say yes here to build support for STMicroelectronics LSM6DSx imu sensor. Supported devices: lsm6ds3, lsm6ds3h, lsm6dsl, lsm6dsm, @@ -23,3 +24,8 @@ config IIO_ST_LSM6DSX_SPI tristate depends on IIO_ST_LSM6DSX select REGMAP_SPI + +config IIO_ST_LSM6DSX_I3C + tristate + depends on IIO_ST_LSM6DSX + select REGMAP_I3C diff --git a/drivers/iio/imu/st_lsm6dsx/Makefile b/drivers/iio/imu/st_lsm6dsx/Makefile index e5f733c..c676965 100644 --- a/drivers/iio/imu/st_lsm6dsx/Makefile +++ b/drivers/iio/imu/st_lsm6dsx/Makefile @@ -4,3 +4,4 @@ st_lsm6dsx-y := st_lsm6dsx_core.o st_lsm6dsx_buffer.o \ obj-$(CONFIG_IIO_ST_LSM6DSX) += st_lsm6dsx.o obj-$(CONFIG_IIO_ST_LSM6DSX_I2C) += st_lsm6dsx_i2c.o obj-$(CONFIG_IIO_ST_LSM6DSX_SPI) += st_lsm6dsx_spi.o +obj-$(CONFIG_IIO_ST_LSM6DSX_I3C) += st_lsm6dsx_i3c.o diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c new file mode 100644 index 0000000..2e89524 --- /dev/null +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c @@ -0,0 +1,58 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2018 Synopsys, Inc. and/or its affiliates. + * + * Author: Vitor Soares <vitor.soares@synopsys.com> + */ + +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/i3c/device.h> +#include <linux/i3c/master.h> +#include <linux/slab.h> +#include <linux/of.h> +#include <linux/regmap.h> + +#include "st_lsm6dsx.h" + +static const struct i3c_device_id st_lsm6dsx_i3c_ids[] = { + I3C_DEVICE(0x0104, 0x006C, (void *)ST_LSM6DSO_ID), + I3C_DEVICE(0x0104, 0x006B, (void *)ST_LSM6DSR_ID), + { /* sentinel */ }, +}; +MODULE_DEVICE_TABLE(i3c, st_lsm6dsx_i3c_ids); + +static const struct regmap_config st_lsm6dsx_i3c_regmap_config = { + .reg_bits = 8, + .val_bits = 8, +}; + +static int st_lsm6dsx_i3c_probe(struct i3c_device *i3cdev) +{ + const struct i3c_device_id *id = i3c_device_match_id(i3cdev, + st_lsm6dsx_i3c_ids); + struct regmap *regmap; + + regmap = devm_regmap_init_i3c(i3cdev, &st_lsm6dsx_i3c_regmap_config); + if (IS_ERR(regmap)) { + dev_err(&i3cdev->dev, "Failed to register i3c regmap %d\n", + (int)PTR_ERR(regmap)); + return PTR_ERR(regmap); + } + + return st_lsm6dsx_probe(&i3cdev->dev, 0, (int)id->data, regmap); +} + +static struct i3c_driver st_lsm6dsx_driver = { + .driver = { + .name = "st_lsm6dsx_i3c", + .pm = &st_lsm6dsx_pm_ops, + }, + .probe = st_lsm6dsx_i3c_probe, + .id_table = st_lsm6dsx_i3c_ids, +}; +module_i3c_driver(st_lsm6dsx_driver); + +MODULE_AUTHOR("Vitor Soares <vitor.soares@synopsys.com>"); +MODULE_DESCRIPTION("STMicroelectronics st_lsm6dsx i3c driver"); +MODULE_LICENSE("GPL v2");