Support ROHM BD73800

From: Matti Vaittinen <[email protected]>

Add bindings for the BUCKs and LDOs on ROHM BD73800. The PMIC state specific voltages can be set in same fashion as with a few other ROHM PMICs (for example with BD718[15,28,37,47,50,79]). Same properties are recycled :)

The LDOs 1 and 4 can use different voltage ranges depending on the OTP configuration.

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

---
 .../regulator/rohm,bd73800-regulator.yaml     | 119 ++++++++++++++++++
 1 file changed, 119 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/rohm,bd73800-regulator.yaml

From: Matti Vaittinen <[email protected]>

Add bindings for the BUCKs and LDOs on ROHM BD73800. The PMIC state specific voltages can be set in same fashion as with a few other ROHM PMICs (for example with BD718[15,28,37,47,50,79]). Same properties are recycled :)

The LDOs 1 and 4 can use different voltage ranges depending on the OTP configuration.

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

---
 .../regulator/rohm,bd73800-regulator.yaml     | 119 ++++++++++++++++++
 1 file changed, 119 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/rohm,bd73800-regulator.yaml

Documentation/devicetree/bindings/regulator/rohm,bd73800-regulator.yaml new file +7 -0

new file mode 100644
index 000000000000..c427a04098ec
 
 
+      rohm,dvs-run-voltage:
+        description:
+          PMIC default "RUN" state voltage in uV. 0 means disabled. See the
+          explanation below for regulator specific details.
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 0
+        maximum: 3500000

Quoted 30 lines · Matti Vaittinen

From: Matti Vaittinen <[email protected]>

Add bindings for the BUCKs and LDOs on ROHM BD73800. The PMIC state specific voltages can be set in same fashion as with a few other ROHM PMICs (for example with BD718[15,28,37,47,50,79]). Same properties are recycled :)

The LDOs 1 and 4 can use different voltage ranges depending on the OTP configuration.

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

---
  .../regulator/rohm,bd73800-regulator.yaml     | 119 ++++++++++++++++++
  1 file changed, 119 insertions(+)

create mode 100644 Documentation/devicetree/bindings/regulator/rohm,bd73800-regulator.yaml

Documentation/devicetree/bindings/regulator/rohm,bd73800-regulator.yaml new file +7 -0

new file mode 100644
index 000000000000..c427a04098ec
 
 
+      rohm,dvs-run-voltage:
+        description:
+          PMIC default "RUN" state voltage in uV. 0 means disabled. See the
+          explanation below for regulator specific details.
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 0
+        maximum: 3500000

[...]

Documentation/devicetree/bindings/regulator/rohm,bd73800-regulator.yaml line 88

Quoted 7 lines

Documentation/devicetree/bindings/regulator/rohm,bd73800-regulator.yaml new file +7 -0

+      rohm,dvs-run-voltage:
+        description:
+          Set the default output state at PMIC's "RUN" state.
+          0 is disabled, 1 is enabled.
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 0
+        maximum: 1

Same property name with 2 different meanings. Not a good design pattern.

Also, if these properties are copied from other schemas, don't duplicate them. Put them in a common schema and reference it here.

Documentation/devicetree/bindings/mfd/rohm,bd73800-pmic.yaml new file +11 -0

+description:
+  BD73800GW is a single-chip power management IC for battery-powered
+  portable devices. It integrates 8 buck converters, 4 LDOs and a current
+  sense amplifier with ADC. Also included is a Real Time Clock (RTC) and a
+  32.768 kHz clock gate. Depending on the OTP configuration the BD73800
+  may also have interrupt controller and GPIOs.
+
+  There are also different variants called BD71851 and BD71885 which are
+  almost identical from the software point of view.
+
+properties:

Documentation/devicetree/bindings/mfd/rohm,bd73800-pmic.yaml new file +15 -0

+          - const: rohm,bd73800
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  # The GPIO1, CLKOUT (GPIO2), FAULT_B and EXTEN_OUT pins can be
+  # configured to interrupt pins by OTP.
+  interrupt-controller: true
+
+  "#interrupt-cells":
+    const: 1
+    description:

Documentation/devicetree/bindings/mfd/rohm,bd73800-pmic.yaml new file +24 -0

+# specific purpose (like ADC input, 32.768 clk output, fault indicator or
+# delivering power sequence to a companion PMIC when multiple PMICs are
+# used) - but also to be either a GPO or GPI. (When used as a GPI the pin
+# can also be used as an IRQ source). The pin purpose is determined by
+# OTP (One Time Programmable memory), typically during device manufacturing.
+# The OTP can't be read at runtime so device-tree should describe the pins.
+  rohm,pin-gpio1:
+    $ref: /schemas/types.yaml#/definitions/string
+    description:
+      Indicate if the GPIO1 pin has been set to GPI or GPO at manufacturing.
+    enum: [gpi, gpo]
+
+  rohm,pin-clkout:
+    $ref: /schemas/types.yaml#/definitions/string
+    description:
+      Indicate if the CLKOUT pin has been set to GPI or GPO at manufacturing.
+    enum: [gpi, gpo]
+
+  rohm,pin-fault-b:
+    $ref: /schemas/types.yaml#/definitions/string
+    description:
+      Indicate if the FAULT_B pin has been set to GPI or GPO at manufacturing.
+    enum: [gpi, gpo]
+

Quoted 11 lines · Matti Vaittinen

Documentation/devicetree/bindings/mfd/rohm,bd73800-pmic.yaml new file +11 -0

+description:
+  BD73800GW is a single-chip power management IC for battery-powered
+  portable devices. It integrates 8 buck converters, 4 LDOs and a current
+  sense amplifier with ADC. Also included is a Real Time Clock (RTC) and a
+  32.768 kHz clock gate. Depending on the OTP configuration the BD73800
+  may also have interrupt controller and GPIOs.
+
+  There are also different variants called BD71851 and BD71885 which are
+  almost identical from the software point of view.
+
+properties:

Documentation/devicetree/bindings/mfd/rohm,bd73800-pmic.yaml line 32

Quoted 10 lines

Documentation/devicetree/bindings/mfd/rohm,bd73800-pmic.yaml new file +10 -0

+  compatible:
+    oneOf:
+      - const: rohm,bd73800
+
+      - items:
+          - const: rohm,bd71851
+          - const: rohm,bd73800
+
+      - items:
+          - const: rohm,bd71885

items:
- enum:

       -
       -

- const: rohm,bd73800

Quoted 24 lines

Documentation/devicetree/bindings/mfd/rohm,bd73800-pmic.yaml new file +23 -0

+          - const: rohm,bd73800
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  # The GPIO1, CLKOUT (GPIO2), FAULT_B and EXTEN_OUT pins can be
+  # configured to interrupt pins by OTP.
+  interrupt-controller: true
+
+  "#interrupt-cells":
+    const: 1
+    description:
+      The IRQ number. 0 is GPIO1, 1 CLKOUT (GPIO2), 2 FAULT_B and 3 EXTEN_OUT.
+      NOTE, A pin can operate as IRQ source only when the OTP
+      configuration for it has been set to GPI.
+
+  gpio-controller: true
+
+  "#gpio-cells":
+    const: 2

Review

Quoted 2 lines

(quoted diff) +2 -0

+
+# The GPIO1, CLKOUT, FAULT_B and EXTEN_OUT pins may be configured for a

Missing two spaces (indent) before the comment, although this should be put into description. I understand it applies to each description - it is fine to add to the first one. Descriptions might be used to generate user-friendly representation of bindings (PDF). Comments won't, so comments are only to explain the binding/schema syntax choices.

Documentation/devicetree/bindings/mfd/rohm,bd73800-pmic.yaml new file +21 -0

+# The GPIO1, CLKOUT, FAULT_B and EXTEN_OUT pins may be configured for a
+# specific purpose (like ADC input, 32.768 clk output, fault indicator or
+# delivering power sequence to a companion PMIC when multiple PMICs are
+# used) - but also to be either a GPO or GPI. (When used as a GPI the pin
+# can also be used as an IRQ source). The pin purpose is determined by
+# OTP (One Time Programmable memory), typically during device manufacturing.
+# The OTP can't be read at runtime so device-tree should describe the pins.
+  rohm,pin-gpio1:
+    $ref: /schemas/types.yaml#/definitions/string
+    description:
+      Indicate if the GPIO1 pin has been set to GPI or GPO at manufacturing.
+    enum: [gpi, gpo]
+
+  rohm,pin-clkout:
+    $ref: /schemas/types.yaml#/definitions/string
+    description:
+      Indicate if the CLKOUT pin has been set to GPI or GPO at manufacturing.
+    enum: [gpi, gpo]
+
+  rohm,pin-fault-b:
+    $ref: /schemas/types.yaml#/definitions/string

Hi Matti,

thanks for your patch!

I have some inevitable pin control nitpicks!

Documentation/devicetree/bindings/mfd/rohm,bd73800-pmic.yaml line 42

Quoted 2 lines · Matti Vaittinen

Documentation/devicetree/bindings/mfd/rohm,bd73800-pmic.yaml new file +2 -0

+  # The GPIO1, CLKOUT (GPIO2), FAULT_B and EXTEN_OUT pins can be
+  # configured to interrupt pins by OTP.

Maybe move this helpful comment into the top description: instead? It's kind of generic helpful info.

Quoted 22 lines

Documentation/devicetree/bindings/mfd/rohm,bd73800-pmic.yaml new file +21 -0

+# The GPIO1, CLKOUT, FAULT_B and EXTEN_OUT pins may be configured for a
+# specific purpose (like ADC input, 32.768 clk output, fault indicator or
+# delivering power sequence to a companion PMIC when multiple PMICs are
+# used) - but also to be either a GPO or GPI. (When used as a GPI the pin
+# can also be used as an IRQ source). The pin purpose is determined by
+# OTP (One Time Programmable memory), typically during device manufacturing.
+# The OTP can't be read at runtime so device-tree should describe the pins.
+  rohm,pin-gpio1:
+    $ref: /schemas/types.yaml#/definitions/string
+    description:
+      Indicate if the GPIO1 pin has been set to GPI or GPO at manufacturing.
+    enum: [gpi, gpo]
+
+  rohm,pin-clkout:
+    $ref: /schemas/types.yaml#/definitions/string
+    description:
+      Indicate if the CLKOUT pin has been set to GPI or GPO at manufacturing.
+    enum: [gpi, gpo]
+
+  rohm,pin-fault-b:
+    $ref: /schemas/types.yaml#/definitions/string

Documentation/devicetree/bindings/mfd/rohm,bd73800-pmic.yaml line 87

Quoted 10 lines

Documentation/devicetree/bindings/mfd/rohm,bd73800-pmic.yaml new file +10 -0

+    description:
+      Indicate if the FAULT_B pin has been set to GPI or GPO at manufacturing.
+    enum: [gpi, gpo]
+
+  rohm,pin-exten:
+    $ref: /schemas/types.yaml#/definitions/string
+    description:
+      Indicate if the EXTEN_OUT pin has been set to GPI or GPO at
+      manufacturing.
+    enum: [gpi, gpo]

Can we explain what "GPI" and "GPO" means in this context?

I read it as "general purpose input" and "general purpose output", but... you just describe the exact purpose? So what is "general purpose" about them in that case?

I would re-use "input-enable" and "output-enable" from:
Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml (I mean don't $rf that, just use these strings).

I suppose:
enum: [input-enable, output-enable]

Documentation/devicetree/bindings/mfd/rohm,bd73800-pmic.yaml line 105

Quoted 6 lines

Documentation/devicetree/bindings/mfd/rohm,bd73800-pmic.yaml new file +5 -0

+  rohm,clkout-open-drain:
+    description: clk32kout mode. Set to 1 for "open-drain" or 0 for "cmos".
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 0
+    maximum: 1

Here I would also reuse the generic pinconf properties, something like;

rohm,clkout-drive-type:
enum: [drive-push-pull, drive-open-drain]

(Push-pull is what is colloquially referred to as "cmos".)

Documentation/devicetree/bindings/mfd/rohm,bd73800-pmic.yaml line 142

Quoted 2 lines

Documentation/devicetree/bindings/mfd/rohm,bd73800-pmic.yaml new file +2 -0

+            rohm,pin-gpio1 = "gpo";
+            rohm,pin-exten = "gpi";

If you instead use nodes with properties you can do this:

rohm,pin-clkout {
    output-enable;
    drive-push-pull;
};

This collects the clkout config in one place and make it obvious what is going on. But I don't know what the DT maintainers think about this idea.

Documentation/devicetree/bindings/mfd/rohm,bd73800-pmic.yaml line 42

Quoted 2 lines · Matti Vaittinen

Documentation/devicetree/bindings/mfd/rohm,bd73800-pmic.yaml new file +2 -0

+  # The GPIO1, CLKOUT (GPIO2), FAULT_B and EXTEN_OUT pins can be
+  # configured to interrupt pins by OTP.

Maybe move this helpful comment into the top description: instead? It's kind of generic helpful info.

Quoted 21 lines

Documentation/devicetree/bindings/mfd/rohm,bd73800-pmic.yaml new file +21 -0

+# The GPIO1, CLKOUT, FAULT_B and EXTEN_OUT pins may be configured for a
+# specific purpose (like ADC input, 32.768 clk output, fault indicator or
+# delivering power sequence to a companion PMIC when multiple PMICs are
+# used) - but also to be either a GPO or GPI. (When used as a GPI the pin
+# can also be used as an IRQ source). The pin purpose is determined by
+# OTP (One Time Programmable memory), typically during device manufacturing.
+# The OTP can't be read at runtime so device-tree should describe the pins.
+  rohm,pin-gpio1:
+    $ref: /schemas/types.yaml#/definitions/string
+    description:
+      Indicate if the GPIO1 pin has been set to GPI or GPO at manufacturing.
+    enum: [gpi, gpo]
+
+  rohm,pin-clkout:
+    $ref: /schemas/types.yaml#/definitions/string
+    description:
+      Indicate if the CLKOUT pin has been set to GPI or GPO at manufacturing.
+    enum: [gpi, gpo]
+
+  rohm,pin-fault-b:
+    $ref: /schemas/types.yaml#/definitions/string

Documentation/devicetree/bindings/mfd/rohm,bd73800-pmic.yaml line 87

Quoted 10 lines

Documentation/devicetree/bindings/mfd/rohm,bd73800-pmic.yaml new file +10 -0

+    description:
+      Indicate if the FAULT_B pin has been set to GPI or GPO at manufacturing.
+    enum: [gpi, gpo]
+
+  rohm,pin-exten:
+    $ref: /schemas/types.yaml#/definitions/string
+    description:
+      Indicate if the EXTEN_OUT pin has been set to GPI or GPO at
+      manufacturing.
+    enum: [gpi, gpo]

Can we explain what "GPI" and "GPO" means in this context?

I read it as "general purpose input" and "general purpose output", but... you just describe the exact purpose? So what is "general purpose" about them in that case?

I would re-use "input-enable" and "output-enable" from:
Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml (I mean don't $rf that, just use these strings).

I suppose:
enum: [input-enable, output-enable]

Documentation/devicetree/bindings/mfd/rohm,bd73800-pmic.yaml line 105

Quoted 5 lines

Documentation/devicetree/bindings/mfd/rohm,bd73800-pmic.yaml new file +5 -0

+  rohm,clkout-open-drain:
+    description: clk32kout mode. Set to 1 for "open-drain" or 0 for "cmos".
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 0
+    maximum: 1

Here I would also reuse the generic pinconf properties, something like;

rohm,clkout-drive-type:
enum: [drive-push-pull, drive-open-drain]

(Push-pull is what is colloquially referred to as "cmos".)

Documentation/devicetree/bindings/mfd/rohm,bd73800-pmic.yaml line 142

Quoted 3 lines

Documentation/devicetree/bindings/mfd/rohm,bd73800-pmic.yaml new file +2 -0

+            rohm,pin-gpio1 = "gpo";
+            rohm,pin-exten = "gpi";

If you instead use nodes with properties you can do this:

rohm,pin-clkout {
     output-enable;
     drive-push-pull;
};

This collects the clkout config in one place and make it obvious what is going on. But I don't know what the DT maintainers think about this idea.

4 GPIOs
- OTP Option for GPIOs instead of I/F signals:
EXTEN_OUT, FAULT_B, CLKOUT, GPIO1
- GPO Supports Open Drain and CMOS Output

From: Matti Vaittinen <[email protected]>

The BD73800 integrates regulators, ADC (intended for accumulating current / voltage / power values), a real-time clock (RTC), clock gate and GPIOs.

Add core support for ROHM BD73800 Power Management IC.

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

---
 drivers/mfd/Kconfig              |  15 +-
 drivers/mfd/rohm-bd71828.c       | 145 ++++++++++++++-
 include/linux/mfd/rohm-bd73800.h | 307 +++++++++++++++++++++++++++++++
 include/linux/mfd/rohm-generic.h |   1 +
 4 files changed, 461 insertions(+), 7 deletions(-)
 create mode 100644 include/linux/mfd/rohm-bd73800.h

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

index a79f354bf5cb..31637777b627 100644

drivers/mfd/rohm-bd71828.c +1 -1

- * ROHM BD718[15/28/79] and BD72720 PMIC driver
+ * ROHM BD718[15/28/79], BD72720 and BD73[8/9]00 PMIC driver
  */
 
 #include <linux/gpio_keys.h>

@@ -249,6 +256,17 @@ static const struct mfd_cell bd72720_mfd_cells[] = {
 	},
 };
 

drivers/mfd/rohm-bd71828.c +3 -0

+	.n_no_ranges = ARRAY_SIZE(bd73800_read_only_ranges),
+};
+
 static const struct regmap_config bd71815_regmap = {
 	.reg_bits = 8,
 	.val_bits = 8,

drivers/mfd/rohm-bd71828.c +15 -0

+	.num_irqs = ARRAY_SIZE(bd73800_irqs),
+	.status_base = BD73800_REG_INT_1_STAT,
+	.unmask_base = BD73800_REG_INT_1_EN,
+	.ack_base = BD73800_REG_INT_1_STAT,
+	.init_ack_masked = true,
+	.num_regs = 5,
+	.num_main_regs = 1,
+	/*
+	 * Ignore mirrored bits [7:5]. They are handled as part of normal INT_4
+	 * handling.
+	 */
+	.num_main_status_bits = 5,
+	.irq_reg_stride = 1,
+};
+
 static int set_clk_mode(struct device *dev, struct regmap *regmap,
 			int clkmode_reg)
 {

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

new file mode 100644
index 000000000000..4bceb20ac6b1

include/linux/mfd/rohm-bd73800.h new file +8 -0

+ * Author: Matti Vaittinen <[email protected]>
+ */
+
+#ifndef _MFD_BD73800_H
+#define _MFD_BD73800_H
+
+#include <linux/regmap.h>
+

/* Sashiko Automation: Reviewed (0 Findings) */

Quoted 16 lines · Matti Vaittinen

From: Matti Vaittinen <[email protected]>

The BD73800 integrates regulators, ADC (intended for accumulating current / voltage / power values), a real-time clock (RTC), clock gate and GPIOs.

Add core support for ROHM BD73800 Power Management IC.

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

---
 drivers/mfd/Kconfig              |  15 +-
 drivers/mfd/rohm-bd71828.c       | 145 ++++++++++++++-
 include/linux/mfd/rohm-bd73800.h | 307 +++++++++++++++++++++++++++++++
 include/linux/mfd/rohm-generic.h |   1 +
 4 files changed, 461 insertions(+), 7 deletions(-)
 create mode 100644 include/linux/mfd/rohm-bd73800.h

From: Matti Vaittinen <[email protected]>

BD73800 contains similar RTC block as BD71828 and BD71815. Only the address offsets seem different. Support also BD73800 RTC using the rtc-bd70528.

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

---
 drivers/rtc/rtc-bd70528.c | 8 ++++++++
 1 file changed, 8 insertions(+)

drivers/rtc/rtc-bd70528.c +8 -0

index 482810b61495..fd415e327ea6 100644
@@ -8,6 +8,7 @@
 #include <linux/mfd/rohm-bd71815.h>
 #include <linux/mfd/rohm-bd71828.h>
 #include <linux/mfd/rohm-bd72720.h>
+#include <linux/mfd/rohm-bd73800.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>

@@ -284,6 +285,12 @@ static int bd70528_probe(struct platform_device *pdev)
 		bd_rtc->bd718xx_alm_block_start = BD72720_REG_RTC_ALM_START;
 		hour_reg = BD72720_REG_RTC_HOUR;
 		break;
+	case ROHM_CHIP_TYPE_BD73800:
+		bd_rtc->reg_time_start = BD73800_REG_RTC_START;
+		bd_rtc->bd718xx_alm_block_start = BD73800_REG_RTC_ALM_START;
+		hour_reg = BD73800_REG_RTC_HOUR;
+		break;
+
 	default:
 		dev_err(&pdev->dev, "Unknown chip\n");
 		return -ENOENT;

@@ -344,6 +351,7 @@ static const struct platform_device_id bd718x7_rtc_id[] = {
 	{ .name = "bd71828-rtc", .driver_data = ROHM_CHIP_TYPE_BD71828 },
 	{ .name = "bd71815-rtc", .driver_data = ROHM_CHIP_TYPE_BD71815 },
 	{ .name = "bd72720-rtc", .driver_data = ROHM_CHIP_TYPE_BD72720 },
+	{ .name = "bd73800-rtc", .driver_data = ROHM_CHIP_TYPE_BD73800 },
 	{ }
 };
 MODULE_DEVICE_TABLE(platform, bd718x7_rtc_id);

Signature · 1 line

2.54.0

From: Matti Vaittinen <[email protected]>

drivers/regulator/bd71828-regulator.c +3 -0

+	nproot = of_get_child_by_name(nproot, "regulators");
+	if (!nproot) {
+		dev_err(dev, "failed to find regulators node\n");

Quoted 5 lines · Matti Vaittinen

From: Matti Vaittinen <[email protected]>

drivers/regulator/bd71828-regulator.c +3 -0

+	nproot = of_get_child_by_name(nproot, "regulators");
+	if (!nproot) {
+		dev_err(dev, "failed to find regulators node\n");

drivers/regulator/bd71828-regulator.c static int bd72720_dt_parse(struct device *dev, · line 2141

Quoted 10 lines

drivers/regulator/bd71828-regulator.c +10 -0

+		return -ENODEV;
+	}
+	for_each_child_of_node(nproot, np) {
+		if (of_node_name_eq(np, LDO1_NODE_NAME))
+			ldo1_use_high_range = of_property_read_bool(np,
+							"rohm,ldo-range-high");
+		if (of_node_name_eq(np, LDO3_NODE_NAME))
+			ldo3_use_high_range = of_property_read_bool(np,
+							"rohm,ldo-range-high");
+	}

Why do we iterate over all nodes rather than doing additional of_get_child_by_name()s?

Quoted 4 lines

drivers/regulator/bd71828-regulator.c +4 -0

+	if (ldo1_use_high_range) {
+		d[BD73800_LDO1].desc.linear_ranges = bd73800_ldo13_high_volts;
+		d[BD73800_LDO1].desc.n_linear_ranges =
+					ARRAY_SIZE(bd73800_ldo13_high_volts);

drivers/regulator/bd71828-regulator.c static int bd72720_dt_parse(struct device *dev, · line 2153

Quoted 6 lines

drivers/regulator/bd71828-regulator.c +6 -0

+	}
+	if (ldo3_use_high_range) {
+		d[BD73800_LDO3].desc.linear_ranges = bd73800_ldo13_high_volts;
+		d[BD73800_LDO3].desc.n_linear_ranges =
+					ARRAY_SIZE(bd73800_ldo13_high_volts);
+	}

You could just do these updates without the intermediate variables.

From: Matti Vaittinen <[email protected]>

The ROHM BD73800 PMIC has 4 pins (named GPIO1, CLKOUT, FAULT_B and EXTEN_OUT) which might have been set to operate as a GPI or GPO when OTP (One Time Programmable memory) is written at device manufacturing. Support the GPI/GPO use-case via GPIO framework.

The default OTP for these pins is to not use any of them as GPI or GPO. (The GPIO1 defaults as an ADC input regardless the naming). Hence the driver assumes none of these pins is a GPI/GPO unless explicitly pointed as GPI or GPO via device tree.

Furthermore, pin's direction can't be changed after OTP configuration is done. Also the default drive type for a GPO (CMOS / Open Drain) is set by the OTP configuration. The BD73800 has a set of undocumented test registers which should allow changing the drive type. Access to the test register area or the test registers aren't documented and so this driver does not support configuring the drive type even though it might be doable.

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

---
 drivers/gpio/Kconfig        |  11 ++
 drivers/gpio/Makefile       |   1 +
 drivers/gpio/gpio-bd73800.c | 234 ++++++++++++++++++++++++++++++++++++
 3 files changed, 246 insertions(+)
 create mode 100644 drivers/gpio/gpio-bd73800.c

drivers/gpio/Kconfig +11 -0

index 28cf6d2e83c2..09d87c3b756f 100644
@@ -1363,6 +1363,17 @@ config GPIO_BD72720
 	  be configured to GPO on the ROHM PMIC. The pin configuration is done
 	  on OTP at manufacturing.
 
+config GPIO_BD73800
+	tristate "ROHM BD73800 GPIO support"
+	depends on MFD_ROHM_BD71828
+	help
+	  Support for GPIOs on ROHM BD73800 PMIC. There can be up to 4
+	  GPI or GPO pins available on the PMIC in total. The purpose of
+	  the pins is decided at the device manufacturing by OTP
+	  configuration and can't be reconfigured later. Enable this
+	  if your PMIC has pins set as GPIs or GPOs and if you wish to
+	  control the pins via the GPIO framework.
+
 config GPIO_BD9571MWV
 	tristate "ROHM BD9571 GPIO support"
 	depends on MFD_BD9571MWV

drivers/gpio/Makefile +1 -0

index 4d0e900402fc..3041c06aa933 100644
@@ -45,6 +45,7 @@ obj-$(CONFIG_GPIO_BCM_XGS_IPROC)	+= gpio-xgs-iproc.o
 obj-$(CONFIG_GPIO_BD71815)		+= gpio-bd71815.o
 obj-$(CONFIG_GPIO_BD71828)		+= gpio-bd71828.o
 obj-$(CONFIG_GPIO_BD72720)		+= gpio-bd72720.o
+obj-$(CONFIG_GPIO_BD73800)		+= gpio-bd73800.o
 obj-$(CONFIG_GPIO_BD9571MWV)		+= gpio-bd9571mwv.o
 obj-$(CONFIG_GPIO_BLZP1600)		+= gpio-blzp1600.o
 obj-$(CONFIG_GPIO_BRCMSTB)		+= gpio-brcmstb.o

drivers/gpio/gpio-bd73800.c new file +66 -0

new file mode 100644
index 000000000000..3fe4b7f167b8
@@ -0,0 +1,234 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Support to GPIOs on ROHM BD73800
+ * Copyright 2024 ROHM Semiconductors.
+ * Author: Matti Vaittinen <[email protected]>
+ */
+
+#include <linux/gpio/driver.h>
+#include <linux/init.h>
+#include <linux/irq.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/mfd/rohm-bd73800.h>
+
+#define BD73800_GPIO_MAX_PINS 4
+/*
+ * The BD73800 has several "one time programmable" (OTP) configurations which
+ * can be set at manufacturing phase. Some of these options allow using
+ * individual pins as GPI or GPO (not both at the same time). The OTP
+ * configuration can't be read at run-time, so drivers rely on device-tree to
+ * advertise the OTP programmed in manufacturing.
+ *
+ * The pins which can be used as GPIO are:
+ * GPIO1, CLKOUT (GPIO2), FAULT_B, EXTEN_OUT.
+ *
+ * The OTP options 2 and 3 state for all the pins:
+ *  - OTP2: GPI (also IRQ source)
+ *  - OTP3: GPO (NOTE: This is actually 2 different OTP options. Either a
+ *    register controllable output or a power-sequence controlled output.
+ *    The "gpo" referred here means only the register controllable output.
+ *    The datasheet refers to this as: "<pin> output is controlled by
+ *    GPIO<N>_OUT or power on/off sequencer to control external VRs. ON/OFF
+ *    sequence timing is configurable."
+ *
+ * The data-sheet further says that the GPI/GPO is not a default OTP
+ * configuration for any of the pins. Hence the GPIO driver defaults to a pin
+ * not being a GPI or GPO, but requires the pin to be explicitly marked as a
+ * GPI or GPO in the device-tree.
+ *
+ * DT properties:
+ * "rohm,pin-gpio1", "rohm,pin-clkout", "rohm,pin-fault-b", "rohm,pin-exten"
+ * can be set to one of the values "gpi" or "gpo" to enable them to be used as
+ * GPIO.
+ */
+
+enum bd73800_gpio_state {
+	BD73800_PIN_UNKNOWN,
+	BD73800_PIN_GPI,
+	BD73800_PIN_GPO,
+};
+
+struct bd73800_gpio_pin_cfg {
+	enum bd73800_gpio_state state;
+	int mask; /* GPIO_OUT and INT_SRC have same bit offsets for GPIO */
+};
+
+struct bd73800_gpio {
+	/* chip.parent points the MFD which provides DT node and regmap */
+	struct gpio_chip chip;
+	struct bd73800_gpio_pin_cfg pin[BD73800_GPIO_MAX_PINS];
+	int num_pins;
+	/* dev points to the platform device for devm and prints */
+	struct device *dev;
+	struct regmap *regmap;
+};

drivers/gpio/gpio-bd73800.c new file +89 -0

+						  properties[i], &val);
+
+		if (ret) {
+			if (ret == -EINVAL)
+				continue;
+
+			return dev_err_probe(g->dev, ret,
+					"pin %d (%s), bad configuration\n", i,
+					properties[i]);
+		}
+
+		if (strcmp(val, "gpi") == 0) {
+			g->pin[g->num_pins].state = BD73800_PIN_GPI;
+			g->pin[g->num_pins].mask = BIT(i);
+			g->num_pins++;
+		} else if (strcmp(val, "gpo") == 0) {
+			g->pin[g->num_pins].state = BD73800_PIN_GPO;
+			g->pin[g->num_pins].mask = BIT(i);
+			g->num_pins++;
+		}
+	}
+
+	return 0;
+}
+
+static int bd73800gpio_get(struct gpio_chip *chip, unsigned int offset)
+{
+	struct bd73800_gpio *bdgpio = gpiochip_get_data(chip);
+	struct bd73800_gpio_pin_cfg *pin = &bdgpio->pin[offset];
+	int ret, val;
+
+	/* Only pins configured as GPI via OTP can have their status read */
+	if (pin->state != BD73800_PIN_GPI) {
+		dev_dbg(bdgpio->dev, "pin %d (%x) not input. State %d\n",
+			offset, pin->mask, pin->state);
+		return -EINVAL;
+	}
+
+	ret = regmap_read(bdgpio->regmap, BD73800_REG_INT_5_SRC, &val);
+	if (ret)
+		return ret;
+
+	return val & pin->mask;
+}
+
+static int bd73800gpo_set(struct gpio_chip *chip, unsigned int offset,
+			  int value)
+{
+	struct bd73800_gpio *bdgpio = gpiochip_get_data(chip);
+	struct bd73800_gpio_pin_cfg *pin = &bdgpio->pin[offset];
+
+	if (pin->state != BD73800_PIN_GPO) {
+		dev_dbg(bdgpio->dev, "pin %d (%d) not output. State %d\n",
+			offset, pin->mask, pin->state);
+
+		return -EINVAL;
+	}
+
+	if (value)
+		return regmap_set_bits(bdgpio->regmap, BD73800_REG_GPO_OUT,
+				       pin->mask);
+
+	return regmap_clear_bits(bdgpio->regmap, BD73800_REG_GPO_OUT, pin->mask);
+}
+
+static int bd73800gpio_direction_get(struct gpio_chip *chip,
+				    unsigned int offset)
+{
+	struct bd73800_gpio *bdgpio = gpiochip_get_data(chip);
+
+	if (bdgpio->pin[offset].state == BD73800_PIN_GPO)
+		return GPIO_LINE_DIRECTION_OUT;
+
+	return GPIO_LINE_DIRECTION_IN;
+}
+
+/*
+ * Template for GPIO chip. The BD73800 GPO supports both CMOS and open drain
+ * configurations. The default however depends on the OTP. The runtime config
+ * can be done via undocumented test registers - but at the moment there is no
+ * support for this.
+ *
+ * NOTE: When the BD73800 GPIO pins are used as IRQ source, the users are
+ * expected to request them directly from the regmap_irq IRQ-chip (implemented
+ * in the MFD driver). This way we don't need to populate another IRQ-chip
+ * here.
+ */
+static const struct gpio_chip bd73800gpio_chip = {
+	.label			= "bd73800",

drivers/gpio/gpio-bd73800.c new file +16 -0

+	dev = &pdev->dev;
+	/* The device-tree and regmap come from MFD => use parent for that */
+	parent = dev->parent;
+
+	g = devm_kzalloc(dev, sizeof(*g), GFP_KERNEL);
+	if (!g)
+		return -ENOMEM;
+
+	g->chip = bd73800gpio_chip;
+	g->chip.base = -1;
+	g->chip.parent = parent;
+	g->regmap = dev_get_regmap(parent, NULL);
+	g->dev = dev;
+
+	ret = bd73800_gpio_get_pins(g);
+	if (ret)

drivers/gpio/gpio-bd73800.c new file +25 -1

+	}
+	g->chip.ngpio = g->num_pins;
+
+	return devm_gpiochip_add_data(dev, &g->chip, g);
+}
+
+static const struct platform_device_id bd73800_gpio_id[] = {
+	{ "bd73800-gpio" },
+	{ },
+};
+MODULE_DEVICE_TABLE(platform, bd73800_gpio_id);
+
+static struct platform_driver gpo_bd73800_driver = {
+	.driver = {
+		.name = "bd73800-gpio",
+		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
+	},
+	.probe = gpo_bd73800_probe,
+	.id_table = bd73800_gpio_id,
+};
+module_platform_driver(gpo_bd73800_driver);
+
+MODULE_AUTHOR("Matti Vaittinen <[email protected]>");
+MODULE_DESCRIPTION("GPIO interface for BD73800");
+MODULE_LICENSE("GPL");
--

2.54.0

Quoted 20 lines · Matti Vaittinen

From: Matti Vaittinen <[email protected]>

The ROHM BD73800 PMIC has 4 pins (named GPIO1, CLKOUT, FAULT_B and EXTEN_OUT) which might have been set to operate as a GPI or GPO when OTP (One Time Programmable memory) is written at device manufacturing. Support the GPI/GPO use-case via GPIO framework.

The default OTP for these pins is to not use any of them as GPI or GPO. (The GPIO1 defaults as an ADC input regardless the naming). Hence the driver assumes none of these pins is a GPI/GPO unless explicitly pointed as GPI or GPO via device tree.

Furthermore, pin's direction can't be changed after OTP configuration is done. Also the default drive type for a GPO (CMOS / Open Drain) is set by the OTP configuration. The BD73800 has a set of undocumented test registers which should allow changing the drive type. Access to the test register area or the test registers aren't documented and so this driver does not support configuring the drive type even though it might be doable.

Quoted 2 lines

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

---

drivers/gpio/gpio-bd73800.c line 76

Quoted 9 lines

drivers/gpio/gpio-bd73800.c new file +9 -0

+static int bd73800_gpio_get_pins(struct bd73800_gpio *g)
+{
+     static const char * const properties[] = {"rohm,pin-gpio1",
+             "rohm,pin-clkout", "rohm,pin-fault-b", "rohm,pin-exten"};
+     const char *val;
+     int i, ret;
+
+     for (i = 0; i < ARRAY_SIZE(properties); i++) {
+             ret = fwnode_property_read_string(dev_fwnode(g->dev->parent),

It would be cleaner with device_property_read_string(g->dev->parent, ...)

drivers/gpio/gpio-bd73800.c line 175

Quoted 4 lines

drivers/gpio/gpio-bd73800.c new file +4 -0

+
+static int gpo_bd73800_probe(struct platform_device *pdev)
+{
+     struct bd73800_gpio *g;

May I suggest a slightly more descriptive name for driver data? Maybe at the very least... "data"? :)

drivers/gpio/gpio-bd73800.c line 182

Quoted 7 lines

drivers/gpio/gpio-bd73800.c new file +7 -0

+     struct device *parent, *dev;
+     int ret;
+
+     /*
+      * Bind devm lifetime to this platform device => use dev for devm.
+      * also the prints should originate from this device.
+      */

Why would you need to comment on it though? It's very much the standard approach, isn't it?

Quoted 16 lines

drivers/gpio/gpio-bd73800.c new file +16 -0

+     dev = &pdev->dev;
+     /* The device-tree and regmap come from MFD => use parent for that */
+     parent = dev->parent;
+
+     g = devm_kzalloc(dev, sizeof(*g), GFP_KERNEL);
+     if (!g)
+             return -ENOMEM;
+
+     g->chip = bd73800gpio_chip;
+     g->chip.base = -1;
+     g->chip.parent = parent;
+     g->regmap = dev_get_regmap(parent, NULL);
+     g->dev = dev;
+
+     ret = bd73800_gpio_get_pins(g);
+     if (ret)

drivers/gpio/gpio-bd73800.c line 208

Quoted 10 lines

drivers/gpio/gpio-bd73800.c new file +10 -0

+             return ret;
+
+     if (!g->num_pins) {
+             /*
+              * The BD73800 may or may not have pins allocated for GPIO
+              * depending on the OTP used at manufacturing. Free the memory
+              * and go out if there is no pins as then we have nothing to do
+              */
+             dev_dbg(dev, "no GPIO pins\n");
+             devm_kfree(dev, g);

No need for that, because...

Quoted 1 line

+ return 0;

... this will still leave a bound device in the system, I'm not sure we want it? The correct error code in this case is -ENODEV.

pe 3.7.2026 klo 16.11 Bartosz Golaszewski ([email protected]) kirjoitti:

Quoted 10 lines

drivers/gpio/gpio-bd73800.c line 208

Quoted 9 lines

drivers/gpio/gpio-bd73800.c new file +9 -0

+
+     if (!g->num_pins) {
+             /*
+              * The BD73800 may or may not have pins allocated for GPIO
+              * depending on the OTP used at manufacturing. Free the memory
+              * and go out if there is no pins as then we have nothing to do
+              */
+             dev_dbg(dev, "no GPIO pins\n");
+             devm_kfree(dev, g);

Quoted 6 lines

No need for that, because...

Quoted 1 line

+ return 0;

... this will still leave a bound device in the system, I'm not sure we want it? The correct error code in this case is -ENODEV.

I am a bit uncertain. This isn't really an error. It should be very much Ok to configure the support for PMICs with GPIOs to be compiled - and also very much Ok to have a PMIC with no GPIOs. Returning an error doesn't feel right. Furthermore, I am afraid that returning an error might hurt the system start-up? Do you see some really problematic consequences if we return Ok without registering the GPIOs. I'd assume that wastes some bytes but I don't see other really severe consequences.

Quoted 18 lines · Matti Vaittinen

pe 3.7.2026 klo 16.11 Bartosz Golaszewski ([email protected]) kirjoitti:

Quoted 10 lines

drivers/gpio/gpio-bd73800.c line 208

Quoted 9 lines

drivers/gpio/gpio-bd73800.c new file +9 -0

+
+     if (!g->num_pins) {
+             /*
+              * The BD73800 may or may not have pins allocated for GPIO
+              * depending on the OTP used at manufacturing. Free the memory
+              * and go out if there is no pins as then we have nothing to do
+              */
+             dev_dbg(dev, "no GPIO pins\n");
+             devm_kfree(dev, g);

Quoted 6 lines

No need for that, because...

Quoted 1 line

+ return 0;

... this will still leave a bound device in the system, I'm not sure we want it? The correct error code in this case is -ENODEV.

Quoted 8 lines

I am a bit uncertain. This isn't really an error. It should be very much Ok to configure the support for PMICs with GPIOs to be compiled - and also very much Ok to have a PMIC with no GPIOs. Returning an error doesn't feel right. Furthermore, I am afraid that returning an error might hurt the system start-up? Do you see some really problematic consequences if we return Ok without registering the GPIOs. I'd assume that wastes some bytes but I don't see other really severe consequences.

But -ENODEV/-ENXIO error codes are meant to do exactly that - tell the driver core to assume this device legitimately cannot be enabled on this system. There will be no error splat in the kernel log and the probe will not be deferred.