Vitor Soares (3):
remap: Add i3c bus support
i3c: Add i3c_get_device_id helper
iio: imu: st_lsm6dsx: Add i3c basic support
drivers/base/regmap/Kconfig | 6 ++-
drivers/base/regmap/Makefile | 1 +
drivers/base/regmap/regmap-i3c.c | 62 ++++++++++++++++++++++++++
drivers/i3c/device.c | 8 ++++
drivers/iio/imu/st_lsm6dsx/Kconfig | 8 +++-
drivers/iio/imu/st_lsm6dsx/Makefile | 1 +
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c | 67 +++++++++++++++++++++++++++++
include/linux/i3c/device.h | 1 +
include/linux/regmap.h | 20 +++++++++
9 files changed, 172 insertions(+), 2 deletions(-)
create mode 100644 drivers/base/regmap/regmap-i3c.c
create mode 100644 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
--
2.7.4
Add basic support for I3C bus.
This is a simple implementation that only give support
for Read and Write commands.
Signed-off-by: Vitor Soares <[email protected]>
---
drivers/base/regmap/Kconfig | 6 +++-
drivers/base/regmap/Makefile | 1 +
drivers/base/regmap/regmap-i3c.c | 62 ++++++++++++++++++++++++++++++++++++++++
include/linux/regmap.h | 20 +++++++++++++
4 files changed, 88 insertions(+), 1 deletion(-)
create mode 100644 drivers/base/regmap/regmap-i3c.c
diff --git a/drivers/base/regmap/Kconfig b/drivers/base/regmap/Kconfig
index 6ad5ef4..c8bbf53 100644
--- a/drivers/base/regmap/Kconfig
+++ b/drivers/base/regmap/Kconfig
@@ -4,7 +4,7 @@
# subsystems should select the appropriate symbols.
config REGMAP
- default y if (REGMAP_I2C || REGMAP_SPI || REGMAP_SPMI || REGMAP_W1 || REGMAP_AC97 || REGMAP_MMIO || REGMAP_IRQ)
+ default y if (REGMAP_I2C || REGMAP_SPI || REGMAP_SPMI || REGMAP_W1 || REGMAP_AC97 || REGMAP_MMIO || REGMAP_IRQ || REGMAP_I3C)
select IRQ_DOMAIN if REGMAP_IRQ
bool
@@ -49,3 +49,7 @@ config REGMAP_SOUNDWIRE
config REGMAP_SCCB
tristate
depends on I2C
+
+config REGMAP_I3C
+ tristate
+ depends on I3C
diff --git a/drivers/base/regmap/Makefile b/drivers/base/regmap/Makefile
index f5b4e88..ff6c7d8 100644
--- a/drivers/base/regmap/Makefile
+++ b/drivers/base/regmap/Makefile
@@ -16,3 +16,4 @@ obj-$(CONFIG_REGMAP_IRQ) += regmap-irq.o
obj-$(CONFIG_REGMAP_W1) += regmap-w1.o
obj-$(CONFIG_REGMAP_SOUNDWIRE) += regmap-sdw.o
obj-$(CONFIG_REGMAP_SCCB) += regmap-sccb.o
+obj-$(CONFIG_REGMAP_I3C) += regmap-i3c.o
diff --git a/drivers/base/regmap/regmap-i3c.c b/drivers/base/regmap/regmap-i3c.c
new file mode 100644
index 0000000..565997b
--- /dev/null
+++ b/drivers/base/regmap/regmap-i3c.c
@@ -0,0 +1,62 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018 Synopsys, Inc. and/or its affiliates.
+ *
+ * Author: Vitor Soares <[email protected]>
+ */
+
+#include <linux/regmap.h>
+#include <linux/i3c/device.h>
+#include <linux/i3c/master.h>
+#include <linux/module.h>
+
+static int regmap_i3c_write(void *context, const void *data, size_t count)
+{
+ struct device *dev = context;
+ struct i3c_device *i3c = dev_to_i3cdev(dev);
+ struct i3c_priv_xfer xfers[] = {
+ {
+ .rnw = false,
+ .len = count,
+ .data.out = data,
+ },
+ };
+
+ return i3c_device_do_priv_xfers(i3c, xfers, 1);
+}
+
+static int regmap_i3c_read(void *context,
+ const void *reg, size_t reg_size,
+ void *val, size_t val_size)
+{
+ struct device *dev = context;
+ struct i3c_device *i3c = dev_to_i3cdev(dev);
+ struct i3c_priv_xfer xfers[2];
+
+ xfers[0].rnw = false;
+ xfers[0].len = reg_size;
+ xfers[0].data.out = reg;
+
+ xfers[1].rnw = true;
+ xfers[1].len = val_size;
+ xfers[1].data.in = val;
+
+ return i3c_device_do_priv_xfers(i3c, xfers, 2);
+}
+
+static struct regmap_bus regmap_i3c = {
+ .write = regmap_i3c_write,
+ .read = regmap_i3c_read,
+};
+
+struct regmap *__devm_regmap_init_i3c(struct i3c_device *i3c,
+ const struct regmap_config *config,
+ struct lock_class_key *lock_key,
+ const char *lock_name)
+{
+ return __devm_regmap_init(&i3c->dev, ®map_i3c, &i3c->dev, config,
+ lock_key, lock_name);
+}
+EXPORT_SYMBOL_GPL(__devm_regmap_init_i3c);
+
+MODULE_LICENSE("GPL");
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index daeec7d..f65984d 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -25,6 +25,7 @@ struct module;
struct clk;
struct device;
struct i2c_client;
+struct i3c_device;
struct irq_domain;
struct slim_device;
struct spi_device;
@@ -624,6 +625,10 @@ struct regmap *__devm_regmap_init_slimbus(struct slim_device *slimbus,
const struct regmap_config *config,
struct lock_class_key *lock_key,
const char *lock_name);
+struct regmap *__devm_regmap_init_i3c(struct i3c_device *i3c,
+ const struct regmap_config *config,
+ struct lock_class_key *lock_key,
+ const char *lock_name);
/*
* Wrapper for regmap_init macros to include a unique lockdep key and name
* for each call. No-op if CONFIG_LOCKDEP is not set.
@@ -982,6 +987,21 @@ bool regmap_ac97_default_volatile(struct device *dev, unsigned int reg);
#define devm_regmap_init_slimbus(slimbus, config) \
__regmap_lockdep_wrapper(__devm_regmap_init_slimbus, #config, \
slimbus, config)
+
+/**
+ * devm_regmap_init_i3c() - Initialise managed register map
+ *
+ * @i3c: Device that will be interacted with
+ * @config: Configuration for register map
+ *
+ * The return value will be an ERR_PTR() on error or a valid pointer
+ * to a struct regmap. The regmap will be automatically freed by the
+ * device management code.
+ */
+#define devm_regmap_init_i3c(i3c, config) \
+ __regmap_lockdep_wrapper(__devm_regmap_init_i3c, #config, \
+ i3c, config)
+
int regmap_mmio_attach_clk(struct regmap *map, struct clk *clk);
void regmap_mmio_detach_clk(struct regmap *map);
void regmap_exit(struct regmap *map);
--
2.7.4
This helper return the i3c_device_id structure in order the client
have access to the driver data.
Signed-off-by: Vitor Soares <[email protected]>
---
drivers/i3c/device.c | 8 ++++++++
include/linux/i3c/device.h | 1 +
2 files changed, 9 insertions(+)
diff --git a/drivers/i3c/device.c b/drivers/i3c/device.c
index 472be99..8ab8e4c 100644
--- a/drivers/i3c/device.c
+++ b/drivers/i3c/device.c
@@ -235,6 +235,14 @@ struct i3c_device *dev_to_i3cdev(struct device *dev)
}
EXPORT_SYMBOL_GPL(dev_to_i3cdev);
+const struct i3c_device_id *i3c_get_device_id(struct i3c_device *i3cdev)
+{
+ const struct i3c_driver *i3cdrv = drv_to_i3cdrv(i3cdev->dev.driver);
+
+ return i3cdrv->id_table;
+}
+EXPORT_SYMBOL_GPL(i3c_get_device_id);
+
/**
* i3c_driver_register_with_owner() - register an I3C device driver
*
diff --git a/include/linux/i3c/device.h b/include/linux/i3c/device.h
index 7ee7e30..ee48886 100644
--- a/include/linux/i3c/device.h
+++ b/include/linux/i3c/device.h
@@ -211,6 +211,7 @@ static inline struct i3c_driver *drv_to_i3cdrv(struct device_driver *drv)
struct device *i3cdev_to_dev(struct i3c_device *i3cdev);
struct i3c_device *dev_to_i3cdev(struct device *dev);
+const struct i3c_device_id *i3c_get_device_id(struct i3c_device *i3cdev);
static inline void i3cdev_set_drvdata(struct i3c_device *i3cdev,
void *data)
--
2.7.4
For today the st_lsm6dsx driver support lsm6dso sensor only in
spi and i2c mode.
The lsm6dso is also i3c capable so lets give i3c support to it.
Signed-off-by: Vitor Soares <[email protected]>
---
drivers/iio/imu/st_lsm6dsx/Kconfig | 8 +++-
drivers/iio/imu/st_lsm6dsx/Makefile | 1 +
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c | 67 +++++++++++++++++++++++++++++
3 files changed, 75 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 094fd00..1ab9bbf 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..2df5e70
--- /dev/null
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
@@ -0,0 +1,67 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018 Synopsys, Inc. and/or its affiliates.
+ *
+ * Author: Vitor Soares <[email protected]>
+ */
+
+#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"
+
+#define NAME_SIZE 32
+
+struct st_lsm6dsx_i3c_data {
+ char name[NAME_SIZE];
+ enum st_lsm6dsx_hw_id id;
+};
+
+static const struct st_lsm6dsx_i3c_data hw_data[] = {
+ { ST_LSM6DSO_DEV_NAME, ST_LSM6DSO_ID },
+};
+
+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_get_device_id(i3cdev);
+ const struct st_lsm6dsx_i3c_data *hw_data = id->data;
+ 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, hw_data->id,
+ hw_data->name, regmap);
+}
+
+
+static const struct i3c_device_id st_lsm6dsx_i3c_ids[] = {
+ I3C_DEVICE(0x0104, 0x006C, &hw_data[0]),
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(i3c, st_lsm6dsx_i3c_ids);
+
+static struct i3c_driver st_lsm6dsx_driver = {
+ .driver.name = "st_lsm6dsx_i3c",
+ .probe = st_lsm6dsx_i3c_probe,
+ .id_table = st_lsm6dsx_i3c_ids,
+};
+module_i3c_driver(st_lsm6dsx_driver);
+
+MODULE_AUTHOR("Vitor Soares <[email protected]>");
+MODULE_DESCRIPTION("STMicroelectronics st_lsm6dsx i3c driver");
+MODULE_LICENSE("GPL v2");
+
--
2.7.4
Typo in the subject: s/remap/regmap/
On Mon, 15 Apr 2019 21:19:39 +0200
Vitor Soares <[email protected]> wrote:
> Add basic support for I3C bus.
> This is a simple implementation that only give support
> for Read and Write commands.
>
> Signed-off-by: Vitor Soares <[email protected]>
> ---
> drivers/base/regmap/Kconfig | 6 +++-
> drivers/base/regmap/Makefile | 1 +
> drivers/base/regmap/regmap-i3c.c | 62 ++++++++++++++++++++++++++++++++++++++++
> include/linux/regmap.h | 20 +++++++++++++
> 4 files changed, 88 insertions(+), 1 deletion(-)
> create mode 100644 drivers/base/regmap/regmap-i3c.c
>
> diff --git a/drivers/base/regmap/Kconfig b/drivers/base/regmap/Kconfig
> index 6ad5ef4..c8bbf53 100644
> --- a/drivers/base/regmap/Kconfig
> +++ b/drivers/base/regmap/Kconfig
> @@ -4,7 +4,7 @@
> # subsystems should select the appropriate symbols.
>
> config REGMAP
> - default y if (REGMAP_I2C || REGMAP_SPI || REGMAP_SPMI || REGMAP_W1 || REGMAP_AC97 || REGMAP_MMIO || REGMAP_IRQ)
> + default y if (REGMAP_I2C || REGMAP_SPI || REGMAP_SPMI || REGMAP_W1 || REGMAP_AC97 || REGMAP_MMIO || REGMAP_IRQ || REGMAP_I3C)
> select IRQ_DOMAIN if REGMAP_IRQ
> bool
>
> @@ -49,3 +49,7 @@ config REGMAP_SOUNDWIRE
> config REGMAP_SCCB
> tristate
> depends on I2C
> +
> +config REGMAP_I3C
> + tristate
> + depends on I3C
> diff --git a/drivers/base/regmap/Makefile b/drivers/base/regmap/Makefile
> index f5b4e88..ff6c7d8 100644
> --- a/drivers/base/regmap/Makefile
> +++ b/drivers/base/regmap/Makefile
> @@ -16,3 +16,4 @@ obj-$(CONFIG_REGMAP_IRQ) += regmap-irq.o
> obj-$(CONFIG_REGMAP_W1) += regmap-w1.o
> obj-$(CONFIG_REGMAP_SOUNDWIRE) += regmap-sdw.o
> obj-$(CONFIG_REGMAP_SCCB) += regmap-sccb.o
> +obj-$(CONFIG_REGMAP_I3C) += regmap-i3c.o
> diff --git a/drivers/base/regmap/regmap-i3c.c b/drivers/base/regmap/regmap-i3c.c
> new file mode 100644
> index 0000000..565997b
> --- /dev/null
> +++ b/drivers/base/regmap/regmap-i3c.c
> @@ -0,0 +1,62 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2018 Synopsys, Inc. and/or its affiliates.
> + *
> + * Author: Vitor Soares <[email protected]>
> + */
> +
> +#include <linux/regmap.h>
> +#include <linux/i3c/device.h>
> +#include <linux/i3c/master.h>
> +#include <linux/module.h>
> +
> +static int regmap_i3c_write(void *context, const void *data, size_t count)
> +{
> + struct device *dev = context;
> + struct i3c_device *i3c = dev_to_i3cdev(dev);
> + struct i3c_priv_xfer xfers[] = {
> + {
> + .rnw = false,
> + .len = count,
> + .data.out = data,
> + },
> + };
> +
> + return i3c_device_do_priv_xfers(i3c, xfers, 1);
> +}
> +
> +static int regmap_i3c_read(void *context,
> + const void *reg, size_t reg_size,
> + void *val, size_t val_size)
> +{
> + struct device *dev = context;
> + struct i3c_device *i3c = dev_to_i3cdev(dev);
> + struct i3c_priv_xfer xfers[2];
> +
> + xfers[0].rnw = false;
> + xfers[0].len = reg_size;
> + xfers[0].data.out = reg;
> +
> + xfers[1].rnw = true;
> + xfers[1].len = val_size;
> + xfers[1].data.in = val;
> +
> + return i3c_device_do_priv_xfers(i3c, xfers, 2);
> +}
> +
> +static struct regmap_bus regmap_i3c = {
> + .write = regmap_i3c_write,
> + .read = regmap_i3c_read,
> +};
> +
> +struct regmap *__devm_regmap_init_i3c(struct i3c_device *i3c,
> + const struct regmap_config *config,
> + struct lock_class_key *lock_key,
> + const char *lock_name)
> +{
> + return __devm_regmap_init(&i3c->dev, ®map_i3c, &i3c->dev, config,
> + lock_key, lock_name);
> +}
> +EXPORT_SYMBOL_GPL(__devm_regmap_init_i3c);
> +
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/regmap.h b/include/linux/regmap.h
> index daeec7d..f65984d 100644
> --- a/include/linux/regmap.h
> +++ b/include/linux/regmap.h
> @@ -25,6 +25,7 @@ struct module;
> struct clk;
> struct device;
> struct i2c_client;
> +struct i3c_device;
> struct irq_domain;
> struct slim_device;
> struct spi_device;
> @@ -624,6 +625,10 @@ struct regmap *__devm_regmap_init_slimbus(struct slim_device *slimbus,
> const struct regmap_config *config,
> struct lock_class_key *lock_key,
> const char *lock_name);
> +struct regmap *__devm_regmap_init_i3c(struct i3c_device *i3c,
> + const struct regmap_config *config,
> + struct lock_class_key *lock_key,
> + const char *lock_name);
> /*
> * Wrapper for regmap_init macros to include a unique lockdep key and name
> * for each call. No-op if CONFIG_LOCKDEP is not set.
> @@ -982,6 +987,21 @@ bool regmap_ac97_default_volatile(struct device *dev, unsigned int reg);
> #define devm_regmap_init_slimbus(slimbus, config) \
> __regmap_lockdep_wrapper(__devm_regmap_init_slimbus, #config, \
> slimbus, config)
> +
> +/**
> + * devm_regmap_init_i3c() - Initialise managed register map
Maybe we should have a more specific name here, as some might want to
access registers using HDR modes (devm_regmap_init_i3c_sdr()?)
> + *
> + * @i3c: Device that will be interacted with
> + * @config: Configuration for register map
> + *
> + * The return value will be an ERR_PTR() on error or a valid pointer
> + * to a struct regmap. The regmap will be automatically freed by the
> + * device management code.
> + */
> +#define devm_regmap_init_i3c(i3c, config) \
> + __regmap_lockdep_wrapper(__devm_regmap_init_i3c, #config, \
> + i3c, config)
> +
> int regmap_mmio_attach_clk(struct regmap *map, struct clk *clk);
> void regmap_mmio_detach_clk(struct regmap *map);
> void regmap_exit(struct regmap *map);
On Mon, 15 Apr 2019 21:19:40 +0200
Vitor Soares <[email protected]> wrote:
> This helper return the i3c_device_id structure in order the client
> have access to the driver data.
>
> Signed-off-by: Vitor Soares <[email protected]>
> ---
> drivers/i3c/device.c | 8 ++++++++
> include/linux/i3c/device.h | 1 +
> 2 files changed, 9 insertions(+)
>
> diff --git a/drivers/i3c/device.c b/drivers/i3c/device.c
> index 472be99..8ab8e4c 100644
> --- a/drivers/i3c/device.c
> +++ b/drivers/i3c/device.c
> @@ -235,6 +235,14 @@ struct i3c_device *dev_to_i3cdev(struct device *dev)
> }
> EXPORT_SYMBOL_GPL(dev_to_i3cdev);
>
> +const struct i3c_device_id *i3c_get_device_id(struct i3c_device *i3cdev)
> +{
> + const struct i3c_driver *i3cdrv = drv_to_i3cdrv(i3cdev->dev.driver);
> +
> + return i3cdrv->id_table;
> +}
> +EXPORT_SYMBOL_GPL(i3c_get_device_id);
I think what you want is i3c_device_match_id(). Just move the function
to drivers/i3c/device.c, export it and define its prototype in
include/i3c/device.h.
> +
> /**
> * i3c_driver_register_with_owner() - register an I3C device driver
> *
> diff --git a/include/linux/i3c/device.h b/include/linux/i3c/device.h
> index 7ee7e30..ee48886 100644
> --- a/include/linux/i3c/device.h
> +++ b/include/linux/i3c/device.h
> @@ -211,6 +211,7 @@ static inline struct i3c_driver *drv_to_i3cdrv(struct device_driver *drv)
>
> struct device *i3cdev_to_dev(struct i3c_device *i3cdev);
> struct i3c_device *dev_to_i3cdev(struct device *dev);
> +const struct i3c_device_id *i3c_get_device_id(struct i3c_device *i3cdev);
>
> static inline void i3cdev_set_drvdata(struct i3c_device *i3cdev,
> void *data)
On Mon, 15 Apr 2019 21:19:41 +0200
Vitor Soares <[email protected]> wrote:
> For today the st_lsm6dsx driver support lsm6dso sensor only in
> spi and i2c mode.
>
> The lsm6dso is also i3c capable so lets give i3c support to it.
>
> Signed-off-by: Vitor Soares <[email protected]>
> ---
> drivers/iio/imu/st_lsm6dsx/Kconfig | 8 +++-
> drivers/iio/imu/st_lsm6dsx/Makefile | 1 +
> drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c | 67 +++++++++++++++++++++++++++++
> 3 files changed, 75 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 094fd00..1ab9bbf 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..2df5e70
> --- /dev/null
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
> @@ -0,0 +1,67 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2018 Synopsys, Inc. and/or its affiliates.
> + *
> + * Author: Vitor Soares <[email protected]>
> + */
> +
> +#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"
> +
> +#define NAME_SIZE 32
> +
> +struct st_lsm6dsx_i3c_data {
> + char name[NAME_SIZE];
const char *name;
> + enum st_lsm6dsx_hw_id id;
> +};
> +
> +static const struct st_lsm6dsx_i3c_data hw_data[] = {
No need to make it an array, and it should probably be named
st_lsm6dso_data not hw_data.
> + { ST_LSM6DSO_DEV_NAME, ST_LSM6DSO_ID },
> +};
> +
> +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_get_device_id(i3cdev);
i3c_device_match_id(i3cdev,
st_lsm6dsx_i3c_ids);
> + const struct st_lsm6dsx_i3c_data *hw_data = id->data;
> + 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, hw_data->id,
> + hw_data->name, regmap);
> +}
> +
> +
> +static const struct i3c_device_id st_lsm6dsx_i3c_ids[] = {
> + I3C_DEVICE(0x0104, 0x006C, &hw_data[0]),
> + { /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(i3c, st_lsm6dsx_i3c_ids);
> +
> +static struct i3c_driver st_lsm6dsx_driver = {
> + .driver.name = "st_lsm6dsx_i3c",
> + .probe = st_lsm6dsx_i3c_probe,
> + .id_table = st_lsm6dsx_i3c_ids,
You should probably set the pm_ops here (st_lsm6dsx_pm_ops).
> +};
> +module_i3c_driver(st_lsm6dsx_driver);
> +
> +MODULE_AUTHOR("Vitor Soares <[email protected]>");
> +MODULE_DESCRIPTION("STMicroelectronics st_lsm6dsx i3c driver");
> +MODULE_LICENSE("GPL v2");
> +
You can remove this blank line.
Hi Vitor,
On Mon, 15 Apr 2019 21:19:38 +0200
Vitor Soares <[email protected]> wrote:
I like the very detailed description in your cover letter :P.
Anyway, glad to see I3C support added to regmap. Also happy to have a
new driver for an I3C device.
Thanks for working on that.
Boris
> Vitor Soares (3):
> remap: Add i3c bus support
> i3c: Add i3c_get_device_id helper
> iio: imu: st_lsm6dsx: Add i3c basic support
>
> drivers/base/regmap/Kconfig | 6 ++-
> drivers/base/regmap/Makefile | 1 +
> drivers/base/regmap/regmap-i3c.c | 62 ++++++++++++++++++++++++++
> drivers/i3c/device.c | 8 ++++
> drivers/iio/imu/st_lsm6dsx/Kconfig | 8 +++-
> drivers/iio/imu/st_lsm6dsx/Makefile | 1 +
> drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c | 67 +++++++++++++++++++++++++++++
> include/linux/i3c/device.h | 1 +
> include/linux/regmap.h | 20 +++++++++
> 9 files changed, 172 insertions(+), 2 deletions(-)
> create mode 100644 drivers/base/regmap/regmap-i3c.c
> create mode 100644 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
>
>
> On Mon, 15 Apr 2019 21:19:41 +0200
> Vitor Soares <[email protected]> wrote:
>
> > For today the st_lsm6dsx driver support lsm6dso sensor only in
> > spi and i2c mode.
> >
> > The lsm6dso is also i3c capable so lets give i3c support to it.
> >
> > Signed-off-by: Vitor Soares <[email protected]>
> > ---
> > drivers/iio/imu/st_lsm6dsx/Kconfig | 8 +++-
> > drivers/iio/imu/st_lsm6dsx/Makefile | 1 +
> > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c | 67 +++++++++++++++++++++++++++++
> > 3 files changed, 75 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 094fd00..1ab9bbf 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..2df5e70
> > --- /dev/null
> > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
> > @@ -0,0 +1,67 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2018 Synopsys, Inc. and/or its affiliates.
> > + *
> > + * Author: Vitor Soares <[email protected]>
> > + */
> > +
> > +#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"
> > +
> > +#define NAME_SIZE 32
> > +
> > +struct st_lsm6dsx_i3c_data {
> > + char name[NAME_SIZE];
>
> const char *name;
>
> > + enum st_lsm6dsx_hw_id id;
> > +};
> > +
> > +static const struct st_lsm6dsx_i3c_data hw_data[] = {
>
> No need to make it an array, and it should probably be named
> st_lsm6dso_data not hw_data.
I guess it will be used even for future devices so I would prefer to
make it an array with a 'general' name
Regards,
Lorenzo
>
> > + { ST_LSM6DSO_DEV_NAME, ST_LSM6DSO_ID },
> > +};
> > +
> > +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_get_device_id(i3cdev);
>
> i3c_device_match_id(i3cdev,
> st_lsm6dsx_i3c_ids);
>
> > + const struct st_lsm6dsx_i3c_data *hw_data = id->data;
> > + 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, hw_data->id,
> > + hw_data->name, regmap);
> > +}
> > +
> > +
> > +static const struct i3c_device_id st_lsm6dsx_i3c_ids[] = {
> > + I3C_DEVICE(0x0104, 0x006C, &hw_data[0]),
> > + { /* sentinel */ },
> > +};
> > +MODULE_DEVICE_TABLE(i3c, st_lsm6dsx_i3c_ids);
> > +
> > +static struct i3c_driver st_lsm6dsx_driver = {
> > + .driver.name = "st_lsm6dsx_i3c",
> > + .probe = st_lsm6dsx_i3c_probe,
> > + .id_table = st_lsm6dsx_i3c_ids,
>
> You should probably set the pm_ops here (st_lsm6dsx_pm_ops).
>
> > +};
> > +module_i3c_driver(st_lsm6dsx_driver);
> > +
> > +MODULE_AUTHOR("Vitor Soares <[email protected]>");
> > +MODULE_DESCRIPTION("STMicroelectronics st_lsm6dsx i3c driver");
> > +MODULE_LICENSE("GPL v2");
> > +
>
> You can remove this blank line.
>
--
UNIX is Sexy: who | grep -i blonde | talk; cd ~; wine; talk; touch;
unzip; touch; strip; gasp; finger; gasp; mount; fsck; more; yes; gasp;
umount; make clean; sleep
On Tue, 16 Apr 2019 09:58:33 +0200
Lorenzo Bianconi <[email protected]> wrote:
> >
> > On Mon, 15 Apr 2019 21:19:41 +0200
> > Vitor Soares <[email protected]> wrote:
> >
> > > For today the st_lsm6dsx driver support lsm6dso sensor only in
> > > spi and i2c mode.
> > >
> > > The lsm6dso is also i3c capable so lets give i3c support to it.
> > >
> > > Signed-off-by: Vitor Soares <[email protected]>
> > > ---
> > > drivers/iio/imu/st_lsm6dsx/Kconfig | 8 +++-
> > > drivers/iio/imu/st_lsm6dsx/Makefile | 1 +
> > > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c | 67 +++++++++++++++++++++++++++++
> > > 3 files changed, 75 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 094fd00..1ab9bbf 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..2df5e70
> > > --- /dev/null
> > > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
> > > @@ -0,0 +1,67 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Copyright (c) 2018 Synopsys, Inc. and/or its affiliates.
> > > + *
> > > + * Author: Vitor Soares <[email protected]>
> > > + */
> > > +
> > > +#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"
> > > +
> > > +#define NAME_SIZE 32
> > > +
> > > +struct st_lsm6dsx_i3c_data {
> > > + char name[NAME_SIZE];
> >
> > const char *name;
> >
> > > + enum st_lsm6dsx_hw_id id;
> > > +};
> > > +
> > > +static const struct st_lsm6dsx_i3c_data hw_data[] = {
> >
> > No need to make it an array, and it should probably be named
> > st_lsm6dso_data not hw_data.
>
> I guess it will be used even for future devices so I would prefer to
> make it an array with a 'general' name
I find it more error-prone to reference an array with an opaque index
number than reference an instance with a name that clearly reflects
what HW it describes (see the I3C_DEVICE() entry at the end of the
driver), but okay.
>
> Regards,
> Lorenzo
>
> >
> > > + { ST_LSM6DSO_DEV_NAME, ST_LSM6DSO_ID },
> > > +};
> > > +
> > > +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_get_device_id(i3cdev);
> >
> > i3c_device_match_id(i3cdev,
> > st_lsm6dsx_i3c_ids);
> >
> > > + const struct st_lsm6dsx_i3c_data *hw_data = id->data;
> > > + 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, hw_data->id,
> > > + hw_data->name, regmap);
> > > +}
> > > +
> > > +
> > > +static const struct i3c_device_id st_lsm6dsx_i3c_ids[] = {
> > > + I3C_DEVICE(0x0104, 0x006C, &hw_data[0]),
> > > + { /* sentinel */ },
> > > +};
> > > +MODULE_DEVICE_TABLE(i3c, st_lsm6dsx_i3c_ids);
> > > +
> > > +static struct i3c_driver st_lsm6dsx_driver = {
> > > + .driver.name = "st_lsm6dsx_i3c",
> > > + .probe = st_lsm6dsx_i3c_probe,
> > > + .id_table = st_lsm6dsx_i3c_ids,
> >
> > You should probably set the pm_ops here (st_lsm6dsx_pm_ops).
> >
> > > +};
> > > +module_i3c_driver(st_lsm6dsx_driver);
> > > +
> > > +MODULE_AUTHOR("Vitor Soares <[email protected]>");
> > > +MODULE_DESCRIPTION("STMicroelectronics st_lsm6dsx i3c driver");
> > > +MODULE_LICENSE("GPL v2");
> > > +
> >
> > You can remove this blank line.
> >
>
>
Hi Boris,
From: Boris Brezillon <[email protected]>
Date: Tue, Apr 16, 2019 at 07:15:51
> Typo in the subject: s/remap/regmap/
>
> On Mon, 15 Apr 2019 21:19:39 +0200
> Vitor Soares <[email protected]> wrote:
>
> > Add basic support for I3C bus.
> > This is a simple implementation that only give support
> > for Read and Write commands.
> >
> > Signed-off-by: Vitor Soares <[email protected]>
> > ---
> > drivers/base/regmap/Kconfig | 6 +++-
> > drivers/base/regmap/Makefile | 1 +
> > drivers/base/regmap/regmap-i3c.c | 62 ++++++++++++++++++++++++++++++++++++++++
> > include/linux/regmap.h | 20 +++++++++++++
> > 4 files changed, 88 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/base/regmap/regmap-i3c.c
> >
> > diff --git a/drivers/base/regmap/Kconfig b/drivers/base/regmap/Kconfig
> > index 6ad5ef4..c8bbf53 100644
> > --- a/drivers/base/regmap/Kconfig
> > +++ b/drivers/base/regmap/Kconfig
> > @@ -4,7 +4,7 @@
> > # subsystems should select the appropriate symbols.
> >
> > config REGMAP
> > - default y if (REGMAP_I2C || REGMAP_SPI || REGMAP_SPMI || REGMAP_W1 || REGMAP_AC97 || REGMAP_MMIO || REGMAP_IRQ)
> > + default y if (REGMAP_I2C || REGMAP_SPI || REGMAP_SPMI || REGMAP_W1 || REGMAP_AC97 || REGMAP_MMIO || REGMAP_IRQ || REGMAP_I3C)
> > select IRQ_DOMAIN if REGMAP_IRQ
> > bool
> >
> > @@ -49,3 +49,7 @@ config REGMAP_SOUNDWIRE
> > config REGMAP_SCCB
> > tristate
> > depends on I2C
> > +
> > +config REGMAP_I3C
> > + tristate
> > + depends on I3C
> > diff --git a/drivers/base/regmap/Makefile b/drivers/base/regmap/Makefile
> > index f5b4e88..ff6c7d8 100644
> > --- a/drivers/base/regmap/Makefile
> > +++ b/drivers/base/regmap/Makefile
> > @@ -16,3 +16,4 @@ obj-$(CONFIG_REGMAP_IRQ) += regmap-irq.o
> > obj-$(CONFIG_REGMAP_W1) += regmap-w1.o
> > obj-$(CONFIG_REGMAP_SOUNDWIRE) += regmap-sdw.o
> > obj-$(CONFIG_REGMAP_SCCB) += regmap-sccb.o
> > +obj-$(CONFIG_REGMAP_I3C) += regmap-i3c.o
> > diff --git a/drivers/base/regmap/regmap-i3c.c b/drivers/base/regmap/regmap-i3c.c
> > new file mode 100644
> > index 0000000..565997b
> > --- /dev/null
> > +++ b/drivers/base/regmap/regmap-i3c.c
> > @@ -0,0 +1,62 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2018 Synopsys, Inc. and/or its affiliates.
> > + *
> > + * Author: Vitor Soares <[email protected]>
> > + */
> > +
> > +#include <linux/regmap.h>
> > +#include <linux/i3c/device.h>
> > +#include <linux/i3c/master.h>
> > +#include <linux/module.h>
> > +
> > +static int regmap_i3c_write(void *context, const void *data, size_t count)
> > +{
> > + struct device *dev = context;
> > + struct i3c_device *i3c = dev_to_i3cdev(dev);
> > + struct i3c_priv_xfer xfers[] = {
> > + {
> > + .rnw = false,
> > + .len = count,
> > + .data.out = data,
> > + },
> > + };
> > +
> > + return i3c_device_do_priv_xfers(i3c, xfers, 1);
> > +}
> > +
> > +static int regmap_i3c_read(void *context,
> > + const void *reg, size_t reg_size,
> > + void *val, size_t val_size)
> > +{
> > + struct device *dev = context;
> > + struct i3c_device *i3c = dev_to_i3cdev(dev);
> > + struct i3c_priv_xfer xfers[2];
> > +
> > + xfers[0].rnw = false;
> > + xfers[0].len = reg_size;
> > + xfers[0].data.out = reg;
> > +
> > + xfers[1].rnw = true;
> > + xfers[1].len = val_size;
> > + xfers[1].data.in = val;
> > +
> > + return i3c_device_do_priv_xfers(i3c, xfers, 2);
> > +}
> > +
> > +static struct regmap_bus regmap_i3c = {
> > + .write = regmap_i3c_write,
> > + .read = regmap_i3c_read,
> > +};
> > +
> > +struct regmap *__devm_regmap_init_i3c(struct i3c_device *i3c,
> > + const struct regmap_config *config,
> > + struct lock_class_key *lock_key,
> > + const char *lock_name)
> > +{
> > + return __devm_regmap_init(&i3c->dev, ®map_i3c, &i3c->dev, config,
> > + lock_key, lock_name);
> > +}
> > +EXPORT_SYMBOL_GPL(__devm_regmap_init_i3c);
> > +
> > +MODULE_LICENSE("GPL");
> > diff --git a/include/linux/regmap.h b/include/linux/regmap.h
> > index daeec7d..f65984d 100644
> > --- a/include/linux/regmap.h
> > +++ b/include/linux/regmap.h
> > @@ -25,6 +25,7 @@ struct module;
> > struct clk;
> > struct device;
> > struct i2c_client;
> > +struct i3c_device;
> > struct irq_domain;
> > struct slim_device;
> > struct spi_device;
> > @@ -624,6 +625,10 @@ struct regmap *__devm_regmap_init_slimbus(struct slim_device *slimbus,
> > const struct regmap_config *config,
> > struct lock_class_key *lock_key,
> > const char *lock_name);
> > +struct regmap *__devm_regmap_init_i3c(struct i3c_device *i3c,
> > + const struct regmap_config *config,
> > + struct lock_class_key *lock_key,
> > + const char *lock_name);
> > /*
> > * Wrapper for regmap_init macros to include a unique lockdep key and name
> > * for each call. No-op if CONFIG_LOCKDEP is not set.
> > @@ -982,6 +987,21 @@ bool regmap_ac97_default_volatile(struct device *dev, unsigned int reg);
> > #define devm_regmap_init_slimbus(slimbus, config) \
> > __regmap_lockdep_wrapper(__devm_regmap_init_slimbus, #config, \
> > slimbus, config)
> > +
> > +/**
> > + * devm_regmap_init_i3c() - Initialise managed register map
>
> Maybe we should have a more specific name here, as some might want to
> access registers using HDR modes (devm_regmap_init_i3c_sdr()?)
That make sense if the device with HDR capability is able to do
everything in HDR mode and I'm not sure about that.
I would like to wait for a couple of commercial devices before that
change.
>
>
> > + *
> > + * @i3c: Device that will be interacted with
> > + * @config: Configuration for register map
> > + *
> > + * The return value will be an ERR_PTR() on error or a valid pointer
> > + * to a struct regmap. The regmap will be automatically freed by the
> > + * device management code.
> > + */
> > +#define devm_regmap_init_i3c(i3c, config) \
> > + __regmap_lockdep_wrapper(__devm_regmap_init_i3c, #config, \
> > + i3c, config)
> > +
> > int regmap_mmio_attach_clk(struct regmap *map, struct clk *clk);
> > void regmap_mmio_detach_clk(struct regmap *map);
> > void regmap_exit(struct regmap *map);
Thanks for your feedback.
Best regards,
Vitor Soares
Hi Boris,
From: Boris Brezillon <[email protected]>
Date: Tue, Apr 16, 2019 at 07:18:25
> On Mon, 15 Apr 2019 21:19:40 +0200
> Vitor Soares <[email protected]> wrote:
>
> > This helper return the i3c_device_id structure in order the client
> > have access to the driver data.
> >
> > Signed-off-by: Vitor Soares <[email protected]>
> > ---
> > drivers/i3c/device.c | 8 ++++++++
> > include/linux/i3c/device.h | 1 +
> > 2 files changed, 9 insertions(+)
> >
> > diff --git a/drivers/i3c/device.c b/drivers/i3c/device.c
> > index 472be99..8ab8e4c 100644
> > --- a/drivers/i3c/device.c
> > +++ b/drivers/i3c/device.c
> > @@ -235,6 +235,14 @@ struct i3c_device *dev_to_i3cdev(struct device *dev)
> > }
> > EXPORT_SYMBOL_GPL(dev_to_i3cdev);
> >
> > +const struct i3c_device_id *i3c_get_device_id(struct i3c_device *i3cdev)
> > +{
> > + const struct i3c_driver *i3cdrv = drv_to_i3cdrv(i3cdev->dev.driver);
> > +
> > + return i3cdrv->id_table;
> > +}
> > +EXPORT_SYMBOL_GPL(i3c_get_device_id);
>
> I think what you want is i3c_device_match_id().
I want the i3c_device_id struct of an i3c_device. What is the name that
you suggest?
>Just move the function
> to drivers/i3c/device.c, export it and define its prototype in
> include/i3c/device.h.
You are right, this should go for the device side.
>
> > +
> > /**
> > * i3c_driver_register_with_owner() - register an I3C device driver
> > *
> > diff --git a/include/linux/i3c/device.h b/include/linux/i3c/device.h
> > index 7ee7e30..ee48886 100644
> > --- a/include/linux/i3c/device.h
> > +++ b/include/linux/i3c/device.h
> > @@ -211,6 +211,7 @@ static inline struct i3c_driver *drv_to_i3cdrv(struct device_driver *drv)
> >
> > struct device *i3cdev_to_dev(struct i3c_device *i3cdev);
> > struct i3c_device *dev_to_i3cdev(struct device *dev);
> > +const struct i3c_device_id *i3c_get_device_id(struct i3c_device *i3cdev);
> >
> > static inline void i3cdev_set_drvdata(struct i3c_device *i3cdev,
> > void *data)
Best regards,
Vitor Soares
Hi Boris and Lorenzo,
From: Boris Brezillon <[email protected]>
Date: Tue, Apr 16, 2019 at 09:05:40
> On Tue, 16 Apr 2019 09:58:33 +0200
> Lorenzo Bianconi <[email protected]> wrote:
>
> > >
> > > On Mon, 15 Apr 2019 21:19:41 +0200
> > > Vitor Soares <[email protected]> wrote:
> > >
> > > > For today the st_lsm6dsx driver support lsm6dso sensor only in
> > > > spi and i2c mode.
> > > >
> > > > The lsm6dso is also i3c capable so lets give i3c support to it.
> > > >
> > > > Signed-off-by: Vitor Soares <[email protected]>
> > > > ---
> > > > drivers/iio/imu/st_lsm6dsx/Kconfig | 8 +++-
> > > > drivers/iio/imu/st_lsm6dsx/Makefile | 1 +
> > > > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c | 67 +++++++++++++++++++++++++++++
> > > > 3 files changed, 75 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 094fd00..1ab9bbf 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..2df5e70
> > > > --- /dev/null
> > > > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
> > > > @@ -0,0 +1,67 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +/*
> > > > + * Copyright (c) 2018 Synopsys, Inc. and/or its affiliates.
> > > > + *
> > > > + * Author: Vitor Soares <[email protected]>
> > > > + */
> > > > +
> > > > +#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"
> > > > +
> > > > +#define NAME_SIZE 32
> > > > +
> > > > +struct st_lsm6dsx_i3c_data {
> > > > + char name[NAME_SIZE];
> > >
> > > const char *name;
I will fix this next patch.
> > >
> > > > + enum st_lsm6dsx_hw_id id;
> > > > +};
> > > > +
> > > > +static const struct st_lsm6dsx_i3c_data hw_data[] = {
> > >
> > > No need to make it an array, and it should probably be named
> > > st_lsm6dso_data not hw_data.
> >
> > I guess it will be used even for future devices so I would prefer to
> > make it an array with a 'general' name
The idea is that one. Are you ok with hw_data name?
Should I add the lsm6dsr support here or send in a different patch?
>
> I find it more error-prone to reference an array with an opaque index
> number than reference an instance with a name that clearly reflects
> what HW it describes (see the I3C_DEVICE() entry at the end of the
> driver), but okay.
I will address this next version.
>
> >
> > Regards,
> > Lorenzo
> >
> > >
> > > > + { ST_LSM6DSO_DEV_NAME, ST_LSM6DSO_ID },
> > > > +};
> > > > +
> > > > +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_get_device_id(i3cdev);
> > >
> > > i3c_device_match_id(i3cdev,
> > > st_lsm6dsx_i3c_ids);
> > >
> > > > + const struct st_lsm6dsx_i3c_data *hw_data = id->data;
> > > > + 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, hw_data->id,
> > > > + hw_data->name, regmap);
> > > > +}
> > > > +
> > > > +
> > > > +static const struct i3c_device_id st_lsm6dsx_i3c_ids[] = {
> > > > + I3C_DEVICE(0x0104, 0x006C, &hw_data[0]),
> > > > + { /* sentinel */ },
> > > > +};
> > > > +MODULE_DEVICE_TABLE(i3c, st_lsm6dsx_i3c_ids);
> > > > +
> > > > +static struct i3c_driver st_lsm6dsx_driver = {
> > > > + .driver.name = "st_lsm6dsx_i3c",
> > > > + .probe = st_lsm6dsx_i3c_probe,
> > > > + .id_table = st_lsm6dsx_i3c_ids,
> > >
> > > You should probably set the pm_ops here (st_lsm6dsx_pm_ops).
> > >
> > > > +};
> > > > +module_i3c_driver(st_lsm6dsx_driver);
> > > > +
> > > > +MODULE_AUTHOR("Vitor Soares <[email protected]>");
> > > > +MODULE_DESCRIPTION("STMicroelectronics st_lsm6dsx i3c driver");
> > > > +MODULE_LICENSE("GPL v2");
> > > > +
> > >
> > > You can remove this blank line.
> > >
> >
> >
Best regards,
Vitor Soares
Hi Boris,
From: Boris Brezillon <[email protected]>
Date: Tue, Apr 16, 2019 at 07:25:39
> > +MODULE_DEVICE_TABLE(i3c, st_lsm6dsx_i3c_ids);
> > +
> > +static struct i3c_driver st_lsm6dsx_driver = {
> > + .driver.name = "st_lsm6dsx_i3c",
> > + .probe = st_lsm6dsx_i3c_probe,
> > + .id_table = st_lsm6dsx_i3c_ids,
>
> You should probably set the pm_ops here (st_lsm6dsx_pm_ops).
Yes, I missed that.
>
> > +};
> > +module_i3c_driver(st_lsm6dsx_driver);
> > +
> > +MODULE_AUTHOR("Vitor Soares <[email protected]>");
> > +MODULE_DESCRIPTION("STMicroelectronics st_lsm6dsx i3c driver");
> > +MODULE_LICENSE("GPL v2");
> > +
>
> You can remove this blank line.
Thanks for your feedback.
Best regards,
Vitor Soares
On Mon, Apr 15, 2019 at 09:19:39PM +0200, Vitor Soares wrote:
> +++ b/drivers/base/regmap/regmap-i3c.c
> @@ -0,0 +1,62 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2018 Synopsys, Inc. and/or its affiliates.
Please make the entire header C++ style so it looks more consistent.
Otherwise this looks good modulo Boris' comment; I'm fine with leaving
extra modes for later so long as they can be introduced without
disrupting existing users so the only question there would be if we
should name the init function in some way that's specific to the I/O
mode being used here.
Hi Mark,
From: Mark Brown <[email protected]>
Date: Tue, Apr 16, 2019 at 16:39:48
> On Mon, Apr 15, 2019 at 09:19:39PM +0200, Vitor Soares wrote:
>
> > +++ b/drivers/base/regmap/regmap-i3c.c
> > @@ -0,0 +1,62 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2018 Synopsys, Inc. and/or its affiliates.
>
> Please make the entire header C++ style so it looks more consistent.
> Otherwise this looks good modulo
I will change it next drop.
> Boris' comment; I'm fine with leaving
> extra modes for later so long as they can be introduced without
> disrupting existing users so the only question there would be if we
> should name the init function in some way that's specific to the I/O
> mode being used here.
My concern is that booth modes (SDR/HDR) might be needed on the device.
e.g. use SDR to configure the device and use HDR to send/receive large
data.
Best regards,
Vitor Soares
On Tue, 23 Apr 2019 14:58:06 +0000
Vitor Soares <[email protected]> wrote:
> Hi Mark,
>
> From: Mark Brown <[email protected]>
> Date: Tue, Apr 16, 2019 at 16:39:48
>
> > On Mon, Apr 15, 2019 at 09:19:39PM +0200, Vitor Soares wrote:
> >
> > > +++ b/drivers/base/regmap/regmap-i3c.c
> > > @@ -0,0 +1,62 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Copyright (c) 2018 Synopsys, Inc. and/or its affiliates.
> >
> > Please make the entire header C++ style so it looks more consistent.
> > Otherwise this looks good modulo
>
> I will change it next drop.
>
> > Boris' comment; I'm fine with leaving
> > extra modes for later so long as they can be introduced without
> > disrupting existing users so the only question there would be if we
> > should name the init function in some way that's specific to the I/O
> > mode being used here.
>
> My concern is that booth modes (SDR/HDR) might be needed on the device.
> e.g. use SDR to configure the device and use HDR to send/receive large
> data.
I'd say that we shouldn't use the regmap abstraction in this case or
have a driver-specific backend implementation for it. I guess the
common case is "regs are accessed in SDR mode", so let's keep the name
as it is now and we'll define devm_regmap_init_i3c_hdr() if we ever
need it. Please make it explicit in the kernel-doc that we're using SDR
transfers here.
Hi Boris,
From: Boris Brezillon <[email protected]>
Date: Tue, Apr 23, 2019 at 16:14:16
> On Tue, 23 Apr 2019 14:58:06 +0000
> Vitor Soares <[email protected]> wrote:
>
> > Hi Mark,
> >
> > From: Mark Brown <[email protected]>
> > Date: Tue, Apr 16, 2019 at 16:39:48
> >
> > > On Mon, Apr 15, 2019 at 09:19:39PM +0200, Vitor Soares wrote:
> > >
> > > > +++ b/drivers/base/regmap/regmap-i3c.c
> > > > @@ -0,0 +1,62 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +/*
> > > > + * Copyright (c) 2018 Synopsys, Inc. and/or its affiliates.
> > >
> > > Please make the entire header C++ style so it looks more consistent.
> > > Otherwise this looks good modulo
> >
> > I will change it next drop.
> >
> > > Boris' comment; I'm fine with leaving
> > > extra modes for later so long as they can be introduced without
> > > disrupting existing users so the only question there would be if we
> > > should name the init function in some way that's specific to the I/O
> > > mode being used here.
> >
> > My concern is that booth modes (SDR/HDR) might be needed on the device.
> > e.g. use SDR to configure the device and use HDR to send/receive large
> > data.
>
> I'd say that we shouldn't use the regmap abstraction in this case or
> have a driver-specific backend implementation for it. I guess the
> common case is "regs are accessed in SDR mode", so let's keep the name
> as it is now and we'll define devm_regmap_init_i3c_hdr() if we ever
> need it. Please make it explicit in the kernel-doc that we're using SDR
> transfers here.
Where should it be documented? Can you give an example?
Thanks,
Vitor Soares