| Message ID | 20200622075145.1464020-1-lee.jones@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [v4,1/1] mfd: Add I2C based System Configuaration (SYSCON) access | expand |
Hi Lee, I'm just trying to use this for my sl28 driver. Some remarks, see below. Am 2020-06-22 09:51, schrieb Lee Jones: > The existing SYSCON implementation only supports MMIO (memory mapped) > accesses, facilitated by Regmap. This extends support for registers > held behind I2C busses. > > Signed-off-by: Lee Jones <lee.jones@linaro.org> > --- > Changelog: > > v3 => v4 > - Add ability to provide a non-default Regmap configuration > > v2 => v3 > - Change 'is CONFIG' present check to include loadable modules > - s/#ifdef CONFIG_MFD_SYSCON_I2C/#if > IS_ENABLED(CONFIG_MFD_SYSCON_I2C)/ > > v1 => v2 > - Remove legacy references to OF > - Allow building as a module (fixes h8300 0-day issue) > > drivers/mfd/Kconfig | 7 +++ > drivers/mfd/Makefile | 1 + > drivers/mfd/syscon-i2c.c | 104 +++++++++++++++++++++++++++++++++ > include/linux/mfd/syscon-i2c.h | 36 ++++++++++++ > 4 files changed, 148 insertions(+) > create mode 100644 drivers/mfd/syscon-i2c.c > create mode 100644 include/linux/mfd/syscon-i2c.h > [..] > +static struct regmap *syscon_i2c_get_regmap(struct i2c_client *client, > + struct regmap_config *regmap_config) > +{ > + struct device *dev = &client->dev; > + struct syscon *entry, *syscon = NULL; > + > + spin_lock(&syscon_i2c_list_slock); > + > + list_for_each_entry(entry, &syscon_i2c_list, list) > + if (entry->dev == dev) { > + syscon = entry; > + break; > + } > + > + spin_unlock(&syscon_i2c_list_slock); > + > + if (!syscon) > + syscon = syscon_i2c_register(client, regmap_config); > + > + if (IS_ERR(syscon)) > + return ERR_CAST(syscon); > + > + return syscon->regmap; > +} > + > +struct regmap *syscon_i2c_to_regmap_config(struct i2c_client *client, > + struct regmap_config *regmap_config) > +{ > + return syscon_i2c_get_regmap(client, regmap_config); > +} > +EXPORT_SYMBOL_GPL(syscon_i2c_to_regmap_config); > + > +struct regmap *syscon_i2c_to_regmap(struct i2c_client *client) > +{ > + return syscon_i2c_get_regmap(client, &syscon_i2c_regmap_config); > +} > +EXPORT_SYMBOL_GPL(syscon_i2c_to_regmap); What do you think about struct regmap *syscon_i2c_to_regmap(struct device *dev) { struct i2c_client *client = i2c_verify_client(dev); if (!client) return ERR_PTR(-EINVAL); return syscon_i2c_get_regmap(client, &syscon_i2c_regmap_config); } Or even move it to syscon_i2c_get_regmap(). This way, (a) a driver doesn't have to use "#include <linux/i2c.h>" just to call to_i2c_client() (or i2c_verify_client()) and (b) you won't do it all over again in all sub drivers. So you could just do a regmap = syscon_i2c_to_regmap(pdev->dev.parent); I've also noticed that the mmio syscon uses device_node as parameter. What was the reason to divert from that? Just curious. -michael
Hi Lee, Am 2020-06-30 11:16, schrieb Michael Walle: > I'm just trying to use this for my sl28 driver. Some remarks, see > below. > > Am 2020-06-22 09:51, schrieb Lee Jones: >> The existing SYSCON implementation only supports MMIO (memory mapped) >> accesses, facilitated by Regmap. This extends support for registers >> held behind I2C busses. >> >> Signed-off-by: Lee Jones <lee.jones@linaro.org> >> --- >> Changelog: >> >> v3 => v4 >> - Add ability to provide a non-default Regmap configuration >> >> v2 => v3 >> - Change 'is CONFIG' present check to include loadable modules >> - s/#ifdef CONFIG_MFD_SYSCON_I2C/#if >> IS_ENABLED(CONFIG_MFD_SYSCON_I2C)/ >> >> v1 => v2 >> - Remove legacy references to OF >> - Allow building as a module (fixes h8300 0-day issue) >> >> drivers/mfd/Kconfig | 7 +++ >> drivers/mfd/Makefile | 1 + >> drivers/mfd/syscon-i2c.c | 104 >> +++++++++++++++++++++++++++++++++ >> include/linux/mfd/syscon-i2c.h | 36 ++++++++++++ >> 4 files changed, 148 insertions(+) >> create mode 100644 drivers/mfd/syscon-i2c.c >> create mode 100644 include/linux/mfd/syscon-i2c.h >> > > [..] > >> +static struct regmap *syscon_i2c_get_regmap(struct i2c_client >> *client, >> + struct regmap_config *regmap_config) >> +{ >> + struct device *dev = &client->dev; >> + struct syscon *entry, *syscon = NULL; >> + >> + spin_lock(&syscon_i2c_list_slock); >> + >> + list_for_each_entry(entry, &syscon_i2c_list, list) >> + if (entry->dev == dev) { >> + syscon = entry; >> + break; >> + } >> + >> + spin_unlock(&syscon_i2c_list_slock); >> + >> + if (!syscon) >> + syscon = syscon_i2c_register(client, regmap_config); >> + >> + if (IS_ERR(syscon)) >> + return ERR_CAST(syscon); >> + >> + return syscon->regmap; >> +} >> + >> +struct regmap *syscon_i2c_to_regmap_config(struct i2c_client *client, >> + struct regmap_config *regmap_config) >> +{ >> + return syscon_i2c_get_regmap(client, regmap_config); >> +} >> +EXPORT_SYMBOL_GPL(syscon_i2c_to_regmap_config); >> + >> +struct regmap *syscon_i2c_to_regmap(struct i2c_client *client) >> +{ >> + return syscon_i2c_get_regmap(client, &syscon_i2c_regmap_config); >> +} >> +EXPORT_SYMBOL_GPL(syscon_i2c_to_regmap); > > What do you think about > > struct regmap *syscon_i2c_to_regmap(struct device *dev) > { > struct i2c_client *client = i2c_verify_client(dev); > > if (!client) > return ERR_PTR(-EINVAL); > > return syscon_i2c_get_regmap(client, &syscon_i2c_regmap_config); > } > > Or even move it to syscon_i2c_get_regmap(). > > This way, (a) a driver doesn't have to use "#include <linux/i2c.h>" > just > to call to_i2c_client() (or i2c_verify_client()) and (b) you won't do > it > all over again in all sub drivers. > > So you could just do a > regmap = syscon_i2c_to_regmap(pdev->dev.parent); > > I've also noticed that the mmio syscon uses device_node as parameter. > What > was the reason to divert from that? Just curious. How is this supposed to be used? I had something like the following in mind: &i2c { cpld@4a { compatible = "simple-mfd"; reg = <0x4a>; gpio@4 { compatible = "vendor,gpio"; reg = <0x4>; }; }; }; But I think the childen are not enumerated if its an I2C device. And the actual i2c driver is also missing. -michael
On Wed, 01 Jul 2020, Michael Walle wrote: > Hi Lee, > > Am 2020-06-30 11:16, schrieb Michael Walle: > > I'm just trying to use this for my sl28 driver. Some remarks, see below. > > > > Am 2020-06-22 09:51, schrieb Lee Jones: > > > The existing SYSCON implementation only supports MMIO (memory mapped) > > > accesses, facilitated by Regmap. This extends support for registers > > > held behind I2C busses. > > > > > > Signed-off-by: Lee Jones <lee.jones@linaro.org> > > > --- > > > Changelog: > > > > > > v3 => v4 > > > - Add ability to provide a non-default Regmap configuration > > > > > > v2 => v3 > > > - Change 'is CONFIG' present check to include loadable modules > > > - s/#ifdef CONFIG_MFD_SYSCON_I2C/#if > > > IS_ENABLED(CONFIG_MFD_SYSCON_I2C)/ > > > > > > v1 => v2 > > > - Remove legacy references to OF > > > - Allow building as a module (fixes h8300 0-day issue) > > > > > > drivers/mfd/Kconfig | 7 +++ > > > drivers/mfd/Makefile | 1 + > > > drivers/mfd/syscon-i2c.c | 104 > > > +++++++++++++++++++++++++++++++++ > > > include/linux/mfd/syscon-i2c.h | 36 ++++++++++++ > > > 4 files changed, 148 insertions(+) > > > create mode 100644 drivers/mfd/syscon-i2c.c > > > create mode 100644 include/linux/mfd/syscon-i2c.h > > > > > > > [..] > > > > > +static struct regmap *syscon_i2c_get_regmap(struct i2c_client > > > *client, > > > + struct regmap_config *regmap_config) > > > +{ > > > + struct device *dev = &client->dev; > > > + struct syscon *entry, *syscon = NULL; > > > + > > > + spin_lock(&syscon_i2c_list_slock); > > > + > > > + list_for_each_entry(entry, &syscon_i2c_list, list) > > > + if (entry->dev == dev) { > > > + syscon = entry; > > > + break; > > > + } > > > + > > > + spin_unlock(&syscon_i2c_list_slock); > > > + > > > + if (!syscon) > > > + syscon = syscon_i2c_register(client, regmap_config); > > > + > > > + if (IS_ERR(syscon)) > > > + return ERR_CAST(syscon); > > > + > > > + return syscon->regmap; > > > +} > > > + > > > +struct regmap *syscon_i2c_to_regmap_config(struct i2c_client *client, > > > + struct regmap_config *regmap_config) > > > +{ > > > + return syscon_i2c_get_regmap(client, regmap_config); > > > +} > > > +EXPORT_SYMBOL_GPL(syscon_i2c_to_regmap_config); > > > + > > > +struct regmap *syscon_i2c_to_regmap(struct i2c_client *client) > > > +{ > > > + return syscon_i2c_get_regmap(client, &syscon_i2c_regmap_config); > > > +} > > > +EXPORT_SYMBOL_GPL(syscon_i2c_to_regmap); > > > > What do you think about > > > > struct regmap *syscon_i2c_to_regmap(struct device *dev) > > { > > struct i2c_client *client = i2c_verify_client(dev); > > > > if (!client) > > return ERR_PTR(-EINVAL); > > > > return syscon_i2c_get_regmap(client, &syscon_i2c_regmap_config); > > } > > > > Or even move it to syscon_i2c_get_regmap(). > > > > This way, (a) a driver doesn't have to use "#include <linux/i2c.h>" just > > to call to_i2c_client() (or i2c_verify_client()) and (b) you won't do it > > all over again in all sub drivers. > > > > So you could just do a > > regmap = syscon_i2c_to_regmap(pdev->dev.parent); > > > > I've also noticed that the mmio syscon uses device_node as parameter. > > What > > was the reason to divert from that? Just curious. > > How is this supposed to be used? > > I had something like the following in mind: > > &i2c { > cpld@4a { > compatible = "simple-mfd"; > reg = <0x4a>; > > gpio@4 { > compatible = "vendor,gpio"; > reg = <0x4>; > }; > }; > }; Yes, that was the idea. > But I think the childen are not enumerated if its an I2C device. And > the actual i2c driver is also missing. What do you mean? Can you elaborate? -- Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog
Am 2020-07-01 09:04, schrieb Lee Jones: > On Wed, 01 Jul 2020, Michael Walle wrote: > >> Hi Lee, >> >> Am 2020-06-30 11:16, schrieb Michael Walle: >> > I'm just trying to use this for my sl28 driver. Some remarks, see below. >> > >> > Am 2020-06-22 09:51, schrieb Lee Jones: >> > > The existing SYSCON implementation only supports MMIO (memory mapped) >> > > accesses, facilitated by Regmap. This extends support for registers >> > > held behind I2C busses. >> > > >> > > Signed-off-by: Lee Jones <lee.jones@linaro.org> >> > > --- >> > > Changelog: >> > > >> > > v3 => v4 >> > > - Add ability to provide a non-default Regmap configuration >> > > >> > > v2 => v3 >> > > - Change 'is CONFIG' present check to include loadable modules >> > > - s/#ifdef CONFIG_MFD_SYSCON_I2C/#if >> > > IS_ENABLED(CONFIG_MFD_SYSCON_I2C)/ >> > > >> > > v1 => v2 >> > > - Remove legacy references to OF >> > > - Allow building as a module (fixes h8300 0-day issue) >> > > >> > > drivers/mfd/Kconfig | 7 +++ >> > > drivers/mfd/Makefile | 1 + >> > > drivers/mfd/syscon-i2c.c | 104 >> > > +++++++++++++++++++++++++++++++++ >> > > include/linux/mfd/syscon-i2c.h | 36 ++++++++++++ >> > > 4 files changed, 148 insertions(+) >> > > create mode 100644 drivers/mfd/syscon-i2c.c >> > > create mode 100644 include/linux/mfd/syscon-i2c.h >> > > >> > >> > [..] >> > >> > > +static struct regmap *syscon_i2c_get_regmap(struct i2c_client >> > > *client, >> > > + struct regmap_config *regmap_config) >> > > +{ >> > > + struct device *dev = &client->dev; >> > > + struct syscon *entry, *syscon = NULL; >> > > + >> > > + spin_lock(&syscon_i2c_list_slock); >> > > + >> > > + list_for_each_entry(entry, &syscon_i2c_list, list) >> > > + if (entry->dev == dev) { >> > > + syscon = entry; >> > > + break; >> > > + } >> > > + >> > > + spin_unlock(&syscon_i2c_list_slock); >> > > + >> > > + if (!syscon) >> > > + syscon = syscon_i2c_register(client, regmap_config); >> > > + >> > > + if (IS_ERR(syscon)) >> > > + return ERR_CAST(syscon); >> > > + >> > > + return syscon->regmap; >> > > +} >> > > + >> > > +struct regmap *syscon_i2c_to_regmap_config(struct i2c_client *client, >> > > + struct regmap_config *regmap_config) >> > > +{ >> > > + return syscon_i2c_get_regmap(client, regmap_config); >> > > +} >> > > +EXPORT_SYMBOL_GPL(syscon_i2c_to_regmap_config); >> > > + >> > > +struct regmap *syscon_i2c_to_regmap(struct i2c_client *client) >> > > +{ >> > > + return syscon_i2c_get_regmap(client, &syscon_i2c_regmap_config); >> > > +} >> > > +EXPORT_SYMBOL_GPL(syscon_i2c_to_regmap); >> > >> > What do you think about >> > >> > struct regmap *syscon_i2c_to_regmap(struct device *dev) >> > { >> > struct i2c_client *client = i2c_verify_client(dev); >> > >> > if (!client) >> > return ERR_PTR(-EINVAL); >> > >> > return syscon_i2c_get_regmap(client, &syscon_i2c_regmap_config); >> > } >> > >> > Or even move it to syscon_i2c_get_regmap(). >> > >> > This way, (a) a driver doesn't have to use "#include <linux/i2c.h>" just >> > to call to_i2c_client() (or i2c_verify_client()) and (b) you won't do it >> > all over again in all sub drivers. >> > >> > So you could just do a >> > regmap = syscon_i2c_to_regmap(pdev->dev.parent); >> > >> > I've also noticed that the mmio syscon uses device_node as parameter. >> > What >> > was the reason to divert from that? Just curious. >> >> How is this supposed to be used? >> >> I had something like the following in mind: >> >> &i2c { >> cpld@4a { >> compatible = "simple-mfd"; >> reg = <0x4a>; >> >> gpio@4 { >> compatible = "vendor,gpio"; >> reg = <0x4>; >> }; >> }; >> }; > > Yes, that was the idea. > >> But I think the childen are not enumerated if its an I2C device. And >> the actual i2c driver is also missing. > > What do you mean? Can you elaborate? There is no i2c_driver instance who would create the regmap. If I'm reading the I2C code correctly, it won't probe any i2c device of a bus if there is no i2c_driver with an associated .probe() or .probe_new(). And even if it is probed, its subnodes won't be enumerated; the "simple-mfd" code only works for MMIO busses, right? Or I'm getting something really wrong here.. -michael
On Wed, 01 Jul 2020, Michael Walle wrote: > Am 2020-07-01 09:04, schrieb Lee Jones: > > On Wed, 01 Jul 2020, Michael Walle wrote: > > > > > Hi Lee, > > > > > > Am 2020-06-30 11:16, schrieb Michael Walle: > > > > I'm just trying to use this for my sl28 driver. Some remarks, see below. > > > > > > > > Am 2020-06-22 09:51, schrieb Lee Jones: > > > > > The existing SYSCON implementation only supports MMIO (memory mapped) > > > > > accesses, facilitated by Regmap. This extends support for registers > > > > > held behind I2C busses. > > > > > > > > > > Signed-off-by: Lee Jones <lee.jones@linaro.org> > > > > > --- > > > > > Changelog: > > > > > > > > > > v3 => v4 > > > > > - Add ability to provide a non-default Regmap configuration > > > > > > > > > > v2 => v3 > > > > > - Change 'is CONFIG' present check to include loadable modules > > > > > - s/#ifdef CONFIG_MFD_SYSCON_I2C/#if > > > > > IS_ENABLED(CONFIG_MFD_SYSCON_I2C)/ > > > > > > > > > > v1 => v2 > > > > > - Remove legacy references to OF > > > > > - Allow building as a module (fixes h8300 0-day issue) > > > > > > > > > > drivers/mfd/Kconfig | 7 +++ > > > > > drivers/mfd/Makefile | 1 + > > > > > drivers/mfd/syscon-i2c.c | 104 > > > > > +++++++++++++++++++++++++++++++++ > > > > > include/linux/mfd/syscon-i2c.h | 36 ++++++++++++ > > > > > 4 files changed, 148 insertions(+) > > > > > create mode 100644 drivers/mfd/syscon-i2c.c > > > > > create mode 100644 include/linux/mfd/syscon-i2c.h [...] > > > > This way, (a) a driver doesn't have to use "#include <linux/i2c.h>" just > > > > to call to_i2c_client() (or i2c_verify_client()) and (b) you won't do it > > > > all over again in all sub drivers. > > > > > > > > So you could just do a > > > > regmap = syscon_i2c_to_regmap(pdev->dev.parent); > > > > > > > > I've also noticed that the mmio syscon uses device_node as parameter. > > > > What > > > > was the reason to divert from that? Just curious. > > > > > > How is this supposed to be used? > > > > > > I had something like the following in mind: > > > > > > &i2c { > > > cpld@4a { > > > compatible = "simple-mfd"; > > > reg = <0x4a>; > > > > > > gpio@4 { > > > compatible = "vendor,gpio"; > > > reg = <0x4>; > > > }; > > > }; > > > }; > > > > Yes, that was the idea. > > > > > But I think the childen are not enumerated if its an I2C device. And > > > the actual i2c driver is also missing. > > > > What do you mean? Can you elaborate? > > There is no i2c_driver instance who would create the regmap. The regmap is created by the first caller of: syscon_i2c_to_regmap{_config}() > If I'm > reading the I2C code correctly, it won't probe any i2c device of a > bus if there is no i2c_driver with an associated .probe() or > .probe_new(). Why wouldn't the children be registered using i2c_driver? > And even if it is probed, its subnodes won't be > enumerated; the "simple-mfd" code only works for MMIO busses, right? > Or I'm getting something really wrong here.. Then how are these I2C based devices able to call of_platform_populate()? drivers/mfd/gateworks-gsc.c drivers/mfd/lochnagar-i2c.c drivers/mfd/palmas.c drivers/mfd/smsc-ece1099.c drivers/mfd/stpmic1.c drivers/mfd/twl-core.c Might require some more research into where your use-case is breaking down. -- Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog
Am 2020-07-02 08:54, schrieb Lee Jones: > On Wed, 01 Jul 2020, Michael Walle wrote: > >> Am 2020-07-01 09:04, schrieb Lee Jones: >> > On Wed, 01 Jul 2020, Michael Walle wrote: >> > >> > > Hi Lee, >> > > >> > > Am 2020-06-30 11:16, schrieb Michael Walle: >> > > > I'm just trying to use this for my sl28 driver. Some remarks, see below. >> > > > >> > > > Am 2020-06-22 09:51, schrieb Lee Jones: >> > > > > The existing SYSCON implementation only supports MMIO (memory mapped) >> > > > > accesses, facilitated by Regmap. This extends support for registers >> > > > > held behind I2C busses. >> > > > > >> > > > > Signed-off-by: Lee Jones <lee.jones@linaro.org> >> > > > > --- >> > > > > Changelog: >> > > > > >> > > > > v3 => v4 >> > > > > - Add ability to provide a non-default Regmap configuration >> > > > > >> > > > > v2 => v3 >> > > > > - Change 'is CONFIG' present check to include loadable modules >> > > > > - s/#ifdef CONFIG_MFD_SYSCON_I2C/#if >> > > > > IS_ENABLED(CONFIG_MFD_SYSCON_I2C)/ >> > > > > >> > > > > v1 => v2 >> > > > > - Remove legacy references to OF >> > > > > - Allow building as a module (fixes h8300 0-day issue) >> > > > > >> > > > > drivers/mfd/Kconfig | 7 +++ >> > > > > drivers/mfd/Makefile | 1 + >> > > > > drivers/mfd/syscon-i2c.c | 104 >> > > > > +++++++++++++++++++++++++++++++++ >> > > > > include/linux/mfd/syscon-i2c.h | 36 ++++++++++++ >> > > > > 4 files changed, 148 insertions(+) >> > > > > create mode 100644 drivers/mfd/syscon-i2c.c >> > > > > create mode 100644 include/linux/mfd/syscon-i2c.h > > [...] > >> > > > This way, (a) a driver doesn't have to use "#include <linux/i2c.h>" just >> > > > to call to_i2c_client() (or i2c_verify_client()) and (b) you won't do it >> > > > all over again in all sub drivers. >> > > > >> > > > So you could just do a >> > > > regmap = syscon_i2c_to_regmap(pdev->dev.parent); >> > > > >> > > > I've also noticed that the mmio syscon uses device_node as parameter. >> > > > What >> > > > was the reason to divert from that? Just curious. >> > > >> > > How is this supposed to be used? >> > > >> > > I had something like the following in mind: >> > > >> > > &i2c { >> > > cpld@4a { >> > > compatible = "simple-mfd"; >> > > reg = <0x4a>; >> > > >> > > gpio@4 { >> > > compatible = "vendor,gpio"; >> > > reg = <0x4>; >> > > }; >> > > }; >> > > }; >> > >> > Yes, that was the idea. >> > >> > > But I think the childen are not enumerated if its an I2C device. And >> > > the actual i2c driver is also missing. >> > >> > What do you mean? Can you elaborate? >> >> There is no i2c_driver instance who would create the regmap. > > The regmap is created by the first caller of: > > syscon_i2c_to_regmap{_config}() But which one is an i2c_driver? All the sub devices are platform drivers and there should be no need for them to know that they are behind an i2c driver (or spi driver or just mmio). All they have to know is how to access the registers. >> If I'm >> reading the I2C code correctly, it won't probe any i2c device of a >> bus if there is no i2c_driver with an associated .probe() or >> .probe_new(). > > Why wouldn't the children be registered using i2c_driver? Where is the code which enumerates the children? >> And even if it is probed, its subnodes won't be >> enumerated; the "simple-mfd" code only works for MMIO busses, right? >> Or I'm getting something really wrong here.. > > Then how are these I2C based devices able to call > of_platform_populate()? I don't mean they can't do it, I mean, I'm calling of_platform_populate() myself. But who is actually calling it when one uses the this patch series? -michael > drivers/mfd/gateworks-gsc.c > drivers/mfd/lochnagar-i2c.c > drivers/mfd/palmas.c > drivers/mfd/smsc-ece1099.c > drivers/mfd/stpmic1.c > drivers/mfd/twl-core.c > > Might require some more research into where your use-case is breaking > down.
On Tue, 30 Jun 2020, Michael Walle wrote: > Hi Lee, > > I'm just trying to use this for my sl28 driver. Some remarks, see below. > > Am 2020-06-22 09:51, schrieb Lee Jones: > > The existing SYSCON implementation only supports MMIO (memory mapped) > > accesses, facilitated by Regmap. This extends support for registers > > held behind I2C busses. > > > > Signed-off-by: Lee Jones <lee.jones@linaro.org> > > --- > > Changelog: > > > > v3 => v4 > > - Add ability to provide a non-default Regmap configuration > > > > v2 => v3 > > - Change 'is CONFIG' present check to include loadable modules > > - s/#ifdef CONFIG_MFD_SYSCON_I2C/#if > > IS_ENABLED(CONFIG_MFD_SYSCON_I2C)/ > > > > v1 => v2 > > - Remove legacy references to OF > > - Allow building as a module (fixes h8300 0-day issue) > > > > drivers/mfd/Kconfig | 7 +++ > > drivers/mfd/Makefile | 1 + > > drivers/mfd/syscon-i2c.c | 104 +++++++++++++++++++++++++++++++++ > > include/linux/mfd/syscon-i2c.h | 36 ++++++++++++ > > 4 files changed, 148 insertions(+) > > create mode 100644 drivers/mfd/syscon-i2c.c > > create mode 100644 include/linux/mfd/syscon-i2c.h > > > > [..] > > > +static struct regmap *syscon_i2c_get_regmap(struct i2c_client *client, > > + struct regmap_config *regmap_config) > > +{ > > + struct device *dev = &client->dev; > > + struct syscon *entry, *syscon = NULL; > > + > > + spin_lock(&syscon_i2c_list_slock); > > + > > + list_for_each_entry(entry, &syscon_i2c_list, list) > > + if (entry->dev == dev) { > > + syscon = entry; > > + break; > > + } > > + > > + spin_unlock(&syscon_i2c_list_slock); > > + > > + if (!syscon) > > + syscon = syscon_i2c_register(client, regmap_config); > > + > > + if (IS_ERR(syscon)) > > + return ERR_CAST(syscon); > > + > > + return syscon->regmap; > > +} > > + > > +struct regmap *syscon_i2c_to_regmap_config(struct i2c_client *client, > > + struct regmap_config *regmap_config) > > +{ > > + return syscon_i2c_get_regmap(client, regmap_config); > > +} > > +EXPORT_SYMBOL_GPL(syscon_i2c_to_regmap_config); > > + > > +struct regmap *syscon_i2c_to_regmap(struct i2c_client *client) > > +{ > > + return syscon_i2c_get_regmap(client, &syscon_i2c_regmap_config); > > +} > > +EXPORT_SYMBOL_GPL(syscon_i2c_to_regmap); > > What do you think about > > struct regmap *syscon_i2c_to_regmap(struct device *dev) > { > struct i2c_client *client = i2c_verify_client(dev); > > if (!client) > return ERR_PTR(-EINVAL); > > return syscon_i2c_get_regmap(client, &syscon_i2c_regmap_config); > } > > Or even move it to syscon_i2c_get_regmap(). > > This way, (a) a driver doesn't have to use "#include <linux/i2c.h>" just > to call to_i2c_client() (or i2c_verify_client()) and (b) you won't do it > all over again in all sub drivers. What is your use-case? This is set-up for based I2C drivers to call into. 'client' is given to them as their .probe() arg. > So you could just do a > regmap = syscon_i2c_to_regmap(pdev->dev.parent); > > I've also noticed that the mmio syscon uses device_node as parameter. What > was the reason to divert from that? Just curious. This is a helper for I2C clients. There aren't any OF helpers in here (yet). If you think they would be helpful we can add them. How do you see them being used? -- Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog
Am 2020-07-02 09:14, schrieb Lee Jones: > On Tue, 30 Jun 2020, Michael Walle wrote: > >> Hi Lee, >> >> I'm just trying to use this for my sl28 driver. Some remarks, see >> below. >> >> Am 2020-06-22 09:51, schrieb Lee Jones: >> > The existing SYSCON implementation only supports MMIO (memory mapped) >> > accesses, facilitated by Regmap. This extends support for registers >> > held behind I2C busses. >> > >> > Signed-off-by: Lee Jones <lee.jones@linaro.org> >> > --- >> > Changelog: >> > >> > v3 => v4 >> > - Add ability to provide a non-default Regmap configuration >> > >> > v2 => v3 >> > - Change 'is CONFIG' present check to include loadable modules >> > - s/#ifdef CONFIG_MFD_SYSCON_I2C/#if >> > IS_ENABLED(CONFIG_MFD_SYSCON_I2C)/ >> > >> > v1 => v2 >> > - Remove legacy references to OF >> > - Allow building as a module (fixes h8300 0-day issue) >> > >> > drivers/mfd/Kconfig | 7 +++ >> > drivers/mfd/Makefile | 1 + >> > drivers/mfd/syscon-i2c.c | 104 +++++++++++++++++++++++++++++++++ >> > include/linux/mfd/syscon-i2c.h | 36 ++++++++++++ >> > 4 files changed, 148 insertions(+) >> > create mode 100644 drivers/mfd/syscon-i2c.c >> > create mode 100644 include/linux/mfd/syscon-i2c.h >> > >> >> [..] >> >> > +static struct regmap *syscon_i2c_get_regmap(struct i2c_client *client, >> > + struct regmap_config *regmap_config) >> > +{ >> > + struct device *dev = &client->dev; >> > + struct syscon *entry, *syscon = NULL; >> > + >> > + spin_lock(&syscon_i2c_list_slock); >> > + >> > + list_for_each_entry(entry, &syscon_i2c_list, list) >> > + if (entry->dev == dev) { >> > + syscon = entry; >> > + break; >> > + } >> > + >> > + spin_unlock(&syscon_i2c_list_slock); >> > + >> > + if (!syscon) >> > + syscon = syscon_i2c_register(client, regmap_config); >> > + >> > + if (IS_ERR(syscon)) >> > + return ERR_CAST(syscon); >> > + >> > + return syscon->regmap; >> > +} >> > + >> > +struct regmap *syscon_i2c_to_regmap_config(struct i2c_client *client, >> > + struct regmap_config *regmap_config) >> > +{ >> > + return syscon_i2c_get_regmap(client, regmap_config); >> > +} >> > +EXPORT_SYMBOL_GPL(syscon_i2c_to_regmap_config); >> > + >> > +struct regmap *syscon_i2c_to_regmap(struct i2c_client *client) >> > +{ >> > + return syscon_i2c_get_regmap(client, &syscon_i2c_regmap_config); >> > +} >> > +EXPORT_SYMBOL_GPL(syscon_i2c_to_regmap); >> >> What do you think about >> >> struct regmap *syscon_i2c_to_regmap(struct device *dev) >> { >> struct i2c_client *client = i2c_verify_client(dev); >> >> if (!client) >> return ERR_PTR(-EINVAL); >> >> return syscon_i2c_get_regmap(client, &syscon_i2c_regmap_config); >> } >> >> Or even move it to syscon_i2c_get_regmap(). >> >> This way, (a) a driver doesn't have to use "#include <linux/i2c.h>" >> just >> to call to_i2c_client() (or i2c_verify_client()) and (b) you won't do >> it >> all over again in all sub drivers. > > What is your use-case? Still my sl28 mfd driver. There the sub devices just need a regmap. > This is set-up for based I2C drivers to call > into. 'client' is given to them as their .probe() arg. Ok, I see. Then this doesn't fit. -michael >> So you could just do a >> regmap = syscon_i2c_to_regmap(pdev->dev.parent); >> >> I've also noticed that the mmio syscon uses device_node as parameter. >> What >> was the reason to divert from that? Just curious. > > This is a helper for I2C clients. There aren't any OF helpers in here > (yet). If you think they would be helpful we can add them. How do > you see them being used?
On Thu, 02 Jul 2020, Michael Walle wrote: > Am 2020-07-02 08:54, schrieb Lee Jones: > > On Wed, 01 Jul 2020, Michael Walle wrote: > > > > > Am 2020-07-01 09:04, schrieb Lee Jones: > > > > On Wed, 01 Jul 2020, Michael Walle wrote: > > > > > > > > > Hi Lee, > > > > > > > > > > Am 2020-06-30 11:16, schrieb Michael Walle: > > > > > > I'm just trying to use this for my sl28 driver. Some remarks, see below. > > > > > > > > > > > > Am 2020-06-22 09:51, schrieb Lee Jones: > > > > > > > The existing SYSCON implementation only supports MMIO (memory mapped) > > > > > > > accesses, facilitated by Regmap. This extends support for registers > > > > > > > held behind I2C busses. > > > > > > > > > > > > > > Signed-off-by: Lee Jones <lee.jones@linaro.org> > > > > > > > --- > > > > > > > Changelog: > > > > > > > > > > > > > > v3 => v4 > > > > > > > - Add ability to provide a non-default Regmap configuration > > > > > > > > > > > > > > v2 => v3 > > > > > > > - Change 'is CONFIG' present check to include loadable modules > > > > > > > - s/#ifdef CONFIG_MFD_SYSCON_I2C/#if > > > > > > > IS_ENABLED(CONFIG_MFD_SYSCON_I2C)/ > > > > > > > > > > > > > > v1 => v2 > > > > > > > - Remove legacy references to OF > > > > > > > - Allow building as a module (fixes h8300 0-day issue) > > > > > > > > > > > > > > drivers/mfd/Kconfig | 7 +++ > > > > > > > drivers/mfd/Makefile | 1 + > > > > > > > drivers/mfd/syscon-i2c.c | 104 > > > > > > > +++++++++++++++++++++++++++++++++ > > > > > > > include/linux/mfd/syscon-i2c.h | 36 ++++++++++++ > > > > > > > 4 files changed, 148 insertions(+) > > > > > > > create mode 100644 drivers/mfd/syscon-i2c.c > > > > > > > create mode 100644 include/linux/mfd/syscon-i2c.h > > > > [...] > > > > > > > > This way, (a) a driver doesn't have to use "#include <linux/i2c.h>" just > > > > > > to call to_i2c_client() (or i2c_verify_client()) and (b) you won't do it > > > > > > all over again in all sub drivers. > > > > > > > > > > > > So you could just do a > > > > > > regmap = syscon_i2c_to_regmap(pdev->dev.parent); > > > > > > > > > > > > I've also noticed that the mmio syscon uses device_node as parameter. > > > > > > What > > > > > > was the reason to divert from that? Just curious. > > > > > > > > > > How is this supposed to be used? > > > > > > > > > > I had something like the following in mind: > > > > > > > > > > &i2c { > > > > > cpld@4a { > > > > > compatible = "simple-mfd"; > > > > > reg = <0x4a>; > > > > > > > > > > gpio@4 { > > > > > compatible = "vendor,gpio"; > > > > > reg = <0x4>; > > > > > }; > > > > > }; > > > > > }; > > > > > > > > Yes, that was the idea. > > > > > > > > > But I think the childen are not enumerated if its an I2C device. And > > > > > the actual i2c driver is also missing. > > > > > > > > What do you mean? Can you elaborate? > > > > > > There is no i2c_driver instance who would create the regmap. > > > > The regmap is created by the first caller of: > > > > syscon_i2c_to_regmap{_config}() > > But which one is an i2c_driver? All the sub devices are platform drivers > and there should be no need for them to know that they are behind an > i2c driver (or spi driver or just mmio). All they have to know is how > to access the registers. > > > > If I'm > > > reading the I2C code correctly, it won't probe any i2c device of a > > > bus if there is no i2c_driver with an associated .probe() or > > > .probe_new(). > > > > Why wouldn't the children be registered using i2c_driver? > > Where is the code which enumerates the children? Yes, I see the problem now. So I2C devices depend on a device registering with the i2c_driver framework, which then probes the device accordingly. Thus a physical driver is required to convert I2C devices to platform devices. So this stops being an MFD problem and starts being a 'simple-i2c' issue. :) (not a genuine suggestion by the way) Wolfram, do we have this correct? -- Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog
Am 2020-07-02 10:18, schrieb Lee Jones: > On Thu, 02 Jul 2020, Michael Walle wrote: > >> Am 2020-07-02 08:54, schrieb Lee Jones: >> > On Wed, 01 Jul 2020, Michael Walle wrote: >> > >> > > Am 2020-07-01 09:04, schrieb Lee Jones: >> > > > On Wed, 01 Jul 2020, Michael Walle wrote: >> > > > >> > > > > Hi Lee, >> > > > > >> > > > > Am 2020-06-30 11:16, schrieb Michael Walle: >> > > > > > I'm just trying to use this for my sl28 driver. Some remarks, see below. >> > > > > > >> > > > > > Am 2020-06-22 09:51, schrieb Lee Jones: >> > > > > > > The existing SYSCON implementation only supports MMIO (memory mapped) >> > > > > > > accesses, facilitated by Regmap. This extends support for registers >> > > > > > > held behind I2C busses. >> > > > > > > >> > > > > > > Signed-off-by: Lee Jones <lee.jones@linaro.org> >> > > > > > > --- >> > > > > > > Changelog: >> > > > > > > >> > > > > > > v3 => v4 >> > > > > > > - Add ability to provide a non-default Regmap configuration >> > > > > > > >> > > > > > > v2 => v3 >> > > > > > > - Change 'is CONFIG' present check to include loadable modules >> > > > > > > - s/#ifdef CONFIG_MFD_SYSCON_I2C/#if >> > > > > > > IS_ENABLED(CONFIG_MFD_SYSCON_I2C)/ >> > > > > > > >> > > > > > > v1 => v2 >> > > > > > > - Remove legacy references to OF >> > > > > > > - Allow building as a module (fixes h8300 0-day issue) >> > > > > > > >> > > > > > > drivers/mfd/Kconfig | 7 +++ >> > > > > > > drivers/mfd/Makefile | 1 + >> > > > > > > drivers/mfd/syscon-i2c.c | 104 >> > > > > > > +++++++++++++++++++++++++++++++++ >> > > > > > > include/linux/mfd/syscon-i2c.h | 36 ++++++++++++ >> > > > > > > 4 files changed, 148 insertions(+) >> > > > > > > create mode 100644 drivers/mfd/syscon-i2c.c >> > > > > > > create mode 100644 include/linux/mfd/syscon-i2c.h >> > >> > [...] >> > >> > > > > > This way, (a) a driver doesn't have to use "#include <linux/i2c.h>" just >> > > > > > to call to_i2c_client() (or i2c_verify_client()) and (b) you won't do it >> > > > > > all over again in all sub drivers. >> > > > > > >> > > > > > So you could just do a >> > > > > > regmap = syscon_i2c_to_regmap(pdev->dev.parent); >> > > > > > >> > > > > > I've also noticed that the mmio syscon uses device_node as parameter. >> > > > > > What >> > > > > > was the reason to divert from that? Just curious. >> > > > > >> > > > > How is this supposed to be used? >> > > > > >> > > > > I had something like the following in mind: >> > > > > >> > > > > &i2c { >> > > > > cpld@4a { >> > > > > compatible = "simple-mfd"; >> > > > > reg = <0x4a>; >> > > > > >> > > > > gpio@4 { >> > > > > compatible = "vendor,gpio"; >> > > > > reg = <0x4>; >> > > > > }; >> > > > > }; >> > > > > }; >> > > > >> > > > Yes, that was the idea. >> > > > >> > > > > But I think the childen are not enumerated if its an I2C device. And >> > > > > the actual i2c driver is also missing. >> > > > >> > > > What do you mean? Can you elaborate? >> > > >> > > There is no i2c_driver instance who would create the regmap. >> > >> > The regmap is created by the first caller of: >> > >> > syscon_i2c_to_regmap{_config}() >> >> But which one is an i2c_driver? All the sub devices are platform >> drivers >> and there should be no need for them to know that they are behind an >> i2c driver (or spi driver or just mmio). All they have to know is how >> to access the registers. >> >> > > If I'm >> > > reading the I2C code correctly, it won't probe any i2c device of a >> > > bus if there is no i2c_driver with an associated .probe() or >> > > .probe_new(). >> > >> > Why wouldn't the children be registered using i2c_driver? >> >> Where is the code which enumerates the children? > > Yes, I see the problem now. So I2C devices depend on a device > registering with the i2c_driver framework, which then probes the > device accordingly. Thus a physical driver is required to convert I2C > devices to platform devices. So this stops being an MFD problem and > starts being a 'simple-i2c' issue. :) Yes, but this is still MFD specifc, because no other normal (in lack of a better word, think of one-function-devices) I2C device has sub-nodes. Thus my proposal for that simple-mfd-i2c driver. And keep in mind that this likely isn't specific to I2C but also to SPI (without having looked at it). So in theory that simple-mfd-i2c, could also register a spi_driver which then registers an SPI regmap and enumerates the children. So this is not only an I2C topic, but IMHO still MFD. -michael
Hi Lee,
I love your patch! Yet something to improve:
[auto build test ERROR on ljones-mfd/for-mfd-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Lee-Jones/mfd-Add-I2C-based-System-Configuaration-SYSCON-access/20200622-155310
base: https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
config: riscv-randconfig-r035-20200705 (attached as .config)
compiler: riscv32-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=riscv
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
riscv32-linux-ld: arch/riscv/kernel/sbi.o: in function `sbi_power_off':
arch/riscv/kernel/sbi.c:544: undefined reference to `sbi_shutdown'
riscv32-linux-ld: drivers/base/regmap/regmap-i2c.o: in function `regmap_smbus_byte_reg_read':
>> drivers/base/regmap/regmap-i2c.c:25: undefined reference to `i2c_smbus_read_byte_data'
riscv32-linux-ld: drivers/base/regmap/regmap-i2c.o: in function `regmap_smbus_byte_reg_write':
>> drivers/base/regmap/regmap-i2c.c:43: undefined reference to `i2c_smbus_write_byte_data'
riscv32-linux-ld: drivers/base/regmap/regmap-i2c.o: in function `regmap_smbus_word_reg_read':
>> drivers/base/regmap/regmap-i2c.c:61: undefined reference to `i2c_smbus_read_word_data'
riscv32-linux-ld: drivers/base/regmap/regmap-i2c.o: in function `i2c_smbus_read_word_swapped':
>> include/linux/i2c.h:159: undefined reference to `i2c_smbus_read_word_data'
riscv32-linux-ld: drivers/base/regmap/regmap-i2c.o: in function `i2c_smbus_write_word_swapped':
>> include/linux/i2c.h:168: undefined reference to `i2c_smbus_write_word_data'
riscv32-linux-ld: drivers/base/regmap/regmap-i2c.o: in function `regmap_smbus_word_reg_write':
>> drivers/base/regmap/regmap-i2c.c:79: undefined reference to `i2c_smbus_write_word_data'
riscv32-linux-ld: drivers/base/regmap/regmap-i2c.o: in function `regmap_i2c_smbus_i2c_read':
>> drivers/base/regmap/regmap-i2c.c:233: undefined reference to `i2c_smbus_read_i2c_block_data'
riscv32-linux-ld: drivers/base/regmap/regmap-i2c.o: in function `regmap_i2c_smbus_i2c_write':
>> drivers/base/regmap/regmap-i2c.c:218: undefined reference to `i2c_smbus_write_i2c_block_data'
riscv32-linux-ld: drivers/base/regmap/regmap-i2c.o: in function `regmap_i2c_read':
>> drivers/base/regmap/regmap-i2c.c:191: undefined reference to `i2c_transfer'
riscv32-linux-ld: drivers/base/regmap/regmap-i2c.o: in function `i2c_master_send':
>> include/linux/i2c.h:105: undefined reference to `i2c_transfer_buffer_flags'
riscv32-linux-ld: drivers/base/regmap/regmap-i2c.o: in function `regmap_i2c_gather_write':
drivers/base/regmap/regmap-i2c.c:161: undefined reference to `i2c_transfer'
vim +159 include/linux/i2c.h
ba98645c7d5464 Wolfram Sang 2017-11-04 93
8a91732b3b3345 Wolfram Sang 2017-11-04 94 /**
8a91732b3b3345 Wolfram Sang 2017-11-04 95 * i2c_master_send - issue a single I2C message in master transmit mode
8a91732b3b3345 Wolfram Sang 2017-11-04 96 * @client: Handle to slave device
8a91732b3b3345 Wolfram Sang 2017-11-04 97 * @buf: Data that will be written to the slave
8a91732b3b3345 Wolfram Sang 2017-11-04 98 * @count: How many bytes to write, must be less than 64k since msg.len is u16
8a91732b3b3345 Wolfram Sang 2017-11-04 99 *
8a91732b3b3345 Wolfram Sang 2017-11-04 100 * Returns negative errno, or else the number of bytes written.
8a91732b3b3345 Wolfram Sang 2017-11-04 101 */
8a91732b3b3345 Wolfram Sang 2017-11-04 102 static inline int i2c_master_send(const struct i2c_client *client,
8a91732b3b3345 Wolfram Sang 2017-11-04 103 const char *buf, int count)
8a91732b3b3345 Wolfram Sang 2017-11-04 104 {
8a91732b3b3345 Wolfram Sang 2017-11-04 @105 return i2c_transfer_buffer_flags(client, (char *)buf, count, 0);
8a91732b3b3345 Wolfram Sang 2017-11-04 106 };
^1da177e4c3f41 Linus Torvalds 2005-04-16 107
ba98645c7d5464 Wolfram Sang 2017-11-04 108 /**
ba98645c7d5464 Wolfram Sang 2017-11-04 109 * i2c_master_send_dmasafe - issue a single I2C message in master transmit mode
ba98645c7d5464 Wolfram Sang 2017-11-04 110 * using a DMA safe buffer
ba98645c7d5464 Wolfram Sang 2017-11-04 111 * @client: Handle to slave device
ba98645c7d5464 Wolfram Sang 2017-11-04 112 * @buf: Data that will be written to the slave, must be safe to use with DMA
ba98645c7d5464 Wolfram Sang 2017-11-04 113 * @count: How many bytes to write, must be less than 64k since msg.len is u16
ba98645c7d5464 Wolfram Sang 2017-11-04 114 *
ba98645c7d5464 Wolfram Sang 2017-11-04 115 * Returns negative errno, or else the number of bytes written.
ba98645c7d5464 Wolfram Sang 2017-11-04 116 */
ba98645c7d5464 Wolfram Sang 2017-11-04 117 static inline int i2c_master_send_dmasafe(const struct i2c_client *client,
ba98645c7d5464 Wolfram Sang 2017-11-04 118 const char *buf, int count)
ba98645c7d5464 Wolfram Sang 2017-11-04 119 {
ba98645c7d5464 Wolfram Sang 2017-11-04 120 return i2c_transfer_buffer_flags(client, (char *)buf, count,
ba98645c7d5464 Wolfram Sang 2017-11-04 121 I2C_M_DMA_SAFE);
ba98645c7d5464 Wolfram Sang 2017-11-04 122 };
ba98645c7d5464 Wolfram Sang 2017-11-04 123
^1da177e4c3f41 Linus Torvalds 2005-04-16 124 /* Transfer num messages.
^1da177e4c3f41 Linus Torvalds 2005-04-16 125 */
c807da539e8276 Luca Ceresoli 2019-12-04 126 int i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num);
b37d2a3a75cb0e Jean Delvare 2012-06-29 127 /* Unlocked flavor */
c807da539e8276 Luca Ceresoli 2019-12-04 128 int __i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num);
^1da177e4c3f41 Linus Torvalds 2005-04-16 129
^1da177e4c3f41 Linus Torvalds 2005-04-16 130 /* This is the very generalized SMBus access routine. You probably do not
^1da177e4c3f41 Linus Torvalds 2005-04-16 131 want to use this, though; one of the functions below may be much easier,
^1da177e4c3f41 Linus Torvalds 2005-04-16 132 and probably just as fast.
^1da177e4c3f41 Linus Torvalds 2005-04-16 133 Note that we use i2c_adapter here, because you do not need a specific
^1da177e4c3f41 Linus Torvalds 2005-04-16 134 smbus adapter to call this function. */
63453b59e41173 Peter Rosin 2018-06-20 135 s32 i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr,
3ae70deef0a5cc Jean Delvare 2008-10-22 136 unsigned short flags, char read_write, u8 command,
63453b59e41173 Peter Rosin 2018-06-20 137 int protocol, union i2c_smbus_data *data);
63453b59e41173 Peter Rosin 2018-06-20 138
63453b59e41173 Peter Rosin 2018-06-20 139 /* Unlocked flavor */
63453b59e41173 Peter Rosin 2018-06-20 140 s32 __i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr,
63453b59e41173 Peter Rosin 2018-06-20 141 unsigned short flags, char read_write, u8 command,
63453b59e41173 Peter Rosin 2018-06-20 142 int protocol, union i2c_smbus_data *data);
^1da177e4c3f41 Linus Torvalds 2005-04-16 143
^1da177e4c3f41 Linus Torvalds 2005-04-16 144 /* Now follow the 'nice' access routines. These also document the calling
ae7193f7fa3e17 Jean Delvare 2008-07-14 145 conventions of i2c_smbus_xfer. */
^1da177e4c3f41 Linus Torvalds 2005-04-16 146
c807da539e8276 Luca Ceresoli 2019-12-04 147 s32 i2c_smbus_read_byte(const struct i2c_client *client);
c807da539e8276 Luca Ceresoli 2019-12-04 148 s32 i2c_smbus_write_byte(const struct i2c_client *client, u8 value);
c807da539e8276 Luca Ceresoli 2019-12-04 149 s32 i2c_smbus_read_byte_data(const struct i2c_client *client, u8 command);
c807da539e8276 Luca Ceresoli 2019-12-04 150 s32 i2c_smbus_write_byte_data(const struct i2c_client *client,
^1da177e4c3f41 Linus Torvalds 2005-04-16 151 u8 command, u8 value);
c807da539e8276 Luca Ceresoli 2019-12-04 152 s32 i2c_smbus_read_word_data(const struct i2c_client *client, u8 command);
c807da539e8276 Luca Ceresoli 2019-12-04 153 s32 i2c_smbus_write_word_data(const struct i2c_client *client,
^1da177e4c3f41 Linus Torvalds 2005-04-16 154 u8 command, u16 value);
06a67848c6681a Jonathan Cameron 2011-10-30 155
06a67848c6681a Jonathan Cameron 2011-10-30 156 static inline s32
06a67848c6681a Jonathan Cameron 2011-10-30 157 i2c_smbus_read_word_swapped(const struct i2c_client *client, u8 command)
06a67848c6681a Jonathan Cameron 2011-10-30 158 {
06a67848c6681a Jonathan Cameron 2011-10-30 @159 s32 value = i2c_smbus_read_word_data(client, command);
06a67848c6681a Jonathan Cameron 2011-10-30 160
06a67848c6681a Jonathan Cameron 2011-10-30 161 return (value < 0) ? value : swab16(value);
06a67848c6681a Jonathan Cameron 2011-10-30 162 }
06a67848c6681a Jonathan Cameron 2011-10-30 163
06a67848c6681a Jonathan Cameron 2011-10-30 164 static inline s32
06a67848c6681a Jonathan Cameron 2011-10-30 165 i2c_smbus_write_word_swapped(const struct i2c_client *client,
06a67848c6681a Jonathan Cameron 2011-10-30 166 u8 command, u16 value)
06a67848c6681a Jonathan Cameron 2011-10-30 167 {
06a67848c6681a Jonathan Cameron 2011-10-30 @168 return i2c_smbus_write_word_data(client, command, swab16(value));
06a67848c6681a Jonathan Cameron 2011-10-30 169 }
06a67848c6681a Jonathan Cameron 2011-10-30 170
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig index 0a59249198d34..f25f80f68edca 100644 --- a/drivers/mfd/Kconfig +++ b/drivers/mfd/Kconfig @@ -1300,6 +1300,13 @@ config MFD_SYSCON Select this option to enable accessing system control registers via regmap. +config MFD_SYSCON_I2C + tristate "System Controller Register R/W Based on I2C Regmap" + select REGMAP_I2C + help + Select this option to enable accessing system control registers + via I2C using regmap. + config MFD_DAVINCI_VOICECODEC tristate select MFD_CORE diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile index f935d10cbf0fc..0aec1f42ac979 100644 --- a/drivers/mfd/Makefile +++ b/drivers/mfd/Makefile @@ -219,6 +219,7 @@ obj-$(CONFIG_MFD_RK808) += rk808.o obj-$(CONFIG_MFD_RN5T618) += rn5t618.o obj-$(CONFIG_MFD_SEC_CORE) += sec-core.o sec-irq.o obj-$(CONFIG_MFD_SYSCON) += syscon.o +obj-$(CONFIG_MFD_SYSCON_I2C) += syscon-i2c.o obj-$(CONFIG_MFD_LM3533) += lm3533-core.o lm3533-ctrlbank.o obj-$(CONFIG_MFD_VEXPRESS_SYSREG) += vexpress-sysreg.o obj-$(CONFIG_MFD_RETU) += retu-mfd.o diff --git a/drivers/mfd/syscon-i2c.c b/drivers/mfd/syscon-i2c.c new file mode 100644 index 0000000000000..f6b429dff2ab1 --- /dev/null +++ b/drivers/mfd/syscon-i2c.c @@ -0,0 +1,104 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * System Control Driver accessed over I2C + * + * Copyright (C) 2020 Linaro Ltd. + * + * Author: Lee Jones <lee.jones@linaro.org> + */ + +#include <linux/device.h> +#include <linux/err.h> +#include <linux/i2c.h> +#include <linux/list.h> +#include <linux/mfd/syscon-i2c.h> +#include <linux/regmap.h> + +static DEFINE_SPINLOCK(syscon_i2c_list_slock); +static LIST_HEAD(syscon_i2c_list); + +struct syscon { + struct device *dev; + struct regmap *regmap; + struct list_head list; +}; + +static struct regmap_config syscon_i2c_regmap_config = { + .reg_bits = 8, + .val_bits = 8, +}; + +static struct syscon *syscon_i2c_register(struct i2c_client *client, + struct regmap_config *regmap_config) +{ + struct device *dev = &client->dev; + struct syscon *syscon; + struct regmap *regmap; + int ret; + + if (!regmap_config) { + dev_err(dev, "No Regmap configuration supplied\n"); + return ERR_PTR(-EINVAL); + } + + syscon = devm_kzalloc(dev, sizeof(*syscon), GFP_KERNEL); + if (!syscon) + return ERR_PTR(-ENOMEM); + + if (!regmap_config->name) + regmap_config->name = dev_name(dev); + + regmap = devm_regmap_init_i2c(client, regmap_config); + if (IS_ERR(regmap)) { + dev_err(dev, "Failed to initialise Regmap I2C\n"); + ret = PTR_ERR(regmap); + return ERR_PTR(ret); + } + + syscon->regmap = regmap; + syscon->dev = dev; + + spin_lock(&syscon_i2c_list_slock); + list_add_tail(&syscon->list, &syscon_i2c_list); + spin_unlock(&syscon_i2c_list_slock); + + return syscon; +} + +static struct regmap *syscon_i2c_get_regmap(struct i2c_client *client, + struct regmap_config *regmap_config) +{ + struct device *dev = &client->dev; + struct syscon *entry, *syscon = NULL; + + spin_lock(&syscon_i2c_list_slock); + + list_for_each_entry(entry, &syscon_i2c_list, list) + if (entry->dev == dev) { + syscon = entry; + break; + } + + spin_unlock(&syscon_i2c_list_slock); + + if (!syscon) + syscon = syscon_i2c_register(client, regmap_config); + + if (IS_ERR(syscon)) + return ERR_CAST(syscon); + + return syscon->regmap; +} + +struct regmap *syscon_i2c_to_regmap_config(struct i2c_client *client, + struct regmap_config *regmap_config) +{ + return syscon_i2c_get_regmap(client, regmap_config); +} +EXPORT_SYMBOL_GPL(syscon_i2c_to_regmap_config); + +struct regmap *syscon_i2c_to_regmap(struct i2c_client *client) +{ + return syscon_i2c_get_regmap(client, &syscon_i2c_regmap_config); +} +EXPORT_SYMBOL_GPL(syscon_i2c_to_regmap); diff --git a/include/linux/mfd/syscon-i2c.h b/include/linux/mfd/syscon-i2c.h new file mode 100644 index 0000000000000..4507572bc4f86 --- /dev/null +++ b/include/linux/mfd/syscon-i2c.h @@ -0,0 +1,36 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * System Control Driver accessed via I2C + * + * Copyright (C) 2020 Linaro Ltd. + * + * Author: Lee Jones <lee.jones@linaro.org> + */ + +#ifndef __LINUX_MFD_SYSCON_I2C_H__ +#define __LINUX_MFD_SYSCON_I2C_H__ + +#include <linux/err.h> +#include <linux/errno.h> +#include <linux/i2c.h> +#include <linux/regmap.h> + +#if IS_ENABLED(CONFIG_MFD_SYSCON_I2C) +extern struct regmap *syscon_i2c_to_regmap(struct i2c_client *client); +extern +struct regmap *syscon_i2c_to_regmap_config(struct i2c_client *client, + struct regmap_config *regmap_config); +#else +static inline struct regmap *syscon_i2c_to_regmap(struct i2c_client *client) +{ + return ERR_PTR(-ENOTSUPP); +} +static inline +struct regmap *syscon_i2c_to_regmap_config(struct i2c_client *client, + struct regmap_config *regmap_config); +{ + return ERR_PTR(-ENOTSUPP); +} +#endif + +#endif /* __LINUX_MFD_SYSCON_I2C_H__ */
The existing SYSCON implementation only supports MMIO (memory mapped) accesses, facilitated by Regmap. This extends support for registers held behind I2C busses. Signed-off-by: Lee Jones <lee.jones@linaro.org> --- Changelog: v3 => v4 - Add ability to provide a non-default Regmap configuration v2 => v3 - Change 'is CONFIG' present check to include loadable modules - s/#ifdef CONFIG_MFD_SYSCON_I2C/#if IS_ENABLED(CONFIG_MFD_SYSCON_I2C)/ v1 => v2 - Remove legacy references to OF - Allow building as a module (fixes h8300 0-day issue) drivers/mfd/Kconfig | 7 +++ drivers/mfd/Makefile | 1 + drivers/mfd/syscon-i2c.c | 104 +++++++++++++++++++++++++++++++++ include/linux/mfd/syscon-i2c.h | 36 ++++++++++++ 4 files changed, 148 insertions(+) create mode 100644 drivers/mfd/syscon-i2c.c create mode 100644 include/linux/mfd/syscon-i2c.h -- 2.25.1