* [PATCH v7 0/6] Support ROHM BD9576MUF and BD9573MUF PMICs
@ 2021-01-22 14:30 Matti Vaittinen
2021-01-22 14:32 ` [PATCH v7 1/6] dt_bindings: mfd: Add " Matti Vaittinen
` (6 more replies)
0 siblings, 7 replies; 15+ messages in thread
From: Matti Vaittinen @ 2021-01-22 14:30 UTC (permalink / raw)
To: matti.vaittinen, mazziesaccount
Cc: Lee Jones, Rob Herring, Matti Vaittinen, Liam Girdwood,
Mark Brown, Wim Van Sebroeck, Guenter Roeck, Geert Uytterhoeven,
devicetree, linux-kernel, linux-power, linux-watchdog,
Yoshihiro Shimoda
Initial support for ROHM BD9576MUF and BD9573MUF PMICs.
These PMICs are primarily intended to be used to power the R-Car family
processors. BD9576MUF includes some additional safety features the
BD9573MUF does not have. This initial version of drivers provides
temperature, over voltage and under voltage warnings is IRQ information
is passed via DT.
This patch series includes MFD and watchdog drivers. Regulator part was
already applied but this series brings the over-/undervoltage and
temperature error notifications which consumer drivers can utilize.
WDG supports
- Enabling and pinging the watchdog
- configuring watchog timeout / window from device-tree
BD9576MUF and BD9573MUF are designed to keep the IRQ line low for whole
duration of error condition. Yet the IRQ must be acked before line is
pulled low. This means that handling the IRQ as edge triggered IRQ
will introduce the classic race of new IRQ occurring while one is
handled. (New status bit being set just after status was read but before
it was acked. That will cause the new IRQ to not be acked which leaves the
IRQ line low. With edge triggered IRQs this means that no more edges will
be seen.) In order to prevent the processor from staying in IRQ handling
loop we only handle the main status register with regmap IRQ
irq-controller (main status bits can be masked) but leave the sub-status
bits (some of which can't be masked) to be handled by the IRQ handlers.
When regulator driver handler is run, it will 'increase the depth' of IRQ
disabling (causing main status bit to stay masked when handler exits)
and arms a 1sec delayed_work to do unmasking. I am open to suggestions as
how to handle this more elegantly. Please note that the BD9573 does not
offer 'warning' interrupts and will only generate IRQ when it is
entering into emergency state and shutting down the power from the SOC.
This far the IRQ logic has only received a limited amount of testing. I
managed to get the OVD/UVD condition by probing break-out board
manually. This however usually did lead the PMIC to go to emergency
state so there may be corner cases not covered by my tests. Basic IRQ
limiting seems to be working though.
Please note that not all of the boards are expected to be utilizing the
warning IRQs. Thus populating the IRQ information from DT is optional.
Changelog v7:
- Added interrupt handling
- Added notifications and error_flag getter to regulators
Changelog v6:
- Fixed watchdog timeout
Changelog v5:
- rebased on top of v5.10-rc2
- few styling fixes in MFD as suggested by Lee
Changelog v4:
- rebased on top of 5.10-rc1
- Fix typo (repeated word maximum) from the DT binding doc
Changelog v3:
- use only one binding to specify watchdog time-out window.
Changelog v2:
- dropped already applied regulator part
- dt_bindings: Fix case for regulator-names in the example
- watchdod: unify probe error check and revise includes
---
Matti Vaittinen (6):
dt_bindings: mfd: Add ROHM BD9576MUF and BD9573MUF PMICs
mfd: Support ROHM BD9576MUF and BD9573MUF
mfd: bd9576: Add IRQ support
wdt: Support wdt on ROHM BD9576MUF and BD9573MUF
MAINTAINERS: Add ROHM BD9576MUF and BD9573MUF drivers
regulator: bd9576: Support error reporting
.../bindings/mfd/rohm,bd9576-pmic.yaml | 123 ++++
MAINTAINERS | 4 +
drivers/mfd/Kconfig | 11 +
drivers/mfd/Makefile | 1 +
drivers/mfd/rohm-bd9576.c | 186 ++++++
drivers/regulator/bd9576-regulator.c | 534 ++++++++++++++----
drivers/watchdog/Kconfig | 13 +
drivers/watchdog/Makefile | 1 +
drivers/watchdog/bd9576_wdt.c | 291 ++++++++++
include/linux/mfd/rohm-bd957x.h | 121 ++++
include/linux/mfd/rohm-generic.h | 2 +
11 files changed, 1165 insertions(+), 122 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mfd/rohm,bd9576-pmic.yaml
create mode 100644 drivers/mfd/rohm-bd9576.c
create mode 100644 drivers/watchdog/bd9576_wdt.c
create mode 100644 include/linux/mfd/rohm-bd957x.h
base-commit: 7c53f6b671f4aba70ff15e1b05148b10d58c2837
--
2.25.4
--
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 =]
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v7 1/6] dt_bindings: mfd: Add ROHM BD9576MUF and BD9573MUF PMICs
2021-01-22 14:30 [PATCH v7 0/6] Support ROHM BD9576MUF and BD9573MUF PMICs Matti Vaittinen
@ 2021-01-22 14:32 ` Matti Vaittinen
2021-01-22 14:32 ` [PATCH v7 2/6] mfd: Support ROHM BD9576MUF and BD9573MUF Matti Vaittinen
` (5 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Matti Vaittinen @ 2021-01-22 14:32 UTC (permalink / raw)
To: matti.vaittinen, mazziesaccount
Cc: Lee Jones, Rob Herring, Matti Vaittinen, Geert Uytterhoeven,
devicetree, linux-kernel, linux-power, Yoshihiro Shimoda
Add bindings for ROHM BD9576MUF and BD9573MUF PMICs. These
PMICs are primarily intended to be used to power the R-Car series
processors. They provide 6 power outputs, safety features and a
watchdog with two functional modes.
Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
Changes since v6:
- No changes
.../bindings/mfd/rohm,bd9576-pmic.yaml | 123 ++++++++++++++++++
1 file changed, 123 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/rohm,bd9576-pmic.yaml
diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd9576-pmic.yaml b/Documentation/devicetree/bindings/mfd/rohm,bd9576-pmic.yaml
new file mode 100644
index 000000000000..6483860da955
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/rohm,bd9576-pmic.yaml
@@ -0,0 +1,123 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/rohm,bd9576-pmic.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ROHM BD9576MUF and BD9573MUF Power Management Integrated Circuit bindings
+
+maintainers:
+ - Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
+
+description: |
+ BD9576MUF and BD9573MUF are power management ICs primarily intended for
+ powering the R-Car series processors.
+ The IC provides 6 power outputs with configurable sequencing and safety
+ monitoring. A watchdog logic with slow ping/windowed modes is also included.
+
+properties:
+ compatible:
+ enum:
+ - rohm,bd9576
+ - rohm,bd9573
+
+ reg:
+ description:
+ I2C slave address.
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ rohm,vout1-en-low:
+ description:
+ BD9576 and BD9573 VOUT1 regulator enable state can be individually
+ controlled by a GPIO. This is dictated by state of vout1-en pin during
+ the PMIC startup. If vout1-en is LOW during PMIC startup then the VOUT1
+ enable sate is controlled via this pin. Set this property if vout1-en
+ is wired to be down at PMIC start-up.
+ type: boolean
+
+ rohm,vout1-en-gpios:
+ description:
+ GPIO specifier to specify the GPIO connected to vout1-en for vout1 ON/OFF
+ state control.
+ maxItems: 1
+
+ rohm,ddr-sel-low:
+ description:
+ The BD9576 and BD9573 output voltage for DDR can be selected by setting
+ the ddr-sel pin low or high. Set this property if ddr-sel is grounded.
+ type: boolean
+
+ rohm,watchdog-enable-gpios:
+ description: The GPIO line used to enable the watchdog.
+ maxItems: 1
+
+ rohm,watchdog-ping-gpios:
+ description: The GPIO line used to ping the watchdog.
+ maxItems: 1
+
+ rohm,hw-timeout-ms:
+ maxItems: 2
+ description:
+ Watchog timeout in milliseconds. If single value is given it is
+ the maximum timeout. Eg. if pinging watchdog is not done within this time
+ limit the watchdog will be triggered. If two values are given watchdog
+ is configured in "window mode". Then first value is limit for short-ping
+ Eg. if watchdog is pinged sooner than that the watchdog will trigger.
+ When two values is given the second value is the maximum timeout.
+ # (HW) minimum for short timeout is 2ms, maximum 220 ms.
+ # (HW) minimum for max timeout is 4ms, maximum 4416 ms.
+
+ regulators:
+ $ref: ../regulator/rohm,bd9576-regulator.yaml
+ description:
+ List of child nodes that specify the regulators.
+
+required:
+ - compatible
+ - reg
+ - regulators
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ #include <dt-bindings/leds/common.h>
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ pmic: pmic@30 {
+ compatible = "rohm,bd9576";
+ reg = <0x30>;
+ rohm,vout1-en-low;
+ rohm,vout1-en-gpios = <&gpio2 6 GPIO_ACTIVE_HIGH>;
+ rohm,ddr-sel-low;
+ rohm,watchdog-enable-gpios = <&gpio2 6 GPIO_ACTIVE_HIGH>;
+ rohm,watchdog-ping-gpios = <&gpio2 7 GPIO_ACTIVE_HIGH>;
+ rohm,hw-timeout-ms = <150>, <2300>;
+
+ regulators {
+ boost1: regulator-vd50 {
+ regulator-name = "VD50";
+ };
+ buck1: regulator-vd18 {
+ regulator-name = "VD18";
+ };
+ buck2: regulator-vdddr {
+ regulator-name = "VDDDR";
+ };
+ buck3: regulator-vd10 {
+ regulator-name = "VD10";
+ };
+ ldo: regulator-voutl1 {
+ regulator-name = "VOUTL1";
+ };
+ sw: regulator-vouts1 {
+ regulator-name = "VOUTS1";
+ };
+ };
+ };
+ };
--
2.25.4
--
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 =]
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v7 2/6] mfd: Support ROHM BD9576MUF and BD9573MUF
2021-01-22 14:30 [PATCH v7 0/6] Support ROHM BD9576MUF and BD9573MUF PMICs Matti Vaittinen
2021-01-22 14:32 ` [PATCH v7 1/6] dt_bindings: mfd: Add " Matti Vaittinen
@ 2021-01-22 14:32 ` Matti Vaittinen
2021-02-09 14:55 ` Lee Jones
2021-01-22 14:33 ` [PATCH v7 3/6] mfd: bd9576: Add IRQ support Matti Vaittinen
` (4 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Matti Vaittinen @ 2021-01-22 14:32 UTC (permalink / raw)
To: matti.vaittinen, mazziesaccount
Cc: Lee Jones, Rob Herring, Matti Vaittinen, Liam Girdwood,
Mark Brown, Wim Van Sebroeck, Guenter Roeck, Geert Uytterhoeven,
devicetree, linux-kernel, linux-power, linux-watchdog,
Yoshihiro Shimoda
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 since v6:
- no changes
drivers/mfd/Kconfig | 11 ++++
drivers/mfd/Makefile | 1 +
drivers/mfd/rohm-bd9576.c | 108 +++++++++++++++++++++++++++++++
include/linux/mfd/rohm-bd957x.h | 59 +++++++++++++++++
include/linux/mfd/rohm-generic.h | 2 +
5 files changed, 181 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..f4dd9e438427
--- /dev/null
+++ b/drivers/mfd/rohm-bd9576.c
@@ -0,0 +1,108 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+//
+// Copyright (C) 2020 ROHM Semiconductors
+//
+// ROHM BD9576MUF and BD9573MUF 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-bd957x.h>
+#include <linux/mfd/rohm-generic.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/regmap.h>
+#include <linux/types.h>
+
+static struct mfd_cell bd9573_mfd_cells[] = {
+ { .name = "bd9573-pmic", },
+ { .name = "bd9576-wdt", },
+};
+
+static struct mfd_cell bd9576_mfd_cells[] = {
+ { .name = "bd9576-pmic", },
+ { .name = "bd9576-wdt", },
+};
+
+static const struct regmap_range volatile_ranges[] = {
+ regmap_reg_range(BD957X_REG_SMRB_ASSERT, BD957X_REG_SMRB_ASSERT),
+ regmap_reg_range(BD957X_REG_PMIC_INTERNAL_STAT,
+ BD957X_REG_PMIC_INTERNAL_STAT),
+ regmap_reg_range(BD957X_REG_INT_THERM_STAT, BD957X_REG_INT_THERM_STAT),
+ regmap_reg_range(BD957X_REG_INT_OVP_STAT, BD957X_REG_INT_SYS_STAT),
+ regmap_reg_range(BD957X_REG_INT_MAIN_STAT, BD957X_REG_INT_MAIN_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 bd957x_regmap = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .volatile_table = &volatile_regs,
+ .max_register = BD957X_MAX_REGISTER,
+ .cache_type = REGCACHE_RBTREE,
+};
+
+static int bd957x_i2c_probe(struct i2c_client *i2c,
+ const struct i2c_device_id *id)
+{
+ int ret;
+ struct regmap *regmap;
+ struct mfd_cell *mfd;
+ int cells;
+ unsigned long chip_type;
+
+ chip_type = (unsigned long)of_device_get_match_data(&i2c->dev);
+
+ switch (chip_type) {
+ case ROHM_CHIP_TYPE_BD9576:
+ mfd = bd9576_mfd_cells;
+ cells = ARRAY_SIZE(bd9576_mfd_cells);
+ break;
+ case ROHM_CHIP_TYPE_BD9573:
+ mfd = bd9573_mfd_cells;
+ cells = ARRAY_SIZE(bd9573_mfd_cells);
+ break;
+ default:
+ dev_err(&i2c->dev, "Unknown device type");
+ return -EINVAL;
+ }
+
+ regmap = devm_regmap_init_i2c(i2c, &bd957x_regmap);
+ if (IS_ERR(regmap)) {
+ dev_err(&i2c->dev, "Failed to initialize Regmap\n");
+ return PTR_ERR(regmap);
+ }
+
+ ret = devm_mfd_add_devices(&i2c->dev, PLATFORM_DEVID_AUTO, mfd, cells,
+ NULL, 0, NULL);
+ if (ret)
+ dev_err(&i2c->dev, "Failed to create subdevices\n");
+
+ return ret;
+}
+
+static const struct of_device_id bd957x_of_match[] = {
+ { .compatible = "rohm,bd9576", .data = (void *)ROHM_CHIP_TYPE_BD9576, },
+ { .compatible = "rohm,bd9573", .data = (void *)ROHM_CHIP_TYPE_BD9573, },
+ { },
+};
+MODULE_DEVICE_TABLE(of, bd957x_of_match);
+
+static struct i2c_driver bd957x_drv = {
+ .driver = {
+ .name = "rohm-bd957x",
+ .of_match_table = bd957x_of_match,
+ },
+ .probe = &bd957x_i2c_probe,
+};
+module_i2c_driver(bd957x_drv);
+
+MODULE_AUTHOR("Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>");
+MODULE_DESCRIPTION("ROHM BD9576MUF and BD9573MUF Power Management IC driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/rohm-bd957x.h b/include/linux/mfd/rohm-bd957x.h
new file mode 100644
index 000000000000..3e7ca6fe5d4f
--- /dev/null
+++ b/include/linux/mfd/rohm-bd957x.h
@@ -0,0 +1,59 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/* Copyright (C) 2020 ROHM Semiconductors */
+
+#ifndef __LINUX_MFD_BD957X_H__
+#define __LINUX_MFD_BD957X_H__
+
+enum {
+ BD957X_VD50,
+ BD957X_VD18,
+ BD957X_VDDDR,
+ BD957X_VD10,
+ BD957X_VOUTL1,
+ BD957X_VOUTS1,
+};
+
+#define BD957X_REG_SMRB_ASSERT 0x15
+#define BD957X_REG_PMIC_INTERNAL_STAT 0x20
+#define BD957X_REG_INT_THERM_STAT 0x23
+#define BD957X_REG_INT_THERM_MASK 0x24
+#define BD957X_REG_INT_OVP_STAT 0x25
+#define BD957X_REG_INT_SCP_STAT 0x26
+#define BD957X_REG_INT_OCP_STAT 0x27
+#define BD957X_REG_INT_OVD_STAT 0x28
+#define BD957X_REG_INT_UVD_STAT 0x29
+#define BD957X_REG_INT_UVP_STAT 0x2a
+#define BD957X_REG_INT_SYS_STAT 0x2b
+#define BD957X_REG_INT_SYS_MASK 0x2c
+#define BD957X_REG_INT_MAIN_STAT 0x30
+#define BD957X_REG_INT_MAIN_MASK 0x31
+
+#define BD957X_REG_WDT_CONF 0x16
+
+#define BD957X_REG_POW_TRIGGER1 0x41
+#define BD957X_REG_POW_TRIGGER2 0x42
+#define BD957X_REG_POW_TRIGGER3 0x43
+#define BD957X_REG_POW_TRIGGER4 0x44
+#define BD957X_REG_POW_TRIGGERL1 0x45
+#define BD957X_REG_POW_TRIGGERS1 0x46
+
+#define BD957X_REGULATOR_EN_MASK 0xff
+#define BD957X_REGULATOR_DIS_VAL 0xff
+
+#define BD957X_VSEL_REG_MASK 0xff
+
+#define BD957X_MASK_VOUT1_TUNE 0x87
+#define BD957X_MASK_VOUT2_TUNE 0x87
+#define BD957X_MASK_VOUT3_TUNE 0x1f
+#define BD957X_MASK_VOUT4_TUNE 0x1f
+#define BD957X_MASK_VOUTL1_TUNE 0x87
+
+#define BD957X_REG_VOUT1_TUNE 0x50
+#define BD957X_REG_VOUT2_TUNE 0x53
+#define BD957X_REG_VOUT3_TUNE 0x56
+#define BD957X_REG_VOUT4_TUNE 0x59
+#define BD957X_REG_VOUTL1_TUNE 0x5c
+
+#define BD957X_MAX_REGISTER 0x61
+
+#endif
diff --git a/include/linux/mfd/rohm-generic.h b/include/linux/mfd/rohm-generic.h
index 4283b5b33e04..58b4f1a0f4af 100644
--- a/include/linux/mfd/rohm-generic.h
+++ b/include/linux/mfd/rohm-generic.h
@@ -12,6 +12,8 @@ enum rohm_chip_type {
ROHM_CHIP_TYPE_BD71847,
ROHM_CHIP_TYPE_BD70528,
ROHM_CHIP_TYPE_BD71828,
+ ROHM_CHIP_TYPE_BD9576,
+ ROHM_CHIP_TYPE_BD9573,
ROHM_CHIP_TYPE_AMOUNT
};
--
2.25.4
--
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 =]
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v7 3/6] mfd: bd9576: Add IRQ support
2021-01-22 14:30 [PATCH v7 0/6] Support ROHM BD9576MUF and BD9573MUF PMICs Matti Vaittinen
2021-01-22 14:32 ` [PATCH v7 1/6] dt_bindings: mfd: Add " Matti Vaittinen
2021-01-22 14:32 ` [PATCH v7 2/6] mfd: Support ROHM BD9576MUF and BD9573MUF Matti Vaittinen
@ 2021-01-22 14:33 ` Matti Vaittinen
2021-02-09 15:25 ` Lee Jones
2021-01-22 14:34 ` [PATCH v7 4/6] wdt: Support wdt on ROHM BD9576MUF and BD9573MUF Matti Vaittinen
` (3 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Matti Vaittinen @ 2021-01-22 14:33 UTC (permalink / raw)
To: matti.vaittinen, mazziesaccount
Cc: Lee Jones, Matti Vaittinen, Liam Girdwood, Mark Brown,
Geert Uytterhoeven, linux-kernel, linux-power, Yoshihiro Shimoda
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 v6:
- new patch
drivers/mfd/rohm-bd9576.c | 80 +++++++++++++++++-
include/linux/mfd/rohm-bd957x.h | 140 +++++++++++++++++++++++---------
2 files changed, 180 insertions(+), 40 deletions(-)
diff --git a/drivers/mfd/rohm-bd9576.c b/drivers/mfd/rohm-bd9576.c
index f4dd9e438427..99fa0f333277 100644
--- a/drivers/mfd/rohm-bd9576.c
+++ b/drivers/mfd/rohm-bd9576.c
@@ -16,12 +16,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-pmic", },
{ .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.
+ */
{ .name = "bd9576-pmic", },
{ .name = "bd9576-wdt", },
};
@@ -48,6 +63,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)
{
@@ -56,6 +94,7 @@ static int bd957x_i2c_probe(struct i2c_client *i2c,
struct mfd_cell *mfd;
int cells;
unsigned long chip_type;
+ struct irq_domain *domain;
chip_type = (unsigned long)of_device_get_match_data(&i2c->dev);
@@ -67,6 +106,11 @@ static int bd957x_i2c_probe(struct i2c_client *i2c,
case ROHM_CHIP_TYPE_BD9573:
mfd = bd9573_mfd_cells;
cells = ARRAY_SIZE(bd9573_mfd_cells);
+ /* BD9573 only supports fatal IRQs which we do not handle */
+ if (i2c->irq) {
+ dev_err(&i2c->dev, "no supported irqs on BD9573\n");
+ return -EINVAL;
+ }
break;
default:
dev_err(&i2c->dev, "Unknown device type");
@@ -78,9 +122,43 @@ static int bd957x_i2c_probe(struct i2c_client *i2c,
dev_err(&i2c->dev, "Failed to initialize Regmap\n");
return PTR_ERR(regmap);
}
+ /*
+ * BD9576 behaves badly. It kepts IRQ asserted for the whole
+ * duration of detected HW condition (like over temp). This does
+ * not play nicely under any condition but we can work around it
+ * except when we have shared IRQs. So we don't require IRQ to be
+ * populated to help those poor sods who did connect IRQ to shared pin.
+ * 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 (i2c->irq) {
+ 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_info(&i2c->dev, "Using IRQs for BD9576MUF\n");
+ } 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, mfd, cells,
- NULL, 0, NULL);
+ 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 3e7ca6fe5d4f..4fa632d8467a 100644
--- a/include/linux/mfd/rohm-bd957x.h
+++ b/include/linux/mfd/rohm-bd957x.h
@@ -13,47 +13,109 @@ 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
+ * and '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 cleared
+ * in HW. This is _really_ annoying for IRQs like the one informing high
+ * temperature. Best mitigation for high temperature for sure is to
+ * keep the processor in IRQ loop, right? (NO!)
+ *
+ * So - 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
-#define BD957X_REG_INT_THERM_MASK 0x24
-#define BD957X_REG_INT_OVP_STAT 0x25
-#define BD957X_REG_INT_SCP_STAT 0x26
-#define BD957X_REG_INT_OCP_STAT 0x27
-#define BD957X_REG_INT_OVD_STAT 0x28
-#define BD957X_REG_INT_UVD_STAT 0x29
-#define BD957X_REG_INT_UVP_STAT 0x2a
-#define BD957X_REG_INT_SYS_STAT 0x2b
-#define BD957X_REG_INT_SYS_MASK 0x2c
-#define BD957X_REG_INT_MAIN_STAT 0x30
-#define BD957X_REG_INT_MAIN_MASK 0x31
-
-#define BD957X_REG_WDT_CONF 0x16
-
-#define BD957X_REG_POW_TRIGGER1 0x41
-#define BD957X_REG_POW_TRIGGER2 0x42
-#define BD957X_REG_POW_TRIGGER3 0x43
-#define BD957X_REG_POW_TRIGGER4 0x44
-#define BD957X_REG_POW_TRIGGERL1 0x45
-#define BD957X_REG_POW_TRIGGERS1 0x46
-
-#define BD957X_REGULATOR_EN_MASK 0xff
-#define BD957X_REGULATOR_DIS_VAL 0xff
-
-#define BD957X_VSEL_REG_MASK 0xff
-
-#define BD957X_MASK_VOUT1_TUNE 0x87
-#define BD957X_MASK_VOUT2_TUNE 0x87
-#define BD957X_MASK_VOUT3_TUNE 0x1f
-#define BD957X_MASK_VOUT4_TUNE 0x1f
-#define BD957X_MASK_VOUTL1_TUNE 0x87
-
-#define BD957X_REG_VOUT1_TUNE 0x50
-#define BD957X_REG_VOUT2_TUNE 0x53
-#define BD957X_REG_VOUT3_TUNE 0x56
-#define BD957X_REG_VOUT4_TUNE 0x59
-#define BD957X_REG_VOUTL1_TUNE 0x5c
-
-#define BD957X_MAX_REGISTER 0x61
+#define BD957X_REG_INT_THERM_MASK 0x24
+#define BD957X_REG_INT_OVP_STAT 0x25
+#define BD957X_REG_INT_SCP_STAT 0x26
+#define BD957X_REG_INT_OCP_STAT 0x27
+#define BD957X_REG_INT_OVD_STAT 0x28
+#define BD957X_REG_INT_UVD_STAT 0x29
+#define BD957X_REG_INT_UVP_STAT 0x2a
+#define BD957X_REG_INT_SYS_STAT 0x2b
+#define BD957X_REG_INT_SYS_MASK 0x2c
+#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
+#define BD957X_REG_POW_TRIGGER2 0x42
+#define BD957X_REG_POW_TRIGGER3 0x43
+#define BD957X_REG_POW_TRIGGER4 0x44
+#define BD957X_REG_POW_TRIGGERL1 0x45
+#define BD957X_REG_POW_TRIGGERS1 0x46
+
+#define BD957X_REGULATOR_EN_MASK 0xff
+#define BD957X_REGULATOR_DIS_VAL 0xff
+
+#define BD957X_VSEL_REG_MASK 0xff
+
+#define BD957X_MASK_VOUT1_TUNE 0x87
+#define BD957X_MASK_VOUT2_TUNE 0x87
+#define BD957X_MASK_VOUT3_TUNE 0x1f
+#define BD957X_MASK_VOUT4_TUNE 0x1f
+#define BD957X_MASK_VOUTL1_TUNE 0x87
+
+#define BD957X_REG_VOUT1_TUNE 0x50
+#define BD957X_REG_VOUT2_TUNE 0x53
+#define BD957X_REG_VOUT3_TUNE 0x56
+#define BD957X_REG_VOUT4_TUNE 0x59
+#define BD957X_REG_VOUTL1_TUNE 0x5c
+
+#define BD957X_MAX_REGISTER 0x61
#endif
--
2.25.4
--
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 =]
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v7 4/6] wdt: Support wdt on ROHM BD9576MUF and BD9573MUF
2021-01-22 14:30 [PATCH v7 0/6] Support ROHM BD9576MUF and BD9573MUF PMICs Matti Vaittinen
` (2 preceding siblings ...)
2021-01-22 14:33 ` [PATCH v7 3/6] mfd: bd9576: Add IRQ support Matti Vaittinen
@ 2021-01-22 14:34 ` Matti Vaittinen
2021-01-22 14:35 ` [PATCH v7 5/6] MAINTAINERS: Add ROHM BD9576MUF and BD9573MUF drivers Matti Vaittinen
` (2 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Matti Vaittinen @ 2021-01-22 14:34 UTC (permalink / raw)
To: matti.vaittinen, mazziesaccount
Cc: Lee Jones, Matti Vaittinen, Wim Van Sebroeck, Guenter Roeck,
Geert Uytterhoeven, linux-kernel, linux-power, linux-watchdog,
Yoshihiro Shimoda
Add Watchdog support for ROHM BD9576MUF and BD9573MUF PMICs which are
mainly used to power the R-Car series processors. The watchdog is
pinged using a GPIO and enabled using another GPIO. Additionally
watchdog time-out can be configured to HW prior starting the watchdog.
Watchdog timeout can be configured to detect only delayed ping or in
a window mode where also too fast pings are detected.
Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
---
Changes since v6:
- no changes
drivers/watchdog/Kconfig | 13 ++
drivers/watchdog/Makefile | 1 +
drivers/watchdog/bd9576_wdt.c | 291 ++++++++++++++++++++++++++++++++++
3 files changed, 305 insertions(+)
create mode 100644 drivers/watchdog/bd9576_wdt.c
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 7ff941e71b79..67eff0cbd7e1 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -172,6 +172,19 @@ config BD70528_WATCHDOG
Alternatively say M to compile the driver as a module,
which will be called bd70528_wdt.
+config BD957XMUF_WATCHDOG
+ tristate "ROHM BD9576MUF and BD9573MUF PMIC Watchdog"
+ depends on MFD_ROHM_BD957XMUF
+ select WATCHDOG_CORE
+ help
+ Support for the watchdog in the ROHM BD9576 and BD9573 PMICs.
+ These PMIC ICs contain watchdog block which can be configured
+ to toggle reset line if SoC fails to ping watchdog via GPIO.
+
+ Say Y here to include support for the ROHM BD9576 or BD9573
+ watchdog. Alternatively say M to compile the driver as a module,
+ which will be called bd9576_wdt.
+
config DA9052_WATCHDOG
tristate "Dialog DA9052 Watchdog"
depends on PMIC_DA9052 || COMPILE_TEST
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 5c74ee19d441..9341f72bf0aa 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -208,6 +208,7 @@ obj-$(CONFIG_XEN_WDT) += xen_wdt.o
# Architecture Independent
obj-$(CONFIG_BD70528_WATCHDOG) += bd70528_wdt.o
+obj-$(CONFIG_BD957XMUF_WATCHDOG) += bd9576_wdt.o
obj-$(CONFIG_DA9052_WATCHDOG) += da9052_wdt.o
obj-$(CONFIG_DA9055_WATCHDOG) += da9055_wdt.o
obj-$(CONFIG_DA9062_WATCHDOG) += da9062_wdt.o
diff --git a/drivers/watchdog/bd9576_wdt.c b/drivers/watchdog/bd9576_wdt.c
new file mode 100644
index 000000000000..0b6999f3b6e8
--- /dev/null
+++ b/drivers/watchdog/bd9576_wdt.c
@@ -0,0 +1,291 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2020 ROHM Semiconductors
+ *
+ * ROHM BD9576MUF and BD9573MUF Watchdog driver
+ */
+
+#include <linux/err.h>
+#include <linux/gpio/consumer.h>
+#include <linux/mfd/rohm-bd957x.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/watchdog.h>
+
+static bool nowayout;
+module_param(nowayout, bool, 0);
+MODULE_PARM_DESC(nowayout,
+ "Watchdog cannot be stopped once started (default=\"false\")");
+
+#define HW_MARGIN_MIN 2
+#define HW_MARGIN_MAX 4416
+#define BD957X_WDT_DEFAULT_MARGIN 4416
+#define WATCHDOG_TIMEOUT 30
+
+struct bd9576_wdt_priv {
+ struct gpio_desc *gpiod_ping;
+ struct gpio_desc *gpiod_en;
+ struct device *dev;
+ struct regmap *regmap;
+ bool always_running;
+ struct watchdog_device wdd;
+};
+
+static void bd9576_wdt_disable(struct bd9576_wdt_priv *priv)
+{
+ gpiod_set_value_cansleep(priv->gpiod_en, 0);
+}
+
+static int bd9576_wdt_ping(struct watchdog_device *wdd)
+{
+ struct bd9576_wdt_priv *priv = watchdog_get_drvdata(wdd);
+
+ /* Pulse */
+ gpiod_set_value_cansleep(priv->gpiod_ping, 1);
+ gpiod_set_value_cansleep(priv->gpiod_ping, 0);
+
+ return 0;
+}
+
+static int bd9576_wdt_start(struct watchdog_device *wdd)
+{
+ struct bd9576_wdt_priv *priv = watchdog_get_drvdata(wdd);
+
+ gpiod_set_value_cansleep(priv->gpiod_en, 1);
+
+ return bd9576_wdt_ping(wdd);
+}
+
+static int bd9576_wdt_stop(struct watchdog_device *wdd)
+{
+ struct bd9576_wdt_priv *priv = watchdog_get_drvdata(wdd);
+
+ if (!priv->always_running)
+ bd9576_wdt_disable(priv);
+ else
+ set_bit(WDOG_HW_RUNNING, &wdd->status);
+
+ return 0;
+}
+
+static const struct watchdog_info bd957x_wdt_ident = {
+ .options = WDIOF_MAGICCLOSE | WDIOF_KEEPALIVEPING |
+ WDIOF_SETTIMEOUT,
+ .identity = "BD957x Watchdog",
+};
+
+static const struct watchdog_ops bd957x_wdt_ops = {
+ .owner = THIS_MODULE,
+ .start = bd9576_wdt_start,
+ .stop = bd9576_wdt_stop,
+ .ping = bd9576_wdt_ping,
+};
+
+/* Unit is hundreds of uS */
+#define FASTNG_MIN 23
+
+static int find_closest_fast(int target, int *sel, int *val)
+{
+ int i;
+ int window = FASTNG_MIN;
+
+ for (i = 0; i < 8 && window < target; i++)
+ window <<= 1;
+
+ *val = window;
+ *sel = i;
+
+ if (i == 8)
+ return -EINVAL;
+
+ return 0;
+
+}
+
+static int find_closest_slow_by_fast(int fast_val, int target, int *slowsel)
+{
+ int sel;
+ static const int multipliers[] = {2, 3, 7, 15};
+
+ for (sel = 0; sel < ARRAY_SIZE(multipliers) &&
+ multipliers[sel] * fast_val < target; sel++)
+ ;
+
+ if (sel == ARRAY_SIZE(multipliers))
+ return -EINVAL;
+
+ *slowsel = sel;
+
+ return 0;
+}
+
+static int find_closest_slow(int target, int *slow_sel, int *fast_sel)
+{
+ static const int multipliers[] = {2, 3, 7, 15};
+ int i, j;
+ int val = 0;
+ int window = FASTNG_MIN;
+
+ for (i = 0; i < 8; i++) {
+ for (j = 0; j < ARRAY_SIZE(multipliers); j++) {
+ int slow;
+
+ slow = window * multipliers[j];
+ if (slow >= target && (!val || slow < val)) {
+ val = slow;
+ *fast_sel = i;
+ *slow_sel = j;
+ }
+ }
+ window <<= 1;
+ }
+ if (!val)
+ return -EINVAL;
+
+ return 0;
+}
+
+#define BD957X_WDG_TYPE_WINDOW BIT(5)
+#define BD957X_WDG_TYPE_SLOW 0
+#define BD957X_WDG_TYPE_MASK BIT(5)
+#define BD957X_WDG_NG_RATIO_MASK 0x18
+#define BD957X_WDG_FASTNG_MASK 0x7
+
+static int bd957x_set_wdt_mode(struct bd9576_wdt_priv *priv, int hw_margin,
+ int hw_margin_min)
+{
+ int ret, fastng, slowng, type, reg, mask;
+ struct device *dev = priv->dev;
+
+ /* convert to 100uS */
+ hw_margin *= 10;
+ hw_margin_min *= 10;
+ if (hw_margin_min) {
+ int min;
+
+ type = BD957X_WDG_TYPE_WINDOW;
+ dev_dbg(dev, "Setting type WINDOW 0x%x\n", type);
+ ret = find_closest_fast(hw_margin_min, &fastng, &min);
+ if (ret) {
+ dev_err(dev, "bad WDT window for fast timeout\n");
+ return ret;
+ }
+
+ ret = find_closest_slow_by_fast(min, hw_margin, &slowng);
+ if (ret) {
+ dev_err(dev, "bad WDT window\n");
+ return ret;
+ }
+
+ } else {
+ type = BD957X_WDG_TYPE_SLOW;
+ dev_dbg(dev, "Setting type SLOW 0x%x\n", type);
+ ret = find_closest_slow(hw_margin, &slowng, &fastng);
+ if (ret) {
+ dev_err(dev, "bad WDT window\n");
+ return ret;
+ }
+ }
+
+ slowng <<= ffs(BD957X_WDG_NG_RATIO_MASK) - 1;
+ reg = type | slowng | fastng;
+ mask = BD957X_WDG_TYPE_MASK | BD957X_WDG_NG_RATIO_MASK |
+ BD957X_WDG_FASTNG_MASK;
+ ret = regmap_update_bits(priv->regmap, BD957X_REG_WDT_CONF,
+ mask, reg);
+
+ return ret;
+}
+
+static int bd9576_wdt_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct device_node *np = dev->parent->of_node;
+ struct bd9576_wdt_priv *priv;
+ u32 hw_margin[2];
+ u32 hw_margin_max = BD957X_WDT_DEFAULT_MARGIN, hw_margin_min = 0;
+ int ret;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, priv);
+
+ priv->dev = dev;
+ priv->regmap = dev_get_regmap(dev->parent, NULL);
+ if (!priv->regmap) {
+ dev_err(dev, "No regmap found\n");
+ return -ENODEV;
+ }
+
+ priv->gpiod_en = devm_gpiod_get_from_of_node(dev, dev->parent->of_node,
+ "rohm,watchdog-enable-gpios",
+ 0, GPIOD_OUT_LOW,
+ "watchdog-enable");
+ if (IS_ERR(priv->gpiod_en))
+ return dev_err_probe(dev, PTR_ERR(priv->gpiod_en),
+ "getting watchdog-enable GPIO failed\n");
+
+ priv->gpiod_ping = devm_gpiod_get_from_of_node(dev, dev->parent->of_node,
+ "rohm,watchdog-ping-gpios",
+ 0, GPIOD_OUT_LOW,
+ "watchdog-ping");
+ if (IS_ERR(priv->gpiod_ping))
+ return dev_err_probe(dev, PTR_ERR(priv->gpiod_ping),
+ "getting watchdog-ping GPIO failed\n");
+
+ ret = of_property_read_variable_u32_array(np, "rohm,hw-timeout-ms",
+ &hw_margin[0], 1, 2);
+ if (ret < 0 && ret != -EINVAL)
+ return ret;
+
+ if (ret == 1)
+ hw_margin_max = hw_margin[0];
+
+ if (ret == 2) {
+ hw_margin_max = hw_margin[1];
+ hw_margin_min = hw_margin[0];
+ }
+
+ ret = bd957x_set_wdt_mode(priv, hw_margin_max, hw_margin_min);
+ if (ret)
+ return ret;
+
+ priv->always_running = of_property_read_bool(np, "always-running");
+
+ watchdog_set_drvdata(&priv->wdd, priv);
+
+ priv->wdd.info = &bd957x_wdt_ident;
+ priv->wdd.ops = &bd957x_wdt_ops;
+ priv->wdd.min_hw_heartbeat_ms = hw_margin_min;
+ priv->wdd.max_hw_heartbeat_ms = hw_margin_max;
+ priv->wdd.parent = dev;
+ priv->wdd.timeout = WATCHDOG_TIMEOUT;
+
+ watchdog_init_timeout(&priv->wdd, 0, dev);
+ watchdog_set_nowayout(&priv->wdd, nowayout);
+
+ watchdog_stop_on_reboot(&priv->wdd);
+
+ if (priv->always_running)
+ bd9576_wdt_start(&priv->wdd);
+
+ return devm_watchdog_register_device(dev, &priv->wdd);
+}
+
+static struct platform_driver bd9576_wdt_driver = {
+ .driver = {
+ .name = "bd9576-wdt",
+ },
+ .probe = bd9576_wdt_probe,
+};
+
+module_platform_driver(bd9576_wdt_driver);
+
+MODULE_AUTHOR("Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>");
+MODULE_DESCRIPTION("ROHM BD9576/BD9573 Watchdog driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:bd9576-wdt");
--
2.25.4
--
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 =]
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v7 5/6] MAINTAINERS: Add ROHM BD9576MUF and BD9573MUF drivers
2021-01-22 14:30 [PATCH v7 0/6] Support ROHM BD9576MUF and BD9573MUF PMICs Matti Vaittinen
` (3 preceding siblings ...)
2021-01-22 14:34 ` [PATCH v7 4/6] wdt: Support wdt on ROHM BD9576MUF and BD9573MUF Matti Vaittinen
@ 2021-01-22 14:35 ` Matti Vaittinen
2021-01-23 18:38 ` Guenter Roeck
2021-01-22 14:36 ` [PATCH v7 6/6] regulator: bd9576: Support error reporting Matti Vaittinen
2021-02-09 14:19 ` [PATCH v7 0/6] Support ROHM BD9576MUF and BD9573MUF PMICs Vaittinen, Matti
6 siblings, 1 reply; 15+ messages in thread
From: Matti Vaittinen @ 2021-01-22 14:35 UTC (permalink / raw)
To: matti.vaittinen, mazziesaccount
Cc: Lee Jones, Rob Herring, Matti Vaittinen, Liam Girdwood,
Mark Brown, Wim Van Sebroeck, Guenter Roeck, Geert Uytterhoeven,
devicetree, linux-kernel, linux-power, linux-watchdog,
Yoshihiro Shimoda
Add maintainer entries for ROHM BD9576MUF and ROHM BD9573MUF drivers.
MFD, regulator and watchdog drivers were introduced for these PMICs.
Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---
Changes since v6:
- no changes
MAINTAINERS | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index cc1e6a5ee6e6..b59b7877258c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15336,16 +15336,20 @@ F: drivers/gpio/gpio-bd71828.c
F: drivers/mfd/rohm-bd70528.c
F: drivers/mfd/rohm-bd71828.c
F: drivers/mfd/rohm-bd718x7.c
+F: drivers/mfd/rohm-bd9576.c
F: drivers/power/supply/bd70528-charger.c
F: drivers/regulator/bd70528-regulator.c
F: drivers/regulator/bd71828-regulator.c
F: drivers/regulator/bd718x7-regulator.c
+F: drivers/regulator/bd9576-regulator.c
F: drivers/regulator/rohm-regulator.c
F: drivers/rtc/rtc-bd70528.c
F: drivers/watchdog/bd70528_wdt.c
+F: drivers/watchdog/bd9576_wdt.c
F: include/linux/mfd/rohm-bd70528.h
F: include/linux/mfd/rohm-bd71828.h
F: include/linux/mfd/rohm-bd718x7.h
+F: include/linux/mfd/rohm-bd957x.h
F: include/linux/mfd/rohm-generic.h
F: include/linux/mfd/rohm-shared.h
--
2.25.4
--
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 =]
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v7 6/6] regulator: bd9576: Support error reporting
2021-01-22 14:30 [PATCH v7 0/6] Support ROHM BD9576MUF and BD9573MUF PMICs Matti Vaittinen
` (4 preceding siblings ...)
2021-01-22 14:35 ` [PATCH v7 5/6] MAINTAINERS: Add ROHM BD9576MUF and BD9573MUF drivers Matti Vaittinen
@ 2021-01-22 14:36 ` Matti Vaittinen
2021-01-24 18:56 ` kernel test robot
2021-02-09 14:19 ` [PATCH v7 0/6] Support ROHM BD9576MUF and BD9573MUF PMICs Vaittinen, Matti
6 siblings, 1 reply; 15+ messages in thread
From: Matti Vaittinen @ 2021-01-22 14:36 UTC (permalink / raw)
To: matti.vaittinen, mazziesaccount
Cc: Lee Jones, Matti Vaittinen, Liam Girdwood, Mark Brown,
Geert Uytterhoeven, linux-kernel, linux-power, Yoshihiro Shimoda
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.
Add support for over and under voltage detection for Vout1 ... Vout4
and VoutL1. Add over-current detection for VoutS1 and finally a
thermal warning (common for all regulators) which alerts shortly
before temperature reaches the thermal shutdown point. This way
consumer drivers can build error-recovery mechanisms.
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).
Disable interrupts at handler and add 1 second timer to re-enable them.
This helps us to avoid staying in IRQ handling loop.
Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---
Changes since v6:
- new patch
Please note that the IRQ handling has only received limited set of
testing. I only managed to manually disrupt the VOUTs on board to
cause an IRQ. Usually this lead to IC being shut down. It seems
to me the basic operation of the IRQ limiter is ok.
drivers/regulator/bd9576-regulator.c | 534 +++++++++++++++++++++------
1 file changed, 412 insertions(+), 122 deletions(-)
diff --git a/drivers/regulator/bd9576-regulator.c b/drivers/regulator/bd9576-regulator.c
index a8b5832a5a1b..737c10f8acbf 100644
--- a/drivers/regulator/bd9576-regulator.c
+++ b/drivers/regulator/bd9576-regulator.c
@@ -2,10 +2,11 @@
// Copyright (C) 2020 ROHM Semiconductors
// ROHM BD9576MUF/BD9573MUF regulator driver
-#include <linux/delay.h>
+//#include <linux/delay.h>
#include <linux/err.h>
#include <linux/gpio/consumer.h>
#include <linux/interrupt.h>
+#include <linux/jiffies.h>
#include <linux/kernel.h>
#include <linux/mfd/rohm-bd957x.h>
#include <linux/mfd/rohm-generic.h>
@@ -16,6 +17,8 @@
#include <linux/regulator/machine.h>
#include <linux/regulator/of_regulator.h>
#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <linux/workqueue.h>
#define BD957X_VOUTS1_VOLT 3300000
#define BD957X_VOUTS4_BASE_VOLT 1030000
@@ -39,6 +42,22 @@ static int voutl1_volt_table[] = {2500000, 2540000, 2580000, 2620000, 2660000,
struct bd957x_regulator_data {
struct regulator_desc desc;
int base_voltage;
+ struct regulator_dev *rdev;
+ int regulator_err; /* Error flag set from IRQ */
+};
+
+#define BD9576_NUM_REGULATORS 6
+struct bd957x_data {
+ struct bd957x_regulator_data regulator_data[BD9576_NUM_REGULATORS];
+ struct regmap *regmap;
+ struct delayed_work therm_irq_suppress;
+ struct delayed_work ovd_irq_suppress;
+ struct delayed_work uvd_irq_suppress;
+ unsigned int therm_irq;
+ unsigned int ovd_irq;
+ unsigned int uvd_irq;
+ spinlock_t err_lock;
+ int regulator_global_err;
};
static int bd957x_vout34_list_voltage(struct regulator_dev *rdev,
@@ -72,151 +91,392 @@ static int bd957x_list_voltage(struct regulator_dev *rdev,
return desc->volt_table[index];
}
+static int bd9576_get_error_flags(struct regulator_dev *rdev,
+ unsigned int *flags)
+{
+ struct bd957x_data *d;
+ struct bd957x_regulator_data *r;
+
+ r = container_of(rdev->desc, struct bd957x_regulator_data, desc);
+ d = rdev_get_drvdata(rdev);
+
+ spin_lock(&d->err_lock);
+ *flags = d->regulator_global_err | r->regulator_err;
+ spin_unlock(&d->err_lock);
+
+ return 0;
+}
+
static const struct regulator_ops bd957x_vout34_ops = {
.is_enabled = regulator_is_enabled_regmap,
.list_voltage = bd957x_vout34_list_voltage,
.get_voltage_sel = regulator_get_voltage_sel_regmap,
+ .get_error_flags = bd9576_get_error_flags,
};
static const struct regulator_ops bd957X_vouts1_regulator_ops = {
.is_enabled = regulator_is_enabled_regmap,
+ .get_error_flags = bd9576_get_error_flags,
};
static const struct regulator_ops bd957x_ops = {
.is_enabled = regulator_is_enabled_regmap,
.list_voltage = bd957x_list_voltage,
.get_voltage_sel = regulator_get_voltage_sel_regmap,
+ .get_error_flags = bd9576_get_error_flags,
};
-static struct bd957x_regulator_data bd9576_regulators[] = {
- {
- .desc = {
- .name = "VD50",
- .of_match = of_match_ptr("regulator-vd50"),
- .regulators_node = of_match_ptr("regulators"),
- .id = BD957X_VD50,
- .type = REGULATOR_VOLTAGE,
- .ops = &bd957x_ops,
- .volt_table = &vout1_volt_table[0],
- .n_voltages = ARRAY_SIZE(vout1_volt_table),
- .vsel_reg = BD957X_REG_VOUT1_TUNE,
- .vsel_mask = BD957X_MASK_VOUT1_TUNE,
- .enable_reg = BD957X_REG_POW_TRIGGER1,
- .enable_mask = BD957X_REGULATOR_EN_MASK,
- .enable_val = BD957X_REGULATOR_DIS_VAL,
- .enable_is_inverted = true,
- .owner = THIS_MODULE,
+static struct bd957x_data bd957x_regulators = {
+ .regulator_data = {
+ {
+ .desc = {
+ .name = "VD50",
+ .of_match = of_match_ptr("regulator-vd50"),
+ .regulators_node = of_match_ptr("regulators"),
+ .id = BD957X_VD50,
+ .type = REGULATOR_VOLTAGE,
+ .ops = &bd957x_ops,
+ .volt_table = &vout1_volt_table[0],
+ .n_voltages = ARRAY_SIZE(vout1_volt_table),
+ .vsel_reg = BD957X_REG_VOUT1_TUNE,
+ .vsel_mask = BD957X_MASK_VOUT1_TUNE,
+ .enable_reg = BD957X_REG_POW_TRIGGER1,
+ .enable_mask = BD957X_REGULATOR_EN_MASK,
+ .enable_val = BD957X_REGULATOR_DIS_VAL,
+ .enable_is_inverted = true,
+ .owner = THIS_MODULE,
+ },
},
- },
- {
- .desc = {
- .name = "VD18",
- .of_match = of_match_ptr("regulator-vd18"),
- .regulators_node = of_match_ptr("regulators"),
- .id = BD957X_VD18,
- .type = REGULATOR_VOLTAGE,
- .ops = &bd957x_ops,
- .volt_table = &vout2_volt_table[0],
- .n_voltages = ARRAY_SIZE(vout2_volt_table),
- .vsel_reg = BD957X_REG_VOUT2_TUNE,
- .vsel_mask = BD957X_MASK_VOUT2_TUNE,
- .enable_reg = BD957X_REG_POW_TRIGGER2,
- .enable_mask = BD957X_REGULATOR_EN_MASK,
- .enable_val = BD957X_REGULATOR_DIS_VAL,
- .enable_is_inverted = true,
- .owner = THIS_MODULE,
+ {
+ .desc = {
+ .name = "VD18",
+ .of_match = of_match_ptr("regulator-vd18"),
+ .regulators_node = of_match_ptr("regulators"),
+ .id = BD957X_VD18,
+ .type = REGULATOR_VOLTAGE,
+ .ops = &bd957x_ops,
+ .volt_table = &vout2_volt_table[0],
+ .n_voltages = ARRAY_SIZE(vout2_volt_table),
+ .vsel_reg = BD957X_REG_VOUT2_TUNE,
+ .vsel_mask = BD957X_MASK_VOUT2_TUNE,
+ .enable_reg = BD957X_REG_POW_TRIGGER2,
+ .enable_mask = BD957X_REGULATOR_EN_MASK,
+ .enable_val = BD957X_REGULATOR_DIS_VAL,
+ .enable_is_inverted = true,
+ .owner = THIS_MODULE,
+ },
},
- },
- {
- .desc = {
- .name = "VDDDR",
- .of_match = of_match_ptr("regulator-vdddr"),
- .regulators_node = of_match_ptr("regulators"),
- .id = BD957X_VDDDR,
- .ops = &bd957x_vout34_ops,
- .type = REGULATOR_VOLTAGE,
- .n_voltages = BD957X_VOUTS34_NUM_VOLT,
- .vsel_reg = BD957X_REG_VOUT3_TUNE,
- .vsel_mask = BD957X_MASK_VOUT3_TUNE,
- .enable_reg = BD957X_REG_POW_TRIGGER3,
- .enable_mask = BD957X_REGULATOR_EN_MASK,
- .enable_val = BD957X_REGULATOR_DIS_VAL,
- .enable_is_inverted = true,
- .owner = THIS_MODULE,
+ {
+ .desc = {
+ .name = "VDDDR",
+ .of_match = of_match_ptr("regulator-vdddr"),
+ .regulators_node = of_match_ptr("regulators"),
+ .id = BD957X_VDDDR,
+ .ops = &bd957x_vout34_ops,
+ .type = REGULATOR_VOLTAGE,
+ .n_voltages = BD957X_VOUTS34_NUM_VOLT,
+ .vsel_reg = BD957X_REG_VOUT3_TUNE,
+ .vsel_mask = BD957X_MASK_VOUT3_TUNE,
+ .enable_reg = BD957X_REG_POW_TRIGGER3,
+ .enable_mask = BD957X_REGULATOR_EN_MASK,
+ .enable_val = BD957X_REGULATOR_DIS_VAL,
+ .enable_is_inverted = true,
+ .owner = THIS_MODULE,
+ },
},
- },
- {
- .desc = {
- .name = "VD10",
- .of_match = of_match_ptr("regulator-vd10"),
- .regulators_node = of_match_ptr("regulators"),
- .id = BD957X_VD10,
- .ops = &bd957x_vout34_ops,
- .type = REGULATOR_VOLTAGE,
- .fixed_uV = BD957X_VOUTS4_BASE_VOLT,
- .n_voltages = BD957X_VOUTS34_NUM_VOLT,
- .vsel_reg = BD957X_REG_VOUT4_TUNE,
- .vsel_mask = BD957X_MASK_VOUT4_TUNE,
- .enable_reg = BD957X_REG_POW_TRIGGER4,
- .enable_mask = BD957X_REGULATOR_EN_MASK,
- .enable_val = BD957X_REGULATOR_DIS_VAL,
- .enable_is_inverted = true,
- .owner = THIS_MODULE,
+ {
+ .desc = {
+ .name = "VD10",
+ .of_match = of_match_ptr("regulator-vd10"),
+ .regulators_node = of_match_ptr("regulators"),
+ .id = BD957X_VD10,
+ .ops = &bd957x_vout34_ops,
+ .type = REGULATOR_VOLTAGE,
+ .fixed_uV = BD957X_VOUTS4_BASE_VOLT,
+ .n_voltages = BD957X_VOUTS34_NUM_VOLT,
+ .vsel_reg = BD957X_REG_VOUT4_TUNE,
+ .vsel_mask = BD957X_MASK_VOUT4_TUNE,
+ .enable_reg = BD957X_REG_POW_TRIGGER4,
+ .enable_mask = BD957X_REGULATOR_EN_MASK,
+ .enable_val = BD957X_REGULATOR_DIS_VAL,
+ .enable_is_inverted = true,
+ .owner = THIS_MODULE,
+ },
},
- },
- {
- .desc = {
- .name = "VOUTL1",
- .of_match = of_match_ptr("regulator-voutl1"),
- .regulators_node = of_match_ptr("regulators"),
- .id = BD957X_VOUTL1,
- .ops = &bd957x_ops,
- .type = REGULATOR_VOLTAGE,
- .volt_table = &voutl1_volt_table[0],
- .n_voltages = ARRAY_SIZE(voutl1_volt_table),
- .vsel_reg = BD957X_REG_VOUTL1_TUNE,
- .vsel_mask = BD957X_MASK_VOUTL1_TUNE,
- .enable_reg = BD957X_REG_POW_TRIGGERL1,
- .enable_mask = BD957X_REGULATOR_EN_MASK,
- .enable_val = BD957X_REGULATOR_DIS_VAL,
- .enable_is_inverted = true,
- .owner = THIS_MODULE,
+ {
+ .desc = {
+ .name = "VOUTL1",
+ .of_match = of_match_ptr("regulator-voutl1"),
+ .regulators_node = of_match_ptr("regulators"),
+ .id = BD957X_VOUTL1,
+ .ops = &bd957x_ops,
+ .type = REGULATOR_VOLTAGE,
+ .volt_table = &voutl1_volt_table[0],
+ .n_voltages = ARRAY_SIZE(voutl1_volt_table),
+ .vsel_reg = BD957X_REG_VOUTL1_TUNE,
+ .vsel_mask = BD957X_MASK_VOUTL1_TUNE,
+ .enable_reg = BD957X_REG_POW_TRIGGERL1,
+ .enable_mask = BD957X_REGULATOR_EN_MASK,
+ .enable_val = BD957X_REGULATOR_DIS_VAL,
+ .enable_is_inverted = true,
+ .owner = THIS_MODULE,
+ },
},
- },
- {
- .desc = {
- .name = "VOUTS1",
- .of_match = of_match_ptr("regulator-vouts1"),
- .regulators_node = of_match_ptr("regulators"),
- .id = BD957X_VOUTS1,
- .ops = &bd957X_vouts1_regulator_ops,
- .type = REGULATOR_VOLTAGE,
- .n_voltages = 1,
- .fixed_uV = BD957X_VOUTS1_VOLT,
- .enable_reg = BD957X_REG_POW_TRIGGERS1,
- .enable_mask = BD957X_REGULATOR_EN_MASK,
- .enable_val = BD957X_REGULATOR_DIS_VAL,
- .enable_is_inverted = true,
- .owner = THIS_MODULE,
+ {
+ .desc = {
+ .name = "VOUTS1",
+ .of_match = of_match_ptr("regulator-vouts1"),
+ .regulators_node = of_match_ptr("regulators"),
+ .id = BD957X_VOUTS1,
+ .ops = &bd957X_vouts1_regulator_ops,
+ .type = REGULATOR_VOLTAGE,
+ .n_voltages = 1,
+ .fixed_uV = BD957X_VOUTS1_VOLT,
+ .enable_reg = BD957X_REG_POW_TRIGGERS1,
+ .enable_mask = BD957X_REGULATOR_EN_MASK,
+ .enable_val = BD957X_REGULATOR_DIS_VAL,
+ .enable_is_inverted = true,
+ .owner = THIS_MODULE,
+ },
},
},
};
+#define BD9576_THERM_IRQ_MASK_TW BIT(5)
+#define BD9576_xVD_IRQ_MASK_VOUTL1 BIT(5)
+#define BD9576_UVD_IRQ_MASK_VOUTS1_OCW BIT(6)
+void delayed_enable_irq(struct delayed_work *w, int irq)
+{
+ /* keep the "main" IRQ masked for 1 sec */
+ disable_irq_nosync(irq);
+ schedule_delayed_work(w, msecs_to_jiffies(IRQS_SILENT_MS));
+}
+
+static void bd9576_vd_err(struct bd957x_data *d,
+ struct bd957x_regulator_data *r, int err)
+{
+ if (!r->rdev)
+ return;
+
+ spin_lock(&d->err_lock);
+ r->regulator_err |= err;
+ spin_unlock(&d->err_lock);
+ regulator_notifier_call_chain(r->rdev, err, NULL);
+}
+
+static irqreturn_t bd9576_irq_uvd(int irq, void *data)
+{
+ struct bd957x_data *d = (struct bd957x_data *)data;
+ int val, ret, i;
+
+ ret = regmap_read(d->regmap, BD957X_REG_INT_UVD_STAT, &val);
+ if (ret)
+ return IRQ_NONE;
+
+ /* bits [0] ... [3] represent xVD source VOUT1 ... VOUT4 */
+ for (i = 0; i < 4; i++)
+ if ((1 << i) & val)
+ bd9576_vd_err(d, &d->regulator_data[i],
+ REGULATOR_ERROR_UNDER_VOLTAGE);
+
+ if (val & BD9576_xVD_IRQ_MASK_VOUTL1)
+ bd9576_vd_err(d, &d->regulator_data[BD957X_VOUTL1],
+ REGULATOR_ERROR_UNDER_VOLTAGE);
+
+ if (val & BD9576_UVD_IRQ_MASK_VOUTS1_OCW)
+ bd9576_vd_err(d, &d->regulator_data[BD957X_VOUTS1],
+ REGULATOR_ERROR_OVER_CURRENT);
+
+ /* Clear the sub-IRQ status */
+ regmap_update_bits(d->regmap, BD957X_REG_INT_UVD_STAT,
+ UVD_IRQ_VALID_MASK, val);
+ delayed_enable_irq(&d->uvd_irq_suppress, irq);
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t bd9576_irq_ovd(int irq, void *data)
+{
+ struct bd957x_data *d = (struct bd957x_data *)data;
+ int val, ret, i;
+
+ ret = regmap_read(d->regmap, BD957X_REG_INT_OVD_STAT, &val);
+ if (ret)
+ return IRQ_NONE;
+
+ if (!(val & OVD_IRQ_VALID_MASK))
+ return IRQ_NONE;
+
+ /* bits [0] ... [3] represent xVD source VOUT1 ... VOUT4 */
+ for (i = 0; i < 4; i++)
+ if ((1 << i) & val)
+ bd9576_vd_err(d, &d->regulator_data[i],
+ REGULATOR_ERROR_REGULATION_OUT);
+
+ if (val & BD9576_xVD_IRQ_MASK_VOUTL1)
+ bd9576_vd_err(d, &d->regulator_data[BD957X_VOUTL1],
+ REGULATOR_ERROR_REGULATION_OUT);
+
+ /* Clear the sub-IRQ status */
+ regmap_update_bits(d->regmap, BD957X_REG_INT_OVD_STAT,
+ OVD_IRQ_VALID_MASK, val);
+ delayed_enable_irq(&d->ovd_irq_suppress, irq);
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t bd9576_irq_thermal(int irq, void *data)
+{
+ struct bd957x_data *d = (struct bd957x_data *)data;
+ int val, ret;
+
+ ret = regmap_read(d->regmap, BD957X_REG_INT_THERM_STAT, &val);
+ if (ret)
+ return IRQ_NONE;
+
+ if (!(val & UVD_IRQ_VALID_MASK))
+ return IRQ_NONE;
+
+ if (val & BD9576_THERM_IRQ_MASK_TW) {
+ int i;
+
+ d->regulator_global_err = REGULATOR_ERROR_OVER_TEMP;
+
+ for (i = 0; i < BD9576_NUM_REGULATORS; i++) {
+ struct regulator_dev *rdev;
+
+ rdev = d->regulator_data[i].rdev;
+ if (rdev)
+ regulator_notifier_call_chain(rdev,
+ REGULATOR_EVENT_OVER_TEMP, NULL);
+ }
+
+ /* Clear the sub-IRQ status */
+ regmap_update_bits(d->regmap, BD957X_REG_INT_THERM_STAT,
+ BD9576_THERM_IRQ_MASK_TW,
+ BD9576_THERM_IRQ_MASK_TW);
+ } else {
+ return IRQ_NONE;
+ }
+ delayed_enable_irq(&d->therm_irq_suppress, irq);
+
+ return IRQ_HANDLED;
+}
+
+/*
+ * BD9576 does not have a register adverticing the current status of errors.
+ * We just clean the errors when unmasking. If problem is still "on" the IRQ
+ * will re-trigger immediately => we send new notification + toggle the error
+ * flag back "on" in IRQ handler.
+ */
+static void therm_irq_work(struct work_struct *w)
+{
+ struct bd957x_data *d;
+
+ d = container_of(w, struct bd957x_data, therm_irq_suppress.work);
+ d->regulator_global_err = 0;
+ enable_irq(d->therm_irq);
+}
+
+static void ovd_irq_work(struct work_struct *w)
+{
+ struct bd957x_data *d;
+ int i, clear = REGULATOR_ERROR_REGULATION_OUT;
+
+ d = container_of(w, struct bd957x_data, ovd_irq_suppress.work);
+ spin_lock(&d->err_lock);
+ for (i = 0; i < BD9576_NUM_REGULATORS; i++)
+ d->regulator_data[i].regulator_err &= clear;
+ spin_unlock(&d->err_lock);
+ enable_irq(d->ovd_irq);
+}
+
+static void uvd_irq_work(struct work_struct *w)
+{
+ int clear, i;
+ struct bd957x_data *d;
+
+ d = container_of(w, struct bd957x_data, uvd_irq_suppress.work);
+ clear = (REGULATOR_ERROR_OVER_CURRENT | REGULATOR_ERROR_UNDER_VOLTAGE);
+
+ spin_lock(&d->err_lock);
+ for (i = 0; i < BD9576_NUM_REGULATORS; i++)
+ d->regulator_data[i].regulator_err &= clear;
+ spin_unlock(&d->err_lock);
+ enable_irq(d->uvd_irq);
+}
+
+static int bd9576_get_irqs(struct platform_device *pdev, struct regmap *regmap,
+ struct bd957x_data *data)
+{
+ int irq, ret;
+
+ spin_lock_init(&data->err_lock);
+ irq = platform_get_irq_byname(pdev, "bd9576-temp");
+ if (irq > 0) {
+ data->therm_irq = irq;
+ INIT_DELAYED_WORK(&data->therm_irq_suppress, therm_irq_work);
+ ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
+ &bd9576_irq_thermal,
+ IRQF_ONESHOT, "bd9576-temp",
+ data);
+ if (ret)
+ return ret;
+ /*
+ * Enable therm IRQ from sub IRQ mask
+ */
+ ret = regmap_update_bits(regmap, BD957X_REG_INT_THERM_MASK,
+ BD9576_THERM_IRQ_MASK_TW, 0);
+ if (ret)
+ return ret;
+ }
+ irq = platform_get_irq_byname(pdev, "bd9576-ovd");
+ if (irq > 0) {
+ data->ovd_irq = irq;
+ INIT_DELAYED_WORK(&data->ovd_irq_suppress, ovd_irq_work);
+ ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
+ &bd9576_irq_ovd,
+ IRQF_ONESHOT, "bd9576-ovd",
+ data);
+ if (ret)
+ return ret;
+
+ }
+ irq = platform_get_irq_byname(pdev, "bd9576-uvd");
+ if (irq > 0) {
+ data->uvd_irq = irq;
+ INIT_DELAYED_WORK(&data->uvd_irq_suppress, uvd_irq_work);
+ ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
+ &bd9576_irq_uvd,
+ IRQF_ONESHOT, "bd9576-uvd",
+ data);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
static int bd957x_probe(struct platform_device *pdev)
{
struct regmap *regmap;
struct regulator_config config = { 0 };
int i, err;
bool vout_mode, ddr_sel;
- const struct bd957x_regulator_data *reg_data = &bd9576_regulators[0];
- unsigned int num_reg_data = ARRAY_SIZE(bd9576_regulators);
+ struct bd957x_data *ic_data;
+ unsigned int num_reg_data;
enum rohm_chip_type chip = platform_get_device_id(pdev)->driver_data;
+ num_reg_data = ARRAY_SIZE(bd957x_regulators.regulator_data);
+
+ ic_data = &bd957x_regulators;
+
regmap = dev_get_regmap(pdev->dev.parent, NULL);
if (!regmap) {
dev_err(&pdev->dev, "No regmap\n");
return -EINVAL;
}
+ ic_data->regmap = regmap;
vout_mode = of_property_read_bool(pdev->dev.parent->of_node,
"rohm,vout1-en-low");
if (vout_mode) {
@@ -263,15 +523,24 @@ static int bd957x_probe(struct platform_device *pdev)
* bytes and use bd9576_regulators directly for non-constant configs
* like DDR voltage selection.
*/
+ platform_set_drvdata(pdev, ic_data);
ddr_sel = of_property_read_bool(pdev->dev.parent->of_node,
"rohm,ddr-sel-low");
if (ddr_sel)
- bd9576_regulators[2].desc.fixed_uV = 1350000;
+ ic_data->regulator_data[2].desc.fixed_uV = 1350000;
else
- bd9576_regulators[2].desc.fixed_uV = 1500000;
+ ic_data->regulator_data[2].desc.fixed_uV = 1500000;
switch (chip) {
case ROHM_CHIP_TYPE_BD9576:
+ /*
+ * And also here we pass pointer to static bd9576_regulators
+ * for IRQs to use. If we one day allocate driver data - then
+ * this needs to be changed too.
+ */
+ err = bd9576_get_irqs(pdev, regmap, ic_data);
+ if (err)
+ goto err_out;
dev_dbg(&pdev->dev, "Found BD9576MUF\n");
break;
case ROHM_CHIP_TYPE_BD9573:
@@ -280,28 +549,26 @@ static int bd957x_probe(struct platform_device *pdev)
default:
dev_err(&pdev->dev, "Unsupported chip type\n");
err = -EINVAL;
- goto err;
+ goto err_out;
}
config.dev = pdev->dev.parent;
config.regmap = regmap;
+ config.driver_data = ic_data;
for (i = 0; i < num_reg_data; i++) {
- const struct regulator_desc *desc;
- struct regulator_dev *rdev;
- const struct bd957x_regulator_data *r;
+ struct bd957x_regulator_data *r = &ic_data->regulator_data[i];
+ const struct regulator_desc *desc = &r->desc;
- r = ®_data[i];
- desc = &r->desc;
-
- rdev = devm_regulator_register(&pdev->dev, desc, &config);
- if (IS_ERR(rdev)) {
+ r->rdev = devm_regulator_register(&pdev->dev, desc,
+ &config);
+ if (IS_ERR(r->rdev)) {
dev_err(&pdev->dev,
"failed to register %s regulator\n",
desc->name);
- err = PTR_ERR(rdev);
- goto err;
+ err = PTR_ERR(r->rdev);
+ goto err_out;
}
/*
* Clear the VOUT1 GPIO setting - rest of the regulators do not
@@ -310,10 +577,32 @@ static int bd957x_probe(struct platform_device *pdev)
config.ena_gpiod = NULL;
}
-err:
+err_out:
+ if (err) {
+ if (ic_data->therm_irq)
+ cancel_delayed_work_sync(&ic_data->therm_irq_suppress);
+ if (ic_data->ovd_irq)
+ cancel_delayed_work_sync(&ic_data->ovd_irq_suppress);
+ if (ic_data->uvd_irq)
+ cancel_delayed_work_sync(&ic_data->uvd_irq_suppress);
+ }
return err;
}
+static int bd957x_remove(struct platform_device *pdev)
+{
+ struct bd957x_data *ic_data = platform_get_drvdata(pdev);
+
+ if (ic_data->therm_irq)
+ cancel_delayed_work_sync(&ic_data->therm_irq_suppress);
+ if (ic_data->ovd_irq)
+ cancel_delayed_work_sync(&ic_data->ovd_irq_suppress);
+ if (ic_data->uvd_irq)
+ cancel_delayed_work_sync(&ic_data->uvd_irq_suppress);
+
+ return 0;
+}
+
static const struct platform_device_id bd957x_pmic_id[] = {
{ "bd9573-pmic", ROHM_CHIP_TYPE_BD9573 },
{ "bd9576-pmic", ROHM_CHIP_TYPE_BD9576 },
@@ -326,6 +615,7 @@ static struct platform_driver bd957x_regulator = {
.name = "bd957x-pmic",
},
.probe = bd957x_probe,
+ .remove = bd957x_remove,
.id_table = bd957x_pmic_id,
};
--
2.25.4
--
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 =]
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v7 5/6] MAINTAINERS: Add ROHM BD9576MUF and BD9573MUF drivers
2021-01-22 14:35 ` [PATCH v7 5/6] MAINTAINERS: Add ROHM BD9576MUF and BD9573MUF drivers Matti Vaittinen
@ 2021-01-23 18:38 ` Guenter Roeck
0 siblings, 0 replies; 15+ messages in thread
From: Guenter Roeck @ 2021-01-23 18:38 UTC (permalink / raw)
To: Matti Vaittinen
Cc: mazziesaccount, Lee Jones, Rob Herring, Liam Girdwood,
Mark Brown, Wim Van Sebroeck, Geert Uytterhoeven, devicetree,
linux-kernel, linux-power, linux-watchdog, Yoshihiro Shimoda
On Fri, Jan 22, 2021 at 04:35:10PM +0200, Matti Vaittinen wrote:
> Add maintainer entries for ROHM BD9576MUF and ROHM BD9573MUF drivers.
> MFD, regulator and watchdog drivers were introduced for these PMICs.
>
> Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
Acked-by: Guenter Roeck <linux@roeck-us.net>
> ---
> Changes since v6:
> - no changes
> MAINTAINERS | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index cc1e6a5ee6e6..b59b7877258c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -15336,16 +15336,20 @@ F: drivers/gpio/gpio-bd71828.c
> F: drivers/mfd/rohm-bd70528.c
> F: drivers/mfd/rohm-bd71828.c
> F: drivers/mfd/rohm-bd718x7.c
> +F: drivers/mfd/rohm-bd9576.c
> F: drivers/power/supply/bd70528-charger.c
> F: drivers/regulator/bd70528-regulator.c
> F: drivers/regulator/bd71828-regulator.c
> F: drivers/regulator/bd718x7-regulator.c
> +F: drivers/regulator/bd9576-regulator.c
> F: drivers/regulator/rohm-regulator.c
> F: drivers/rtc/rtc-bd70528.c
> F: drivers/watchdog/bd70528_wdt.c
> +F: drivers/watchdog/bd9576_wdt.c
> F: include/linux/mfd/rohm-bd70528.h
> F: include/linux/mfd/rohm-bd71828.h
> F: include/linux/mfd/rohm-bd718x7.h
> +F: include/linux/mfd/rohm-bd957x.h
> F: include/linux/mfd/rohm-generic.h
> F: include/linux/mfd/rohm-shared.h
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v7 6/6] regulator: bd9576: Support error reporting
2021-01-22 14:36 ` [PATCH v7 6/6] regulator: bd9576: Support error reporting Matti Vaittinen
@ 2021-01-24 18:56 ` kernel test robot
0 siblings, 0 replies; 15+ messages in thread
From: kernel test robot @ 2021-01-24 18:56 UTC (permalink / raw)
To: Matti Vaittinen, Matti Vaittinen
Cc: kbuild-all, Lee Jones, Matti Vaittinen, Matti Vaittinen,
Liam Girdwood, Mark Brown, Geert Uytterhoeven, linux-kernel,
linux-power, Yoshihiro Shimoda
[-- Attachment #1: Type: text/plain, Size: 2007 bytes --]
Hi Matti,
I love your patch! Perhaps something to improve:
[auto build test WARNING on 7c53f6b671f4aba70ff15e1b05148b10d58c2837]
url: https://github.com/0day-ci/linux/commits/Matti-Vaittinen/Support-ROHM-BD9576MUF-and-BD9573MUF-PMICs/20210124-214317
base: 7c53f6b671f4aba70ff15e1b05148b10d58c2837
config: nios2-allyesconfig (attached as .config)
compiler: nios2-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/a8fb3903668748b6a0b662b462c84825df47cf45
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Matti-Vaittinen/Support-ROHM-BD9576MUF-and-BD9573MUF-PMICs/20210124-214317
git checkout a8fb3903668748b6a0b662b462c84825df47cf45
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=nios2
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/regulator/bd9576-regulator.c:248:6: warning: no previous prototype for 'delayed_enable_irq' [-Wmissing-prototypes]
248 | void delayed_enable_irq(struct delayed_work *w, int irq)
| ^~~~~~~~~~~~~~~~~~
vim +/delayed_enable_irq +248 drivers/regulator/bd9576-regulator.c
244
245 #define BD9576_THERM_IRQ_MASK_TW BIT(5)
246 #define BD9576_xVD_IRQ_MASK_VOUTL1 BIT(5)
247 #define BD9576_UVD_IRQ_MASK_VOUTS1_OCW BIT(6)
> 248 void delayed_enable_irq(struct delayed_work *w, int irq)
249 {
250 /* keep the "main" IRQ masked for 1 sec */
251 disable_irq_nosync(irq);
252 schedule_delayed_work(w, msecs_to_jiffies(IRQS_SILENT_MS));
253 }
254
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 58811 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v7 0/6] Support ROHM BD9576MUF and BD9573MUF PMICs
2021-01-22 14:30 [PATCH v7 0/6] Support ROHM BD9576MUF and BD9573MUF PMICs Matti Vaittinen
` (5 preceding siblings ...)
2021-01-22 14:36 ` [PATCH v7 6/6] regulator: bd9576: Support error reporting Matti Vaittinen
@ 2021-02-09 14:19 ` Vaittinen, Matti
6 siblings, 0 replies; 15+ messages in thread
From: Vaittinen, Matti @ 2021-02-09 14:19 UTC (permalink / raw)
To: Mazziesaccount, Vaittinen, Matti
Cc: linux-power, linux, wim, broonie, devicetree, lee.jones,
linux-kernel, geert+renesas, linux-watchdog, lgirdwood, robh+dt,
yoshihiro.shimoda.uh
Hello Lee, Mark All,
On Fri, 2021-01-22 at 16:30 +0200, Matti Vaittinen wrote:
> Initial support for ROHM BD9576MUF and BD9573MUF PMICs.
>
> These PMICs are primarily intended to be used to power the R-Car
> family
> processors. BD9576MUF includes some additional safety features the
> BD9573MUF does not have. This initial version of drivers provides
> temperature, over voltage and under voltage warnings is IRQ
> information
> is passed via DT.
>
> This patch series includes MFD and watchdog drivers. Regulator part
> was
> already applied but this series brings the over-/undervoltage and
> temperature error notifications which consumer drivers can utilize.
I had some discussion with Mark and Angelo about creating a helper for
handling this kind of regulator notification IRQs.
(For anyone interested: the discussion can be seen here:
https://lore.kernel.org/lkml/6046836e22b8252983f08d5621c35ececb97820d.camel@fi.rohmeurope.com/
)
I've now drafted RFCv1 for that support (not sent it yet). The RFC
converts the BD9576 regulator driver to use the new helper and adds
some new definitions to MFD headers.
What would be the most convenient way of handling this? Should I merge
this series in the RFC and make it just one big series? Or should I
keep these as two separated series? If I keep these as separate series,
should I then omit all the MFD patches from RFC series - and add
potential MFD changes (like OVD/UVD configuration registers) in this
series (which makes bd9576 regulators not compiling) - or should the
MFD parts be included in both series - in which case we need to somehow
stay on track what parts of MFD is reviewed.
Simplest for me would be if we could get the oldest patches 1,2,4 and 5
from this series in-tree (only MFD is not acked) - but I guess it won't
happen at this point of the development cycle - and bring in the IRQ
(patch 3) and regulator notifications (patch 6) using the RFC series.
How do you see it? Should I meld this in the RFC or keep two separate
series - in which case, how should I handle the MFD changes brought by
the RFC series?
Best Regards
Matti Vaittinen
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v7 2/6] mfd: Support ROHM BD9576MUF and BD9573MUF
2021-01-22 14:32 ` [PATCH v7 2/6] mfd: Support ROHM BD9576MUF and BD9573MUF Matti Vaittinen
@ 2021-02-09 14:55 ` Lee Jones
2021-02-10 6:15 ` Matti Vaittinen
0 siblings, 1 reply; 15+ messages in thread
From: Lee Jones @ 2021-02-09 14:55 UTC (permalink / raw)
To: Matti Vaittinen
Cc: mazziesaccount, Rob Herring, Liam Girdwood, Mark Brown,
Wim Van Sebroeck, Guenter Roeck, Geert Uytterhoeven, devicetree,
linux-kernel, linux-power, linux-watchdog, Yoshihiro Shimoda
On Fri, 22 Jan 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 since v6:
> - no changes
> drivers/mfd/Kconfig | 11 ++++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/rohm-bd9576.c | 108 +++++++++++++++++++++++++++++++
> include/linux/mfd/rohm-bd957x.h | 59 +++++++++++++++++
> include/linux/mfd/rohm-generic.h | 2 +
> 5 files changed, 181 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..f4dd9e438427
> --- /dev/null
> +++ b/drivers/mfd/rohm-bd9576.c
> @@ -0,0 +1,108 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +//
> +// Copyright (C) 2020 ROHM Semiconductors
> +//
> +// ROHM BD9576MUF and BD9573MUF PMIC driver
No C++ comments (save the SPDX line) please.
> +#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-bd957x.h>
> +#include <linux/mfd/rohm-generic.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/regmap.h>
> +#include <linux/types.h>
> +
> +static struct mfd_cell bd9573_mfd_cells[] = {
> + { .name = "bd9573-pmic", },
> + { .name = "bd9576-wdt", },
> +};
> +
> +static struct mfd_cell bd9576_mfd_cells[] = {
> + { .name = "bd9576-pmic", },
> + { .name = "bd9576-wdt", },
> +};
What is a PMIC in this context?
To me a PMIC is a bunch of devices. What is this probing?
Maybe this is *-regulator?
> +static const struct regmap_range volatile_ranges[] = {
> + regmap_reg_range(BD957X_REG_SMRB_ASSERT, BD957X_REG_SMRB_ASSERT),
> + regmap_reg_range(BD957X_REG_PMIC_INTERNAL_STAT,
> + BD957X_REG_PMIC_INTERNAL_STAT),
> + regmap_reg_range(BD957X_REG_INT_THERM_STAT, BD957X_REG_INT_THERM_STAT),
> + regmap_reg_range(BD957X_REG_INT_OVP_STAT, BD957X_REG_INT_SYS_STAT),
> + regmap_reg_range(BD957X_REG_INT_MAIN_STAT, BD957X_REG_INT_MAIN_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 bd957x_regmap = {
> + .reg_bits = 8,
> + .val_bits = 8,
> + .volatile_table = &volatile_regs,
> + .max_register = BD957X_MAX_REGISTER,
> + .cache_type = REGCACHE_RBTREE,
> +};
> +
> +static int bd957x_i2c_probe(struct i2c_client *i2c,
> + const struct i2c_device_id *id)
> +{
> + int ret;
> + struct regmap *regmap;
> + struct mfd_cell *mfd;
> + int cells;
> + unsigned long chip_type;
> +
> + chip_type = (unsigned long)of_device_get_match_data(&i2c->dev);
> +
> + switch (chip_type) {
> + case ROHM_CHIP_TYPE_BD9576:
> + mfd = bd9576_mfd_cells;
> + cells = ARRAY_SIZE(bd9576_mfd_cells);
> + break;
> + case ROHM_CHIP_TYPE_BD9573:
> + mfd = bd9573_mfd_cells;
> + cells = ARRAY_SIZE(bd9573_mfd_cells);
> + break;
> + default:
> + dev_err(&i2c->dev, "Unknown device type");
> + return -EINVAL;
> + }
> +
> + regmap = devm_regmap_init_i2c(i2c, &bd957x_regmap);
> + if (IS_ERR(regmap)) {
> + dev_err(&i2c->dev, "Failed to initialize Regmap\n");
> + return PTR_ERR(regmap);
> + }
> +
> + ret = devm_mfd_add_devices(&i2c->dev, PLATFORM_DEVID_AUTO, mfd, cells,
This nomenclature is confusing.
cells and num_cells would clear it up.
> + NULL, 0, NULL);
> + if (ret)
> + dev_err(&i2c->dev, "Failed to create subdevices\n");
> +
> + return ret;
> +}
> +
> +static const struct of_device_id bd957x_of_match[] = {
> + { .compatible = "rohm,bd9576", .data = (void *)ROHM_CHIP_TYPE_BD9576, },
> + { .compatible = "rohm,bd9573", .data = (void *)ROHM_CHIP_TYPE_BD9573, },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, bd957x_of_match);
> +
> +static struct i2c_driver bd957x_drv = {
> + .driver = {
> + .name = "rohm-bd957x",
> + .of_match_table = bd957x_of_match,
> + },
> + .probe = &bd957x_i2c_probe,
> +};
> +module_i2c_driver(bd957x_drv);
> +
> +MODULE_AUTHOR("Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>");
> +MODULE_DESCRIPTION("ROHM BD9576MUF and BD9573MUF Power Management IC driver");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/mfd/rohm-bd957x.h b/include/linux/mfd/rohm-bd957x.h
> new file mode 100644
> index 000000000000..3e7ca6fe5d4f
> --- /dev/null
> +++ b/include/linux/mfd/rohm-bd957x.h
> @@ -0,0 +1,59 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/* Copyright (C) 2020 ROHM Semiconductors */
> +
> +#ifndef __LINUX_MFD_BD957X_H__
> +#define __LINUX_MFD_BD957X_H__
> +
> +enum {
> + BD957X_VD50,
> + BD957X_VD18,
> + BD957X_VDDDR,
> + BD957X_VD10,
> + BD957X_VOUTL1,
> + BD957X_VOUTS1,
> +};
> +
> +#define BD957X_REG_SMRB_ASSERT 0x15
> +#define BD957X_REG_PMIC_INTERNAL_STAT 0x20
> +#define BD957X_REG_INT_THERM_STAT 0x23
> +#define BD957X_REG_INT_THERM_MASK 0x24
> +#define BD957X_REG_INT_OVP_STAT 0x25
> +#define BD957X_REG_INT_SCP_STAT 0x26
> +#define BD957X_REG_INT_OCP_STAT 0x27
> +#define BD957X_REG_INT_OVD_STAT 0x28
> +#define BD957X_REG_INT_UVD_STAT 0x29
> +#define BD957X_REG_INT_UVP_STAT 0x2a
> +#define BD957X_REG_INT_SYS_STAT 0x2b
> +#define BD957X_REG_INT_SYS_MASK 0x2c
> +#define BD957X_REG_INT_MAIN_STAT 0x30
> +#define BD957X_REG_INT_MAIN_MASK 0x31
> +
> +#define BD957X_REG_WDT_CONF 0x16
> +
> +#define BD957X_REG_POW_TRIGGER1 0x41
> +#define BD957X_REG_POW_TRIGGER2 0x42
> +#define BD957X_REG_POW_TRIGGER3 0x43
> +#define BD957X_REG_POW_TRIGGER4 0x44
> +#define BD957X_REG_POW_TRIGGERL1 0x45
> +#define BD957X_REG_POW_TRIGGERS1 0x46
> +
> +#define BD957X_REGULATOR_EN_MASK 0xff
> +#define BD957X_REGULATOR_DIS_VAL 0xff
> +
> +#define BD957X_VSEL_REG_MASK 0xff
> +
> +#define BD957X_MASK_VOUT1_TUNE 0x87
> +#define BD957X_MASK_VOUT2_TUNE 0x87
> +#define BD957X_MASK_VOUT3_TUNE 0x1f
> +#define BD957X_MASK_VOUT4_TUNE 0x1f
> +#define BD957X_MASK_VOUTL1_TUNE 0x87
> +
> +#define BD957X_REG_VOUT1_TUNE 0x50
> +#define BD957X_REG_VOUT2_TUNE 0x53
> +#define BD957X_REG_VOUT3_TUNE 0x56
> +#define BD957X_REG_VOUT4_TUNE 0x59
> +#define BD957X_REG_VOUTL1_TUNE 0x5c
> +
> +#define BD957X_MAX_REGISTER 0x61
Nit: Can you tab these out for improved readability please?
> +#endif
> diff --git a/include/linux/mfd/rohm-generic.h b/include/linux/mfd/rohm-generic.h
> index 4283b5b33e04..58b4f1a0f4af 100644
> --- a/include/linux/mfd/rohm-generic.h
> +++ b/include/linux/mfd/rohm-generic.h
> @@ -12,6 +12,8 @@ enum rohm_chip_type {
> ROHM_CHIP_TYPE_BD71847,
> ROHM_CHIP_TYPE_BD70528,
> ROHM_CHIP_TYPE_BD71828,
> + ROHM_CHIP_TYPE_BD9576,
> + ROHM_CHIP_TYPE_BD9573,
> ROHM_CHIP_TYPE_AMOUNT
> };
>
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v7 3/6] mfd: bd9576: Add IRQ support
2021-01-22 14:33 ` [PATCH v7 3/6] mfd: bd9576: Add IRQ support Matti Vaittinen
@ 2021-02-09 15:25 ` Lee Jones
2021-02-10 7:01 ` Matti Vaittinen
0 siblings, 1 reply; 15+ messages in thread
From: Lee Jones @ 2021-02-09 15:25 UTC (permalink / raw)
To: Matti Vaittinen
Cc: mazziesaccount, Liam Girdwood, Mark Brown, Geert Uytterhoeven,
linux-kernel, linux-power, Yoshihiro Shimoda
On Fri, 22 Jan 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 v6:
> - new patch
>
> drivers/mfd/rohm-bd9576.c | 80 +++++++++++++++++-
> include/linux/mfd/rohm-bd957x.h | 140 +++++++++++++++++++++++---------
> 2 files changed, 180 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/mfd/rohm-bd9576.c b/drivers/mfd/rohm-bd9576.c
> index f4dd9e438427..99fa0f333277 100644
> --- a/drivers/mfd/rohm-bd9576.c
> +++ b/drivers/mfd/rohm-bd9576.c
> @@ -16,12 +16,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-pmic", },
> { .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.
> + */
> { .name = "bd9576-pmic", },
> { .name = "bd9576-wdt", },
> };
> @@ -48,6 +63,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)
> {
> @@ -56,6 +94,7 @@ static int bd957x_i2c_probe(struct i2c_client *i2c,
> struct mfd_cell *mfd;
> int cells;
> unsigned long chip_type;
> + struct irq_domain *domain;
>
> chip_type = (unsigned long)of_device_get_match_data(&i2c->dev);
>
> @@ -67,6 +106,11 @@ static int bd957x_i2c_probe(struct i2c_client *i2c,
> case ROHM_CHIP_TYPE_BD9573:
> mfd = bd9573_mfd_cells;
> cells = ARRAY_SIZE(bd9573_mfd_cells);
> + /* BD9573 only supports fatal IRQs which we do not handle */
Why not?
> + if (i2c->irq) {
> + dev_err(&i2c->dev, "no supported irqs on BD9573\n");
"IRQs"
> + return -EINVAL;
> + }
> break;
> default:
> dev_err(&i2c->dev, "Unknown device type");
> @@ -78,9 +122,43 @@ static int bd957x_i2c_probe(struct i2c_client *i2c,
> dev_err(&i2c->dev, "Failed to initialize Regmap\n");
> return PTR_ERR(regmap);
> }
> + /*
> + * BD9576 behaves badly. It kepts IRQ asserted for the whole
This is solution is less than pretty.
> + * duration of detected HW condition (like over temp). This does
"over-temperature"
> + * not play nicely under any condition but we can work around it
> + * except when we have shared IRQs. So we don't require IRQ to be
> + * populated to help those poor sods who did connect IRQ to shared pin.
No swearing in comments please.
How do you know if an IRQ is shared?
> + * 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.
> + */
This situation doesn't sound totally crazy. Is there no way to handle
'persistent IRQ' conditions in the kernel?
> + if (i2c->irq) {
> + 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_info(&i2c->dev, "Using IRQs for BD9576MUF\n");
> + } else {
> + ret = regmap_update_bits(regmap, BD957X_REG_INT_MAIN_MASK,
> + BD957X_MASK_INT_ALL,
> + BD957X_MASK_INT_ALL);
What's the default state of the interrupts? Unmasked?
> + if (ret)
> + return ret;
> + domain = NULL;
> + }
>
> ret = devm_mfd_add_devices(&i2c->dev, PLATFORM_DEVID_AUTO, mfd, cells,
> - NULL, 0, NULL);
> + 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 3e7ca6fe5d4f..4fa632d8467a 100644
> --- a/include/linux/mfd/rohm-bd957x.h
> +++ b/include/linux/mfd/rohm-bd957x.h
> @@ -13,47 +13,109 @@ enum {
> BD957X_VOUTS1,
> };
>
> +/* The BD9576 has own IRQ 'blocks' for:
Comments start on line 2.
> + * I2C/THERMAL,
Does this precede each line?
> + * Over voltage protection
> + * Short-circuit protection
> + * Over current protection
> + * Over voltage detection
> + * Under voltage detection
> + * Under voltage protection
> + * and '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 cleared
> + * in HW. This is _really_ annoying for IRQs like the one informing high
> + * temperature. Best mitigation for high temperature for sure is to
> + * keep the processor in IRQ loop, right? (NO!)
No sarcasm in comments please it can be easily lost in translation.
Please stay helpful and keep to the facts.
> + * So - 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
> -#define BD957X_REG_INT_THERM_MASK 0x24
> -#define BD957X_REG_INT_OVP_STAT 0x25
> -#define BD957X_REG_INT_SCP_STAT 0x26
> -#define BD957X_REG_INT_OCP_STAT 0x27
> -#define BD957X_REG_INT_OVD_STAT 0x28
> -#define BD957X_REG_INT_UVD_STAT 0x29
> -#define BD957X_REG_INT_UVP_STAT 0x2a
> -#define BD957X_REG_INT_SYS_STAT 0x2b
> -#define BD957X_REG_INT_SYS_MASK 0x2c
> -#define BD957X_REG_INT_MAIN_STAT 0x30
> -#define BD957X_REG_INT_MAIN_MASK 0x31
> -
> -#define BD957X_REG_WDT_CONF 0x16
> -
> -#define BD957X_REG_POW_TRIGGER1 0x41
> -#define BD957X_REG_POW_TRIGGER2 0x42
> -#define BD957X_REG_POW_TRIGGER3 0x43
> -#define BD957X_REG_POW_TRIGGER4 0x44
> -#define BD957X_REG_POW_TRIGGERL1 0x45
> -#define BD957X_REG_POW_TRIGGERS1 0x46
> -
> -#define BD957X_REGULATOR_EN_MASK 0xff
> -#define BD957X_REGULATOR_DIS_VAL 0xff
> -
> -#define BD957X_VSEL_REG_MASK 0xff
> -
> -#define BD957X_MASK_VOUT1_TUNE 0x87
> -#define BD957X_MASK_VOUT2_TUNE 0x87
> -#define BD957X_MASK_VOUT3_TUNE 0x1f
> -#define BD957X_MASK_VOUT4_TUNE 0x1f
> -#define BD957X_MASK_VOUTL1_TUNE 0x87
> -
> -#define BD957X_REG_VOUT1_TUNE 0x50
> -#define BD957X_REG_VOUT2_TUNE 0x53
> -#define BD957X_REG_VOUT3_TUNE 0x56
> -#define BD957X_REG_VOUT4_TUNE 0x59
> -#define BD957X_REG_VOUTL1_TUNE 0x5c
> -
> -#define BD957X_MAX_REGISTER 0x61
> +#define BD957X_REG_INT_THERM_MASK 0x24
> +#define BD957X_REG_INT_OVP_STAT 0x25
> +#define BD957X_REG_INT_SCP_STAT 0x26
> +#define BD957X_REG_INT_OCP_STAT 0x27
> +#define BD957X_REG_INT_OVD_STAT 0x28
> +#define BD957X_REG_INT_UVD_STAT 0x29
> +#define BD957X_REG_INT_UVP_STAT 0x2a
> +#define BD957X_REG_INT_SYS_STAT 0x2b
> +#define BD957X_REG_INT_SYS_MASK 0x2c
> +#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
> +#define BD957X_REG_POW_TRIGGER2 0x42
> +#define BD957X_REG_POW_TRIGGER3 0x43
> +#define BD957X_REG_POW_TRIGGER4 0x44
> +#define BD957X_REG_POW_TRIGGERL1 0x45
> +#define BD957X_REG_POW_TRIGGERS1 0x46
> +
> +#define BD957X_REGULATOR_EN_MASK 0xff
> +#define BD957X_REGULATOR_DIS_VAL 0xff
> +
> +#define BD957X_VSEL_REG_MASK 0xff
> +
> +#define BD957X_MASK_VOUT1_TUNE 0x87
> +#define BD957X_MASK_VOUT2_TUNE 0x87
> +#define BD957X_MASK_VOUT3_TUNE 0x1f
> +#define BD957X_MASK_VOUT4_TUNE 0x1f
> +#define BD957X_MASK_VOUTL1_TUNE 0x87
> +
> +#define BD957X_REG_VOUT1_TUNE 0x50
> +#define BD957X_REG_VOUT2_TUNE 0x53
> +#define BD957X_REG_VOUT3_TUNE 0x56
> +#define BD957X_REG_VOUT4_TUNE 0x59
> +#define BD957X_REG_VOUTL1_TUNE 0x5c
> +
> +#define BD957X_MAX_REGISTER 0x61
Line all these up please.
> #endif
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v7 2/6] mfd: Support ROHM BD9576MUF and BD9573MUF
2021-02-09 14:55 ` Lee Jones
@ 2021-02-10 6:15 ` Matti Vaittinen
0 siblings, 0 replies; 15+ messages in thread
From: Matti Vaittinen @ 2021-02-10 6:15 UTC (permalink / raw)
To: Lee Jones
Cc: Rob Herring, Liam Girdwood, Mark Brown, Wim Van Sebroeck,
Guenter Roeck, Geert Uytterhoeven, devicetree, linux-kernel,
linux-power, linux-watchdog, Yoshihiro Shimoda
Hello Lee,
On Tue, 2021-02-09 at 14:55 +0000, Lee Jones wrote:
> On Fri, 22 Jan 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>
> > ---
> > +
> > +static struct mfd_cell bd9573_mfd_cells[] = {
> > + { .name = "bd9573-pmic", },
> > + { .name = "bd9576-wdt", },
> > +};
> > +
> > +static struct mfd_cell bd9576_mfd_cells[] = {
> > + { .name = "bd9576-pmic", },
> > + { .name = "bd9576-wdt", },
> > +};
>
> What is a PMIC in this context?
>
> To me a PMIC is a bunch of devices. What is this probing?
I agree. PMIC is the IC as a whole. This name was not the best one.
>
> Maybe this is *-regulator?
That would be more descriptive and I can change this. However, it means
I need to change the already applied regulator part too. Furthermore,
all other ROHM PMIC drivers I've written use the <part-name>-pmic for
regulators and so does a few other drivers at least for ICs from Maxim,
Samsung and TI. That's why I don't think the <partname>-pmic is that
confusing. If it was my decision, I would stick with the pmic for the
sake of the consistency.
+
> > + ret = devm_mfd_add_devices(&i2c->dev, PLATFORM_DEVID_AUTO, mfd,
> > cells,
>
> This nomenclature is confusing.
>
> cells and num_cells would clear it up.
I can change it.
+
> > +#define BD957X_MAX_REGISTER 0x61
>
> Nit: Can you tab these out for improved readability please?
Sure, no problem.
Thanks for the review!
Best Regards
--Matti
--
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 for the translation Simon)
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v7 3/6] mfd: bd9576: Add IRQ support
2021-02-09 15:25 ` Lee Jones
@ 2021-02-10 7:01 ` Matti Vaittinen
2021-02-11 7:15 ` Vaittinen, Matti
0 siblings, 1 reply; 15+ messages in thread
From: Matti Vaittinen @ 2021-02-10 7:01 UTC (permalink / raw)
To: Lee Jones
Cc: Liam Girdwood, Mark Brown, Geert Uytterhoeven, linux-kernel,
linux-power, Yoshihiro Shimoda
Hello Lee,
I appreciate your thorough reviews :) Thanks.
On Tue, 2021-02-09 at 15:25 +0000, Lee Jones wrote:
> On Fri, 22 Jan 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>
> > case ROHM_CHIP_TYPE_BD9573:
> > mfd = bd9573_mfd_cells;
> > cells = ARRAY_SIZE(bd9573_mfd_cells);
> > + /* BD9573 only supports fatal IRQs which we do not
> > handle */
>
> Why not?
Because 'fatal' in the context of this comment means that when this
condition occurs the PMIC will do emergency shut down for power outputs
- which means the processor will not be able to handle the IRQ as it
loses the power. Maybe I'd better clarify the meaning of 'fatal' here.
+ /*
> > + * BD9576 behaves badly. It kepts IRQ asserted for the whole
>
> This is solution is less than pretty.
Um, sorry, What are you referring to?
> > + * duration of detected HW condition (like over temp). This
> > does
>
> "over-temperature"
Right. Thanks :)
> > + * not play nicely under any condition but we can work around
> > it
> > + * except when we have shared IRQs. So we don't require IRQ to
> > be
> > + * populated to help those poor sods who did connect IRQ to
> > shared pin.
>
> No swearing in comments please.
Ok. This is actually a good reminder for me that I can't know how
something sounds like for a reader. (That phrase sounds quite innocent
to me but I've no idea how 'severe' swearing that is for the rest of
the world). I'll clean this up.
> How do you know if an IRQ is shared?
I don't. This is something that board designer does know. And my
thinking here was to allow board designer to omit the IRQ information
from DT if he prefers to not use these IRQs. I just tried to explain
that the driver does not _require_ IRQ information 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.
> > + */
>
> This situation doesn't sound totally crazy. Is there no way to
> handle
> 'persistent IRQ' conditions in the kernel?
Actually there is. Even for shared IRQs in this case. I made a mistake
at the beginning when I noticed that not all of these IRQs have mask
bits in the sub-IRQ registers. So I thought that for these IRQs the
device can't be told to revert IRQ back to normal. That would have
meaned that only way to prevent IRQ storm was to disable IRQs from the
processor end. But I was mistaken. All of the IRQs can be masked from
the 'main IRQ' level register. So we can mask the whole set of IRQs
form BD9576 when IRQ triggers - and then we can get the BD9576 to
restore the IRQ line.
So yes - we can make this to somehow work. And more importantly, we
don't completely spoil the shared IRQs. Still, the IRQ handling for
BD9576 is ... how to put it ... hacky. And I think few of the setups
might not actually have use for the notifications - so making IRQs
optional just sounded like the best course of action (to me).
+ } else {
> > + ret = regmap_update_bits(regmap,
> > BD957X_REG_INT_MAIN_MASK,
> > + BD957X_MASK_INT_ALL,
> > + BD957X_MASK_INT_ALL);
>
> What's the default state of the interrupts? Unmasked?
I've learned that I'd rather not assume the default state with ROHM
ICs. I've seen all kinds of defaults changing between IC revisions. And
occasionally I've seen same IC versions having different set of
defaults depending on the OTP version. I guess this comes from
traditional operation model where ICs have been tailored to meet needs
of the different customers.
> > diff --git a/include/linux/mfd/rohm-bd957x.h
> > b/include/linux/mfd/rohm-bd957x.h
> > index 3e7ca6fe5d4f..4fa632d8467a 100644
> > --- a/include/linux/mfd/rohm-bd957x.h
> > +++ b/include/linux/mfd/rohm-bd957x.h
> > @@ -13,47 +13,109 @@ enum {
> > BD957X_VOUTS1,
> > };
> >
> > +/* The BD9576 has own IRQ 'blocks' for:
>
> Comments start on line 2.
Do you mean I should move this comment block top of the file? The idea
of this comment is to clarify the IRQs in the hardware. Hence I placed
it in the section where IRQ definitions dwell.
> > + * I2C/THERMAL,
>
> Does this precede each line?
You mean the I2C/THERMAL? No. The first IRQ block is combined set of
I2C/thermal IRQs. Caps are misleading, right?
> + * temperature. Best mitigation for high temperature for sure
> > is to
> > + * keep the processor in IRQ loop, right? (NO!)
>
> No sarcasm in comments please it can be easily lost in translation.
>
> Please stay helpful and keep to the facts.
Right. I'll clean this up.
>
> > #define BD957X_REG_SMRB_ASSERT 0x15
> > #define BD957X_REG_PMIC_INTERNAL_STAT 0x20
> > #define BD957X_REG_INT_THERM_STAT 0x23
> > -#define BD957X_REG_INT_THERM_MASK 0x24
> > -#define BD957X_REG_INT_OVP_STAT 0x25
> > -#define BD957X_REG_INT_SCP_STAT 0x26
> > -#define BD957X_REG_INT_OCP_STAT 0x27
//snip
> > +
> > +#define BD957X_REGULATOR_EN_MASK 0xff
> > +#define BD957X_REGULATOR_DIS_VAL 0xff
> > +
> > +#define BD957X_VSEL_REG_MASK 0xff
> > +
> > +#define BD957X_MASK_VOUT1_TUNE 0x87
> > +#define BD957X_MASK_VOUT2_TUNE 0x87
> > +#define BD957X_MASK_VOUT3_TUNE 0x1f
> > +#define BD957X_MASK_VOUT4_TUNE 0x1f
> > +#define BD957X_MASK_VOUTL1_TUNE 0x87
> > +
> > +#define BD957X_REG_VOUT1_TUNE 0x50
> > +#define BD957X_REG_VOUT2_TUNE 0x53
> > +#define BD957X_REG_VOUT3_TUNE 0x56
> > +#define BD957X_REG_VOUT4_TUNE 0x59
> > +#define BD957X_REG_VOUTL1_TUNE 0x5c
> > +
> > +#define BD957X_MAX_REGISTER 0x61
>
> Line all these up please.
By 'line up' you mean I should remove the empty lines, right?
Best Regards
Matti Vaittinen
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v7 3/6] mfd: bd9576: Add IRQ support
2021-02-10 7:01 ` Matti Vaittinen
@ 2021-02-11 7:15 ` Vaittinen, Matti
0 siblings, 0 replies; 15+ messages in thread
From: Vaittinen, Matti @ 2021-02-11 7:15 UTC (permalink / raw)
To: lee.jones
Cc: lgirdwood, linux-power, broonie, yoshihiro.shimoda.uh,
geert+renesas, linux-kernel
Hello Again Lee & All,
On Wed, 2021-02-10 at 09:01 +0200, Matti Vaittinen wrote:
> On Tue, 2021-02-09 at 15:25 +0000, Lee Jones wrote:
> > On Fri, 22 Jan 2021, Matti Vaittinen wrote:
> > + /* BD9573 only supports fatal IRQs which we do not
> > > handle */
> >
> > Why not?
>
> Because 'fatal' in the context of this comment means that when this
> condition occurs the PMIC will do emergency shut down for power
> outputs
After having a fresh look at this I see the wisdom in your comment. The
HW provides IRQs and if they are listed in DT for BD9573 does not mean
we should stop here. Thanks for pointing it out.
Best Regards
Matti Vaittinen
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2021-02-11 7:17 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-22 14:30 [PATCH v7 0/6] Support ROHM BD9576MUF and BD9573MUF PMICs Matti Vaittinen
2021-01-22 14:32 ` [PATCH v7 1/6] dt_bindings: mfd: Add " Matti Vaittinen
2021-01-22 14:32 ` [PATCH v7 2/6] mfd: Support ROHM BD9576MUF and BD9573MUF Matti Vaittinen
2021-02-09 14:55 ` Lee Jones
2021-02-10 6:15 ` Matti Vaittinen
2021-01-22 14:33 ` [PATCH v7 3/6] mfd: bd9576: Add IRQ support Matti Vaittinen
2021-02-09 15:25 ` Lee Jones
2021-02-10 7:01 ` Matti Vaittinen
2021-02-11 7:15 ` Vaittinen, Matti
2021-01-22 14:34 ` [PATCH v7 4/6] wdt: Support wdt on ROHM BD9576MUF and BD9573MUF Matti Vaittinen
2021-01-22 14:35 ` [PATCH v7 5/6] MAINTAINERS: Add ROHM BD9576MUF and BD9573MUF drivers Matti Vaittinen
2021-01-23 18:38 ` Guenter Roeck
2021-01-22 14:36 ` [PATCH v7 6/6] regulator: bd9576: Support error reporting Matti Vaittinen
2021-01-24 18:56 ` kernel test robot
2021-02-09 14:19 ` [PATCH v7 0/6] Support ROHM BD9576MUF and BD9573MUF PMICs Vaittinen, Matti
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).