[PATCH 0/3] Add ST lsm6dso i3c suppport

[PATCH 0/3] Add ST lsm6dso i3c suppport

[Vitor Soares]

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

[PATCH 0/3] Add ST lsm6dso i3c suppport

[Vitor Soares]

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


_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH 1/3] remap: Add I3C bus support

[Vitor Soares]

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 <vitor.soares@synopsys.com>
---
 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 <vitor.soares@synopsys.com>
+ */
+
+#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, &regmap_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

[PATCH 1/3] remap: Add I3C bus support

[Vitor Soares]

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 <vitor.soares@synopsys.com>
---
 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 <vitor.soares@synopsys.com>
+ */
+
+#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, &regmap_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


_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 1/3] remap: Add I3C bus support

[Boris Brezillon]

Typo in the subject: s/remap/regmap/

On Mon, 15 Apr 2019 21:19:39 +0200
Vitor Soares <vitor.soares@synopsys.com> 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 <vitor.soares@synopsys.com>
> ---
>  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 <vitor.soares@synopsys.com>
> + */
> +
> +#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, &regmap_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);

Re: [PATCH 1/3] remap: Add I3C bus support

[Boris Brezillon]

Typo in the subject: s/remap/regmap/

On Mon, 15 Apr 2019 21:19:39 +0200
Vitor Soares <vitor.soares@synopsys.com> 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 <vitor.soares@synopsys.com>
> ---
>  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 <vitor.soares@synopsys.com>
> + */
> +
> +#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, &regmap_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);


_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

RE: [PATCH 1/3] remap: Add I3C bus support

[Vitor Soares]

Hi Boris,

From: Boris Brezillon <boris.brezillon@collabora.com>
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 <vitor.soares@synopsys.com> 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 <vitor.soares@synopsys.com>
> > ---
> >  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 <vitor.soares@synopsys.com>
> > + */
> > +
> > +#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, &regmap_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

RE: [PATCH 1/3] remap: Add I3C bus support

[Vitor Soares]

Hi Boris,

From: Boris Brezillon <boris.brezillon@collabora.com>
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 <vitor.soares@synopsys.com> 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 <vitor.soares@synopsys.com>
> > ---
> >  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 <vitor.soares@synopsys.com>
> > + */
> > +
> > +#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, &regmap_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


_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 1/3] remap: Add I3C bus support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 595 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

RE: [PATCH 1/3] remap: Add I3C bus support

[Vitor Soares]

Hi Mark,

From: Mark Brown <broonie@kernel.org>
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

RE: [PATCH 1/3] remap: Add I3C bus support

[Vitor Soares]

Hi Mark,

From: Mark Brown <broonie@kernel.org>
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

_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 1/3] remap: Add I3C bus support

[Boris Brezillon]

On Tue, 23 Apr 2019 14:58:06 +0000
Vitor Soares <Vitor.Soares@synopsys.com> wrote:

> Hi Mark,
> 
> From: Mark Brown <broonie@kernel.org>
> 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.

Re: [PATCH 1/3] remap: Add I3C bus support

[Boris Brezillon]

On Tue, 23 Apr 2019 14:58:06 +0000
Vitor Soares <Vitor.Soares@synopsys.com> wrote:

> Hi Mark,
> 
> From: Mark Brown <broonie@kernel.org>
> 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.

_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

RE: [PATCH 1/3] remap: Add I3C bus support

[Vitor Soares]

Hi Boris,

From: Boris Brezillon <boris.brezillon@collabora.com>
Date: Tue, Apr 23, 2019 at 16:14:16

> On Tue, 23 Apr 2019 14:58:06 +0000
> Vitor Soares <Vitor.Soares@synopsys.com> wrote:
> 
> > Hi Mark,
> > 
> > From: Mark Brown <broonie@kernel.org>
> > 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

RE: [PATCH 1/3] remap: Add I3C bus support

[Vitor Soares]

Hi Boris,

From: Boris Brezillon <boris.brezillon@collabora.com>
Date: Tue, Apr 23, 2019 at 16:14:16

> On Tue, 23 Apr 2019 14:58:06 +0000
> Vitor Soares <Vitor.Soares@synopsys.com> wrote:
> 
> > Hi Mark,
> > 
> > From: Mark Brown <broonie@kernel.org>
> > 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

_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH 2/3] i3c: Add i3c_get_device_id helper

[Vitor Soares]

This helper return the i3c_device_id structure in order the client
have access to the driver data.

Signed-off-by: Vitor Soares <vitor.soares@synopsys.com>
---
 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

[PATCH 2/3] i3c: Add i3c_get_device_id helper

[Vitor Soares]

This helper return the i3c_device_id structure in order the client
have access to the driver data.

Signed-off-by: Vitor Soares <vitor.soares@synopsys.com>
---
 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


_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 2/3] i3c: Add i3c_get_device_id helper

[Boris Brezillon]

On Mon, 15 Apr 2019 21:19:40 +0200
Vitor Soares <vitor.soares@synopsys.com> wrote:

> This helper return the i3c_device_id structure in order the client
> have access to the driver data.
> 
> Signed-off-by: Vitor Soares <vitor.soares@synopsys.com>
> ---
>  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)

Re: [PATCH 2/3] i3c: Add i3c_get_device_id helper

[Boris Brezillon]

On Mon, 15 Apr 2019 21:19:40 +0200
Vitor Soares <vitor.soares@synopsys.com> wrote:

> This helper return the i3c_device_id structure in order the client
> have access to the driver data.
> 
> Signed-off-by: Vitor Soares <vitor.soares@synopsys.com>
> ---
>  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)


_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

RE: [PATCH 2/3] i3c: Add i3c_get_device_id helper

[Vitor Soares]

Hi Boris,

From: Boris Brezillon <boris.brezillon@collabora.com>
Date: Tue, Apr 16, 2019 at 07:18:25

> On Mon, 15 Apr 2019 21:19:40 +0200
> Vitor Soares <vitor.soares@synopsys.com> wrote:
> 
> > This helper return the i3c_device_id structure in order the client
> > have access to the driver data.
> > 
> > Signed-off-by: Vitor Soares <vitor.soares@synopsys.com>
> > ---
> >  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

RE: [PATCH 2/3] i3c: Add i3c_get_device_id helper

[Vitor Soares]

Hi Boris,

From: Boris Brezillon <boris.brezillon@collabora.com>
Date: Tue, Apr 16, 2019 at 07:18:25

> On Mon, 15 Apr 2019 21:19:40 +0200
> Vitor Soares <vitor.soares@synopsys.com> wrote:
> 
> > This helper return the i3c_device_id structure in order the client
> > have access to the driver data.
> > 
> > Signed-off-by: Vitor Soares <vitor.soares@synopsys.com>
> > ---
> >  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

_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH 3/3] iio: imu: st_lsm6dsx: Add i3c basic support

[Vitor Soares]

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 <vitor.soares@synopsys.com>
---
 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 <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"
+
+#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 <vitor.soares@synopsys.com>");
+MODULE_DESCRIPTION("STMicroelectronics st_lsm6dsx i3c driver");
+MODULE_LICENSE("GPL v2");
+
-- 
2.7.4

[PATCH 3/3] iio: imu: st_lsm6dsx: Add i3c basic support

[Vitor Soares]

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 <vitor.soares@synopsys.com>
---
 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 <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"
+
+#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 <vitor.soares@synopsys.com>");
+MODULE_DESCRIPTION("STMicroelectronics st_lsm6dsx i3c driver");
+MODULE_LICENSE("GPL v2");
+
-- 
2.7.4


_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 3/3] iio: imu: st_lsm6dsx: Add i3c basic support

[Lorenzo Bianconi]

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

Hi Vitor,

thx, this is very cool :)
Just a couple of nit inline

Regards,
Lorenzo

> Signed-off-by: Vitor Soares <vitor.soares@synopsys.com>
> ---
>  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 <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"
> +
> +#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);

In order to support the hw FIFO, I guess you are planning to support
interrupts/IBI here

> +}
> +

please remove 'new line' here

> +
> +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 <vitor.soares@synopsys.com>");
> +MODULE_DESCRIPTION("STMicroelectronics st_lsm6dsx i3c driver");
> +MODULE_LICENSE("GPL v2");
> +
> --
> 2.7.4
>


-- 
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

Re: [PATCH 3/3] iio: imu: st_lsm6dsx: Add i3c basic support

[Lorenzo Bianconi]

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

Hi Vitor,

thx, this is very cool :)
Just a couple of nit inline

Regards,
Lorenzo

> Signed-off-by: Vitor Soares <vitor.soares@synopsys.com>
> ---
>  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 <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"
> +
> +#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);

In order to support the hw FIFO, I guess you are planning to support
interrupts/IBI here

> +}
> +

please remove 'new line' here

> +
> +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 <vitor.soares@synopsys.com>");
> +MODULE_DESCRIPTION("STMicroelectronics st_lsm6dsx i3c driver");
> +MODULE_LICENSE("GPL v2");
> +
> --
> 2.7.4
>


-- 
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

_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

RE: [PATCH 3/3] iio: imu: st_lsm6dsx: Add i3c basic support

[Vitor Soares]

HI Lorenzo,

From: Lorenzo Bianconi <lorenzo.bianconi83@gmail.com>
Date: Mon, Apr 15, 2019 at 23:17:50

> >
> > 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.
> >
> 
> Hi Vitor,
> 
> thx, this is very cool :)
> Just a couple of nit inline
> 
> Regards,
> Lorenzo
> 
> > Signed-off-by: Vitor Soares <vitor.soares@synopsys.com>
> > ---
> >  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 <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"
> > +
> > +#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);
> 
> In order to support the hw FIFO, I guess you are planning to support
> interrupts/IBI here

My idea is to register the ibi and handler in this file.
The handler would call a buffer function passing the device and payload. 
On st_lsm6dsx_probe we can add a flag for IBI configuration on the sensor 
side.

> 
> > +}
> > +
> 
> please remove 'new line' here

Sure.

> 
> > +
> > +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 <vitor.soares@synopsys.com>");
> > +MODULE_DESCRIPTION("STMicroelectronics st_lsm6dsx i3c driver");
> > +MODULE_LICENSE("GPL v2");
> > +
> > --
> > 2.7.4
> >
> 
> 
> -- 
> 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

Thanks for your feedback.

Best regards,
Vitor Soares

RE: [PATCH 3/3] iio: imu: st_lsm6dsx: Add i3c basic support

[Vitor Soares]

HI Lorenzo,

From: Lorenzo Bianconi <lorenzo.bianconi83@gmail.com>
Date: Mon, Apr 15, 2019 at 23:17:50

> >
> > 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.
> >
> 
> Hi Vitor,
> 
> thx, this is very cool :)
> Just a couple of nit inline
> 
> Regards,
> Lorenzo
> 
> > Signed-off-by: Vitor Soares <vitor.soares@synopsys.com>
> > ---
> >  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 <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"
> > +
> > +#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);
> 
> In order to support the hw FIFO, I guess you are planning to support
> interrupts/IBI here

My idea is to register the ibi and handler in this file.
The handler would call a buffer function passing the device and payload. 
On st_lsm6dsx_probe we can add a flag for IBI configuration on the sensor 
side.

> 
> > +}
> > +
> 
> please remove 'new line' here

Sure.

> 
> > +
> > +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 <vitor.soares@synopsys.com>");
> > +MODULE_DESCRIPTION("STMicroelectronics st_lsm6dsx i3c driver");
> > +MODULE_LICENSE("GPL v2");
> > +
> > --
> > 2.7.4
> >
> 
> 
> -- 
> 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

Thanks for your feedback.

Best regards,
Vitor Soares

_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 3/3] iio: imu: st_lsm6dsx: Add i3c basic support

[Boris Brezillon]

On Mon, 15 Apr 2019 21:19:41 +0200
Vitor Soares <vitor.soares@synopsys.com> 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 <vitor.soares@synopsys.com>
> ---
>  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 <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"
> +
> +#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 <vitor.soares@synopsys.com>");
> +MODULE_DESCRIPTION("STMicroelectronics st_lsm6dsx i3c driver");
> +MODULE_LICENSE("GPL v2");
> +

You can remove this blank line.

Re: [PATCH 3/3] iio: imu: st_lsm6dsx: Add i3c basic support

[Boris Brezillon]

On Mon, 15 Apr 2019 21:19:41 +0200
Vitor Soares <vitor.soares@synopsys.com> 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 <vitor.soares@synopsys.com>
> ---
>  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 <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"
> +
> +#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 <vitor.soares@synopsys.com>");
> +MODULE_DESCRIPTION("STMicroelectronics st_lsm6dsx i3c driver");
> +MODULE_LICENSE("GPL v2");
> +

You can remove this blank line.


_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 3/3] iio: imu: st_lsm6dsx: Add i3c basic support

[Lorenzo Bianconi]

>
> On Mon, 15 Apr 2019 21:19:41 +0200
> Vitor Soares <vitor.soares@synopsys.com> 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 <vitor.soares@synopsys.com>
> > ---
> >  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 <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"
> > +
> > +#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 <vitor.soares@synopsys.com>");
> > +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

Re: [PATCH 3/3] iio: imu: st_lsm6dsx: Add i3c basic support

[Lorenzo Bianconi]

>
> On Mon, 15 Apr 2019 21:19:41 +0200
> Vitor Soares <vitor.soares@synopsys.com> 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 <vitor.soares@synopsys.com>
> > ---
> >  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 <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"
> > +
> > +#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 <vitor.soares@synopsys.com>");
> > +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

_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 3/3] iio: imu: st_lsm6dsx: Add i3c basic support

[Boris Brezillon]

On Tue, 16 Apr 2019 09:58:33 +0200
Lorenzo Bianconi <lorenzo.bianconi83@gmail.com> wrote:

> >
> > On Mon, 15 Apr 2019 21:19:41 +0200
> > Vitor Soares <vitor.soares@synopsys.com> 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 <vitor.soares@synopsys.com>
> > > ---
> > >  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 <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"
> > > +
> > > +#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 <vitor.soares@synopsys.com>");
> > > +MODULE_DESCRIPTION("STMicroelectronics st_lsm6dsx i3c driver");
> > > +MODULE_LICENSE("GPL v2");
> > > +  
> >
> > You can remove this blank line.
> >  
> 
> 

Re: [PATCH 3/3] iio: imu: st_lsm6dsx: Add i3c basic support

[Boris Brezillon]

On Tue, 16 Apr 2019 09:58:33 +0200
Lorenzo Bianconi <lorenzo.bianconi83@gmail.com> wrote:

> >
> > On Mon, 15 Apr 2019 21:19:41 +0200
> > Vitor Soares <vitor.soares@synopsys.com> 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 <vitor.soares@synopsys.com>
> > > ---
> > >  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 <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"
> > > +
> > > +#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 <vitor.soares@synopsys.com>");
> > > +MODULE_DESCRIPTION("STMicroelectronics st_lsm6dsx i3c driver");
> > > +MODULE_LICENSE("GPL v2");
> > > +  
> >
> > You can remove this blank line.
> >  
> 
> 


_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

RE: [PATCH 3/3] iio: imu: st_lsm6dsx: Add i3c basic support

[Vitor Soares]

Hi Boris  and Lorenzo,

From: Boris Brezillon <boris.brezillon@collabora.com>
Date: Tue, Apr 16, 2019 at 09:05:40

> On Tue, 16 Apr 2019 09:58:33 +0200
> Lorenzo Bianconi <lorenzo.bianconi83@gmail.com> wrote:
> 
> > >
> > > On Mon, 15 Apr 2019 21:19:41 +0200
> > > Vitor Soares <vitor.soares@synopsys.com> 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 <vitor.soares@synopsys.com>
> > > > ---
> > > >  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 <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"
> > > > +
> > > > +#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 <vitor.soares@synopsys.com>");
> > > > +MODULE_DESCRIPTION("STMicroelectronics st_lsm6dsx i3c driver");
> > > > +MODULE_LICENSE("GPL v2");
> > > > +  
> > >
> > > You can remove this blank line.
> > >  
> > 
> > 

Best regards,
Vitor Soares

RE: [PATCH 3/3] iio: imu: st_lsm6dsx: Add i3c basic support

[Vitor Soares]

Hi Boris  and Lorenzo,

From: Boris Brezillon <boris.brezillon@collabora.com>
Date: Tue, Apr 16, 2019 at 09:05:40

> On Tue, 16 Apr 2019 09:58:33 +0200
> Lorenzo Bianconi <lorenzo.bianconi83@gmail.com> wrote:
> 
> > >
> > > On Mon, 15 Apr 2019 21:19:41 +0200
> > > Vitor Soares <vitor.soares@synopsys.com> 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 <vitor.soares@synopsys.com>
> > > > ---
> > > >  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 <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"
> > > > +
> > > > +#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 <vitor.soares@synopsys.com>");
> > > > +MODULE_DESCRIPTION("STMicroelectronics st_lsm6dsx i3c driver");
> > > > +MODULE_LICENSE("GPL v2");
> > > > +  
> > >
> > > You can remove this blank line.
> > >  
> > 
> > 

Best regards,
Vitor Soares

_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

RE: [PATCH 3/3] iio: imu: st_lsm6dsx: Add i3c basic support

[Vitor Soares]

Hi Boris,

From: Boris Brezillon <boris.brezillon@collabora.com>
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 <vitor.soares@synopsys.com>");
> > +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

RE: [PATCH 3/3] iio: imu: st_lsm6dsx: Add i3c basic support

[Vitor Soares]

Hi Boris,

From: Boris Brezillon <boris.brezillon@collabora.com>
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 <vitor.soares@synopsys.com>");
> > +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



_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 0/3] Add ST lsm6dso i3c suppport

[Boris Brezillon]

Hi Vitor,

On Mon, 15 Apr 2019 21:19:38 +0200
Vitor Soares <vitor.soares@synopsys.com> 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
> 

Re: [PATCH 0/3] Add ST lsm6dso i3c suppport

[Boris Brezillon]

Hi Vitor,

On Mon, 15 Apr 2019 21:19:38 +0200
Vitor Soares <vitor.soares@synopsys.com> 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
> 


_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c