[PATCH v2 0/2] mfd: lm3533: convert to use OF

[PATCH v2 0/2] mfd: lm3533: convert to use OF

[Svyatoslav Ryhel]

Add schema and add support for lm3533 mfd to use device
tree bindings.

---
Changes on switching from v1 to v2:
- added unit seffix where it is suitable
- added vendor prefixes where it is suitable
- light sensor mover out of pattern properties
- added references to common schemas
- added detailed descriptions of properties
- removed platform data use
- devices bind and configure themself entirely
  using device tree
---

Svyatoslav Ryhel (2):
  dt-bindings: mfd: Document TI LM3533 MFD
  mfd: lm3533: convert to use OF

 .../devicetree/bindings/mfd/ti,lm3533.yaml    | 231 ++++++++++++++++++
 drivers/iio/light/lm3533-als.c                |  40 ++-
 drivers/leds/leds-lm3533.c                    |  45 ++--
 drivers/mfd/lm3533-core.c                     | 199 ++++++---------
 drivers/video/backlight/lm3533_bl.c           |  68 ++++--
 include/linux/mfd/lm3533.h                    |  35 +--
 6 files changed, 417 insertions(+), 201 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/ti,lm3533.yaml

-- 
2.43.0

[PATCH v2 1/2] dt-bindings: mfd: Document TI LM3533 MFD

[Svyatoslav Ryhel]

Add bindings for the LM3533 - a complete power source for
backlight, keypad, and indicator LEDs in smartphone handsets.
The high-voltage inductive boost converter provides the
power for two series LED strings display backlight and keypad
functions.

Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 .../devicetree/bindings/mfd/ti,lm3533.yaml    | 231 ++++++++++++++++++
 1 file changed, 231 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/ti,lm3533.yaml

diff --git a/Documentation/devicetree/bindings/mfd/ti,lm3533.yaml b/Documentation/devicetree/bindings/mfd/ti,lm3533.yaml
new file mode 100644
index 000000000000..83542f0c7bf7
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/ti,lm3533.yaml
@@ -0,0 +1,231 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/ti,lm3533.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TI LM3533 Complete Lighting Power Solution
+
+description: |
+  The LM3533 is a complete power source for backlight,
+  keypad, and indicator LEDs in smartphone handsets. The
+  high-voltage inductive boost converter provides the
+  power for two series LED strings display backlight and
+  keypad functions.
+  https://www.ti.com/product/LM3533
+
+maintainers:
+  - Svyatoslav Ryhel <clamor95@gmail.com>
+
+properties:
+  compatible:
+    const: ti,lm3533
+
+  reg:
+    maxItems: 1
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 0
+
+  enable-gpios:
+    description: GPIO to use to enable/disable the backlight (HWEN pin).
+    maxItems: 1
+
+  ti,boost-ovp-microvolt:
+    description:
+      Boost OVP select (16V, 24V, 32V, 40V)
+    enum: [ 16000000, 24000000, 32000000, 40000000 ]
+    default: 16000000
+
+  ti,boost-freq-hz:
+    description:
+      Boost frequency select (500KHz or 1MHz)
+    enum: [ 500000, 1000000 ]
+    default: 500000
+
+  light-sensor@0:
+    type: object
+    description:
+      Properties for an illumination sensor.
+
+    properties:
+      compatible:
+        const: ti,lm3533-als
+
+      reg:
+        const: 0
+
+      ti,resistor-value-ohm:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description: |
+          Internal configuration resister value when ALS is in Analog Sensor
+          mode and PWM mode is disabled.
+        minimum: 1575
+        maximum: 200000
+
+      ti,pwm-mode:
+        type: boolean
+        description: |
+          Switch for mode in which ALS is running. If this propertly is
+          set then ALS is running in PWM mode, internal resistor value is
+          set to high-impedance (0) and resistor-value-ohm propertly is
+          ignored.
+
+    required:
+      - compatible
+      - reg
+
+    additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - '#address-cells'
+  - '#size-cells'
+  - enable-gpios
+
+patternProperties:
+  "^backlight@[01]$":
+    type: object
+    description:
+      Properties for a backlight device.
+
+    $ref: /schemas/leds/backlight/common.yaml#
+
+    properties:
+      compatible:
+        const: ti,lm3533-backlight
+
+      reg:
+        description: |
+          The control bank that is used to program the two current sinks. The
+          LM3533 has two control banks (A and B) and are represented as 0 or 1
+          in this property. The two current sinks can be controlled
+          independently with both banks, or bank A can be configured to control
+          both sinks with the led-sources property.
+        minimum: 0
+        maximum: 1
+
+      default-brightness: true
+
+      ti,max-current-microamp:
+        description:
+          Maximum current in µA with a 800 µA step.
+        enum: [ 5000, 5800, 6600, 7400, 8200, 9000, 9800,
+                10600, 11400, 12200, 13000, 13800, 14600,
+                15400, 16200, 17000, 17800, 18600, 19400,
+                20200, 21000, 21800, 22600, 23400, 24200,
+                25000, 25800, 26600, 27400, 28200, 29000,
+                29800 ]
+        default: 5000
+
+      ti,pwm-config-mask:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description: |
+          Control Bank PWM Configuration Register mask that allows to configure
+          PWM input in Zones 0-4
+          BIT(0) - PWM Input is enabled
+          BIT(1) - PWM Input is enabled in Zone 0
+          BIT(2) - PWM Input is enabled in Zone 1
+          BIT(3) - PWM Input is enabled in Zone 2
+          BIT(4) - PWM Input is enabled in Zone 3
+          BIT(5) - PWM Input is enabled in Zone 4
+
+      ti,linear-mapping-mode:
+        description: |
+          Enable linear mapping mode. If disabled, then it will use exponential
+          mapping mode in which the ramp up/down appears to have a more uniform
+          transition to the human eye.
+        type: boolean
+
+      ti,hardware-controlled:
+        description: |
+          Each backlight has its own voltage Control Bank (A and B) and there are
+          two HVLED sinks which by default are linked to respective Bank. Setting
+          this property will link both sinks to a Control Bank of backlight where
+          property is defined.
+        type: boolean
+
+    required:
+      - compatible
+      - reg
+
+    additionalProperties: false
+
+  "^led@[0-3]$":
+    type: object
+    description:
+      Properties for a led device.
+
+    $ref: /schemas/leds/common.yaml#
+
+    properties:
+      compatible:
+        const: ti,lm3533-leds
+
+      reg:
+        description:
+          4 led banks
+        minimum: 0
+        maximum: 3
+
+      linux,default-trigger: true
+
+      ti,max-current-microamp:
+        description:
+          Maximum current in µA with a 800 µA step.
+        enum: [ 5000, 5800, 6600, 7400, 8200, 9000, 9800,
+                10600, 11400, 12200, 13000, 13800, 14600,
+                15400, 16200, 17000, 17800, 18600, 19400,
+                20200, 21000, 21800, 22600, 23400, 24200,
+                25000, 25800, 26600, 27400, 28200, 29000,
+                29800 ]
+        default: 5000
+
+      ti,pwm-config-mask:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description:
+          Same descryption and function as for backlight.
+
+    required:
+      - compatible
+      - reg
+
+    additionalProperties: false
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        led-controller@36 {
+            compatible = "ti,lm3533";
+            reg = <0x36>;
+
+            enable-gpios = <&gpio 110 GPIO_ACTIVE_HIGH>;
+
+            ti,boost-ovp-microvolt = <24000000>;
+            ti,boost-freq-hz = <500000>;
+
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            backlight@0 {
+                compatible = "ti,lm3533-backlight";
+                reg = <0>;
+
+                ti,max-current-microamp = <23400>;
+                default-brightness = <113>;
+                ti,hardware-controlled;
+            };
+        };
+    };
+...
-- 
2.43.0

[PATCH v2 2/2] mfd: lm3533: convert to use OF

[Svyatoslav Ryhel]

Remove platform data and fully relay on OF and device tree
parsing and binding devices.

Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 drivers/iio/light/lm3533-als.c      |  40 ++++--
 drivers/leds/leds-lm3533.c          |  45 +++++--
 drivers/mfd/lm3533-core.c           | 199 ++++++++++------------------
 drivers/video/backlight/lm3533_bl.c |  68 +++++++---
 include/linux/mfd/lm3533.h          |  35 +----
 5 files changed, 186 insertions(+), 201 deletions(-)

diff --git a/drivers/iio/light/lm3533-als.c b/drivers/iio/light/lm3533-als.c
index 99f0b903018c..931ea66cad1a 100644
--- a/drivers/iio/light/lm3533-als.c
+++ b/drivers/iio/light/lm3533-als.c
@@ -16,9 +16,12 @@
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/mfd/core.h>
+#include <linux/mod_devicetable.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 #include <linux/slab.h>
 #include <linux/uaccess.h>
+#include <linux/units.h>
 
 #include <linux/mfd/lm3533.h>
 
@@ -56,6 +59,9 @@ struct lm3533_als {
 
 	atomic_t zone;
 	struct mutex thresh_mutex;
+
+	unsigned pwm_mode:1;		/* PWM input mode (default analog) */
+	u8 r_select;			/* 1 - 127 (ignored in PWM-mode) */
 };
 
 
@@ -753,18 +759,17 @@ static int lm3533_als_set_resistor(struct lm3533_als *als, u8 val)
 	return 0;
 }
 
-static int lm3533_als_setup(struct lm3533_als *als,
-			    const struct lm3533_als_platform_data *pdata)
+static int lm3533_als_setup(struct lm3533_als *als)
 {
 	int ret;
 
-	ret = lm3533_als_set_input_mode(als, pdata->pwm_mode);
+	ret = lm3533_als_set_input_mode(als, als->pwm_mode);
 	if (ret)
 		return ret;
 
 	/* ALS input is always high impedance in PWM-mode. */
-	if (!pdata->pwm_mode) {
-		ret = lm3533_als_set_resistor(als, pdata->r_select);
+	if (!als->pwm_mode) {
+		ret = lm3533_als_set_resistor(als, als->r_select);
 		if (ret)
 			return ret;
 	}
@@ -828,22 +833,16 @@ static const struct iio_info lm3533_als_info = {
 
 static int lm3533_als_probe(struct platform_device *pdev)
 {
-	const struct lm3533_als_platform_data *pdata;
 	struct lm3533 *lm3533;
 	struct lm3533_als *als;
 	struct iio_dev *indio_dev;
+	u32 val;
 	int ret;
 
 	lm3533 = dev_get_drvdata(pdev->dev.parent);
 	if (!lm3533)
 		return -EINVAL;
 
-	pdata = dev_get_platdata(&pdev->dev);
-	if (!pdata) {
-		dev_err(&pdev->dev, "no platform data\n");
-		return -EINVAL;
-	}
-
 	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*als));
 	if (!indio_dev)
 		return -ENOMEM;
@@ -864,13 +863,21 @@ static int lm3533_als_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, indio_dev);
 
+	val = 200 * KILO; /* 200kOhm */
+	device_property_read_u32(&pdev->dev, "ti,resistor-value-ohm", &val);
+
+	/* Convert resitance into R_ALS value with 2v / 10uA * R */
+	als->r_select = DIV_ROUND_UP(2 * MICRO, 10 * val);
+
+	als->pwm_mode = device_property_read_bool(&pdev->dev, "ti,pwm-mode");
+
 	if (als->irq) {
 		ret = lm3533_als_setup_irq(als, indio_dev);
 		if (ret)
 			return ret;
 	}
 
-	ret = lm3533_als_setup(als, pdata);
+	ret = lm3533_als_setup(als);
 	if (ret)
 		goto err_free_irq;
 
@@ -907,9 +914,16 @@ static void lm3533_als_remove(struct platform_device *pdev)
 		free_irq(als->irq, indio_dev);
 }
 
+static const struct of_device_id lm3533_als_match_table[] = {
+	{ .compatible = "ti,lm3533-als" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, lm3533_match_table);
+
 static struct platform_driver lm3533_als_driver = {
 	.driver	= {
 		.name	= "lm3533-als",
+		.of_match_table = lm3533_als_match_table,
 	},
 	.probe		= lm3533_als_probe,
 	.remove		= lm3533_als_remove,
diff --git a/drivers/leds/leds-lm3533.c b/drivers/leds/leds-lm3533.c
index 45795f2a1042..e955c4ef0af5 100644
--- a/drivers/leds/leds-lm3533.c
+++ b/drivers/leds/leds-lm3533.c
@@ -10,8 +10,10 @@
 #include <linux/module.h>
 #include <linux/leds.h>
 #include <linux/mfd/core.h>
+#include <linux/mod_devicetable.h>
 #include <linux/mutex.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 #include <linux/slab.h>
 
 #include <linux/mfd/lm3533.h>
@@ -48,6 +50,9 @@ struct lm3533_led {
 
 	struct mutex mutex;
 	unsigned long flags;
+
+	u16 max_current;		/* 5000 - 29800 uA (800 uA step) */
+	u8 pwm;				/* 0 - 0x3f */
 };
 
 
@@ -632,23 +637,22 @@ static const struct attribute_group *lm3533_led_attribute_groups[] = {
 	NULL
 };
 
-static int lm3533_led_setup(struct lm3533_led *led,
-					struct lm3533_led_platform_data *pdata)
+static int lm3533_led_setup(struct lm3533_led *led)
 {
 	int ret;
 
-	ret = lm3533_ctrlbank_set_max_current(&led->cb, pdata->max_current);
+	ret = lm3533_ctrlbank_set_max_current(&led->cb, led->max_current);
 	if (ret)
 		return ret;
 
-	return lm3533_ctrlbank_set_pwm(&led->cb, pdata->pwm);
+	return lm3533_ctrlbank_set_pwm(&led->cb, led->pwm);
 }
 
 static int lm3533_led_probe(struct platform_device *pdev)
 {
 	struct lm3533 *lm3533;
-	struct lm3533_led_platform_data *pdata;
 	struct lm3533_led *led;
+	u32 val;
 	int ret;
 
 	dev_dbg(&pdev->dev, "%s\n", __func__);
@@ -657,12 +661,6 @@ static int lm3533_led_probe(struct platform_device *pdev)
 	if (!lm3533)
 		return -EINVAL;
 
-	pdata = dev_get_platdata(&pdev->dev);
-	if (!pdata) {
-		dev_err(&pdev->dev, "no platform data\n");
-		return -EINVAL;
-	}
-
 	if (pdev->id < 0 || pdev->id >= LM3533_LVCTRLBANK_COUNT) {
 		dev_err(&pdev->dev, "illegal LED id %d\n", pdev->id);
 		return -EINVAL;
@@ -673,8 +671,10 @@ static int lm3533_led_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	led->lm3533 = lm3533;
-	led->cdev.name = pdata->name;
-	led->cdev.default_trigger = pdata->default_trigger;
+	led->cdev.name = pdev->name;
+	led->cdev.default_trigger = "none";
+	device_property_read_string(&pdev->dev, "linux,default-trigger",
+				    &led->cdev.default_trigger);
 	led->cdev.brightness_set_blocking = lm3533_led_set;
 	led->cdev.brightness_get = lm3533_led_get;
 	led->cdev.blink_set = lm3533_led_blink_set;
@@ -702,7 +702,17 @@ static int lm3533_led_probe(struct platform_device *pdev)
 
 	led->cb.dev = led->cdev.dev;
 
-	ret = lm3533_led_setup(led, pdata);
+	/* 5000 - 29800 uA (800 uA step) */
+	val = 5000;
+	device_property_read_u32(&pdev->dev, "ti,max-current-microamp", &val);
+	led->max_current = val;
+
+	/* 0 - 0x3f */
+	val = 0;
+	device_property_read_u32(&pdev->dev, "ti,pwm-config-mask", &val);
+	led->pwm = val;
+
+	ret = lm3533_led_setup(led);
 	if (ret)
 		goto err_deregister;
 
@@ -739,9 +749,16 @@ static void lm3533_led_shutdown(struct platform_device *pdev)
 	lm3533_led_set(&led->cdev, LED_OFF);		/* disable blink */
 }
 
+static const struct of_device_id lm3533_led_match_table[] = {
+	{ .compatible = "ti,lm3533-leds" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, lm3533_match_table);
+
 static struct platform_driver lm3533_led_driver = {
 	.driver = {
 		.name = "lm3533-leds",
+		.of_match_table = lm3533_led_match_table,
 	},
 	.probe		= lm3533_led_probe,
 	.remove		= lm3533_led_remove,
diff --git a/drivers/mfd/lm3533-core.c b/drivers/mfd/lm3533-core.c
index 0a2409d00b2e..f535c09add6a 100644
--- a/drivers/mfd/lm3533-core.c
+++ b/drivers/mfd/lm3533-core.c
@@ -14,10 +14,13 @@
 #include <linux/gpio/consumer.h>
 #include <linux/i2c.h>
 #include <linux/mfd/core.h>
+#include <linux/mod_devicetable.h>
+#include <linux/property.h>
 #include <linux/regmap.h>
 #include <linux/seq_file.h>
 #include <linux/slab.h>
 #include <linux/uaccess.h>
+#include <linux/units.h>
 
 #include <linux/mfd/lm3533.h>
 
@@ -42,44 +45,6 @@
 
 #define LM3533_REG_MAX			0xb2
 
-
-static struct mfd_cell lm3533_als_devs[] = {
-	{
-		.name	= "lm3533-als",
-		.id	= -1,
-	},
-};
-
-static struct mfd_cell lm3533_bl_devs[] = {
-	{
-		.name	= "lm3533-backlight",
-		.id	= 0,
-	},
-	{
-		.name	= "lm3533-backlight",
-		.id	= 1,
-	},
-};
-
-static struct mfd_cell lm3533_led_devs[] = {
-	{
-		.name	= "lm3533-leds",
-		.id	= 0,
-	},
-	{
-		.name	= "lm3533-leds",
-		.id	= 1,
-	},
-	{
-		.name	= "lm3533-leds",
-		.id	= 2,
-	},
-	{
-		.name	= "lm3533-leds",
-		.id	= 3,
-	},
-};
-
 int lm3533_read(struct lm3533 *lm3533, u8 reg, u8 *val)
 {
 	int tmp;
@@ -376,136 +341,113 @@ static struct attribute_group lm3533_attribute_group = {
 	.attrs		= lm3533_attributes
 };
 
-static int lm3533_device_als_init(struct lm3533 *lm3533)
+static int lm3533_device_setup(struct lm3533 *lm3533)
 {
-	struct lm3533_platform_data *pdata = dev_get_platdata(lm3533->dev);
 	int ret;
 
-	if (!pdata->als)
-		return 0;
-
-	lm3533_als_devs[0].platform_data = pdata->als;
-	lm3533_als_devs[0].pdata_size = sizeof(*pdata->als);
-
-	ret = mfd_add_devices(lm3533->dev, 0, lm3533_als_devs, 1, NULL,
-			      0, NULL);
-	if (ret) {
-		dev_err(lm3533->dev, "failed to add ALS device\n");
+	ret = lm3533_set_boost_freq(lm3533, lm3533->boost_freq);
+	if (ret)
 		return ret;
-	}
 
-	lm3533->have_als = 1;
+	return lm3533_set_boost_ovp(lm3533, lm3533->boost_ovp);
+}
 
-	return 0;
+static void lm3533_remove_subdev(void *subdev)
+{
+	platform_device_unregister(subdev);
 }
 
-static int lm3533_device_bl_init(struct lm3533 *lm3533)
+static int lm3533_init_components(struct lm3533 *lm3533)
 {
-	struct lm3533_platform_data *pdata = dev_get_platdata(lm3533->dev);
-	int i;
+	struct device *dev = lm3533->dev;
+	struct platform_device_info subdev_info;
+	struct platform_device *subdev;
+	struct fwnode_handle *child;
+	const char *comatible;
+	u32 id = PLATFORM_DEVID_AUTO;
 	int ret;
 
-	if (!pdata->backlights || pdata->num_backlights == 0)
-		return 0;
+	memset(&subdev_info, 0, sizeof(subdev_info));
+	subdev_info.parent = dev;
 
-	if (pdata->num_backlights > ARRAY_SIZE(lm3533_bl_devs))
-		pdata->num_backlights = ARRAY_SIZE(lm3533_bl_devs);
+	device_for_each_child_node(dev, child) {
+		if (!fwnode_device_is_available(child))
+			continue;
 
-	for (i = 0; i < pdata->num_backlights; ++i) {
-		lm3533_bl_devs[i].platform_data = &pdata->backlights[i];
-		lm3533_bl_devs[i].pdata_size = sizeof(pdata->backlights[i]);
-	}
+		/* Fill up children flags used for attributes */
+		fwnode_property_read_string(child, "compatible", &comatible);
+		fwnode_property_read_u32(child, "reg", &id);
 
-	ret = mfd_add_devices(lm3533->dev, 0, lm3533_bl_devs,
-			      pdata->num_backlights, NULL, 0, NULL);
-	if (ret) {
-		dev_err(lm3533->dev, "failed to add backlight devices\n");
-		return ret;
-	}
+		if (!strcmp(comatible, "ti,lm3533-als"))
+			lm3533->have_als = 1;
 
-	lm3533->have_backlights = 1;
+		if (!strcmp(comatible, "ti,lm3533-backlight"))
+			lm3533->have_backlights = 1;
 
-	return 0;
-}
+		if (!strcmp(comatible, "ti,lm3533-leds"))
+			lm3533->have_leds = 1;
 
-static int lm3533_device_led_init(struct lm3533 *lm3533)
-{
-	struct lm3533_platform_data *pdata = dev_get_platdata(lm3533->dev);
-	int i;
-	int ret;
-
-	if (!pdata->leds || pdata->num_leds == 0)
-		return 0;
+		subdev_info.id = id;
+		subdev_info.fwnode = child;
+		subdev_info.name = fwnode_get_name(child);
+		subdev = platform_device_register_full(&subdev_info);
+		if (IS_ERR(subdev))
+			return dev_err_probe(dev, PTR_ERR(subdev),
+					     "register subdev for %s",
+					     subdev_info.name);
 
-	if (pdata->num_leds > ARRAY_SIZE(lm3533_led_devs))
-		pdata->num_leds = ARRAY_SIZE(lm3533_led_devs);
-
-	for (i = 0; i < pdata->num_leds; ++i) {
-		lm3533_led_devs[i].platform_data = &pdata->leds[i];
-		lm3533_led_devs[i].pdata_size = sizeof(pdata->leds[i]);
-	}
-
-	ret = mfd_add_devices(lm3533->dev, 0, lm3533_led_devs,
-			      pdata->num_leds, NULL, 0, NULL);
-	if (ret) {
-		dev_err(lm3533->dev, "failed to add LED devices\n");
-		return ret;
+		ret = devm_add_action_or_reset(dev, lm3533_remove_subdev, subdev);
+		if (ret)
+			return dev_err_probe(dev, ret,
+					     "register subdev cleanup action for %s",
+					     subdev_info.name);
 	}
 
-	lm3533->have_leds = 1;
-
 	return 0;
 }
 
-static int lm3533_device_setup(struct lm3533 *lm3533,
-					struct lm3533_platform_data *pdata)
+static int lm3533_device_init(struct lm3533 *lm3533)
 {
+	u32 val;
 	int ret;
 
-	ret = lm3533_set_boost_freq(lm3533, pdata->boost_freq);
-	if (ret)
-		return ret;
-
-	return lm3533_set_boost_ovp(lm3533, pdata->boost_ovp);
-}
+	lm3533->hwen = devm_gpiod_get(lm3533->dev, "enable", GPIOD_OUT_LOW);
+	if (IS_ERR(lm3533->hwen))
+		return dev_err_probe(lm3533->dev, PTR_ERR(lm3533->hwen),
+				     "failed to request HWEN GPIO\n");
 
-static int lm3533_device_init(struct lm3533 *lm3533)
-{
-	struct lm3533_platform_data *pdata = dev_get_platdata(lm3533->dev);
-	int ret;
+	val = 16 * MICRO; /* 16V */
+	device_property_read_u32(lm3533->dev, "ti,boost-ovp-microvolt", &val);
 
-	dev_dbg(lm3533->dev, "%s\n", __func__);
+	/* boost_ovp is defined in microvolts, convert to enum value */
+	lm3533->boost_ovp = val / (8 * MICRO) - 2;
 
-	if (!pdata) {
-		dev_err(lm3533->dev, "no platform data\n");
-		return -EINVAL;
-	}
+	val = 500 * KILO; /* 500kHz */
+	device_property_read_u32(lm3533->dev, "ti,boost-freq-hz", &val);
 
-	lm3533->hwen = devm_gpiod_get(lm3533->dev, NULL, GPIOD_OUT_LOW);
-	if (IS_ERR(lm3533->hwen))
-		return dev_err_probe(lm3533->dev, PTR_ERR(lm3533->hwen), "failed to request HWEN GPIO\n");
-	gpiod_set_consumer_name(lm3533->hwen, "lm3533-hwen");
+	/* boost_freq is defined in Hz, convert to enum value */
+	lm3533->boost_freq = val / (500 * KILO) - 1;
 
 	lm3533_enable(lm3533);
 
-	ret = lm3533_device_setup(lm3533, pdata);
+	ret = lm3533_device_setup(lm3533);
 	if (ret)
 		goto err_disable;
 
-	lm3533_device_als_init(lm3533);
-	lm3533_device_bl_init(lm3533);
-	lm3533_device_led_init(lm3533);
+	ret = lm3533_init_components(lm3533);
+	if (ret) {
+		dev_err(lm3533->dev, "failed to init components\n");
+		goto err_disable;
+	}
 
 	ret = sysfs_create_group(&lm3533->dev->kobj, &lm3533_attribute_group);
 	if (ret < 0) {
 		dev_err(lm3533->dev, "failed to create sysfs attributes\n");
-		goto err_unregister;
+		goto err_disable;
 	}
 
 	return 0;
 
-err_unregister:
-	mfd_remove_devices(lm3533->dev);
 err_disable:
 	lm3533_disable(lm3533);
 
@@ -517,8 +459,6 @@ static void lm3533_device_exit(struct lm3533 *lm3533)
 	dev_dbg(lm3533->dev, "%s\n", __func__);
 
 	sysfs_remove_group(&lm3533->dev->kobj, &lm3533_attribute_group);
-
-	mfd_remove_devices(lm3533->dev);
 	lm3533_disable(lm3533);
 }
 
@@ -603,6 +543,12 @@ static void lm3533_i2c_remove(struct i2c_client *i2c)
 	lm3533_device_exit(lm3533);
 }
 
+static const struct of_device_id lm3533_match_table[] = {
+	{ .compatible = "ti,lm3533" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, lm3533_match_table);
+
 static const struct i2c_device_id lm3533_i2c_ids[] = {
 	{ "lm3533" },
 	{ }
@@ -612,6 +558,7 @@ MODULE_DEVICE_TABLE(i2c, lm3533_i2c_ids);
 static struct i2c_driver lm3533_i2c_driver = {
 	.driver = {
 		   .name = "lm3533",
+		   .of_match_table = lm3533_match_table,
 	},
 	.id_table	= lm3533_i2c_ids,
 	.probe		= lm3533_i2c_probe,
diff --git a/drivers/video/backlight/lm3533_bl.c b/drivers/video/backlight/lm3533_bl.c
index babfd3ceec86..38278bfa228b 100644
--- a/drivers/video/backlight/lm3533_bl.c
+++ b/drivers/video/backlight/lm3533_bl.c
@@ -9,7 +9,9 @@
 
 #include <linux/module.h>
 #include <linux/init.h>
+#include <linux/mod_devicetable.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 #include <linux/backlight.h>
 #include <linux/slab.h>
 
@@ -19,6 +21,7 @@
 #define LM3533_HVCTRLBANK_COUNT		2
 #define LM3533_BL_MAX_BRIGHTNESS	255
 
+#define LM3533_REG_OUTPUT_CONF1		0x10
 #define LM3533_REG_CTRLBANK_AB_BCONF	0x1a
 
 
@@ -27,6 +30,11 @@ struct lm3533_bl {
 	struct lm3533_ctrlbank cb;
 	struct backlight_device *bd;
 	int id;
+
+	u16 max_current;		/* 5000 - 29800 uA (800 uA step) */
+	u8 pwm;				/* 0 - 0x3f */
+	bool linear;
+	bool hvled;
 };
 
 
@@ -246,25 +254,39 @@ static struct attribute_group lm3533_bl_attribute_group = {
 	.attrs		= lm3533_bl_attributes
 };
 
-static int lm3533_bl_setup(struct lm3533_bl *bl,
-					struct lm3533_bl_platform_data *pdata)
+static int lm3533_bl_setup(struct lm3533_bl *bl)
 {
+	int id = lm3533_bl_get_ctrlbank_id(bl);
 	int ret;
 
-	ret = lm3533_ctrlbank_set_max_current(&bl->cb, pdata->max_current);
+	if (bl->linear) {
+		ret = lm3533_update(bl->lm3533, LM3533_REG_CTRLBANK_AB_BCONF,
+				    1 << (2 * id + 1), 1 << (2 * id + 1));
+		if (ret)
+			return ret;
+	}
+
+	if (bl->hvled) {
+		ret = lm3533_update(bl->lm3533, LM3533_REG_OUTPUT_CONF1,
+				    id | id << 1, BIT(0) | BIT(1));
+		if (ret)
+			return ret;
+	}
+
+	ret = lm3533_ctrlbank_set_max_current(&bl->cb, bl->max_current);
 	if (ret)
 		return ret;
 
-	return lm3533_ctrlbank_set_pwm(&bl->cb, pdata->pwm);
+	return lm3533_ctrlbank_set_pwm(&bl->cb, bl->pwm);
 }
 
 static int lm3533_bl_probe(struct platform_device *pdev)
 {
 	struct lm3533 *lm3533;
-	struct lm3533_bl_platform_data *pdata;
 	struct lm3533_bl *bl;
 	struct backlight_device *bd;
 	struct backlight_properties props;
+	u32 val;
 	int ret;
 
 	dev_dbg(&pdev->dev, "%s\n", __func__);
@@ -273,12 +295,6 @@ static int lm3533_bl_probe(struct platform_device *pdev)
 	if (!lm3533)
 		return -EINVAL;
 
-	pdata = dev_get_platdata(&pdev->dev);
-	if (!pdata) {
-		dev_err(&pdev->dev, "no platform data\n");
-		return -EINVAL;
-	}
-
 	if (pdev->id < 0 || pdev->id >= LM3533_HVCTRLBANK_COUNT) {
 		dev_err(&pdev->dev, "illegal backlight id %d\n", pdev->id);
 		return -EINVAL;
@@ -298,10 +314,10 @@ static int lm3533_bl_probe(struct platform_device *pdev)
 	memset(&props, 0, sizeof(props));
 	props.type = BACKLIGHT_RAW;
 	props.max_brightness = LM3533_BL_MAX_BRIGHTNESS;
-	props.brightness = pdata->default_brightness;
-	bd = devm_backlight_device_register(&pdev->dev, pdata->name,
-					pdev->dev.parent, bl, &lm3533_bl_ops,
-					&props);
+	props.brightness = LM3533_BL_MAX_BRIGHTNESS;
+	device_property_read_u32(&pdev->dev, "default-brightness", &props.brightness);
+	bd = devm_backlight_device_register(&pdev->dev, pdev->name, pdev->dev.parent,
+					    bl, &lm3533_bl_ops, &props);
 	if (IS_ERR(bd)) {
 		dev_err(&pdev->dev, "failed to register backlight device\n");
 		return PTR_ERR(bd);
@@ -320,7 +336,20 @@ static int lm3533_bl_probe(struct platform_device *pdev)
 
 	backlight_update_status(bd);
 
-	ret = lm3533_bl_setup(bl, pdata);
+	/* 5000 - 29800 uA (800 uA step) */
+	val = 5000;
+	device_property_read_u32(&pdev->dev, "ti,max-current-microamp", &val);
+	bl->max_current = val;
+
+	/* 0 - 0x3f */
+	val = 0;
+	device_property_read_u32(&pdev->dev, "ti,pwm-config-mask", &val);
+	bl->pwm = val;
+
+	bl->linear = device_property_read_bool(&pdev->dev, "ti,linear-mapping-mode");
+	bl->hvled = device_property_read_bool(&pdev->dev, "ti,hardware-controlled");
+
+	ret = lm3533_bl_setup(bl);
 	if (ret)
 		goto err_sysfs_remove;
 
@@ -381,10 +410,17 @@ static void lm3533_bl_shutdown(struct platform_device *pdev)
 	lm3533_ctrlbank_disable(&bl->cb);
 }
 
+static const struct of_device_id lm3533_bl_match_table[] = {
+	{ .compatible = "ti,lm3533-backlight" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, lm3533_match_table);
+
 static struct platform_driver lm3533_bl_driver = {
 	.driver = {
 		.name	= "lm3533-backlight",
 		.pm	= &lm3533_bl_pm_ops,
+		.of_match_table = lm3533_bl_match_table,
 	},
 	.probe		= lm3533_bl_probe,
 	.remove		= lm3533_bl_remove,
diff --git a/include/linux/mfd/lm3533.h b/include/linux/mfd/lm3533.h
index 69059a7a2ce5..3b28fc0970f6 100644
--- a/include/linux/mfd/lm3533.h
+++ b/include/linux/mfd/lm3533.h
@@ -27,6 +27,9 @@ struct lm3533 {
 	struct gpio_desc *hwen;
 	int irq;
 
+	u32 boost_ovp;
+	u32 boost_freq;
+
 	unsigned have_als:1;
 	unsigned have_backlights:1;
 	unsigned have_leds:1;
@@ -38,25 +41,6 @@ struct lm3533_ctrlbank {
 	int id;
 };
 
-struct lm3533_als_platform_data {
-	unsigned pwm_mode:1;		/* PWM input mode (default analog) */
-	u8 r_select;			/* 1 - 127 (ignored in PWM-mode) */
-};
-
-struct lm3533_bl_platform_data {
-	char *name;
-	u16 max_current;		/* 5000 - 29800 uA (800 uA step) */
-	u8 default_brightness;		/* 0 - 255 */
-	u8 pwm;				/* 0 - 0x3f */
-};
-
-struct lm3533_led_platform_data {
-	char *name;
-	const char *default_trigger;
-	u16 max_current;		/* 5000 - 29800 uA (800 uA step) */
-	u8 pwm;				/* 0 - 0x3f */
-};
-
 enum lm3533_boost_freq {
 	LM3533_BOOST_FREQ_500KHZ,
 	LM3533_BOOST_FREQ_1000KHZ,
@@ -69,19 +53,6 @@ enum lm3533_boost_ovp {
 	LM3533_BOOST_OVP_40V,
 };
 
-struct lm3533_platform_data {
-	enum lm3533_boost_ovp boost_ovp;
-	enum lm3533_boost_freq boost_freq;
-
-	struct lm3533_als_platform_data *als;
-
-	struct lm3533_bl_platform_data *backlights;
-	int num_backlights;
-
-	struct lm3533_led_platform_data *leds;
-	int num_leds;
-};
-
 extern int lm3533_ctrlbank_enable(struct lm3533_ctrlbank *cb);
 extern int lm3533_ctrlbank_disable(struct lm3533_ctrlbank *cb);
 
-- 
2.43.0

Re: [PATCH v2 2/2] mfd: lm3533: convert to use OF

[Andy Shevchenko]

On Tue, Feb 18, 2025 at 03:27:00PM +0200, Svyatoslav Ryhel wrote:
> Remove platform data and fully relay on OF and device tree
> parsing and binding devices.

Thanks for following the advice, but the problem with this change as it does
too much at once. It should be split to a few simpler ones.
On top of that, this removes MFD participation from the driver but leaves it
under MFD realm. Moreover, looking briefly at the code it looks like it open
codes the parts of MFD. The latter needs a very goo justification which commit
message is missing.

...

> +static const struct of_device_id lm3533_als_match_table[] = {
> +	{ .compatible = "ti,lm3533-als" },
> +	{ },

No comma for the terminator entry. I think I already pointed that out earlier.

> +};

...

> +	device_property_read_string(&pdev->dev, "linux,default-trigger",
> +				    &led->cdev.default_trigger);

One prerequisite patch you probably want is an introduction of

	struct device *dev = &pdev->dev;

in the respective ->probe() implementations. This, in particular, makes the
above lines shorter and fit one line.

...

> +static const struct of_device_id lm3533_led_match_table[] = {
> +	{ .compatible = "ti,lm3533-leds" },
> +	{ },

As per above.

> +};

...

> +		if (!strcmp(comatible, "ti,lm3533-als"))
> +			lm3533->have_als = 1;

If you end up having this, it's not the best what we can do. OF ID tables have
a driver_data field exactly for the cases like this.

...

> +		if (!strcmp(comatible, "ti,lm3533-backlight"))
> +			lm3533->have_backlights = 1;

Ditto.

...

> +		if (!strcmp(comatible, "ti,lm3533-leds"))
> +			lm3533->have_leds = 1;

Ditto.

...

> +		ret = lm3533_update(bl->lm3533, LM3533_REG_CTRLBANK_AB_BCONF,
> +				    1 << (2 * id + 1), 1 << (2 * id + 1));

BIT() and better to use a temporary variable for this calculation.

> +		if (ret)
> +			return ret;

...

> +		ret = lm3533_update(bl->lm3533, LM3533_REG_OUTPUT_CONF1,
> +				    id | id << 1, BIT(0) | BIT(1));

		mask = GENMASK();
		..., id ? mask : 0, mask);

> +		if (ret)
> +			return ret;
> +	}

...

> +	bd = devm_backlight_device_register(&pdev->dev, pdev->name, pdev->dev.parent,
> +					    bl, &lm3533_bl_ops, &props);


With the advice from above:

	bd = devm_backlight_device_register(dev, pdev->name, dev->parent, bl, &lm3533_bl_ops,
					    &props);


>  	if (IS_ERR(bd)) {
>  		dev_err(&pdev->dev, "failed to register backlight device\n");
>  		return PTR_ERR(bd);

Consider another prerequisite patch (which should come before the firstly
proposed one):

	struct device *dev = &pdev->dev; // yes, this can go in this change
	...

	if (IS_ERR(bd))
		return dev_err_probe(dev, PTR_ERR(bd), "failed to register backlight device\n");

...

> +static const struct of_device_id lm3533_bl_match_table[] = {
> +	{ .compatible = "ti,lm3533-backlight" },
> +	{ },

As per above.

> +};

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/2] mfd: lm3533: convert to use OF

[Svyatoslav Ryhel]

ср, 19 лют. 2025 р. о 16:27 Andy Shevchenko
<andriy.shevchenko@linux.intel.com> пише:
>
> On Tue, Feb 18, 2025 at 03:27:00PM +0200, Svyatoslav Ryhel wrote:
> > Remove platform data and fully relay on OF and device tree
> > parsing and binding devices.
>
> Thanks for following the advice, but the problem with this change as it does
> too much at once. It should be split to a few simpler ones.
> On top of that, this removes MFD participation from the driver but leaves it
> under MFD realm. Moreover, looking briefly at the code it looks like it open
> codes the parts of MFD. The latter needs a very goo justification which commit
> message is missing.
>

Splitting this into a set of commits would be nearly impossible,
original driver does not relay on OF, it relays on platform data.
Ripping out platform data will leave behind a broken useless driver.
So it has to be done simultaneously.

MFD part is removed since MFD cells binding is unconditional, while
the device supports any amount of children grater then one. For
example, my  device uses only backlight at bank A, while all other
subdevices are not present and used. This patch switches to dynamic
bind of children.

> ...
>
> > +static const struct of_device_id lm3533_als_match_table[] = {
> > +     { .compatible = "ti,lm3533-als" },
> > +     { },
>
> No comma for the terminator entry. I think I already pointed that out earlier.
>
> > +};
>
> ...
>
> > +     device_property_read_string(&pdev->dev, "linux,default-trigger",
> > +                                 &led->cdev.default_trigger);
>
> One prerequisite patch you probably want is an introduction of
>
>         struct device *dev = &pdev->dev;
>
> in the respective ->probe() implementations. This, in particular, makes the
> above lines shorter and fit one line.
>

This is not a scope of this patchset. Original driver uses &pdev->dev

> ...
>
> > +static const struct of_device_id lm3533_led_match_table[] = {
> > +     { .compatible = "ti,lm3533-leds" },
> > +     { },
>
> As per above.
>
> > +};
>
> ...
>
> > +             if (!strcmp(comatible, "ti,lm3533-als"))
> > +                     lm3533->have_als = 1;
>
> If you end up having this, it's not the best what we can do. OF ID tables have
> a driver_data field exactly for the cases like this.
>

This is required by core driver to handle some attributes and is here
solely not to touch those in this patch.

> ...
>
> > +             if (!strcmp(comatible, "ti,lm3533-backlight"))
> > +                     lm3533->have_backlights = 1;
>
> Ditto.
>
> ...
>
> > +             if (!strcmp(comatible, "ti,lm3533-leds"))
> > +                     lm3533->have_leds = 1;
>
> Ditto.
>
> ...
>
> > +             ret = lm3533_update(bl->lm3533, LM3533_REG_CTRLBANK_AB_BCONF,
> > +                                 1 << (2 * id + 1), 1 << (2 * id + 1));
>
> BIT() and better to use a temporary variable for this calculation.
>
> > +             if (ret)
> > +                     return ret;
>
> ...
>
> > +             ret = lm3533_update(bl->lm3533, LM3533_REG_OUTPUT_CONF1,
> > +                                 id | id << 1, BIT(0) | BIT(1));
>
>                 mask = GENMASK();
>                 ..., id ? mask : 0, mask);
>
> > +             if (ret)
> > +                     return ret;
> > +     }
>
> ...
>
> > +     bd = devm_backlight_device_register(&pdev->dev, pdev->name, pdev->dev.parent,
> > +                                         bl, &lm3533_bl_ops, &props);
>
>
> With the advice from above:
>
>         bd = devm_backlight_device_register(dev, pdev->name, dev->parent, bl, &lm3533_bl_ops,
>                                             &props);
>
>
> >       if (IS_ERR(bd)) {
> >               dev_err(&pdev->dev, "failed to register backlight device\n");
> >               return PTR_ERR(bd);
>
> Consider another prerequisite patch (which should come before the firstly
> proposed one):
>
>         struct device *dev = &pdev->dev; // yes, this can go in this change
>         ...
>
>         if (IS_ERR(bd))
>                 return dev_err_probe(dev, PTR_ERR(bd), "failed to register backlight device\n");
>
> ...
>
> > +static const struct of_device_id lm3533_bl_match_table[] = {
> > +     { .compatible = "ti,lm3533-backlight" },
> > +     { },
>
> As per above.
>
> > +};
>
> --
> With Best Regards,
> Andy Shevchenko
>
>

Re: [PATCH v2 2/2] mfd: lm3533: convert to use OF

[Andy Shevchenko]

On Wed, Feb 19, 2025 at 04:36:38PM +0200, Svyatoslav Ryhel wrote:
> ср, 19 лют. 2025 р. о 16:27 Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> пише:
> > On Tue, Feb 18, 2025 at 03:27:00PM +0200, Svyatoslav Ryhel wrote:
> > > Remove platform data and fully relay on OF and device tree
> > > parsing and binding devices.
> >
> > Thanks for following the advice, but the problem with this change as it does
> > too much at once. It should be split to a few simpler ones.
> > On top of that, this removes MFD participation from the driver but leaves it
> > under MFD realm. Moreover, looking briefly at the code it looks like it open
> > codes the parts of MFD. The latter needs a very goo justification which commit
> > message is missing.

...

> Splitting this into a set of commits would be nearly impossible,

I don't buy this.
One patch can introduce device property support.
Another one removes the old platform data interface.

So, at bare minimum there will be two patches. (Besides the advice to have
two more.)

> original driver does not relay on OF, it relays on platform data.

And?..

> Ripping out platform data will leave behind a broken useless driver.

Hmm... This cna be the case if and only if we have the user in kernel.
Is this the case?

> So it has to be done simultaneously.

Nope.

> MFD part is removed since MFD cells binding is unconditional, while
> the device supports any amount of children grater then one. For
> example, my  device uses only backlight at bank A, while all other
> subdevices are not present and used. This patch switches to dynamic
> bind of children.

MFD does the same. Please, take your time and get familiar with how MFD works.

...

> > > +     device_property_read_string(&pdev->dev, "linux,default-trigger",
> > > +                                 &led->cdev.default_trigger);
> >
> > One prerequisite patch you probably want is an introduction of
> >
> >         struct device *dev = &pdev->dev;
> >
> > in the respective ->probe() implementations. This, in particular, makes the
> > above lines shorter and fit one line.
> 
> This is not a scope of this patchset. Original driver uses &pdev->dev

Indirectly it is. The change you are proposing tries to continue using this
construction with making needlessly longer.

...

> > > +             if (!strcmp(comatible, "ti,lm3533-als"))
> > > +                     lm3533->have_als = 1;
> >
> > If you end up having this, it's not the best what we can do. OF ID tables have
> > a driver_data field exactly for the cases like this.
> 
> This is required by core driver to handle some attributes and is here
> solely not to touch those in this patch.

What does this mean?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/2] mfd: lm3533: convert to use OF

[Svyatoslav Ryhel]

ср, 19 лют. 2025 р. о 17:07 Andy Shevchenko
<andriy.shevchenko@linux.intel.com> пише:
>
> On Wed, Feb 19, 2025 at 04:36:38PM +0200, Svyatoslav Ryhel wrote:
> > ср, 19 лют. 2025 р. о 16:27 Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> пише:
> > > On Tue, Feb 18, 2025 at 03:27:00PM +0200, Svyatoslav Ryhel wrote:
> > > > Remove platform data and fully relay on OF and device tree
> > > > parsing and binding devices.
> > >
> > > Thanks for following the advice, but the problem with this change as it does
> > > too much at once. It should be split to a few simpler ones.
> > > On top of that, this removes MFD participation from the driver but leaves it
> > > under MFD realm. Moreover, looking briefly at the code it looks like it open
> > > codes the parts of MFD. The latter needs a very goo justification which commit
> > > message is missing.
>
> ...
>
> > Splitting this into a set of commits would be nearly impossible,
>
> I don't buy this.
> One patch can introduce device property support.
> Another one removes the old platform data interface.
>
> So, at bare minimum there will be two patches. (Besides the advice to have
> two more.)
>
> > original driver does not relay on OF, it relays on platform data.
>
> And?..
>
> > Ripping out platform data will leave behind a broken useless driver.
>
> Hmm... This cna be the case if and only if we have the user in kernel.
> Is this the case?
>
> > So it has to be done simultaneously.
>
> Nope.
>
> > MFD part is removed since MFD cells binding is unconditional, while
> > the device supports any amount of children grater then one. For
> > example, my  device uses only backlight at bank A, while all other
> > subdevices are not present and used. This patch switches to dynamic
> > bind of children.
>
> MFD does the same. Please, take your time and get familiar with how MFD works.
>

It does not. I have tried. If mfd cell binding is missing, driver will
complain and fail.

> ...
>
> > > > +     device_property_read_string(&pdev->dev, "linux,default-trigger",
> > > > +                                 &led->cdev.default_trigger);
> > >
> > > One prerequisite patch you probably want is an introduction of
> > >
> > >         struct device *dev = &pdev->dev;
> > >
> > > in the respective ->probe() implementations. This, in particular, makes the
> > > above lines shorter and fit one line.
> >
> > This is not a scope of this patchset. Original driver uses &pdev->dev
>
> Indirectly it is. The change you are proposing tries to continue using this
> construction with making needlessly longer.
>
> ...
>
> > > > +             if (!strcmp(comatible, "ti,lm3533-als"))
> > > > +                     lm3533->have_als = 1;
> > >
> > > If you end up having this, it's not the best what we can do. OF ID tables have
> > > a driver_data field exactly for the cases like this.
> >
> > This is required by core driver to handle some attributes and is here
> > solely not to touch those in this patch.
>
> What does this mean?
>
> --
> With Best Regards,
> Andy Shevchenko
>
>

Let this driver rot for now, I might return to it. At some point

Re: [PATCH v2 2/2] mfd: lm3533: convert to use OF

[Andy Shevchenko]

On Wed, Feb 19, 2025 at 05:13:15PM +0200, Svyatoslav Ryhel wrote:
> ср, 19 лют. 2025 р. о 17:07 Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> пише:
> > On Wed, Feb 19, 2025 at 04:36:38PM +0200, Svyatoslav Ryhel wrote:
> > > ср, 19 лют. 2025 р. о 16:27 Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> пише:

...

> > > MFD part is removed since MFD cells binding is unconditional, while
> > > the device supports any amount of children grater then one. For
> > > example, my  device uses only backlight at bank A, while all other
> > > subdevices are not present and used. This patch switches to dynamic
> > > bind of children.
> >
> > MFD does the same. Please, take your time and get familiar with how MFD works.
> 
> It does not. I have tried. If mfd cell binding is missing, driver will
> complain and fail.

I really don't know what exactly is going on here, you can check it later, but
I'm 100% sure that MFD works for only driver that are present. What you are
describing is how component framework is designed.

To proove my words you can check drivers/mfd/intel_soc_pmic_*.c and find listed
cells that never had a driver in the Linux kernel. They are just placeholders.

...

> > --
> > With Best Regards,
> > Andy Shevchenko

Please, when answering, remove unrelated context from the replies.

...

> Let this driver rot for now, I might return to it. At some point

Up to you. But thanks for trying!

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/2] mfd: lm3533: convert to use OF

[kernel test robot]

Hi Svyatoslav,

kernel test robot noticed the following build errors:

[auto build test ERROR on lee-mfd/for-mfd-next]
[also build test ERROR on lee-leds/for-leds-next robh/for-next linus/master v6.14-rc3 next-20250219]
[cannot apply to lee-mfd/for-mfd-fixes]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Svyatoslav-Ryhel/dt-bindings-mfd-Document-TI-LM3533-MFD/20250218-212857
base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
patch link:    https://lore.kernel.org/r/20250218132702.114669-3-clamor95%40gmail.com
patch subject: [PATCH v2 2/2] mfd: lm3533: convert to use OF
config: i386-buildonly-randconfig-003-20250219 (https://download.01.org/0day-ci/archive/20250219/202502192343.twEQ3SSs-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250219/202502192343.twEQ3SSs-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202502192343.twEQ3SSs-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from include/linux/device/driver.h:21,
                    from include/linux/device.h:32,
                    from include/linux/iio/iio.h:10,
                    from drivers/iio/light/lm3533-als.c:15:
>> drivers/iio/light/lm3533-als.c:921:25: error: 'lm3533_match_table' undeclared here (not in a function); did you mean 'lm3533_als_match_table'?
     921 | MODULE_DEVICE_TABLE(of, lm3533_match_table);
         |                         ^~~~~~~~~~~~~~~~~~
   include/linux/module.h:250:15: note: in definition of macro 'MODULE_DEVICE_TABLE'
     250 | extern typeof(name) __mod_device_table__##type##__##name                \
         |               ^~~~
   include/linux/module.h:250:21: error: '__mod_device_table__of__lm3533_match_table' aliased to undefined symbol 'lm3533_match_table'
     250 | extern typeof(name) __mod_device_table__##type##__##name                \
         |                     ^~~~~~~~~~~~~~~~~~~~
   drivers/iio/light/lm3533-als.c:921:1: note: in expansion of macro 'MODULE_DEVICE_TABLE'
     921 | MODULE_DEVICE_TABLE(of, lm3533_match_table);
         | ^~~~~~~~~~~~~~~~~~~


vim +921 drivers/iio/light/lm3533-als.c

   916	
   917	static const struct of_device_id lm3533_als_match_table[] = {
   918		{ .compatible = "ti,lm3533-als" },
   919		{ },
   920	};
 > 921	MODULE_DEVICE_TABLE(of, lm3533_match_table);
   922	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 2/2] mfd: lm3533: convert to use OF

[Svyatoslav Ryhel]

ср, 19 лют. 2025 р. о 17:56 Andy Shevchenko
<andriy.shevchenko@linux.intel.com> пише:
>
> On Wed, Feb 19, 2025 at 05:13:15PM +0200, Svyatoslav Ryhel wrote:
> > ср, 19 лют. 2025 р. о 17:07 Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> пише:
> > > On Wed, Feb 19, 2025 at 04:36:38PM +0200, Svyatoslav Ryhel wrote:
> > > > ср, 19 лют. 2025 р. о 16:27 Andy Shevchenko
> > > > <andriy.shevchenko@linux.intel.com> пише:
>
> ...
>
> > > > MFD part is removed since MFD cells binding is unconditional, while
> > > > the device supports any amount of children grater then one. For
> > > > example, my  device uses only backlight at bank A, while all other
> > > > subdevices are not present and used. This patch switches to dynamic
> > > > bind of children.
> > >
> > > MFD does the same. Please, take your time and get familiar with how MFD works.
> >
> > It does not. I have tried. If mfd cell binding is missing, driver will
> > complain and fail.
>
> I really don't know what exactly is going on here, you can check it later, but
> I'm 100% sure that MFD works for only driver that are present. What you are
> describing is how component framework is designed.
>
> To proove my words you can check drivers/mfd/intel_soc_pmic_*.c and find listed
> cells that never had a driver in the Linux kernel. They are just placeholders.
>

This example is not valid since those drivers do not use device tree.

> ...
>
> > > --
> > > With Best Regards,
> > > Andy Shevchenko
>
> Please, when answering, remove unrelated context from the replies.
>
> ...
>
> > Let this driver rot for now, I might return to it. At some point
>
> Up to you. But thanks for trying!
>

Thank you for suggestions. Once I complete more urgent tasks, I might
do some tinkering with this driver.

> --
> With Best Regards,
> Andy Shevchenko
>
>

Re: [PATCH v2 2/2] mfd: lm3533: convert to use OF

[Andy Shevchenko]

On Wed, Feb 19, 2025 at 07:39:29PM +0200, Svyatoslav Ryhel wrote:
> ср, 19 лют. 2025 р. о 17:56 Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> пише:
> > On Wed, Feb 19, 2025 at 05:13:15PM +0200, Svyatoslav Ryhel wrote:
> > > ср, 19 лют. 2025 р. о 17:07 Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> пише:
> > > > On Wed, Feb 19, 2025 at 04:36:38PM +0200, Svyatoslav Ryhel wrote:
> > > > > ср, 19 лют. 2025 р. о 16:27 Andy Shevchenko
> > > > > <andriy.shevchenko@linux.intel.com> пише:

...

> > > > > MFD part is removed since MFD cells binding is unconditional, while
> > > > > the device supports any amount of children grater then one. For
> > > > > example, my  device uses only backlight at bank A, while all other
> > > > > subdevices are not present and used. This patch switches to dynamic
> > > > > bind of children.
> > > >
> > > > MFD does the same. Please, take your time and get familiar with how MFD works.
> > >
> > > It does not. I have tried. If mfd cell binding is missing, driver will
> > > complain and fail.
> >
> > I really don't know what exactly is going on here, you can check it later, but
> > I'm 100% sure that MFD works for only driver that are present. What you are
> > describing is how component framework is designed.
> >
> > To proove my words you can check drivers/mfd/intel_soc_pmic_*.c and find listed
> > cells that never had a driver in the Linux kernel. They are just placeholders.
> 
> This example is not valid since those drivers do not use device tree.

You didn't get the point. I'm telling that it should not matter if there is a
device driver present or absent. The rest will be enumerated as usual.

I even checked the code that handles of_compatible member from struct mfd_cell and
at a brief glance I do not see that it can do otherwise.

Care to elaborate what and where is the error happened exactly?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/2] mfd: lm3533: convert to use OF

[Andy Shevchenko]

On Thu, Feb 20, 2025 at 12:02:51AM +0800, kernel test robot wrote:
> Hi Svyatoslav,
> 
> kernel test robot noticed the following build errors:

It a second time you send not even compiled code.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/2] mfd: lm3533: convert to use OF

[kernel test robot]

Hi Svyatoslav,

kernel test robot noticed the following build errors:

[auto build test ERROR on lee-mfd/for-mfd-next]
[also build test ERROR on lee-leds/for-leds-next robh/for-next linus/master v6.14-rc3 next-20250219]
[cannot apply to lee-mfd/for-mfd-fixes]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Svyatoslav-Ryhel/dt-bindings-mfd-Document-TI-LM3533-MFD/20250218-212857
base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
patch link:    https://lore.kernel.org/r/20250218132702.114669-3-clamor95%40gmail.com
patch subject: [PATCH v2 2/2] mfd: lm3533: convert to use OF
config: arm-randconfig-002-20250219 (https://download.01.org/0day-ci/archive/20250220/202502200718.H8t6Uv7b-lkp@intel.com/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250220/202502200718.H8t6Uv7b-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202502200718.H8t6Uv7b-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/video/backlight/lm3533_bl.c:417:25: error: use of undeclared identifier 'lm3533_match_table'; did you mean 'lm3533_bl_match_table'?
     417 | MODULE_DEVICE_TABLE(of, lm3533_match_table);
         |                         ^~~~~~~~~~~~~~~~~~
         |                         lm3533_bl_match_table
   include/linux/module.h:250:15: note: expanded from macro 'MODULE_DEVICE_TABLE'
     250 | extern typeof(name) __mod_device_table__##type##__##name                \
         |               ^
   drivers/video/backlight/lm3533_bl.c:413:34: note: 'lm3533_bl_match_table' declared here
     413 | static const struct of_device_id lm3533_bl_match_table[] = {
         |                                  ^
   1 error generated.


vim +417 drivers/video/backlight/lm3533_bl.c

   412	
   413	static const struct of_device_id lm3533_bl_match_table[] = {
   414		{ .compatible = "ti,lm3533-backlight" },
   415		{ },
   416	};
 > 417	MODULE_DEVICE_TABLE(of, lm3533_match_table);
   418	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 1/2] dt-bindings: mfd: Document TI LM3533 MFD

[Rob Herring]

On Tue, Feb 18, 2025 at 03:26:59PM +0200, Svyatoslav Ryhel wrote:
> Add bindings for the LM3533 - a complete power source for
> backlight, keypad, and indicator LEDs in smartphone handsets.
> The high-voltage inductive boost converter provides the
> power for two series LED strings display backlight and keypad
> functions.
> 
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
>  .../devicetree/bindings/mfd/ti,lm3533.yaml    | 231 ++++++++++++++++++
>  1 file changed, 231 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/ti,lm3533.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mfd/ti,lm3533.yaml b/Documentation/devicetree/bindings/mfd/ti,lm3533.yaml
> new file mode 100644
> index 000000000000..83542f0c7bf7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/ti,lm3533.yaml
> @@ -0,0 +1,231 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/ti,lm3533.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TI LM3533 Complete Lighting Power Solution
> +
> +description: |

Use '>' rather than '|' if only formatting is paragraphs.

> +  The LM3533 is a complete power source for backlight,
> +  keypad, and indicator LEDs in smartphone handsets. The
> +  high-voltage inductive boost converter provides the
> +  power for two series LED strings display backlight and
> +  keypad functions.

Wrap lines at 80

blank line here

> +  https://www.ti.com/product/LM3533
> +
> +maintainers:
> +  - Svyatoslav Ryhel <clamor95@gmail.com>
> +
> +properties:
> +  compatible:
> +    const: ti,lm3533
> +
> +  reg:
> +    maxItems: 1
> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 0
> +
> +  enable-gpios:
> +    description: GPIO to use to enable/disable the backlight (HWEN pin).
> +    maxItems: 1
> +
> +  ti,boost-ovp-microvolt:
> +    description:
> +      Boost OVP select (16V, 24V, 32V, 40V)
> +    enum: [ 16000000, 24000000, 32000000, 40000000 ]
> +    default: 16000000
> +
> +  ti,boost-freq-hz:
> +    description:
> +      Boost frequency select (500KHz or 1MHz)
> +    enum: [ 500000, 1000000 ]
> +    default: 500000
> +
> +  light-sensor@0:
> +    type: object
> +    description:
> +      Properties for an illumination sensor.
> +
> +    properties:
> +      compatible:
> +        const: ti,lm3533-als
> +
> +      reg:
> +        const: 0
> +
> +      ti,resistor-value-ohm:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description: |

Don't need '|'. Elsewhere too.

> +          Internal configuration resister value when ALS is in Analog Sensor
> +          mode and PWM mode is disabled.
> +        minimum: 1575
> +        maximum: 200000
> +
> +      ti,pwm-mode:
> +        type: boolean
> +        description: |
> +          Switch for mode in which ALS is running. If this propertly is
> +          set then ALS is running in PWM mode, internal resistor value is
> +          set to high-impedance (0) and resistor-value-ohm propertly is
> +          ignored.
> +
> +    required:
> +      - compatible
> +      - reg
> +
> +    additionalProperties: false

Move this above 'properties'.

> +
> +required:
> +  - compatible
> +  - reg
> +  - '#address-cells'
> +  - '#size-cells'
> +  - enable-gpios
> +
> +patternProperties:
> +  "^backlight@[01]$":
> +    type: object
> +    description:
> +      Properties for a backlight device.
> +
> +    $ref: /schemas/leds/backlight/common.yaml#
> +
> +    properties:
> +      compatible:
> +        const: ti,lm3533-backlight
> +
> +      reg:
> +        description: |
> +          The control bank that is used to program the two current sinks. The
> +          LM3533 has two control banks (A and B) and are represented as 0 or 1
> +          in this property. The two current sinks can be controlled
> +          independently with both banks, or bank A can be configured to control
> +          both sinks with the led-sources property.
> +        minimum: 0
> +        maximum: 1
> +
> +      default-brightness: true
> +
> +      ti,max-current-microamp:
> +        description:
> +          Maximum current in µA with a 800 µA step.
> +        enum: [ 5000, 5800, 6600, 7400, 8200, 9000, 9800,
> +                10600, 11400, 12200, 13000, 13800, 14600,
> +                15400, 16200, 17000, 17800, 18600, 19400,
> +                20200, 21000, 21800, 22600, 23400, 24200,
> +                25000, 25800, 26600, 27400, 28200, 29000,
> +                29800 ]
> +        default: 5000
> +
> +      ti,pwm-config-mask:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description: |
> +          Control Bank PWM Configuration Register mask that allows to configure
> +          PWM input in Zones 0-4
> +          BIT(0) - PWM Input is enabled
> +          BIT(1) - PWM Input is enabled in Zone 0
> +          BIT(2) - PWM Input is enabled in Zone 1
> +          BIT(3) - PWM Input is enabled in Zone 2
> +          BIT(4) - PWM Input is enabled in Zone 3
> +          BIT(5) - PWM Input is enabled in Zone 4
> +
> +      ti,linear-mapping-mode:
> +        description: |
> +          Enable linear mapping mode. If disabled, then it will use exponential
> +          mapping mode in which the ramp up/down appears to have a more uniform
> +          transition to the human eye.
> +        type: boolean
> +
> +      ti,hardware-controlled:
> +        description: |
> +          Each backlight has its own voltage Control Bank (A and B) and there are
> +          two HVLED sinks which by default are linked to respective Bank. Setting
> +          this property will link both sinks to a Control Bank of backlight where
> +          property is defined.
> +        type: boolean
> +
> +    required:
> +      - compatible
> +      - reg
> +
> +    additionalProperties: false
> +
> +  "^led@[0-3]$":
> +    type: object
> +    description:
> +      Properties for a led device.
> +
> +    $ref: /schemas/leds/common.yaml#
> +
> +    properties:
> +      compatible:
> +        const: ti,lm3533-leds
> +
> +      reg:
> +        description:
> +          4 led banks
> +        minimum: 0
> +        maximum: 3
> +
> +      linux,default-trigger: true
> +
> +      ti,max-current-microamp:
> +        description:
> +          Maximum current in µA with a 800 µA step.
> +        enum: [ 5000, 5800, 6600, 7400, 8200, 9000, 9800,
> +                10600, 11400, 12200, 13000, 13800, 14600,
> +                15400, 16200, 17000, 17800, 18600, 19400,
> +                20200, 21000, 21800, 22600, 23400, 24200,
> +                25000, 25800, 26600, 27400, 28200, 29000,
> +                29800 ]
> +        default: 5000
> +
> +      ti,pwm-config-mask:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description:
> +          Same descryption and function as for backlight.
> +
> +    required:
> +      - compatible
> +      - reg
> +
> +    additionalProperties: false
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        led-controller@36 {
> +            compatible = "ti,lm3533";
> +            reg = <0x36>;
> +
> +            enable-gpios = <&gpio 110 GPIO_ACTIVE_HIGH>;
> +
> +            ti,boost-ovp-microvolt = <24000000>;
> +            ti,boost-freq-hz = <500000>;
> +
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            backlight@0 {
> +                compatible = "ti,lm3533-backlight";
> +                reg = <0>;
> +
> +                ti,max-current-microamp = <23400>;
> +                default-brightness = <113>;
> +                ti,hardware-controlled;
> +            };

Please make the example complete.

> +        };
> +    };
> +...
> -- 
> 2.43.0
> 

Re: [PATCH v2 1/2] dt-bindings: mfd: Document TI LM3533 MFD

[Svyatoslav Ryhel]

пт, 21 лют. 2025 р. о 22:38 Rob Herring <robh@kernel.org> пише:
>
> On Tue, Feb 18, 2025 at 03:26:59PM +0200, Svyatoslav Ryhel wrote:
> > Add bindings for the LM3533 - a complete power source for
> > backlight, keypad, and indicator LEDs in smartphone handsets.
> > The high-voltage inductive boost converter provides the
> > power for two series LED strings display backlight and keypad
> > functions.
> >
> > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > ---
> >  .../devicetree/bindings/mfd/ti,lm3533.yaml    | 231 ++++++++++++++++++
> >  1 file changed, 231 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mfd/ti,lm3533.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/mfd/ti,lm3533.yaml b/Documentation/devicetree/bindings/mfd/ti,lm3533.yaml
> > new file mode 100644
> > index 000000000000..83542f0c7bf7
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/ti,lm3533.yaml
> > @@ -0,0 +1,231 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/mfd/ti,lm3533.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: TI LM3533 Complete Lighting Power Solution
> > +
> > +description: |
>
> Use '>' rather than '|' if only formatting is paragraphs.
>

yaml check did not complain, fine.

> > +  The LM3533 is a complete power source for backlight,
> > +  keypad, and indicator LEDs in smartphone handsets. The
> > +  high-voltage inductive boost converter provides the
> > +  power for two series LED strings display backlight and
> > +  keypad functions.
>
> Wrap lines at 80
>

Checkpatch and yaml check do not complain, why? 80 char limit was removed, no?

> blank line here
>
> > +  https://www.ti.com/product/LM3533
> > +
> > +maintainers:
> > +  - Svyatoslav Ryhel <clamor95@gmail.com>
> > +
> > +properties:
> > +  compatible:
> > +    const: ti,lm3533
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  '#address-cells':
> > +    const: 1
> > +
> > +  '#size-cells':
> > +    const: 0
> > +
> > +  enable-gpios:
> > +    description: GPIO to use to enable/disable the backlight (HWEN pin).
> > +    maxItems: 1
> > +
> > +  ti,boost-ovp-microvolt:
> > +    description:
> > +      Boost OVP select (16V, 24V, 32V, 40V)
> > +    enum: [ 16000000, 24000000, 32000000, 40000000 ]
> > +    default: 16000000
> > +
> > +  ti,boost-freq-hz:
> > +    description:
> > +      Boost frequency select (500KHz or 1MHz)
> > +    enum: [ 500000, 1000000 ]
> > +    default: 500000
> > +
> > +  light-sensor@0:
> > +    type: object
> > +    description:
> > +      Properties for an illumination sensor.
> > +
> > +    properties:
> > +      compatible:
> > +        const: ti,lm3533-als
> > +
> > +      reg:
> > +        const: 0
> > +
> > +      ti,resistor-value-ohm:
> > +        $ref: /schemas/types.yaml#/definitions/uint32
> > +        description: |
>
> Don't need '|'. Elsewhere too.
>
> > +          Internal configuration resister value when ALS is in Analog Sensor
> > +          mode and PWM mode is disabled.
> > +        minimum: 1575
> > +        maximum: 200000
> > +
> > +      ti,pwm-mode:
> > +        type: boolean
> > +        description: |
> > +          Switch for mode in which ALS is running. If this propertly is
> > +          set then ALS is running in PWM mode, internal resistor value is
> > +          set to high-impedance (0) and resistor-value-ohm propertly is
> > +          ignored.
> > +
> > +    required:
> > +      - compatible
> > +      - reg
> > +
> > +    additionalProperties: false
>
> Move this above 'properties'.
>

yaml check did not complain. additionalProperties is always set after
all properties description, no?

> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - '#address-cells'
> > +  - '#size-cells'
> > +  - enable-gpios
> > +
> > +patternProperties:
> > +  "^backlight@[01]$":
> > +    type: object
> > +    description:
> > +      Properties for a backlight device.
> > +
> > +    $ref: /schemas/leds/backlight/common.yaml#
> > +
> > +    properties:
> > +      compatible:
> > +        const: ti,lm3533-backlight
> > +
> > +      reg:
> > +        description: |
> > +          The control bank that is used to program the two current sinks. The
> > +          LM3533 has two control banks (A and B) and are represented as 0 or 1
> > +          in this property. The two current sinks can be controlled
> > +          independently with both banks, or bank A can be configured to control
> > +          both sinks with the led-sources property.
> > +        minimum: 0
> > +        maximum: 1
> > +
> > +      default-brightness: true
> > +
> > +      ti,max-current-microamp:
> > +        description:
> > +          Maximum current in 渙 with a 800 渙 step.
> > +        enum: [ 5000, 5800, 6600, 7400, 8200, 9000, 9800,
> > +                10600, 11400, 12200, 13000, 13800, 14600,
> > +                15400, 16200, 17000, 17800, 18600, 19400,
> > +                20200, 21000, 21800, 22600, 23400, 24200,
> > +                25000, 25800, 26600, 27400, 28200, 29000,
> > +                29800 ]
> > +        default: 5000
> > +
> > +      ti,pwm-config-mask:
> > +        $ref: /schemas/types.yaml#/definitions/uint32
> > +        description: |
> > +          Control Bank PWM Configuration Register mask that allows to configure
> > +          PWM input in Zones 0-4
> > +          BIT(0) - PWM Input is enabled
> > +          BIT(1) - PWM Input is enabled in Zone 0
> > +          BIT(2) - PWM Input is enabled in Zone 1
> > +          BIT(3) - PWM Input is enabled in Zone 2
> > +          BIT(4) - PWM Input is enabled in Zone 3
> > +          BIT(5) - PWM Input is enabled in Zone 4
> > +
> > +      ti,linear-mapping-mode:
> > +        description: |
> > +          Enable linear mapping mode. If disabled, then it will use exponential
> > +          mapping mode in which the ramp up/down appears to have a more uniform
> > +          transition to the human eye.
> > +        type: boolean
> > +
> > +      ti,hardware-controlled:
> > +        description: |
> > +          Each backlight has its own voltage Control Bank (A and B) and there are
> > +          two HVLED sinks which by default are linked to respective Bank. Setting
> > +          this property will link both sinks to a Control Bank of backlight where
> > +          property is defined.
> > +        type: boolean
> > +
> > +    required:
> > +      - compatible
> > +      - reg
> > +
> > +    additionalProperties: false
> > +
> > +  "^led@[0-3]$":
> > +    type: object
> > +    description:
> > +      Properties for a led device.
> > +
> > +    $ref: /schemas/leds/common.yaml#
> > +
> > +    properties:
> > +      compatible:
> > +        const: ti,lm3533-leds
> > +
> > +      reg:
> > +        description:
> > +          4 led banks
> > +        minimum: 0
> > +        maximum: 3
> > +
> > +      linux,default-trigger: true
> > +
> > +      ti,max-current-microamp:
> > +        description:
> > +          Maximum current in 渙 with a 800 渙 step.
> > +        enum: [ 5000, 5800, 6600, 7400, 8200, 9000, 9800,
> > +                10600, 11400, 12200, 13000, 13800, 14600,
> > +                15400, 16200, 17000, 17800, 18600, 19400,
> > +                20200, 21000, 21800, 22600, 23400, 24200,
> > +                25000, 25800, 26600, 27400, 28200, 29000,
> > +                29800 ]
> > +        default: 5000
> > +
> > +      ti,pwm-config-mask:
> > +        $ref: /schemas/types.yaml#/definitions/uint32
> > +        description:
> > +          Same descryption and function as for backlight.
> > +
> > +    required:
> > +      - compatible
> > +      - reg
> > +
> > +    additionalProperties: false
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/gpio/gpio.h>
> > +
> > +    i2c {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        led-controller@36 {
> > +            compatible = "ti,lm3533";
> > +            reg = <0x36>;
> > +
> > +            enable-gpios = <&gpio 110 GPIO_ACTIVE_HIGH>;
> > +
> > +            ti,boost-ovp-microvolt = <24000000>;
> > +            ti,boost-freq-hz = <500000>;
> > +
> > +            #address-cells = <1>;
> > +            #size-cells = <0>;
> > +
> > +            backlight@0 {
> > +                compatible = "ti,lm3533-backlight";
> > +                reg = <0>;
> > +
> > +                ti,max-current-microamp = <23400>;
> > +                default-brightness = <113>;
> > +                ti,hardware-controlled;
> > +            };
>
> Please make the example complete.
>
> > +        };
> > +    };
> > +...
> > --
> > 2.43.0
> >

Re: [PATCH v2 1/2] dt-bindings: mfd: Document TI LM3533 MFD

[Jonathan Cameron]

On Sat, 22 Feb 2025 09:01:18 +0200
Svyatoslav Ryhel <clamor95@gmail.com> wrote:

> пт, 21 лют. 2025 р. о 22:38 Rob Herring <robh@kernel.org> пише:
> >
> > On Tue, Feb 18, 2025 at 03:26:59PM +0200, Svyatoslav Ryhel wrote:  
> > > Add bindings for the LM3533 - a complete power source for
> > > backlight, keypad, and indicator LEDs in smartphone handsets.
> > > The high-voltage inductive boost converter provides the
> > > power for two series LED strings display backlight and keypad
> > > functions.
> > >
> > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > ---
> > >  .../devicetree/bindings/mfd/ti,lm3533.yaml    | 231 ++++++++++++++++++
> > >  1 file changed, 231 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/mfd/ti,lm3533.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/mfd/ti,lm3533.yaml b/Documentation/devicetree/bindings/mfd/ti,lm3533.yaml
> > > new file mode 100644
> > > index 000000000000..83542f0c7bf7
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/mfd/ti,lm3533.yaml
> > > @@ -0,0 +1,231 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/mfd/ti,lm3533.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: TI LM3533 Complete Lighting Power Solution
> > > +
> > > +description: |  
> >
> > Use '>' rather than '|' if only formatting is paragraphs.
> >  
> 
> yaml check did not complain, fine.

It can't tell whether you want it formatted exactly or if paragraphs
is enough. 

> 
> > > +  The LM3533 is a complete power source for backlight,
> > > +  keypad, and indicator LEDs in smartphone handsets. The
> > > +  high-voltage inductive boost converter provides the
> > > +  power for two series LED strings display backlight and
> > > +  keypad functions.  
> >
> > Wrap lines at 80
> >  
> 
> Checkpatch and yaml check do not complain, why? 80 char limit was removed, no?

Check was to prevent over 80 chars, here you are far too short.

Re: [PATCH v2 1/2] dt-bindings: mfd: Document TI LM3533 MFD

[Jonathan Cameron]

On Tue, 18 Feb 2025 15:26:59 +0200
Svyatoslav Ryhel <clamor95@gmail.com> wrote:

> Add bindings for the LM3533 - a complete power source for
> backlight, keypad, and indicator LEDs in smartphone handsets.
> The high-voltage inductive boost converter provides the
> power for two series LED strings display backlight and keypad
> functions.

Wrap patch descriptions to 75 chars as describe in submitting-patches.rst

> 
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>

Re: [PATCH v2 1/2] dt-bindings: mfd: Document TI LM3533 MFD

[Svyatoslav Ryhel]

сб, 22 лют. 2025 р. о 16:29 Jonathan Cameron <jic23@kernel.org> пише:
>
> On Tue, 18 Feb 2025 15:26:59 +0200
> Svyatoslav Ryhel <clamor95@gmail.com> wrote:
>
> > Add bindings for the LM3533 - a complete power source for
> > backlight, keypad, and indicator LEDs in smartphone handsets.
> > The high-voltage inductive boost converter provides the
> > power for two series LED strings display backlight and keypad
> > functions.
>
> Wrap patch descriptions to 75 chars as describe in submitting-patches.rst
>

Alright, though why then checkpatch script has max line length 100 chars?

https://github.com/torvalds/linux/commit/bdc48fa11e46f867ea4d75fa59ee87a7f48be144

> >
> > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>

Re: [PATCH v2 1/2] dt-bindings: mfd: Document TI LM3533 MFD

[Jonathan Cameron]

On Sat, 22 Feb 2025 16:39:31 +0200
Svyatoslav Ryhel <clamor95@gmail.com> wrote:

> сб, 22 лют. 2025 р. о 16:29 Jonathan Cameron <jic23@kernel.org> пише:
> >
> > On Tue, 18 Feb 2025 15:26:59 +0200
> > Svyatoslav Ryhel <clamor95@gmail.com> wrote:
> >  
> > > Add bindings for the LM3533 - a complete power source for
> > > backlight, keypad, and indicator LEDs in smartphone handsets.
> > > The high-voltage inductive boost converter provides the
> > > power for two series LED strings display backlight and keypad
> > > functions.  
> >
> > Wrap patch descriptions to 75 chars as describe in submitting-patches.rst
> >  
> 
> Alright, though why then checkpatch script has max line length 100 chars?
> 
> https://github.com/torvalds/linux/commit/bdc48fa11e46f867ea4d75fa59ee87a7f48be144

Because that script is intended to catch when things are very wrong not
slightly so.  It provides guidance that you should look at and consider
whether to respond to.  Checking for short wrap is trickier to do as
perhaps the formatting is intended in some cases.


Jonathan

> 
> > >
> > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>