Support ROHM BD96801 Scalable PMIC

struct irq_domain *__irq_domain_add(struct fwnode_handle *fwnode, unsigned int size,

include/linux/irqdomain.h +1 -1

 				    irq_hw_number_t hwirq_max, int direct_max,
 				    const struct irq_domain_ops *ops,
-				    void *host_data);
+				    void *host_data, const char *name_suffix);
 struct irq_domain *irq_domain_create_simple(struct fwnode_handle *fwnode,
 					    unsigned int size,
 					    unsigned int first_irq,

Matti!

Quoted 15 lines · Matti Vaittinen

struct irq_domain *__irq_domain_add(struct fwnode_handle *fwnode, unsigned int size,
irq_hw_number_t hwirq_max, int direct_max, const struct irq_domain_ops *ops,

include/linux/irqdomain.h +3 -2

-				    void *host_data);
+				    void *host_data, const char *name_suffix);
  struct irq_domain *irq_domain_create_simple(struct fwnode_handle *fwnode,
  					    unsigned int size,
  					    unsigned int first_irq,

@@ -350,7 +350,8 @@ static inline struct irq_domain *irq_domain_add_linear(struct device_node *of_no
  					 const struct irq_domain_ops *ops,
  					 void *host_data)
  {
-	return __irq_domain_add(of_node_to_fwnode(of_node), size, size, 0, ops, host_data);
+	return __irq_domain_add(of_node_to_fwnode(of_node), size, size, 0, ops,
+				host_data, NULL);

....

Looking at the resulting amount of churn to add that argument, I'm not really enthused. There is some other unrelated change required in this area:

   https://lore.kernel.org/all/8734pr5yq1.ffs@tglx

My suggestion to convert all of this mess into a template based mechanism would nicely solve your problem too.

Can you please have a look and eventually team up with Herve (CC'ed) to sort this out? I'm happy to help and give guidance.

Herve, do you have any idea when you plan to do further sketching on this? Do you want me to try seeing if I can add the struct irq_domain_info and maybe use that in the __irq_domain_add() to get the name-suffix added? I might be able to send one version out during next week - but then I plan to be offline for couple of weeks ... so it may be I am not much of a help here.

Hi Matti,

On Fri, 7 Jun 2024 09:38:31 +0300
Matti Vaittinen <[email protected]> wrote:

...

Quoted 7 lines

Herve, do you have any idea when you plan to do further sketching on this? Do you want me to try seeing if I can add the struct irq_domain_info and maybe use that in the __irq_domain_add() to get the name-suffix added? I might be able to send one version out during next week - but then I plan to be offline for couple of weeks ... so it may be I am not much of a help here.

On my side, I plan to work on it next week too.
If you are off a couple of weeks after, I think I can start and move forward on this topic.

Quoted 15 lines · Herve Codina

Hi Matti,

On Fri, 7 Jun 2024 09:38:31 +0300
Matti Vaittinen <[email protected]> wrote:

...

Quoted 7 lines

Herve, do you have any idea when you plan to do further sketching on this? Do you want me to try seeing if I can add the struct irq_domain_info and maybe use that in the __irq_domain_add() to get the name-suffix added? I might be able to send one version out during next week - but then I plan to be offline for couple of weeks ... so it may be I am not much of a help here.

Quoted 3 lines

On my side, I plan to work on it next week too.
If you are off a couple of weeks after, I think I can start and move forward on this topic.

Thanks for the prompt reply and thanks for working with this :) I'll leave it to you for now then, as I don't think it makes much sense to intentionally do conflicting changes. I'll see what you've been up to when I return to the keyboard :) I'd appreciated if you added me to CC when sending the irqdomain refactoring patches (but I can dig them up if you don't).

Have fun!

Quoted 20 lines · On Fri, 7 Jun 2024 11:49:07 +0300 Matti Vaittinen <[email protected]>…

Quoted 18 lines · Herve Codina

Hi Matti,

On Fri, 7 Jun 2024 09:38:31 +0300
Matti Vaittinen <[email protected]> wrote:

...

Quoted 7 lines

Herve, do you have any idea when you plan to do further sketching on this? Do you want me to try seeing if I can add the struct irq_domain_info and maybe use that in the __irq_domain_add() to get the name-suffix added? I might be able to send one version out during next week - but then I plan to be offline for couple of weeks ... so it may be I am not much of a help here.

On my side, I plan to work on it next week too.
If you are off a couple of weeks after, I think I can start and move forward on this topic.

Quoted 6 lines

Thanks for the prompt reply and thanks for working with this :) I'll leave it to you for now then, as I don't think it makes much sense to intentionally do conflicting changes. I'll see what you've been up to when I return to the keyboard :) I'd appreciated if you added me to CC when sending the irqdomain refactoring patches (but I can dig them up if you don't).

Sure, you will CC you.

Also, I am not sure that I will perfectly take into account your use-case but it should not be a big deal to add it on top of my commits afterwards.

The ROHM BD96801 PMIC is highly customizable automotive grade PMIC which integrates regulator and watchdog funtionalities.

Provide INTB IRQ and register accesses for regulator/watchdog drivers.

Signed-off-by Matti Vaittinen <[email protected]>

---
Changelog:
v2 =>:
- No changes

v1 => v2:
- Drop unused enum
- Improve error prints
- improve comments

RFCv2 => v1:
- drop ERRB interrupts (for now)
- bd96801: Unlock registers in core driver

Changelog: RFCv1 => RFCv2
- Work-around the IRQ domain name conflict
- Add watchdog IRQ
- Various styling fixes based on review by Lee

---
 drivers/mfd/Kconfig              |  13 ++
 drivers/mfd/Makefile             |   1 +
 drivers/mfd/rohm-bd96801.c       | 273 +++++++++++++++++++++++++++++++
 include/linux/mfd/rohm-bd96801.h | 215 ++++++++++++++++++++++++
 include/linux/mfd/rohm-generic.h |   1 +
 5 files changed, 503 insertions(+)
 create mode 100644 drivers/mfd/rohm-bd96801.c
 create mode 100644 include/linux/mfd/rohm-bd96801.h

drivers/mfd/Kconfig +13 -0

index 266b4f54af60..68cc7af9ef97 100644
@@ -2089,6 +2089,19 @@ config MFD_ROHM_BD957XMUF
 	  BD9573MUF Power Management ICs. BD9576 and BD9573 are primarily
 	  designed to be used to power R-Car series processors.
 
+config MFD_ROHM_BD96801
+	tristate "ROHM BD96801 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 BD96801 Power
+	  Management IC. The ROHM BD96801 is a highly scalable Power Management
+	  IC for industrial and automotive use. The BD96801 can be used as a
+	  master PMIC in a chained PMIC solution with suitable companion PMICs.
+
 config MFD_STM32_LPTIMER
 	tristate "Support for STM32 Low-Power Timer"
 	depends on (ARCH_STM32 && OF) || COMPILE_TEST

drivers/mfd/Makefile +1 -0

index c66f07edcd0e..e792892d4a8b 100644
@@ -264,6 +264,7 @@ obj-$(CONFIG_RAVE_SP_CORE)	+= rave-sp.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_ROHM_BD96801)	+= rohm-bd96801.o
 obj-$(CONFIG_MFD_STMFX) 	+= stmfx.o
 obj-$(CONFIG_MFD_KHADAS_MCU) 	+= khadas-mcu.o
 obj-$(CONFIG_MFD_ACER_A500_EC)	+= acer-ec-a500.o

drivers/mfd/rohm-bd96801.c new file +37 -0

new file mode 100644
index 000000000000..1c2a9591be7b
@@ -0,0 +1,273 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2024 ROHM Semiconductors
+ *
+ * ROHM BD96801 PMIC driver
+ *
+ * This version of the "BD86801 scalable PMIC"'s driver supports only very
+ * basic set of the PMIC features. Most notably, there is no support for
+ * the ERRB interrupt and the configurations which should be done when the
+ * PMIC is in STBY mode.
+ *
+ * Supporting the ERRB interrupt would require dropping the regmap-IRQ
+ * usage or working around (or accepting a presense of) a naming conflict
+ * in debugFS IRQs.
+ *
+ * Being able to reliably do the configurations like changing the
+ * regulator safety limits (like limits for the over/under -voltages, over
+ * current, thermal protection) would require the configuring driver to be
+ * synchronized with entity causing the PMIC state transitions. Eg, one
+ * should be able to ensure the PMIC is in STBY state when the
+ * configurations are applied to the hardware. How and when the PMIC state
+ * transitions are to be done is likely to be very system specific, as will
+ * be the need to configure these safety limits. Hence it's not simple to
+ * come up with a generic solution.
+ *
+ * Users who require the ERRB handling and STBY state configurations can
+ * have a look at the original RFC:
+ * https://lore.kernel.org/all/[email protected]/
+ * which implements a workaround to debugFS naming conflict and some of
+ * the safety limit configurations - but leaves the state change handling
+ * and synchronization to be implemented.
+ *
+ * It would be great to hear (and receive a patch!) if you implement the
+ * STBY configuration support or a proper fix to the debugFS naming
+ * conflict in your downstream driver ;)
+ */
+

drivers/mfd/rohm-bd96801.c new file +39 -0

+static const struct resource regulator_intb_irqs[] = {
+	DEFINE_RES_IRQ_NAMED(BD96801_TW_STAT, "bd96801-core-thermal"),
+
+	DEFINE_RES_IRQ_NAMED(BD96801_BUCK1_OCPH_STAT, "bd96801-buck1-overcurr-h"),
+	DEFINE_RES_IRQ_NAMED(BD96801_BUCK1_OCPL_STAT, "bd96801-buck1-overcurr-l"),
+	DEFINE_RES_IRQ_NAMED(BD96801_BUCK1_OCPN_STAT, "bd96801-buck1-overcurr-n"),
+	DEFINE_RES_IRQ_NAMED(BD96801_BUCK1_OVD_STAT, "bd96801-buck1-overvolt"),
+	DEFINE_RES_IRQ_NAMED(BD96801_BUCK1_UVD_STAT, "bd96801-buck1-undervolt"),
+	DEFINE_RES_IRQ_NAMED(BD96801_BUCK1_TW_CH_STAT, "bd96801-buck1-thermal"),
+
+	DEFINE_RES_IRQ_NAMED(BD96801_BUCK2_OCPH_STAT, "bd96801-buck2-overcurr-h"),
+	DEFINE_RES_IRQ_NAMED(BD96801_BUCK2_OCPL_STAT, "bd96801-buck2-overcurr-l"),
+	DEFINE_RES_IRQ_NAMED(BD96801_BUCK2_OCPN_STAT, "bd96801-buck2-overcurr-n"),
+	DEFINE_RES_IRQ_NAMED(BD96801_BUCK2_OVD_STAT, "bd96801-buck2-overvolt"),
+	DEFINE_RES_IRQ_NAMED(BD96801_BUCK2_UVD_STAT, "bd96801-buck2-undervolt"),
+	DEFINE_RES_IRQ_NAMED(BD96801_BUCK2_TW_CH_STAT, "bd96801-buck2-thermal"),
+
+	DEFINE_RES_IRQ_NAMED(BD96801_BUCK3_OCPH_STAT, "bd96801-buck3-overcurr-h"),
+	DEFINE_RES_IRQ_NAMED(BD96801_BUCK3_OCPL_STAT, "bd96801-buck3-overcurr-l"),
+	DEFINE_RES_IRQ_NAMED(BD96801_BUCK3_OCPN_STAT, "bd96801-buck3-overcurr-n"),
+	DEFINE_RES_IRQ_NAMED(BD96801_BUCK3_OVD_STAT, "bd96801-buck3-overvolt"),
+	DEFINE_RES_IRQ_NAMED(BD96801_BUCK3_UVD_STAT, "bd96801-buck3-undervolt"),
+	DEFINE_RES_IRQ_NAMED(BD96801_BUCK3_TW_CH_STAT, "bd96801-buck3-thermal"),
+
+	DEFINE_RES_IRQ_NAMED(BD96801_BUCK4_OCPH_STAT, "bd96801-buck4-overcurr-h"),
+	DEFINE_RES_IRQ_NAMED(BD96801_BUCK4_OCPL_STAT, "bd96801-buck4-overcurr-l"),
+	DEFINE_RES_IRQ_NAMED(BD96801_BUCK4_OCPN_STAT, "bd96801-buck4-overcurr-n"),
+	DEFINE_RES_IRQ_NAMED(BD96801_BUCK4_OVD_STAT, "bd96801-buck4-overvolt"),
+	DEFINE_RES_IRQ_NAMED(BD96801_BUCK4_UVD_STAT, "bd96801-buck4-undervolt"),
+	DEFINE_RES_IRQ_NAMED(BD96801_BUCK4_TW_CH_STAT, "bd96801-buck4-thermal"),
+
+	DEFINE_RES_IRQ_NAMED(BD96801_LDO5_OCPH_STAT, "bd96801-ldo5-overcurr"),
+	DEFINE_RES_IRQ_NAMED(BD96801_LDO5_OVD_STAT, "bd96801-ldo5-overvolt"),
+	DEFINE_RES_IRQ_NAMED(BD96801_LDO5_UVD_STAT, "bd96801-ldo5-undervolt"),
+
+	DEFINE_RES_IRQ_NAMED(BD96801_LDO6_OCPH_STAT, "bd96801-ldo6-overcurr"),
+	DEFINE_RES_IRQ_NAMED(BD96801_LDO6_OVD_STAT, "bd96801-ldo6-overvolt"),
+	DEFINE_RES_IRQ_NAMED(BD96801_LDO6_UVD_STAT, "bd96801-ldo6-undervolt"),
+

drivers/mfd/rohm-bd96801.c new file +124 -0

+		.resources = regulator_intb_irqs,
+		.num_resources = ARRAY_SIZE(regulator_intb_irqs),
+	},
+};
+
+static const struct regmap_range bd96801_volatile_ranges[] = {
+	/* Status registers */
+	regmap_reg_range(BD96801_REG_WD_FEED, BD96801_REG_WD_FAILCOUNT),
+	regmap_reg_range(BD96801_REG_WD_ASK, BD96801_REG_WD_ASK),
+	regmap_reg_range(BD96801_REG_WD_STATUS, BD96801_REG_WD_STATUS),
+	regmap_reg_range(BD96801_REG_PMIC_STATE, BD96801_REG_INT_LDO7_INTB),
+	/* Registers which do not update value unless PMIC is in STBY */
+	regmap_reg_range(BD96801_REG_SSCG_CTRL, BD96801_REG_SHD_INTB),
+	regmap_reg_range(BD96801_REG_BUCK_OVP, BD96801_REG_BOOT_OVERTIME),
+	/*
+	 * LDO control registers have single bit (LDO MODE) which does not
+	 * change when we write it unless PMIC is in STBY. It's safer to not
+	 * cache it.
+	 */
+	regmap_reg_range(BD96801_LDO5_VOL_LVL_REG, BD96801_LDO7_VOL_LVL_REG),
+};
+
+static const struct regmap_access_table volatile_regs = {
+	.yes_ranges = bd96801_volatile_ranges,
+	.n_yes_ranges = ARRAY_SIZE(bd96801_volatile_ranges),
+};
+
+static const struct regmap_irq bd96801_intb_irqs[] = {
+	/* STATUS SYSTEM INTB */
+	REGMAP_IRQ_REG(BD96801_TW_STAT, 0, BD96801_TW_STAT_MASK),
+	REGMAP_IRQ_REG(BD96801_WDT_ERR_STAT, 0, BD96801_WDT_ERR_STAT_MASK),
+	REGMAP_IRQ_REG(BD96801_I2C_ERR_STAT, 0, BD96801_I2C_ERR_STAT_MASK),
+	REGMAP_IRQ_REG(BD96801_CHIP_IF_ERR_STAT, 0, BD96801_CHIP_IF_ERR_STAT_MASK),
+	/* STATUS BUCK1 INTB */
+	REGMAP_IRQ_REG(BD96801_BUCK1_OCPH_STAT, 1, BD96801_BUCK_OCPH_STAT_MASK),
+	REGMAP_IRQ_REG(BD96801_BUCK1_OCPL_STAT, 1, BD96801_BUCK_OCPL_STAT_MASK),
+	REGMAP_IRQ_REG(BD96801_BUCK1_OCPN_STAT, 1, BD96801_BUCK_OCPN_STAT_MASK),
+	REGMAP_IRQ_REG(BD96801_BUCK1_OVD_STAT, 1, BD96801_BUCK_OVD_STAT_MASK),
+	REGMAP_IRQ_REG(BD96801_BUCK1_UVD_STAT, 1, BD96801_BUCK_UVD_STAT_MASK),
+	REGMAP_IRQ_REG(BD96801_BUCK1_TW_CH_STAT, 1, BD96801_BUCK_TW_CH_STAT_MASK),
+	/* BUCK 2 INTB */
+	REGMAP_IRQ_REG(BD96801_BUCK2_OCPH_STAT, 2, BD96801_BUCK_OCPH_STAT_MASK),
+	REGMAP_IRQ_REG(BD96801_BUCK2_OCPL_STAT, 2, BD96801_BUCK_OCPL_STAT_MASK),
+	REGMAP_IRQ_REG(BD96801_BUCK2_OCPN_STAT, 2, BD96801_BUCK_OCPN_STAT_MASK),
+	REGMAP_IRQ_REG(BD96801_BUCK2_OVD_STAT, 2, BD96801_BUCK_OVD_STAT_MASK),
+	REGMAP_IRQ_REG(BD96801_BUCK2_UVD_STAT, 2, BD96801_BUCK_UVD_STAT_MASK),
+	REGMAP_IRQ_REG(BD96801_BUCK2_TW_CH_STAT, 2, BD96801_BUCK_TW_CH_STAT_MASK),
+	/* BUCK 3 INTB */
+	REGMAP_IRQ_REG(BD96801_BUCK3_OCPH_STAT, 3, BD96801_BUCK_OCPH_STAT_MASK),
+	REGMAP_IRQ_REG(BD96801_BUCK3_OCPL_STAT, 3, BD96801_BUCK_OCPL_STAT_MASK),
+	REGMAP_IRQ_REG(BD96801_BUCK3_OCPN_STAT, 3, BD96801_BUCK_OCPN_STAT_MASK),
+	REGMAP_IRQ_REG(BD96801_BUCK3_OVD_STAT, 3, BD96801_BUCK_OVD_STAT_MASK),
+	REGMAP_IRQ_REG(BD96801_BUCK3_UVD_STAT, 3, BD96801_BUCK_UVD_STAT_MASK),
+	REGMAP_IRQ_REG(BD96801_BUCK3_TW_CH_STAT, 3, BD96801_BUCK_TW_CH_STAT_MASK),
+	/* BUCK 4 INTB */
+	REGMAP_IRQ_REG(BD96801_BUCK4_OCPH_STAT, 4, BD96801_BUCK_OCPH_STAT_MASK),
+	REGMAP_IRQ_REG(BD96801_BUCK4_OCPL_STAT, 4, BD96801_BUCK_OCPL_STAT_MASK),
+	REGMAP_IRQ_REG(BD96801_BUCK4_OCPN_STAT, 4, BD96801_BUCK_OCPN_STAT_MASK),
+	REGMAP_IRQ_REG(BD96801_BUCK4_OVD_STAT, 4, BD96801_BUCK_OVD_STAT_MASK),
+	REGMAP_IRQ_REG(BD96801_BUCK4_UVD_STAT, 4, BD96801_BUCK_UVD_STAT_MASK),
+	REGMAP_IRQ_REG(BD96801_BUCK4_TW_CH_STAT, 4, BD96801_BUCK_TW_CH_STAT_MASK),
+	/* LDO5 INTB */
+	REGMAP_IRQ_REG(BD96801_LDO5_OCPH_STAT, 5, BD96801_LDO_OCPH_STAT_MASK),
+	REGMAP_IRQ_REG(BD96801_LDO5_OVD_STAT, 5, BD96801_LDO_OVD_STAT_MASK),
+	REGMAP_IRQ_REG(BD96801_LDO5_UVD_STAT, 5, BD96801_LDO_UVD_STAT_MASK),
+	/* LDO6 INTB */
+	REGMAP_IRQ_REG(BD96801_LDO6_OCPH_STAT, 6, BD96801_LDO_OCPH_STAT_MASK),
+	REGMAP_IRQ_REG(BD96801_LDO6_OVD_STAT, 6, BD96801_LDO_OVD_STAT_MASK),
+	REGMAP_IRQ_REG(BD96801_LDO6_UVD_STAT, 6, BD96801_LDO_UVD_STAT_MASK),
+	/* LDO7 INTB */
+	REGMAP_IRQ_REG(BD96801_LDO7_OCPH_STAT, 7, BD96801_LDO_OCPH_STAT_MASK),
+	REGMAP_IRQ_REG(BD96801_LDO7_OVD_STAT, 7, BD96801_LDO_OVD_STAT_MASK),
+	REGMAP_IRQ_REG(BD96801_LDO7_UVD_STAT, 7, BD96801_LDO_UVD_STAT_MASK),
+};
+
+static struct regmap_irq_chip bd96801_irq_chip_intb = {
+	.name = "bd96801-irq-intb",
+	.main_status = BD96801_REG_INT_MAIN,
+	.num_main_regs = 1,
+	.irqs = &bd96801_intb_irqs[0],
+	.num_irqs = ARRAY_SIZE(bd96801_intb_irqs),
+	.status_base = BD96801_REG_INT_SYS_INTB,
+	.mask_base = BD96801_REG_MASK_SYS_INTB,
+	.ack_base = BD96801_REG_INT_SYS_INTB,
+	.init_ack_masked = true,
+	.num_regs = 8,
+	.irq_reg_stride = 1,
+};
+
+static const struct regmap_config bd96801_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.volatile_table = &volatile_regs,
+	.cache_type = REGCACHE_RBTREE,
+};
+
+static int bd96801_i2c_probe(struct i2c_client *i2c)
+{
+	struct regmap_irq_chip_data *intb_irq_data;
+	const struct fwnode_handle *fwnode;
+	struct irq_domain *intb_domain;
+	struct regmap *regmap;
+	int ret, intb_irq;
+
+	fwnode = dev_fwnode(&i2c->dev);
+	if (!fwnode)
+		return dev_err_probe(&i2c->dev, -EINVAL, "Failed to find fwnode\n");
+
+	intb_irq = fwnode_irq_get_byname(fwnode, "intb");
+	if (intb_irq < 0)
+		return dev_err_probe(&i2c->dev, intb_irq, "INTB IRQ not configured\n");
+
+	regmap = devm_regmap_init_i2c(i2c, &bd96801_regmap_config);
+	if (IS_ERR(regmap))
+		return dev_err_probe(&i2c->dev, PTR_ERR(regmap),
+				    "Regmap initialization failed\n");
+
+	ret = regmap_write(regmap, BD96801_LOCK_REG, BD96801_UNLOCK);
+	if (ret)
+		return dev_err_probe(&i2c->dev, ret, "Failed to unlock PMIC\n");
+
+	ret = devm_regmap_add_irq_chip(&i2c->dev, regmap, intb_irq,
+				       IRQF_ONESHOT, 0, &bd96801_irq_chip_intb,
+				       &intb_irq_data);

drivers/mfd/rohm-bd96801.c new file +37 -0

+	if (ret)
+		dev_err(&i2c->dev, "Failed to create subdevices\n");
+
+	return ret;
+}
+
+static const struct of_device_id bd96801_of_match[] = {
+	{ .compatible = "rohm,bd96801",	},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, bd96801_of_match);
+
+static struct i2c_driver bd96801_i2c_driver = {
+	.driver = {
+		.name = "rohm-bd96801",
+		.of_match_table = bd96801_of_match,
+	},
+	.probe = bd96801_i2c_probe,
+};
+
+static int __init bd96801_i2c_init(void)
+{
+	return i2c_add_driver(&bd96801_i2c_driver);
+}
+
+/* Initialise early so consumer devices can complete system boot */
+subsys_initcall(bd96801_i2c_init);
+
+static void __exit bd96801_i2c_exit(void)
+{
+	i2c_del_driver(&bd96801_i2c_driver);
+}
+module_exit(bd96801_i2c_exit);
+
+MODULE_AUTHOR("Matti Vaittinen <[email protected]>");
+MODULE_DESCRIPTION("ROHM BD96801 Power Management IC driver");
+MODULE_LICENSE("GPL");

include/linux/mfd/rohm-bd96801.h new file +215 -0

new file mode 100644
index 000000000000..e2d9e10b6364
@@ -0,0 +1,215 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/* Copyright (C) 2024 ROHM Semiconductors */
+
+#ifndef __MFD_BD96801_H__
+#define __MFD_BD96801_H__
+
+#define BD96801_REG_SSCG_CTRL		0x09
+#define BD96801_REG_SHD_INTB            0x20
+#define BD96801_LDO5_VOL_LVL_REG	0x2c
+#define BD96801_LDO6_VOL_LVL_REG	0x2d
+#define BD96801_LDO7_VOL_LVL_REG	0x2e
+#define BD96801_REG_BUCK_OVP		0x30
+#define BD96801_REG_BUCK_OVD		0x35
+#define BD96801_REG_LDO_OVP		0x31
+#define BD96801_REG_LDO_OVD		0x36
+#define BD96801_REG_BOOT_OVERTIME	0x3a
+#define BD96801_REG_WD_TMO		0x40
+#define BD96801_REG_WD_CONF		0x41
+#define BD96801_REG_WD_FEED		0x42
+#define BD96801_REG_WD_FAILCOUNT	0x43
+#define BD96801_REG_WD_ASK		0x46
+#define BD96801_REG_WD_STATUS		0x4a
+#define BD96801_REG_PMIC_STATE		0x4f
+#define BD96801_REG_EXT_STATE		0x50
+
+#define BD96801_STATE_STBY		0x09
+
+#define BD96801_LOCK_REG		0x04
+#define BD96801_UNLOCK			0x9d
+#define BD96801_LOCK			0x00
+
+/* IRQ register area */
+#define BD96801_REG_INT_MAIN		0x51
+
+/*
+ * The BD96801 has two physical IRQ lines, INTB and ERRB.
+ *
+ * The 'main status register' is located at 0x51.
+ * The ERRB status registers are located at 0x52 ... 0x5B
+ * INTB status registers are at range 0x5c ... 0x63
+ */
+#define BD96801_REG_INT_SYS_ERRB1	0x52
+#define BD96801_REG_INT_SYS_INTB	0x5c
+#define BD96801_REG_INT_LDO7_INTB	0x63
+
+/* MASK registers */
+#define BD96801_REG_MASK_SYS_INTB	0x73
+#define BD96801_REG_MASK_SYS_ERRB	0x69
+
+#define BD96801_MAX_REGISTER		0x7a
+
+#define BD96801_OTP_ERR_MASK		BIT(0)
+#define BD96801_DBIST_ERR_MASK		BIT(1)
+#define BD96801_EEP_ERR_MASK		BIT(2)
+#define BD96801_ABIST_ERR_MASK		BIT(3)
+#define BD96801_PRSTB_ERR_MASK		BIT(4)
+#define BD96801_DRMOS1_ERR_MASK		BIT(5)
+#define BD96801_DRMOS2_ERR_MASK		BIT(6)
+#define BD96801_SLAVE_ERR_MASK		BIT(7)
+#define BD96801_VREF_ERR_MASK		BIT(0)
+#define BD96801_TSD_ERR_MASK		BIT(1)
+#define BD96801_UVLO_ERR_MASK		BIT(2)
+#define BD96801_OVLO_ERR_MASK		BIT(3)
+#define BD96801_OSC_ERR_MASK		BIT(4)
+#define BD96801_PON_ERR_MASK		BIT(5)
+#define BD96801_POFF_ERR_MASK		BIT(6)
+#define BD96801_CMD_SHDN_ERR_MASK	BIT(7)
+#define BD96801_INT_PRSTB_WDT_ERR_MASK	BIT(0)
+#define BD96801_INT_CHIP_IF_ERR_MASK	BIT(3)
+#define BD96801_INT_SHDN_ERR_MASK	BIT(7)
+#define BD96801_OUT_PVIN_ERR_MASK	BIT(0)
+#define BD96801_OUT_OVP_ERR_MASK	BIT(1)
+#define BD96801_OUT_UVP_ERR_MASK	BIT(2)
+#define BD96801_OUT_SHDN_ERR_MASK	BIT(7)
+
+/* ERRB IRQs */
+enum {
+	/* Reg 0x52, 0x53, 0x54 - ERRB system IRQs */
+	BD96801_OTP_ERR_STAT,
+	BD96801_DBIST_ERR_STAT,
+	BD96801_EEP_ERR_STAT,
+	BD96801_ABIST_ERR_STAT,
+	BD96801_PRSTB_ERR_STAT,
+	BD96801_DRMOS1_ERR_STAT,
+	BD96801_DRMOS2_ERR_STAT,
+	BD96801_SLAVE_ERR_STAT,
+	BD96801_VREF_ERR_STAT,
+	BD96801_TSD_ERR_STAT,
+	BD96801_UVLO_ERR_STAT,
+	BD96801_OVLO_ERR_STAT,
+	BD96801_OSC_ERR_STAT,
+	BD96801_PON_ERR_STAT,
+	BD96801_POFF_ERR_STAT,
+	BD96801_CMD_SHDN_ERR_STAT,
+	BD96801_INT_PRSTB_WDT_ERR,
+	BD96801_INT_CHIP_IF_ERR,
+	BD96801_INT_SHDN_ERR_STAT,
+
+	/* Reg 0x55 BUCK1 ERR IRQs */
+	BD96801_BUCK1_PVIN_ERR_STAT,
+	BD96801_BUCK1_OVP_ERR_STAT,
+	BD96801_BUCK1_UVP_ERR_STAT,
+	BD96801_BUCK1_SHDN_ERR_STAT,
+
+	/* Reg 0x56 BUCK2 ERR IRQs */
+	BD96801_BUCK2_PVIN_ERR_STAT,
+	BD96801_BUCK2_OVP_ERR_STAT,
+	BD96801_BUCK2_UVP_ERR_STAT,
+	BD96801_BUCK2_SHDN_ERR_STAT,
+
+	/* Reg 0x57 BUCK3 ERR IRQs */
+	BD96801_BUCK3_PVIN_ERR_STAT,
+	BD96801_BUCK3_OVP_ERR_STAT,
+	BD96801_BUCK3_UVP_ERR_STAT,
+	BD96801_BUCK3_SHDN_ERR_STAT,
+
+	/* Reg 0x58 BUCK4 ERR IRQs */
+	BD96801_BUCK4_PVIN_ERR_STAT,
+	BD96801_BUCK4_OVP_ERR_STAT,
+	BD96801_BUCK4_UVP_ERR_STAT,
+	BD96801_BUCK4_SHDN_ERR_STAT,
+
+	/* Reg 0x59 LDO5 ERR IRQs */
+	BD96801_LDO5_PVIN_ERR_STAT,
+	BD96801_LDO5_OVP_ERR_STAT,
+	BD96801_LDO5_UVP_ERR_STAT,
+	BD96801_LDO5_SHDN_ERR_STAT,
+
+	/* Reg 0x5a LDO6 ERR IRQs */
+	BD96801_LDO6_PVIN_ERR_STAT,
+	BD96801_LDO6_OVP_ERR_STAT,
+	BD96801_LDO6_UVP_ERR_STAT,
+	BD96801_LDO6_SHDN_ERR_STAT,
+
+	/* Reg 0x5b LDO7 ERR IRQs */
+	BD96801_LDO7_PVIN_ERR_STAT,
+	BD96801_LDO7_OVP_ERR_STAT,
+	BD96801_LDO7_UVP_ERR_STAT,
+	BD96801_LDO7_SHDN_ERR_STAT,
+};
+
+/* INTB IRQs */
+enum {
+	/* Reg 0x5c (System INTB) */
+	BD96801_TW_STAT,
+	BD96801_WDT_ERR_STAT,
+	BD96801_I2C_ERR_STAT,
+	BD96801_CHIP_IF_ERR_STAT,
+
+	/* Reg 0x5d (BUCK1 INTB) */
+	BD96801_BUCK1_OCPH_STAT,
+	BD96801_BUCK1_OCPL_STAT,
+	BD96801_BUCK1_OCPN_STAT,
+	BD96801_BUCK1_OVD_STAT,
+	BD96801_BUCK1_UVD_STAT,
+	BD96801_BUCK1_TW_CH_STAT,
+
+	/* Reg 0x5e (BUCK2 INTB) */
+	BD96801_BUCK2_OCPH_STAT,
+	BD96801_BUCK2_OCPL_STAT,
+	BD96801_BUCK2_OCPN_STAT,
+	BD96801_BUCK2_OVD_STAT,
+	BD96801_BUCK2_UVD_STAT,
+	BD96801_BUCK2_TW_CH_STAT,
+
+	/* Reg 0x5f (BUCK3 INTB)*/
+	BD96801_BUCK3_OCPH_STAT,
+	BD96801_BUCK3_OCPL_STAT,
+	BD96801_BUCK3_OCPN_STAT,
+	BD96801_BUCK3_OVD_STAT,
+	BD96801_BUCK3_UVD_STAT,
+	BD96801_BUCK3_TW_CH_STAT,
+
+	/* Reg 0x60 (BUCK4 INTB)*/
+	BD96801_BUCK4_OCPH_STAT,
+	BD96801_BUCK4_OCPL_STAT,
+	BD96801_BUCK4_OCPN_STAT,
+	BD96801_BUCK4_OVD_STAT,
+	BD96801_BUCK4_UVD_STAT,
+	BD96801_BUCK4_TW_CH_STAT,
+
+	/* Reg 0x61 (LDO5 INTB) */
+	BD96801_LDO5_OCPH_STAT, /* bit [0] */
+	BD96801_LDO5_OVD_STAT,	/* bit [3] */
+	BD96801_LDO5_UVD_STAT,  /* bit [4] */
+
+	/* Reg 0x62 (LDO6 INTB) */
+	BD96801_LDO6_OCPH_STAT, /* bit [0] */
+	BD96801_LDO6_OVD_STAT,	/* bit [3] */
+	BD96801_LDO6_UVD_STAT,  /* bit [4] */
+
+	/* Reg 0x63 (LDO7 INTB) */
+	BD96801_LDO7_OCPH_STAT, /* bit [0] */
+	BD96801_LDO7_OVD_STAT,	/* bit [3] */
+	BD96801_LDO7_UVD_STAT,  /* bit [4] */
+};
+
+/* IRQ MASKs */
+#define BD96801_TW_STAT_MASK		BIT(0)
+#define BD96801_WDT_ERR_STAT_MASK	BIT(1)
+#define BD96801_I2C_ERR_STAT_MASK	BIT(2)
+#define BD96801_CHIP_IF_ERR_STAT_MASK	BIT(3)
+
+#define BD96801_BUCK_OCPH_STAT_MASK	BIT(0)
+#define BD96801_BUCK_OCPL_STAT_MASK	BIT(1)
+#define BD96801_BUCK_OCPN_STAT_MASK	BIT(2)
+#define BD96801_BUCK_OVD_STAT_MASK	BIT(3)
+#define BD96801_BUCK_UVD_STAT_MASK	BIT(4)
+#define BD96801_BUCK_TW_CH_STAT_MASK	BIT(5)
+
+#define BD96801_LDO_OCPH_STAT_MASK	BIT(0)
+#define BD96801_LDO_OVD_STAT_MASK	BIT(3)
+#define BD96801_LDO_UVD_STAT_MASK	BIT(4)
+
+#endif

include/linux/mfd/rohm-generic.h +1 -0

index 4eeb22876bad..e7d4e6afe388 100644
@@ -16,6 +16,7 @@ enum rohm_chip_type {
 	ROHM_CHIP_TYPE_BD71828,
 	ROHM_CHIP_TYPE_BD71837,
 	ROHM_CHIP_TYPE_BD71847,
+	ROHM_CHIP_TYPE_BD96801,
 	ROHM_CHIP_TYPE_AMOUNT
 };
 

Signature · 14 lines

2.45.1


-- 
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 =] 

The ROHM BD96801 "scalable PMIC" provides two physical IRQs. The ERRB handling can in many cases be omitted because it is used to inform fatal IRQs, which usually kill the power from the SOC.

There may however be use-cases where the SOC has a 'back-up' emergency power source which allows some very short time of operation to try to gracefully shut down sensitive hardware. Furthermore, it is possible the processor controlling the PMIC is not powered by the PMIC. In such cases handling the ERRB IRQs may be beneficial.

Add support for ERRB IRQs.

Signed-off-by Matti Vaittinen <[email protected]>

---
Revision history:
v2 =>:
- No changes
v1 => v2:
- New patch

---
 drivers/mfd/rohm-bd96801.c | 291 ++++++++++++++++++++++++++++++++-----
 1 file changed, 253 insertions(+), 38 deletions(-)

drivers/mfd/rohm-bd96801.c +0 -0

index 1c2a9591be7b..b7f073318873 100644

drivers/mfd/rohm-bd96801.c +183 -21

+ * basic set of the PMIC features.
+ * Most notably, there is no support for the configurations which should
+ * be done when the PMIC is in STBY mode.
  *
  * Being able to reliably do the configurations like changing the
  * regulator safety limits (like limits for the over/under -voltages, over

@@ -23,16 +19,14 @@
  * be the need to configure these safety limits. Hence it's not simple to
  * come up with a generic solution.
  *
- * Users who require the ERRB handling and STBY state configurations can
- * have a look at the original RFC:
+ * Users who require the STBY state configurations can  have a look at the
+ * original RFC:
  * https://lore.kernel.org/all/[email protected]/
- * which implements a workaround to debugFS naming conflict and some of
- * the safety limit configurations - but leaves the state change handling
- * and synchronization to be implemented.
+ * which implements some of the safety limit configurations - but leaves the
+ * state change handling and synchronization to be implemented.
  *
  * It would be great to hear (and receive a patch!) if you implement the
- * STBY configuration support or a proper fix to the debugFS naming
- * conflict in your downstream driver ;)
+ * STBY configuration support or a proper fix in your downstream driver ;)
  */
 
 #include <linux/i2c.h>

@@ -45,6 +39,65 @@
 
 #include <linux/mfd/rohm-bd96801.h>
 #include <linux/mfd/rohm-generic.h>
+
+static const struct resource regulator_errb_irqs[] = {
+	DEFINE_RES_IRQ_NAMED(BD96801_OTP_ERR_STAT, "bd96801-otp-err"),
+	DEFINE_RES_IRQ_NAMED(BD96801_DBIST_ERR_STAT, "bd96801-dbist-err"),
+	DEFINE_RES_IRQ_NAMED(BD96801_EEP_ERR_STAT, "bd96801-eep-err"),
+	DEFINE_RES_IRQ_NAMED(BD96801_ABIST_ERR_STAT, "bd96801-abist-err"),
+	DEFINE_RES_IRQ_NAMED(BD96801_PRSTB_ERR_STAT, "bd96801-prstb-err"),
+	DEFINE_RES_IRQ_NAMED(BD96801_DRMOS1_ERR_STAT, "bd96801-drmoserr1"),
+	DEFINE_RES_IRQ_NAMED(BD96801_DRMOS2_ERR_STAT, "bd96801-drmoserr2"),
+	DEFINE_RES_IRQ_NAMED(BD96801_SLAVE_ERR_STAT, "bd96801-slave-err"),
+	DEFINE_RES_IRQ_NAMED(BD96801_VREF_ERR_STAT, "bd96801-vref-err"),
+	DEFINE_RES_IRQ_NAMED(BD96801_TSD_ERR_STAT, "bd96801-tsd"),
+	DEFINE_RES_IRQ_NAMED(BD96801_UVLO_ERR_STAT, "bd96801-uvlo-err"),
+	DEFINE_RES_IRQ_NAMED(BD96801_OVLO_ERR_STAT, "bd96801-ovlo-err"),
+	DEFINE_RES_IRQ_NAMED(BD96801_OSC_ERR_STAT, "bd96801-osc-err"),
+	DEFINE_RES_IRQ_NAMED(BD96801_PON_ERR_STAT, "bd96801-pon-err"),
+	DEFINE_RES_IRQ_NAMED(BD96801_POFF_ERR_STAT, "bd96801-poff-err"),
+	DEFINE_RES_IRQ_NAMED(BD96801_CMD_SHDN_ERR_STAT, "bd96801-cmd-shdn-err"),
+
+	DEFINE_RES_IRQ_NAMED(BD96801_INT_PRSTB_WDT_ERR, "bd96801-prstb-wdt-err"),
+	DEFINE_RES_IRQ_NAMED(BD96801_INT_CHIP_IF_ERR, "bd96801-chip-if-err"),
+	DEFINE_RES_IRQ_NAMED(BD96801_INT_SHDN_ERR_STAT, "bd96801-int-shdn-err"),
+
+	DEFINE_RES_IRQ_NAMED(BD96801_BUCK1_PVIN_ERR_STAT, "bd96801-buck1-pvin-err"),
+	DEFINE_RES_IRQ_NAMED(BD96801_BUCK1_OVP_ERR_STAT, "bd96801-buck1-ovp-err"),
+	DEFINE_RES_IRQ_NAMED(BD96801_BUCK1_UVP_ERR_STAT, "bd96801-buck1-uvp-err"),
+	DEFINE_RES_IRQ_NAMED(BD96801_BUCK1_SHDN_ERR_STAT, "bd96801-buck1-shdn-err"),
+
+	DEFINE_RES_IRQ_NAMED(BD96801_BUCK2_PVIN_ERR_STAT, "bd96801-buck2-pvin-err"),
+	DEFINE_RES_IRQ_NAMED(BD96801_BUCK2_OVP_ERR_STAT, "bd96801-buck2-ovp-err"),
+	DEFINE_RES_IRQ_NAMED(BD96801_BUCK2_UVP_ERR_STAT, "bd96801-buck2-uvp-err"),
+	DEFINE_RES_IRQ_NAMED(BD96801_BUCK2_SHDN_ERR_STAT, "bd96801-buck2-shdn-err"),
+
+	DEFINE_RES_IRQ_NAMED(BD96801_BUCK3_PVIN_ERR_STAT, "bd96801-buck3-pvin-err"),
+	DEFINE_RES_IRQ_NAMED(BD96801_BUCK3_OVP_ERR_STAT, "bd96801-buck3-ovp-err"),
+	DEFINE_RES_IRQ_NAMED(BD96801_BUCK3_UVP_ERR_STAT, "bd96801-buck3-uvp-err"),
+	DEFINE_RES_IRQ_NAMED(BD96801_BUCK3_SHDN_ERR_STAT, "bd96801-buck3-shdn-err"),
+
+	DEFINE_RES_IRQ_NAMED(BD96801_BUCK4_PVIN_ERR_STAT, "bd96801-buck4-pvin-err"),
+	DEFINE_RES_IRQ_NAMED(BD96801_BUCK4_OVP_ERR_STAT, "bd96801-buck4-ovp-err"),
+	DEFINE_RES_IRQ_NAMED(BD96801_BUCK4_UVP_ERR_STAT, "bd96801-buck4-uvp-err"),
+	DEFINE_RES_IRQ_NAMED(BD96801_BUCK4_SHDN_ERR_STAT, "bd96801-buck4-shdn-err"),
+
+	DEFINE_RES_IRQ_NAMED(BD96801_LDO5_PVIN_ERR_STAT, "bd96801-ldo5-pvin-err"),
+	DEFINE_RES_IRQ_NAMED(BD96801_LDO5_OVP_ERR_STAT, "bd96801-ldo5-ovp-err"),
+	DEFINE_RES_IRQ_NAMED(BD96801_LDO5_UVP_ERR_STAT, "bd96801-ldo5-uvp-err"),
+	DEFINE_RES_IRQ_NAMED(BD96801_LDO5_SHDN_ERR_STAT, "bd96801-ldo5-shdn-err"),
+
+	DEFINE_RES_IRQ_NAMED(BD96801_LDO6_PVIN_ERR_STAT, "bd96801-ldo6-pvin-err"),
+	DEFINE_RES_IRQ_NAMED(BD96801_LDO6_OVP_ERR_STAT, "bd96801-ldo6-ovp-err"),
+	DEFINE_RES_IRQ_NAMED(BD96801_LDO6_UVP_ERR_STAT, "bd96801-ldo6-uvp-err"),
+	DEFINE_RES_IRQ_NAMED(BD96801_LDO6_SHDN_ERR_STAT, "bd96801-ldo6-shdn-err"),
+
+	DEFINE_RES_IRQ_NAMED(BD96801_LDO7_PVIN_ERR_STAT, "bd96801-ldo7-pvin-err"),
+	DEFINE_RES_IRQ_NAMED(BD96801_LDO7_OVP_ERR_STAT, "bd96801-ldo7-ovp-err"),
+	DEFINE_RES_IRQ_NAMED(BD96801_LDO7_UVP_ERR_STAT, "bd96801-ldo7-uvp-err"),
+	DEFINE_RES_IRQ_NAMED(BD96801_LDO7_SHDN_ERR_STAT, "bd96801-ldo7-shdn-err"),
+};
+
 static const struct resource regulator_intb_irqs[] = {
 	DEFINE_RES_IRQ_NAMED(BD96801_TW_STAT, "bd96801-core-thermal"),
 

@@ -89,20 +142,14 @@ static const struct resource regulator_intb_irqs[] = {
 	DEFINE_RES_IRQ_NAMED(BD96801_LDO7_UVD_STAT, "bd96801-ldo7-undervolt"),
 };
 
-static const struct resource wdg_intb_irqs[] = {
-	DEFINE_RES_IRQ_NAMED(BD96801_WDT_ERR_STAT, "bd96801-wdg"),
+enum {
+	WDG_CELL = 0,
+	REGULATOR_CELL,
 };
 
 static struct mfd_cell bd96801_mfd_cells[] = {
-	{
-		.name = "bd96801-wdt",
-		.resources = wdg_intb_irqs,
-		.num_resources = ARRAY_SIZE(wdg_intb_irqs),
-	}, {
-		.name = "bd96801-pmic",
-		.resources = regulator_intb_irqs,
-		.num_resources = ARRAY_SIZE(regulator_intb_irqs),
-	},
+	[WDG_CELL] = { .name = "bd96801-wdt", },
+	[REGULATOR_CELL] = { .name = "bd96801-pmic", },
 };
 
 static const struct regmap_range bd96801_volatile_ranges[] = {

@@ -127,6 +174,91 @@ static const struct regmap_access_table volatile_regs = {
 	.n_yes_ranges = ARRAY_SIZE(bd96801_volatile_ranges),
 };
 
+/*
+ * For ERRB we need main register bit mapping as bit(0) indicates active IRQ
+ * in one of the first 3 sub IRQ registers, For INTB we can use default 1 to 1
+ * mapping.
+ */
+static unsigned int bit0_offsets[] = {0, 1, 2};	/* System stat, 3 registers */
+static unsigned int bit1_offsets[] = {3};	/* Buck 1 stat */
+static unsigned int bit2_offsets[] = {4};	/* Buck 2 stat */
+static unsigned int bit3_offsets[] = {5};	/* Buck 3 stat */
+static unsigned int bit4_offsets[] = {6};	/* Buck 4 stat */
+static unsigned int bit5_offsets[] = {7};	/* LDO 5 stat */
+static unsigned int bit6_offsets[] = {8};	/* LDO 6 stat */
+static unsigned int bit7_offsets[] = {9};	/* LDO 7 stat */
+
+static struct regmap_irq_sub_irq_map errb_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 const struct regmap_irq bd96801_errb_irqs[] = {
+	/* Reg 0x52 Fatal ERRB1 */
+	REGMAP_IRQ_REG(BD96801_OTP_ERR_STAT, 0, BD96801_OTP_ERR_MASK),
+	REGMAP_IRQ_REG(BD96801_DBIST_ERR_STAT, 0, BD96801_DBIST_ERR_MASK),
+	REGMAP_IRQ_REG(BD96801_EEP_ERR_STAT, 0, BD96801_EEP_ERR_MASK),
+	REGMAP_IRQ_REG(BD96801_ABIST_ERR_STAT, 0, BD96801_ABIST_ERR_MASK),
+	REGMAP_IRQ_REG(BD96801_PRSTB_ERR_STAT, 0, BD96801_PRSTB_ERR_MASK),
+	REGMAP_IRQ_REG(BD96801_DRMOS1_ERR_STAT, 0, BD96801_DRMOS1_ERR_MASK),
+	REGMAP_IRQ_REG(BD96801_DRMOS2_ERR_STAT, 0, BD96801_DRMOS2_ERR_MASK),
+	REGMAP_IRQ_REG(BD96801_SLAVE_ERR_STAT, 0, BD96801_SLAVE_ERR_MASK),
+	/* 0x53 Fatal ERRB2 */
+	REGMAP_IRQ_REG(BD96801_VREF_ERR_STAT, 1, BD96801_VREF_ERR_MASK),
+	REGMAP_IRQ_REG(BD96801_TSD_ERR_STAT, 1, BD96801_TSD_ERR_MASK),
+	REGMAP_IRQ_REG(BD96801_UVLO_ERR_STAT, 1, BD96801_UVLO_ERR_MASK),
+	REGMAP_IRQ_REG(BD96801_OVLO_ERR_STAT, 1, BD96801_OVLO_ERR_MASK),
+	REGMAP_IRQ_REG(BD96801_OSC_ERR_STAT, 1, BD96801_OSC_ERR_MASK),
+	REGMAP_IRQ_REG(BD96801_PON_ERR_STAT, 1, BD96801_PON_ERR_MASK),
+	REGMAP_IRQ_REG(BD96801_POFF_ERR_STAT, 1, BD96801_POFF_ERR_MASK),
+	REGMAP_IRQ_REG(BD96801_CMD_SHDN_ERR_STAT, 1, BD96801_CMD_SHDN_ERR_MASK),
+	/* 0x54 Fatal INTB shadowed to ERRB */
+	REGMAP_IRQ_REG(BD96801_INT_PRSTB_WDT_ERR, 2, BD96801_INT_PRSTB_WDT_ERR_MASK),
+	REGMAP_IRQ_REG(BD96801_INT_CHIP_IF_ERR, 2, BD96801_INT_CHIP_IF_ERR_MASK),
+	REGMAP_IRQ_REG(BD96801_INT_SHDN_ERR_STAT, 2, BD96801_INT_SHDN_ERR_MASK),
+	/* Reg 0x55 BUCK1 ERR IRQs */
+	REGMAP_IRQ_REG(BD96801_BUCK1_PVIN_ERR_STAT, 3, BD96801_OUT_PVIN_ERR_MASK),
+	REGMAP_IRQ_REG(BD96801_BUCK1_OVP_ERR_STAT, 3, BD96801_OUT_OVP_ERR_MASK),
+	REGMAP_IRQ_REG(BD96801_BUCK1_UVP_ERR_STAT, 3, BD96801_OUT_UVP_ERR_MASK),
+	REGMAP_IRQ_REG(BD96801_BUCK1_SHDN_ERR_STAT, 3, BD96801_OUT_SHDN_ERR_MASK),
+	/* Reg 0x56 BUCK2 ERR IRQs */
+	REGMAP_IRQ_REG(BD96801_BUCK2_PVIN_ERR_STAT, 4, BD96801_OUT_PVIN_ERR_MASK),
+	REGMAP_IRQ_REG(BD96801_BUCK2_OVP_ERR_STAT, 4, BD96801_OUT_OVP_ERR_MASK),
+	REGMAP_IRQ_REG(BD96801_BUCK2_UVP_ERR_STAT, 4, BD96801_OUT_UVP_ERR_MASK),
+	REGMAP_IRQ_REG(BD96801_BUCK2_SHDN_ERR_STAT, 4, BD96801_OUT_SHDN_ERR_MASK),
+	/* Reg 0x57 BUCK3 ERR IRQs */
+	REGMAP_IRQ_REG(BD96801_BUCK3_PVIN_ERR_STAT, 5, BD96801_OUT_PVIN_ERR_MASK),
+	REGMAP_IRQ_REG(BD96801_BUCK3_OVP_ERR_STAT, 5, BD96801_OUT_OVP_ERR_MASK),
+	REGMAP_IRQ_REG(BD96801_BUCK3_UVP_ERR_STAT, 5, BD96801_OUT_UVP_ERR_MASK),
+	REGMAP_IRQ_REG(BD96801_BUCK3_SHDN_ERR_STAT, 5, BD96801_OUT_SHDN_ERR_MASK),
+	/* Reg 0x58 BUCK4 ERR IRQs */
+	REGMAP_IRQ_REG(BD96801_BUCK4_PVIN_ERR_STAT, 6, BD96801_OUT_PVIN_ERR_MASK),
+	REGMAP_IRQ_REG(BD96801_BUCK4_OVP_ERR_STAT, 6, BD96801_OUT_OVP_ERR_MASK),
+	REGMAP_IRQ_REG(BD96801_BUCK4_UVP_ERR_STAT, 6, BD96801_OUT_UVP_ERR_MASK),
+	REGMAP_IRQ_REG(BD96801_BUCK4_SHDN_ERR_STAT, 6, BD96801_OUT_SHDN_ERR_MASK),
+	/* Reg 0x59 LDO5 ERR IRQs */
+	REGMAP_IRQ_REG(BD96801_LDO5_PVIN_ERR_STAT, 7, BD96801_OUT_PVIN_ERR_MASK),
+	REGMAP_IRQ_REG(BD96801_LDO5_OVP_ERR_STAT, 7, BD96801_OUT_OVP_ERR_MASK),
+	REGMAP_IRQ_REG(BD96801_LDO5_UVP_ERR_STAT, 7, BD96801_OUT_UVP_ERR_MASK),
+	REGMAP_IRQ_REG(BD96801_LDO5_SHDN_ERR_STAT, 7, BD96801_OUT_SHDN_ERR_MASK),
+	/* Reg 0x5a LDO6 ERR IRQs */
+	REGMAP_IRQ_REG(BD96801_LDO6_PVIN_ERR_STAT, 8, BD96801_OUT_PVIN_ERR_MASK),
+	REGMAP_IRQ_REG(BD96801_LDO6_OVP_ERR_STAT, 8, BD96801_OUT_OVP_ERR_MASK),
+	REGMAP_IRQ_REG(BD96801_LDO6_UVP_ERR_STAT, 8, BD96801_OUT_UVP_ERR_MASK),
+	REGMAP_IRQ_REG(BD96801_LDO6_SHDN_ERR_STAT, 8, BD96801_OUT_SHDN_ERR_MASK),
+	/* Reg 0x5b LDO7 ERR IRQs */
+	REGMAP_IRQ_REG(BD96801_LDO7_PVIN_ERR_STAT, 9, BD96801_OUT_PVIN_ERR_MASK),
+	REGMAP_IRQ_REG(BD96801_LDO7_OVP_ERR_STAT, 9, BD96801_OUT_OVP_ERR_MASK),
+	REGMAP_IRQ_REG(BD96801_LDO7_UVP_ERR_STAT, 9, BD96801_OUT_UVP_ERR_MASK),
+	REGMAP_IRQ_REG(BD96801_LDO7_SHDN_ERR_STAT, 9, BD96801_OUT_SHDN_ERR_MASK),
+};
+
 static const struct regmap_irq bd96801_intb_irqs[] = {
 	/* STATUS SYSTEM INTB */
 	REGMAP_IRQ_REG(BD96801_TW_STAT, 0, BD96801_TW_STAT_MASK),

@@ -175,8 +307,25 @@ static const struct regmap_irq bd96801_intb_irqs[] = {
 	REGMAP_IRQ_REG(BD96801_LDO7_UVD_STAT, 7, BD96801_LDO_UVD_STAT_MASK),
 };
 
+static struct regmap_irq_chip bd96801_irq_chip_errb = {
+	.name = "bd96801-irq-errb",
+	.domain_suffix = "errb",
+	.main_status = BD96801_REG_INT_MAIN,
+	.num_main_regs = 1,
+	.irqs = &bd96801_errb_irqs[0],
+	.num_irqs = ARRAY_SIZE(bd96801_errb_irqs),
+	.status_base = BD96801_REG_INT_SYS_ERRB1,
+	.mask_base = BD96801_REG_MASK_SYS_ERRB,
+	.ack_base = BD96801_REG_INT_SYS_ERRB1,
+	.init_ack_masked = true,
+	.num_regs = 10,
+	.irq_reg_stride = 1,
+	.sub_reg_offsets = &errb_sub_irq_offsets[0],
+};
+
 static struct regmap_irq_chip bd96801_irq_chip_intb = {
 	.name = "bd96801-irq-intb",
+	.domain_suffix = "intb",
 	.main_status = BD96801_REG_INT_MAIN,
 	.num_main_regs = 1,
 	.irqs = &bd96801_intb_irqs[0],

@@ -198,11 +347,14 @@ static const struct regmap_config bd96801_regmap_config = {
 
 static int bd96801_i2c_probe(struct i2c_client *i2c)
 {
-	struct regmap_irq_chip_data *intb_irq_data;
+	int i, ret, intb_irq, errb_irq, num_regu_irqs, num_intb, num_errb = 0;
+	int wdg_irq_no;
+	struct regmap_irq_chip_data *intb_irq_data, *errb_irq_data;
+	struct irq_domain *intb_domain, *errb_domain;
+	struct resource wdg_irq;
 	const struct fwnode_handle *fwnode;
-	struct irq_domain *intb_domain;
+	struct resource *regulator_res;
 	struct regmap *regmap;
-	int ret, intb_irq;
 
 	fwnode = dev_fwnode(&i2c->dev);
 	if (!fwnode)

@@ -212,10 +364,28 @@ static int bd96801_i2c_probe(struct i2c_client *i2c)
 	if (intb_irq < 0)
 		return dev_err_probe(&i2c->dev, intb_irq, "INTB IRQ not configured\n");
 
+	num_intb =  ARRAY_SIZE(regulator_intb_irqs);
+
+	/* ERRB may be omitted if processor is powered by the PMIC */

drivers/mfd/rohm-bd96801.c +20 -9

+	if (!regulator_res)
+		return -ENOMEM;
+
 	regmap = devm_regmap_init_i2c(i2c, &bd96801_regmap_config);
-	if (IS_ERR(regmap))
-		return dev_err_probe(&i2c->dev, PTR_ERR(regmap),
+	if (IS_ERR(regmap)) {
+		ret = dev_err_probe(&i2c->dev, PTR_ERR(regmap),
 				    "Regmap initialization failed\n");
+		goto free_out;
+	}
 
 	ret = regmap_write(regmap, BD96801_LOCK_REG, BD96801_UNLOCK);
 	if (ret)

@@ -224,18 +394,63 @@ static int bd96801_i2c_probe(struct i2c_client *i2c)
 	ret = devm_regmap_add_irq_chip(&i2c->dev, regmap, intb_irq,
 				       IRQF_ONESHOT, 0, &bd96801_irq_chip_intb,
 				       &intb_irq_data);
-	if (ret)
-		return dev_err_probe(&i2c->dev, ret, "Failed to add INTB IRQ chip\n");
+	if (ret) {
+		dev_err_probe(&i2c->dev, ret, "Failed to add INTB irq_chip\n");
+		goto free_out;
+	}
 
 	intb_domain = regmap_irq_get_domain(intb_irq_data);
 
-	ret = devm_mfd_add_devices(&i2c->dev, PLATFORM_DEVID_AUTO,
-				   bd96801_mfd_cells,
-				   ARRAY_SIZE(bd96801_mfd_cells), NULL, 0,
-				   intb_domain);
-
+	/*
+	 * MFD core code is built to handle only one IRQ domain. BD96801
+	 * has two domains so we do IRQ mapping here and provide the
+	 * already mapped IRQ numbers to sub-devices.
+	 */
+	for (i = 0; i < num_intb; i++) {
+		struct resource *res = &regulator_res[i];
+
+		*res = regulator_intb_irqs[i];

drivers/mfd/rohm-bd96801.c +10 -0

+		ret = devm_regmap_add_irq_chip(&i2c->dev, regmap, errb_irq,
+					       IRQF_ONESHOT, 0,
+					       &bd96801_irq_chip_errb,
+					       &errb_irq_data);
+		if (ret) {
+			dev_err_probe(&i2c->dev, ret,
+				      "Failed to add ERRB (%d) irq_chip\n",
+				      errb_irq);
+			goto free_out;
+		}

drivers/mfd/rohm-bd96801.c +9 -1

+	bd96801_mfd_cells[REGULATOR_CELL].resources = regulator_res;
+	bd96801_mfd_cells[REGULATOR_CELL].num_resources = num_regu_irqs;
+
+	ret = devm_mfd_add_devices(&i2c->dev, PLATFORM_DEVID_AUTO, bd96801_mfd_cells,
+				   ARRAY_SIZE(bd96801_mfd_cells), NULL, 0, NULL);
 	if (ret)
-		dev_err(&i2c->dev, "Failed to create subdevices\n");
+		dev_err_probe(&i2c->dev, ret, "Failed to create subdevices\n");
+
+free_out:
+	kfree(regulator_res);
 
 	return ret;
 }

Signature · 14 lines

2.45.1


-- 
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 =] 

Quoted 32 lines · Matti Vaittinen

The ROHM BD96801 PMIC is highly customizable automotive grade PMIC which integrates regulator and watchdog funtionalities.

Provide INTB IRQ and register accesses for regulator/watchdog drivers.

Signed-off-by Matti Vaittinen <[email protected]>

---
Changelog:
v2 =>:
- No changes

v1 => v2:
- Drop unused enum
- Improve error prints
- improve comments

RFCv2 => v1:
- drop ERRB interrupts (for now)
- bd96801: Unlock registers in core driver

Changelog: RFCv1 => RFCv2
- Work-around the IRQ domain name conflict
- Add watchdog IRQ
- Various styling fixes based on review by Lee

---
  drivers/mfd/Kconfig              |  13 ++
  drivers/mfd/Makefile             |   1 +
  drivers/mfd/rohm-bd96801.c       | 273 +++++++++++++++++++++++++++++++
  include/linux/mfd/rohm-bd96801.h | 215 ++++++++++++++++++++++++
  include/linux/mfd/rohm-generic.h |   1 +
  5 files changed, 503 insertions(+)

Quoted 2 lines

create mode 100644 drivers/mfd/rohm-bd96801.c create mode 100644 include/linux/mfd/rohm-bd96801.h

Pretty nice. Uses generic interfaces. Just a couple of nits.

drivers/mfd/rohm-bd96801.c line 102

Quoted 7 lines

drivers/mfd/rohm-bd96801.c new file +6 -0

+	{
+		.name = "bd96801-wdt",
+		.resources = wdg_intb_irqs,
+		.num_resources = ARRAY_SIZE(wdg_intb_irqs),
+	}, {
+		.name = "bd96801-pmic",

I thought this was the PMIC?

What is this device? Regulators?

"bd96801-regulator"?

drivers/mfd/rohm-bd96801.c line 106

Quoted 5 lines

drivers/mfd/rohm-bd96801.c new file +4 -0

+		.resources = regulator_intb_irqs,
+		.num_resources = ARRAY_SIZE(regulator_intb_irqs),
+	},
+};

Quoted 32 lines · Matti Vaittinen

The ROHM BD96801 "scalable PMIC" provides two physical IRQs. The ERRB handling can in many cases be omitted because it is used to inform fatal IRQs, which usually kill the power from the SOC.

There may however be use-cases where the SOC has a 'back-up' emergency power source which allows some very short time of operation to try to gracefully shut down sensitive hardware. Furthermore, it is possible the processor controlling the PMIC is not powered by the PMIC. In such cases handling the ERRB IRQs may be beneficial.

Add support for ERRB IRQs.

Signed-off-by Matti Vaittinen <[email protected]>

---
Revision history:
v2 =>:
- No changes
v1 => v2:
- New patch

---
  drivers/mfd/rohm-bd96801.c | 291 ++++++++++++++++++++++++++++++++-----
  1 file changed, 253 insertions(+), 38 deletions(-)

drivers/mfd/rohm-bd96801.c +0 -1

index 1c2a9591be7b..b7f073318873 100644
@@ -5,13 +5,9 @@
   * ROHM BD96801 PMIC driver
   *
   * This version of the "BD86801 scalable PMIC"'s driver supports only very
- * basic set of the PMIC features. Most notably, there is no support for

drivers/mfd/rohm-bd96801.c line 14

Quoted 6 lines

drivers/mfd/rohm-bd96801.c +0 -6

- * the ERRB interrupt and the configurations which should be done when the
- * PMIC is in STBY mode.
- *
- * Supporting the ERRB interrupt would require dropping the regmap-IRQ
- * usage or working around (or accepting a presense of) a naming conflict
- * in debugFS IRQs.

Why bother adding all that blurb in the first place?

Quoted 27 lines

drivers/mfd/rohm-bd96801.c +8 -7

+ * basic set of the PMIC features.
+ * Most notably, there is no support for the configurations which should
+ * be done when the PMIC is in STBY mode.
   *
   * Being able to reliably do the configurations like changing the
   * regulator safety limits (like limits for the over/under -voltages, over

@@ -23,16 +19,14 @@
   * be the need to configure these safety limits. Hence it's not simple to
   * come up with a generic solution.
   *
- * Users who require the ERRB handling and STBY state configurations can
- * have a look at the original RFC:
+ * Users who require the STBY state configurations can  have a look at the
+ * original RFC:
   * https://lore.kernel.org/all/[email protected]/
- * which implements a workaround to debugFS naming conflict and some of
- * the safety limit configurations - but leaves the state change handling
- * and synchronization to be implemented.
+ * which implements some of the safety limit configurations - but leaves the
+ * state change handling and synchronization to be implemented.
   *
   * It would be great to hear (and receive a patch!) if you implement the
- * STBY configuration support or a proper fix to the debugFS naming
- * conflict in your downstream driver ;)
+ * STBY configuration support or a proper fix in your downstream driver ;)
   */

Quoted 20 lines

  static int bd96801_i2c_probe(struct i2c_client *i2c)
  {

drivers/mfd/rohm-bd96801.c +6 -3

-	struct regmap_irq_chip_data *intb_irq_data;
+	int i, ret, intb_irq, errb_irq, num_regu_irqs, num_intb, num_errb = 0;
+	int wdg_irq_no;
+	struct regmap_irq_chip_data *intb_irq_data, *errb_irq_data;
+	struct irq_domain *intb_domain, *errb_domain;
+	struct resource wdg_irq;
  	const struct fwnode_handle *fwnode;
-	struct irq_domain *intb_domain;
+	struct resource *regulator_res;
  	struct regmap *regmap;
-	int ret, intb_irq;
 
  	fwnode = dev_fwnode(&i2c->dev);
  	if (!fwnode)

@@ -212,10 +364,28 @@ static int bd96801_i2c_probe(struct i2c_client *i2c)
  	if (intb_irq < 0)
  		return dev_err_probe(&i2c->dev, intb_irq, "INTB IRQ not configured\n");
 

drivers/mfd/rohm-bd96801.c static int bd96801_i2c_probe(struct i2c_client *i2c) · line 379

Quoted 13 lines

drivers/mfd/rohm-bd96801.c +13 -0

+	num_intb =  ARRAY_SIZE(regulator_intb_irqs);
+
+	/* ERRB may be omitted if processor is powered by the PMIC */
+	errb_irq = fwnode_irq_get_byname(fwnode, "errb");
+	if (errb_irq < 0)
+		errb_irq = 0;
+
+	if (errb_irq)
+		num_errb = ARRAY_SIZE(regulator_errb_irqs);
+
+	num_regu_irqs = num_intb + num_errb;
+
+	regulator_res = kcalloc(num_regu_irqs, sizeof(*regulator_res), GFP_KERNEL);

Why not devm_* and omit the kfree()?

Quoted 47 lines

drivers/mfd/rohm-bd96801.c +25 -9

+	if (!regulator_res)
+		return -ENOMEM;
+
  	regmap = devm_regmap_init_i2c(i2c, &bd96801_regmap_config);
-	if (IS_ERR(regmap))
-		return dev_err_probe(&i2c->dev, PTR_ERR(regmap),
+	if (IS_ERR(regmap)) {
+		ret = dev_err_probe(&i2c->dev, PTR_ERR(regmap),
  				    "Regmap initialization failed\n");
+		goto free_out;
+	}
 
  	ret = regmap_write(regmap, BD96801_LOCK_REG, BD96801_UNLOCK);
  	if (ret)

@@ -224,18 +394,63 @@ static int bd96801_i2c_probe(struct i2c_client *i2c)
  	ret = devm_regmap_add_irq_chip(&i2c->dev, regmap, intb_irq,
  				       IRQF_ONESHOT, 0, &bd96801_irq_chip_intb,
  				       &intb_irq_data);
-	if (ret)
-		return dev_err_probe(&i2c->dev, ret, "Failed to add INTB IRQ chip\n");
+	if (ret) {
+		dev_err_probe(&i2c->dev, ret, "Failed to add INTB irq_chip\n");
+		goto free_out;
+	}
 
  	intb_domain = regmap_irq_get_domain(intb_irq_data);
 
-	ret = devm_mfd_add_devices(&i2c->dev, PLATFORM_DEVID_AUTO,
-				   bd96801_mfd_cells,
-				   ARRAY_SIZE(bd96801_mfd_cells), NULL, 0,
-				   intb_domain);
-
+	/*
+	 * MFD core code is built to handle only one IRQ domain. BD96801
+	 * has two domains so we do IRQ mapping here and provide the
+	 * already mapped IRQ numbers to sub-devices.
+	 */
+	for (i = 0; i < num_intb; i++) {
+		struct resource *res = &regulator_res[i];
+
+		*res = regulator_intb_irqs[i];
+		res->start = res->end = irq_create_mapping(intb_domain,
+							    res->start);
+	}
+
+	wdg_irq_no = irq_create_mapping(intb_domain, BD96801_WDT_ERR_STAT);

drivers/mfd/rohm-bd96801.c static int bd96801_i2c_probe(struct i2c_client *i2c) · line 422

Quoted 5 lines

drivers/mfd/rohm-bd96801.c +5 -0

+	wdg_irq = DEFINE_RES_IRQ_NAMED(wdg_irq_no, "bd96801-wdg");
+	bd96801_mfd_cells[WDG_CELL].resources = &wdg_irq;
+	bd96801_mfd_cells[WDG_CELL].num_resources = 1;
+
+	if (num_errb) {

	if (!num_errb)
		goto skip_errb;

Quoted 15 lines

drivers/mfd/rohm-bd96801.c +14 -0

+		ret = devm_regmap_add_irq_chip(&i2c->dev, regmap, errb_irq,
+					       IRQF_ONESHOT, 0,
+					       &bd96801_irq_chip_errb,
+					       &errb_irq_data);
+		if (ret) {
+			dev_err_probe(&i2c->dev, ret,
+				      "Failed to add ERRB (%d) irq_chip\n",
+				      errb_irq);
+			goto free_out;
+		}
+		errb_domain = regmap_irq_get_domain(errb_irq_data);
+
+		for (i = 0; i < num_errb; i++) {
+			struct resource *res = &regulator_res[num_intb + i];

drivers/mfd/rohm-bd96801.c static int bd96801_i2c_probe(struct i2c_client *i2c) · line 442

Quoted 6 lines

drivers/mfd/rohm-bd96801.c +6 -0

+
+			*res = regulator_errb_irqs[i];
+			res->start = res->end = irq_create_mapping(errb_domain,
+								   res->start);
+		}
+	}

skip_errb:

Quoted 34 lines · Lee Jones

Quoted 32 lines · Matti Vaittinen

The ROHM BD96801 "scalable PMIC" provides two physical IRQs. The ERRB handling can in many cases be omitted because it is used to inform fatal IRQs, which usually kill the power from the SOC.

There may however be use-cases where the SOC has a 'back-up' emergency power source which allows some very short time of operation to try to gracefully shut down sensitive hardware. Furthermore, it is possible the processor controlling the PMIC is not powered by the PMIC. In such cases handling the ERRB IRQs may be beneficial.

Add support for ERRB IRQs.

Signed-off-by Matti Vaittinen <[email protected]>

---
Revision history:
v2 =>:
- No changes
v1 => v2:
- New patch

---
  drivers/mfd/rohm-bd96801.c | 291 ++++++++++++++++++++++++++++++++-----
  1 file changed, 253 insertions(+), 38 deletions(-)

drivers/mfd/rohm-bd96801.c +0 -1

index 1c2a9591be7b..b7f073318873 100644
@@ -5,13 +5,9 @@
   * ROHM BD96801 PMIC driver
   *
   * This version of the "BD86801 scalable PMIC"'s driver supports only very
- * basic set of the PMIC features. Most notably, there is no support for

Quoted 8 lines

drivers/mfd/rohm-bd96801.c line 14

Quoted 6 lines

drivers/mfd/rohm-bd96801.c +0 -6

- * the ERRB interrupt and the configurations which should be done when the
- * PMIC is in STBY mode.
- *
- * Supporting the ERRB interrupt would require dropping the regmap-IRQ
- * usage or working around (or accepting a presense of) a naming conflict
- * in debugFS IRQs.

Why bother adding all that blurb in the first place?

Because, I assume there are users who would like to have the ERRB in use. The main purpose of this comment is that any such users could
a) see this version does not support ERRB.
b) can find the original RFC with ERRB supportn and a workaround.
c) know why this version does not work with ERRB and thus fix this

It seems this ERRB support may be missing from upstream for a while, hence I think having this note is worthy until (if) this ERRB patch lands in upstream.

Quoted 26 lines

Quoted 26 lines

drivers/mfd/rohm-bd96801.c +8 -7

+ * basic set of the PMIC features.
+ * Most notably, there is no support for the configurations which should
+ * be done when the PMIC is in STBY mode.
   *
   * Being able to reliably do the configurations like changing the
   * regulator safety limits (like limits for the over/under -voltages, over

@@ -23,16 +19,14 @@
   * be the need to configure these safety limits. Hence it's not simple to
   * come up with a generic solution.
   *
- * Users who require the ERRB handling and STBY state configurations can
- * have a look at the original RFC:
+ * Users who require the STBY state configurations can  have a look at the
+ * original RFC:
   * https://lore.kernel.org/all/[email protected]/
- * which implements a workaround to debugFS naming conflict and some of
- * the safety limit configurations - but leaves the state change handling
- * and synchronization to be implemented.
+ * which implements some of the safety limit configurations - but leaves the
+ * state change handling and synchronization to be implemented.
   *
   * It would be great to hear (and receive a patch!) if you implement the
- * STBY configuration support or a proper fix to the debugFS naming
- * conflict in your downstream driver ;)
+ * STBY configuration support or a proper fix in your downstream driver ;)
   */

...

Quoted 25 lines

Quoted 25 lines

  static int bd96801_i2c_probe(struct i2c_client *i2c)
  {

drivers/mfd/rohm-bd96801.c +13 -3

-	struct regmap_irq_chip_data *intb_irq_data;
+	int i, ret, intb_irq, errb_irq, num_regu_irqs, num_intb, num_errb = 0;
+	int wdg_irq_no;
+	struct regmap_irq_chip_data *intb_irq_data, *errb_irq_data;
+	struct irq_domain *intb_domain, *errb_domain;
+	struct resource wdg_irq;
  	const struct fwnode_handle *fwnode;
-	struct irq_domain *intb_domain;
+	struct resource *regulator_res;
  	struct regmap *regmap;
-	int ret, intb_irq;
  	fwnode = dev_fwnode(&i2c->dev);
  	if (!fwnode)

@@ -212,10 +364,28 @@ static int bd96801_i2c_probe(struct i2c_client *i2c)
  	if (intb_irq < 0)
  		return dev_err_probe(&i2c->dev, intb_irq, "INTB IRQ not configured\n");
+	num_intb =  ARRAY_SIZE(regulator_intb_irqs);
+
+	/* ERRB may be omitted if processor is powered by the PMIC */
+	errb_irq = fwnode_irq_get_byname(fwnode, "errb");
+	if (errb_irq < 0)
+		errb_irq = 0;
+

Quoted 8 lines

drivers/mfd/rohm-bd96801.c static int bd96801_i2c_probe(struct i2c_client *i2c) · line 379

Quoted 6 lines

drivers/mfd/rohm-bd96801.c +6 -0

+	if (errb_irq)
+		num_errb = ARRAY_SIZE(regulator_errb_irqs);
+
+	num_regu_irqs = num_intb + num_errb;
+
+	regulator_res = kcalloc(num_regu_irqs, sizeof(*regulator_res), GFP_KERNEL);

Why not devm_* and omit the kfree()?

I used kcalloc() because this memory is only temporarily needed. It is not needed after devm_mfd_add_devices() returns.

Sure the devm_* would simplify the error paths... Thanks!

Quoted 44 lines

Quoted 44 lines

drivers/mfd/rohm-bd96801.c +25 -9

+	if (!regulator_res)
+		return -ENOMEM;
+
  	regmap = devm_regmap_init_i2c(i2c, &bd96801_regmap_config);
-	if (IS_ERR(regmap))
-		return dev_err_probe(&i2c->dev, PTR_ERR(regmap),
+	if (IS_ERR(regmap)) {
+		ret = dev_err_probe(&i2c->dev, PTR_ERR(regmap),
  				    "Regmap initialization failed\n");
+		goto free_out;
+	}
  	ret = regmap_write(regmap, BD96801_LOCK_REG, BD96801_UNLOCK);
  	if (ret)

@@ -224,18 +394,63 @@ static int bd96801_i2c_probe(struct i2c_client *i2c)
  	ret = devm_regmap_add_irq_chip(&i2c->dev, regmap, intb_irq,
  				       IRQF_ONESHOT, 0, &bd96801_irq_chip_intb,
  				       &intb_irq_data);
-	if (ret)
-		return dev_err_probe(&i2c->dev, ret, "Failed to add INTB IRQ chip\n");
+	if (ret) {
+		dev_err_probe(&i2c->dev, ret, "Failed to add INTB irq_chip\n");
+		goto free_out;
+	}
  	intb_domain = regmap_irq_get_domain(intb_irq_data);
-	ret = devm_mfd_add_devices(&i2c->dev, PLATFORM_DEVID_AUTO,
-				   bd96801_mfd_cells,
-				   ARRAY_SIZE(bd96801_mfd_cells), NULL, 0,
-				   intb_domain);
-
+	/*
+	 * MFD core code is built to handle only one IRQ domain. BD96801
+	 * has two domains so we do IRQ mapping here and provide the
+	 * already mapped IRQ numbers to sub-devices.
+	 */
+	for (i = 0; i < num_intb; i++) {
+		struct resource *res = &regulator_res[i];
+
+		*res = regulator_intb_irqs[i];
+		res->start = res->end = irq_create_mapping(intb_domain,
+							    res->start);
+	}
+
+	wdg_irq_no = irq_create_mapping(intb_domain, BD96801_WDT_ERR_STAT);

Quoted 8 lines

drivers/mfd/rohm-bd96801.c static int bd96801_i2c_probe(struct i2c_client *i2c) · line 422

Quoted 5 lines

drivers/mfd/rohm-bd96801.c +5 -0

+	wdg_irq = DEFINE_RES_IRQ_NAMED(wdg_irq_no, "bd96801-wdg");
+	bd96801_mfd_cells[WDG_CELL].resources = &wdg_irq;
+	bd96801_mfd_cells[WDG_CELL].num_resources = 1;
+
+	if (num_errb) {

	if (!num_errb)
		goto skip_errb;

Ok, can do.

Quoted 15 lines

Quoted 15 lines

drivers/mfd/rohm-bd96801.c +14 -0

+		ret = devm_regmap_add_irq_chip(&i2c->dev, regmap, errb_irq,
+					       IRQF_ONESHOT, 0,
+					       &bd96801_irq_chip_errb,
+					       &errb_irq_data);
+		if (ret) {
+			dev_err_probe(&i2c->dev, ret,
+				      "Failed to add ERRB (%d) irq_chip\n",
+				      errb_irq);
+			goto free_out;
+		}
+		errb_domain = regmap_irq_get_domain(errb_irq_data);
+
+		for (i = 0; i < num_errb; i++) {
+			struct resource *res = &regulator_res[num_intb + i];

Quoted 8 lines

drivers/mfd/rohm-bd96801.c static int bd96801_i2c_probe(struct i2c_client *i2c) · line 442

Quoted 6 lines

drivers/mfd/rohm-bd96801.c +6 -0

+
+			*res = regulator_errb_irqs[i];
+			res->start = res->end = irq_create_mapping(errb_domain,
+								   res->start);
+		}
+	}

skip_errb:

...

Thanks for comments Lee. Reworking this will have to wait for the irqdomain name suffix, which I will continue after Hervé has done his part of the irqdomain changes. I will omit this patch from the next re-spin of the series.

Quoted 50 lines · Matti Vaittinen

Quoted 34 lines · Lee Jones

Quoted 32 lines · Matti Vaittinen

The ROHM BD96801 "scalable PMIC" provides two physical IRQs. The ERRB handling can in many cases be omitted because it is used to inform fatal IRQs, which usually kill the power from the SOC.

There may however be use-cases where the SOC has a 'back-up' emergency power source which allows some very short time of operation to try to gracefully shut down sensitive hardware. Furthermore, it is possible the processor controlling the PMIC is not powered by the PMIC. In such cases handling the ERRB IRQs may be beneficial.

Add support for ERRB IRQs.

Signed-off-by Matti Vaittinen <[email protected]>

---
Revision history:
v2 =>:
- No changes
v1 => v2:
- New patch

---
   drivers/mfd/rohm-bd96801.c | 291 ++++++++++++++++++++++++++++++++-----
   1 file changed, 253 insertions(+), 38 deletions(-)

drivers/mfd/rohm-bd96801.c +0 -1

index 1c2a9591be7b..b7f073318873 100644
@@ -5,13 +5,9 @@
    * ROHM BD96801 PMIC driver
    *
    * This version of the "BD86801 scalable PMIC"'s driver supports only very
- * basic set of the PMIC features. Most notably, there is no support for

Quoted 8 lines

drivers/mfd/rohm-bd96801.c line 14

Quoted 6 lines

drivers/mfd/rohm-bd96801.c +0 -6

- * the ERRB interrupt and the configurations which should be done when the
- * PMIC is in STBY mode.
- *
- * Supporting the ERRB interrupt would require dropping the regmap-IRQ
- * usage or working around (or accepting a presense of) a naming conflict
- * in debugFS IRQs.

Why bother adding all that blurb in the first place?

Because, I assume there are users who would like to have the ERRB in use. The main purpose of this comment is that any such users could
a) see this version does not support ERRB.
b) can find the original RFC with ERRB supportn and a workaround.
c) know why this version does not work with ERRB and thus fix this

Quoted 3 lines

It seems this ERRB support may be missing from upstream for a while, hence I think having this note is worthy until (if) this ERRB patch lands in upstream.

What I mean is - you're adding all of these extra lines in patch 3 and removing them in patch 9.

Quoted 29 lines

Quoted 26 lines

Quoted 26 lines

drivers/mfd/rohm-bd96801.c +8 -7

+ * basic set of the PMIC features.
+ * Most notably, there is no support for the configurations which should
+ * be done when the PMIC is in STBY mode.
    *
    * Being able to reliably do the configurations like changing the
    * regulator safety limits (like limits for the over/under -voltages, over

@@ -23,16 +19,14 @@
    * be the need to configure these safety limits. Hence it's not simple to
    * come up with a generic solution.
    *
- * Users who require the ERRB handling and STBY state configurations can
- * have a look at the original RFC:
+ * Users who require the STBY state configurations can  have a look at the
+ * original RFC:
    * https://lore.kernel.org/all/[email protected]/
- * which implements a workaround to debugFS naming conflict and some of
- * the safety limit configurations - but leaves the state change handling
- * and synchronization to be implemented.
+ * which implements some of the safety limit configurations - but leaves the
+ * state change handling and synchronization to be implemented.
    *
    * It would be great to hear (and receive a patch!) if you implement the
- * STBY configuration support or a proper fix to the debugFS naming
- * conflict in your downstream driver ;)
+ * STBY configuration support or a proper fix in your downstream driver ;)
    */

...

Quoted 4 lines

Thanks for comments Lee. Reworking this will have to wait for the irqdomain name suffix, which I will continue after Hervé has done his part of the irqdomain changes. I will omit this patch from the next re-spin of the series.

I'm in no rush. :)

Quoted 52 lines · Lee Jones

Quoted 50 lines · Matti Vaittinen

Quoted 34 lines · Lee Jones

Quoted 32 lines · Matti Vaittinen

The ROHM BD96801 "scalable PMIC" provides two physical IRQs. The ERRB handling can in many cases be omitted because it is used to inform fatal IRQs, which usually kill the power from the SOC.

There may however be use-cases where the SOC has a 'back-up' emergency power source which allows some very short time of operation to try to gracefully shut down sensitive hardware. Furthermore, it is possible the processor controlling the PMIC is not powered by the PMIC. In such cases handling the ERRB IRQs may be beneficial.

Add support for ERRB IRQs.

Signed-off-by Matti Vaittinen <[email protected]>

---
Revision history:
v2 =>:
- No changes
v1 => v2:
- New patch

---
   drivers/mfd/rohm-bd96801.c | 291 ++++++++++++++++++++++++++++++++-----
   1 file changed, 253 insertions(+), 38 deletions(-)

drivers/mfd/rohm-bd96801.c +0 -1

index 1c2a9591be7b..b7f073318873 100644
@@ -5,13 +5,9 @@
    * ROHM BD96801 PMIC driver
    *
    * This version of the "BD86801 scalable PMIC"'s driver supports only very
- * basic set of the PMIC features. Most notably, there is no support for

Quoted 8 lines

drivers/mfd/rohm-bd96801.c line 14

Quoted 6 lines

drivers/mfd/rohm-bd96801.c +0 -6

- * the ERRB interrupt and the configurations which should be done when the
- * PMIC is in STBY mode.
- *
- * Supporting the ERRB interrupt would require dropping the regmap-IRQ
- * usage or working around (or accepting a presense of) a naming conflict
- * in debugFS IRQs.

Why bother adding all that blurb in the first place?

Because, I assume there are users who would like to have the ERRB in use. The main purpose of this comment is that any such users could
a) see this version does not support ERRB.
b) can find the original RFC with ERRB supportn and a workaround.
c) know why this version does not work with ERRB and thus fix this

Quoted 7 lines

Quoted 3 lines

It seems this ERRB support may be missing from upstream for a while, hence I think having this note is worthy until (if) this ERRB patch lands in upstream.

What I mean is - you're adding all of these extra lines in patch 3 and removing them in patch 9.

True. This is because I had a feeling the irqdomain changes might not get merged that fast as it seemed like something that is not completely trivial. This comment is useful if patches 7-10 aren't merged together with 1-6 - which I now also hope is the case XD

Quoted 34 lines

Quoted 33 lines

Quoted 18 lines

Quoted 18 lines

drivers/mfd/rohm-bd96801.c +5 -5

+ * basic set of the PMIC features.
+ * Most notably, there is no support for the configurations which should
+ * be done when the PMIC is in STBY mode.
    *
    * Being able to reliably do the configurations like changing the
    * regulator safety limits (like limits for the over/under -voltages, over

@@ -23,16 +19,14 @@
    * be the need to configure these safety limits. Hence it's not simple to
    * come up with a generic solution.
    *
- * Users who require the ERRB handling and STBY state configurations can
- * have a look at the original RFC:
+ * Users who require the STBY state configurations can  have a look at the
+ * original RFC:
    * https://lore.kernel.org/all/[email protected]/
- * which implements a workaround to debugFS naming conflict and some of
- * the safety limit configurations - but leaves the state change handling
- * and synchronization to be implemented.

Quoted 8 lines

drivers/mfd/rohm-bd96801.c line 30

Quoted 8 lines

drivers/mfd/rohm-bd96801.c +3 -2

+ * which implements some of the safety limit configurations - but leaves the
+ * state change handling and synchronization to be implemented.
    *
    * It would be great to hear (and receive a patch!) if you implement the
- * STBY configuration support or a proper fix to the debugFS naming
- * conflict in your downstream driver ;)
+ * STBY configuration support or a proper fix in your downstream driver ;)
    */

...

Thanks for comments Lee. Reworking this will have to wait for the irqdomain name suffix, which I will continue after Hervé has done his part of the irqdomain changes. I will omit this patch from the next re-spin of the series.

Quoted 1 line

I'm in no rush. :)

Well, glad to hear ;) I still usually try to avoid delaying sending the follow-up patches. I am under impression it is easier to review the new revision if the previous revision was not reviewed too long ago... ;)

I feel it is polite to tell the reviewers there will be some delay when I know it.