* [PATCH v1 0/6] Support ROHM BD96801 scalable PMIC
@ 2024-04-30 11:59 Matti Vaittinen
2024-04-30 11:59 ` [PATCH v1 1/6] dt-bindings: ROHM BD96801 PMIC regulators Matti Vaittinen
` (5 more replies)
0 siblings, 6 replies; 14+ messages in thread
From: Matti Vaittinen @ 2024-04-30 11:59 UTC (permalink / raw)
To: Matti Vaittinen, Matti Vaittinen
Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Liam Girdwood, Mark Brown, Matti Vaittinen, Wim Van Sebroeck,
Guenter Roeck, devicetree, linux-kernel, linux-watchdog
[-- Attachment #1: Type: text/plain, Size: 3422 bytes --]
Support ROHM BD96801 "scalable" PMIC.
The ROHM BD96801 is automotive grade PMIC, intended to be usable in
multiple solutions. The BD96801 can be used as a stand-alone, or together
with separate 'companion PMICs'. This modular approach aims to make this
PMIC suitable for various use-cases.
This series brings only limited support. The more complete set of
features was sent in the RFC:
https://lore.kernel.org/lkml/cover.1712058690.git.mazziesaccount@gmail.com/
As writing of this there is no known system using the ERRB interrupts,
or doing configurations which require the PMIC to be in STBY state using
Linux driver. Hence it felt natural to upstream only partial support for
now, while leaving a note about the RFC series with more complete
support for those who may need it later.
Revision history still tries to summarize changes from the RFC for the
reviewers.
Revision history:
RFCv2 => v1:
- Drop ERRB IRQ from drivers (but not DT bindings).
- Drop configuration which requires STBY - state.
- Fix the register lock race by moving it from the regulator
driver to the MFD driver.
- Fix watchdog timeout handling
RFCv1 => RFCv2:
- Tidying code based on feedback form Krzysztof Kozlowski and
Lee Jones.
- Documented undocumented watchdog related DT properties.
- Added usage of the watchdog IRQ.
- Use irq_domain_update_bus_token() to work-around debugFS name
collision for IRQ domains.
---
Matti Vaittinen (6):
dt-bindings: ROHM BD96801 PMIC regulators
dt-bindings: mfd: bd96801 PMIC core
mfd: support ROHM BD96801 PMIC core
regulator: bd96801: ROHM BD96801 PMIC regulators
watchdog: ROHM BD96801 PMIC WDG driver
MAINTAINERS: Add ROHM BD96801 'scalable PMIC' entries
.../bindings/mfd/rohm,bd96801-pmic.yaml | 172 ++++
.../regulator/rohm,bd96801-regulator.yaml | 62 ++
MAINTAINERS | 4 +
drivers/mfd/Kconfig | 13 +
drivers/mfd/Makefile | 1 +
drivers/mfd/rohm-bd96801.c | 278 ++++++
drivers/regulator/Kconfig | 12 +
drivers/regulator/Makefile | 2 +
drivers/regulator/bd96801-regulator.c | 896 ++++++++++++++++++
drivers/watchdog/Kconfig | 13 +
drivers/watchdog/Makefile | 1 +
drivers/watchdog/bd96801_wdt.c | 400 ++++++++
include/linux/mfd/rohm-bd96801.h | 215 +++++
include/linux/mfd/rohm-generic.h | 1 +
14 files changed, 2070 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/rohm,bd96801-pmic.yaml
create mode 100644 Documentation/devicetree/bindings/regulator/rohm,bd96801-regulator.yaml
create mode 100644 drivers/mfd/rohm-bd96801.c
create mode 100644 drivers/regulator/bd96801-regulator.c
create mode 100644 drivers/watchdog/bd96801_wdt.c
create mode 100644 include/linux/mfd/rohm-bd96801.h
base-commit: 4cece764965020c22cff7665b18a012006359095
--
2.44.0
--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND
~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =]
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v1 1/6] dt-bindings: ROHM BD96801 PMIC regulators
2024-04-30 11:59 [PATCH v1 0/6] Support ROHM BD96801 scalable PMIC Matti Vaittinen
@ 2024-04-30 11:59 ` Matti Vaittinen
2024-05-02 16:20 ` Conor Dooley
2024-04-30 12:00 ` [PATCH v1 2/6] dt-bindings: mfd: bd96801 PMIC core Matti Vaittinen
` (4 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Matti Vaittinen @ 2024-04-30 11:59 UTC (permalink / raw)
To: Matti Vaittinen, Matti Vaittinen
Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Liam Girdwood, Mark Brown, Matti Vaittinen, Wim Van Sebroeck,
Guenter Roeck, devicetree, linux-kernel, linux-watchdog
[-- Attachment #1: Type: text/plain, Size: 3152 bytes --]
ROHM BD96801 is a highly configurable automotive grade PMIC. Introduce
DT bindings for the BD96801 regulators.
Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
---
RFCv2 => v1
- Drop regulator-name pattern requirement
- do not require regulator-name
---
.../regulator/rohm,bd96801-regulator.yaml | 62 +++++++++++++++++++
1 file changed, 62 insertions(+)
create mode 100644 Documentation/devicetree/bindings/regulator/rohm,bd96801-regulator.yaml
diff --git a/Documentation/devicetree/bindings/regulator/rohm,bd96801-regulator.yaml b/Documentation/devicetree/bindings/regulator/rohm,bd96801-regulator.yaml
new file mode 100644
index 000000000000..1b96ae60064d
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/rohm,bd96801-regulator.yaml
@@ -0,0 +1,62 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/rohm,bd96801-regulator.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ROHM BD96801 Power Management Integrated Circuit regulators
+
+maintainers:
+ - Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
+
+description: |
+ This module is part of the ROHM BD96801 MFD device. For more details
+ see Documentation/devicetree/bindings/mfd/rohm,bd96801-pmic.yaml.
+
+ The regulator controller is represented as a sub-node of the PMIC node
+ on the device tree.
+
+ Regulator nodes should be named to BUCK_<number> and LDO_<number>.
+ The valid names for BD96801 regulator nodes are
+ BUCK1, BUCK2, BUCK3, BUCK4, LDO5, LDO6, LDO7
+
+patternProperties:
+ "^LDO[5-7]$":
+ type: object
+ description:
+ Properties for single LDO regulator.
+ $ref: regulator.yaml#
+
+ properties:
+ rohm,initial-voltage-microvolt:
+ description:
+ Initial voltage for regulator. Voltage can be tuned +/-150 mV from
+ this value. NOTE, This can be modified via I2C only when PMIC is in
+ STBY state.
+ minimum: 300000
+ maximum: 3300000
+
+ unevaluatedProperties: false
+
+ "^BUCK[1-4]$":
+ type: object
+ description:
+ Properties for single BUCK regulator.
+ $ref: regulator.yaml#
+
+ properties:
+ rohm,initial-voltage-microvolt:
+ description:
+ Initial voltage for regulator. Voltage can be tuned +/-150 mV from
+ this value. NOTE, This can be modified via I2C only when PMIC is in
+ STBY state.
+ minimum: 500000
+ maximum: 3300000
+ rohm,keep-on-stby:
+ description:
+ Keep the regulator powered when PMIC transitions to STBY state.
+ type: boolean
+
+ unevaluatedProperties: false
+
+additionalProperties: false
--
2.44.0
--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND
~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =]
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v1 2/6] dt-bindings: mfd: bd96801 PMIC core
2024-04-30 11:59 [PATCH v1 0/6] Support ROHM BD96801 scalable PMIC Matti Vaittinen
2024-04-30 11:59 ` [PATCH v1 1/6] dt-bindings: ROHM BD96801 PMIC regulators Matti Vaittinen
@ 2024-04-30 12:00 ` Matti Vaittinen
2024-04-30 12:00 ` [PATCH v1 3/6] mfd: support ROHM BD96801 " Matti Vaittinen
` (3 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Matti Vaittinen @ 2024-04-30 12:00 UTC (permalink / raw)
To: Matti Vaittinen, Matti Vaittinen
Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Liam Girdwood, Mark Brown, Matti Vaittinen, Wim Van Sebroeck,
Guenter Roeck, devicetree, linux-kernel, linux-watchdog
[-- Attachment #1: Type: text/plain, Size: 7701 bytes --]
ROHM BD96801 is a highly configurable automotive grade PMIC. Introduce
DT bindings for the BD96801 core.
Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
---
Revision history:
RFCv2 => v1
- minor cleaning
- add timeout-sec
RFCv1 => RFCv2:
- Document rohm,hw-timeout-ms
- Document rohm,wdg-action
---
.../bindings/mfd/rohm,bd96801-pmic.yaml | 172 ++++++++++++++++++
1 file changed, 172 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/rohm,bd96801-pmic.yaml
diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd96801-pmic.yaml b/Documentation/devicetree/bindings/mfd/rohm,bd96801-pmic.yaml
new file mode 100644
index 000000000000..fce338681302
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/rohm,bd96801-pmic.yaml
@@ -0,0 +1,172 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/rohm,bd96801-pmic.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ROHM BD96801 Scalable Power Management Integrated Circuit
+
+maintainers:
+ - Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
+
+description:
+ BD96801 is an automotive grade single-chip power management IC.
+ It integrates 4 buck converters and 3 LDOs with safety features like
+ over-/under voltage and over current detection and a watchdog.
+
+properties:
+ compatible:
+ const: rohm,bd96801
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ description:
+ The PMIC provides intb and errb IRQ lines. The errb IRQ line is used
+ for fatal IRQs which will cause the PMIC to shut down power outputs.
+ In many systems this will shut down the SoC contolling the PMIC and
+ connecting/handling the errb can be omitted. However, there are cases
+ where the SoC is not powered by the PMIC. In that case it may be
+ useful to connect the errb and handle errb events.
+ minItems: 1
+ maxItems: 2
+
+ interrupt-names:
+ minItems: 1
+ items:
+ - enum: [intb, errb]
+ - const: errb
+
+ rohm,hw-timeout-ms:
+ description:
+ Watchdog timeout value(s). First walue is timeout limit. Second value is
+ optional value for 'too early' watchdog ping if window timeout mode is
+ to be used.
+ minItems: 1
+ maxItems: 2
+
+ rohm,wdg-action:
+ description:
+ Whether the watchdog failure must turn off the regulator power outputs or
+ just toggle the INTB line.
+ enum:
+ - prstb
+ - intb-only
+
+ timeout-sec:
+ maxItems: 2
+
+ regulators:
+ $ref: /schemas/regulator/rohm,bd96801-regulator.yaml
+ description:
+ List of child nodes that specify the regulators.
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - interrupt-names
+ - regulators
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/leds/common.h>
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ pmic: pmic@60 {
+ reg = <0x60>;
+ compatible = "rohm,bd96801";
+ interrupt-parent = <&gpio1>;
+ interrupts = <29 IRQ_TYPE_LEVEL_LOW>, <6 IRQ_TYPE_LEVEL_LOW>;
+ interrupt-names = "intb", "errb";
+
+ regulators {
+ buck1: BUCK1 {
+ regulator-name = "buck1";
+ regulator-ramp-delay = <1250>;
+ /* 0.5V min INITIAL - 150 mV tune */
+ regulator-min-microvolt = <350000>;
+ /* 3.3V + 150mV tune */
+ regulator-max-microvolt = <3450000>;
+
+ /* These can be set only when PMIC is in STBY */
+ rohm,initial-voltage-microvolt = <500000>;
+ regulator-ov-error-microvolt = <230000>;
+ regulator-uv-error-microvolt = <230000>;
+ regulator-temp-protection-kelvin = <1>;
+ regulator-temp-warn-kelvin = <0>;
+ };
+ buck2: BUCK2 {
+ regulator-name = "buck2";
+ regulator-min-microvolt = <350000>;
+ regulator-max-microvolt = <3450000>;
+
+ rohm,initial-voltage-microvolt = <3000000>;
+ regulator-ov-error-microvolt = <18000>;
+ regulator-uv-error-microvolt = <18000>;
+ regulator-temp-protection-kelvin = <1>;
+ regulator-temp-warn-kelvin = <1>;
+ };
+ buck3: BUCK3 {
+ regulator-name = "buck3";
+ regulator-min-microvolt = <350000>;
+ regulator-max-microvolt = <3450000>;
+
+ rohm,initial-voltage-microvolt = <600000>;
+ regulator-ov-warn-microvolt = <18000>;
+ regulator-uv-warn-microvolt = <18000>;
+ regulator-temp-protection-kelvin = <1>;
+ regulator-temp-error-kelvin = <0>;
+ };
+ buck4: BUCK4 {
+ regulator-name = "buck4";
+ regulator-min-microvolt = <350000>;
+ regulator-max-microvolt = <3450000>;
+
+ rohm,initial-voltage-microvolt = <600000>;
+ regulator-ov-warn-microvolt = <18000>;
+ regulator-uv-warn-microvolt = <18000>;
+ regulator-temp-protection-kelvin = <1>;
+ regulator-temp-error-kelvin = <0>;
+ };
+ ldo5: LDO5 {
+ regulator-name = "ldo5";
+ regulator-min-microvolt = <300000>;
+ regulator-max-microvolt = <3300000>;
+
+ rohm,initial-voltage-microvolt = <500000>;
+ regulator-ov-error-microvolt = <36000>;
+ regulator-uv-error-microvolt = <34000>;
+ regulator-temp-protection-kelvin = <1>;
+ regulator-temp-warn-kelvin = <0>;
+ };
+ ldo6: LDO6 {
+ regulator-name = "ldo6";
+ regulator-min-microvolt = <300000>;
+ regulator-max-microvolt = <3300000>;
+
+ rohm,initial-voltage-microvolt = <300000>;
+ regulator-ov-error-microvolt = <36000>;
+ regulator-uv-error-microvolt = <34000>;
+ regulator-temp-protection-kelvin = <1>;
+ regulator-temp-warn-kelvin = <0>;
+ };
+ ldo7: LDO7 {
+ regulator-name = "ldo7";
+ regulator-min-microvolt = <300000>;
+ regulator-max-microvolt = <3300000>;
+
+ rohm,initial-voltage-microvolt = <500000>;
+ regulator-ov-error-microvolt = <36000>;
+ regulator-uv-error-microvolt = <34000>;
+ regulator-temp-protection-kelvin = <1>;
+ regulator-temp-warn-kelvin = <0>;
+ };
+ };
+ };
+ };
--
2.44.0
--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND
~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =]
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v1 3/6] mfd: support ROHM BD96801 PMIC core
2024-04-30 11:59 [PATCH v1 0/6] Support ROHM BD96801 scalable PMIC Matti Vaittinen
2024-04-30 11:59 ` [PATCH v1 1/6] dt-bindings: ROHM BD96801 PMIC regulators Matti Vaittinen
2024-04-30 12:00 ` [PATCH v1 2/6] dt-bindings: mfd: bd96801 PMIC core Matti Vaittinen
@ 2024-04-30 12:00 ` Matti Vaittinen
2024-05-03 7:41 ` Lee Jones
2024-04-30 12:01 ` [PATCH v1 4/6] regulator: bd96801: ROHM BD96801 PMIC regulators Matti Vaittinen
` (2 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Matti Vaittinen @ 2024-04-30 12:00 UTC (permalink / raw)
To: Matti Vaittinen, Matti Vaittinen
Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Liam Girdwood, Mark Brown, Matti Vaittinen, Wim Van Sebroeck,
Guenter Roeck, devicetree, linux-kernel, linux-watchdog
[-- Attachment #1: Type: text/plain, Size: 21848 bytes --]
The ROHM BD96801 PMIC is highly customizable automotive grade PMIC
which integrates regulator and watchdog funtionalities.
Provide INTB IRQ and register accesses for regulator/watchdog drivers.
Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
---
Changelog: RFCv2 => v1:
- drop ERRB interrupts (for now)
- bd96801: Unlock registers in core driver
Changelog: RFCv1 => RFCv2
- Work-around the IRQ domain name conflict
- Add watchdog IRQ
- Various styling fixes based on review by Lee
---
drivers/mfd/Kconfig | 13 ++
drivers/mfd/Makefile | 1 +
drivers/mfd/rohm-bd96801.c | 278 +++++++++++++++++++++++++++++++
include/linux/mfd/rohm-bd96801.h | 215 ++++++++++++++++++++++++
include/linux/mfd/rohm-generic.h | 1 +
5 files changed, 508 insertions(+)
create mode 100644 drivers/mfd/rohm-bd96801.c
create mode 100644 include/linux/mfd/rohm-bd96801.h
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 4b023ee229cf..9e874453d94d 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -2089,6 +2089,19 @@ config MFD_ROHM_BD957XMUF
BD9573MUF Power Management ICs. BD9576 and BD9573 are primarily
designed to be used to power R-Car series processors.
+config MFD_ROHM_BD96801
+ tristate "ROHM BD96801 Power Management IC"
+ depends on I2C=y
+ depends on OF
+ select REGMAP_I2C
+ select REGMAP_IRQ
+ select MFD_CORE
+ help
+ Select this option to get support for the ROHM BD96801 Power
+ Management IC. The ROHM BD96801 is a highly scalable Power Management
+ IC for industrial and automotive use. The BD96801 can be used as a
+ master PMIC in a chained PMIC solution with suitable companion PMICs.
+
config MFD_STM32_LPTIMER
tristate "Support for STM32 Low-Power Timer"
depends on (ARCH_STM32 && OF) || COMPILE_TEST
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index c66f07edcd0e..e792892d4a8b 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -264,6 +264,7 @@ obj-$(CONFIG_RAVE_SP_CORE) += rave-sp.o
obj-$(CONFIG_MFD_ROHM_BD71828) += rohm-bd71828.o
obj-$(CONFIG_MFD_ROHM_BD718XX) += rohm-bd718x7.o
obj-$(CONFIG_MFD_ROHM_BD957XMUF) += rohm-bd9576.o
+obj-$(CONFIG_MFD_ROHM_BD96801) += rohm-bd96801.o
obj-$(CONFIG_MFD_STMFX) += stmfx.o
obj-$(CONFIG_MFD_KHADAS_MCU) += khadas-mcu.o
obj-$(CONFIG_MFD_ACER_A500_EC) += acer-ec-a500.o
diff --git a/drivers/mfd/rohm-bd96801.c b/drivers/mfd/rohm-bd96801.c
new file mode 100644
index 000000000000..1e9c49c857c1
--- /dev/null
+++ b/drivers/mfd/rohm-bd96801.c
@@ -0,0 +1,278 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2024 ROHM Semiconductors
+ *
+ * ROHM BD96801 PMIC driver
+ *
+ * This version of the "BD86801 scalable PMIC"'s driver supports only very
+ * basic set of the PMIC features. Most notably, there is no support for
+ * the ERRB interrupt and the configurations which should be done when the
+ * PMIC is in STBY mode.
+ *
+ * Supporting the ERRB interrupt would require dropping the regmap-IRQ
+ * usage or working around (or accepting a presense of) a naming conflict
+ * in debugFS IRQs.
+ *
+ * Being able to reliably do the configurations like changing the
+ * regulator safety limits (like limits for the over/under -voltages, over
+ * current, thermal protection) would require the configuring driver to be
+ * synchronized with entity causing the PMIC state transitions. Eg, one
+ * should be able to ensure the PMIC is in STBY state when the
+ * configurations are applied to the hardware. How and when the PMIC state
+ * transitions are to be done is likely to be very system specific, as will
+ * be the need to configure these safety limits. Hence it's not simple to
+ * come up with a generic solution.
+ *
+ * Users who require the ERRB handling and STBY state configurations can
+ * have a look at the original RFC:
+ * https://lore.kernel.org/all/cover.1712920132.git.mazziesaccount@gmail.com/
+ * which implements a workaround to debugFS naming conflict and some of
+ * the safety limit configurations - but leaves the state change handling
+ * and synchronization to be implemented.
+ *
+ * It would be great to hear (and receive a patch!) if you implement the
+ * STBY configuration support or a proper fix to the debugFS naming
+ * conflict in your downstream driver ;)
+ */
+
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/mfd/core.h>
+#include <linux/module.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+#include <linux/types.h>
+
+#include <linux/mfd/rohm-bd96801.h>
+#include <linux/mfd/rohm-generic.h>
+static const struct resource regulator_intb_irqs[] = {
+ DEFINE_RES_IRQ_NAMED(BD96801_TW_STAT, "bd96801-core-thermal"),
+
+ DEFINE_RES_IRQ_NAMED(BD96801_BUCK1_OCPH_STAT, "bd96801-buck1-overcurr-h"),
+ DEFINE_RES_IRQ_NAMED(BD96801_BUCK1_OCPL_STAT, "bd96801-buck1-overcurr-l"),
+ DEFINE_RES_IRQ_NAMED(BD96801_BUCK1_OCPN_STAT, "bd96801-buck1-overcurr-n"),
+ DEFINE_RES_IRQ_NAMED(BD96801_BUCK1_OVD_STAT, "bd96801-buck1-overvolt"),
+ DEFINE_RES_IRQ_NAMED(BD96801_BUCK1_UVD_STAT, "bd96801-buck1-undervolt"),
+ DEFINE_RES_IRQ_NAMED(BD96801_BUCK1_TW_CH_STAT, "bd96801-buck1-thermal"),
+
+ DEFINE_RES_IRQ_NAMED(BD96801_BUCK2_OCPH_STAT, "bd96801-buck2-overcurr-h"),
+ DEFINE_RES_IRQ_NAMED(BD96801_BUCK2_OCPL_STAT, "bd96801-buck2-overcurr-l"),
+ DEFINE_RES_IRQ_NAMED(BD96801_BUCK2_OCPN_STAT, "bd96801-buck2-overcurr-n"),
+ DEFINE_RES_IRQ_NAMED(BD96801_BUCK2_OVD_STAT, "bd96801-buck2-overvolt"),
+ DEFINE_RES_IRQ_NAMED(BD96801_BUCK2_UVD_STAT, "bd96801-buck2-undervolt"),
+ DEFINE_RES_IRQ_NAMED(BD96801_BUCK2_TW_CH_STAT, "bd96801-buck2-thermal"),
+
+ DEFINE_RES_IRQ_NAMED(BD96801_BUCK3_OCPH_STAT, "bd96801-buck3-overcurr-h"),
+ DEFINE_RES_IRQ_NAMED(BD96801_BUCK3_OCPL_STAT, "bd96801-buck3-overcurr-l"),
+ DEFINE_RES_IRQ_NAMED(BD96801_BUCK3_OCPN_STAT, "bd96801-buck3-overcurr-n"),
+ DEFINE_RES_IRQ_NAMED(BD96801_BUCK3_OVD_STAT, "bd96801-buck3-overvolt"),
+ DEFINE_RES_IRQ_NAMED(BD96801_BUCK3_UVD_STAT, "bd96801-buck3-undervolt"),
+ DEFINE_RES_IRQ_NAMED(BD96801_BUCK3_TW_CH_STAT, "bd96801-buck3-thermal"),
+
+ DEFINE_RES_IRQ_NAMED(BD96801_BUCK4_OCPH_STAT, "bd96801-buck4-overcurr-h"),
+ DEFINE_RES_IRQ_NAMED(BD96801_BUCK4_OCPL_STAT, "bd96801-buck4-overcurr-l"),
+ DEFINE_RES_IRQ_NAMED(BD96801_BUCK4_OCPN_STAT, "bd96801-buck4-overcurr-n"),
+ DEFINE_RES_IRQ_NAMED(BD96801_BUCK4_OVD_STAT, "bd96801-buck4-overvolt"),
+ DEFINE_RES_IRQ_NAMED(BD96801_BUCK4_UVD_STAT, "bd96801-buck4-undervolt"),
+ DEFINE_RES_IRQ_NAMED(BD96801_BUCK4_TW_CH_STAT, "bd96801-buck4-thermal"),
+
+ DEFINE_RES_IRQ_NAMED(BD96801_LDO5_OCPH_STAT, "bd96801-ldo5-overcurr"),
+ DEFINE_RES_IRQ_NAMED(BD96801_LDO5_OVD_STAT, "bd96801-ldo5-overvolt"),
+ DEFINE_RES_IRQ_NAMED(BD96801_LDO5_UVD_STAT, "bd96801-ldo5-undervolt"),
+
+ DEFINE_RES_IRQ_NAMED(BD96801_LDO6_OCPH_STAT, "bd96801-ldo6-overcurr"),
+ DEFINE_RES_IRQ_NAMED(BD96801_LDO6_OVD_STAT, "bd96801-ldo6-overvolt"),
+ DEFINE_RES_IRQ_NAMED(BD96801_LDO6_UVD_STAT, "bd96801-ldo6-undervolt"),
+
+ DEFINE_RES_IRQ_NAMED(BD96801_LDO7_OCPH_STAT, "bd96801-ldo7-overcurr"),
+ DEFINE_RES_IRQ_NAMED(BD96801_LDO7_OVD_STAT, "bd96801-ldo7-overvolt"),
+ DEFINE_RES_IRQ_NAMED(BD96801_LDO7_UVD_STAT, "bd96801-ldo7-undervolt"),
+};
+
+static const struct resource wdg_intb_irqs[] = {
+ DEFINE_RES_IRQ_NAMED(BD96801_WDT_ERR_STAT, "bd96801-wdg"),
+};
+
+enum {
+ WDG_CELL = 0,
+ REGULATOR_CELL,
+};
+
+static struct mfd_cell bd96801_mfd_cells[] = {
+ {
+ .name = "bd96801-wdt",
+ .resources = wdg_intb_irqs,
+ .num_resources = ARRAY_SIZE(wdg_intb_irqs),
+ }, {
+ .name = "bd96801-pmic",
+ .resources = regulator_intb_irqs,
+ .num_resources = ARRAY_SIZE(regulator_intb_irqs),
+ },
+};
+
+static const struct regmap_range bd96801_volatile_ranges[] = {
+ /* Status regs */
+ regmap_reg_range(BD96801_REG_WD_FEED, BD96801_REG_WD_FAILCOUNT),
+ regmap_reg_range(BD96801_REG_WD_ASK, BD96801_REG_WD_ASK),
+ regmap_reg_range(BD96801_REG_WD_STATUS, BD96801_REG_WD_STATUS),
+ regmap_reg_range(BD96801_REG_PMIC_STATE, BD96801_REG_INT_LDO7_INTB),
+ /* Registers which do not update value unless PMIC is in STBY */
+ regmap_reg_range(BD96801_REG_SSCG_CTRL, BD96801_REG_SHD_INTB),
+ regmap_reg_range(BD96801_REG_BUCK_OVP, BD96801_REG_BOOT_OVERTIME),
+ /*
+ * LDO control registers have single bit (LDO MODE) which does not
+ * change when we write it unless PMIC is in STBY. It's safer to not
+ * cache it.
+ */
+ regmap_reg_range(BD96801_LDO5_VOL_LVL_REG, BD96801_LDO7_VOL_LVL_REG),
+};
+
+static const struct regmap_access_table volatile_regs = {
+ .yes_ranges = bd96801_volatile_ranges,
+ .n_yes_ranges = ARRAY_SIZE(bd96801_volatile_ranges),
+};
+
+static const struct regmap_irq bd96801_intb_irqs[] = {
+ /* STATUS SYSTEM INTB */
+ REGMAP_IRQ_REG(BD96801_TW_STAT, 0, BD96801_TW_STAT_MASK),
+ REGMAP_IRQ_REG(BD96801_WDT_ERR_STAT, 0, BD96801_WDT_ERR_STAT_MASK),
+ REGMAP_IRQ_REG(BD96801_I2C_ERR_STAT, 0, BD96801_I2C_ERR_STAT_MASK),
+ REGMAP_IRQ_REG(BD96801_CHIP_IF_ERR_STAT, 0, BD96801_CHIP_IF_ERR_STAT_MASK),
+ /* STATUS BUCK1 INTB */
+ REGMAP_IRQ_REG(BD96801_BUCK1_OCPH_STAT, 1, BD96801_BUCK_OCPH_STAT_MASK),
+ REGMAP_IRQ_REG(BD96801_BUCK1_OCPL_STAT, 1, BD96801_BUCK_OCPL_STAT_MASK),
+ REGMAP_IRQ_REG(BD96801_BUCK1_OCPN_STAT, 1, BD96801_BUCK_OCPN_STAT_MASK),
+ REGMAP_IRQ_REG(BD96801_BUCK1_OVD_STAT, 1, BD96801_BUCK_OVD_STAT_MASK),
+ REGMAP_IRQ_REG(BD96801_BUCK1_UVD_STAT, 1, BD96801_BUCK_UVD_STAT_MASK),
+ REGMAP_IRQ_REG(BD96801_BUCK1_TW_CH_STAT, 1, BD96801_BUCK_TW_CH_STAT_MASK),
+ /* BUCK 2 INTB */
+ REGMAP_IRQ_REG(BD96801_BUCK2_OCPH_STAT, 2, BD96801_BUCK_OCPH_STAT_MASK),
+ REGMAP_IRQ_REG(BD96801_BUCK2_OCPL_STAT, 2, BD96801_BUCK_OCPL_STAT_MASK),
+ REGMAP_IRQ_REG(BD96801_BUCK2_OCPN_STAT, 2, BD96801_BUCK_OCPN_STAT_MASK),
+ REGMAP_IRQ_REG(BD96801_BUCK2_OVD_STAT, 2, BD96801_BUCK_OVD_STAT_MASK),
+ REGMAP_IRQ_REG(BD96801_BUCK2_UVD_STAT, 2, BD96801_BUCK_UVD_STAT_MASK),
+ REGMAP_IRQ_REG(BD96801_BUCK2_TW_CH_STAT, 2, BD96801_BUCK_TW_CH_STAT_MASK),
+ /* BUCK 3 INTB */
+ REGMAP_IRQ_REG(BD96801_BUCK3_OCPH_STAT, 3, BD96801_BUCK_OCPH_STAT_MASK),
+ REGMAP_IRQ_REG(BD96801_BUCK3_OCPL_STAT, 3, BD96801_BUCK_OCPL_STAT_MASK),
+ REGMAP_IRQ_REG(BD96801_BUCK3_OCPN_STAT, 3, BD96801_BUCK_OCPN_STAT_MASK),
+ REGMAP_IRQ_REG(BD96801_BUCK3_OVD_STAT, 3, BD96801_BUCK_OVD_STAT_MASK),
+ REGMAP_IRQ_REG(BD96801_BUCK3_UVD_STAT, 3, BD96801_BUCK_UVD_STAT_MASK),
+ REGMAP_IRQ_REG(BD96801_BUCK3_TW_CH_STAT, 3, BD96801_BUCK_TW_CH_STAT_MASK),
+ /* BUCK 4 INTB */
+ REGMAP_IRQ_REG(BD96801_BUCK4_OCPH_STAT, 4, BD96801_BUCK_OCPH_STAT_MASK),
+ REGMAP_IRQ_REG(BD96801_BUCK4_OCPL_STAT, 4, BD96801_BUCK_OCPL_STAT_MASK),
+ REGMAP_IRQ_REG(BD96801_BUCK4_OCPN_STAT, 4, BD96801_BUCK_OCPN_STAT_MASK),
+ REGMAP_IRQ_REG(BD96801_BUCK4_OVD_STAT, 4, BD96801_BUCK_OVD_STAT_MASK),
+ REGMAP_IRQ_REG(BD96801_BUCK4_UVD_STAT, 4, BD96801_BUCK_UVD_STAT_MASK),
+ REGMAP_IRQ_REG(BD96801_BUCK4_TW_CH_STAT, 4, BD96801_BUCK_TW_CH_STAT_MASK),
+ /* LDO5 INTB */
+ REGMAP_IRQ_REG(BD96801_LDO5_OCPH_STAT, 5, BD96801_LDO_OCPH_STAT_MASK),
+ REGMAP_IRQ_REG(BD96801_LDO5_OVD_STAT, 5, BD96801_LDO_OVD_STAT_MASK),
+ REGMAP_IRQ_REG(BD96801_LDO5_UVD_STAT, 5, BD96801_LDO_UVD_STAT_MASK),
+ /* LDO6 INTB */
+ REGMAP_IRQ_REG(BD96801_LDO6_OCPH_STAT, 6, BD96801_LDO_OCPH_STAT_MASK),
+ REGMAP_IRQ_REG(BD96801_LDO6_OVD_STAT, 6, BD96801_LDO_OVD_STAT_MASK),
+ REGMAP_IRQ_REG(BD96801_LDO6_UVD_STAT, 6, BD96801_LDO_UVD_STAT_MASK),
+ /* LDO7 INTB */
+ REGMAP_IRQ_REG(BD96801_LDO7_OCPH_STAT, 7, BD96801_LDO_OCPH_STAT_MASK),
+ REGMAP_IRQ_REG(BD96801_LDO7_OVD_STAT, 7, BD96801_LDO_OVD_STAT_MASK),
+ REGMAP_IRQ_REG(BD96801_LDO7_UVD_STAT, 7, BD96801_LDO_UVD_STAT_MASK),
+};
+
+static struct regmap_irq_chip bd96801_irq_chip_intb = {
+ .name = "bd96801-irq-intb",
+ .main_status = BD96801_REG_INT_MAIN,
+ .num_main_regs = 1,
+ .irqs = &bd96801_intb_irqs[0],
+ .num_irqs = ARRAY_SIZE(bd96801_intb_irqs),
+ .status_base = BD96801_REG_INT_SYS_INTB,
+ .mask_base = BD96801_REG_MASK_SYS_INTB,
+ .ack_base = BD96801_REG_INT_SYS_INTB,
+ .init_ack_masked = true,
+ .num_regs = 8,
+ .irq_reg_stride = 1,
+};
+
+static const struct regmap_config bd96801_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .volatile_table = &volatile_regs,
+ .cache_type = REGCACHE_RBTREE,
+};
+
+static int bd96801_i2c_probe(struct i2c_client *i2c)
+{
+ struct regmap *regmap;
+ int ret, intb_irq;
+ struct regmap_irq_chip_data *intb_irq_data;
+ struct irq_domain *intb_domain;
+ const struct fwnode_handle *fwnode;
+
+ fwnode = dev_fwnode(&i2c->dev);
+ if (!fwnode)
+ return dev_err_probe(&i2c->dev, -EINVAL, "no fwnode\n");
+
+ intb_irq = fwnode_irq_get_byname(fwnode, "intb");
+ if (intb_irq < 0)
+ return dev_err_probe(&i2c->dev, intb_irq, "No INTB IRQ configured\n");
+
+ regmap = devm_regmap_init_i2c(i2c, &bd96801_regmap_config);
+ if (IS_ERR(regmap))
+ return dev_err_probe(&i2c->dev, PTR_ERR(regmap),
+ "regmap initialization failed\n");
+
+ ret = regmap_write(regmap, BD96801_LOCK_REG, BD96801_UNLOCK);
+ if (ret)
+ return dev_err_probe(&i2c->dev, ret, "Can't unlock PMIC\n");
+
+ ret = devm_regmap_add_irq_chip(&i2c->dev, regmap, intb_irq,
+ IRQF_ONESHOT, 0, &bd96801_irq_chip_intb,
+ &intb_irq_data);
+ if (ret)
+ return dev_err_probe(&i2c->dev, ret, "Failed to add INTB irq_chip\n");
+
+ intb_domain = regmap_irq_get_domain(intb_irq_data);
+
+ ret = devm_mfd_add_devices(&i2c->dev, PLATFORM_DEVID_AUTO,
+ bd96801_mfd_cells,
+ ARRAY_SIZE(bd96801_mfd_cells), NULL, 0,
+ intb_domain);
+
+ if (ret)
+ dev_err(&i2c->dev, "Failed to create subdevices\n");
+
+ return ret;
+}
+
+static const struct of_device_id bd96801_of_match[] = {
+ { .compatible = "rohm,bd96801", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, bd96801_of_match);
+
+static struct i2c_driver bd96801_i2c_driver = {
+ .driver = {
+ .name = "rohm-bd96801",
+ .of_match_table = bd96801_of_match,
+ },
+ .probe = bd96801_i2c_probe,
+};
+
+static int __init bd96801_i2c_init(void)
+{
+ return i2c_add_driver(&bd96801_i2c_driver);
+}
+
+/* Initialise early so consumer devices can complete system boot? */
+subsys_initcall(bd96801_i2c_init);
+
+static void __exit bd96801_i2c_exit(void)
+{
+ i2c_del_driver(&bd96801_i2c_driver);
+}
+module_exit(bd96801_i2c_exit);
+
+MODULE_AUTHOR("Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>");
+MODULE_DESCRIPTION("ROHM BD96801 Power Management IC driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/rohm-bd96801.h b/include/linux/mfd/rohm-bd96801.h
new file mode 100644
index 000000000000..e2d9e10b6364
--- /dev/null
+++ b/include/linux/mfd/rohm-bd96801.h
@@ -0,0 +1,215 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/* Copyright (C) 2024 ROHM Semiconductors */
+
+#ifndef __MFD_BD96801_H__
+#define __MFD_BD96801_H__
+
+#define BD96801_REG_SSCG_CTRL 0x09
+#define BD96801_REG_SHD_INTB 0x20
+#define BD96801_LDO5_VOL_LVL_REG 0x2c
+#define BD96801_LDO6_VOL_LVL_REG 0x2d
+#define BD96801_LDO7_VOL_LVL_REG 0x2e
+#define BD96801_REG_BUCK_OVP 0x30
+#define BD96801_REG_BUCK_OVD 0x35
+#define BD96801_REG_LDO_OVP 0x31
+#define BD96801_REG_LDO_OVD 0x36
+#define BD96801_REG_BOOT_OVERTIME 0x3a
+#define BD96801_REG_WD_TMO 0x40
+#define BD96801_REG_WD_CONF 0x41
+#define BD96801_REG_WD_FEED 0x42
+#define BD96801_REG_WD_FAILCOUNT 0x43
+#define BD96801_REG_WD_ASK 0x46
+#define BD96801_REG_WD_STATUS 0x4a
+#define BD96801_REG_PMIC_STATE 0x4f
+#define BD96801_REG_EXT_STATE 0x50
+
+#define BD96801_STATE_STBY 0x09
+
+#define BD96801_LOCK_REG 0x04
+#define BD96801_UNLOCK 0x9d
+#define BD96801_LOCK 0x00
+
+/* IRQ register area */
+#define BD96801_REG_INT_MAIN 0x51
+
+/*
+ * The BD96801 has two physical IRQ lines, INTB and ERRB.
+ *
+ * The 'main status register' is located at 0x51.
+ * The ERRB status registers are located at 0x52 ... 0x5B
+ * INTB status registers are at range 0x5c ... 0x63
+ */
+#define BD96801_REG_INT_SYS_ERRB1 0x52
+#define BD96801_REG_INT_SYS_INTB 0x5c
+#define BD96801_REG_INT_LDO7_INTB 0x63
+
+/* MASK registers */
+#define BD96801_REG_MASK_SYS_INTB 0x73
+#define BD96801_REG_MASK_SYS_ERRB 0x69
+
+#define BD96801_MAX_REGISTER 0x7a
+
+#define BD96801_OTP_ERR_MASK BIT(0)
+#define BD96801_DBIST_ERR_MASK BIT(1)
+#define BD96801_EEP_ERR_MASK BIT(2)
+#define BD96801_ABIST_ERR_MASK BIT(3)
+#define BD96801_PRSTB_ERR_MASK BIT(4)
+#define BD96801_DRMOS1_ERR_MASK BIT(5)
+#define BD96801_DRMOS2_ERR_MASK BIT(6)
+#define BD96801_SLAVE_ERR_MASK BIT(7)
+#define BD96801_VREF_ERR_MASK BIT(0)
+#define BD96801_TSD_ERR_MASK BIT(1)
+#define BD96801_UVLO_ERR_MASK BIT(2)
+#define BD96801_OVLO_ERR_MASK BIT(3)
+#define BD96801_OSC_ERR_MASK BIT(4)
+#define BD96801_PON_ERR_MASK BIT(5)
+#define BD96801_POFF_ERR_MASK BIT(6)
+#define BD96801_CMD_SHDN_ERR_MASK BIT(7)
+#define BD96801_INT_PRSTB_WDT_ERR_MASK BIT(0)
+#define BD96801_INT_CHIP_IF_ERR_MASK BIT(3)
+#define BD96801_INT_SHDN_ERR_MASK BIT(7)
+#define BD96801_OUT_PVIN_ERR_MASK BIT(0)
+#define BD96801_OUT_OVP_ERR_MASK BIT(1)
+#define BD96801_OUT_UVP_ERR_MASK BIT(2)
+#define BD96801_OUT_SHDN_ERR_MASK BIT(7)
+
+/* ERRB IRQs */
+enum {
+ /* Reg 0x52, 0x53, 0x54 - ERRB system IRQs */
+ BD96801_OTP_ERR_STAT,
+ BD96801_DBIST_ERR_STAT,
+ BD96801_EEP_ERR_STAT,
+ BD96801_ABIST_ERR_STAT,
+ BD96801_PRSTB_ERR_STAT,
+ BD96801_DRMOS1_ERR_STAT,
+ BD96801_DRMOS2_ERR_STAT,
+ BD96801_SLAVE_ERR_STAT,
+ BD96801_VREF_ERR_STAT,
+ BD96801_TSD_ERR_STAT,
+ BD96801_UVLO_ERR_STAT,
+ BD96801_OVLO_ERR_STAT,
+ BD96801_OSC_ERR_STAT,
+ BD96801_PON_ERR_STAT,
+ BD96801_POFF_ERR_STAT,
+ BD96801_CMD_SHDN_ERR_STAT,
+ BD96801_INT_PRSTB_WDT_ERR,
+ BD96801_INT_CHIP_IF_ERR,
+ BD96801_INT_SHDN_ERR_STAT,
+
+ /* Reg 0x55 BUCK1 ERR IRQs */
+ BD96801_BUCK1_PVIN_ERR_STAT,
+ BD96801_BUCK1_OVP_ERR_STAT,
+ BD96801_BUCK1_UVP_ERR_STAT,
+ BD96801_BUCK1_SHDN_ERR_STAT,
+
+ /* Reg 0x56 BUCK2 ERR IRQs */
+ BD96801_BUCK2_PVIN_ERR_STAT,
+ BD96801_BUCK2_OVP_ERR_STAT,
+ BD96801_BUCK2_UVP_ERR_STAT,
+ BD96801_BUCK2_SHDN_ERR_STAT,
+
+ /* Reg 0x57 BUCK3 ERR IRQs */
+ BD96801_BUCK3_PVIN_ERR_STAT,
+ BD96801_BUCK3_OVP_ERR_STAT,
+ BD96801_BUCK3_UVP_ERR_STAT,
+ BD96801_BUCK3_SHDN_ERR_STAT,
+
+ /* Reg 0x58 BUCK4 ERR IRQs */
+ BD96801_BUCK4_PVIN_ERR_STAT,
+ BD96801_BUCK4_OVP_ERR_STAT,
+ BD96801_BUCK4_UVP_ERR_STAT,
+ BD96801_BUCK4_SHDN_ERR_STAT,
+
+ /* Reg 0x59 LDO5 ERR IRQs */
+ BD96801_LDO5_PVIN_ERR_STAT,
+ BD96801_LDO5_OVP_ERR_STAT,
+ BD96801_LDO5_UVP_ERR_STAT,
+ BD96801_LDO5_SHDN_ERR_STAT,
+
+ /* Reg 0x5a LDO6 ERR IRQs */
+ BD96801_LDO6_PVIN_ERR_STAT,
+ BD96801_LDO6_OVP_ERR_STAT,
+ BD96801_LDO6_UVP_ERR_STAT,
+ BD96801_LDO6_SHDN_ERR_STAT,
+
+ /* Reg 0x5b LDO7 ERR IRQs */
+ BD96801_LDO7_PVIN_ERR_STAT,
+ BD96801_LDO7_OVP_ERR_STAT,
+ BD96801_LDO7_UVP_ERR_STAT,
+ BD96801_LDO7_SHDN_ERR_STAT,
+};
+
+/* INTB IRQs */
+enum {
+ /* Reg 0x5c (System INTB) */
+ BD96801_TW_STAT,
+ BD96801_WDT_ERR_STAT,
+ BD96801_I2C_ERR_STAT,
+ BD96801_CHIP_IF_ERR_STAT,
+
+ /* Reg 0x5d (BUCK1 INTB) */
+ BD96801_BUCK1_OCPH_STAT,
+ BD96801_BUCK1_OCPL_STAT,
+ BD96801_BUCK1_OCPN_STAT,
+ BD96801_BUCK1_OVD_STAT,
+ BD96801_BUCK1_UVD_STAT,
+ BD96801_BUCK1_TW_CH_STAT,
+
+ /* Reg 0x5e (BUCK2 INTB) */
+ BD96801_BUCK2_OCPH_STAT,
+ BD96801_BUCK2_OCPL_STAT,
+ BD96801_BUCK2_OCPN_STAT,
+ BD96801_BUCK2_OVD_STAT,
+ BD96801_BUCK2_UVD_STAT,
+ BD96801_BUCK2_TW_CH_STAT,
+
+ /* Reg 0x5f (BUCK3 INTB)*/
+ BD96801_BUCK3_OCPH_STAT,
+ BD96801_BUCK3_OCPL_STAT,
+ BD96801_BUCK3_OCPN_STAT,
+ BD96801_BUCK3_OVD_STAT,
+ BD96801_BUCK3_UVD_STAT,
+ BD96801_BUCK3_TW_CH_STAT,
+
+ /* Reg 0x60 (BUCK4 INTB)*/
+ BD96801_BUCK4_OCPH_STAT,
+ BD96801_BUCK4_OCPL_STAT,
+ BD96801_BUCK4_OCPN_STAT,
+ BD96801_BUCK4_OVD_STAT,
+ BD96801_BUCK4_UVD_STAT,
+ BD96801_BUCK4_TW_CH_STAT,
+
+ /* Reg 0x61 (LDO5 INTB) */
+ BD96801_LDO5_OCPH_STAT, /* bit [0] */
+ BD96801_LDO5_OVD_STAT, /* bit [3] */
+ BD96801_LDO5_UVD_STAT, /* bit [4] */
+
+ /* Reg 0x62 (LDO6 INTB) */
+ BD96801_LDO6_OCPH_STAT, /* bit [0] */
+ BD96801_LDO6_OVD_STAT, /* bit [3] */
+ BD96801_LDO6_UVD_STAT, /* bit [4] */
+
+ /* Reg 0x63 (LDO7 INTB) */
+ BD96801_LDO7_OCPH_STAT, /* bit [0] */
+ BD96801_LDO7_OVD_STAT, /* bit [3] */
+ BD96801_LDO7_UVD_STAT, /* bit [4] */
+};
+
+/* IRQ MASKs */
+#define BD96801_TW_STAT_MASK BIT(0)
+#define BD96801_WDT_ERR_STAT_MASK BIT(1)
+#define BD96801_I2C_ERR_STAT_MASK BIT(2)
+#define BD96801_CHIP_IF_ERR_STAT_MASK BIT(3)
+
+#define BD96801_BUCK_OCPH_STAT_MASK BIT(0)
+#define BD96801_BUCK_OCPL_STAT_MASK BIT(1)
+#define BD96801_BUCK_OCPN_STAT_MASK BIT(2)
+#define BD96801_BUCK_OVD_STAT_MASK BIT(3)
+#define BD96801_BUCK_UVD_STAT_MASK BIT(4)
+#define BD96801_BUCK_TW_CH_STAT_MASK BIT(5)
+
+#define BD96801_LDO_OCPH_STAT_MASK BIT(0)
+#define BD96801_LDO_OVD_STAT_MASK BIT(3)
+#define BD96801_LDO_UVD_STAT_MASK BIT(4)
+
+#endif
diff --git a/include/linux/mfd/rohm-generic.h b/include/linux/mfd/rohm-generic.h
index 4eeb22876bad..e7d4e6afe388 100644
--- a/include/linux/mfd/rohm-generic.h
+++ b/include/linux/mfd/rohm-generic.h
@@ -16,6 +16,7 @@ enum rohm_chip_type {
ROHM_CHIP_TYPE_BD71828,
ROHM_CHIP_TYPE_BD71837,
ROHM_CHIP_TYPE_BD71847,
+ ROHM_CHIP_TYPE_BD96801,
ROHM_CHIP_TYPE_AMOUNT
};
--
2.44.0
--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND
~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =]
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v1 4/6] regulator: bd96801: ROHM BD96801 PMIC regulators
2024-04-30 11:59 [PATCH v1 0/6] Support ROHM BD96801 scalable PMIC Matti Vaittinen
` (2 preceding siblings ...)
2024-04-30 12:00 ` [PATCH v1 3/6] mfd: support ROHM BD96801 " Matti Vaittinen
@ 2024-04-30 12:01 ` Matti Vaittinen
2024-04-30 12:01 ` [PATCH v1 5/6] watchdog: ROHM BD96801 PMIC WDG driver Matti Vaittinen
2024-04-30 12:02 ` [PATCH v1 6/6] MAINTAINERS: Add ROHM BD96801 'scalable PMIC' entries Matti Vaittinen
5 siblings, 0 replies; 14+ messages in thread
From: Matti Vaittinen @ 2024-04-30 12:01 UTC (permalink / raw)
To: Matti Vaittinen, Matti Vaittinen
Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Liam Girdwood, Mark Brown, Matti Vaittinen, Wim Van Sebroeck,
Guenter Roeck, devicetree, linux-kernel, linux-watchdog
[-- Attachment #1: Type: text/plain, Size: 32802 bytes --]
The ROHM BD96801 "Scalable PMIC" is an automotive grade PMIC which can
scale to different applications by allowing chaining of PMICs. The PMIC
also supports various protection features which can be configured either
to fire IRQs - or to shut down power outputs when failure is detected.
The driver implements basic voltage control and sending error
notifications.
NOTE:
The driver does not support doing configuration which require the PMIC
to be in STBY state. The omitted feature set includes setting safety
limit values, changing LDO voltages and controlling enable state for
some regulators.
Also, the ERRB IRQ is not handled.
Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
---
Revision history:
RFCv2 => v1:
- Drop STBY mode configurations
- Drop ERRB IRQs
- drop PMIC register unlock (moved to MFD to prevent races)
RFCv1 => RFCv2:
- Use devm_kcalloc() instead of devm_kzalloc() where appropriate
- Use devm_kmemdup()
- Add MODULE_DEVICE_TABLE
- drop MODULE_ALIAS
---
drivers/regulator/Kconfig | 12 +
drivers/regulator/Makefile | 2 +
drivers/regulator/bd96801-regulator.c | 896 ++++++++++++++++++++++++++
3 files changed, 910 insertions(+)
create mode 100644 drivers/regulator/bd96801-regulator.c
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 7db0a29b5b8d..2e8187a92952 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -268,6 +268,18 @@ config REGULATOR_BD957XMUF
This driver can also be built as a module. If so, the module
will be called bd9576-regulator.
+config REGULATOR_BD96801
+ tristate "ROHM BD96801 Power Regulator"
+ depends on MFD_ROHM_BD96801
+ select REGULATOR_ROHM
+ help
+ This driver supports voltage regulators on ROHM BD96801 PMIC.
+ This will enable support for the software controllable buck
+ and LDO regulators.
+
+ This driver can also be built as a module. If so, the module
+ will be called bd96801-regulator.
+
config REGULATOR_CPCAP
tristate "Motorola CPCAP regulator"
depends on MFD_CPCAP
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 46fb569e6be8..ae8896e879d5 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -37,6 +37,8 @@ obj-$(CONFIG_REGULATOR_BD718XX) += bd718x7-regulator.o
obj-$(CONFIG_REGULATOR_BD9571MWV) += bd9571mwv-regulator.o
obj-$(CONFIG_REGULATOR_BD957XMUF) += bd9576-regulator.o
obj-$(CONFIG_REGULATOR_DA903X) += da903x-regulator.o
+obj-$(CONFIG_REGULATOR_BD96801) += bd96801-regulator.o
+obj-$(CONFIG_REGULATOR_DA903X) += da903x.o
obj-$(CONFIG_REGULATOR_DA9052) += da9052-regulator.o
obj-$(CONFIG_REGULATOR_DA9055) += da9055-regulator.o
obj-$(CONFIG_REGULATOR_DA9062) += da9062-regulator.o
diff --git a/drivers/regulator/bd96801-regulator.c b/drivers/regulator/bd96801-regulator.c
new file mode 100644
index 000000000000..02857754b532
--- /dev/null
+++ b/drivers/regulator/bd96801-regulator.c
@@ -0,0 +1,896 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2024 ROHM Semiconductors
+// bd96801-regulator.c ROHM BD96801 regulator driver
+
+/*
+ * This version of the "BD86801 scalable PMIC"'s driver supports only very
+ * basic set of the PMIC features. Most notably, there is no support for
+ * the ERRB interrupt and the configurations which should be done when the
+ * PMIC is in STBY mode.
+ *
+ * Supporting the ERRB interrupt would require dropping the regmap-IRQ
+ * usage or working around (or accepting a presense of) a naming conflict
+ * in debugFS IRQs.
+ *
+ * Being able to reliably do the configurations like changing the
+ * regulator safety limits (like limits for the over/under -voltages, over
+ * current, thermal protection) would require the configuring driver to be
+ * synchronized with entity causing the PMIC state transitions. Eg, one
+ * should be able to ensure the PMIC is in STBY state when the
+ * configurations are applied to the hardware. How and when the PMIC state
+ * transitions are to be done is likely to be very system specific, as will
+ * be the need to configure these safety limits. Hence it's not simple to
+ * come up with a generic solution.
+ *
+ * Users who require the ERRB handling and STBY state configurations can
+ * have a look at the original RFC:
+ * https://lore.kernel.org/all/cover.1712920132.git.mazziesaccount@gmail.com/
+ * which implements a workaround to debugFS naming conflict and some of
+ * the safety limit configurations - but leaves the state change handling
+ * and synchronization to be implemented.
+ *
+ * It would be great to hear (and receive a patch!) if you implement the
+ * STBY configuration support or a proper fix to the debugFS naming
+ * conflict in your downstream driver ;)
+ */
+
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/linear_range.h>
+#include <linux/mfd/rohm-generic.h>
+#include <linux/mfd/rohm-bd96801.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/regulator/coupler.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+#include <linux/regulator/of_regulator.h>
+#include <linux/slab.h>
+#include <linux/timer.h>
+
+enum {
+ BD96801_BUCK1,
+ BD96801_BUCK2,
+ BD96801_BUCK3,
+ BD96801_BUCK4,
+ BD96801_LDO5,
+ BD96801_LDO6,
+ BD96801_LDO7,
+ BD96801_REGULATOR_AMOUNT,
+};
+
+enum {
+ BD96801_PROT_OVP,
+ BD96801_PROT_UVP,
+ BD96801_PROT_OCP,
+ BD96801_PROT_TEMP,
+ BD96801_NUM_PROT,
+};
+
+#define BD96801_ALWAYS_ON_REG 0x3c
+#define BD96801_REG_ENABLE 0x0b
+#define BD96801_BUCK1_EN_MASK BIT(0)
+#define BD96801_BUCK2_EN_MASK BIT(1)
+#define BD96801_BUCK3_EN_MASK BIT(2)
+#define BD96801_BUCK4_EN_MASK BIT(3)
+#define BD96801_LDO5_EN_MASK BIT(4)
+#define BD96801_LDO6_EN_MASK BIT(5)
+#define BD96801_LDO7_EN_MASK BIT(6)
+
+#define BD96801_BUCK1_VSEL_REG 0x28
+#define BD96801_BUCK2_VSEL_REG 0x29
+#define BD96801_BUCK3_VSEL_REG 0x2a
+#define BD96801_BUCK4_VSEL_REG 0x2b
+#define BD96801_LDO5_VSEL_REG 0x25
+#define BD96801_LDO6_VSEL_REG 0x26
+#define BD96801_LDO7_VSEL_REG 0x27
+#define BD96801_BUCK_VSEL_MASK 0x1F
+#define BD96801_LDO_VSEL_MASK 0xff
+
+#define BD96801_MASK_RAMP_DELAY 0xc0
+#define BD96801_INT_VOUT_BASE_REG 0x21
+#define BD96801_BUCK_INT_VOUT_MASK 0xff
+
+#define BD96801_BUCK_VOLTS 256
+#define BD96801_LDO_VOLTS 256
+
+#define BD96801_OVP_MASK 0x03
+#define BD96801_MASK_BUCK1_OVP_SHIFT 0x00
+#define BD96801_MASK_BUCK2_OVP_SHIFT 0x02
+#define BD96801_MASK_BUCK3_OVP_SHIFT 0x04
+#define BD96801_MASK_BUCK4_OVP_SHIFT 0x06
+#define BD96801_MASK_LDO5_OVP_SHIFT 0x00
+#define BD96801_MASK_LDO6_OVP_SHIFT 0x02
+#define BD96801_MASK_LDO7_OVP_SHIFT 0x04
+
+#define BD96801_PROT_LIMIT_OCP_MIN 0x00
+#define BD96801_PROT_LIMIT_LOW 0x01
+#define BD96801_PROT_LIMIT_MID 0x02
+#define BD96801_PROT_LIMIT_HI 0x03
+
+#define BD96801_REG_BUCK1_OCP 0x32
+#define BD96801_REG_BUCK2_OCP 0x32
+#define BD96801_REG_BUCK3_OCP 0x33
+#define BD96801_REG_BUCK4_OCP 0x33
+
+#define BD96801_MASK_BUCK1_OCP_SHIFT 0x00
+#define BD96801_MASK_BUCK2_OCP_SHIFT 0x04
+#define BD96801_MASK_BUCK3_OCP_SHIFT 0x00
+#define BD96801_MASK_BUCK4_OCP_SHIFT 0x04
+
+#define BD96801_REG_LDO5_OCP 0x34
+#define BD96801_REG_LDO6_OCP 0x34
+#define BD96801_REG_LDO7_OCP 0x34
+
+#define BD96801_MASK_LDO5_OCP_SHIFT 0x00
+#define BD96801_MASK_LDO6_OCP_SHIFT 0x02
+#define BD96801_MASK_LDO7_OCP_SHIFT 0x04
+
+#define BD96801_MASK_SHD_INTB BIT(7)
+#define BD96801_INTB_FATAL BIT(7)
+
+#define BD96801_NUM_REGULATORS 7
+#define BD96801_NUM_LDOS 4
+
+/*
+ * Ramp rates for bucks are controlled by bits [7:6] as follows:
+ * 00 => 1 mV/uS
+ * 01 => 5 mV/uS
+ * 10 => 10 mV/uS
+ * 11 => 20 mV/uS
+ */
+static const unsigned int buck_ramp_table[] = { 1000, 5000, 10000, 20000 };
+
+/*
+ * This is a voltage range that get's appended to selected
+ * bd96801_buck_init_volts value. The range from 0x0 to 0xF is actually
+ * bd96801_buck_init_volts + 0 ... bd96801_buck_init_volts + 150mV
+ * and the range from 0x10 to 0x1f is bd96801_buck_init_volts - 150mV ...
+ * bd96801_buck_init_volts - 0. But as the members of linear_range
+ * are all unsigned I will apply offset of -150 mV to value in
+ * linear_range - which should increase these ranges with
+ * 150 mV getting all the values to >= 0.
+ */
+static const struct linear_range bd96801_tune_volts[] = {
+ REGULATOR_LINEAR_RANGE(150000, 0x00, 0xF, 10000),
+ REGULATOR_LINEAR_RANGE(0, 0x10, 0x1F, 10000),
+};
+
+static const struct linear_range bd96801_buck_init_volts[] = {
+ REGULATOR_LINEAR_RANGE(500000 - 150000, 0x00, 0xc8, 5000),
+ REGULATOR_LINEAR_RANGE(1550000 - 150000, 0xc9, 0xec, 50000),
+ REGULATOR_LINEAR_RANGE(3300000 - 150000, 0xed, 0xff, 0),
+};
+
+static const struct linear_range bd96801_ldo_int_volts[] = {
+ REGULATOR_LINEAR_RANGE(300000, 0x00, 0x78, 25000),
+ REGULATOR_LINEAR_RANGE(3300000, 0x79, 0xff, 0),
+};
+
+#define BD96801_LDO_SD_VOLT_MASK 0x1
+#define BD96801_LDO_MODE_MASK 0x6
+#define BD96801_LDO_MODE_INT 0x0
+#define BD96801_LDO_MODE_SD 0x2
+#define BD96801_LDO_MODE_DDR 0x4
+
+static int ldo_ddr_volt_table[] = {500000, 300000};
+static int ldo_sd_volt_table[] = {3300000, 1800000};
+
+/* Constant IRQ initialization data (templates) */
+struct bd96801_irqinfo {
+ int type;
+ struct regulator_irq_desc irq_desc;
+ int err_cfg;
+ int wrn_cfg;
+ const char *irq_name;
+};
+
+#define BD96801_IRQINFO(_type, _name, _irqoff_ms, _irqname) \
+{ \
+ .type = (_type), \
+ .err_cfg = -1, \
+ .wrn_cfg = -1, \
+ .irq_name = (_irqname), \
+ .irq_desc = { \
+ .name = (_name), \
+ .irq_off_ms = (_irqoff_ms), \
+ .map_event = regulator_irq_map_event_simple, \
+ }, \
+}
+
+static const struct bd96801_irqinfo buck1_irqinfo[] = {
+ BD96801_IRQINFO(BD96801_PROT_OCP, "buck1-over-curr-h", 500,
+ "bd96801-buck1-overcurr-h"),
+ BD96801_IRQINFO(BD96801_PROT_OCP, "buck1-over-curr-l", 500,
+ "bd96801-buck1-overcurr-l"),
+ BD96801_IRQINFO(BD96801_PROT_OCP, "buck1-over-curr-n", 500,
+ "bd96801-buck1-overcurr-n"),
+ BD96801_IRQINFO(BD96801_PROT_OVP, "buck1-over-voltage", 500,
+ "bd96801-buck1-overvolt"),
+ BD96801_IRQINFO(BD96801_PROT_UVP, "buck1-under-voltage", 500,
+ "bd96801-buck1-undervolt"),
+ BD96801_IRQINFO(BD96801_PROT_TEMP, "buck1-over-temp", 500,
+ "bd96801-buck1-thermal")
+};
+
+static const struct bd96801_irqinfo buck2_irqinfo[] = {
+ BD96801_IRQINFO(BD96801_PROT_OCP, "buck2-over-curr-h", 500,
+ "bd96801-buck2-overcurr-h"),
+ BD96801_IRQINFO(BD96801_PROT_OCP, "buck2-over-curr-l", 500,
+ "bd96801-buck2-overcurr-l"),
+ BD96801_IRQINFO(BD96801_PROT_OCP, "buck2-over-curr-n", 500,
+ "bd96801-buck2-overcurr-n"),
+ BD96801_IRQINFO(BD96801_PROT_OVP, "buck2-over-voltage", 500,
+ "bd96801-buck2-overvolt"),
+ BD96801_IRQINFO(BD96801_PROT_UVP, "buck2-under-voltage", 500,
+ "bd96801-buck2-undervolt"),
+ BD96801_IRQINFO(BD96801_PROT_TEMP, "buck2-over-temp", 500,
+ "bd96801-buck2-thermal")
+};
+
+static const struct bd96801_irqinfo buck3_irqinfo[] = {
+ BD96801_IRQINFO(BD96801_PROT_OCP, "buck3-over-curr-h", 500,
+ "bd96801-buck3-overcurr-h"),
+ BD96801_IRQINFO(BD96801_PROT_OCP, "buck3-over-curr-l", 500,
+ "bd96801-buck3-overcurr-l"),
+ BD96801_IRQINFO(BD96801_PROT_OCP, "buck3-over-curr-n", 500,
+ "bd96801-buck3-overcurr-n"),
+ BD96801_IRQINFO(BD96801_PROT_OVP, "buck3-over-voltage", 500,
+ "bd96801-buck3-overvolt"),
+ BD96801_IRQINFO(BD96801_PROT_UVP, "buck3-under-voltage", 500,
+ "bd96801-buck3-undervolt"),
+ BD96801_IRQINFO(BD96801_PROT_TEMP, "buck3-over-temp", 500,
+ "bd96801-buck3-thermal")
+};
+
+static const struct bd96801_irqinfo buck4_irqinfo[] = {
+ BD96801_IRQINFO(BD96801_PROT_OCP, "buck4-over-curr-h", 500,
+ "bd96801-buck4-overcurr-h"),
+ BD96801_IRQINFO(BD96801_PROT_OCP, "buck4-over-curr-l", 500,
+ "bd96801-buck4-overcurr-l"),
+ BD96801_IRQINFO(BD96801_PROT_OCP, "buck4-over-curr-n", 500,
+ "bd96801-buck4-overcurr-n"),
+ BD96801_IRQINFO(BD96801_PROT_OVP, "buck4-over-voltage", 500,
+ "bd96801-buck4-overvolt"),
+ BD96801_IRQINFO(BD96801_PROT_UVP, "buck4-under-voltage", 500,
+ "bd96801-buck4-undervolt"),
+ BD96801_IRQINFO(BD96801_PROT_TEMP, "buck4-over-temp", 500,
+ "bd96801-buck4-thermal")
+};
+
+static const struct bd96801_irqinfo ldo5_irqinfo[] = {
+ BD96801_IRQINFO(BD96801_PROT_OCP, "ldo5-overcurr", 500,
+ "bd96801-ldo5-overcurr"),
+ BD96801_IRQINFO(BD96801_PROT_OVP, "ldo5-over-voltage", 500,
+ "bd96801-ldo5-overvolt"),
+ BD96801_IRQINFO(BD96801_PROT_UVP, "ldo5-under-voltage", 500,
+ "bd96801-ldo5-undervolt"),
+};
+
+static const struct bd96801_irqinfo ldo6_irqinfo[] = {
+ BD96801_IRQINFO(BD96801_PROT_OCP, "ldo6-overcurr", 500,
+ "bd96801-ldo6-overcurr"),
+ BD96801_IRQINFO(BD96801_PROT_OVP, "ldo6-over-voltage", 500,
+ "bd96801-ldo6-overvolt"),
+ BD96801_IRQINFO(BD96801_PROT_UVP, "ldo6-under-voltage", 500,
+ "bd96801-ldo6-undervolt"),
+};
+
+static const struct bd96801_irqinfo ldo7_irqinfo[] = {
+ BD96801_IRQINFO(BD96801_PROT_OCP, "ldo7-overcurr", 500,
+ "bd96801-ldo7-overcurr"),
+ BD96801_IRQINFO(BD96801_PROT_OVP, "ldo7-over-voltage", 500,
+ "bd96801-ldo7-overvolt"),
+ BD96801_IRQINFO(BD96801_PROT_UVP, "ldo7-under-voltage", 500,
+ "bd96801-ldo7-undervolt"),
+};
+
+struct bd96801_irq_desc {
+ struct bd96801_irqinfo *irqinfo;
+ int num_irqs;
+};
+
+struct bd96801_regulator_data {
+ struct regulator_desc desc;
+ const struct linear_range *init_ranges;
+ int num_ranges;
+ struct bd96801_irq_desc irq_desc;
+ int initial_voltage;
+ int ldo_vol_lvl;
+ int ldo_errs;
+};
+
+struct bd96801_pmic_data {
+ struct bd96801_regulator_data regulator_data[BD96801_NUM_REGULATORS];
+ struct regmap *regmap;
+ int fatal_ind;
+};
+
+static int ldo_map_notif(int irq, struct regulator_irq_data *rid,
+ unsigned long *dev_mask)
+{
+ int i;
+
+ for (i = 0; i < rid->num_states; i++) {
+ struct bd96801_regulator_data *rdata;
+ struct regulator_dev *rdev;
+
+ rdev = rid->states[i].rdev;
+ rdata = container_of(rdev->desc, struct bd96801_regulator_data,
+ desc);
+ rid->states[i].notifs = regulator_err2notif(rdata->ldo_errs);
+ rid->states[i].errors = rdata->ldo_errs;
+ *dev_mask |= BIT(i);
+ }
+ return 0;
+}
+
+static int bd96801_list_voltage_lr(struct regulator_dev *rdev,
+ unsigned int selector)
+{
+ int voltage;
+ struct bd96801_regulator_data *data;
+
+ data = container_of(rdev->desc, struct bd96801_regulator_data, desc);
+
+ /*
+ * The BD096801 has voltage setting in two registers. One giving the
+ * "initial voltage" (can be changed only when regulator is disabled.
+ * This driver caches the value and sets it only at startup. The other
+ * register is voltage tuning value which applies -150 mV ... +150 mV
+ * offset to the voltage.
+ *
+ * Note that the cached initial voltage stored in regulator data is
+ * 'scaled down' by the 150 mV so that all of our tuning values are
+ * >= 0. This is done because the linear_ranges uses unsigned values.
+ *
+ * As a result, we increase the tuning voltage which we get based on
+ * the selector by the stored initial_voltage.
+ */
+ voltage = regulator_list_voltage_linear_range(rdev, selector);
+ if (voltage < 0)
+ return voltage;
+
+ return voltage + data->initial_voltage;
+}
+
+
+static const struct regulator_ops bd96801_ldo_table_ops = {
+ .is_enabled = regulator_is_enabled_regmap,
+ .list_voltage = regulator_list_voltage_table,
+ .get_voltage_sel = regulator_get_voltage_sel_regmap,
+};
+
+static const struct regulator_ops bd96801_buck_ops = {
+ .is_enabled = regulator_is_enabled_regmap,
+ .list_voltage = bd96801_list_voltage_lr,
+ .set_voltage_sel = regulator_set_voltage_sel_regmap,
+ .get_voltage_sel = regulator_get_voltage_sel_regmap,
+ .set_voltage_time_sel = regulator_set_voltage_time_sel,
+ .set_ramp_delay = regulator_set_ramp_delay_regmap,
+};
+
+static const struct regulator_ops bd96801_ldo_ops = {
+ .is_enabled = regulator_is_enabled_regmap,
+ .list_voltage = regulator_list_voltage_linear_range,
+ .get_voltage_sel = regulator_get_voltage_sel_regmap,
+};
+
+static int buck_get_initial_voltage(struct regmap *regmap, struct device *dev,
+ struct bd96801_regulator_data *data)
+{
+ int ret = 0, sel, initial_uv;
+ int reg = BD96801_INT_VOUT_BASE_REG + data->desc.id;
+
+ if (data->num_ranges) {
+ ret = regmap_read(regmap, reg, &sel);
+ sel &= BD96801_BUCK_INT_VOUT_MASK;
+
+ ret = linear_range_get_value_array(data->init_ranges,
+ data->num_ranges, sel,
+ &initial_uv);
+ if (ret)
+ return ret;
+
+ data->initial_voltage = initial_uv;
+ dev_dbg(dev, "Tune-scaled initial voltage %u\n",
+ data->initial_voltage);
+ }
+
+ return 0;
+}
+
+static int get_ldo_initial_voltage(struct regmap *regmap,
+ struct device *dev,
+ struct bd96801_regulator_data *data)
+{
+ int ret;
+ int cfgreg;
+
+ ret = regmap_read(regmap, data->ldo_vol_lvl, &cfgreg);
+ if (ret)
+ return ret;
+
+ switch (cfgreg & BD96801_LDO_MODE_MASK) {
+ case BD96801_LDO_MODE_DDR:
+ data->desc.volt_table = ldo_ddr_volt_table;
+ data->desc.n_voltages = ARRAY_SIZE(ldo_ddr_volt_table);
+ break;
+ case BD96801_LDO_MODE_SD:
+ data->desc.volt_table = ldo_sd_volt_table;
+ data->desc.n_voltages = ARRAY_SIZE(ldo_sd_volt_table);
+ break;
+ default:
+ dev_info(dev, "Leaving LDO to normal mode");
+ return 0;
+ }
+
+ /* SD or DDR mode => override default ops */
+ data->desc.ops = &bd96801_ldo_table_ops,
+ data->desc.vsel_mask = 1;
+ data->desc.vsel_reg = data->ldo_vol_lvl;
+
+ return 0;
+}
+
+static int get_initial_voltage(struct device *dev, struct regmap *regmap,
+ struct bd96801_regulator_data *data)
+{
+ /* BUCK */
+ if (data->desc.id <= BD96801_BUCK4)
+ return buck_get_initial_voltage(regmap, dev, data);
+
+ /* LDO */
+ return get_ldo_initial_voltage(regmap, dev, data);
+}
+
+static int bd96801_walk_regulator_dt(struct device *dev, struct regmap *regmap,
+ struct bd96801_regulator_data *data,
+ int num)
+{
+ int i, ret;
+ struct device_node *np;
+ struct device_node *nproot = dev->parent->of_node;
+
+ nproot = of_get_child_by_name(nproot, "regulators");
+ if (!nproot) {
+ dev_err(dev, "failed to find regulators node\n");
+ return -ENODEV;
+ }
+ for_each_child_of_node(nproot, np)
+ for (i = 0; i < num; i++) {
+ if (!of_node_name_eq(np, data[i].desc.of_match))
+ continue;
+ /*
+ * If STBY configs are supported, we must pass node
+ * here to extract the initial voltages from the DT.
+ * Thus we do the initial voltage getting in this
+ * loop.
+ */
+ ret = get_initial_voltage(dev, regmap, &data[i]);
+ if (ret) {
+ dev_err(dev,
+ "Initializing voltages for %s failed\n",
+ data[i].desc.name);
+ of_node_put(np);
+ of_node_put(nproot);
+
+ return ret;
+ }
+ if (of_property_read_bool(np, "rohm,keep-on-stby")) {
+ ret = regmap_set_bits(regmap,
+ BD96801_ALWAYS_ON_REG,
+ 1 << data[i].desc.id);
+ if (ret) {
+ dev_err(dev,
+ "failed to set %s on-at-stby\n",
+ data[i].desc.name);
+ of_node_put(np);
+ of_node_put(nproot);
+
+ return ret;
+ }
+ }
+ }
+ of_node_put(nproot);
+
+ return 0;
+}
+
+/*
+ * Template for regulator data. Probe will allocate dynamic / driver instance
+ * struct so we should be on a safe side even if there were multiple PMICs to
+ * control. Note that there is a plan to allow multiple PMICs to be used so
+ * systems can scale better. I am however still slightly unsure how the
+ * multi-PMIC case will be handled. I don't know if the processor will have I2C
+ * acces to all of the PMICs or only the first one. I'd guess there will be
+ * access provided to all PMICs for voltage scaling - but the errors will only
+ * be informed via the master PMIC. Eg, we should prepare to support multiple
+ * driver instances - either with or without the IRQs... Well, let's first
+ * just support the simple and clear single-PMIC setup and ponder the multi PMIC
+ * case later. What we can easly do for preparing is to not use static global
+ * data for regulators though.
+ */
+static const struct bd96801_pmic_data bd96801_data = {
+ .regulator_data = {
+ {
+ .desc = {
+ .name = "buck1",
+ .of_match = of_match_ptr("BUCK1"),
+ .regulators_node = of_match_ptr("regulators"),
+ .id = BD96801_BUCK1,
+ .ops = &bd96801_buck_ops,
+ .type = REGULATOR_VOLTAGE,
+ .linear_ranges = bd96801_tune_volts,
+ .n_linear_ranges = ARRAY_SIZE(bd96801_tune_volts),
+ .n_voltages = BD96801_BUCK_VOLTS,
+ .enable_reg = BD96801_REG_ENABLE,
+ .enable_mask = BD96801_BUCK1_EN_MASK,
+ .enable_is_inverted = true,
+ .vsel_reg = BD96801_BUCK1_VSEL_REG,
+ .vsel_mask = BD96801_BUCK_VSEL_MASK,
+ .ramp_reg = BD96801_BUCK1_VSEL_REG,
+ .ramp_mask = BD96801_MASK_RAMP_DELAY,
+ .ramp_delay_table = &buck_ramp_table[0],
+ .n_ramp_values = ARRAY_SIZE(buck_ramp_table),
+ .owner = THIS_MODULE,
+ },
+ .init_ranges = bd96801_buck_init_volts,
+ .num_ranges = ARRAY_SIZE(bd96801_buck_init_volts),
+ .irq_desc = {
+ .irqinfo = (struct bd96801_irqinfo *)&buck1_irqinfo[0],
+ .num_irqs = ARRAY_SIZE(buck1_irqinfo),
+ },
+ }, {
+ .desc = {
+ .name = "buck2",
+ .of_match = of_match_ptr("BUCK2"),
+ .regulators_node = of_match_ptr("regulators"),
+ .id = BD96801_BUCK2,
+ .ops = &bd96801_buck_ops,
+ .type = REGULATOR_VOLTAGE,
+ .linear_ranges = bd96801_tune_volts,
+ .n_linear_ranges = ARRAY_SIZE(bd96801_tune_volts),
+ .n_voltages = BD96801_BUCK_VOLTS,
+ .enable_reg = BD96801_REG_ENABLE,
+ .enable_mask = BD96801_BUCK2_EN_MASK,
+ .enable_is_inverted = true,
+ .vsel_reg = BD96801_BUCK2_VSEL_REG,
+ .vsel_mask = BD96801_BUCK_VSEL_MASK,
+ .ramp_reg = BD96801_BUCK2_VSEL_REG,
+ .ramp_mask = BD96801_MASK_RAMP_DELAY,
+ .ramp_delay_table = &buck_ramp_table[0],
+ .n_ramp_values = ARRAY_SIZE(buck_ramp_table),
+ .owner = THIS_MODULE,
+ },
+ .irq_desc = {
+ .irqinfo = (struct bd96801_irqinfo *)&buck2_irqinfo[0],
+ .num_irqs = ARRAY_SIZE(buck2_irqinfo),
+ },
+ .init_ranges = bd96801_buck_init_volts,
+ .num_ranges = ARRAY_SIZE(bd96801_buck_init_volts),
+ }, {
+ .desc = {
+ .name = "buck3",
+ .of_match = of_match_ptr("BUCK3"),
+ .regulators_node = of_match_ptr("regulators"),
+ .id = BD96801_BUCK3,
+ .ops = &bd96801_buck_ops,
+ .type = REGULATOR_VOLTAGE,
+ .linear_ranges = bd96801_tune_volts,
+ .n_linear_ranges = ARRAY_SIZE(bd96801_tune_volts),
+ .n_voltages = BD96801_BUCK_VOLTS,
+ .enable_reg = BD96801_REG_ENABLE,
+ .enable_mask = BD96801_BUCK3_EN_MASK,
+ .enable_is_inverted = true,
+ .vsel_reg = BD96801_BUCK3_VSEL_REG,
+ .vsel_mask = BD96801_BUCK_VSEL_MASK,
+ .ramp_reg = BD96801_BUCK3_VSEL_REG,
+ .ramp_mask = BD96801_MASK_RAMP_DELAY,
+ .ramp_delay_table = &buck_ramp_table[0],
+ .n_ramp_values = ARRAY_SIZE(buck_ramp_table),
+ .owner = THIS_MODULE,
+ },
+ .irq_desc = {
+ .irqinfo = (struct bd96801_irqinfo *)&buck3_irqinfo[0],
+ .num_irqs = ARRAY_SIZE(buck3_irqinfo),
+ },
+ .init_ranges = bd96801_buck_init_volts,
+ .num_ranges = ARRAY_SIZE(bd96801_buck_init_volts),
+ }, {
+ .desc = {
+ .name = "buck4",
+ .of_match = of_match_ptr("BUCK4"),
+ .regulators_node = of_match_ptr("regulators"),
+ .id = BD96801_BUCK4,
+ .ops = &bd96801_buck_ops,
+ .type = REGULATOR_VOLTAGE,
+ .linear_ranges = bd96801_tune_volts,
+ .n_linear_ranges = ARRAY_SIZE(bd96801_tune_volts),
+ .n_voltages = BD96801_BUCK_VOLTS,
+ .enable_reg = BD96801_REG_ENABLE,
+ .enable_mask = BD96801_BUCK4_EN_MASK,
+ .enable_is_inverted = true,
+ .vsel_reg = BD96801_BUCK4_VSEL_REG,
+ .vsel_mask = BD96801_BUCK_VSEL_MASK,
+ .ramp_reg = BD96801_BUCK4_VSEL_REG,
+ .ramp_mask = BD96801_MASK_RAMP_DELAY,
+ .ramp_delay_table = &buck_ramp_table[0],
+ .n_ramp_values = ARRAY_SIZE(buck_ramp_table),
+ .owner = THIS_MODULE,
+ },
+ .irq_desc = {
+ .irqinfo = (struct bd96801_irqinfo *)&buck4_irqinfo[0],
+ .num_irqs = ARRAY_SIZE(buck4_irqinfo),
+ },
+ .init_ranges = bd96801_buck_init_volts,
+ .num_ranges = ARRAY_SIZE(bd96801_buck_init_volts),
+ }, {
+ .desc = {
+ .name = "ldo5",
+ .of_match = of_match_ptr("LDO5"),
+ .regulators_node = of_match_ptr("regulators"),
+ .id = BD96801_LDO5,
+ .ops = &bd96801_ldo_ops,
+ .type = REGULATOR_VOLTAGE,
+ .linear_ranges = bd96801_ldo_int_volts,
+ .n_linear_ranges = ARRAY_SIZE(bd96801_ldo_int_volts),
+ .n_voltages = BD96801_LDO_VOLTS,
+ .enable_reg = BD96801_REG_ENABLE,
+ .enable_mask = BD96801_LDO5_EN_MASK,
+ .enable_is_inverted = true,
+ .vsel_reg = BD96801_LDO5_VSEL_REG,
+ .vsel_mask = BD96801_LDO_VSEL_MASK,
+ .owner = THIS_MODULE,
+ },
+ .irq_desc = {
+ .irqinfo = (struct bd96801_irqinfo *)&ldo5_irqinfo[0],
+ .num_irqs = ARRAY_SIZE(ldo5_irqinfo),
+ },
+ .ldo_vol_lvl = BD96801_LDO5_VOL_LVL_REG,
+ }, {
+ .desc = {
+ .name = "ldo6",
+ .of_match = of_match_ptr("LDO6"),
+ .regulators_node = of_match_ptr("regulators"),
+ .id = BD96801_LDO6,
+ .ops = &bd96801_ldo_ops,
+ .type = REGULATOR_VOLTAGE,
+ .linear_ranges = bd96801_ldo_int_volts,
+ .n_linear_ranges = ARRAY_SIZE(bd96801_ldo_int_volts),
+ .n_voltages = BD96801_LDO_VOLTS,
+ .enable_reg = BD96801_REG_ENABLE,
+ .enable_mask = BD96801_LDO6_EN_MASK,
+ .enable_is_inverted = true,
+ .vsel_reg = BD96801_LDO6_VSEL_REG,
+ .vsel_mask = BD96801_LDO_VSEL_MASK,
+ .owner = THIS_MODULE,
+ },
+ .irq_desc = {
+ .irqinfo = (struct bd96801_irqinfo *)&ldo6_irqinfo[0],
+ .num_irqs = ARRAY_SIZE(ldo6_irqinfo),
+ },
+ .ldo_vol_lvl = BD96801_LDO6_VOL_LVL_REG,
+ }, {
+ .desc = {
+ .name = "ldo7",
+ .of_match = of_match_ptr("LDO7"),
+ .regulators_node = of_match_ptr("regulators"),
+ .id = BD96801_LDO7,
+ .ops = &bd96801_ldo_ops,
+ .type = REGULATOR_VOLTAGE,
+ .linear_ranges = bd96801_ldo_int_volts,
+ .n_linear_ranges = ARRAY_SIZE(bd96801_ldo_int_volts),
+ .n_voltages = BD96801_LDO_VOLTS,
+ .enable_reg = BD96801_REG_ENABLE,
+ .enable_mask = BD96801_LDO7_EN_MASK,
+ .enable_is_inverted = true,
+ .vsel_reg = BD96801_LDO7_VSEL_REG,
+ .vsel_mask = BD96801_LDO_VSEL_MASK,
+ .owner = THIS_MODULE,
+ },
+ .irq_desc = {
+ .irqinfo = (struct bd96801_irqinfo *)&ldo7_irqinfo[0],
+ .num_irqs = ARRAY_SIZE(ldo7_irqinfo),
+ },
+ .ldo_vol_lvl = BD96801_LDO7_VOL_LVL_REG,
+ },
+ },
+};
+
+static int initialize_pmic_data(struct device *dev,
+ struct bd96801_pmic_data *pdata)
+{
+ int r, i;
+
+ /*
+ * Allocate and initialize IRQ data for all of the regulators. We
+ * wish to modify IRQ information independently for each driver
+ * instance.
+ */
+ for (r = 0; r < BD96801_NUM_REGULATORS; r++) {
+ const struct bd96801_irqinfo *template;
+ struct bd96801_irqinfo *new;
+ int num_infos;
+
+ template = pdata->regulator_data[r].irq_desc.irqinfo;
+ num_infos = pdata->regulator_data[r].irq_desc.num_irqs;
+
+ new = devm_kcalloc(dev, num_infos, sizeof(*new), GFP_KERNEL);
+ if (!new)
+ return -ENOMEM;
+
+ pdata->regulator_data[r].irq_desc.irqinfo = new;
+
+ for (i = 0; i < num_infos; i++)
+ new[i] = template[i];
+ }
+
+ return 0;
+}
+
+static int bd96801_probe(struct platform_device *pdev)
+{
+ struct device *parent;
+ int i, ret, irq;
+ void *retp;
+ struct regulator_config config = {};
+ struct bd96801_regulator_data *rdesc;
+ struct bd96801_pmic_data *pdata;
+ struct regulator_dev *ldo_errs_rdev_arr[BD96801_NUM_LDOS];
+ int ldo_errs_arr[BD96801_NUM_LDOS];
+ int temp_notif_ldos = 0;
+
+ parent = pdev->dev.parent;
+
+ pdata = devm_kmemdup(&pdev->dev, &bd96801_data, sizeof(bd96801_data),
+ GFP_KERNEL);
+ if (!pdata)
+ return -ENOMEM;
+
+ if (initialize_pmic_data(&pdev->dev, pdata))
+ return -ENOMEM;
+
+ pdata->regmap = dev_get_regmap(parent, NULL);
+ if (!pdata->regmap) {
+ dev_err(&pdev->dev, "No register map found\n");
+ return -ENODEV;
+ }
+
+ rdesc = &pdata->regulator_data[0];
+
+ config.driver_data = pdata;
+ config.regmap = pdata->regmap;
+ config.dev = parent;
+
+ ret = bd96801_walk_regulator_dt(&pdev->dev, pdata->regmap, rdesc,
+ BD96801_NUM_REGULATORS);
+ if (ret)
+ return ret;
+
+ for (i = 0; i < ARRAY_SIZE(pdata->regulator_data); i++) {
+ struct regulator_dev *rdev;
+ struct regulator_dev *rdev_arr[1];
+ struct bd96801_irq_desc *idesc = &rdesc[i].irq_desc;
+ int j;
+
+ rdev = devm_regulator_register(&pdev->dev,
+ &rdesc[i].desc, &config);
+ if (IS_ERR(rdev)) {
+ dev_err(&pdev->dev,
+ "failed to register %s regulator\n",
+ rdesc[i].desc.name);
+ return PTR_ERR(rdev);
+ }
+ /*
+ * LDOs don't have own temperature monitoring. If temperature
+ * notification was requested for this LDO from DT then we will
+ * add the regulator to be notified if central IC temperature
+ * exceeds threshold.
+ */
+ if (rdesc[i].ldo_errs) {
+ ldo_errs_rdev_arr[temp_notif_ldos] = rdev;
+ ldo_errs_arr[temp_notif_ldos] = rdesc[i].ldo_errs;
+ temp_notif_ldos++;
+ }
+ if (!idesc)
+ continue;
+ /*
+ * TODO: Can we split adding the INTB notifiers in own
+ * function ?
+ */
+ /* Register INTB handlers for configured protections */
+ for (j = 0; j < idesc->num_irqs; j++) {
+ struct bd96801_irqinfo *iinfo;
+ int err = 0;
+ int err_flags[] = {
+ [BD96801_PROT_OVP] = REGULATOR_ERROR_REGULATION_OUT,
+ [BD96801_PROT_UVP] = REGULATOR_ERROR_UNDER_VOLTAGE,
+ [BD96801_PROT_OCP] = REGULATOR_ERROR_OVER_CURRENT,
+ [BD96801_PROT_TEMP] = REGULATOR_ERROR_OVER_TEMP,
+
+ };
+ int wrn_flags[] = {
+ [BD96801_PROT_OVP] = REGULATOR_ERROR_OVER_VOLTAGE_WARN,
+ [BD96801_PROT_UVP] = REGULATOR_ERROR_UNDER_VOLTAGE_WARN,
+ [BD96801_PROT_OCP] = REGULATOR_ERROR_OVER_CURRENT_WARN,
+ [BD96801_PROT_TEMP] = REGULATOR_ERROR_OVER_TEMP_WARN,
+ };
+
+ iinfo = &idesc->irqinfo[j];
+ /*
+ * Don't install IRQ handler if both error and warning
+ * notifications are explicitly disabled
+ */
+ if (!iinfo->err_cfg && !iinfo->wrn_cfg)
+ continue;
+
+ if (WARN_ON(iinfo->type >= BD96801_NUM_PROT))
+ return -EINVAL;
+
+ if (iinfo->err_cfg)
+ err = err_flags[iinfo->type];
+ else if (iinfo->wrn_cfg)
+ err = wrn_flags[iinfo->type];
+
+ iinfo->irq_desc.data = pdata;
+ irq = platform_get_irq_byname(pdev, iinfo->irq_name);
+ if (irq < 0)
+ return irq;
+ /* Find notifications for this IRQ (WARN/ERR) */
+
+ rdev_arr[0] = rdev;
+ retp = devm_regulator_irq_helper(&pdev->dev,
+ &iinfo->irq_desc, irq,
+ 0, err, NULL, rdev_arr,
+ 1);
+ if (IS_ERR(retp))
+ return PTR_ERR(retp);
+ }
+ }
+ if (temp_notif_ldos) {
+ int irq;
+ struct regulator_irq_desc tw_desc = {
+ .name = "bd96801-core-thermal",
+ .irq_off_ms = 500,
+ .map_event = ldo_map_notif,
+ };
+
+ irq = platform_get_irq_byname(pdev, "bd96801-core-thermal");
+ if (irq < 0)
+ return irq;
+
+ retp = devm_regulator_irq_helper(&pdev->dev, &tw_desc, irq, 0,
+ 0, &ldo_errs_arr[0],
+ &ldo_errs_rdev_arr[0],
+ temp_notif_ldos);
+ if (IS_ERR(retp))
+ return PTR_ERR(retp);
+ }
+
+ return 0;
+}
+
+static const struct platform_device_id bd96801_pmic_id[] = {
+ { "bd96801-pmic", },
+ { }
+};
+MODULE_DEVICE_TABLE(platform, bd96801_pmic_id);
+
+static struct platform_driver bd96801_regulator = {
+ .driver = {
+ .name = "bd96801-pmic"
+ },
+ .probe = bd96801_probe,
+ .id_table = bd96801_pmic_id,
+};
+
+module_platform_driver(bd96801_regulator);
+
+MODULE_AUTHOR("Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>");
+MODULE_DESCRIPTION("BD96801 voltage regulator driver");
+MODULE_LICENSE("GPL");
--
2.44.0
--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND
~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =]
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v1 5/6] watchdog: ROHM BD96801 PMIC WDG driver
2024-04-30 11:59 [PATCH v1 0/6] Support ROHM BD96801 scalable PMIC Matti Vaittinen
` (3 preceding siblings ...)
2024-04-30 12:01 ` [PATCH v1 4/6] regulator: bd96801: ROHM BD96801 PMIC regulators Matti Vaittinen
@ 2024-04-30 12:01 ` Matti Vaittinen
2024-04-30 16:24 ` Guenter Roeck
2024-04-30 12:02 ` [PATCH v1 6/6] MAINTAINERS: Add ROHM BD96801 'scalable PMIC' entries Matti Vaittinen
5 siblings, 1 reply; 14+ messages in thread
From: Matti Vaittinen @ 2024-04-30 12:01 UTC (permalink / raw)
To: Matti Vaittinen, Matti Vaittinen
Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Liam Girdwood, Mark Brown, Matti Vaittinen, Wim Van Sebroeck,
Guenter Roeck, devicetree, linux-kernel, linux-watchdog
[-- Attachment #1: Type: text/plain, Size: 13551 bytes --]
Introduce driver for WDG block on ROHM BD96801 scalable PMIC.
This driver only supports watchdog with I2C feeding and delayed
response detection. Whether the watchdog toggles PRSTB pin or
just causes an interrupt can be configured via device-tree.
The BD96801 PMIC HW supports also window watchdog (too early
feeding detection) and Q&A mode. These are not supported by
this driver.
Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
---
Revision history:
RFCv2 => v1:
- Fix watchdog time-outs to match DS4
- Fix target timeout overflow
- Improve dbg prints
RFCv1 => RFCv2:
- remove always running
- add IRQ handling
- call emergency_restart()
- drop MODULE_ALIAS and add MODULE_DEVICE_TABLE
---
drivers/watchdog/Kconfig | 13 ++
drivers/watchdog/Makefile | 1 +
drivers/watchdog/bd96801_wdt.c | 400 +++++++++++++++++++++++++++++++++
3 files changed, 414 insertions(+)
create mode 100644 drivers/watchdog/bd96801_wdt.c
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 6bee137cfbe0..d97e735e1faa 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -181,6 +181,19 @@ config BD957XMUF_WATCHDOG
watchdog. Alternatively say M to compile the driver as a module,
which will be called bd9576_wdt.
+config BD96801_WATCHDOG
+ tristate "ROHM BD96801 PMIC Watchdog"
+ depends on MFD_ROHM_BD96801
+ select WATCHDOG_CORE
+ help
+ Support for the watchdog in the ROHM BD96801 PMIC. Watchdog can be
+ configured to only generate IRQ or to trigger system reset via reset
+ pin.
+
+ Say Y here to include support for the ROHM BD96801 watchdog.
+ Alternatively say M to compile the driver as a module,
+ which will be called bd96801_wdt.
+
config CROS_EC_WATCHDOG
tristate "ChromeOS EC-based watchdog"
select WATCHDOG_CORE
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 3710c218f05e..31bc94436c81 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -217,6 +217,7 @@ obj-$(CONFIG_XEN_WDT) += xen_wdt.o
# Architecture Independent
obj-$(CONFIG_BD957XMUF_WATCHDOG) += bd9576_wdt.o
+obj-$(CONFIG_BD96801_WATCHDOG) += bd96801_wdt.o
obj-$(CONFIG_CROS_EC_WATCHDOG) += cros_ec_wdt.o
obj-$(CONFIG_DA9052_WATCHDOG) += da9052_wdt.o
obj-$(CONFIG_DA9055_WATCHDOG) += da9055_wdt.o
diff --git a/drivers/watchdog/bd96801_wdt.c b/drivers/watchdog/bd96801_wdt.c
new file mode 100644
index 000000000000..6069f1d75356
--- /dev/null
+++ b/drivers/watchdog/bd96801_wdt.c
@@ -0,0 +1,400 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2024 ROHM Semiconductors
+ *
+ * ROHM BD96801 watchdog driver
+ */
+
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/mfd/rohm-bd96801.h>
+#include <linux/mfd/rohm-generic.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 BD96801_WD_TMO_SHORT_MASK 0x70
+#define BD96801_WD_RATIO_MASK 0x3
+#define BD96801_WD_TYPE_MASK 0x4
+#define BD96801_WD_TYPE_SLOW 0x4
+#define BD96801_WD_TYPE_WIN 0x0
+
+#define BD96801_WD_EN_MASK 0x3
+#define BD96801_WD_IF_EN 0x1
+#define BD96801_WD_QA_EN 0x2
+#define BD96801_WD_DISABLE 0x0
+
+#define BD96801_WD_ASSERT_MASK 0x8
+#define BD96801_WD_ASSERT_RST 0x8
+#define BD96801_WD_ASSERT_IRQ 0x0
+
+#define BD96801_WD_FEED_MASK 0x1
+#define BD96801_WD_FEED 0x1
+
+/* units in uS */
+#define FASTNG_MIN 11
+
+#define BD96801_WDT_DEFAULT_MARGIN_MS 1843
+/* Unit is seconds */
+#define DEFAULT_TIMEOUT 30
+
+/*
+ * BD96801 WDG supports window mode so the TMO consists of SHORT and LONG
+ * timeout values. SHORT time is meaningfull only in window mode where feeding
+ * period shorter than SHORT would be an error. LONG time is used to detect if
+ * feeding is not occurring within given time limit (SoC SW hangs). The LONG
+ * timeout time is a multiple of (2, 4, 8 0r 16 times) the SHORT timeout.
+ */
+
+struct wdtbd96801 {
+ struct device *dev;
+ struct regmap *regmap;
+ struct watchdog_device wdt;
+};
+
+static int bd96801_wdt_ping(struct watchdog_device *wdt)
+{
+ struct wdtbd96801 *w = watchdog_get_drvdata(wdt);
+
+ return regmap_update_bits(w->regmap, BD96801_REG_WD_FEED,
+ BD96801_WD_FEED_MASK, BD96801_WD_FEED);
+}
+
+static int bd96801_wdt_start(struct watchdog_device *wdt)
+{
+ struct wdtbd96801 *w = watchdog_get_drvdata(wdt);
+
+ return regmap_update_bits(w->regmap, BD96801_REG_WD_CONF,
+ BD96801_WD_EN_MASK, BD96801_WD_IF_EN);
+}
+
+static int bd96801_wdt_stop(struct watchdog_device *wdt)
+{
+ struct wdtbd96801 *w = watchdog_get_drvdata(wdt);
+
+ return regmap_update_bits(w->regmap, BD96801_REG_WD_CONF,
+ BD96801_WD_EN_MASK, BD96801_WD_DISABLE);
+}
+
+static const struct watchdog_info bd96801_wdt_info = {
+ .options = WDIOF_MAGICCLOSE | WDIOF_KEEPALIVEPING |
+ WDIOF_SETTIMEOUT,
+ .identity = "BD96801 Watchdog",
+};
+
+static const struct watchdog_ops bd96801_wdt_ops = {
+ .start = bd96801_wdt_start,
+ .stop = bd96801_wdt_stop,
+ .ping = bd96801_wdt_ping,
+};
+
+static int find_closest_fast(unsigned int target, int *sel, unsigned int *val)
+{
+ unsigned int window = FASTNG_MIN;
+ int i;
+
+ 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(unsigned int fast_val, unsigned int *target, int *slowsel)
+{
+ static const int multipliers[] = {2, 4, 8, 16};
+ int sel;
+
+ for (sel = 0; sel < ARRAY_SIZE(multipliers) &&
+ multipliers[sel] * fast_val < *target; sel++)
+ ;
+
+ if (sel == ARRAY_SIZE(multipliers))
+ return -EINVAL;
+
+ *slowsel = sel;
+ *target = multipliers[sel] * fast_val;
+
+ return 0;
+}
+
+static int find_closest_slow(unsigned int *target, int *slow_sel, int *fast_sel)
+{
+ static const int multipliers[] = {2, 4, 8, 16};
+ unsigned int window = FASTNG_MIN;
+ unsigned int val = 0;
+ int i, j;
+
+ for (i = 0; i < 8; i++) {
+ for (j = 0; j < ARRAY_SIZE(multipliers); j++) {
+ unsigned 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;
+
+ *target = val;
+
+ return 0;
+}
+
+static int bd96801_set_wdt_mode(struct wdtbd96801 *w, unsigned int hw_margin,
+ unsigned int hw_margin_min)
+{
+ int fastng, slowng, type, ret, reg, mask;
+ struct device *dev = w->dev;
+
+ /*
+ * Convert to 100uS to guarantee reasonable timeouts fit in
+ * 32bit maintaining also a decent accuracy.
+ */
+ hw_margin *= 10;
+ hw_margin_min *= 10;
+
+ if (hw_margin_min) {
+ unsigned int min;
+
+ type = BD96801_WD_TYPE_WIN;
+ 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;
+ }
+ w->wdt.min_hw_heartbeat_ms = min / 10;
+ } else {
+ type = BD96801_WD_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;
+ }
+ }
+
+ w->wdt.max_hw_heartbeat_ms = hw_margin / 10;
+
+ fastng <<= ffs(BD96801_WD_TMO_SHORT_MASK) - 1;
+
+ reg = slowng | fastng;
+ mask = BD96801_WD_RATIO_MASK | BD96801_WD_TMO_SHORT_MASK;
+ ret = regmap_update_bits(w->regmap, BD96801_REG_WD_TMO,
+ mask, reg);
+ if (ret)
+ return ret;
+
+ ret = regmap_update_bits(w->regmap, BD96801_REG_WD_CONF,
+ BD96801_WD_TYPE_MASK, type);
+
+ return ret;
+}
+
+static int bd96801_set_heartbeat_from_hw(struct wdtbd96801 *w,
+ unsigned int conf_reg)
+{
+ int ret;
+ unsigned int val, sel, fast;
+
+ /*
+ * The BD96801 supports a somewhat peculiar QA-mode, which we do not
+ * support in this driver. If the QA-mode is enabled then we just
+ * warn and bail-out.
+ */
+ if ((conf_reg & BD96801_WD_EN_MASK) != BD96801_WD_IF_EN) {
+ dev_warn(w->dev, "watchdog set to Q&A mode - exiting\n");
+ return -EINVAL;
+ }
+
+ ret = regmap_read(w->regmap, BD96801_REG_WD_TMO, &val);
+ if (ret)
+ return ret;
+
+ sel = val & BD96801_WD_TMO_SHORT_MASK;
+ sel >>= ffs(BD96801_WD_TMO_SHORT_MASK) - 1;
+ fast = FASTNG_MIN << sel;
+
+ sel = (val & BD96801_WD_RATIO_MASK) + 1;
+ w->wdt.max_hw_heartbeat_ms = (fast << sel) / USEC_PER_MSEC;
+
+ if ((conf_reg & BD96801_WD_TYPE_MASK) == BD96801_WD_TYPE_WIN)
+ w->wdt.min_hw_heartbeat_ms = fast / USEC_PER_MSEC;
+
+ return 0;
+}
+
+static int init_wdg_hw(struct wdtbd96801 *w)
+{
+ u32 hw_margin[2];
+ int count, ret;
+ u32 hw_margin_max = BD96801_WDT_DEFAULT_MARGIN_MS, hw_margin_min = 0;
+
+ count = device_property_count_u32(w->dev->parent, "rohm,hw-timeout-ms");
+ if (count < 0 && count != -EINVAL)
+ return count;
+
+ if (count > 0) {
+ if (count > ARRAY_SIZE(hw_margin))
+ return -EINVAL;
+
+ ret = device_property_read_u32_array(w->dev->parent,
+ "rohm,hw-timeout-ms",
+ &hw_margin[0], count);
+ if (ret < 0)
+ return ret;
+
+ if (count == 1)
+ hw_margin_max = hw_margin[0];
+
+ if (count == 2) {
+ if (hw_margin[1] > hw_margin[0]) {
+ hw_margin_max = hw_margin[1];
+ hw_margin_min = hw_margin[0];
+ } else {
+ hw_margin_max = hw_margin[0];
+ hw_margin_min = hw_margin[1];
+ }
+ }
+ }
+
+ ret = bd96801_set_wdt_mode(w, hw_margin_max, hw_margin_min);
+ if (ret)
+ return ret;
+
+ ret = device_property_match_string(w->dev->parent, "rohm,wdg-action",
+ "prstb");
+ if (ret >= 0) {
+ ret = regmap_update_bits(w->regmap, BD96801_REG_WD_CONF,
+ BD96801_WD_ASSERT_MASK,
+ BD96801_WD_ASSERT_RST);
+ return ret;
+ }
+
+ ret = device_property_match_string(w->dev->parent, "rohm,wdg-action",
+ "intb-only");
+ if (ret >= 0) {
+ ret = regmap_update_bits(w->regmap, BD96801_REG_WD_CONF,
+ BD96801_WD_ASSERT_MASK,
+ BD96801_WD_ASSERT_IRQ);
+ return ret;
+ }
+
+ return 0;
+}
+
+extern void emergency_restart(void);
+static irqreturn_t bd96801_irq_hnd(int irq, void *data)
+{
+ emergency_restart();
+
+ return IRQ_NONE;
+}
+
+static int bd96801_wdt_probe(struct platform_device *pdev)
+{
+ struct wdtbd96801 *w;
+ int ret, irq;
+ unsigned int val;
+
+ w = devm_kzalloc(&pdev->dev, sizeof(*w), GFP_KERNEL);
+ if (!w)
+ return -ENOMEM;
+
+ w->regmap = dev_get_regmap(pdev->dev.parent, NULL);
+ w->dev = &pdev->dev;
+
+ w->wdt.info = &bd96801_wdt_info;
+ w->wdt.ops = &bd96801_wdt_ops;
+ w->wdt.parent = pdev->dev.parent;
+ w->wdt.timeout = DEFAULT_TIMEOUT;
+ watchdog_set_drvdata(&w->wdt, w);
+
+ ret = regmap_read(w->regmap, BD96801_REG_WD_CONF, &val);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret,
+ "Failed to get the watchdog state\n");
+
+ /*
+ * If the WDG is already enabled we assume it is configured by boot.
+ * In this case we just update the hw-timeout based on values set to
+ * the timeout / mode registers and leave the hardware configs
+ * untouched.
+ */
+ if ((val & BD96801_WD_EN_MASK) != BD96801_WD_DISABLE) {
+ dev_dbg(&pdev->dev, "watchdog was running during probe\n");
+ ret = bd96801_set_heartbeat_from_hw(w, val);
+ if (ret)
+ return ret;
+
+ set_bit(WDOG_HW_RUNNING, &w->wdt.status);
+ } else {
+ /* If WDG is not running so we will initializate it */
+ ret = init_wdg_hw(w);
+ if (ret)
+ return ret;
+ }
+
+ dev_dbg(w->dev, "heartbeat set to %u - %u\n",
+ w->wdt.min_hw_heartbeat_ms, w->wdt.max_hw_heartbeat_ms);
+
+ watchdog_init_timeout(&w->wdt, 0, pdev->dev.parent);
+ watchdog_set_nowayout(&w->wdt, nowayout);
+ watchdog_stop_on_reboot(&w->wdt);
+
+ irq = platform_get_irq_byname(pdev, "bd96801-wdg");
+ if (irq > 0) {
+ ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
+ bd96801_irq_hnd,
+ IRQF_ONESHOT, "bd96801-wdg",
+ NULL);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret,
+ "Failed to register IRQ\n");
+ }
+
+ return devm_watchdog_register_device(&pdev->dev, &w->wdt);
+}
+
+static const struct platform_device_id bd96801_wdt_id[] = {
+ { "bd96801-wdt", },
+ { }
+};
+MODULE_DEVICE_TABLE(platform, bd96801_wdt_id);
+
+static struct platform_driver bd96801_wdt = {
+ .driver = {
+ .name = "bd96801-wdt"
+ },
+ .probe = bd96801_wdt_probe,
+ .id_table = bd96801_wdt_id,
+};
+module_platform_driver(bd96801_wdt);
+
+MODULE_AUTHOR("Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>");
+MODULE_DESCRIPTION("BD96801 watchdog driver");
+MODULE_LICENSE("GPL");
--
2.44.0
--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND
~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =]
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v1 6/6] MAINTAINERS: Add ROHM BD96801 'scalable PMIC' entries
2024-04-30 11:59 [PATCH v1 0/6] Support ROHM BD96801 scalable PMIC Matti Vaittinen
` (4 preceding siblings ...)
2024-04-30 12:01 ` [PATCH v1 5/6] watchdog: ROHM BD96801 PMIC WDG driver Matti Vaittinen
@ 2024-04-30 12:02 ` Matti Vaittinen
5 siblings, 0 replies; 14+ messages in thread
From: Matti Vaittinen @ 2024-04-30 12:02 UTC (permalink / raw)
To: Matti Vaittinen, Matti Vaittinen
Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Liam Girdwood, Mark Brown, Matti Vaittinen, Wim Van Sebroeck,
Guenter Roeck, devicetree, linux-kernel, linux-watchdog
[-- Attachment #1: Type: text/plain, Size: 1510 bytes --]
Add maintainer entries for ROHM BD96801 a.k.a 'scalable PMIC'
drivers to be reviewed by ROHM people.
Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
---
MAINTAINERS | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index aa3b947fb080..da68144d51ae 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19111,17 +19111,21 @@ F: drivers/gpio/gpio-bd71828.c
F: drivers/mfd/rohm-bd71828.c
F: drivers/mfd/rohm-bd718x7.c
F: drivers/mfd/rohm-bd9576.c
+F: drivers/mfd/rohm-bd96801.c
F: drivers/regulator/bd71815-regulator.c
F: drivers/regulator/bd71828-regulator.c
F: drivers/regulator/bd718x7-regulator.c
F: drivers/regulator/bd9576-regulator.c
+F: drivers/regulator/bd96801-regulator.c
F: drivers/regulator/rohm-regulator.c
F: drivers/rtc/rtc-bd70528.c
F: drivers/watchdog/bd9576_wdt.c
+F: drivers/watchdog/bd96801_wdt.c
F: include/linux/mfd/rohm-bd71815.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-bd96801.h
F: include/linux/mfd/rohm-generic.h
F: include/linux/mfd/rohm-shared.h
--
2.44.0
--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND
~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =]
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v1 5/6] watchdog: ROHM BD96801 PMIC WDG driver
2024-04-30 12:01 ` [PATCH v1 5/6] watchdog: ROHM BD96801 PMIC WDG driver Matti Vaittinen
@ 2024-04-30 16:24 ` Guenter Roeck
2024-05-01 10:16 ` Matti Vaittinen
0 siblings, 1 reply; 14+ messages in thread
From: Guenter Roeck @ 2024-04-30 16:24 UTC (permalink / raw)
To: Matti Vaittinen, Matti Vaittinen
Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Liam Girdwood, Mark Brown, Wim Van Sebroeck, devicetree,
linux-kernel, linux-watchdog
On 4/30/24 05:01, Matti Vaittinen wrote:
> Introduce driver for WDG block on ROHM BD96801 scalable PMIC.
>
> This driver only supports watchdog with I2C feeding and delayed
> response detection. Whether the watchdog toggles PRSTB pin or
> just causes an interrupt can be configured via device-tree.
>
> The BD96801 PMIC HW supports also window watchdog (too early
> feeding detection) and Q&A mode. These are not supported by
> this driver.
>
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
>
> ---
> Revision history:
> RFCv2 => v1:
> - Fix watchdog time-outs to match DS4
> - Fix target timeout overflow
> - Improve dbg prints
>
> RFCv1 => RFCv2:
> - remove always running
> - add IRQ handling
> - call emergency_restart()
> - drop MODULE_ALIAS and add MODULE_DEVICE_TABLE
> ---
> drivers/watchdog/Kconfig | 13 ++
> drivers/watchdog/Makefile | 1 +
> drivers/watchdog/bd96801_wdt.c | 400 +++++++++++++++++++++++++++++++++
> 3 files changed, 414 insertions(+)
> create mode 100644 drivers/watchdog/bd96801_wdt.c
>
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 6bee137cfbe0..d97e735e1faa 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -181,6 +181,19 @@ config BD957XMUF_WATCHDOG
> watchdog. Alternatively say M to compile the driver as a module,
> which will be called bd9576_wdt.
>
> +config BD96801_WATCHDOG
> + tristate "ROHM BD96801 PMIC Watchdog"
> + depends on MFD_ROHM_BD96801
> + select WATCHDOG_CORE
> + help
> + Support for the watchdog in the ROHM BD96801 PMIC. Watchdog can be
> + configured to only generate IRQ or to trigger system reset via reset
> + pin.
> +
> + Say Y here to include support for the ROHM BD96801 watchdog.
> + Alternatively say M to compile the driver as a module,
> + which will be called bd96801_wdt.
> +
> config CROS_EC_WATCHDOG
> tristate "ChromeOS EC-based watchdog"
> select WATCHDOG_CORE
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index 3710c218f05e..31bc94436c81 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -217,6 +217,7 @@ obj-$(CONFIG_XEN_WDT) += xen_wdt.o
>
> # Architecture Independent
> obj-$(CONFIG_BD957XMUF_WATCHDOG) += bd9576_wdt.o
> +obj-$(CONFIG_BD96801_WATCHDOG) += bd96801_wdt.o
> obj-$(CONFIG_CROS_EC_WATCHDOG) += cros_ec_wdt.o
> obj-$(CONFIG_DA9052_WATCHDOG) += da9052_wdt.o
> obj-$(CONFIG_DA9055_WATCHDOG) += da9055_wdt.o
> diff --git a/drivers/watchdog/bd96801_wdt.c b/drivers/watchdog/bd96801_wdt.c
> new file mode 100644
> index 000000000000..6069f1d75356
> --- /dev/null
> +++ b/drivers/watchdog/bd96801_wdt.c
> @@ -0,0 +1,400 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2024 ROHM Semiconductors
> + *
> + * ROHM BD96801 watchdog driver
> + */
> +
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/mfd/rohm-bd96801.h>
> +#include <linux/mfd/rohm-generic.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 BD96801_WD_TMO_SHORT_MASK 0x70
> +#define BD96801_WD_RATIO_MASK 0x3
> +#define BD96801_WD_TYPE_MASK 0x4
> +#define BD96801_WD_TYPE_SLOW 0x4
> +#define BD96801_WD_TYPE_WIN 0x0
> +
> +#define BD96801_WD_EN_MASK 0x3
> +#define BD96801_WD_IF_EN 0x1
> +#define BD96801_WD_QA_EN 0x2
> +#define BD96801_WD_DISABLE 0x0
> +
> +#define BD96801_WD_ASSERT_MASK 0x8
> +#define BD96801_WD_ASSERT_RST 0x8
> +#define BD96801_WD_ASSERT_IRQ 0x0
> +
> +#define BD96801_WD_FEED_MASK 0x1
> +#define BD96801_WD_FEED 0x1
> +
> +/* units in uS */
> +#define FASTNG_MIN 11
> +
> +#define BD96801_WDT_DEFAULT_MARGIN_MS 1843
> +/* Unit is seconds */
> +#define DEFAULT_TIMEOUT 30
> +
> +/*
> + * BD96801 WDG supports window mode so the TMO consists of SHORT and LONG
> + * timeout values. SHORT time is meaningfull only in window mode where feeding
meaningful
> + * period shorter than SHORT would be an error. LONG time is used to detect if
> + * feeding is not occurring within given time limit (SoC SW hangs). The LONG
> + * timeout time is a multiple of (2, 4, 8 0r 16 times) the SHORT timeout.
0r -> or
> + */
> +
> +struct wdtbd96801 {
> + struct device *dev;
> + struct regmap *regmap;
> + struct watchdog_device wdt;
> +};
> +
> +static int bd96801_wdt_ping(struct watchdog_device *wdt)
> +{
> + struct wdtbd96801 *w = watchdog_get_drvdata(wdt);
> +
> + return regmap_update_bits(w->regmap, BD96801_REG_WD_FEED,
> + BD96801_WD_FEED_MASK, BD96801_WD_FEED);
> +}
> +
> +static int bd96801_wdt_start(struct watchdog_device *wdt)
> +{
> + struct wdtbd96801 *w = watchdog_get_drvdata(wdt);
> +
> + return regmap_update_bits(w->regmap, BD96801_REG_WD_CONF,
> + BD96801_WD_EN_MASK, BD96801_WD_IF_EN);
> +}
> +
> +static int bd96801_wdt_stop(struct watchdog_device *wdt)
> +{
> + struct wdtbd96801 *w = watchdog_get_drvdata(wdt);
> +
> + return regmap_update_bits(w->regmap, BD96801_REG_WD_CONF,
> + BD96801_WD_EN_MASK, BD96801_WD_DISABLE);
> +}
> +
> +static const struct watchdog_info bd96801_wdt_info = {
> + .options = WDIOF_MAGICCLOSE | WDIOF_KEEPALIVEPING |
> + WDIOF_SETTIMEOUT,
> + .identity = "BD96801 Watchdog",
> +};
> +
> +static const struct watchdog_ops bd96801_wdt_ops = {
> + .start = bd96801_wdt_start,
> + .stop = bd96801_wdt_stop,
> + .ping = bd96801_wdt_ping,
> +};
> +
> +static int find_closest_fast(unsigned int target, int *sel, unsigned int *val)
> +{
> + unsigned int window = FASTNG_MIN;
> + int i;
> +
> + for (i = 0; i < 8 && window < target; i++)
> + window <<= 1;
> +
> + *val = window;
> + *sel = i;
> +
> + if (i == 8)
> + return -EINVAL;
> +
It might make sense to have the error check before assigning return values,
similar to the other find functions.
> + return 0;
> +}
> +
> +static int find_closest_slow_by_fast(unsigned int fast_val, unsigned int *target, int *slowsel)
> +{
> + static const int multipliers[] = {2, 4, 8, 16};
> + int sel;
> +
> + for (sel = 0; sel < ARRAY_SIZE(multipliers) &&
> + multipliers[sel] * fast_val < *target; sel++)
> + ;
> +
> + if (sel == ARRAY_SIZE(multipliers))
> + return -EINVAL;
> +
> + *slowsel = sel;
> + *target = multipliers[sel] * fast_val;
> +
> + return 0;
> +}
> +
> +static int find_closest_slow(unsigned int *target, int *slow_sel, int *fast_sel)
> +{
> + static const int multipliers[] = {2, 4, 8, 16};
> + unsigned int window = FASTNG_MIN;
> + unsigned int val = 0;
> + int i, j;
> +
> + for (i = 0; i < 8; i++) {
> + for (j = 0; j < ARRAY_SIZE(multipliers); j++) {
> + unsigned 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;
> +
> + *target = val;
> +
> + return 0;
> +}
> +
> +static int bd96801_set_wdt_mode(struct wdtbd96801 *w, unsigned int hw_margin,
> + unsigned int hw_margin_min)
> +{
> + int fastng, slowng, type, ret, reg, mask;
> + struct device *dev = w->dev;
> +
> + /*
> + * Convert to 100uS to guarantee reasonable timeouts fit in
> + * 32bit maintaining also a decent accuracy.
> + */
> + hw_margin *= 10;
> + hw_margin_min *= 10;
> +
> + if (hw_margin_min) {
> + unsigned int min;
> +
> + type = BD96801_WD_TYPE_WIN;
> + 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;
> + }
> + w->wdt.min_hw_heartbeat_ms = min / 10;
> + } else {
> + type = BD96801_WD_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");
What is the value of those error messages ? To me they only leave big
question marks. One would see "bad WDT window" or "bad WDT window for
fast timeout" and then what ? If the cause is bad values for hw_margin
and/or hw_margin_min, the message should include the offending values
and not leave the user in the dark.
> + return ret;
> + }
> + }
> +
> + w->wdt.max_hw_heartbeat_ms = hw_margin / 10;
> +
> + fastng <<= ffs(BD96801_WD_TMO_SHORT_MASK) - 1;
Any reason fore not using standard functionality such as FIELD_PREP here ?
> +
> + reg = slowng | fastng;
> + mask = BD96801_WD_RATIO_MASK | BD96801_WD_TMO_SHORT_MASK;
> + ret = regmap_update_bits(w->regmap, BD96801_REG_WD_TMO,
> + mask, reg);
> + if (ret)
> + return ret;
> +
> + ret = regmap_update_bits(w->regmap, BD96801_REG_WD_CONF,
> + BD96801_WD_TYPE_MASK, type);
> +
> + return ret;
> +}
> +
> +static int bd96801_set_heartbeat_from_hw(struct wdtbd96801 *w,
> + unsigned int conf_reg)
> +{
> + int ret;
> + unsigned int val, sel, fast;
> +
> + /*
> + * The BD96801 supports a somewhat peculiar QA-mode, which we do not
> + * support in this driver. If the QA-mode is enabled then we just
> + * warn and bail-out.
> + */
> + if ((conf_reg & BD96801_WD_EN_MASK) != BD96801_WD_IF_EN) {
> + dev_warn(w->dev, "watchdog set to Q&A mode - exiting\n");
This should be dev_err(). It is not just a warning.
> + return -EINVAL;
> + }
> +
> + ret = regmap_read(w->regmap, BD96801_REG_WD_TMO, &val);
> + if (ret)
> + return ret;
> +
> + sel = val & BD96801_WD_TMO_SHORT_MASK;
> + sel >>= ffs(BD96801_WD_TMO_SHORT_MASK) - 1;
Same question as above: Why not use FIELD_GET() ?
> + fast = FASTNG_MIN << sel;
> +
> + sel = (val & BD96801_WD_RATIO_MASK) + 1;
> + w->wdt.max_hw_heartbeat_ms = (fast << sel) / USEC_PER_MSEC;
> +
> + if ((conf_reg & BD96801_WD_TYPE_MASK) == BD96801_WD_TYPE_WIN)
> + w->wdt.min_hw_heartbeat_ms = fast / USEC_PER_MSEC;
> +
> + return 0;
> +}
> +
> +static int init_wdg_hw(struct wdtbd96801 *w)
> +{
> + u32 hw_margin[2];
> + int count, ret;
> + u32 hw_margin_max = BD96801_WDT_DEFAULT_MARGIN_MS, hw_margin_min = 0;
> +
> + count = device_property_count_u32(w->dev->parent, "rohm,hw-timeout-ms");
> + if (count < 0 && count != -EINVAL)
> + return count;
> +
> + if (count > 0) {
> + if (count > ARRAY_SIZE(hw_margin))
> + return -EINVAL;
> +
> + ret = device_property_read_u32_array(w->dev->parent,
> + "rohm,hw-timeout-ms",
> + &hw_margin[0], count);
> + if (ret < 0)
> + return ret;
> +
> + if (count == 1)
> + hw_margin_max = hw_margin[0];
> +
> + if (count == 2) {
> + if (hw_margin[1] > hw_margin[0]) {
> + hw_margin_max = hw_margin[1];
> + hw_margin_min = hw_margin[0];
> + } else {
> + hw_margin_max = hw_margin[0];
> + hw_margin_min = hw_margin[1];
> + }
> + }
> + }
> +
> + ret = bd96801_set_wdt_mode(w, hw_margin_max, hw_margin_min);
> + if (ret)
> + return ret;
> +
> + ret = device_property_match_string(w->dev->parent, "rohm,wdg-action",
> + "prstb");
> + if (ret >= 0) {
> + ret = regmap_update_bits(w->regmap, BD96801_REG_WD_CONF,
> + BD96801_WD_ASSERT_MASK,
> + BD96801_WD_ASSERT_RST);
> + return ret;
> + }
> +
> + ret = device_property_match_string(w->dev->parent, "rohm,wdg-action",
> + "intb-only");
> + if (ret >= 0) {
> + ret = regmap_update_bits(w->regmap, BD96801_REG_WD_CONF,
> + BD96801_WD_ASSERT_MASK,
> + BD96801_WD_ASSERT_IRQ);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +extern void emergency_restart(void);
No. include linux/reboot.h instead.
> +static irqreturn_t bd96801_irq_hnd(int irq, void *data)
> +{
> + emergency_restart();
> +
> + return IRQ_NONE;
> +}
> +
> +static int bd96801_wdt_probe(struct platform_device *pdev)
> +{
> + struct wdtbd96801 *w;
> + int ret, irq;
> + unsigned int val;
> +
> + w = devm_kzalloc(&pdev->dev, sizeof(*w), GFP_KERNEL);
> + if (!w)
> + return -ENOMEM;
> +
> + w->regmap = dev_get_regmap(pdev->dev.parent, NULL);
> + w->dev = &pdev->dev;
> +
> + w->wdt.info = &bd96801_wdt_info;
> + w->wdt.ops = &bd96801_wdt_ops;
> + w->wdt.parent = pdev->dev.parent;
> + w->wdt.timeout = DEFAULT_TIMEOUT;
> + watchdog_set_drvdata(&w->wdt, w);
> +
> + ret = regmap_read(w->regmap, BD96801_REG_WD_CONF, &val);
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret,
> + "Failed to get the watchdog state\n");
> +
> + /*
> + * If the WDG is already enabled we assume it is configured by boot.
> + * In this case we just update the hw-timeout based on values set to
> + * the timeout / mode registers and leave the hardware configs
> + * untouched.
> + */
> + if ((val & BD96801_WD_EN_MASK) != BD96801_WD_DISABLE) {
> + dev_dbg(&pdev->dev, "watchdog was running during probe\n");
> + ret = bd96801_set_heartbeat_from_hw(w, val);
> + if (ret)
> + return ret;
> +
> + set_bit(WDOG_HW_RUNNING, &w->wdt.status);
> + } else {
> + /* If WDG is not running so we will initializate it */
> + ret = init_wdg_hw(w);
> + if (ret)
> + return ret;
> + }
> +
> + dev_dbg(w->dev, "heartbeat set to %u - %u\n",
> + w->wdt.min_hw_heartbeat_ms, w->wdt.max_hw_heartbeat_ms);
> +
> + watchdog_init_timeout(&w->wdt, 0, pdev->dev.parent);
> + watchdog_set_nowayout(&w->wdt, nowayout);
> + watchdog_stop_on_reboot(&w->wdt);
> +
> + irq = platform_get_irq_byname(pdev, "bd96801-wdg");
> + if (irq > 0) {
> + ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
> + bd96801_irq_hnd,
> + IRQF_ONESHOT, "bd96801-wdg",
> + NULL);
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret,
> + "Failed to register IRQ\n");
> + }
> +
> + return devm_watchdog_register_device(&pdev->dev, &w->wdt);
> +}
> +
> +static const struct platform_device_id bd96801_wdt_id[] = {
> + { "bd96801-wdt", },
> + { }
> +};
> +MODULE_DEVICE_TABLE(platform, bd96801_wdt_id);
> +
> +static struct platform_driver bd96801_wdt = {
> + .driver = {
> + .name = "bd96801-wdt"
> + },
> + .probe = bd96801_wdt_probe,
> + .id_table = bd96801_wdt_id,
> +};
> +module_platform_driver(bd96801_wdt);
> +
> +MODULE_AUTHOR("Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>");
> +MODULE_DESCRIPTION("BD96801 watchdog driver");
> +MODULE_LICENSE("GPL");
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 5/6] watchdog: ROHM BD96801 PMIC WDG driver
2024-04-30 16:24 ` Guenter Roeck
@ 2024-05-01 10:16 ` Matti Vaittinen
0 siblings, 0 replies; 14+ messages in thread
From: Matti Vaittinen @ 2024-05-01 10:16 UTC (permalink / raw)
To: Guenter Roeck, Matti Vaittinen
Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Liam Girdwood, Mark Brown, Wim Van Sebroeck, devicetree,
linux-kernel, linux-watchdog
Hi Guenter,
Thanks again for the review :) I agree with all of your comments, thanks!
On 4/30/24 19:24, Guenter Roeck wrote:
> On 4/30/24 05:01, Matti Vaittinen wrote:
>> Introduce driver for WDG block on ROHM BD96801 scalable PMIC.
>>
>> This driver only supports watchdog with I2C feeding and delayed
>> response detection. Whether the watchdog toggles PRSTB pin or
>> just causes an interrupt can be configured via device-tree.
>>
>> The BD96801 PMIC HW supports also window watchdog (too early
>> feeding detection) and Q&A mode. These are not supported by
>> this driver.
>>
>> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
>>
>> ---
>> Revision history:
>> RFCv2 => v1:
>> - Fix watchdog time-outs to match DS4
>> - Fix target timeout overflow
>> - Improve dbg prints
>>
>> RFCv1 => RFCv2:
>> - remove always running
>> - add IRQ handling
>> - call emergency_restart()
>> - drop MODULE_ALIAS and add MODULE_DEVICE_TABLE
>> ---
...
>> +
>> +static int bd96801_set_wdt_mode(struct wdtbd96801 *w, unsigned int
>> hw_margin,
>> + unsigned int hw_margin_min)
>> +{
>> + int fastng, slowng, type, ret, reg, mask;
>> + struct device *dev = w->dev;
>> +
>> + /*
>> + * Convert to 100uS to guarantee reasonable timeouts fit in
>> + * 32bit maintaining also a decent accuracy.
>> + */
>> + hw_margin *= 10;
>> + hw_margin_min *= 10;
>> +
>> + if (hw_margin_min) {
>> + unsigned int min;
>> +
>> + type = BD96801_WD_TYPE_WIN;
>> + 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;
>> + }
>> + w->wdt.min_hw_heartbeat_ms = min / 10;
>> + } else {
>> + type = BD96801_WD_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");
>
> What is the value of those error messages ? To me they only leave big
> question marks. One would see "bad WDT window" or "bad WDT window for
> fast timeout" and then what ? If the cause is bad values for hw_margin
> and/or hw_margin_min, the message should include the offending values
> and not leave the user in the dark.
Well, at least one can grep the error from module which printed it :)
But yes, you have a very valid point. I will see how to improve, thanks!
>
>> + return ret;
>> + }
>> + }
>> +
>> + w->wdt.max_hw_heartbeat_ms = hw_margin / 10;
>> +
>> + fastng <<= ffs(BD96801_WD_TMO_SHORT_MASK) - 1;
>
> Any reason fore not using standard functionality such as FIELD_PREP here ?
A reason, yes. A good reason, no :) Reason is that I wrote this before I
learned about the FIELD_PREP() and FIELD_GET(). And actually, I have
always found those two more confusing than the open-coded shift, '|' and
'&'. Still, I believe the FIELD_GET() and FIELD_PREP() make it obvious
for a reader that the bit magic is just a standard register field stuff.
So, no good reason. I'll fix this for the next version.
...
>> +extern void emergency_restart(void);
>
> No. include linux/reboot.h instead.
Hmm.. I am sure I had checked the headers for prototypes before doing
something like this... I think I must've missed something. I'll fix this
for the next version, or add a good explanation if there indeed was some
problem finding the prototype.
Yours,
-- Matti
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 1/6] dt-bindings: ROHM BD96801 PMIC regulators
2024-04-30 11:59 ` [PATCH v1 1/6] dt-bindings: ROHM BD96801 PMIC regulators Matti Vaittinen
@ 2024-05-02 16:20 ` Conor Dooley
2024-05-03 4:54 ` Matti Vaittinen
0 siblings, 1 reply; 14+ messages in thread
From: Conor Dooley @ 2024-05-02 16:20 UTC (permalink / raw)
To: Matti Vaittinen
Cc: Matti Vaittinen, Lee Jones, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Liam Girdwood, Mark Brown, Wim Van Sebroeck,
Guenter Roeck, devicetree, linux-kernel, linux-watchdog
[-- Attachment #1: Type: text/plain, Size: 654 bytes --]
On Tue, Apr 30, 2024 at 02:59:50PM +0300, Matti Vaittinen wrote:
> ROHM BD96801 is a highly configurable automotive grade PMIC. Introduce
> DT bindings for the BD96801 regulators.
>
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
>
> ---
> RFCv2 => v1
RFC is a status, not a version - ideally this would have been v3 and the
next version v4.
> - Drop regulator-name pattern requirement
> - do not require regulator-name
Krzysztof had some comments on the buck/ldo node names and on the
initial value properties that I'm not sure if have been addressed, so
gonna leave this series to him.
Cheers,
Conor.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 1/6] dt-bindings: ROHM BD96801 PMIC regulators
2024-05-02 16:20 ` Conor Dooley
@ 2024-05-03 4:54 ` Matti Vaittinen
2024-05-03 7:25 ` Matti Vaittinen
0 siblings, 1 reply; 14+ messages in thread
From: Matti Vaittinen @ 2024-05-03 4:54 UTC (permalink / raw)
To: Conor Dooley
Cc: Matti Vaittinen, Lee Jones, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Liam Girdwood, Mark Brown, Wim Van Sebroeck,
Guenter Roeck, devicetree, linux-kernel, linux-watchdog
Hi Conor,
On 5/2/24 19:20, Conor Dooley wrote:
> On Tue, Apr 30, 2024 at 02:59:50PM +0300, Matti Vaittinen wrote:
>> ROHM BD96801 is a highly configurable automotive grade PMIC. Introduce
>> DT bindings for the BD96801 regulators.
>>
>> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
>>
>> ---
>> RFCv2 => v1
>
> RFC is a status, not a version - ideally this would have been v3 and the
> next version v4.
Thanks for the clarification. I've always wondered if an RFC should be
seen as a separate series. Previously I've ended up just dropping the
RFC and pumping up the version. This time the switch from RFC => non RFC
was somewhat radical as a lot of the features were dropped. Furthermore,
I've developed the 'simple' version (this non RFC one) and
'experimental' version (the RFC one) in separate branches - which made
the separation even stronger in my mind - I probably started thinking
these as two different patch series.
But, as I said, thanks for the clarification! I guess it's still better
to make next version v2 (and not v4) to not add even more confusion...
>> - Drop regulator-name pattern requirement
>> - do not require regulator-name
>
>
> Krzysztof had some comments on the buck/ldo node names
I think Krzysztof pointed out that the regulator-name property should
not match the data-sheet but the board. If he had something to say about
the node names, then I've missed his comment!
> and on the
> initial value properties that I'm not sure if have been addressed, so
> gonna leave this series to him.
Thanks for pointing out I may have missed addressing some of his
concerns. I though I fixed all issues he pointed to me but it may be I
missed some - or accidentally dropped some change(s) when merging fixes
from the 'experimental' branch to the 'simple'. I'll revise Krzysztof's
feedback to the RFC before sending the next version!
Thanks!
Yours,
-- Matti
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 1/6] dt-bindings: ROHM BD96801 PMIC regulators
2024-05-03 4:54 ` Matti Vaittinen
@ 2024-05-03 7:25 ` Matti Vaittinen
0 siblings, 0 replies; 14+ messages in thread
From: Matti Vaittinen @ 2024-05-03 7:25 UTC (permalink / raw)
To: Conor Dooley
Cc: Matti Vaittinen, Lee Jones, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Liam Girdwood, Mark Brown, Wim Van Sebroeck,
Guenter Roeck, devicetree, linux-kernel, linux-watchdog
On 5/3/24 07:54, Matti Vaittinen wrote:
> Hi Conor,
>
> On 5/2/24 19:20, Conor Dooley wrote:
>> On Tue, Apr 30, 2024 at 02:59:50PM +0300, Matti Vaittinen wrote:
>>
>> Krzysztof had some comments on the buck/ldo node names
>
> I think Krzysztof pointed out that the regulator-name property should
> not match the data-sheet but the board. If he had something to say about
> the node names, then I've missed his comment!
>
>> and on the
>> initial value properties that I'm not sure if have been addressed, so
>> gonna leave this series to him.
>
> Thanks for pointing out I may have missed addressing some of his
> concerns. I though I fixed all issues he pointed to me but it may be I
> missed some - or accidentally dropped some change(s) when merging fixes
> from the 'experimental' branch to the 'simple'. I'll revise Krzysztof's
> feedback to the RFC before sending the next version!
You were right Conor. I checked it and yes, I omitted a few of the fixes
to regulator bindings. Please skip reviewing this version of these
bindings, I'll handle the rest of the comments in next one!
Yours,
-- Matti
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 3/6] mfd: support ROHM BD96801 PMIC core
2024-04-30 12:00 ` [PATCH v1 3/6] mfd: support ROHM BD96801 " Matti Vaittinen
@ 2024-05-03 7:41 ` Lee Jones
2024-05-03 8:43 ` Matti Vaittinen
0 siblings, 1 reply; 14+ messages in thread
From: Lee Jones @ 2024-05-03 7:41 UTC (permalink / raw)
To: Matti Vaittinen
Cc: Matti Vaittinen, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Liam Girdwood, Mark Brown, Wim Van Sebroeck, Guenter Roeck,
devicetree, linux-kernel, linux-watchdog
On Tue, 30 Apr 2024, Matti Vaittinen wrote:
> The ROHM BD96801 PMIC is highly customizable automotive grade PMIC
> which integrates regulator and watchdog funtionalities.
>
> Provide INTB IRQ and register accesses for regulator/watchdog drivers.
>
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
>
> ---
> Changelog: RFCv2 => v1:
> - drop ERRB interrupts (for now)
> - bd96801: Unlock registers in core driver
>
> Changelog: RFCv1 => RFCv2
> - Work-around the IRQ domain name conflict
> - Add watchdog IRQ
> - Various styling fixes based on review by Lee
> ---
> drivers/mfd/Kconfig | 13 ++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/rohm-bd96801.c | 278 +++++++++++++++++++++++++++++++
> include/linux/mfd/rohm-bd96801.h | 215 ++++++++++++++++++++++++
> include/linux/mfd/rohm-generic.h | 1 +
> 5 files changed, 508 insertions(+)
> create mode 100644 drivers/mfd/rohm-bd96801.c
> create mode 100644 include/linux/mfd/rohm-bd96801.h
Still some nits, sorry.
I probably wouldn't have been so picky if I hadn't have found the unused enum.
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 4b023ee229cf..9e874453d94d 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -2089,6 +2089,19 @@ config MFD_ROHM_BD957XMUF
> BD9573MUF Power Management ICs. BD9576 and BD9573 are primarily
> designed to be used to power R-Car series processors.
>
> +config MFD_ROHM_BD96801
> + tristate "ROHM BD96801 Power Management IC"
> + depends on I2C=y
> + depends on OF
> + select REGMAP_I2C
> + select REGMAP_IRQ
> + select MFD_CORE
> + help
> + Select this option to get support for the ROHM BD96801 Power
> + Management IC. The ROHM BD96801 is a highly scalable Power Management
> + IC for industrial and automotive use. The BD96801 can be used as a
> + master PMIC in a chained PMIC solution with suitable companion PMICs.
> +
> config MFD_STM32_LPTIMER
> tristate "Support for STM32 Low-Power Timer"
> depends on (ARCH_STM32 && OF) || COMPILE_TEST
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index c66f07edcd0e..e792892d4a8b 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -264,6 +264,7 @@ obj-$(CONFIG_RAVE_SP_CORE) += rave-sp.o
> obj-$(CONFIG_MFD_ROHM_BD71828) += rohm-bd71828.o
> obj-$(CONFIG_MFD_ROHM_BD718XX) += rohm-bd718x7.o
> obj-$(CONFIG_MFD_ROHM_BD957XMUF) += rohm-bd9576.o
> +obj-$(CONFIG_MFD_ROHM_BD96801) += rohm-bd96801.o
> obj-$(CONFIG_MFD_STMFX) += stmfx.o
> obj-$(CONFIG_MFD_KHADAS_MCU) += khadas-mcu.o
> obj-$(CONFIG_MFD_ACER_A500_EC) += acer-ec-a500.o
> diff --git a/drivers/mfd/rohm-bd96801.c b/drivers/mfd/rohm-bd96801.c
> new file mode 100644
> index 000000000000..1e9c49c857c1
> --- /dev/null
> +++ b/drivers/mfd/rohm-bd96801.c
> @@ -0,0 +1,278 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2024 ROHM Semiconductors
> + *
> + * ROHM BD96801 PMIC driver
> + *
> + * This version of the "BD86801 scalable PMIC"'s driver supports only very
> + * basic set of the PMIC features. Most notably, there is no support for
> + * the ERRB interrupt and the configurations which should be done when the
> + * PMIC is in STBY mode.
> + *
> + * Supporting the ERRB interrupt would require dropping the regmap-IRQ
> + * usage or working around (or accepting a presense of) a naming conflict
> + * in debugFS IRQs.
> + *
> + * Being able to reliably do the configurations like changing the
> + * regulator safety limits (like limits for the over/under -voltages, over
> + * current, thermal protection) would require the configuring driver to be
> + * synchronized with entity causing the PMIC state transitions. Eg, one
> + * should be able to ensure the PMIC is in STBY state when the
> + * configurations are applied to the hardware. How and when the PMIC state
> + * transitions are to be done is likely to be very system specific, as will
> + * be the need to configure these safety limits. Hence it's not simple to
> + * come up with a generic solution.
> + *
> + * Users who require the ERRB handling and STBY state configurations can
> + * have a look at the original RFC:
> + * https://lore.kernel.org/all/cover.1712920132.git.mazziesaccount@gmail.com/
> + * which implements a workaround to debugFS naming conflict and some of
> + * the safety limit configurations - but leaves the state change handling
> + * and synchronization to be implemented.
> + *
> + * It would be great to hear (and receive a patch!) if you implement the
> + * STBY configuration support or a proper fix to the debugFS naming
> + * conflict in your downstream driver ;)
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/mfd/core.h>
> +#include <linux/module.h>
> +#include <linux/property.h>
> +#include <linux/regmap.h>
> +#include <linux/types.h>
> +
> +#include <linux/mfd/rohm-bd96801.h>
> +#include <linux/mfd/rohm-generic.h>
> +static const struct resource regulator_intb_irqs[] = {
> + DEFINE_RES_IRQ_NAMED(BD96801_TW_STAT, "bd96801-core-thermal"),
> +
> + DEFINE_RES_IRQ_NAMED(BD96801_BUCK1_OCPH_STAT, "bd96801-buck1-overcurr-h"),
> + DEFINE_RES_IRQ_NAMED(BD96801_BUCK1_OCPL_STAT, "bd96801-buck1-overcurr-l"),
> + DEFINE_RES_IRQ_NAMED(BD96801_BUCK1_OCPN_STAT, "bd96801-buck1-overcurr-n"),
> + DEFINE_RES_IRQ_NAMED(BD96801_BUCK1_OVD_STAT, "bd96801-buck1-overvolt"),
> + DEFINE_RES_IRQ_NAMED(BD96801_BUCK1_UVD_STAT, "bd96801-buck1-undervolt"),
> + DEFINE_RES_IRQ_NAMED(BD96801_BUCK1_TW_CH_STAT, "bd96801-buck1-thermal"),
> +
> + DEFINE_RES_IRQ_NAMED(BD96801_BUCK2_OCPH_STAT, "bd96801-buck2-overcurr-h"),
> + DEFINE_RES_IRQ_NAMED(BD96801_BUCK2_OCPL_STAT, "bd96801-buck2-overcurr-l"),
> + DEFINE_RES_IRQ_NAMED(BD96801_BUCK2_OCPN_STAT, "bd96801-buck2-overcurr-n"),
> + DEFINE_RES_IRQ_NAMED(BD96801_BUCK2_OVD_STAT, "bd96801-buck2-overvolt"),
> + DEFINE_RES_IRQ_NAMED(BD96801_BUCK2_UVD_STAT, "bd96801-buck2-undervolt"),
> + DEFINE_RES_IRQ_NAMED(BD96801_BUCK2_TW_CH_STAT, "bd96801-buck2-thermal"),
> +
> + DEFINE_RES_IRQ_NAMED(BD96801_BUCK3_OCPH_STAT, "bd96801-buck3-overcurr-h"),
> + DEFINE_RES_IRQ_NAMED(BD96801_BUCK3_OCPL_STAT, "bd96801-buck3-overcurr-l"),
> + DEFINE_RES_IRQ_NAMED(BD96801_BUCK3_OCPN_STAT, "bd96801-buck3-overcurr-n"),
> + DEFINE_RES_IRQ_NAMED(BD96801_BUCK3_OVD_STAT, "bd96801-buck3-overvolt"),
> + DEFINE_RES_IRQ_NAMED(BD96801_BUCK3_UVD_STAT, "bd96801-buck3-undervolt"),
> + DEFINE_RES_IRQ_NAMED(BD96801_BUCK3_TW_CH_STAT, "bd96801-buck3-thermal"),
> +
> + DEFINE_RES_IRQ_NAMED(BD96801_BUCK4_OCPH_STAT, "bd96801-buck4-overcurr-h"),
> + DEFINE_RES_IRQ_NAMED(BD96801_BUCK4_OCPL_STAT, "bd96801-buck4-overcurr-l"),
> + DEFINE_RES_IRQ_NAMED(BD96801_BUCK4_OCPN_STAT, "bd96801-buck4-overcurr-n"),
> + DEFINE_RES_IRQ_NAMED(BD96801_BUCK4_OVD_STAT, "bd96801-buck4-overvolt"),
> + DEFINE_RES_IRQ_NAMED(BD96801_BUCK4_UVD_STAT, "bd96801-buck4-undervolt"),
> + DEFINE_RES_IRQ_NAMED(BD96801_BUCK4_TW_CH_STAT, "bd96801-buck4-thermal"),
> +
> + DEFINE_RES_IRQ_NAMED(BD96801_LDO5_OCPH_STAT, "bd96801-ldo5-overcurr"),
> + DEFINE_RES_IRQ_NAMED(BD96801_LDO5_OVD_STAT, "bd96801-ldo5-overvolt"),
> + DEFINE_RES_IRQ_NAMED(BD96801_LDO5_UVD_STAT, "bd96801-ldo5-undervolt"),
> +
> + DEFINE_RES_IRQ_NAMED(BD96801_LDO6_OCPH_STAT, "bd96801-ldo6-overcurr"),
> + DEFINE_RES_IRQ_NAMED(BD96801_LDO6_OVD_STAT, "bd96801-ldo6-overvolt"),
> + DEFINE_RES_IRQ_NAMED(BD96801_LDO6_UVD_STAT, "bd96801-ldo6-undervolt"),
> +
> + DEFINE_RES_IRQ_NAMED(BD96801_LDO7_OCPH_STAT, "bd96801-ldo7-overcurr"),
> + DEFINE_RES_IRQ_NAMED(BD96801_LDO7_OVD_STAT, "bd96801-ldo7-overvolt"),
> + DEFINE_RES_IRQ_NAMED(BD96801_LDO7_UVD_STAT, "bd96801-ldo7-undervolt"),
> +};
> +
> +static const struct resource wdg_intb_irqs[] = {
> + DEFINE_RES_IRQ_NAMED(BD96801_WDT_ERR_STAT, "bd96801-wdg"),
> +};
> +
> +enum {
> + WDG_CELL = 0,
> + REGULATOR_CELL,
> +};
Dead code?
> +static struct mfd_cell bd96801_mfd_cells[] = {
> + {
> + .name = "bd96801-wdt",
> + .resources = wdg_intb_irqs,
> + .num_resources = ARRAY_SIZE(wdg_intb_irqs),
> + }, {
> + .name = "bd96801-pmic",
> + .resources = regulator_intb_irqs,
> + .num_resources = ARRAY_SIZE(regulator_intb_irqs),
> + },
> +};
> +
> +static const struct regmap_range bd96801_volatile_ranges[] = {
> + /* Status regs */
Full words in comments please.
> + regmap_reg_range(BD96801_REG_WD_FEED, BD96801_REG_WD_FAILCOUNT),
> + regmap_reg_range(BD96801_REG_WD_ASK, BD96801_REG_WD_ASK),
> + regmap_reg_range(BD96801_REG_WD_STATUS, BD96801_REG_WD_STATUS),
> + regmap_reg_range(BD96801_REG_PMIC_STATE, BD96801_REG_INT_LDO7_INTB),
> + /* Registers which do not update value unless PMIC is in STBY */
> + regmap_reg_range(BD96801_REG_SSCG_CTRL, BD96801_REG_SHD_INTB),
> + regmap_reg_range(BD96801_REG_BUCK_OVP, BD96801_REG_BOOT_OVERTIME),
> + /*
> + * LDO control registers have single bit (LDO MODE) which does not
> + * change when we write it unless PMIC is in STBY. It's safer to not
> + * cache it.
> + */
> + regmap_reg_range(BD96801_LDO5_VOL_LVL_REG, BD96801_LDO7_VOL_LVL_REG),
> +};
> +
> +static const struct regmap_access_table volatile_regs = {
> + .yes_ranges = bd96801_volatile_ranges,
> + .n_yes_ranges = ARRAY_SIZE(bd96801_volatile_ranges),
> +};
> +
> +static const struct regmap_irq bd96801_intb_irqs[] = {
> + /* STATUS SYSTEM INTB */
> + REGMAP_IRQ_REG(BD96801_TW_STAT, 0, BD96801_TW_STAT_MASK),
> + REGMAP_IRQ_REG(BD96801_WDT_ERR_STAT, 0, BD96801_WDT_ERR_STAT_MASK),
> + REGMAP_IRQ_REG(BD96801_I2C_ERR_STAT, 0, BD96801_I2C_ERR_STAT_MASK),
> + REGMAP_IRQ_REG(BD96801_CHIP_IF_ERR_STAT, 0, BD96801_CHIP_IF_ERR_STAT_MASK),
> + /* STATUS BUCK1 INTB */
> + REGMAP_IRQ_REG(BD96801_BUCK1_OCPH_STAT, 1, BD96801_BUCK_OCPH_STAT_MASK),
> + REGMAP_IRQ_REG(BD96801_BUCK1_OCPL_STAT, 1, BD96801_BUCK_OCPL_STAT_MASK),
> + REGMAP_IRQ_REG(BD96801_BUCK1_OCPN_STAT, 1, BD96801_BUCK_OCPN_STAT_MASK),
> + REGMAP_IRQ_REG(BD96801_BUCK1_OVD_STAT, 1, BD96801_BUCK_OVD_STAT_MASK),
> + REGMAP_IRQ_REG(BD96801_BUCK1_UVD_STAT, 1, BD96801_BUCK_UVD_STAT_MASK),
> + REGMAP_IRQ_REG(BD96801_BUCK1_TW_CH_STAT, 1, BD96801_BUCK_TW_CH_STAT_MASK),
> + /* BUCK 2 INTB */
> + REGMAP_IRQ_REG(BD96801_BUCK2_OCPH_STAT, 2, BD96801_BUCK_OCPH_STAT_MASK),
> + REGMAP_IRQ_REG(BD96801_BUCK2_OCPL_STAT, 2, BD96801_BUCK_OCPL_STAT_MASK),
> + REGMAP_IRQ_REG(BD96801_BUCK2_OCPN_STAT, 2, BD96801_BUCK_OCPN_STAT_MASK),
> + REGMAP_IRQ_REG(BD96801_BUCK2_OVD_STAT, 2, BD96801_BUCK_OVD_STAT_MASK),
> + REGMAP_IRQ_REG(BD96801_BUCK2_UVD_STAT, 2, BD96801_BUCK_UVD_STAT_MASK),
> + REGMAP_IRQ_REG(BD96801_BUCK2_TW_CH_STAT, 2, BD96801_BUCK_TW_CH_STAT_MASK),
> + /* BUCK 3 INTB */
> + REGMAP_IRQ_REG(BD96801_BUCK3_OCPH_STAT, 3, BD96801_BUCK_OCPH_STAT_MASK),
> + REGMAP_IRQ_REG(BD96801_BUCK3_OCPL_STAT, 3, BD96801_BUCK_OCPL_STAT_MASK),
> + REGMAP_IRQ_REG(BD96801_BUCK3_OCPN_STAT, 3, BD96801_BUCK_OCPN_STAT_MASK),
> + REGMAP_IRQ_REG(BD96801_BUCK3_OVD_STAT, 3, BD96801_BUCK_OVD_STAT_MASK),
> + REGMAP_IRQ_REG(BD96801_BUCK3_UVD_STAT, 3, BD96801_BUCK_UVD_STAT_MASK),
> + REGMAP_IRQ_REG(BD96801_BUCK3_TW_CH_STAT, 3, BD96801_BUCK_TW_CH_STAT_MASK),
> + /* BUCK 4 INTB */
> + REGMAP_IRQ_REG(BD96801_BUCK4_OCPH_STAT, 4, BD96801_BUCK_OCPH_STAT_MASK),
> + REGMAP_IRQ_REG(BD96801_BUCK4_OCPL_STAT, 4, BD96801_BUCK_OCPL_STAT_MASK),
> + REGMAP_IRQ_REG(BD96801_BUCK4_OCPN_STAT, 4, BD96801_BUCK_OCPN_STAT_MASK),
> + REGMAP_IRQ_REG(BD96801_BUCK4_OVD_STAT, 4, BD96801_BUCK_OVD_STAT_MASK),
> + REGMAP_IRQ_REG(BD96801_BUCK4_UVD_STAT, 4, BD96801_BUCK_UVD_STAT_MASK),
> + REGMAP_IRQ_REG(BD96801_BUCK4_TW_CH_STAT, 4, BD96801_BUCK_TW_CH_STAT_MASK),
> + /* LDO5 INTB */
> + REGMAP_IRQ_REG(BD96801_LDO5_OCPH_STAT, 5, BD96801_LDO_OCPH_STAT_MASK),
> + REGMAP_IRQ_REG(BD96801_LDO5_OVD_STAT, 5, BD96801_LDO_OVD_STAT_MASK),
> + REGMAP_IRQ_REG(BD96801_LDO5_UVD_STAT, 5, BD96801_LDO_UVD_STAT_MASK),
> + /* LDO6 INTB */
> + REGMAP_IRQ_REG(BD96801_LDO6_OCPH_STAT, 6, BD96801_LDO_OCPH_STAT_MASK),
> + REGMAP_IRQ_REG(BD96801_LDO6_OVD_STAT, 6, BD96801_LDO_OVD_STAT_MASK),
> + REGMAP_IRQ_REG(BD96801_LDO6_UVD_STAT, 6, BD96801_LDO_UVD_STAT_MASK),
> + /* LDO7 INTB */
> + REGMAP_IRQ_REG(BD96801_LDO7_OCPH_STAT, 7, BD96801_LDO_OCPH_STAT_MASK),
> + REGMAP_IRQ_REG(BD96801_LDO7_OVD_STAT, 7, BD96801_LDO_OVD_STAT_MASK),
> + REGMAP_IRQ_REG(BD96801_LDO7_UVD_STAT, 7, BD96801_LDO_UVD_STAT_MASK),
> +};
> +
> +static struct regmap_irq_chip bd96801_irq_chip_intb = {
> + .name = "bd96801-irq-intb",
> + .main_status = BD96801_REG_INT_MAIN,
> + .num_main_regs = 1,
> + .irqs = &bd96801_intb_irqs[0],
> + .num_irqs = ARRAY_SIZE(bd96801_intb_irqs),
> + .status_base = BD96801_REG_INT_SYS_INTB,
> + .mask_base = BD96801_REG_MASK_SYS_INTB,
> + .ack_base = BD96801_REG_INT_SYS_INTB,
> + .init_ack_masked = true,
> + .num_regs = 8,
> + .irq_reg_stride = 1,
> +};
> +
> +static const struct regmap_config bd96801_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> + .volatile_table = &volatile_regs,
> + .cache_type = REGCACHE_RBTREE,
> +};
> +
> +static int bd96801_i2c_probe(struct i2c_client *i2c)
> +{
> + struct regmap *regmap;
> + int ret, intb_irq;
Nit: Pop these at the bottom.
> + struct regmap_irq_chip_data *intb_irq_data;
> + struct irq_domain *intb_domain;
> + const struct fwnode_handle *fwnode;
> +
> + fwnode = dev_fwnode(&i2c->dev);
> + if (!fwnode)
> + return dev_err_probe(&i2c->dev, -EINVAL, "no fwnode\n");
Make error messages consistent and clear.
"Failed to find fwnode"
> + intb_irq = fwnode_irq_get_byname(fwnode, "intb");
> + if (intb_irq < 0)
> + return dev_err_probe(&i2c->dev, intb_irq, "No INTB IRQ configured\n");
I don't know what it is about the "no ..." comments that I don't like.
"INTB IRQ not configured" just sounds more professional.
> + regmap = devm_regmap_init_i2c(i2c, &bd96801_regmap_config);
> + if (IS_ERR(regmap))
> + return dev_err_probe(&i2c->dev, PTR_ERR(regmap),
> + "regmap initialization failed\n");
Your other errors start with an uppercase char. Consistency.
> + ret = regmap_write(regmap, BD96801_LOCK_REG, BD96801_UNLOCK);
> + if (ret)
> + return dev_err_probe(&i2c->dev, ret, "Can't unlock PMIC\n");
"Failed to ..." like everywhere else.
> + ret = devm_regmap_add_irq_chip(&i2c->dev, regmap, intb_irq,
> + IRQF_ONESHOT, 0, &bd96801_irq_chip_intb,
> + &intb_irq_data);
> + if (ret)
> + return dev_err_probe(&i2c->dev, ret, "Failed to add INTB irq_chip\n");
"INTB IRQ chip", it's not a variable and this is user facing.
> + intb_domain = regmap_irq_get_domain(intb_irq_data);
> +
> + ret = devm_mfd_add_devices(&i2c->dev, PLATFORM_DEVID_AUTO,
> + bd96801_mfd_cells,
> + ARRAY_SIZE(bd96801_mfd_cells), NULL, 0,
> + intb_domain);
> +
> + if (ret)
> + dev_err(&i2c->dev, "Failed to create subdevices\n");
> +
> + return ret;
> +}
> +
> +static const struct of_device_id bd96801_of_match[] = {
> + { .compatible = "rohm,bd96801", },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, bd96801_of_match);
> +
> +static struct i2c_driver bd96801_i2c_driver = {
> + .driver = {
> + .name = "rohm-bd96801",
> + .of_match_table = bd96801_of_match,
> + },
> + .probe = bd96801_i2c_probe,
> +};
> +
> +static int __init bd96801_i2c_init(void)
> +{
> + return i2c_add_driver(&bd96801_i2c_driver);
> +}
> +
> +/* Initialise early so consumer devices can complete system boot? */
Why the question mark? What are you unsure about?
Why doesn't -EPROBE_DEFER work for this?
> +subsys_initcall(bd96801_i2c_init);
> +
> +static void __exit bd96801_i2c_exit(void)
> +{
> + i2c_del_driver(&bd96801_i2c_driver);
> +}
> +module_exit(bd96801_i2c_exit);
> +
> +MODULE_AUTHOR("Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>");
> +MODULE_DESCRIPTION("ROHM BD96801 Power Management IC driver");
> +MODULE_LICENSE("GPL");
--
Lee Jones [李琼斯]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 3/6] mfd: support ROHM BD96801 PMIC core
2024-05-03 7:41 ` Lee Jones
@ 2024-05-03 8:43 ` Matti Vaittinen
0 siblings, 0 replies; 14+ messages in thread
From: Matti Vaittinen @ 2024-05-03 8:43 UTC (permalink / raw)
To: Lee Jones
Cc: Matti Vaittinen, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Liam Girdwood, Mark Brown, Wim Van Sebroeck, Guenter Roeck,
devicetree, linux-kernel, linux-watchdog
On 5/3/24 10:41, Lee Jones wrote:
> On Tue, 30 Apr 2024, Matti Vaittinen wrote:
>
>> The ROHM BD96801 PMIC is highly customizable automotive grade PMIC
>> which integrates regulator and watchdog funtionalities.
>>
>> Provide INTB IRQ and register accesses for regulator/watchdog drivers.
>>
>> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
>>
>> ---
>> Changelog: RFCv2 => v1:
>> - drop ERRB interrupts (for now)
>> - bd96801: Unlock registers in core driver
>>
>> Changelog: RFCv1 => RFCv2
>> - Work-around the IRQ domain name conflict
>> - Add watchdog IRQ
>> - Various styling fixes based on review by Lee
>> ---
>> drivers/mfd/Kconfig | 13 ++
>> drivers/mfd/Makefile | 1 +
>> drivers/mfd/rohm-bd96801.c | 278 +++++++++++++++++++++++++++++++
>> include/linux/mfd/rohm-bd96801.h | 215 ++++++++++++++++++++++++
>> include/linux/mfd/rohm-generic.h | 1 +
>> 5 files changed, 508 insertions(+)
>> create mode 100644 drivers/mfd/rohm-bd96801.c
>> create mode 100644 include/linux/mfd/rohm-bd96801.h
>
> Still some nits, sorry.
No need to be sorry :) And, thanks for the review!
> I probably wouldn't have been so picky if I hadn't have found the unused enum.
Yeah, that's what you say ... XD
It's Ok. Besides, I like your style of providing the better alternatives
along with your comments. (ref the print comments which I agreed with
and dropped from this reply).
>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
>> index 4b023ee229cf..9e874453d94d 100644
>> --- a/drivers/mfd/Kconfig
>> +++ b/drivers/mfd/Kconfig
>> @@ -2089,6 +2089,19 @@ config MFD_ROHM_BD957XMUF
>> BD9573MUF Power Management ICs. BD9576 and BD9573 are primarily
>> designed to be used to power R-Car series processors.
>>
>> +config MFD_ROHM_BD96801
>> + tristate "ROHM BD96801 Power Management IC"
>> + depends on I2C=y
>> + depends on OF
>> + select REGMAP_I2C
>> + select REGMAP_IRQ
>> + select MFD_CORE
>> + help
>> + Select this option to get support for the ROHM BD96801 Power
>> + Management IC. The ROHM BD96801 is a highly scalable Power Management
>> + IC for industrial and automotive use. The BD96801 can be used as a
>> + master PMIC in a chained PMIC solution with suitable companion PMICs.
>> +
>> config MFD_STM32_LPTIMER
>> tristate "Support for STM32 Low-Power Timer"
>> depends on (ARCH_STM32 && OF) || COMPILE_TEST
>> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
>> index c66f07edcd0e..e792892d4a8b 100644
>> --- a/drivers/mfd/Makefile
>> +++ b/drivers/mfd/Makefile
>> @@ -264,6 +264,7 @@ obj-$(CONFIG_RAVE_SP_CORE) += rave-sp.o
>> obj-$(CONFIG_MFD_ROHM_BD71828) += rohm-bd71828.o
>> obj-$(CONFIG_MFD_ROHM_BD718XX) += rohm-bd718x7.o
>> obj-$(CONFIG_MFD_ROHM_BD957XMUF) += rohm-bd9576.o
>> +obj-$(CONFIG_MFD_ROHM_BD96801) += rohm-bd96801.o
>> obj-$(CONFIG_MFD_STMFX) += stmfx.o
>> obj-$(CONFIG_MFD_KHADAS_MCU) += khadas-mcu.o
>> obj-$(CONFIG_MFD_ACER_A500_EC) += acer-ec-a500.o
>> diff --git a/drivers/mfd/rohm-bd96801.c b/drivers/mfd/rohm-bd96801.c
>> new file mode 100644
>> index 000000000000..1e9c49c857c1
>> --- /dev/null
>> +++ b/drivers/mfd/rohm-bd96801.c
...
>> +
>> +enum {
>> + WDG_CELL = 0,
>> + REGULATOR_CELL,
>> +};
>
> Dead code?
Yep. A leftover from the version with the ERRB stuff. Thanks for
pointing it out!
...
>> +
>> +static int __init bd96801_i2c_init(void)
>> +{
>> + return i2c_add_driver(&bd96801_i2c_driver);
>> +}
>> +
>> +/* Initialise early so consumer devices can complete system boot? */
>
> Why the question mark? What are you unsure about?
>
> Why doesn't -EPROBE_DEFER work for this?
I am unsure of what kind of dependencies the real setups using these
PMICs have at boot time. Hence the (?). I've written drivers for a few
PMICs, and I've sometimes just done the usual:
module_i2c_driver();
This, however, tends to get converted to boilerplate + subsys_initcall()
by customers who actually start using the drivers - so I believe there
is a problem in getting the consumers powered if the PMIC(s) are
initialized too late. Anyways, it's probably nicer to try making it so
it works better on different setups out of the box :)
I'd appreciate if someone wanted to shed some more light on this though!
Yours,
-- Matti
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2024-05-03 8:43 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-30 11:59 [PATCH v1 0/6] Support ROHM BD96801 scalable PMIC Matti Vaittinen
2024-04-30 11:59 ` [PATCH v1 1/6] dt-bindings: ROHM BD96801 PMIC regulators Matti Vaittinen
2024-05-02 16:20 ` Conor Dooley
2024-05-03 4:54 ` Matti Vaittinen
2024-05-03 7:25 ` Matti Vaittinen
2024-04-30 12:00 ` [PATCH v1 2/6] dt-bindings: mfd: bd96801 PMIC core Matti Vaittinen
2024-04-30 12:00 ` [PATCH v1 3/6] mfd: support ROHM BD96801 " Matti Vaittinen
2024-05-03 7:41 ` Lee Jones
2024-05-03 8:43 ` Matti Vaittinen
2024-04-30 12:01 ` [PATCH v1 4/6] regulator: bd96801: ROHM BD96801 PMIC regulators Matti Vaittinen
2024-04-30 12:01 ` [PATCH v1 5/6] watchdog: ROHM BD96801 PMIC WDG driver Matti Vaittinen
2024-04-30 16:24 ` Guenter Roeck
2024-05-01 10:16 ` Matti Vaittinen
2024-04-30 12:02 ` [PATCH v1 6/6] MAINTAINERS: Add ROHM BD96801 'scalable PMIC' entries Matti Vaittinen
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).