| Message ID | cover.1576054779.git.matti.vaittinen@fi.rohmeurope.com |
|---|---|
| Headers | show |
| Series | Support ROHM BD71828 PMIC | expand |
Hi, I just realised the subject is missing a colon, it should be: rtc: bd70528: add BD71828 support Please fix it in case you ever have to resend for another reason. On 11/12/2019 11:48:35+0200, Matti Vaittinen wrote: > ROHM BD71828 PMIC RTC block is from many parts similar to one > on BD70528. Support BD71828 RTC using BD70528 RTC driver and > avoid re-inventing the wheel. > > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> > Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com> > > --- > Changes since v5: > - Added missing blank line > > drivers/rtc/Kconfig | 3 +- > drivers/rtc/rtc-bd70528.c | 168 ++++++++++++++++++++++++++++--- > include/linux/mfd/rohm-bd70528.h | 13 +-- > include/linux/mfd/rohm-bd71828.h | 4 +- > include/linux/mfd/rohm-shared.h | 27 +++++ > 5 files changed, 186 insertions(+), 29 deletions(-) > create mode 100644 include/linux/mfd/rohm-shared.h > > diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig > index d77515d8382c..df7a3843069d 100644 > --- a/drivers/rtc/Kconfig > +++ b/drivers/rtc/Kconfig > @@ -498,12 +498,13 @@ config RTC_DRV_M41T80_WDT > help > If you say Y here you will get support for the > watchdog timer in the ST M41T60 and M41T80 RTC chips series. > + > config RTC_DRV_BD70528 > tristate "ROHM BD70528 PMIC RTC" > depends on MFD_ROHM_BD70528 && (BD70528_WATCHDOG || !BD70528_WATCHDOG) > help > If you say Y here you will get support for the RTC > - on ROHM BD70528 Power Management IC. > + block on ROHM BD70528 and BD71828 Power Management IC. > > This driver can also be built as a module. If so, the module > will be called rtc-bd70528. > diff --git a/drivers/rtc/rtc-bd70528.c b/drivers/rtc/rtc-bd70528.c > index 627037aa66a8..2ce202040556 100644 > --- a/drivers/rtc/rtc-bd70528.c > +++ b/drivers/rtc/rtc-bd70528.c > @@ -6,6 +6,7 @@ > > #include <linux/bcd.h> > #include <linux/mfd/rohm-bd70528.h> > +#include <linux/mfd/rohm-bd71828.h> > #include <linux/module.h> > #include <linux/of.h> > #include <linux/platform_device.h> > @@ -15,7 +16,7 @@ > /* > * We read regs RTC_SEC => RTC_YEAR > * this struct is ordered according to chip registers. > - * Keep it u8 only to avoid padding issues. > + * Keep it u8 only (or packed) to avoid padding issues. > */ > struct bd70528_rtc_day { > u8 sec; > @@ -36,6 +37,13 @@ struct bd70528_rtc_wake { > u8 ctrl; > } __packed; > > +struct bd71828_rtc_alm { > + struct bd70528_rtc_data alm0; > + struct bd70528_rtc_data alm1; > + u8 alm_mask; > + u8 alm1_mask; > +} __packed; > + > struct bd70528_rtc_alm { > struct bd70528_rtc_data data; > u8 alm_mask; > @@ -45,6 +53,8 @@ struct bd70528_rtc_alm { > struct bd70528_rtc { > struct rohm_regmap_dev *mfd; > struct device *dev; > + u8 reg_time_start; > + bool has_rtc_timers; > }; > > static int bd70528_set_wake(struct rohm_regmap_dev *bd70528, > @@ -152,12 +162,18 @@ static int bd70528_set_rtc_based_timers(struct bd70528_rtc *r, int new_state, > static int bd70528_re_enable_rtc_based_timers(struct bd70528_rtc *r, > int old_state) > { > + if (!r->has_rtc_timers) > + return 0; > + > return bd70528_set_rtc_based_timers(r, old_state, NULL); > } > > static int bd70528_disable_rtc_based_timers(struct bd70528_rtc *r, > int *old_state) > { > + if (!r->has_rtc_timers) > + return 0; > + > return bd70528_set_rtc_based_timers(r, 0, old_state); > } > > @@ -213,6 +229,36 @@ static inline void rtc2tm(struct bd70528_rtc_data *r, struct rtc_time *t) > t->tm_wday = bcd2bin(r->week & BD70528_MASK_RTC_WEEK); > } > > +static int bd71828_set_alarm(struct device *dev, struct rtc_wkalrm *a) > +{ > + int ret; > + struct bd71828_rtc_alm alm; > + struct bd70528_rtc *r = dev_get_drvdata(dev); > + struct rohm_regmap_dev *bd71828 = r->mfd; > + > + ret = regmap_bulk_read(bd71828->regmap, BD71828_REG_RTC_ALM_START, > + &alm, sizeof(alm)); > + if (ret) { > + dev_err(dev, "Failed to read alarm regs\n"); > + return ret; > + } > + > + tm2rtc(&a->time, &alm.alm0); > + > + if (!a->enabled) > + alm.alm_mask &= ~BD70528_MASK_ALM_EN; > + else > + alm.alm_mask |= BD70528_MASK_ALM_EN; > + > + ret = regmap_bulk_write(bd71828->regmap, BD71828_REG_RTC_ALM_START, > + &alm, sizeof(alm)); > + if (ret) > + dev_err(dev, "Failed to set alarm time\n"); > + > + return ret; > + > +} > + > static int bd70528_set_alarm(struct device *dev, struct rtc_wkalrm *a) > { > struct bd70528_rtc_wake wake; > @@ -261,6 +307,30 @@ static int bd70528_set_alarm(struct device *dev, struct rtc_wkalrm *a) > return ret; > } > > +static int bd71828_read_alarm(struct device *dev, struct rtc_wkalrm *a) > +{ > + int ret; > + struct bd71828_rtc_alm alm; > + struct bd70528_rtc *r = dev_get_drvdata(dev); > + struct rohm_regmap_dev *bd71828 = r->mfd; > + > + ret = regmap_bulk_read(bd71828->regmap, BD71828_REG_RTC_ALM_START, > + &alm, sizeof(alm)); > + if (ret) { > + dev_err(dev, "Failed to read alarm regs\n"); > + return ret; > + } > + > + rtc2tm(&alm.alm0, &a->time); > + a->time.tm_mday = -1; > + a->time.tm_mon = -1; > + a->time.tm_year = -1; > + a->enabled = !!(alm.alm_mask & BD70528_MASK_ALM_EN); > + a->pending = 0; > + > + return 0; > +} > + > static int bd70528_read_alarm(struct device *dev, struct rtc_wkalrm *a) > { > struct bd70528_rtc_alm alm; > @@ -297,7 +367,7 @@ static int bd70528_set_time_locked(struct device *dev, struct rtc_time *t) > return ret; > > tmpret = regmap_bulk_read(bd70528->regmap, > - BD70528_REG_RTC_START, &rtc_data, > + r->reg_time_start, &rtc_data, > sizeof(rtc_data)); > if (tmpret) { > dev_err(dev, "Failed to read RTC time registers\n"); > @@ -306,7 +376,7 @@ static int bd70528_set_time_locked(struct device *dev, struct rtc_time *t) > tm2rtc(t, &rtc_data); > > tmpret = regmap_bulk_write(bd70528->regmap, > - BD70528_REG_RTC_START, &rtc_data, > + r->reg_time_start, &rtc_data, > sizeof(rtc_data)); > if (tmpret) { > dev_err(dev, "Failed to set RTC time\n"); > @@ -321,6 +391,11 @@ static int bd70528_set_time_locked(struct device *dev, struct rtc_time *t) > return ret; > } > > +static int bd71828_set_time(struct device *dev, struct rtc_time *t) > +{ > + return bd70528_set_time_locked(dev, t); > +} > + > static int bd70528_set_time(struct device *dev, struct rtc_time *t) > { > int ret; > @@ -341,7 +416,7 @@ static int bd70528_get_time(struct device *dev, struct rtc_time *t) > > /* read the RTC date and time registers all at once */ > ret = regmap_bulk_read(bd70528->regmap, > - BD70528_REG_RTC_START, &rtc_data, > + r->reg_time_start, &rtc_data, > sizeof(rtc_data)); > if (ret) { > dev_err(dev, "Failed to read RTC time (err %d)\n", ret); > @@ -378,6 +453,23 @@ static int bd70528_alm_enable(struct device *dev, unsigned int enabled) > return ret; > } > > +static int bd71828_alm_enable(struct device *dev, unsigned int enabled) > +{ > + int ret; > + struct bd70528_rtc *r = dev_get_drvdata(dev); > + unsigned int enableval = BD70528_MASK_ALM_EN; > + > + if (!enabled) > + enableval = 0; > + > + ret = regmap_update_bits(r->mfd->regmap, BD71828_REG_RTC_ALM0_MASK, > + BD70528_MASK_ALM_EN, enableval); > + if (ret) > + dev_err(dev, "Failed to change alarm state\n"); > + > + return ret; > +} > + > static const struct rtc_class_ops bd70528_rtc_ops = { > .read_time = bd70528_get_time, > .set_time = bd70528_set_time, > @@ -386,6 +478,14 @@ static const struct rtc_class_ops bd70528_rtc_ops = { > .alarm_irq_enable = bd70528_alm_enable, > }; > > +static const struct rtc_class_ops bd71828_rtc_ops = { > + .read_time = bd70528_get_time, > + .set_time = bd71828_set_time, > + .read_alarm = bd71828_read_alarm, > + .set_alarm = bd71828_set_alarm, > + .alarm_irq_enable = bd71828_alm_enable, > +}; > + > static irqreturn_t alm_hndlr(int irq, void *data) > { > struct rtc_device *rtc = data; > @@ -397,11 +497,16 @@ static irqreturn_t alm_hndlr(int irq, void *data) > static int bd70528_probe(struct platform_device *pdev) > { > struct bd70528_rtc *bd_rtc; > + const struct rtc_class_ops *rtc_ops; > struct rohm_regmap_dev *mfd; > + const char *irq_name; > int ret; > struct rtc_device *rtc; > int irq; > unsigned int hr; > + bool enable_main_irq = false; > + u8 hour_reg; > + enum rohm_chip_type chip = platform_get_device_id(pdev)->driver_data; > > mfd = dev_get_drvdata(pdev->dev.parent); > if (!mfd) { > @@ -415,13 +520,36 @@ static int bd70528_probe(struct platform_device *pdev) > bd_rtc->mfd = mfd; > bd_rtc->dev = &pdev->dev; > > - irq = platform_get_irq_byname(pdev, "bd70528-rtc-alm"); > - if (irq < 0) > + switch (chip) { > + case ROHM_CHIP_TYPE_BD70528: > + irq_name = "bd70528-rtc-alm"; > + bd_rtc->has_rtc_timers = true; > + bd_rtc->reg_time_start = BD70528_REG_RTC_START; > + hour_reg = BD70528_REG_RTC_HOUR; > + enable_main_irq = true; > + rtc_ops = &bd70528_rtc_ops; > + break; > + case ROHM_CHIP_TYPE_BD71828: > + irq_name = "bd71828-rtc-alm-0"; > + bd_rtc->reg_time_start = BD71828_REG_RTC_START; > + hour_reg = BD71828_REG_RTC_HOUR; > + rtc_ops = &bd71828_rtc_ops; > + break; > + default: > + dev_err(&pdev->dev, "Unknown chip\n"); > + return -ENOENT; > + } > + > + irq = platform_get_irq_byname(pdev, irq_name); > + > + if (irq < 0) { > + dev_err(&pdev->dev, "Failed to get irq\n"); > return irq; > + } > > platform_set_drvdata(pdev, bd_rtc); > > - ret = regmap_read(mfd->regmap, BD70528_REG_RTC_HOUR, &hr); > + ret = regmap_read(mfd->regmap, hour_reg, &hr); > > if (ret) { > dev_err(&pdev->dev, "Failed to reag RTC clock\n"); > @@ -431,10 +559,10 @@ static int bd70528_probe(struct platform_device *pdev) > if (!(hr & BD70528_MASK_RTC_HOUR_24H)) { > struct rtc_time t; > > - ret = bd70528_get_time(&pdev->dev, &t); > + ret = rtc_ops->read_time(&pdev->dev, &t); > > if (!ret) > - ret = bd70528_set_time(&pdev->dev, &t); > + ret = rtc_ops->set_time(&pdev->dev, &t); > > if (ret) { > dev_err(&pdev->dev, > @@ -454,7 +582,7 @@ static int bd70528_probe(struct platform_device *pdev) > > rtc->range_min = RTC_TIMESTAMP_BEGIN_2000; > rtc->range_max = RTC_TIMESTAMP_END_2099; > - rtc->ops = &bd70528_rtc_ops; > + rtc->ops = rtc_ops; > > /* Request alarm IRQ prior to registerig the RTC */ > ret = devm_request_threaded_irq(&pdev->dev, irq, NULL, &alm_hndlr, > @@ -468,27 +596,37 @@ static int bd70528_probe(struct platform_device *pdev) > * leave them enabled as irq-controller should disable irqs > * from sub-registers when IRQ is disabled or freed. > */ > - ret = regmap_update_bits(mfd->regmap, > + if (enable_main_irq) { > + ret = regmap_update_bits(mfd->regmap, > BD70528_REG_INT_MAIN_MASK, > BD70528_INT_RTC_MASK, 0); > - if (ret) { > - dev_err(&pdev->dev, "Failed to enable RTC interrupts\n"); > - return ret; > + if (ret) { > + dev_err(&pdev->dev, "Failed to enable RTC interrupts\n"); > + return ret; > + } > } > > return rtc_register_device(rtc); > } > > +static const struct platform_device_id bd718x7_rtc_id[] = { > + { "bd70528-rtc", ROHM_CHIP_TYPE_BD70528 }, > + { "bd71828-rtc", ROHM_CHIP_TYPE_BD71828 }, > + { }, > +}; > +MODULE_DEVICE_TABLE(platform, bd718x7_rtc_id); > + > static struct platform_driver bd70528_rtc = { > .driver = { > .name = "bd70528-rtc" > }, > .probe = bd70528_probe, > + .id_table = bd718x7_rtc_id, > }; > > module_platform_driver(bd70528_rtc); > > MODULE_AUTHOR("Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>"); > -MODULE_DESCRIPTION("BD70528 RTC driver"); > +MODULE_DESCRIPTION("ROHM BD70528 and BD71828 PMIC RTC driver"); > MODULE_LICENSE("GPL"); > MODULE_ALIAS("platform:bd70528-rtc"); > diff --git a/include/linux/mfd/rohm-bd70528.h b/include/linux/mfd/rohm-bd70528.h > index 2ad2320d0a96..a57af878fd0c 100644 > --- a/include/linux/mfd/rohm-bd70528.h > +++ b/include/linux/mfd/rohm-bd70528.h > @@ -7,6 +7,7 @@ > #include <linux/bits.h> > #include <linux/device.h> > #include <linux/mfd/rohm-generic.h> > +#include <linux/mfd/rohm-shared.h> > #include <linux/regmap.h> > > enum { > @@ -307,17 +308,6 @@ enum { > > /* RTC masks to mask out reserved bits */ > > -#define BD70528_MASK_RTC_SEC 0x7f > -#define BD70528_MASK_RTC_MINUTE 0x7f > -#define BD70528_MASK_RTC_HOUR_24H 0x80 > -#define BD70528_MASK_RTC_HOUR_PM 0x20 > -#define BD70528_MASK_RTC_HOUR 0x1f > -#define BD70528_MASK_RTC_DAY 0x3f > -#define BD70528_MASK_RTC_WEEK 0x07 > -#define BD70528_MASK_RTC_MONTH 0x1f > -#define BD70528_MASK_RTC_YEAR 0xff > -#define BD70528_MASK_RTC_COUNT_L 0x7f > - > #define BD70528_MASK_ELAPSED_TIMER_EN 0x1 > /* Mask second, min and hour fields > * HW would support ALM irq for over 24h > @@ -326,7 +316,6 @@ enum { > * wake-up we limit ALM to 24H and only > * unmask sec, min and hour > */ > -#define BD70528_MASK_ALM_EN 0x7 > #define BD70528_MASK_WAKE_EN 0x1 > > /* WDT masks */ > diff --git a/include/linux/mfd/rohm-bd71828.h b/include/linux/mfd/rohm-bd71828.h > index d013e03f742d..017a4c01cb31 100644 > --- a/include/linux/mfd/rohm-bd71828.h > +++ b/include/linux/mfd/rohm-bd71828.h > @@ -5,6 +5,7 @@ > #define __LINUX_MFD_BD71828_H__ > > #include <linux/mfd/rohm-generic.h> > +#include <linux/mfd/rohm-shared.h> > > /* Regulator IDs */ > enum { > @@ -160,6 +161,7 @@ enum { > #define BD71828_REG_RTC_YEAR 0x52 > > #define BD71828_REG_RTC_ALM0_SEC 0x53 > +#define BD71828_REG_RTC_ALM_START BD71828_REG_RTC_ALM0_SEC > #define BD71828_REG_RTC_ALM0_MINUTE 0x54 > #define BD71828_REG_RTC_ALM0_HOUR 0x55 > #define BD71828_REG_RTC_ALM0_WEEK 0x56 > @@ -178,6 +180,7 @@ enum { > #define BD71828_REG_RTC_ALM1_MASK 0x62 > > #define BD71828_REG_RTC_ALM2 0x63 > +#define BD71828_REG_RTC_START BD71828_REG_RTC_SEC > > /* Charger/Battey */ > #define BD71828_REG_CHG_STATE 0x65 > @@ -204,7 +207,6 @@ enum { > #define BD71828_REG_INT_MASK_TEMP 0xdd > #define BD71828_REG_INT_MASK_RTC 0xde > > - > #define BD71828_REG_INT_MAIN 0xdf > #define BD71828_REG_INT_BUCK 0xe0 > #define BD71828_REG_INT_DCIN1 0xe1 > diff --git a/include/linux/mfd/rohm-shared.h b/include/linux/mfd/rohm-shared.h > new file mode 100644 > index 000000000000..f16fc3b5000e > --- /dev/null > +++ b/include/linux/mfd/rohm-shared.h > @@ -0,0 +1,27 @@ > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > +/* Copyright (C) 2018 ROHM Semiconductors */ > + > + > +#ifndef __LINUX_MFD_ROHM_SHARED_H__ > +#define __LINUX_MFD_ROHM_SHARED_H__ > + > +/* > + * RTC definitions shared between > + * > + * BD70528 > + * and BD71828 > + */ > + > + > +#define BD70528_MASK_RTC_SEC 0x7f > +#define BD70528_MASK_RTC_MINUTE 0x7f > +#define BD70528_MASK_RTC_HOUR_24H 0x80 > +#define BD70528_MASK_RTC_HOUR_PM 0x20 > +#define BD70528_MASK_RTC_HOUR 0x3f > +#define BD70528_MASK_RTC_DAY 0x3f > +#define BD70528_MASK_RTC_WEEK 0x07 > +#define BD70528_MASK_RTC_MONTH 0x1f > +#define BD70528_MASK_RTC_YEAR 0xff > +#define BD70528_MASK_ALM_EN 0x7 > + > +#endif /* __LINUX_MFD_ROHM_SHARED_H__ */ > -- > 2.21.0 > > > -- > Matti Vaittinen, Linux device drivers > ROHM Semiconductors, Finland SWDC > Kiviharjunlenkki 1E > 90220 OULU > FINLAND > > ~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~ > Simon says - in Latin please. > ~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~ > Thanks to Simon Glass for the translation =]
Hello Alexandre, On Wed, 2019-12-11 at 12:38 +0100, Alexandre Belloni wrote: > Hi, > > I just realised the subject is missing a colon, it should be: > > rtc: bd70528: add BD71828 support Right. Thanks for pointing it out :) > Please fix it in case you ever have to resend for another reason. Will do :) Br, Matti Vaittinen
On Wed, Dec 11, 2019 at 10:47 AM Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> wrote: > Bunch of MFD sub-devices which are instantiated by MFD do not have > own device-tree nodes but have (for example) the GPIO consumer > information in parent device's DT node. Add resource managed > devm_gpiod_get_array() for such devices so that they can get the > consumer information from parent DT while still binding the GPIO > reservation life-time to this sub-device life time. > > If devm_gpiod_get_array is used as such - then unloading and then > re-loading the child device fails as the GPIOs reserved during first > load are not freed when driver for sub-device is unload (if parent > stays there). > > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> > --- > > Changes since v5: > - renamed internal function (no __ - prefixes for Linus :] ) Thanks, as there are things happening in the GPIO subsystem I have put this one patch on an immutable branch here: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git/log/?h=ib-devm-gpiod-get-parent-array Please ask the maintainer (I guess Lee?) to pull this into wherever the rest of the patches should be merged if you want patches beyond this point to be applied for the next (v5.6) merge window, then this patch is not needed in the series. Yours, Linus Walleij
On Wed, Dec 11, 2019 at 10:48 AM Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> wrote: > devm_gpiod_get_array and devm_gpiod_get_array_optional were missing > from the list. Add them. > > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> > --- > No changes sinnce v5 Patch applied. No need to merge this patch into MFD or other subsystem where the rest gets merged. Yours, Linus Walleij
On Wed, Dec 11, 2019 at 10:49 AM Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> wrote: > ROHM BD71828 PMIC contains 4 pins which can be configured by OTP > to be used for general purposes. First 3 can be used as outputs > and 4.th pin can be used as input. Allow them to be controlled > via GPIO framework. > > The driver assumes all of the pins are configured as GPIOs and > trusts that the reserved pins in other OTP configurations are > excluded from control using "gpio-reserved-ranges" device tree > property (or left untouched by GPIO users). > > Typical use for 4.th pin (input) is to use it as HALL sensor > input so that this pin state is toggled when HALL sensor detects > LID position change (from close to open or open to close). PMIC > HW implements some extra logic which allows PMIC to power-up the > system when this pin is toggled. Please see the data sheet for > details of GPIO options which can be selected by OTP settings. > > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> > Reviewed-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
Hi dee Ho peeps! (Linus & Lee + other interested), On Mon, 2019-12-16 at 09:29 +0100, Linus Walleij wrote: > On Wed, Dec 11, 2019 at 10:47 AM Matti Vaittinen > <matti.vaittinen@fi.rohmeurope.com> wrote: > > > Bunch of MFD sub-devices which are instantiated by MFD do not have > > own device-tree nodes but have (for example) the GPIO consumer > > information in parent device's DT node. Add resource managed > > devm_gpiod_get_array() for such devices so that they can get the > > consumer information from parent DT while still binding the GPIO > > reservation life-time to this sub-device life time. > > > > If devm_gpiod_get_array is used as such - then unloading and then > > re-loading the child device fails as the GPIOs reserved during > > first > > load are not freed when driver for sub-device is unload (if parent > > stays there). > > > > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> > > --- > > > > Changes since v5: > > - renamed internal function (no __ - prefixes for Linus :] ) > > Thanks, as there are things happening in the GPIO subsystem I > have put this one patch on an immutable branch here: > https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git/log/?h=ib-devm-gpiod-get-parent-array > > Please ask the maintainer (I guess Lee?) to pull this into wherever > the rest of the patches should be merged if you want patches beyond > this point to be applied for the next (v5.6) merge window, then this > patch is not needed in the series. I dropped the run-level support from regulator patch (for now at least). This means that I no longer have GPIO consumers needing this new API in the series. Which means two things: 1. There is no in-tree users of this new API as of now. This API has valid use-case immediately when an MFD sub-device which has no own (sub-device specific) compatible in DT wants to get the GPIO array and use devm - but I am not sure if we have any in-tree. (I checked current devm_gpiod_get_array() users and didn't see any MFD sub-devices which would have errorneously used the parent device for management - but I didn't check if there is any non-devm variant users that might benefit from this API) 2. There is no dependency from this series to the new API. So, there is two other options one can consider: 1. Drop this patch completely from the series and GPIO tree. 2. Apply it to GPIO tree only and drop it from this series (if this is still seen useful). Br, Matti
On Mon, Dec 16, 2019 at 9:59 AM Vaittinen, Matti <Matti.Vaittinen@fi.rohmeurope.com> wrote: > I dropped the run-level support from regulator patch (for now at > least). This means that I no longer have GPIO consumers needing this > new API in the series. OK I dropped it for now, we can add it when needed. Yours, Linus Walleij
On Wed, Dec 11, 2019 at 11:46:11AM +0200, Matti Vaittinen wrote: > +static int bd71828_ldo6_get_voltage(struct regulator_dev *rdev) > +{ > + return BD71828_LDO_6_VOLTAGE; > +} > + > +static const struct regulator_ops bd71828_ldo6_ops = { > + .enable = regulator_enable_regmap, > + .disable = regulator_disable_regmap, > + .get_voltage = bd71828_ldo6_get_voltage, You can just set fixed_uV in the regulator_desc, you don't need a get_voltage() operation here. Otherwise this looks good, I'll apply it and please send an incremental fix for this.
On Wed, 11 Dec 2019, Matti Vaittinen wrote: > Thanks to Stephen Boyd I today learned we can use platform_device_id > to do device and module matching for MFD sub-devices! > > Do device matching using the platform_device_id instead of using > explicit module_aliases to load modules and custom parent-data field > to do module loading and sub-device matching. > > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> > --- > > No changes since v5 > > drivers/clk/clk-bd718x7.c | 12 ++++++++- > drivers/regulator/bd718x7-regulator.c | 17 +++++++++--- > drivers/mfd/rohm-bd70528.c | 3 +-- > drivers/mfd/rohm-bd718x7.c | 39 ++++++++++++++++++++++----- > include/linux/mfd/rohm-generic.h | 3 +-- For my own reference: Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
On Wed, 11 Dec 2019, Matti Vaittinen wrote: > BD71828GW is a single-chip power management IC for battery-powered portable > devices. The IC integrates 7 buck converters, 7 LDOs, and a 1500 mA > single-cell linear charger. Also included is a Coulomb counter, a real-time > clock (RTC), 3 GPO/regulator control pins, HALL input and a 32.768 kHz > clock gate. > > Add MFD core driver providing interrupt controller facilities and i2c > access to sub device drivers. > > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> > --- > > Changes since v5: > - No changes > > drivers/mfd/Kconfig | 15 ++ > drivers/mfd/Makefile | 2 +- > drivers/mfd/rohm-bd71828.c | 319 +++++++++++++++++++++++ > include/linux/mfd/rohm-bd71828.h | 425 +++++++++++++++++++++++++++++++ > include/linux/mfd/rohm-generic.h | 1 + > 5 files changed, 761 insertions(+), 1 deletion(-) > create mode 100644 drivers/mfd/rohm-bd71828.c > create mode 100644 include/linux/mfd/rohm-bd71828.h Couple of small nits. Once fixed, please apply my: For my own reference: Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org> > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > index 420900852166..c3c9432ef51c 100644 > --- a/drivers/mfd/Kconfig > +++ b/drivers/mfd/Kconfig > @@ -1906,6 +1906,21 @@ config MFD_ROHM_BD70528 > 10 bits SAR ADC for battery temperature monitor and 1S battery > charger. > > +config MFD_ROHM_BD71828 > + tristate "ROHM BD71828 Power Management IC" > + depends on I2C=y > + depends on OF > + select REGMAP_I2C > + select REGMAP_IRQ > + select MFD_CORE > + help > + Select this option to get support for the ROHM BD71828 Power > + Management IC. BD71828GW is a single-chip power management IC for > + battery-powered portable devices. The IC integrates 7 buck > + converters, 7 LDOs, and a 1500 mA single-cell linear charger. > + Also included is a Coulomb counter, a real-time clock (RTC), and > + a 32.768 kHz clock gate. > + > config MFD_STM32_LPTIMER > tristate "Support for STM32 Low-Power Timer" > depends on (ARCH_STM32 && OF) || COMPILE_TEST > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile > index aed99f08739f..ca2d55c679c5 100644 > --- a/drivers/mfd/Makefile > +++ b/drivers/mfd/Makefile > @@ -252,6 +252,6 @@ obj-$(CONFIG_MFD_MXS_LRADC) += mxs-lradc.o > obj-$(CONFIG_MFD_SC27XX_PMIC) += sprd-sc27xx-spi.o > obj-$(CONFIG_RAVE_SP_CORE) += rave-sp.o > obj-$(CONFIG_MFD_ROHM_BD70528) += rohm-bd70528.o > +obj-$(CONFIG_MFD_ROHM_BD71828) += rohm-bd71828.o > obj-$(CONFIG_MFD_ROHM_BD718XX) += rohm-bd718x7.o > obj-$(CONFIG_MFD_STMFX) += stmfx.o > - Nit: This is an unrelated change and should not really be in this patch. > diff --git a/drivers/mfd/rohm-bd71828.c b/drivers/mfd/rohm-bd71828.c > new file mode 100644 > index 000000000000..7f445d699fd9 > --- /dev/null > +++ b/drivers/mfd/rohm-bd71828.c > @@ -0,0 +1,319 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +// > +// Copyright (C) 2019 ROHM Semiconductors > +// > +// ROHM BD71828 PMIC driver > + > +#include <linux/i2c.h> > +#include <linux/interrupt.h> > +#include <linux/ioport.h> > +#include <linux/irq.h> > +#include <linux/mfd/core.h> > +#include <linux/mfd/rohm-bd71828.h> > +#include <linux/module.h> > +#include <linux/of_device.h> > +#include <linux/regmap.h> > +#include <linux/types.h> > + > +static const struct resource rtc_irqs[] = { > + DEFINE_RES_IRQ_NAMED(BD71828_INT_RTC0, "bd71828-rtc-alm-0"), > + DEFINE_RES_IRQ_NAMED(BD71828_INT_RTC1, "bd71828-rtc-alm-1"), > + DEFINE_RES_IRQ_NAMED(BD71828_INT_RTC2, "bd71828-rtc-alm-2"), > +}; > + > +static struct mfd_cell bd71828_mfd_cells[] = { > + { .name = "bd71828-pmic", }, > + { .name = "bd71828-gpio", }, > + { .name = "bd71828-led", .of_compatible = "rohm,bd71828-leds" }, > + /* > + * We use BD71837 driver to drive the clock block. Only differences to > + * BD70528 clock gate are the register address and mask. > + */ > + { .name = "bd71828-clk", }, > + { .name = "bd71827-power", }, > + { > + .name = "bd71828-rtc", > + .resources = rtc_irqs, > + .num_resources = ARRAY_SIZE(rtc_irqs), > + }, > +}; > + > +static const struct regmap_range volatile_ranges[] = { > + { > + .range_min = BD71828_REG_PS_CTRL_1, > + .range_max = BD71828_REG_PS_CTRL_1, > + }, { > + .range_min = BD71828_REG_PS_CTRL_3, > + .range_max = BD71828_REG_PS_CTRL_3, > + }, { > + .range_min = BD71828_REG_RTC_SEC, > + .range_max = BD71828_REG_RTC_YEAR, > + }, { > + /* > + * For now make all charger registers volatile because many > + * needs to be and because the charger block is not that > + * performance critical. > + */ > + .range_min = BD71828_REG_CHG_STATE, > + .range_max = BD71828_REG_CHG_FULL, > + }, { > + .range_min = BD71828_REG_INT_MAIN, > + .range_max = BD71828_REG_IO_STAT, > + }, > +}; > + > +static const struct regmap_access_table volatile_regs = { > + .yes_ranges = &volatile_ranges[0], > + .n_yes_ranges = ARRAY_SIZE(volatile_ranges), > +}; > + > +static struct regmap_config bd71828_regmap = { > + .reg_bits = 8, > + .val_bits = 8, > + .volatile_table = &volatile_regs, > + .max_register = BD71828_MAX_REGISTER, > + .cache_type = REGCACHE_RBTREE, > +}; > + > +/* > + * Mapping of main IRQ register bits to sub-IRQ register offsets so that we can > + * access corect sub-IRQ registers based on bits that are set in main IRQ > + * register. > + */ > + > +static unsigned int bit0_offsets[] = {11}; /* RTC IRQ */ > +static unsigned int bit1_offsets[] = {10}; /* TEMP IRQ */ > +static unsigned int bit2_offsets[] = {6, 7, 8, 9}; /* BAT MON IRQ */ > +static unsigned int bit3_offsets[] = {5}; /* BAT IRQ */ > +static unsigned int bit4_offsets[] = {4}; /* CHG IRQ */ > +static unsigned int bit5_offsets[] = {3}; /* VSYS IRQ */ > +static unsigned int bit6_offsets[] = {1, 2}; /* DCIN IRQ */ > +static unsigned int bit7_offsets[] = {0}; /* BUCK IRQ */ > + > +static struct regmap_irq_sub_irq_map bd71828_sub_irq_offsets[] = { > + REGMAP_IRQ_MAIN_REG_OFFSET(bit0_offsets), > + REGMAP_IRQ_MAIN_REG_OFFSET(bit1_offsets), > + REGMAP_IRQ_MAIN_REG_OFFSET(bit2_offsets), > + REGMAP_IRQ_MAIN_REG_OFFSET(bit3_offsets), > + REGMAP_IRQ_MAIN_REG_OFFSET(bit4_offsets), > + REGMAP_IRQ_MAIN_REG_OFFSET(bit5_offsets), > + REGMAP_IRQ_MAIN_REG_OFFSET(bit6_offsets), > + REGMAP_IRQ_MAIN_REG_OFFSET(bit7_offsets), > +}; > + > +static struct regmap_irq bd71828_irqs[] = { > + REGMAP_IRQ_REG(BD71828_INT_BUCK1_OCP, 0, BD71828_INT_BUCK1_OCP_MASK), > + REGMAP_IRQ_REG(BD71828_INT_BUCK2_OCP, 0, BD71828_INT_BUCK2_OCP_MASK), > + REGMAP_IRQ_REG(BD71828_INT_BUCK3_OCP, 0, BD71828_INT_BUCK3_OCP_MASK), > + REGMAP_IRQ_REG(BD71828_INT_BUCK4_OCP, 0, BD71828_INT_BUCK4_OCP_MASK), > + REGMAP_IRQ_REG(BD71828_INT_BUCK5_OCP, 0, BD71828_INT_BUCK5_OCP_MASK), > + REGMAP_IRQ_REG(BD71828_INT_BUCK6_OCP, 0, BD71828_INT_BUCK6_OCP_MASK), > + REGMAP_IRQ_REG(BD71828_INT_BUCK7_OCP, 0, BD71828_INT_BUCK7_OCP_MASK), > + REGMAP_IRQ_REG(BD71828_INT_PGFAULT, 0, BD71828_INT_PGFAULT_MASK), > + /* DCIN1 interrupts */ > + REGMAP_IRQ_REG(BD71828_INT_DCIN_DET, 1, BD71828_INT_DCIN_DET_MASK), > + REGMAP_IRQ_REG(BD71828_INT_DCIN_RMV, 1, BD71828_INT_DCIN_RMV_MASK), > + REGMAP_IRQ_REG(BD71828_INT_CLPS_OUT, 1, BD71828_INT_CLPS_OUT_MASK), > + REGMAP_IRQ_REG(BD71828_INT_CLPS_IN, 1, BD71828_INT_CLPS_IN_MASK), > + /* DCIN2 interrupts */ > + REGMAP_IRQ_REG(BD71828_INT_DCIN_MON_RES, 2, > + BD71828_INT_DCIN_MON_RES_MASK), > + REGMAP_IRQ_REG(BD71828_INT_DCIN_MON_DET, 2, > + BD71828_INT_DCIN_MON_DET_MASK), > + REGMAP_IRQ_REG(BD71828_INT_LONGPUSH, 2, BD71828_INT_LONGPUSH_MASK), > + REGMAP_IRQ_REG(BD71828_INT_MIDPUSH, 2, BD71828_INT_MIDPUSH_MASK), > + REGMAP_IRQ_REG(BD71828_INT_SHORTPUSH, 2, BD71828_INT_SHORTPUSH_MASK), > + REGMAP_IRQ_REG(BD71828_INT_PUSH, 2, BD71828_INT_PUSH_MASK), > + REGMAP_IRQ_REG(BD71828_INT_WDOG, 2, BD71828_INT_WDOG_MASK), > + REGMAP_IRQ_REG(BD71828_INT_SWRESET, 2, BD71828_INT_SWRESET_MASK), > + /* Vsys */ > + REGMAP_IRQ_REG(BD71828_INT_VSYS_UV_RES, 3, > + BD71828_INT_VSYS_UV_RES_MASK), > + REGMAP_IRQ_REG(BD71828_INT_VSYS_UV_DET, 3, > + BD71828_INT_VSYS_UV_DET_MASK), > + REGMAP_IRQ_REG(BD71828_INT_VSYS_LOW_RES, 3, > + BD71828_INT_VSYS_LOW_RES_MASK), > + REGMAP_IRQ_REG(BD71828_INT_VSYS_LOW_DET, 3, > + BD71828_INT_VSYS_LOW_DET_MASK), > + REGMAP_IRQ_REG(BD71828_INT_VSYS_HALL_IN, 3, > + BD71828_INT_VSYS_HALL_IN_MASK), > + REGMAP_IRQ_REG(BD71828_INT_VSYS_HALL_TOGGLE, 3, > + BD71828_INT_VSYS_HALL_TOGGLE_MASK), > + REGMAP_IRQ_REG(BD71828_INT_VSYS_MON_RES, 3, > + BD71828_INT_VSYS_MON_RES_MASK), > + REGMAP_IRQ_REG(BD71828_INT_VSYS_MON_DET, 3, > + BD71828_INT_VSYS_MON_DET_MASK), > + /* Charger */ > + REGMAP_IRQ_REG(BD71828_INT_CHG_DCIN_ILIM, 4, > + BD71828_INT_CHG_DCIN_ILIM_MASK), > + REGMAP_IRQ_REG(BD71828_INT_CHG_TOPOFF_TO_DONE, 4, > + BD71828_INT_CHG_TOPOFF_TO_DONE_MASK), > + REGMAP_IRQ_REG(BD71828_INT_CHG_WDG_TEMP, 4, > + BD71828_INT_CHG_WDG_TEMP_MASK), > + REGMAP_IRQ_REG(BD71828_INT_CHG_WDG_TIME, 4, > + BD71828_INT_CHG_WDG_TIME_MASK), > + REGMAP_IRQ_REG(BD71828_INT_CHG_RECHARGE_RES, 4, > + BD71828_INT_CHG_RECHARGE_RES_MASK), > + REGMAP_IRQ_REG(BD71828_INT_CHG_RECHARGE_DET, 4, > + BD71828_INT_CHG_RECHARGE_DET_MASK), > + REGMAP_IRQ_REG(BD71828_INT_CHG_RANGED_TEMP_TRANSITION, 4, > + BD71828_INT_CHG_RANGED_TEMP_TRANSITION_MASK), > + REGMAP_IRQ_REG(BD71828_INT_CHG_STATE_TRANSITION, 4, > + BD71828_INT_CHG_STATE_TRANSITION_MASK), > + /* Battery */ > + REGMAP_IRQ_REG(BD71828_INT_BAT_TEMP_NORMAL, 5, > + BD71828_INT_BAT_TEMP_NORMAL_MASK), > + REGMAP_IRQ_REG(BD71828_INT_BAT_TEMP_ERANGE, 5, > + BD71828_INT_BAT_TEMP_ERANGE_MASK), > + REGMAP_IRQ_REG(BD71828_INT_BAT_TEMP_WARN, 5, > + BD71828_INT_BAT_TEMP_WARN_MASK), > + REGMAP_IRQ_REG(BD71828_INT_BAT_REMOVED, 5, > + BD71828_INT_BAT_REMOVED_MASK), > + REGMAP_IRQ_REG(BD71828_INT_BAT_DETECTED, 5, > + BD71828_INT_BAT_DETECTED_MASK), > + REGMAP_IRQ_REG(BD71828_INT_THERM_REMOVED, 5, > + BD71828_INT_THERM_REMOVED_MASK), > + REGMAP_IRQ_REG(BD71828_INT_THERM_DETECTED, 5, > + BD71828_INT_THERM_DETECTED_MASK), > + /* Battery Mon 1 */ > + REGMAP_IRQ_REG(BD71828_INT_BAT_DEAD, 6, BD71828_INT_BAT_DEAD_MASK), > + REGMAP_IRQ_REG(BD71828_INT_BAT_SHORTC_RES, 6, > + BD71828_INT_BAT_SHORTC_RES_MASK), > + REGMAP_IRQ_REG(BD71828_INT_BAT_SHORTC_DET, 6, > + BD71828_INT_BAT_SHORTC_DET_MASK), > + REGMAP_IRQ_REG(BD71828_INT_BAT_LOW_VOLT_RES, 6, > + BD71828_INT_BAT_LOW_VOLT_RES_MASK), > + REGMAP_IRQ_REG(BD71828_INT_BAT_LOW_VOLT_DET, 6, > + BD71828_INT_BAT_LOW_VOLT_DET_MASK), > + REGMAP_IRQ_REG(BD71828_INT_BAT_OVER_VOLT_RES, 6, > + BD71828_INT_BAT_OVER_VOLT_RES_MASK), > + REGMAP_IRQ_REG(BD71828_INT_BAT_OVER_VOLT_DET, 6, > + BD71828_INT_BAT_OVER_VOLT_DET_MASK), > + /* Battery Mon 2 */ > + REGMAP_IRQ_REG(BD71828_INT_BAT_MON_RES, 7, > + BD71828_INT_BAT_MON_RES_MASK), > + REGMAP_IRQ_REG(BD71828_INT_BAT_MON_DET, 7, > + BD71828_INT_BAT_MON_DET_MASK), > + /* Battery Mon 3 (Coulomb counter) */ > + REGMAP_IRQ_REG(BD71828_INT_BAT_CC_MON1, 8, > + BD71828_INT_BAT_CC_MON1_MASK), > + REGMAP_IRQ_REG(BD71828_INT_BAT_CC_MON2, 8, > + BD71828_INT_BAT_CC_MON2_MASK), > + REGMAP_IRQ_REG(BD71828_INT_BAT_CC_MON3, 8, > + BD71828_INT_BAT_CC_MON3_MASK), > + /* Battery Mon 4 */ > + REGMAP_IRQ_REG(BD71828_INT_BAT_OVER_CURR_1_RES, 9, > + BD71828_INT_BAT_OVER_CURR_1_RES_MASK), > + REGMAP_IRQ_REG(BD71828_INT_BAT_OVER_CURR_1_DET, 9, > + BD71828_INT_BAT_OVER_CURR_1_DET_MASK), > + REGMAP_IRQ_REG(BD71828_INT_BAT_OVER_CURR_2_RES, 9, > + BD71828_INT_BAT_OVER_CURR_2_RES_MASK), > + REGMAP_IRQ_REG(BD71828_INT_BAT_OVER_CURR_2_DET, 9, > + BD71828_INT_BAT_OVER_CURR_2_DET_MASK), > + REGMAP_IRQ_REG(BD71828_INT_BAT_OVER_CURR_3_RES, 9, > + BD71828_INT_BAT_OVER_CURR_3_RES_MASK), > + REGMAP_IRQ_REG(BD71828_INT_BAT_OVER_CURR_3_DET, 9, > + BD71828_INT_BAT_OVER_CURR_3_DET_MASK), > + /* Temperature */ > + REGMAP_IRQ_REG(BD71828_INT_TEMP_BAT_LOW_RES, 10, > + BD71828_INT_TEMP_BAT_LOW_RES_MASK), > + REGMAP_IRQ_REG(BD71828_INT_TEMP_BAT_LOW_DET, 10, > + BD71828_INT_TEMP_BAT_LOW_DET_MASK), > + REGMAP_IRQ_REG(BD71828_INT_TEMP_BAT_HI_RES, 10, > + BD71828_INT_TEMP_BAT_HI_RES_MASK), > + REGMAP_IRQ_REG(BD71828_INT_TEMP_BAT_HI_DET, 10, > + BD71828_INT_TEMP_BAT_HI_DET_MASK), > + REGMAP_IRQ_REG(BD71828_INT_TEMP_CHIP_OVER_125_RES, 10, > + BD71828_INT_TEMP_CHIP_OVER_125_RES_MASK), > + REGMAP_IRQ_REG(BD71828_INT_TEMP_CHIP_OVER_125_DET, 10, > + BD71828_INT_TEMP_CHIP_OVER_125_DET_MASK), > + REGMAP_IRQ_REG(BD71828_INT_TEMP_CHIP_OVER_VF_DET, 10, > + BD71828_INT_TEMP_CHIP_OVER_VF_DET_MASK), > + REGMAP_IRQ_REG(BD71828_INT_TEMP_CHIP_OVER_VF_RES, 10, > + BD71828_INT_TEMP_CHIP_OVER_VF_RES_MASK), > + /* RTC Alarm */ > + REGMAP_IRQ_REG(BD71828_INT_RTC0, 11, BD71828_INT_RTC0_MASK), > + REGMAP_IRQ_REG(BD71828_INT_RTC1, 11, BD71828_INT_RTC1_MASK), > + REGMAP_IRQ_REG(BD71828_INT_RTC2, 11, BD71828_INT_RTC2_MASK), > +}; > + > +static struct regmap_irq_chip bd71828_irq_chip = { > + .name = "bd71828_irq", > + .main_status = BD71828_REG_INT_MAIN, > + .irqs = &bd71828_irqs[0], > + .num_irqs = ARRAY_SIZE(bd71828_irqs), > + .status_base = BD71828_REG_INT_BUCK, > + .mask_base = BD71828_REG_INT_MASK_BUCK, > + .ack_base = BD71828_REG_INT_BUCK, > + .mask_invert = true, > + .init_ack_masked = true, > + .num_regs = 12, > + .num_main_regs = 1, > + .sub_reg_offsets = &bd71828_sub_irq_offsets[0], > + .num_main_status_bits = 8, > + .irq_reg_stride = 1, > +}; > + > +static int bd71828_i2c_probe(struct i2c_client *i2c) > +{ > + struct rohm_regmap_dev *chip; > + struct regmap_irq_chip_data *irq_data; > + int ret; > + > + if (!i2c->irq) { > + dev_err(&i2c->dev, "No IRQ configured\n"); > + return -EINVAL; > + } > + > + chip = devm_kzalloc(&i2c->dev, sizeof(*chip), GFP_KERNEL); > + if (!chip) > + return -ENOMEM; > + > + dev_set_drvdata(&i2c->dev, chip); > + > + chip->regmap = devm_regmap_init_i2c(i2c, &bd71828_regmap); > + if (IS_ERR(chip->regmap)) { > + dev_err(&i2c->dev, "Failed to initialize Regmap\n"); > + return PTR_ERR(chip->regmap); > + } > + > + ret = devm_regmap_add_irq_chip(&i2c->dev, chip->regmap, > + i2c->irq, IRQF_ONESHOT, 0, > + &bd71828_irq_chip, &irq_data); > + if (ret) { > + dev_err(&i2c->dev, "Failed to add IRQ chip\n"); > + return ret; > + } > + > + dev_dbg(&i2c->dev, "Registered %d IRQs for chip\n", > + bd71828_irq_chip.num_irqs); > + > + ret = devm_mfd_add_devices(&i2c->dev, PLATFORM_DEVID_AUTO, > + bd71828_mfd_cells, > + ARRAY_SIZE(bd71828_mfd_cells), NULL, 0, > + regmap_irq_get_domain(irq_data)); > + if (ret) > + dev_err(&i2c->dev, "Failed to create subdevices\n"); > + > + return ret; > +} > + > +static const struct of_device_id bd71828_of_match[] = { > + { .compatible = "rohm,bd71828", }, > + { }, > +}; > +MODULE_DEVICE_TABLE(of, bd71828_of_match); > + > +static struct i2c_driver bd71828_drv = { > + .driver = { > + .name = "rohm-bd71828", > + .of_match_table = bd71828_of_match, > + }, > + .probe_new = &bd71828_i2c_probe, > +}; > + Nit: You can remove this line. > +module_i2c_driver(bd71828_drv); > + > +MODULE_AUTHOR("Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>"); > +MODULE_DESCRIPTION("ROHM BD71828 Power Management IC driver"); > +MODULE_LICENSE("GPL"); This does not match the header.
On Wed, 11 Dec 2019, Matti Vaittinen wrote: > BD71828GW is a single-chip power management IC for battery-powered portable > devices. Add support for controlling BD71828 clk using bd718x7 driver. > > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> > --- > > No changes since v5 > > drivers/clk/Kconfig | 6 ++--- > drivers/clk/clk-bd718x7.c | 38 +++++++++++++++++++++++--------- > include/linux/mfd/rohm-bd70528.h | 6 ----- > include/linux/mfd/rohm-bd71828.h | 4 ---- > include/linux/mfd/rohm-bd718x7.h | 6 ----- For my own reference: Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
Thanks Mark, On Mon, 2019-12-16 at 14:55 +0000, Mark Brown wrote: > On Wed, Dec 11, 2019 at 11:46:11AM +0200, Matti Vaittinen wrote: > > > +static int bd71828_ldo6_get_voltage(struct regulator_dev *rdev) > > +{ > > + return BD71828_LDO_6_VOLTAGE; > > +} > > + > > +static const struct regulator_ops bd71828_ldo6_ops = { > > + .enable = regulator_enable_regmap, > > + .disable = regulator_disable_regmap, > > + .get_voltage = bd71828_ldo6_get_voltage, > > You can just set fixed_uV in the regulator_desc, you don't need a > get_voltage() operation here. Otherwise this looks good, I'll apply > it > and please send an incremental fix for this. Will do :) Br, Matti
Hello Mark, On Mon, 2019-12-16 at 14:55 +0000, Mark Brown wrote: > On Wed, Dec 11, 2019 at 11:46:11AM +0200, Matti Vaittinen wrote: > > > +static int bd71828_ldo6_get_voltage(struct regulator_dev *rdev) > > +{ > > + return BD71828_LDO_6_VOLTAGE; > > +} > > + > > +static const struct regulator_ops bd71828_ldo6_ops = { > > + .enable = regulator_enable_regmap, > > + .disable = regulator_disable_regmap, > > + .get_voltage = bd71828_ldo6_get_voltage, > > You can just set fixed_uV in the regulator_desc, you don't need a > get_voltage() operation here. Otherwise this looks good, I'll apply > it > and please send an incremental fix for this. Just to confirm - are you also taking in the [PATCH v6 08/15] regulator: bd718x7: Split driver to common and bd718x7 specific parts I think there is a dependency. (I am preparing next version of the series so I'll drop the already applied patches.) Br, Matti Vaittinen
Hello Lee, On Mon, 2019-12-16 at 16:46 +0000, Lee Jones wrote: > On Wed, 11 Dec 2019, Matti Vaittinen wrote: > > > BD71828GW is a single-chip power management IC for battery-powered > > portable > > devices. The IC integrates 7 buck converters, 7 LDOs, and a 1500 mA > > single-cell linear charger. Also included is a Coulomb counter, a > > real-time > > clock (RTC), 3 GPO/regulator control pins, HALL input and a 32.768 > > kHz > > clock gate. > > > > Add MFD core driver providing interrupt controller facilities and > > i2c > > access to sub device drivers. > > > > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> > > --- > > > > Changes since v5: > > - No changes > > > > drivers/mfd/Kconfig | 15 ++ > > drivers/mfd/Makefile | 2 +- > > drivers/mfd/rohm-bd71828.c | 319 +++++++++++++++++++++++ > > include/linux/mfd/rohm-bd71828.h | 425 > > +++++++++++++++++++++++++++++++ > > include/linux/mfd/rohm-generic.h | 1 + > > 5 files changed, 761 insertions(+), 1 deletion(-) > > create mode 100644 drivers/mfd/rohm-bd71828.c > > create mode 100644 include/linux/mfd/rohm-bd71828.h > > Couple of small nits. Once fixed, please apply my: > > For my own reference: > Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org> > > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > > index 420900852166..c3c9432ef51c 100644 > > --- a/drivers/mfd/Kconfig > > +++ b/drivers/mfd/Kconfig > > @@ -1906,6 +1906,21 @@ config MFD_ROHM_BD70528 > > 10 bits SAR ADC for battery temperature monitor and 1S > > battery > > charger. > > > > +config MFD_ROHM_BD71828 > > + tristate "ROHM BD71828 Power Management IC" > > + depends on I2C=y > > + depends on OF > > + select REGMAP_I2C > > + select REGMAP_IRQ > > + select MFD_CORE > > + help > > + Select this option to get support for the ROHM BD71828 Power > > + Management IC. BD71828GW is a single-chip power management IC > > for > > + battery-powered portable devices. The IC integrates 7 buck > > + converters, 7 LDOs, and a 1500 mA single-cell linear charger. > > + Also included is a Coulomb counter, a real-time clock (RTC), > > and > > + a 32.768 kHz clock gate. > > + > > config MFD_STM32_LPTIMER > > tristate "Support for STM32 Low-Power Timer" > > depends on (ARCH_STM32 && OF) || COMPILE_TEST > > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile > > index aed99f08739f..ca2d55c679c5 100644 > > --- a/drivers/mfd/Makefile > > +++ b/drivers/mfd/Makefile > > @@ -252,6 +252,6 @@ obj-$(CONFIG_MFD_MXS_LRADC) += mxs-lradc.o > > obj-$(CONFIG_MFD_SC27XX_PMIC) += sprd-sc27xx-spi.o > > obj-$(CONFIG_RAVE_SP_CORE) += rave-sp.o > > obj-$(CONFIG_MFD_ROHM_BD70528) += rohm-bd70528.o > > +obj-$(CONFIG_MFD_ROHM_BD71828) += rohm-bd71828.o > > obj-$(CONFIG_MFD_ROHM_BD718XX) += rohm-bd718x7.o > > obj-$(CONFIG_MFD_STMFX) += stmfx.o > > - > > Nit: This is an unrelated change and should not really be in this > patch. Ok. Will get rid of it. > > > diff --git a/drivers/mfd/rohm-bd71828.c b/drivers/mfd/rohm- > > bd71828.c > > new file mode 100644 > > index 000000000000..7f445d699fd9 > > --- /dev/null > > +++ b/drivers/mfd/rohm-bd71828.c > > @@ -0,0 +1,319 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +// > > +// Copyright (C) 2019 ROHM Semiconductors > > +// > > +// ROHM BD71828 PMIC driver > > + //snip > > + > > +static struct i2c_driver bd71828_drv = { > > + .driver = { > > + .name = "rohm-bd71828", > > + .of_match_table = bd71828_of_match, > > + }, > > + .probe_new = &bd71828_i2c_probe, > > +}; > > + > > Nit: You can remove this line. Will do. > > > +module_i2c_driver(bd71828_drv); > > + > > +MODULE_AUTHOR("Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> > > "); > > +MODULE_DESCRIPTION("ROHM BD71828 Power Management IC driver"); > > +MODULE_LICENSE("GPL"); > > This does not match the header. How is that? This is what is stated in module.h for the MODULE_LICENSE: /* * The following license idents are currently accepted as indicating free * software modules * * "GPL" [GNU Public License v2] * "GPL v2" [GNU Public License v2] * "GPL and additional rights" [GNU Public License v2 rights and more] * "Dual BSD/GPL" [GNU Public License v2 * or BSD license choice] * "Dual MIT/GPL" [GNU Public License v2 * or MIT license choice] * "Dual MPL/GPL" [GNU Public License v2 * or Mozilla license choice] * * The following other idents are available * * "Proprietary" [Non free products] * * Both "GPL v2" and "GPL" (the latter also in dual licensed strings) are * merely stating that the module is licensed under the GPL v2, but are not * telling whether "GPL v2 only" or "GPL v2 or later". The reason why there * are two variants is a historic and failed attempt to convey more * information in the MODULE_LICENSE string. For module loading the * "only/or later" distinction is completely irrelevant and does neither * replace the proper license identifiers in the corresponding source file * nor amends them in any way. The sole purpose is to make the * 'Proprietary' flagging work and to refuse to bind symbols which are * exported with EXPORT_SYMBOL_GPL when a non free module is loaded. * * In the same way "BSD" is not a clear license information. It merely * states, that the module is licensed under one of the compatible BSD * license variants. The detailed and correct license information is again * to be found in the corresponding source files. * * There are dual licensed components, but when running with Linux it is the * GPL that is relevant so this is a non issue. Similarly LGPL linked with GPL * is a GPL combined work. * * This exists for several reasons * 1. So modinfo can show license info for users wanting to vet their setup * is free * 2. So the community can ignore bug reports including proprietary modules * 3. So vendors can do likewise based on their own policies */ #define MODULE_LICENSE(_license) MODULE_INFO(license, _license) I have no objections on changing the license if needed but can you please tell me what is Ok combos then - I am having hard time when trying to select licenses which are acceptable for all. Br, Matti Vaittinen
On Tue, 17 Dec 2019, Vaittinen, Matti wrote: > Hello Lee, > > On Mon, 2019-12-16 at 16:46 +0000, Lee Jones wrote: > > On Wed, 11 Dec 2019, Matti Vaittinen wrote: > > > > > BD71828GW is a single-chip power management IC for battery-powered > > > portable > > > devices. The IC integrates 7 buck converters, 7 LDOs, and a 1500 mA > > > single-cell linear charger. Also included is a Coulomb counter, a > > > real-time > > > clock (RTC), 3 GPO/regulator control pins, HALL input and a 32.768 > > > kHz > > > clock gate. > > > > > > Add MFD core driver providing interrupt controller facilities and > > > i2c > > > access to sub device drivers. > > > > > > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> > > > --- > > > > > > Changes since v5: > > > - No changes > > > > > > drivers/mfd/Kconfig | 15 ++ > > > drivers/mfd/Makefile | 2 +- > > > drivers/mfd/rohm-bd71828.c | 319 +++++++++++++++++++++++ > > > include/linux/mfd/rohm-bd71828.h | 425 > > > +++++++++++++++++++++++++++++++ > > > include/linux/mfd/rohm-generic.h | 1 + > > > 5 files changed, 761 insertions(+), 1 deletion(-) > > > create mode 100644 drivers/mfd/rohm-bd71828.c > > > create mode 100644 include/linux/mfd/rohm-bd71828.h > > > > Couple of small nits. Once fixed, please apply my: > > > > For my own reference: > > Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org> > > > > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > > > index 420900852166..c3c9432ef51c 100644 > > > --- a/drivers/mfd/Kconfig > > > +++ b/drivers/mfd/Kconfig > > > @@ -1906,6 +1906,21 @@ config MFD_ROHM_BD70528 > > > 10 bits SAR ADC for battery temperature monitor and 1S > > > battery > > > charger. > > > > > > +config MFD_ROHM_BD71828 > > > + tristate "ROHM BD71828 Power Management IC" > > > + depends on I2C=y > > > + depends on OF > > > + select REGMAP_I2C > > > + select REGMAP_IRQ > > > + select MFD_CORE > > > + help > > > + Select this option to get support for the ROHM BD71828 Power > > > + Management IC. BD71828GW is a single-chip power management IC > > > for > > > + battery-powered portable devices. The IC integrates 7 buck > > > + converters, 7 LDOs, and a 1500 mA single-cell linear charger. > > > + Also included is a Coulomb counter, a real-time clock (RTC), > > > and > > > + a 32.768 kHz clock gate. > > > + > > > config MFD_STM32_LPTIMER > > > tristate "Support for STM32 Low-Power Timer" > > > depends on (ARCH_STM32 && OF) || COMPILE_TEST > > > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile > > > index aed99f08739f..ca2d55c679c5 100644 > > > --- a/drivers/mfd/Makefile > > > +++ b/drivers/mfd/Makefile > > > @@ -252,6 +252,6 @@ obj-$(CONFIG_MFD_MXS_LRADC) += mxs-lradc.o > > > obj-$(CONFIG_MFD_SC27XX_PMIC) += sprd-sc27xx-spi.o > > > obj-$(CONFIG_RAVE_SP_CORE) += rave-sp.o > > > obj-$(CONFIG_MFD_ROHM_BD70528) += rohm-bd70528.o > > > +obj-$(CONFIG_MFD_ROHM_BD71828) += rohm-bd71828.o > > > obj-$(CONFIG_MFD_ROHM_BD718XX) += rohm-bd718x7.o > > > obj-$(CONFIG_MFD_STMFX) += stmfx.o > > > - > > > > Nit: This is an unrelated change and should not really be in this > > patch. > > Ok. Will get rid of it. > > > > > > diff --git a/drivers/mfd/rohm-bd71828.c b/drivers/mfd/rohm- > > > bd71828.c > > > new file mode 100644 > > > index 000000000000..7f445d699fd9 > > > --- /dev/null > > > +++ b/drivers/mfd/rohm-bd71828.c > > > @@ -0,0 +1,319 @@ > > > +// SPDX-License-Identifier: GPL-2.0-only > > > +// > > > +// Copyright (C) 2019 ROHM Semiconductors > > > +// > > > +// ROHM BD71828 PMIC driver > > > + > > //snip > > > > + > > > +static struct i2c_driver bd71828_drv = { > > > + .driver = { > > > + .name = "rohm-bd71828", > > > + .of_match_table = bd71828_of_match, > > > + }, > > > + .probe_new = &bd71828_i2c_probe, > > > +}; > > > + > > > > Nit: You can remove this line. > > Will do. > > > > > > +module_i2c_driver(bd71828_drv); > > > + > > > +MODULE_AUTHOR("Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> > > > "); > > > +MODULE_DESCRIPTION("ROHM BD71828 Power Management IC driver"); > > > +MODULE_LICENSE("GPL"); > > > > This does not match the header. > > How is that? This is what is stated in module.h for the > MODULE_LICENSE: > > /* > * The following license idents are currently accepted as indicating > free > * software modules > * > * "GPL" [GNU Public License v2] > * "GPL v2" [GNU Public License v2] > * "GPL and additional rights" [GNU Public License v2 rights > and more] > * "Dual BSD/GPL" [GNU Public License v2 > * or BSD license choice] > * "Dual MIT/GPL" [GNU Public License v2 > * or MIT license choice] > * "Dual MPL/GPL" [GNU Public License v2 > * or Mozilla license choice] > * > * The following other idents are available > * > * "Proprietary" [Non free products] > * > * Both "GPL v2" and "GPL" (the latter also in dual licensed strings) > are > * merely stating that the module is licensed under the GPL v2, but are > not > * telling whether "GPL v2 only" or "GPL v2 or later". The reason why > there > * are two variants is a historic and failed attempt to convey more > * information in the MODULE_LICENSE string. For module loading the > * "only/or later" distinction is completely irrelevant and does > neither > * replace the proper license identifiers in the corresponding source > file > * nor amends them in any way. The sole purpose is to make the > * 'Proprietary' flagging work and to refuse to bind symbols which are > * exported with EXPORT_SYMBOL_GPL when a non free module is loaded. > * > * In the same way "BSD" is not a clear license information. It merely > * states, that the module is licensed under one of the compatible BSD > * license variants. The detailed and correct license information is > again > * to be found in the corresponding source files. > * > * There are dual licensed components, but when running with Linux it > is the > * GPL that is relevant so this is a non issue. Similarly LGPL linked > with GPL > * is a GPL combined work. > * > * This exists for several reasons > * 1. So modinfo can show license info for users wanting to vet their > setup > * is free > * 2. So the community can ignore bug reports including proprietary > modules > * 3. So vendors can do likewise based on their own policies > */ > #define MODULE_LICENSE(_license) MODULE_INFO(license, _license) > > I have no objections on changing the license if needed but can you > please tell me what is Ok combos then - I am having hard time when > trying to select licenses which are acceptable for all. If you have this in your header: GPL-2.0-only Your MODULE tags should read: MODULE_LICENSE("GPL v2");
On Tue, Dec 17, 2019 at 01:54:30PM +0000, Lee Jones wrote: > On Tue, 17 Dec 2019, Vaittinen, Matti wrote: > > > Hello Lee, > > > > On Mon, 2019-12-16 at 16:46 +0000, Lee Jones wrote: > > > On Wed, 11 Dec 2019, Matti Vaittinen wrote: > > > > > > > BD71828GW is a single-chip power management IC for battery-powered > > > > portable > > > > devices. The IC integrates 7 buck converters, 7 LDOs, and a 1500 mA > > > > single-cell linear charger. Also included is a Coulomb counter, a > > > > real-time > > > > clock (RTC), 3 GPO/regulator control pins, HALL input and a 32.768 > > > > kHz > > > > clock gate. > > > > > > > > Add MFD core driver providing interrupt controller facilities and > > > > i2c > > > > access to sub device drivers. > > > > > > > > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> > > > > --- > > > > > > > > Changes since v5: > > > > - No changes > > > > > > > > drivers/mfd/Kconfig | 15 ++ > > > > drivers/mfd/Makefile | 2 +- > > > > drivers/mfd/rohm-bd71828.c | 319 +++++++++++++++++++++++ > > > > include/linux/mfd/rohm-bd71828.h | 425 > > > > +++++++++++++++++++++++++++++++ > > > > include/linux/mfd/rohm-generic.h | 1 + > > > > 5 files changed, 761 insertions(+), 1 deletion(-) > > > > create mode 100644 drivers/mfd/rohm-bd71828.c > > > > create mode 100644 include/linux/mfd/rohm-bd71828.h > > > > > > Couple of small nits. Once fixed, please apply my: > > > > > > For my own reference: > > > Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org> > > > > > > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > > > > index 420900852166..c3c9432ef51c 100644 > > > > --- a/drivers/mfd/Kconfig > > > > +++ b/drivers/mfd/Kconfig > > > > @@ -1906,6 +1906,21 @@ config MFD_ROHM_BD70528 > > > > 10 bits SAR ADC for battery temperature monitor and 1S > > > > battery > > > > charger. > > > > > > > > +config MFD_ROHM_BD71828 > > > > + tristate "ROHM BD71828 Power Management IC" > > > > + depends on I2C=y > > > > + depends on OF > > > > + select REGMAP_I2C > > > > + select REGMAP_IRQ > > > > + select MFD_CORE > > > > + help > > > > + Select this option to get support for the ROHM BD71828 Power > > > > + Management IC. BD71828GW is a single-chip power management IC > > > > for > > > > + battery-powered portable devices. The IC integrates 7 buck > > > > + converters, 7 LDOs, and a 1500 mA single-cell linear charger. > > > > + Also included is a Coulomb counter, a real-time clock (RTC), > > > > and > > > > + a 32.768 kHz clock gate. > > > > + > > > > config MFD_STM32_LPTIMER > > > > tristate "Support for STM32 Low-Power Timer" > > > > depends on (ARCH_STM32 && OF) || COMPILE_TEST > > > > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile > > > > index aed99f08739f..ca2d55c679c5 100644 > > > > --- a/drivers/mfd/Makefile > > > > +++ b/drivers/mfd/Makefile > > > > @@ -252,6 +252,6 @@ obj-$(CONFIG_MFD_MXS_LRADC) += mxs-lradc.o > > > > obj-$(CONFIG_MFD_SC27XX_PMIC) += sprd-sc27xx-spi.o > > > > obj-$(CONFIG_RAVE_SP_CORE) += rave-sp.o > > > > obj-$(CONFIG_MFD_ROHM_BD70528) += rohm-bd70528.o > > > > +obj-$(CONFIG_MFD_ROHM_BD71828) += rohm-bd71828.o > > > > obj-$(CONFIG_MFD_ROHM_BD718XX) += rohm-bd718x7.o > > > > obj-$(CONFIG_MFD_STMFX) += stmfx.o > > > > - > > > > > > Nit: This is an unrelated change and should not really be in this > > > patch. > > > > Ok. Will get rid of it. > > > > > > > > > diff --git a/drivers/mfd/rohm-bd71828.c b/drivers/mfd/rohm- > > > > bd71828.c > > > > new file mode 100644 > > > > index 000000000000..7f445d699fd9 > > > > --- /dev/null > > > > +++ b/drivers/mfd/rohm-bd71828.c > > > > @@ -0,0 +1,319 @@ > > > > +// SPDX-License-Identifier: GPL-2.0-only > > > > +// > > > > +// Copyright (C) 2019 ROHM Semiconductors > > > > +// > > > > +// ROHM BD71828 PMIC driver > > > > + > > > > //snip > > > > > > + > > > > +static struct i2c_driver bd71828_drv = { > > > > + .driver = { > > > > + .name = "rohm-bd71828", > > > > + .of_match_table = bd71828_of_match, > > > > + }, > > > > + .probe_new = &bd71828_i2c_probe, > > > > +}; > > > > + > > > > > > Nit: You can remove this line. > > > > Will do. > > > > > > > > > +module_i2c_driver(bd71828_drv); > > > > + > > > > +MODULE_AUTHOR("Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> > > > > "); > > > > +MODULE_DESCRIPTION("ROHM BD71828 Power Management IC driver"); > > > > +MODULE_LICENSE("GPL"); > > > > > > This does not match the header. > > > > How is that? This is what is stated in module.h for the > > MODULE_LICENSE: > > > > /* > > * The following license idents are currently accepted as indicating > > free > > * software modules > > * > > * "GPL" [GNU Public License v2] > > * "GPL v2" [GNU Public License v2] > > * "GPL and additional rights" [GNU Public License v2 rights > > and more] > > * "Dual BSD/GPL" [GNU Public License v2 > > * or BSD license choice] > > * "Dual MIT/GPL" [GNU Public License v2 > > * or MIT license choice] > > * "Dual MPL/GPL" [GNU Public License v2 > > * or Mozilla license choice] > > * > > * The following other idents are available > > * > > * "Proprietary" [Non free products] > > * > > * Both "GPL v2" and "GPL" (the latter also in dual licensed strings) > > are > > * merely stating that the module is licensed under the GPL v2, but are > > not > > * telling whether "GPL v2 only" or "GPL v2 or later". The reason why > > there > > * are two variants is a historic and failed attempt to convey more > > * information in the MODULE_LICENSE string. For module loading the > > * "only/or later" distinction is completely irrelevant and does > > neither > > * replace the proper license identifiers in the corresponding source > > file > > * nor amends them in any way. The sole purpose is to make the > > * 'Proprietary' flagging work and to refuse to bind symbols which are > > * exported with EXPORT_SYMBOL_GPL when a non free module is loaded. > > * > > * In the same way "BSD" is not a clear license information. It merely > > * states, that the module is licensed under one of the compatible BSD > > * license variants. The detailed and correct license information is > > again > > * to be found in the corresponding source files. > > * > > * There are dual licensed components, but when running with Linux it > > is the > > * GPL that is relevant so this is a non issue. Similarly LGPL linked > > with GPL > > * is a GPL combined work. > > * > > * This exists for several reasons > > * 1. So modinfo can show license info for users wanting to vet their > > setup > > * is free > > * 2. So the community can ignore bug reports including proprietary > > modules > > * 3. So vendors can do likewise based on their own policies > > */ > > #define MODULE_LICENSE(_license) MODULE_INFO(license, _license) > > > > I have no objections on changing the license if needed but can you > > please tell me what is Ok combos then - I am having hard time when > > trying to select licenses which are acceptable for all. > > If you have this in your header: > > GPL-2.0-only > > Your MODULE tags should read: > > MODULE_LICENSE("GPL v2"); Nope, as per module.h, which is quoted here, either: MODULE_LICENSE("GPL"); or: MODULE_LICENSE("GPL v2"); mean the exact same thing. thanks, greg k-h
On Tue, 17 Dec 2019, gregkh@linuxfoundation.org wrote: > On Tue, Dec 17, 2019 at 01:54:30PM +0000, Lee Jones wrote: > > On Tue, 17 Dec 2019, Vaittinen, Matti wrote: > > > On Mon, 2019-12-16 at 16:46 +0000, Lee Jones wrote: > > > > On Wed, 11 Dec 2019, Matti Vaittinen wrote: > > > > > > > > > BD71828GW is a single-chip power management IC for battery-powered > > > > > portable > > > > > devices. The IC integrates 7 buck converters, 7 LDOs, and a 1500 mA > > > > > single-cell linear charger. Also included is a Coulomb counter, a > > > > > real-time > > > > > clock (RTC), 3 GPO/regulator control pins, HALL input and a 32.768 > > > > > kHz > > > > > clock gate. > > > > > > > > > > Add MFD core driver providing interrupt controller facilities and > > > > > i2c > > > > > access to sub device drivers. > > > > > > > > > > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> > > > > > --- > > > > > > > > > > Changes since v5: > > > > > - No changes > > > > > > > > > > drivers/mfd/Kconfig | 15 ++ > > > > > drivers/mfd/Makefile | 2 +- > > > > > drivers/mfd/rohm-bd71828.c | 319 +++++++++++++++++++++++ > > > > > include/linux/mfd/rohm-bd71828.h | 425 > > > > > +++++++++++++++++++++++++++++++ > > > > > include/linux/mfd/rohm-generic.h | 1 + > > > > > 5 files changed, 761 insertions(+), 1 deletion(-) > > > > > create mode 100644 drivers/mfd/rohm-bd71828.c > > > > > create mode 100644 include/linux/mfd/rohm-bd71828.h [...] > > > > If you have this in your header: > > > > GPL-2.0-only > > > > Your MODULE tags should read: > > > > MODULE_LICENSE("GPL v2"); > > Nope, as per module.h, which is quoted here, either: > MODULE_LICENSE("GPL"); > or: > MODULE_LICENSE("GPL v2"); > mean the exact same thing. Interesting. I always took a non-specified version to mean: "... and any other future version of the licence" Educated, thanks!
On Wed, Dec 11, 2019 at 11:44:51AM +0200, Matti Vaittinen wrote: > Few ROHM PMICs allow setting the voltage states for different system states > like RUN, IDLE, SUSPEND and LPSR. States are then changed via SoC specific > mechanisms. bd718x7 driver implemented device-tree parsing functions for > these state specific voltages. The parsing functions can be re-used by > other ROHM chip drivers like bd71828. Split the generic functions from > bd718x7-regulator.c to rohm-regulator.c and export them for other modules > to use. This doesn't apply against current code, please check and resend.
On Wed, Dec 11, 2019 at 11:44:51AM +0200, Matti Vaittinen wrote: > Few ROHM PMICs allow setting the voltage states for different system states > like RUN, IDLE, SUSPEND and LPSR. States are then changed via SoC specific > mechanisms. bd718x7 driver implemented device-tree parsing functions for > these state specific voltages. The parsing functions can be re-used by > other ROHM chip drivers like bd71828. Split the generic functions from > bd718x7-regulator.c to rohm-regulator.c and export them for other modules > to use. Acked-by: Mark Brown <broonie@kernel.org>
Patch series introducing support for ROHM BD71828 PMIC ROHM BD71828 is a power management IC containing 7 bucks and 7 LDOs. All regulators can be controlled individually via I2C. Bucks 1,2,6 and 7 can also be assigned to a "regulator group" controlled by run-levels. Eg. Run level specific voltages and enable/disable statuses for each of these bucks can be set via register interface. The buck run-level group assignment (selection if buck is to be controlled individually or via run-levels) can be changed at run-time via I2C. This patch series brings only the basic support for controlling regulators individually via I2C. In addition to the bucks and LDOs there are: - The usual clk gate - 4 IO pins (mostly usable as GPO or tied to specific purpose) - power button support - RTC - two LEDs - battery charger - HALL sensor input This patch series adds support to regulators, clk, RTC, GPIOs and LEDs. Power-supply driver for charger is not included in this series. The series also adds LED DT-node lookup based on node name or given property name/value pair in LED core. It also adds generic default-state and default-trigger property handling to LED core. Follow-up patches simplifying few other LED drivers should follow. In GPIO framework this series adds devm-support for gpio_array getting for MFD sub-devices whose GPIO consumer information may be in parent device's DT node. And while I was at it I also added few missing GPIO devm functions to the documentaton listing. Changelog v6: Rebased on top of v5.5-rc1 LED core: - Do new fw-node look-up only if the new match data is given. That way behaviour for existing drivers is not changed - Handle generic LED properties by core only if explisitly requested in init-data. That way behaviour for existing drivers is not changed until they are verified to work. BD71828 LEDs: - Fix module loading by adding "dummy" of_device_id table. DT bindings: All: - Remove regulator run-level properties as run-level support was dropped for now. - Change SPDX to dual lisence LED: - added select: false - replace oneOf + const by enum Regulator: - remove forgotten comments - comment indenting MFD: - remove unnecessary descriptions Regulators: - Dropped patch 12 with run-level controls - Dropped unnecessary ramp_delay_supported() - ram_delay ops were already only filled for DVS bucks. GPIO: - rename internal function. RTC: - Added missing blank line Changelog v5: Only LED patch (patch 15) changed, rest as in v4. LED: - Fixed issues reported by Dan Carpenter and kbuild-bot static analysis. Changelog v4 (first non RFC): General: - Changed subdevice loading and chip version identification to use platform ID. - License identifiers changed to GPL-2.0-only MFD: - Styling fixes mostly DT-Bindings: - a few more checks as suggested by Rob Herring. - Order of DT patches changed. - me as maintainer - standard units to new properties (microvolts, ohms) - runlevel values in an array LED: - BD71828 driver added (back) - Added DT support - Added LED DT node lookup in led framework when init_data is given with DT node match information. - Added common property parsing for default-state and default-trigger. Regulators: - dropped sysfs interfaces - fixed module unload/reload by binding gpio consumer information to regulator device not to MFD. GPIO: - Added devm_gpiod_get_parent_array - added few missing devm functions to documentation Changelog v3: DT-Bindings: - yamlify - add LED binding doc CLK: - Move clk register definitions from MFD headers to clk driver GPIO: - Add generic direction define and use it. LED: - Drop LED driver from the series (for now). Changelog v2: Mainly RTC and GPIO fixes suggested by Alexandre and Bartosz General: -Patch ordering changed to provide dt binding documents right after the MFD core. DT-Bindings for regulators (Patch 3) -Fix typo in PMIC model number RTC (patch 11) -Reverted renaming in order to reduce patch size. -Reworded commit message BD71828 regulator (patch 7) -Add MODULE_ALIAS GPIO (patch 12) -Remove file-name from comment -prefix IN and OUT defines with chip type -improved documentation for the INPUT only pin. -removed empty left-over function -removed unnecessary #ifdef CONFIG_OF_GPIO -removed unnecessary error print -Add MODULE_ALIAS Patch 1: dt-bindings for regulators on BD71828 PMIC Patch 2: dt-bindings for LEDs on BD71828 PMIC Patch 3: dt-bindings for BD71828 PMIC Patch 4: Convert rohm PMICs with common sub-devices to use platform_ device_id to match MFD sub-devices Patch 5: BD71828 MFD core. Patch 6: Power button support using GPIO keys. Patch 7: CLK gate support using existing clk-bd718x7 Patch 8: Split existing bd718x7 regulator driver to generic ROHM dt parsing portion (used by more than one ROHM drivers) and bd718x8 specific parts Patch 9: Basic regulator support (individual control via I2C). This should be pretty standard stuff. Patch 10: Add devm_gpiod_get_parent_array Patch 11: Add missing managed GPIO array get functions to documentation Patch 12: Support BD71828 RTC block using BD70528 RTC driver Patch 13: Allow control of GP(I)O pins on BD71828 via GPIO subsystem Patch 14: Add LED node lookup and common LED binding parsing support to LED class/core Patch 15: Support toggling the LEDs on BD71828. This patch series is based on v5.5-rc1 --- Matti Vaittinen (15): dt-bindings: regulator: Document ROHM BD71282 regulator bindings dt-bindings: leds: ROHM BD71282 PMIC LED driver dt-bindings: mfd: Document ROHM BD71828 bindings mfd: rohm PMICs - use platform_device_id to match MFD sub-devices mfd: bd71828: Support ROHM BD71828 PMIC - core mfd: input: bd71828: Add power-key support clk: bd718x7: Support ROHM BD71828 clk block regulator: bd718x7: Split driver to common and bd718x7 specific parts regulator: bd71828: Basic support for ROHM bd71828 PMIC regulators gpio: devres: Add devm_gpiod_get_parent_array docs: driver-model: Add missing managed GPIO array get functions rtc: bd70528 add BD71828 support gpio: bd71828: Initial support for ROHM BD71828 PMIC GPIOs leds: Add common LED binding parsing support to LED class/core led: bd71828: Support LED outputs on ROHM BD71828 PMIC .../bindings/leds/rohm,bd71828-leds.yaml | 52 ++ .../bindings/mfd/rohm,bd71828-pmic.yaml | 193 +++++ .../regulator/rohm,bd71828-regulator.yaml | 107 +++ .../driver-api/driver-model/devres.rst | 3 + drivers/clk/Kconfig | 6 +- drivers/clk/clk-bd718x7.c | 50 +- drivers/gpio/Kconfig | 12 + drivers/gpio/Makefile | 1 + drivers/gpio/gpio-bd71828.c | 159 ++++ drivers/gpio/gpiolib-devres.c | 65 +- drivers/leds/Kconfig | 10 + drivers/leds/Makefile | 1 + drivers/leds/led-class.c | 99 ++- drivers/leds/led-core.c | 258 +++++- drivers/leds/leds-bd71828.c | 118 +++ drivers/mfd/Kconfig | 15 + drivers/mfd/Makefile | 2 +- drivers/mfd/rohm-bd70528.c | 3 +- drivers/mfd/rohm-bd71828.c | 345 ++++++++ drivers/mfd/rohm-bd718x7.c | 39 +- drivers/regulator/Kconfig | 16 + drivers/regulator/Makefile | 2 + drivers/regulator/bd71828-regulator.c | 812 ++++++++++++++++++ drivers/regulator/bd718x7-regulator.c | 200 ++--- drivers/regulator/rohm-regulator.c | 95 ++ drivers/rtc/Kconfig | 3 +- drivers/rtc/rtc-bd70528.c | 168 +++- include/linux/gpio/consumer.h | 5 + include/linux/leds.h | 94 +- include/linux/mfd/rohm-bd70528.h | 19 +- include/linux/mfd/rohm-bd71828.h | 423 +++++++++ include/linux/mfd/rohm-bd718x7.h | 6 - include/linux/mfd/rohm-generic.h | 48 +- include/linux/mfd/rohm-shared.h | 27 + 34 files changed, 3177 insertions(+), 279 deletions(-) create mode 100644 Documentation/devicetree/bindings/leds/rohm,bd71828-leds.yaml create mode 100644 Documentation/devicetree/bindings/mfd/rohm,bd71828-pmic.yaml create mode 100644 Documentation/devicetree/bindings/regulator/rohm,bd71828-regulator.yaml create mode 100644 drivers/gpio/gpio-bd71828.c create mode 100644 drivers/leds/leds-bd71828.c create mode 100644 drivers/mfd/rohm-bd71828.c create mode 100644 drivers/regulator/bd71828-regulator.c create mode 100644 drivers/regulator/rohm-regulator.c create mode 100644 include/linux/mfd/rohm-bd71828.h create mode 100644 include/linux/mfd/rohm-shared.h