max597x is multifunction device with hot swap controller, fault
protection & upto four indication leds.
max5978 has single hot swap controller whereas max5970 has two hot swap
controllers.
Changes in V9:
- Update properties description
- update required property
Change in V8:
- Set additionalproperties to false
Change in V7:
- Update id
- Remove empty line
Changes in V6:
- Update missing vendor prefix
- Update indentation in example
Changes in V5:
- Fix dt schema error
Changes in V4:
- Add NULL entry for of_device_id
- Memory allocation check
Changes in V3:
- Address code review comment
Changes in V2:
- Update depends in Kconfig.
Marcello Sylvester Bauer (1):
dt-bindings: mfd: Add dt-schema MAX5970 and MAX5978
Patrick Rudolph (1):
mfd: max597x: Add support for MAX5970 and MAX5978
.../bindings/mfd/maxim,max5970.yaml | 172 ++++++++++++++++++
drivers/mfd/Kconfig | 12 ++
drivers/mfd/Makefile | 1 +
drivers/mfd/max597x.c | 92 ++++++++++
include/linux/mfd/max597x.h | 103 +++++++++++
5 files changed, 380 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/maxim,max5970.yaml
create mode 100644 drivers/mfd/max597x.c
create mode 100644 include/linux/mfd/max597x.h
base-commit: 6b780408be034213edfb5946889882cb29f8f159
--
2.37.3
From: Marcello Sylvester Bauer <[email protected]>
The MAX597x is a hot swap controller with configurable fault protection.
It also has 10bit ADC for current & voltage measurements.
Signed-off-by: Patrick Rudolph <[email protected]>
Signed-off-by: Marcello Sylvester Bauer <[email protected]>
Signed-off-by: Naresh Solanki <[email protected]>
---
.../bindings/mfd/maxim,max5970.yaml | 172 ++++++++++++++++++
1 file changed, 172 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/maxim,max5970.yaml
diff --git a/Documentation/devicetree/bindings/mfd/maxim,max5970.yaml b/Documentation/devicetree/bindings/mfd/maxim,max5970.yaml
new file mode 100644
index 000000000000..a93b6e009b9a
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/maxim,max5970.yaml
@@ -0,0 +1,172 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/maxim,max5970.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Regulator for MAX5970 smart switch from Maxim Integrated.
+
+maintainers:
+ - Patrick Rudolph <[email protected]>
+
+description: |
+ The smart switch provides no output regulation, but independent fault protection
+ and voltage and current sensing.
+ Programming is done through I2C bus.
+
+ Datasheets:
+ https://datasheets.maximintegrated.com/en/ds/MAX5970.pdf
+ https://datasheets.maximintegrated.com/en/ds/MAX5978.pdf
+
+properties:
+ compatible:
+ enum:
+ - maxim,max5970
+ - maxim,max5978
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ leds:
+ type: object
+ description:
+ Properties for four LEDS.
+
+ properties:
+ "#address-cells":
+ const: 1
+
+ "#size-cells":
+ const: 0
+
+ patternProperties:
+ "^led@[0-3]$":
+ $ref: /schemas/leds/common.yaml#
+ type: object
+
+ additionalProperties: false
+
+ vss1-supply:
+ description: Supply of the first channel.
+
+ vss2-supply:
+ description: Supply of the second channel.
+
+ "#io-channel-cells":
+ const: 1
+
+ regulators:
+ type: object
+ description:
+ Properties for regulator.
+
+ patternProperties:
+ "^sw[0-1]$":
+ $ref: /schemas/regulator/regulator.yaml#
+ type: object
+ properties:
+ shunt-resistor-micro-ohms:
+ description: |
+ The value of curent sense resistor in microohms.
+
+ required:
+ - shunt-resistor-micro-ohms
+
+ additionalProperties: false
+
+required:
+ - compatible
+ - reg
+ - regulators
+ - vss1-supply
+
+allOf:
+ - if:
+ properties:
+ compatible:
+ enum:
+ - maxim,max5970
+ then:
+ properties:
+ io-channels:
+ items:
+ - description: voltage first channel
+ - description: current first channel
+ - description: voltage second channel
+ - description: current second channel
+ description: |
+ Voltage and current for first and second channel.
+ required:
+ - vss2-supply
+ else:
+ properties:
+ io-channels:
+ items:
+ - description: voltage first channel
+ - description: current first channel
+ description: |
+ Voltage and current for first channel.
+
+additionalProperties: false
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ regulator@3a {
+ compatible = "maxim,max5978";
+ reg = <0x3a>;
+ vss1-supply = <&p3v3>;
+
+ regulators {
+ sw0_ref_0: sw0 {
+ regulator-compatible = "SW0";
+ shunt-resistor-micro-ohms = <12000>;
+ };
+ };
+
+ leds {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ led@0 {
+ reg = <0>;
+ label = "led0";
+ default-state = "on";
+ };
+ led@1 {
+ reg = <1>;
+ label = "led1";
+ default-state = "on";
+ };
+ };
+ };
+ };
+
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ regulator@3a {
+ compatible = "maxim,max5970";
+ reg = <0x3a>;
+ vss1-supply = <&p3v3>;
+ vss2-supply = <&p5v>;
+
+ regulators {
+ sw0_ref_1: sw0 {
+ regulator-compatible = "SW0";
+ shunt-resistor-micro-ohms = <12000>;
+ };
+ sw1_ref_1: sw1 {
+ regulator-compatible = "SW1";
+ shunt-resistor-micro-ohms = <10000>;
+ };
+ };
+ };
+ };
+...
--
2.37.3
On 14/11/2022 08:57, Naresh Solanki wrote:
> From: Marcello Sylvester Bauer <[email protected]>
Subject: I asked to drop the redundant words, but you re-added now in
different form. So again, drop "dt-schema".
>
> The MAX597x is a hot swap controller with configurable fault protection.
> It also has 10bit ADC for current & voltage measurements.
>
> Signed-off-by: Patrick Rudolph <[email protected]>
> Signed-off-by: Marcello Sylvester Bauer <[email protected]>
> Signed-off-by: Naresh Solanki <[email protected]>
> ---
> .../bindings/mfd/maxim,max5970.yaml | 172 ++++++++++++++++++
> 1 file changed, 172 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/maxim,max5970.yaml
>
> diff --git a/Documentation/devicetree/bindings/mfd/maxim,max5970.yaml b/Documentation/devicetree/bindings/mfd/maxim,max5970.yaml
> new file mode 100644
> index 000000000000..a93b6e009b9a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/maxim,max5970.yaml
> @@ -0,0 +1,172 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/maxim,max5970.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Regulator for MAX5970 smart switch from Maxim Integrated.
> +
> +maintainers:
> + - Patrick Rudolph <[email protected]>
> +
> +description: |
> + The smart switch provides no output regulation, but independent fault protection
> + and voltage and current sensing.
> + Programming is done through I2C bus.
> +
> + Datasheets:
> + https://datasheets.maximintegrated.com/en/ds/MAX5970.pdf
> + https://datasheets.maximintegrated.com/en/ds/MAX5978.pdf
> +
> +properties:
> + compatible:
> + enum:
> + - maxim,max5970
> + - maxim,max5978
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + leds:
> + type: object
> + description:
> + Properties for four LEDS.
> +
> + properties:
> + "#address-cells":
> + const: 1
> +
> + "#size-cells":
> + const: 0
> +
> + patternProperties:
> + "^led@[0-3]$":
> + $ref: /schemas/leds/common.yaml#
> + type: object
> +
> + additionalProperties: false
> +
> + vss1-supply:
> + description: Supply of the first channel.
> +
> + vss2-supply:
> + description: Supply of the second channel.
> +
> + "#io-channel-cells":
> + const: 1
> +
> + regulators:
> + type: object
> + description:
> + Properties for regulator.
That's not correct description. This is not one regulator.
> +
> + patternProperties:
> + "^sw[0-1]$":
> + $ref: /schemas/regulator/regulator.yaml#
> + type: object
unevaluatedProperties: false
> + properties:
> + shunt-resistor-micro-ohms:
> + description: |
> + The value of curent sense resistor in microohms.
Typo: current
> +
> + required:
> + - shunt-resistor-micro-ohms
> +
> + additionalProperties: false
> +
> +required:
> + - compatible
> + - reg
> + - regulators
> + - vss1-supply
> +
> +allOf:
> + - if:
> + properties:
> + compatible:
> + enum:
> + - maxim,max5970
> + then:
> + properties:
> + io-channels:
> + items:
> + - description: voltage first channel
> + - description: current first channel
> + - description: voltage second channel
> + - description: current second channel
> + description: |
> + Voltage and current for first and second channel.
> + required:
> + - vss2-supply
This is a friendly reminder during the review process.
It seems my previous comments were not fully addressed. Maybe my
feedback got lost between the quotes, maybe you just forgot to apply it.
Please go back to the previous discussion and either implement all
requested changes or keep discussing them.
Comment was:
"Also, add it in the existing example."
> + else:
> + properties:
> + io-channels:
> + items:
> + - description: voltage first channel
> + - description: current first channel
> + description: |
> + Voltage and current for first channel.
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + regulator@3a {
> + compatible = "maxim,max5978";
> + reg = <0x3a>;
> + vss1-supply = <&p3v3>;
> +
> + regulators {
> + sw0_ref_0: sw0 {
> + regulator-compatible = "SW0";
Use 4 spaces for example indentation.
> + shunt-resistor-micro-ohms = <12000>;
> + };
> + };
> +
> + leds {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + led@0 {
> + reg = <0>;
> + label = "led0";
> + default-state = "on";
> + };
> + led@1 {
> + reg = <1>;
> + label = "led1";
> + default-state = "on";
> + };
> + };
> + };
> + };
> +
> + - |
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + regulator@3a {
> + compatible = "maxim,max5970";
> + reg = <0x3a>;
> + vss1-supply = <&p3v3>;
> + vss2-supply = <&p5v>;
> +
> + regulators {
> + sw0_ref_1: sw0 {
> + regulator-compatible = "SW0";
> + shunt-resistor-micro-ohms = <12000>;
> + };
> + sw1_ref_1: sw1 {
> + regulator-compatible = "SW1";
> + shunt-resistor-micro-ohms = <10000>;
> + };
> + };
> + };
> + };
> +...
Best regards,
Krzysztof
From: Patrick Rudolph <[email protected]>
Implement a regulator driver with IRQ support for fault management.
Written against documentation [1] and [2] and tested on real hardware.
Every channel has its own regulator supplies nammed 'vss1-supply' and
'vss2-supply'. The regulator supply is used to determine the output
voltage, as the smart switch provides no output regulation.
The driver requires the 'shunt-resistor-micro-ohms' property to be
present in Device Tree to properly calculate current related
values.
Datasheet links:
1: https://datasheets.maximintegrated.com/en/ds/MAX5970.pdf
2: https://datasheets.maximintegrated.com/en/ds/MAX5978.pdf
Signed-off-by: Patrick Rudolph <[email protected]>
Signed-off-by: Marcello Sylvester Bauer <[email protected]>
Signed-off-by: Naresh Solanki <[email protected]>
---
drivers/mfd/Kconfig | 12 +++++
drivers/mfd/Makefile | 1 +
drivers/mfd/max597x.c | 92 ++++++++++++++++++++++++++++++++
include/linux/mfd/max597x.h | 103 ++++++++++++++++++++++++++++++++++++
4 files changed, 208 insertions(+)
create mode 100644 drivers/mfd/max597x.c
create mode 100644 include/linux/mfd/max597x.h
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 8b93856de432..416fe7986b7b 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -253,6 +253,18 @@ config MFD_MADERA_SPI
Support for the Cirrus Logic Madera platform audio SoC
core functionality controlled via SPI.
+config MFD_MAX597X
+ tristate "Maxim 597x Power Switch and Monitor"
+ depends on I2C
+ depends on OF
+ select MFD_CORE
+ select REGMAP_I2C
+ help
+ This driver controls a Maxim 5970/5978 switch via I2C bus.
+ The MAX5970/5978 is a smart switch with no output regulation, but
+ fault protection and voltage and current monitoring capabilities.
+ Also it supports upto 4 indication LEDs.
+
config MFD_CS47L15
bool "Cirrus Logic CS47L15"
select PINCTRL_CS47L15
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 7ed3ef4a698c..819d711fa748 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -161,6 +161,7 @@ obj-$(CONFIG_MFD_DA9063) += da9063.o
obj-$(CONFIG_MFD_DA9150) += da9150-core.o
obj-$(CONFIG_MFD_MAX14577) += max14577.o
+obj-$(CONFIG_MFD_MAX597X) += max597x.o
obj-$(CONFIG_MFD_MAX77620) += max77620.o
obj-$(CONFIG_MFD_MAX77650) += max77650.o
obj-$(CONFIG_MFD_MAX77686) += max77686.o
diff --git a/drivers/mfd/max597x.c b/drivers/mfd/max597x.c
new file mode 100644
index 000000000000..ae43d57cde26
--- /dev/null
+++ b/drivers/mfd/max597x.c
@@ -0,0 +1,92 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Maxim MAX5970/MAX5978 Power Switch & Monitor
+ *
+ * Copyright (c) 2022 9elements GmbH
+ *
+ * Author: Patrick Rudolph <[email protected]>
+ */
+
+#include <linux/i2c.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/max597x.h>
+#include <linux/regmap.h>
+
+static const struct regmap_config max597x_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .max_register = MAX_REGISTERS,
+};
+
+static const struct mfd_cell max597x_cells[] = {
+ { .name = "max597x-regulator", },
+ { .name = "max597x-iio", },
+ { .name = "max597x-led", },
+};
+
+static int max597x_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
+{
+ struct max597x_data *ddata;
+ enum max597x_chip_type chip = id->driver_data;
+
+ ddata = devm_kzalloc(&i2c->dev, sizeof(*ddata), GFP_KERNEL);
+ if (!ddata)
+ return -ENOMEM;
+
+ /*
+ * Based on chip type, Initialize the number of switch. This is needed by
+ * regulator & iio cells.
+ */
+ switch (chip) {
+ case MAX597x_TYPE_MAX5970:
+ ddata->num_switches = MAX5970_NUM_SWITCHES;
+ break;
+ case MAX597x_TYPE_MAX5978:
+ ddata->num_switches = MAX5978_NUM_SWITCHES;
+ break;
+ }
+
+ ddata->regmap = devm_regmap_init_i2c(i2c, &max597x_regmap_config);
+ if (IS_ERR(ddata->regmap)) {
+ dev_err(&i2c->dev, "Failed to initialize regmap");
+ return PTR_ERR(ddata->regmap);
+ }
+
+ /* IRQ used by regulator cell */
+ ddata->irq = i2c->irq;
+ ddata->dev = &i2c->dev;
+ i2c_set_clientdata(i2c, ddata);
+
+ return devm_mfd_add_devices(ddata->dev, PLATFORM_DEVID_AUTO,
+ max597x_cells, ARRAY_SIZE(max597x_cells),
+ NULL, 0, NULL);
+}
+
+static const struct i2c_device_id max597x_table[] = {
+ { .name = "max5970", MAX597x_TYPE_MAX5970 },
+ { .name = "max5978", MAX597x_TYPE_MAX5978 },
+};
+
+MODULE_DEVICE_TABLE(i2c, max597x_table);
+
+static const struct of_device_id max597x_of_match[] = {
+ { .compatible = "maxim,max5970", .data = (void *)MAX597x_TYPE_MAX5970 },
+ { .compatible = "maxim,max5978", .data = (void *)MAX597x_TYPE_MAX5978 },
+ {}
+};
+
+MODULE_DEVICE_TABLE(of, max597x_of_match);
+
+static struct i2c_driver max597x_driver = {
+ .id_table = max597x_table,
+ .driver = {
+ .name = "max597x",
+ .of_match_table = of_match_ptr(max597x_of_match),
+ },
+ .probe = max597x_probe,
+};
+module_i2c_driver(max597x_driver);
+
+MODULE_AUTHOR("Patrick Rudolph <[email protected]>");
+MODULE_DESCRIPTION("MAX597X Power Switch and Monitor");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mfd/max597x.h b/include/linux/mfd/max597x.h
new file mode 100644
index 000000000000..99a047e66b44
--- /dev/null
+++ b/include/linux/mfd/max597x.h
@@ -0,0 +1,103 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Maxim MAX5970/MAX5978 Power Switch & Monitor
+ *
+ * Copyright (c) 2022 9elements GmbH
+ *
+ * Author: Patrick Rudolph <[email protected]>
+ */
+
+#ifndef MFD_MAX597X_H
+#define MFD_MAX597X_H
+
+#include <linux/device.h>
+#include <linux/regmap.h>
+
+/* Number of switch based on chip variant */
+#define MAX5970_NUM_SWITCHES 2
+#define MAX5978_NUM_SWITCHES 1
+/* Both chip variant have 4 indication LEDs used by LED cell */
+#define MAX597X_NUM_LEDS 4
+
+enum max597x_chip_type {
+ MAX597x_TYPE_MAX5978 = 1,
+ MAX597x_TYPE_MAX5970,
+};
+
+#define MAX5970_REG_CURRENT_L(ch) (0x01 + (ch) * 4)
+#define MAX5970_REG_CURRENT_H(ch) (0x00 + (ch) * 4)
+#define MAX5970_REG_VOLTAGE_L(ch) (0x03 + (ch) * 4)
+#define MAX5970_REG_VOLTAGE_H(ch) (0x02 + (ch) * 4)
+#define MAX5970_REG_MON_RANGE 0x18
+#define MAX5970_MON_MASK 0x3
+#define MAX5970_MON(reg, ch) (((reg) >> ((ch) * 2)) & MAX5970_MON_MASK)
+#define MAX5970_MON_MAX_RANGE_UV 16000000
+
+#define MAX5970_REG_CH_UV_WARN_H(ch) (0x1A + (ch) * 10)
+#define MAX5970_REG_CH_UV_WARN_L(ch) (0x1B + (ch) * 10)
+#define MAX5970_REG_CH_UV_CRIT_H(ch) (0x1C + (ch) * 10)
+#define MAX5970_REG_CH_UV_CRIT_L(ch) (0x1D + (ch) * 10)
+#define MAX5970_REG_CH_OV_WARN_H(ch) (0x1E + (ch) * 10)
+#define MAX5970_REG_CH_OV_WARN_L(ch) (0x1F + (ch) * 10)
+#define MAX5970_REG_CH_OV_CRIT_H(ch) (0x20 + (ch) * 10)
+#define MAX5970_REG_CH_OV_CRIT_L(ch) (0x21 + (ch) * 10)
+
+#define MAX5970_VAL2REG_H(x) (((x) >> 2) & 0xFF)
+#define MAX5970_VAL2REG_L(x) ((x) & 0x3)
+
+#define MAX5970_REG_DAC_FAST(ch) (0x2E + (ch))
+
+#define MAX5970_FAST2SLOW_RATIO 200
+
+#define MAX5970_REG_STATUS0 0x31
+#define MAX5970_CB_IFAULTF(ch) (1 << (ch))
+#define MAX5970_CB_IFAULTS(ch) (1 << ((ch) + 4))
+
+#define MAX5970_REG_STATUS1 0x32
+#define STATUS1_PROT_MASK 0x3
+#define STATUS1_PROT(reg) \
+ (((reg) >> 6) & STATUS1_PROT_MASK)
+#define STATUS1_PROT_SHUTDOWN 0
+#define STATUS1_PROT_CLEAR_PG 1
+#define STATUS1_PROT_ALERT_ONLY 2
+
+#define MAX5970_REG_STATUS2 0x33
+#define MAX5970_IRNG_MASK 0x3
+#define MAX5970_IRNG(reg, ch) \
+ (((reg) >> ((ch) * 2)) & MAX5970_IRNG_MASK)
+
+#define MAX5970_REG_STATUS3 0x34
+#define MAX5970_STATUS3_ALERT BIT(4)
+#define MAX5970_STATUS3_PG(ch) BIT(ch)
+
+#define MAX5970_REG_FAULT0 0x35
+#define UV_STATUS_WARN(ch) BIT(ch)
+#define UV_STATUS_CRIT(ch) BIT(ch + 4)
+
+#define MAX5970_REG_FAULT1 0x36
+#define OV_STATUS_WARN(ch) BIT(ch)
+#define OV_STATUS_CRIT(ch) BIT(ch + 4)
+
+#define MAX5970_REG_FAULT2 0x37
+#define OC_STATUS_WARN(ch) BIT(ch)
+
+#define MAX5970_REG_CHXEN 0x3b
+#define CHXEN(ch) (3 << (ch * 2))
+
+#define MAX5970_REG_LED_FLASH 0x43
+
+#define MAX_REGISTERS 0x49
+#define ADC_MASK 0x3FF
+
+struct max597x_data {
+ struct device *dev;
+ int irq;
+ int num_switches;
+ struct regmap *regmap;
+ /* Chip specific parameters needed by regulator & iio cells */
+ u32 irng[MAX5970_NUM_SWITCHES];
+ u32 mon_rng[MAX5970_NUM_SWITCHES];
+ u32 shunt_micro_ohms[MAX5970_NUM_SWITCHES];
+};
+
+#endif
\ No newline at end of file
--
2.37.3
Hi Krzysztof,
On 14-11-2022 01:40 pm, Krzysztof Kozlowski wrote:
> On 14/11/2022 08:57, Naresh Solanki wrote:
>> From: Marcello Sylvester Bauer <[email protected]>
>
> Subject: I asked to drop the redundant words, but you re-added now in
> different form. So again, drop "dt-schema".
Sure
>
>>
>> The MAX597x is a hot swap controller with configurable fault protection.
>> It also has 10bit ADC for current & voltage measurements.
>>
>> Signed-off-by: Patrick Rudolph <[email protected]>
>> Signed-off-by: Marcello Sylvester Bauer <[email protected]>
>> Signed-off-by: Naresh Solanki <[email protected]>
>> ---
>> .../bindings/mfd/maxim,max5970.yaml | 172 ++++++++++++++++++
>> 1 file changed, 172 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/mfd/maxim,max5970.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/maxim,max5970.yaml b/Documentation/devicetree/bindings/mfd/maxim,max5970.yaml
>> new file mode 100644
>> index 000000000000..a93b6e009b9a
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mfd/maxim,max5970.yaml
>> @@ -0,0 +1,172 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/mfd/maxim,max5970.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Regulator for MAX5970 smart switch from Maxim Integrated.
>> +
>> +maintainers:
>> + - Patrick Rudolph <[email protected]>
>> +
>> +description: |
>> + The smart switch provides no output regulation, but independent fault protection
>> + and voltage and current sensing.
>> + Programming is done through I2C bus.
>> +
>> + Datasheets:
>> + https://datasheets.maximintegrated.com/en/ds/MAX5970.pdf
>> + https://datasheets.maximintegrated.com/en/ds/MAX5978.pdf
>> +
>> +properties:
>> + compatible:
>> + enum:
>> + - maxim,max5970
>> + - maxim,max5978
>> +
>> + reg:
>> + maxItems: 1
>> +
>> + interrupts:
>> + maxItems: 1
>> +
>> + leds:
>> + type: object
>> + description:
>> + Properties for four LEDS.
>> +
>> + properties:
>> + "#address-cells":
>> + const: 1
>> +
>> + "#size-cells":
>> + const: 0
>> +
>> + patternProperties:
>> + "^led@[0-3]$":
>> + $ref: /schemas/leds/common.yaml#
>> + type: object
>> +
>> + additionalProperties: false
>> +
>> + vss1-supply:
>> + description: Supply of the first channel.
>> +
>> + vss2-supply:
>> + description: Supply of the second channel.
>> +
>> + "#io-channel-cells":
>> + const: 1
>> +
>> + regulators:
>> + type: object
>> + description:
>> + Properties for regulator.
>
> That's not correct description. This is not one regulator.
Sure. Will add description something like:
Properties for both regulators. Also specify value for shunt resistor
used for current sense.
>
>> +
>> + patternProperties:
>> + "^sw[0-1]$":
>> + $ref: /schemas/regulator/regulator.yaml#
>> + type: object
>
> unevaluatedProperties: false
Sure will add this.
>
>> + properties:
>> + shunt-resistor-micro-ohms:
>> + description: |
>> + The value of curent sense resistor in microohms.
>
> Typo: current
Sure. will fix this.
>
>> +
>> + required:
>> + - shunt-resistor-micro-ohms
>> +
>> + additionalProperties: false
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - regulators
>> + - vss1-supply
>> +
>> +allOf:
>> + - if:
>> + properties:
>> + compatible:
>> + enum:
>> + - maxim,max5970
>> + then:
>> + properties:
>> + io-channels:
>> + items:
>> + - description: voltage first channel
>> + - description: current first channel
>> + - description: voltage second channel
>> + - description: current second channel
>> + description: |
>> + Voltage and current for first and second channel.
>> + required:
>> + - vss2-supply
>
> This is a friendly reminder during the review process.
>
> It seems my previous comments were not fully addressed. Maybe my
> feedback got lost between the quotes, maybe you just forgot to apply it.
> Please go back to the previous discussion and either implement all
> requested changes or keep discussing them.
>
> Comment was:
> "Also, add it in the existing example."
This property isn't used. hence will remove in next version.
>
>> + else:
>> + properties:
>> + io-channels:
>> + items:
>> + - description: voltage first channel
>> + - description: current first channel
>> + description: |
>> + Voltage and current for first channel.
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> + i2c {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + regulator@3a {
>> + compatible = "maxim,max5978";
>> + reg = <0x3a>;
>> + vss1-supply = <&p3v3>;
>> +
>> + regulators {
>> + sw0_ref_0: sw0 {
>> + regulator-compatible = "SW0";
>
> Use 4 spaces for example indentation.
Yeah. some place have 3 spaces instead. Will fix in next version for sure.
>
>> + shunt-resistor-micro-ohms = <12000>;
>> + };
>> + };
>> +
>> + leds {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + led@0 {
>> + reg = <0>;
>> + label = "led0";
>> + default-state = "on";
>> + };
>> + led@1 {
>> + reg = <1>;
>> + label = "led1";
>> + default-state = "on";
>> + };
>> + };
>> + };
>> + };
>> +
>> + - |
>> + i2c {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + regulator@3a {
>> + compatible = "maxim,max5970";
>> + reg = <0x3a>;
>> + vss1-supply = <&p3v3>;
>> + vss2-supply = <&p5v>;
>> +
>> + regulators {
>> + sw0_ref_1: sw0 {
>> + regulator-compatible = "SW0";
>> + shunt-resistor-micro-ohms = <12000>;
>> + };
>> + sw1_ref_1: sw1 {
>> + regulator-compatible = "SW1";
>> + shunt-resistor-micro-ohms = <10000>;
>> + };
>> + };
>> + };
>> + };
>> +...
>
> Best regards,
> Krzysztof
>
Regards,
Naresh
Hi Naresh,
I love your patch! Yet something to improve:
[auto build test ERROR on 6b780408be034213edfb5946889882cb29f8f159]
url: https://github.com/intel-lab-lkp/linux/commits/Naresh-Solanki/mfd-max597x-Add-support-for-max597x/20221114-155921
base: 6b780408be034213edfb5946889882cb29f8f159
patch link: https://lore.kernel.org/r/20221114075739.4117439-3-Naresh.Solanki%409elements.com
patch subject: [PATCH v9 2/2] mfd: max597x: Add support for MAX5970 and MAX5978
config: powerpc-allmodconfig
compiler: powerpc-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/b366152ee770ac05dd1d825b277a550657ff6782
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Naresh-Solanki/mfd-max597x-Add-support-for-max597x/20221114-155921
git checkout b366152ee770ac05dd1d825b277a550657ff6782
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=powerpc SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>, old ones prefixed by <<):
drivers/mfd/max597x: struct i2c_device_id is 24 bytes. The last of 2 is:
0x6d 0x61 0x78 0x35 0x39 0x37 0x38 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x01
>> FATAL: modpost: drivers/mfd/max597x: struct i2c_device_id is not terminated with a NULL entry!
--
0-DAY CI Kernel Test Service
https://01.org/lkp
On Mon, 14 Nov 2022, Naresh Solanki wrote:
> From: Patrick Rudolph <[email protected]>
>
> Implement a regulator driver with IRQ support for fault management.
> Written against documentation [1] and [2] and tested on real hardware.
>
> Every channel has its own regulator supplies nammed 'vss1-supply' and
> 'vss2-supply'. The regulator supply is used to determine the output
> voltage, as the smart switch provides no output regulation.
> The driver requires the 'shunt-resistor-micro-ohms' property to be
> present in Device Tree to properly calculate current related
> values.
>
> Datasheet links:
> 1: https://datasheets.maximintegrated.com/en/ds/MAX5970.pdf
> 2: https://datasheets.maximintegrated.com/en/ds/MAX5978.pdf
>
> Signed-off-by: Patrick Rudolph <[email protected]>
> Signed-off-by: Marcello Sylvester Bauer <[email protected]>
> Signed-off-by: Naresh Solanki <[email protected]>
> ---
> drivers/mfd/Kconfig | 12 +++++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/max597x.c | 92 ++++++++++++++++++++++++++++++++
> include/linux/mfd/max597x.h | 103 ++++++++++++++++++++++++++++++++++++
> 4 files changed, 208 insertions(+)
> create mode 100644 drivers/mfd/max597x.c
> create mode 100644 include/linux/mfd/max597x.h
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 8b93856de432..416fe7986b7b 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -253,6 +253,18 @@ config MFD_MADERA_SPI
> Support for the Cirrus Logic Madera platform audio SoC
> core functionality controlled via SPI.
>
> +config MFD_MAX597X
> + tristate "Maxim 597x Power Switch and Monitor"
> + depends on I2C
> + depends on OF
> + select MFD_CORE
> + select REGMAP_I2C
> + help
> + This driver controls a Maxim 5970/5978 switch via I2C bus.
> + The MAX5970/5978 is a smart switch with no output regulation, but
> + fault protection and voltage and current monitoring capabilities.
> + Also it supports upto 4 indication LEDs.
> +
> config MFD_CS47L15
> bool "Cirrus Logic CS47L15"
> select PINCTRL_CS47L15
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 7ed3ef4a698c..819d711fa748 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -161,6 +161,7 @@ obj-$(CONFIG_MFD_DA9063) += da9063.o
> obj-$(CONFIG_MFD_DA9150) += da9150-core.o
>
> obj-$(CONFIG_MFD_MAX14577) += max14577.o
> +obj-$(CONFIG_MFD_MAX597X) += max597x.o
> obj-$(CONFIG_MFD_MAX77620) += max77620.o
> obj-$(CONFIG_MFD_MAX77650) += max77650.o
> obj-$(CONFIG_MFD_MAX77686) += max77686.o
> diff --git a/drivers/mfd/max597x.c b/drivers/mfd/max597x.c
> new file mode 100644
> index 000000000000..ae43d57cde26
> --- /dev/null
> +++ b/drivers/mfd/max597x.c
> @@ -0,0 +1,92 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Maxim MAX5970/MAX5978 Power Switch & Monitor
> + *
> + * Copyright (c) 2022 9elements GmbH
> + *
> + * Author: Patrick Rudolph <[email protected]>
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/max597x.h>
> +#include <linux/regmap.h>
> +
> +static const struct regmap_config max597x_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> + .max_register = MAX_REGISTERS,
> +};
> +
> +static const struct mfd_cell max597x_cells[] = {
> + { .name = "max597x-regulator", },
> + { .name = "max597x-iio", },
> + { .name = "max597x-led", },
> +};
> +
> +static int max597x_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
> +{
> + struct max597x_data *ddata;
> + enum max597x_chip_type chip = id->driver_data;
> +
> + ddata = devm_kzalloc(&i2c->dev, sizeof(*ddata), GFP_KERNEL);
> + if (!ddata)
> + return -ENOMEM;
> +
> + /*
> + * Based on chip type, Initialize the number of switch. This is needed by
> + * regulator & iio cells.
> + */
> + switch (chip) {
> + case MAX597x_TYPE_MAX5970:
> + ddata->num_switches = MAX5970_NUM_SWITCHES;
> + break;
> + case MAX597x_TYPE_MAX5978:
> + ddata->num_switches = MAX5978_NUM_SWITCHES;
> + break;
> + }
> +
> + ddata->regmap = devm_regmap_init_i2c(i2c, &max597x_regmap_config);
> + if (IS_ERR(ddata->regmap)) {
> + dev_err(&i2c->dev, "Failed to initialize regmap");
> + return PTR_ERR(ddata->regmap);
> + }
> +
> + /* IRQ used by regulator cell */
> + ddata->irq = i2c->irq;
> + ddata->dev = &i2c->dev;
You won't need both. You can derive one from the other.
> + i2c_set_clientdata(i2c, ddata);
> +
> + return devm_mfd_add_devices(ddata->dev, PLATFORM_DEVID_AUTO,
> + max597x_cells, ARRAY_SIZE(max597x_cells),
> + NULL, 0, NULL);
> +}
> +
> +static const struct i2c_device_id max597x_table[] = {
> + { .name = "max5970", MAX597x_TYPE_MAX5970 },
> + { .name = "max5978", MAX597x_TYPE_MAX5978 },
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, max597x_table);
> +
> +static const struct of_device_id max597x_of_match[] = {
> + { .compatible = "maxim,max5970", .data = (void *)MAX597x_TYPE_MAX5970 },
> + { .compatible = "maxim,max5978", .data = (void *)MAX597x_TYPE_MAX5978 },
> + {}
> +};
> +
> +MODULE_DEVICE_TABLE(of, max597x_of_match);
> +
> +static struct i2c_driver max597x_driver = {
> + .id_table = max597x_table,
> + .driver = {
> + .name = "max597x",
> + .of_match_table = of_match_ptr(max597x_of_match),
> + },
> + .probe = max597x_probe,
> +};
> +module_i2c_driver(max597x_driver);
> +
> +MODULE_AUTHOR("Patrick Rudolph <[email protected]>");
> +MODULE_DESCRIPTION("MAX597X Power Switch and Monitor");
> +MODULE_LICENSE("GPL v2");
This driver looks *very* simple.
Are you able to use simple-mfd-i2c.c instead?
I suspect the answer to that would be "yes".
> diff --git a/include/linux/mfd/max597x.h b/include/linux/mfd/max597x.h
> new file mode 100644
> index 000000000000..99a047e66b44
> --- /dev/null
> +++ b/include/linux/mfd/max597x.h
> @@ -0,0 +1,103 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Maxim MAX5970/MAX5978 Power Switch & Monitor
> + *
> + * Copyright (c) 2022 9elements GmbH
> + *
> + * Author: Patrick Rudolph <[email protected]>
> + */
> +
> +#ifndef MFD_MAX597X_H
> +#define MFD_MAX597X_H
> +
> +#include <linux/device.h>
> +#include <linux/regmap.h>
> +
> +/* Number of switch based on chip variant */
> +#define MAX5970_NUM_SWITCHES 2
> +#define MAX5978_NUM_SWITCHES 1
> +/* Both chip variant have 4 indication LEDs used by LED cell */
> +#define MAX597X_NUM_LEDS 4
> +
> +enum max597x_chip_type {
> + MAX597x_TYPE_MAX5978 = 1,
> + MAX597x_TYPE_MAX5970,
> +};
> +
> +#define MAX5970_REG_CURRENT_L(ch) (0x01 + (ch) * 4)
> +#define MAX5970_REG_CURRENT_H(ch) (0x00 + (ch) * 4)
> +#define MAX5970_REG_VOLTAGE_L(ch) (0x03 + (ch) * 4)
> +#define MAX5970_REG_VOLTAGE_H(ch) (0x02 + (ch) * 4)
> +#define MAX5970_REG_MON_RANGE 0x18
> +#define MAX5970_MON_MASK 0x3
> +#define MAX5970_MON(reg, ch) (((reg) >> ((ch) * 2)) & MAX5970_MON_MASK)
> +#define MAX5970_MON_MAX_RANGE_UV 16000000
> +
> +#define MAX5970_REG_CH_UV_WARN_H(ch) (0x1A + (ch) * 10)
> +#define MAX5970_REG_CH_UV_WARN_L(ch) (0x1B + (ch) * 10)
> +#define MAX5970_REG_CH_UV_CRIT_H(ch) (0x1C + (ch) * 10)
> +#define MAX5970_REG_CH_UV_CRIT_L(ch) (0x1D + (ch) * 10)
> +#define MAX5970_REG_CH_OV_WARN_H(ch) (0x1E + (ch) * 10)
> +#define MAX5970_REG_CH_OV_WARN_L(ch) (0x1F + (ch) * 10)
> +#define MAX5970_REG_CH_OV_CRIT_H(ch) (0x20 + (ch) * 10)
> +#define MAX5970_REG_CH_OV_CRIT_L(ch) (0x21 + (ch) * 10)
> +
> +#define MAX5970_VAL2REG_H(x) (((x) >> 2) & 0xFF)
> +#define MAX5970_VAL2REG_L(x) ((x) & 0x3)
> +
> +#define MAX5970_REG_DAC_FAST(ch) (0x2E + (ch))
> +
> +#define MAX5970_FAST2SLOW_RATIO 200
> +
> +#define MAX5970_REG_STATUS0 0x31
> +#define MAX5970_CB_IFAULTF(ch) (1 << (ch))
> +#define MAX5970_CB_IFAULTS(ch) (1 << ((ch) + 4))
> +
> +#define MAX5970_REG_STATUS1 0x32
> +#define STATUS1_PROT_MASK 0x3
> +#define STATUS1_PROT(reg) \
> + (((reg) >> 6) & STATUS1_PROT_MASK)
> +#define STATUS1_PROT_SHUTDOWN 0
> +#define STATUS1_PROT_CLEAR_PG 1
> +#define STATUS1_PROT_ALERT_ONLY 2
> +
> +#define MAX5970_REG_STATUS2 0x33
> +#define MAX5970_IRNG_MASK 0x3
> +#define MAX5970_IRNG(reg, ch) \
> + (((reg) >> ((ch) * 2)) & MAX5970_IRNG_MASK)
> +
> +#define MAX5970_REG_STATUS3 0x34
> +#define MAX5970_STATUS3_ALERT BIT(4)
> +#define MAX5970_STATUS3_PG(ch) BIT(ch)
> +
> +#define MAX5970_REG_FAULT0 0x35
> +#define UV_STATUS_WARN(ch) BIT(ch)
> +#define UV_STATUS_CRIT(ch) BIT(ch + 4)
> +
> +#define MAX5970_REG_FAULT1 0x36
> +#define OV_STATUS_WARN(ch) BIT(ch)
> +#define OV_STATUS_CRIT(ch) BIT(ch + 4)
> +
> +#define MAX5970_REG_FAULT2 0x37
> +#define OC_STATUS_WARN(ch) BIT(ch)
> +
> +#define MAX5970_REG_CHXEN 0x3b
> +#define CHXEN(ch) (3 << (ch * 2))
> +
> +#define MAX5970_REG_LED_FLASH 0x43
> +
> +#define MAX_REGISTERS 0x49
> +#define ADC_MASK 0x3FF
> +
> +struct max597x_data {
> + struct device *dev;
> + int irq;
> + int num_switches;
> + struct regmap *regmap;
> + /* Chip specific parameters needed by regulator & iio cells */
> + u32 irng[MAX5970_NUM_SWITCHES];
> + u32 mon_rng[MAX5970_NUM_SWITCHES];
> + u32 shunt_micro_ohms[MAX5970_NUM_SWITCHES];
> +};
> +
> +#endif
> \ No newline at end of file
???
--
Lee Jones [李琼斯]
Hi Lee,
On 16-11-2022 10:59 pm, Lee Jones wrote:
> On Mon, 14 Nov 2022, Naresh Solanki wrote:
>
>> From: Patrick Rudolph <[email protected]>
>>
>> Implement a regulator driver with IRQ support for fault management.
>> Written against documentation [1] and [2] and tested on real hardware.
>>
>> Every channel has its own regulator supplies nammed 'vss1-supply' and
>> 'vss2-supply'. The regulator supply is used to determine the output
>> voltage, as the smart switch provides no output regulation.
>> The driver requires the 'shunt-resistor-micro-ohms' property to be
>> present in Device Tree to properly calculate current related
>> values.
>>
>> Datasheet links:
>> 1: https://datasheets.maximintegrated.com/en/ds/MAX5970.pdf
>> 2: https://datasheets.maximintegrated.com/en/ds/MAX5978.pdf
>>
>> Signed-off-by: Patrick Rudolph <[email protected]>
>> Signed-off-by: Marcello Sylvester Bauer <[email protected]>
>> Signed-off-by: Naresh Solanki <[email protected]>
>> ---
>> drivers/mfd/Kconfig | 12 +++++
>> drivers/mfd/Makefile | 1 +
>> drivers/mfd/max597x.c | 92 ++++++++++++++++++++++++++++++++
>> include/linux/mfd/max597x.h | 103 ++++++++++++++++++++++++++++++++++++
>> 4 files changed, 208 insertions(+)
>> create mode 100644 drivers/mfd/max597x.c
>> create mode 100644 include/linux/mfd/max597x.h
>>
>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
>> index 8b93856de432..416fe7986b7b 100644
>> --- a/drivers/mfd/Kconfig
>> +++ b/drivers/mfd/Kconfig
>> @@ -253,6 +253,18 @@ config MFD_MADERA_SPI
>> Support for the Cirrus Logic Madera platform audio SoC
>> core functionality controlled via SPI.
>>
>> +config MFD_MAX597X
>> + tristate "Maxim 597x Power Switch and Monitor"
>> + depends on I2C
>> + depends on OF
>> + select MFD_CORE
>> + select REGMAP_I2C
>> + help
>> + This driver controls a Maxim 5970/5978 switch via I2C bus.
>> + The MAX5970/5978 is a smart switch with no output regulation, but
>> + fault protection and voltage and current monitoring capabilities.
>> + Also it supports upto 4 indication LEDs.
>> +
>> config MFD_CS47L15
>> bool "Cirrus Logic CS47L15"
>> select PINCTRL_CS47L15
>> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
>> index 7ed3ef4a698c..819d711fa748 100644
>> --- a/drivers/mfd/Makefile
>> +++ b/drivers/mfd/Makefile
>> @@ -161,6 +161,7 @@ obj-$(CONFIG_MFD_DA9063) += da9063.o
>> obj-$(CONFIG_MFD_DA9150) += da9150-core.o
>>
>> obj-$(CONFIG_MFD_MAX14577) += max14577.o
>> +obj-$(CONFIG_MFD_MAX597X) += max597x.o
>> obj-$(CONFIG_MFD_MAX77620) += max77620.o
>> obj-$(CONFIG_MFD_MAX77650) += max77650.o
>> obj-$(CONFIG_MFD_MAX77686) += max77686.o
>> diff --git a/drivers/mfd/max597x.c b/drivers/mfd/max597x.c
>> new file mode 100644
>> index 000000000000..ae43d57cde26
>> --- /dev/null
>> +++ b/drivers/mfd/max597x.c
>> @@ -0,0 +1,92 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Maxim MAX5970/MAX5978 Power Switch & Monitor
>> + *
>> + * Copyright (c) 2022 9elements GmbH
>> + *
>> + * Author: Patrick Rudolph <[email protected]>
>> + */
>> +
>> +#include <linux/i2c.h>
>> +#include <linux/mfd/core.h>
>> +#include <linux/mfd/max597x.h>
>> +#include <linux/regmap.h>
>> +
>> +static const struct regmap_config max597x_regmap_config = {
>> + .reg_bits = 8,
>> + .val_bits = 8,
>> + .max_register = MAX_REGISTERS,
>> +};
>> +
>> +static const struct mfd_cell max597x_cells[] = {
>> + { .name = "max597x-regulator", },
>> + { .name = "max597x-iio", },
>> + { .name = "max597x-led", },
>> +};
>> +
>> +static int max597x_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
>> +{
>> + struct max597x_data *ddata;
>> + enum max597x_chip_type chip = id->driver_data;
>> +
>> + ddata = devm_kzalloc(&i2c->dev, sizeof(*ddata), GFP_KERNEL);
>> + if (!ddata)
>> + return -ENOMEM;
>> +
>> + /*
>> + * Based on chip type, Initialize the number of switch. This is needed by
>> + * regulator & iio cells.
>> + */
>> + switch (chip) {
>> + case MAX597x_TYPE_MAX5970:
>> + ddata->num_switches = MAX5970_NUM_SWITCHES;
>> + break;
>> + case MAX597x_TYPE_MAX5978:
>> + ddata->num_switches = MAX5978_NUM_SWITCHES;
>> + break;
>> + }
>> +
>> + ddata->regmap = devm_regmap_init_i2c(i2c, &max597x_regmap_config);
>> + if (IS_ERR(ddata->regmap)) {
>> + dev_err(&i2c->dev, "Failed to initialize regmap");
>> + return PTR_ERR(ddata->regmap);
>> + }
>> +
>> + /* IRQ used by regulator cell */
>> + ddata->irq = i2c->irq;
>> + ddata->dev = &i2c->dev;
>
> You won't need both. You can derive one from the other.
>
>> + i2c_set_clientdata(i2c, ddata);
>> +
>> + return devm_mfd_add_devices(ddata->dev, PLATFORM_DEVID_AUTO,
>> + max597x_cells, ARRAY_SIZE(max597x_cells),
>> + NULL, 0, NULL);
>> +}
>> +
>> +static const struct i2c_device_id max597x_table[] = {
>> + { .name = "max5970", MAX597x_TYPE_MAX5970 },
>> + { .name = "max5978", MAX597x_TYPE_MAX5978 },
>> +};
>> +
>> +MODULE_DEVICE_TABLE(i2c, max597x_table);
>> +
>> +static const struct of_device_id max597x_of_match[] = {
>> + { .compatible = "maxim,max5970", .data = (void *)MAX597x_TYPE_MAX5970 },
>> + { .compatible = "maxim,max5978", .data = (void *)MAX597x_TYPE_MAX5978 },
>> + {}
>> +};
>> +
>> +MODULE_DEVICE_TABLE(of, max597x_of_match);
>> +
>> +static struct i2c_driver max597x_driver = {
>> + .id_table = max597x_table,
>> + .driver = {
>> + .name = "max597x",
>> + .of_match_table = of_match_ptr(max597x_of_match),
>> + },
>> + .probe = max597x_probe,
>> +};
>> +module_i2c_driver(max597x_driver);
>> +
>> +MODULE_AUTHOR("Patrick Rudolph <[email protected]>");
>> +MODULE_DESCRIPTION("MAX597X Power Switch and Monitor");
>> +MODULE_LICENSE("GPL v2");
>
> This driver looks *very* simple.
>
> Are you able to use simple-mfd-i2c.c instead?
>
> I suspect the answer to that would be "yes".
>
>> diff --git a/include/linux/mfd/max597x.h b/include/linux/mfd/max597x.h
>> new file mode 100644
>> index 000000000000..99a047e66b44
>> --- /dev/null
>> +++ b/include/linux/mfd/max597x.h
>> @@ -0,0 +1,103 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Maxim MAX5970/MAX5978 Power Switch & Monitor
>> + *
>> + * Copyright (c) 2022 9elements GmbH
>> + *
>> + * Author: Patrick Rudolph <[email protected]>
>> + */
>> +
>> +#ifndef MFD_MAX597X_H
>> +#define MFD_MAX597X_H
>> +
>> +#include <linux/device.h>
>> +#include <linux/regmap.h>
>> +
>> +/* Number of switch based on chip variant */
>> +#define MAX5970_NUM_SWITCHES 2
>> +#define MAX5978_NUM_SWITCHES 1
>> +/* Both chip variant have 4 indication LEDs used by LED cell */
>> +#define MAX597X_NUM_LEDS 4
>> +
>> +enum max597x_chip_type {
>> + MAX597x_TYPE_MAX5978 = 1,
>> + MAX597x_TYPE_MAX5970,
>> +};
>> +
>> +#define MAX5970_REG_CURRENT_L(ch) (0x01 + (ch) * 4)
>> +#define MAX5970_REG_CURRENT_H(ch) (0x00 + (ch) * 4)
>> +#define MAX5970_REG_VOLTAGE_L(ch) (0x03 + (ch) * 4)
>> +#define MAX5970_REG_VOLTAGE_H(ch) (0x02 + (ch) * 4)
>> +#define MAX5970_REG_MON_RANGE 0x18
>> +#define MAX5970_MON_MASK 0x3
>> +#define MAX5970_MON(reg, ch) (((reg) >> ((ch) * 2)) & MAX5970_MON_MASK)
>> +#define MAX5970_MON_MAX_RANGE_UV 16000000
>> +
>> +#define MAX5970_REG_CH_UV_WARN_H(ch) (0x1A + (ch) * 10)
>> +#define MAX5970_REG_CH_UV_WARN_L(ch) (0x1B + (ch) * 10)
>> +#define MAX5970_REG_CH_UV_CRIT_H(ch) (0x1C + (ch) * 10)
>> +#define MAX5970_REG_CH_UV_CRIT_L(ch) (0x1D + (ch) * 10)
>> +#define MAX5970_REG_CH_OV_WARN_H(ch) (0x1E + (ch) * 10)
>> +#define MAX5970_REG_CH_OV_WARN_L(ch) (0x1F + (ch) * 10)
>> +#define MAX5970_REG_CH_OV_CRIT_H(ch) (0x20 + (ch) * 10)
>> +#define MAX5970_REG_CH_OV_CRIT_L(ch) (0x21 + (ch) * 10)
>> +
>> +#define MAX5970_VAL2REG_H(x) (((x) >> 2) & 0xFF)
>> +#define MAX5970_VAL2REG_L(x) ((x) & 0x3)
>> +
>> +#define MAX5970_REG_DAC_FAST(ch) (0x2E + (ch))
>> +
>> +#define MAX5970_FAST2SLOW_RATIO 200
>> +
>> +#define MAX5970_REG_STATUS0 0x31
>> +#define MAX5970_CB_IFAULTF(ch) (1 << (ch))
>> +#define MAX5970_CB_IFAULTS(ch) (1 << ((ch) + 4))
>> +
>> +#define MAX5970_REG_STATUS1 0x32
>> +#define STATUS1_PROT_MASK 0x3
>> +#define STATUS1_PROT(reg) \
>> + (((reg) >> 6) & STATUS1_PROT_MASK)
>> +#define STATUS1_PROT_SHUTDOWN 0
>> +#define STATUS1_PROT_CLEAR_PG 1
>> +#define STATUS1_PROT_ALERT_ONLY 2
>> +
>> +#define MAX5970_REG_STATUS2 0x33
>> +#define MAX5970_IRNG_MASK 0x3
>> +#define MAX5970_IRNG(reg, ch) \
>> + (((reg) >> ((ch) * 2)) & MAX5970_IRNG_MASK)
>> +
>> +#define MAX5970_REG_STATUS3 0x34
>> +#define MAX5970_STATUS3_ALERT BIT(4)
>> +#define MAX5970_STATUS3_PG(ch) BIT(ch)
>> +
>> +#define MAX5970_REG_FAULT0 0x35
>> +#define UV_STATUS_WARN(ch) BIT(ch)
>> +#define UV_STATUS_CRIT(ch) BIT(ch + 4)
>> +
>> +#define MAX5970_REG_FAULT1 0x36
>> +#define OV_STATUS_WARN(ch) BIT(ch)
>> +#define OV_STATUS_CRIT(ch) BIT(ch + 4)
>> +
>> +#define MAX5970_REG_FAULT2 0x37
>> +#define OC_STATUS_WARN(ch) BIT(ch)
>> +
>> +#define MAX5970_REG_CHXEN 0x3b
>> +#define CHXEN(ch) (3 << (ch * 2))
>> +
>> +#define MAX5970_REG_LED_FLASH 0x43
>> +
>> +#define MAX_REGISTERS 0x49
>> +#define ADC_MASK 0x3FF
>> +
>> +struct max597x_data {
>> + struct device *dev;
>> + int irq;
>> + int num_switches;
>> + struct regmap *regmap;
>> + /* Chip specific parameters needed by regulator & iio cells */
>> + u32 irng[MAX5970_NUM_SWITCHES];
>> + u32 mon_rng[MAX5970_NUM_SWITCHES];
>> + u32 shunt_micro_ohms[MAX5970_NUM_SWITCHES];
>> +};
>> +
>> +#endif
>
>> \ No newline at end of file
>
> ???
>
Will clean up in next version.
Regards,
Naresh