* [PATCH v4 0/3] Add ST lsm6dso i3c support
@ 2019-07-12 11:53 Vitor Soares
2019-07-12 11:53 ` [PATCH v4 1/3] regmap: add i3c bus support Vitor Soares
` (3 more replies)
0 siblings, 4 replies; 17+ messages in thread
From: Vitor Soares @ 2019-07-12 11:53 UTC (permalink / raw)
To: linux-iio, linux-i3c, linux-kernel
Cc: Joao.Pinto, bbrezillon, gregkh, rafael, Vitor Soares, lorenzo
This patch series add i3c support for STM LSM6DSO and LSM6DSR sensors.
It is also introduced i3c support on regmap api. Due the lack of
i3c devices HDR capables on the market the support for now is only for
i3c sdr mode by using i3c_device_do_priv_xfers() method.
The i3c regmap api is already available in the Git repository at:
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git
tags/regmap-i3c
Change in v4:
remover hw_id variable from st_lsm6dsx_i3c_probe()
Change in v3:
Update st_lsm6dsx_probe() call
Remove i3c_get_device_id() and use i3c_device_match_id()
Changes in v2:
Change i3c_get_device_id() to drivers/i3c/device.c
Add support for LSM6DSR
Vitor Soares (3):
regmap: add i3c bus support
i3c: add i3c_get_device_id helper
iio: imu: st_lsm6dsx: add i3c basic support for LSM6DSO and LSM6DSR
drivers/base/regmap/Kconfig | 6 ++-
drivers/base/regmap/Makefile | 1 +
drivers/base/regmap/regmap-i3c.c | 60 +++++++++++++++++++++++++++++
drivers/i3c/device.c | 46 ++++++++++++++++++++++
drivers/i3c/master.c | 45 ----------------------
drivers/iio/imu/st_lsm6dsx/Kconfig | 8 +++-
drivers/iio/imu/st_lsm6dsx/Makefile | 1 +
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c | 58 ++++++++++++++++++++++++++++
include/linux/i3c/device.h | 4 ++
include/linux/regmap.h | 20 ++++++++++
10 files changed, 202 insertions(+), 47 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
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v4 1/3] regmap: add i3c bus support
2019-07-12 11:53 [PATCH v4 0/3] Add ST lsm6dso i3c support Vitor Soares
@ 2019-07-12 11:53 ` Vitor Soares
2019-07-12 15:59 ` Boris Brezillon
2019-07-12 11:53 ` [PATCH v4 2/3] i3c: move i3c_device_match_id to device.c and export it Vitor Soares
` (2 subsequent siblings)
3 siblings, 1 reply; 17+ messages in thread
From: Vitor Soares @ 2019-07-12 11:53 UTC (permalink / raw)
To: linux-iio, linux-i3c, linux-kernel
Cc: Joao.Pinto, bbrezillon, gregkh, rafael, Vitor Soares, lorenzo
Add basic support for i3c bus.
This is a simple implementation that only give support
for SDR 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 | 60 ++++++++++++++++++++++++++++++++++++++++
include/linux/regmap.h | 20 ++++++++++++++
4 files changed, 86 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..1578fb5
--- /dev/null
+++ b/drivers/base/regmap/regmap-i3c.c
@@ -0,0 +1,60 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2018 Synopsys, Inc. and/or its affiliates.
+
+#include <linux/regmap.h>
+#include <linux/i3c/device.h>
+#include <linux/i3c/master.h>
+#include <linux/module.h>
+
+static int regmap_i3c_write(void *context, const void *data, size_t count)
+{
+ struct device *dev = context;
+ struct i3c_device *i3c = dev_to_i3cdev(dev);
+ struct i3c_priv_xfer xfers[] = {
+ {
+ .rnw = false,
+ .len = count,
+ .data.out = data,
+ },
+ };
+
+ return i3c_device_do_priv_xfers(i3c, xfers, 1);
+}
+
+static int regmap_i3c_read(void *context,
+ const void *reg, size_t reg_size,
+ void *val, size_t val_size)
+{
+ struct device *dev = context;
+ struct i3c_device *i3c = dev_to_i3cdev(dev);
+ struct i3c_priv_xfer xfers[2];
+
+ xfers[0].rnw = false;
+ xfers[0].len = reg_size;
+ xfers[0].data.out = reg;
+
+ xfers[1].rnw = true;
+ xfers[1].len = val_size;
+ xfers[1].data.in = val;
+
+ return i3c_device_do_priv_xfers(i3c, xfers, 2);
+}
+
+static struct regmap_bus regmap_i3c = {
+ .write = regmap_i3c_write,
+ .read = regmap_i3c_read,
+};
+
+struct regmap *__devm_regmap_init_i3c(struct i3c_device *i3c,
+ const struct regmap_config *config,
+ struct lock_class_key *lock_key,
+ const char *lock_name)
+{
+ return __devm_regmap_init(&i3c->dev, ®map_i3c, &i3c->dev, config,
+ lock_key, lock_name);
+}
+EXPORT_SYMBOL_GPL(__devm_regmap_init_i3c);
+
+MODULE_AUTHOR("Vitor Soares <vitor.soares@synopsys.com>");
+MODULE_DESCRIPTION("Regmap I3C Module");
+MODULE_LICENSE("GPL v2");
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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v4 2/3] i3c: move i3c_device_match_id to device.c and export it
2019-07-12 11:53 [PATCH v4 0/3] Add ST lsm6dso i3c support Vitor Soares
2019-07-12 11:53 ` [PATCH v4 1/3] regmap: add i3c bus support Vitor Soares
@ 2019-07-12 11:53 ` Vitor Soares
2019-07-12 16:03 ` Boris Brezillon
2019-07-12 11:53 ` [PATCH v4 3/3] iio: imu: st_lsm6dsx: add i3c basic support for LSM6DSO and LSM6DSR Vitor Soares
2019-07-12 15:58 ` [PATCH v4 0/3] Add ST lsm6dso i3c support Boris Brezillon
3 siblings, 1 reply; 17+ messages in thread
From: Vitor Soares @ 2019-07-12 11:53 UTC (permalink / raw)
To: linux-iio, linux-i3c, linux-kernel
Cc: Joao.Pinto, bbrezillon, gregkh, rafael, Vitor Soares, lorenzo
The i3c device driver needs the i3c_device_id table.
Lets move it to device.c and export it to be used.
Signed-off-by: Vitor Soares <vitor.soares@synopsys.com>
---
Changes in v4:
None
Changes in v3:
Remove i3c_get_device_id
Move i3c_device_match_id from drivers/i3c/master.c to drivers/i3c/device.c
Export i3c_device_match_id
Changes in v2:
move this function to drivers/i3c/device.c
drivers/i3c/device.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
drivers/i3c/master.c | 45 ---------------------------------------------
include/linux/i3c/device.h | 4 ++++
3 files changed, 50 insertions(+), 45 deletions(-)
diff --git a/drivers/i3c/device.c b/drivers/i3c/device.c
index 69cc040..383df3b 100644
--- a/drivers/i3c/device.c
+++ b/drivers/i3c/device.c
@@ -200,6 +200,52 @@ struct i3c_device *dev_to_i3cdev(struct device *dev)
}
EXPORT_SYMBOL_GPL(dev_to_i3cdev);
+const struct i3c_device_id *
+i3c_device_match_id(struct i3c_device *i3cdev,
+ const struct i3c_device_id *id_table)
+{
+ struct i3c_device_info devinfo;
+ const struct i3c_device_id *id;
+
+ i3c_device_get_info(i3cdev, &devinfo);
+
+ /*
+ * The lower 32bits of the provisional ID is just filled with a random
+ * value, try to match using DCR info.
+ */
+ if (!I3C_PID_RND_LOWER_32BITS(devinfo.pid)) {
+ u16 manuf = I3C_PID_MANUF_ID(devinfo.pid);
+ u16 part = I3C_PID_PART_ID(devinfo.pid);
+ u16 ext_info = I3C_PID_EXTRA_INFO(devinfo.pid);
+
+ /* First try to match by manufacturer/part ID. */
+ for (id = id_table; id->match_flags != 0; id++) {
+ if ((id->match_flags & I3C_MATCH_MANUF_AND_PART) !=
+ I3C_MATCH_MANUF_AND_PART)
+ continue;
+
+ if (manuf != id->manuf_id || part != id->part_id)
+ continue;
+
+ if ((id->match_flags & I3C_MATCH_EXTRA_INFO) &&
+ ext_info != id->extra_info)
+ continue;
+
+ return id;
+ }
+ }
+
+ /* Fallback to DCR match. */
+ for (id = id_table; id->match_flags != 0; id++) {
+ if ((id->match_flags & I3C_MATCH_DCR) &&
+ id->dcr == devinfo.dcr)
+ return id;
+ }
+
+ return NULL;
+}
+EXPORT_SYMBOL_GPL(i3c_device_match_id);
+
/**
* i3c_driver_register_with_owner() - register an I3C device driver
*
diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index 5f4bd52..7667f84 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -270,51 +270,6 @@ static const struct device_type i3c_device_type = {
.uevent = i3c_device_uevent,
};
-static const struct i3c_device_id *
-i3c_device_match_id(struct i3c_device *i3cdev,
- const struct i3c_device_id *id_table)
-{
- struct i3c_device_info devinfo;
- const struct i3c_device_id *id;
-
- i3c_device_get_info(i3cdev, &devinfo);
-
- /*
- * The lower 32bits of the provisional ID is just filled with a random
- * value, try to match using DCR info.
- */
- if (!I3C_PID_RND_LOWER_32BITS(devinfo.pid)) {
- u16 manuf = I3C_PID_MANUF_ID(devinfo.pid);
- u16 part = I3C_PID_PART_ID(devinfo.pid);
- u16 ext_info = I3C_PID_EXTRA_INFO(devinfo.pid);
-
- /* First try to match by manufacturer/part ID. */
- for (id = id_table; id->match_flags != 0; id++) {
- if ((id->match_flags & I3C_MATCH_MANUF_AND_PART) !=
- I3C_MATCH_MANUF_AND_PART)
- continue;
-
- if (manuf != id->manuf_id || part != id->part_id)
- continue;
-
- if ((id->match_flags & I3C_MATCH_EXTRA_INFO) &&
- ext_info != id->extra_info)
- continue;
-
- return id;
- }
- }
-
- /* Fallback to DCR match. */
- for (id = id_table; id->match_flags != 0; id++) {
- if ((id->match_flags & I3C_MATCH_DCR) &&
- id->dcr == devinfo.dcr)
- return id;
- }
-
- return NULL;
-}
-
static int i3c_device_match(struct device *dev, struct device_driver *drv)
{
struct i3c_device *i3cdev;
diff --git a/include/linux/i3c/device.h b/include/linux/i3c/device.h
index 5ecb055..de102e4 100644
--- a/include/linux/i3c/device.h
+++ b/include/linux/i3c/device.h
@@ -188,6 +188,10 @@ 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_device_match_id(struct i3c_device *i3cdev,
+ const struct i3c_device_id *id_table);
+
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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v4 3/3] iio: imu: st_lsm6dsx: add i3c basic support for LSM6DSO and LSM6DSR
2019-07-12 11:53 [PATCH v4 0/3] Add ST lsm6dso i3c support Vitor Soares
2019-07-12 11:53 ` [PATCH v4 1/3] regmap: add i3c bus support Vitor Soares
2019-07-12 11:53 ` [PATCH v4 2/3] i3c: move i3c_device_match_id to device.c and export it Vitor Soares
@ 2019-07-12 11:53 ` Vitor Soares
2019-07-12 16:14 ` Boris Brezillon
2019-07-12 15:58 ` [PATCH v4 0/3] Add ST lsm6dso i3c support Boris Brezillon
3 siblings, 1 reply; 17+ messages in thread
From: Vitor Soares @ 2019-07-12 11:53 UTC (permalink / raw)
To: linux-iio, linux-i3c, linux-kernel
Cc: Joao.Pinto, bbrezillon, gregkh, rafael, Vitor Soares, lorenzo
For today the st_lsm6dsx driver support LSM6DSO and LSM6DSR sensor only in
spi and i2c mode.
The LSM6DSO and LSM6DSR are also i3c capable so lets give i3c support to
them.
Signed-off-by: Vitor Soares <vitor.soares@synopsys.com>
Acked-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
Changes in v4:
Remove hw_id variable
Changes in v3:
Remove unnecessary st_lsm6dsx_i3c_data table used to hold device name
Use st_lsm6dsx_probe new form
Changes in v2:
Add support for LSM6DSR
Set pm_ops to st_lsm6dsx_pm_ops
drivers/iio/imu/st_lsm6dsx/Kconfig | 8 +++-
drivers/iio/imu/st_lsm6dsx/Makefile | 1 +
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c | 58 +++++++++++++++++++++++++++++
3 files changed, 66 insertions(+), 1 deletion(-)
create mode 100644 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
diff --git a/drivers/iio/imu/st_lsm6dsx/Kconfig b/drivers/iio/imu/st_lsm6dsx/Kconfig
index 9e59297..6b5a73c 100644
--- a/drivers/iio/imu/st_lsm6dsx/Kconfig
+++ b/drivers/iio/imu/st_lsm6dsx/Kconfig
@@ -1,11 +1,12 @@
config IIO_ST_LSM6DSX
tristate "ST_LSM6DSx driver for STM 6-axis IMU MEMS sensors"
- depends on (I2C || SPI)
+ depends on (I2C || SPI || I3C)
select IIO_BUFFER
select IIO_KFIFO_BUF
select IIO_ST_LSM6DSX_I2C if (I2C)
select IIO_ST_LSM6DSX_SPI if (SPI_MASTER)
+ select IIO_ST_LSM6DSX_I3C if (I3C)
help
Say yes here to build support for STMicroelectronics LSM6DSx imu
sensor. Supported devices: lsm6ds3, lsm6ds3h, lsm6dsl, lsm6dsm,
@@ -23,3 +24,8 @@ config IIO_ST_LSM6DSX_SPI
tristate
depends on IIO_ST_LSM6DSX
select REGMAP_SPI
+
+config IIO_ST_LSM6DSX_I3C
+ tristate
+ depends on IIO_ST_LSM6DSX
+ select REGMAP_I3C
diff --git a/drivers/iio/imu/st_lsm6dsx/Makefile b/drivers/iio/imu/st_lsm6dsx/Makefile
index e5f733c..c676965 100644
--- a/drivers/iio/imu/st_lsm6dsx/Makefile
+++ b/drivers/iio/imu/st_lsm6dsx/Makefile
@@ -4,3 +4,4 @@ st_lsm6dsx-y := st_lsm6dsx_core.o st_lsm6dsx_buffer.o \
obj-$(CONFIG_IIO_ST_LSM6DSX) += st_lsm6dsx.o
obj-$(CONFIG_IIO_ST_LSM6DSX_I2C) += st_lsm6dsx_i2c.o
obj-$(CONFIG_IIO_ST_LSM6DSX_SPI) += st_lsm6dsx_spi.o
+obj-$(CONFIG_IIO_ST_LSM6DSX_I3C) += st_lsm6dsx_i3c.o
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
new file mode 100644
index 0000000..2e89524
--- /dev/null
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
@@ -0,0 +1,58 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018 Synopsys, Inc. and/or its affiliates.
+ *
+ * Author: Vitor Soares <vitor.soares@synopsys.com>
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/i3c/device.h>
+#include <linux/i3c/master.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/regmap.h>
+
+#include "st_lsm6dsx.h"
+
+static const struct i3c_device_id st_lsm6dsx_i3c_ids[] = {
+ I3C_DEVICE(0x0104, 0x006C, (void *)ST_LSM6DSO_ID),
+ I3C_DEVICE(0x0104, 0x006B, (void *)ST_LSM6DSR_ID),
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(i3c, st_lsm6dsx_i3c_ids);
+
+static const struct regmap_config st_lsm6dsx_i3c_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+};
+
+static int st_lsm6dsx_i3c_probe(struct i3c_device *i3cdev)
+{
+ const struct i3c_device_id *id = i3c_device_match_id(i3cdev,
+ st_lsm6dsx_i3c_ids);
+ struct regmap *regmap;
+
+ regmap = devm_regmap_init_i3c(i3cdev, &st_lsm6dsx_i3c_regmap_config);
+ if (IS_ERR(regmap)) {
+ dev_err(&i3cdev->dev, "Failed to register i3c regmap %d\n",
+ (int)PTR_ERR(regmap));
+ return PTR_ERR(regmap);
+ }
+
+ return st_lsm6dsx_probe(&i3cdev->dev, 0, (int)id->data, regmap);
+}
+
+static struct i3c_driver st_lsm6dsx_driver = {
+ .driver = {
+ .name = "st_lsm6dsx_i3c",
+ .pm = &st_lsm6dsx_pm_ops,
+ },
+ .probe = st_lsm6dsx_i3c_probe,
+ .id_table = st_lsm6dsx_i3c_ids,
+};
+module_i3c_driver(st_lsm6dsx_driver);
+
+MODULE_AUTHOR("Vitor Soares <vitor.soares@synopsys.com>");
+MODULE_DESCRIPTION("STMicroelectronics st_lsm6dsx i3c driver");
+MODULE_LICENSE("GPL v2");
--
2.7.4
_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v4 0/3] Add ST lsm6dso i3c support
2019-07-12 11:53 [PATCH v4 0/3] Add ST lsm6dso i3c support Vitor Soares
` (2 preceding siblings ...)
2019-07-12 11:53 ` [PATCH v4 3/3] iio: imu: st_lsm6dsx: add i3c basic support for LSM6DSO and LSM6DSR Vitor Soares
@ 2019-07-12 15:58 ` Boris Brezillon
3 siblings, 0 replies; 17+ messages in thread
From: Boris Brezillon @ 2019-07-12 15:58 UTC (permalink / raw)
To: Vitor Soares
Cc: Joao.Pinto, rafael, linux-iio, gregkh, bbrezillon, linux-kernel,
linux-i3c, lorenzo
On Fri, 12 Jul 2019 13:53:27 +0200
Vitor Soares <Vitor.Soares@synopsys.com> wrote:
> This patch series add i3c support for STM LSM6DSO and LSM6DSR sensors.
>
> It is also introduced i3c support on regmap api. Due the lack of
> i3c devices HDR capables on the market the support for now is only for
> i3c sdr mode by using i3c_device_do_priv_xfers() method.
>
> The i3c regmap api is already available in the Git repository at:
> https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git
> tags/regmap-i3c
>
> Change in v4:
> remover hw_id variable from st_lsm6dsx_i3c_probe()
>
> Change in v3:
> Update st_lsm6dsx_probe() call
> Remove i3c_get_device_id() and use i3c_device_match_id()
Did not receive this v3...
>
> Changes in v2:
> Change i3c_get_device_id() to drivers/i3c/device.c
> Add support for LSM6DSR
>
> Vitor Soares (3):
> regmap: add i3c bus support
> i3c: add i3c_get_device_id helper
> iio: imu: st_lsm6dsx: add i3c basic support for LSM6DSO and LSM6DSR
>
> drivers/base/regmap/Kconfig | 6 ++-
> drivers/base/regmap/Makefile | 1 +
> drivers/base/regmap/regmap-i3c.c | 60 +++++++++++++++++++++++++++++
> drivers/i3c/device.c | 46 ++++++++++++++++++++++
> drivers/i3c/master.c | 45 ----------------------
> drivers/iio/imu/st_lsm6dsx/Kconfig | 8 +++-
> drivers/iio/imu/st_lsm6dsx/Makefile | 1 +
> drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c | 58 ++++++++++++++++++++++++++++
> include/linux/i3c/device.h | 4 ++
> include/linux/regmap.h | 20 ++++++++++
> 10 files changed, 202 insertions(+), 47 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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 1/3] regmap: add i3c bus support
2019-07-12 11:53 ` [PATCH v4 1/3] regmap: add i3c bus support Vitor Soares
@ 2019-07-12 15:59 ` Boris Brezillon
2019-07-12 16:14 ` Vitor Soares
0 siblings, 1 reply; 17+ messages in thread
From: Boris Brezillon @ 2019-07-12 15:59 UTC (permalink / raw)
To: Vitor Soares
Cc: Joao.Pinto, rafael, linux-iio, gregkh, bbrezillon, linux-kernel,
linux-i3c, lorenzo
On Fri, 12 Jul 2019 13:53:28 +0200
Vitor Soares <Vitor.Soares@synopsys.com> wrote:
> Add basic support for i3c bus.
> This is a simple implementation that only give support
> for SDR Read and Write commands.
>
This patch has been applied by Mark already. Please make sure to drop
already applied patches when submitting a new version.
> 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 | 60 ++++++++++++++++++++++++++++++++++++++++
> include/linux/regmap.h | 20 ++++++++++++++
> 4 files changed, 86 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..1578fb5
> --- /dev/null
> +++ b/drivers/base/regmap/regmap-i3c.c
> @@ -0,0 +1,60 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2018 Synopsys, Inc. and/or its affiliates.
> +
> +#include <linux/regmap.h>
> +#include <linux/i3c/device.h>
> +#include <linux/i3c/master.h>
> +#include <linux/module.h>
> +
> +static int regmap_i3c_write(void *context, const void *data, size_t count)
> +{
> + struct device *dev = context;
> + struct i3c_device *i3c = dev_to_i3cdev(dev);
> + struct i3c_priv_xfer xfers[] = {
> + {
> + .rnw = false,
> + .len = count,
> + .data.out = data,
> + },
> + };
> +
> + return i3c_device_do_priv_xfers(i3c, xfers, 1);
> +}
> +
> +static int regmap_i3c_read(void *context,
> + const void *reg, size_t reg_size,
> + void *val, size_t val_size)
> +{
> + struct device *dev = context;
> + struct i3c_device *i3c = dev_to_i3cdev(dev);
> + struct i3c_priv_xfer xfers[2];
> +
> + xfers[0].rnw = false;
> + xfers[0].len = reg_size;
> + xfers[0].data.out = reg;
> +
> + xfers[1].rnw = true;
> + xfers[1].len = val_size;
> + xfers[1].data.in = val;
> +
> + return i3c_device_do_priv_xfers(i3c, xfers, 2);
> +}
> +
> +static struct regmap_bus regmap_i3c = {
> + .write = regmap_i3c_write,
> + .read = regmap_i3c_read,
> +};
> +
> +struct regmap *__devm_regmap_init_i3c(struct i3c_device *i3c,
> + const struct regmap_config *config,
> + struct lock_class_key *lock_key,
> + const char *lock_name)
> +{
> + return __devm_regmap_init(&i3c->dev, ®map_i3c, &i3c->dev, config,
> + lock_key, lock_name);
> +}
> +EXPORT_SYMBOL_GPL(__devm_regmap_init_i3c);
> +
> +MODULE_AUTHOR("Vitor Soares <vitor.soares@synopsys.com>");
> +MODULE_DESCRIPTION("Regmap I3C Module");
> +MODULE_LICENSE("GPL v2");
> 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);
_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 2/3] i3c: move i3c_device_match_id to device.c and export it
2019-07-12 11:53 ` [PATCH v4 2/3] i3c: move i3c_device_match_id to device.c and export it Vitor Soares
@ 2019-07-12 16:03 ` Boris Brezillon
2019-07-12 16:21 ` Vitor Soares
0 siblings, 1 reply; 17+ messages in thread
From: Boris Brezillon @ 2019-07-12 16:03 UTC (permalink / raw)
To: Vitor Soares
Cc: Joao.Pinto, rafael, linux-iio, gregkh, bbrezillon, linux-kernel,
linux-i3c, lorenzo
On Fri, 12 Jul 2019 13:53:29 +0200
Vitor Soares <Vitor.Soares@synopsys.com> wrote:
> The i3c device driver needs the i3c_device_id table.
"Some I3C device drivers need to know which entry matches the
i3c_device object passed to the probe function"
> Lets move to device.c and export it to be used.
"Let's move i3c_device_match_id() to device.c and export it so it can be
used by drivers."
>
> Signed-off-by: Vitor Soares <vitor.soares@synopsys.com>
> ---
> Changes in v4:
> None
>
> Changes in v3:
> Remove i3c_get_device_id
> Move i3c_device_match_id from drivers/i3c/master.c to drivers/i3c/device.c
> Export i3c_device_match_id
>
> Changes in v2:
> move this function to drivers/i3c/device.c
>
> drivers/i3c/device.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
> drivers/i3c/master.c | 45 ---------------------------------------------
> include/linux/i3c/device.h | 4 ++++
> 3 files changed, 50 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/i3c/device.c b/drivers/i3c/device.c
> index 69cc040..383df3b 100644
> --- a/drivers/i3c/device.c
> +++ b/drivers/i3c/device.c
> @@ -200,6 +200,52 @@ struct i3c_device *dev_to_i3cdev(struct device *dev)
> }
> EXPORT_SYMBOL_GPL(dev_to_i3cdev);
>
You're missing a kerneldoc here.
> +const struct i3c_device_id *
> +i3c_device_match_id(struct i3c_device *i3cdev,
> + const struct i3c_device_id *id_table)
> +{
> + struct i3c_device_info devinfo;
> + const struct i3c_device_id *id;
> +
> + i3c_device_get_info(i3cdev, &devinfo);
> +
> + /*
> + * The lower 32bits of the provisional ID is just filled with a random
> + * value, try to match using DCR info.
> + */
> + if (!I3C_PID_RND_LOWER_32BITS(devinfo.pid)) {
> + u16 manuf = I3C_PID_MANUF_ID(devinfo.pid);
> + u16 part = I3C_PID_PART_ID(devinfo.pid);
> + u16 ext_info = I3C_PID_EXTRA_INFO(devinfo.pid);
> +
> + /* First try to match by manufacturer/part ID. */
> + for (id = id_table; id->match_flags != 0; id++) {
> + if ((id->match_flags & I3C_MATCH_MANUF_AND_PART) !=
> + I3C_MATCH_MANUF_AND_PART)
> + continue;
> +
> + if (manuf != id->manuf_id || part != id->part_id)
> + continue;
> +
> + if ((id->match_flags & I3C_MATCH_EXTRA_INFO) &&
> + ext_info != id->extra_info)
> + continue;
> +
> + return id;
> + }
> + }
> +
> + /* Fallback to DCR match. */
> + for (id = id_table; id->match_flags != 0; id++) {
> + if ((id->match_flags & I3C_MATCH_DCR) &&
> + id->dcr == devinfo.dcr)
> + return id;
> + }
> +
> + return NULL;
> +}
> +EXPORT_SYMBOL_GPL(i3c_device_match_id);
> +
> /**
> * i3c_driver_register_with_owner() - register an I3C device driver
> *
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 5f4bd52..7667f84 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -270,51 +270,6 @@ static const struct device_type i3c_device_type = {
> .uevent = i3c_device_uevent,
> };
>
> -static const struct i3c_device_id *
> -i3c_device_match_id(struct i3c_device *i3cdev,
> - const struct i3c_device_id *id_table)
> -{
> - struct i3c_device_info devinfo;
> - const struct i3c_device_id *id;
> -
> - i3c_device_get_info(i3cdev, &devinfo);
> -
> - /*
> - * The lower 32bits of the provisional ID is just filled with a random
> - * value, try to match using DCR info.
> - */
> - if (!I3C_PID_RND_LOWER_32BITS(devinfo.pid)) {
> - u16 manuf = I3C_PID_MANUF_ID(devinfo.pid);
> - u16 part = I3C_PID_PART_ID(devinfo.pid);
> - u16 ext_info = I3C_PID_EXTRA_INFO(devinfo.pid);
> -
> - /* First try to match by manufacturer/part ID. */
> - for (id = id_table; id->match_flags != 0; id++) {
> - if ((id->match_flags & I3C_MATCH_MANUF_AND_PART) !=
> - I3C_MATCH_MANUF_AND_PART)
> - continue;
> -
> - if (manuf != id->manuf_id || part != id->part_id)
> - continue;
> -
> - if ((id->match_flags & I3C_MATCH_EXTRA_INFO) &&
> - ext_info != id->extra_info)
> - continue;
> -
> - return id;
> - }
> - }
> -
> - /* Fallback to DCR match. */
> - for (id = id_table; id->match_flags != 0; id++) {
> - if ((id->match_flags & I3C_MATCH_DCR) &&
> - id->dcr == devinfo.dcr)
> - return id;
> - }
> -
> - return NULL;
> -}
> -
> static int i3c_device_match(struct device *dev, struct device_driver *drv)
> {
> struct i3c_device *i3cdev;
> diff --git a/include/linux/i3c/device.h b/include/linux/i3c/device.h
> index 5ecb055..de102e4 100644
> --- a/include/linux/i3c/device.h
> +++ b/include/linux/i3c/device.h
> @@ -188,6 +188,10 @@ 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_device_match_id(struct i3c_device *i3cdev,
> + const struct i3c_device_id *id_table);
> +
> 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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 3/3] iio: imu: st_lsm6dsx: add i3c basic support for LSM6DSO and LSM6DSR
2019-07-12 11:53 ` [PATCH v4 3/3] iio: imu: st_lsm6dsx: add i3c basic support for LSM6DSO and LSM6DSR Vitor Soares
@ 2019-07-12 16:14 ` Boris Brezillon
2019-07-12 16:28 ` Vitor Soares
0 siblings, 1 reply; 17+ messages in thread
From: Boris Brezillon @ 2019-07-12 16:14 UTC (permalink / raw)
To: Vitor Soares
Cc: Joao.Pinto, rafael, linux-iio, gregkh, bbrezillon, linux-kernel,
linux-i3c, lorenzo
On Fri, 12 Jul 2019 13:53:30 +0200
Vitor Soares <Vitor.Soares@synopsys.com> wrote:
> For today the st_lsm6dsx driver support LSM6DSO and LSM6DSR sensor only in
> spi and i2c mode.
>
> The LSM6DSO and LSM6DSR are also i3c capable so lets give i3c support to
> them.
>
> Signed-off-by: Vitor Soares <vitor.soares@synopsys.com>
> Acked-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
> Changes in v4:
> Remove hw_id variable
>
> Changes in v3:
> Remove unnecessary st_lsm6dsx_i3c_data table used to hold device name
> Use st_lsm6dsx_probe new form
>
> Changes in v2:
> Add support for LSM6DSR
> Set pm_ops to st_lsm6dsx_pm_ops
>
> drivers/iio/imu/st_lsm6dsx/Kconfig | 8 +++-
> drivers/iio/imu/st_lsm6dsx/Makefile | 1 +
> drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c | 58 +++++++++++++++++++++++++++++
> 3 files changed, 66 insertions(+), 1 deletion(-)
> create mode 100644 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
>
> diff --git a/drivers/iio/imu/st_lsm6dsx/Kconfig b/drivers/iio/imu/st_lsm6dsx/Kconfig
> index 9e59297..6b5a73c 100644
> --- a/drivers/iio/imu/st_lsm6dsx/Kconfig
> +++ b/drivers/iio/imu/st_lsm6dsx/Kconfig
> @@ -1,11 +1,12 @@
>
> config IIO_ST_LSM6DSX
> tristate "ST_LSM6DSx driver for STM 6-axis IMU MEMS sensors"
> - depends on (I2C || SPI)
> + depends on (I2C || SPI || I3C)
> select IIO_BUFFER
> select IIO_KFIFO_BUF
> select IIO_ST_LSM6DSX_I2C if (I2C)
> select IIO_ST_LSM6DSX_SPI if (SPI_MASTER)
> + select IIO_ST_LSM6DSX_I3C if (I3C)
> help
> Say yes here to build support for STMicroelectronics LSM6DSx imu
> sensor. Supported devices: lsm6ds3, lsm6ds3h, lsm6dsl, lsm6dsm,
> @@ -23,3 +24,8 @@ config IIO_ST_LSM6DSX_SPI
> tristate
> depends on IIO_ST_LSM6DSX
> select REGMAP_SPI
> +
> +config IIO_ST_LSM6DSX_I3C
> + tristate
> + depends on IIO_ST_LSM6DSX
> + select REGMAP_I3C
> diff --git a/drivers/iio/imu/st_lsm6dsx/Makefile b/drivers/iio/imu/st_lsm6dsx/Makefile
> index e5f733c..c676965 100644
> --- a/drivers/iio/imu/st_lsm6dsx/Makefile
> +++ b/drivers/iio/imu/st_lsm6dsx/Makefile
> @@ -4,3 +4,4 @@ st_lsm6dsx-y := st_lsm6dsx_core.o st_lsm6dsx_buffer.o \
> obj-$(CONFIG_IIO_ST_LSM6DSX) += st_lsm6dsx.o
> obj-$(CONFIG_IIO_ST_LSM6DSX_I2C) += st_lsm6dsx_i2c.o
> obj-$(CONFIG_IIO_ST_LSM6DSX_SPI) += st_lsm6dsx_spi.o
> +obj-$(CONFIG_IIO_ST_LSM6DSX_I3C) += st_lsm6dsx_i3c.o
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
> new file mode 100644
> index 0000000..2e89524
> --- /dev/null
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
> @@ -0,0 +1,58 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2018 Synopsys, Inc. and/or its affiliates.
> + *
> + * Author: Vitor Soares <vitor.soares@synopsys.com>
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/i3c/device.h>
> +#include <linux/i3c/master.h>
> +#include <linux/slab.h>
> +#include <linux/of.h>
> +#include <linux/regmap.h>
> +
> +#include "st_lsm6dsx.h"
> +
> +static const struct i3c_device_id st_lsm6dsx_i3c_ids[] = {
> + I3C_DEVICE(0x0104, 0x006C, (void *)ST_LSM6DSO_ID),
> + I3C_DEVICE(0x0104, 0x006B, (void *)ST_LSM6DSR_ID),
I think you need an uintptr_t cast here:
I3C_DEVICE(0x0104, 0x006C, (void *)(uintptr_t)ST_LSM6DSO_ID),
I3C_DEVICE(0x0104, 0x006B, (void *)(uintptr_t)ST_LSM6DSR_ID),
otherwise gcc might complain that the integer and pointer do not have
the same size (on 64-bit architectures).
> + { /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(i3c, st_lsm6dsx_i3c_ids);
> +
> +static const struct regmap_config st_lsm6dsx_i3c_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> +};
This can be moved ...
> +
> +static int st_lsm6dsx_i3c_probe(struct i3c_device *i3cdev)
> +{
... here without the static and const qualifiers:
struct regmap_config regmap_config = {
.reg_bits = 8,
.val_bits = 8,
};
> + const struct i3c_device_id *id = i3c_device_match_id(i3cdev,
> + st_lsm6dsx_i3c_ids);
> + struct regmap *regmap;
> +
> + regmap = devm_regmap_init_i3c(i3cdev, &st_lsm6dsx_i3c_regmap_config);
> + if (IS_ERR(regmap)) {
> + dev_err(&i3cdev->dev, "Failed to register i3c regmap %d\n",
> + (int)PTR_ERR(regmap));
> + return PTR_ERR(regmap);
> + }
> +
> + return st_lsm6dsx_probe(&i3cdev->dev, 0, (int)id->data, regmap);
uintptr_t cast here.
> +}
> +
> +static struct i3c_driver st_lsm6dsx_driver = {
> + .driver = {
> + .name = "st_lsm6dsx_i3c",
> + .pm = &st_lsm6dsx_pm_ops,
> + },
> + .probe = st_lsm6dsx_i3c_probe,
> + .id_table = st_lsm6dsx_i3c_ids,
> +};
> +module_i3c_driver(st_lsm6dsx_driver);
> +
> +MODULE_AUTHOR("Vitor Soares <vitor.soares@synopsys.com>");
> +MODULE_DESCRIPTION("STMicroelectronics st_lsm6dsx i3c driver");
> +MODULE_LICENSE("GPL v2");
_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH v4 1/3] regmap: add i3c bus support
2019-07-12 15:59 ` Boris Brezillon
@ 2019-07-12 16:14 ` Vitor Soares
0 siblings, 0 replies; 17+ messages in thread
From: Vitor Soares @ 2019-07-12 16:14 UTC (permalink / raw)
To: Boris Brezillon, Vitor Soares
Cc: Joao.Pinto, rafael, linux-iio, gregkh, bbrezillon, linux-kernel,
linux-i3c, lorenzo
Hi Boris,
From: Boris Brezillon <boris.brezillon@collabora.com>
Date: Fri, Jul 12, 2019 at 16:59:15
> On Fri, 12 Jul 2019 13:53:28 +0200
> Vitor Soares <Vitor.Soares@synopsys.com> wrote:
>
> > Add basic support for i3c bus.
> > This is a simple implementation that only give support
> > for SDR Read and Write commands.
> >
>
> This patch has been applied by Mark already. Please make sure to drop
> already applied patches when submitting a new version.
I mention that in the cover letter and I kept it for reference.
Next time I will drop it.
>
> > 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 | 60 ++++++++++++++++++++++++++++++++++++++++
> > include/linux/regmap.h | 20 ++++++++++++++
> > 4 files changed, 86 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..1578fb5
> > --- /dev/null
> > +++ b/drivers/base/regmap/regmap-i3c.c
> > @@ -0,0 +1,60 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +// Copyright (c) 2018 Synopsys, Inc. and/or its affiliates.
> > +
> > +#include <linux/regmap.h>
> > +#include <linux/i3c/device.h>
> > +#include <linux/i3c/master.h>
> > +#include <linux/module.h>
> > +
> > +static int regmap_i3c_write(void *context, const void *data, size_t count)
> > +{
> > + struct device *dev = context;
> > + struct i3c_device *i3c = dev_to_i3cdev(dev);
> > + struct i3c_priv_xfer xfers[] = {
> > + {
> > + .rnw = false,
> > + .len = count,
> > + .data.out = data,
> > + },
> > + };
> > +
> > + return i3c_device_do_priv_xfers(i3c, xfers, 1);
> > +}
> > +
> > +static int regmap_i3c_read(void *context,
> > + const void *reg, size_t reg_size,
> > + void *val, size_t val_size)
> > +{
> > + struct device *dev = context;
> > + struct i3c_device *i3c = dev_to_i3cdev(dev);
> > + struct i3c_priv_xfer xfers[2];
> > +
> > + xfers[0].rnw = false;
> > + xfers[0].len = reg_size;
> > + xfers[0].data.out = reg;
> > +
> > + xfers[1].rnw = true;
> > + xfers[1].len = val_size;
> > + xfers[1].data.in = val;
> > +
> > + return i3c_device_do_priv_xfers(i3c, xfers, 2);
> > +}
> > +
> > +static struct regmap_bus regmap_i3c = {
> > + .write = regmap_i3c_write,
> > + .read = regmap_i3c_read,
> > +};
> > +
> > +struct regmap *__devm_regmap_init_i3c(struct i3c_device *i3c,
> > + const struct regmap_config *config,
> > + struct lock_class_key *lock_key,
> > + const char *lock_name)
> > +{
> > + return __devm_regmap_init(&i3c->dev, ®map_i3c, &i3c->dev, config,
> > + lock_key, lock_name);
> > +}
> > +EXPORT_SYMBOL_GPL(__devm_regmap_init_i3c);
> > +
> > +MODULE_AUTHOR("Vitor Soares <vitor.soares@synopsys.com>");
> > +MODULE_DESCRIPTION("Regmap I3C Module");
> > +MODULE_LICENSE("GPL v2");
> > 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);
_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH v4 2/3] i3c: move i3c_device_match_id to device.c and export it
2019-07-12 16:03 ` Boris Brezillon
@ 2019-07-12 16:21 ` Vitor Soares
2019-07-12 16:34 ` Boris Brezillon
0 siblings, 1 reply; 17+ messages in thread
From: Vitor Soares @ 2019-07-12 16:21 UTC (permalink / raw)
To: Boris Brezillon, Vitor Soares
Cc: Joao.Pinto, bbrezillon, linux-iio, gregkh, rafael, linux-kernel,
linux-i3c, lorenzo
Hi Boris,
From: Boris Brezillon <boris.brezillon@collabora.com>
Date: Fri, Jul 12, 2019 at 17:03:38
> On Fri, 12 Jul 2019 13:53:29 +0200
> Vitor Soares <Vitor.Soares@synopsys.com> wrote:
>
> > The i3c device driver needs the i3c_device_id table.
>
> "Some I3C device drivers need to know which entry matches the
> i3c_device object passed to the probe function"
>
> > Lets move to device.c and export it to be used.
>
> "Let's move i3c_device_match_id() to device.c and export it so it can be
> used by drivers."
>
Fix in next drop.
> >
> > Signed-off-by: Vitor Soares <vitor.soares@synopsys.com>
> > ---
> > Changes in v4:
> > None
> >
> > Changes in v3:
> > Remove i3c_get_device_id
> > Move i3c_device_match_id from drivers/i3c/master.c to drivers/i3c/device.c
> > Export i3c_device_match_id
> >
> > Changes in v2:
> > move this function to drivers/i3c/device.c
> >
> > drivers/i3c/device.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
> > drivers/i3c/master.c | 45 ---------------------------------------------
> > include/linux/i3c/device.h | 4 ++++
> > 3 files changed, 50 insertions(+), 45 deletions(-)
> >
> > diff --git a/drivers/i3c/device.c b/drivers/i3c/device.c
> > index 69cc040..383df3b 100644
> > --- a/drivers/i3c/device.c
> > +++ b/drivers/i3c/device.c
> > @@ -200,6 +200,52 @@ struct i3c_device *dev_to_i3cdev(struct device *dev)
> > }
> > EXPORT_SYMBOL_GPL(dev_to_i3cdev);
> >
>
> You're missing a kerneldoc here.
I will do that. Can you clarify why we need that?
>
> > +const struct i3c_device_id *
> > +i3c_device_match_id(struct i3c_device *i3cdev,
> > + const struct i3c_device_id *id_table)
> > +{
> > + struct i3c_device_info devinfo;
> > + const struct i3c_device_id *id;
> > +
> > + i3c_device_get_info(i3cdev, &devinfo);
> > +
> > + /*
> > + * The lower 32bits of the provisional ID is just filled with a random
> > + * value, try to match using DCR info.
> > + */
> > + if (!I3C_PID_RND_LOWER_32BITS(devinfo.pid)) {
> > + u16 manuf = I3C_PID_MANUF_ID(devinfo.pid);
> > + u16 part = I3C_PID_PART_ID(devinfo.pid);
> > + u16 ext_info = I3C_PID_EXTRA_INFO(devinfo.pid);
> > +
> > + /* First try to match by manufacturer/part ID. */
> > + for (id = id_table; id->match_flags != 0; id++) {
> > + if ((id->match_flags & I3C_MATCH_MANUF_AND_PART) !=
> > + I3C_MATCH_MANUF_AND_PART)
> > + continue;
> > +
> > + if (manuf != id->manuf_id || part != id->part_id)
> > + continue;
> > +
> > + if ((id->match_flags & I3C_MATCH_EXTRA_INFO) &&
> > + ext_info != id->extra_info)
> > + continue;
> > +
> > + return id;
> > + }
> > + }
> > +
> > + /* Fallback to DCR match. */
> > + for (id = id_table; id->match_flags != 0; id++) {
> > + if ((id->match_flags & I3C_MATCH_DCR) &&
> > + id->dcr == devinfo.dcr)
> > + return id;
> > + }
> > +
> > + return NULL;
> > +}
> > +EXPORT_SYMBOL_GPL(i3c_device_match_id);
> > +
> > /**
> > * i3c_driver_register_with_owner() - register an I3C device driver
> > *
> > diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> > index 5f4bd52..7667f84 100644
> > --- a/drivers/i3c/master.c
> > +++ b/drivers/i3c/master.c
> > @@ -270,51 +270,6 @@ static const struct device_type i3c_device_type = {
> > .uevent = i3c_device_uevent,
> > };
> >
> > -static const struct i3c_device_id *
> > -i3c_device_match_id(struct i3c_device *i3cdev,
> > - const struct i3c_device_id *id_table)
> > -{
> > - struct i3c_device_info devinfo;
> > - const struct i3c_device_id *id;
> > -
> > - i3c_device_get_info(i3cdev, &devinfo);
> > -
> > - /*
> > - * The lower 32bits of the provisional ID is just filled with a random
> > - * value, try to match using DCR info.
> > - */
> > - if (!I3C_PID_RND_LOWER_32BITS(devinfo.pid)) {
> > - u16 manuf = I3C_PID_MANUF_ID(devinfo.pid);
> > - u16 part = I3C_PID_PART_ID(devinfo.pid);
> > - u16 ext_info = I3C_PID_EXTRA_INFO(devinfo.pid);
> > -
> > - /* First try to match by manufacturer/part ID. */
> > - for (id = id_table; id->match_flags != 0; id++) {
> > - if ((id->match_flags & I3C_MATCH_MANUF_AND_PART) !=
> > - I3C_MATCH_MANUF_AND_PART)
> > - continue;
> > -
> > - if (manuf != id->manuf_id || part != id->part_id)
> > - continue;
> > -
> > - if ((id->match_flags & I3C_MATCH_EXTRA_INFO) &&
> > - ext_info != id->extra_info)
> > - continue;
> > -
> > - return id;
> > - }
> > - }
> > -
> > - /* Fallback to DCR match. */
> > - for (id = id_table; id->match_flags != 0; id++) {
> > - if ((id->match_flags & I3C_MATCH_DCR) &&
> > - id->dcr == devinfo.dcr)
> > - return id;
> > - }
> > -
> > - return NULL;
> > -}
> > -
> > static int i3c_device_match(struct device *dev, struct device_driver *drv)
> > {
> > struct i3c_device *i3cdev;
> > diff --git a/include/linux/i3c/device.h b/include/linux/i3c/device.h
> > index 5ecb055..de102e4 100644
> > --- a/include/linux/i3c/device.h
> > +++ b/include/linux/i3c/device.h
> > @@ -188,6 +188,10 @@ 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_device_match_id(struct i3c_device *i3cdev,
> > + const struct i3c_device_id *id_table);
> > +
> > static inline void i3cdev_set_drvdata(struct i3c_device *i3cdev,
> > void *data)
> > {
>
>
> _______________________________________________
> linux-i3c mailing list
> linux-i3c@lists.infradead.org
> https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.infradead.org_mailman_listinfo_linux-2Di3c&d=DwICAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=qVuU64u9x77Y0Kd0PhDK_lpxFgg6PK9PateHwjb_DY0&m=3BEF5uxfJXeA-aip7adilYENAWYp6xNoJXvidZZZpY4&s=JcBzwEcWAIoOCufxAYFd4YjCZppBQDA2-nVqfV1Cj5g&e=
Thanks for your comments.
Best regards,
Vitor Soares
_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH v4 3/3] iio: imu: st_lsm6dsx: add i3c basic support for LSM6DSO and LSM6DSR
2019-07-12 16:14 ` Boris Brezillon
@ 2019-07-12 16:28 ` Vitor Soares
2019-07-12 16:43 ` Boris Brezillon
0 siblings, 1 reply; 17+ messages in thread
From: Vitor Soares @ 2019-07-12 16:28 UTC (permalink / raw)
To: Boris Brezillon, Vitor Soares
Cc: Joao.Pinto, rafael, linux-iio, gregkh, bbrezillon, linux-kernel,
linux-i3c, lorenzo
From: Boris Brezillon <boris.brezillon@collabora.com>
Date: Fri, Jul 12, 2019 at 17:14:29
> On Fri, 12 Jul 2019 13:53:30 +0200
> Vitor Soares <Vitor.Soares@synopsys.com> wrote:
>
> > For today the st_lsm6dsx driver support LSM6DSO and LSM6DSR sensor only in
> > spi and i2c mode.
> >
> > The LSM6DSO and LSM6DSR are also i3c capable so lets give i3c support to
> > them.
> >
> > Signed-off-by: Vitor Soares <vitor.soares@synopsys.com>
> > Acked-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > ---
> > Changes in v4:
> > Remove hw_id variable
> >
> > Changes in v3:
> > Remove unnecessary st_lsm6dsx_i3c_data table used to hold device name
> > Use st_lsm6dsx_probe new form
> >
> > Changes in v2:
> > Add support for LSM6DSR
> > Set pm_ops to st_lsm6dsx_pm_ops
> >
> > drivers/iio/imu/st_lsm6dsx/Kconfig | 8 +++-
> > drivers/iio/imu/st_lsm6dsx/Makefile | 1 +
> > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c | 58 +++++++++++++++++++++++++++++
> > 3 files changed, 66 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
> >
> > diff --git a/drivers/iio/imu/st_lsm6dsx/Kconfig b/drivers/iio/imu/st_lsm6dsx/Kconfig
> > index 9e59297..6b5a73c 100644
> > --- a/drivers/iio/imu/st_lsm6dsx/Kconfig
> > +++ b/drivers/iio/imu/st_lsm6dsx/Kconfig
> > @@ -1,11 +1,12 @@
> >
> > config IIO_ST_LSM6DSX
> > tristate "ST_LSM6DSx driver for STM 6-axis IMU MEMS sensors"
> > - depends on (I2C || SPI)
> > + depends on (I2C || SPI || I3C)
> > select IIO_BUFFER
> > select IIO_KFIFO_BUF
> > select IIO_ST_LSM6DSX_I2C if (I2C)
> > select IIO_ST_LSM6DSX_SPI if (SPI_MASTER)
> > + select IIO_ST_LSM6DSX_I3C if (I3C)
> > help
> > Say yes here to build support for STMicroelectronics LSM6DSx imu
> > sensor. Supported devices: lsm6ds3, lsm6ds3h, lsm6dsl, lsm6dsm,
> > @@ -23,3 +24,8 @@ config IIO_ST_LSM6DSX_SPI
> > tristate
> > depends on IIO_ST_LSM6DSX
> > select REGMAP_SPI
> > +
> > +config IIO_ST_LSM6DSX_I3C
> > + tristate
> > + depends on IIO_ST_LSM6DSX
> > + select REGMAP_I3C
> > diff --git a/drivers/iio/imu/st_lsm6dsx/Makefile b/drivers/iio/imu/st_lsm6dsx/Makefile
> > index e5f733c..c676965 100644
> > --- a/drivers/iio/imu/st_lsm6dsx/Makefile
> > +++ b/drivers/iio/imu/st_lsm6dsx/Makefile
> > @@ -4,3 +4,4 @@ st_lsm6dsx-y := st_lsm6dsx_core.o st_lsm6dsx_buffer.o \
> > obj-$(CONFIG_IIO_ST_LSM6DSX) += st_lsm6dsx.o
> > obj-$(CONFIG_IIO_ST_LSM6DSX_I2C) += st_lsm6dsx_i2c.o
> > obj-$(CONFIG_IIO_ST_LSM6DSX_SPI) += st_lsm6dsx_spi.o
> > +obj-$(CONFIG_IIO_ST_LSM6DSX_I3C) += st_lsm6dsx_i3c.o
> > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
> > new file mode 100644
> > index 0000000..2e89524
> > --- /dev/null
> > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
> > @@ -0,0 +1,58 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2018 Synopsys, Inc. and/or its affiliates.
> > + *
> > + * Author: Vitor Soares <vitor.soares@synopsys.com>
> > + */
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/i3c/device.h>
> > +#include <linux/i3c/master.h>
> > +#include <linux/slab.h>
> > +#include <linux/of.h>
> > +#include <linux/regmap.h>
> > +
> > +#include "st_lsm6dsx.h"
> > +
> > +static const struct i3c_device_id st_lsm6dsx_i3c_ids[] = {
> > + I3C_DEVICE(0x0104, 0x006C, (void *)ST_LSM6DSO_ID),
> > + I3C_DEVICE(0x0104, 0x006B, (void *)ST_LSM6DSR_ID),
>
> I think you need an uintptr_t cast here:
>
> I3C_DEVICE(0x0104, 0x006C, (void *)(uintptr_t)ST_LSM6DSO_ID),
> I3C_DEVICE(0x0104, 0x006B, (void *)(uintptr_t)ST_LSM6DSR_ID),
>
> otherwise gcc might complain that the integer and pointer do not have
> the same size (on 64-bit architectures).
I don't understand this part. Can you provide or point some background?
>
> > + { /* sentinel */ },
> > +};
> > +MODULE_DEVICE_TABLE(i3c, st_lsm6dsx_i3c_ids);
> > +
> > +static const struct regmap_config st_lsm6dsx_i3c_regmap_config = {
> > + .reg_bits = 8,
> > + .val_bits = 8,
> > +};
>
> This can be moved ...
>
> > +
> > +static int st_lsm6dsx_i3c_probe(struct i3c_device *i3cdev)
> > +{
>
> ... here without the static and const qualifiers:
I understand that can be move to here, but I don't understand the
advantages. Can you explain?
>
> struct regmap_config regmap_config = {
> .reg_bits = 8,
> .val_bits = 8,
> };
>
> > + const struct i3c_device_id *id = i3c_device_match_id(i3cdev,
> > + st_lsm6dsx_i3c_ids);
> > + struct regmap *regmap;
> > +
> > + regmap = devm_regmap_init_i3c(i3cdev, &st_lsm6dsx_i3c_regmap_config);
> > + if (IS_ERR(regmap)) {
> > + dev_err(&i3cdev->dev, "Failed to register i3c regmap %d\n",
> > + (int)PTR_ERR(regmap));
> > + return PTR_ERR(regmap);
> > + }
> > +
> > + return st_lsm6dsx_probe(&i3cdev->dev, 0, (int)id->data, regmap);
>
> uintptr_t cast here.
>
> > +}
> > +
> > +static struct i3c_driver st_lsm6dsx_driver = {
> > + .driver = {
> > + .name = "st_lsm6dsx_i3c",
> > + .pm = &st_lsm6dsx_pm_ops,
> > + },
> > + .probe = st_lsm6dsx_i3c_probe,
> > + .id_table = st_lsm6dsx_i3c_ids,
> > +};
> > +module_i3c_driver(st_lsm6dsx_driver);
> > +
> > +MODULE_AUTHOR("Vitor Soares <vitor.soares@synopsys.com>");
> > +MODULE_DESCRIPTION("STMicroelectronics st_lsm6dsx i3c driver");
> > +MODULE_LICENSE("GPL v2");
Best regards,
Vitor Soares
_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 2/3] i3c: move i3c_device_match_id to device.c and export it
2019-07-12 16:21 ` Vitor Soares
@ 2019-07-12 16:34 ` Boris Brezillon
0 siblings, 0 replies; 17+ messages in thread
From: Boris Brezillon @ 2019-07-12 16:34 UTC (permalink / raw)
To: Vitor Soares
Cc: Joao.Pinto, bbrezillon, linux-iio, gregkh, rafael, linux-kernel,
linux-i3c, lorenzo
On Fri, 12 Jul 2019 16:21:49 +0000
Vitor Soares <Vitor.Soares@synopsys.com> wrote:
> >
> > You're missing a kerneldoc here.
>
> I will do that. Can you clarify why we need that?
>
So the function is properly documented here [1].
> >
> > > +const struct i3c_device_id *
> > > +i3c_device_match_id(struct i3c_device *i3cdev,
> > > + const struct i3c_device_id *id_table)
> > > +{
> > > + struct i3c_device_info devinfo;
> > > + const struct i3c_device_id *id;
> > > +
> > > + i3c_device_get_info(i3cdev, &devinfo);
> > > +
> > > + /*
> > > + * The lower 32bits of the provisional ID is just filled with a random
> > > + * value, try to match using DCR info.
> > > + */
> > > + if (!I3C_PID_RND_LOWER_32BITS(devinfo.pid)) {
> > > + u16 manuf = I3C_PID_MANUF_ID(devinfo.pid);
> > > + u16 part = I3C_PID_PART_ID(devinfo.pid);
> > > + u16 ext_info = I3C_PID_EXTRA_INFO(devinfo.pid);
> > > +
> > > + /* First try to match by manufacturer/part ID. */
> > > + for (id = id_table; id->match_flags != 0; id++) {
> > > + if ((id->match_flags & I3C_MATCH_MANUF_AND_PART) !=
> > > + I3C_MATCH_MANUF_AND_PART)
> > > + continue;
> > > +
> > > + if (manuf != id->manuf_id || part != id->part_id)
> > > + continue;
> > > +
> > > + if ((id->match_flags & I3C_MATCH_EXTRA_INFO) &&
> > > + ext_info != id->extra_info)
> > > + continue;
> > > +
> > > + return id;
> > > + }
> > > + }
> > > +
> > > + /* Fallback to DCR match. */
> > > + for (id = id_table; id->match_flags != 0; id++) {
> > > + if ((id->match_flags & I3C_MATCH_DCR) &&
> > > + id->dcr == devinfo.dcr)
> > > + return id;
> > > + }
> > > +
> > > + return NULL;
> > > +}
> > > +EXPORT_SYMBOL_GPL(i3c_device_match_id);
> > > +
[1]https://www.kernel.org/doc/html/latest/driver-api/i3c/device-driver-api.html
_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 3/3] iio: imu: st_lsm6dsx: add i3c basic support for LSM6DSO and LSM6DSR
2019-07-12 16:28 ` Vitor Soares
@ 2019-07-12 16:43 ` Boris Brezillon
2019-07-12 18:40 ` Vitor Soares
0 siblings, 1 reply; 17+ messages in thread
From: Boris Brezillon @ 2019-07-12 16:43 UTC (permalink / raw)
To: Vitor Soares
Cc: Joao.Pinto, rafael, linux-iio, gregkh, bbrezillon, linux-kernel,
linux-i3c, lorenzo
On Fri, 12 Jul 2019 16:28:02 +0000
Vitor Soares <Vitor.Soares@synopsys.com> wrote:
> From: Boris Brezillon <boris.brezillon@collabora.com>
> Date: Fri, Jul 12, 2019 at 17:14:29
>
> > On Fri, 12 Jul 2019 13:53:30 +0200
> > Vitor Soares <Vitor.Soares@synopsys.com> wrote:
> >
> > > For today the st_lsm6dsx driver support LSM6DSO and LSM6DSR sensor only in
> > > spi and i2c mode.
> > >
> > > The LSM6DSO and LSM6DSR are also i3c capable so lets give i3c support to
> > > them.
> > >
> > > Signed-off-by: Vitor Soares <vitor.soares@synopsys.com>
> > > Acked-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > > ---
> > > Changes in v4:
> > > Remove hw_id variable
> > >
> > > Changes in v3:
> > > Remove unnecessary st_lsm6dsx_i3c_data table used to hold device name
> > > Use st_lsm6dsx_probe new form
> > >
> > > Changes in v2:
> > > Add support for LSM6DSR
> > > Set pm_ops to st_lsm6dsx_pm_ops
> > >
> > > drivers/iio/imu/st_lsm6dsx/Kconfig | 8 +++-
> > > drivers/iio/imu/st_lsm6dsx/Makefile | 1 +
> > > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c | 58 +++++++++++++++++++++++++++++
> > > 3 files changed, 66 insertions(+), 1 deletion(-)
> > > create mode 100644 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
> > >
> > > diff --git a/drivers/iio/imu/st_lsm6dsx/Kconfig b/drivers/iio/imu/st_lsm6dsx/Kconfig
> > > index 9e59297..6b5a73c 100644
> > > --- a/drivers/iio/imu/st_lsm6dsx/Kconfig
> > > +++ b/drivers/iio/imu/st_lsm6dsx/Kconfig
> > > @@ -1,11 +1,12 @@
> > >
> > > config IIO_ST_LSM6DSX
> > > tristate "ST_LSM6DSx driver for STM 6-axis IMU MEMS sensors"
> > > - depends on (I2C || SPI)
> > > + depends on (I2C || SPI || I3C)
> > > select IIO_BUFFER
> > > select IIO_KFIFO_BUF
> > > select IIO_ST_LSM6DSX_I2C if (I2C)
> > > select IIO_ST_LSM6DSX_SPI if (SPI_MASTER)
> > > + select IIO_ST_LSM6DSX_I3C if (I3C)
> > > help
> > > Say yes here to build support for STMicroelectronics LSM6DSx imu
> > > sensor. Supported devices: lsm6ds3, lsm6ds3h, lsm6dsl, lsm6dsm,
> > > @@ -23,3 +24,8 @@ config IIO_ST_LSM6DSX_SPI
> > > tristate
> > > depends on IIO_ST_LSM6DSX
> > > select REGMAP_SPI
> > > +
> > > +config IIO_ST_LSM6DSX_I3C
> > > + tristate
> > > + depends on IIO_ST_LSM6DSX
> > > + select REGMAP_I3C
> > > diff --git a/drivers/iio/imu/st_lsm6dsx/Makefile b/drivers/iio/imu/st_lsm6dsx/Makefile
> > > index e5f733c..c676965 100644
> > > --- a/drivers/iio/imu/st_lsm6dsx/Makefile
> > > +++ b/drivers/iio/imu/st_lsm6dsx/Makefile
> > > @@ -4,3 +4,4 @@ st_lsm6dsx-y := st_lsm6dsx_core.o st_lsm6dsx_buffer.o \
> > > obj-$(CONFIG_IIO_ST_LSM6DSX) += st_lsm6dsx.o
> > > obj-$(CONFIG_IIO_ST_LSM6DSX_I2C) += st_lsm6dsx_i2c.o
> > > obj-$(CONFIG_IIO_ST_LSM6DSX_SPI) += st_lsm6dsx_spi.o
> > > +obj-$(CONFIG_IIO_ST_LSM6DSX_I3C) += st_lsm6dsx_i3c.o
> > > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
> > > new file mode 100644
> > > index 0000000..2e89524
> > > --- /dev/null
> > > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
> > > @@ -0,0 +1,58 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Copyright (c) 2018 Synopsys, Inc. and/or its affiliates.
> > > + *
> > > + * Author: Vitor Soares <vitor.soares@synopsys.com>
> > > + */
> > > +
> > > +#include <linux/kernel.h>
> > > +#include <linux/module.h>
> > > +#include <linux/i3c/device.h>
> > > +#include <linux/i3c/master.h>
> > > +#include <linux/slab.h>
> > > +#include <linux/of.h>
> > > +#include <linux/regmap.h>
> > > +
> > > +#include "st_lsm6dsx.h"
> > > +
> > > +static const struct i3c_device_id st_lsm6dsx_i3c_ids[] = {
> > > + I3C_DEVICE(0x0104, 0x006C, (void *)ST_LSM6DSO_ID),
> > > + I3C_DEVICE(0x0104, 0x006B, (void *)ST_LSM6DSR_ID),
> >
> > I think you need an uintptr_t cast here:
> >
> > I3C_DEVICE(0x0104, 0x006C, (void *)(uintptr_t)ST_LSM6DSO_ID),
> > I3C_DEVICE(0x0104, 0x006B, (void *)(uintptr_t)ST_LSM6DSR_ID),
> >
> > otherwise gcc might complain that the integer and pointer do not have
> > the same size (on 64-bit architectures).
>
> I don't understand this part. Can you provide or point some background?
If you don't do that you'll get the following warning:
warning: cast to 'void *' from smaller integer type 'int' [-Wint-to-void-pointer-cast]
>
> >
> > > + { /* sentinel */ },
> > > +};
> > > +MODULE_DEVICE_TABLE(i3c, st_lsm6dsx_i3c_ids);
> > > +
> > > +static const struct regmap_config st_lsm6dsx_i3c_regmap_config = {
> > > + .reg_bits = 8,
> > > + .val_bits = 8,
> > > +};
> >
> > This can be moved ...
> >
> > > +
> > > +static int st_lsm6dsx_i3c_probe(struct i3c_device *i3cdev)
> > > +{
> >
> > ... here without the static and const qualifiers:
>
> I understand that can be move to here, but I don't understand the
> advantages. Can you explain?
It reduces the variable scope (this variable is only needed in the
probe path) and avoids consuming space in the .bss section, though the
second point is not so important.
_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH v4 3/3] iio: imu: st_lsm6dsx: add i3c basic support for LSM6DSO and LSM6DSR
2019-07-12 16:43 ` Boris Brezillon
@ 2019-07-12 18:40 ` Vitor Soares
2019-07-12 20:03 ` Boris Brezillon
0 siblings, 1 reply; 17+ messages in thread
From: Vitor Soares @ 2019-07-12 18:40 UTC (permalink / raw)
To: Boris Brezillon, Vitor Soares
Cc: Joao.Pinto, rafael, linux-iio, gregkh, bbrezillon, linux-kernel,
linux-i3c, lorenzo
From: Boris Brezillon <boris.brezillon@collabora.com>
Date: Fri, Jul 12, 2019 at 17:43:23
> On Fri, 12 Jul 2019 16:28:02 +0000
> Vitor Soares <Vitor.Soares@synopsys.com> wrote:
>
> > From: Boris Brezillon <boris.brezillon@collabora.com>
> > Date: Fri, Jul 12, 2019 at 17:14:29
> >
> > > On Fri, 12 Jul 2019 13:53:30 +0200
> > > Vitor Soares <Vitor.Soares@synopsys.com> wrote:
> > >
> > > > For today the st_lsm6dsx driver support LSM6DSO and LSM6DSR sensor only in
> > > > spi and i2c mode.
> > > >
> > > > The LSM6DSO and LSM6DSR are also i3c capable so lets give i3c support to
> > > > them.
> > > >
> > > > Signed-off-by: Vitor Soares <vitor.soares@synopsys.com>
> > > > Acked-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > > > ---
> > > > Changes in v4:
> > > > Remove hw_id variable
> > > >
> > > > Changes in v3:
> > > > Remove unnecessary st_lsm6dsx_i3c_data table used to hold device name
> > > > Use st_lsm6dsx_probe new form
> > > >
> > > > Changes in v2:
> > > > Add support for LSM6DSR
> > > > Set pm_ops to st_lsm6dsx_pm_ops
> > > >
> > > > drivers/iio/imu/st_lsm6dsx/Kconfig | 8 +++-
> > > > drivers/iio/imu/st_lsm6dsx/Makefile | 1 +
> > > > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c | 58 +++++++++++++++++++++++++++++
> > > > 3 files changed, 66 insertions(+), 1 deletion(-)
> > > > create mode 100644 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
> > > >
> > > > diff --git a/drivers/iio/imu/st_lsm6dsx/Kconfig b/drivers/iio/imu/st_lsm6dsx/Kconfig
> > > > index 9e59297..6b5a73c 100644
> > > > --- a/drivers/iio/imu/st_lsm6dsx/Kconfig
> > > > +++ b/drivers/iio/imu/st_lsm6dsx/Kconfig
> > > > @@ -1,11 +1,12 @@
> > > >
> > > > config IIO_ST_LSM6DSX
> > > > tristate "ST_LSM6DSx driver for STM 6-axis IMU MEMS sensors"
> > > > - depends on (I2C || SPI)
> > > > + depends on (I2C || SPI || I3C)
> > > > select IIO_BUFFER
> > > > select IIO_KFIFO_BUF
> > > > select IIO_ST_LSM6DSX_I2C if (I2C)
> > > > select IIO_ST_LSM6DSX_SPI if (SPI_MASTER)
> > > > + select IIO_ST_LSM6DSX_I3C if (I3C)
> > > > help
> > > > Say yes here to build support for STMicroelectronics LSM6DSx imu
> > > > sensor. Supported devices: lsm6ds3, lsm6ds3h, lsm6dsl, lsm6dsm,
> > > > @@ -23,3 +24,8 @@ config IIO_ST_LSM6DSX_SPI
> > > > tristate
> > > > depends on IIO_ST_LSM6DSX
> > > > select REGMAP_SPI
> > > > +
> > > > +config IIO_ST_LSM6DSX_I3C
> > > > + tristate
> > > > + depends on IIO_ST_LSM6DSX
> > > > + select REGMAP_I3C
> > > > diff --git a/drivers/iio/imu/st_lsm6dsx/Makefile b/drivers/iio/imu/st_lsm6dsx/Makefile
> > > > index e5f733c..c676965 100644
> > > > --- a/drivers/iio/imu/st_lsm6dsx/Makefile
> > > > +++ b/drivers/iio/imu/st_lsm6dsx/Makefile
> > > > @@ -4,3 +4,4 @@ st_lsm6dsx-y := st_lsm6dsx_core.o st_lsm6dsx_buffer.o \
> > > > obj-$(CONFIG_IIO_ST_LSM6DSX) += st_lsm6dsx.o
> > > > obj-$(CONFIG_IIO_ST_LSM6DSX_I2C) += st_lsm6dsx_i2c.o
> > > > obj-$(CONFIG_IIO_ST_LSM6DSX_SPI) += st_lsm6dsx_spi.o
> > > > +obj-$(CONFIG_IIO_ST_LSM6DSX_I3C) += st_lsm6dsx_i3c.o
> > > > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
> > > > new file mode 100644
> > > > index 0000000..2e89524
> > > > --- /dev/null
> > > > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
> > > > @@ -0,0 +1,58 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +/*
> > > > + * Copyright (c) 2018 Synopsys, Inc. and/or its affiliates.
> > > > + *
> > > > + * Author: Vitor Soares <vitor.soares@synopsys.com>
> > > > + */
> > > > +
> > > > +#include <linux/kernel.h>
> > > > +#include <linux/module.h>
> > > > +#include <linux/i3c/device.h>
> > > > +#include <linux/i3c/master.h>
> > > > +#include <linux/slab.h>
> > > > +#include <linux/of.h>
> > > > +#include <linux/regmap.h>
> > > > +
> > > > +#include "st_lsm6dsx.h"
> > > > +
> > > > +static const struct i3c_device_id st_lsm6dsx_i3c_ids[] = {
> > > > + I3C_DEVICE(0x0104, 0x006C, (void *)ST_LSM6DSO_ID),
> > > > + I3C_DEVICE(0x0104, 0x006B, (void *)ST_LSM6DSR_ID),
> > >
> > > I think you need an uintptr_t cast here:
> > >
> > > I3C_DEVICE(0x0104, 0x006C, (void *)(uintptr_t)ST_LSM6DSO_ID),
> > > I3C_DEVICE(0x0104, 0x006B, (void *)(uintptr_t)ST_LSM6DSR_ID),
> > >
> > > otherwise gcc might complain that the integer and pointer do not have
> > > the same size (on 64-bit architectures).
> >
> > I don't understand this part. Can you provide or point some background?
>
> If you don't do that you'll get the following warning:
>
> warning: cast to 'void *' from smaller integer type 'int' [-Wint-to-void-pointer-cast]
I don't get the warning during compilation. Is there any flag to enable
or so?
>
> >
> > >
> > > > + { /* sentinel */ },
> > > > +};
> > > > +MODULE_DEVICE_TABLE(i3c, st_lsm6dsx_i3c_ids);
> > > > +
> > > > +static const struct regmap_config st_lsm6dsx_i3c_regmap_config = {
> > > > + .reg_bits = 8,
> > > > + .val_bits = 8,
> > > > +};
> > >
> > > This can be moved ...
> > >
> > > > +
> > > > +static int st_lsm6dsx_i3c_probe(struct i3c_device *i3cdev)
> > > > +{
> > >
> > > ... here without the static and const qualifiers:
> >
> > I understand that can be move to here, but I don't understand the
> > advantages. Can you explain?
>
> It reduces the variable scope (this variable is only needed in the
> probe path) and avoids consuming space in the .bss section, though the
> second point is not so important.
Thanks,
Vitor Soares
_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 3/3] iio: imu: st_lsm6dsx: add i3c basic support for LSM6DSO and LSM6DSR
2019-07-12 18:40 ` Vitor Soares
@ 2019-07-12 20:03 ` Boris Brezillon
2019-07-16 13:22 ` Vitor Soares
0 siblings, 1 reply; 17+ messages in thread
From: Boris Brezillon @ 2019-07-12 20:03 UTC (permalink / raw)
To: Vitor Soares
Cc: Joao.Pinto, rafael, linux-iio, gregkh, bbrezillon, linux-kernel,
linux-i3c, lorenzo
On Fri, 12 Jul 2019 18:40:14 +0000
Vitor Soares <Vitor.Soares@synopsys.com> wrote:
> From: Boris Brezillon <boris.brezillon@collabora.com>
> Date: Fri, Jul 12, 2019 at 17:43:23
>
> > On Fri, 12 Jul 2019 16:28:02 +0000
> > Vitor Soares <Vitor.Soares@synopsys.com> wrote:
> >
> > > From: Boris Brezillon <boris.brezillon@collabora.com>
> > > Date: Fri, Jul 12, 2019 at 17:14:29
> > >
> > > > On Fri, 12 Jul 2019 13:53:30 +0200
> > > > Vitor Soares <Vitor.Soares@synopsys.com> wrote:
> > > >
> > > > > For today the st_lsm6dsx driver support LSM6DSO and LSM6DSR sensor only in
> > > > > spi and i2c mode.
> > > > >
> > > > > The LSM6DSO and LSM6DSR are also i3c capable so lets give i3c support to
> > > > > them.
> > > > >
> > > > > Signed-off-by: Vitor Soares <vitor.soares@synopsys.com>
> > > > > Acked-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > > > > ---
> > > > > Changes in v4:
> > > > > Remove hw_id variable
> > > > >
> > > > > Changes in v3:
> > > > > Remove unnecessary st_lsm6dsx_i3c_data table used to hold device name
> > > > > Use st_lsm6dsx_probe new form
> > > > >
> > > > > Changes in v2:
> > > > > Add support for LSM6DSR
> > > > > Set pm_ops to st_lsm6dsx_pm_ops
> > > > >
> > > > > drivers/iio/imu/st_lsm6dsx/Kconfig | 8 +++-
> > > > > drivers/iio/imu/st_lsm6dsx/Makefile | 1 +
> > > > > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c | 58 +++++++++++++++++++++++++++++
> > > > > 3 files changed, 66 insertions(+), 1 deletion(-)
> > > > > create mode 100644 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
> > > > >
> > > > > diff --git a/drivers/iio/imu/st_lsm6dsx/Kconfig b/drivers/iio/imu/st_lsm6dsx/Kconfig
> > > > > index 9e59297..6b5a73c 100644
> > > > > --- a/drivers/iio/imu/st_lsm6dsx/Kconfig
> > > > > +++ b/drivers/iio/imu/st_lsm6dsx/Kconfig
> > > > > @@ -1,11 +1,12 @@
> > > > >
> > > > > config IIO_ST_LSM6DSX
> > > > > tristate "ST_LSM6DSx driver for STM 6-axis IMU MEMS sensors"
> > > > > - depends on (I2C || SPI)
> > > > > + depends on (I2C || SPI || I3C)
> > > > > select IIO_BUFFER
> > > > > select IIO_KFIFO_BUF
> > > > > select IIO_ST_LSM6DSX_I2C if (I2C)
> > > > > select IIO_ST_LSM6DSX_SPI if (SPI_MASTER)
> > > > > + select IIO_ST_LSM6DSX_I3C if (I3C)
> > > > > help
> > > > > Say yes here to build support for STMicroelectronics LSM6DSx imu
> > > > > sensor. Supported devices: lsm6ds3, lsm6ds3h, lsm6dsl, lsm6dsm,
> > > > > @@ -23,3 +24,8 @@ config IIO_ST_LSM6DSX_SPI
> > > > > tristate
> > > > > depends on IIO_ST_LSM6DSX
> > > > > select REGMAP_SPI
> > > > > +
> > > > > +config IIO_ST_LSM6DSX_I3C
> > > > > + tristate
> > > > > + depends on IIO_ST_LSM6DSX
> > > > > + select REGMAP_I3C
> > > > > diff --git a/drivers/iio/imu/st_lsm6dsx/Makefile b/drivers/iio/imu/st_lsm6dsx/Makefile
> > > > > index e5f733c..c676965 100644
> > > > > --- a/drivers/iio/imu/st_lsm6dsx/Makefile
> > > > > +++ b/drivers/iio/imu/st_lsm6dsx/Makefile
> > > > > @@ -4,3 +4,4 @@ st_lsm6dsx-y := st_lsm6dsx_core.o st_lsm6dsx_buffer.o \
> > > > > obj-$(CONFIG_IIO_ST_LSM6DSX) += st_lsm6dsx.o
> > > > > obj-$(CONFIG_IIO_ST_LSM6DSX_I2C) += st_lsm6dsx_i2c.o
> > > > > obj-$(CONFIG_IIO_ST_LSM6DSX_SPI) += st_lsm6dsx_spi.o
> > > > > +obj-$(CONFIG_IIO_ST_LSM6DSX_I3C) += st_lsm6dsx_i3c.o
> > > > > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
> > > > > new file mode 100644
> > > > > index 0000000..2e89524
> > > > > --- /dev/null
> > > > > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
> > > > > @@ -0,0 +1,58 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0
> > > > > +/*
> > > > > + * Copyright (c) 2018 Synopsys, Inc. and/or its affiliates.
> > > > > + *
> > > > > + * Author: Vitor Soares <vitor.soares@synopsys.com>
> > > > > + */
> > > > > +
> > > > > +#include <linux/kernel.h>
> > > > > +#include <linux/module.h>
> > > > > +#include <linux/i3c/device.h>
> > > > > +#include <linux/i3c/master.h>
> > > > > +#include <linux/slab.h>
> > > > > +#include <linux/of.h>
> > > > > +#include <linux/regmap.h>
> > > > > +
> > > > > +#include "st_lsm6dsx.h"
> > > > > +
> > > > > +static const struct i3c_device_id st_lsm6dsx_i3c_ids[] = {
> > > > > + I3C_DEVICE(0x0104, 0x006C, (void *)ST_LSM6DSO_ID),
> > > > > + I3C_DEVICE(0x0104, 0x006B, (void *)ST_LSM6DSR_ID),
> > > >
> > > > I think you need an uintptr_t cast here:
> > > >
> > > > I3C_DEVICE(0x0104, 0x006C, (void *)(uintptr_t)ST_LSM6DSO_ID),
> > > > I3C_DEVICE(0x0104, 0x006B, (void *)(uintptr_t)ST_LSM6DSR_ID),
> > > >
> > > > otherwise gcc might complain that the integer and pointer do not have
> > > > the same size (on 64-bit architectures).
> > >
> > > I don't understand this part. Can you provide or point some background?
> >
> > If you don't do that you'll get the following warning:
> >
> > warning: cast to 'void *' from smaller integer type 'int' [-Wint-to-void-pointer-cast]
>
> I don't get the warning during compilation. Is there any flag to enable
> or so?
Nope, nothing specific to enable, just enable this driver on an arm64
config. Note that that gcc doesn't seem to complain about this
int -> void * cast (there's probably some kind of auto-promotion to
pointer size), but it does complains about the following void * -> int
cast:
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c: In function ‘st_lsm6dsx_i3c_probe’:
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c:43:43: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
43 | return st_lsm6dsx_probe(&i3cdev->dev, 0, (int)id->data, regmap);
|
_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH v4 3/3] iio: imu: st_lsm6dsx: add i3c basic support for LSM6DSO and LSM6DSR
2019-07-12 20:03 ` Boris Brezillon
@ 2019-07-16 13:22 ` Vitor Soares
2019-07-16 13:40 ` Boris Brezillon
0 siblings, 1 reply; 17+ messages in thread
From: Vitor Soares @ 2019-07-16 13:22 UTC (permalink / raw)
To: Boris Brezillon, Vitor Soares
Cc: Joao.Pinto, rafael, linux-iio, gregkh, bbrezillon, linux-kernel,
linux-i3c, lorenzo
Hi Boris,
From: Boris Brezillon <boris.brezillon@collabora.com>
Date: Fri, Jul 12, 2019 at 21:03:20
> On Fri, 12 Jul 2019 18:40:14 +0000
> Vitor Soares <Vitor.Soares@synopsys.com> wrote:
>
> > From: Boris Brezillon <boris.brezillon@collabora.com>
> > Date: Fri, Jul 12, 2019 at 17:43:23
> >
> > > On Fri, 12 Jul 2019 16:28:02 +0000
> > > Vitor Soares <Vitor.Soares@synopsys.com> wrote:
> > >
> > > > From: Boris Brezillon <boris.brezillon@collabora.com>
> > > > Date: Fri, Jul 12, 2019 at 17:14:29
> > > >
> > > > > On Fri, 12 Jul 2019 13:53:30 +0200
> > > > > Vitor Soares <Vitor.Soares@synopsys.com> wrote:
> > > > >
> > > > > > For today the st_lsm6dsx driver support LSM6DSO and LSM6DSR sensor only in
> > > > > > spi and i2c mode.
> > > > > >
> > > > > > The LSM6DSO and LSM6DSR are also i3c capable so lets give i3c support to
> > > > > > them.
> > > > > >
> > > > > > Signed-off-by: Vitor Soares <vitor.soares@synopsys.com>
> > > > > > Acked-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > > > > > ---
> > > > > > Changes in v4:
> > > > > > Remove hw_id variable
> > > > > >
> > > > > > Changes in v3:
> > > > > > Remove unnecessary st_lsm6dsx_i3c_data table used to hold device name
> > > > > > Use st_lsm6dsx_probe new form
> > > > > >
> > > > > > Changes in v2:
> > > > > > Add support for LSM6DSR
> > > > > > Set pm_ops to st_lsm6dsx_pm_ops
> > > > > >
> > > > > > drivers/iio/imu/st_lsm6dsx/Kconfig | 8 +++-
> > > > > > drivers/iio/imu/st_lsm6dsx/Makefile | 1 +
> > > > > > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c | 58 +++++++++++++++++++++++++++++
> > > > > > 3 files changed, 66 insertions(+), 1 deletion(-)
> > > > > > create mode 100644 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
> > > > > >
> > > > > > diff --git a/drivers/iio/imu/st_lsm6dsx/Kconfig b/drivers/iio/imu/st_lsm6dsx/Kconfig
> > > > > > index 9e59297..6b5a73c 100644
> > > > > > --- a/drivers/iio/imu/st_lsm6dsx/Kconfig
> > > > > > +++ b/drivers/iio/imu/st_lsm6dsx/Kconfig
> > > > > > @@ -1,11 +1,12 @@
> > > > > >
> > > > > > config IIO_ST_LSM6DSX
> > > > > > tristate "ST_LSM6DSx driver for STM 6-axis IMU MEMS sensors"
> > > > > > - depends on (I2C || SPI)
> > > > > > + depends on (I2C || SPI || I3C)
> > > > > > select IIO_BUFFER
> > > > > > select IIO_KFIFO_BUF
> > > > > > select IIO_ST_LSM6DSX_I2C if (I2C)
> > > > > > select IIO_ST_LSM6DSX_SPI if (SPI_MASTER)
> > > > > > + select IIO_ST_LSM6DSX_I3C if (I3C)
> > > > > > help
> > > > > > Say yes here to build support for STMicroelectronics LSM6DSx imu
> > > > > > sensor. Supported devices: lsm6ds3, lsm6ds3h, lsm6dsl, lsm6dsm,
> > > > > > @@ -23,3 +24,8 @@ config IIO_ST_LSM6DSX_SPI
> > > > > > tristate
> > > > > > depends on IIO_ST_LSM6DSX
> > > > > > select REGMAP_SPI
> > > > > > +
> > > > > > +config IIO_ST_LSM6DSX_I3C
> > > > > > + tristate
> > > > > > + depends on IIO_ST_LSM6DSX
> > > > > > + select REGMAP_I3C
> > > > > > diff --git a/drivers/iio/imu/st_lsm6dsx/Makefile b/drivers/iio/imu/st_lsm6dsx/Makefile
> > > > > > index e5f733c..c676965 100644
> > > > > > --- a/drivers/iio/imu/st_lsm6dsx/Makefile
> > > > > > +++ b/drivers/iio/imu/st_lsm6dsx/Makefile
> > > > > > @@ -4,3 +4,4 @@ st_lsm6dsx-y := st_lsm6dsx_core.o st_lsm6dsx_buffer.o \
> > > > > > obj-$(CONFIG_IIO_ST_LSM6DSX) += st_lsm6dsx.o
> > > > > > obj-$(CONFIG_IIO_ST_LSM6DSX_I2C) += st_lsm6dsx_i2c.o
> > > > > > obj-$(CONFIG_IIO_ST_LSM6DSX_SPI) += st_lsm6dsx_spi.o
> > > > > > +obj-$(CONFIG_IIO_ST_LSM6DSX_I3C) += st_lsm6dsx_i3c.o
> > > > > > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
> > > > > > new file mode 100644
> > > > > > index 0000000..2e89524
> > > > > > --- /dev/null
> > > > > > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
> > > > > > @@ -0,0 +1,58 @@
> > > > > > +// SPDX-License-Identifier: GPL-2.0
> > > > > > +/*
> > > > > > + * Copyright (c) 2018 Synopsys, Inc. and/or its affiliates.
> > > > > > + *
> > > > > > + * Author: Vitor Soares <vitor.soares@synopsys.com>
> > > > > > + */
> > > > > > +
> > > > > > +#include <linux/kernel.h>
> > > > > > +#include <linux/module.h>
> > > > > > +#include <linux/i3c/device.h>
> > > > > > +#include <linux/i3c/master.h>
> > > > > > +#include <linux/slab.h>
> > > > > > +#include <linux/of.h>
> > > > > > +#include <linux/regmap.h>
> > > > > > +
> > > > > > +#include "st_lsm6dsx.h"
> > > > > > +
> > > > > > +static const struct i3c_device_id st_lsm6dsx_i3c_ids[] = {
> > > > > > + I3C_DEVICE(0x0104, 0x006C, (void *)ST_LSM6DSO_ID),
> > > > > > + I3C_DEVICE(0x0104, 0x006B, (void *)ST_LSM6DSR_ID),
> > > > >
> > > > > I think you need an uintptr_t cast here:
> > > > >
> > > > > I3C_DEVICE(0x0104, 0x006C, (void *)(uintptr_t)ST_LSM6DSO_ID),
> > > > > I3C_DEVICE(0x0104, 0x006B, (void *)(uintptr_t)ST_LSM6DSR_ID),
> > > > >
> > > > > otherwise gcc might complain that the integer and pointer do not have
> > > > > the same size (on 64-bit architectures).
> > > >
> > > > I don't understand this part. Can you provide or point some background?
> > >
> > > If you don't do that you'll get the following warning:
> > >
> > > warning: cast to 'void *' from smaller integer type 'int' [-Wint-to-void-pointer-cast]
> >
> > I don't get the warning during compilation. Is there any flag to enable
> > or so?
>
> Nope, nothing specific to enable, just enable this driver on an arm64
> config. Note that that gcc doesn't seem to complain about this
> int -> void * cast (there's probably some kind of auto-promotion to
> pointer size), but it does complains about the following void * -> int
> cast:
>
> drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c: In function ‘st_lsm6dsx_i3c_probe’:
> drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c:43:43: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
> 43 | return st_lsm6dsx_probe(&i3cdev->dev, 0, (int)id->data, regmap);
> |
I fixed the warning by changing:
st_lsm6dsx_probe(&i3cdev->dev, 0, (int)id->data, regmap);
to
st_lsm6dsx_probe(&i3cdev->dev, 0, (int)(uintptr_t)id->data, regmap);
But I wonder if it isn't more save to change the following too:
I3C_DEVICE(0x0104, 0x006C, (void *)(uintptr_t)ST_LSM6DSO_ID),
I3C_DEVICE(0x0104, 0x006B, (void *)(uintptr_t)ST_LSM6DSR_ID),
What do you think?
Best regards,
Vitor Soares
_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 3/3] iio: imu: st_lsm6dsx: add i3c basic support for LSM6DSO and LSM6DSR
2019-07-16 13:22 ` Vitor Soares
@ 2019-07-16 13:40 ` Boris Brezillon
0 siblings, 0 replies; 17+ messages in thread
From: Boris Brezillon @ 2019-07-16 13:40 UTC (permalink / raw)
To: Vitor Soares
Cc: Joao.Pinto, rafael, linux-iio, gregkh, bbrezillon, linux-kernel,
linux-i3c, lorenzo
On Tue, 16 Jul 2019 13:22:25 +0000
Vitor Soares <Vitor.Soares@synopsys.com> wrote:
> Hi Boris,
>
> From: Boris Brezillon <boris.brezillon@collabora.com>
> Date: Fri, Jul 12, 2019 at 21:03:20
>
> > On Fri, 12 Jul 2019 18:40:14 +0000
> > Vitor Soares <Vitor.Soares@synopsys.com> wrote:
> >
> > > From: Boris Brezillon <boris.brezillon@collabora.com>
> > > Date: Fri, Jul 12, 2019 at 17:43:23
> > >
> > > > On Fri, 12 Jul 2019 16:28:02 +0000
> > > > Vitor Soares <Vitor.Soares@synopsys.com> wrote:
> > > >
> > > > > From: Boris Brezillon <boris.brezillon@collabora.com>
> > > > > Date: Fri, Jul 12, 2019 at 17:14:29
> > > > >
> > > > > > On Fri, 12 Jul 2019 13:53:30 +0200
> > > > > > Vitor Soares <Vitor.Soares@synopsys.com> wrote:
> > > > > >
> > > > > > > For today the st_lsm6dsx driver support LSM6DSO and LSM6DSR sensor only in
> > > > > > > spi and i2c mode.
> > > > > > >
> > > > > > > The LSM6DSO and LSM6DSR are also i3c capable so lets give i3c support to
> > > > > > > them.
> > > > > > >
> > > > > > > Signed-off-by: Vitor Soares <vitor.soares@synopsys.com>
> > > > > > > Acked-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > > > > > > ---
> > > > > > > Changes in v4:
> > > > > > > Remove hw_id variable
> > > > > > >
> > > > > > > Changes in v3:
> > > > > > > Remove unnecessary st_lsm6dsx_i3c_data table used to hold device name
> > > > > > > Use st_lsm6dsx_probe new form
> > > > > > >
> > > > > > > Changes in v2:
> > > > > > > Add support for LSM6DSR
> > > > > > > Set pm_ops to st_lsm6dsx_pm_ops
> > > > > > >
> > > > > > > drivers/iio/imu/st_lsm6dsx/Kconfig | 8 +++-
> > > > > > > drivers/iio/imu/st_lsm6dsx/Makefile | 1 +
> > > > > > > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c | 58 +++++++++++++++++++++++++++++
> > > > > > > 3 files changed, 66 insertions(+), 1 deletion(-)
> > > > > > > create mode 100644 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
> > > > > > >
> > > > > > > diff --git a/drivers/iio/imu/st_lsm6dsx/Kconfig b/drivers/iio/imu/st_lsm6dsx/Kconfig
> > > > > > > index 9e59297..6b5a73c 100644
> > > > > > > --- a/drivers/iio/imu/st_lsm6dsx/Kconfig
> > > > > > > +++ b/drivers/iio/imu/st_lsm6dsx/Kconfig
> > > > > > > @@ -1,11 +1,12 @@
> > > > > > >
> > > > > > > config IIO_ST_LSM6DSX
> > > > > > > tristate "ST_LSM6DSx driver for STM 6-axis IMU MEMS sensors"
> > > > > > > - depends on (I2C || SPI)
> > > > > > > + depends on (I2C || SPI || I3C)
> > > > > > > select IIO_BUFFER
> > > > > > > select IIO_KFIFO_BUF
> > > > > > > select IIO_ST_LSM6DSX_I2C if (I2C)
> > > > > > > select IIO_ST_LSM6DSX_SPI if (SPI_MASTER)
> > > > > > > + select IIO_ST_LSM6DSX_I3C if (I3C)
> > > > > > > help
> > > > > > > Say yes here to build support for STMicroelectronics LSM6DSx imu
> > > > > > > sensor. Supported devices: lsm6ds3, lsm6ds3h, lsm6dsl, lsm6dsm,
> > > > > > > @@ -23,3 +24,8 @@ config IIO_ST_LSM6DSX_SPI
> > > > > > > tristate
> > > > > > > depends on IIO_ST_LSM6DSX
> > > > > > > select REGMAP_SPI
> > > > > > > +
> > > > > > > +config IIO_ST_LSM6DSX_I3C
> > > > > > > + tristate
> > > > > > > + depends on IIO_ST_LSM6DSX
> > > > > > > + select REGMAP_I3C
> > > > > > > diff --git a/drivers/iio/imu/st_lsm6dsx/Makefile b/drivers/iio/imu/st_lsm6dsx/Makefile
> > > > > > > index e5f733c..c676965 100644
> > > > > > > --- a/drivers/iio/imu/st_lsm6dsx/Makefile
> > > > > > > +++ b/drivers/iio/imu/st_lsm6dsx/Makefile
> > > > > > > @@ -4,3 +4,4 @@ st_lsm6dsx-y := st_lsm6dsx_core.o st_lsm6dsx_buffer.o \
> > > > > > > obj-$(CONFIG_IIO_ST_LSM6DSX) += st_lsm6dsx.o
> > > > > > > obj-$(CONFIG_IIO_ST_LSM6DSX_I2C) += st_lsm6dsx_i2c.o
> > > > > > > obj-$(CONFIG_IIO_ST_LSM6DSX_SPI) += st_lsm6dsx_spi.o
> > > > > > > +obj-$(CONFIG_IIO_ST_LSM6DSX_I3C) += st_lsm6dsx_i3c.o
> > > > > > > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
> > > > > > > new file mode 100644
> > > > > > > index 0000000..2e89524
> > > > > > > --- /dev/null
> > > > > > > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
> > > > > > > @@ -0,0 +1,58 @@
> > > > > > > +// SPDX-License-Identifier: GPL-2.0
> > > > > > > +/*
> > > > > > > + * Copyright (c) 2018 Synopsys, Inc. and/or its affiliates.
> > > > > > > + *
> > > > > > > + * Author: Vitor Soares <vitor.soares@synopsys.com>
> > > > > > > + */
> > > > > > > +
> > > > > > > +#include <linux/kernel.h>
> > > > > > > +#include <linux/module.h>
> > > > > > > +#include <linux/i3c/device.h>
> > > > > > > +#include <linux/i3c/master.h>
> > > > > > > +#include <linux/slab.h>
> > > > > > > +#include <linux/of.h>
> > > > > > > +#include <linux/regmap.h>
> > > > > > > +
> > > > > > > +#include "st_lsm6dsx.h"
> > > > > > > +
> > > > > > > +static const struct i3c_device_id st_lsm6dsx_i3c_ids[] = {
> > > > > > > + I3C_DEVICE(0x0104, 0x006C, (void *)ST_LSM6DSO_ID),
> > > > > > > + I3C_DEVICE(0x0104, 0x006B, (void *)ST_LSM6DSR_ID),
> > > > > >
> > > > > > I think you need an uintptr_t cast here:
> > > > > >
> > > > > > I3C_DEVICE(0x0104, 0x006C, (void *)(uintptr_t)ST_LSM6DSO_ID),
> > > > > > I3C_DEVICE(0x0104, 0x006B, (void *)(uintptr_t)ST_LSM6DSR_ID),
> > > > > >
> > > > > > otherwise gcc might complain that the integer and pointer do not have
> > > > > > the same size (on 64-bit architectures).
> > > > >
> > > > > I don't understand this part. Can you provide or point some background?
> > > >
> > > > If you don't do that you'll get the following warning:
> > > >
> > > > warning: cast to 'void *' from smaller integer type 'int' [-Wint-to-void-pointer-cast]
> > >
> > > I don't get the warning during compilation. Is there any flag to enable
> > > or so?
> >
> > Nope, nothing specific to enable, just enable this driver on an arm64
> > config. Note that that gcc doesn't seem to complain about this
> > int -> void * cast (there's probably some kind of auto-promotion to
> > pointer size), but it does complains about the following void * -> int
> > cast:
> >
> > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c: In function ‘st_lsm6dsx_i3c_probe’:
> > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c:43:43: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
> > 43 | return st_lsm6dsx_probe(&i3cdev->dev, 0, (int)id->data, regmap);
> > |
>
> I fixed the warning by changing:
>
> st_lsm6dsx_probe(&i3cdev->dev, 0, (int)id->data, regmap);
>
> to
>
> st_lsm6dsx_probe(&i3cdev->dev, 0, (int)(uintptr_t)id->data, regmap);
The (int) cast is implicit, no need to add it here.
>
> But I wonder if it isn't more save to change the following too:
> I3C_DEVICE(0x0104, 0x006C, (void *)(uintptr_t)ST_LSM6DSO_ID),
> I3C_DEVICE(0x0104, 0x006B, (void *)(uintptr_t)ST_LSM6DSR_ID),
>
> What do you think?
I think we're good, we would have a problem if you were defining
ST_LSM6DSO_ID as an ULL (unsigned long long, AKA u64) and trying to
cast that value to a void pointer on a 32-bit arch.
_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2019-07-22 9:43 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-12 11:53 [PATCH v4 0/3] Add ST lsm6dso i3c support Vitor Soares
2019-07-12 11:53 ` [PATCH v4 1/3] regmap: add i3c bus support Vitor Soares
2019-07-12 15:59 ` Boris Brezillon
2019-07-12 16:14 ` Vitor Soares
2019-07-12 11:53 ` [PATCH v4 2/3] i3c: move i3c_device_match_id to device.c and export it Vitor Soares
2019-07-12 16:03 ` Boris Brezillon
2019-07-12 16:21 ` Vitor Soares
2019-07-12 16:34 ` Boris Brezillon
2019-07-12 11:53 ` [PATCH v4 3/3] iio: imu: st_lsm6dsx: add i3c basic support for LSM6DSO and LSM6DSR Vitor Soares
2019-07-12 16:14 ` Boris Brezillon
2019-07-12 16:28 ` Vitor Soares
2019-07-12 16:43 ` Boris Brezillon
2019-07-12 18:40 ` Vitor Soares
2019-07-12 20:03 ` Boris Brezillon
2019-07-16 13:22 ` Vitor Soares
2019-07-16 13:40 ` Boris Brezillon
2019-07-12 15:58 ` [PATCH v4 0/3] Add ST lsm6dso i3c support Boris Brezillon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).