[PATCH v2 0/3] mfd: aat2870: Convert to use OF bindings

[PATCH v2 0/3] mfd: aat2870: Convert to use OF bindings

[Svyatoslav Ryhel]

Document Skyworks AAT2870 into schema, convert existing driver to
use OF bindings and remove platform data, add support for
in-supplies.

---
Changes in v2:
- fixed led-max-microamp scaling
- fixed max_brightness parsing
- fixed regulator init_data and of_node passing
- of_platform_populate > devm_of_platform_populate
---

Svyatoslav Ryhel (3):
  dt-bindings: mfd: Document Skyworks AAT2870
  mfd: aat2870: Convert to use OF bindings
  mfd: aat2870: Add support for VIN and IN-LDO power supplies

 .../bindings/leds/skyworks,aat2870.yaml       | 127 ++++++++++++++++++
 drivers/mfd/aat2870-core.c                    | 120 +++++------------
 drivers/regulator/aat2870-regulator.c         |  77 ++++++++---
 drivers/video/backlight/aat2870_bl.c          |  55 ++++----
 include/linux/mfd/aat2870.h                   |  95 +------------
 5 files changed, 244 insertions(+), 230 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/leds/skyworks,aat2870.yaml

-- 
2.53.0

[PATCH v2 1/3] dt-bindings: mfd: Document Skyworks AAT2870

[Svyatoslav Ryhel]

Document Skyworks AAT2870 LED Backlight Driver and Multiple LDO Lighting
Management Unit.

Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 .../bindings/leds/skyworks,aat2870.yaml       | 127 ++++++++++++++++++
 1 file changed, 127 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/skyworks,aat2870.yaml

diff --git a/Documentation/devicetree/bindings/leds/skyworks,aat2870.yaml b/Documentation/devicetree/bindings/leds/skyworks,aat2870.yaml
new file mode 100644
index 0000000000000..014678616076f
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/skyworks,aat2870.yaml
@@ -0,0 +1,127 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/skyworks,aat2870.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: AAT2870 LED Backlight Driver and Multiple LDO Lighting Management Unit
+
+maintainers:
+  - Svyatoslav Ryhel <clamor95@gmail.com>
+
+properties:
+  compatible:
+    const: skyworks,aat2870
+
+  reg:
+    maxItems: 1
+
+  enable-gpios:
+    description: GPIO connected to EN pin.
+    maxItems: 1
+
+  vin-supply:
+    description: Regulator providing power to the "IN" pin.
+
+  backlight:
+    $ref: /schemas/leds/backlight/common.yaml#
+    unevaluatedProperties: false
+
+    properties:
+      compatible:
+        const: skyworks,aat2870-backlight
+
+      led-max-microamp:
+        minimum: 450
+        maximum: 27900
+        default: 450
+
+      max-brightness:
+        default: 255
+
+      skyworks,channels:
+        description: layout of active channels, where each bit represents
+          one of 8 channels.
+        $ref: /schemas/types.yaml#/definitions/uint8
+        default: 0xff
+
+    required:
+      - compatible
+
+  regulators:
+    type: object
+    additionalProperties: false
+
+    properties:
+      compatible:
+        const: skyworks,aat2870-regulator
+
+      in-ldo-supply:
+        description: Phandle to parent supply of regulators populated under
+          regulators node. They all share same parent.
+
+    patternProperties:
+      "^(ldo-(a|b|c|d))$":
+        $ref: /schemas/regulator/regulator.yaml#
+        unevaluatedProperties: false
+
+    required:
+      - compatible
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        led-controller@60 {
+            compatible = "skyworks,aat2870";
+            reg = <0x60>;
+
+            enable-gpios = <&gpio 139 GPIO_ACTIVE_HIGH>;
+            vin-supply = <&vdd_3v3_vbat>;
+
+            backlight {
+                compatible = "skyworks,aat2870-backlight";
+
+                skyworks,channels = /bits/ 8 <0xff>;
+                led-max-microamp = <27900>;
+            };
+
+            regulators {
+                compatible = "skyworks,aat2870-regulator";
+
+                in-ldo-supply = <&vdd_3v3_vbat>;
+
+                ldo-a {
+                    regulator-min-microvolt = <1200000>;
+                    regulator-max-microvolt = <1200000>;
+                };
+
+                ldo-b {
+                    regulator-min-microvolt = <3300000>;
+                    regulator-max-microvolt = <3300000>;
+                    regulator-always-on;
+                };
+
+                ldo-c {
+                    regulator-min-microvolt = <1800000>;
+                    regulator-max-microvolt = <3300000>;
+                    regulator-boot-on;
+                };
+
+                ldo-d {
+                    regulator-min-microvolt = <2700000>;
+                    regulator-max-microvolt = <2800000>;
+                };
+            };
+        };
+    };
-- 
2.53.0

[PATCH v2 2/3] mfd: aat2870: Convert to use OF bindings

[Svyatoslav Ryhel]

Conversion of the AAT2870 driver to use OF bindings requires a few complex
changes that should be done simultaneously.

The AAT2870 essentially provides two functions via child devices:
backlight and regulators. Both functions are fairly self-sufficient and
may not be populated on the final board. Consequently, the MFD
registration API was replaced with of_platform_populate(), and each
sub-device was given its own compatible string.

Additionally, aat2870-core utilizes an enable GPIO. Obtaining this GPIO
was converted to use modern gpiod/OF helpers, allowing the redundant
aat2870_enable() and aat2870_disable() helpers to be removed.

The aat2870-regulator driver was updated to register each of the four LDOs
from a dedicated OF node.

The aat2870-backlight driver was changed to populate all required
properties from a dedicated Device Tree node. Its channel map was updated
to use u8 instead of int. Furthermore, because the maximum current is now
parsed as an absolute value rather than an enum entry, the calculation and
application of the maximum current were updated accordingly.

All of the changes above allow platform data to be removed entirely.

Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 drivers/mfd/aat2870-core.c            | 115 ++++++--------------------
 drivers/regulator/aat2870-regulator.c |  76 ++++++++++++-----
 drivers/video/backlight/aat2870_bl.c  |  55 ++++++------
 include/linux/mfd/aat2870.h           |  95 +--------------------
 4 files changed, 111 insertions(+), 230 deletions(-)

diff --git a/drivers/mfd/aat2870-core.c b/drivers/mfd/aat2870-core.c
index 0d56cd6fbc6a5..909c14ee60323 100644
--- a/drivers/mfd/aat2870-core.c
+++ b/drivers/mfd/aat2870-core.c
@@ -16,8 +16,15 @@
 #include <linux/gpio/legacy.h>
 #include <linux/mfd/core.h>
 #include <linux/mfd/aat2870.h>
+#include <linux/of_platform.h>
 #include <linux/regulator/machine.h>
 
+struct aat2870_register {
+	bool readable;
+	bool writeable;
+	u8 value;
+};
+
 static struct aat2870_register aat2870_regs[AAT2870_REG_NUM] = {
 	/* readable, writeable, value */
 	{ 0, 1, 0x00 },	/* 0x00 AAT2870_BL_CH_EN */
@@ -61,34 +68,6 @@ static struct aat2870_register aat2870_regs[AAT2870_REG_NUM] = {
 	{ 0, 1, 0x00 },	/* 0x26 AAT2870_LDO_EN */
 };
 
-static struct mfd_cell aat2870_devs[] = {
-	{
-		.name = "aat2870-backlight",
-		.id = AAT2870_ID_BL,
-		.pdata_size = sizeof(struct aat2870_bl_platform_data),
-	},
-	{
-		.name = "aat2870-regulator",
-		.id = AAT2870_ID_LDOA,
-		.pdata_size = sizeof(struct regulator_init_data),
-	},
-	{
-		.name = "aat2870-regulator",
-		.id = AAT2870_ID_LDOB,
-		.pdata_size = sizeof(struct regulator_init_data),
-	},
-	{
-		.name = "aat2870-regulator",
-		.id = AAT2870_ID_LDOC,
-		.pdata_size = sizeof(struct regulator_init_data),
-	},
-	{
-		.name = "aat2870-regulator",
-		.id = AAT2870_ID_LDOD,
-		.pdata_size = sizeof(struct regulator_init_data),
-	},
-};
-
 static int __aat2870_read(struct aat2870_data *aat2870, u8 addr, u8 *val)
 {
 	int ret;
@@ -196,22 +175,6 @@ static int aat2870_update(struct aat2870_data *aat2870, u8 addr, u8 mask,
 	return ret;
 }
 
-static inline void aat2870_enable(struct aat2870_data *aat2870)
-{
-	if (aat2870->en_pin >= 0)
-		gpio_set_value(aat2870->en_pin, 1);
-
-	aat2870->is_enable = 1;
-}
-
-static inline void aat2870_disable(struct aat2870_data *aat2870)
-{
-	if (aat2870->en_pin >= 0)
-		gpio_set_value(aat2870->en_pin, 0);
-
-	aat2870->is_enable = 0;
-}
-
 #ifdef CONFIG_DEBUG_FS
 static ssize_t aat2870_dump_reg(struct aat2870_data *aat2870, char *buf)
 {
@@ -332,9 +295,7 @@ static inline void aat2870_init_debugfs(struct aat2870_data *aat2870)
 
 static int aat2870_i2c_probe(struct i2c_client *client)
 {
-	struct aat2870_platform_data *pdata = dev_get_platdata(&client->dev);
 	struct aat2870_data *aat2870;
-	int i, j;
 	int ret = 0;
 
 	aat2870 = devm_kzalloc(&client->dev, sizeof(struct aat2870_data),
@@ -348,60 +309,29 @@ static int aat2870_i2c_probe(struct i2c_client *client)
 
 	aat2870->reg_cache = aat2870_regs;
 
-	if (pdata->en_pin < 0)
-		aat2870->en_pin = -1;
-	else
-		aat2870->en_pin = pdata->en_pin;
+	aat2870->en_pin = devm_gpiod_get_optional(&client->dev, "enable",
+						  GPIOD_OUT_HIGH);
+	if (IS_ERR(aat2870->en_pin))
+		return dev_err_probe(&client->dev, PTR_ERR(aat2870->en_pin),
+				     "Failed to get EN GPIO\n");
 
-	aat2870->init = pdata->init;
-	aat2870->uninit = pdata->uninit;
 	aat2870->read = aat2870_read;
 	aat2870->write = aat2870_write;
 	aat2870->update = aat2870_update;
 
 	mutex_init(&aat2870->io_lock);
 
-	if (aat2870->init)
-		aat2870->init(aat2870);
-
-	if (aat2870->en_pin >= 0) {
-		ret = devm_gpio_request_one(&client->dev, aat2870->en_pin,
-					GPIOF_OUT_INIT_HIGH, "aat2870-en");
-		if (ret < 0) {
-			dev_err(&client->dev,
-				"Failed to request GPIO %d\n", aat2870->en_pin);
-			return ret;
-		}
-	}
-
-	aat2870_enable(aat2870);
-
-	for (i = 0; i < pdata->num_subdevs; i++) {
-		for (j = 0; j < ARRAY_SIZE(aat2870_devs); j++) {
-			if ((pdata->subdevs[i].id == aat2870_devs[j].id) &&
-					!strcmp(pdata->subdevs[i].name,
-						aat2870_devs[j].name)) {
-				aat2870_devs[j].platform_data =
-					pdata->subdevs[i].platform_data;
-				break;
-			}
-		}
-	}
+	gpiod_set_value(aat2870->en_pin, 1);
 
-	ret = mfd_add_devices(aat2870->dev, 0, aat2870_devs,
-			      ARRAY_SIZE(aat2870_devs), NULL, 0, NULL);
-	if (ret != 0) {
-		dev_err(aat2870->dev, "Failed to add subdev: %d\n", ret);
-		goto out_disable;
+	ret = devm_of_platform_populate(&client->dev);
+	if (ret) {
+		gpiod_set_value(aat2870->en_pin, 0);
+		return dev_err_probe(&client->dev, ret, "Failed to populate cells\n");
 	}
 
 	aat2870_init_debugfs(aat2870);
 
 	return 0;
-
-out_disable:
-	aat2870_disable(aat2870);
-	return ret;
 }
 
 static int aat2870_i2c_suspend(struct device *dev)
@@ -409,7 +339,7 @@ static int aat2870_i2c_suspend(struct device *dev)
 	struct i2c_client *client = to_i2c_client(dev);
 	struct aat2870_data *aat2870 = i2c_get_clientdata(client);
 
-	aat2870_disable(aat2870);
+	gpiod_set_value(aat2870->en_pin, 0);
 
 	return 0;
 }
@@ -421,7 +351,7 @@ static int aat2870_i2c_resume(struct device *dev)
 	struct aat2870_register *reg = NULL;
 	int i;
 
-	aat2870_enable(aat2870);
+	gpiod_set_value(aat2870->en_pin, 1);
 
 	/* restore registers */
 	for (i = 0; i < AAT2870_REG_NUM; i++) {
@@ -436,6 +366,12 @@ static int aat2870_i2c_resume(struct device *dev)
 static DEFINE_SIMPLE_DEV_PM_OPS(aat2870_pm_ops, aat2870_i2c_suspend,
 				aat2870_i2c_resume);
 
+static const struct of_device_id aat2870_match_table[] = {
+	{ .compatible = "skyworks,aat2870" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, aat2870_match_table);
+
 static const struct i2c_device_id aat2870_i2c_id_table[] = {
 	{ "aat2870" },
 	{ }
@@ -444,6 +380,7 @@ static const struct i2c_device_id aat2870_i2c_id_table[] = {
 static struct i2c_driver aat2870_i2c_driver = {
 	.driver = {
 		.name			= "aat2870",
+		.of_match_table		= aat2870_match_table,
 		.pm			= pm_sleep_ptr(&aat2870_pm_ops),
 		.suppress_bind_attrs	= true,
 	},
diff --git a/drivers/regulator/aat2870-regulator.c b/drivers/regulator/aat2870-regulator.c
index 970d86f2bbb81..8a58eb27f3f5f 100644
--- a/drivers/regulator/aat2870-regulator.c
+++ b/drivers/regulator/aat2870-regulator.c
@@ -15,6 +15,15 @@
 #include <linux/regulator/driver.h>
 #include <linux/regulator/machine.h>
 #include <linux/mfd/aat2870.h>
+#include <linux/regulator/of_regulator.h>
+
+/* Device IDs */
+enum aat2870_regulator_id {
+	AAT2870_ID_LDOA,
+	AAT2870_ID_LDOB,
+	AAT2870_ID_LDOC,
+	AAT2870_ID_LDOD
+};
 
 struct aat2870_regulator {
 	struct aat2870_data *aat2870;
@@ -121,6 +130,13 @@ static struct aat2870_regulator aat2870_regulators[] = {
 	AAT2870_LDO(LDOD),
 };
 
+static struct of_regulator_match aat2870_regulator_matches[] = {
+	{ .name = "ldo-a" },
+	{ .name = "ldo-b" },
+	{ .name = "ldo-c" },
+	{ .name = "ldo-d" },
+};
+
 static struct aat2870_regulator *aat2870_get_regulator(int id)
 {
 	struct aat2870_regulator *ri = NULL;
@@ -136,12 +152,11 @@ static struct aat2870_regulator *aat2870_get_regulator(int id)
 		return NULL;
 
 	ri->enable_addr = AAT2870_LDO_EN;
-	ri->enable_shift = id - AAT2870_ID_LDOA;
+	ri->enable_shift = id;
 	ri->enable_mask = 0x1 << ri->enable_shift;
 
-	ri->voltage_addr = (id - AAT2870_ID_LDOA) / 2 ?
-			   AAT2870_LDO_CD : AAT2870_LDO_AB;
-	ri->voltage_shift = (id - AAT2870_ID_LDOA) % 2 ? 0 : 4;
+	ri->voltage_addr = id / 2 ? AAT2870_LDO_CD : AAT2870_LDO_AB;
+	ri->voltage_shift = id % 2 ? 0 : 4;
 	ri->voltage_mask = 0xF << ri->voltage_shift;
 
 	return ri;
@@ -152,33 +167,52 @@ static int aat2870_regulator_probe(struct platform_device *pdev)
 	struct aat2870_regulator *ri;
 	struct regulator_config config = { };
 	struct regulator_dev *rdev;
+	int ret;
 
-	ri = aat2870_get_regulator(pdev->id);
-	if (!ri) {
-		dev_err(&pdev->dev, "Invalid device ID, %d\n", pdev->id);
-		return -EINVAL;
-	}
-	ri->aat2870 = dev_get_drvdata(pdev->dev.parent);
-
-	config.dev = &pdev->dev;
-	config.driver_data = ri;
-	config.init_data = dev_get_platdata(&pdev->dev);
-
-	rdev = devm_regulator_register(&pdev->dev, &ri->desc, &config);
-	if (IS_ERR(rdev)) {
-		dev_err(&pdev->dev, "Failed to register regulator %s\n",
-			ri->desc.name);
-		return PTR_ERR(rdev);
+	ret = of_regulator_match(&pdev->dev, pdev->dev.of_node,
+				 aat2870_regulator_matches,
+				 ARRAY_SIZE(aat2870_regulator_matches));
+	if (ret < 0)
+		return dev_err_probe(&pdev->dev, ret,
+				     "Parsing of regulator node failed\n");
+
+	for (int idx = 0; idx < ARRAY_SIZE(aat2870_regulator_matches); idx++) {
+		if (!aat2870_regulator_matches[idx].of_node)
+			continue;
+
+		ri = aat2870_get_regulator(idx);
+		if (!ri)
+			return dev_err_probe(&pdev->dev, -EINVAL,
+					     "Invalid device ID, %d\n", idx);
+
+		ri->aat2870 = dev_get_drvdata(pdev->dev.parent);
+
+		config.dev = &pdev->dev;
+		config.init_data = aat2870_regulator_matches[idx].init_data;
+		config.driver_data = ri;
+		config.of_node = aat2870_regulator_matches[idx].of_node;
+
+		rdev = devm_regulator_register(&pdev->dev, &ri->desc, &config);
+		if (IS_ERR(rdev))
+			return dev_err_probe(&pdev->dev, PTR_ERR(rdev),
+					     "Failed to register regulator %s\n",
+					     ri->desc.name);
 	}
-	platform_set_drvdata(pdev, rdev);
 
 	return 0;
 }
 
+static const struct of_device_id aat2870_regulator_match_table[] = {
+	{ .compatible = "skyworks,aat2870-regulator" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, aat2870_regulator_match_table);
+
 static struct platform_driver aat2870_regulator_driver = {
 	.driver = {
 		.name	= "aat2870-regulator",
 		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
+		.of_match_table = aat2870_regulator_match_table,
 	},
 	.probe	= aat2870_regulator_probe,
 };
diff --git a/drivers/video/backlight/aat2870_bl.c b/drivers/video/backlight/aat2870_bl.c
index 8b790df1e842b..933a8f728f66a 100644
--- a/drivers/video/backlight/aat2870_bl.c
+++ b/drivers/video/backlight/aat2870_bl.c
@@ -15,11 +15,19 @@
 #include <linux/backlight.h>
 #include <linux/mfd/aat2870.h>
 
+/* Backlight has 8 channels, each bit represents one channel */
+#define AAT2870_BL_CH_ALL	0xff
+
+/* Backlight current magnitude (uA), 450uA current is eq to 0 */
+#define AAT2870_CURRENT_MIN	450
+#define AAT2870_CURRENT_MAX	27900
+#define AAT2870_CURRENT_STEP	900
+
 struct aat2870_bl_driver_data {
 	struct platform_device *pdev;
 	struct backlight_device *bd;
 
-	int channels;
+	u8 channels;
 	int max_current;
 	int brightness; /* current brightness */
 };
@@ -30,7 +38,7 @@ static inline int aat2870_brightness(struct aat2870_bl_driver_data *aat2870_bl,
 	struct backlight_device *bd = aat2870_bl->bd;
 	int val;
 
-	val = brightness * (aat2870_bl->max_current - 1);
+	val = brightness * aat2870_bl->max_current;
 	val /= bd->props.max_brightness;
 
 	return val;
@@ -42,7 +50,7 @@ static inline int aat2870_bl_enable(struct aat2870_bl_driver_data *aat2870_bl)
 			= dev_get_drvdata(aat2870_bl->pdev->dev.parent);
 
 	return aat2870->write(aat2870, AAT2870_BL_CH_EN,
-			      (u8)aat2870_bl->channels);
+			      aat2870_bl->channels);
 }
 
 static inline int aat2870_bl_disable(struct aat2870_bl_driver_data *aat2870_bl)
@@ -96,24 +104,12 @@ static const struct backlight_ops aat2870_bl_ops = {
 
 static int aat2870_bl_probe(struct platform_device *pdev)
 {
-	struct aat2870_bl_platform_data *pdata = dev_get_platdata(&pdev->dev);
 	struct aat2870_bl_driver_data *aat2870_bl;
 	struct backlight_device *bd;
 	struct backlight_properties props;
+	u32 max_brightness = 0;
 	int ret = 0;
 
-	if (!pdata) {
-		dev_err(&pdev->dev, "No platform data\n");
-		ret = -ENXIO;
-		goto out;
-	}
-
-	if (pdev->id != AAT2870_ID_BL) {
-		dev_err(&pdev->dev, "Invalid device ID, %d\n", pdev->id);
-		ret = -EINVAL;
-		goto out;
-	}
-
 	aat2870_bl = devm_kzalloc(&pdev->dev,
 				  sizeof(struct aat2870_bl_driver_data),
 				  GFP_KERNEL);
@@ -140,18 +136,18 @@ static int aat2870_bl_probe(struct platform_device *pdev)
 
 	aat2870_bl->bd = bd;
 
-	if (pdata->channels > 0)
-		aat2870_bl->channels = pdata->channels;
-	else
-		aat2870_bl->channels = AAT2870_BL_CH_ALL;
+	aat2870_bl->channels = AAT2870_BL_CH_ALL;
+	device_property_read_u8(&pdev->dev, "skyworks,channels", &aat2870_bl->channels);
 
-	if (pdata->max_current > 0)
-		aat2870_bl->max_current = pdata->max_current;
-	else
-		aat2870_bl->max_current = AAT2870_CURRENT_27_9;
+	device_property_read_u32(&pdev->dev, "led-max-microamp", &aat2870_bl->max_current);
+	aat2870_bl->max_current = clamp(aat2870_bl->max_current, AAT2870_CURRENT_MIN,
+					AAT2870_CURRENT_MAX);
+	aat2870_bl->max_current /= AAT2870_CURRENT_STEP;
 
-	if (pdata->max_brightness > 0)
-		bd->props.max_brightness = pdata->max_brightness;
+	/* If max-brightness property is missing or set to zero, use chip's max value */
+	device_property_read_u32(&pdev->dev, "max-brightness", &max_brightness);
+	if (max_brightness)
+		bd->props.max_brightness = max_brightness;
 	else
 		bd->props.max_brightness = 255;
 
@@ -181,9 +177,16 @@ static void aat2870_bl_remove(struct platform_device *pdev)
 	backlight_update_status(bd);
 }
 
+static const struct of_device_id aat2870_bl_match_table[] = {
+	{ .compatible = "skyworks,aat2870-backlight" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, aat2870_bl_match_table);
+
 static struct platform_driver aat2870_bl_driver = {
 	.driver = {
 		.name	= "aat2870-backlight",
+		.of_match_table = aat2870_bl_match_table,
 	},
 	.probe		= aat2870_bl_probe,
 	.remove		= aat2870_bl_remove,
diff --git a/include/linux/mfd/aat2870.h b/include/linux/mfd/aat2870.h
index c7a3c53eba681..a7b482b790b4e 100644
--- a/include/linux/mfd/aat2870.h
+++ b/include/linux/mfd/aat2870.h
@@ -54,80 +54,13 @@
 #define AAT2870_LDO_EN		0x26
 #define AAT2870_REG_NUM		0x27
 
-/* Device IDs */
-enum aat2870_id {
-	AAT2870_ID_BL,
-	AAT2870_ID_LDOA,
-	AAT2870_ID_LDOB,
-	AAT2870_ID_LDOC,
-	AAT2870_ID_LDOD
-};
-
-/* Backlight channels */
-#define AAT2870_BL_CH1		0x01
-#define AAT2870_BL_CH2		0x02
-#define AAT2870_BL_CH3		0x04
-#define AAT2870_BL_CH4		0x08
-#define AAT2870_BL_CH5		0x10
-#define AAT2870_BL_CH6		0x20
-#define AAT2870_BL_CH7		0x40
-#define AAT2870_BL_CH8		0x80
-#define AAT2870_BL_CH_ALL	0xFF
-
-/* Backlight current magnitude (mA) */
-enum aat2870_current {
-	AAT2870_CURRENT_0_45 = 1,
-	AAT2870_CURRENT_0_90,
-	AAT2870_CURRENT_1_80,
-	AAT2870_CURRENT_2_70,
-	AAT2870_CURRENT_3_60,
-	AAT2870_CURRENT_4_50,
-	AAT2870_CURRENT_5_40,
-	AAT2870_CURRENT_6_30,
-	AAT2870_CURRENT_7_20,
-	AAT2870_CURRENT_8_10,
-	AAT2870_CURRENT_9_00,
-	AAT2870_CURRENT_9_90,
-	AAT2870_CURRENT_10_8,
-	AAT2870_CURRENT_11_7,
-	AAT2870_CURRENT_12_6,
-	AAT2870_CURRENT_13_5,
-	AAT2870_CURRENT_14_4,
-	AAT2870_CURRENT_15_3,
-	AAT2870_CURRENT_16_2,
-	AAT2870_CURRENT_17_1,
-	AAT2870_CURRENT_18_0,
-	AAT2870_CURRENT_18_9,
-	AAT2870_CURRENT_19_8,
-	AAT2870_CURRENT_20_7,
-	AAT2870_CURRENT_21_6,
-	AAT2870_CURRENT_22_5,
-	AAT2870_CURRENT_23_4,
-	AAT2870_CURRENT_24_3,
-	AAT2870_CURRENT_25_2,
-	AAT2870_CURRENT_26_1,
-	AAT2870_CURRENT_27_0,
-	AAT2870_CURRENT_27_9
-};
-
-struct aat2870_register {
-	bool readable;
-	bool writeable;
-	u8 value;
-};
-
 struct aat2870_data {
 	struct device *dev;
 	struct i2c_client *client;
 
 	struct mutex io_lock;
 	struct aat2870_register *reg_cache; /* register cache */
-	int en_pin; /* enable GPIO pin (if < 0, ignore this value) */
-	bool is_enable;
-
-	/* init and uninit for platform specified */
-	int (*init)(struct aat2870_data *aat2870);
-	void (*uninit)(struct aat2870_data *aat2870);
+	struct gpio_desc *en_pin;
 
 	/* i2c io funcntions */
 	int (*read)(struct aat2870_data *aat2870, u8 addr, u8 *val);
@@ -135,30 +68,4 @@ struct aat2870_data {
 	int (*update)(struct aat2870_data *aat2870, u8 addr, u8 mask, u8 val);
 };
 
-struct aat2870_subdev_info {
-	int id;
-	const char *name;
-	void *platform_data;
-};
-
-struct aat2870_platform_data {
-	int en_pin; /* enable GPIO pin (if < 0, ignore this value) */
-
-	struct aat2870_subdev_info *subdevs;
-	int num_subdevs;
-
-	/* init and uninit for platform specified */
-	int (*init)(struct aat2870_data *aat2870);
-	void (*uninit)(struct aat2870_data *aat2870);
-};
-
-struct aat2870_bl_platform_data {
-	/* backlight channels, default is AAT2870_BL_CH_ALL */
-	int channels;
-	/* backlight current magnitude, default is AAT2870_CURRENT_27_9 */
-	int max_current;
-	/* maximum brightness, default is 255 */
-	int max_brightness;
-};
-
 #endif /* __LINUX_MFD_AAT2870_H */
-- 
2.53.0

[PATCH v2 3/3] mfd: aat2870: Add support for VIN and IN-LDO power supplies

[Svyatoslav Ryhel]

The AAT2870 supports a dedicated main power supply input alongside parent
power supply for its internal regulators.

Add support for retrieving and managing these supply regulators
(vin-supply and in-ldo-supply) using the regulator framework to ensure
proper power supply dependencies and power-up sequencing.

Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 drivers/mfd/aat2870-core.c            | 5 +++++
 drivers/regulator/aat2870-regulator.c | 1 +
 2 files changed, 6 insertions(+)

diff --git a/drivers/mfd/aat2870-core.c b/drivers/mfd/aat2870-core.c
index 909c14ee60323..0bd430b90bc1b 100644
--- a/drivers/mfd/aat2870-core.c
+++ b/drivers/mfd/aat2870-core.c
@@ -17,6 +17,7 @@
 #include <linux/mfd/core.h>
 #include <linux/mfd/aat2870.h>
 #include <linux/of_platform.h>
+#include <linux/regulator/consumer.h>
 #include <linux/regulator/machine.h>
 
 struct aat2870_register {
@@ -309,6 +310,10 @@ static int aat2870_i2c_probe(struct i2c_client *client)
 
 	aat2870->reg_cache = aat2870_regs;
 
+	ret = devm_regulator_get_enable(&client->dev, "vin");
+	if (ret)
+		return dev_err_probe(&client->dev, ret, "Failed to enable regulator\n");
+
 	aat2870->en_pin = devm_gpiod_get_optional(&client->dev, "enable",
 						  GPIOD_OUT_HIGH);
 	if (IS_ERR(aat2870->en_pin))
diff --git a/drivers/regulator/aat2870-regulator.c b/drivers/regulator/aat2870-regulator.c
index 8a58eb27f3f5f..a921e40ca47dd 100644
--- a/drivers/regulator/aat2870-regulator.c
+++ b/drivers/regulator/aat2870-regulator.c
@@ -114,6 +114,7 @@ static const unsigned int aat2870_ldo_voltages[] = {
 	{						\
 		.desc = {				\
 			.name = #ids,			\
+			.supply_name = "in-ldo",	\
 			.id = AAT2870_ID_##ids,		\
 			.n_voltages = ARRAY_SIZE(aat2870_ldo_voltages),	\
 			.volt_table = aat2870_ldo_voltages, \
-- 
2.53.0

Re: [PATCH v2 1/3] dt-bindings: mfd: Document Skyworks AAT2870

[Daniel Thompson]

On Sun, Oct 04, 2026 at 07:41:19PM +0300, Svyatoslav Ryhel wrote:
> Document Skyworks AAT2870 LED Backlight Driver and Multiple LDO Lighting
> Management Unit.
>
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
>  .../bindings/leds/skyworks,aat2870.yaml       | 127 ++++++++++++++++++
>  1 file changed, 127 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/skyworks,aat2870.yaml
>
> diff --git a/Documentation/devicetree/bindings/leds/skyworks,aat2870.yaml b/Documentation/devicetree/bindings/leds/skyworks,aat2870.yaml
> new file mode 100644
> index 0000000000000..014678616076f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/skyworks,aat2870.yaml
> @@ -0,0 +1,127 @@
> [snip]
> +  backlight:
> +    $ref: /schemas/leds/backlight/common.yaml#
> +    unevaluatedProperties: false
> +
> +    properties:
> +      compatible:
> +        const: skyworks,aat2870-backlight
> +
> +      led-max-microamp:
> +        minimum: 450
> +        maximum: 27900
> +        default: 450
> +
> +      max-brightness:
> +        default: 255

Should there be a min/max here as well?


Daniel.

Re: [PATCH v2 2/3] mfd: aat2870: Convert to use OF bindings

[Daniel Thompson]

On Sun, Oct 04, 2026 at 07:41:20PM +0300, Svyatoslav Ryhel wrote:
> Conversion of the AAT2870 driver to use OF bindings requires a few complex
> changes that should be done simultaneously.
>
> The AAT2870 essentially provides two functions via child devices:
> backlight and regulators. Both functions are fairly self-sufficient and
> may not be populated on the final board. Consequently, the MFD
> registration API was replaced with of_platform_populate(), and each
> sub-device was given its own compatible string.
>
> Additionally, aat2870-core utilizes an enable GPIO. Obtaining this GPIO
> was converted to use modern gpiod/OF helpers, allowing the redundant
> aat2870_enable() and aat2870_disable() helpers to be removed.
>
> The aat2870-regulator driver was updated to register each of the four LDOs
> from a dedicated OF node.
>
> The aat2870-backlight driver was changed to populate all required
> properties from a dedicated Device Tree node. Its channel map was updated
> to use u8 instead of int. Furthermore, because the maximum current is now
> parsed as an absolute value rather than an enum entry, the calculation and
> application of the maximum current were updated accordingly.
>
> All of the changes above allow platform data to be removed entirely.
>
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
>  drivers/mfd/aat2870-core.c            | 115 ++++++--------------------
>  drivers/regulator/aat2870-regulator.c |  76 ++++++++++++-----
>  drivers/video/backlight/aat2870_bl.c  |  55 ++++++------
>  include/linux/mfd/aat2870.h           |  95 +--------------------
>  4 files changed, 111 insertions(+), 230 deletions(-)
>
> [snip]
>
> diff --git a/drivers/video/backlight/aat2870_bl.c b/drivers/video/backlight/aat2870_bl.c
> index 8b790df1e842b..933a8f728f66a 100644
> --- a/drivers/video/backlight/aat2870_bl.c
> +++ b/drivers/video/backlight/aat2870_bl.c
> @@ -15,11 +15,19 @@
>  #include <linux/backlight.h>
>  #include <linux/mfd/aat2870.h>
>
> +/* Backlight has 8 channels, each bit represents one channel */
> +#define AAT2870_BL_CH_ALL	0xff
> +
> +/* Backlight current magnitude (uA), 450uA current is eq to 0 */
> +#define AAT2870_CURRENT_MIN	450
> +#define AAT2870_CURRENT_MAX	27900
> +#define AAT2870_CURRENT_STEP	900
> +
>  struct aat2870_bl_driver_data {
>  	struct platform_device *pdev;
>  	struct backlight_device *bd;
>
> -	int channels;
> +	u8 channels;
>  	int max_current;
>  	int brightness; /* current brightness */
>  };
> @@ -30,7 +38,7 @@ static inline int aat2870_brightness(struct aat2870_bl_driver_data *aat2870_bl,
>  	struct backlight_device *bd = aat2870_bl->bd;
>  	int val;
>
> -	val = brightness * (aat2870_bl->max_current - 1);
> +	val = brightness * aat2870_bl->max_current;
>  	val /= bd->props.max_brightness;

What is the purpose of max_brightness?

Normally it is used to limit brightness but max_current is already doing
that. Is it just being used to reduce the number of steps in the
brightness scale (and if so, why is that useful)?


>
>  	return val;
> @@ -42,7 +50,7 @@ static inline int aat2870_bl_enable(struct aat2870_bl_driver_data *aat2870_bl)
>  			= dev_get_drvdata(aat2870_bl->pdev->dev.parent);
>
>  	return aat2870->write(aat2870, AAT2870_BL_CH_EN,
> -			      (u8)aat2870_bl->channels);
> +			      aat2870_bl->channels);
>  }
>
>  static inline int aat2870_bl_disable(struct aat2870_bl_driver_data *aat2870_bl)
> @@ -96,24 +104,12 @@ static const struct backlight_ops aat2870_bl_ops = {
>
>  static int aat2870_bl_probe(struct platform_device *pdev)
>  {
> -	struct aat2870_bl_platform_data *pdata = dev_get_platdata(&pdev->dev);
>  	struct aat2870_bl_driver_data *aat2870_bl;
>  	struct backlight_device *bd;
>  	struct backlight_properties props;
> +	u32 max_brightness = 0;
>  	int ret = 0;
>
> -	if (!pdata) {
> -		dev_err(&pdev->dev, "No platform data\n");
> -		ret = -ENXIO;
> -		goto out;
> -	}
> -
> -	if (pdev->id != AAT2870_ID_BL) {
> -		dev_err(&pdev->dev, "Invalid device ID, %d\n", pdev->id);
> -		ret = -EINVAL;
> -		goto out;
> -	}
> -
>  	aat2870_bl = devm_kzalloc(&pdev->dev,
>  				  sizeof(struct aat2870_bl_driver_data),
>  				  GFP_KERNEL);
> @@ -140,18 +136,18 @@ static int aat2870_bl_probe(struct platform_device *pdev)
>
>  	aat2870_bl->bd = bd;
>
> -	if (pdata->channels > 0)
> -		aat2870_bl->channels = pdata->channels;
> -	else
> -		aat2870_bl->channels = AAT2870_BL_CH_ALL;
> +	aat2870_bl->channels = AAT2870_BL_CH_ALL;
> +	device_property_read_u8(&pdev->dev, "skyworks,channels", &aat2870_bl->channels);
>
> -	if (pdata->max_current > 0)
> -		aat2870_bl->max_current = pdata->max_current;
> -	else
> -		aat2870_bl->max_current = AAT2870_CURRENT_27_9;
> +	device_property_read_u32(&pdev->dev, "led-max-microamp", &aat2870_bl->max_current);
> +	aat2870_bl->max_current = clamp(aat2870_bl->max_current, AAT2870_CURRENT_MIN,
> +					AAT2870_CURRENT_MAX);
> +	aat2870_bl->max_current /= AAT2870_CURRENT_STEP;
>
> -	if (pdata->max_brightness > 0)
> -		bd->props.max_brightness = pdata->max_brightness;
> +	/* If max-brightness property is missing or set to zero, use chip's max value */
> +	device_property_read_u32(&pdev->dev, "max-brightness", &max_brightness);
> +	if (max_brightness)
> +		bd->props.max_brightness = max_brightness;
>  	else
>  		bd->props.max_brightness = 255;

IIUC max_current can be zero (since AAT2870_CURRENT_MIN <
AAT2870_CURRENT_STEP). Since that means aat2870_brightness() will
always return 0 then there might need to be a special case
max_brightness for this case.


> @@ -181,9 +177,16 @@ static void aat2870_bl_remove(struct platform_device *pdev)
>  	backlight_update_status(bd);
>  }
>
> +static const struct of_device_id aat2870_bl_match_table[] = {
> +	{ .compatible = "skyworks,aat2870-backlight" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, aat2870_bl_match_table);
> +
>  static struct platform_driver aat2870_bl_driver = {
>  	.driver = {
>  		.name	= "aat2870-backlight",
> +		.of_match_table = aat2870_bl_match_table,
>  	},
>  	.probe		= aat2870_bl_probe,
>  	.remove		= aat2870_bl_remove,


Daniel.

Re: [PATCH v2 1/3] dt-bindings: mfd: Document Skyworks AAT2870

[Svyatoslav Ryhel]

пн, 5 жовт. 2026 р. о 19:31 Daniel Thompson <danielt@kernel.org> пише:
>
> On Sun, Oct 04, 2026 at 07:41:19PM +0300, Svyatoslav Ryhel wrote:
> > Document Skyworks AAT2870 LED Backlight Driver and Multiple LDO Lighting
> > Management Unit.
> >
> > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > ---
> >  .../bindings/leds/skyworks,aat2870.yaml       | 127 ++++++++++++++++++
> >  1 file changed, 127 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/leds/skyworks,aat2870.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/leds/skyworks,aat2870.yaml b/Documentation/devicetree/bindings/leds/skyworks,aat2870.yaml
> > new file mode 100644
> > index 0000000000000..014678616076f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/leds/skyworks,aat2870.yaml
> > @@ -0,0 +1,127 @@
> > [snip]
> > +  backlight:
> > +    $ref: /schemas/leds/backlight/common.yaml#
> > +    unevaluatedProperties: false
> > +
> > +    properties:
> > +      compatible:
> > +        const: skyworks,aat2870-backlight
> > +
> > +      led-max-microamp:
> > +        minimum: 450
> > +        maximum: 27900
> > +        default: 450
> > +
> > +      max-brightness:
> > +        default: 255
>
> Should there be a min/max here as well?
>

Seems that you are correct. Thank you!

>
> Daniel.

Re: [PATCH v2 2/3] mfd: aat2870: Convert to use OF bindings

[Svyatoslav Ryhel]

пн, 5 жовт. 2026 р. о 19:51 Daniel Thompson <danielt@kernel.org> пише:
>
> On Sun, Oct 04, 2026 at 07:41:20PM +0300, Svyatoslav Ryhel wrote:
> > Conversion of the AAT2870 driver to use OF bindings requires a few complex
> > changes that should be done simultaneously.
> >
> > The AAT2870 essentially provides two functions via child devices:
> > backlight and regulators. Both functions are fairly self-sufficient and
> > may not be populated on the final board. Consequently, the MFD
> > registration API was replaced with of_platform_populate(), and each
> > sub-device was given its own compatible string.
> >
> > Additionally, aat2870-core utilizes an enable GPIO. Obtaining this GPIO
> > was converted to use modern gpiod/OF helpers, allowing the redundant
> > aat2870_enable() and aat2870_disable() helpers to be removed.
> >
> > The aat2870-regulator driver was updated to register each of the four LDOs
> > from a dedicated OF node.
> >
> > The aat2870-backlight driver was changed to populate all required
> > properties from a dedicated Device Tree node. Its channel map was updated
> > to use u8 instead of int. Furthermore, because the maximum current is now
> > parsed as an absolute value rather than an enum entry, the calculation and
> > application of the maximum current were updated accordingly.
> >
> > All of the changes above allow platform data to be removed entirely.
> >
> > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > ---
> >  drivers/mfd/aat2870-core.c            | 115 ++++++--------------------
> >  drivers/regulator/aat2870-regulator.c |  76 ++++++++++++-----
> >  drivers/video/backlight/aat2870_bl.c  |  55 ++++++------
> >  include/linux/mfd/aat2870.h           |  95 +--------------------
> >  4 files changed, 111 insertions(+), 230 deletions(-)
> >
> > [snip]
> >
> > diff --git a/drivers/video/backlight/aat2870_bl.c b/drivers/video/backlight/aat2870_bl.c
> > index 8b790df1e842b..933a8f728f66a 100644
> > --- a/drivers/video/backlight/aat2870_bl.c
> > +++ b/drivers/video/backlight/aat2870_bl.c
> > @@ -15,11 +15,19 @@
> >  #include <linux/backlight.h>
> >  #include <linux/mfd/aat2870.h>
> >
> > +/* Backlight has 8 channels, each bit represents one channel */
> > +#define AAT2870_BL_CH_ALL    0xff
> > +
> > +/* Backlight current magnitude (uA), 450uA current is eq to 0 */
> > +#define AAT2870_CURRENT_MIN  450
> > +#define AAT2870_CURRENT_MAX  27900
> > +#define AAT2870_CURRENT_STEP 900
> > +
> >  struct aat2870_bl_driver_data {
> >       struct platform_device *pdev;
> >       struct backlight_device *bd;
> >
> > -     int channels;
> > +     u8 channels;
> >       int max_current;
> >       int brightness; /* current brightness */
> >  };
> > @@ -30,7 +38,7 @@ static inline int aat2870_brightness(struct aat2870_bl_driver_data *aat2870_bl,
> >       struct backlight_device *bd = aat2870_bl->bd;
> >       int val;
> >
> > -     val = brightness * (aat2870_bl->max_current - 1);
> > +     val = brightness * aat2870_bl->max_current;
> >       val /= bd->props.max_brightness;
>
> What is the purpose of max_brightness?
>
> Normally it is used to limit brightness but max_current is already doing
> that. Is it just being used to reduce the number of steps in the
> brightness scale (and if so, why is that useful)?
>

This is the original driver behavior; I did not modify it.

The AAT2870 backlight intensity is regulated by the Main Backlight
Current Magnitude register. It has 31 steps from 450uA (0) to 27.9mA
(31), so even setting max_current to 0 will not disable the backlight.
The max current value may be limited for some configurations, which is
why this property exists.

The max_brightness property determines the maximum number of steps the
backlight can have and is mapped to the available current range
provided by the hardware. The backlight is controlled in steps, not in
uA.

>
> >
> >       return val;
> > @@ -42,7 +50,7 @@ static inline int aat2870_bl_enable(struct aat2870_bl_driver_data *aat2870_bl)
> >                       = dev_get_drvdata(aat2870_bl->pdev->dev.parent);
> >
> >       return aat2870->write(aat2870, AAT2870_BL_CH_EN,
> > -                           (u8)aat2870_bl->channels);
> > +                           aat2870_bl->channels);
> >  }
> >
> >  static inline int aat2870_bl_disable(struct aat2870_bl_driver_data *aat2870_bl)
> > @@ -96,24 +104,12 @@ static const struct backlight_ops aat2870_bl_ops = {
> >
> >  static int aat2870_bl_probe(struct platform_device *pdev)
> >  {
> > -     struct aat2870_bl_platform_data *pdata = dev_get_platdata(&pdev->dev);
> >       struct aat2870_bl_driver_data *aat2870_bl;
> >       struct backlight_device *bd;
> >       struct backlight_properties props;
> > +     u32 max_brightness = 0;
> >       int ret = 0;
> >
> > -     if (!pdata) {
> > -             dev_err(&pdev->dev, "No platform data\n");
> > -             ret = -ENXIO;
> > -             goto out;
> > -     }
> > -
> > -     if (pdev->id != AAT2870_ID_BL) {
> > -             dev_err(&pdev->dev, "Invalid device ID, %d\n", pdev->id);
> > -             ret = -EINVAL;
> > -             goto out;
> > -     }
> > -
> >       aat2870_bl = devm_kzalloc(&pdev->dev,
> >                                 sizeof(struct aat2870_bl_driver_data),
> >                                 GFP_KERNEL);
> > @@ -140,18 +136,18 @@ static int aat2870_bl_probe(struct platform_device *pdev)
> >
> >       aat2870_bl->bd = bd;
> >
> > -     if (pdata->channels > 0)
> > -             aat2870_bl->channels = pdata->channels;
> > -     else
> > -             aat2870_bl->channels = AAT2870_BL_CH_ALL;
> > +     aat2870_bl->channels = AAT2870_BL_CH_ALL;
> > +     device_property_read_u8(&pdev->dev, "skyworks,channels", &aat2870_bl->channels);
> >
> > -     if (pdata->max_current > 0)
> > -             aat2870_bl->max_current = pdata->max_current;
> > -     else
> > -             aat2870_bl->max_current = AAT2870_CURRENT_27_9;
> > +     device_property_read_u32(&pdev->dev, "led-max-microamp", &aat2870_bl->max_current);
> > +     aat2870_bl->max_current = clamp(aat2870_bl->max_current, AAT2870_CURRENT_MIN,
> > +                                     AAT2870_CURRENT_MAX);
> > +     aat2870_bl->max_current /= AAT2870_CURRENT_STEP;
> >
> > -     if (pdata->max_brightness > 0)
> > -             bd->props.max_brightness = pdata->max_brightness;
> > +     /* If max-brightness property is missing or set to zero, use chip's max value */
> > +     device_property_read_u32(&pdev->dev, "max-brightness", &max_brightness);
> > +     if (max_brightness)
> > +             bd->props.max_brightness = max_brightness;
> >       else
> >               bd->props.max_brightness = 255;
>
> IIUC max_current can be zero (since AAT2870_CURRENT_MIN <
> AAT2870_CURRENT_STEP). Since that means aat2870_brightness() will
> always return 0 then there might need to be a special case
> max_brightness for this case.
>
>
> > @@ -181,9 +177,16 @@ static void aat2870_bl_remove(struct platform_device *pdev)
> >       backlight_update_status(bd);
> >  }
> >
> > +static const struct of_device_id aat2870_bl_match_table[] = {
> > +     { .compatible = "skyworks,aat2870-backlight" },
> > +     { }
> > +};
> > +MODULE_DEVICE_TABLE(of, aat2870_bl_match_table);
> > +
> >  static struct platform_driver aat2870_bl_driver = {
> >       .driver = {
> >               .name   = "aat2870-backlight",
> > +             .of_match_table = aat2870_bl_match_table,
> >       },
> >       .probe          = aat2870_bl_probe,
> >       .remove         = aat2870_bl_remove,
>
>
> Daniel.

Re: [PATCH v2 2/3] mfd: aat2870: Convert to use OF bindings

[Daniel Thompson]

On Mon, Oct 05, 2026 at 08:18:31PM +0300, Svyatoslav Ryhel wrote:
> пн, 5 жовт. 2026 р. о 19:51 Daniel Thompson <danielt@kernel.org> пише:
> >
> > On Sun, Oct 04, 2026 at 07:41:20PM +0300, Svyatoslav Ryhel wrote:
> > > Conversion of the AAT2870 driver to use OF bindings requires a few complex
> > > changes that should be done simultaneously.
> > >
> > > The AAT2870 essentially provides two functions via child devices:
> > > backlight and regulators. Both functions are fairly self-sufficient and
> > > may not be populated on the final board. Consequently, the MFD
> > > registration API was replaced with of_platform_populate(), and each
> > > sub-device was given its own compatible string.
> > >
> > > Additionally, aat2870-core utilizes an enable GPIO. Obtaining this GPIO
> > > was converted to use modern gpiod/OF helpers, allowing the redundant
> > > aat2870_enable() and aat2870_disable() helpers to be removed.
> > >
> > > The aat2870-regulator driver was updated to register each of the four LDOs
> > > from a dedicated OF node.
> > >
> > > The aat2870-backlight driver was changed to populate all required
> > > properties from a dedicated Device Tree node. Its channel map was updated
> > > to use u8 instead of int. Furthermore, because the maximum current is now
> > > parsed as an absolute value rather than an enum entry, the calculation and
> > > application of the maximum current were updated accordingly.
> > >
> > > All of the changes above allow platform data to be removed entirely.
> > >
> > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > ---
> > >  drivers/mfd/aat2870-core.c            | 115 ++++++--------------------
> > >  drivers/regulator/aat2870-regulator.c |  76 ++++++++++++-----
> > >  drivers/video/backlight/aat2870_bl.c  |  55 ++++++------
> > >  include/linux/mfd/aat2870.h           |  95 +--------------------
> > >  4 files changed, 111 insertions(+), 230 deletions(-)
> > >
> > > [snip]
> > >
> > > diff --git a/drivers/video/backlight/aat2870_bl.c b/drivers/video/backlight/aat2870_bl.c
> > > index 8b790df1e842b..933a8f728f66a 100644
> > > --- a/drivers/video/backlight/aat2870_bl.c
> > > +++ b/drivers/video/backlight/aat2870_bl.c
> > > @@ -15,11 +15,19 @@
> > >  #include <linux/backlight.h>
> > >  #include <linux/mfd/aat2870.h>
> > >
> > > +/* Backlight has 8 channels, each bit represents one channel */
> > > +#define AAT2870_BL_CH_ALL    0xff
> > > +
> > > +/* Backlight current magnitude (uA), 450uA current is eq to 0 */
> > > +#define AAT2870_CURRENT_MIN  450
> > > +#define AAT2870_CURRENT_MAX  27900
> > > +#define AAT2870_CURRENT_STEP 900
> > > +
> > >  struct aat2870_bl_driver_data {
> > >       struct platform_device *pdev;
> > >       struct backlight_device *bd;
> > >
> > > -     int channels;
> > > +     u8 channels;
> > >       int max_current;
> > >       int brightness; /* current brightness */
> > >  };
> > > @@ -30,7 +38,7 @@ static inline int aat2870_brightness(struct aat2870_bl_driver_data *aat2870_bl,
> > >       struct backlight_device *bd = aat2870_bl->bd;
> > >       int val;
> > >
> > > -     val = brightness * (aat2870_bl->max_current - 1);
> > > +     val = brightness * aat2870_bl->max_current;
> > >       val /= bd->props.max_brightness;
> >
> > What is the purpose of max_brightness?
> >
> > Normally it is used to limit brightness but max_current is already doing
> > that. Is it just being used to reduce the number of steps in the
> > brightness scale (and if so, why is that useful)?
> >
>
> This is the original driver behavior; I did not modify it.

I can see that but the parameters for these (rather dubious) previous
behaviours are now being copied into the devicetree bindings where they
become stable ABI.


> The AAT2870 backlight intensity is regulated by the Main Backlight
> Current Magnitude register. It has 31 steps from 450uA (0) to 27.9mA
> (31), so even setting max_current to 0 will not disable the backlight.
> The max current value may be limited for some configurations, which is
> why this property exists.
>
> The max_brightness property determines the maximum number of steps the
> backlight can have and is mapped to the available current range
> provided by the hardware. The backlight is controlled in steps, not in
> uA.

This behaviour does not match the devicetree bindings documentation for
max-brightness:

  Normally the maximum brightness is determined by the hardware and this
  property is not required. This property is used to put a software limit
  on the brightness apart from what the driver says, as it could happen
  that a LED can be made so bright that it gets damaged or causes damage
  due to restrictions in a specific system, such as mounting conditions.

Hence the question, why is the current behaviour useful?

If I understand your summary correctly the hardware has 32 steps
including 0) and the effect of max_current is to ban all brightness
values greater than max_current / AAT2870_CURRENT_STEP.

If that is the case then max_brightness if fundamentally broken because
it's default value offers more steps than exists in the hardware.
bd->props.max_brightness should be set to max_current /
AAT2870_CURRENT_STEP and the whole of aat2870_brightness() should be
removed (use backlight_get_brightness() instead).

IMHO the patchset will be cleaner if the weird max_brightness logic is
removed in a separate before doing the DT changes.


Daniel.

Re: [PATCH v2 2/3] mfd: aat2870: Convert to use OF bindings

[Svyatoslav Ryhel]

вт, 6 жовт. 2026 р. о 12:38 Daniel Thompson <danielt@kernel.org> пише:
>
> On Mon, Oct 05, 2026 at 08:18:31PM +0300, Svyatoslav Ryhel wrote:
> > пн, 5 жовт. 2026 р. о 19:51 Daniel Thompson <danielt@kernel.org> пише:
> > >
> > > On Sun, Oct 04, 2026 at 07:41:20PM +0300, Svyatoslav Ryhel wrote:
> > > > Conversion of the AAT2870 driver to use OF bindings requires a few complex
> > > > changes that should be done simultaneously.
> > > >
> > > > The AAT2870 essentially provides two functions via child devices:
> > > > backlight and regulators. Both functions are fairly self-sufficient and
> > > > may not be populated on the final board. Consequently, the MFD
> > > > registration API was replaced with of_platform_populate(), and each
> > > > sub-device was given its own compatible string.
> > > >
> > > > Additionally, aat2870-core utilizes an enable GPIO. Obtaining this GPIO
> > > > was converted to use modern gpiod/OF helpers, allowing the redundant
> > > > aat2870_enable() and aat2870_disable() helpers to be removed.
> > > >
> > > > The aat2870-regulator driver was updated to register each of the four LDOs
> > > > from a dedicated OF node.
> > > >
> > > > The aat2870-backlight driver was changed to populate all required
> > > > properties from a dedicated Device Tree node. Its channel map was updated
> > > > to use u8 instead of int. Furthermore, because the maximum current is now
> > > > parsed as an absolute value rather than an enum entry, the calculation and
> > > > application of the maximum current were updated accordingly.
> > > >
> > > > All of the changes above allow platform data to be removed entirely.
> > > >
> > > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > > ---
> > > >  drivers/mfd/aat2870-core.c            | 115 ++++++--------------------
> > > >  drivers/regulator/aat2870-regulator.c |  76 ++++++++++++-----
> > > >  drivers/video/backlight/aat2870_bl.c  |  55 ++++++------
> > > >  include/linux/mfd/aat2870.h           |  95 +--------------------
> > > >  4 files changed, 111 insertions(+), 230 deletions(-)
> > > >
> > > > [snip]
> > > >
> > > > diff --git a/drivers/video/backlight/aat2870_bl.c b/drivers/video/backlight/aat2870_bl.c
> > > > index 8b790df1e842b..933a8f728f66a 100644
> > > > --- a/drivers/video/backlight/aat2870_bl.c
> > > > +++ b/drivers/video/backlight/aat2870_bl.c
> > > > @@ -15,11 +15,19 @@
> > > >  #include <linux/backlight.h>
> > > >  #include <linux/mfd/aat2870.h>
> > > >
> > > > +/* Backlight has 8 channels, each bit represents one channel */
> > > > +#define AAT2870_BL_CH_ALL    0xff
> > > > +
> > > > +/* Backlight current magnitude (uA), 450uA current is eq to 0 */
> > > > +#define AAT2870_CURRENT_MIN  450
> > > > +#define AAT2870_CURRENT_MAX  27900
> > > > +#define AAT2870_CURRENT_STEP 900
> > > > +
> > > >  struct aat2870_bl_driver_data {
> > > >       struct platform_device *pdev;
> > > >       struct backlight_device *bd;
> > > >
> > > > -     int channels;
> > > > +     u8 channels;
> > > >       int max_current;
> > > >       int brightness; /* current brightness */
> > > >  };
> > > > @@ -30,7 +38,7 @@ static inline int aat2870_brightness(struct aat2870_bl_driver_data *aat2870_bl,
> > > >       struct backlight_device *bd = aat2870_bl->bd;
> > > >       int val;
> > > >
> > > > -     val = brightness * (aat2870_bl->max_current - 1);
> > > > +     val = brightness * aat2870_bl->max_current;
> > > >       val /= bd->props.max_brightness;
> > >
> > > What is the purpose of max_brightness?
> > >
> > > Normally it is used to limit brightness but max_current is already doing
> > > that. Is it just being used to reduce the number of steps in the
> > > brightness scale (and if so, why is that useful)?
> > >
> >
> > This is the original driver behavior; I did not modify it.
>
> I can see that but the parameters for these (rather dubious) previous
> behaviours are now being copied into the devicetree bindings where they
> become stable ABI.
>
>
> > The AAT2870 backlight intensity is regulated by the Main Backlight
> > Current Magnitude register. It has 31 steps from 450uA (0) to 27.9mA
> > (31), so even setting max_current to 0 will not disable the backlight.
> > The max current value may be limited for some configurations, which is
> > why this property exists.
> >
> > The max_brightness property determines the maximum number of steps the
> > backlight can have and is mapped to the available current range
> > provided by the hardware. The backlight is controlled in steps, not in
> > uA.
>
> This behaviour does not match the devicetree bindings documentation for
> max-brightness:
>
>   Normally the maximum brightness is determined by the hardware and this
>   property is not required. This property is used to put a software limit
>   on the brightness apart from what the driver says, as it could happen
>   that a LED can be made so bright that it gets damaged or causes damage
>   due to restrictions in a specific system, such as mounting conditions.
>
> Hence the question, why is the current behaviour useful?
>
> If I understand your summary correctly the hardware has 32 steps
> including 0) and the effect of max_current is to ban all brightness
> values greater than max_current / AAT2870_CURRENT_STEP.
>
> If that is the case then max_brightness if fundamentally broken because
> it's default value offers more steps than exists in the hardware.
> bd->props.max_brightness should be set to max_current /
> AAT2870_CURRENT_STEP and the whole of aat2870_brightness() should be
> removed (use backlight_get_brightness() instead).
>
> IMHO the patchset will be cleaner if the weird max_brightness logic is
> removed in a separate before doing the DT changes.
>

Noted, I will see what I can do. Thank you.

>
> Daniel.