* [PATCH v1 0/6] mfd: lm3533: convert to OF bindings, improve support
@ 2026-05-17 7:43 Svyatoslav Ryhel
2026-05-17 7:43 ` [PATCH v1 1/6] dt-bindings: leds: Document TI LM3533 LED controller Svyatoslav Ryhel
` (6 more replies)
0 siblings, 7 replies; 24+ messages in thread
From: Svyatoslav Ryhel @ 2026-05-17 7:43 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: dri-devel, linux-leds, devicetree, linux-kernel, linux-iio,
linux-fbdev
Convert LM3533 to OF bindings, add missing VIN supply, add support for
setting mapping mode and LED sources based on device tree.
Svyatoslav Ryhel (6):
dt-bindings: leds: Document TI LM3533 LED controller
mfd: lm3533: Convert to use OF bindings
mfd: lm3533: Add support for VIN power supply
mfd: lm3533: set DMA mask
video: backlight: lm3533_bl: Set initial mapping mode from DT
video: leds: backlight: lm3533: Support getting LED sources from DT
.../leds/backlight/ti,lm3533-backlight.yaml | 68 +++++
.../bindings/leds/ti,lm3533-leds.yaml | 66 ++++
.../devicetree/bindings/leds/ti,lm3533.yaml | 190 ++++++++++++
drivers/iio/light/lm3533-als.c | 123 +++-----
drivers/leds/leds-lm3533.c | 117 ++++++--
drivers/mfd/lm3533-core.c | 281 +++++++-----------
drivers/video/backlight/lm3533_bl.c | 123 ++++++--
include/linux/mfd/lm3533.h | 52 +---
8 files changed, 660 insertions(+), 360 deletions(-)
create mode 100644 Documentation/devicetree/bindings/leds/backlight/ti,lm3533-backlight.yaml
create mode 100644 Documentation/devicetree/bindings/leds/ti,lm3533-leds.yaml
create mode 100644 Documentation/devicetree/bindings/leds/ti,lm3533.yaml
--
2.51.0
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH v1 1/6] dt-bindings: leds: Document TI LM3533 LED controller 2026-05-17 7:43 [PATCH v1 0/6] mfd: lm3533: convert to OF bindings, improve support Svyatoslav Ryhel @ 2026-05-17 7:43 ` Svyatoslav Ryhel 2026-05-17 7:58 ` sashiko-bot 2026-05-17 13:44 ` Jonathan Cameron 2026-05-17 7:43 ` [PATCH v1 2/6] mfd: lm3533: Convert to use OF bindings Svyatoslav Ryhel ` (5 subsequent siblings) 6 siblings, 2 replies; 24+ messages in thread From: Svyatoslav Ryhel @ 2026-05-17 7:43 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: 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> --- .../leds/backlight/ti,lm3533-backlight.yaml | 68 +++++++ .../bindings/leds/ti,lm3533-leds.yaml | 66 ++++++ .../devicetree/bindings/leds/ti,lm3533.yaml | 190 ++++++++++++++++++ 3 files changed, 324 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..866b0fb8ed04 --- /dev/null +++ b/Documentation/devicetree/bindings/leds/backlight/ti,lm3533-backlight.yaml @@ -0,0 +1,68 @@ +# 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 + + 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..a582d7d2f955 --- /dev/null +++ b/Documentation/devicetree/bindings/leds/ti,lm3533-leds.yaml @@ -0,0 +1,66 @@ +# 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: 5 + + 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 + +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..2e200f172400 --- /dev/null +++ b/Documentation/devicetree/bindings/leds/ti,lm3533.yaml @@ -0,0 +1,190 @@ +# 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 ALS + sensor support. + + 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-ohm: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + Internal configuration resister value when ALS is in Analog Sensor + mode and PWM mode is disabled. + minimum: 1575 + maximum: 200000 + + ti,pwm-mode: + type: boolean + description: + Switch for mode in which ALS is running. If this property is set + then ALS is running in PWM mode, internal resistor value is set to + high-impedance (0) and ti,resistor-ohm property is ignored. + + required: + - compatible + + anyOf: + - required: + - ti,resistor-ohm + - 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 + - light-sensor + - backlight@0 + - backlight@1 + - led@2 + - led@3 + - led@4 + - led@5 + +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>; + }; + + backlight@1 { + compatible = "ti,lm3533-backlight"; + reg = <1>; + + default-brightness = <113>; + + led-max-microamp = <23400>; + led-sources = <1>; + }; + + led@2 { + compatible = "ti,lm3533-leds"; + reg = <2>; + + led-max-microamp = <23400>; + led-sources = <0>; + }; + + led@3 { + compatible = "ti,lm3533-leds"; + reg = <3>; + + led-max-microamp = <23400>; + led-sources = <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.51.0 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v1 1/6] dt-bindings: leds: Document TI LM3533 LED controller 2026-05-17 7:43 ` [PATCH v1 1/6] dt-bindings: leds: Document TI LM3533 LED controller Svyatoslav Ryhel @ 2026-05-17 7:58 ` sashiko-bot 2026-05-17 13:44 ` Jonathan Cameron 1 sibling, 0 replies; 24+ messages in thread From: sashiko-bot @ 2026-05-17 7:58 UTC (permalink / raw) To: Svyatoslav Ryhel; +Cc: robh, conor+dt, devicetree Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider: - [High] The top-level `ti,lm3533.yaml` mandates the presence of all sub-nodes through the `required` list, which is an anti-pattern for optional hardware blocks. - [High] The `ti,lm3533-backlight.yaml` schema uses standard LED properties (`led-sources` and `led-max-microamp`) without defining their types or pulling them in from the core LED schema. - [High] The `light-sensor` node will fail `dt_binding_check` because `additionalProperties: false` rejects standard properties like `interrupt-parent`. - [Medium] The `led-sources` item constraint in `ti,lm3533-leds.yaml` allows an out-of-bounds index due to an off-by-one error. - [Medium] The parent node `ti,lm3533.yaml` enforces child nodes with unit addresses but fails to require `#address-cells` and `#size-cells`. -- commit 79de640d527303f53ad8d32d99e560b245a95df7 Author: Svyatoslav Ryhel <clamor95@gmail.com> dt-bindings: leds: Document TI LM3533 LED controller Document the LM3533 - a complete power source for backlight, keypad and indicator LEDs in smartphone handsets. > 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 0000000000000..866b0fb8ed047 > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/backlight/ti,lm3533-backlight.yaml > @@ -0,0 +1,68 @@ [ ... ] > +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: The common backlight schema does not define types for led-sources or led-max-microamp. Since this schema neither includes leds/common.yaml nor provides explicit type references, will this lack strict type validation and potentially cause dt_binding_check to fail? Should this include a reference to /schemas/leds/common.yaml#? [ ... ] > 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 0000000000000..a582d7d2f955b > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/ti,lm3533-leds.yaml > @@ -0,0 +1,66 @@ [ ... ] > + 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: 5 The description explicitly defines the valid LVLED source indices as 0 through 4. Does the maximum constraint of 5 allow an invalid index of 5 to pass schema validation? Should it be restricted to maximum: 4? [ ... ] > diff --git a/Documentation/devicetree/bindings/leds/ti,lm3533.yaml b/Documentation/devicetree/bindings/leds/ti,lm3533.yaml > new file mode 100644 > index 0000000000000..2e200f1724004 > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/ti,lm3533.yaml > @@ -0,0 +1,190 @@ [ ... ] > + light-sensor: > + type: object > + additionalProperties: false > + > + properties: > + compatible: > + const: ti,lm3533-als > + > + interrupts: > + maxItems: 1 This object uses additionalProperties: false but does not list standard properties like interrupt-parent in its properties block. Will this cause schema validation to fail when processing device trees that use interrupt-parent, as shown in the example later in this binding? [ ... ] > +required: > + - compatible > + - reg > + - light-sensor > + - backlight@0 > + - backlight@1 > + - led@2 > + - led@3 > + - led@4 > + - led@5 Are all of these sub-nodes physically required for every device tree? Including backlight@0, backlight@1, led@2 through led@5, and light-sensor in the required list forces every device tree to instantiate them even if they are unconnected or unused on the board. Since this parent node hosts multiple subnodes that utilize the reg property, shouldn't #address-cells and #size-cells also be included in the required array to ensure they are always present when child nodes are defined? [ ... ] -- Sashiko AI review · https://sashiko.dev/#/patchset/20260517074306.30937-1-clamor95@gmail.com?part=1 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v1 1/6] dt-bindings: leds: Document TI LM3533 LED controller 2026-05-17 7:43 ` [PATCH v1 1/6] dt-bindings: leds: Document TI LM3533 LED controller Svyatoslav Ryhel 2026-05-17 7:58 ` sashiko-bot @ 2026-05-17 13:44 ` Jonathan Cameron 2026-05-17 14:26 ` Svyatoslav Ryhel 1 sibling, 1 reply; 24+ messages in thread From: Jonathan Cameron @ 2026-05-17 13:44 UTC (permalink / raw) To: Svyatoslav Ryhel Cc: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring, Krzysztof Kozlowski, Conor Dooley, David Lechner, Nuno Sá, Andy Shevchenko, Helge Deller, dri-devel, linux-leds, devicetree, linux-kernel, linux-iio, linux-fbdev On Sun, 17 May 2026 10:43:01 +0300 Svyatoslav Ryhel <clamor95@gmail.com> wrote: > Document the LM3533 - a complete power source for backlight, keypad and > indicator LEDs in smartphone handsets. The high-voltage inductive boost > converter provides the power for two series LED strings display backlight > and keypad functions. > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com> Hi Svyatoslav, I focused on the ALS part.. A few comments. > +# 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..2e200f172400 > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/ti,lm3533.yaml > @@ -0,0 +1,190 @@ > +# 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 ALS > + sensor support. Mention it is an interface for an external ALS. > + light-sensor: > + type: object > + additionalProperties: false > + > + properties: > + compatible: > + const: ti,lm3533-als > + > + interrupts: > + maxItems: 1 > + > + ti,resistor-ohm: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: > + Internal configuration resister value when ALS is in Analog Sensor > + mode and PWM mode is disabled. Good to note why this is a firmware thing rather than a userspace controlled thing. I looked it up, it's because expectation is the input is a current from the external analog ALS and these are used to convert it to a voltage with target range. > + minimum: 1575 > + maximum: 200000 > + > + ti,pwm-mode: > + type: boolean > + description: > + Switch for mode in which ALS is running. If this property is set > + then ALS is running in PWM mode, internal resistor value is set to > + high-impedance (0) and ti,resistor-ohm property is ignored. I'd mention the ALS is an external device - so this is saying what interface thing thing connected is using. > + > + required: > + - compatible > + > + anyOf: > + - required: > + - ti,resistor-ohm > + - 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 > + - light-sensor > + - backlight@0 > + - backlight@1 Similar for the led nodes. > + - led@2 > + - led@3 > + - led@4 > + - led@5 Curious - why are all the led nodes required? What if some aren't wired to anything? > + > +additionalProperties: false ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v1 1/6] dt-bindings: leds: Document TI LM3533 LED controller 2026-05-17 13:44 ` Jonathan Cameron @ 2026-05-17 14:26 ` Svyatoslav Ryhel 0 siblings, 0 replies; 24+ messages in thread From: Svyatoslav Ryhel @ 2026-05-17 14:26 UTC (permalink / raw) To: Jonathan Cameron Cc: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring, Krzysztof Kozlowski, Conor Dooley, David Lechner, Nuno Sá, Andy Shevchenko, Helge Deller, dri-devel, linux-leds, devicetree, linux-kernel, linux-iio, linux-fbdev нд, 17 трав. 2026 р. о 16:45 Jonathan Cameron <jic23@kernel.org> пише: > > On Sun, 17 May 2026 10:43:01 +0300 > Svyatoslav Ryhel <clamor95@gmail.com> wrote: > > > Document the LM3533 - a complete power source for backlight, keypad and > > indicator LEDs in smartphone handsets. The high-voltage inductive boost > > converter provides the power for two series LED strings display backlight > > and keypad functions. > > > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com> > Hi Svyatoslav, > > I focused on the ALS part.. A few comments. > > > +# 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..2e200f172400 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/leds/ti,lm3533.yaml > > @@ -0,0 +1,190 @@ > > +# 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 ALS > > + sensor support. > Mention it is an interface for an external ALS. > Noted, thank you! > > + light-sensor: > > + type: object > > + additionalProperties: false > > + > > + properties: > > + compatible: > > + const: ti,lm3533-als > > + > > + interrupts: > > + maxItems: 1 > > + > > + ti,resistor-ohm: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + description: > > + Internal configuration resister value when ALS is in Analog Sensor > > + mode and PWM mode is disabled. > > Good to note why this is a firmware thing rather than a userspace controlled > thing. I looked it up, it's because expectation is the input is a current from > the external analog ALS and these are used to convert it to a voltage with target > range. > I will include your explanation. > > + minimum: 1575 > > + maximum: 200000 > > + > > + ti,pwm-mode: > > + type: boolean > > + description: > > + Switch for mode in which ALS is running. If this property is set > > + then ALS is running in PWM mode, internal resistor value is set to > > + high-impedance (0) and ti,resistor-ohm property is ignored. > I'd mention the ALS is an external device - so this is saying what interface thing > thing connected is using. > Noted > > + > > + required: > > + - compatible > > + > > + anyOf: > > + - required: > > + - ti,resistor-ohm > > + - 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 > > + - light-sensor > > + - backlight@0 > > + - backlight@1 > > Similar for the led nodes. > > > + - led@2 > > + - led@3 > > + - led@4 > > + - led@5 > > Curious - why are all the led nodes required? What if some aren't wired to anything? > This is limitations of mfd devices. If it has subnodes, all subnodes must be present to reflect mfd composition. Unused nodes should be disabled. However, I have asked Lee if he is fine if I will program dynamic mfd composition based on the device tree. If he is fine, "required" props/nodes list will be reduced. > > + > > +additionalProperties: false > ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v1 2/6] mfd: lm3533: Convert to use OF bindings 2026-05-17 7:43 [PATCH v1 0/6] mfd: lm3533: convert to OF bindings, improve support Svyatoslav Ryhel 2026-05-17 7:43 ` [PATCH v1 1/6] dt-bindings: leds: Document TI LM3533 LED controller Svyatoslav Ryhel @ 2026-05-17 7:43 ` Svyatoslav Ryhel 2026-05-17 7:55 ` Andy Shevchenko ` (2 more replies) 2026-05-17 7:43 ` [PATCH v1 3/6] mfd: lm3533: Add support for VIN power supply Svyatoslav Ryhel ` (4 subsequent siblings) 6 siblings, 3 replies; 24+ messages in thread From: Svyatoslav Ryhel @ 2026-05-17 7:43 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: 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. Additionally, optimize functions used only by platform data. Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com> --- drivers/iio/light/lm3533-als.c | 123 +++++-------- drivers/leds/leds-lm3533.c | 60 ++++--- drivers/mfd/lm3533-core.c | 257 +++++++++------------------- drivers/video/backlight/lm3533_bl.c | 52 ++++-- include/linux/mfd/lm3533.h | 51 +----- 5 files changed, 202 insertions(+), 341 deletions(-) diff --git a/drivers/iio/light/lm3533-als.c b/drivers/iio/light/lm3533-als.c index 99f0b903018c..853abb96e13f 100644 --- a/drivers/iio/light/lm3533-als.c +++ b/drivers/iio/light/lm3533-als.c @@ -16,15 +16,18 @@ #include <linux/module.h> #include <linux/mutex.h> #include <linux/mfd/core.h> +#include <linux/mod_devicetable.h> #include <linux/platform_device.h> +#include <linux/property.h> #include <linux/slab.h> #include <linux/uaccess.h> +#include <linux/units.h> #include <linux/mfd/lm3533.h> -#define LM3533_ALS_RESISTOR_MIN 1 -#define LM3533_ALS_RESISTOR_MAX 127 +#define LM3533_ALS_RESISTOR_MIN 1575 +#define LM3533_ALS_RESISTOR_MAX 200000 #define LM3533_ALS_CHANNEL_CURRENT_MAX 2 #define LM3533_ALS_THRESH_MAX 3 #define LM3533_ALS_ZONE_MAX 4 @@ -56,6 +59,9 @@ struct lm3533_als { atomic_t zone; struct mutex thresh_mutex; + + bool pwm_mode; + u32 r_select; }; @@ -714,64 +720,6 @@ 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) -{ - 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); - 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 = lm3533_write(als->lm3533, LM3533_REG_ALS_RESISTOR_SELECT, val); - if (ret) { - dev_err(&als->pdev->dev, "failed to set resistor\n"); - return ret; - } - - return 0; -} - -static int lm3533_als_setup(struct lm3533_als *als, - const struct lm3533_als_platform_data *pdata) -{ - int ret; - - ret = lm3533_als_set_input_mode(als, pdata->pwm_mode); - if (ret) - return ret; - - /* ALS input is always high impedance in PWM-mode. */ - if (!pdata->pwm_mode) { - ret = lm3533_als_set_resistor(als, pdata->r_select); - if (ret) - return ret; - } - - return 0; -} - static int lm3533_als_setup_irq(struct lm3533_als *als, void *dev) { u8 mask = LM3533_ALS_INT_ENABLE_MASK; @@ -784,7 +732,8 @@ static int lm3533_als_setup_irq(struct lm3533_als *als, void *dev) return ret; } - ret = request_threaded_irq(als->irq, NULL, lm3533_als_isr, + ret = devm_request_threaded_irq(&als->pdev->dev, als->irq, NULL, + lm3533_als_isr, IRQF_TRIGGER_LOW | IRQF_ONESHOT, dev_name(&als->pdev->dev), dev); if (ret) { @@ -828,7 +777,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; @@ -838,12 +786,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; @@ -852,31 +794,52 @@ 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->lm3533 = lm3533; als->pdev = pdev; - als->irq = lm3533->irq; + als->irq = platform_get_irq_optional(pdev, 0); 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); + device_property_read_u32(&pdev->dev, "ti,resistor-value-ohm", + &als->r_select); + + als->r_select = clamp(als->r_select, LM3533_ALS_RESISTOR_MIN, + LM3533_ALS_RESISTOR_MAX); + als->r_select = DIV_ROUND_UP(2 * MICRO, 10 * als->r_select); + + als->pwm_mode = device_property_read_bool(&pdev->dev, "ti,pwm-mode"); + + ret = lm3533_update(lm3533, LM3533_REG_ALS_CONF, als->pwm_mode ? + LM3533_ALS_INPUT_MODE_MASK : 0, + LM3533_ALS_INPUT_MODE_MASK); if (ret) - goto err_free_irq; + return dev_err_probe(&pdev->dev, ret, + "failed to set input mode %d\n", + als->pwm_mode); + + /* ALS input is always high impedance in PWM-mode. */ + if (!als->pwm_mode) { + ret = lm3533_write(lm3533, LM3533_REG_ALS_RESISTOR_SELECT, + (u8)als->r_select); + if (ret) + return dev_err_probe(&pdev->dev, ret, + "failed to set resistor\n"); + } ret = lm3533_als_enable(als); if (ret) - goto err_free_irq; + return ret; ret = iio_device_register(indio_dev); if (ret) { @@ -888,9 +851,6 @@ static int lm3533_als_probe(struct platform_device *pdev) err_disable: lm3533_als_disable(als); -err_free_irq: - if (als->irq) - free_irq(als->irq, indio_dev); return ret; } @@ -903,13 +863,18 @@ 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) - 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 45795f2a1042..f6345bc1f443 100644 --- a/drivers/leds/leds-lm3533.c +++ b/drivers/leds/leds-lm3533.c @@ -10,8 +10,10 @@ #include <linux/module.h> #include <linux/leds.h> #include <linux/mfd/core.h> +#include <linux/mod_devicetable.h> #include <linux/mutex.h> #include <linux/platform_device.h> +#include <linux/property.h> #include <linux/slab.h> #include <linux/mfd/lm3533.h> @@ -48,6 +50,9 @@ struct lm3533_led { struct mutex mutex; unsigned long flags; + + u32 max_current; + u32 pwm; }; @@ -632,22 +637,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; @@ -657,30 +660,30 @@ 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; - } + if (pdev->id < LM3533_LVCTRLBANK_MIN || pdev->id > LM3533_LVCTRLBANK_MAX) + return dev_err_probe(&pdev->dev, -EINVAL, + "illegal LED id %d\n", pdev->id); led = devm_kzalloc(&pdev->dev, sizeof(*led), GFP_KERNEL); if (!led) return -ENOMEM; led->lm3533 = lm3533; - 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; led->cdev.brightness = LED_OFF; led->cdev.groups = lm3533_led_attribute_groups; - led->id = pdev->id; + led->id = pdev->id - LM3533_LVCTRLBANK_MIN; + + 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); @@ -694,15 +697,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_LED_MAX_CURRENT_MIN, + LM3533_LED_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; @@ -739,9 +750,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 0a2409d00b2e..83ebd780f39d 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 16000000 +#define LM3533_BOOST_OVP_MAX 40000000 #define LM3533_BOOST_FREQ_MASK 0x01 #define LM3533_BOOST_FREQ_SHIFT 0 +#define LM3533_BOOST_FREQ_MIN 500000 +#define LM3533_BOOST_FREQ_MAX 1000000 #define LM3533_BL_ID_MASK 1 #define LM3533_LED_ID_MASK 3 @@ -42,42 +49,14 @@ #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, - }, +static struct mfd_cell lm3533_child_devices[] = { + MFD_CELL_OF("lm3533-als", NULL, NULL, 0, 0, "ti,lm3533-als"), + MFD_CELL_OF_REG("lm3533-backlight", NULL, NULL, 0, 0, "ti,lm3533-backlight", 0), + MFD_CELL_OF_REG("lm3533-backlight", NULL, NULL, 0, 1, "ti,lm3533-backlight", 1), + MFD_CELL_OF_REG("lm3533-leds", NULL, NULL, 0, 0, "ti,lm3533-leds", 2), + MFD_CELL_OF_REG("lm3533-leds", NULL, NULL, 0, 1, "ti,lm3533-leds", 3), + MFD_CELL_OF_REG("lm3533-leds", NULL, NULL, 0, 2, "ti,lm3533-leds", 4), + MFD_CELL_OF_REG("lm3533-leds", NULL, NULL, 0, 3, "ti,lm3533-leds", 5), }; int lm3533_read(struct lm3533 *lm3533, u8 reg, u8 *val) @@ -132,35 +111,6 @@ int lm3533_update(struct lm3533 *lm3533, u8 reg, u8 val, u8 mask) } 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); - 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 = lm3533_update(lm3533, LM3533_REG_BOOST_PWM, - ovp << LM3533_BOOST_OVP_SHIFT, - LM3533_BOOST_OVP_MASK); - if (ret) - dev_err(lm3533->dev, "failed to set boost ovp\n"); - - return ret; -} - /* * HVLED output config -- output hvled controlled by backlight bl */ @@ -376,136 +326,45 @@ static struct attribute_group lm3533_attribute_group = { .attrs = lm3533_attributes }; -static int lm3533_device_als_init(struct lm3533 *lm3533) +static int lm3533_device_init(struct lm3533 *lm3533) { - struct lm3533_platform_data *pdata = dev_get_platdata(lm3533->dev); + struct device *dev = 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); + lm3533_enable(lm3533); - ret = mfd_add_devices(lm3533->dev, 0, lm3533_als_devs, 1, NULL, - 0, NULL); + ret = lm3533_update(lm3533, LM3533_REG_BOOST_PWM, + lm3533->boost_freq << LM3533_BOOST_FREQ_SHIFT, + LM3533_BOOST_FREQ_MASK); 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]); + dev_err(dev, "failed to set boost frequency\n"); + goto err_disable; } - ret = mfd_add_devices(lm3533->dev, 0, lm3533_bl_devs, - pdata->num_backlights, NULL, 0, NULL); + ret = lm3533_update(lm3533, LM3533_REG_BOOST_PWM, + lm3533->boost_ovp << LM3533_BOOST_OVP_SHIFT, + LM3533_BOOST_OVP_MASK); if (ret) { - dev_err(lm3533->dev, "failed to add backlight devices\n"); - return ret; - } - - lm3533->have_backlights = 1; - - return 0; -} - -static int lm3533_device_led_init(struct lm3533 *lm3533) -{ - struct lm3533_platform_data *pdata = dev_get_platdata(lm3533->dev); - int i; - int ret; - - if (!pdata->leds || pdata->num_leds == 0) - return 0; - - if (pdata->num_leds > ARRAY_SIZE(lm3533_led_devs)) - pdata->num_leds = ARRAY_SIZE(lm3533_led_devs); - - for (i = 0; i < pdata->num_leds; ++i) { - lm3533_led_devs[i].platform_data = &pdata->leds[i]; - lm3533_led_devs[i].pdata_size = sizeof(pdata->leds[i]); + dev_err(dev, "failed to set boost ovp\n"); + goto err_disable; } - ret = mfd_add_devices(lm3533->dev, 0, lm3533_led_devs, - pdata->num_leds, NULL, 0, NULL); + ret = devm_mfd_add_devices(dev, 0, lm3533_child_devices, + ARRAY_SIZE(lm3533_child_devices), + NULL, 0, NULL); if (ret) { - dev_err(lm3533->dev, "failed to add LED devices\n"); - return ret; - } - - lm3533->have_leds = 1; - - return 0; -} - -static int lm3533_device_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); - int ret; - - dev_dbg(lm3533->dev, "%s\n", __func__); - - if (!pdata) { - dev_err(lm3533->dev, "no platform data\n"); - return -EINVAL; + dev_err(dev, "failed to add MFD devices: %d\n", ret); + goto err_disable; } - 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 = lm3533_device_setup(lm3533, pdata); - if (ret) + ret = sysfs_create_group(&dev->kobj, &lm3533_attribute_group); + if (ret) { + dev_err(dev, "failed to create sysfs attributes\n"); goto err_disable; - - lm3533_device_als_init(lm3533); - lm3533_device_bl_init(lm3533); - lm3533_device_led_init(lm3533); - - ret = sysfs_create_group(&lm3533->dev->kobj, &lm3533_attribute_group); - if (ret < 0) { - dev_err(lm3533->dev, "failed to create sysfs attributes\n"); - goto err_unregister; } return 0; -err_unregister: - mfd_remove_devices(lm3533->dev); err_disable: lm3533_disable(lm3533); @@ -517,8 +376,6 @@ static void lm3533_device_exit(struct lm3533 *lm3533) dev_dbg(lm3533->dev, "%s\n", __func__); sysfs_remove_group(&lm3533->dev->kobj, &lm3533_attribute_group); - - mfd_remove_devices(lm3533->dev); lm3533_disable(lm3533); } @@ -589,7 +446,40 @@ 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; + + device_for_each_child_node_scoped(lm3533->dev, child) { + if (!fwnode_device_is_available(child)) + continue; + + if (fwnode_device_is_compatible(child, "ti,lm3533-als")) + lm3533->have_als = true; + + if (fwnode_device_is_compatible(child, "ti,lm3533-backlight")) + lm3533->have_backlights = true; + + if (fwnode_device_is_compatible(child, "ti,lm3533-leds")) + lm3533->have_leds = true; + } return lm3533_device_init(lm3533); } @@ -603,6 +493,12 @@ static void lm3533_i2c_remove(struct i2c_client *i2c) lm3533_device_exit(lm3533); } +static const struct of_device_id lm3533_match_table[] = { + { .compatible = "ti,lm3533" }, + { } +}; +MODULE_DEVICE_TABLE(of, lm3533_match_table); + static const struct i2c_device_id lm3533_i2c_ids[] = { { "lm3533" }, { } @@ -612,6 +508,7 @@ MODULE_DEVICE_TABLE(i2c, lm3533_i2c_ids); static struct i2c_driver lm3533_i2c_driver = { .driver = { .name = "lm3533", + .of_match_table = lm3533_match_table, }, .id_table = lm3533_i2c_ids, .probe = lm3533_i2c_probe, diff --git a/drivers/video/backlight/lm3533_bl.c b/drivers/video/backlight/lm3533_bl.c index babfd3ceec86..42da652df58d 100644 --- a/drivers/video/backlight/lm3533_bl.c +++ b/drivers/video/backlight/lm3533_bl.c @@ -9,7 +9,9 @@ #include <linux/module.h> #include <linux/init.h> +#include <linux/mod_devicetable.h> #include <linux/platform_device.h> +#include <linux/property.h> #include <linux/backlight.h> #include <linux/slab.h> @@ -27,6 +29,9 @@ struct lm3533_bl { struct lm3533_ctrlbank cb; struct backlight_device *bd; int id; + + u32 max_current; + u32 pwm; }; @@ -246,25 +251,24 @@ 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; int ret; dev_dbg(&pdev->dev, "%s\n", __func__); @@ -273,12 +277,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; @@ -295,13 +293,20 @@ 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; + memset(&props, 0, sizeof(props)); props.type = BACKLIGHT_RAW; props.max_brightness = LM3533_BL_MAX_BRIGHTNESS; - props.brightness = pdata->default_brightness; - bd = devm_backlight_device_register(&pdev->dev, pdata->name, - pdev->dev.parent, bl, &lm3533_bl_ops, - &props); + props.brightness = LM3533_BL_MAX_BRIGHTNESS; + device_property_read_u32(&pdev->dev, "default-brightness", + &props.brightness); + + bd = devm_backlight_device_register(&pdev->dev, 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); @@ -320,7 +325,15 @@ static int lm3533_bl_probe(struct platform_device *pdev) backlight_update_status(bd); - ret = lm3533_bl_setup(bl, pdata); + device_property_read_u32(&pdev->dev, "led-max-microamp", + &bl->max_current); + bl->max_current = clamp(bl->max_current, LM3533_LED_MAX_CURRENT_MIN, + LM3533_LED_MAX_CURRENT_MAX); + + bl->pwm = 0; + device_property_read_u32(&pdev->dev, "ti,pwm-config-mask", &bl->pwm); + + ret = lm3533_bl_setup(bl); if (ret) goto err_sysfs_remove; @@ -381,10 +394,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 69059a7a2ce5..3aa962d4c747 100644 --- a/include/linux/mfd/lm3533.h +++ b/include/linux/mfd/lm3533.h @@ -15,6 +15,9 @@ #define LM3533_ATTR_RW(_name) \ DEVICE_ATTR(_name, S_IRUGO | S_IWUSR , show_##_name, store_##_name) +#define LM3533_LED_MAX_CURRENT_MIN 5000 +#define LM3533_LED_MAX_CURRENT_MAX 29800 + struct device; struct gpio_desc; struct regmap; @@ -25,7 +28,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 +43,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; -}; - extern int lm3533_ctrlbank_enable(struct lm3533_ctrlbank *cb); extern int lm3533_ctrlbank_disable(struct lm3533_ctrlbank *cb); -- 2.51.0 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v1 2/6] mfd: lm3533: Convert to use OF bindings 2026-05-17 7:43 ` [PATCH v1 2/6] mfd: lm3533: Convert to use OF bindings Svyatoslav Ryhel @ 2026-05-17 7:55 ` Andy Shevchenko 2026-05-17 10:11 ` Svyatoslav Ryhel 2026-05-17 8:32 ` sashiko-bot 2026-05-17 11:10 ` Svyatoslav Ryhel 2 siblings, 1 reply; 24+ messages in thread From: Andy Shevchenko @ 2026-05-17 7:55 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, dri-devel, linux-leds, devicetree, linux-kernel, linux-iio, linux-fbdev On Sun, May 17, 2026 at 10:43:02AM +0300, Svyatoslav Ryhel wrote: > Since there are no users of this driver via platform data, remove the > platform data support and switch to using Device Tree bindings. > Additionally, optimize functions used only by platform data. This is a mixture of at least (!) three patches: - devm conversion - the change of some constants - the rest -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v1 2/6] mfd: lm3533: Convert to use OF bindings 2026-05-17 7:55 ` Andy Shevchenko @ 2026-05-17 10:11 ` Svyatoslav Ryhel 0 siblings, 0 replies; 24+ messages in thread From: Svyatoslav Ryhel @ 2026-05-17 10:11 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, dri-devel, linux-leds, devicetree, linux-kernel, linux-iio, linux-fbdev нд, 17 трав. 2026 р. о 10:55 Andy Shevchenko <andriy.shevchenko@intel.com> пише: > > On Sun, May 17, 2026 at 10:43:02AM +0300, Svyatoslav Ryhel wrote: > > Since there are no users of this driver via platform data, remove the > > platform data support and switch to using Device Tree bindings. > > Additionally, optimize functions used only by platform data. > > This is a mixture of at least (!) three patches: Maybe you can make a list of how to split it then. I have tried to make it complete and remain readable. I am open to your suggestions. > - devm conversion This will be dropped. > - the change of some constants If you mean LM3533_ALS_RESISTOR_MIN and LM3533_ALS_RESISTOR_MAX they are used explicitly in r_select value. Previously it was passed via pdata as register value and now it is obtained from the device tree as resistance in Ohms and register value is actually calculated. Naturally ALS_RESISTOR boundaries changed. > - the rest > > -- > With Best Regards, > Andy Shevchenko > > ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v1 2/6] mfd: lm3533: Convert to use OF bindings 2026-05-17 7:43 ` [PATCH v1 2/6] mfd: lm3533: Convert to use OF bindings Svyatoslav Ryhel 2026-05-17 7:55 ` Andy Shevchenko @ 2026-05-17 8:32 ` sashiko-bot 2026-05-17 11:10 ` Svyatoslav Ryhel 2 siblings, 0 replies; 24+ messages in thread From: sashiko-bot @ 2026-05-17 8:32 UTC (permalink / raw) To: Svyatoslav Ryhel; +Cc: conor+dt, robh, devicetree Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider: - [Critical] Pre-existing Use-After-Free race in ALS IRQ teardown. - [High] Unconditional MFD cell registration creates phantom devices. - [High] Devres cleanup inversion causes I2C accesses on a powered-down device. - [High] LED child devices fail to probe unconditionally due to incorrect `pdev->id` boundary checks. - [High] Silent loss of interrupts due to bypassing `-EPROBE_DEFER` on optional IRQs. - [Medium] Implicit integer truncation of `ti,pwm-config-mask` circumvents bounds validation. -- commit e97028edbaad36911b58da2763bddbf8e9cb91e9 Author: Svyatoslav Ryhel <clamor95@gmail.com> 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. Additionally, optimize functions used only by platform data. > diff --git a/drivers/iio/light/lm3533-als.c b/drivers/iio/light/lm3533-als.c > --- a/drivers/iio/light/lm3533-als.c > +++ b/drivers/iio/light/lm3533-als.c [ ... ] > @@ -838,12 +786,6 @@ static int lm3533_als_probe(struct platform_device *pdev) [ ... ] > als = iio_priv(indio_dev); > als->lm3533 = lm3533; > als->pdev = pdev; > - als->irq = lm3533->irq; > + als->irq = platform_get_irq_optional(pdev, 0); > atomic_set(&als->zone, 0); > mutex_init(&als->thresh_mutex); > > platform_set_drvdata(pdev, indio_dev); > > - if (als->irq) { > + if (als->irq > 0) { Could this silently lose interrupts if the controller is not yet ready? If the underlying interrupt controller is not ready, platform_get_irq_optional() returns -EPROBE_DEFER. Checking for als->irq > 0 skips the IRQ setup for all negative error codes, ignoring the deferral and continuing without interrupts. [ ... ] > @@ -903,13 +863,18 @@ static void lm3533_als_remove(struct platform_device *pdev) > { > struct iio_dev *indio_dev = platform_get_drvdata(pdev); > struct lm3533_als *als = iio_priv(indio_dev); > > lm3533_als_set_int_mode(indio_dev, false); > iio_device_unregister(indio_dev); > lm3533_als_disable(als); > - if (als->irq) > - free_irq(als->irq, indio_dev); > } This may be a pre-existing issue, but does this create a use-after-free race during teardown? By calling iio_device_unregister() before the threaded IRQ is freed via devres, the IIO event interface is destroyed and ev_int is freed. If the threaded IRQ handler lm3533_als_isr() executes concurrently or is already scheduled, could it call iio_push_event(), which reads the stale ev_int pointer? > diff --git a/drivers/leds/leds-lm3533.c b/drivers/leds/leds-lm3533.c > --- a/drivers/leds/leds-lm3533.c > +++ b/drivers/leds/leds-lm3533.c [ ... ] > @@ -657,30 +660,30 @@ 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; > - } > + if (pdev->id < LM3533_LVCTRLBANK_MIN || pdev->id > LM3533_LVCTRLBANK_MAX) > + return dev_err_probe(&pdev->dev, -EINVAL, > + "illegal LED id %d\n", pdev->id); Will this boundary check cause the LED devices to fail probing? In lm3533-core.c, the MFD cells for ti,lm3533-leds are defined with ids 0, 1, 2, and 3. The MFD core assigns these directly to pdev->id. Since LM3533_LVCTRLBANK_MIN is 2, pdev->id 0 and 1 will fail this check and return -EINVAL. For pdev->id 2 and 3, they pass the check but are then assigned led->id = pdev->id - LM3533_LVCTRLBANK_MIN, which maps them incorrectly to internal control banks 0 and 1 instead of 2 and 3. [ ... ] > @@ -694,15 +697,23 @@ static int lm3533_led_probe(struct platform_device *pdev) [ ... ] > + led->pwm = 0; > + device_property_read_u32(&pdev->dev, "ti,pwm-config-mask", &led->pwm); > + > + ret = lm3533_led_setup(led); Could this result in an implicit integer truncation that circumvents bounds validation? The property is read into the 32-bit integer led->pwm (and similarly bl->pwm in lm3533_bl.c) and then passed to lm3533_ctrlbank_set_pwm() which expects a u8 value. If an invalid, out-of-bounds value is present in the Device Tree (like 256), it would be silently truncated to 0, completely bypassing the safety check if (val > LM3533_PWM_MAX) inside lm3533_ctrlbank_set_pwm(). > diff --git a/drivers/mfd/lm3533-core.c b/drivers/mfd/lm3533-core.c > --- a/drivers/mfd/lm3533-core.c > +++ b/drivers/mfd/lm3533-core.c [ ... ] > @@ -376,136 +326,45 @@ static struct attribute_group lm3533_attribute_group = { [ ... ] > + ret = devm_mfd_add_devices(dev, 0, lm3533_child_devices, > + ARRAY_SIZE(lm3533_child_devices), > + NULL, 0, NULL); Does this unconditionally register phantom devices? By using devm_mfd_add_devices() for all 7 child cells, the MFD core instantiates platform devices regardless of whether corresponding nodes exist in the Device Tree. Since the child drivers do not verify fwnode_device_is_available() during probe, could this expose nonexistent hardware to user-space and result in sending I2C commands to unconnected pins? [ ... ] > @@ -517,8 +376,6 @@ static void lm3533_device_exit(struct lm3533 *lm3533) > dev_dbg(lm3533->dev, "%s\n", __func__); > > sysfs_remove_group(&lm3533->dev->kobj, &lm3533_attribute_group); > - > - mfd_remove_devices(lm3533->dev); > lm3533_disable(lm3533); > } Could this lead to I2C accesses on a powered-down device? With the conversion to devm_mfd_add_devices(), the child devices are now unregistered by devres after lm3533_device_exit() returns. However, lm3533_device_exit() still calls lm3533_disable() to physically power down the chip. When devres subsequently unbinds the child devices, their remove callbacks will attempt to shut down their components over I2C on the disabled chip, which could result in timeouts or transfer errors. -- Sashiko AI review · https://sashiko.dev/#/patchset/20260517074306.30937-1-clamor95@gmail.com?part=2 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v1 2/6] mfd: lm3533: Convert to use OF bindings 2026-05-17 7:43 ` [PATCH v1 2/6] mfd: lm3533: Convert to use OF bindings Svyatoslav Ryhel 2026-05-17 7:55 ` Andy Shevchenko 2026-05-17 8:32 ` sashiko-bot @ 2026-05-17 11:10 ` Svyatoslav Ryhel 2 siblings, 0 replies; 24+ messages in thread From: Svyatoslav Ryhel @ 2026-05-17 11:10 UTC (permalink / raw) To: Lee Jones Cc: Daniel Thompson, Jingoo Han, Rob Herring, Pavel Machek, Krzysztof Kozlowski, Conor Dooley, David Lechner, Jonathan Cameron, Nuno Sá, Helge Deller, dri-devel, linux-leds, devicetree, linux-kernel, linux-iio, linux-fbdev, Svyatoslav Ryhel, Andy Shevchenko нд, 17 трав. 2026 р. о 10:43 Svyatoslav Ryhel <clamor95@gmail.com> пише: > > Since there are no users of this driver via platform data, remove the > platform data support and switch to using Device Tree bindings. > Additionally, optimize functions used only by platform data. > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com> > --- > drivers/iio/light/lm3533-als.c | 123 +++++-------- > drivers/leds/leds-lm3533.c | 60 ++++--- > drivers/mfd/lm3533-core.c | 257 +++++++++------------------- > drivers/video/backlight/lm3533_bl.c | 52 ++++-- > include/linux/mfd/lm3533.h | 51 +----- > 5 files changed, 202 insertions(+), 341 deletions(-) > > diff --git a/drivers/iio/light/lm3533-als.c b/drivers/iio/light/lm3533-als.c > index 99f0b903018c..853abb96e13f 100644 > --- a/drivers/iio/light/lm3533-als.c > +++ b/drivers/iio/light/lm3533-als.c > @@ -16,15 +16,18 @@ > #include <linux/module.h> > #include <linux/mutex.h> > #include <linux/mfd/core.h> > +#include <linux/mod_devicetable.h> > #include <linux/platform_device.h> > +#include <linux/property.h> > #include <linux/slab.h> > #include <linux/uaccess.h> > +#include <linux/units.h> > > #include <linux/mfd/lm3533.h> > > > -#define LM3533_ALS_RESISTOR_MIN 1 > -#define LM3533_ALS_RESISTOR_MAX 127 > +#define LM3533_ALS_RESISTOR_MIN 1575 > +#define LM3533_ALS_RESISTOR_MAX 200000 > #define LM3533_ALS_CHANNEL_CURRENT_MAX 2 > #define LM3533_ALS_THRESH_MAX 3 > #define LM3533_ALS_ZONE_MAX 4 > @@ -56,6 +59,9 @@ struct lm3533_als { > > atomic_t zone; > struct mutex thresh_mutex; > + > + bool pwm_mode; > + u32 r_select; > }; > > > @@ -714,64 +720,6 @@ 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) > -{ > - 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); > - 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 = lm3533_write(als->lm3533, LM3533_REG_ALS_RESISTOR_SELECT, val); > - if (ret) { > - dev_err(&als->pdev->dev, "failed to set resistor\n"); > - return ret; > - } > - > - return 0; > -} > - > -static int lm3533_als_setup(struct lm3533_als *als, > - const struct lm3533_als_platform_data *pdata) > -{ > - int ret; > - > - ret = lm3533_als_set_input_mode(als, pdata->pwm_mode); > - if (ret) > - return ret; > - > - /* ALS input is always high impedance in PWM-mode. */ > - if (!pdata->pwm_mode) { > - ret = lm3533_als_set_resistor(als, pdata->r_select); > - if (ret) > - return ret; > - } > - > - return 0; > -} > - > static int lm3533_als_setup_irq(struct lm3533_als *als, void *dev) > { > u8 mask = LM3533_ALS_INT_ENABLE_MASK; > @@ -784,7 +732,8 @@ static int lm3533_als_setup_irq(struct lm3533_als *als, void *dev) > return ret; > } > > - ret = request_threaded_irq(als->irq, NULL, lm3533_als_isr, > + ret = devm_request_threaded_irq(&als->pdev->dev, als->irq, NULL, > + lm3533_als_isr, > IRQF_TRIGGER_LOW | IRQF_ONESHOT, > dev_name(&als->pdev->dev), dev); > if (ret) { > @@ -828,7 +777,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; > @@ -838,12 +786,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; > @@ -852,31 +794,52 @@ 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->lm3533 = lm3533; > als->pdev = pdev; > - als->irq = lm3533->irq; > + als->irq = platform_get_irq_optional(pdev, 0); > 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); > + device_property_read_u32(&pdev->dev, "ti,resistor-value-ohm", > + &als->r_select); > + > + als->r_select = clamp(als->r_select, LM3533_ALS_RESISTOR_MIN, > + LM3533_ALS_RESISTOR_MAX); > + als->r_select = DIV_ROUND_UP(2 * MICRO, 10 * als->r_select); > + > + als->pwm_mode = device_property_read_bool(&pdev->dev, "ti,pwm-mode"); > + > + ret = lm3533_update(lm3533, LM3533_REG_ALS_CONF, als->pwm_mode ? > + LM3533_ALS_INPUT_MODE_MASK : 0, > + LM3533_ALS_INPUT_MODE_MASK); > if (ret) > - goto err_free_irq; > + return dev_err_probe(&pdev->dev, ret, > + "failed to set input mode %d\n", > + als->pwm_mode); > + > + /* ALS input is always high impedance in PWM-mode. */ > + if (!als->pwm_mode) { > + ret = lm3533_write(lm3533, LM3533_REG_ALS_RESISTOR_SELECT, > + (u8)als->r_select); > + if (ret) > + return dev_err_probe(&pdev->dev, ret, > + "failed to set resistor\n"); > + } > > ret = lm3533_als_enable(als); > if (ret) > - goto err_free_irq; > + return ret; > > ret = iio_device_register(indio_dev); > if (ret) { > @@ -888,9 +851,6 @@ static int lm3533_als_probe(struct platform_device *pdev) > > err_disable: > lm3533_als_disable(als); > -err_free_irq: > - if (als->irq) > - free_irq(als->irq, indio_dev); > > return ret; > } > @@ -903,13 +863,18 @@ 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) > - 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 45795f2a1042..f6345bc1f443 100644 > --- a/drivers/leds/leds-lm3533.c > +++ b/drivers/leds/leds-lm3533.c > @@ -10,8 +10,10 @@ > #include <linux/module.h> > #include <linux/leds.h> > #include <linux/mfd/core.h> > +#include <linux/mod_devicetable.h> > #include <linux/mutex.h> > #include <linux/platform_device.h> > +#include <linux/property.h> > #include <linux/slab.h> > > #include <linux/mfd/lm3533.h> > @@ -48,6 +50,9 @@ struct lm3533_led { > > struct mutex mutex; > unsigned long flags; > + > + u32 max_current; > + u32 pwm; > }; > > > @@ -632,22 +637,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; > > @@ -657,30 +660,30 @@ 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; > - } > + if (pdev->id < LM3533_LVCTRLBANK_MIN || pdev->id > LM3533_LVCTRLBANK_MAX) > + return dev_err_probe(&pdev->dev, -EINVAL, > + "illegal LED id %d\n", pdev->id); > > led = devm_kzalloc(&pdev->dev, sizeof(*led), GFP_KERNEL); > if (!led) > return -ENOMEM; > > led->lm3533 = lm3533; > - 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; > led->cdev.brightness = LED_OFF; > led->cdev.groups = lm3533_led_attribute_groups; > - led->id = pdev->id; > + led->id = pdev->id - LM3533_LVCTRLBANK_MIN; > + > + 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); > > @@ -694,15 +697,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_LED_MAX_CURRENT_MIN, > + LM3533_LED_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; > > @@ -739,9 +750,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 0a2409d00b2e..83ebd780f39d 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 16000000 > +#define LM3533_BOOST_OVP_MAX 40000000 > > #define LM3533_BOOST_FREQ_MASK 0x01 > #define LM3533_BOOST_FREQ_SHIFT 0 > +#define LM3533_BOOST_FREQ_MIN 500000 > +#define LM3533_BOOST_FREQ_MAX 1000000 > > #define LM3533_BL_ID_MASK 1 > #define LM3533_LED_ID_MASK 3 > @@ -42,42 +49,14 @@ > > #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, > - }, > +static struct mfd_cell lm3533_child_devices[] = { > + MFD_CELL_OF("lm3533-als", NULL, NULL, 0, 0, "ti,lm3533-als"), > + MFD_CELL_OF_REG("lm3533-backlight", NULL, NULL, 0, 0, "ti,lm3533-backlight", 0), > + MFD_CELL_OF_REG("lm3533-backlight", NULL, NULL, 0, 1, "ti,lm3533-backlight", 1), > + MFD_CELL_OF_REG("lm3533-leds", NULL, NULL, 0, 0, "ti,lm3533-leds", 2), > + MFD_CELL_OF_REG("lm3533-leds", NULL, NULL, 0, 1, "ti,lm3533-leds", 3), > + MFD_CELL_OF_REG("lm3533-leds", NULL, NULL, 0, 2, "ti,lm3533-leds", 4), > + MFD_CELL_OF_REG("lm3533-leds", NULL, NULL, 0, 3, "ti,lm3533-leds", 5), > }; > > int lm3533_read(struct lm3533 *lm3533, u8 reg, u8 *val) > @@ -132,35 +111,6 @@ int lm3533_update(struct lm3533 *lm3533, u8 reg, u8 val, u8 mask) > } > 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); > - 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 = lm3533_update(lm3533, LM3533_REG_BOOST_PWM, > - ovp << LM3533_BOOST_OVP_SHIFT, > - LM3533_BOOST_OVP_MASK); > - if (ret) > - dev_err(lm3533->dev, "failed to set boost ovp\n"); > - > - return ret; > -} > - > /* > * HVLED output config -- output hvled controlled by backlight bl > */ > @@ -376,136 +326,45 @@ static struct attribute_group lm3533_attribute_group = { > .attrs = lm3533_attributes > }; > > -static int lm3533_device_als_init(struct lm3533 *lm3533) > +static int lm3533_device_init(struct lm3533 *lm3533) > { > - struct lm3533_platform_data *pdata = dev_get_platdata(lm3533->dev); > + struct device *dev = 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); > + lm3533_enable(lm3533); > > - ret = mfd_add_devices(lm3533->dev, 0, lm3533_als_devs, 1, NULL, > - 0, NULL); > + ret = lm3533_update(lm3533, LM3533_REG_BOOST_PWM, > + lm3533->boost_freq << LM3533_BOOST_FREQ_SHIFT, > + LM3533_BOOST_FREQ_MASK); > 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]); > + dev_err(dev, "failed to set boost frequency\n"); > + goto err_disable; > } > > - ret = mfd_add_devices(lm3533->dev, 0, lm3533_bl_devs, > - pdata->num_backlights, NULL, 0, NULL); > + ret = lm3533_update(lm3533, LM3533_REG_BOOST_PWM, > + lm3533->boost_ovp << LM3533_BOOST_OVP_SHIFT, > + LM3533_BOOST_OVP_MASK); > if (ret) { > - dev_err(lm3533->dev, "failed to add backlight devices\n"); > - return ret; > - } > - > - lm3533->have_backlights = 1; > - > - return 0; > -} > - > -static int lm3533_device_led_init(struct lm3533 *lm3533) > -{ > - struct lm3533_platform_data *pdata = dev_get_platdata(lm3533->dev); > - int i; > - int ret; > - > - if (!pdata->leds || pdata->num_leds == 0) > - return 0; > - > - if (pdata->num_leds > ARRAY_SIZE(lm3533_led_devs)) > - pdata->num_leds = ARRAY_SIZE(lm3533_led_devs); > - > - for (i = 0; i < pdata->num_leds; ++i) { > - lm3533_led_devs[i].platform_data = &pdata->leds[i]; > - lm3533_led_devs[i].pdata_size = sizeof(pdata->leds[i]); > + dev_err(dev, "failed to set boost ovp\n"); > + goto err_disable; > } > > - ret = mfd_add_devices(lm3533->dev, 0, lm3533_led_devs, > - pdata->num_leds, NULL, 0, NULL); > + ret = devm_mfd_add_devices(dev, 0, lm3533_child_devices, > + ARRAY_SIZE(lm3533_child_devices), > + NULL, 0, NULL); Question to Lee Jones. Would you find acceptable if the driver will build cell list dynamically based on the nodes in the device tree? This is LED controller after all, not all leds can be populated and same LED control bank can be linked to all LVLEDs for example. If you are ok, would this implementation satisfy you? struct mfd_cell lm3533_cells[LM3533_CELLS_MAX]; u32 count = 0, reg; int ret; device_for_each_child_node_scoped(lm3533->dev, child) { if (!fwnode_device_is_available(child)) continue; if (count >= LM3533_CELLS_MAX) break; if (fwnode_device_is_compatible(child, "ti,lm3533-als")) { lm3533_cells[count].name = "lm3533-als"; lm3533_cells[count].id = PLATFORM_DEVID_NONE; lm3533_cells[count].of_compatible = "ti,lm3533-als"; lm3533->have_als = true; } 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 reg %d\n", reg); continue; } lm3533_cells[count].name = "lm3533-backlight"; lm3533_cells[count].id = reg; lm3533_cells[count].of_compatible = "ti,lm3533-backlight"; lm3533->have_backlights = true; } 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 reg %d\n", reg); continue; } lm3533_cells[count].name = "lm3533-leds"; lm3533_cells[count].id = reg - LM3533_HVLED_ID_MAX; lm3533_cells[count].of_compatible = "ti,lm3533-leds"; lm3533->have_leds = true; } count++; } > if (ret) { > - dev_err(lm3533->dev, "failed to add LED devices\n"); > - return ret; > - } > - > - lm3533->have_leds = 1; > - > - 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); > - int ret; > - > - dev_dbg(lm3533->dev, "%s\n", __func__); > - > - if (!pdata) { > - dev_err(lm3533->dev, "no platform data\n"); > - return -EINVAL; > + dev_err(dev, "failed to add MFD devices: %d\n", ret); > + goto err_disable; > } > > - 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 = lm3533_device_setup(lm3533, pdata); > - if (ret) > + ret = sysfs_create_group(&dev->kobj, &lm3533_attribute_group); > + if (ret) { > + dev_err(dev, "failed to create sysfs attributes\n"); > goto err_disable; > - > - lm3533_device_als_init(lm3533); > - lm3533_device_bl_init(lm3533); > - lm3533_device_led_init(lm3533); > - > - ret = sysfs_create_group(&lm3533->dev->kobj, &lm3533_attribute_group); > - if (ret < 0) { > - dev_err(lm3533->dev, "failed to create sysfs attributes\n"); > - goto err_unregister; > } > > return 0; > > -err_unregister: > - mfd_remove_devices(lm3533->dev); > err_disable: > lm3533_disable(lm3533); > > @@ -517,8 +376,6 @@ static void lm3533_device_exit(struct lm3533 *lm3533) > dev_dbg(lm3533->dev, "%s\n", __func__); > > sysfs_remove_group(&lm3533->dev->kobj, &lm3533_attribute_group); > - > - mfd_remove_devices(lm3533->dev); > lm3533_disable(lm3533); > } > > @@ -589,7 +446,40 @@ 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; > + > + device_for_each_child_node_scoped(lm3533->dev, child) { > + if (!fwnode_device_is_available(child)) > + continue; > + > + if (fwnode_device_is_compatible(child, "ti,lm3533-als")) > + lm3533->have_als = true; > + > + if (fwnode_device_is_compatible(child, "ti,lm3533-backlight")) > + lm3533->have_backlights = true; > + > + if (fwnode_device_is_compatible(child, "ti,lm3533-leds")) > + lm3533->have_leds = true; > + } > > return lm3533_device_init(lm3533); > } > @@ -603,6 +493,12 @@ static void lm3533_i2c_remove(struct i2c_client *i2c) > lm3533_device_exit(lm3533); > } > > +static const struct of_device_id lm3533_match_table[] = { > + { .compatible = "ti,lm3533" }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, lm3533_match_table); > + > static const struct i2c_device_id lm3533_i2c_ids[] = { > { "lm3533" }, > { } > @@ -612,6 +508,7 @@ MODULE_DEVICE_TABLE(i2c, lm3533_i2c_ids); > static struct i2c_driver lm3533_i2c_driver = { > .driver = { > .name = "lm3533", > + .of_match_table = lm3533_match_table, > }, > .id_table = lm3533_i2c_ids, > .probe = lm3533_i2c_probe, > diff --git a/drivers/video/backlight/lm3533_bl.c b/drivers/video/backlight/lm3533_bl.c > index babfd3ceec86..42da652df58d 100644 > --- a/drivers/video/backlight/lm3533_bl.c > +++ b/drivers/video/backlight/lm3533_bl.c > @@ -9,7 +9,9 @@ > > #include <linux/module.h> > #include <linux/init.h> > +#include <linux/mod_devicetable.h> > #include <linux/platform_device.h> > +#include <linux/property.h> > #include <linux/backlight.h> > #include <linux/slab.h> > > @@ -27,6 +29,9 @@ struct lm3533_bl { > struct lm3533_ctrlbank cb; > struct backlight_device *bd; > int id; > + > + u32 max_current; > + u32 pwm; > }; > > > @@ -246,25 +251,24 @@ 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; > int ret; > > dev_dbg(&pdev->dev, "%s\n", __func__); > @@ -273,12 +277,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; > @@ -295,13 +293,20 @@ 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; > + > memset(&props, 0, sizeof(props)); > props.type = BACKLIGHT_RAW; > props.max_brightness = LM3533_BL_MAX_BRIGHTNESS; > - props.brightness = pdata->default_brightness; > - bd = devm_backlight_device_register(&pdev->dev, pdata->name, > - pdev->dev.parent, bl, &lm3533_bl_ops, > - &props); > + props.brightness = LM3533_BL_MAX_BRIGHTNESS; > + device_property_read_u32(&pdev->dev, "default-brightness", > + &props.brightness); > + > + bd = devm_backlight_device_register(&pdev->dev, 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); > @@ -320,7 +325,15 @@ static int lm3533_bl_probe(struct platform_device *pdev) > > backlight_update_status(bd); > > - ret = lm3533_bl_setup(bl, pdata); > + device_property_read_u32(&pdev->dev, "led-max-microamp", > + &bl->max_current); > + bl->max_current = clamp(bl->max_current, LM3533_LED_MAX_CURRENT_MIN, > + LM3533_LED_MAX_CURRENT_MAX); > + > + bl->pwm = 0; > + device_property_read_u32(&pdev->dev, "ti,pwm-config-mask", &bl->pwm); > + > + ret = lm3533_bl_setup(bl); > if (ret) > goto err_sysfs_remove; > > @@ -381,10 +394,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 69059a7a2ce5..3aa962d4c747 100644 > --- a/include/linux/mfd/lm3533.h > +++ b/include/linux/mfd/lm3533.h > @@ -15,6 +15,9 @@ > #define LM3533_ATTR_RW(_name) \ > DEVICE_ATTR(_name, S_IRUGO | S_IWUSR , show_##_name, store_##_name) > > +#define LM3533_LED_MAX_CURRENT_MIN 5000 > +#define LM3533_LED_MAX_CURRENT_MAX 29800 > + > struct device; > struct gpio_desc; > struct regmap; > @@ -25,7 +28,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 +43,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; > -}; > - > extern int lm3533_ctrlbank_enable(struct lm3533_ctrlbank *cb); > extern int lm3533_ctrlbank_disable(struct lm3533_ctrlbank *cb); > > -- > 2.51.0 > ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v1 3/6] mfd: lm3533: Add support for VIN power supply 2026-05-17 7:43 [PATCH v1 0/6] mfd: lm3533: convert to OF bindings, improve support Svyatoslav Ryhel 2026-05-17 7:43 ` [PATCH v1 1/6] dt-bindings: leds: Document TI LM3533 LED controller Svyatoslav Ryhel 2026-05-17 7:43 ` [PATCH v1 2/6] mfd: lm3533: Convert to use OF bindings Svyatoslav Ryhel @ 2026-05-17 7:43 ` Svyatoslav Ryhel 2026-05-17 8:47 ` sashiko-bot 2026-05-17 7:43 ` [PATCH v1 4/6] mfd: lm3533: set DMA mask Svyatoslav Ryhel ` (3 subsequent siblings) 6 siblings, 1 reply; 24+ messages in thread From: Svyatoslav Ryhel @ 2026-05-17 7:43 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: 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 | 22 ++++++++++++++++++++-- include/linux/mfd/lm3533.h | 1 + 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/drivers/mfd/lm3533-core.c b/drivers/mfd/lm3533-core.c index 83ebd780f39d..131eb1a1c8eb 100644 --- a/drivers/mfd/lm3533-core.c +++ b/drivers/mfd/lm3533-core.c @@ -173,14 +173,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 { @@ -331,7 +342,9 @@ static int lm3533_device_init(struct lm3533 *lm3533) struct device *dev = lm3533->dev; int ret; - lm3533_enable(lm3533); + ret = lm3533_enable(lm3533); + if (ret) + return ret; ret = lm3533_update(lm3533, LM3533_REG_BOOST_PWM, lm3533->boost_freq << LM3533_BOOST_FREQ_SHIFT, @@ -453,6 +466,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 3aa962d4c747..e355a3ac982e 100644 --- a/include/linux/mfd/lm3533.h +++ b/include/linux/mfd/lm3533.h @@ -28,6 +28,7 @@ struct lm3533 { struct regmap *regmap; struct gpio_desc *hwen; + struct regulator *vin_supply; u32 boost_ovp; u32 boost_freq; -- 2.51.0 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v1 3/6] mfd: lm3533: Add support for VIN power supply 2026-05-17 7:43 ` [PATCH v1 3/6] mfd: lm3533: Add support for VIN power supply Svyatoslav Ryhel @ 2026-05-17 8:47 ` sashiko-bot 0 siblings, 0 replies; 24+ messages in thread From: sashiko-bot @ 2026-05-17 8:47 UTC (permalink / raw) To: Svyatoslav Ryhel; +Cc: conor+dt, robh, devicetree Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] Implicit dependency on <linux/regulator/consumer.h> without an explicit include. -- commit a9ff3766520433ef45982257dcd46aa034405482 Author: Svyatoslav Ryhel <clamor95@gmail.com> mfd: lm3533: Add support for VIN power supply Add support for 2.7V-5.5V VIN power supply. > diff --git a/drivers/mfd/lm3533-core.c b/drivers/mfd/lm3533-core.c > index 83ebd780f39d5..131eb1a1c8eba 100644 > --- a/drivers/mfd/lm3533-core.c > +++ b/drivers/mfd/lm3533-core.c [ ... ] > @@ -173,14 +173,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); This isn't a bug, but should we include <linux/regulator/consumer.h> in this file? Relying on implicit includes can lead to build breakages if upstream headers are reorganized. > + if (ret) { > + dev_err(lm3533->dev, "failed to enable vin power supply\n"); > + return ret; > + } > + > gpiod_set_value(lm3533->hwen, 1); > + > + return 0; > } [ ... ] > diff --git a/include/linux/mfd/lm3533.h b/include/linux/mfd/lm3533.h > index 3aa962d4c7473..e355a3ac982ec 100644 > --- a/include/linux/mfd/lm3533.h > +++ b/include/linux/mfd/lm3533.h > @@ -28,6 +28,7 @@ struct lm3533 { > struct regmap *regmap; > > struct gpio_desc *hwen; > + struct regulator *vin_supply; Could this header use a forward declaration for struct regulator? > > u32 boost_ovp; > u32 boost_freq; -- Sashiko AI review · https://sashiko.dev/#/patchset/20260517074306.30937-1-clamor95@gmail.com?part=3 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v1 4/6] mfd: lm3533: set DMA mask 2026-05-17 7:43 [PATCH v1 0/6] mfd: lm3533: convert to OF bindings, improve support Svyatoslav Ryhel ` (2 preceding siblings ...) 2026-05-17 7:43 ` [PATCH v1 3/6] mfd: lm3533: Add support for VIN power supply Svyatoslav Ryhel @ 2026-05-17 7:43 ` Svyatoslav Ryhel 2026-05-17 9:09 ` sashiko-bot 2026-05-17 7:43 ` [PATCH v1 5/6] video: backlight: lm3533_bl: Set initial mapping mode from DT Svyatoslav Ryhel ` (2 subsequent siblings) 6 siblings, 1 reply; 24+ messages in thread From: Svyatoslav Ryhel @ 2026-05-17 7:43 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: 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 131eb1a1c8eb..c7914afd564c 100644 --- a/drivers/mfd/lm3533-core.c +++ b/drivers/mfd/lm3533-core.c @@ -499,6 +499,10 @@ static int lm3533_i2c_probe(struct i2c_client *i2c) lm3533->have_leds = true; } + /* Parent I2C controller uses DMA, LM3533 and child devices do not */ + i2c->dev.coherent_dma_mask = 0; + i2c->dev.dma_mask = &i2c->dev.coherent_dma_mask; + return lm3533_device_init(lm3533); } -- 2.51.0 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v1 4/6] mfd: lm3533: set DMA mask 2026-05-17 7:43 ` [PATCH v1 4/6] mfd: lm3533: set DMA mask Svyatoslav Ryhel @ 2026-05-17 9:09 ` sashiko-bot 0 siblings, 0 replies; 24+ messages in thread From: sashiko-bot @ 2026-05-17 9:09 UTC (permalink / raw) To: Svyatoslav Ryhel; +Cc: robh, devicetree, conor+dt Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Bypassing the kernel DMA API by directly modifying 'dev.dma_mask' and 'dev.coherent_dma_mask' to fake a 0-bit DMA mask. -- commit e60afe532ed2b9233017faa2b381f88d84566aea Author: Svyatoslav Ryhel <clamor95@gmail.com> mfd: lm3533: set DMA mask This commit explicitly sets the dma_mask and coherent_dma_mask of the i2c client device to zero. This is done to suppress a "DMA mask not set" platform warning, as the device and its children do not use DMA. > diff --git a/drivers/mfd/lm3533-core.c b/drivers/mfd/lm3533-core.c > index 131eb1a1c8eba..c7914afd564cf 100644 > --- a/drivers/mfd/lm3533-core.c > +++ b/drivers/mfd/lm3533-core.c [ ... ] > @@ -499,6 +499,10 @@ static int lm3533_i2c_probe(struct i2c_client *i2c) > lm3533->have_leds = true; > } > > + /* Parent I2C controller uses DMA, LM3533 and child devices do not */ > + i2c->dev.coherent_dma_mask = 0; > + i2c->dev.dma_mask = &i2c->dev.coherent_dma_mask; Are we bypassing the kernel DMA API here by manually modifying the struct device fields instead of using a helper like dma_coerce_mask_and_coherent()? Directly assigning to dev.dma_mask and dev.coherent_dma_mask circumvents architecture-specific DMA validation. Additionally, is a 0-bit DMA mask conceptually valid? Standard validation functions might reject a zero mask. Bypassing the API to inject an invalid mask solely to silence a subsystem warning could misrepresent hardware capabilities and violate architectural layering. > + > return lm3533_device_init(lm3533); > } -- Sashiko AI review · https://sashiko.dev/#/patchset/20260517074306.30937-1-clamor95@gmail.com?part=4 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v1 5/6] video: backlight: lm3533_bl: Set initial mapping mode from DT 2026-05-17 7:43 [PATCH v1 0/6] mfd: lm3533: convert to OF bindings, improve support Svyatoslav Ryhel ` (3 preceding siblings ...) 2026-05-17 7:43 ` [PATCH v1 4/6] mfd: lm3533: set DMA mask Svyatoslav Ryhel @ 2026-05-17 7:43 ` Svyatoslav Ryhel 2026-05-17 9:28 ` sashiko-bot 2026-05-17 7:43 ` [PATCH v1 6/6] video: leds: backlight: lm3533: Support getting LED sources " Svyatoslav Ryhel 2026-05-17 7:59 ` [PATCH v1 0/6] mfd: lm3533: convert to OF bindings, improve support Andy Shevchenko 6 siblings, 1 reply; 24+ messages in thread From: Svyatoslav Ryhel @ 2026-05-17 7:43 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: 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. Additionally, update the linear sysfs code, which uses a similar coding pattern. Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com> --- drivers/video/backlight/lm3533_bl.c | 32 ++++++++++++++++------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/drivers/video/backlight/lm3533_bl.c b/drivers/video/backlight/lm3533_bl.c index 42da652df58d..be9114b7e0ad 100644 --- a/drivers/video/backlight/lm3533_bl.c +++ b/drivers/video/backlight/lm3533_bl.c @@ -22,6 +22,7 @@ #define LM3533_BL_MAX_BRIGHTNESS 255 #define LM3533_REG_CTRLBANK_AB_BCONF 0x1a +#define CTRLBANK_AB_BCONF_MODE(n) BIT(2 * (n) + 1) struct lm3533_bl { @@ -32,6 +33,7 @@ struct lm3533_bl { u32 max_current; u32 pwm; + bool linear; }; @@ -135,8 +137,9 @@ static ssize_t show_linear(struct device *dev, struct device_attribute *attr, char *buf) { struct lm3533_bl *bl = dev_get_drvdata(dev); + int id = lm3533_bl_get_ctrlbank_id(bl); + u8 mask = CTRLBANK_AB_BCONF_MODE(id); u8 val; - u8 mask; int linear; int ret; @@ -144,8 +147,6 @@ static ssize_t show_linear(struct device *dev, if (ret) return ret; - mask = 1 << (2 * lm3533_bl_get_ctrlbank_id(bl) + 1); - if (val & mask) linear = 1; else @@ -159,23 +160,16 @@ static ssize_t store_linear(struct device *dev, const char *buf, size_t len) { struct lm3533_bl *bl = dev_get_drvdata(dev); + int id = lm3533_bl_get_ctrlbank_id(bl); unsigned long linear; - u8 mask; - u8 val; int ret; if (kstrtoul(buf, 0, &linear)) return -EINVAL; - 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 = lm3533_update(bl->lm3533, LM3533_REG_CTRLBANK_AB_BCONF, + linear ? CTRLBANK_AB_BCONF_MODE(id) : 0, + CTRLBANK_AB_BCONF_MODE(id)); if (ret) return ret; @@ -253,8 +247,15 @@ static struct attribute_group lm3533_bl_attribute_group = { static int lm3533_bl_setup(struct lm3533_bl *bl) { + int id = lm3533_bl_get_ctrlbank_id(bl); int ret; + ret = lm3533_update(bl->lm3533, LM3533_REG_CTRLBANK_AB_BCONF, + bl->linear ? CTRLBANK_AB_BCONF_MODE(id) : 0, + CTRLBANK_AB_BCONF_MODE(id)); + if (ret) + return ret; + ret = lm3533_ctrlbank_set_max_current(&bl->cb, bl->max_current); if (ret) return ret; @@ -333,6 +334,9 @@ static int lm3533_bl_probe(struct platform_device *pdev) bl->pwm = 0; device_property_read_u32(&pdev->dev, "ti,pwm-config-mask", &bl->pwm); + bl->linear = device_property_read_bool(&pdev->dev, + "ti,linear-mapping-mode"); + ret = lm3533_bl_setup(bl); if (ret) goto err_sysfs_remove; -- 2.51.0 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v1 5/6] video: backlight: lm3533_bl: Set initial mapping mode from DT 2026-05-17 7:43 ` [PATCH v1 5/6] video: backlight: lm3533_bl: Set initial mapping mode from DT Svyatoslav Ryhel @ 2026-05-17 9:28 ` sashiko-bot 0 siblings, 0 replies; 24+ messages in thread From: sashiko-bot @ 2026-05-17 9:28 UTC (permalink / raw) To: Svyatoslav Ryhel; +Cc: devicetree, conor+dt, robh Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] The driver unconditionally clears the hardware linear mapping mode if the new DT property is absent, breaking existing configurations. - [High] The `linear` sysfs attribute is exposed to userspace before hardware initialization completes, allowing userspace configurations to be silently discarded. -- commit cebf8bbd6f9d389030ebbf6dbf1725031e2f46a7 Author: Svyatoslav Ryhel <clamor95@gmail.com> video: backlight: lm3533_bl: Set initial mapping mode from DT This commit adds support to configure the initial backlight mapping mode (linear or exponential) from the device tree instead of leaving it unconfigured. It also updates the linear sysfs attribute handling to use similar patterns. > diff --git a/drivers/video/backlight/lm3533_bl.c b/drivers/video/backlight/lm3533_bl.c > index 42da652df58d9..be9114b7e0ad7 100644 > --- a/drivers/video/backlight/lm3533_bl.c > +++ b/drivers/video/backlight/lm3533_bl.c [ ... ] > @@ -253,8 +247,15 @@ static struct attribute_group lm3533_bl_attribute_group = { > > static int lm3533_bl_setup(struct lm3533_bl *bl) > { > + int id = lm3533_bl_get_ctrlbank_id(bl); > int ret; > > + ret = lm3533_update(bl->lm3533, LM3533_REG_CTRLBANK_AB_BCONF, > + bl->linear ? CTRLBANK_AB_BCONF_MODE(id) : 0, > + CTRLBANK_AB_BCONF_MODE(id)); Could this break existing hardware configurations where the DT property is absent but the bootloader already configured linear mode? Previously, this register was left unconfigured. Because device_property_read_bool() returns false when the property is missing, this unconditionally forces the mapping mode to exponential (0), silently reverting any existing setups lacking the new ti,linear-mapping-mode property. [ ... ] > @@ -333,6 +334,9 @@ static int lm3533_bl_probe(struct platform_device *pdev) > bl->pwm = 0; > device_property_read_u32(&pdev->dev, "ti,pwm-config-mask", &bl->pwm); > > + bl->linear = device_property_read_bool(&pdev->dev, > + "ti,linear-mapping-mode"); > + > ret = lm3533_bl_setup(bl); Does this sequence silently discard userspace configuration? Earlier in lm3533_bl_probe(), sysfs_create_group() creates the linear sysfs attribute before we read the device properties and call lm3533_bl_setup(). If a udev rule writes to the linear attribute, it will update the hardware register via store_linear(), but the setup function here will subsequently overwrite the register with the bl->linear value cached from DT, discarding the userspace setting. -- Sashiko AI review · https://sashiko.dev/#/patchset/20260517074306.30937-1-clamor95@gmail.com?part=5 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v1 6/6] video: leds: backlight: lm3533: Support getting LED sources from DT 2026-05-17 7:43 [PATCH v1 0/6] mfd: lm3533: convert to OF bindings, improve support Svyatoslav Ryhel ` (4 preceding siblings ...) 2026-05-17 7:43 ` [PATCH v1 5/6] video: backlight: lm3533_bl: Set initial mapping mode from DT Svyatoslav Ryhel @ 2026-05-17 7:43 ` Svyatoslav Ryhel 2026-05-17 9:45 ` sashiko-bot 2026-05-17 7:59 ` [PATCH v1 0/6] mfd: lm3533: convert to OF bindings, improve support Andy Shevchenko 6 siblings, 1 reply; 24+ messages in thread From: Svyatoslav Ryhel @ 2026-05-17 7:43 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: 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 | 55 +++++++++++++++++++++++++++-- drivers/video/backlight/lm3533_bl.c | 39 +++++++++++++++++++- 2 files changed, 91 insertions(+), 3 deletions(-) diff --git a/drivers/leds/leds-lm3533.c b/drivers/leds/leds-lm3533.c index f6345bc1f443..c4eaf30880a1 100644 --- a/drivers/leds/leds-lm3533.c +++ b/drivers/leds/leds-lm3533.c @@ -7,6 +7,7 @@ * Author: Johan Hovold <jhovold@gmail.com> */ +#include <linux/bits.h> #include <linux/module.h> #include <linux/leds.h> #include <linux/mfd/core.h> @@ -26,6 +27,12 @@ #define LM3533_ALS_CHANNEL_LV_MIN 1 #define LM3533_ALS_CHANNEL_LV_MAX 2 +#define LM3533_REG_OUTPUT_CONF1 0x10 +#define OUTPUT_CONF1_MASK GENMASK(7, 2) +#define OUTPUT_CONF1_SHIFT 2 +#define LM3533_REG_OUTPUT_CONF2 0x11 +#define OUTPUT_CONF2_MASK GENMASK(3, 0) +#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 @@ -40,7 +47,7 @@ #define LM3533_REG_CTRLBANK_BCONF_ALS_CHANNEL_MASK 0x01 #define LM3533_LED_FLAG_PATTERN_ENABLE 1 - +#define LM3533_MAX_LEDS 5 struct lm3533_led { struct lm3533 *lm3533; @@ -53,6 +60,9 @@ struct lm3533_led { u32 max_current; u32 pwm; + + u32 num_leds; + u32 leds[LM3533_MAX_LEDS]; }; @@ -639,7 +649,30 @@ 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; + int ret, i; + + if (led->num_leds) { + for (i = 0; i < led->num_leds; i++) { + output_cfg_shift = led->leds[i] * 2; + output_cfg_val |= led->id << output_cfg_shift; + } + + /* LVLED1, LVLED2 and LVLED3 */ + ret = lm3533_update(led->lm3533, LM3533_REG_OUTPUT_CONF1, + output_cfg_val << OUTPUT_CONF1_SHIFT, + OUTPUT_CONF1_MASK); + if (ret) + return ret; + + /* LVLED4 and LVLED5 */ + ret = lm3533_update(led->lm3533, LM3533_REG_OUTPUT_CONF1, + output_cfg_val >> OUTPUT_CONF2_SHIFT, + OUTPUT_CONF2_MASK); + if (ret) + return ret; + } ret = lm3533_ctrlbank_set_max_current(&led->cb, led->max_current); if (ret) @@ -713,6 +746,24 @@ 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-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 || led->num_leds > LM3533_MAX_LEDS) + led->num_leds = 0; + + if (led->num_leds > 0 && led->num_leds < LM3533_MAX_LEDS) { + 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 be9114b7e0ad..2898cb229643 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> @@ -21,9 +22,12 @@ #define LM3533_HVCTRLBANK_COUNT 2 #define LM3533_BL_MAX_BRIGHTNESS 255 +#define LM3533_REG_OUTPUT_CONF1 0x10 +#define OUTPUT_CONF1_MASK GENMASK(1, 0) #define LM3533_REG_CTRLBANK_AB_BCONF 0x1a #define CTRLBANK_AB_BCONF_MODE(n) BIT(2 * (n) + 1) +#define LM3533_MAX_LED_STRINGS 2 struct lm3533_bl { struct lm3533 *lm3533; @@ -34,6 +38,9 @@ struct lm3533_bl { u32 max_current; u32 pwm; bool linear; + + u32 num_leds; + u32 led_strings[LM3533_MAX_LED_STRINGS]; }; @@ -248,7 +255,8 @@ static struct attribute_group lm3533_bl_attribute_group = { static int lm3533_bl_setup(struct lm3533_bl *bl) { int id = lm3533_bl_get_ctrlbank_id(bl); - int ret; + u32 output_cfg_val = 0; + int ret, i; ret = lm3533_update(bl->lm3533, LM3533_REG_CTRLBANK_AB_BCONF, bl->linear ? CTRLBANK_AB_BCONF_MODE(id) : 0, @@ -256,6 +264,16 @@ static int lm3533_bl_setup(struct lm3533_bl *bl) if (ret) return ret; + if (bl->num_leds) { + for (i = 0; i < bl->num_leds; i++) + output_cfg_val |= id << bl->led_strings[i]; + + ret = lm3533_update(bl->lm3533, LM3533_REG_OUTPUT_CONF1, + output_cfg_val, OUTPUT_CONF1_MASK); + if (ret) + return ret; + } + ret = lm3533_ctrlbank_set_max_current(&bl->cb, bl->max_current); if (ret) return ret; @@ -337,6 +355,25 @@ static int lm3533_bl_probe(struct platform_device *pdev) bl->linear = device_property_read_bool(&pdev->dev, "ti,linear-mapping-mode"); + bl->num_leds = device_property_count_u32(&pdev->dev, "led-sources"); + + /* + * 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 || bl->num_leds > LM3533_MAX_LED_STRINGS) + bl->num_leds = 0; + + if (bl->num_leds > 0 && bl->num_leds < LM3533_MAX_LED_STRINGS) { + ret = device_property_read_u32_array(&pdev->dev, "led-sources", + bl->led_strings, + bl->num_leds); + if (ret) { + dev_err(&pdev->dev, "failed to get led-sources\n"); + goto err_sysfs_remove; + } + } + ret = lm3533_bl_setup(bl); if (ret) goto err_sysfs_remove; -- 2.51.0 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v1 6/6] video: leds: backlight: lm3533: Support getting LED sources from DT 2026-05-17 7:43 ` [PATCH v1 6/6] video: leds: backlight: lm3533: Support getting LED sources " Svyatoslav Ryhel @ 2026-05-17 9:45 ` sashiko-bot 0 siblings, 0 replies; 24+ messages in thread From: sashiko-bot @ 2026-05-17 9:45 UTC (permalink / raw) To: Svyatoslav Ryhel; +Cc: robh, conor+dt, devicetree Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider: - [High] An off-by-one bounds check skips parsing `led-sources` when exactly the maximum number of items is provided. - [High] `lm3533_update()` uses static global masks, clobbering the configuration of LEDs belonging to other control banks. - [High] LVLED4 and LVLED5 configuration is written to the wrong register (`LM3533_REG_OUTPUT_CONF1`), corrupting backlight settings. - [Medium] Assigning the negative error code of `device_property_count_u32` to an unsigned integer circumvents error checks. - [Medium] Unvalidated device tree array values are used as bit shift amounts, risking Undefined Behavior. -- commit a94501f0a6935a4f57e9e0fb89310c12e86759a9 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 > --- a/drivers/leds/leds-lm3533.c > +++ b/drivers/leds/leds-lm3533.c [ ... ] > @@ -639,7 +649,30 @@ 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; > + int ret, i; > + > + if (led->num_leds) { > + for (i = 0; i < led->num_leds; i++) { > + output_cfg_shift = led->leds[i] * 2; Are the values in led->leds[i] validated against hardware bounds before being used to calculate the bit shift? If the device tree provides excessively large values, the shift amount could exceed the integer bit width. > + output_cfg_val |= led->id << output_cfg_shift; > + } > + > + /* LVLED1, LVLED2 and LVLED3 */ > + ret = lm3533_update(led->lm3533, LM3533_REG_OUTPUT_CONF1, > + output_cfg_val << OUTPUT_CONF1_SHIFT, > + OUTPUT_CONF1_MASK); Since OUTPUT_CONF1_MASK spans all LVLED1-3 bits and control banks are probed sequentially, will this update clear the routing configurations of previously probed banks? > + if (ret) > + return ret; > + > + /* LVLED4 and LVLED5 */ > + ret = lm3533_update(led->lm3533, LM3533_REG_OUTPUT_CONF1, Is LM3533_REG_OUTPUT_CONF1 the correct register for LVLED4 and LVLED5? Writing to LM3533_REG_OUTPUT_CONF1 with OUTPUT_CONF2_MASK clears its lower bits, which might overwrite the hardware mapping for the high-voltage backlight and LVLED1 settings instead of configuring LVLED4 and LVLED5. > + output_cfg_val >> OUTPUT_CONF2_SHIFT, > + OUTPUT_CONF2_MASK); > + if (ret) > + return ret; > + } [ ... ] > @@ -713,6 +746,24 @@ 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-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 || led->num_leds > LM3533_MAX_LEDS) Does assigning the result of device_property_count_u32() directly to the unsigned led->num_leds cause negative error codes like -EINVAL to wrap into large positive values? This causes the led->num_leds <= 0 check to fail. Should a signed integer be used for the return value check before assigning it? > + led->num_leds = 0; > + > + if (led->num_leds > 0 && led->num_leds < LM3533_MAX_LEDS) { Does the strict inequality led->num_leds < LM3533_MAX_LEDS prevent the driver from reading the led-sources array when exactly the maximum allowed number of items is specified? > + 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; > + } > + } > + [ ... ] > 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 [ ... ] > @@ -248,7 +255,8 @@ static struct attribute_group lm3533_bl_attribute_group = { > static int lm3533_bl_setup(struct lm3533_bl *bl) > { > int id = lm3533_bl_get_ctrlbank_id(bl); > - int ret; > + u32 output_cfg_val = 0; > + int ret, i; > > ret = lm3533_update(bl->lm3533, LM3533_REG_CTRLBANK_AB_BCONF, > bl->linear ? CTRLBANK_AB_BCONF_MODE(id) : 0, > @@ -256,6 +264,16 @@ static int lm3533_bl_setup(struct lm3533_bl *bl) > if (ret) > return ret; > > + if (bl->num_leds) { > + for (i = 0; i < bl->num_leds; i++) > + output_cfg_val |= id << bl->led_strings[i]; > + > + ret = lm3533_update(bl->lm3533, LM3533_REG_OUTPUT_CONF1, > + output_cfg_val, OUTPUT_CONF1_MASK); Similar to the LED setup, will applying OUTPUT_CONF1_MASK without preserving existing bit states overwrite the routing of other control banks? > + if (ret) > + return ret; > + } > + [ ... ] > @@ -337,6 +355,25 @@ static int lm3533_bl_probe(struct platform_device *pdev) > bl->linear = device_property_read_bool(&pdev->dev, > "ti,linear-mapping-mode"); > > + bl->num_leds = device_property_count_u32(&pdev->dev, "led-sources"); > + > + /* > + * 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 || bl->num_leds > LM3533_MAX_LED_STRINGS) As with the LED probe function above, does this unsigned variable wrapper prevent catching negative error codes from device_property_count_u32()? > + bl->num_leds = 0; > + > + if (bl->num_leds > 0 && bl->num_leds < LM3533_MAX_LED_STRINGS) { Does this also skip array parsing if exactly LM3533_MAX_LED_STRINGS items are provided? > + ret = device_property_read_u32_array(&pdev->dev, "led-sources", > + bl->led_strings, > + bl->num_leds); > + if (ret) { > + dev_err(&pdev->dev, "failed to get led-sources\n"); > + goto err_sysfs_remove; > + } > + } > + -- Sashiko AI review · https://sashiko.dev/#/patchset/20260517074306.30937-1-clamor95@gmail.com?part=6 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v1 0/6] mfd: lm3533: convert to OF bindings, improve support 2026-05-17 7:43 [PATCH v1 0/6] mfd: lm3533: convert to OF bindings, improve support Svyatoslav Ryhel ` (5 preceding siblings ...) 2026-05-17 7:43 ` [PATCH v1 6/6] video: leds: backlight: lm3533: Support getting LED sources " Svyatoslav Ryhel @ 2026-05-17 7:59 ` Andy Shevchenko 2026-05-17 10:13 ` Svyatoslav Ryhel 6 siblings, 1 reply; 24+ messages in thread From: Andy Shevchenko @ 2026-05-17 7:59 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, dri-devel, linux-leds, devicetree, linux-kernel, linux-iio, linux-fbdev On Sun, May 17, 2026 at 10:43:00AM +0300, Svyatoslav Ryhel wrote: > Convert LM3533 to OF bindings, add missing VIN supply, add support for > setting mapping mode and LED sources based on device tree. How is this being different to https://lore.kernel.org/lkml/20250218132702.114669-1-clamor95@gmail.com/ ? What about this comment https://lore.kernel.org/lkml/ZmBcvtLCzllQDWVX@hovoldconsulting.com/ ? Have you talked to Bjorn A and Johan? -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v1 0/6] mfd: lm3533: convert to OF bindings, improve support 2026-05-17 7:59 ` [PATCH v1 0/6] mfd: lm3533: convert to OF bindings, improve support Andy Shevchenko @ 2026-05-17 10:13 ` Svyatoslav Ryhel 2026-05-17 10:20 ` Andy Shevchenko 0 siblings, 1 reply; 24+ messages in thread From: Svyatoslav Ryhel @ 2026-05-17 10:13 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, dri-devel, linux-leds, devicetree, linux-kernel, linux-iio, linux-fbdev нд, 17 трав. 2026 р. о 10:59 Andy Shevchenko <andriy.shevchenko@intel.com> пише: > > On Sun, May 17, 2026 at 10:43:00AM +0300, Svyatoslav Ryhel wrote: > > Convert LM3533 to OF bindings, add missing VIN supply, add support for > > setting mapping mode and LED sources based on device tree. > > How is this being different to > https://lore.kernel.org/lkml/20250218132702.114669-1-clamor95@gmail.com/ > ? > I have decided to have a fresh look, this is continuation. > What about this comment > https://lore.kernel.org/lkml/ZmBcvtLCzllQDWVX@hovoldconsulting.com/ > ? Have you talked to Bjorn A and Johan? > No, but since grep -r "lm3533_platform_data" * gives 0 results there are no platform_data users of lm3533. > -- > With Best Regards, > Andy Shevchenko > > ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v1 0/6] mfd: lm3533: convert to OF bindings, improve support 2026-05-17 10:13 ` Svyatoslav Ryhel @ 2026-05-17 10:20 ` Andy Shevchenko 2026-05-17 10:34 ` Svyatoslav Ryhel 0 siblings, 1 reply; 24+ messages in thread From: Andy Shevchenko @ 2026-05-17 10:20 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, dri-devel, linux-leds, devicetree, linux-kernel, linux-iio, linux-fbdev On Sun, May 17, 2026 at 01:13:22PM +0300, Svyatoslav Ryhel wrote: > нд, 17 трав. 2026 р. о 10:59 Andy Shevchenko <andriy.shevchenko@intel.com> пише: > > On Sun, May 17, 2026 at 10:43:00AM +0300, Svyatoslav Ryhel wrote: > > > Convert LM3533 to OF bindings, add missing VIN supply, add support for > > > setting mapping mode and LED sources based on device tree. > > > > How is this being different to > > https://lore.kernel.org/lkml/20250218132702.114669-1-clamor95@gmail.com/ > > ? > > I have decided to have a fresh look, this is continuation. Then it should be something like v4? > > What about this comment > > https://lore.kernel.org/lkml/ZmBcvtLCzllQDWVX@hovoldconsulting.com/ > > ? Have you talked to Bjorn A and Johan? > > No, but since grep -r "lm3533_platform_data" * gives 0 results there > are no platform_data users of lm3533. Johan was against driver removal (while it sounds logical due to above) and referred to some other patches from somebody else. I think you need to synchronise with the people to have a clear roadmap that all stakeholders are agree with. With that, the split and other technical issues can be solved during the normal process. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v1 0/6] mfd: lm3533: convert to OF bindings, improve support 2026-05-17 10:20 ` Andy Shevchenko @ 2026-05-17 10:34 ` Svyatoslav Ryhel 2026-05-17 10:40 ` Andy Shevchenko 0 siblings, 1 reply; 24+ messages in thread From: Svyatoslav Ryhel @ 2026-05-17 10:34 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, dri-devel, linux-leds, devicetree, linux-kernel, linux-iio, linux-fbdev нд, 17 трав. 2026 р. о 13:20 Andy Shevchenko <andriy.shevchenko@intel.com> пише: > > On Sun, May 17, 2026 at 01:13:22PM +0300, Svyatoslav Ryhel wrote: > > нд, 17 трав. 2026 р. о 10:59 Andy Shevchenko <andriy.shevchenko@intel.com> пише: > > > On Sun, May 17, 2026 at 10:43:00AM +0300, Svyatoslav Ryhel wrote: > > > > Convert LM3533 to OF bindings, add missing VIN supply, add support for > > > > setting mapping mode and LED sources based on device tree. > > > > > > How is this being different to > > > https://lore.kernel.org/lkml/20250218132702.114669-1-clamor95@gmail.com/ > > > ? > > > > I have decided to have a fresh look, this is continuation. > > Then it should be something like v4? > That was more than a year ago, lets start fresh. Those patches are gone, though I did add many of suggestions from there. > > > What about this comment > > > https://lore.kernel.org/lkml/ZmBcvtLCzllQDWVX@hovoldconsulting.com/ > > > ? Have you talked to Bjorn A and Johan? > > > > No, but since grep -r "lm3533_platform_data" * gives 0 results there > > are no platform_data users of lm3533. > > Johan was against driver removal (while it sounds logical due to above) and > referred to some other patches from somebody else. I think you need to > synchronise with the people to have a clear roadmap that all stakeholders > are agree with. With that, the split and other technical issues can be solved > during the normal process. > And this discussion was 2 years ago, and "This device is used in a bunch of Sony phones and Bjorn A posted a series adding devicetree bindings a few years ago" They have had more than enough time, don't you think? You would definitely know if there were any activity since you are IIO reviewer and would be included, were there any activity? > -- > With Best Regards, > Andy Shevchenko > > ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v1 0/6] mfd: lm3533: convert to OF bindings, improve support 2026-05-17 10:34 ` Svyatoslav Ryhel @ 2026-05-17 10:40 ` Andy Shevchenko 2026-05-17 10:44 ` Svyatoslav Ryhel 0 siblings, 1 reply; 24+ messages in thread From: Andy Shevchenko @ 2026-05-17 10:40 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, dri-devel, linux-leds, devicetree, linux-kernel, linux-iio, linux-fbdev On Sun, May 17, 2026 at 01:34:32PM +0300, Svyatoslav Ryhel wrote: > нд, 17 трав. 2026 р. о 13:20 Andy Shevchenko <andriy.shevchenko@intel.com> пише: > > On Sun, May 17, 2026 at 01:13:22PM +0300, Svyatoslav Ryhel wrote: > > > нд, 17 трав. 2026 р. о 10:59 Andy Shevchenko <andriy.shevchenko@intel.com> пише: > > > > On Sun, May 17, 2026 at 10:43:00AM +0300, Svyatoslav Ryhel wrote: > > > > > Convert LM3533 to OF bindings, add missing VIN supply, add support for > > > > > setting mapping mode and LED sources based on device tree. > > > > > > > > How is this being different to > > > > https://lore.kernel.org/lkml/20250218132702.114669-1-clamor95@gmail.com/ > > > > ? > > > > > > I have decided to have a fresh look, this is continuation. > > > > Then it should be something like v4? > > That was more than a year ago, lets start fresh. Those patches are > gone, though I did add many of suggestions from there. > > > > > What about this comment > > > > https://lore.kernel.org/lkml/ZmBcvtLCzllQDWVX@hovoldconsulting.com/ > > > > ? Have you talked to Bjorn A and Johan? > > > > > > No, but since grep -r "lm3533_platform_data" * gives 0 results there > > > are no platform_data users of lm3533. > > > > Johan was against driver removal (while it sounds logical due to above) and > > referred to some other patches from somebody else. I think you need to > > synchronise with the people to have a clear roadmap that all stakeholders > > are agree with. With that, the split and other technical issues can be solved > > during the normal process. > > And this discussion was 2 years ago, and > > "This device is used in a bunch of Sony phones and Bjorn A posted a > series adding devicetree bindings a few years ago" > > They have had more than enough time, don't you think? You would > definitely know if there were any activity since you are IIO reviewer > and would be included, were there any activity? At least it's polite to Cc them your version. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v1 0/6] mfd: lm3533: convert to OF bindings, improve support 2026-05-17 10:40 ` Andy Shevchenko @ 2026-05-17 10:44 ` Svyatoslav Ryhel 0 siblings, 0 replies; 24+ messages in thread From: Svyatoslav Ryhel @ 2026-05-17 10:44 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, dri-devel, linux-leds, devicetree, linux-kernel, linux-iio, linux-fbdev, johan нд, 17 трав. 2026 р. о 13:40 Andy Shevchenko <andriy.shevchenko@intel.com> пише: > > On Sun, May 17, 2026 at 01:34:32PM +0300, Svyatoslav Ryhel wrote: > > нд, 17 трав. 2026 р. о 13:20 Andy Shevchenko <andriy.shevchenko@intel.com> пише: > > > On Sun, May 17, 2026 at 01:13:22PM +0300, Svyatoslav Ryhel wrote: > > > > нд, 17 трав. 2026 р. о 10:59 Andy Shevchenko <andriy.shevchenko@intel.com> пише: > > > > > On Sun, May 17, 2026 at 10:43:00AM +0300, Svyatoslav Ryhel wrote: > > > > > > Convert LM3533 to OF bindings, add missing VIN supply, add support for > > > > > > setting mapping mode and LED sources based on device tree. > > > > > > > > > > How is this being different to > > > > > https://lore.kernel.org/lkml/20250218132702.114669-1-clamor95@gmail.com/ > > > > > ? > > > > > > > > I have decided to have a fresh look, this is continuation. > > > > > > Then it should be something like v4? > > > > That was more than a year ago, lets start fresh. Those patches are > > gone, though I did add many of suggestions from there. > > > > > > > What about this comment > > > > > https://lore.kernel.org/lkml/ZmBcvtLCzllQDWVX@hovoldconsulting.com/ > > > > > ? Have you talked to Bjorn A and Johan? > > > > > > > > No, but since grep -r "lm3533_platform_data" * gives 0 results there > > > > are no platform_data users of lm3533. > > > > > > Johan was against driver removal (while it sounds logical due to above) and > > > referred to some other patches from somebody else. I think you need to > > > synchronise with the people to have a clear roadmap that all stakeholders > > > are agree with. With that, the split and other technical issues can be solved > > > during the normal process. > > > > And this discussion was 2 years ago, and > > > > "This device is used in a bunch of Sony phones and Bjorn A posted a > > series adding devicetree bindings a few years ago" > > > > They have had more than enough time, don't you think? You would > > definitely know if there were any activity since you are IIO reviewer > > and would be included, were there any activity? > > At least it's polite to Cc them your version. > I assume yes, I have added Johan but there is no email for Bjorn A. > -- > With Best Regards, > Andy Shevchenko > > ^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2026-05-17 14:27 UTC | newest] Thread overview: 24+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-05-17 7:43 [PATCH v1 0/6] mfd: lm3533: convert to OF bindings, improve support Svyatoslav Ryhel 2026-05-17 7:43 ` [PATCH v1 1/6] dt-bindings: leds: Document TI LM3533 LED controller Svyatoslav Ryhel 2026-05-17 7:58 ` sashiko-bot 2026-05-17 13:44 ` Jonathan Cameron 2026-05-17 14:26 ` Svyatoslav Ryhel 2026-05-17 7:43 ` [PATCH v1 2/6] mfd: lm3533: Convert to use OF bindings Svyatoslav Ryhel 2026-05-17 7:55 ` Andy Shevchenko 2026-05-17 10:11 ` Svyatoslav Ryhel 2026-05-17 8:32 ` sashiko-bot 2026-05-17 11:10 ` Svyatoslav Ryhel 2026-05-17 7:43 ` [PATCH v1 3/6] mfd: lm3533: Add support for VIN power supply Svyatoslav Ryhel 2026-05-17 8:47 ` sashiko-bot 2026-05-17 7:43 ` [PATCH v1 4/6] mfd: lm3533: set DMA mask Svyatoslav Ryhel 2026-05-17 9:09 ` sashiko-bot 2026-05-17 7:43 ` [PATCH v1 5/6] video: backlight: lm3533_bl: Set initial mapping mode from DT Svyatoslav Ryhel 2026-05-17 9:28 ` sashiko-bot 2026-05-17 7:43 ` [PATCH v1 6/6] video: leds: backlight: lm3533: Support getting LED sources " Svyatoslav Ryhel 2026-05-17 9:45 ` sashiko-bot 2026-05-17 7:59 ` [PATCH v1 0/6] mfd: lm3533: convert to OF bindings, improve support Andy Shevchenko 2026-05-17 10:13 ` Svyatoslav Ryhel 2026-05-17 10:20 ` Andy Shevchenko 2026-05-17 10:34 ` Svyatoslav Ryhel 2026-05-17 10:40 ` Andy Shevchenko 2026-05-17 10:44 ` Svyatoslav Ryhel
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox