| Message ID | c38e3cb9e8524af58c995a9cf1f79ddba69b47b4.1555354268.git.vitor.soares@synopsys.com (mailing list archive) |
|---|---|
| State | New, archived |
| 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
On Mon, Apr 15, 2019 at 09:19:39PM +0200, Vitor Soares wrote: > +++ b/drivers/base/regmap/regmap-i3c.c > @@ -0,0 +1,62 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2018 Synopsys, Inc. and/or its affiliates. Please make the entire header C++ style so it looks more consistent. Otherwise this looks good modulo Boris' comment; I'm fine with leaving extra modes for later so long as they can be introduced without disrupting existing users so the only question there would be if we should name the init function in some way that's specific to the I/O mode being used here.
Hi Mark, From: Mark Brown <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