[PATCH v5 00/14] mfd: lm3533: convert to OF bindings, improve support

[PATCH v5 00/14] mfd: lm3533: convert to OF bindings, improve support

[Svyatoslav Ryhel]

Convert LM3533 to OF bindings, add missing VIN supply, add support for
setting mapping mode and LED sources based on device tree. 

---
Changes in v2:

schema
- maximum led sources for leds set to 4
- anyOf > oneOf in ALS
- improved ALS descriptions
- adjusted example
drivers
- dropped devm convertion of irq and mfd helpers
- all als configuration moved into lm3533_als_setup
- added regulator/consumer.h
- lm3533_bl_setup set before sysfs_create_group in backlight
- added check if LVLED is valid
- LM3533_REG_OUTPUT_CONF1 > LM3533_REG_OUTPUT_CONF2 for LVLED4 and LVLED5

Changes in v3:

- ohm -> ohms
- added default PWM mask to schema
- added commit removing driver regmap wrappers
- added commits which flatten pdata helpers
- adjusted check if als->irq > 0 since als->irq can be negative if error/missing
- added count for children defined in device tree and mfd_cell is allocated
  via devm_kcalloc to be able to contain all of them
- fixed backlight boundary check
- fixed count increased unconditionally
- dev_err returns node name instead of reg value
- LM3533_MAX_CURRENT_* defines moved to common header since they are used to clamp
  current values read from the tree
- mapping mode commit splitted into 2 (sysfs logic improvement > mode from DT)
- backlight mode configuration set before sysfs group is created
- fixed LVLED check on LEDs setup
- added check if HVLED is valid
- fixed wrong err_sysfs_remove goto in lm3533_bl
- added commit implementing backlight_scale property

Changes in v4:

- fixed regmap_update_bits val and mask swap in lm3533_als_set_int_mode
- fixed als->irq check in store_thresh_either_en
- removed clamping of r_select in als driver
- added use_of_reg and of_reg for backlight and leds registration
- added intermediate step in asigning props.brightness
- fixed HVLED/LVLED configuration overwriting
- added bounds check for num_leds in leds and backlight driver
- switched to regmap_clean/set/assign/test_bits helpers where appropriate
- dropped extern from header function declartions
- switched lm3533 pointer in cell structures to regmap
- sysfs_create_group > device_add_group
- changes in backlight sysfs were expanded to all properties

Changes in v5:

- aligned driver helpers to use u32
- child nodes parsing linked with 'else if' to ensure there is one call per node
- removed redundant pwm var init for leds
- fixed regmap_test_bits return check
- fixed leds and backlight counting handling
- iterator variables made scoped
- lm3533_als_setup() set to exit after mode set if m0de is pwm
---

Svyatoslav Ryhel (14):
  dt-bindings: leds: Document TI LM3533 LED controller
  mfd: lm3533: Remove driver specific regmap wrappers
  mfd: lm3533: Remove extern from shared functions in the header
  mfd: lm3533: Pass only regmap and light sensor presence to child
    devices
  iio: light: lm3533-als: Remove redundant pdata helpers
  mfd: lm3533-core: Remove redundant pdata helpers
  mfd: lm3533: Use dev_groups in struct device_driver
  mfd: lm3533: Convert to use OF bindings
  mfd: lm3533: Add support for VIN power supply
  mfd: lm3533: Set DMA mask
  video: backlight: lm3533_bl: Improve logic of sysfs functions
  video: backlight: lm3533_bl: Set initial mapping mode from DT
  video: backlight: lm3533_bl: Implement backlight_scale property
  video: leds: backlight: lm3533: Support getting LED sources from DT

 .../leds/backlight/ti,lm3533-backlight.yaml   |  69 ++++
 .../bindings/leds/ti,lm3533-leds.yaml         |  67 ++++
 .../devicetree/bindings/leds/ti,lm3533.yaml   | 169 ++++++++
 drivers/iio/light/lm3533-als.c                | 180 ++++-----
 drivers/leds/leds-lm3533.c                    | 184 ++++++---
 drivers/mfd/lm3533-core.c                     | 375 +++++++-----------
 drivers/mfd/lm3533-ctrlbank.c                 |  33 +-
 drivers/video/backlight/lm3533_bl.c           | 232 ++++++-----
 include/linux/mfd/lm3533.h                    |  75 +---
 9 files changed, 821 insertions(+), 563 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/leds/backlight/ti,lm3533-backlight.yaml
 create mode 100644 Documentation/devicetree/bindings/leds/ti,lm3533-leds.yaml
 create mode 100644 Documentation/devicetree/bindings/leds/ti,lm3533.yaml

-- 
2.53.0

[PATCH v5 01/14] dt-bindings: leds: Document TI LM3533 LED controller

[Svyatoslav Ryhel]

Document 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>
Reviewed-by: Jonathan Cameron <jic23@kernel.org> #for light sensor
Reviewed-by: Daniel Thompson (RISCstar) <danielt@kernel.org> #for backlight
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
---
 .../leds/backlight/ti,lm3533-backlight.yaml   |  69 +++++++
 .../bindings/leds/ti,lm3533-leds.yaml         |  67 +++++++
 .../devicetree/bindings/leds/ti,lm3533.yaml   | 169 ++++++++++++++++++
 3 files changed, 305 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/backlight/ti,lm3533-backlight.yaml
 create mode 100644 Documentation/devicetree/bindings/leds/ti,lm3533-leds.yaml
 create mode 100644 Documentation/devicetree/bindings/leds/ti,lm3533.yaml

diff --git a/Documentation/devicetree/bindings/leds/backlight/ti,lm3533-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/ti,lm3533-backlight.yaml
new file mode 100644
index 000000000000..a25846e9b375
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/backlight/ti,lm3533-backlight.yaml
@@ -0,0 +1,69 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/backlight/ti,lm3533-backlight.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TI LM3533 high voltage series LED strings
+
+description:
+  This is part of the TI LM3533 MFD device. It represents two high voltage series
+  LED strings for display backlight controlled by the TI LM3533.
+
+maintainers:
+  - Svyatoslav Ryhel <clamor95@gmail.com>
+
+allOf:
+  - $ref: /schemas/leds/backlight/common.yaml#
+
+properties:
+  compatible:
+    const: ti,lm3533-backlight
+
+  reg:
+    description: Control bank selection (0 = bank A, 1 = bank B).
+    maximum: 1
+
+  led-max-microamp:
+    description: maximum current in uA with a 800 uA step.
+    minimum: 5000
+    maximum: 29800
+    default: 5000
+
+  led-sources:
+    description: |
+      HVLED strings associated with this control bank:
+        0 - HVLED1
+        1 - HVLED2
+    minItems: 1
+    maxItems: 2
+    items:
+      maximum: 1
+
+  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
+    default: 0
+
+  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
+
+required:
+  - compatible
+  - reg
+
+unevaluatedProperties: false
+
+# see ti,lm3533.yaml for an example
diff --git a/Documentation/devicetree/bindings/leds/ti,lm3533-leds.yaml b/Documentation/devicetree/bindings/leds/ti,lm3533-leds.yaml
new file mode 100644
index 000000000000..d8dbefef3323
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/ti,lm3533-leds.yaml
@@ -0,0 +1,67 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/ti,lm3533-leds.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TI LM3533 low voltage control banks for individual LEDs
+
+description:
+  This is part of the TI LM3533 MFD device. It represents four low voltage
+  control banks for individual LEDs provided by the TI LM3533.
+
+maintainers:
+  - Svyatoslav Ryhel <clamor95@gmail.com>
+
+allOf:
+  - $ref: /schemas/leds/common.yaml#
+
+properties:
+  compatible:
+    const: ti,lm3533-leds
+
+  reg:
+    description:
+      Control bank selection (2 = bank C, 3 = bank D, 4 = bank E, 5 = bank F).
+    minimum: 2
+    maximum: 5
+
+  led-max-microamp:
+    description: maximum current in uA with a 800 uA step.
+    minimum: 5000
+    maximum: 29800
+    default: 5000
+
+  led-sources:
+    description: |
+      LVLED associated with this control bank. May be more than 1 source per bank.
+        0 - LVLED1
+        1 - LVLED2
+        2 - LVLED3
+        3 - LVLED4
+        4 - LVLED5
+    minItems: 1
+    maxItems: 5
+    items:
+      maximum: 4
+
+  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
+    default: 0
+
+required:
+  - compatible
+  - reg
+
+unevaluatedProperties: false
+
+# see ti,lm3533.yaml for an example
diff --git a/Documentation/devicetree/bindings/leds/ti,lm3533.yaml b/Documentation/devicetree/bindings/leds/ti,lm3533.yaml
new file mode 100644
index 000000000000..bdc05a9f3d66
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/ti,lm3533.yaml
@@ -0,0 +1,169 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/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 high voltage series LED strings for display backlight and four low
+  voltage control banks for individual LEDs. Additionally, LM3533 features an
+  interface for an external light sensor.
+
+  https://www.ti.com/product/LM3533
+
+maintainers:
+  - Svyatoslav Ryhel <clamor95@gmail.com>
+
+properties:
+  compatible:
+    const: ti,lm3533
+
+  reg:
+    maxItems: 1
+
+  enable-gpios:
+    description: GPIO connected to the HWEN pin.
+    maxItems: 1
+
+  vin-supply:
+    description: Supply connected to the IN line (2.7 V to 5.5 V).
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 0
+
+  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:
+    type: object
+    additionalProperties: false
+
+    properties:
+      compatible:
+        const: ti,lm3533-als
+
+      interrupts:
+        maxItems: 1
+
+      ti,resistor-ohms:
+        description:
+          Internal configuration resistor value when light sensor is in
+          Analog Sensor mode and PWM mode is disabled. The expectation is
+          the input is a current from the external analog light sensor and
+          this is used to convert it to a voltage within the target range.
+        minimum: 1575
+        maximum: 200000
+
+      ti,pwm-mode:
+        type: boolean
+        description:
+          Switch for mode in which light sensor interface is running. If
+          this property is set then the light sensor interface is running
+          in PWM mode, internal resistor value is set to high-impedance (0)
+          and ti,resistor-ohms property is ignored.
+
+    required:
+      - compatible
+
+    oneOf:
+      - required:
+          - ti,resistor-ohms
+      - required:
+          - ti,pwm-mode
+
+patternProperties:
+  "^backlight@[01]$":
+    $ref: /schemas/leds/backlight/ti,lm3533-backlight.yaml#
+
+  "^led@[2-5]$":
+    $ref: /schemas/leds/ti,lm3533-leds.yaml#
+
+required:
+  - compatible
+  - reg
+  - '#address-cells'
+  - '#size-cells'
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        led-controller@36 {
+            compatible = "ti,lm3533";
+            reg = <0x36>;
+
+            enable-gpios = <&gpio 110 GPIO_ACTIVE_HIGH>;
+            vin-supply = <&vdd_3v3_bat>;
+
+            ti,boost-ovp-microvolt = <24000000>;
+            ti,boost-freq-hz = <500000>;
+
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            backlight@0 {
+                compatible = "ti,lm3533-backlight";
+                reg = <0>;
+
+                default-brightness = <113>;
+
+                led-max-microamp = <23400>;
+                led-sources = <0 1>;
+            };
+
+            led@2 {
+                compatible = "ti,lm3533-leds";
+                reg = <2>;
+
+                led-max-microamp = <23400>;
+                led-sources = <0 1>;
+            };
+
+            led@4 {
+                compatible = "ti,lm3533-leds";
+                reg = <4>;
+
+                led-max-microamp = <23400>;
+                led-sources = <2>;
+            };
+
+            led@5 {
+                compatible = "ti,lm3533-leds";
+                reg = <5>;
+
+                led-max-microamp = <23400>;
+                led-sources = <3 4>;
+            };
+
+            light-sensor {
+                compatible = "ti,lm3533-als";
+
+                interrupt-parent = <&gpio>;
+                interrupts = <80 IRQ_TYPE_LEVEL_LOW>;
+
+                ti,pwm-mode;
+            };
+        };
+    };
+...
-- 
2.53.0

[PATCH v5 02/14] mfd: lm3533: Remove driver specific regmap wrappers

[Svyatoslav Ryhel]

Remove driver-specific regmap wrappers in favor of using regmap helpers
directly.

Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 drivers/iio/light/lm3533-als.c      | 79 +++++++++++++----------------
 drivers/leds/leds-lm3533.c          | 62 ++++++++--------------
 drivers/mfd/lm3533-core.c           | 73 ++++----------------------
 drivers/mfd/lm3533-ctrlbank.c       | 28 +++++-----
 drivers/video/backlight/lm3533_bl.c | 37 +++++---------
 include/linux/mfd/lm3533.h          | 15 ++----
 6 files changed, 100 insertions(+), 194 deletions(-)

diff --git a/drivers/iio/light/lm3533-als.c b/drivers/iio/light/lm3533-als.c
index 99f0b903018c..44b104c2d77f 100644
--- a/drivers/iio/light/lm3533-als.c
+++ b/drivers/iio/light/lm3533-als.c
@@ -17,6 +17,7 @@
 #include <linux/mutex.h>
 #include <linux/mfd/core.h>
 #include <linux/platform_device.h>
+#include <linux/regmap.h>
 #include <linux/slab.h>
 #include <linux/uaccess.h>
 
@@ -64,7 +65,7 @@ static int lm3533_als_get_adc(struct iio_dev *indio_dev, bool average,
 {
 	struct lm3533_als *als = iio_priv(indio_dev);
 	u8 reg;
-	u8 val;
+	u32 val;
 	int ret;
 
 	if (average)
@@ -72,7 +73,7 @@ static int lm3533_als_get_adc(struct iio_dev *indio_dev, bool average,
 	else
 		reg = LM3533_REG_ALS_READ_ADC_RAW;
 
-	ret = lm3533_read(als->lm3533, reg, &val);
+	ret = regmap_read(als->lm3533->regmap, reg, &val);
 	if (ret) {
 		dev_err(&indio_dev->dev, "failed to read adc\n");
 		return ret;
@@ -86,10 +87,10 @@ static int lm3533_als_get_adc(struct iio_dev *indio_dev, bool average,
 static int _lm3533_als_get_zone(struct iio_dev *indio_dev, u8 *zone)
 {
 	struct lm3533_als *als = iio_priv(indio_dev);
-	u8 val;
+	u32 val;
 	int ret;
 
-	ret = lm3533_read(als->lm3533, LM3533_REG_ALS_ZONE_INFO, &val);
+	ret = regmap_read(als->lm3533->regmap, LM3533_REG_ALS_ZONE_INFO, &val);
 	if (ret) {
 		dev_err(&indio_dev->dev, "failed to read zone\n");
 		return ret;
@@ -127,7 +128,7 @@ static inline u8 lm3533_als_get_target_reg(unsigned channel, unsigned zone)
 }
 
 static int lm3533_als_get_target(struct iio_dev *indio_dev, unsigned channel,
-							unsigned zone, u8 *val)
+				 unsigned zone, u32 *val)
 {
 	struct lm3533_als *als = iio_priv(indio_dev);
 	u8 reg;
@@ -140,7 +141,7 @@ static int lm3533_als_get_target(struct iio_dev *indio_dev, unsigned channel,
 		return -EINVAL;
 
 	reg = lm3533_als_get_target_reg(channel, zone);
-	ret = lm3533_read(als->lm3533, reg, val);
+	ret = regmap_read(als->lm3533->regmap, reg, val);
 	if (ret)
 		dev_err(&indio_dev->dev, "failed to get target current\n");
 
@@ -161,7 +162,7 @@ static int lm3533_als_set_target(struct iio_dev *indio_dev, unsigned channel,
 		return -EINVAL;
 
 	reg = lm3533_als_get_target_reg(channel, zone);
-	ret = lm3533_write(als->lm3533, reg, val);
+	ret = regmap_write(als->lm3533->regmap, reg, val);
 	if (ret)
 		dev_err(&indio_dev->dev, "failed to set target current\n");
 
@@ -171,8 +172,8 @@ static int lm3533_als_set_target(struct iio_dev *indio_dev, unsigned channel,
 static int lm3533_als_get_current(struct iio_dev *indio_dev, unsigned channel,
 								int *val)
 {
+	u32 target;
 	u8 zone;
-	u8 target;
 	int ret;
 
 	ret = lm3533_als_get_zone(indio_dev, &zone);
@@ -271,16 +272,10 @@ static irqreturn_t lm3533_als_isr(int irq, void *dev_id)
 static int lm3533_als_set_int_mode(struct iio_dev *indio_dev, int enable)
 {
 	struct lm3533_als *als = iio_priv(indio_dev);
-	u8 mask = LM3533_ALS_INT_ENABLE_MASK;
-	u8 val;
 	int ret;
 
-	if (enable)
-		val = mask;
-	else
-		val = 0;
-
-	ret = lm3533_update(als->lm3533, LM3533_REG_ALS_ZONE_INFO, val, mask);
+	ret = regmap_assign_bits(als->lm3533->regmap, LM3533_REG_ALS_ZONE_INFO,
+				 LM3533_ALS_INT_ENABLE_MASK, enable);
 	if (ret) {
 		dev_err(&indio_dev->dev, "failed to set int mode %d\n",
 								enable);
@@ -293,17 +288,16 @@ static int lm3533_als_set_int_mode(struct iio_dev *indio_dev, int enable)
 static int lm3533_als_get_int_mode(struct iio_dev *indio_dev, int *enable)
 {
 	struct lm3533_als *als = iio_priv(indio_dev);
-	u8 mask = LM3533_ALS_INT_ENABLE_MASK;
-	u8 val;
 	int ret;
 
-	ret = lm3533_read(als->lm3533, LM3533_REG_ALS_ZONE_INFO, &val);
-	if (ret) {
+	ret = regmap_test_bits(als->lm3533->regmap, LM3533_REG_ALS_ZONE_INFO,
+			       LM3533_ALS_INT_ENABLE_MASK);
+	if (ret < 0) {
 		dev_err(&indio_dev->dev, "failed to get int mode\n");
 		return ret;
 	}
 
-	*enable = !!(val & mask);
+	*enable = ret;
 
 	return 0;
 }
@@ -316,7 +310,7 @@ static inline u8 lm3533_als_get_threshold_reg(unsigned nr, bool raising)
 }
 
 static int lm3533_als_get_threshold(struct iio_dev *indio_dev, unsigned nr,
-							bool raising, u8 *val)
+				    bool raising, u32 *val)
 {
 	struct lm3533_als *als = iio_priv(indio_dev);
 	u8 reg;
@@ -326,7 +320,7 @@ static int lm3533_als_get_threshold(struct iio_dev *indio_dev, unsigned nr,
 		return -EINVAL;
 
 	reg = lm3533_als_get_threshold_reg(nr, raising);
-	ret = lm3533_read(als->lm3533, reg, val);
+	ret = regmap_read(als->lm3533->regmap, reg, val);
 	if (ret)
 		dev_err(&indio_dev->dev, "failed to get threshold\n");
 
@@ -337,8 +331,8 @@ static int lm3533_als_set_threshold(struct iio_dev *indio_dev, unsigned nr,
 							bool raising, u8 val)
 {
 	struct lm3533_als *als = iio_priv(indio_dev);
-	u8 val2;
 	u8 reg, reg2;
+	u32 val2;
 	int ret;
 
 	if (nr > LM3533_ALS_THRESH_MAX)
@@ -348,7 +342,7 @@ static int lm3533_als_set_threshold(struct iio_dev *indio_dev, unsigned nr,
 	reg2 = lm3533_als_get_threshold_reg(nr, !raising);
 
 	mutex_lock(&als->thresh_mutex);
-	ret = lm3533_read(als->lm3533, reg2, &val2);
+	ret = regmap_read(als->lm3533->regmap, reg2, &val2);
 	if (ret) {
 		dev_err(&indio_dev->dev, "failed to get threshold\n");
 		goto out;
@@ -363,7 +357,7 @@ static int lm3533_als_set_threshold(struct iio_dev *indio_dev, unsigned nr,
 		goto out;
 	}
 
-	ret = lm3533_write(als->lm3533, reg, val);
+	ret = regmap_write(als->lm3533->regmap, reg, val);
 	if (ret) {
 		dev_err(&indio_dev->dev, "failed to set threshold\n");
 		goto out;
@@ -375,11 +369,11 @@ static int lm3533_als_set_threshold(struct iio_dev *indio_dev, unsigned nr,
 }
 
 static int lm3533_als_get_hysteresis(struct iio_dev *indio_dev, unsigned nr,
-								u8 *val)
+				     u32 *val)
 {
 	struct lm3533_als *als = iio_priv(indio_dev);
-	u8 falling;
-	u8 raising;
+	u32 falling;
+	u32 raising;
 	int ret;
 
 	if (nr > LM3533_ALS_THRESH_MAX)
@@ -503,7 +497,7 @@ static ssize_t show_als_attr(struct device *dev,
 {
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 	struct lm3533_als_attribute *als_attr = to_lm3533_als_attr(attr);
-	u8 val;
+	u32 val;
 	int ret;
 
 	switch (als_attr->type) {
@@ -716,16 +710,10 @@ static const struct attribute_group lm3533_als_attribute_group = {
 
 static int lm3533_als_set_input_mode(struct lm3533_als *als, bool pwm_mode)
 {
-	u8 mask = LM3533_ALS_INPUT_MODE_MASK;
-	u8 val;
 	int ret;
 
-	if (pwm_mode)
-		val = mask;	/* pwm input */
-	else
-		val = 0;	/* analog input */
-
-	ret = lm3533_update(als->lm3533, LM3533_REG_ALS_CONF, val, mask);
+	ret = regmap_assign_bits(als->lm3533->regmap, LM3533_REG_ALS_CONF,
+				 LM3533_ALS_INPUT_MODE_MASK, pwm_mode);
 	if (ret) {
 		dev_err(&als->pdev->dev, "failed to set input mode %d\n",
 								pwm_mode);
@@ -744,7 +732,8 @@ static int lm3533_als_set_resistor(struct lm3533_als *als, u8 val)
 		return -EINVAL;
 	}
 
-	ret = lm3533_write(als->lm3533, LM3533_REG_ALS_RESISTOR_SELECT, val);
+	ret = regmap_write(als->lm3533->regmap, LM3533_REG_ALS_RESISTOR_SELECT,
+			   val);
 	if (ret) {
 		dev_err(&als->pdev->dev, "failed to set resistor\n");
 		return ret;
@@ -774,11 +763,11 @@ static int lm3533_als_setup(struct lm3533_als *als,
 
 static int lm3533_als_setup_irq(struct lm3533_als *als, void *dev)
 {
-	u8 mask = LM3533_ALS_INT_ENABLE_MASK;
 	int ret;
 
 	/* Make sure interrupts are disabled. */
-	ret = lm3533_update(als->lm3533, LM3533_REG_ALS_ZONE_INFO, 0, mask);
+	ret = regmap_clear_bits(als->lm3533->regmap, LM3533_REG_ALS_ZONE_INFO,
+				LM3533_ALS_INT_ENABLE_MASK);
 	if (ret) {
 		dev_err(&als->pdev->dev, "failed to disable interrupts\n");
 		return ret;
@@ -798,10 +787,10 @@ static int lm3533_als_setup_irq(struct lm3533_als *als, void *dev)
 
 static int lm3533_als_enable(struct lm3533_als *als)
 {
-	u8 mask = LM3533_ALS_ENABLE_MASK;
 	int ret;
 
-	ret = lm3533_update(als->lm3533, LM3533_REG_ALS_CONF, mask, mask);
+	ret = regmap_set_bits(als->lm3533->regmap, LM3533_REG_ALS_CONF,
+			      LM3533_ALS_ENABLE_MASK);
 	if (ret)
 		dev_err(&als->pdev->dev, "failed to enable ALS\n");
 
@@ -810,10 +799,10 @@ static int lm3533_als_enable(struct lm3533_als *als)
 
 static int lm3533_als_disable(struct lm3533_als *als)
 {
-	u8 mask = LM3533_ALS_ENABLE_MASK;
 	int ret;
 
-	ret = lm3533_update(als->lm3533, LM3533_REG_ALS_CONF, 0, mask);
+	ret = regmap_clear_bits(als->lm3533->regmap, LM3533_REG_ALS_CONF,
+				LM3533_ALS_ENABLE_MASK);
 	if (ret)
 		dev_err(&als->pdev->dev, "failed to disable ALS\n");
 
diff --git a/drivers/leds/leds-lm3533.c b/drivers/leds/leds-lm3533.c
index 45795f2a1042..335c758071f3 100644
--- a/drivers/leds/leds-lm3533.c
+++ b/drivers/leds/leds-lm3533.c
@@ -12,6 +12,7 @@
 #include <linux/mfd/core.h>
 #include <linux/mutex.h>
 #include <linux/platform_device.h>
+#include <linux/regmap.h>
 #include <linux/slab.h>
 
 #include <linux/mfd/lm3533.h>
@@ -80,7 +81,6 @@ static inline u8 lm3533_led_get_pattern_reg(struct lm3533_led *led,
 static int lm3533_led_pattern_enable(struct lm3533_led *led, int enable)
 {
 	u8 mask;
-	u8 val;
 	int pattern;
 	int state;
 	int ret = 0;
@@ -96,12 +96,8 @@ static int lm3533_led_pattern_enable(struct lm3533_led *led, int enable)
 	pattern = lm3533_led_get_pattern(led);
 	mask = 1 << (2 * pattern);
 
-	if (enable)
-		val = mask;
-	else
-		val = 0;
-
-	ret = lm3533_update(led->lm3533, LM3533_REG_PATTERN_ENABLE, val, mask);
+	ret = regmap_assign_bits(led->lm3533->regmap,
+				 LM3533_REG_PATTERN_ENABLE, mask, enable);
 	if (ret) {
 		dev_err(led->cdev.dev, "failed to enable pattern %d (%d)\n",
 							pattern, enable);
@@ -131,7 +127,7 @@ static int lm3533_led_set(struct led_classdev *cdev,
 static enum led_brightness lm3533_led_get(struct led_classdev *cdev)
 {
 	struct lm3533_led *led = to_lm3533_led(cdev);
-	u8 val;
+	u32 val;
 	int ret;
 
 	ret = lm3533_ctrlbank_get_brightness(&led->cb, &val);
@@ -259,7 +255,7 @@ static u8 lm3533_led_delay_set(struct lm3533_led *led, u8 base,
 	dev_dbg(led->cdev.dev, "%s - %lu: %u (0x%02x)\n", __func__,
 							*delay, t, val);
 	reg = lm3533_led_get_pattern_reg(led, base);
-	ret = lm3533_write(led->lm3533, reg, val);
+	ret = regmap_write(led->lm3533->regmap, reg, val);
 	if (ret)
 		dev_err(led->cdev.dev, "failed to set delay (%02x)\n", reg);
 
@@ -337,10 +333,10 @@ static ssize_t show_risefalltime(struct device *dev,
 	struct lm3533_led *led = to_lm3533_led(led_cdev);
 	ssize_t ret;
 	u8 reg;
-	u8 val;
+	u32 val;
 
 	reg = lm3533_led_get_pattern_reg(led, base);
-	ret = lm3533_read(led->lm3533, reg, &val);
+	ret = regmap_read(led->lm3533->regmap, reg, &val);
 	if (ret)
 		return ret;
 
@@ -375,7 +371,7 @@ static ssize_t store_risefalltime(struct device *dev,
 		return -EINVAL;
 
 	reg = lm3533_led_get_pattern_reg(led, base);
-	ret = lm3533_write(led->lm3533, reg, val);
+	ret = regmap_write(led->lm3533->regmap, reg, val);
 	if (ret)
 		return ret;
 
@@ -405,11 +401,11 @@ static ssize_t show_als_channel(struct device *dev,
 	struct lm3533_led *led = to_lm3533_led(led_cdev);
 	unsigned channel;
 	u8 reg;
-	u8 val;
+	u32 val;
 	int ret;
 
 	reg = lm3533_led_get_lv_reg(led, LM3533_REG_CTRLBANK_BCONF_BASE);
-	ret = lm3533_read(led->lm3533, reg, &val);
+	ret = regmap_read(led->lm3533->regmap, reg, &val);
 	if (ret)
 		return ret;
 
@@ -441,7 +437,7 @@ static ssize_t store_als_channel(struct device *dev,
 	mask = LM3533_REG_CTRLBANK_BCONF_ALS_CHANNEL_MASK;
 	val = channel - 1;
 
-	ret = lm3533_update(led->lm3533, reg, val, mask);
+	ret = regmap_update_bits(led->lm3533->regmap, reg, mask, val);
 	if (ret)
 		return ret;
 
@@ -455,11 +451,11 @@ static ssize_t show_als_en(struct device *dev,
 	struct lm3533_led *led = to_lm3533_led(led_cdev);
 	bool enable;
 	u8 reg;
-	u8 val;
+	u32 val;
 	int ret;
 
 	reg = lm3533_led_get_lv_reg(led, LM3533_REG_CTRLBANK_BCONF_BASE);
-	ret = lm3533_read(led->lm3533, reg, &val);
+	ret = regmap_read(led->lm3533->regmap, reg, &val);
 	if (ret)
 		return ret;
 
@@ -476,22 +472,15 @@ static ssize_t store_als_en(struct device *dev,
 	struct lm3533_led *led = to_lm3533_led(led_cdev);
 	unsigned enable;
 	u8 reg;
-	u8 mask;
-	u8 val;
 	int ret;
 
 	if (kstrtouint(buf, 0, &enable))
 		return -EINVAL;
 
 	reg = lm3533_led_get_lv_reg(led, LM3533_REG_CTRLBANK_BCONF_BASE);
-	mask = LM3533_REG_CTRLBANK_BCONF_ALS_EN_MASK;
 
-	if (enable)
-		val = mask;
-	else
-		val = 0;
-
-	ret = lm3533_update(led->lm3533, reg, val, mask);
+	ret = regmap_assign_bits(led->lm3533->regmap, reg,
+				 LM3533_REG_CTRLBANK_BCONF_ALS_EN_MASK, enable);
 	if (ret)
 		return ret;
 
@@ -504,12 +493,12 @@ static ssize_t show_linear(struct device *dev,
 	struct led_classdev *led_cdev = dev_get_drvdata(dev);
 	struct lm3533_led *led = to_lm3533_led(led_cdev);
 	u8 reg;
-	u8 val;
+	u32 val;
 	int linear;
 	int ret;
 
 	reg = lm3533_led_get_lv_reg(led, LM3533_REG_CTRLBANK_BCONF_BASE);
-	ret = lm3533_read(led->lm3533, reg, &val);
+	ret = regmap_read(led->lm3533->regmap, reg, &val);
 	if (ret)
 		return ret;
 
@@ -529,22 +518,15 @@ static ssize_t store_linear(struct device *dev,
 	struct lm3533_led *led = to_lm3533_led(led_cdev);
 	unsigned long linear;
 	u8 reg;
-	u8 mask;
-	u8 val;
 	int ret;
 
 	if (kstrtoul(buf, 0, &linear))
 		return -EINVAL;
 
 	reg = lm3533_led_get_lv_reg(led, LM3533_REG_CTRLBANK_BCONF_BASE);
-	mask = LM3533_REG_CTRLBANK_BCONF_MAPPING_MASK;
 
-	if (linear)
-		val = mask;
-	else
-		val = 0;
-
-	ret = lm3533_update(led->lm3533, reg, val, mask);
+	ret = regmap_assign_bits(led->lm3533->regmap, reg,
+				 LM3533_REG_CTRLBANK_BCONF_MAPPING_MASK, linear);
 	if (ret)
 		return ret;
 
@@ -557,7 +539,7 @@ static ssize_t show_pwm(struct device *dev,
 {
 	struct led_classdev *led_cdev = dev_get_drvdata(dev);
 	struct lm3533_led *led = to_lm3533_led(led_cdev);
-	u8 val;
+	u32 val;
 	int ret;
 
 	ret = lm3533_ctrlbank_get_pwm(&led->cb, &val);
@@ -573,10 +555,10 @@ static ssize_t store_pwm(struct device *dev,
 {
 	struct led_classdev *led_cdev = dev_get_drvdata(dev);
 	struct lm3533_led *led = to_lm3533_led(led_cdev);
-	u8 val;
+	u32 val;
 	int ret;
 
-	if (kstrtou8(buf, 0, &val))
+	if (kstrtou32(buf, 0, &val))
 		return -EINVAL;
 
 	ret = lm3533_ctrlbank_set_pwm(&led->cb, val);
diff --git a/drivers/mfd/lm3533-core.c b/drivers/mfd/lm3533-core.c
index 0a2409d00b2e..f9bdc0a1d9be 100644
--- a/drivers/mfd/lm3533-core.c
+++ b/drivers/mfd/lm3533-core.c
@@ -80,66 +80,14 @@ static struct mfd_cell lm3533_led_devs[] = {
 	},
 };
 
-int lm3533_read(struct lm3533 *lm3533, u8 reg, u8 *val)
-{
-	int tmp;
-	int ret;
-
-	ret = regmap_read(lm3533->regmap, reg, &tmp);
-	if (ret < 0) {
-		dev_err(lm3533->dev, "failed to read register %02x: %d\n",
-								reg, ret);
-		return ret;
-	}
-
-	*val = tmp;
-
-	dev_dbg(lm3533->dev, "read [%02x]: %02x\n", reg, *val);
-
-	return ret;
-}
-EXPORT_SYMBOL_GPL(lm3533_read);
-
-int lm3533_write(struct lm3533 *lm3533, u8 reg, u8 val)
-{
-	int ret;
-
-	dev_dbg(lm3533->dev, "write [%02x]: %02x\n", reg, val);
-
-	ret = regmap_write(lm3533->regmap, reg, val);
-	if (ret < 0) {
-		dev_err(lm3533->dev, "failed to write register %02x: %d\n",
-								reg, ret);
-	}
-
-	return ret;
-}
-EXPORT_SYMBOL_GPL(lm3533_write);
-
-int lm3533_update(struct lm3533 *lm3533, u8 reg, u8 val, u8 mask)
-{
-	int ret;
-
-	dev_dbg(lm3533->dev, "update [%02x]: %02x/%02x\n", reg, val, mask);
-
-	ret = regmap_update_bits(lm3533->regmap, reg, mask, val);
-	if (ret < 0) {
-		dev_err(lm3533->dev, "failed to update register %02x: %d\n",
-								reg, ret);
-	}
-
-	return ret;
-}
-EXPORT_SYMBOL_GPL(lm3533_update);
-
 static int lm3533_set_boost_freq(struct lm3533 *lm3533,
 						enum lm3533_boost_freq freq)
 {
 	int ret;
 
-	ret = lm3533_update(lm3533, LM3533_REG_BOOST_PWM,
-					freq << LM3533_BOOST_FREQ_SHIFT,
-					LM3533_BOOST_FREQ_MASK);
+	ret = regmap_update_bits(lm3533->regmap, LM3533_REG_BOOST_PWM,
+				 LM3533_BOOST_FREQ_MASK,
+				 freq << LM3533_BOOST_FREQ_SHIFT);
 	if (ret)
 		dev_err(lm3533->dev, "failed to set boost frequency\n");
 
@@ -152,9 +100,9 @@ static int lm3533_set_boost_ovp(struct lm3533 *lm3533,
 {
 	int ret;
 
-	ret = lm3533_update(lm3533, LM3533_REG_BOOST_PWM,
-					ovp << LM3533_BOOST_OVP_SHIFT,
-					LM3533_BOOST_OVP_MASK);
+	ret = regmap_update_bits(lm3533->regmap, LM3533_REG_BOOST_PWM,
+				 LM3533_BOOST_OVP_MASK,
+				 ovp << LM3533_BOOST_OVP_SHIFT);
 	if (ret)
 		dev_err(lm3533->dev, "failed to set boost ovp\n");
 
@@ -181,7 +129,8 @@ static int lm3533_set_hvled_config(struct lm3533 *lm3533, u8 hvled, u8 bl)
 	mask = LM3533_BL_ID_MASK << shift;
 	val = bl << shift;
 
-	ret = lm3533_update(lm3533, LM3533_REG_OUTPUT_CONF1, val, mask);
+	ret = regmap_update_bits(lm3533->regmap, LM3533_REG_OUTPUT_CONF1,
+				 mask, val);
 	if (ret)
 		dev_err(lm3533->dev, "failed to set hvled config\n");
 
@@ -216,7 +165,7 @@ static int lm3533_set_lvled_config(struct lm3533 *lm3533, u8 lvled, u8 led)
 	mask = LM3533_LED_ID_MASK << shift;
 	val = led << shift;
 
-	ret = lm3533_update(lm3533, reg, val, mask);
+	ret = regmap_update_bits(lm3533->regmap, reg, mask, val);
 	if (ret)
 		dev_err(lm3533->dev, "failed to set lvled config\n");
 
@@ -258,7 +207,7 @@ static ssize_t show_output(struct device *dev,
 	struct lm3533_device_attribute *lattr = to_lm3533_dev_attr(attr);
 	int id = lattr->u.output.id;
 	u8 reg;
-	u8 val;
+	u32 val;
 	u8 mask;
 	int shift;
 	int ret;
@@ -278,7 +227,7 @@ static ssize_t show_output(struct device *dev,
 		mask = LM3533_LED_ID_MASK << shift;
 	}
 
-	ret = lm3533_read(lm3533, reg, &val);
+	ret = regmap_read(lm3533->regmap, reg, &val);
 	if (ret)
 		return ret;
 
diff --git a/drivers/mfd/lm3533-ctrlbank.c b/drivers/mfd/lm3533-ctrlbank.c
index 2537dfade51c..c2b5fd38b81d 100644
--- a/drivers/mfd/lm3533-ctrlbank.c
+++ b/drivers/mfd/lm3533-ctrlbank.c
@@ -9,6 +9,7 @@
 
 #include <linux/device.h>
 #include <linux/module.h>
+#include <linux/regmap.h>
 
 #include <linux/mfd/lm3533.h>
 
@@ -38,8 +39,8 @@ int lm3533_ctrlbank_enable(struct lm3533_ctrlbank *cb)
 	dev_dbg(cb->dev, "%s - %d\n", __func__, cb->id);
 
 	mask = 1 << cb->id;
-	ret = lm3533_update(cb->lm3533, LM3533_REG_CTRLBANK_ENABLE,
-								mask, mask);
+	ret = regmap_set_bits(cb->lm3533->regmap, LM3533_REG_CTRLBANK_ENABLE,
+			      mask);
 	if (ret)
 		dev_err(cb->dev, "failed to enable ctrlbank %d\n", cb->id);
 
@@ -55,7 +56,8 @@ int lm3533_ctrlbank_disable(struct lm3533_ctrlbank *cb)
 	dev_dbg(cb->dev, "%s - %d\n", __func__, cb->id);
 
 	mask = 1 << cb->id;
-	ret = lm3533_update(cb->lm3533, LM3533_REG_CTRLBANK_ENABLE, 0, mask);
+	ret = regmap_clear_bits(cb->lm3533->regmap, LM3533_REG_CTRLBANK_ENABLE,
+				mask);
 	if (ret)
 		dev_err(cb->dev, "failed to disable ctrlbank %d\n", cb->id);
 
@@ -68,7 +70,7 @@ EXPORT_SYMBOL_GPL(lm3533_ctrlbank_disable);
  *
  * imax		5000 - 29800 uA (800 uA step)
  */
-int lm3533_ctrlbank_set_max_current(struct lm3533_ctrlbank *cb, u16 imax)
+int lm3533_ctrlbank_set_max_current(struct lm3533_ctrlbank *cb, u32 imax)
 {
 	u8 reg;
 	u8 val;
@@ -80,7 +82,7 @@ int lm3533_ctrlbank_set_max_current(struct lm3533_ctrlbank *cb, u16 imax)
 	val = (imax - LM3533_MAX_CURRENT_MIN) / LM3533_MAX_CURRENT_STEP;
 
 	reg = lm3533_ctrlbank_get_reg(cb, LM3533_REG_MAX_CURRENT_BASE);
-	ret = lm3533_write(cb->lm3533, reg, val);
+	ret = regmap_write(cb->lm3533->regmap, reg, val);
 	if (ret)
 		dev_err(cb->dev, "failed to set max current\n");
 
@@ -88,13 +90,13 @@ int lm3533_ctrlbank_set_max_current(struct lm3533_ctrlbank *cb, u16 imax)
 }
 EXPORT_SYMBOL_GPL(lm3533_ctrlbank_set_max_current);
 
-int lm3533_ctrlbank_set_brightness(struct lm3533_ctrlbank *cb, u8 val)
+int lm3533_ctrlbank_set_brightness(struct lm3533_ctrlbank *cb, u32 val)
 {
 	u8 reg;
 	int ret;
 
 	reg = lm3533_ctrlbank_get_reg(cb, LM3533_REG_BRIGHTNESS_BASE);
-	ret = lm3533_write(cb->lm3533, reg, val);
+	ret = regmap_write(cb->lm3533->regmap, reg, val);
 	if (ret)
 		dev_err(cb->dev, "failed to set brightness\n");
 
@@ -102,13 +104,13 @@ int lm3533_ctrlbank_set_brightness(struct lm3533_ctrlbank *cb, u8 val)
 }
 EXPORT_SYMBOL_GPL(lm3533_ctrlbank_set_brightness);
 
-int lm3533_ctrlbank_get_brightness(struct lm3533_ctrlbank *cb, u8 *val)
+int lm3533_ctrlbank_get_brightness(struct lm3533_ctrlbank *cb, u32 *val)
 {
 	u8 reg;
 	int ret;
 
 	reg = lm3533_ctrlbank_get_reg(cb, LM3533_REG_BRIGHTNESS_BASE);
-	ret = lm3533_read(cb->lm3533, reg, val);
+	ret = regmap_read(cb->lm3533->regmap, reg, val);
 	if (ret)
 		dev_err(cb->dev, "failed to get brightness\n");
 
@@ -126,7 +128,7 @@ EXPORT_SYMBOL_GPL(lm3533_ctrlbank_get_brightness);
  *   bit 1 - PWM-input enabled in Zone 0
  *   bit 0 - PWM-input enabled
  */
-int lm3533_ctrlbank_set_pwm(struct lm3533_ctrlbank *cb, u8 val)
+int lm3533_ctrlbank_set_pwm(struct lm3533_ctrlbank *cb, u32 val)
 {
 	u8 reg;
 	int ret;
@@ -135,7 +137,7 @@ int lm3533_ctrlbank_set_pwm(struct lm3533_ctrlbank *cb, u8 val)
 		return -EINVAL;
 
 	reg = lm3533_ctrlbank_get_reg(cb, LM3533_REG_PWM_BASE);
-	ret = lm3533_write(cb->lm3533, reg, val);
+	ret = regmap_write(cb->lm3533->regmap, reg, val);
 	if (ret)
 		dev_err(cb->dev, "failed to set PWM mask\n");
 
@@ -143,13 +145,13 @@ int lm3533_ctrlbank_set_pwm(struct lm3533_ctrlbank *cb, u8 val)
 }
 EXPORT_SYMBOL_GPL(lm3533_ctrlbank_set_pwm);
 
-int lm3533_ctrlbank_get_pwm(struct lm3533_ctrlbank *cb, u8 *val)
+int lm3533_ctrlbank_get_pwm(struct lm3533_ctrlbank *cb, u32 *val)
 {
 	u8 reg;
 	int ret;
 
 	reg = lm3533_ctrlbank_get_reg(cb, LM3533_REG_PWM_BASE);
-	ret = lm3533_read(cb->lm3533, reg, val);
+	ret = regmap_read(cb->lm3533->regmap, reg, val);
 	if (ret)
 		dev_err(cb->dev, "failed to get PWM mask\n");
 
diff --git a/drivers/video/backlight/lm3533_bl.c b/drivers/video/backlight/lm3533_bl.c
index babfd3ceec86..cd21c64b6485 100644
--- a/drivers/video/backlight/lm3533_bl.c
+++ b/drivers/video/backlight/lm3533_bl.c
@@ -11,6 +11,7 @@
 #include <linux/init.h>
 #include <linux/platform_device.h>
 #include <linux/backlight.h>
+#include <linux/regmap.h>
 #include <linux/slab.h>
 
 #include <linux/mfd/lm3533.h>
@@ -45,7 +46,7 @@ static int lm3533_bl_update_status(struct backlight_device *bd)
 static int lm3533_bl_get_brightness(struct backlight_device *bd)
 {
 	struct lm3533_bl *bl = bl_get_data(bd);
-	u8 val;
+	u32 val;
 	int ret;
 
 	ret = lm3533_ctrlbank_get_brightness(&bl->cb, &val);
@@ -82,12 +83,12 @@ static ssize_t show_als_en(struct device *dev,
 {
 	struct lm3533_bl *bl = dev_get_drvdata(dev);
 	int ctrlbank = lm3533_bl_get_ctrlbank_id(bl);
-	u8 val;
+	u32 val;
 	u8 mask;
 	bool enable;
 	int ret;
 
-	ret = lm3533_read(bl->lm3533, LM3533_REG_CTRLBANK_AB_BCONF, &val);
+	ret = regmap_read(bl->lm3533->regmap, LM3533_REG_CTRLBANK_AB_BCONF, &val);
 	if (ret)
 		return ret;
 
@@ -104,7 +105,6 @@ static ssize_t store_als_en(struct device *dev,
 	struct lm3533_bl *bl = dev_get_drvdata(dev);
 	int ctrlbank = lm3533_bl_get_ctrlbank_id(bl);
 	int enable;
-	u8 val;
 	u8 mask;
 	int ret;
 
@@ -113,13 +113,8 @@ static ssize_t store_als_en(struct device *dev,
 
 	mask = 1 << (2 * ctrlbank);
 
-	if (enable)
-		val = mask;
-	else
-		val = 0;
-
-	ret = lm3533_update(bl->lm3533, LM3533_REG_CTRLBANK_AB_BCONF, val,
-									mask);
+	ret = regmap_assign_bits(bl->lm3533->regmap, LM3533_REG_CTRLBANK_AB_BCONF,
+				 mask, enable);
 	if (ret)
 		return ret;
 
@@ -130,12 +125,12 @@ static ssize_t show_linear(struct device *dev,
 				struct device_attribute *attr, char *buf)
 {
 	struct lm3533_bl *bl = dev_get_drvdata(dev);
-	u8 val;
+	u32 val;
 	u8 mask;
 	int linear;
 	int ret;
 
-	ret = lm3533_read(bl->lm3533, LM3533_REG_CTRLBANK_AB_BCONF, &val);
+	ret = regmap_read(bl->lm3533->regmap, LM3533_REG_CTRLBANK_AB_BCONF, &val);
 	if (ret)
 		return ret;
 
@@ -156,7 +151,6 @@ static ssize_t store_linear(struct device *dev,
 	struct lm3533_bl *bl = dev_get_drvdata(dev);
 	unsigned long linear;
 	u8 mask;
-	u8 val;
 	int ret;
 
 	if (kstrtoul(buf, 0, &linear))
@@ -164,13 +158,8 @@ static ssize_t store_linear(struct device *dev,
 
 	mask = 1 << (2 * lm3533_bl_get_ctrlbank_id(bl) + 1);
 
-	if (linear)
-		val = mask;
-	else
-		val = 0;
-
-	ret = lm3533_update(bl->lm3533, LM3533_REG_CTRLBANK_AB_BCONF, val,
-									mask);
+	ret = regmap_assign_bits(bl->lm3533->regmap, LM3533_REG_CTRLBANK_AB_BCONF,
+				 mask, linear);
 	if (ret)
 		return ret;
 
@@ -182,7 +171,7 @@ static ssize_t show_pwm(struct device *dev,
 					char *buf)
 {
 	struct lm3533_bl *bl = dev_get_drvdata(dev);
-	u8 val;
+	u32 val;
 	int ret;
 
 	ret = lm3533_ctrlbank_get_pwm(&bl->cb, &val);
@@ -197,10 +186,10 @@ static ssize_t store_pwm(struct device *dev,
 					const char *buf, size_t len)
 {
 	struct lm3533_bl *bl = dev_get_drvdata(dev);
-	u8 val;
+	u32 val;
 	int ret;
 
-	if (kstrtou8(buf, 0, &val))
+	if (kstrtou32(buf, 0, &val))
 		return -EINVAL;
 
 	ret = lm3533_ctrlbank_set_pwm(&bl->cb, val);
diff --git a/include/linux/mfd/lm3533.h b/include/linux/mfd/lm3533.h
index 69059a7a2ce5..3463b164d0c1 100644
--- a/include/linux/mfd/lm3533.h
+++ b/include/linux/mfd/lm3533.h
@@ -85,15 +85,10 @@ struct lm3533_platform_data {
 extern int lm3533_ctrlbank_enable(struct lm3533_ctrlbank *cb);
 extern int lm3533_ctrlbank_disable(struct lm3533_ctrlbank *cb);
 
-extern int lm3533_ctrlbank_set_brightness(struct lm3533_ctrlbank *cb, u8 val);
-extern int lm3533_ctrlbank_get_brightness(struct lm3533_ctrlbank *cb, u8 *val);
-extern int lm3533_ctrlbank_set_max_current(struct lm3533_ctrlbank *cb,
-								u16 imax);
-extern int lm3533_ctrlbank_set_pwm(struct lm3533_ctrlbank *cb, u8 val);
-extern int lm3533_ctrlbank_get_pwm(struct lm3533_ctrlbank *cb, u8 *val);
-
-extern int lm3533_read(struct lm3533 *lm3533, u8 reg, u8 *val);
-extern int lm3533_write(struct lm3533 *lm3533, u8 reg, u8 val);
-extern int lm3533_update(struct lm3533 *lm3533, u8 reg, u8 val, u8 mask);
+extern int lm3533_ctrlbank_set_brightness(struct lm3533_ctrlbank *cb, u32 val);
+extern int lm3533_ctrlbank_get_brightness(struct lm3533_ctrlbank *cb, u32 *val);
+extern int lm3533_ctrlbank_set_max_current(struct lm3533_ctrlbank *cb, u32 imax);
+extern int lm3533_ctrlbank_set_pwm(struct lm3533_ctrlbank *cb, u32 val);
+extern int lm3533_ctrlbank_get_pwm(struct lm3533_ctrlbank *cb, u32 *val);
 
 #endif	/* __LINUX_MFD_LM3533_H */
-- 
2.53.0

[PATCH v5 03/14] mfd: lm3533: Remove extern from shared functions in the header

[Svyatoslav Ryhel]

Using 'extern' is not required, drop them.

Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
---
 include/linux/mfd/lm3533.h | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/include/linux/mfd/lm3533.h b/include/linux/mfd/lm3533.h
index 3463b164d0c1..c4c38876c09b 100644
--- a/include/linux/mfd/lm3533.h
+++ b/include/linux/mfd/lm3533.h
@@ -82,13 +82,13 @@ struct lm3533_platform_data {
 	int num_leds;
 };
 
-extern int lm3533_ctrlbank_enable(struct lm3533_ctrlbank *cb);
-extern int lm3533_ctrlbank_disable(struct lm3533_ctrlbank *cb);
-
-extern int lm3533_ctrlbank_set_brightness(struct lm3533_ctrlbank *cb, u32 val);
-extern int lm3533_ctrlbank_get_brightness(struct lm3533_ctrlbank *cb, u32 *val);
-extern int lm3533_ctrlbank_set_max_current(struct lm3533_ctrlbank *cb, u32 imax);
-extern int lm3533_ctrlbank_set_pwm(struct lm3533_ctrlbank *cb, u32 val);
-extern int lm3533_ctrlbank_get_pwm(struct lm3533_ctrlbank *cb, u32 *val);
+int lm3533_ctrlbank_enable(struct lm3533_ctrlbank *cb);
+int lm3533_ctrlbank_disable(struct lm3533_ctrlbank *cb);
+
+int lm3533_ctrlbank_set_brightness(struct lm3533_ctrlbank *cb, u32 val);
+int lm3533_ctrlbank_get_brightness(struct lm3533_ctrlbank *cb, u32 *val);
+int lm3533_ctrlbank_set_max_current(struct lm3533_ctrlbank *cb, u32 imax);
+int lm3533_ctrlbank_set_pwm(struct lm3533_ctrlbank *cb, u32 val);
+int lm3533_ctrlbank_get_pwm(struct lm3533_ctrlbank *cb, u32 *val);
 
 #endif	/* __LINUX_MFD_LM3533_H */
-- 
2.53.0

[PATCH v5 04/14] mfd: lm3533: Pass only regmap and light sensor presence to child devices

[Svyatoslav Ryhel]

Instead of passing the entire lm3533 core data structure, only pass the
regmap and the light sensor presence flag to child devices.

Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 drivers/iio/light/lm3533-als.c      | 32 ++++++++++++++---------------
 drivers/leds/leds-lm3533.c          | 32 ++++++++++++++++-------------
 drivers/mfd/lm3533-ctrlbank.c       | 14 ++++++-------
 drivers/video/backlight/lm3533_bl.c | 19 +++++++++--------
 include/linux/mfd/lm3533.h          |  2 +-
 5 files changed, 53 insertions(+), 46 deletions(-)

diff --git a/drivers/iio/light/lm3533-als.c b/drivers/iio/light/lm3533-als.c
index 44b104c2d77f..a9af8e2b965f 100644
--- a/drivers/iio/light/lm3533-als.c
+++ b/drivers/iio/light/lm3533-als.c
@@ -49,7 +49,7 @@
 
 
 struct lm3533_als {
-	struct lm3533 *lm3533;
+	struct regmap *regmap;
 	struct platform_device *pdev;
 
 	unsigned long flags;
@@ -73,7 +73,7 @@ static int lm3533_als_get_adc(struct iio_dev *indio_dev, bool average,
 	else
 		reg = LM3533_REG_ALS_READ_ADC_RAW;
 
-	ret = regmap_read(als->lm3533->regmap, reg, &val);
+	ret = regmap_read(als->regmap, reg, &val);
 	if (ret) {
 		dev_err(&indio_dev->dev, "failed to read adc\n");
 		return ret;
@@ -90,7 +90,7 @@ static int _lm3533_als_get_zone(struct iio_dev *indio_dev, u8 *zone)
 	u32 val;
 	int ret;
 
-	ret = regmap_read(als->lm3533->regmap, LM3533_REG_ALS_ZONE_INFO, &val);
+	ret = regmap_read(als->regmap, LM3533_REG_ALS_ZONE_INFO, &val);
 	if (ret) {
 		dev_err(&indio_dev->dev, "failed to read zone\n");
 		return ret;
@@ -141,7 +141,7 @@ static int lm3533_als_get_target(struct iio_dev *indio_dev, unsigned channel,
 		return -EINVAL;
 
 	reg = lm3533_als_get_target_reg(channel, zone);
-	ret = regmap_read(als->lm3533->regmap, reg, val);
+	ret = regmap_read(als->regmap, reg, val);
 	if (ret)
 		dev_err(&indio_dev->dev, "failed to get target current\n");
 
@@ -162,7 +162,7 @@ static int lm3533_als_set_target(struct iio_dev *indio_dev, unsigned channel,
 		return -EINVAL;
 
 	reg = lm3533_als_get_target_reg(channel, zone);
-	ret = regmap_write(als->lm3533->regmap, reg, val);
+	ret = regmap_write(als->regmap, reg, val);
 	if (ret)
 		dev_err(&indio_dev->dev, "failed to set target current\n");
 
@@ -274,7 +274,7 @@ static int lm3533_als_set_int_mode(struct iio_dev *indio_dev, int enable)
 	struct lm3533_als *als = iio_priv(indio_dev);
 	int ret;
 
-	ret = regmap_assign_bits(als->lm3533->regmap, LM3533_REG_ALS_ZONE_INFO,
+	ret = regmap_assign_bits(als->regmap, LM3533_REG_ALS_ZONE_INFO,
 				 LM3533_ALS_INT_ENABLE_MASK, enable);
 	if (ret) {
 		dev_err(&indio_dev->dev, "failed to set int mode %d\n",
@@ -290,7 +290,7 @@ static int lm3533_als_get_int_mode(struct iio_dev *indio_dev, int *enable)
 	struct lm3533_als *als = iio_priv(indio_dev);
 	int ret;
 
-	ret = regmap_test_bits(als->lm3533->regmap, LM3533_REG_ALS_ZONE_INFO,
+	ret = regmap_test_bits(als->regmap, LM3533_REG_ALS_ZONE_INFO,
 			       LM3533_ALS_INT_ENABLE_MASK);
 	if (ret < 0) {
 		dev_err(&indio_dev->dev, "failed to get int mode\n");
@@ -320,7 +320,7 @@ static int lm3533_als_get_threshold(struct iio_dev *indio_dev, unsigned nr,
 		return -EINVAL;
 
 	reg = lm3533_als_get_threshold_reg(nr, raising);
-	ret = regmap_read(als->lm3533->regmap, reg, val);
+	ret = regmap_read(als->regmap, reg, val);
 	if (ret)
 		dev_err(&indio_dev->dev, "failed to get threshold\n");
 
@@ -342,7 +342,7 @@ static int lm3533_als_set_threshold(struct iio_dev *indio_dev, unsigned nr,
 	reg2 = lm3533_als_get_threshold_reg(nr, !raising);
 
 	mutex_lock(&als->thresh_mutex);
-	ret = regmap_read(als->lm3533->regmap, reg2, &val2);
+	ret = regmap_read(als->regmap, reg2, &val2);
 	if (ret) {
 		dev_err(&indio_dev->dev, "failed to get threshold\n");
 		goto out;
@@ -357,7 +357,7 @@ static int lm3533_als_set_threshold(struct iio_dev *indio_dev, unsigned nr,
 		goto out;
 	}
 
-	ret = regmap_write(als->lm3533->regmap, reg, val);
+	ret = regmap_write(als->regmap, reg, val);
 	if (ret) {
 		dev_err(&indio_dev->dev, "failed to set threshold\n");
 		goto out;
@@ -712,7 +712,7 @@ static int lm3533_als_set_input_mode(struct lm3533_als *als, bool pwm_mode)
 {
 	int ret;
 
-	ret = regmap_assign_bits(als->lm3533->regmap, LM3533_REG_ALS_CONF,
+	ret = regmap_assign_bits(als->regmap, LM3533_REG_ALS_CONF,
 				 LM3533_ALS_INPUT_MODE_MASK, pwm_mode);
 	if (ret) {
 		dev_err(&als->pdev->dev, "failed to set input mode %d\n",
@@ -732,7 +732,7 @@ static int lm3533_als_set_resistor(struct lm3533_als *als, u8 val)
 		return -EINVAL;
 	}
 
-	ret = regmap_write(als->lm3533->regmap, LM3533_REG_ALS_RESISTOR_SELECT,
+	ret = regmap_write(als->regmap, LM3533_REG_ALS_RESISTOR_SELECT,
 			   val);
 	if (ret) {
 		dev_err(&als->pdev->dev, "failed to set resistor\n");
@@ -766,7 +766,7 @@ static int lm3533_als_setup_irq(struct lm3533_als *als, void *dev)
 	int ret;
 
 	/* Make sure interrupts are disabled. */
-	ret = regmap_clear_bits(als->lm3533->regmap, LM3533_REG_ALS_ZONE_INFO,
+	ret = regmap_clear_bits(als->regmap, LM3533_REG_ALS_ZONE_INFO,
 				LM3533_ALS_INT_ENABLE_MASK);
 	if (ret) {
 		dev_err(&als->pdev->dev, "failed to disable interrupts\n");
@@ -789,7 +789,7 @@ static int lm3533_als_enable(struct lm3533_als *als)
 {
 	int ret;
 
-	ret = regmap_set_bits(als->lm3533->regmap, LM3533_REG_ALS_CONF,
+	ret = regmap_set_bits(als->regmap, LM3533_REG_ALS_CONF,
 			      LM3533_ALS_ENABLE_MASK);
 	if (ret)
 		dev_err(&als->pdev->dev, "failed to enable ALS\n");
@@ -801,7 +801,7 @@ static int lm3533_als_disable(struct lm3533_als *als)
 {
 	int ret;
 
-	ret = regmap_clear_bits(als->lm3533->regmap, LM3533_REG_ALS_CONF,
+	ret = regmap_clear_bits(als->regmap, LM3533_REG_ALS_CONF,
 				LM3533_ALS_ENABLE_MASK);
 	if (ret)
 		dev_err(&als->pdev->dev, "failed to disable ALS\n");
@@ -845,7 +845,7 @@ static int lm3533_als_probe(struct platform_device *pdev)
 	indio_dev->modes = INDIO_DIRECT_MODE;
 
 	als = iio_priv(indio_dev);
-	als->lm3533 = lm3533;
+	als->regmap = lm3533->regmap;
 	als->pdev = pdev;
 	als->irq = lm3533->irq;
 	atomic_set(&als->zone, 0);
diff --git a/drivers/leds/leds-lm3533.c b/drivers/leds/leds-lm3533.c
index 335c758071f3..0cb0585eb960 100644
--- a/drivers/leds/leds-lm3533.c
+++ b/drivers/leds/leds-lm3533.c
@@ -42,13 +42,15 @@
 
 
 struct lm3533_led {
-	struct lm3533 *lm3533;
+	struct regmap *regmap;
 	struct lm3533_ctrlbank cb;
 	struct led_classdev cdev;
 	int id;
 
 	struct mutex mutex;
 	unsigned long flags;
+
+	bool have_als;
 };
 
 
@@ -96,7 +98,7 @@ static int lm3533_led_pattern_enable(struct lm3533_led *led, int enable)
 	pattern = lm3533_led_get_pattern(led);
 	mask = 1 << (2 * pattern);
 
-	ret = regmap_assign_bits(led->lm3533->regmap,
+	ret = regmap_assign_bits(led->regmap,
 				 LM3533_REG_PATTERN_ENABLE, mask, enable);
 	if (ret) {
 		dev_err(led->cdev.dev, "failed to enable pattern %d (%d)\n",
@@ -255,7 +257,7 @@ static u8 lm3533_led_delay_set(struct lm3533_led *led, u8 base,
 	dev_dbg(led->cdev.dev, "%s - %lu: %u (0x%02x)\n", __func__,
 							*delay, t, val);
 	reg = lm3533_led_get_pattern_reg(led, base);
-	ret = regmap_write(led->lm3533->regmap, reg, val);
+	ret = regmap_write(led->regmap, reg, val);
 	if (ret)
 		dev_err(led->cdev.dev, "failed to set delay (%02x)\n", reg);
 
@@ -336,7 +338,7 @@ static ssize_t show_risefalltime(struct device *dev,
 	u32 val;
 
 	reg = lm3533_led_get_pattern_reg(led, base);
-	ret = regmap_read(led->lm3533->regmap, reg, &val);
+	ret = regmap_read(led->regmap, reg, &val);
 	if (ret)
 		return ret;
 
@@ -371,7 +373,7 @@ static ssize_t store_risefalltime(struct device *dev,
 		return -EINVAL;
 
 	reg = lm3533_led_get_pattern_reg(led, base);
-	ret = regmap_write(led->lm3533->regmap, reg, val);
+	ret = regmap_write(led->regmap, reg, val);
 	if (ret)
 		return ret;
 
@@ -405,7 +407,7 @@ static ssize_t show_als_channel(struct device *dev,
 	int ret;
 
 	reg = lm3533_led_get_lv_reg(led, LM3533_REG_CTRLBANK_BCONF_BASE);
-	ret = regmap_read(led->lm3533->regmap, reg, &val);
+	ret = regmap_read(led->regmap, reg, &val);
 	if (ret)
 		return ret;
 
@@ -437,7 +439,7 @@ static ssize_t store_als_channel(struct device *dev,
 	mask = LM3533_REG_CTRLBANK_BCONF_ALS_CHANNEL_MASK;
 	val = channel - 1;
 
-	ret = regmap_update_bits(led->lm3533->regmap, reg, mask, val);
+	ret = regmap_update_bits(led->regmap, reg, mask, val);
 	if (ret)
 		return ret;
 
@@ -455,7 +457,7 @@ static ssize_t show_als_en(struct device *dev,
 	int ret;
 
 	reg = lm3533_led_get_lv_reg(led, LM3533_REG_CTRLBANK_BCONF_BASE);
-	ret = regmap_read(led->lm3533->regmap, reg, &val);
+	ret = regmap_read(led->regmap, reg, &val);
 	if (ret)
 		return ret;
 
@@ -479,7 +481,7 @@ static ssize_t store_als_en(struct device *dev,
 
 	reg = lm3533_led_get_lv_reg(led, LM3533_REG_CTRLBANK_BCONF_BASE);
 
-	ret = regmap_assign_bits(led->lm3533->regmap, reg,
+	ret = regmap_assign_bits(led->regmap, reg,
 				 LM3533_REG_CTRLBANK_BCONF_ALS_EN_MASK, enable);
 	if (ret)
 		return ret;
@@ -498,7 +500,7 @@ static ssize_t show_linear(struct device *dev,
 	int ret;
 
 	reg = lm3533_led_get_lv_reg(led, LM3533_REG_CTRLBANK_BCONF_BASE);
-	ret = regmap_read(led->lm3533->regmap, reg, &val);
+	ret = regmap_read(led->regmap, reg, &val);
 	if (ret)
 		return ret;
 
@@ -525,7 +527,7 @@ static ssize_t store_linear(struct device *dev,
 
 	reg = lm3533_led_get_lv_reg(led, LM3533_REG_CTRLBANK_BCONF_BASE);
 
-	ret = regmap_assign_bits(led->lm3533->regmap, reg,
+	ret = regmap_assign_bits(led->regmap, reg,
 				 LM3533_REG_CTRLBANK_BCONF_MAPPING_MASK, linear);
 	if (ret)
 		return ret;
@@ -597,7 +599,7 @@ static umode_t lm3533_led_attr_is_visible(struct kobject *kobj,
 
 	if (attr == &dev_attr_als_channel.attr ||
 					attr == &dev_attr_als_en.attr) {
-		if (!led->lm3533->have_als)
+		if (!led->have_als)
 			mode = 0;
 	}
 
@@ -654,7 +656,9 @@ static int lm3533_led_probe(struct platform_device *pdev)
 	if (!led)
 		return -ENOMEM;
 
-	led->lm3533 = lm3533;
+	led->regmap = lm3533->regmap;
+	led->have_als = lm3533->have_als;
+
 	led->cdev.name = pdata->name;
 	led->cdev.default_trigger = pdata->default_trigger;
 	led->cdev.brightness_set_blocking = lm3533_led_set;
@@ -670,7 +674,7 @@ static int lm3533_led_probe(struct platform_device *pdev)
 	 * registration so use parent device (for error reporting) until
 	 * registered.
 	 */
-	led->cb.lm3533 = lm3533;
+	led->cb.regmap = lm3533->regmap;
 	led->cb.id = lm3533_led_get_ctrlbank_id(led);
 	led->cb.dev = lm3533->dev;
 
diff --git a/drivers/mfd/lm3533-ctrlbank.c b/drivers/mfd/lm3533-ctrlbank.c
index c2b5fd38b81d..91e13cfa3cf0 100644
--- a/drivers/mfd/lm3533-ctrlbank.c
+++ b/drivers/mfd/lm3533-ctrlbank.c
@@ -39,7 +39,7 @@ int lm3533_ctrlbank_enable(struct lm3533_ctrlbank *cb)
 	dev_dbg(cb->dev, "%s - %d\n", __func__, cb->id);
 
 	mask = 1 << cb->id;
-	ret = regmap_set_bits(cb->lm3533->regmap, LM3533_REG_CTRLBANK_ENABLE,
+	ret = regmap_set_bits(cb->regmap, LM3533_REG_CTRLBANK_ENABLE,
 			      mask);
 	if (ret)
 		dev_err(cb->dev, "failed to enable ctrlbank %d\n", cb->id);
@@ -56,7 +56,7 @@ int lm3533_ctrlbank_disable(struct lm3533_ctrlbank *cb)
 	dev_dbg(cb->dev, "%s - %d\n", __func__, cb->id);
 
 	mask = 1 << cb->id;
-	ret = regmap_clear_bits(cb->lm3533->regmap, LM3533_REG_CTRLBANK_ENABLE,
+	ret = regmap_clear_bits(cb->regmap, LM3533_REG_CTRLBANK_ENABLE,
 				mask);
 	if (ret)
 		dev_err(cb->dev, "failed to disable ctrlbank %d\n", cb->id);
@@ -82,7 +82,7 @@ int lm3533_ctrlbank_set_max_current(struct lm3533_ctrlbank *cb, u32 imax)
 	val = (imax - LM3533_MAX_CURRENT_MIN) / LM3533_MAX_CURRENT_STEP;
 
 	reg = lm3533_ctrlbank_get_reg(cb, LM3533_REG_MAX_CURRENT_BASE);
-	ret = regmap_write(cb->lm3533->regmap, reg, val);
+	ret = regmap_write(cb->regmap, reg, val);
 	if (ret)
 		dev_err(cb->dev, "failed to set max current\n");
 
@@ -96,7 +96,7 @@ int lm3533_ctrlbank_set_brightness(struct lm3533_ctrlbank *cb, u32 val)
 	int ret;
 
 	reg = lm3533_ctrlbank_get_reg(cb, LM3533_REG_BRIGHTNESS_BASE);
-	ret = regmap_write(cb->lm3533->regmap, reg, val);
+	ret = regmap_write(cb->regmap, reg, val);
 	if (ret)
 		dev_err(cb->dev, "failed to set brightness\n");
 
@@ -110,7 +110,7 @@ int lm3533_ctrlbank_get_brightness(struct lm3533_ctrlbank *cb, u32 *val)
 	int ret;
 
 	reg = lm3533_ctrlbank_get_reg(cb, LM3533_REG_BRIGHTNESS_BASE);
-	ret = regmap_read(cb->lm3533->regmap, reg, val);
+	ret = regmap_read(cb->regmap, reg, val);
 	if (ret)
 		dev_err(cb->dev, "failed to get brightness\n");
 
@@ -137,7 +137,7 @@ int lm3533_ctrlbank_set_pwm(struct lm3533_ctrlbank *cb, u32 val)
 		return -EINVAL;
 
 	reg = lm3533_ctrlbank_get_reg(cb, LM3533_REG_PWM_BASE);
-	ret = regmap_write(cb->lm3533->regmap, reg, val);
+	ret = regmap_write(cb->regmap, reg, val);
 	if (ret)
 		dev_err(cb->dev, "failed to set PWM mask\n");
 
@@ -151,7 +151,7 @@ int lm3533_ctrlbank_get_pwm(struct lm3533_ctrlbank *cb, u32 *val)
 	int ret;
 
 	reg = lm3533_ctrlbank_get_reg(cb, LM3533_REG_PWM_BASE);
-	ret = regmap_read(cb->lm3533->regmap, reg, val);
+	ret = regmap_read(cb->regmap, reg, val);
 	if (ret)
 		dev_err(cb->dev, "failed to get PWM mask\n");
 
diff --git a/drivers/video/backlight/lm3533_bl.c b/drivers/video/backlight/lm3533_bl.c
index cd21c64b6485..4d6f68033480 100644
--- a/drivers/video/backlight/lm3533_bl.c
+++ b/drivers/video/backlight/lm3533_bl.c
@@ -24,10 +24,12 @@
 
 
 struct lm3533_bl {
-	struct lm3533 *lm3533;
+	struct regmap *regmap;
 	struct lm3533_ctrlbank cb;
 	struct backlight_device *bd;
 	int id;
+
+	bool have_als;
 };
 
 
@@ -88,7 +90,7 @@ static ssize_t show_als_en(struct device *dev,
 	bool enable;
 	int ret;
 
-	ret = regmap_read(bl->lm3533->regmap, LM3533_REG_CTRLBANK_AB_BCONF, &val);
+	ret = regmap_read(bl->regmap, LM3533_REG_CTRLBANK_AB_BCONF, &val);
 	if (ret)
 		return ret;
 
@@ -113,7 +115,7 @@ static ssize_t store_als_en(struct device *dev,
 
 	mask = 1 << (2 * ctrlbank);
 
-	ret = regmap_assign_bits(bl->lm3533->regmap, LM3533_REG_CTRLBANK_AB_BCONF,
+	ret = regmap_assign_bits(bl->regmap, LM3533_REG_CTRLBANK_AB_BCONF,
 				 mask, enable);
 	if (ret)
 		return ret;
@@ -130,7 +132,7 @@ static ssize_t show_linear(struct device *dev,
 	int linear;
 	int ret;
 
-	ret = regmap_read(bl->lm3533->regmap, LM3533_REG_CTRLBANK_AB_BCONF, &val);
+	ret = regmap_read(bl->regmap, LM3533_REG_CTRLBANK_AB_BCONF, &val);
 	if (ret)
 		return ret;
 
@@ -158,7 +160,7 @@ static ssize_t store_linear(struct device *dev,
 
 	mask = 1 << (2 * lm3533_bl_get_ctrlbank_id(bl) + 1);
 
-	ret = regmap_assign_bits(bl->lm3533->regmap, LM3533_REG_CTRLBANK_AB_BCONF,
+	ret = regmap_assign_bits(bl->regmap, LM3533_REG_CTRLBANK_AB_BCONF,
 				 mask, linear);
 	if (ret)
 		return ret;
@@ -223,7 +225,7 @@ static umode_t lm3533_bl_attr_is_visible(struct kobject *kobj,
 
 	if (attr == &dev_attr_als_channel.attr ||
 					attr == &dev_attr_als_en.attr) {
-		if (!bl->lm3533->have_als)
+		if (!bl->have_als)
 			mode = 0;
 	}
 
@@ -277,10 +279,11 @@ static int lm3533_bl_probe(struct platform_device *pdev)
 	if (!bl)
 		return -ENOMEM;
 
-	bl->lm3533 = lm3533;
+	bl->regmap = lm3533->regmap;
+	bl->have_als = lm3533->have_als;
 	bl->id = pdev->id;
 
-	bl->cb.lm3533 = lm3533;
+	bl->cb.regmap = lm3533->regmap;
 	bl->cb.id = lm3533_bl_get_ctrlbank_id(bl);
 	bl->cb.dev = NULL;			/* until registered */
 
diff --git a/include/linux/mfd/lm3533.h b/include/linux/mfd/lm3533.h
index c4c38876c09b..74cfb52c3bd1 100644
--- a/include/linux/mfd/lm3533.h
+++ b/include/linux/mfd/lm3533.h
@@ -33,7 +33,7 @@ struct lm3533 {
 };
 
 struct lm3533_ctrlbank {
-	struct lm3533 *lm3533;
+	struct regmap *regmap;
 	struct device *dev;
 	int id;
 };
-- 
2.53.0

[PATCH v5 05/14] iio: light: lm3533-als: Remove redundant pdata helpers

[Svyatoslav Ryhel]

The lm3533_als_set_input_mode() and lm3533_als_set_resistor() functions
are used only in lm3533_als_setup(). Incorporate their code into
lm3533_als_setup() directly to simplify driver readability.

Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 drivers/iio/light/lm3533-als.c | 56 ++++++++++------------------------
 1 file changed, 16 insertions(+), 40 deletions(-)

diff --git a/drivers/iio/light/lm3533-als.c b/drivers/iio/light/lm3533-als.c
index a9af8e2b965f..69bac1b202f1 100644
--- a/drivers/iio/light/lm3533-als.c
+++ b/drivers/iio/light/lm3533-als.c
@@ -708,55 +708,31 @@ static const struct attribute_group lm3533_als_attribute_group = {
 	.attrs = lm3533_als_attributes
 };
 
-static int lm3533_als_set_input_mode(struct lm3533_als *als, bool pwm_mode)
+static int lm3533_als_setup(struct lm3533_als *als,
+			    const struct lm3533_als_platform_data *pdata)
 {
+	struct device *dev = &als->pdev->dev;
 	int ret;
 
 	ret = regmap_assign_bits(als->regmap, LM3533_REG_ALS_CONF,
-				 LM3533_ALS_INPUT_MODE_MASK, pwm_mode);
-	if (ret) {
-		dev_err(&als->pdev->dev, "failed to set input mode %d\n",
-								pwm_mode);
-		return ret;
-	}
-
-	return 0;
-}
+				 LM3533_ALS_INPUT_MODE_MASK, pdata->pwm_mode);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to set input mode %d\n",
+				     pdata->pwm_mode);
 
-static int lm3533_als_set_resistor(struct lm3533_als *als, u8 val)
-{
-	int ret;
+	/* Bail out when in PWM-mode */
+	if (pdata->pwm_mode)
+		return 0;
 
-	if (val < LM3533_ALS_RESISTOR_MIN || val > LM3533_ALS_RESISTOR_MAX) {
-		dev_err(&als->pdev->dev, "invalid resistor value\n");
-		return -EINVAL;
-	}
+	if (pdata->r_select < LM3533_ALS_RESISTOR_MIN ||
+	    pdata->r_select > LM3533_ALS_RESISTOR_MAX)
+		return dev_err_probe(dev, -EINVAL,
+				     "invalid resistor value\n");
 
 	ret = regmap_write(als->regmap, LM3533_REG_ALS_RESISTOR_SELECT,
-			   val);
-	if (ret) {
-		dev_err(&als->pdev->dev, "failed to set resistor\n");
-		return ret;
-	}
-
-	return 0;
-}
-
-static int lm3533_als_setup(struct lm3533_als *als,
-			    const struct lm3533_als_platform_data *pdata)
-{
-	int ret;
-
-	ret = lm3533_als_set_input_mode(als, pdata->pwm_mode);
+			   pdata->r_select);
 	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 (ret)
-			return ret;
-	}
+		return dev_err_probe(dev, ret, "failed to set resistor\n");
 
 	return 0;
 }
-- 
2.53.0

[PATCH v5 06/14] mfd: lm3533-core: Remove redundant pdata helpers

[Svyatoslav Ryhel]

The lm3533_set_boost_freq() and lm3533_set_boost_ovp() functions are used
only in lm3533_device_setup(), which in turn is only called by
lm3533_device_init(). Incorporate their code directly into
lm3533_device_init() to simplify driver readability.

Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 drivers/mfd/lm3533-core.c | 57 ++++++++++-----------------------------
 1 file changed, 14 insertions(+), 43 deletions(-)

diff --git a/drivers/mfd/lm3533-core.c b/drivers/mfd/lm3533-core.c
index f9bdc0a1d9be..45e7f7481aa0 100644
--- a/drivers/mfd/lm3533-core.c
+++ b/drivers/mfd/lm3533-core.c
@@ -80,35 +80,6 @@ static struct mfd_cell lm3533_led_devs[] = {
 	},
 };
 
-static int lm3533_set_boost_freq(struct lm3533 *lm3533,
-						enum lm3533_boost_freq freq)
-{
-	int ret;
-
-	ret = regmap_update_bits(lm3533->regmap, LM3533_REG_BOOST_PWM,
-				 LM3533_BOOST_FREQ_MASK,
-				 freq << LM3533_BOOST_FREQ_SHIFT);
-	if (ret)
-		dev_err(lm3533->dev, "failed to set boost frequency\n");
-
-	return ret;
-}
-
-
-static int lm3533_set_boost_ovp(struct lm3533 *lm3533,
-						enum lm3533_boost_ovp ovp)
-{
-	int ret;
-
-	ret = regmap_update_bits(lm3533->regmap, LM3533_REG_BOOST_PWM,
-				 LM3533_BOOST_OVP_MASK,
-				 ovp << LM3533_BOOST_OVP_SHIFT);
-	if (ret)
-		dev_err(lm3533->dev, "failed to set boost ovp\n");
-
-	return ret;
-}
-
 /*
  * HVLED output config -- output hvled controlled by backlight bl
  */
@@ -406,18 +377,6 @@ static int lm3533_device_led_init(struct lm3533 *lm3533)
 	return 0;
 }
 
-static int lm3533_device_setup(struct lm3533 *lm3533,
-					struct lm3533_platform_data *pdata)
-{
-	int ret;
-
-	ret = lm3533_set_boost_freq(lm3533, pdata->boost_freq);
-	if (ret)
-		return ret;
-
-	return lm3533_set_boost_ovp(lm3533, pdata->boost_ovp);
-}
-
 static int lm3533_device_init(struct lm3533 *lm3533)
 {
 	struct lm3533_platform_data *pdata = dev_get_platdata(lm3533->dev);
@@ -437,9 +396,21 @@ static int lm3533_device_init(struct lm3533 *lm3533)
 
 	lm3533_enable(lm3533);
 
-	ret = lm3533_device_setup(lm3533, pdata);
-	if (ret)
+	ret = regmap_update_bits(lm3533->regmap, LM3533_REG_BOOST_PWM,
+				 LM3533_BOOST_FREQ_MASK,
+				 pdata->boost_freq << LM3533_BOOST_FREQ_SHIFT);
+	if (ret) {
+		dev_err(lm3533->dev, "failed to set boost frequency\n");
 		goto err_disable;
+	}
+
+	ret = regmap_update_bits(lm3533->regmap, LM3533_REG_BOOST_PWM,
+				 LM3533_BOOST_OVP_MASK,
+				 pdata->boost_ovp << LM3533_BOOST_OVP_SHIFT);
+	if (ret) {
+		dev_err(lm3533->dev, "failed to set boost ovp\n");
+		goto err_disable;
+	}
 
 	lm3533_device_als_init(lm3533);
 	lm3533_device_bl_init(lm3533);
-- 
2.53.0

[PATCH v5 07/14] mfd: lm3533: Use dev_groups in struct device_driver

[Svyatoslav Ryhel]

Instead of creating and removing the device sysfs attributes directly
during probe and remove of the driver, respectively, use dev_groups in
struct device_driver to point to the attribute definitions and let the
core take care of creating and removing them.

No intentional functional impact.

Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 drivers/mfd/lm3533-core.c           | 16 ++++++----------
 drivers/video/backlight/lm3533_bl.c | 22 ++++++++--------------
 2 files changed, 14 insertions(+), 24 deletions(-)

diff --git a/drivers/mfd/lm3533-core.c b/drivers/mfd/lm3533-core.c
index 45e7f7481aa0..b03a3ae96c10 100644
--- a/drivers/mfd/lm3533-core.c
+++ b/drivers/mfd/lm3533-core.c
@@ -296,6 +296,11 @@ static struct attribute_group lm3533_attribute_group = {
 	.attrs		= lm3533_attributes
 };
 
+static const struct attribute_group *lm3533_attribute_groups[] = {
+	&lm3533_attribute_group,
+	NULL,
+};
+
 static int lm3533_device_als_init(struct lm3533 *lm3533)
 {
 	struct lm3533_platform_data *pdata = dev_get_platdata(lm3533->dev);
@@ -416,16 +421,8 @@ static int lm3533_device_init(struct lm3533 *lm3533)
 	lm3533_device_bl_init(lm3533);
 	lm3533_device_led_init(lm3533);
 
-	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;
-	}
-
 	return 0;
 
-err_unregister:
-	mfd_remove_devices(lm3533->dev);
 err_disable:
 	lm3533_disable(lm3533);
 
@@ -436,8 +433,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);
 }
@@ -532,6 +527,7 @@ MODULE_DEVICE_TABLE(i2c, lm3533_i2c_ids);
 static struct i2c_driver lm3533_i2c_driver = {
 	.driver = {
 		   .name = "lm3533",
+		   .dev_groups = lm3533_attribute_groups,
 	},
 	.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 4d6f68033480..9ef171d3aaea 100644
--- a/drivers/video/backlight/lm3533_bl.c
+++ b/drivers/video/backlight/lm3533_bl.c
@@ -237,6 +237,11 @@ static struct attribute_group lm3533_bl_attribute_group = {
 	.attrs		= lm3533_bl_attributes
 };
 
+static const struct attribute_group *lm3533_bl_attribute_groups[] = {
+	&lm3533_bl_attribute_group,
+	NULL,
+};
+
 static int lm3533_bl_setup(struct lm3533_bl *bl,
 					struct lm3533_bl_platform_data *pdata)
 {
@@ -304,28 +309,17 @@ static int lm3533_bl_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, bl);
 
-	ret = sysfs_create_group(&bd->dev.kobj, &lm3533_bl_attribute_group);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "failed to create sysfs attributes\n");
-		return ret;
-	}
-
 	backlight_update_status(bd);
 
 	ret = lm3533_bl_setup(bl, pdata);
 	if (ret)
-		goto err_sysfs_remove;
+		return ret;
 
 	ret = lm3533_ctrlbank_enable(&bl->cb);
 	if (ret)
-		goto err_sysfs_remove;
+		return ret;
 
 	return 0;
-
-err_sysfs_remove:
-	sysfs_remove_group(&bd->dev.kobj, &lm3533_bl_attribute_group);
-
-	return ret;
 }
 
 static void lm3533_bl_remove(struct platform_device *pdev)
@@ -339,7 +333,6 @@ static void lm3533_bl_remove(struct platform_device *pdev)
 	bd->props.brightness = 0;
 
 	lm3533_ctrlbank_disable(&bl->cb);
-	sysfs_remove_group(&bd->dev.kobj, &lm3533_bl_attribute_group);
 }
 
 #ifdef CONFIG_PM_SLEEP
@@ -377,6 +370,7 @@ static struct platform_driver lm3533_bl_driver = {
 	.driver = {
 		.name	= "lm3533-backlight",
 		.pm	= &lm3533_bl_pm_ops,
+		.dev_groups = lm3533_bl_attribute_groups,
 	},
 	.probe		= lm3533_bl_probe,
 	.remove		= lm3533_bl_remove,
-- 
2.53.0

[PATCH v5 08/14] mfd: lm3533: Convert to use OF bindings

[Svyatoslav Ryhel]

Since there are no users of this driver via platform data, remove the
platform data support and switch to using Device Tree bindings.

Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
Reviewed-by: Daniel Thompson (RISCstar) <danielt@kernel.org> #for backlight
---
 drivers/iio/light/lm3533-als.c      |  67 +++++---
 drivers/leds/leds-lm3533.c          |  50 ++++--
 drivers/mfd/lm3533-core.c           | 236 ++++++++++++----------------
 drivers/mfd/lm3533-ctrlbank.c       |   5 -
 drivers/video/backlight/lm3533_bl.c |  55 +++++--
 include/linux/mfd/lm3533.h          |  52 +-----
 6 files changed, 220 insertions(+), 245 deletions(-)

diff --git a/drivers/iio/light/lm3533-als.c b/drivers/iio/light/lm3533-als.c
index 69bac1b202f1..80f8fa700263 100644
--- a/drivers/iio/light/lm3533-als.c
+++ b/drivers/iio/light/lm3533-als.c
@@ -16,16 +16,19 @@
 #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/regmap.h>
 #include <linux/slab.h>
 #include <linux/uaccess.h>
+#include <linux/units.h>
 
 #include <linux/mfd/lm3533.h>
 
 
-#define LM3533_ALS_RESISTOR_MIN			1
-#define LM3533_ALS_RESISTOR_MAX			127
+#define LM3533_ALS_RESISTOR_MIN_OHMS		1575
+#define LM3533_ALS_RESISTOR_MAX_OHMS		200000
 #define LM3533_ALS_CHANNEL_CURRENT_MAX		2
 #define LM3533_ALS_THRESH_MAX			3
 #define LM3533_ALS_ZONE_MAX			4
@@ -57,6 +60,9 @@ struct lm3533_als {
 
 	atomic_t zone;
 	struct mutex thresh_mutex;
+
+	u32 r_select;
+	bool pwm_mode;
 };
 
 
@@ -403,7 +409,7 @@ static ssize_t show_thresh_either_en(struct device *dev,
 	int enable;
 	int ret;
 
-	if (als->irq) {
+	if (als->irq > 0) {
 		ret = lm3533_als_get_int_mode(indio_dev, &enable);
 		if (ret)
 			return ret;
@@ -425,7 +431,7 @@ static ssize_t store_thresh_either_en(struct device *dev,
 	u8 zone;
 	int ret;
 
-	if (!als->irq)
+	if (als->irq <= 0)
 		return -EBUSY;
 
 	if (kstrtoul(buf, 0, &enable))
@@ -708,29 +714,38 @@ static const struct attribute_group lm3533_als_attribute_group = {
 	.attrs = lm3533_als_attributes
 };
 
-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)
 {
 	struct device *dev = &als->pdev->dev;
 	int ret;
 
+	als->pwm_mode = device_property_read_bool(dev, "ti,pwm-mode");
+
 	ret = regmap_assign_bits(als->regmap, LM3533_REG_ALS_CONF,
-				 LM3533_ALS_INPUT_MODE_MASK, pdata->pwm_mode);
+				 LM3533_ALS_INPUT_MODE_MASK, als->pwm_mode);
 	if (ret)
 		return dev_err_probe(dev, ret, "failed to set input mode %d\n",
-				     pdata->pwm_mode);
+				     als->pwm_mode);
 
 	/* Bail out when in PWM-mode */
-	if (pdata->pwm_mode)
+	if (als->pwm_mode)
 		return 0;
 
-	if (pdata->r_select < LM3533_ALS_RESISTOR_MIN ||
-	    pdata->r_select > LM3533_ALS_RESISTOR_MAX)
+	ret = device_property_read_u32(dev, "ti,resistor-ohms",
+				       &als->r_select);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "failed to ger resistor value\n");
+
+	if (als->r_select < LM3533_ALS_RESISTOR_MIN_OHMS ||
+	    als->r_select > LM3533_ALS_RESISTOR_MAX_OHMS)
 		return dev_err_probe(dev, -EINVAL,
 				     "invalid resistor value\n");
 
+	als->r_select = DIV_ROUND_UP(2 * MICRO, 10 * als->r_select);
+
 	ret = regmap_write(als->regmap, LM3533_REG_ALS_RESISTOR_SELECT,
-			   pdata->r_select);
+			   als->r_select);
 	if (ret)
 		return dev_err_probe(dev, ret, "failed to set resistor\n");
 
@@ -793,7 +808,6 @@ 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;
@@ -803,12 +817,6 @@ static int lm3533_als_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;
-	}
-
 	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*als));
 	if (!indio_dev)
 		return -ENOMEM;
@@ -817,25 +825,27 @@ static int lm3533_als_probe(struct platform_device *pdev)
 	indio_dev->channels = lm3533_als_channels;
 	indio_dev->num_channels = ARRAY_SIZE(lm3533_als_channels);
 	indio_dev->name = dev_name(&pdev->dev);
-	iio_device_set_parent(indio_dev, pdev->dev.parent);
 	indio_dev->modes = INDIO_DIRECT_MODE;
 
 	als = iio_priv(indio_dev);
 	als->regmap = lm3533->regmap;
 	als->pdev = pdev;
-	als->irq = lm3533->irq;
+	als->irq = platform_get_irq_optional(pdev, 0);
+	if (als->irq == -EPROBE_DEFER)
+		return -EPROBE_DEFER;
+
 	atomic_set(&als->zone, 0);
 	mutex_init(&als->thresh_mutex);
 
 	platform_set_drvdata(pdev, indio_dev);
 
-	if (als->irq) {
+	if (als->irq > 0) {
 		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;
 
@@ -854,7 +864,7 @@ static int lm3533_als_probe(struct platform_device *pdev)
 err_disable:
 	lm3533_als_disable(als);
 err_free_irq:
-	if (als->irq)
+	if (als->irq > 0)
 		free_irq(als->irq, indio_dev);
 
 	return ret;
@@ -868,13 +878,20 @@ static void lm3533_als_remove(struct platform_device *pdev)
 	lm3533_als_set_int_mode(indio_dev, false);
 	iio_device_unregister(indio_dev);
 	lm3533_als_disable(als);
-	if (als->irq)
+	if (als->irq > 0)
 		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_als_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 0cb0585eb960..ed810c23f30f 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/regmap.h>
 #include <linux/slab.h>
 
@@ -50,6 +52,9 @@ struct lm3533_led {
 	struct mutex mutex;
 	unsigned long flags;
 
+	u32 max_current;
+	u32 pwm;
+
 	bool have_als;
 };
 
@@ -616,22 +621,20 @@ 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;
 	int ret;
 
@@ -641,12 +644,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;
@@ -659,8 +656,6 @@ static int lm3533_led_probe(struct platform_device *pdev)
 	led->regmap = lm3533->regmap;
 	led->have_als = lm3533->have_als;
 
-	led->cdev.name = pdata->name;
-	led->cdev.default_trigger = pdata->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;
@@ -668,6 +663,15 @@ static int lm3533_led_probe(struct platform_device *pdev)
 	led->cdev.groups = lm3533_led_attribute_groups;
 	led->id = pdev->id;
 
+	led->cdev.name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s-%d",
+					pdev->name, led->id);
+	if (!led->cdev.name)
+		return -ENOMEM;
+
+	led->cdev.default_trigger = "none";
+	device_property_read_string(&pdev->dev, "linux,default-trigger",
+				    &led->cdev.default_trigger);
+
 	mutex_init(&led->mutex);
 
 	/* The class framework makes a callback to get brightness during
@@ -680,15 +684,22 @@ static int lm3533_led_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, led);
 
-	ret = led_classdev_register(pdev->dev.parent, &led->cdev);
+	ret = led_classdev_register(&pdev->dev, &led->cdev);
 	if (ret) {
-		dev_err(&pdev->dev, "failed to register LED %d\n", pdev->id);
+		dev_err(&pdev->dev, "failed to register LED %d\n", led->id);
 		return ret;
 	}
 
 	led->cb.dev = led->cdev.dev;
 
-	ret = lm3533_led_setup(led, pdata);
+	device_property_read_u32(&pdev->dev, "led-max-microamp",
+				 &led->max_current);
+	led->max_current = clamp(led->max_current, LM3533_MAX_CURRENT_MIN,
+				 LM3533_MAX_CURRENT_MAX);
+
+	device_property_read_u32(&pdev->dev, "ti,pwm-config-mask", &led->pwm);
+
+	ret = lm3533_led_setup(led);
 	if (ret)
 		goto err_deregister;
 
@@ -725,9 +736,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_led_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 b03a3ae96c10..a5aa7da9668b 100644
--- a/drivers/mfd/lm3533-core.c
+++ b/drivers/mfd/lm3533-core.c
@@ -14,19 +14,26 @@
 #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>
 
 
 #define LM3533_BOOST_OVP_MASK		0x06
 #define LM3533_BOOST_OVP_SHIFT		1
+#define LM3533_BOOST_OVP_MIN		(16 * MICRO)
+#define LM3533_BOOST_OVP_MAX		(40 * MICRO)
 
 #define LM3533_BOOST_FREQ_MASK		0x01
 #define LM3533_BOOST_FREQ_SHIFT		0
+#define LM3533_BOOST_FREQ_MIN		(500 * HZ_PER_KHZ)
+#define LM3533_BOOST_FREQ_MAX		(1000 * HZ_PER_KHZ)
 
 #define LM3533_BL_ID_MASK		1
 #define LM3533_LED_ID_MASK		3
@@ -35,6 +42,7 @@
 
 #define LM3533_HVLED_ID_MAX		2
 #define LM3533_LVLED_ID_MAX		5
+#define LM3533_CELLS_MAX		7
 
 #define LM3533_REG_OUTPUT_CONF1		0x10
 #define LM3533_REG_OUTPUT_CONF2		0x11
@@ -42,44 +50,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,
-	},
-};
-
 /*
  * HVLED output config -- output hvled controlled by backlight bl
  */
@@ -301,125 +271,91 @@ static const struct attribute_group *lm3533_attribute_groups[] = {
 	NULL,
 };
 
-static int lm3533_device_als_init(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");
-		return ret;
-	}
-
-	lm3533->have_als = 1;
-
-	return 0;
-}
-
-static int lm3533_device_bl_init(struct lm3533 *lm3533)
-{
-	struct lm3533_platform_data *pdata = dev_get_platdata(lm3533->dev);
-	int i;
-	int ret;
-
-	if (!pdata->backlights || pdata->num_backlights == 0)
-		return 0;
-
-	if (pdata->num_backlights > ARRAY_SIZE(lm3533_bl_devs))
-		pdata->num_backlights = ARRAY_SIZE(lm3533_bl_devs);
-
-	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]);
-	}
-
-	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;
-	}
-
-	lm3533->have_backlights = 1;
-
-	return 0;
-}
-
-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;
-
-	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;
-	}
-
-	lm3533->have_leds = 1;
-
-	return 0;
-}
-
 static int lm3533_device_init(struct lm3533 *lm3533)
 {
-	struct lm3533_platform_data *pdata = dev_get_platdata(lm3533->dev);
+	struct device *dev = lm3533->dev;
+	struct mfd_cell *lm3533_devices;
+	u32 count = 0, reg, nchilds;
 	int ret;
 
-	dev_dbg(lm3533->dev, "%s\n", __func__);
+	nchilds = device_get_child_node_count(dev);
+	if (!nchilds || nchilds > LM3533_CELLS_MAX)
+		return dev_err_probe(dev, -ENODEV,
+				     "num of child nodes is not supported\n");
 
-	if (!pdata) {
-		dev_err(lm3533->dev, "no platform data\n");
-		return -EINVAL;
-	}
+	lm3533_devices = devm_kcalloc(dev, nchilds, sizeof(*lm3533_devices),
+				      GFP_KERNEL);
+	if (!lm3533_devices)
+		return -ENOMEM;
 
-	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");
+	device_for_each_child_node_scoped(dev, child) {
+		if (count >= nchilds)
+			break;
+
+		if (fwnode_device_is_compatible(child, "ti,lm3533-als")) {
+			lm3533_devices[count].name = "lm3533-als";
+			lm3533_devices[count].of_compatible = "ti,lm3533-als";
+			lm3533_devices[count].id = PLATFORM_DEVID_NONE;
+
+			lm3533->have_als = true;
+			count++;
+		} else if (fwnode_device_is_compatible(child, "ti,lm3533-backlight")) {
+			ret = fwnode_property_read_u32(child, "reg", &reg);
+			if (ret || reg >= LM3533_HVLED_ID_MAX) {
+				dev_err(dev, "invalid backlight node %pfw\n", child);
+				continue;
+			}
+
+			lm3533_devices[count].name = "lm3533-backlight";
+			lm3533_devices[count].of_compatible = "ti,lm3533-backlight";
+			lm3533_devices[count].id = reg;
+			lm3533_devices[count].of_reg = reg;
+			lm3533_devices[count].use_of_reg = true;
+
+			lm3533->have_backlights = true;
+			count++;
+		} else if (fwnode_device_is_compatible(child, "ti,lm3533-leds")) {
+			ret = fwnode_property_read_u32(child, "reg", &reg);
+			if (ret || reg < LM3533_HVLED_ID_MAX ||
+			    reg > LM3533_LVLED_ID_MAX) {
+				dev_err(dev, "invalid LED node %pfw\n", child);
+				continue;
+			}
+
+			lm3533_devices[count].name = "lm3533-leds";
+			lm3533_devices[count].of_compatible = "ti,lm3533-leds";
+			lm3533_devices[count].id = reg - LM3533_HVLED_ID_MAX;
+			lm3533_devices[count].of_reg = reg;
+			lm3533_devices[count].use_of_reg = true;
+
+			lm3533->have_leds = true;
+			count++;
+		}
+	}
 
 	lm3533_enable(lm3533);
 
 	ret = regmap_update_bits(lm3533->regmap, LM3533_REG_BOOST_PWM,
 				 LM3533_BOOST_FREQ_MASK,
-				 pdata->boost_freq << LM3533_BOOST_FREQ_SHIFT);
+				 lm3533->boost_freq << LM3533_BOOST_FREQ_SHIFT);
 	if (ret) {
-		dev_err(lm3533->dev, "failed to set boost frequency\n");
+		dev_err(dev, "failed to set boost frequency\n");
 		goto err_disable;
 	}
 
 	ret = regmap_update_bits(lm3533->regmap, LM3533_REG_BOOST_PWM,
 				 LM3533_BOOST_OVP_MASK,
-				 pdata->boost_ovp << LM3533_BOOST_OVP_SHIFT);
+				 lm3533->boost_ovp << LM3533_BOOST_OVP_SHIFT);
 	if (ret) {
-		dev_err(lm3533->dev, "failed to set boost ovp\n");
+		dev_err(dev, "failed to set boost ovp\n");
 		goto err_disable;
 	}
 
-	lm3533_device_als_init(lm3533);
-	lm3533_device_bl_init(lm3533);
-	lm3533_device_led_init(lm3533);
+	ret = mfd_add_devices(dev, 0, lm3533_devices, count, NULL, 0, NULL);
+	if (ret) {
+		dev_err(dev, "failed to add MFD devices: %d\n", ret);
+		goto err_disable;
+	}
 
 	return 0;
 
@@ -504,7 +440,26 @@ static int lm3533_i2c_probe(struct i2c_client *i2c)
 		return PTR_ERR(lm3533->regmap);
 
 	lm3533->dev = &i2c->dev;
-	lm3533->irq = i2c->irq;
+
+	lm3533->hwen = devm_gpiod_get_optional(lm3533->dev, "enable",
+					       GPIOD_OUT_LOW);
+	if (IS_ERR(lm3533->hwen))
+		return dev_err_probe(lm3533->dev, PTR_ERR(lm3533->hwen),
+				     "failed to get HWEN GPIO\n");
+
+	device_property_read_u32(lm3533->dev, "ti,boost-ovp-microvolt",
+				 &lm3533->boost_ovp);
+
+	lm3533->boost_ovp = clamp(lm3533->boost_ovp, LM3533_BOOST_OVP_MIN,
+				  LM3533_BOOST_OVP_MAX);
+	lm3533->boost_ovp = lm3533->boost_ovp / (8 * MICRO) - 2;
+
+	device_property_read_u32(lm3533->dev, "ti,boost-freq-hz",
+				 &lm3533->boost_freq);
+
+	lm3533->boost_freq = clamp(lm3533->boost_freq, LM3533_BOOST_FREQ_MIN,
+				   LM3533_BOOST_FREQ_MAX);
+	lm3533->boost_freq = lm3533->boost_freq / (500 * KILO) - 1;
 
 	return lm3533_device_init(lm3533);
 }
@@ -518,6 +473,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" },
 	{ }
@@ -528,6 +489,7 @@ static struct i2c_driver lm3533_i2c_driver = {
 	.driver = {
 		   .name = "lm3533",
 		   .dev_groups = lm3533_attribute_groups,
+		   .of_match_table = lm3533_match_table,
 	},
 	.id_table	= lm3533_i2c_ids,
 	.probe		= lm3533_i2c_probe,
diff --git a/drivers/mfd/lm3533-ctrlbank.c b/drivers/mfd/lm3533-ctrlbank.c
index 91e13cfa3cf0..3aab8ece4e8c 100644
--- a/drivers/mfd/lm3533-ctrlbank.c
+++ b/drivers/mfd/lm3533-ctrlbank.c
@@ -13,11 +13,6 @@
 
 #include <linux/mfd/lm3533.h>
 
-
-#define LM3533_MAX_CURRENT_MIN		5000
-#define LM3533_MAX_CURRENT_MAX		29800
-#define LM3533_MAX_CURRENT_STEP		800
-
 #define LM3533_PWM_MAX			0x3f
 
 #define LM3533_REG_PWM_BASE		0x14
diff --git a/drivers/video/backlight/lm3533_bl.c b/drivers/video/backlight/lm3533_bl.c
index 9ef171d3aaea..2c24647fc17a 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/regmap.h>
 #include <linux/slab.h>
@@ -29,6 +31,9 @@ struct lm3533_bl {
 	struct backlight_device *bd;
 	int id;
 
+	u32 max_current;
+	u32 pwm;
+
 	bool have_als;
 };
 
@@ -242,25 +247,25 @@ static const struct attribute_group *lm3533_bl_attribute_groups[] = {
 	NULL,
 };
 
-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 ret;
 
-	ret = lm3533_ctrlbank_set_max_current(&bl->cb, pdata->max_current);
+	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;
+	char *name = NULL;
+	u32 default_brightness = LM3533_BL_MAX_BRIGHTNESS;
 	int ret;
 
 	dev_dbg(&pdev->dev, "%s\n", __func__);
@@ -269,12 +274,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;
@@ -292,13 +291,21 @@ static int lm3533_bl_probe(struct platform_device *pdev)
 	bl->cb.id = lm3533_bl_get_ctrlbank_id(bl);
 	bl->cb.dev = NULL;			/* until registered */
 
+	name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s-%d",
+			      pdev->name, pdev->id);
+	if (!name)
+		return -ENOMEM;
+
+	device_property_read_u32(&pdev->dev, "default-brightness",
+				 &default_brightness);
+
 	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 = default_brightness;
+
+	bd = devm_backlight_device_register(&pdev->dev, name, &pdev->dev,
+					    bl, &lm3533_bl_ops, &props);
 	if (IS_ERR(bd)) {
 		dev_err(&pdev->dev, "failed to register backlight device\n");
 		return PTR_ERR(bd);
@@ -309,12 +316,19 @@ static int lm3533_bl_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, bl);
 
-	backlight_update_status(bd);
+	device_property_read_u32(&pdev->dev, "led-max-microamp",
+				 &bl->max_current);
+	bl->max_current = clamp(bl->max_current, LM3533_MAX_CURRENT_MIN,
+				LM3533_MAX_CURRENT_MAX);
 
-	ret = lm3533_bl_setup(bl, pdata);
+	device_property_read_u32(&pdev->dev, "ti,pwm-config-mask", &bl->pwm);
+
+	ret = lm3533_bl_setup(bl);
 	if (ret)
 		return ret;
 
+	backlight_update_status(bd);
+
 	ret = lm3533_ctrlbank_enable(&bl->cb);
 	if (ret)
 		return ret;
@@ -366,11 +380,18 @@ 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_bl_match_table);
+
 static struct platform_driver lm3533_bl_driver = {
 	.driver = {
 		.name	= "lm3533-backlight",
 		.pm	= &lm3533_bl_pm_ops,
 		.dev_groups = lm3533_bl_attribute_groups,
+		.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 74cfb52c3bd1..5710e88da45d 100644
--- a/include/linux/mfd/lm3533.h
+++ b/include/linux/mfd/lm3533.h
@@ -15,6 +15,10 @@
 #define LM3533_ATTR_RW(_name) \
 	DEVICE_ATTR(_name, S_IRUGO | S_IWUSR , show_##_name, store_##_name)
 
+#define LM3533_MAX_CURRENT_MIN		5000
+#define LM3533_MAX_CURRENT_MAX		29800
+#define LM3533_MAX_CURRENT_STEP		800
+
 struct device;
 struct gpio_desc;
 struct regmap;
@@ -25,7 +29,9 @@ struct lm3533 {
 	struct regmap *regmap;
 
 	struct gpio_desc *hwen;
-	int irq;
+
+	u32 boost_ovp;
+	u32 boost_freq;
 
 	unsigned have_als:1;
 	unsigned have_backlights:1;
@@ -38,50 +44,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,
-};
-
-enum lm3533_boost_ovp {
-	LM3533_BOOST_OVP_16V,
-	LM3533_BOOST_OVP_24V,
-	LM3533_BOOST_OVP_32V,
-	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;
-};
-
 int lm3533_ctrlbank_enable(struct lm3533_ctrlbank *cb);
 int lm3533_ctrlbank_disable(struct lm3533_ctrlbank *cb);
 
-- 
2.53.0

[PATCH v5 09/14] mfd: lm3533: Add support for VIN power supply

[Svyatoslav Ryhel]

Add support for 2.7V-5.5V VIN power supply.

Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 drivers/mfd/lm3533-core.c  | 23 +++++++++++++++++++++--
 include/linux/mfd/lm3533.h |  2 ++
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/lm3533-core.c b/drivers/mfd/lm3533-core.c
index a5aa7da9668b..4b5d94e9ed27 100644
--- a/drivers/mfd/lm3533-core.c
+++ b/drivers/mfd/lm3533-core.c
@@ -17,6 +17,7 @@
 #include <linux/mod_devicetable.h>
 #include <linux/property.h>
 #include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
 #include <linux/seq_file.h>
 #include <linux/slab.h>
 #include <linux/uaccess.h>
@@ -113,14 +114,25 @@ static int lm3533_set_lvled_config(struct lm3533 *lm3533, u8 lvled, u8 led)
 	return ret;
 }
 
-static void lm3533_enable(struct lm3533 *lm3533)
+static int lm3533_enable(struct lm3533 *lm3533)
 {
+	int ret;
+
+	ret = regulator_enable(lm3533->vin_supply);
+	if (ret) {
+		dev_err(lm3533->dev, "failed to enable vin power supply\n");
+		return ret;
+	}
+
 	gpiod_set_value(lm3533->hwen, 1);
+
+	return 0;
 }
 
 static void lm3533_disable(struct lm3533 *lm3533)
 {
 	gpiod_set_value(lm3533->hwen, 0);
+	regulator_disable(lm3533->vin_supply);
 }
 
 enum lm3533_attribute_type {
@@ -333,7 +345,9 @@ static int lm3533_device_init(struct lm3533 *lm3533)
 		}
 	}
 
-	lm3533_enable(lm3533);
+	ret = lm3533_enable(lm3533);
+	if (ret)
+		return ret;
 
 	ret = regmap_update_bits(lm3533->regmap, LM3533_REG_BOOST_PWM,
 				 LM3533_BOOST_FREQ_MASK,
@@ -447,6 +461,11 @@ static int lm3533_i2c_probe(struct i2c_client *i2c)
 		return dev_err_probe(lm3533->dev, PTR_ERR(lm3533->hwen),
 				     "failed to get HWEN GPIO\n");
 
+	lm3533->vin_supply = devm_regulator_get(lm3533->dev, "vin");
+	if (IS_ERR(lm3533->vin_supply))
+		return dev_err_probe(lm3533->dev, PTR_ERR(lm3533->vin_supply),
+				     "failed to get vin-supply\n");
+
 	device_property_read_u32(lm3533->dev, "ti,boost-ovp-microvolt",
 				 &lm3533->boost_ovp);
 
diff --git a/include/linux/mfd/lm3533.h b/include/linux/mfd/lm3533.h
index 5710e88da45d..8f72dd41e8f0 100644
--- a/include/linux/mfd/lm3533.h
+++ b/include/linux/mfd/lm3533.h
@@ -22,6 +22,7 @@
 struct device;
 struct gpio_desc;
 struct regmap;
+struct regulator;
 
 struct lm3533 {
 	struct device *dev;
@@ -29,6 +30,7 @@ struct lm3533 {
 	struct regmap *regmap;
 
 	struct gpio_desc *hwen;
+	struct regulator *vin_supply;
 
 	u32 boost_ovp;
 	u32 boost_freq;
-- 
2.53.0

[PATCH v5 10/14] mfd: lm3533: Set DMA mask

[Svyatoslav Ryhel]

Missing coherent_dma_mask assigning triggers the following warning in
dmesg:

[    3.287872] platform lm3533-backlight.0: DMA mask not set

Since this warning might be elevated to an error in the future, set
coherent_dma_mask to zero because both the core and cells do not utilize
DMA.

Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 drivers/mfd/lm3533-core.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/mfd/lm3533-core.c b/drivers/mfd/lm3533-core.c
index 4b5d94e9ed27..db8581d1b073 100644
--- a/drivers/mfd/lm3533-core.c
+++ b/drivers/mfd/lm3533-core.c
@@ -480,6 +480,10 @@ static int lm3533_i2c_probe(struct i2c_client *i2c)
 				   LM3533_BOOST_FREQ_MAX);
 	lm3533->boost_freq = lm3533->boost_freq / (500 * KILO) - 1;
 
+	/* LM3533 and child devices do not use DMA */
+	i2c->dev.coherent_dma_mask = 0;
+	i2c->dev.dma_mask = &i2c->dev.coherent_dma_mask;
+
 	return lm3533_device_init(lm3533);
 }
 
-- 
2.53.0

[PATCH v5 11/14] video: backlight: lm3533_bl: Improve logic of sysfs functions

[Svyatoslav Ryhel]

Simplify the sysfs logic of properties by switching to macros and proper
regmap helpers.

Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
Reviewed-by: Daniel Thompson (RISCstar) <danielt@kernel.org>
---
 drivers/video/backlight/lm3533_bl.c | 62 ++++++++++-------------------
 1 file changed, 22 insertions(+), 40 deletions(-)

diff --git a/drivers/video/backlight/lm3533_bl.c b/drivers/video/backlight/lm3533_bl.c
index 2c24647fc17a..9eb0db640948 100644
--- a/drivers/video/backlight/lm3533_bl.c
+++ b/drivers/video/backlight/lm3533_bl.c
@@ -23,6 +23,8 @@
 #define LM3533_BL_MAX_BRIGHTNESS	255
 
 #define LM3533_REG_CTRLBANK_AB_BCONF	0x1a
+#define   CTRLBANK_AB_BCONF_ALS(n)	BIT(2 * (n))
+#define   CTRLBANK_AB_BCONF_MODE(n)	BIT(2 * (n) + 1)
 
 
 struct lm3533_bl {
@@ -85,88 +87,68 @@ static ssize_t show_als_channel(struct device *dev,
 	return scnprintf(buf, PAGE_SIZE, "%u\n", channel);
 }
 
-static ssize_t show_als_en(struct device *dev,
-				struct device_attribute *attr, char *buf)
+static ssize_t show_als_en(struct device *dev, struct device_attribute *attr,
+			   char *buf)
 {
 	struct lm3533_bl *bl = dev_get_drvdata(dev);
 	int ctrlbank = lm3533_bl_get_ctrlbank_id(bl);
-	u32 val;
-	u8 mask;
-	bool enable;
 	int ret;
 
-	ret = regmap_read(bl->regmap, LM3533_REG_CTRLBANK_AB_BCONF, &val);
-	if (ret)
+	ret = regmap_test_bits(bl->regmap, LM3533_REG_CTRLBANK_AB_BCONF,
+			       CTRLBANK_AB_BCONF_ALS(ctrlbank));
+	if (ret < 0)
 		return ret;
 
-	mask = 1 << (2 * ctrlbank);
-	enable = val & mask;
-
-	return scnprintf(buf, PAGE_SIZE, "%d\n", enable);
+	return scnprintf(buf, PAGE_SIZE, "%d\n", ret);
 }
 
-static ssize_t store_als_en(struct device *dev,
-					struct device_attribute *attr,
-					const char *buf, size_t len)
+static ssize_t store_als_en(struct device *dev, struct device_attribute *attr,
+			    const char *buf, size_t len)
 {
 	struct lm3533_bl *bl = dev_get_drvdata(dev);
 	int ctrlbank = lm3533_bl_get_ctrlbank_id(bl);
 	int enable;
-	u8 mask;
 	int ret;
 
 	if (kstrtoint(buf, 0, &enable))
 		return -EINVAL;
 
-	mask = 1 << (2 * ctrlbank);
-
 	ret = regmap_assign_bits(bl->regmap, LM3533_REG_CTRLBANK_AB_BCONF,
-				 mask, enable);
+				 CTRLBANK_AB_BCONF_ALS(ctrlbank), enable);
 	if (ret)
 		return ret;
 
 	return len;
 }
 
-static ssize_t show_linear(struct device *dev,
-				struct device_attribute *attr, char *buf)
+static ssize_t show_linear(struct device *dev, struct device_attribute *attr,
+			   char *buf)
 {
 	struct lm3533_bl *bl = dev_get_drvdata(dev);
-	u32 val;
-	u8 mask;
-	int linear;
+	int ctrlbank = lm3533_bl_get_ctrlbank_id(bl);
 	int ret;
 
-	ret = regmap_read(bl->regmap, LM3533_REG_CTRLBANK_AB_BCONF, &val);
-	if (ret)
+	ret = regmap_test_bits(bl->regmap, LM3533_REG_CTRLBANK_AB_BCONF,
+			       CTRLBANK_AB_BCONF_MODE(ctrlbank));
+	if (ret < 0)
 		return ret;
 
-	mask = 1 << (2 * lm3533_bl_get_ctrlbank_id(bl) + 1);
-
-	if (val & mask)
-		linear = 1;
-	else
-		linear = 0;
-
-	return scnprintf(buf, PAGE_SIZE, "%x\n", linear);
+	return scnprintf(buf, PAGE_SIZE, "%x\n", ret);
 }
 
-static ssize_t store_linear(struct device *dev,
-					struct device_attribute *attr,
-					const char *buf, size_t len)
+static ssize_t store_linear(struct device *dev, struct device_attribute *attr,
+			    const char *buf, size_t len)
 {
 	struct lm3533_bl *bl = dev_get_drvdata(dev);
+	int ctrlbank = lm3533_bl_get_ctrlbank_id(bl);
 	unsigned long linear;
-	u8 mask;
 	int ret;
 
 	if (kstrtoul(buf, 0, &linear))
 		return -EINVAL;
 
-	mask = 1 << (2 * lm3533_bl_get_ctrlbank_id(bl) + 1);
-
 	ret = regmap_assign_bits(bl->regmap, LM3533_REG_CTRLBANK_AB_BCONF,
-				 mask, linear);
+				 CTRLBANK_AB_BCONF_MODE(ctrlbank), linear);
 	if (ret)
 		return ret;
 
-- 
2.53.0

[PATCH v5 12/14] video: backlight: lm3533_bl: Set initial mapping mode from DT

[Svyatoslav Ryhel]

Add support to obtain the initial mapping mode from DT instead of leaving
it unconfigured.

Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
Reviewed-by: Daniel Thompson (RISCstar) <danielt@kernel.org>
---
 drivers/video/backlight/lm3533_bl.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/video/backlight/lm3533_bl.c b/drivers/video/backlight/lm3533_bl.c
index 9eb0db640948..d003d5802508 100644
--- a/drivers/video/backlight/lm3533_bl.c
+++ b/drivers/video/backlight/lm3533_bl.c
@@ -37,6 +37,7 @@ struct lm3533_bl {
 	u32 pwm;
 
 	bool have_als;
+	bool linear;
 };
 
 
@@ -231,8 +232,14 @@ static const struct attribute_group *lm3533_bl_attribute_groups[] = {
 
 static int lm3533_bl_setup(struct lm3533_bl *bl)
 {
+	int ctrlbank = lm3533_bl_get_ctrlbank_id(bl);
 	int ret;
 
+	ret = regmap_assign_bits(bl->regmap, LM3533_REG_CTRLBANK_AB_BCONF,
+				 CTRLBANK_AB_BCONF_MODE(ctrlbank), bl->linear);
+	if (ret)
+		return ret;
+
 	ret = lm3533_ctrlbank_set_max_current(&bl->cb, bl->max_current);
 	if (ret)
 		return ret;
@@ -286,6 +293,9 @@ static int lm3533_bl_probe(struct platform_device *pdev)
 	props.max_brightness = LM3533_BL_MAX_BRIGHTNESS;
 	props.brightness = default_brightness;
 
+	bl->linear = device_property_read_bool(&pdev->dev,
+					       "ti,linear-mapping-mode");
+
 	bd = devm_backlight_device_register(&pdev->dev, name, &pdev->dev,
 					    bl, &lm3533_bl_ops, &props);
 	if (IS_ERR(bd)) {
-- 
2.53.0

[PATCH v5 13/14] video: backlight: lm3533_bl: Implement backlight_scale property

[Svyatoslav Ryhel]

Since the device supports linear and non-linear modes, implement the
backlight_scale property to describe this state.

Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
Reviewed-by: Daniel Thompson (RISCstar) <danielt@kernel.org>
---
 drivers/video/backlight/lm3533_bl.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/video/backlight/lm3533_bl.c b/drivers/video/backlight/lm3533_bl.c
index d003d5802508..c99fc68cb669 100644
--- a/drivers/video/backlight/lm3533_bl.c
+++ b/drivers/video/backlight/lm3533_bl.c
@@ -142,6 +142,7 @@ static ssize_t store_linear(struct device *dev, struct device_attribute *attr,
 {
 	struct lm3533_bl *bl = dev_get_drvdata(dev);
 	int ctrlbank = lm3533_bl_get_ctrlbank_id(bl);
+	struct backlight_device *bd = bl->bd;
 	unsigned long linear;
 	int ret;
 
@@ -153,6 +154,9 @@ static ssize_t store_linear(struct device *dev, struct device_attribute *attr,
 	if (ret)
 		return ret;
 
+	bd->props.scale = linear ? BACKLIGHT_SCALE_LINEAR :
+				   BACKLIGHT_SCALE_NON_LINEAR;
+
 	return len;
 }
 
@@ -295,6 +299,8 @@ static int lm3533_bl_probe(struct platform_device *pdev)
 
 	bl->linear = device_property_read_bool(&pdev->dev,
 					       "ti,linear-mapping-mode");
+	props.scale = bl->linear ? BACKLIGHT_SCALE_LINEAR :
+				   BACKLIGHT_SCALE_NON_LINEAR;
 
 	bd = devm_backlight_device_register(&pdev->dev, name, &pdev->dev,
 					    bl, &lm3533_bl_ops, &props);
-- 
2.53.0

[PATCH v5 14/14] video: leds: backlight: lm3533: Support getting LED sources from DT

[Svyatoslav Ryhel]

Add Control Bank to HVLED/LVLED muxing support based on the led-sources
defined in the device tree.

Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 drivers/leds/leds-lm3533.c          | 60 +++++++++++++++++++++++++++++
 drivers/video/backlight/lm3533_bl.c | 45 ++++++++++++++++++++++
 2 files changed, 105 insertions(+)

diff --git a/drivers/leds/leds-lm3533.c b/drivers/leds/leds-lm3533.c
index ed810c23f30f..9e07953814fd 100644
--- a/drivers/leds/leds-lm3533.c
+++ b/drivers/leds/leds-lm3533.c
@@ -27,6 +27,11 @@
 #define LM3533_ALS_CHANNEL_LV_MIN	1
 #define LM3533_ALS_CHANNEL_LV_MAX	2
 
+#define LM3533_REG_OUTPUT_CONF1			0x10
+#define   OUTPUT_CONF1_SHIFT			2
+#define   OUTPUT_LVLED_MASK			0x3
+#define LM3533_REG_OUTPUT_CONF2			0x11
+#define   OUTPUT_CONF2_SHIFT			6
 #define LM3533_REG_CTRLBANK_BCONF_BASE		0x1b
 #define LM3533_REG_PATTERN_ENABLE		0x28
 #define LM3533_REG_PATTERN_LOW_TIME_BASE	0x71
@@ -55,6 +60,9 @@ struct lm3533_led {
 	u32 max_current;
 	u32 pwm;
 
+	int num_leds;
+	u32 leds[LM3533_LVCTRLBANK_MAX];
+
 	bool have_als;
 };
 
@@ -623,8 +631,36 @@ static const struct attribute_group *lm3533_led_attribute_groups[] = {
 
 static int lm3533_led_setup(struct lm3533_led *led)
 {
+	u32 output_cfg_shift = 0;
+	u32 output_cfg_val = 0;
+	u32 output_cfg_mask = 0;
 	int ret;
 
+	if (led->num_leds) {
+		for (int i = 0; i < led->num_leds; i++) {
+			if (led->leds[i] >= LM3533_LVCTRLBANK_MAX)
+				continue;
+
+			output_cfg_shift = led->leds[i] * 2;
+			output_cfg_val |= led->id << output_cfg_shift;
+			output_cfg_mask |= OUTPUT_LVLED_MASK << output_cfg_shift;
+		}
+
+		/* LVLED1, LVLED2 and LVLED3 */
+		ret = regmap_update_bits(led->regmap, LM3533_REG_OUTPUT_CONF1,
+					 output_cfg_mask << OUTPUT_CONF1_SHIFT,
+					 output_cfg_val << OUTPUT_CONF1_SHIFT);
+		if (ret)
+			return ret;
+
+		/* LVLED4 and LVLED5 */
+		ret = regmap_update_bits(led->regmap, LM3533_REG_OUTPUT_CONF2,
+					 output_cfg_mask >> OUTPUT_CONF2_SHIFT,
+					 output_cfg_val >> OUTPUT_CONF2_SHIFT);
+		if (ret)
+			return ret;
+	}
+
 	ret = lm3533_ctrlbank_set_max_current(&led->cb, led->max_current);
 	if (ret)
 		return ret;
@@ -699,6 +735,30 @@ static int lm3533_led_probe(struct platform_device *pdev)
 
 	device_property_read_u32(&pdev->dev, "ti,pwm-config-mask", &led->pwm);
 
+	/*
+	 * If led-sources property is not set then either this Control Bank uses
+	 * its default LVLED or is not linked to any LVLED at all.
+	 */
+	led->num_leds = device_property_count_u32(&pdev->dev, "led-sources");
+	if (led->num_leds > LM3533_LVCTRLBANK_MAX) {
+		dev_err(&pdev->dev, "num of LED sources exceeds max %d: %d\n",
+			LM3533_LVCTRLBANK_MAX, led->num_leds);
+		ret = -EINVAL;
+		goto err_deregister;
+	}
+
+	if (led->num_leds < 0)
+		led->num_leds = 0;
+
+	if (led->num_leds > 0) {
+		ret = device_property_read_u32_array(&pdev->dev, "led-sources",
+						     led->leds, led->num_leds);
+		if (ret) {
+			dev_err(&pdev->dev, "failed to get led-sources\n");
+			goto err_deregister;
+		}
+	}
+
 	ret = lm3533_led_setup(led);
 	if (ret)
 		goto err_deregister;
diff --git a/drivers/video/backlight/lm3533_bl.c b/drivers/video/backlight/lm3533_bl.c
index c99fc68cb669..b3e5b3042d34 100644
--- a/drivers/video/backlight/lm3533_bl.c
+++ b/drivers/video/backlight/lm3533_bl.c
@@ -7,6 +7,7 @@
  * Author: Johan Hovold <jhovold@gmail.com>
  */
 
+#include <linux/bits.h>
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/mod_devicetable.h>
@@ -22,6 +23,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
 #define   CTRLBANK_AB_BCONF_ALS(n)	BIT(2 * (n))
 #define   CTRLBANK_AB_BCONF_MODE(n)	BIT(2 * (n) + 1)
@@ -36,6 +38,9 @@ struct lm3533_bl {
 	u32 max_current;
 	u32 pwm;
 
+	int num_leds;
+	u32 led_strings[LM3533_HVCTRLBANK_COUNT];
+
 	bool have_als;
 	bool linear;
 };
@@ -237,6 +242,8 @@ static const struct attribute_group *lm3533_bl_attribute_groups[] = {
 static int lm3533_bl_setup(struct lm3533_bl *bl)
 {
 	int ctrlbank = lm3533_bl_get_ctrlbank_id(bl);
+	u32 output_cfg_val = 0;
+	u32 output_cfg_mask = 0;
 	int ret;
 
 	ret = regmap_assign_bits(bl->regmap, LM3533_REG_CTRLBANK_AB_BCONF,
@@ -244,6 +251,21 @@ static int lm3533_bl_setup(struct lm3533_bl *bl)
 	if (ret)
 		return ret;
 
+	if (bl->num_leds) {
+		for (int i = 0; i < bl->num_leds; i++) {
+			if (bl->led_strings[i] >= LM3533_HVCTRLBANK_COUNT)
+				continue;
+
+			output_cfg_val |= ctrlbank << bl->led_strings[i];
+			output_cfg_mask |= BIT(bl->led_strings[i]);
+		}
+
+		ret = regmap_update_bits(bl->regmap, LM3533_REG_OUTPUT_CONF1,
+					 output_cfg_mask, output_cfg_val);
+		if (ret)
+			return ret;
+	}
+
 	ret = lm3533_ctrlbank_set_max_current(&bl->cb, bl->max_current);
 	if (ret)
 		return ret;
@@ -321,6 +343,29 @@ static int lm3533_bl_probe(struct platform_device *pdev)
 
 	device_property_read_u32(&pdev->dev, "ti,pwm-config-mask", &bl->pwm);
 
+	/*
+	 * If led-sources property is not set then either this Control Bank uses
+	 * its default HVLED or is not linked to any HVLED at all.
+	 */
+	bl->num_leds = device_property_count_u32(&pdev->dev, "led-sources");
+	if (bl->num_leds > LM3533_HVCTRLBANK_COUNT) {
+		dev_err(&pdev->dev, "num of LED sources %d exceeds max %d\n",
+			bl->num_leds, LM3533_HVCTRLBANK_COUNT);
+		return -EINVAL;
+	}
+
+	if (bl->num_leds < 0)
+		bl->num_leds = 0;
+
+	if (bl->num_leds > 0) {
+		ret = device_property_read_u32_array(&pdev->dev, "led-sources",
+						     bl->led_strings,
+						     bl->num_leds);
+		if (ret)
+			return dev_err_probe(&pdev->dev, ret,
+					     "failed to get led-sources\n");
+	}
+
 	ret = lm3533_bl_setup(bl);
 	if (ret)
 		return ret;
-- 
2.53.0

Re: [PATCH v5 02/14] mfd: lm3533: Remove driver specific regmap wrappers

[Andy Shevchenko]

On Wed, Jun 17, 2026 at 11:00:19AM +0300, Svyatoslav Ryhel wrote:
> Remove driver-specific regmap wrappers in favor of using regmap helpers
> directly.

OK, let's go with this variant.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>

Some side notes below for the record.

...

>  	struct lm3533_led *led = to_lm3533_led(led_cdev);
>  	unsigned enable;

Oh, besides using the old way of declaring unsigned int, it most likely
just needs to be kstrtobool().

>  	u8 reg;
> -	u8 mask;
> -	u8 val;
>  	int ret;
>  
>  	if (kstrtouint(buf, 0, &enable))
>  		return -EINVAL;

We should unshadow error codes (it may return more than -EINVAL).

>  	reg = lm3533_led_get_lv_reg(led, LM3533_REG_CTRLBANK_BCONF_BASE);
> -	mask = LM3533_REG_CTRLBANK_BCONF_ALS_EN_MASK;
>  
> -	if (enable)
> -		val = mask;
> -	else
> -		val = 0;
> -

...

> -	if (kstrtou8(buf, 0, &val))
> +	if (kstrtou32(buf, 0, &val))
>  		return -EINVAL;

Like in the previous case we should unshadow error codes.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 14/14] video: leds: backlight: lm3533: Support getting LED sources from DT

[Daniel Thompson]

On Wed, Jun 17, 2026 at 11:00:31AM +0300, Svyatoslav Ryhel wrote:
> Add Control Bank to HVLED/LVLED muxing support based on the led-sources
> defined in the device tree.
>
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>

Reviewed-by: Daniel Thompson (RISCstar) <danielt@kernel.org>


Daniel.

Re: [PATCH v5 07/14] mfd: lm3533: Use dev_groups in struct device_driver

[Andy Shevchenko]

On Wed, Jun 17, 2026 at 11:00:24AM +0300, Svyatoslav Ryhel wrote:
> Instead of creating and removing the device sysfs attributes directly
> during probe and remove of the driver, respectively, use dev_groups in
> struct device_driver to point to the attribute definitions and let the
> core take care of creating and removing them.
> 
> No intentional functional impact.

Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
And thanks for doing that!

...

>  	.attrs		= lm3533_attributes
>  };
>  
> +static const struct attribute_group *lm3533_attribute_groups[] = {
> +	&lm3533_attribute_group,
> +	NULL,
> +};

We have ATTRIBUTE_GROUPS() macro.

...

> +++ b/drivers/video/backlight/lm3533_bl.c

Same as per above.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 07/14] mfd: lm3533: Use dev_groups in struct device_driver

[Andy Shevchenko]

On Thu, Jun 25, 2026 at 10:24:00AM +0300, Andy Shevchenko wrote:
> On Wed, Jun 17, 2026 at 11:00:24AM +0300, Svyatoslav Ryhel wrote:

...

> >  	.attrs		= lm3533_attributes
> >  };
> >  
> > +static const struct attribute_group *lm3533_attribute_groups[] = {
> > +	&lm3533_attribute_group,
> > +	NULL,
> > +};
> 
> We have ATTRIBUTE_GROUPS() macro.

Okay, it uses is_visible, so __ATTRIBUTE_GROUPS() that we can still use.

...

> > +++ b/drivers/video/backlight/lm3533_bl.c
> 
> Same as per above.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 02/14] mfd: lm3533: Remove driver specific regmap wrappers

[Lee Jones]

On Wed, 17 Jun 2026, Andy Shevchenko wrote:

> On Wed, Jun 17, 2026 at 11:00:19AM +0300, Svyatoslav Ryhel wrote:
> > Remove driver-specific regmap wrappers in favor of using regmap helpers
> > directly.
> 
> OK, let's go with this variant.
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> 
> Some side notes below for the record.
> 
> ...
> 
> >  	struct lm3533_led *led = to_lm3533_led(led_cdev);
> >  	unsigned enable;
> 
> Oh, besides using the old way of declaring unsigned int, it most likely
> just needs to be kstrtobool().
> 
> >  	u8 reg;
> > -	u8 mask;
> > -	u8 val;
> >  	int ret;
> >  
> >  	if (kstrtouint(buf, 0, &enable))
> >  		return -EINVAL;
> 
> We should unshadow error codes (it may return more than -EINVAL).

Right, this masks -ERANGE.

We should fix that.

> >  	reg = lm3533_led_get_lv_reg(led, LM3533_REG_CTRLBANK_BCONF_BASE);
> > -	mask = LM3533_REG_CTRLBANK_BCONF_ALS_EN_MASK;
> >  
> > -	if (enable)
> > -		val = mask;
> > -	else
> > -		val = 0;
> > -
> 
> ...
> 
> > -	if (kstrtou8(buf, 0, &val))
> > +	if (kstrtou32(buf, 0, &val))
> >  		return -EINVAL;
> 
> Like in the previous case we should unshadow error codes.
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

-- 
Lee Jones

Re: [PATCH v5 00/14] mfd: lm3533: convert to OF bindings, improve support

[Lee Jones]

On Wed, 17 Jun 2026, Svyatoslav Ryhel wrote:

> Convert LM3533 to OF bindings, add missing VIN supply, add support for
> setting mapping mode and LED sources based on device tree. 
> 
> ---
> Changes in v2:
> 
> schema
> - maximum led sources for leds set to 4
> - anyOf > oneOf in ALS
> - improved ALS descriptions
> - adjusted example
> drivers
> - dropped devm convertion of irq and mfd helpers
> - all als configuration moved into lm3533_als_setup
> - added regulator/consumer.h
> - lm3533_bl_setup set before sysfs_create_group in backlight
> - added check if LVLED is valid
> - LM3533_REG_OUTPUT_CONF1 > LM3533_REG_OUTPUT_CONF2 for LVLED4 and LVLED5

This set looks good to me now.

Let me know when you have all of the Acks and I'll merge it via MFD.

-- 
Lee Jones

Re: [PATCH v5 05/14] iio: light: lm3533-als: Remove redundant pdata helpers

[Jonathan Cameron]

On Wed, 17 Jun 2026 11:00:22 +0300
Svyatoslav Ryhel <clamor95@gmail.com> wrote:

> The lm3533_als_set_input_mode() and lm3533_als_set_resistor() functions
> are used only in lm3533_als_setup(). Incorporate their code into
> lm3533_als_setup() directly to simplify driver readability.
> 
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
LGTM
Reviewed-by: Jonathan Cameron <jic23@kernel.org>

Re: [PATCH v5 00/14] mfd: lm3533: convert to OF bindings, improve support

[Svyatoslav Ryhel]

чт, 2 лип. 2026 р. о 19:18 Lee Jones <lee@kernel.org> пише:
>
> On Wed, 17 Jun 2026, Svyatoslav Ryhel wrote:
>
> > Convert LM3533 to OF bindings, add missing VIN supply, add support for
> > setting mapping mode and LED sources based on device tree.
> >
> > ---
> > Changes in v2:
> >
> > schema
> > - maximum led sources for leds set to 4
> > - anyOf > oneOf in ALS
> > - improved ALS descriptions
> > - adjusted example
> > drivers
> > - dropped devm convertion of irq and mfd helpers
> > - all als configuration moved into lm3533_als_setup
> > - added regulator/consumer.h
> > - lm3533_bl_setup set before sysfs_create_group in backlight
> > - added check if LVLED is valid
> > - LM3533_REG_OUTPUT_CONF1 > LM3533_REG_OUTPUT_CONF2 for LVLED4 and LVLED5
>
> This set looks good to me now.
>
> Let me know when you have all of the Acks and I'll merge it via MFD.
>

Hello there!

Jonathan and Daniel added their Reviewed-by for IIO and backlight
subsystems, so I assume that should be all required.

Best regards,
Svyatoslav R.

> --
> Lee Jones

Re: [PATCH v5 02/14] mfd: lm3533: Remove driver specific regmap wrappers

[Johan Hovold]

On Wed, Jun 17, 2026 at 11:00:19AM +0300, Svyatoslav Ryhel wrote:
> Remove driver-specific regmap wrappers in favor of using regmap helpers
> directly.

The commit message should explain why you think this is a good idea.

Based on a quick look you lose the errnos that were logged on errors and
also the register debugging provided by those helpers.

Johan

Re: [PATCH v5 01/14] dt-bindings: leds: Document TI LM3533 LED controller

[Johan Hovold]

On Wed, Jun 17, 2026 at 11:00:18AM +0300, Svyatoslav Ryhel wrote:
> Document 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>
> Reviewed-by: Jonathan Cameron <jic23@kernel.org> #for light sensor
> Reviewed-by: Daniel Thompson (RISCstar) <danielt@kernel.org> #for backlight
> Reviewed-by: Rob Herring (Arm) <robh@kernel.org>

> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        led-controller@36 {
> +            compatible = "ti,lm3533";
> +            reg = <0x36>;
> +
> +            enable-gpios = <&gpio 110 GPIO_ACTIVE_HIGH>;
> +            vin-supply = <&vdd_3v3_bat>;
> +
> +            ti,boost-ovp-microvolt = <24000000>;
> +            ti,boost-freq-hz = <500000>;
> +
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            backlight@0 {
> +                compatible = "ti,lm3533-backlight";
> +                reg = <0>;
> +
> +                default-brightness = <113>;
> +
> +                led-max-microamp = <23400>;
> +                led-sources = <0 1>;
> +            };
> +
> +            led@2 {
> +                compatible = "ti,lm3533-leds";
> +                reg = <2>;
> +
> +                led-max-microamp = <23400>;
> +                led-sources = <0 1>;
> +            };
> +
> +            led@4 {
> +                compatible = "ti,lm3533-leds";
> +                reg = <4>;
> +
> +                led-max-microamp = <23400>;
> +                led-sources = <2>;
> +            };
> +
> +            led@5 {
> +                compatible = "ti,lm3533-leds";
> +                reg = <5>;
> +
> +                led-max-microamp = <23400>;
> +                led-sources = <3 4>;
> +            };
> +
> +            light-sensor {

Doesn't the binding (or dts) checker complain about this node not having
an address?

> +                compatible = "ti,lm3533-als";
> +
> +                interrupt-parent = <&gpio>;
> +                interrupts = <80 IRQ_TYPE_LEVEL_LOW>;
> +
> +                ti,pwm-mode;
> +            };
> +        };
> +    };
> +...

Johan

Re: [PATCH v5 04/14] mfd: lm3533: Pass only regmap and light sensor presence to child devices

[Johan Hovold]

On Wed, Jun 17, 2026 at 11:00:21AM +0300, Svyatoslav Ryhel wrote:
> Instead of passing the entire lm3533 core data structure, only pass the
> regmap and the light sensor presence flag to child devices.

Again, why?

Johan

Re: [PATCH v5 05/14] iio: light: lm3533-als: Remove redundant pdata helpers

[Johan Hovold]

On Wed, Jun 17, 2026 at 11:00:22AM +0300, Svyatoslav Ryhel wrote:
> The lm3533_als_set_input_mode() and lm3533_als_set_resistor() functions
> are used only in lm3533_als_setup(). Incorporate their code into
> lm3533_als_setup() directly to simplify driver readability.

That's a debatable claim.

> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
>  drivers/iio/light/lm3533-als.c | 56 ++++++++++------------------------
>  1 file changed, 16 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/iio/light/lm3533-als.c b/drivers/iio/light/lm3533-als.c
> index a9af8e2b965f..69bac1b202f1 100644
> --- a/drivers/iio/light/lm3533-als.c
> +++ b/drivers/iio/light/lm3533-als.c

> -static int lm3533_als_setup(struct lm3533_als *als,
> -			    const struct lm3533_als_platform_data *pdata)
> -{
> -	int ret;
> -
> -	ret = lm3533_als_set_input_mode(als, pdata->pwm_mode);
> +			   pdata->r_select);
>  	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 (ret)
> -			return ret;
> -	}
> +		return dev_err_probe(dev, ret, "failed to set resistor\n");
>  	return 0;
>  }

There's nothing hard to read about the above. To the contrary the logic
is more obvious this way.

You also remove the high-impedance comment for no good reason.

Johan

Re: [PATCH v5 06/14] mfd: lm3533-core: Remove redundant pdata helpers

[Johan Hovold]

On Wed, Jun 17, 2026 at 11:00:23AM +0300, Svyatoslav Ryhel wrote:
> The lm3533_set_boost_freq() and lm3533_set_boost_ovp() functions are used
> only in lm3533_device_setup(), which in turn is only called by
> lm3533_device_init(). Incorporate their code directly into
> lm3533_device_init() to simplify driver readability.

Again, this isn't needed. Why are you removing abstraction that improve
readability?

Johan

Re: [PATCH v5 07/14] mfd: lm3533: Use dev_groups in struct device_driver

[Johan Hovold]

On Wed, Jun 17, 2026 at 11:00:24AM +0300, Svyatoslav Ryhel wrote:
> Instead of creating and removing the device sysfs attributes directly
> during probe and remove of the driver, respectively, use dev_groups in
> struct device_driver to point to the attribute definitions and let the
> core take care of creating and removing them.
> 
> No intentional functional impact.
> 
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>

Reviewed-by: Johan Hovold <johan@kernel.org>

Re: [PATCH v5 08/14] mfd: lm3533: Convert to use OF bindings

[Johan Hovold]

On Wed, Jun 17, 2026 at 11:00:25AM +0300, Svyatoslav Ryhel wrote:
> Since there are no users of this driver via platform data, remove the
> platform data support and switch to using Device Tree bindings.
> 
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> Reviewed-by: Daniel Thompson (RISCstar) <danielt@kernel.org> #for backlight
> ---
>  drivers/iio/light/lm3533-als.c      |  67 +++++---
>  drivers/leds/leds-lm3533.c          |  50 ++++--
>  drivers/mfd/lm3533-core.c           | 236 ++++++++++++----------------
>  drivers/mfd/lm3533-ctrlbank.c       |   5 -
>  drivers/video/backlight/lm3533_bl.c |  55 +++++--
>  include/linux/mfd/lm3533.h          |  52 +-----
>  6 files changed, 220 insertions(+), 245 deletions(-)
 
>  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;
> @@ -803,12 +817,6 @@ static int lm3533_als_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;
> -	}
> -
>  	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*als));
>  	if (!indio_dev)
>  		return -ENOMEM;
> @@ -817,25 +825,27 @@ static int lm3533_als_probe(struct platform_device *pdev)
>  	indio_dev->channels = lm3533_als_channels;
>  	indio_dev->num_channels = ARRAY_SIZE(lm3533_als_channels);
>  	indio_dev->name = dev_name(&pdev->dev);
> -	iio_device_set_parent(indio_dev, pdev->dev.parent);

Why are you reparenting the iio device here? 

That's an ABI break.

> +static const struct of_device_id lm3533_als_match_table[] = {
> +	{ .compatible = "ti,lm3533-als" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, lm3533_als_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,

You should also remove the platform module alias below.

> diff --git a/drivers/leds/leds-lm3533.c b/drivers/leds/leds-lm3533.c
> index 0cb0585eb960..ed810c23f30f 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/regmap.h>
>  #include <linux/slab.h>
>  
> @@ -50,6 +52,9 @@ struct lm3533_led {
>  	struct mutex mutex;
>  	unsigned long flags;
>  
> +	u32 max_current;
> +	u32 pwm;
> +
>  	bool have_als;
>  };
>  
> @@ -616,22 +621,20 @@ 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;
>  	int ret;
>  
> @@ -641,12 +644,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;
> @@ -659,8 +656,6 @@ static int lm3533_led_probe(struct platform_device *pdev)
>  	led->regmap = lm3533->regmap;
>  	led->have_als = lm3533->have_als;
>  
> -	led->cdev.name = pdata->name;
> -	led->cdev.default_trigger = pdata->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;
> @@ -668,6 +663,15 @@ static int lm3533_led_probe(struct platform_device *pdev)
>  	led->cdev.groups = lm3533_led_attribute_groups;
>  	led->id = pdev->id;
>  
> +	led->cdev.name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s-%d",
> +					pdev->name, led->id);

Is "led-2", etc. unique enough here?

> +	if (!led->cdev.name)
> +		return -ENOMEM;
> +
> +	led->cdev.default_trigger = "none";
> +	device_property_read_string(&pdev->dev, "linux,default-trigger",
> +				    &led->cdev.default_trigger);
> +
>  	mutex_init(&led->mutex);
>  
>  	/* The class framework makes a callback to get brightness during
> @@ -680,15 +684,22 @@ static int lm3533_led_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, led);
>  
> -	ret = led_classdev_register(pdev->dev.parent, &led->cdev);
> +	ret = led_classdev_register(&pdev->dev, &led->cdev);

Here too you appear to be reparenting the class devices.

>  	if (ret) {
> -		dev_err(&pdev->dev, "failed to register LED %d\n", pdev->id);
> +		dev_err(&pdev->dev, "failed to register LED %d\n", led->id);

This does not seem to be necessary.

>  		return ret;
>  	}
>  
>  	led->cb.dev = led->cdev.dev;
>  
> -	ret = lm3533_led_setup(led, pdata);
> +	device_property_read_u32(&pdev->dev, "led-max-microamp",
> +				 &led->max_current);
> +	led->max_current = clamp(led->max_current, LM3533_MAX_CURRENT_MIN,
> +				 LM3533_MAX_CURRENT_MAX);

Why clamp instead of having lm3533_led_setup() fail below? 

> +
> +	device_property_read_u32(&pdev->dev, "ti,pwm-config-mask", &led->pwm);
> +
> +	ret = lm3533_led_setup(led);
>  	if (ret)
>  		goto err_deregister;
>  
> @@ -725,9 +736,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_led_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,

Remove platform alias below as well.

> diff --git a/drivers/mfd/lm3533-core.c b/drivers/mfd/lm3533-core.c
> index b03a3ae96c10..a5aa7da9668b 100644
> --- a/drivers/mfd/lm3533-core.c
> +++ b/drivers/mfd/lm3533-core.c
> @@ -14,19 +14,26 @@
>  #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>
>  
>  
>  #define LM3533_BOOST_OVP_MASK		0x06
>  #define LM3533_BOOST_OVP_SHIFT		1
> +#define LM3533_BOOST_OVP_MIN		(16 * MICRO)
> +#define LM3533_BOOST_OVP_MAX		(40 * MICRO)
>  
>  #define LM3533_BOOST_FREQ_MASK		0x01
>  #define LM3533_BOOST_FREQ_SHIFT		0
> +#define LM3533_BOOST_FREQ_MIN		(500 * HZ_PER_KHZ)
> +#define LM3533_BOOST_FREQ_MAX		(1000 * HZ_PER_KHZ)
>  
>  #define LM3533_BL_ID_MASK		1
>  #define LM3533_LED_ID_MASK		3
> @@ -35,6 +42,7 @@
>  
>  #define LM3533_HVLED_ID_MAX		2
>  #define LM3533_LVLED_ID_MAX		5
> +#define LM3533_CELLS_MAX		7
>  
>  #define LM3533_REG_OUTPUT_CONF1		0x10
>  #define LM3533_REG_OUTPUT_CONF2		0x11
> @@ -42,44 +50,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,
> -	},
> -};
> -
>  /*
>   * HVLED output config -- output hvled controlled by backlight bl
>   */
> @@ -301,125 +271,91 @@ static const struct attribute_group *lm3533_attribute_groups[] = {
>  	NULL,
>  };
>  
> -static int lm3533_device_als_init(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");
> -		return ret;
> -	}
> -
> -	lm3533->have_als = 1;
> -
> -	return 0;
> -}
> -
> -static int lm3533_device_bl_init(struct lm3533 *lm3533)
> -{
> -	struct lm3533_platform_data *pdata = dev_get_platdata(lm3533->dev);
> -	int i;
> -	int ret;
> -
> -	if (!pdata->backlights || pdata->num_backlights == 0)
> -		return 0;
> -
> -	if (pdata->num_backlights > ARRAY_SIZE(lm3533_bl_devs))
> -		pdata->num_backlights = ARRAY_SIZE(lm3533_bl_devs);
> -
> -	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]);
> -	}
> -
> -	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;
> -	}
> -
> -	lm3533->have_backlights = 1;
> -
> -	return 0;
> -}
> -
> -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;
> -
> -	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;
> -	}
> -
> -	lm3533->have_leds = 1;
> -
> -	return 0;
> -}
> -
>  static int lm3533_device_init(struct lm3533 *lm3533)
>  {
> -	struct lm3533_platform_data *pdata = dev_get_platdata(lm3533->dev);
> +	struct device *dev = lm3533->dev;
> +	struct mfd_cell *lm3533_devices;
> +	u32 count = 0, reg, nchilds;

Don't mix multiple declarations with initialisation like this.

>  	int ret;
>  
> -	dev_dbg(lm3533->dev, "%s\n", __func__);
> +	nchilds = device_get_child_node_count(dev);
> +	if (!nchilds || nchilds > LM3533_CELLS_MAX)
> +		return dev_err_probe(dev, -ENODEV,
> +				     "num of child nodes is not supported\n");
>  
> -	if (!pdata) {
> -		dev_err(lm3533->dev, "no platform data\n");
> -		return -EINVAL;
> -	}
> +	lm3533_devices = devm_kcalloc(dev, nchilds, sizeof(*lm3533_devices),
> +				      GFP_KERNEL);
> +	if (!lm3533_devices)
> +		return -ENOMEM;
>  
> -	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");
> +	device_for_each_child_node_scoped(dev, child) {
> +		if (count >= nchilds)
> +			break;

How could count be larger than nchilds?

> +
> +		if (fwnode_device_is_compatible(child, "ti,lm3533-als")) {
> +			lm3533_devices[count].name = "lm3533-als";
> +			lm3533_devices[count].of_compatible = "ti,lm3533-als";
> +			lm3533_devices[count].id = PLATFORM_DEVID_NONE;
> +
> +			lm3533->have_als = true;
> +			count++;
> +		} else if (fwnode_device_is_compatible(child, "ti,lm3533-backlight")) {
> +			ret = fwnode_property_read_u32(child, "reg", &reg);
> +			if (ret || reg >= LM3533_HVLED_ID_MAX) {
> +				dev_err(dev, "invalid backlight node %pfw\n", child);
> +				continue;
> +			}
> +
> +			lm3533_devices[count].name = "lm3533-backlight";
> +			lm3533_devices[count].of_compatible = "ti,lm3533-backlight";
> +			lm3533_devices[count].id = reg;
> +			lm3533_devices[count].of_reg = reg;
> +			lm3533_devices[count].use_of_reg = true;
> +
> +			lm3533->have_backlights = true;
> +			count++;
> +		} else if (fwnode_device_is_compatible(child, "ti,lm3533-leds")) {
> +			ret = fwnode_property_read_u32(child, "reg", &reg);
> +			if (ret || reg < LM3533_HVLED_ID_MAX ||
> +			    reg > LM3533_LVLED_ID_MAX) {
> +				dev_err(dev, "invalid LED node %pfw\n", child);
> +				continue;
> +			}
> +
> +			lm3533_devices[count].name = "lm3533-leds";
> +			lm3533_devices[count].of_compatible = "ti,lm3533-leds";
> +			lm3533_devices[count].id = reg - LM3533_HVLED_ID_MAX;
> +			lm3533_devices[count].of_reg = reg;
> +			lm3533_devices[count].use_of_reg = true;
> +
> +			lm3533->have_leds = true;
> +			count++;
> +		}
> +	}

Why do you need the above at all? Shouldn't you be able to just use
of_platform_populate().

>  
>  	lm3533_enable(lm3533);
>  
>  	ret = regmap_update_bits(lm3533->regmap, LM3533_REG_BOOST_PWM,
>  				 LM3533_BOOST_FREQ_MASK,
> -				 pdata->boost_freq << LM3533_BOOST_FREQ_SHIFT);
> +				 lm3533->boost_freq << LM3533_BOOST_FREQ_SHIFT);
>  	if (ret) {
> -		dev_err(lm3533->dev, "failed to set boost frequency\n");
> +		dev_err(dev, "failed to set boost frequency\n");
>  		goto err_disable;
>  	}
>  
>  	ret = regmap_update_bits(lm3533->regmap, LM3533_REG_BOOST_PWM,
>  				 LM3533_BOOST_OVP_MASK,
> -				 pdata->boost_ovp << LM3533_BOOST_OVP_SHIFT);
> +				 lm3533->boost_ovp << LM3533_BOOST_OVP_SHIFT);
>  	if (ret) {
> -		dev_err(lm3533->dev, "failed to set boost ovp\n");
> +		dev_err(dev, "failed to set boost ovp\n");
>  		goto err_disable;
>  	}
>  
> -	lm3533_device_als_init(lm3533);
> -	lm3533_device_bl_init(lm3533);
> -	lm3533_device_led_init(lm3533);
> +	ret = mfd_add_devices(dev, 0, lm3533_devices, count, NULL, 0, NULL);
> +	if (ret) {
> +		dev_err(dev, "failed to add MFD devices: %d\n", ret);
> +		goto err_disable;
> +	}
>  
>  	return 0;
>  
> @@ -504,7 +440,26 @@ static int lm3533_i2c_probe(struct i2c_client *i2c)
>  		return PTR_ERR(lm3533->regmap);
>  
>  	lm3533->dev = &i2c->dev;
> -	lm3533->irq = i2c->irq;
> +
> +	lm3533->hwen = devm_gpiod_get_optional(lm3533->dev, "enable",
> +					       GPIOD_OUT_LOW);
> +	if (IS_ERR(lm3533->hwen))
> +		return dev_err_probe(lm3533->dev, PTR_ERR(lm3533->hwen),
> +				     "failed to get HWEN GPIO\n");

Please use brackets around multline statements for readability
throughout.

> +
> +	device_property_read_u32(lm3533->dev, "ti,boost-ovp-microvolt",
> +				 &lm3533->boost_ovp);
> +
> +	lm3533->boost_ovp = clamp(lm3533->boost_ovp, LM3533_BOOST_OVP_MIN,
> +				  LM3533_BOOST_OVP_MAX);
> +	lm3533->boost_ovp = lm3533->boost_ovp / (8 * MICRO) - 2;
> +
> +	device_property_read_u32(lm3533->dev, "ti,boost-freq-hz",
> +				 &lm3533->boost_freq);
> +
> +	lm3533->boost_freq = clamp(lm3533->boost_freq, LM3533_BOOST_FREQ_MIN,
> +				   LM3533_BOOST_FREQ_MAX);
> +	lm3533->boost_freq = lm3533->boost_freq / (500 * KILO) - 1;

Again, why clamp instead of failing probe?

>  	return lm3533_device_init(lm3533);
>  }
> @@ -518,6 +473,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" },
>  	{ }
> @@ -528,6 +489,7 @@ static struct i2c_driver lm3533_i2c_driver = {
>  	.driver = {
>  		   .name = "lm3533",
>  		   .dev_groups = lm3533_attribute_groups,
> +		   .of_match_table = lm3533_match_table,
>  	},
>  	.id_table	= lm3533_i2c_ids,
>  	.probe		= lm3533_i2c_probe,
> diff --git a/drivers/mfd/lm3533-ctrlbank.c b/drivers/mfd/lm3533-ctrlbank.c
> index 91e13cfa3cf0..3aab8ece4e8c 100644
> --- a/drivers/mfd/lm3533-ctrlbank.c
> +++ b/drivers/mfd/lm3533-ctrlbank.c
> @@ -13,11 +13,6 @@
>  
>  #include <linux/mfd/lm3533.h>
>  
> -
> -#define LM3533_MAX_CURRENT_MIN		5000
> -#define LM3533_MAX_CURRENT_MAX		29800
> -#define LM3533_MAX_CURRENT_STEP		800
> -
>  #define LM3533_PWM_MAX			0x3f
>  
>  #define LM3533_REG_PWM_BASE		0x14
> diff --git a/drivers/video/backlight/lm3533_bl.c b/drivers/video/backlight/lm3533_bl.c
> index 9ef171d3aaea..2c24647fc17a 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/regmap.h>
>  #include <linux/slab.h>
> @@ -29,6 +31,9 @@ struct lm3533_bl {
>  	struct backlight_device *bd;
>  	int id;
>  
> +	u32 max_current;
> +	u32 pwm;
> +
>  	bool have_als;
>  };
>  
> @@ -242,25 +247,25 @@ static const struct attribute_group *lm3533_bl_attribute_groups[] = {
>  	NULL,
>  };
>  
> -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 ret;
>  
> -	ret = lm3533_ctrlbank_set_max_current(&bl->cb, pdata->max_current);
> +	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;
> +	char *name = NULL;
> +	u32 default_brightness = LM3533_BL_MAX_BRIGHTNESS;
>  	int ret;
>  
>  	dev_dbg(&pdev->dev, "%s\n", __func__);
> @@ -269,12 +274,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;
> @@ -292,13 +291,21 @@ static int lm3533_bl_probe(struct platform_device *pdev)
>  	bl->cb.id = lm3533_bl_get_ctrlbank_id(bl);
>  	bl->cb.dev = NULL;			/* until registered */
>  
> +	name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s-%d",
> +			      pdev->name, pdev->id);

Unique enough (e.g. backlight-0)?

> +	if (!name)
> +		return -ENOMEM;
> +
> +	device_property_read_u32(&pdev->dev, "default-brightness",
> +				 &default_brightness);
> +
>  	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 = default_brightness;
> +
> +	bd = devm_backlight_device_register(&pdev->dev, name, &pdev->dev,
> +					    bl, &lm3533_bl_ops, &props);

Here too you are reparenting, which results in an ABI break.

>  	if (IS_ERR(bd)) {
>  		dev_err(&pdev->dev, "failed to register backlight device\n");
>  		return PTR_ERR(bd);
> @@ -309,12 +316,19 @@ static int lm3533_bl_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, bl);
>  
> -	backlight_update_status(bd);
> +	device_property_read_u32(&pdev->dev, "led-max-microamp",
> +				 &bl->max_current);
> +	bl->max_current = clamp(bl->max_current, LM3533_MAX_CURRENT_MIN,
> +				LM3533_MAX_CURRENT_MAX);

Clamping instead of failing.

>  
> -	ret = lm3533_bl_setup(bl, pdata);
> +	device_property_read_u32(&pdev->dev, "ti,pwm-config-mask", &bl->pwm);
> +
> +	ret = lm3533_bl_setup(bl);
>  	if (ret)
>  		return ret;
>  
> +	backlight_update_status(bd);
> +
>  	ret = lm3533_ctrlbank_enable(&bl->cb);
>  	if (ret)
>  		return ret;
> @@ -366,11 +380,18 @@ 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_bl_match_table);
> +
>  static struct platform_driver lm3533_bl_driver = {
>  	.driver = {
>  		.name	= "lm3533-backlight",
>  		.pm	= &lm3533_bl_pm_ops,
>  		.dev_groups = lm3533_bl_attribute_groups,
> +		.of_match_table = lm3533_bl_match_table,
>  	},
>  	.probe		= lm3533_bl_probe,
>  	.remove		= lm3533_bl_remove,

Drop platform module alias below.

Johan

Re: [PATCH v5 09/14] mfd: lm3533: Add support for VIN power supply

[Johan Hovold]

On Wed, Jun 17, 2026 at 11:00:26AM +0300, Svyatoslav Ryhel wrote:
> Add support for 2.7V-5.5V VIN power supply.
> 
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
 
> -static void lm3533_enable(struct lm3533 *lm3533)
> +static int lm3533_enable(struct lm3533 *lm3533)
>  {
> +	int ret;
> +
> +	ret = regulator_enable(lm3533->vin_supply);
> +	if (ret) {
> +		dev_err(lm3533->dev, "failed to enable vin power supply\n");
> +		return ret;
> +	}
> +

No delay needed?

>  	gpiod_set_value(lm3533->hwen, 1);
> +
> +	return 0;
>  }

Johan

Re: [PATCH v5 10/14] mfd: lm3533: Set DMA mask

[Johan Hovold]

On Wed, Jun 17, 2026 at 11:00:27AM +0300, Svyatoslav Ryhel wrote:
> Missing coherent_dma_mask assigning triggers the following warning in
> dmesg:
> 
> [    3.287872] platform lm3533-backlight.0: DMA mask not set
> 
> Since this warning might be elevated to an error in the future, set
> coherent_dma_mask to zero because both the core and cells do not utilize
> DMA.

IIUC this warning is introduced by the OF conversion and should
therefore be moved before it as a preparatory patch.

But you need to describe the problem in more detail here as this looks
fishy.

Johan

Re: [PATCH v5 02/14] mfd: lm3533: Remove driver specific regmap wrappers

[Svyatoslav Ryhel]

пт, 3 лип. 2026 р. о 12:44 Johan Hovold <johan@kernel.org> пише:
>
> On Wed, Jun 17, 2026 at 11:00:19AM +0300, Svyatoslav Ryhel wrote:
> > Remove driver-specific regmap wrappers in favor of using regmap helpers
> > directly.
>
> The commit message should explain why you think this is a good idea.
>

Removing custom wrappers over standard regmap helpers is always a good
idea, that is self explanatory.

> Based on a quick look you lose the errnos that were logged on errors and
> also the register debugging provided by those helpers.
>

That is not true, regmap helpers will provide logging and you should
not include debug stuff into the final submitted driver unless you
have a extremely valid justification for this, debug logs should have
been removed once the driver development was completed.

> Johan

Re: [PATCH v5 01/14] dt-bindings: leds: Document TI LM3533 LED controller

[Svyatoslav Ryhel]

пт, 3 лип. 2026 р. о 12:49 Johan Hovold <johan@kernel.org> пише:
>
> On Wed, Jun 17, 2026 at 11:00:18AM +0300, Svyatoslav Ryhel wrote:
> > Document 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>
> > Reviewed-by: Jonathan Cameron <jic23@kernel.org> #for light sensor
> > Reviewed-by: Daniel Thompson (RISCstar) <danielt@kernel.org> #for backlight
> > Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
>
> > +examples:
> > +  - |
> > +    #include <dt-bindings/gpio/gpio.h>
> > +    #include <dt-bindings/interrupt-controller/irq.h>
> > +
> > +    i2c {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        led-controller@36 {
> > +            compatible = "ti,lm3533";
> > +            reg = <0x36>;
> > +
> > +            enable-gpios = <&gpio 110 GPIO_ACTIVE_HIGH>;
> > +            vin-supply = <&vdd_3v3_bat>;
> > +
> > +            ti,boost-ovp-microvolt = <24000000>;
> > +            ti,boost-freq-hz = <500000>;
> > +
> > +            #address-cells = <1>;
> > +            #size-cells = <0>;
> > +
> > +            backlight@0 {
> > +                compatible = "ti,lm3533-backlight";
> > +                reg = <0>;
> > +
> > +                default-brightness = <113>;
> > +
> > +                led-max-microamp = <23400>;
> > +                led-sources = <0 1>;
> > +            };
> > +
> > +            led@2 {
> > +                compatible = "ti,lm3533-leds";
> > +                reg = <2>;
> > +
> > +                led-max-microamp = <23400>;
> > +                led-sources = <0 1>;
> > +            };
> > +
> > +            led@4 {
> > +                compatible = "ti,lm3533-leds";
> > +                reg = <4>;
> > +
> > +                led-max-microamp = <23400>;
> > +                led-sources = <2>;
> > +            };
> > +
> > +            led@5 {
> > +                compatible = "ti,lm3533-leds";
> > +                reg = <5>;
> > +
> > +                led-max-microamp = <23400>;
> > +                led-sources = <3 4>;
> > +            };
> > +
> > +            light-sensor {
>
> Doesn't the binding (or dts) checker complain about this node not having
> an address?
>

No it does not

> > +                compatible = "ti,lm3533-als";
> > +
> > +                interrupt-parent = <&gpio>;
> > +                interrupts = <80 IRQ_TYPE_LEVEL_LOW>;
> > +
> > +                ti,pwm-mode;
> > +            };
> > +        };
> > +    };
> > +...
>
> Johan

Re: [PATCH v5 04/14] mfd: lm3533: Pass only regmap and light sensor presence to child devices

[Svyatoslav Ryhel]

пт, 3 лип. 2026 р. о 12:50 Johan Hovold <johan@kernel.org> пише:
>
> On Wed, Jun 17, 2026 at 11:00:21AM +0300, Svyatoslav Ryhel wrote:
> > Instead of passing the entire lm3533 core data structure, only pass the
> > regmap and the light sensor presence flag to child devices.
>
> Again, why?
>

Because none of the child cells needs entire parents private
structure, regmap is all that is used.

> Johan

Re: [PATCH v5 05/14] iio: light: lm3533-als: Remove redundant pdata helpers

[Svyatoslav Ryhel]

пт, 3 лип. 2026 р. о 12:55 Johan Hovold <johan@kernel.org> пише:
>
> On Wed, Jun 17, 2026 at 11:00:22AM +0300, Svyatoslav Ryhel wrote:
> > The lm3533_als_set_input_mode() and lm3533_als_set_resistor() functions
> > are used only in lm3533_als_setup(). Incorporate their code into
> > lm3533_als_setup() directly to simplify driver readability.
>
> That's a debatable claim.
>

Adding helpers to wrap custom regmap wrappers seems redundant twice.

> > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > ---
> >  drivers/iio/light/lm3533-als.c | 56 ++++++++++------------------------
> >  1 file changed, 16 insertions(+), 40 deletions(-)
> >
> > diff --git a/drivers/iio/light/lm3533-als.c b/drivers/iio/light/lm3533-als.c
> > index a9af8e2b965f..69bac1b202f1 100644
> > --- a/drivers/iio/light/lm3533-als.c
> > +++ b/drivers/iio/light/lm3533-als.c
>
> > -static int lm3533_als_setup(struct lm3533_als *als,
> > -                         const struct lm3533_als_platform_data *pdata)
> > -{
> > -     int ret;
> > -
> > -     ret = lm3533_als_set_input_mode(als, pdata->pwm_mode);
> > +                        pdata->r_select);
> >       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 (ret)
> > -                     return ret;
> > -     }
> > +             return dev_err_probe(dev, ret, "failed to set resistor\n");
> >       return 0;
> >  }
>
> There's nothing hard to read about the above. To the contrary the logic
> is more obvious this way.
>
> You also remove the high-impedance comment for no good reason.
>

Removing one level of indentations without obscuring readability is
always a good idea.

> Johan

Re: [PATCH v5 06/14] mfd: lm3533-core: Remove redundant pdata helpers

[Svyatoslav Ryhel]

пт, 3 лип. 2026 р. о 12:57 Johan Hovold <johan@kernel.org> пише:
>
> On Wed, Jun 17, 2026 at 11:00:23AM +0300, Svyatoslav Ryhel wrote:
> > The lm3533_set_boost_freq() and lm3533_set_boost_ovp() functions are used
> > only in lm3533_device_setup(), which in turn is only called by
> > lm3533_device_init(). Incorporate their code directly into
> > lm3533_device_init() to simplify driver readability.
>
> Again, this isn't needed. Why are you removing abstraction that improve
> readability?
>

Adding helpers to wrap custom regmap wrappers seems redundant. Regmap
operations are pretty self explanatory without need in one-time-use
helpers.

> Johan

Re: [PATCH v5 08/14] mfd: lm3533: Convert to use OF bindings

[Svyatoslav Ryhel]

пт, 3 лип. 2026 р. о 14:03 Johan Hovold <johan@kernel.org> пише:
>
> On Wed, Jun 17, 2026 at 11:00:25AM +0300, Svyatoslav Ryhel wrote:
> > Since there are no users of this driver via platform data, remove the
> > platform data support and switch to using Device Tree bindings.
> >
> > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > Reviewed-by: Daniel Thompson (RISCstar) <danielt@kernel.org> #for backlight
> > ---
> >  drivers/iio/light/lm3533-als.c      |  67 +++++---
> >  drivers/leds/leds-lm3533.c          |  50 ++++--
> >  drivers/mfd/lm3533-core.c           | 236 ++++++++++++----------------
> >  drivers/mfd/lm3533-ctrlbank.c       |   5 -
> >  drivers/video/backlight/lm3533_bl.c |  55 +++++--
> >  include/linux/mfd/lm3533.h          |  52 +-----
> >  6 files changed, 220 insertions(+), 245 deletions(-)
>
> >  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;
> > @@ -803,12 +817,6 @@ static int lm3533_als_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;
> > -     }
> > -
> >       indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*als));
> >       if (!indio_dev)
> >               return -ENOMEM;
> > @@ -817,25 +825,27 @@ static int lm3533_als_probe(struct platform_device *pdev)
> >       indio_dev->channels = lm3533_als_channels;
> >       indio_dev->num_channels = ARRAY_SIZE(lm3533_als_channels);
> >       indio_dev->name = dev_name(&pdev->dev);
> > -     iio_device_set_parent(indio_dev, pdev->dev.parent);
>
> Why are you reparenting the iio device here?
>

Because every cell has its own binding now and using phandle to parent
when device has its own node is not a good practice.

> That's an ABI break.
>

This driver does not have any active users in the kernel and no
activity for more then 2 years.

> > +static const struct of_device_id lm3533_als_match_table[] = {
> > +     { .compatible = "ti,lm3533-als" },
> > +     { }
> > +};
> > +MODULE_DEVICE_TABLE(of, lm3533_als_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,
>
> You should also remove the platform module alias below.
>

Why?

> > diff --git a/drivers/leds/leds-lm3533.c b/drivers/leds/leds-lm3533.c
> > index 0cb0585eb960..ed810c23f30f 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/regmap.h>
> >  #include <linux/slab.h>
> >
> > @@ -50,6 +52,9 @@ struct lm3533_led {
> >       struct mutex mutex;
> >       unsigned long flags;
> >
> > +     u32 max_current;
> > +     u32 pwm;
> > +
> >       bool have_als;
> >  };
> >
> > @@ -616,22 +621,20 @@ 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;
> >       int ret;
> >
> > @@ -641,12 +644,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;
> > @@ -659,8 +656,6 @@ static int lm3533_led_probe(struct platform_device *pdev)
> >       led->regmap = lm3533->regmap;
> >       led->have_als = lm3533->have_als;
> >
> > -     led->cdev.name = pdata->name;
> > -     led->cdev.default_trigger = pdata->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;
> > @@ -668,6 +663,15 @@ static int lm3533_led_probe(struct platform_device *pdev)
> >       led->cdev.groups = lm3533_led_attribute_groups;
> >       led->id = pdev->id;
> >
> > +     led->cdev.name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s-%d",
> > +                                     pdev->name, led->id);
>
> Is "led-2", etc. unique enough here?
>

lm3533-leds-2

> > +     if (!led->cdev.name)
> > +             return -ENOMEM;
> > +
> > +     led->cdev.default_trigger = "none";
> > +     device_property_read_string(&pdev->dev, "linux,default-trigger",
> > +                                 &led->cdev.default_trigger);
> > +
> >       mutex_init(&led->mutex);
> >
> >       /* The class framework makes a callback to get brightness during
> > @@ -680,15 +684,22 @@ static int lm3533_led_probe(struct platform_device *pdev)
> >
> >       platform_set_drvdata(pdev, led);
> >
> > -     ret = led_classdev_register(pdev->dev.parent, &led->cdev);
> > +     ret = led_classdev_register(&pdev->dev, &led->cdev);
>
> Here too you appear to be reparenting the class devices.
>
> >       if (ret) {
> > -             dev_err(&pdev->dev, "failed to register LED %d\n", pdev->id);
> > +             dev_err(&pdev->dev, "failed to register LED %d\n", led->id);
>
> This does not seem to be necessary.
>

Agreed.

> >               return ret;
> >       }
> >
> >       led->cb.dev = led->cdev.dev;
> >
> > -     ret = lm3533_led_setup(led, pdata);
> > +     device_property_read_u32(&pdev->dev, "led-max-microamp",
> > +                              &led->max_current);
> > +     led->max_current = clamp(led->max_current, LM3533_MAX_CURRENT_MIN,
> > +                              LM3533_MAX_CURRENT_MAX);
>
> Why clamp instead of having lm3533_led_setup() fail below?
>

According to OF schema default lower margin is set to
LM3533_MAX_CURRENT_MIN so clamping seems a good option here, even
though it will clamp max value.

> > +
> > +     device_property_read_u32(&pdev->dev, "ti,pwm-config-mask", &led->pwm);
> > +
> > +     ret = lm3533_led_setup(led);
> >       if (ret)
> >               goto err_deregister;
> >
> > @@ -725,9 +736,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_led_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,
>
> Remove platform alias below as well.
>

Why?

> > diff --git a/drivers/mfd/lm3533-core.c b/drivers/mfd/lm3533-core.c
> > index b03a3ae96c10..a5aa7da9668b 100644
> > --- a/drivers/mfd/lm3533-core.c
> > +++ b/drivers/mfd/lm3533-core.c
> > @@ -14,19 +14,26 @@
> >  #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>
> >
> >
> >  #define LM3533_BOOST_OVP_MASK                0x06
> >  #define LM3533_BOOST_OVP_SHIFT               1
> > +#define LM3533_BOOST_OVP_MIN         (16 * MICRO)
> > +#define LM3533_BOOST_OVP_MAX         (40 * MICRO)
> >
> >  #define LM3533_BOOST_FREQ_MASK               0x01
> >  #define LM3533_BOOST_FREQ_SHIFT              0
> > +#define LM3533_BOOST_FREQ_MIN                (500 * HZ_PER_KHZ)
> > +#define LM3533_BOOST_FREQ_MAX                (1000 * HZ_PER_KHZ)
> >
> >  #define LM3533_BL_ID_MASK            1
> >  #define LM3533_LED_ID_MASK           3
> > @@ -35,6 +42,7 @@
> >
> >  #define LM3533_HVLED_ID_MAX          2
> >  #define LM3533_LVLED_ID_MAX          5
> > +#define LM3533_CELLS_MAX             7
> >
> >  #define LM3533_REG_OUTPUT_CONF1              0x10
> >  #define LM3533_REG_OUTPUT_CONF2              0x11
> > @@ -42,44 +50,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,
> > -     },
> > -};
> > -
> >  /*
> >   * HVLED output config -- output hvled controlled by backlight bl
> >   */
> > @@ -301,125 +271,91 @@ static const struct attribute_group *lm3533_attribute_groups[] = {
> >       NULL,
> >  };
> >
> > -static int lm3533_device_als_init(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");
> > -             return ret;
> > -     }
> > -
> > -     lm3533->have_als = 1;
> > -
> > -     return 0;
> > -}
> > -
> > -static int lm3533_device_bl_init(struct lm3533 *lm3533)
> > -{
> > -     struct lm3533_platform_data *pdata = dev_get_platdata(lm3533->dev);
> > -     int i;
> > -     int ret;
> > -
> > -     if (!pdata->backlights || pdata->num_backlights == 0)
> > -             return 0;
> > -
> > -     if (pdata->num_backlights > ARRAY_SIZE(lm3533_bl_devs))
> > -             pdata->num_backlights = ARRAY_SIZE(lm3533_bl_devs);
> > -
> > -     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]);
> > -     }
> > -
> > -     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;
> > -     }
> > -
> > -     lm3533->have_backlights = 1;
> > -
> > -     return 0;
> > -}
> > -
> > -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;
> > -
> > -     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;
> > -     }
> > -
> > -     lm3533->have_leds = 1;
> > -
> > -     return 0;
> > -}
> > -
> >  static int lm3533_device_init(struct lm3533 *lm3533)
> >  {
> > -     struct lm3533_platform_data *pdata = dev_get_platdata(lm3533->dev);
> > +     struct device *dev = lm3533->dev;
> > +     struct mfd_cell *lm3533_devices;
> > +     u32 count = 0, reg, nchilds;
>
> Don't mix multiple declarations with initialisation like this.
>

Checkpatch does not complain on style issue, hence this is not prohibited.

> >       int ret;
> >
> > -     dev_dbg(lm3533->dev, "%s\n", __func__);
> > +     nchilds = device_get_child_node_count(dev);
> > +     if (!nchilds || nchilds > LM3533_CELLS_MAX)
> > +             return dev_err_probe(dev, -ENODEV,
> > +                                  "num of child nodes is not supported\n");
> >
> > -     if (!pdata) {
> > -             dev_err(lm3533->dev, "no platform data\n");
> > -             return -EINVAL;
> > -     }
> > +     lm3533_devices = devm_kcalloc(dev, nchilds, sizeof(*lm3533_devices),
> > +                                   GFP_KERNEL);
> > +     if (!lm3533_devices)
> > +             return -ENOMEM;
> >
> > -     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");
> > +     device_for_each_child_node_scoped(dev, child) {
> > +             if (count >= nchilds)
> > +                     break;
>
> How could count be larger than nchilds?
>

Only if the tree is malformed, hence this check was added.

> > +
> > +             if (fwnode_device_is_compatible(child, "ti,lm3533-als")) {
> > +                     lm3533_devices[count].name = "lm3533-als";
> > +                     lm3533_devices[count].of_compatible = "ti,lm3533-als";
> > +                     lm3533_devices[count].id = PLATFORM_DEVID_NONE;
> > +
> > +                     lm3533->have_als = true;
> > +                     count++;
> > +             } else if (fwnode_device_is_compatible(child, "ti,lm3533-backlight")) {
> > +                     ret = fwnode_property_read_u32(child, "reg", &reg);
> > +                     if (ret || reg >= LM3533_HVLED_ID_MAX) {
> > +                             dev_err(dev, "invalid backlight node %pfw\n", child);
> > +                             continue;
> > +                     }
> > +
> > +                     lm3533_devices[count].name = "lm3533-backlight";
> > +                     lm3533_devices[count].of_compatible = "ti,lm3533-backlight";
> > +                     lm3533_devices[count].id = reg;
> > +                     lm3533_devices[count].of_reg = reg;
> > +                     lm3533_devices[count].use_of_reg = true;
> > +
> > +                     lm3533->have_backlights = true;
> > +                     count++;
> > +             } else if (fwnode_device_is_compatible(child, "ti,lm3533-leds")) {
> > +                     ret = fwnode_property_read_u32(child, "reg", &reg);
> > +                     if (ret || reg < LM3533_HVLED_ID_MAX ||
> > +                         reg > LM3533_LVLED_ID_MAX) {
> > +                             dev_err(dev, "invalid LED node %pfw\n", child);
> > +                             continue;
> > +                     }
> > +
> > +                     lm3533_devices[count].name = "lm3533-leds";
> > +                     lm3533_devices[count].of_compatible = "ti,lm3533-leds";
> > +                     lm3533_devices[count].id = reg - LM3533_HVLED_ID_MAX;
> > +                     lm3533_devices[count].of_reg = reg;
> > +                     lm3533_devices[count].use_of_reg = true;
> > +
> > +                     lm3533->have_leds = true;
> > +                     count++;
> > +             }
> > +     }
>
> Why do you need the above at all? Shouldn't you be able to just use
> of_platform_populate().
>

of_platform_populate() is not a part of mfd framework.

> >
> >       lm3533_enable(lm3533);
> >
> >       ret = regmap_update_bits(lm3533->regmap, LM3533_REG_BOOST_PWM,
> >                                LM3533_BOOST_FREQ_MASK,
> > -                              pdata->boost_freq << LM3533_BOOST_FREQ_SHIFT);
> > +                              lm3533->boost_freq << LM3533_BOOST_FREQ_SHIFT);
> >       if (ret) {
> > -             dev_err(lm3533->dev, "failed to set boost frequency\n");
> > +             dev_err(dev, "failed to set boost frequency\n");
> >               goto err_disable;
> >       }
> >
> >       ret = regmap_update_bits(lm3533->regmap, LM3533_REG_BOOST_PWM,
> >                                LM3533_BOOST_OVP_MASK,
> > -                              pdata->boost_ovp << LM3533_BOOST_OVP_SHIFT);
> > +                              lm3533->boost_ovp << LM3533_BOOST_OVP_SHIFT);
> >       if (ret) {
> > -             dev_err(lm3533->dev, "failed to set boost ovp\n");
> > +             dev_err(dev, "failed to set boost ovp\n");
> >               goto err_disable;
> >       }
> >
> > -     lm3533_device_als_init(lm3533);
> > -     lm3533_device_bl_init(lm3533);
> > -     lm3533_device_led_init(lm3533);
> > +     ret = mfd_add_devices(dev, 0, lm3533_devices, count, NULL, 0, NULL);
> > +     if (ret) {
> > +             dev_err(dev, "failed to add MFD devices: %d\n", ret);
> > +             goto err_disable;
> > +     }
> >
> >       return 0;
> >
> > @@ -504,7 +440,26 @@ static int lm3533_i2c_probe(struct i2c_client *i2c)
> >               return PTR_ERR(lm3533->regmap);
> >
> >       lm3533->dev = &i2c->dev;
> > -     lm3533->irq = i2c->irq;
> > +
> > +     lm3533->hwen = devm_gpiod_get_optional(lm3533->dev, "enable",
> > +                                            GPIOD_OUT_LOW);
> > +     if (IS_ERR(lm3533->hwen))
> > +             return dev_err_probe(lm3533->dev, PTR_ERR(lm3533->hwen),
> > +                                  "failed to get HWEN GPIO\n");
>
> Please use brackets around multline statements for readability
> throughout.
>

Checkpatch does not complain on style issue, hence this is not prohibited.

> > +
> > +     device_property_read_u32(lm3533->dev, "ti,boost-ovp-microvolt",
> > +                              &lm3533->boost_ovp);
> > +
> > +     lm3533->boost_ovp = clamp(lm3533->boost_ovp, LM3533_BOOST_OVP_MIN,
> > +                               LM3533_BOOST_OVP_MAX);
> > +     lm3533->boost_ovp = lm3533->boost_ovp / (8 * MICRO) - 2;
> > +
> > +     device_property_read_u32(lm3533->dev, "ti,boost-freq-hz",
> > +                              &lm3533->boost_freq);
> > +
> > +     lm3533->boost_freq = clamp(lm3533->boost_freq, LM3533_BOOST_FREQ_MIN,
> > +                                LM3533_BOOST_FREQ_MAX);
> > +     lm3533->boost_freq = lm3533->boost_freq / (500 * KILO) - 1;
>
> Again, why clamp instead of failing probe?
>

According to OF schema default lower margin is set to
LM3533_BOOST_FREQ_MIN so clamping seems a good option here, even
though it will clamp max value.

> >       return lm3533_device_init(lm3533);
> >  }
> > @@ -518,6 +473,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" },
> >       { }
> > @@ -528,6 +489,7 @@ static struct i2c_driver lm3533_i2c_driver = {
> >       .driver = {
> >                  .name = "lm3533",
> >                  .dev_groups = lm3533_attribute_groups,
> > +                .of_match_table = lm3533_match_table,
> >       },
> >       .id_table       = lm3533_i2c_ids,
> >       .probe          = lm3533_i2c_probe,
> > diff --git a/drivers/mfd/lm3533-ctrlbank.c b/drivers/mfd/lm3533-ctrlbank.c
> > index 91e13cfa3cf0..3aab8ece4e8c 100644
> > --- a/drivers/mfd/lm3533-ctrlbank.c
> > +++ b/drivers/mfd/lm3533-ctrlbank.c
> > @@ -13,11 +13,6 @@
> >
> >  #include <linux/mfd/lm3533.h>
> >
> > -
> > -#define LM3533_MAX_CURRENT_MIN               5000
> > -#define LM3533_MAX_CURRENT_MAX               29800
> > -#define LM3533_MAX_CURRENT_STEP              800
> > -
> >  #define LM3533_PWM_MAX                       0x3f
> >
> >  #define LM3533_REG_PWM_BASE          0x14
> > diff --git a/drivers/video/backlight/lm3533_bl.c b/drivers/video/backlight/lm3533_bl.c
> > index 9ef171d3aaea..2c24647fc17a 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/regmap.h>
> >  #include <linux/slab.h>
> > @@ -29,6 +31,9 @@ struct lm3533_bl {
> >       struct backlight_device *bd;
> >       int id;
> >
> > +     u32 max_current;
> > +     u32 pwm;
> > +
> >       bool have_als;
> >  };
> >
> > @@ -242,25 +247,25 @@ static const struct attribute_group *lm3533_bl_attribute_groups[] = {
> >       NULL,
> >  };
> >
> > -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 ret;
> >
> > -     ret = lm3533_ctrlbank_set_max_current(&bl->cb, pdata->max_current);
> > +     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;
> > +     char *name = NULL;
> > +     u32 default_brightness = LM3533_BL_MAX_BRIGHTNESS;
> >       int ret;
> >
> >       dev_dbg(&pdev->dev, "%s\n", __func__);
> > @@ -269,12 +274,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;
> > @@ -292,13 +291,21 @@ static int lm3533_bl_probe(struct platform_device *pdev)
> >       bl->cb.id = lm3533_bl_get_ctrlbank_id(bl);
> >       bl->cb.dev = NULL;                      /* until registered */
> >
> > +     name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s-%d",
> > +                           pdev->name, pdev->id);
>
> Unique enough (e.g. backlight-0)?
>

lm3533-backlight-0

> > +     if (!name)
> > +             return -ENOMEM;
> > +
> > +     device_property_read_u32(&pdev->dev, "default-brightness",
> > +                              &default_brightness);
> > +
> >       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 = default_brightness;
> > +
> > +     bd = devm_backlight_device_register(&pdev->dev, name, &pdev->dev,
> > +                                         bl, &lm3533_bl_ops, &props);
>
> Here too you are reparenting, which results in an ABI break.
>
> >       if (IS_ERR(bd)) {
> >               dev_err(&pdev->dev, "failed to register backlight device\n");
> >               return PTR_ERR(bd);
> > @@ -309,12 +316,19 @@ static int lm3533_bl_probe(struct platform_device *pdev)
> >
> >       platform_set_drvdata(pdev, bl);
> >
> > -     backlight_update_status(bd);
> > +     device_property_read_u32(&pdev->dev, "led-max-microamp",
> > +                              &bl->max_current);
> > +     bl->max_current = clamp(bl->max_current, LM3533_MAX_CURRENT_MIN,
> > +                             LM3533_MAX_CURRENT_MAX);
>
> Clamping instead of failing.
>
> >
> > -     ret = lm3533_bl_setup(bl, pdata);
> > +     device_property_read_u32(&pdev->dev, "ti,pwm-config-mask", &bl->pwm);
> > +
> > +     ret = lm3533_bl_setup(bl);
> >       if (ret)
> >               return ret;
> >
> > +     backlight_update_status(bd);
> > +
> >       ret = lm3533_ctrlbank_enable(&bl->cb);
> >       if (ret)
> >               return ret;
> > @@ -366,11 +380,18 @@ 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_bl_match_table);
> > +
> >  static struct platform_driver lm3533_bl_driver = {
> >       .driver = {
> >               .name   = "lm3533-backlight",
> >               .pm     = &lm3533_bl_pm_ops,
> >               .dev_groups = lm3533_bl_attribute_groups,
> > +             .of_match_table = lm3533_bl_match_table,
> >       },
> >       .probe          = lm3533_bl_probe,
> >       .remove         = lm3533_bl_remove,
>
> Drop platform module alias below.
>
> Johan

Re: [PATCH v5 09/14] mfd: lm3533: Add support for VIN power supply

[Svyatoslav Ryhel]

пт, 3 лип. 2026 р. о 14:09 Johan Hovold <johan@kernel.org> пише:
>
> On Wed, Jun 17, 2026 at 11:00:26AM +0300, Svyatoslav Ryhel wrote:
> > Add support for 2.7V-5.5V VIN power supply.
> >
> > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
>
> > -static void lm3533_enable(struct lm3533 *lm3533)
> > +static int lm3533_enable(struct lm3533 *lm3533)
> >  {
> > +     int ret;
> > +
> > +     ret = regulator_enable(lm3533->vin_supply);
> > +     if (ret) {
> > +             dev_err(lm3533->dev, "failed to enable vin power supply\n");
> > +             return ret;
> > +     }
> > +
>
> No delay needed?
>

I have not found any in the datasheet, if you are aware of the delay
duration needed please elaborate.

> >       gpiod_set_value(lm3533->hwen, 1);
> > +
> > +     return 0;
> >  }
>
> Johan

Re: [PATCH v5 06/14] mfd: lm3533-core: Remove redundant pdata helpers

[Jonathan Cameron]

On Wed, 17 Jun 2026 11:00:23 +0300
Svyatoslav Ryhel <clamor95@gmail.com> wrote:

> The lm3533_set_boost_freq() and lm3533_set_boost_ovp() functions are used
> only in lm3533_device_setup(), which in turn is only called by
> lm3533_device_init(). Incorporate their code directly into
> lm3533_device_init() to simplify driver readability.
> 
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>

>  static int lm3533_device_init(struct lm3533 *lm3533)
>  {
>  	struct lm3533_platform_data *pdata = dev_get_platdata(lm3533->dev);
> @@ -437,9 +396,21 @@ static int lm3533_device_init(struct lm3533 *lm3533)
>  
>  	lm3533_enable(lm3533);
>  
> -	ret = lm3533_device_setup(lm3533, pdata);
> -	if (ret)
> +	ret = regmap_update_bits(lm3533->regmap, LM3533_REG_BOOST_PWM,
> +				 LM3533_BOOST_FREQ_MASK,
> +				 pdata->boost_freq << LM3533_BOOST_FREQ_SHIFT);

As you are touching this maybe get rid of the shifts and have one source
of info on where the field lies in the register.

				 FIELD_PREP(LM3533_BOOST_FREQ_MASK, pdata->boost_freq));

> +	if (ret) {
> +		dev_err(lm3533->dev, "failed to set boost frequency\n");
>  		goto err_disable;
> +	}
> +
> +	ret = regmap_update_bits(lm3533->regmap, LM3533_REG_BOOST_PWM,
> +				 LM3533_BOOST_OVP_MASK,
> +				 pdata->boost_ovp << LM3533_BOOST_OVP_SHIFT);
> +	if (ret) {
> +		dev_err(lm3533->dev, "failed to set boost ovp\n");
> +		goto err_disable;
> +	}
>  
>  	lm3533_device_als_init(lm3533);
>  	lm3533_device_bl_init(lm3533);

Re: [PATCH v5 08/14] mfd: lm3533: Convert to use OF bindings

[Jonathan Cameron]

On Tue, 14 Jul 2026 16:57:01 +0300
Svyatoslav Ryhel <clamor95@gmail.com> wrote:

> пт, 3 лип. 2026 р. о 14:03 Johan Hovold <johan@kernel.org> пише:
> >
> > On Wed, Jun 17, 2026 at 11:00:25AM +0300, Svyatoslav Ryhel wrote:  
> > > Since there are no users of this driver via platform data, remove the
> > > platform data support and switch to using Device Tree bindings.
> > >
> > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > Reviewed-by: Daniel Thompson (RISCstar) <danielt@kernel.org> #for backlight
> > >       indio_dev->name = dev_name(&pdev->dev);
> > > -     iio_device_set_parent(indio_dev, pdev->dev.parent);  
> >
> > Why are you reparenting the iio device here?
> >  
> 
> Because every cell has its own binding now and using phandle to parent
> when device has its own node is not a good practice.

It is however a good point.  So should at very least the ABI break should
be called out in the patch description with a brief mention of what the
path change actually is.

> 
> > That's an ABI break.
> >  
> 
> This driver does not have any active users in the kernel and no
> activity for more then 2 years.

...

> > > +     u32 count = 0, reg, nchilds;  
> >
> > Don't mix multiple declarations with initialisation like this.
> >  
> 
> Checkpatch does not complain on style issue, hence this is not prohibited.

Prohibition vs taste.  I fully agree with John on this one!  Those mixes are
messy and hard to read.

Checkpatch is a helpful tool but there are both rules and preferred style things
it does not catch.

Jonathan

> 
> > >       int ret;
> > >

Re: [PATCH v5 06/14] mfd: lm3533-core: Remove redundant pdata helpers

[Svyatoslav Ryhel]

ср, 22 лип. 2026 р. о 04:25 Jonathan Cameron <jic23@kernel.org> пише:
>
> On Wed, 17 Jun 2026 11:00:23 +0300
> Svyatoslav Ryhel <clamor95@gmail.com> wrote:
>
> > The lm3533_set_boost_freq() and lm3533_set_boost_ovp() functions are used
> > only in lm3533_device_setup(), which in turn is only called by
> > lm3533_device_init(). Incorporate their code directly into
> > lm3533_device_init() to simplify driver readability.
> >
> > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
>
> >  static int lm3533_device_init(struct lm3533 *lm3533)
> >  {
> >       struct lm3533_platform_data *pdata = dev_get_platdata(lm3533->dev);
> > @@ -437,9 +396,21 @@ static int lm3533_device_init(struct lm3533 *lm3533)
> >
> >       lm3533_enable(lm3533);
> >
> > -     ret = lm3533_device_setup(lm3533, pdata);
> > -     if (ret)
> > +     ret = regmap_update_bits(lm3533->regmap, LM3533_REG_BOOST_PWM,
> > +                              LM3533_BOOST_FREQ_MASK,
> > +                              pdata->boost_freq << LM3533_BOOST_FREQ_SHIFT);
>
> As you are touching this maybe get rid of the shifts and have one source
> of info on where the field lies in the register.
>
>                                  FIELD_PREP(LM3533_BOOST_FREQ_MASK, pdata->boost_freq));
>

Very decent suggestion, thanks!

> > +     if (ret) {
> > +             dev_err(lm3533->dev, "failed to set boost frequency\n");
> >               goto err_disable;
> > +     }
> > +
> > +     ret = regmap_update_bits(lm3533->regmap, LM3533_REG_BOOST_PWM,
> > +                              LM3533_BOOST_OVP_MASK,
> > +                              pdata->boost_ovp << LM3533_BOOST_OVP_SHIFT);
> > +     if (ret) {
> > +             dev_err(lm3533->dev, "failed to set boost ovp\n");
> > +             goto err_disable;
> > +     }
> >
> >       lm3533_device_als_init(lm3533);
> >       lm3533_device_bl_init(lm3533);
>

Re: [PATCH v5 08/14] mfd: lm3533: Convert to use OF bindings

[Svyatoslav Ryhel]

ср, 22 лип. 2026 р. о 04:30 Jonathan Cameron <jic23@kernel.org> пише:
>
> On Tue, 14 Jul 2026 16:57:01 +0300
> Svyatoslav Ryhel <clamor95@gmail.com> wrote:
>
> > пт, 3 лип. 2026 р. о 14:03 Johan Hovold <johan@kernel.org> пише:
> > >
> > > On Wed, Jun 17, 2026 at 11:00:25AM +0300, Svyatoslav Ryhel wrote:
> > > > Since there are no users of this driver via platform data, remove the
> > > > platform data support and switch to using Device Tree bindings.
> > > >
> > > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > > Reviewed-by: Daniel Thompson (RISCstar) <danielt@kernel.org> #for backlight
> > > >       indio_dev->name = dev_name(&pdev->dev);
> > > > -     iio_device_set_parent(indio_dev, pdev->dev.parent);
> > >
> > > Why are you reparenting the iio device here?
> > >
> >
> > Because every cell has its own binding now and using phandle to parent
> > when device has its own node is not a good practice.
>
> It is however a good point.  So should at very least the ABI break should
> be called out in the patch description with a brief mention of what the
> path change actually is.
>

Ok, this is a good idea to add chapter into commit description.

> >
> > > That's an ABI break.
> > >
> >
> > This driver does not have any active users in the kernel and no
> > activity for more then 2 years.
>
> ...
>
> > > > +     u32 count = 0, reg, nchilds;
> > >
> > > Don't mix multiple declarations with initialisation like this.
> > >
> >
> > Checkpatch does not complain on style issue, hence this is not prohibited.
>
> Prohibition vs taste.  I fully agree with John on this one!  Those mixes are
> messy and hard to read.
>
> Checkpatch is a helpful tool but there are both rules and preferred style things
> it does not catch.
>

Ok, fine.

> Jonathan
>
> >
> > > >       int ret;
> > > >

Re: [PATCH v5 02/14] mfd: lm3533: Remove driver specific regmap wrappers

[Johan Hovold]

On Tue, Jul 14, 2026 at 04:26:49PM +0300, Svyatoslav Ryhel wrote:
> пт, 3 лип. 2026 р. о 12:44 Johan Hovold <johan@kernel.org> пише:
> >
> > On Wed, Jun 17, 2026 at 11:00:19AM +0300, Svyatoslav Ryhel wrote:
> > > Remove driver-specific regmap wrappers in favor of using regmap helpers
> > > directly.
> >
> > The commit message should explain why you think this is a good idea.
> >
> 
> Removing custom wrappers over standard regmap helpers is always a good
> idea, that is self explanatory.

No, and especially not if you're removing functionality.

> > Based on a quick look you lose the errnos that were logged on errors and
> > also the register debugging provided by those helpers.
> >
> 
> That is not true, regmap helpers will provide logging and you should
> not include debug stuff into the final submitted driver unless you
> have a extremely valid justification for this, debug logs should have
> been removed once the driver development was completed.

The regmap logging is a compile-time thing (added after the driver was
merged), and not equivalent to what the driver provides currently.

The driver also currently relies on these helpers for logging errnos.

Johan

Re: [PATCH v5 01/14] dt-bindings: leds: Document TI LM3533 LED controller

[Johan Hovold]

On Tue, Jul 14, 2026 at 04:27:08PM +0300, Svyatoslav Ryhel wrote:
> пт, 3 лип. 2026 р. о 12:49 Johan Hovold <johan@kernel.org> пише:

> > > +            light-sensor {
> >
> > Doesn't the binding (or dts) checker complain about this node not having
> > an address?
> >
> 
> No it does not

Thanks for confirming.

Johan

Re: [PATCH v5 04/14] mfd: lm3533: Pass only regmap and light sensor presence to child devices

[Johan Hovold]

On Tue, Jul 14, 2026 at 04:28:24PM +0300, Svyatoslav Ryhel wrote:
> пт, 3 лип. 2026 р. о 12:50 Johan Hovold <johan@kernel.org> пише:
> >
> > On Wed, Jun 17, 2026 at 11:00:21AM +0300, Svyatoslav Ryhel wrote:
> > > Instead of passing the entire lm3533 core data structure, only pass the
> > > regmap and the light sensor presence flag to child devices.
> >
> > Again, why?
> >
> 
> Because none of the child cells needs entire parents private
> structure, regmap is all that is used.

Just because you can do something doesn't necessarily mean you should.

Johan

Re: [PATCH v5 05/14] iio: light: lm3533-als: Remove redundant pdata helpers

[Johan Hovold]

On Tue, Jul 14, 2026 at 04:31:08PM +0300, Svyatoslav Ryhel wrote:
> пт, 3 лип. 2026 р. о 12:55 Johan Hovold <johan@kernel.org> пише:
> >
> > On Wed, Jun 17, 2026 at 11:00:22AM +0300, Svyatoslav Ryhel wrote:
> > > The lm3533_als_set_input_mode() and lm3533_als_set_resistor() functions
> > > are used only in lm3533_als_setup(). Incorporate their code into
> > > lm3533_als_setup() directly to simplify driver readability.
> >
> > That's a debatable claim.
> >
> 
> Adding helpers to wrap custom regmap wrappers seems redundant twice.
> 
> > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > ---
> > >  drivers/iio/light/lm3533-als.c | 56 ++++++++++------------------------
> > >  1 file changed, 16 insertions(+), 40 deletions(-)
> > >
> > > diff --git a/drivers/iio/light/lm3533-als.c b/drivers/iio/light/lm3533-als.c
> > > index a9af8e2b965f..69bac1b202f1 100644
> > > --- a/drivers/iio/light/lm3533-als.c
> > > +++ b/drivers/iio/light/lm3533-als.c
> >
> > > -static int lm3533_als_setup(struct lm3533_als *als,
> > > -                         const struct lm3533_als_platform_data *pdata)
> > > -{
> > > -     int ret;
> > > -
> > > -     ret = lm3533_als_set_input_mode(als, pdata->pwm_mode);
> > > +                        pdata->r_select);
> > >       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 (ret)
> > > -                     return ret;
> > > -     }
> > > +             return dev_err_probe(dev, ret, "failed to set resistor\n");
> > >       return 0;
> > >  }
> >
> > There's nothing hard to read about the above. To the contrary the logic
> > is more obvious this way.
> >
> > You also remove the high-impedance comment for no good reason.
> >
> 
> Removing one level of indentations without obscuring readability is
> always a good idea.

Again, no. You're obscuring the current logic which is perfectly fine as
is.

Johan

Re: [PATCH v5 06/14] mfd: lm3533-core: Remove redundant pdata helpers

[Johan Hovold]

On Tue, Jul 14, 2026 at 04:32:40PM +0300, Svyatoslav Ryhel wrote:
> пт, 3 лип. 2026 р. о 12:57 Johan Hovold <johan@kernel.org> пише:
> >
> > On Wed, Jun 17, 2026 at 11:00:23AM +0300, Svyatoslav Ryhel wrote:
> > > The lm3533_set_boost_freq() and lm3533_set_boost_ovp() functions are used
> > > only in lm3533_device_setup(), which in turn is only called by
> > > lm3533_device_init(). Incorporate their code directly into
> > > lm3533_device_init() to simplify driver readability.
> >
> > Again, this isn't needed. Why are you removing abstraction that improve
> > readability?
> >
> 
> Adding helpers to wrap custom regmap wrappers seems redundant. Regmap
> operations are pretty self explanatory without need in one-time-use
> helpers.

They are clearly not as easy to read so this obscures what is being done
and makes lm3533_device_init() harder to read.

Johan

Re: [PATCH v5 08/14] mfd: lm3533: Convert to use OF bindings

[Johan Hovold]

On Tue, Jul 14, 2026 at 04:57:01PM +0300, Svyatoslav Ryhel wrote:
> пт, 3 лип. 2026 р. о 14:03 Johan Hovold <johan@kernel.org> пише:
> >
> > On Wed, Jun 17, 2026 at 11:00:25AM +0300, Svyatoslav Ryhel wrote:
> > > Since there are no users of this driver via platform data, remove the
> > > platform data support and switch to using Device Tree bindings.
> > >
> > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > Reviewed-by: Daniel Thompson (RISCstar) <danielt@kernel.org> #for backlight
> > > ---
> > >  drivers/iio/light/lm3533-als.c      |  67 +++++---
> > >  drivers/leds/leds-lm3533.c          |  50 ++++--
> > >  drivers/mfd/lm3533-core.c           | 236 ++++++++++++----------------
> > >  drivers/mfd/lm3533-ctrlbank.c       |   5 -
> > >  drivers/video/backlight/lm3533_bl.c |  55 +++++--
> > >  include/linux/mfd/lm3533.h          |  52 +-----
> > >  6 files changed, 220 insertions(+), 245 deletions(-)
> >
> > >  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;
> > > @@ -803,12 +817,6 @@ static int lm3533_als_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;
> > > -     }
> > > -
> > >       indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*als));
> > >       if (!indio_dev)
> > >               return -ENOMEM;
> > > @@ -817,25 +825,27 @@ static int lm3533_als_probe(struct platform_device *pdev)
> > >       indio_dev->channels = lm3533_als_channels;
> > >       indio_dev->num_channels = ARRAY_SIZE(lm3533_als_channels);
> > >       indio_dev->name = dev_name(&pdev->dev);
> > > -     iio_device_set_parent(indio_dev, pdev->dev.parent);
> >
> > Why are you reparenting the iio device here?
> >
> 
> Because every cell has its own binding now and using phandle to parent
> when device has its own node is not a good practice.
> 
> > That's an ABI break.
> >
> 
> This driver does not have any active users in the kernel and no
> activity for more then 2 years.

We have never required board files to be upstream.

And a working driver does not need to be changed every year.

In any case, something like this at a minimum needs to be highlighted in
the commit message.

> > > +static const struct of_device_id lm3533_als_match_table[] = {
> > > +     { .compatible = "ti,lm3533-als" },
> > > +     { }
> > > +};
> > > +MODULE_DEVICE_TABLE(of, lm3533_als_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,
> >
> > You should also remove the platform module alias below.
> >
> 
> Why?

Because your change makes it obsolete. The driver now only supports OF
probing.

> > > @@ -680,15 +684,22 @@ static int lm3533_led_probe(struct platform_device *pdev)
> > >
> > >       platform_set_drvdata(pdev, led);
> > >
> > > -     ret = led_classdev_register(pdev->dev.parent, &led->cdev);
> > > +     ret = led_classdev_register(&pdev->dev, &led->cdev);
> >
> > Here too you appear to be reparenting the class devices.
> >
> > >       if (ret) {
> > > -             dev_err(&pdev->dev, "failed to register LED %d\n", pdev->id);
> > > +             dev_err(&pdev->dev, "failed to register LED %d\n", led->id);
> >
> > This does not seem to be necessary.
> >
> 
> Agreed.
> 
> > >               return ret;
> > >       }
> > >
> > >       led->cb.dev = led->cdev.dev;
> > >
> > > -     ret = lm3533_led_setup(led, pdata);
> > > +     device_property_read_u32(&pdev->dev, "led-max-microamp",
> > > +                              &led->max_current);
> > > +     led->max_current = clamp(led->max_current, LM3533_MAX_CURRENT_MIN,
> > > +                              LM3533_MAX_CURRENT_MAX);
> >
> > Why clamp instead of having lm3533_led_setup() fail below?
> >
> 
> According to OF schema default lower margin is set to
> LM3533_MAX_CURRENT_MIN so clamping seems a good option here, even
> though it will clamp max value.

Just let it fail to probe. The driver already have the necessary checks.

> > > +
> > > +     device_property_read_u32(&pdev->dev, "ti,pwm-config-mask", &led->pwm);
> > > +
> > > +     ret = lm3533_led_setup(led);
> > >       if (ret)
> > >               goto err_deregister;
> > >
> > > @@ -725,9 +736,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_led_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,
> >
> > Remove platform alias below as well.
> >
> 
> Why?

Same reason as above.

> > >  static int lm3533_device_init(struct lm3533 *lm3533)
> > >  {
> > > -     struct lm3533_platform_data *pdata = dev_get_platdata(lm3533->dev);
> > > +     struct device *dev = lm3533->dev;
> > > +     struct mfd_cell *lm3533_devices;
> > > +     u32 count = 0, reg, nchilds;
> >
> > Don't mix multiple declarations with initialisation like this.
> >
> 
> Checkpatch does not complain on style issue, hence this is not prohibited.

Checkpatch does not define good style.

> > >       int ret;
> > >
> > > -     dev_dbg(lm3533->dev, "%s\n", __func__);
> > > +     nchilds = device_get_child_node_count(dev);
> > > +     if (!nchilds || nchilds > LM3533_CELLS_MAX)
> > > +             return dev_err_probe(dev, -ENODEV,
> > > +                                  "num of child nodes is not supported\n");
> > >
> > > -     if (!pdata) {
> > > -             dev_err(lm3533->dev, "no platform data\n");
> > > -             return -EINVAL;
> > > -     }
> > > +     lm3533_devices = devm_kcalloc(dev, nchilds, sizeof(*lm3533_devices),
> > > +                                   GFP_KERNEL);
> > > +     if (!lm3533_devices)
> > > +             return -ENOMEM;
> > >
> > > -     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");
> > > +     device_for_each_child_node_scoped(dev, child) {
> > > +             if (count >= nchilds)
> > > +                     break;
> >
> > How could count be larger than nchilds?
> >
> 
> Only if the tree is malformed, hence this check was added.

But you've just retrieved nchilds by parsing the tree and counting the
child nodes. So how can count possibly be larger than nchilds here?

> > > +
> > > +             if (fwnode_device_is_compatible(child, "ti,lm3533-als")) {
> > > +                     lm3533_devices[count].name = "lm3533-als";
> > > +                     lm3533_devices[count].of_compatible = "ti,lm3533-als";
> > > +                     lm3533_devices[count].id = PLATFORM_DEVID_NONE;
> > > +
> > > +                     lm3533->have_als = true;
> > > +                     count++;
> > > +             } else if (fwnode_device_is_compatible(child, "ti,lm3533-backlight")) {
> > > +                     ret = fwnode_property_read_u32(child, "reg", &reg);
> > > +                     if (ret || reg >= LM3533_HVLED_ID_MAX) {
> > > +                             dev_err(dev, "invalid backlight node %pfw\n", child);
> > > +                             continue;
> > > +                     }
> > > +
> > > +                     lm3533_devices[count].name = "lm3533-backlight";
> > > +                     lm3533_devices[count].of_compatible = "ti,lm3533-backlight";
> > > +                     lm3533_devices[count].id = reg;
> > > +                     lm3533_devices[count].of_reg = reg;
> > > +                     lm3533_devices[count].use_of_reg = true;
> > > +
> > > +                     lm3533->have_backlights = true;
> > > +                     count++;
> > > +             } else if (fwnode_device_is_compatible(child, "ti,lm3533-leds")) {
> > > +                     ret = fwnode_property_read_u32(child, "reg", &reg);
> > > +                     if (ret || reg < LM3533_HVLED_ID_MAX ||
> > > +                         reg > LM3533_LVLED_ID_MAX) {
> > > +                             dev_err(dev, "invalid LED node %pfw\n", child);
> > > +                             continue;
> > > +                     }
> > > +
> > > +                     lm3533_devices[count].name = "lm3533-leds";
> > > +                     lm3533_devices[count].of_compatible = "ti,lm3533-leds";
> > > +                     lm3533_devices[count].id = reg - LM3533_HVLED_ID_MAX;
> > > +                     lm3533_devices[count].of_reg = reg;
> > > +                     lm3533_devices[count].use_of_reg = true;
> > > +
> > > +                     lm3533->have_leds = true;
> > > +                     count++;
> > > +             }
> > > +     }
> >
> > Why do you need the above at all? Shouldn't you be able to just use
> > of_platform_populate().
> >
> 
> of_platform_populate() is not a part of mfd framework.

We have several MFD drivers using of_platform_populate().

> > >
> > >       lm3533_enable(lm3533);
> > >
> > >       ret = regmap_update_bits(lm3533->regmap, LM3533_REG_BOOST_PWM,
> > >                                LM3533_BOOST_FREQ_MASK,
> > > -                              pdata->boost_freq << LM3533_BOOST_FREQ_SHIFT);
> > > +                              lm3533->boost_freq << LM3533_BOOST_FREQ_SHIFT);
> > >       if (ret) {
> > > -             dev_err(lm3533->dev, "failed to set boost frequency\n");
> > > +             dev_err(dev, "failed to set boost frequency\n");
> > >               goto err_disable;
> > >       }
> > >
> > >       ret = regmap_update_bits(lm3533->regmap, LM3533_REG_BOOST_PWM,
> > >                                LM3533_BOOST_OVP_MASK,
> > > -                              pdata->boost_ovp << LM3533_BOOST_OVP_SHIFT);
> > > +                              lm3533->boost_ovp << LM3533_BOOST_OVP_SHIFT);
> > >       if (ret) {
> > > -             dev_err(lm3533->dev, "failed to set boost ovp\n");
> > > +             dev_err(dev, "failed to set boost ovp\n");
> > >               goto err_disable;
> > >       }
> > >
> > > -     lm3533_device_als_init(lm3533);
> > > -     lm3533_device_bl_init(lm3533);
> > > -     lm3533_device_led_init(lm3533);
> > > +     ret = mfd_add_devices(dev, 0, lm3533_devices, count, NULL, 0, NULL);
> > > +     if (ret) {
> > > +             dev_err(dev, "failed to add MFD devices: %d\n", ret);
> > > +             goto err_disable;
> > > +     }
> > >
> > >       return 0;
> > >
> > > @@ -504,7 +440,26 @@ static int lm3533_i2c_probe(struct i2c_client *i2c)
> > >               return PTR_ERR(lm3533->regmap);
> > >
> > >       lm3533->dev = &i2c->dev;
> > > -     lm3533->irq = i2c->irq;
> > > +
> > > +     lm3533->hwen = devm_gpiod_get_optional(lm3533->dev, "enable",
> > > +                                            GPIOD_OUT_LOW);
> > > +     if (IS_ERR(lm3533->hwen))
> > > +             return dev_err_probe(lm3533->dev, PTR_ERR(lm3533->hwen),
> > > +                                  "failed to get HWEN GPIO\n");
> >
> > Please use brackets around multline statements for readability
> > throughout.
> >
> 
> Checkpatch does not complain on style issue, hence this is not prohibited.

Checkpatch is irrelevant.

> > > +
> > > +     device_property_read_u32(lm3533->dev, "ti,boost-ovp-microvolt",
> > > +                              &lm3533->boost_ovp);
> > > +
> > > +     lm3533->boost_ovp = clamp(lm3533->boost_ovp, LM3533_BOOST_OVP_MIN,
> > > +                               LM3533_BOOST_OVP_MAX);
> > > +     lm3533->boost_ovp = lm3533->boost_ovp / (8 * MICRO) - 2;
> > > +
> > > +     device_property_read_u32(lm3533->dev, "ti,boost-freq-hz",
> > > +                              &lm3533->boost_freq);
> > > +
> > > +     lm3533->boost_freq = clamp(lm3533->boost_freq, LM3533_BOOST_FREQ_MIN,
> > > +                                LM3533_BOOST_FREQ_MAX);
> > > +     lm3533->boost_freq = lm3533->boost_freq / (500 * KILO) - 1;
> >
> > Again, why clamp instead of failing probe?
> >
> 
> According to OF schema default lower margin is set to
> LM3533_BOOST_FREQ_MIN so clamping seems a good option here, even
> though it will clamp max value.

Just let the driver fail to probe as the sanity checks are already
there.

> > >       return lm3533_device_init(lm3533);
> > >  }
> > > @@ -518,6 +473,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" },
> > >       { }

Shouldn't you drop i2c probing now as well?

> > > @@ -528,6 +489,7 @@ static struct i2c_driver lm3533_i2c_driver = {
> > >       .driver = {
> > >                  .name = "lm3533",
> > >                  .dev_groups = lm3533_attribute_groups,
> > > +                .of_match_table = lm3533_match_table,
> > >       },
> > >       .id_table       = lm3533_i2c_ids,
> > >       .probe          = lm3533_i2c_probe,

Johan