Message ID | cover.1613031055.git.matti.vaittinen@fi.rohmeurope.com |
---|---|
Headers | show |
Series | Support ROHM BD9576MUF and BD9573MUF PMICs | expand |
On Thu, 11 Feb 2021, Matti Vaittinen wrote: > Add core support for ROHM BD9576MUF and BD9573MUF PMICs which are > mainly used to power the R-Car series processors. > > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> > --- > Changes: > - Comments fixed based on suggestions from Lee > - Name of regulator cell changed as suggested by Lee > - Renamed MFD cell variables for better readability > - Aligned header definitions for better readability > > drivers/mfd/Kconfig | 11 ++++ > drivers/mfd/Makefile | 1 + > drivers/mfd/rohm-bd9576.c | 109 +++++++++++++++++++++++++++++++ > include/linux/mfd/rohm-bd957x.h | 59 +++++++++++++++++ > include/linux/mfd/rohm-generic.h | 2 + > 5 files changed, 182 insertions(+) > create mode 100644 drivers/mfd/rohm-bd9576.c > create mode 100644 include/linux/mfd/rohm-bd957x.h > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > index bdfce7b15621..53c7c96283bd 100644 > --- a/drivers/mfd/Kconfig > +++ b/drivers/mfd/Kconfig > @@ -1998,6 +1998,17 @@ config MFD_ROHM_BD71828 > Also included is a Coulomb counter, a real-time clock (RTC), and > a 32.768 kHz clock gate. > > +config MFD_ROHM_BD957XMUF > + tristate "ROHM BD9576MUF and BD9573MUF Power Management ICs" > + depends on I2C=y > + depends on OF > + select REGMAP_I2C > + select MFD_CORE > + help > + Select this option to get support for the ROHM BD9576MUF and > + BD9573MUF Power Management ICs. BD9576 and BD9573 are primarily > + designed to be used to power R-Car series processors. > + > 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 14fdb188af02..e58fae024bb2 100644 > --- a/drivers/mfd/Makefile > +++ b/drivers/mfd/Makefile > @@ -262,6 +262,7 @@ 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_ROHM_BD957XMUF) += rohm-bd9576.o > obj-$(CONFIG_MFD_STMFX) += stmfx.o > obj-$(CONFIG_MFD_KHADAS_MCU) += khadas-mcu.o > > diff --git a/drivers/mfd/rohm-bd9576.c b/drivers/mfd/rohm-bd9576.c > new file mode 100644 > index 000000000000..efd439677c9e > --- /dev/null > +++ b/drivers/mfd/rohm-bd9576.c > @@ -0,0 +1,109 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Copyright (C) 2020 ROHM Semiconductors If you get a chance, could you please update these? > + * ROHM BD9576MUF and BD9573MUF PMIC driver > + */ For my own reference (apply this as-is to your sign-off block): Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
On Thu, 11 Feb 2021, Matti Vaittinen wrote: > BD9573 and BD9576 support set of "protection" interrupts for "fatal" > issues. Those lead to SOC reset as PMIC shuts the power outputs. Thus > there is no relevant IRQ handling for them. > > Few "detection" interrupts were added to the BD9576 with the idea that > SOC could take some recovery-action before error gets unrecoverable. > > Unfortunately the BD9576 interrupt logic was not re-evaluated. IRQs > are not designed to be properly acknowleged - and IRQ line is kept > active for whole duration of error condition (in comparison to > informing only about state change). > > For above reason, do not consider missing IRQ as error. > > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> > --- > Changes since v7: > - Do not fail probe is BD9573 IRQ information is populated > - Comment clean-up/clarifications as suggested by Lee > > drivers/mfd/rohm-bd9576.c | 80 ++++++++++++++++++++++++++++++++- > include/linux/mfd/rohm-bd957x.h | 62 +++++++++++++++++++++++++ > 2 files changed, 141 insertions(+), 1 deletion(-) > > diff --git a/drivers/mfd/rohm-bd9576.c b/drivers/mfd/rohm-bd9576.c > index efd439677c9e..6084f9a0aa1c 100644 > --- a/drivers/mfd/rohm-bd9576.c > +++ b/drivers/mfd/rohm-bd9576.c > @@ -17,12 +17,27 @@ > #include <linux/regmap.h> > #include <linux/types.h> > > +/* > + * Due to the BD9576MUF nasty IRQ behaiour we don't always populate IRQs. > + * These will be added to regulator resources only if IRQ information for the > + * PMIC is populated in device-tree. > + */ > +static const struct resource bd9576_regulator_irqs[] = { > + DEFINE_RES_IRQ_NAMED(BD9576_INT_THERM, "bd9576-temp"), > + DEFINE_RES_IRQ_NAMED(BD9576_INT_OVD, "bd9576-ovd"), > + DEFINE_RES_IRQ_NAMED(BD9576_INT_UVD, "bd9576-uvd"), > +}; > + > static struct mfd_cell bd9573_mfd_cells[] = { > { .name = "bd9573-regulator", }, > { .name = "bd9576-wdt", }, > }; > > static struct mfd_cell bd9576_mfd_cells[] = { > + /* > + * Please keep regulators as first cell as resources may be overwritten > + * from probe and the code expects regulators to be at index 0. > + */ Thought I'd mentioned this before (must have slipped my mind). This is not acceptable as it's much too fragile. Please use defined indexes to look into the array entries instead. [BD9576_REGULATOR] = { .name = "bd9576-regulator" } Etc. > { .name = "bd9576-regulator", }, > { .name = "bd9576-wdt", }, > }; > @@ -49,6 +64,29 @@ static struct regmap_config bd957x_regmap = { > .cache_type = REGCACHE_RBTREE, > }; > > +static struct regmap_irq bd9576_irqs[] = { > + REGMAP_IRQ_REG(BD9576_INT_THERM, 0, BD957X_MASK_INT_MAIN_THERM), > + REGMAP_IRQ_REG(BD9576_INT_OVP, 0, BD957X_MASK_INT_MAIN_OVP), > + REGMAP_IRQ_REG(BD9576_INT_SCP, 0, BD957X_MASK_INT_MAIN_SCP), > + REGMAP_IRQ_REG(BD9576_INT_OCP, 0, BD957X_MASK_INT_MAIN_OCP), > + REGMAP_IRQ_REG(BD9576_INT_OVD, 0, BD957X_MASK_INT_MAIN_OVD), > + REGMAP_IRQ_REG(BD9576_INT_UVD, 0, BD957X_MASK_INT_MAIN_UVD), > + REGMAP_IRQ_REG(BD9576_INT_UVP, 0, BD957X_MASK_INT_MAIN_UVP), > + REGMAP_IRQ_REG(BD9576_INT_SYS, 0, BD957X_MASK_INT_MAIN_SYS), > +}; > + > +static struct regmap_irq_chip bd9576_irq_chip = { > + .name = "bd9576_irq", > + .irqs = &bd9576_irqs[0], > + .num_irqs = ARRAY_SIZE(bd9576_irqs), > + .status_base = BD957X_REG_INT_MAIN_STAT, > + .mask_base = BD957X_REG_INT_MAIN_MASK, > + .ack_base = BD957X_REG_INT_MAIN_STAT, > + .init_ack_masked = true, > + .num_regs = 1, > + .irq_reg_stride = 1, > +}; > + > static int bd957x_i2c_probe(struct i2c_client *i2c, > const struct i2c_device_id *id) > { > @@ -57,6 +95,8 @@ static int bd957x_i2c_probe(struct i2c_client *i2c, > struct mfd_cell *cells; > int num_cells; > unsigned long chip_type; > + struct irq_domain *domain; > + bool usable_irqs; > > chip_type = (unsigned long)of_device_get_match_data(&i2c->dev); > > @@ -64,10 +104,16 @@ static int bd957x_i2c_probe(struct i2c_client *i2c, > case ROHM_CHIP_TYPE_BD9576: > cells = bd9576_mfd_cells; > num_cells = ARRAY_SIZE(bd9576_mfd_cells); > + usable_irqs = !!i2c->irq; > break; > case ROHM_CHIP_TYPE_BD9573: > cells = bd9573_mfd_cells; > num_cells = ARRAY_SIZE(bd9573_mfd_cells); > + /* > + * BD9573 only supports fatal IRQs which we can not handle > + * because SoC is going to lose the power. > + */ > + usable_irqs = false; > break; > default: > dev_err(&i2c->dev, "Unknown device type"); > @@ -79,9 +125,41 @@ static int bd957x_i2c_probe(struct i2c_client *i2c, > dev_err(&i2c->dev, "Failed to initialize Regmap\n"); > return PTR_ERR(regmap); > } Nit: '\n' > + /* > + * BD9576 behaves badly. It kepts IRQ line asserted for the whole > + * duration of detected HW condition (like over temperature). So we > + * don't require IRQ to be populated. > + * If IRQ information is not given, then we mask all IRQs and do not > + * provide IRQ resources to regulator driver - which then just omits > + * the notifiers. > + */ > + if (usable_irqs) { > + struct regmap_irq_chip_data *irq_data; > + struct mfd_cell *regulators = &bd9576_mfd_cells[0]; > + > + regulators->resources = bd9576_regulator_irqs; > + regulators->num_resources = ARRAY_SIZE(bd9576_regulator_irqs); > + > + ret = devm_regmap_add_irq_chip(&i2c->dev, regmap, i2c->irq, > + IRQF_ONESHOT, 0, > + &bd9576_irq_chip, &irq_data); > + if (ret) { > + dev_err(&i2c->dev, "Failed to add IRQ chip\n"); > + return ret; > + } > + domain = regmap_irq_get_domain(irq_data); > + dev_dbg(&i2c->dev, "Using IRQs for BD9576MUF\n"); Doesn't seem overly helpful. Please remove it. > + } else { > + ret = regmap_update_bits(regmap, BD957X_REG_INT_MAIN_MASK, > + BD957X_MASK_INT_ALL, > + BD957X_MASK_INT_ALL); > + if (ret) > + return ret; > + domain = NULL; > + } > > ret = devm_mfd_add_devices(&i2c->dev, PLATFORM_DEVID_AUTO, cells, > - num_cells, NULL, 0, NULL); > + num_cells, NULL, 0, domain); > if (ret) > dev_err(&i2c->dev, "Failed to create subdevices\n"); > > diff --git a/include/linux/mfd/rohm-bd957x.h b/include/linux/mfd/rohm-bd957x.h > index ae59c0f7188d..3f351a1ae4ab 100644 > --- a/include/linux/mfd/rohm-bd957x.h > +++ b/include/linux/mfd/rohm-bd957x.h > @@ -13,6 +13,55 @@ enum { > BD957X_VOUTS1, > }; > > +/* > + * The BD9576 has own IRQ 'blocks' for: > + * - I2C/thermal, > + * - Over voltage protection > + * - Short-circuit protection > + * - Over current protection > + * - Over voltage detection > + * - Under voltage detection > + * - Under voltage protection > + * - 'system interrupt'. > + * > + * Each of the blocks have a status register giving more accurate IRQ source > + * information - for example which of the regulators have over-voltage. > + * > + * On top of this, there is "main IRQ" status register where each bit indicates > + * which of sub-blocks have active IRQs. Fine. That would fit regmap-irq main > + * status handling. Except that: > + * - Only some sub-IRQs can be masked. > + * - The IRQ informs us about fault-condition, not when fault state changes. > + * The IRQ line it is kept asserted until the detected condition is acked > + * AND cleared in HW. This is annoying for IRQs like the one informing high > + * temperature because if IRQ is not disabled it keeps the CPU in IRQ > + * handling loop. > + * > + * For now we do just use the main-IRQ register as source for our IRQ > + * information and bind the regmap-irq to this. We leave fine-grained sub-IRQ > + * register handling to handlers in sub-devices. The regulator driver shall > + * read which regulators are source for problem - or if the detected error is > + * regulator temperature error. The sub-drivers do also handle masking of "sub- > + * IRQs" if this is supported/needed. > + * > + * To overcome the problem with HW keeping IRQ asserted we do call > + * disable_irq_nosync() from sub-device handler and add a delayed work to > + * re-enable IRQ roughly 1 second later. This should keep our CPU out of > + * busy-loop. > + */ > +#define IRQS_SILENT_MS 1000 > + > +enum { > + BD9576_INT_THERM, > + BD9576_INT_OVP, > + BD9576_INT_SCP, > + BD9576_INT_OCP, > + BD9576_INT_OVD, > + BD9576_INT_UVD, > + BD9576_INT_UVP, > + BD9576_INT_SYS, > +}; > + > #define BD957X_REG_SMRB_ASSERT 0x15 > #define BD957X_REG_PMIC_INTERNAL_STAT 0x20 > #define BD957X_REG_INT_THERM_STAT 0x23 > @@ -28,6 +77,19 @@ enum { > #define BD957X_REG_INT_MAIN_STAT 0x30 > #define BD957X_REG_INT_MAIN_MASK 0x31 > > +#define UVD_IRQ_VALID_MASK 0x6F > +#define OVD_IRQ_VALID_MASK 0x2F > + > +#define BD957X_MASK_INT_MAIN_THERM BIT(0) > +#define BD957X_MASK_INT_MAIN_OVP BIT(1) > +#define BD957X_MASK_INT_MAIN_SCP BIT(2) > +#define BD957X_MASK_INT_MAIN_OCP BIT(3) > +#define BD957X_MASK_INT_MAIN_OVD BIT(4) > +#define BD957X_MASK_INT_MAIN_UVD BIT(5) > +#define BD957X_MASK_INT_MAIN_UVP BIT(6) > +#define BD957X_MASK_INT_MAIN_SYS BIT(7) > +#define BD957X_MASK_INT_ALL 0xff > + > #define BD957X_REG_WDT_CONF 0x16 > > #define BD957X_REG_POW_TRIGGER1 0x41
On Thu, 11 Feb 2021, Matti Vaittinen wrote: > ROHM BD9576 contains safety features like over/under voltage detection, > over curren detection and over temperature detection. Add the > configuration register information. > > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> > --- > > Changes since v7: > New patch > > include/linux/mfd/rohm-bd957x.h | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/include/linux/mfd/rohm-bd957x.h b/include/linux/mfd/rohm-bd957x.h > index 3f351a1ae4ab..2bd35260ee8e 100644 > --- a/include/linux/mfd/rohm-bd957x.h > +++ b/include/linux/mfd/rohm-bd957x.h > @@ -116,6 +116,25 @@ enum { > #define BD957X_REG_VOUT4_TUNE 0x59 > #define BD957X_REG_VOUTL1_TUNE 0x5c > > +#define BD9576_REG_VOUT1_OVD 0x51 > +#define BD9576_REG_VOUT1_UVD 0x52 > +#define BD9576_REG_VOUT2_OVD 0x54 > +#define BD9576_REG_VOUT2_UVD 0x55 > +#define BD9576_REG_VOUT3_OVD 0x57 > +#define BD9576_REG_VOUT3_UVD 0x58 > +#define BD9576_REG_VOUT4_OVD 0x5a > +#define BD9576_REG_VOUT4_UVD 0x5b > +#define BD9576_REG_VOUTL1_OVD 0x5d > +#define BD9576_REG_VOUTL1_UVD 0x5e > + > +#define BD9576_MASK_XVD 0x7f > + > +#define BD9576_REG_VOUT1S_OCW 0x5f > +#define BD9576_REG_VOUT1S_OCP 0x60 > + > +#define BD9576_MASK_VOUT1S_OCW 0x3f > +#define BD9576_MASK_VOUT1S_OCP 0x3f Just a quick check, are these meant to be the same value? If so: For my own reference (apply this as-is to your sign-off block): Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
Hello Lee, On Mon, 2021-03-08 at 13:36 +0000, Lee Jones wrote: > On Thu, 11 Feb 2021, Matti Vaittinen wrote: > > > Add core support for ROHM BD9576MUF and BD9573MUF PMICs which are > > mainly used to power the R-Car series processors. > > > > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> > > --- > > Changes: > > - Comments fixed based on suggestions from Lee > > - Name of regulator cell changed as suggested by Lee > > - Renamed MFD cell variables for better readability > > - Aligned header definitions for better readability > > > > drivers/mfd/Kconfig | 11 ++++ > > drivers/mfd/Makefile | 1 + > > drivers/mfd/rohm-bd9576.c | 109 > > +++++++++++++++++++++++++++++++ > > include/linux/mfd/rohm-bd957x.h | 59 +++++++++++++++++ > > include/linux/mfd/rohm-generic.h | 2 + > > 5 files changed, 182 insertions(+) > > create mode 100644 drivers/mfd/rohm-bd9576.c > > create mode 100644 include/linux/mfd/rohm-bd957x.h > > > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > > index bdfce7b15621..53c7c96283bd 100644 > > --- a/drivers/mfd/Kconfig > > +++ b/drivers/mfd/Kconfig > > @@ -1998,6 +1998,17 @@ config MFD_ROHM_BD71828 > > Also included is a Coulomb counter, a real-time clock (RTC), > > and > > a 32.768 kHz clock gate. > > > > +config MFD_ROHM_BD957XMUF > > + tristate "ROHM BD9576MUF and BD9573MUF Power Management ICs" > > + depends on I2C=y > > + depends on OF > > + select REGMAP_I2C > > + select MFD_CORE > > + help > > + Select this option to get support for the ROHM BD9576MUF and > > + BD9573MUF Power Management ICs. BD9576 and BD9573 are > > primarily > > + designed to be used to power R-Car series processors. > > + > > 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 14fdb188af02..e58fae024bb2 100644 > > --- a/drivers/mfd/Makefile > > +++ b/drivers/mfd/Makefile > > @@ -262,6 +262,7 @@ 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_ROHM_BD957XMUF) += rohm-bd9576.o > > obj-$(CONFIG_MFD_STMFX) += stmfx.o > > obj-$(CONFIG_MFD_KHADAS_MCU) += khadas-mcu.o > > > > diff --git a/drivers/mfd/rohm-bd9576.c b/drivers/mfd/rohm-bd9576.c > > new file mode 100644 > > index 000000000000..efd439677c9e > > --- /dev/null > > +++ b/drivers/mfd/rohm-bd9576.c > > @@ -0,0 +1,109 @@ > > +// SPDX-License-Identifier: GPL-2.0-or-later > > +/* > > + * Copyright (C) 2020 ROHM Semiconductors > > If you get a chance, could you please update these? I'll respin the series as I'll add designated initializers for MFD regulator array cell as you suggested. So I'll update this at the same time. > > > + * ROHM BD9576MUF and BD9573MUF PMIC driver > > + */ > > For my own reference (apply this as-is to your sign-off block): > > Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org> >
On Mon, 2021-03-08 at 13:47 +0000, Lee Jones wrote: > On Thu, 11 Feb 2021, Matti Vaittinen wrote: > > > ROHM BD9576 contains safety features like over/under voltage > > detection, > > over curren detection and over temperature detection. Add the > > configuration register information. > > > > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> > > --- > > > > Changes since v7: > > New patch > > > > include/linux/mfd/rohm-bd957x.h | 19 +++++++++++++++++++ > > 1 file changed, 19 insertions(+) > > > > diff --git a/include/linux/mfd/rohm-bd957x.h > > b/include/linux/mfd/rohm-bd957x.h > > index 3f351a1ae4ab..2bd35260ee8e 100644 > > --- a/include/linux/mfd/rohm-bd957x.h > > +++ b/include/linux/mfd/rohm-bd957x.h > > @@ -116,6 +116,25 @@ enum { > > #define BD957X_REG_VOUT4_TUNE 0x59 > > #define BD957X_REG_VOUTL1_TUNE 0x5c > > > > +#define BD9576_REG_VOUT1_OVD 0x51 > > +#define BD9576_REG_VOUT1_UVD 0x52 > > +#define BD9576_REG_VOUT2_OVD 0x54 > > +#define BD9576_REG_VOUT2_UVD 0x55 > > +#define BD9576_REG_VOUT3_OVD 0x57 > > +#define BD9576_REG_VOUT3_UVD 0x58 > > +#define BD9576_REG_VOUT4_OVD 0x5a > > +#define BD9576_REG_VOUT4_UVD 0x5b > > +#define BD9576_REG_VOUTL1_OVD 0x5d > > +#define BD9576_REG_VOUTL1_UVD 0x5e > > + > > +#define BD9576_MASK_XVD 0x7f > > + > > +#define BD9576_REG_VOUT1S_OCW 0x5f > > +#define BD9576_REG_VOUT1S_OCP 0x60 > > + > > +#define BD9576_MASK_VOUT1S_OCW 0x3f > > +#define BD9576_MASK_VOUT1S_OCP 0x3f > > Just a quick check, are these meant to be the same value? Thanks for the check. They are both same value. > > If so: > > For my own reference (apply this as-is to your sign-off block): > > Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org> > Thanks, I'll add your ack. Best Regards Matti