* [PATCH v4 01/14] dt-bindings: leds: Document TI LM3533 LED controller
2026-06-06 4:57 [PATCH v4 00/14] mfd: lm3533: convert to OF bindings, improve support Svyatoslav Ryhel
@ 2026-06-06 4:57 ` Svyatoslav Ryhel
2026-06-06 4:57 ` [PATCH v4 02/14] mfd: lm3533: Remove driver specific regmap wrappers Svyatoslav Ryhel
` (12 subsequent siblings)
13 siblings, 0 replies; 24+ messages in thread
From: Svyatoslav Ryhel @ 2026-06-06 4:57 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Helge Deller,
Svyatoslav Ryhel
Cc: Johan Hovold, dri-devel, linux-leds, devicetree, linux-kernel,
linux-iio, linux-fbdev
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
---
.../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
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v4 02/14] mfd: lm3533: Remove driver specific regmap wrappers
2026-06-06 4:57 [PATCH v4 00/14] mfd: lm3533: convert to OF bindings, improve support Svyatoslav Ryhel
2026-06-06 4:57 ` [PATCH v4 01/14] dt-bindings: leds: Document TI LM3533 LED controller Svyatoslav Ryhel
@ 2026-06-06 4:57 ` Svyatoslav Ryhel
2026-06-06 6:53 ` Andy Shevchenko
2026-06-06 4:57 ` [PATCH v4 03/14] mfd: lm3533: Remove extern from shared functions in the header Svyatoslav Ryhel
` (11 subsequent siblings)
13 siblings, 1 reply; 24+ messages in thread
From: Svyatoslav Ryhel @ 2026-06-06 4:57 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Helge Deller,
Svyatoslav Ryhel
Cc: Johan Hovold, dri-devel, linux-leds, devicetree, linux-kernel,
linux-iio, linux-fbdev
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 | 58 ++++++++-------------
drivers/mfd/lm3533-core.c | 73 ++++----------------------
drivers/mfd/lm3533-ctrlbank.c | 22 ++++----
drivers/video/backlight/lm3533_bl.c | 33 ++++--------
include/linux/mfd/lm3533.h | 8 +--
6 files changed, 90 insertions(+), 183 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..a48991958eae 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);
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..b78c130d7712 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);
@@ -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");
@@ -94,7 +96,7 @@ int lm3533_ctrlbank_set_brightness(struct lm3533_ctrlbank *cb, u8 val)
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");
@@ -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..03cce6aeb014 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);
diff --git a/include/linux/mfd/lm3533.h b/include/linux/mfd/lm3533.h
index 69059a7a2ce5..db0a5e5c9e39 100644
--- a/include/linux/mfd/lm3533.h
+++ b/include/linux/mfd/lm3533.h
@@ -86,14 +86,10 @@ 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_get_brightness(struct lm3533_ctrlbank *cb, u32 *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_get_pwm(struct lm3533_ctrlbank *cb, u32 *val);
#endif /* __LINUX_MFD_LM3533_H */
--
2.53.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v4 02/14] mfd: lm3533: Remove driver specific regmap wrappers
2026-06-06 4:57 ` [PATCH v4 02/14] mfd: lm3533: Remove driver specific regmap wrappers Svyatoslav Ryhel
@ 2026-06-06 6:53 ` Andy Shevchenko
2026-06-06 7:22 ` Svyatoslav Ryhel
0 siblings, 1 reply; 24+ messages in thread
From: Andy Shevchenko @ 2026-06-06 6:53 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Helge Deller,
Johan Hovold, dri-devel, linux-leds, devicetree, linux-kernel,
linux-iio, linux-fbdev
On Sat, Jun 06, 2026 at 07:57:26AM +0300, Svyatoslav Ryhel wrote:
> Remove driver-specific regmap wrappers in favor of using regmap helpers
> directly.
I like the idea of this patch. Nevertheless I have some suggestions below.
...
> {
> struct lm3533_als *als = iio_priv(indio_dev);
> u8 reg;
> - u8 val;
> + u32 val;
Strictly speaking this should be unsigned int. The regmap API use unsigned int.
...
> 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);
In cases like this perhaps leaving mask would be fine and together with
struct regmap *map = als->lm3533->regmap;
this be nice one-liner:
ret = regmap_assign_bits(map, LM3533_REG_ALS_ZONE_INFO, mask, enable);
> if (ret) {
> dev_err(&indio_dev->dev, "failed to set int mode %d\n",
> enable);
In many cases it won't increase LoC count.
...
> 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_get_brightness(struct lm3533_ctrlbank *cb, u32 *val);
> 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_ctrlbank_get_pwm(struct lm3533_ctrlbank *cb, u32 *val);
Now they become asymmetrical. Perhaps to replace setters, but be careful about
upper bits.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v4 02/14] mfd: lm3533: Remove driver specific regmap wrappers
2026-06-06 6:53 ` Andy Shevchenko
@ 2026-06-06 7:22 ` Svyatoslav Ryhel
0 siblings, 0 replies; 24+ messages in thread
From: Svyatoslav Ryhel @ 2026-06-06 7:22 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Helge Deller,
Johan Hovold, dri-devel, linux-leds, devicetree, linux-kernel,
linux-iio, linux-fbdev
сб, 6 черв. 2026 р. о 09:53 Andy Shevchenko <andriy.shevchenko@intel.com> пише:
>
> On Sat, Jun 06, 2026 at 07:57:26AM +0300, Svyatoslav Ryhel wrote:
> > Remove driver-specific regmap wrappers in favor of using regmap helpers
> > directly.
>
> I like the idea of this patch. Nevertheless I have some suggestions below.
>
> ...
>
> > {
> > struct lm3533_als *als = iio_priv(indio_dev);
> > u8 reg;
> > - u8 val;
> > + u32 val;
>
> Strictly speaking this should be unsigned int. The regmap API use unsigned int.
>
Yes, though regmap defines only 8 bit to be used so it should not matter
> ...
>
> > 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);
>
> In cases like this perhaps leaving mask would be fine and together with
I prefer to remove intermediate variables it the helper allows to
directly pass needed value.
>
> struct regmap *map = als->lm3533->regmap;
>
next patch drops lm3533 so there will be als->regmap which IMHO is
more logical instead of passing entire lm3533 to child devices.
> this be nice one-liner:
>
> ret = regmap_assign_bits(map, LM3533_REG_ALS_ZONE_INFO, mask, enable);
>
> > if (ret) {
> > dev_err(&indio_dev->dev, "failed to set int mode %d\n",
> > enable);
>
> In many cases it won't increase LoC count.
>
> ...
>
> > 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_get_brightness(struct lm3533_ctrlbank *cb, u32 *val);
>
> > 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_ctrlbank_get_pwm(struct lm3533_ctrlbank *cb, u32 *val);
>
> Now they become asymmetrical. Perhaps to replace setters, but be careful about
> upper bits.
>
Yes, I have same thoughts. Upper beats should be irrelevant since
regmap should use only fist 8 bits but I will be careful, thanks.
> --
> With Best Regards,
> Andy Shevchenko
>
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v4 03/14] mfd: lm3533: Remove extern from shared functions in the header
2026-06-06 4:57 [PATCH v4 00/14] mfd: lm3533: convert to OF bindings, improve support Svyatoslav Ryhel
2026-06-06 4:57 ` [PATCH v4 01/14] dt-bindings: leds: Document TI LM3533 LED controller Svyatoslav Ryhel
2026-06-06 4:57 ` [PATCH v4 02/14] mfd: lm3533: Remove driver specific regmap wrappers Svyatoslav Ryhel
@ 2026-06-06 4:57 ` Svyatoslav Ryhel
2026-06-06 6:46 ` Andy Shevchenko
2026-06-06 4:57 ` [PATCH v4 04/14] mfd: lm3533: Pass only regmap and light sensor presence to child devices Svyatoslav Ryhel
` (10 subsequent siblings)
13 siblings, 1 reply; 24+ messages in thread
From: Svyatoslav Ryhel @ 2026-06-06 4:57 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Helge Deller,
Svyatoslav Ryhel
Cc: Johan Hovold, dri-devel, linux-leds, devicetree, linux-kernel,
linux-iio, linux-fbdev
Using 'extern' is not required, drop them.
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
include/linux/mfd/lm3533.h | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/include/linux/mfd/lm3533.h b/include/linux/mfd/lm3533.h
index db0a5e5c9e39..aab8f08dfcb0 100644
--- a/include/linux/mfd/lm3533.h
+++ b/include/linux/mfd/lm3533.h
@@ -82,14 +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, u8 val);
-extern int lm3533_ctrlbank_get_brightness(struct lm3533_ctrlbank *cb, u32 *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, 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, u8 val);
+int lm3533_ctrlbank_get_brightness(struct lm3533_ctrlbank *cb, u32 *val);
+int lm3533_ctrlbank_set_max_current(struct lm3533_ctrlbank *cb, u16 imax);
+int lm3533_ctrlbank_set_pwm(struct lm3533_ctrlbank *cb, u8 val);
+int lm3533_ctrlbank_get_pwm(struct lm3533_ctrlbank *cb, u32 *val);
#endif /* __LINUX_MFD_LM3533_H */
--
2.53.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v4 03/14] mfd: lm3533: Remove extern from shared functions in the header
2026-06-06 4:57 ` [PATCH v4 03/14] mfd: lm3533: Remove extern from shared functions in the header Svyatoslav Ryhel
@ 2026-06-06 6:46 ` Andy Shevchenko
0 siblings, 0 replies; 24+ messages in thread
From: Andy Shevchenko @ 2026-06-06 6:46 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Helge Deller,
Johan Hovold, dri-devel, linux-leds, devicetree, linux-kernel,
linux-iio, linux-fbdev
On Sat, Jun 06, 2026 at 07:57:27AM +0300, Svyatoslav Ryhel wrote:
> Using 'extern' is not required, drop them.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v4 04/14] mfd: lm3533: Pass only regmap and light sensor presence to child devices
2026-06-06 4:57 [PATCH v4 00/14] mfd: lm3533: convert to OF bindings, improve support Svyatoslav Ryhel
` (2 preceding siblings ...)
2026-06-06 4:57 ` [PATCH v4 03/14] mfd: lm3533: Remove extern from shared functions in the header Svyatoslav Ryhel
@ 2026-06-06 4:57 ` Svyatoslav Ryhel
2026-06-06 4:57 ` [PATCH v4 05/14] iio: light: lm3533-als: Remove redundant pdata helpers Svyatoslav Ryhel
` (9 subsequent siblings)
13 siblings, 0 replies; 24+ messages in thread
From: Svyatoslav Ryhel @ 2026-06-06 4:57 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Helge Deller,
Svyatoslav Ryhel
Cc: Johan Hovold, dri-devel, linux-leds, devicetree, linux-kernel,
linux-iio, linux-fbdev
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 a48991958eae..f57d97201816 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 b78c130d7712..07aec46e8c0a 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, u16 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, u8 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, u8 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 03cce6aeb014..39f438114c48 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 aab8f08dfcb0..696014deaa7c 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
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v4 05/14] iio: light: lm3533-als: Remove redundant pdata helpers
2026-06-06 4:57 [PATCH v4 00/14] mfd: lm3533: convert to OF bindings, improve support Svyatoslav Ryhel
` (3 preceding siblings ...)
2026-06-06 4:57 ` [PATCH v4 04/14] mfd: lm3533: Pass only regmap and light sensor presence to child devices Svyatoslav Ryhel
@ 2026-06-06 4:57 ` Svyatoslav Ryhel
2026-06-06 4:57 ` [PATCH v4 06/14] mfd: lm3533-core: " Svyatoslav Ryhel
` (8 subsequent siblings)
13 siblings, 0 replies; 24+ messages in thread
From: Svyatoslav Ryhel @ 2026-06-06 4:57 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Helge Deller,
Svyatoslav Ryhel
Cc: Johan Hovold, dri-devel, linux-leds, devicetree, linux-kernel,
linux-iio, linux-fbdev
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 | 51 +++++++++-------------------------
1 file changed, 13 insertions(+), 38 deletions(-)
diff --git a/drivers/iio/light/lm3533-als.c b/drivers/iio/light/lm3533-als.c
index a9af8e2b965f..c4035d1c8d57 100644
--- a/drivers/iio/light/lm3533-als.c
+++ b/drivers/iio/light/lm3533-als.c
@@ -708,54 +708,29 @@ 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)
-{
- 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;
-}
-
-static int lm3533_als_set_resistor(struct lm3533_als *als, u8 val)
-{
- int ret;
-
- if (val < LM3533_ALS_RESISTOR_MIN || val > LM3533_ALS_RESISTOR_MAX) {
- dev_err(&als->pdev->dev, "invalid resistor value\n");
- return -EINVAL;
- }
-
- 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)
{
+ struct device *dev = &als->pdev->dev;
int ret;
- ret = lm3533_als_set_input_mode(als, pdata->pwm_mode);
+ ret = regmap_assign_bits(als->regmap, LM3533_REG_ALS_CONF,
+ LM3533_ALS_INPUT_MODE_MASK, pdata->pwm_mode);
if (ret)
- return ret;
+ return dev_err_probe(dev, ret, "failed to set input mode %d\n",
+ pdata->pwm_mode);
/* ALS input is always high impedance in PWM-mode. */
if (!pdata->pwm_mode) {
- ret = lm3533_als_set_resistor(als, pdata->r_select);
+ 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,
+ pdata->r_select);
if (ret)
- return ret;
+ return dev_err_probe(dev, ret, "failed to set resistor\n");
}
return 0;
--
2.53.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v4 06/14] mfd: lm3533-core: Remove redundant pdata helpers
2026-06-06 4:57 [PATCH v4 00/14] mfd: lm3533: convert to OF bindings, improve support Svyatoslav Ryhel
` (4 preceding siblings ...)
2026-06-06 4:57 ` [PATCH v4 05/14] iio: light: lm3533-als: Remove redundant pdata helpers Svyatoslav Ryhel
@ 2026-06-06 4:57 ` Svyatoslav Ryhel
2026-06-06 4:57 ` [PATCH v4 07/14] mfd: lm3533: Switch sysfs_create_group() to device_add_group() Svyatoslav Ryhel
` (7 subsequent siblings)
13 siblings, 0 replies; 24+ messages in thread
From: Svyatoslav Ryhel @ 2026-06-06 4:57 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Helge Deller,
Svyatoslav Ryhel
Cc: Johan Hovold, dri-devel, linux-leds, devicetree, linux-kernel,
linux-iio, linux-fbdev
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
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v4 07/14] mfd: lm3533: Switch sysfs_create_group() to device_add_group()
2026-06-06 4:57 [PATCH v4 00/14] mfd: lm3533: convert to OF bindings, improve support Svyatoslav Ryhel
` (5 preceding siblings ...)
2026-06-06 4:57 ` [PATCH v4 06/14] mfd: lm3533-core: " Svyatoslav Ryhel
@ 2026-06-06 4:57 ` Svyatoslav Ryhel
2026-06-06 4:57 ` [PATCH v4 08/14] mfd: lm3533: Convert to use OF bindings Svyatoslav Ryhel
` (6 subsequent siblings)
13 siblings, 0 replies; 24+ messages in thread
From: Svyatoslav Ryhel @ 2026-06-06 4:57 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Helge Deller,
Svyatoslav Ryhel
Cc: Johan Hovold, dri-devel, linux-leds, devicetree, linux-kernel,
linux-iio, linux-fbdev
Switch from sysfs_create_group() to device_add_group() including device
managed where appropriate.
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
drivers/mfd/lm3533-core.c | 4 ++--
drivers/video/backlight/lm3533_bl.c | 19 ++++++-------------
2 files changed, 8 insertions(+), 15 deletions(-)
diff --git a/drivers/mfd/lm3533-core.c b/drivers/mfd/lm3533-core.c
index 45e7f7481aa0..e3efc9f6f348 100644
--- a/drivers/mfd/lm3533-core.c
+++ b/drivers/mfd/lm3533-core.c
@@ -416,7 +416,7 @@ 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);
+ ret = device_add_group(lm3533->dev, &lm3533_attribute_group);
if (ret < 0) {
dev_err(lm3533->dev, "failed to create sysfs attributes\n");
goto err_unregister;
@@ -436,7 +436,7 @@ static void lm3533_device_exit(struct lm3533 *lm3533)
{
dev_dbg(lm3533->dev, "%s\n", __func__);
- sysfs_remove_group(&lm3533->dev->kobj, &lm3533_attribute_group);
+ device_remove_group(lm3533->dev, &lm3533_attribute_group);
mfd_remove_devices(lm3533->dev);
lm3533_disable(lm3533);
diff --git a/drivers/video/backlight/lm3533_bl.c b/drivers/video/backlight/lm3533_bl.c
index 39f438114c48..b4363a30b2c8 100644
--- a/drivers/video/backlight/lm3533_bl.c
+++ b/drivers/video/backlight/lm3533_bl.c
@@ -304,28 +304,22 @@ 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;
- }
+ ret = devm_device_add_group(&bd->dev, &lm3533_bl_attribute_group);
+ if (ret < 0)
+ return dev_err_probe(&pdev->dev, ret,
+ "failed to create sysfs attributes\n");
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
--
2.53.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v4 08/14] mfd: lm3533: Convert to use OF bindings
2026-06-06 4:57 [PATCH v4 00/14] mfd: lm3533: convert to OF bindings, improve support Svyatoslav Ryhel
` (6 preceding siblings ...)
2026-06-06 4:57 ` [PATCH v4 07/14] mfd: lm3533: Switch sysfs_create_group() to device_add_group() Svyatoslav Ryhel
@ 2026-06-06 4:57 ` Svyatoslav Ryhel
2026-06-06 5:14 ` sashiko-bot
2026-06-06 4:57 ` [PATCH v4 09/14] mfd: lm3533: Add support for VIN power supply Svyatoslav Ryhel
` (5 subsequent siblings)
13 siblings, 1 reply; 24+ messages in thread
From: Svyatoslav Ryhel @ 2026-06-06 4:57 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Helge Deller,
Svyatoslav Ryhel
Cc: Johan Hovold, dri-devel, linux-leds, devicetree, linux-kernel,
linux-iio, linux-fbdev
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 | 51 ++++--
drivers/mfd/lm3533-core.c | 236 ++++++++++++----------------
drivers/mfd/lm3533-ctrlbank.c | 5 -
drivers/video/backlight/lm3533_bl.c | 59 ++++---
include/linux/mfd/lm3533.h | 52 +-----
6 files changed, 225 insertions(+), 245 deletions(-)
diff --git a/drivers/iio/light/lm3533-als.c b/drivers/iio/light/lm3533-als.c
index c4035d1c8d57..d51cf96970b9 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,27 +714,36 @@ 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);
/* ALS input is always high impedance in PWM-mode. */
- if (!pdata->pwm_mode) {
- if (pdata->r_select < LM3533_ALS_RESISTOR_MIN ||
- pdata->r_select > LM3533_ALS_RESISTOR_MAX)
+ if (!als->pwm_mode) {
+ 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");
}
@@ -792,7 +807,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;
@@ -802,12 +816,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;
@@ -816,25 +824,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;
@@ -853,7 +863,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;
@@ -867,13 +877,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 f57d97201816..a661aedcdc60 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,23 @@ 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);
+
+ led->pwm = 0;
+ device_property_read_u32(&pdev->dev, "ti,pwm-config-mask", &led->pwm);
+
+ ret = lm3533_led_setup(led);
if (ret)
goto err_deregister;
@@ -725,9 +737,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 e3efc9f6f348..5af1d5476d3e 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
*/
@@ -296,129 +266,99 @@ static struct attribute_group lm3533_attribute_group = {
.attrs = lm3533_attributes
};
-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)
+static int lm3533_device_init(struct lm3533 *lm3533)
{
- struct lm3533_platform_data *pdata = dev_get_platdata(lm3533->dev);
- int i;
+ struct device *dev = lm3533->dev;
+ struct mfd_cell *lm3533_devices;
+ u32 count = 0, reg, nchilds;
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);
+ 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");
- 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_devices = devm_kcalloc(dev, nchilds, sizeof(*lm3533_devices),
+ GFP_KERNEL);
+ if (!lm3533_devices)
+ return -ENOMEM;
- lm3533->have_leds = 1;
+ device_for_each_child_node_scoped(dev, child) {
+ if (count >= nchilds)
+ break;
- return 0;
-}
+ 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;
-static int lm3533_device_init(struct lm3533 *lm3533)
-{
- struct lm3533_platform_data *pdata = dev_get_platdata(lm3533->dev);
- int ret;
+ lm3533->have_als = true;
+ count++;
+ }
- dev_dbg(lm3533->dev, "%s\n", __func__);
+ if (fwnode_device_is_compatible(child, "ti,lm3533-backlight")) {
+ ret = fwnode_property_read_u32(child, "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++;
+ }
- if (!pdata) {
- dev_err(lm3533->dev, "no platform data\n");
- return -EINVAL;
+ if (fwnode_device_is_compatible(child, "ti,lm3533-leds")) {
+ ret = fwnode_property_read_u32(child, "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->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");
-
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;
+ }
- ret = device_add_group(lm3533->dev, &lm3533_attribute_group);
+ ret = device_add_group(dev, &lm3533_attribute_group);
if (ret < 0) {
- dev_err(lm3533->dev, "failed to create sysfs attributes\n");
+ dev_err(dev, "failed to create sysfs attributes\n");
goto err_unregister;
}
@@ -509,7 +449,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);
}
@@ -523,6 +482,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" },
{ }
@@ -532,6 +497,7 @@ MODULE_DEVICE_TABLE(i2c, lm3533_i2c_ids);
static struct i2c_driver lm3533_i2c_driver = {
.driver = {
.name = "lm3533",
+ .of_match_table = lm3533_match_table,
},
.id_table = lm3533_i2c_ids,
.probe = lm3533_i2c_probe,
diff --git a/drivers/mfd/lm3533-ctrlbank.c b/drivers/mfd/lm3533-ctrlbank.c
index 07aec46e8c0a..0454b4bd2da2 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 b4363a30b2c8..e91c598342ad 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;
};
@@ -237,25 +242,25 @@ static struct attribute_group lm3533_bl_attribute_group = {
.attrs = lm3533_bl_attributes
};
-static int lm3533_bl_setup(struct lm3533_bl *bl,
- struct lm3533_bl_platform_data *pdata)
+static int lm3533_bl_setup(struct lm3533_bl *bl)
{
int 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__);
@@ -264,12 +269,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;
@@ -287,13 +286,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);
@@ -304,6 +311,17 @@ static int lm3533_bl_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, bl);
+ 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);
+
+ device_property_read_u32(&pdev->dev, "ti,pwm-config-mask", &bl->pwm);
+
+ ret = lm3533_bl_setup(bl);
+ if (ret)
+ return ret;
+
ret = devm_device_add_group(&bd->dev, &lm3533_bl_attribute_group);
if (ret < 0)
return dev_err_probe(&pdev->dev, ret,
@@ -311,10 +329,6 @@ static int lm3533_bl_probe(struct platform_device *pdev)
backlight_update_status(bd);
- ret = lm3533_bl_setup(bl, pdata);
- if (ret)
- return ret;
-
ret = lm3533_ctrlbank_enable(&bl->cb);
if (ret)
return ret;
@@ -366,10 +380,17 @@ static void lm3533_bl_shutdown(struct platform_device *pdev)
lm3533_ctrlbank_disable(&bl->cb);
}
+static const struct of_device_id lm3533_bl_match_table[] = {
+ { .compatible = "ti,lm3533-backlight" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, lm3533_bl_match_table);
+
static struct platform_driver lm3533_bl_driver = {
.driver = {
.name = "lm3533-backlight",
.pm = &lm3533_bl_pm_ops,
+ .of_match_table = lm3533_bl_match_table,
},
.probe = lm3533_bl_probe,
.remove = lm3533_bl_remove,
diff --git a/include/linux/mfd/lm3533.h b/include/linux/mfd/lm3533.h
index 696014deaa7c..d3f7b07dbefc 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
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v4 08/14] mfd: lm3533: Convert to use OF bindings
2026-06-06 4:57 ` [PATCH v4 08/14] mfd: lm3533: Convert to use OF bindings Svyatoslav Ryhel
@ 2026-06-06 5:14 ` sashiko-bot
0 siblings, 0 replies; 24+ messages in thread
From: sashiko-bot @ 2026-06-06 5:14 UTC (permalink / raw)
To: Svyatoslav Ryhel; +Cc: devicetree, dri-devel, conor+dt, robh
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] A crafted device tree with a single child node containing multiple matching compatible strings causes a heap buffer overflow in `lm3533_devices`.
- [High] `lm3533_led_probe` mistakenly returns 0 (success) on an error path, causing the device to remain bound and leading to a double-free on removal.
- [Medium] `lm3533_bl_probe` returns a positive integer on an error path, violating the probe function API and triggering a kernel warning.
- [Low] Missing 'led-sources' property causes unintended I2C transactions due to improper checking of negative error codes.
- [Low] The `ti,pwm-config-mask` property is implicitly truncated from 32 bits to 8 bits before validation, allowing invalid values to bypass the bounds check.
--
commit 064deeedced51b760172cc0805ab8c80bd2dafdf
Author: Svyatoslav Ryhel <clamor95@gmail.com>
Date: Sat Jun 6 04:57:32 2026 +0000
mfd: lm3533: Convert to use OF bindings
Since there are no users of this driver via platform data, remove the
platform data support and switch to using Device Tree bindings.
> diff --git a/drivers/mfd/lm3533-core.c b/drivers/mfd/lm3533-core.c
[ ... ]
> @@ -296,129 +266,99 @@ static struct attribute_group lm3533_attribute_group = {
[ ... ]
> +static int lm3533_device_init(struct lm3533 *lm3533)
> {
[ ... ]
> + 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++;
> + }
>
> + if (fwnode_device_is_compatible(child, "ti,lm3533-backlight")) {
> + ret = fwnode_property_read_u32(child, "reg", ®);
[Severity: High]
Can this loop overflow the lm3533_devices array?
Since the bounds check if (count >= nchilds) break; only happens at the top
of the loop, if a single child node specifies multiple compatible strings (e.g.,
compatible = "ti,lm3533-als", "ti,lm3533-backlight"), multiple if branches
will execute during the same iteration.
This would increment the count multiple times per node, bypassing the bounds
check and writing past the end of the allocated lm3533_devices buffer.
Should these be else if conditions to ensure only one match per child node?
> diff --git a/drivers/leds/leds-lm3533.c b/drivers/leds/leds-lm3533.c
[ ... ]
> @@ -680,15 +684,23 @@ static int lm3533_led_probe(struct platform_device *pdev)
[ ... ]
> 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);
> +
> + led->pwm = 0;
> + device_property_read_u32(&pdev->dev, "ti,pwm-config-mask", &led->pwm);
> +
[Severity: Low]
Could this allow invalid values to bypass validation?
The property ti,pwm-config-mask is read into the u32 variable led->pwm,
but it is later passed to lm3533_ctrlbank_set_pwm() which accepts a u8
argument. This implicitly truncates the 32-bit integer to 8 bits.
If a device tree mistakenly specifies a value exceeding 255 (like <0x13f>),
it will be truncated to 0x3f before validation. Inside
lm3533_ctrlbank_set_pwm(), the truncated value will easily pass the
val > LM3533_PWM_MAX (0x3f) bounds check, resulting in a silent
misconfiguration.
> + ret = lm3533_led_setup(led);
> if (ret)
> goto err_deregister;
[Severity: Low]
Does this code correctly handle negative error codes when led-sources is
missing?
In lm3533_led_probe(), led->num_leds is assigned the result of
device_property_count_u32(..., "led-sources"). If this property is absent,
it returns -EINVAL.
Later in lm3533_led_setup(), the condition if (led->num_leds) will
evaluate to true for negative values. While the inner loop is skipped,
regmap_update_bits() executes with a mask of 0, causing unnecessary hardware
I2C reads.
Should the condition be > 0 to properly skip the hardware update
when the property is absent? (Note: This same behavior exists in
lm3533_bl_setup() for bl->num_leds).
[Severity: High]
Does the error path for led-sources leak a successful return code?
In lm3533_led_probe(), if the count of led-sources exceeds
LM3533_LVCTRLBANK_MAX, the code logs an error and jumps to err_deregister.
However, the ret variable at this point is 0 from the earlier successful
led_classdev_register() call.
Returning 0 falsely informs the driver core that probe succeeded. When the
module is removed, lm3533_led_remove() will invoke led_classdev_unregister()
on the already-unregistered LED class device, resulting in a use-after-free or
double-free.
Should ret be set to a negative error code (e.g., -EINVAL) before jumping
to the error label?
> diff --git a/drivers/video/backlight/lm3533_bl.c b/drivers/video/backlight/lm3533_bl.c
[ ... ]
> @@ -304,6 +311,17 @@ static int lm3533_bl_probe(struct platform_device *pdev)
[ ... ]
> + 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);
> +
> + device_property_read_u32(&pdev->dev, "ti,pwm-config-mask", &bl->pwm);
> +
> + ret = lm3533_bl_setup(bl);
> + if (ret)
> + return ret;
> +
[Severity: Medium]
Does the error handling for led-sources violate the probe function API?
In lm3533_bl_probe(), if the count of led-sources exceeds
LM3533_HVCTRLBANK_COUNT, the code executes:
return dev_err_probe(&pdev->dev, bl->num_leds,
"num of LED sources exceeds max %d\n",
LM3533_HVCTRLBANK_COUNT);
dev_err_probe() returns its second argument as the error code. Because
bl->num_leds is a positive value, the probe function returns a positive integer
instead of a negative error code.
This violates the API and causes the driver core's really_probe() to emit a
warning before overriding the return code to -ENXIO.
Should this return a negative error code like -EINVAL instead?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260606045738.21050-1-clamor95@gmail.com?part=8
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v4 09/14] mfd: lm3533: Add support for VIN power supply
2026-06-06 4:57 [PATCH v4 00/14] mfd: lm3533: convert to OF bindings, improve support Svyatoslav Ryhel
` (7 preceding siblings ...)
2026-06-06 4:57 ` [PATCH v4 08/14] mfd: lm3533: Convert to use OF bindings Svyatoslav Ryhel
@ 2026-06-06 4:57 ` Svyatoslav Ryhel
2026-06-06 4:57 ` [PATCH v4 10/14] mfd: lm3533: Set DMA mask Svyatoslav Ryhel
` (4 subsequent siblings)
13 siblings, 0 replies; 24+ messages in thread
From: Svyatoslav Ryhel @ 2026-06-06 4:57 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Helge Deller,
Svyatoslav Ryhel
Cc: Johan Hovold, dri-devel, linux-leds, devicetree, linux-kernel,
linux-iio, linux-fbdev
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 5af1d5476d3e..500cec3cceba 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 {
@@ -332,7 +344,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,
@@ -456,6 +470,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 d3f7b07dbefc..3f9f4c9ef9f3 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
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v4 10/14] mfd: lm3533: Set DMA mask
2026-06-06 4:57 [PATCH v4 00/14] mfd: lm3533: convert to OF bindings, improve support Svyatoslav Ryhel
` (8 preceding siblings ...)
2026-06-06 4:57 ` [PATCH v4 09/14] mfd: lm3533: Add support for VIN power supply Svyatoslav Ryhel
@ 2026-06-06 4:57 ` Svyatoslav Ryhel
2026-06-06 5:08 ` sashiko-bot
2026-06-06 4:57 ` [PATCH v4 11/14] video: backlight: lm3533_bl: Improve logic of sysfs functions Svyatoslav Ryhel
` (3 subsequent siblings)
13 siblings, 1 reply; 24+ messages in thread
From: Svyatoslav Ryhel @ 2026-06-06 4:57 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Helge Deller,
Svyatoslav Ryhel
Cc: Johan Hovold, dri-devel, linux-leds, devicetree, linux-kernel,
linux-iio, linux-fbdev
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 500cec3cceba..5d17da1d998b 100644
--- a/drivers/mfd/lm3533-core.c
+++ b/drivers/mfd/lm3533-core.c
@@ -489,6 +489,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
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v4 10/14] mfd: lm3533: Set DMA mask
2026-06-06 4:57 ` [PATCH v4 10/14] mfd: lm3533: Set DMA mask Svyatoslav Ryhel
@ 2026-06-06 5:08 ` sashiko-bot
0 siblings, 0 replies; 24+ messages in thread
From: sashiko-bot @ 2026-06-06 5:08 UTC (permalink / raw)
To: Svyatoslav Ryhel; +Cc: robh, conor+dt, devicetree, dri-devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but an out-of-bounds array write occurs in `lm3533_device_init` when a child node contains multiple matching compatible strings.
--
commit 5fa77e39c45bca237d41fbd7e2b8b8b97be664ec
Author: Svyatoslav Ryhel <clamor95@gmail.com>
mfd: lm3533: Set DMA mask
This commit sets the DMA mask to zero to prevent a warning in dmesg about
the DMA mask not being set for the platform device. It applies to both
the coherent and regular DMA mask, as neither the core nor the cells
utilize DMA.
> diff --git a/drivers/mfd/lm3533-core.c b/drivers/mfd/lm3533-core.c
> index 500cec3ccebad..5d17da1d998b5 100644
> --- a/drivers/mfd/lm3533-core.c
> +++ b/drivers/mfd/lm3533-core.c
[ ... ]
> @@ -489,6 +489,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);
> }
[Severity: High]
This is a pre-existing issue, but can the device initialization in
lm3533_device_init() write past the bounds of the lm3533_devices array?
The array is allocated based on the total number of child nodes:
lm3533_devices = devm_kcalloc(dev, nchilds, sizeof(*lm3533_devices),
GFP_KERNEL);
However, inside the device_for_each_child_node_scoped() loop, the
compatibility checks are independent if statements without an else if or
continue:
if (fwnode_device_is_compatible(child, "ti,lm3533-als")) {
lm3533_devices[count].name = "lm3533-als";
...
count++;
}
if (fwnode_device_is_compatible(child, "ti,lm3533-backlight")) {
...
count++;
}
If a single child node contains multiple matching compatible strings,
such as "ti,lm3533-als" and "ti,lm3533-backlight", count will be
incremented multiple times during a single loop iteration.
Since the bounds check if (count >= nchilds) break; only happens at the
start of the loop, this sequence would allow count to exceed nchilds,
writing out of bounds on the lm3533_devices array.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260606045738.21050-1-clamor95@gmail.com?part=10
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v4 11/14] video: backlight: lm3533_bl: Improve logic of sysfs functions
2026-06-06 4:57 [PATCH v4 00/14] mfd: lm3533: convert to OF bindings, improve support Svyatoslav Ryhel
` (9 preceding siblings ...)
2026-06-06 4:57 ` [PATCH v4 10/14] mfd: lm3533: Set DMA mask Svyatoslav Ryhel
@ 2026-06-06 4:57 ` Svyatoslav Ryhel
2026-06-06 5:12 ` sashiko-bot
2026-06-06 4:57 ` [PATCH v4 12/14] video: backlight: lm3533_bl: Set initial mapping mode from DT Svyatoslav Ryhel
` (2 subsequent siblings)
13 siblings, 1 reply; 24+ messages in thread
From: Svyatoslav Ryhel @ 2026-06-06 4:57 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Helge Deller,
Svyatoslav Ryhel
Cc: Johan Hovold, dri-devel, linux-leds, devicetree, linux-kernel,
linux-iio, linux-fbdev
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 | 58 ++++++++++-------------------
1 file changed, 20 insertions(+), 38 deletions(-)
diff --git a/drivers/video/backlight/lm3533_bl.c b/drivers/video/backlight/lm3533_bl.c
index e91c598342ad..9ad7ea1bbbcc 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);
+ ret = regmap_test_bits(bl->regmap, LM3533_REG_CTRLBANK_AB_BCONF,
+ CTRLBANK_AB_BCONF_ALS(ctrlbank));
if (ret)
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);
+ ret = regmap_test_bits(bl->regmap, LM3533_REG_CTRLBANK_AB_BCONF,
+ CTRLBANK_AB_BCONF_MODE(ctrlbank));
if (ret)
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
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v4 11/14] video: backlight: lm3533_bl: Improve logic of sysfs functions
2026-06-06 4:57 ` [PATCH v4 11/14] video: backlight: lm3533_bl: Improve logic of sysfs functions Svyatoslav Ryhel
@ 2026-06-06 5:12 ` sashiko-bot
0 siblings, 0 replies; 24+ messages in thread
From: sashiko-bot @ 2026-06-06 5:12 UTC (permalink / raw)
To: Svyatoslav Ryhel; +Cc: robh, conor+dt, dri-devel, devicetree
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] sysfs show functions `show_als_en` and `show_linear` return prematurely when the tested bits are set, breaking the sysfs read and returning an empty/uninitialized buffer.
--
commit 549a7f316072e9f93bfcd3d182e73739a35c0c7a
Author: Svyatoslav Ryhel <clamor95@gmail.com>
video: backlight: lm3533_bl: Improve logic of sysfs functions
Simplify the sysfs logic of properties by switching to macros and proper
regmap helpers.
> diff --git a/drivers/video/backlight/lm3533_bl.c b/drivers/video/backlight/lm3533_bl.c
> --- a/drivers/video/backlight/lm3533_bl.c
> +++ b/drivers/video/backlight/lm3533_bl.c
[ ... ]
> @@ -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);
> + ret = regmap_test_bits(bl->regmap, LM3533_REG_CTRLBANK_AB_BCONF,
> + CTRLBANK_AB_BCONF_ALS(ctrlbank));
> if (ret)
> return ret;
[Severity: High]
Does this error check prematurely exit the function when the bit is set?
regmap_test_bits() returns 1 if the specified bit is set. If the bit is set,
the if (ret) condition evaluates to true, causing the function to return 1
immediately instead of formatting the buffer.
This could break the sysfs API contract and return a single null byte to
userspace instead of the expected string. Should this check be if (ret < 0)
instead?
>
> - 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 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);
> + ret = regmap_test_bits(bl->regmap, LM3533_REG_CTRLBANK_AB_BCONF,
> + CTRLBANK_AB_BCONF_MODE(ctrlbank));
> if (ret)
> return ret;
[Severity: High]
Could this have the same issue as show_als_en() above?
When the linear mode bit is set, regmap_test_bits() returns 1, triggering
this error path and returning 1 directly instead of calling scnprintf().
>
> - 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);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260606045738.21050-1-clamor95@gmail.com?part=11
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v4 12/14] video: backlight: lm3533_bl: Set initial mapping mode from DT
2026-06-06 4:57 [PATCH v4 00/14] mfd: lm3533: convert to OF bindings, improve support Svyatoslav Ryhel
` (10 preceding siblings ...)
2026-06-06 4:57 ` [PATCH v4 11/14] video: backlight: lm3533_bl: Improve logic of sysfs functions Svyatoslav Ryhel
@ 2026-06-06 4:57 ` Svyatoslav Ryhel
2026-06-06 5:16 ` sashiko-bot
2026-06-06 4:57 ` [PATCH v4 13/14] video: backlight: lm3533_bl: Implement backlight_scale property Svyatoslav Ryhel
2026-06-06 4:57 ` [PATCH v4 14/14] video: leds: backlight: lm3533: Support getting LED sources from DT Svyatoslav Ryhel
13 siblings, 1 reply; 24+ messages in thread
From: Svyatoslav Ryhel @ 2026-06-06 4:57 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Helge Deller,
Svyatoslav Ryhel
Cc: Johan Hovold, dri-devel, linux-leds, devicetree, linux-kernel,
linux-iio, linux-fbdev
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 9ad7ea1bbbcc..4f8cde9f290d 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;
};
@@ -226,8 +227,14 @@ static struct attribute_group lm3533_bl_attribute_group = {
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;
@@ -281,6 +288,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
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v4 12/14] video: backlight: lm3533_bl: Set initial mapping mode from DT
2026-06-06 4:57 ` [PATCH v4 12/14] video: backlight: lm3533_bl: Set initial mapping mode from DT Svyatoslav Ryhel
@ 2026-06-06 5:16 ` sashiko-bot
0 siblings, 0 replies; 24+ messages in thread
From: sashiko-bot @ 2026-06-06 5:16 UTC (permalink / raw)
To: Svyatoslav Ryhel; +Cc: dri-devel, devicetree, conor+dt, robh
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Sysfs show functions `show_linear` and `show_als_en` incorrectly handle the boolean return value of `regmap_test_bits()`, causing them to return early without formatting output when the bit is set.
--
commit be155a9e9c191a4d2a13bb2df4a54c1332803f87
Author: Svyatoslav Ryhel <clamor95@gmail.com>
video: backlight: lm3533_bl: Set initial mapping mode from DT
Add support to obtain the initial mapping mode from DT instead of leaving
it unconfigured.
> diff --git a/drivers/video/backlight/lm3533_bl.c b/drivers/video/backlight/lm3533_bl.c
> index 9ad7ea1bbbccb..4f8cde9f290d0 100644
> --- a/drivers/video/backlight/lm3533_bl.c
> +++ b/drivers/video/backlight/lm3533_bl.c
[ ... ]
I noticed a regression in show_linear() and show_als_en() (modified by the
patch 'video: backlight: lm3533_bl: Improve logic of sysfs functions' in
this series) where regmap_test_bits() is called to check if a specific
bit is set:
drivers/video/backlight/lm3533_bl.c:show_als_en() {
...
ret = regmap_test_bits(bl->regmap, LM3533_REG_CTRLBANK_AB_BCONF,
CTRLBANK_AB_BCONF_ALS(ctrlbank));
if (ret)
return ret;
return scnprintf(buf, PAGE_SIZE, "%d\n", ret);
}
[Severity: High]
Does this code handle the boolean return value of regmap_test_bits()
correctly?
Since regmap_test_bits() returns 1 if the bit is set, this error check
triggers when the bit is set and returns 1 directly instead of formatting
the string.
Returning 1 from a sysfs show function signals to the sysfs core that 1 byte
was written to the buffer. Because the sysfs buffer is zero-initialized prior
to the call, doesn't this return a null byte (\0) to userspace instead of the
expected string "1\n"?
The same pattern appears in show_linear():
drivers/video/backlight/lm3533_bl.c:show_linear() {
...
ret = regmap_test_bits(bl->regmap, LM3533_REG_CTRLBANK_AB_BCONF,
CTRLBANK_AB_BCONF_MODE(ctrlbank));
if (ret)
return ret;
return scnprintf(buf, PAGE_SIZE, "%x\n", ret);
}
[Severity: High]
Will this also fail to format the output string and return a null byte to
userspace when the feature is enabled?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260606045738.21050-1-clamor95@gmail.com?part=12
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v4 13/14] video: backlight: lm3533_bl: Implement backlight_scale property
2026-06-06 4:57 [PATCH v4 00/14] mfd: lm3533: convert to OF bindings, improve support Svyatoslav Ryhel
` (11 preceding siblings ...)
2026-06-06 4:57 ` [PATCH v4 12/14] video: backlight: lm3533_bl: Set initial mapping mode from DT Svyatoslav Ryhel
@ 2026-06-06 4:57 ` Svyatoslav Ryhel
2026-06-06 5:17 ` sashiko-bot
2026-06-06 4:57 ` [PATCH v4 14/14] video: leds: backlight: lm3533: Support getting LED sources from DT Svyatoslav Ryhel
13 siblings, 1 reply; 24+ messages in thread
From: Svyatoslav Ryhel @ 2026-06-06 4:57 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Helge Deller,
Svyatoslav Ryhel
Cc: Johan Hovold, dri-devel, linux-leds, devicetree, linux-kernel,
linux-iio, linux-fbdev
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 4f8cde9f290d..2aa4b86042ff 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;
}
@@ -290,6 +294,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
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v4 13/14] video: backlight: lm3533_bl: Implement backlight_scale property
2026-06-06 4:57 ` [PATCH v4 13/14] video: backlight: lm3533_bl: Implement backlight_scale property Svyatoslav Ryhel
@ 2026-06-06 5:17 ` sashiko-bot
0 siblings, 0 replies; 24+ messages in thread
From: sashiko-bot @ 2026-06-06 5:17 UTC (permalink / raw)
To: Svyatoslav Ryhel; +Cc: conor+dt, robh, dri-devel, devicetree
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Sysfs show functions `show_linear` and `show_als_en` incorrectly treat a set hardware bit as an error, leaking uninitialized kernel memory to userspace.
--
commit 5b473ee9a07b0b9fd223194515f44e1ffe1b934d
Author: Svyatoslav Ryhel <clamor95@gmail.com>
video: backlight: lm3533_bl: Implement backlight_scale property
Since the device supports linear and non-linear modes, implement the
backlight_scale property to describe this state.
> diff --git a/drivers/video/backlight/lm3533_bl.c b/drivers/video/backlight/lm3533_bl.c
> index 4f8cde9f290d0..2aa4b86042ff2 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;
> }
[Severity: High]
This isn't a bug introduced by this patch, but looking at the adjacent
show_linear() and show_als_en() functions:
drivers/video/backlight/lm3533_bl.c:show_linear() {
...
ret = regmap_test_bits(bl->regmap, LM3533_REG_CTRLBANK_AB_BCONF,
CTRLBANK_AB_BCONF_MODE(ctrlbank));
if (ret)
return ret;
...
}
If the hardware bit is set, regmap_test_bits() returns 1. Since the check is
if (ret), it immediately returns 1 to the sysfs core without calling
scnprintf() to populate the buffer.
Does this cause the sysfs core to interpret this as one byte successfully
written and copy the first byte of the unwritten buffer to userspace, leaking
potentially uninitialized kernel memory?
Should the check instead be `if (ret < 0)`?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260606045738.21050-1-clamor95@gmail.com?part=13
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v4 14/14] video: leds: backlight: lm3533: Support getting LED sources from DT
2026-06-06 4:57 [PATCH v4 00/14] mfd: lm3533: convert to OF bindings, improve support Svyatoslav Ryhel
` (12 preceding siblings ...)
2026-06-06 4:57 ` [PATCH v4 13/14] video: backlight: lm3533_bl: Implement backlight_scale property Svyatoslav Ryhel
@ 2026-06-06 4:57 ` Svyatoslav Ryhel
2026-06-06 5:21 ` sashiko-bot
13 siblings, 1 reply; 24+ messages in thread
From: Svyatoslav Ryhel @ 2026-06-06 4:57 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Cameron,
David Lechner, Nuno Sá, Andy Shevchenko, Helge Deller,
Svyatoslav Ryhel
Cc: Johan Hovold, dri-devel, linux-leds, devicetree, linux-kernel,
linux-iio, linux-fbdev
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 | 58 ++++++++++++++++++++++++++++-
drivers/video/backlight/lm3533_bl.c | 43 ++++++++++++++++++++-
2 files changed, 99 insertions(+), 2 deletions(-)
diff --git a/drivers/leds/leds-lm3533.c b/drivers/leds/leds-lm3533.c
index a661aedcdc60..fe3e01b265e1 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,7 +631,35 @@ static const struct attribute_group *lm3533_led_attribute_groups[] = {
static int lm3533_led_setup(struct lm3533_led *led)
{
- int ret;
+ u32 output_cfg_shift = 0;
+ u32 output_cfg_val = 0;
+ u32 output_cfg_mask = 0;
+ int ret, i;
+
+ if (led->num_leds) {
+ for (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)
@@ -700,6 +736,26 @@ static int lm3533_led_probe(struct platform_device *pdev)
led->pwm = 0;
device_property_read_u32(&pdev->dev, "ti,pwm-config-mask", &led->pwm);
+ 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);
+ goto err_deregister;
+ }
+
+ /*
+ * 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.
+ */
+ 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 2aa4b86042ff..ed358fb3c759 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;
};
@@ -232,13 +237,30 @@ static struct attribute_group lm3533_bl_attribute_group = {
static int lm3533_bl_setup(struct lm3533_bl *bl)
{
int ctrlbank = lm3533_bl_get_ctrlbank_id(bl);
- int ret;
+ u32 output_cfg_val = 0;
+ u32 output_cfg_mask = 0;
+ int ret, i;
ret = regmap_assign_bits(bl->regmap, LM3533_REG_CTRLBANK_AB_BCONF,
CTRLBANK_AB_BCONF_MODE(ctrlbank), bl->linear);
if (ret)
return ret;
+ if (bl->num_leds) {
+ for (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;
@@ -316,6 +338,25 @@ static int lm3533_bl_probe(struct platform_device *pdev)
device_property_read_u32(&pdev->dev, "ti,pwm-config-mask", &bl->pwm);
+ bl->num_leds = device_property_count_u32(&pdev->dev, "led-sources");
+ if (bl->num_leds > LM3533_HVCTRLBANK_COUNT)
+ return dev_err_probe(&pdev->dev, bl->num_leds,
+ "num of LED sources exceeds max %d\n",
+ LM3533_HVCTRLBANK_COUNT);
+
+ /*
+ * 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.
+ */
+ 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
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v4 14/14] video: leds: backlight: lm3533: Support getting LED sources from DT
2026-06-06 4:57 ` [PATCH v4 14/14] video: leds: backlight: lm3533: Support getting LED sources from DT Svyatoslav Ryhel
@ 2026-06-06 5:21 ` sashiko-bot
0 siblings, 0 replies; 24+ messages in thread
From: sashiko-bot @ 2026-06-06 5:21 UTC (permalink / raw)
To: Svyatoslav Ryhel; +Cc: dri-devel, robh, devicetree, conor+dt
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Probe returns 0 on failure during LED sources validation, causing a double-free on removal.
- [High] Out-of-bounds bitmask calculation for LM3533_REG_OUTPUT_CONF1 corrupts regcache and causes probe failures on SMBus.
- [Medium] Probe returns a positive integer on failure, violating the driver core API and generating an incorrect error code.
- [Low] Negative error codes from missing properties are evaluated as true, causing redundant regmap operations.
--
commit a17954bc9b86db17d86986ace3623a64fc46dd3b
Author: Svyatoslav Ryhel <clamor95@gmail.com>
video: leds: backlight: lm3533: Support getting LED sources from DT
Add Control Bank to HVLED/LVLED muxing support based on the led-sources
defined in the device tree.
> diff --git a/drivers/leds/leds-lm3533.c b/drivers/leds/leds-lm3533.c
> index a661aedcdc60a..fe3e01b265e10 100644
> --- a/drivers/leds/leds-lm3533.c
> +++ b/drivers/leds/leds-lm3533.c
[ ... ]
> @@ -623,7 +631,35 @@ static const struct attribute_group *lm3533_led_attribute_groups[] = {
>
> static int lm3533_led_setup(struct lm3533_led *led)
> {
> - int ret;
> + u32 output_cfg_shift = 0;
> + u32 output_cfg_val = 0;
> + u32 output_cfg_mask = 0;
> + int ret, i;
> +
> + if (led->num_leds) {
[Severity: Low]
If the led-sources property is not present, device_property_count_u32()
returns a negative error code. Does this cause the condition to evaluate
to true, skipping the loop but triggering an unnecessary regmap_update_bits()
with a 0 mask?
> + for (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);
[Severity: High]
For LVLED4 and LVLED5, output_cfg_mask could have bits 6 or 8 set. When
shifted left by OUTPUT_CONF1_SHIFT (2), the mask exceeds 8 bits. Since
LM3533_REG_OUTPUT_CONF1 is an 8-bit register, does this corrupt the regmap
cache or cause SMBus probe failures by passing a mask larger than 0xFF?
> + 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;
> + }
[ ... ]
> @@ -700,6 +736,26 @@ static int lm3533_led_probe(struct platform_device *pdev)
> led->pwm = 0;
> device_property_read_u32(&pdev->dev, "ti,pwm-config-mask", &led->pwm);
>
> + 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);
> + goto err_deregister;
[Severity: High]
Since led_classdev_register() was already called earlier in this function and
returned 0, ret is still 0 here. Will jumping to err_deregister return 0 on
failure, leading to a double-free on driver removal when
led_classdev_unregister() is called again and corrupting leds_list?
> + }
[ ... ]
> diff --git a/drivers/video/backlight/lm3533_bl.c b/drivers/video/backlight/lm3533_bl.c
> index 2aa4b86042ff2..ed358fb3c7596 100644
> --- a/drivers/video/backlight/lm3533_bl.c
> +++ b/drivers/video/backlight/lm3533_bl.c
[ ... ]
> @@ -232,13 +237,30 @@ static struct attribute_group lm3533_bl_attribute_group = {
> static int lm3533_bl_setup(struct lm3533_bl *bl)
> {
> int ctrlbank = lm3533_bl_get_ctrlbank_id(bl);
> - int ret;
> + u32 output_cfg_val = 0;
> + u32 output_cfg_mask = 0;
> + int ret, i;
>
> ret = regmap_assign_bits(bl->regmap, LM3533_REG_CTRLBANK_AB_BCONF,
> CTRLBANK_AB_BCONF_MODE(ctrlbank), bl->linear);
> if (ret)
> return ret;
>
> + if (bl->num_leds) {
[Severity: Low]
Similar to lm3533_led_setup(), if the led-sources property is absent,
device_property_count_u32() returns a negative error code. Will this cause
the condition to evaluate to true and trigger an unnecessary I2C write with
a 0 mask?
> + for (i = 0; i < bl->num_leds; i++) {
> + if (bl->led_strings[i] >= LM3533_HVCTRLBANK_COUNT)
> + continue;
[ ... ]
> @@ -316,6 +338,25 @@ static int lm3533_bl_probe(struct platform_device *pdev)
>
> device_property_read_u32(&pdev->dev, "ti,pwm-config-mask", &bl->pwm);
>
> + bl->num_leds = device_property_count_u32(&pdev->dev, "led-sources");
> + if (bl->num_leds > LM3533_HVCTRLBANK_COUNT)
> + return dev_err_probe(&pdev->dev, bl->num_leds,
> + "num of LED sources exceeds max %d\n",
> + LM3533_HVCTRLBANK_COUNT);
[Severity: Medium]
Because bl->num_leds is strictly positive here, does returning it via
dev_err_probe() violate the driver core API which expects a negative error
code? The driver core negates positive return values, which could result in an
unrelated error code like -ESRCH.
> +
> + /*
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260606045738.21050-1-clamor95@gmail.com?part=14
^ permalink raw reply [flat|nested] 24+ messages in thread