| Message ID | c38e3cb9e8524af58c995a9cf1f79ddba69b47b4.1555354268.git.vitor.soares@synopsys.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add ST lsm6dso i3c suppport | expand |
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, ®map_i3c, &i3c->dev, config, > + lock_key, lock_name); > +} > +EXPORT_SYMBOL_GPL(__devm_regmap_init_i3c); > + > +MODULE_LICENSE("GPL"); > diff --git a/include/linux/regmap.h b/include/linux/regmap.h > index daeec7d..f65984d 100644 > --- a/include/linux/regmap.h > +++ b/include/linux/regmap.h > @@ -25,6 +25,7 @@ struct module; > struct clk; > struct device; > struct i2c_client; > +struct i3c_device; > struct irq_domain; > struct slim_device; > struct spi_device; > @@ -624,6 +625,10 @@ struct regmap *__devm_regmap_init_slimbus(struct slim_device *slimbus, > const struct regmap_config *config, > struct lock_class_key *lock_key, > const char *lock_name); > +struct regmap *__devm_regmap_init_i3c(struct i3c_device *i3c, > + const struct regmap_config *config, > + struct lock_class_key *lock_key, > + const char *lock_name); > /* > * Wrapper for regmap_init macros to include a unique lockdep key and name > * for each call. No-op if CONFIG_LOCKDEP is not set. > @@ -982,6 +987,21 @@ bool regmap_ac97_default_volatile(struct device *dev, unsigned int reg); > #define devm_regmap_init_slimbus(slimbus, config) \ > __regmap_lockdep_wrapper(__devm_regmap_init_slimbus, #config, \ > slimbus, config) > + > +/** > + * devm_regmap_init_i3c() - Initialise managed register map Maybe we should have a more specific name here, as some might want to access registers using HDR modes (devm_regmap_init_i3c_sdr()?) > + * > + * @i3c: Device that will be interacted with > + * @config: Configuration for register map > + * > + * The return value will be an ERR_PTR() on error or a valid pointer > + * to a struct regmap. The regmap will be automatically freed by the > + * device management code. > + */ > +#define devm_regmap_init_i3c(i3c, config) \ > + __regmap_lockdep_wrapper(__devm_regmap_init_i3c, #config, \ > + i3c, config) > + > int regmap_mmio_attach_clk(struct regmap *map, struct clk *clk); > void regmap_mmio_detach_clk(struct regmap *map); > void regmap_exit(struct regmap *map);
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, ®map_i3c, &i3c->dev, config, > > + lock_key, lock_name); > > +} > > +EXPORT_SYMBOL_GPL(__devm_regmap_init_i3c); > > + > > +MODULE_LICENSE("GPL"); > > diff --git a/include/linux/regmap.h b/include/linux/regmap.h > > index daeec7d..f65984d 100644 > > --- a/include/linux/regmap.h > > +++ b/include/linux/regmap.h > > @@ -25,6 +25,7 @@ struct module; > > struct clk; > > struct device; > > struct i2c_client; > > +struct i3c_device; > > struct irq_domain; > > struct slim_device; > > struct spi_device; > > @@ -624,6 +625,10 @@ struct regmap *__devm_regmap_init_slimbus(struct slim_device *slimbus, > > const struct regmap_config *config, > > struct lock_class_key *lock_key, > > const char *lock_name); > > +struct regmap *__devm_regmap_init_i3c(struct i3c_device *i3c, > > + const struct regmap_config *config, > > + struct lock_class_key *lock_key, > > + const char *lock_name); > > /* > > * Wrapper for regmap_init macros to include a unique lockdep key and name > > * for each call. No-op if CONFIG_LOCKDEP is not set. > > @@ -982,6 +987,21 @@ bool regmap_ac97_default_volatile(struct device *dev, unsigned int reg); > > #define devm_regmap_init_slimbus(slimbus, config) \ > > __regmap_lockdep_wrapper(__devm_regmap_init_slimbus, #config, \ > > slimbus, config) > > + > > +/** > > + * devm_regmap_init_i3c() - Initialise managed register map > > Maybe we should have a more specific name here, as some might want to > access registers using HDR modes (devm_regmap_init_i3c_sdr()?) That make sense if the device with HDR capability is able to do everything in HDR mode and I'm not sure about that. I would like to wait for a couple of commercial devices before that change. > > > > + * > > + * @i3c: Device that will be interacted with > > + * @config: Configuration for register map > > + * > > + * The return value will be an ERR_PTR() on error or a valid pointer > > + * to a struct regmap. The regmap will be automatically freed by the > > + * device management code. > > + */ > > +#define devm_regmap_init_i3c(i3c, config) \ > > + __regmap_lockdep_wrapper(__devm_regmap_init_i3c, #config, \ > > + i3c, config) > > + > > int regmap_mmio_attach_clk(struct regmap *map, struct clk *clk); > > void regmap_mmio_detach_clk(struct regmap *map); > > void regmap_exit(struct regmap *map); Thanks for your feedback. Best regards, Vitor Soares
Hi 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
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.
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
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, ®map_i3c, &i3c->dev, config, + lock_key, lock_name); +} +EXPORT_SYMBOL_GPL(__devm_regmap_init_i3c); + +MODULE_LICENSE("GPL"); diff --git a/include/linux/regmap.h b/include/linux/regmap.h index daeec7d..f65984d 100644 --- a/include/linux/regmap.h +++ b/include/linux/regmap.h @@ -25,6 +25,7 @@ struct module; struct clk; struct device; struct i2c_client; +struct i3c_device; struct irq_domain; struct slim_device; struct spi_device; @@ -624,6 +625,10 @@ struct regmap *__devm_regmap_init_slimbus(struct slim_device *slimbus, const struct regmap_config *config, struct lock_class_key *lock_key, const char *lock_name); +struct regmap *__devm_regmap_init_i3c(struct i3c_device *i3c, + const struct regmap_config *config, + struct lock_class_key *lock_key, + const char *lock_name); /* * Wrapper for regmap_init macros to include a unique lockdep key and name * for each call. No-op if CONFIG_LOCKDEP is not set. @@ -982,6 +987,21 @@ bool regmap_ac97_default_volatile(struct device *dev, unsigned int reg); #define devm_regmap_init_slimbus(slimbus, config) \ __regmap_lockdep_wrapper(__devm_regmap_init_slimbus, #config, \ slimbus, config) + +/** + * devm_regmap_init_i3c() - Initialise managed register map + * + * @i3c: Device that will be interacted with + * @config: Configuration for register map + * + * The return value will be an ERR_PTR() on error or a valid pointer + * to a struct regmap. The regmap will be automatically freed by the + * device management code. + */ +#define devm_regmap_init_i3c(i3c, config) \ + __regmap_lockdep_wrapper(__devm_regmap_init_i3c, #config, \ + i3c, config) + int regmap_mmio_attach_clk(struct regmap *map, struct clk *clk); void regmap_mmio_detach_clk(struct regmap *map); void regmap_exit(struct regmap *map);
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