Message ID | 1444840342-9233-4-git-send-email-wens@csie.org (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Hi Chen-Yu, [auto build test ERROR on next-20151013 -- if it's inappropriate base, please suggest rules for selecting the more suitable base] url: https://github.com/0day-ci/linux/commits/Chen-Yu-Tsai/mfd-axp20x-Add-support-for-RSB-based-AXP223/20151015-004334 config: i386-randconfig-s1-201541 (attached as .config) reproduce: # save the attached .config to linux build tree make ARCH=i386 All error/warnings (new ones prefixed by >>): warning: (MFD_AXP20X) selects MFD_AXP20X_RSB which has unmet direct dependencies (HAS_IOMEM && SUNXI_RSB) drivers/built-in.o: In function `axp20x_sunxi_rsb_probe': >> axp20x-rsb.c:(.text+0xe00d7): undefined reference to `__devm_regmap_init_sunxi_rsb' drivers/built-in.o: In function `axp20x_sunxi_rsb_driver_init': >> axp20x-rsb.c:(.init.text+0x81ce): undefined reference to `sunxi_rsb_driver_register' --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Chen-Yu, [auto build test ERROR on next-20151013 -- if it's inappropriate base, please suggest rules for selecting the more suitable base] url: https://github.com/0day-ci/linux/commits/Chen-Yu-Tsai/mfd-axp20x-Add-support-for-RSB-based-AXP223/20151015-004334 config: x86_64-allmodconfig (attached as .config) reproduce: # save the attached .config to linux build tree make ARCH=x86_64 All errors (new ones prefixed by >>): drivers/built-in.o: In function `axp20x_i2c_remove': axp20x-i2c.c:(.text+0x22810b): undefined reference to `axp20x_device_remove' drivers/built-in.o: In function `axp20x_i2c_probe': axp20x-i2c.c:(.text+0x22827d): undefined reference to `axp20x_match_device' axp20x-i2c.c:(.text+0x2282ff): undefined reference to `__devm_regmap_init_i2c' axp20x-i2c.c:(.text+0x22834c): undefined reference to `axp20x_device_probe' drivers/built-in.o: In function `axp20x_sunxi_rsb_remove': >> axp20x-rsb.c:(.text+0x228388): undefined reference to `axp20x_device_remove' drivers/built-in.o: In function `axp20x_sunxi_rsb_probe': >> axp20x-rsb.c:(.text+0x22844d): undefined reference to `axp20x_match_device' axp20x-rsb.c:(.text+0x2284cb): undefined reference to `__devm_regmap_init_sunxi_rsb' >> axp20x-rsb.c:(.text+0x228518): undefined reference to `axp20x_device_probe' drivers/built-in.o: In function `axp20x_i2c_driver_init': axp20x-i2c.c:(.init.text+0x1a8c9): undefined reference to `i2c_register_driver' drivers/built-in.o: In function `axp20x_sunxi_rsb_driver_init': axp20x-rsb.c:(.init.text+0x1a8e9): undefined reference to `sunxi_rsb_driver_register' drivers/built-in.o: In function `axp20x_i2c_driver_exit': axp20x-i2c.c:(.exit.text+0x683): undefined reference to `i2c_del_driver' --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Thu, Oct 15, 2015 at 12:32:19AM +0800, Chen-Yu Tsai wrote: > The AXP223 is a new PMIC commonly paired with Allwinner A23/A33 SoCs. > It is functionally identical to AXP221; only the regulator default > voltage/status and the external host interface are different. > > Signed-off-by: Chen-Yu Tsai <wens@csie.org> > --- > drivers/mfd/Kconfig | 12 ++++++ > drivers/mfd/Makefile | 1 + > drivers/mfd/axp20x-core.c | 2 + > drivers/mfd/axp20x-rsb.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++ > include/linux/mfd/axp20x.h | 1 + > 5 files changed, 109 insertions(+) > create mode 100644 drivers/mfd/axp20x-rsb.c > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > index 9ba3feb3f2fc..6e5edb61d42e 100644 > --- a/drivers/mfd/Kconfig > +++ b/drivers/mfd/Kconfig > @@ -84,6 +84,7 @@ config MFD_BCM590XX > config MFD_AXP20X > bool "X-Powers AXP series PMICs" > select MFD_AXP20X_I2C > + select MFD_AXP20X_RSB > > config MFD_AXP20X_CORE > bool > @@ -102,6 +103,17 @@ config MFD_AXP20X_I2C > components like regulators or the PEK (Power Enable Key) under the > corresponding menus. > > +config MFD_AXP20X_RSB > + bool "X-Powers AXP series RSB PMICs" > + select MFD_AXP20X_CORE > + depends on SUNXI_RSB=y Do we need that? Even if the bus is compiled as a module, the driver will not be probed before that, will it? > + help > + If you say Y here you get support for the X-Powers AXP series RSB > + based power management ICs (PMICs). > + This driver include only the core APIs. You have to select individual > + components like regulators or the PEK (Power Enable Key) under the > + corresponding menus. > + > config MFD_CROS_EC > tristate "ChromeOS Embedded Controller" > select MFD_CORE > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile > index ce3ad5fd4e2f..1eb278619dd6 100644 > --- a/drivers/mfd/Makefile > +++ b/drivers/mfd/Makefile > @@ -108,6 +108,7 @@ obj-$(CONFIG_MFD_DA9052_SPI) += da9052-spi.o > obj-$(CONFIG_MFD_DA9052_I2C) += da9052-i2c.o > obj-$(CONFIG_MFD_AXP20X_CORE) += axp20x-core.o > obj-$(CONFIG_MFD_AXP20X_I2C) += axp20x-i2c.o > +obj-$(CONFIG_MFD_AXP20X_RSB) += axp20x-rsb.o > > obj-$(CONFIG_MFD_LP3943) += lp3943.o > obj-$(CONFIG_MFD_LP8788) += lp8788.o lp8788-irq.o > diff --git a/drivers/mfd/axp20x-core.c b/drivers/mfd/axp20x-core.c > index dd33548d93c4..baecccb6d400 100644 > --- a/drivers/mfd/axp20x-core.c > +++ b/drivers/mfd/axp20x-core.c > @@ -32,6 +32,7 @@ static const char * const axp20x_model_names[] = { > "AXP202", > "AXP209", > "AXP221", > + "AXP223", > "AXP288", > }; > > @@ -575,6 +576,7 @@ int axp20x_match_device(struct axp20x_dev *axp20x, struct device *dev) > axp20x->regmap_irq_chip = &axp20x_regmap_irq_chip; > break; > case AXP221_ID: > + case AXP223_ID: > axp20x->nr_cells = ARRAY_SIZE(axp22x_cells); > axp20x->cells = axp22x_cells; > axp20x->regmap_cfg = &axp22x_regmap_config; > diff --git a/drivers/mfd/axp20x-rsb.c b/drivers/mfd/axp20x-rsb.c > new file mode 100644 > index 000000000000..5d053d177717 > --- /dev/null > +++ b/drivers/mfd/axp20x-rsb.c > @@ -0,0 +1,93 @@ > +/* > + * axp20x-rsb.c - RSB driver for the X-Powers' Power Management ICs > + * > + * AXP20x typically comprises an adaptive USB-Compatible PWM charger, BUCK DC-DC > + * converters, LDOs, multiple 12-bit ADCs of voltage, current and temperature > + * as well as configurable GPIOs. > + * > + * This driver supports the RSB variants. > + * > + * Author: Chen-Yu Tsai <wens@csie.org> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + */ > + > +#include <linux/acpi.h> > +#include <linux/err.h> > +#include <linux/module.h> > +#include <linux/mfd/axp20x.h> > +#include <linux/of_device.h> > +#include <linux/regmap.h> > +#include <linux/slab.h> > +#include <linux/soc/sunxi/sunxi_rsb.h> > + > +static const struct of_device_id axp20x_sunxi_rsb_of_match[] = { > + { .compatible = "x-powers,axp223", .data = (void *) AXP223_ID }, > + { }, > +}; > +MODULE_DEVICE_TABLE(of, axp20x_sunxi_rsb_of_match); > + > +static int axp20x_sunxi_rsb_match_device(struct axp20x_dev *axp20x, > + struct device *dev) > +{ > + const struct of_device_id *of_id; > + > + of_id = of_match_device(axp20x_sunxi_rsb_of_match, dev); > + if (!of_id) { > + dev_err(dev, "Unable to match OF ID\n"); > + return -ENODEV; > + } > + axp20x->variant = (long) of_id->data; > + > + return axp20x_match_device(axp20x, dev); > +} > + > +static int axp20x_sunxi_rsb_probe(struct sunxi_rsb_device *rdev) > +{ > + struct axp20x_dev *axp20x; > + int ret; > + > + axp20x = devm_kzalloc(&rdev->dev, sizeof(*axp20x), GFP_KERNEL); > + if (!axp20x) > + return -ENOMEM; > + > + ret = axp20x_sunxi_rsb_match_device(axp20x, &rdev->dev); > + if (ret) > + return ret; > + > + axp20x->dev = &rdev->dev; > + axp20x->irq = rdev->irq; > + sunxi_rsb_device_set_drvdata(rdev, axp20x); > + > + axp20x->regmap = devm_regmap_init_sunxi_rsb(rdev, axp20x->regmap_cfg); > + if (IS_ERR(axp20x->regmap)) { > + ret = PTR_ERR(axp20x->regmap); > + dev_err(&rdev->dev, "regmap init failed: %d\n", ret); > + return ret; > + } > + > + return axp20x_device_probe(axp20x); > +} > + > +static int axp20x_sunxi_rsb_remove(struct sunxi_rsb_device *rdev) > +{ > + struct axp20x_dev *axp20x = sunxi_rsb_device_get_drvdata(rdev); > + > + return axp20x_device_remove(axp20x); > +} > + > +static struct sunxi_rsb_driver axp20x_sunxi_rsb_driver = { > + .driver = { > + .name = "axp20x-sunxi-rsb", Do we need to be that verbose in the name of the driver? axp20x-rsb should be enough, especially since it's also the name of your file. Looks good otherwise, thanks! Maxime
On Fri, Oct 16, 2015 at 2:41 PM, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > On Thu, Oct 15, 2015 at 12:32:19AM +0800, Chen-Yu Tsai wrote: >> The AXP223 is a new PMIC commonly paired with Allwinner A23/A33 SoCs. >> It is functionally identical to AXP221; only the regulator default >> voltage/status and the external host interface are different. >> >> Signed-off-by: Chen-Yu Tsai <wens@csie.org> >> --- >> drivers/mfd/Kconfig | 12 ++++++ >> drivers/mfd/Makefile | 1 + >> drivers/mfd/axp20x-core.c | 2 + >> drivers/mfd/axp20x-rsb.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++ >> include/linux/mfd/axp20x.h | 1 + >> 5 files changed, 109 insertions(+) >> create mode 100644 drivers/mfd/axp20x-rsb.c >> >> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig >> index 9ba3feb3f2fc..6e5edb61d42e 100644 >> --- a/drivers/mfd/Kconfig >> +++ b/drivers/mfd/Kconfig >> @@ -84,6 +84,7 @@ config MFD_BCM590XX >> config MFD_AXP20X >> bool "X-Powers AXP series PMICs" >> select MFD_AXP20X_I2C >> + select MFD_AXP20X_RSB >> >> config MFD_AXP20X_CORE >> bool >> @@ -102,6 +103,17 @@ config MFD_AXP20X_I2C >> components like regulators or the PEK (Power Enable Key) under the >> corresponding menus. >> >> +config MFD_AXP20X_RSB >> + bool "X-Powers AXP series RSB PMICs" >> + select MFD_AXP20X_CORE >> + depends on SUNXI_RSB=y > > Do we need that? Even if the bus is compiled as a module, the driver > will not be probed before that, will it? There's a compile/link dependency on the __devm_regmap_init_sunxi_rsb(). And both drivers are bool, i.e. can't be compiled as a module. What we don't want is enabling MFD_AXP20X_RSB without SUNXI_RSB. AFAIK the same goes for the I2C version. >> + help >> + If you say Y here you get support for the X-Powers AXP series RSB >> + based power management ICs (PMICs). >> + This driver include only the core APIs. You have to select individual >> + components like regulators or the PEK (Power Enable Key) under the >> + corresponding menus. >> + >> config MFD_CROS_EC >> tristate "ChromeOS Embedded Controller" >> select MFD_CORE >> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile >> index ce3ad5fd4e2f..1eb278619dd6 100644 >> --- a/drivers/mfd/Makefile >> +++ b/drivers/mfd/Makefile >> @@ -108,6 +108,7 @@ obj-$(CONFIG_MFD_DA9052_SPI) += da9052-spi.o >> obj-$(CONFIG_MFD_DA9052_I2C) += da9052-i2c.o >> obj-$(CONFIG_MFD_AXP20X_CORE) += axp20x-core.o >> obj-$(CONFIG_MFD_AXP20X_I2C) += axp20x-i2c.o >> +obj-$(CONFIG_MFD_AXP20X_RSB) += axp20x-rsb.o >> >> obj-$(CONFIG_MFD_LP3943) += lp3943.o >> obj-$(CONFIG_MFD_LP8788) += lp8788.o lp8788-irq.o >> diff --git a/drivers/mfd/axp20x-core.c b/drivers/mfd/axp20x-core.c >> index dd33548d93c4..baecccb6d400 100644 >> --- a/drivers/mfd/axp20x-core.c >> +++ b/drivers/mfd/axp20x-core.c >> @@ -32,6 +32,7 @@ static const char * const axp20x_model_names[] = { >> "AXP202", >> "AXP209", >> "AXP221", >> + "AXP223", >> "AXP288", >> }; >> >> @@ -575,6 +576,7 @@ int axp20x_match_device(struct axp20x_dev *axp20x, struct device *dev) >> axp20x->regmap_irq_chip = &axp20x_regmap_irq_chip; >> break; >> case AXP221_ID: >> + case AXP223_ID: >> axp20x->nr_cells = ARRAY_SIZE(axp22x_cells); >> axp20x->cells = axp22x_cells; >> axp20x->regmap_cfg = &axp22x_regmap_config; >> diff --git a/drivers/mfd/axp20x-rsb.c b/drivers/mfd/axp20x-rsb.c >> new file mode 100644 >> index 000000000000..5d053d177717 >> --- /dev/null >> +++ b/drivers/mfd/axp20x-rsb.c >> @@ -0,0 +1,93 @@ >> +/* >> + * axp20x-rsb.c - RSB driver for the X-Powers' Power Management ICs >> + * >> + * AXP20x typically comprises an adaptive USB-Compatible PWM charger, BUCK DC-DC >> + * converters, LDOs, multiple 12-bit ADCs of voltage, current and temperature >> + * as well as configurable GPIOs. >> + * >> + * This driver supports the RSB variants. >> + * >> + * Author: Chen-Yu Tsai <wens@csie.org> >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License version 2 as >> + * published by the Free Software Foundation. >> + */ >> + >> +#include <linux/acpi.h> >> +#include <linux/err.h> >> +#include <linux/module.h> >> +#include <linux/mfd/axp20x.h> >> +#include <linux/of_device.h> >> +#include <linux/regmap.h> >> +#include <linux/slab.h> >> +#include <linux/soc/sunxi/sunxi_rsb.h> >> + >> +static const struct of_device_id axp20x_sunxi_rsb_of_match[] = { >> + { .compatible = "x-powers,axp223", .data = (void *) AXP223_ID }, >> + { }, >> +}; >> +MODULE_DEVICE_TABLE(of, axp20x_sunxi_rsb_of_match); >> + >> +static int axp20x_sunxi_rsb_match_device(struct axp20x_dev *axp20x, >> + struct device *dev) >> +{ >> + const struct of_device_id *of_id; >> + >> + of_id = of_match_device(axp20x_sunxi_rsb_of_match, dev); >> + if (!of_id) { >> + dev_err(dev, "Unable to match OF ID\n"); >> + return -ENODEV; >> + } >> + axp20x->variant = (long) of_id->data; >> + >> + return axp20x_match_device(axp20x, dev); >> +} >> + >> +static int axp20x_sunxi_rsb_probe(struct sunxi_rsb_device *rdev) >> +{ >> + struct axp20x_dev *axp20x; >> + int ret; >> + >> + axp20x = devm_kzalloc(&rdev->dev, sizeof(*axp20x), GFP_KERNEL); >> + if (!axp20x) >> + return -ENOMEM; >> + >> + ret = axp20x_sunxi_rsb_match_device(axp20x, &rdev->dev); >> + if (ret) >> + return ret; >> + >> + axp20x->dev = &rdev->dev; >> + axp20x->irq = rdev->irq; >> + sunxi_rsb_device_set_drvdata(rdev, axp20x); >> + >> + axp20x->regmap = devm_regmap_init_sunxi_rsb(rdev, axp20x->regmap_cfg); >> + if (IS_ERR(axp20x->regmap)) { >> + ret = PTR_ERR(axp20x->regmap); >> + dev_err(&rdev->dev, "regmap init failed: %d\n", ret); >> + return ret; >> + } >> + >> + return axp20x_device_probe(axp20x); >> +} >> + >> +static int axp20x_sunxi_rsb_remove(struct sunxi_rsb_device *rdev) >> +{ >> + struct axp20x_dev *axp20x = sunxi_rsb_device_get_drvdata(rdev); >> + >> + return axp20x_device_remove(axp20x); >> +} >> + >> +static struct sunxi_rsb_driver axp20x_sunxi_rsb_driver = { >> + .driver = { >> + .name = "axp20x-sunxi-rsb", > > Do we need to be that verbose in the name of the driver? > > axp20x-rsb should be enough, especially since it's also the name of > your file. Sure. > Looks good otherwise, thanks! Thanks! ChenYu
On Fri, Oct 16, 2015 at 02:46:23PM +0800, Chen-Yu Tsai wrote: > On Fri, Oct 16, 2015 at 2:41 PM, Maxime Ripard > <maxime.ripard@free-electrons.com> wrote: > > On Thu, Oct 15, 2015 at 12:32:19AM +0800, Chen-Yu Tsai wrote: > >> The AXP223 is a new PMIC commonly paired with Allwinner A23/A33 SoCs. > >> It is functionally identical to AXP221; only the regulator default > >> voltage/status and the external host interface are different. > >> > >> Signed-off-by: Chen-Yu Tsai <wens@csie.org> > >> --- > >> drivers/mfd/Kconfig | 12 ++++++ > >> drivers/mfd/Makefile | 1 + > >> drivers/mfd/axp20x-core.c | 2 + > >> drivers/mfd/axp20x-rsb.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++ > >> include/linux/mfd/axp20x.h | 1 + > >> 5 files changed, 109 insertions(+) > >> create mode 100644 drivers/mfd/axp20x-rsb.c > >> > >> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > >> index 9ba3feb3f2fc..6e5edb61d42e 100644 > >> --- a/drivers/mfd/Kconfig > >> +++ b/drivers/mfd/Kconfig > >> @@ -84,6 +84,7 @@ config MFD_BCM590XX > >> config MFD_AXP20X > >> bool "X-Powers AXP series PMICs" > >> select MFD_AXP20X_I2C > >> + select MFD_AXP20X_RSB > >> > >> config MFD_AXP20X_CORE > >> bool > >> @@ -102,6 +103,17 @@ config MFD_AXP20X_I2C > >> components like regulators or the PEK (Power Enable Key) under the > >> corresponding menus. > >> > >> +config MFD_AXP20X_RSB > >> + bool "X-Powers AXP series RSB PMICs" > >> + select MFD_AXP20X_CORE > >> + depends on SUNXI_RSB=y > > > > Do we need that? Even if the bus is compiled as a module, the driver > > will not be probed before that, will it? > > There's a compile/link dependency on the __devm_regmap_init_sunxi_rsb(). If it's exported, everything should be fine, no? > And both drivers are bool, i.e. can't be compiled as a module. What we > don't want is enabling MFD_AXP20X_RSB without SUNXI_RSB. What would really be the issue here? The driver wouldn't be probed, and that's it. Or am I missing something? Maxime > AFAIK the same goes for the I2C version.
On Mon, Oct 19, 2015 at 2:02 PM, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > On Fri, Oct 16, 2015 at 02:46:23PM +0800, Chen-Yu Tsai wrote: >> On Fri, Oct 16, 2015 at 2:41 PM, Maxime Ripard >> <maxime.ripard@free-electrons.com> wrote: >> > On Thu, Oct 15, 2015 at 12:32:19AM +0800, Chen-Yu Tsai wrote: >> >> The AXP223 is a new PMIC commonly paired with Allwinner A23/A33 SoCs. >> >> It is functionally identical to AXP221; only the regulator default >> >> voltage/status and the external host interface are different. >> >> >> >> Signed-off-by: Chen-Yu Tsai <wens@csie.org> >> >> --- >> >> drivers/mfd/Kconfig | 12 ++++++ >> >> drivers/mfd/Makefile | 1 + >> >> drivers/mfd/axp20x-core.c | 2 + >> >> drivers/mfd/axp20x-rsb.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++ >> >> include/linux/mfd/axp20x.h | 1 + >> >> 5 files changed, 109 insertions(+) >> >> create mode 100644 drivers/mfd/axp20x-rsb.c >> >> >> >> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig >> >> index 9ba3feb3f2fc..6e5edb61d42e 100644 >> >> --- a/drivers/mfd/Kconfig >> >> +++ b/drivers/mfd/Kconfig >> >> @@ -84,6 +84,7 @@ config MFD_BCM590XX >> >> config MFD_AXP20X >> >> bool "X-Powers AXP series PMICs" >> >> select MFD_AXP20X_I2C >> >> + select MFD_AXP20X_RSB >> >> >> >> config MFD_AXP20X_CORE >> >> bool >> >> @@ -102,6 +103,17 @@ config MFD_AXP20X_I2C >> >> components like regulators or the PEK (Power Enable Key) under the >> >> corresponding menus. >> >> >> >> +config MFD_AXP20X_RSB >> >> + bool "X-Powers AXP series RSB PMICs" >> >> + select MFD_AXP20X_CORE >> >> + depends on SUNXI_RSB=y >> > >> > Do we need that? Even if the bus is compiled as a module, the driver >> > will not be probed before that, will it? >> >> There's a compile/link dependency on the __devm_regmap_init_sunxi_rsb(). > > If it's exported, everything should be fine, no? > >> And both drivers are bool, i.e. can't be compiled as a module. What we >> don't want is enabling MFD_AXP20X_RSB without SUNXI_RSB. > > What would really be the issue here? The driver wouldn't be probed, > and that's it. Or am I missing something? The RSB bus / slave device functions have been merged into the RSB driver itself. Enabling MFD_AXP20X_RSB without enabling SUNXI_RSB means that RSB bus/device related functions are not compiled, i.e. link error: drivers/built-in.o: In function `axp20x_rsb_probe': /home/wens/sunxi/linux/drivers/mfd/axp20x-rsb.c:64: undefined reference to `__devm_regmap_init_sunxi_rsb' drivers/built-in.o: In function `axp20x_rsb_driver_init': /home/wens/sunxi/linux/drivers/mfd/axp20x-rsb.c:89: undefined reference to `sunxi_rsb_driver_register' Makefile:927: recipe for target 'vmlinux' failed The dependency is like "depends on I2C=y" for the I2C version. If you're asking about why "=y", I guess it's because MFD_AXP20X_RSB is bool, and if the depended on symbol is a tristate, which it actually is for I2c, we'd want it to be compiled in, and not built as a module, or again we'd get a undefined reference link error. Would it make sense to have SUNXI_RSB as a tristate symbol, i.e. can be built as a module? I'm nore sure. For multi-platform kernels, probably? Currently it isn't. Hope this clears it up. Regards ChenYu
On Mon, Oct 19, 2015 at 02:20:29PM +0800, Chen-Yu Tsai wrote: > On Mon, Oct 19, 2015 at 2:02 PM, Maxime Ripard > <maxime.ripard@free-electrons.com> wrote: > > On Fri, Oct 16, 2015 at 02:46:23PM +0800, Chen-Yu Tsai wrote: > >> On Fri, Oct 16, 2015 at 2:41 PM, Maxime Ripard > >> <maxime.ripard@free-electrons.com> wrote: > >> > On Thu, Oct 15, 2015 at 12:32:19AM +0800, Chen-Yu Tsai wrote: > >> >> The AXP223 is a new PMIC commonly paired with Allwinner A23/A33 SoCs. > >> >> It is functionally identical to AXP221; only the regulator default > >> >> voltage/status and the external host interface are different. > >> >> > >> >> Signed-off-by: Chen-Yu Tsai <wens@csie.org> > >> >> --- > >> >> drivers/mfd/Kconfig | 12 ++++++ > >> >> drivers/mfd/Makefile | 1 + > >> >> drivers/mfd/axp20x-core.c | 2 + > >> >> drivers/mfd/axp20x-rsb.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++ > >> >> include/linux/mfd/axp20x.h | 1 + > >> >> 5 files changed, 109 insertions(+) > >> >> create mode 100644 drivers/mfd/axp20x-rsb.c > >> >> > >> >> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > >> >> index 9ba3feb3f2fc..6e5edb61d42e 100644 > >> >> --- a/drivers/mfd/Kconfig > >> >> +++ b/drivers/mfd/Kconfig > >> >> @@ -84,6 +84,7 @@ config MFD_BCM590XX > >> >> config MFD_AXP20X > >> >> bool "X-Powers AXP series PMICs" > >> >> select MFD_AXP20X_I2C > >> >> + select MFD_AXP20X_RSB > >> >> > >> >> config MFD_AXP20X_CORE > >> >> bool > >> >> @@ -102,6 +103,17 @@ config MFD_AXP20X_I2C > >> >> components like regulators or the PEK (Power Enable Key) under the > >> >> corresponding menus. > >> >> > >> >> +config MFD_AXP20X_RSB > >> >> + bool "X-Powers AXP series RSB PMICs" > >> >> + select MFD_AXP20X_CORE > >> >> + depends on SUNXI_RSB=y > >> > > >> > Do we need that? Even if the bus is compiled as a module, the driver > >> > will not be probed before that, will it? > >> > >> There's a compile/link dependency on the __devm_regmap_init_sunxi_rsb(). > > > > If it's exported, everything should be fine, no? > > > >> And both drivers are bool, i.e. can't be compiled as a module. What we > >> don't want is enabling MFD_AXP20X_RSB without SUNXI_RSB. > > > > What would really be the issue here? The driver wouldn't be probed, > > and that's it. Or am I missing something? > > The RSB bus / slave device functions have been merged into the RSB driver > itself. Enabling MFD_AXP20X_RSB without enabling SUNXI_RSB means that RSB > bus/device related functions are not compiled, i.e. link error: > > drivers/built-in.o: In function `axp20x_rsb_probe': > /home/wens/sunxi/linux/drivers/mfd/axp20x-rsb.c:64: undefined > reference to `__devm_regmap_init_sunxi_rsb' > drivers/built-in.o: In function `axp20x_rsb_driver_init': > /home/wens/sunxi/linux/drivers/mfd/axp20x-rsb.c:89: undefined > reference to `sunxi_rsb_driver_register' > Makefile:927: recipe for target 'vmlinux' failed > > The dependency is like "depends on I2C=y" for the I2C version. > > If you're asking about why "=y", I guess it's because MFD_AXP20X_RSB is bool, > and if the depended on symbol is a tristate, which it actually is for I2c, > we'd want it to be compiled in, and not built as a module, or again we'd get > a undefined reference link error. Yeah, but my point was more why not have both the RSB driver and MFD as a module? The part where RSB is a module and the driver is statically built doesn't make sense (and I don't think a depends on allow that), but having both make sense. > Would it make sense to have SUNXI_RSB as a tristate symbol, i.e. can be built > as a module? I'm nore sure. For multi-platform kernels, probably? Currently it > isn't. Yes, it's better for multi-platform / distro kernels. Maxime
On Tue, Oct 20, 2015 at 2:48 AM, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > On Mon, Oct 19, 2015 at 02:20:29PM +0800, Chen-Yu Tsai wrote: >> On Mon, Oct 19, 2015 at 2:02 PM, Maxime Ripard >> <maxime.ripard@free-electrons.com> wrote: >> > On Fri, Oct 16, 2015 at 02:46:23PM +0800, Chen-Yu Tsai wrote: >> >> On Fri, Oct 16, 2015 at 2:41 PM, Maxime Ripard >> >> <maxime.ripard@free-electrons.com> wrote: >> >> > On Thu, Oct 15, 2015 at 12:32:19AM +0800, Chen-Yu Tsai wrote: >> >> >> The AXP223 is a new PMIC commonly paired with Allwinner A23/A33 SoCs. >> >> >> It is functionally identical to AXP221; only the regulator default >> >> >> voltage/status and the external host interface are different. >> >> >> >> >> >> Signed-off-by: Chen-Yu Tsai <wens@csie.org> >> >> >> --- >> >> >> drivers/mfd/Kconfig | 12 ++++++ >> >> >> drivers/mfd/Makefile | 1 + >> >> >> drivers/mfd/axp20x-core.c | 2 + >> >> >> drivers/mfd/axp20x-rsb.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++ >> >> >> include/linux/mfd/axp20x.h | 1 + >> >> >> 5 files changed, 109 insertions(+) >> >> >> create mode 100644 drivers/mfd/axp20x-rsb.c >> >> >> >> >> >> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig >> >> >> index 9ba3feb3f2fc..6e5edb61d42e 100644 >> >> >> --- a/drivers/mfd/Kconfig >> >> >> +++ b/drivers/mfd/Kconfig >> >> >> @@ -84,6 +84,7 @@ config MFD_BCM590XX >> >> >> config MFD_AXP20X >> >> >> bool "X-Powers AXP series PMICs" >> >> >> select MFD_AXP20X_I2C >> >> >> + select MFD_AXP20X_RSB >> >> >> >> >> >> config MFD_AXP20X_CORE >> >> >> bool >> >> >> @@ -102,6 +103,17 @@ config MFD_AXP20X_I2C >> >> >> components like regulators or the PEK (Power Enable Key) under the >> >> >> corresponding menus. >> >> >> >> >> >> +config MFD_AXP20X_RSB >> >> >> + bool "X-Powers AXP series RSB PMICs" >> >> >> + select MFD_AXP20X_CORE >> >> >> + depends on SUNXI_RSB=y >> >> > >> >> > Do we need that? Even if the bus is compiled as a module, the driver >> >> > will not be probed before that, will it? >> >> >> >> There's a compile/link dependency on the __devm_regmap_init_sunxi_rsb(). >> > >> > If it's exported, everything should be fine, no? >> > >> >> And both drivers are bool, i.e. can't be compiled as a module. What we >> >> don't want is enabling MFD_AXP20X_RSB without SUNXI_RSB. >> > >> > What would really be the issue here? The driver wouldn't be probed, >> > and that's it. Or am I missing something? >> >> The RSB bus / slave device functions have been merged into the RSB driver >> itself. Enabling MFD_AXP20X_RSB without enabling SUNXI_RSB means that RSB >> bus/device related functions are not compiled, i.e. link error: >> >> drivers/built-in.o: In function `axp20x_rsb_probe': >> /home/wens/sunxi/linux/drivers/mfd/axp20x-rsb.c:64: undefined >> reference to `__devm_regmap_init_sunxi_rsb' >> drivers/built-in.o: In function `axp20x_rsb_driver_init': >> /home/wens/sunxi/linux/drivers/mfd/axp20x-rsb.c:89: undefined >> reference to `sunxi_rsb_driver_register' >> Makefile:927: recipe for target 'vmlinux' failed >> >> The dependency is like "depends on I2C=y" for the I2C version. >> >> If you're asking about why "=y", I guess it's because MFD_AXP20X_RSB is bool, >> and if the depended on symbol is a tristate, which it actually is for I2c, >> we'd want it to be compiled in, and not built as a module, or again we'd get >> a undefined reference link error. > > Yeah, but my point was more why not have both the RSB driver and MFD > as a module? The part where RSB is a module and the driver is > statically built doesn't make sense (and I don't think a depends on > allow that), but having both make sense. Ok. I have no problem with building them as modules. I was just following what the original driver did. It seems half the mfd driver can be built as modules, while the other half can only be built-in. I don't know what the criteria is here. >> Would it make sense to have SUNXI_RSB as a tristate symbol, i.e. can be built >> as a module? I'm nore sure. For multi-platform kernels, probably? Currently it >> isn't. > > Yes, it's better for multi-platform / distro kernels. I guess I'll do a follow up patch for sunxi-rsb? Regards ChenYu
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig index 9ba3feb3f2fc..6e5edb61d42e 100644 --- a/drivers/mfd/Kconfig +++ b/drivers/mfd/Kconfig @@ -84,6 +84,7 @@ config MFD_BCM590XX config MFD_AXP20X bool "X-Powers AXP series PMICs" select MFD_AXP20X_I2C + select MFD_AXP20X_RSB config MFD_AXP20X_CORE bool @@ -102,6 +103,17 @@ config MFD_AXP20X_I2C components like regulators or the PEK (Power Enable Key) under the corresponding menus. +config MFD_AXP20X_RSB + bool "X-Powers AXP series RSB PMICs" + select MFD_AXP20X_CORE + depends on SUNXI_RSB=y + help + If you say Y here you get support for the X-Powers AXP series RSB + based power management ICs (PMICs). + This driver include only the core APIs. You have to select individual + components like regulators or the PEK (Power Enable Key) under the + corresponding menus. + config MFD_CROS_EC tristate "ChromeOS Embedded Controller" select MFD_CORE diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile index ce3ad5fd4e2f..1eb278619dd6 100644 --- a/drivers/mfd/Makefile +++ b/drivers/mfd/Makefile @@ -108,6 +108,7 @@ obj-$(CONFIG_MFD_DA9052_SPI) += da9052-spi.o obj-$(CONFIG_MFD_DA9052_I2C) += da9052-i2c.o obj-$(CONFIG_MFD_AXP20X_CORE) += axp20x-core.o obj-$(CONFIG_MFD_AXP20X_I2C) += axp20x-i2c.o +obj-$(CONFIG_MFD_AXP20X_RSB) += axp20x-rsb.o obj-$(CONFIG_MFD_LP3943) += lp3943.o obj-$(CONFIG_MFD_LP8788) += lp8788.o lp8788-irq.o diff --git a/drivers/mfd/axp20x-core.c b/drivers/mfd/axp20x-core.c index dd33548d93c4..baecccb6d400 100644 --- a/drivers/mfd/axp20x-core.c +++ b/drivers/mfd/axp20x-core.c @@ -32,6 +32,7 @@ static const char * const axp20x_model_names[] = { "AXP202", "AXP209", "AXP221", + "AXP223", "AXP288", }; @@ -575,6 +576,7 @@ int axp20x_match_device(struct axp20x_dev *axp20x, struct device *dev) axp20x->regmap_irq_chip = &axp20x_regmap_irq_chip; break; case AXP221_ID: + case AXP223_ID: axp20x->nr_cells = ARRAY_SIZE(axp22x_cells); axp20x->cells = axp22x_cells; axp20x->regmap_cfg = &axp22x_regmap_config; diff --git a/drivers/mfd/axp20x-rsb.c b/drivers/mfd/axp20x-rsb.c new file mode 100644 index 000000000000..5d053d177717 --- /dev/null +++ b/drivers/mfd/axp20x-rsb.c @@ -0,0 +1,93 @@ +/* + * axp20x-rsb.c - RSB driver for the X-Powers' Power Management ICs + * + * AXP20x typically comprises an adaptive USB-Compatible PWM charger, BUCK DC-DC + * converters, LDOs, multiple 12-bit ADCs of voltage, current and temperature + * as well as configurable GPIOs. + * + * This driver supports the RSB variants. + * + * Author: Chen-Yu Tsai <wens@csie.org> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include <linux/acpi.h> +#include <linux/err.h> +#include <linux/module.h> +#include <linux/mfd/axp20x.h> +#include <linux/of_device.h> +#include <linux/regmap.h> +#include <linux/slab.h> +#include <linux/soc/sunxi/sunxi_rsb.h> + +static const struct of_device_id axp20x_sunxi_rsb_of_match[] = { + { .compatible = "x-powers,axp223", .data = (void *) AXP223_ID }, + { }, +}; +MODULE_DEVICE_TABLE(of, axp20x_sunxi_rsb_of_match); + +static int axp20x_sunxi_rsb_match_device(struct axp20x_dev *axp20x, + struct device *dev) +{ + const struct of_device_id *of_id; + + of_id = of_match_device(axp20x_sunxi_rsb_of_match, dev); + if (!of_id) { + dev_err(dev, "Unable to match OF ID\n"); + return -ENODEV; + } + axp20x->variant = (long) of_id->data; + + return axp20x_match_device(axp20x, dev); +} + +static int axp20x_sunxi_rsb_probe(struct sunxi_rsb_device *rdev) +{ + struct axp20x_dev *axp20x; + int ret; + + axp20x = devm_kzalloc(&rdev->dev, sizeof(*axp20x), GFP_KERNEL); + if (!axp20x) + return -ENOMEM; + + ret = axp20x_sunxi_rsb_match_device(axp20x, &rdev->dev); + if (ret) + return ret; + + axp20x->dev = &rdev->dev; + axp20x->irq = rdev->irq; + sunxi_rsb_device_set_drvdata(rdev, axp20x); + + axp20x->regmap = devm_regmap_init_sunxi_rsb(rdev, axp20x->regmap_cfg); + if (IS_ERR(axp20x->regmap)) { + ret = PTR_ERR(axp20x->regmap); + dev_err(&rdev->dev, "regmap init failed: %d\n", ret); + return ret; + } + + return axp20x_device_probe(axp20x); +} + +static int axp20x_sunxi_rsb_remove(struct sunxi_rsb_device *rdev) +{ + struct axp20x_dev *axp20x = sunxi_rsb_device_get_drvdata(rdev); + + return axp20x_device_remove(axp20x); +} + +static struct sunxi_rsb_driver axp20x_sunxi_rsb_driver = { + .driver = { + .name = "axp20x-sunxi-rsb", + .of_match_table = of_match_ptr(axp20x_sunxi_rsb_of_match), + }, + .probe = axp20x_sunxi_rsb_probe, + .remove = axp20x_sunxi_rsb_remove, +}; +module_sunxi_rsb_driver(axp20x_sunxi_rsb_driver); + +MODULE_DESCRIPTION("PMIC MFD sunXi RSB driver for AXP20X"); +MODULE_AUTHOR("Chen-Yu Tsai <wens@csie.org>"); +MODULE_LICENSE("GPL v2"); diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h index 908f97f6e2d7..fef8dea18e66 100644 --- a/include/linux/mfd/axp20x.h +++ b/include/linux/mfd/axp20x.h @@ -18,6 +18,7 @@ enum { AXP202_ID, AXP209_ID, AXP221_ID, + AXP223_ID, AXP288_ID, NR_AXP20X_VARIANTS, };
The AXP223 is a new PMIC commonly paired with Allwinner A23/A33 SoCs. It is functionally identical to AXP221; only the regulator default voltage/status and the external host interface are different. Signed-off-by: Chen-Yu Tsai <wens@csie.org> --- drivers/mfd/Kconfig | 12 ++++++ drivers/mfd/Makefile | 1 + drivers/mfd/axp20x-core.c | 2 + drivers/mfd/axp20x-rsb.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++ include/linux/mfd/axp20x.h | 1 + 5 files changed, 109 insertions(+) create mode 100644 drivers/mfd/axp20x-rsb.c