2022-07-05 13:11:31

by Naresh Solanki

[permalink] [raw]
Subject: [PATCH 1/5] dt-bindings: mfd: Add bindings for MAX5970 and MAX5978

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]>
---
.../devicetree/bindings/mfd/max5970.yaml | 164 ++++++++++++++++++
1 file changed, 164 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/max5970.yaml

diff --git a/Documentation/devicetree/bindings/mfd/max5970.yaml b/Documentation/devicetree/bindings/mfd/max5970.yaml
new file mode 100644
index 000000000000..a0cc6a7543b5
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/max5970.yaml
@@ -0,0 +1,164 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/max5970.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Regulator driver 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 single channel.
+
+ patternProperties:
+ "^led@[0-3]$":
+ $ref: /schemas/leds/common.yaml#
+ type: object
+
+ additionalProperties: true
+
+ vss1-supply:
+ description: Supply of the first channel.
+
+ "#io-channel-cells":
+ const: 1
+
+ regulators:
+ type: object
+ description:
+ Properties for single channel.
+
+ patternProperties:
+ "^(sw[0-1])$":
+ $ref: /schemas/regulator/regulator.yaml#
+ type: object
+
+ shunt-resistor-micro-ohms:
+ description: |
+ The value of curent sense resistor in microohms.
+ Must be specified for each channel.
+
+ additionalProperties: true
+
+required:
+ - compatible
+ - reg
+ - regulators
+ - vss1-supply
+
+allOf:
+ - $ref: /schemas/regulator/regulator.yaml#
+ - if:
+ properties:
+ compatible:
+ enum:
+ - maxim,max5970
+ then:
+ properties:
+ vss2-supply:
+ description: Supply of the second channel.
+
+ 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>;
+ 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";
+ };
+ };
+ regulator@3a {
+ reg = <0x3a>;
+ vss1-supply = <&p3v3>;
+ compatible = "maxim,max5978";
+
+ regulators {
+ sw0_ref_0: SW0 {
+ regulator-compatible = "SW0";
+ shunt-resistor-micro-ohms = <12000>;
+ };
+ };
+ };
+ };
+
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ regulator@3a {
+ reg = <0x3a>;
+ vss1-supply = <&p3v3>;
+ vss2-supply = <&p5v>;
+ compatible = "maxim,max5970";
+
+ 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.35.3


2022-07-05 13:11:55

by Naresh Solanki

[permalink] [raw]
Subject: [PATCH 5/5] leds: max597x: Add support for max597x

From: Patrick Rudolph <[email protected]>

max597x is hot swap controller with indication led support.
This driver uses DT property to configure led during boot time &
also provide the led control in sysfs.

Signed-off-by: Patrick Rudolph <[email protected]>
Signed-off-by: Naresh Solanki <[email protected]>
---
drivers/leds/Kconfig | 10 +++
drivers/leds/Makefile | 1 +
drivers/leds/leds-max597x.c | 130 ++++++++++++++++++++++++++++++++++++
3 files changed, 141 insertions(+)
create mode 100644 drivers/leds/leds-max597x.c

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index a49979f41eee..682748097276 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -598,6 +598,16 @@ config LEDS_ADP5520
To compile this driver as a module, choose M here: the module will
be called leds-adp5520.

+config LEDS_MAX597X
+ tristate "Maxim 597x leds"
+ depends on I2C
+ depends on OF
+ depends on LEDS_CLASS
+ select MFD_MAX597X
+ help
+ This driver controls a Maxim 5970/5978 indication led via I2C bus.
+ The MAX5970/5978 is a smart switch with 4 indication leds
+
config LEDS_MC13783
tristate "LED Support for MC13XXX PMIC"
depends on LEDS_CLASS
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 4fd2f92cd198..c44b0e9559ab 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -54,6 +54,7 @@ obj-$(CONFIG_LEDS_LP8501) += leds-lp8501.o
obj-$(CONFIG_LEDS_LP8788) += leds-lp8788.o
obj-$(CONFIG_LEDS_LP8860) += leds-lp8860.o
obj-$(CONFIG_LEDS_LT3593) += leds-lt3593.o
+obj-$(CONFIG_LEDS_MAX597X) += leds-max597x.o
obj-$(CONFIG_LEDS_MAX77650) += leds-max77650.o
obj-$(CONFIG_LEDS_MAX8997) += leds-max8997.o
obj-$(CONFIG_LEDS_MC13783) += leds-mc13783.o
diff --git a/drivers/leds/leds-max597x.c b/drivers/leds/leds-max597x.c
new file mode 100644
index 000000000000..645d6cdcfa34
--- /dev/null
+++ b/drivers/leds/leds-max597x.c
@@ -0,0 +1,130 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Device driver for regulators in MAX5970 and MAX5978 IC
+ *
+ * Copyright (c) 2022 9elements GmbH
+ *
+ * Author: Patrick Rudolph <[email protected]>
+ */
+
+#include <linux/bitops.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/i2c.h>
+#include <linux/mfd/max597x.h>
+#include <linux/regmap.h>
+#include <linux/version.h>
+#include <linux/platform_device.h>
+
+struct max597x_led {
+ struct regmap *regmap;
+ struct led_classdev led;
+ unsigned int index;
+};
+
+static int max597x_led_set_brightness(struct led_classdev *cdev,
+ enum led_brightness brightness)
+{
+ struct max597x_led *led = cdev->driver_data;
+ int ret;
+
+ if (!led || !led->regmap)
+ return -1;
+
+ ret = regmap_update_bits(led->regmap, MAX5970_REG_LED_FLASH,
+ 1 << led->index, ~brightness << led->index);
+ if (ret < 0)
+ dev_err(cdev->dev, "failed to set brightness %d\n", ret);
+ return ret;
+}
+
+static int max597x_led(struct max597x_data *max597x, struct device_node *nc, u32 reg)
+{
+ struct max597x_led *led;
+ const char *state;
+ int ret = 0;
+
+ led = devm_kzalloc(max597x->dev, sizeof(struct max597x_led),
+ GFP_KERNEL);
+ if (!led)
+ return -ENOMEM;
+
+ if (of_property_read_string(nc, "label", &led->led.name))
+ led->led.name = nc->name;
+
+ led->led.max_brightness = 1;
+ led->led.brightness_set_blocking = max597x_led_set_brightness;
+ led->led.default_trigger = "none";
+ led->index = reg;
+ led->regmap = max597x->regmap;
+ ret = led_classdev_register(max597x->dev, &led->led);
+ if (ret) {
+ dev_err(max597x->dev, "Error in initializing led %s", led->led.name);
+ devm_kfree(max597x->dev, led);
+ return ret;
+ }
+ led->led.driver_data = led;
+ led->led.dev = max597x->dev;
+ if (!of_property_read_string(nc, "default-state", &state)) {
+ if (!strcmp(state, "on")) {
+ led->led.brightness = 1;
+ led_set_brightness(&led->led, led->led.brightness);
+ }
+ }
+ return 0;
+}
+
+static int max597x_led_probe(struct platform_device *pdev)
+{
+
+
+ struct max597x_data *max597x = dev_get_drvdata(pdev->dev.parent);
+ struct device_node *np = dev_of_node(pdev->dev.parent);
+ struct device_node *led_node;
+ struct device_node *child;
+ int ret = 0;
+
+
+ led_node = of_get_child_by_name(np, "leds");
+ if (!led_node)
+ return -ENODEV;
+
+ for_each_available_child_of_node(led_node, child) {
+ u32 reg;
+
+ if (of_property_read_u32(child, "reg", &reg))
+ continue;
+
+ if (reg >= MAX597X_NUM_LEDS) {
+ dev_err(max597x->dev, "invalid LED (%u >= %d)\n", reg,
+ MAX597X_NUM_LEDS);
+ continue;
+ }
+
+ ret = max597x_led(max597x, child, reg);
+ if (ret < 0) {
+ of_node_put(child);
+ return ret;
+ }
+ }
+
+ return ret;
+}
+
+static struct platform_driver max597x_led_driver = {
+ .driver = {
+ .name = "max597x-led",
+ },
+ .probe = max597x_led_probe,
+};
+
+module_platform_driver(max597x_led_driver);
+
+
+MODULE_AUTHOR("Patrick Rudolph <[email protected]>");
+MODULE_DESCRIPTION("MAX5970_hot-swap controller driver");
+MODULE_LICENSE("GPL v2");
--
2.35.3

2022-07-05 13:15:49

by Naresh Solanki

[permalink] [raw]
Subject: [PATCH 3/5] regulator: max597x: Add support for max597x regulator

From: Patrick Rudolph <[email protected]>

max597x is hot swap controller.
This regulator driver controls the same & also configures fault
protection features supported by the chip.

Signed-off-by: Patrick Rudolph <[email protected]>
Signed-off-by: Marcello Sylvester Bauer <[email protected]>
Signed-off-by: Naresh Solanki <[email protected]>
---
drivers/regulator/Kconfig | 10 +
drivers/regulator/Makefile | 1 +
drivers/regulator/max597x-regulator.c | 506 ++++++++++++++++++++++++++
3 files changed, 517 insertions(+)
create mode 100644 drivers/regulator/max597x-regulator.c

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index cbe0f96ca342..775d87a5d59d 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -546,6 +546,16 @@ config REGULATOR_MAX1586
regulator via I2C bus. The provided regulator is suitable
for PXA27x chips to control VCC_CORE and VCC_USIM voltages.

+config REGULATOR_MAX597X
+ tristate "Maxim 597x power switch and monitor"
+ depends on I2C
+ depends on OF
+ select MFD_MAX597X
+ 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.
+
config REGULATOR_MAX77620
tristate "Maxim 77620/MAX20024 voltage regulator"
depends on MFD_MAX77620 || COMPILE_TEST
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 8d3ee8b6d41d..4a8a42998561 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -67,6 +67,7 @@ obj-$(CONFIG_REGULATOR_LTC3589) += ltc3589.o
obj-$(CONFIG_REGULATOR_LTC3676) += ltc3676.o
obj-$(CONFIG_REGULATOR_MAX14577) += max14577-regulator.o
obj-$(CONFIG_REGULATOR_MAX1586) += max1586.o
+obj-$(CONFIG_REGULATOR_MAX597X) += max597x-regulator.o
obj-$(CONFIG_REGULATOR_MAX77620) += max77620-regulator.o
obj-$(CONFIG_REGULATOR_MAX77650) += max77650-regulator.o
obj-$(CONFIG_REGULATOR_MAX8649) += max8649.o
diff --git a/drivers/regulator/max597x-regulator.c b/drivers/regulator/max597x-regulator.c
new file mode 100644
index 000000000000..f95acd1c5e3a
--- /dev/null
+++ b/drivers/regulator/max597x-regulator.c
@@ -0,0 +1,506 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Device driver for regulators in MAX5970 and MAX5978 IC
+ *
+ * Copyright (c) 2022 9elements GmbH
+ *
+ * Author: Patrick Rudolph <[email protected]>
+ */
+
+#include <linux/bitops.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/module.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/i2c.h>
+#include <linux/regmap.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+#include <linux/regulator/of_regulator.h>
+#include <linux/version.h>
+#include <linux/platform_device.h>
+
+#include <linux/mfd/max597x.h>
+
+struct max597x_regulator {
+ int num_switches, mon_rng, irng, shunt_micro_ohms, lim_uA;
+ struct regmap *regmap;
+};
+
+enum max597x_regulator_id {
+ MAX597X_SW0,
+ MAX597X_SW1,
+};
+
+static int max597x_uvp_ovp_check_mode(struct regulator_dev *rdev, int severity)
+{
+ int ret, reg;
+
+ /* Status1 register contains the soft strap values sampled at POR */
+ ret = regmap_read(rdev->regmap, MAX5970_REG_STATUS1, &reg);
+ if (ret)
+ return ret;
+
+ /* Check soft straps match requested mode */
+ if (severity == REGULATOR_SEVERITY_PROT) {
+ if (STATUS1_PROT(reg) != STATUS1_PROT_SHUTDOWN)
+ return -EOPNOTSUPP;
+
+ return 0;
+ }
+ if (STATUS1_PROT(reg) == STATUS1_PROT_SHUTDOWN)
+ return -EOPNOTSUPP;
+
+ return 0;
+}
+
+static int max597x_set_vp(struct regulator_dev *rdev, int lim_uV, int severity,
+ bool enable, bool overvoltage)
+{
+ int off_h, off_l, reg, ret;
+ struct max597x_regulator *data = rdev_get_drvdata(rdev);
+ int channel = rdev_get_id(rdev);
+
+ if (overvoltage) {
+ if (severity == REGULATOR_SEVERITY_WARN) {
+ off_h = MAX5970_REG_CH_OV_WARN_H(channel);
+ off_l = MAX5970_REG_CH_OV_WARN_L(channel);
+ } else {
+ off_h = MAX5970_REG_CH_OV_CRIT_H(channel);
+ off_l = MAX5970_REG_CH_OV_CRIT_L(channel);
+ }
+ } else {
+ if (severity == REGULATOR_SEVERITY_WARN) {
+ off_h = MAX5970_REG_CH_UV_WARN_H(channel);
+ off_l = MAX5970_REG_CH_UV_WARN_L(channel);
+ } else {
+ off_h = MAX5970_REG_CH_UV_CRIT_H(channel);
+ off_l = MAX5970_REG_CH_UV_CRIT_L(channel);
+ }
+ }
+
+ if (enable)
+ /* reg = ADC_MASK * (lim_uV / 1000000) / (data->mon_rng / 1000000) */
+ reg = ADC_MASK * lim_uV / data->mon_rng;
+ else
+ reg = 0;
+
+ ret = regmap_write(rdev->regmap, off_h, MAX5970_VAL2REG_H(reg));
+ if (ret)
+ return ret;
+
+ ret = regmap_write(rdev->regmap, off_l, MAX5970_VAL2REG_L(reg));
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static int max597x_set_uvp(struct regulator_dev *rdev, int lim_uV, int severity,
+ bool enable)
+{
+ int ret;
+
+ /*
+ * MAX5970 has enable control as a special value in limit reg. Can't
+ * set limit but keep feature disabled or enable W/O given limit.
+ */
+ if ((lim_uV && !enable) || (!lim_uV && enable))
+ return -EINVAL;
+
+ ret = max597x_uvp_ovp_check_mode(rdev, severity);
+ if (ret)
+ return ret;
+
+ return max597x_set_vp(rdev, lim_uV, severity, enable, false);
+}
+
+static int max597x_set_ovp(struct regulator_dev *rdev, int lim_uV, int severity,
+ bool enable)
+{
+ int ret;
+
+ /*
+ * MAX5970 has enable control as a special value in limit reg. Can't
+ * set limit but keep feature disabled or enable W/O given limit.
+ */
+ if ((lim_uV && !enable) || (!lim_uV && enable))
+ return -EINVAL;
+
+ ret = max597x_uvp_ovp_check_mode(rdev, severity);
+ if (ret)
+ return ret;
+
+ return max597x_set_vp(rdev, lim_uV, severity, enable, true);
+}
+
+static int max597x_set_ocp(struct regulator_dev *rdev, int lim_uA,
+ int severity, bool enable)
+{
+ int ret, val, reg;
+ unsigned int vthst, vthfst;
+
+ struct max597x_regulator *data = rdev_get_drvdata(rdev);
+ int rdev_id = rdev_get_id(rdev);
+ /*
+ * MAX5970 doesn't has enable control for ocp.
+ * If limit is specified but enable is not set then hold the value in
+ * variable & later use it when ocp needs to be enabled.
+ */
+ if (lim_uA != 0 && lim_uA != data->lim_uA)
+ data->lim_uA = lim_uA;
+
+ if (severity != REGULATOR_SEVERITY_PROT)
+ return -EINVAL;
+
+ if (enable) {
+
+ /* Calc Vtrip threshold in uV. */
+ vthst =
+ div_u64(mul_u32_u32(data->shunt_micro_ohms, data->lim_uA),
+ 1000000);
+
+ /*
+ * As recommended in datasheed, add 20% margin to avoid
+ * spurious event & passive component tolerance.
+ */
+ vthst = div_u64(mul_u32_u32(vthst, 120), 100);
+
+ /* Calc fast Vtrip threshold in uV */
+ vthfst = vthst * (MAX5970_FAST2SLOW_RATIO / 100);
+
+ if (vthfst > data->irng) {
+ dev_err(&rdev->dev, "Current limit out of range\n");
+ return -EINVAL;
+ }
+ /* Fast trip threshold to be programmed */
+ val = div_u64(mul_u32_u32(0xFF, vthfst), data->irng);
+ } else
+ /*
+ * Since there is no option to disable ocp, set limit to max
+ * value
+ */
+ val = 0xFF;
+
+ reg = MAX5970_REG_DAC_FAST(rdev_id);
+ ret = regmap_write(rdev->regmap, reg, val);
+
+ return ret;
+}
+
+static int max597x_get_status(struct regulator_dev *rdev)
+{
+ int val, ret;
+
+ ret = regmap_read(rdev->regmap, MAX5970_REG_STATUS3, &val);
+ if (ret)
+ return REGULATOR_FAILED_RETRY;
+
+ if (val & MAX5970_STATUS3_ALERT)
+ return REGULATOR_STATUS_ERROR;
+
+ ret = regulator_is_enabled_regmap(rdev);
+ if (ret < 0)
+ return ret;
+
+ if (ret)
+ return REGULATOR_STATUS_ON;
+
+ return REGULATOR_STATUS_OFF;
+}
+
+static const struct regulator_ops max597x_switch_ops = {
+ .enable = regulator_enable_regmap,
+ .disable = regulator_disable_regmap,
+ .is_enabled = regulator_is_enabled_regmap,
+ .get_status = max597x_get_status,
+ .set_over_voltage_protection = max597x_set_ovp,
+ .set_under_voltage_protection = max597x_set_uvp,
+ .set_over_current_protection = max597x_set_ocp,
+};
+
+static int max597x_dt_parse(struct device_node *np,
+ const struct regulator_desc *desc,
+ struct regulator_config *cfg)
+{
+ struct max597x_regulator *data = cfg->driver_data;
+ int ret = 0;
+
+ ret =
+ of_property_read_u32(np, "shunt-resistor-micro-ohms",
+ &data->shunt_micro_ohms);
+ if (ret < 0)
+ dev_err(cfg->dev,
+ "property 'shunt-resistor-micro-ohms' not found, err %d\n",
+ ret);
+ return ret;
+
+}
+
+#define MAX597X_SWITCH(_ID, _ereg, _chan, _supply) { \
+ .name = #_ID, \
+ .of_match = of_match_ptr(#_ID), \
+ .ops = &max597x_switch_ops, \
+ .regulators_node = of_match_ptr("regulators"), \
+ .type = REGULATOR_VOLTAGE, \
+ .id = MAX597X_##_ID, \
+ .owner = THIS_MODULE, \
+ .supply_name = _supply, \
+ .enable_reg = _ereg, \
+ .enable_mask = CHXEN((_chan)), \
+ .of_parse_cb = max597x_dt_parse, \
+}
+
+static const struct regulator_desc regulators[] = {
+ MAX597X_SWITCH(SW0, MAX5970_REG_CHXEN, 0, "vss1"),
+ MAX597X_SWITCH(SW1, MAX5970_REG_CHXEN, 1, "vss2"),
+};
+
+static int max597x_regmap_read_clear(struct regmap *map, unsigned int reg,
+ unsigned int *val)
+{
+ int ret;
+
+ ret = regmap_read(map, reg, val);
+ if (ret)
+ return ret;
+
+ if (*val)
+ return regmap_write(map, reg, *val);
+
+ return 0;
+}
+
+static int max597x_irq_handler(int irq, struct regulator_irq_data *rid,
+ unsigned long *dev_mask)
+{
+ struct regulator_err_state *stat;
+ struct max597x_regulator *d = (struct max597x_regulator *)rid->data;
+ int val, ret, i;
+
+ ret = max597x_regmap_read_clear(d->regmap, MAX5970_REG_FAULT0, &val);
+ if (ret)
+ return REGULATOR_FAILED_RETRY;
+
+ *dev_mask = 0;
+ for (i = 0; i < d->num_switches; i++) {
+ stat = &rid->states[i];
+ stat->notifs = 0;
+ stat->errors = 0;
+ }
+
+ for (i = 0; i < d->num_switches; i++) {
+ stat = &rid->states[i];
+
+ if (val & UV_STATUS_CRIT(i)) {
+ *dev_mask |= 1 << i;
+ stat->notifs |= REGULATOR_EVENT_UNDER_VOLTAGE;
+ stat->errors |= REGULATOR_ERROR_UNDER_VOLTAGE;
+ } else if (val & UV_STATUS_WARN(i)) {
+ *dev_mask |= 1 << i;
+ stat->notifs |= REGULATOR_EVENT_UNDER_VOLTAGE_WARN;
+ stat->errors |= REGULATOR_ERROR_UNDER_VOLTAGE_WARN;
+ }
+ }
+
+ ret = max597x_regmap_read_clear(d->regmap, MAX5970_REG_FAULT1, &val);
+ if (ret)
+ return REGULATOR_FAILED_RETRY;
+
+ for (i = 0; i < d->num_switches; i++) {
+ stat = &rid->states[i];
+
+ if (val & OV_STATUS_CRIT(i)) {
+ *dev_mask |= 1 << i;
+ stat->notifs |= REGULATOR_EVENT_REGULATION_OUT;
+ stat->errors |= REGULATOR_ERROR_REGULATION_OUT;
+ } else if (val & OV_STATUS_WARN(i)) {
+ *dev_mask |= 1 << i;
+ stat->notifs |= REGULATOR_EVENT_OVER_VOLTAGE_WARN;
+ stat->errors |= REGULATOR_ERROR_OVER_VOLTAGE_WARN;
+ }
+ }
+
+ ret = max597x_regmap_read_clear(d->regmap, MAX5970_REG_FAULT2, &val);
+ if (ret)
+ return REGULATOR_FAILED_RETRY;
+
+ for (i = 0; i < d->num_switches; i++) {
+ stat = &rid->states[i];
+
+ if (val & OC_STATUS_WARN(i)) {
+ *dev_mask |= 1 << i;
+ stat->notifs |= REGULATOR_EVENT_OVER_CURRENT_WARN;
+ stat->errors |= REGULATOR_ERROR_OVER_CURRENT_WARN;
+ }
+ }
+
+ ret = regmap_read(d->regmap, MAX5970_REG_STATUS0, &val);
+ if (ret)
+ return REGULATOR_FAILED_RETRY;
+
+ for (i = 0; i < d->num_switches; i++) {
+ stat = &rid->states[i];
+
+ if ((val & MAX5970_CB_IFAULTF(i))
+ || (val & MAX5970_CB_IFAULTS(i))) {
+ *dev_mask |= 1 << i;
+ stat->notifs |=
+ REGULATOR_EVENT_OVER_CURRENT |
+ REGULATOR_EVENT_DISABLE;
+ stat->errors |=
+ REGULATOR_ERROR_OVER_CURRENT | REGULATOR_ERROR_FAIL;
+
+ /* Clear the sub-IRQ status */
+ regulator_disable_regmap(stat->rdev);
+ }
+ }
+ return 0;
+}
+
+static const struct regmap_config max597x_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .max_register = MAX_REGISTERS,
+};
+
+static int max597x_adc_range(struct regmap *regmap, const int ch,
+ u32 *irng, u32 *mon_rng)
+{
+ unsigned int reg;
+ int ret;
+
+ /* Decode current ADC range */
+ ret = regmap_read(regmap, MAX5970_REG_STATUS2, &reg);
+ if (ret)
+ return ret;
+ switch (MAX5970_IRNG(reg, ch)) {
+ case 0:
+ *irng = 100000; /* 100 mV */
+ break;
+ case 1:
+ *irng = 50000; /* 50 mV */
+ break;
+ case 2:
+ *irng = 25000; /* 25 mV */
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ /* Decode current voltage monitor range */
+ ret = regmap_read(regmap, MAX5970_REG_MON_RANGE, &reg);
+ if (ret)
+ return ret;
+
+ *mon_rng = MAX5970_MON_MAX_RANGE_UV >> MAX5970_MON(reg, ch);
+
+ return 0;
+}
+
+static int max597x_setup_irq(struct device *dev,
+ int irq,
+ struct regulator_dev *rdevs[MAX5970_NUM_SWITCHES],
+ int num_switches, struct max597x_regulator *data)
+{
+ struct regulator_irq_desc max597x_notif = {
+ .name = "max597x-irq",
+ .map_event = max597x_irq_handler,
+ .data = data,
+ };
+ int errs = REGULATOR_ERROR_UNDER_VOLTAGE |
+ REGULATOR_ERROR_UNDER_VOLTAGE_WARN |
+ REGULATOR_ERROR_OVER_VOLTAGE_WARN |
+ REGULATOR_ERROR_REGULATION_OUT |
+ REGULATOR_ERROR_OVER_CURRENT |
+ REGULATOR_ERROR_OVER_CURRENT_WARN | REGULATOR_ERROR_FAIL;
+ void *irq_helper;
+
+ /* Register notifiers - can fail if IRQ is not given */
+ irq_helper = devm_regulator_irq_helper(dev, &max597x_notif,
+ irq, 0, errs, NULL,
+ &rdevs[0], num_switches);
+ if (IS_ERR(irq_helper)) {
+ if (PTR_ERR(irq_helper) == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+
+ dev_warn(dev, "IRQ disabled %pe\n", irq_helper);
+ }
+
+ return 0;
+}
+
+static int max597x_regulator_probe(struct platform_device *pdev)
+{
+
+
+ struct max597x_data *max597x = dev_get_drvdata(pdev->dev.parent);
+ struct max597x_regulator *data;
+
+ struct regulator_config config = { };
+ struct regulator_dev *rdev;
+ struct regulator_dev *rdevs[MAX5970_NUM_SWITCHES];
+ int num_switches = max597x->num_switches;
+ int ret, i;
+
+ for (i = 0; i < num_switches; i++) {
+ data =
+ devm_kzalloc(max597x->dev, sizeof(struct max597x_regulator),
+ GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ data->num_switches = num_switches;
+ data->regmap = max597x->regmap;
+
+ if (ret < 0)
+ return ret;
+
+ ret = max597x_adc_range(data->regmap, i, &max597x->irng[i], &max597x->mon_rng[i]);
+ if (ret < 0)
+ return ret;
+
+ data->irng = max597x->irng[i];
+ data->mon_rng = max597x->mon_rng[i];
+
+ config.dev = max597x->dev;
+ config.driver_data = (void *)data;
+ config.regmap = data->regmap;
+ rdev = devm_regulator_register(max597x->dev,
+ &regulators[i], &config);
+ if (IS_ERR(rdev)) {
+ dev_err(max597x->dev, "failed to register regulator %s\n",
+ regulators[i].name);
+ return PTR_ERR(rdev);
+ }
+ rdevs[i] = rdev;
+ max597x->shunt_micro_ohms[i] = data->shunt_micro_ohms;
+ }
+
+ if (max597x->irq) {
+ ret =
+ max597x_setup_irq(max597x->dev, max597x->irq, rdevs, num_switches,
+ data);
+ if (ret) {
+ dev_err(max597x->dev, "IRQ setup failed");
+ return ret;
+ }
+ }
+
+ return ret;
+}
+
+static struct platform_driver max597x_regulator_driver = {
+ .driver = {
+ .name = "max597x-regulator",
+ },
+ .probe = max597x_regulator_probe,
+};
+
+module_platform_driver(max597x_regulator_driver);
+
+
+MODULE_AUTHOR("Patrick Rudolph <[email protected]>");
+MODULE_DESCRIPTION("MAX5970_hot-swap controller driver");
+MODULE_LICENSE("GPL v2");
--
2.35.3

2022-07-05 13:21:13

by Naresh Solanki

[permalink] [raw]
Subject: [PATCH 4/5] iio: max597x: Add support for max597x

From: Patrick Rudolph <[email protected]>

max597x has 10bit ADC for voltage & current monitoring.
Use iio framework to expose the same in sysfs.

Signed-off-by: Patrick Rudolph <[email protected]>
Signed-off-by: Naresh Solanki <[email protected]>
---
drivers/iio/adc/Kconfig | 9 ++
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/max597x-iio.c | 156 ++++++++++++++++++++++++++++++++++
3 files changed, 166 insertions(+)
create mode 100644 drivers/iio/adc/max597x-iio.c

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 48ace7412874..d4676eefb60f 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -683,6 +683,15 @@ config MAX1363
To compile this driver as a module, choose M here: the module will be
called max1363.

+config MAX597X_IIO
+ tristate "Maxim 597x power switch and monitor"
+ depends on I2C
+ depends on OF
+ select MFD_MAX597X
+ help
+ This driver exposes Maxim 5970/5978 voltage/current monitoring
+ interface using iio framework.
+
config MAX9611
tristate "Maxim max9611/max9612 ADC driver"
depends on I2C
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 39d806f6d457..f8cb5a30a946 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -63,6 +63,7 @@ obj-$(CONFIG_MAX11100) += max11100.o
obj-$(CONFIG_MAX1118) += max1118.o
obj-$(CONFIG_MAX1241) += max1241.o
obj-$(CONFIG_MAX1363) += max1363.o
+obj-$(CONFIG_MAX597X_IIO) += max597x-iio.o
obj-$(CONFIG_MAX9611) += max9611.o
obj-$(CONFIG_MCP320X) += mcp320x.o
obj-$(CONFIG_MCP3422) += mcp3422.o
diff --git a/drivers/iio/adc/max597x-iio.c b/drivers/iio/adc/max597x-iio.c
new file mode 100644
index 000000000000..de0ea762c5c8
--- /dev/null
+++ b/drivers/iio/adc/max597x-iio.c
@@ -0,0 +1,156 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Device driver for regulators in MAX5970 and MAX5978 IC
+ *
+ * Copyright (c) 2022 9elements GmbH
+ *
+ * Author: Patrick Rudolph <[email protected]>
+ */
+
+#include <linux/bitops.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/module.h>
+#include <linux/io.h>
+#include <linux/iio/iio.h>
+#include <linux/of.h>
+#include <linux/i2c.h>
+#include <linux/mfd/max597x.h>
+#include <linux/regmap.h>
+#include <linux/version.h>
+#include <linux/platform_device.h>
+
+struct max597x_iio {
+ struct regmap *regmap;
+ int shunt_micro_ohms[MAX5970_NUM_SWITCHES];
+ unsigned int irng[MAX5970_NUM_SWITCHES];
+ unsigned int mon_rng[MAX5970_NUM_SWITCHES];
+};
+
+#define MAX597X_ADC_CHANNEL(_idx, _type) { \
+ .type = IIO_ ## _type, \
+ .indexed = 1, \
+ .channel = (_idx), \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
+ BIT(IIO_CHAN_INFO_SCALE), \
+ .address = MAX5970_REG_ ## _type ## _L(_idx), \
+}
+
+static const struct iio_chan_spec max5978_adc_iio_channels[] = {
+ MAX597X_ADC_CHANNEL(0, VOLTAGE),
+ MAX597X_ADC_CHANNEL(0, CURRENT),
+};
+
+static const struct iio_chan_spec max5970_adc_iio_channels[] = {
+ MAX597X_ADC_CHANNEL(0, VOLTAGE),
+ MAX597X_ADC_CHANNEL(0, CURRENT),
+ MAX597X_ADC_CHANNEL(1, VOLTAGE),
+ MAX597X_ADC_CHANNEL(1, CURRENT),
+};
+
+static int max597x_iio_read_raw(struct iio_dev *iio_dev,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2, long info)
+{
+ int ret;
+ struct max597x_iio *data = iio_priv(iio_dev);
+ unsigned int reg_l, reg_h;
+
+ switch (info) {
+ case IIO_CHAN_INFO_RAW:
+ ret = regmap_read(data->regmap, chan->address, &reg_l);
+ if (ret < 0)
+ return ret;
+ ret = regmap_read(data->regmap, chan->address - 1, &reg_h);
+ if (ret < 0)
+ return ret;
+ *val = (reg_h << 2) | (reg_l & 3);
+
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_SCALE:
+
+ switch (chan->address) {
+ case MAX5970_REG_CURRENT_L(0):
+ fallthrough;
+ case MAX5970_REG_CURRENT_L(1):
+ /* in A, convert to mA */
+ *val = data->irng[chan->channel] * 1000;
+ *val2 =
+ data->shunt_micro_ohms[chan->channel] * ADC_MASK;
+ return IIO_VAL_FRACTIONAL;
+
+ case MAX5970_REG_VOLTAGE_L(0):
+ fallthrough;
+ case MAX5970_REG_VOLTAGE_L(1):
+ /* in uV, convert to mV */
+ *val = data->mon_rng[chan->channel];
+ *val2 = ADC_MASK * 1000;
+ return IIO_VAL_FRACTIONAL;
+ }
+
+ break;
+ }
+ return -EINVAL;
+}
+
+static const struct iio_info max597x_adc_iio_info = {
+ .read_raw = &max597x_iio_read_raw,
+};
+
+static int max597x_iio_probe(struct platform_device *pdev)
+{
+ struct max597x_data *max597x = dev_get_drvdata(pdev->dev.parent);
+ struct iio_dev *indio_dev;
+ struct max597x_iio *priv;
+ int ret, i;
+
+ /* registering iio */
+ indio_dev = devm_iio_device_alloc(max597x->dev, sizeof(*priv));
+ if (!indio_dev) {
+ dev_err(max597x->dev, "failed allocating iio device\n");
+ return -ENOMEM;
+ }
+ indio_dev->name = dev_name(max597x->dev);
+ indio_dev->info = &max597x_adc_iio_info;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+
+ switch (max597x->num_switches) {
+ case MAX597x_TYPE_MAX5970:
+ indio_dev->channels = max5970_adc_iio_channels;
+ indio_dev->num_channels = ARRAY_SIZE(max5970_adc_iio_channels);
+ break;
+ case MAX597x_TYPE_MAX5978:
+ indio_dev->channels = max5978_adc_iio_channels;
+ indio_dev->num_channels = ARRAY_SIZE(max5978_adc_iio_channels);
+ break;
+ }
+
+ priv = iio_priv(indio_dev);
+ priv->regmap = max597x->regmap;
+ for (i = 0; i < indio_dev->num_channels; i++) {
+ priv->irng[i] = max597x->irng[i];
+ priv->mon_rng[i] = max597x->mon_rng[i];
+ priv->shunt_micro_ohms[i] = max597x->shunt_micro_ohms[i];
+ }
+
+ ret = devm_iio_device_register(max597x->dev, indio_dev);
+ if (ret)
+ dev_err(max597x->dev, "could not register iio device");
+
+ return ret;
+
+}
+
+static struct platform_driver max597x_iio_driver = {
+ .driver = {
+ .name = "max597x-iio",
+ },
+ .probe = max597x_iio_probe,
+};
+
+module_platform_driver(max597x_iio_driver);
+
+
+MODULE_AUTHOR("Patrick Rudolph <[email protected]>");
+MODULE_DESCRIPTION("MAX5970_hot-swap controller driver");
+MODULE_LICENSE("GPL v2");
--
2.35.3

2022-07-06 11:05:18

by Mark Brown

[permalink] [raw]
Subject: Re: (subset) [PATCH 1/5] dt-bindings: mfd: Add bindings for MAX5970 and MAX5978

On Tue, 5 Jul 2022 14:22:39 +0200, Naresh Solanki wrote:
> 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.
>
>

Applied to

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next

Thanks!

[3/5] regulator: max597x: Add support for max597x regulator
commit: 38493f008deb435577361d4c4cdd69f7bb30f4b9

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

2022-07-12 07:35:10

by Matti Vaittinen

[permalink] [raw]
Subject: Re: [PATCH 3/5] regulator: max597x: Add support for max597x regulator

On 7/5/22 15:22, Naresh Solanki wrote:
> From: Patrick Rudolph <[email protected]>
>
> max597x is hot swap controller.
> This regulator driver controls the same & also configures fault
> protection features supported by the chip.
>
> Signed-off-by: Patrick Rudolph <[email protected]>
> Signed-off-by: Marcello Sylvester Bauer <[email protected]>
> Signed-off-by: Naresh Solanki <[email protected]>

I like the way the IRQ helpers have been used here. It'd be cool to hear
how the rest of the system you're dealing with utilize the WARN level
events :)

> +static int max597x_set_ocp(struct regulator_dev *rdev, int lim_uA,
> + int severity, bool enable)
> +{
> + int ret, val, reg;
> + unsigned int vthst, vthfst;
> +
> + struct max597x_regulator *data = rdev_get_drvdata(rdev);
> + int rdev_id = rdev_get_id(rdev);
> + /*
> + * MAX5970 doesn't has enable control for ocp.
> + * If limit is specified but enable is not set then hold the value in
> + * variable & later use it when ocp needs to be enabled.
> + */

Is this a possible scenario? I think that if a non zero limit is given
in a "regulator-oc-protection-microamp"-property, then the protection
should always be enabled. Am I overlooking something?

> + if (lim_uA != 0 && lim_uA != data->lim_uA)
> + data->lim_uA = lim_uA;
> +
> + if (severity != REGULATOR_SEVERITY_PROT)
> + return -EINVAL;
> +
> + if (enable) {
> +
> + /* Calc Vtrip threshold in uV. */
> + vthst =
> + div_u64(mul_u32_u32(data->shunt_micro_ohms, data->lim_uA),
> + 1000000);
> +
> + /*
> + * As recommended in datasheed, add 20% margin to avoid
> + * spurious event & passive component tolerance.
> + */
> + vthst = div_u64(mul_u32_u32(vthst, 120), 100);
> +
> + /* Calc fast Vtrip threshold in uV */
> + vthfst = vthst * (MAX5970_FAST2SLOW_RATIO / 100);
> +
> + if (vthfst > data->irng) {
> + dev_err(&rdev->dev, "Current limit out of range\n");
> + return -EINVAL;
> + }
> + /* Fast trip threshold to be programmed */
> + val = div_u64(mul_u32_u32(0xFF, vthfst), data->irng);
> + } else
> + /*
> + * Since there is no option to disable ocp, set limit to max
> + * value
> + */
> + val = 0xFF;
> +
> + reg = MAX5970_REG_DAC_FAST(rdev_id);
> + ret = regmap_write(rdev->regmap, reg, val);
> +
> + return ret;
> +}
> +

> +static int max597x_irq_handler(int irq, struct regulator_irq_data *rid,
> + unsigned long *dev_mask)
> +{
> + struct regulator_err_state *stat;
> + struct max597x_regulator *d = (struct max597x_regulator *)rid->data;
> + int val, ret, i;
> + > + ret = max597x_regmap_read_clear(d->regmap, MAX5970_REG_FAULT0, &val);
> + if (ret)
> + return REGULATOR_FAILED_RETRY;

This "read_clear" smells like a race-by-design to me...

> +
> + *dev_mask = 0;
> + for (i = 0; i < d->num_switches; i++) {
> + stat = &rid->states[i];
> + stat->notifs = 0;
> + stat->errors = 0;
> + }
> +
> + for (i = 0; i < d->num_switches; i++) {
> + stat = &rid->states[i];
> +
> + if (val & UV_STATUS_CRIT(i)) {
> + *dev_mask |= 1 << i;
> + stat->notifs |= REGULATOR_EVENT_UNDER_VOLTAGE;
> + stat->errors |= REGULATOR_ERROR_UNDER_VOLTAGE;
> + } else if (val & UV_STATUS_WARN(i)) {
> + *dev_mask |= 1 << i;
> + stat->notifs |= REGULATOR_EVENT_UNDER_VOLTAGE_WARN;
> + stat->errors |= REGULATOR_ERROR_UNDER_VOLTAGE_WARN;
> + }
> + }
> +
> + ret = max597x_regmap_read_clear(d->regmap, MAX5970_REG_FAULT1, &val);
> + if (ret)
> + return REGULATOR_FAILED_RETRY;

... and same here...

> +
> + for (i = 0; i < d->num_switches; i++) {
> + stat = &rid->states[i];
> +
> + if (val & OV_STATUS_CRIT(i)) {
> + *dev_mask |= 1 << i;
> + stat->notifs |= REGULATOR_EVENT_REGULATION_OUT;
> + stat->errors |= REGULATOR_ERROR_REGULATION_OUT;
> + } else if (val & OV_STATUS_WARN(i)) {
> + *dev_mask |= 1 << i;
> + stat->notifs |= REGULATOR_EVENT_OVER_VOLTAGE_WARN;
> + stat->errors |= REGULATOR_ERROR_OVER_VOLTAGE_WARN;
> + }
> + }
> +
> + ret = max597x_regmap_read_clear(d->regmap, MAX5970_REG_FAULT2, &val);
> + if (ret)
> + return REGULATOR_FAILED_RETRY;
> +

... and here. I wonder if the reason for "clearing" would be worth
commenting?

> + for (i = 0; i < d->num_switches; i++) {
> + stat = &rid->states[i];
> +
> + if (val & OC_STATUS_WARN(i)) {
> + *dev_mask |= 1 << i;
> + stat->notifs |= REGULATOR_EVENT_OVER_CURRENT_WARN;
> + stat->errors |= REGULATOR_ERROR_OVER_CURRENT_WARN;
> + }
> + }
> +
> + ret = regmap_read(d->regmap, MAX5970_REG_STATUS0, &val);
> + if (ret)
> + return REGULATOR_FAILED_RETRY;
> +
> + for (i = 0; i < d->num_switches; i++) {
> + stat = &rid->states[i];
> +
> + if ((val & MAX5970_CB_IFAULTF(i))
> + || (val & MAX5970_CB_IFAULTS(i))) {
> + *dev_mask |= 1 << i;
> + stat->notifs |=
> + REGULATOR_EVENT_OVER_CURRENT |
> + REGULATOR_EVENT_DISABLE;
> + stat->errors |=
> + REGULATOR_ERROR_OVER_CURRENT | REGULATOR_ERROR_FAIL;
> +
> + /* Clear the sub-IRQ status */
> + regulator_disable_regmap(stat->rdev);
> + }
> + }
> + return 0;
> +}
> +

--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~

Discuss - Estimate - Plan - Report and finally accomplish this:
void do_work(int time) __attribute__ ((const));

2022-07-17 12:53:16

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH 5/5] leds: max597x: Add support for max597x

Hi!

> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index a49979f41eee..682748097276 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -598,6 +598,16 @@ config LEDS_ADP5520
> To compile this driver as a module, choose M here: the module will
> be called leds-adp5520.
>
> +config LEDS_MAX597X
> + tristate "Maxim 597x leds"
> + depends on I2C
> + depends on OF
> + depends on LEDS_CLASS
> + select MFD_MAX597X
> + help
> + This driver controls a Maxim 5970/5978 indication led via I2C bus.
> + The MAX5970/5978 is a smart switch with 4 indication leds

led->LED.

Add "to compile this driver as a module, choose M here: the module will
be called ..".

Add "." at end of sentence.

> +++ b/drivers/leds/leds-max597x.c
> @@ -0,0 +1,130 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Device driver for regulators in MAX5970 and MAX5978 IC

?

> +static int max597x_led_set_brightness(struct led_classdev *cdev,
> + enum led_brightness brightness)
> +{
> + struct max597x_led *led = cdev->driver_data;
> + int ret;
> +
> + if (!led || !led->regmap)
> + return -1;

-ERRNO.

> +static int max597x_led_probe(struct platform_device *pdev)
> +{
> +
> +
> + struct max597x_data *max597x = dev_get_drvdata(pdev->dev.parent);
> + struct device_node *np = dev_of_node(pdev->dev.parent);

Delete empty lines.

> + struct device_node *led_node;
> + struct device_node *child;
> + int ret = 0;
> +
> +

One empty line would be enough.

> +module_platform_driver(max597x_led_driver);
> +
> +
> +MODULE_AUTHOR("Patrick Rudolph <[email protected]>");

One empty line would be enough.

Best regards,
Pavel
--
People of Russia, stop Putin before his war on Ukraine escalates.


Attachments:
(No filename) (1.77 kB)
signature.asc (201.00 B)
Download all attachments

2022-07-19 16:40:42

by Naresh Solanki

[permalink] [raw]
Subject: Re: [PATCH 3/5] regulator: max597x: Add support for max597x regulator

> I like the way the IRQ helpers have been used here. It'd be cool to hear
> how the rest of the system you're dealing with utilize the WARN level
> events :)
>
Userspace applications would receive sysfs notify for these events in realtime
(there is another patch that enables sysfs notify on regulator events)
This will enable taking necessary action if the regulator is not in good state.

> > +static int max597x_set_ocp(struct regulator_dev *rdev, int lim_uA,
> > + int severity, bool enable)
> > +{
> > + int ret, val, reg;
> > + unsigned int vthst, vthfst;
> > +
> > + struct max597x_regulator *data = rdev_get_drvdata(rdev);
> > + int rdev_id = rdev_get_id(rdev);
> > + /*
> > + * MAX5970 doesn't has enable control for ocp.
> > + * If limit is specified but enable is not set then hold the value in
> > + * variable & later use it when ocp needs to be enabled.
> > + */
>
> Is this a possible scenario? I think that if a non zero limit is given
> in a "regulator-oc-protection-microamp"-property, then the protection
> should always be enabled. Am I overlooking something?
>
Yes in the current scenario, if non-zero ocp is provided then
protection should always be enabled.
But since there is an enable switch in function call, I felt like it
should be addressed in this way.

> > +static int max597x_irq_handler(int irq, struct regulator_irq_data *rid,
> > + unsigned long *dev_mask)
> > +{
> > + struct regulator_err_state *stat;
> > + struct max597x_regulator *d = (struct max597x_regulator *)rid->data;
> > + int val, ret, i;
> > + > + ret = max597x_regmap_read_clear(d->regmap, MAX5970_REG_FAULT0, &val);
> > + if (ret)
> > + return REGULATOR_FAILED_RETRY;
>
> This "read_clear" smells like a race-by-design to me...
>
I'm not sure what best way would be to address this but if Under/Over
Voltage & Over current scenario
occurs then the interrupt line remains low until things are normal.
The fault register bit can be clear only if output is normal or power cycled.



> > +
> > + *dev_mask = 0;
> > + for (i = 0; i < d->num_switches; i++) {
> > + stat = &rid->states[i];
> > + stat->notifs = 0;
> > + stat->errors = 0;
> > + }
> > +
> > + for (i = 0; i < d->num_switches; i++) {
> > + stat = &rid->states[i];
> > +
> > + if (val & UV_STATUS_CRIT(i)) {
> > + *dev_mask |= 1 << i;
> > + stat->notifs |= REGULATOR_EVENT_UNDER_VOLTAGE;
> > + stat->errors |= REGULATOR_ERROR_UNDER_VOLTAGE;
> > + } else if (val & UV_STATUS_WARN(i)) {
> > + *dev_mask |= 1 << i;
> > + stat->notifs |= REGULATOR_EVENT_UNDER_VOLTAGE_WARN;
> > + stat->errors |= REGULATOR_ERROR_UNDER_VOLTAGE_WARN;
> > + }
> > + }
> > +
> > + ret = max597x_regmap_read_clear(d->regmap, MAX5970_REG_FAULT1, &val);
> > + if (ret)
> > + return REGULATOR_FAILED_RETRY;
>
> ... and same here...
>
> > +
> > + for (i = 0; i < d->num_switches; i++) {
> > + stat = &rid->states[i];
> > +
> > + if (val & OV_STATUS_CRIT(i)) {
> > + *dev_mask |= 1 << i;
> > + stat->notifs |= REGULATOR_EVENT_REGULATION_OUT;
> > + stat->errors |= REGULATOR_ERROR_REGULATION_OUT;
> > + } else if (val & OV_STATUS_WARN(i)) {
> > + *dev_mask |= 1 << i;
> > + stat->notifs |= REGULATOR_EVENT_OVER_VOLTAGE_WARN;
> > + stat->errors |= REGULATOR_ERROR_OVER_VOLTAGE_WARN;
> > + }
> > + }
> > +
> > + ret = max597x_regmap_read_clear(d->regmap, MAX5970_REG_FAULT2, &val);
> > + if (ret)
> > + return REGULATOR_FAILED_RETRY;
> > +
>
> ... and here. I wonder if the reason for "clearing" would be worth
> commenting?
>
> > + for (i = 0; i < d->num_switches; i++) {
> > + stat = &rid->states[i];
> > +
> > + if (val & OC_STATUS_WARN(i)) {
> > + *dev_mask |= 1 << i;
> > + stat->notifs |= REGULATOR_EVENT_OVER_CURRENT_WARN;
> > + stat->errors |= REGULATOR_ERROR_OVER_CURRENT_WARN;
> > + }
> > + }
> > +
> > + ret = regmap_read(d->regmap, MAX5970_REG_STATUS0, &val);
> > + if (ret)
> > + return REGULATOR_FAILED_RETRY;
> > +
> > + for (i = 0; i < d->num_switches; i++) {
> > + stat = &rid->states[i];
> > +
> > + if ((val & MAX5970_CB_IFAULTF(i))
> > + || (val & MAX5970_CB_IFAULTS(i))) {
> > + *dev_mask |= 1 << i;
> > + stat->notifs |=
> > + REGULATOR_EVENT_OVER_CURRENT |
> > + REGULATOR_EVENT_DISABLE;
> > + stat->errors |=
> > + REGULATOR_ERROR_OVER_CURRENT | REGULATOR_ERROR_FAIL;
> > +
> > + /* Clear the sub-IRQ status */
> > + regulator_disable_regmap(stat->rdev);
> > + }
> > + }
> > + return 0;
> > +}
> > +
>
> --
> Matti Vaittinen
> Linux kernel developer at ROHM Semiconductors
> Oulu Finland
>
> ~~ When things go utterly wrong vim users can always type :help! ~~
>
> Discuss - Estimate - Plan - Report and finally accomplish this:
> void do_work(int time) __attribute__ ((const));
>

2022-07-19 17:08:52

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 3/5] regulator: max597x: Add support for max597x regulator

On Tue, Jul 19, 2022 at 09:29:15PM +0530, Naresh Solanki wrote:
> > I like the way the IRQ helpers have been used here. It'd be cool to hear
> > how the rest of the system you're dealing with utilize the WARN level
> > events :)

> Userspace applications would receive sysfs notify for these events in realtime
> (there is another patch that enables sysfs notify on regulator events)
> This will enable taking necessary action if the regulator is not in good state.

Right, the question is more what the action that might be taken is.


Attachments:
(No filename) (545.00 B)
signature.asc (499.00 B)
Download all attachments