* [PATCH v5 0/4] backlight: add new max25014 backlight driver
@ 2025-11-07 12:49 Maud Spierings via B4 Relay
2025-11-07 12:49 ` [PATCH v5 1/4] dt-bindings: backlight: Add max25014 support Maud Spierings via B4 Relay
` (3 more replies)
0 siblings, 4 replies; 19+ messages in thread
From: Maud Spierings via B4 Relay @ 2025-11-07 12:49 UTC (permalink / raw)
To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Helge Deller, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Liam Girdwood, Mark Brown
Cc: dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, imx,
linux-arm-kernel, Maud Spierings
The Maxim MAX25014 is an automotive grade backlight driver IC. Its
datasheet can be found at [1].
With its integrated boost controller, it can power 4 channels (led
strings) and has a number of different modes using pwm and or i2c.
Currently implemented is only i2c control.
link: https://www.analog.com/media/en/technical-documentation/data-sheets/MAX25014.pdf [1]
Signed-off-by: Maud Spierings <maudspierings@gocontroll.com>
---
Changes in v5:
- moved comment about current functions of the driver to the actual
comment section of the commit
- fixed the led@0 property, regex patternProperty is not needed as of
now
- added extra clarification about the ISET field/register
- moved #address-cells and #size-cells to the correct location
- remove leftover default-brightness in backlight nodes
- Link to v4: https://lore.kernel.org/r/20251009-max25014-v4-0-6adb2a0aa35f@gocontroll.com
Changes in v4:
- use a led node to describe the backlight
- use led-sources to enable specific channels
- also wait 2ms when there is a supply but no enable
- change dev_warn() to dev_err() in error path in max25014_check_errors()
- set backlight_properties.scale to BACKLIGHT_SCALE_LINEAR
- rebase latest next
- add address-cells and size-cells to i2c4 in av101hdt-a10.dtso
- Link to v3: https://lore.kernel.org/r/20250911-max25014-v3-0-d03f4eba375e@gocontroll.com
Changes in v3:
- fixed commit message type intgrated -> integrated
- added maximum and description to maxim,iset-property
- dropped unused labels and pinctrl in bindings example
- put the compatible first in the bindings example and dts
- removed brackets around defines
- removed the leftover pdata struct field
- removed the initial_brightness struct field
- Link to v2: https://lore.kernel.org/r/20250819-max25014-v2-0-5fd7aeb141ea@gocontroll.com
Changes in v2:
- Remove leftover unused property from the bindings example
- Complete the bindings example with all properties
- Remove some double info from the maxim,iset property
- Remove platform_data header, fold its data into the max25014 struct
- Don't force defines to be unsigned
- Remove stray struct max25014 declaration
- Remove chipname and device from the max25014 struct
- Inline the max25014_backlight_register() and strings_mask() functions
- Remove CONFIG_OF ifdef
- Link to v1: https://lore.kernel.org/r/20250725-max25014-v1-0-0e8cce92078e@gocontroll.com
---
Maud Spierings (4):
dt-bindings: backlight: Add max25014 support
backlight: add max25014atg backlight
arm64: dts: freescale: moduline-display-av101hdt-a10: add backlight
arm64: dts: freescale: moduline-display-av123z7m-n17: add backlight
.../bindings/leds/backlight/maxim,max25014.yaml | 107 ++++++
MAINTAINERS | 6 +
...x8p-ml81-moduline-display-106-av101hdt-a10.dtso | 30 ++
...x8p-ml81-moduline-display-106-av123z7m-n17.dtso | 25 +-
drivers/video/backlight/Kconfig | 7 +
drivers/video/backlight/Makefile | 1 +
drivers/video/backlight/max25014.c | 409 +++++++++++++++++++++
7 files changed, 584 insertions(+), 1 deletion(-)
---
base-commit: 9c0826a5d9aa4d52206dd89976858457a2a8a7ed
change-id: 20250626-max25014-4207591e1af5
Best regards,
--
Maud Spierings <maudspierings@gocontroll.com>
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH v5 1/4] dt-bindings: backlight: Add max25014 support 2025-11-07 12:49 [PATCH v5 0/4] backlight: add new max25014 backlight driver Maud Spierings via B4 Relay @ 2025-11-07 12:49 ` Maud Spierings via B4 Relay 2025-11-07 15:35 ` [PATCH v5 1/4] dt-bindings: backlight: Add max25014 supporty Frank Li 2025-11-07 18:16 ` [PATCH v5 1/4] dt-bindings: backlight: Add max25014 support Conor Dooley 2025-11-07 12:49 ` [PATCH v5 2/4] backlight: add max25014atg backlight Maud Spierings via B4 Relay ` (2 subsequent siblings) 3 siblings, 2 replies; 19+ messages in thread From: Maud Spierings via B4 Relay @ 2025-11-07 12:49 UTC (permalink / raw) To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Helge Deller, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Liam Girdwood, Mark Brown Cc: dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, imx, linux-arm-kernel, Maud Spierings From: Maud Spierings <maudspierings@gocontroll.com> The Maxim MAX25014 is a 4-channel automotive grade backlight driver IC with integrated boost controller. Signed-off-by: Maud Spierings <maudspierings@gocontroll.com> --- In the current implementation the control registers for channel 1, control all channels. So only one led subnode with led-sources is supported right now. If at some point the driver functionality is expanded the bindings can be easily extended with it. --- .../bindings/leds/backlight/maxim,max25014.yaml | 107 +++++++++++++++++++++ MAINTAINERS | 5 + 2 files changed, 112 insertions(+) diff --git a/Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml b/Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml new file mode 100644 index 000000000000..e83723224b07 --- /dev/null +++ b/Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml @@ -0,0 +1,107 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/leds/backlight/maxim,max25014.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Maxim max25014 backlight controller + +maintainers: + - Maud Spierings <maudspierings@gocontroll.com> + +properties: + compatible: + enum: + - maxim,max25014 + + reg: + maxItems: 1 + + "#address-cells": + const: 1 + + "#size-cells": + const: 0 + + enable-gpios: + maxItems: 1 + + interrupts: + maxItems: 1 + + power-supply: + description: Regulator which controls the boost converter input rail. + + pwms: + maxItems: 1 + + maxim,iset: + $ref: /schemas/types.yaml#/definitions/uint32 + maximum: 15 + default: 11 + description: + Value of the ISET field in the ISET register. This controls the current + scale of the outputs, a higher number means more current. + + led@0: + type: object + description: Properties for a string of connected LEDs. + $ref: common.yaml# + + properties: + reg: + const: 0 + + led-sources: + allOf: + - minItems: 1 + maxItems: 4 + items: + minimum: 0 + maximum: 3 + default: [0, 1, 2, 3] + + default-brightness: + minimum: 0 + maximum: 100 + default: 50 + + required: + - reg + + additionalProperties: false + +required: + - compatible + - reg + +additionalProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + #include <dt-bindings/interrupt-controller/irq.h> + + i2c { + #address-cells = <1>; + #size-cells = <0>; + + backlight@6f { + compatible = "maxim,max25014"; + reg = <0x6f>; + #address-cells = <1>; + #size-cells = <0>; + enable-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>; + interrupt-parent = <&gpio1>; + interrupts = <2 IRQ_TYPE_EDGE_FALLING>; + power-supply = <®_backlight>; + pwms = <&pwm1>; + maxim,iset = <7>; + + led@0 { + reg = <0>; + led-sources = <0 1 2 3>; + default-brightness = <50>; + }; + }; + }; diff --git a/MAINTAINERS b/MAINTAINERS index 58c7e3f678d8..606ce086f758 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -15261,6 +15261,11 @@ F: Documentation/userspace-api/media/drivers/max2175.rst F: drivers/media/i2c/max2175* F: include/uapi/linux/max2175.h +MAX25014 BACKLIGHT DRIVER +M: Maud Spierings <maudspierings@gocontroll.com> +S: Maintained +F: Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml + MAX31335 RTC DRIVER M: Antoniu Miclaus <antoniu.miclaus@analog.com> L: linux-rtc@vger.kernel.org -- 2.51.2 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v5 1/4] dt-bindings: backlight: Add max25014 supporty 2025-11-07 12:49 ` [PATCH v5 1/4] dt-bindings: backlight: Add max25014 support Maud Spierings via B4 Relay @ 2025-11-07 15:35 ` Frank Li 2025-11-07 17:14 ` Conor Dooley 2025-11-10 7:59 ` Maud Spierings 2025-11-07 18:16 ` [PATCH v5 1/4] dt-bindings: backlight: Add max25014 support Conor Dooley 1 sibling, 2 replies; 19+ messages in thread From: Frank Li @ 2025-11-07 15:35 UTC (permalink / raw) To: maudspierings Cc: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Helge Deller, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Liam Girdwood, Mark Brown, dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, imx, linux-arm-kernel On Fri, Nov 07, 2025 at 01:49:58PM +0100, Maud Spierings via B4 Relay wrote: > From: Maud Spierings <maudspierings@gocontroll.com> > > The Maxim MAX25014 is a 4-channel automotive grade backlight driver IC > with integrated boost controller. > > Signed-off-by: Maud Spierings <maudspierings@gocontroll.com> > > --- > > In the current implementation the control registers for channel 1, > control all channels. So only one led subnode with led-sources is > supported right now. If at some point the driver functionality is > expanded the bindings can be easily extended with it. > --- > .../bindings/leds/backlight/maxim,max25014.yaml | 107 +++++++++++++++++++++ > MAINTAINERS | 5 + > 2 files changed, 112 insertions(+) > > diff --git a/Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml b/Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml > new file mode 100644 > index 000000000000..e83723224b07 > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml > @@ -0,0 +1,107 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/leds/backlight/maxim,max25014.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Maxim max25014 backlight controller > + > +maintainers: > + - Maud Spierings <maudspierings@gocontroll.com> > + > +properties: > + compatible: > + enum: > + - maxim,max25014 > + > + reg: > + maxItems: 1 > + > + "#address-cells": > + const: 1 > + > + "#size-cells": > + const: 0 > + > + enable-gpios: > + maxItems: 1 > + > + interrupts: > + maxItems: 1 > + > + power-supply: > + description: Regulator which controls the boost converter input rail. > + > + pwms: > + maxItems: 1 > + > + maxim,iset: > + $ref: /schemas/types.yaml#/definitions/uint32 > + maximum: 15 > + default: 11 > + description: > + Value of the ISET field in the ISET register. This controls the current > + scale of the outputs, a higher number means more current. > + > + led@0: > + type: object > + description: Properties for a string of connected LEDs. > + $ref: common.yaml# > + > + properties: > + reg: > + const: 0 If reg is const 0, why need use led@0? Frank > + > + led-sources: > + allOf: > + - minItems: 1 > + maxItems: 4 > + items: > + minimum: 0 > + maximum: 3 > + default: [0, 1, 2, 3] > + > + default-brightness: > + minimum: 0 > + maximum: 100 > + default: 50 > + > + required: > + - reg > + > + additionalProperties: false > + > +required: > + - compatible > + - reg > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/gpio/gpio.h> > + #include <dt-bindings/interrupt-controller/irq.h> > + > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + backlight@6f { > + compatible = "maxim,max25014"; > + reg = <0x6f>; > + #address-cells = <1>; > + #size-cells = <0>; > + enable-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>; > + interrupt-parent = <&gpio1>; > + interrupts = <2 IRQ_TYPE_EDGE_FALLING>; > + power-supply = <®_backlight>; > + pwms = <&pwm1>; > + maxim,iset = <7>; > + > + led@0 { > + reg = <0>; > + led-sources = <0 1 2 3>; > + default-brightness = <50>; > + }; > + }; > + }; > diff --git a/MAINTAINERS b/MAINTAINERS > index 58c7e3f678d8..606ce086f758 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -15261,6 +15261,11 @@ F: Documentation/userspace-api/media/drivers/max2175.rst > F: drivers/media/i2c/max2175* > F: include/uapi/linux/max2175.h > > +MAX25014 BACKLIGHT DRIVER > +M: Maud Spierings <maudspierings@gocontroll.com> > +S: Maintained > +F: Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml > + > MAX31335 RTC DRIVER > M: Antoniu Miclaus <antoniu.miclaus@analog.com> > L: linux-rtc@vger.kernel.org > > -- > 2.51.2 > > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v5 1/4] dt-bindings: backlight: Add max25014 supporty 2025-11-07 15:35 ` [PATCH v5 1/4] dt-bindings: backlight: Add max25014 supporty Frank Li @ 2025-11-07 17:14 ` Conor Dooley 2025-11-10 7:59 ` Maud Spierings 1 sibling, 0 replies; 19+ messages in thread From: Conor Dooley @ 2025-11-07 17:14 UTC (permalink / raw) To: Frank Li Cc: maudspierings, Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Helge Deller, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Liam Girdwood, Mark Brown, dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, imx, linux-arm-kernel [-- Attachment #1: Type: text/plain, Size: 914 bytes --] On Fri, Nov 07, 2025 at 10:35:04AM -0500, Frank Li wrote: > On Fri, Nov 07, 2025 at 01:49:58PM +0100, Maud Spierings via B4 Relay wrote: > > From: Maud Spierings <maudspierings@gocontroll.com> > > > > The Maxim MAX25014 is a 4-channel automotive grade backlight driver IC > > with integrated boost controller. > > > > Signed-off-by: Maud Spierings <maudspierings@gocontroll.com> > > + led@0: > > + type: object > > + description: Properties for a string of connected LEDs. > > + $ref: common.yaml# > > + > > + properties: > > + reg: > > + const: 0 > > If reg is const 0, why need use led@0? > > In the current implementation the control registers for channel 1, > > control all channels. So only one led subnode with led-sources is > > supported right now. If at some point the driver functionality is > > expanded the bindings can be easily extended with it. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v5 1/4] dt-bindings: backlight: Add max25014 supporty 2025-11-07 15:35 ` [PATCH v5 1/4] dt-bindings: backlight: Add max25014 supporty Frank Li 2025-11-07 17:14 ` Conor Dooley @ 2025-11-10 7:59 ` Maud Spierings 1 sibling, 0 replies; 19+ messages in thread From: Maud Spierings @ 2025-11-10 7:59 UTC (permalink / raw) To: Frank Li Cc: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Helge Deller, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Liam Girdwood, Mark Brown, dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, imx, linux-arm-kernel On 11/7/25 16:35, Frank Li wrote: > On Fri, Nov 07, 2025 at 01:49:58PM +0100, Maud Spierings via B4 Relay wrote: >> From: Maud Spierings <maudspierings@gocontroll.com> >> >> The Maxim MAX25014 is a 4-channel automotive grade backlight driver IC >> with integrated boost controller. >> >> Signed-off-by: Maud Spierings <maudspierings@gocontroll.com> >> >> --- >> >> In the current implementation the control registers for channel 1, >> control all channels. So only one led subnode with led-sources is >> supported right now. If at some point the driver functionality is >> expanded the bindings can be easily extended with it. >> --- >> .../bindings/leds/backlight/maxim,max25014.yaml | 107 +++++++++++++++++++++ >> MAINTAINERS | 5 + >> 2 files changed, 112 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml b/Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml >> new file mode 100644 >> index 000000000000..e83723224b07 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml >> @@ -0,0 +1,107 @@ >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/leds/backlight/maxim,max25014.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Maxim max25014 backlight controller >> + >> +maintainers: >> + - Maud Spierings <maudspierings@gocontroll.com> >> + >> +properties: >> + compatible: >> + enum: >> + - maxim,max25014 >> + >> + reg: >> + maxItems: 1 >> + >> + "#address-cells": >> + const: 1 >> + >> + "#size-cells": >> + const: 0 >> + >> + enable-gpios: >> + maxItems: 1 >> + >> + interrupts: >> + maxItems: 1 >> + >> + power-supply: >> + description: Regulator which controls the boost converter input rail. >> + >> + pwms: >> + maxItems: 1 >> + >> + maxim,iset: >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + maximum: 15 >> + default: 11 >> + description: >> + Value of the ISET field in the ISET register. This controls the current >> + scale of the outputs, a higher number means more current. >> + >> + led@0: >> + type: object >> + description: Properties for a string of connected LEDs. >> + $ref: common.yaml# >> + >> + properties: >> + reg: >> + const: 0 > > If reg is const 0, why need use led@0? > I made it this way so that at a later point the driver can be expanded with 4 led subnodes when that functionality gets supported. If I were to lock it to just led: right now I feel that may cause incompatibility later on. If there is a better way to do this, I'm open to suggestions. Kind regards, Maud > >> + >> + led-sources: >> + allOf: >> + - minItems: 1 >> + maxItems: 4 >> + items: >> + minimum: 0 >> + maximum: 3 >> + default: [0, 1, 2, 3] >> + >> + default-brightness: >> + minimum: 0 >> + maximum: 100 >> + default: 50 >> + >> + required: >> + - reg >> + >> + additionalProperties: false >> + >> +required: >> + - compatible >> + - reg >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + #include <dt-bindings/gpio/gpio.h> >> + #include <dt-bindings/interrupt-controller/irq.h> >> + >> + i2c { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + backlight@6f { >> + compatible = "maxim,max25014"; >> + reg = <0x6f>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> + enable-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>; >> + interrupt-parent = <&gpio1>; >> + interrupts = <2 IRQ_TYPE_EDGE_FALLING>; >> + power-supply = <®_backlight>; >> + pwms = <&pwm1>; >> + maxim,iset = <7>; >> + >> + led@0 { >> + reg = <0>; >> + led-sources = <0 1 2 3>; >> + default-brightness = <50>; >> + }; >> + }; >> + }; >> diff --git a/MAINTAINERS b/MAINTAINERS >> index 58c7e3f678d8..606ce086f758 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -15261,6 +15261,11 @@ F: Documentation/userspace-api/media/drivers/max2175.rst >> F: drivers/media/i2c/max2175* >> F: include/uapi/linux/max2175.h >> >> +MAX25014 BACKLIGHT DRIVER >> +M: Maud Spierings <maudspierings@gocontroll.com> >> +S: Maintained >> +F: Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml >> + >> MAX31335 RTC DRIVER >> M: Antoniu Miclaus <antoniu.miclaus@analog.com> >> L: linux-rtc@vger.kernel.org >> >> -- >> 2.51.2 >> >> ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v5 1/4] dt-bindings: backlight: Add max25014 support 2025-11-07 12:49 ` [PATCH v5 1/4] dt-bindings: backlight: Add max25014 support Maud Spierings via B4 Relay 2025-11-07 15:35 ` [PATCH v5 1/4] dt-bindings: backlight: Add max25014 supporty Frank Li @ 2025-11-07 18:16 ` Conor Dooley 2025-11-10 7:55 ` Maud Spierings 1 sibling, 1 reply; 19+ messages in thread From: Conor Dooley @ 2025-11-07 18:16 UTC (permalink / raw) To: maudspierings Cc: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Helge Deller, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Liam Girdwood, Mark Brown, dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, imx, linux-arm-kernel [-- Attachment #1: Type: text/plain, Size: 4739 bytes --] On Fri, Nov 07, 2025 at 01:49:58PM +0100, Maud Spierings via B4 Relay wrote: > From: Maud Spierings <maudspierings@gocontroll.com> > > The Maxim MAX25014 is a 4-channel automotive grade backlight driver IC > with integrated boost controller. > > Signed-off-by: Maud Spierings <maudspierings@gocontroll.com> > > --- > > In the current implementation the control registers for channel 1, > control all channels. So only one led subnode with led-sources is > supported right now. If at some point the driver functionality is > expanded the bindings can be easily extended with it. I'm sorry if I asked this before and forgot or w/e, but how backwards compatible is this? If they control all channels and it gets changed to only control channel one, how will a changed kernel understand the difference between a new devicetree that only wants to control channel 1 and an old devicetree that is trying to use channel 1 to control all channels? Cheers, Conor. > --- > .../bindings/leds/backlight/maxim,max25014.yaml | 107 +++++++++++++++++++++ > MAINTAINERS | 5 + > 2 files changed, 112 insertions(+) > > diff --git a/Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml b/Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml > new file mode 100644 > index 000000000000..e83723224b07 > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml > @@ -0,0 +1,107 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/leds/backlight/maxim,max25014.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Maxim max25014 backlight controller > + > +maintainers: > + - Maud Spierings <maudspierings@gocontroll.com> > + > +properties: > + compatible: > + enum: > + - maxim,max25014 > + > + reg: > + maxItems: 1 > + > + "#address-cells": > + const: 1 > + > + "#size-cells": > + const: 0 > + > + enable-gpios: > + maxItems: 1 > + > + interrupts: > + maxItems: 1 > + > + power-supply: > + description: Regulator which controls the boost converter input rail. > + > + pwms: > + maxItems: 1 > + > + maxim,iset: > + $ref: /schemas/types.yaml#/definitions/uint32 > + maximum: 15 > + default: 11 > + description: > + Value of the ISET field in the ISET register. This controls the current > + scale of the outputs, a higher number means more current. > + > + led@0: > + type: object > + description: Properties for a string of connected LEDs. > + $ref: common.yaml# > + > + properties: > + reg: > + const: 0 > + > + led-sources: > + allOf: > + - minItems: 1 > + maxItems: 4 > + items: > + minimum: 0 > + maximum: 3 > + default: [0, 1, 2, 3] > + > + default-brightness: > + minimum: 0 > + maximum: 100 > + default: 50 > + > + required: > + - reg > + > + additionalProperties: false > + > +required: > + - compatible > + - reg > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/gpio/gpio.h> > + #include <dt-bindings/interrupt-controller/irq.h> > + > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + backlight@6f { > + compatible = "maxim,max25014"; > + reg = <0x6f>; > + #address-cells = <1>; > + #size-cells = <0>; > + enable-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>; > + interrupt-parent = <&gpio1>; > + interrupts = <2 IRQ_TYPE_EDGE_FALLING>; > + power-supply = <®_backlight>; > + pwms = <&pwm1>; > + maxim,iset = <7>; > + > + led@0 { > + reg = <0>; > + led-sources = <0 1 2 3>; > + default-brightness = <50>; > + }; > + }; > + }; > diff --git a/MAINTAINERS b/MAINTAINERS > index 58c7e3f678d8..606ce086f758 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -15261,6 +15261,11 @@ F: Documentation/userspace-api/media/drivers/max2175.rst > F: drivers/media/i2c/max2175* > F: include/uapi/linux/max2175.h > > +MAX25014 BACKLIGHT DRIVER > +M: Maud Spierings <maudspierings@gocontroll.com> > +S: Maintained > +F: Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml > + > MAX31335 RTC DRIVER > M: Antoniu Miclaus <antoniu.miclaus@analog.com> > L: linux-rtc@vger.kernel.org > > -- > 2.51.2 > > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v5 1/4] dt-bindings: backlight: Add max25014 support 2025-11-07 18:16 ` [PATCH v5 1/4] dt-bindings: backlight: Add max25014 support Conor Dooley @ 2025-11-10 7:55 ` Maud Spierings 0 siblings, 0 replies; 19+ messages in thread From: Maud Spierings @ 2025-11-10 7:55 UTC (permalink / raw) To: Conor Dooley Cc: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Helge Deller, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Liam Girdwood, Mark Brown, dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, imx, linux-arm-kernel On 11/7/25 19:16, Conor Dooley wrote: > On Fri, Nov 07, 2025 at 01:49:58PM +0100, Maud Spierings via B4 Relay wrote: >> From: Maud Spierings <maudspierings@gocontroll.com> >> >> The Maxim MAX25014 is a 4-channel automotive grade backlight driver IC >> with integrated boost controller. >> >> Signed-off-by: Maud Spierings <maudspierings@gocontroll.com> >> >> --- >> >> In the current implementation the control registers for channel 1, >> control all channels. So only one led subnode with led-sources is >> supported right now. If at some point the driver functionality is >> expanded the bindings can be easily extended with it. > > I'm sorry if I asked this before and forgot or w/e, but how backwards > compatible is this? If they control all channels and it gets changed to > only control channel one, how will a changed kernel understand the > difference between a new devicetree that only wants to control channel 1 > and an old devicetree that is trying to use channel 1 to control all > channels? > The idea is that an implementation like that will add multiple led@ subnodes without the led-sources property. So devicetrees controlled by one channel will still have only one led with multiple sources in the devicetree. Kind regards, Maud > >> --- >> .../bindings/leds/backlight/maxim,max25014.yaml | 107 +++++++++++++++++++++ >> MAINTAINERS | 5 + >> 2 files changed, 112 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml b/Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml >> new file mode 100644 >> index 000000000000..e83723224b07 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml >> @@ -0,0 +1,107 @@ >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/leds/backlight/maxim,max25014.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Maxim max25014 backlight controller >> + >> +maintainers: >> + - Maud Spierings <maudspierings@gocontroll.com> >> + >> +properties: >> + compatible: >> + enum: >> + - maxim,max25014 >> + >> + reg: >> + maxItems: 1 >> + >> + "#address-cells": >> + const: 1 >> + >> + "#size-cells": >> + const: 0 >> + >> + enable-gpios: >> + maxItems: 1 >> + >> + interrupts: >> + maxItems: 1 >> + >> + power-supply: >> + description: Regulator which controls the boost converter input rail. >> + >> + pwms: >> + maxItems: 1 >> + >> + maxim,iset: >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + maximum: 15 >> + default: 11 >> + description: >> + Value of the ISET field in the ISET register. This controls the current >> + scale of the outputs, a higher number means more current. >> + >> + led@0: >> + type: object >> + description: Properties for a string of connected LEDs. >> + $ref: common.yaml# >> + >> + properties: >> + reg: >> + const: 0 >> + >> + led-sources: >> + allOf: >> + - minItems: 1 >> + maxItems: 4 >> + items: >> + minimum: 0 >> + maximum: 3 >> + default: [0, 1, 2, 3] >> + >> + default-brightness: >> + minimum: 0 >> + maximum: 100 >> + default: 50 >> + >> + required: >> + - reg >> + >> + additionalProperties: false >> + >> +required: >> + - compatible >> + - reg >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + #include <dt-bindings/gpio/gpio.h> >> + #include <dt-bindings/interrupt-controller/irq.h> >> + >> + i2c { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + backlight@6f { >> + compatible = "maxim,max25014"; >> + reg = <0x6f>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> + enable-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>; >> + interrupt-parent = <&gpio1>; >> + interrupts = <2 IRQ_TYPE_EDGE_FALLING>; >> + power-supply = <®_backlight>; >> + pwms = <&pwm1>; >> + maxim,iset = <7>; >> + >> + led@0 { >> + reg = <0>; >> + led-sources = <0 1 2 3>; >> + default-brightness = <50>; >> + }; >> + }; >> + }; >> diff --git a/MAINTAINERS b/MAINTAINERS >> index 58c7e3f678d8..606ce086f758 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -15261,6 +15261,11 @@ F: Documentation/userspace-api/media/drivers/max2175.rst >> F: drivers/media/i2c/max2175* >> F: include/uapi/linux/max2175.h >> >> +MAX25014 BACKLIGHT DRIVER >> +M: Maud Spierings <maudspierings@gocontroll.com> >> +S: Maintained >> +F: Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml >> + >> MAX31335 RTC DRIVER >> M: Antoniu Miclaus <antoniu.miclaus@analog.com> >> L: linux-rtc@vger.kernel.org >> >> -- >> 2.51.2 >> >> ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v5 2/4] backlight: add max25014atg backlight 2025-11-07 12:49 [PATCH v5 0/4] backlight: add new max25014 backlight driver Maud Spierings via B4 Relay 2025-11-07 12:49 ` [PATCH v5 1/4] dt-bindings: backlight: Add max25014 support Maud Spierings via B4 Relay @ 2025-11-07 12:49 ` Maud Spierings via B4 Relay 2025-11-07 15:51 ` [PATCH v5 2/4] backlight: add max25014atg backlighty Frank Li 2025-11-07 16:14 ` [PATCH v5 2/4] backlight: add max25014atg backlight Daniel Thompson 2025-11-07 12:50 ` [PATCH v5 3/4] arm64: dts: freescale: moduline-display-av101hdt-a10: add backlight Maud Spierings via B4 Relay 2025-11-07 12:50 ` [PATCH v5 4/4] arm64: dts: freescale: moduline-display-av123z7m-n17: " Maud Spierings via B4 Relay 3 siblings, 2 replies; 19+ messages in thread From: Maud Spierings via B4 Relay @ 2025-11-07 12:49 UTC (permalink / raw) To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Helge Deller, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Liam Girdwood, Mark Brown Cc: dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, imx, linux-arm-kernel, Maud Spierings From: Maud Spierings <maudspierings@gocontroll.com> The Maxim MAX25014 is a 4-channel automotive grade backlight driver IC with integrated boost controller. Signed-off-by: Maud Spierings <maudspierings@gocontroll.com> --- MAINTAINERS | 1 + drivers/video/backlight/Kconfig | 7 + drivers/video/backlight/Makefile | 1 + drivers/video/backlight/max25014.c | 409 +++++++++++++++++++++++++++++++++++++ 4 files changed, 418 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 606ce086f758..d082d3f8cfae 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -15265,6 +15265,7 @@ MAX25014 BACKLIGHT DRIVER M: Maud Spierings <maudspierings@gocontroll.com> S: Maintained F: Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml +F: drivers/video/backlight/max25014.c MAX31335 RTC DRIVER M: Antoniu Miclaus <antoniu.miclaus@analog.com> diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig index d9374d208cee..d3bb6ccd4185 100644 --- a/drivers/video/backlight/Kconfig +++ b/drivers/video/backlight/Kconfig @@ -262,6 +262,13 @@ config BACKLIGHT_DA9052 help Enable the Backlight Driver for DA9052-BC and DA9053-AA/Bx PMICs. +config BACKLIGHT_MAX25014 + tristate "Backlight driver for the Maxim MAX25014 chip" + depends on I2C + select REGMAP_I2C + help + If you are using a MAX25014 chip as a backlight driver say Y to enable it. + config BACKLIGHT_MAX8925 tristate "Backlight driver for MAX8925" depends on MFD_MAX8925 diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile index dfbb169bf6ea..1170d9ec40b8 100644 --- a/drivers/video/backlight/Makefile +++ b/drivers/video/backlight/Makefile @@ -45,6 +45,7 @@ obj-$(CONFIG_BACKLIGHT_LOCOMO) += locomolcd.o obj-$(CONFIG_BACKLIGHT_LP855X) += lp855x_bl.o obj-$(CONFIG_BACKLIGHT_LP8788) += lp8788_bl.o obj-$(CONFIG_BACKLIGHT_LV5207LP) += lv5207lp.o +obj-$(CONFIG_BACKLIGHT_MAX25014) += max25014.o obj-$(CONFIG_BACKLIGHT_MAX8925) += max8925_bl.o obj-$(CONFIG_BACKLIGHT_MP3309C) += mp3309c.o obj-$(CONFIG_BACKLIGHT_MT6370) += mt6370-backlight.o diff --git a/drivers/video/backlight/max25014.c b/drivers/video/backlight/max25014.c new file mode 100644 index 000000000000..36bae508697e --- /dev/null +++ b/drivers/video/backlight/max25014.c @@ -0,0 +1,409 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Backlight driver for Maxim MAX25014 + * + * Copyright (C) 2025 GOcontroll B.V. + * Author: Maud Spierings <maudspierings@gocontroll.com> + */ + +#include <linux/backlight.h> +#include <linux/gpio/consumer.h> +#include <linux/i2c.h> +#include <linux/regmap.h> +#include <linux/slab.h> + +#define MAX25014_ISET_DEFAULT_100 11 +#define MAX_BRIGHTNESS 100 +#define MIN_BRIGHTNESS 0 +#define TON_MAX 130720 /* @153Hz */ +#define TON_STEP 1307 /* @153Hz */ +#define TON_MIN 0 + +#define MAX25014_DEV_ID 0x00 +#define MAX25014_REV_ID 0x01 +#define MAX25014_ISET 0x02 +#define MAX25014_IMODE 0x03 +#define MAX25014_TON1H 0x04 +#define MAX25014_TON1L 0x05 +#define MAX25014_TON2H 0x06 +#define MAX25014_TON2L 0x07 +#define MAX25014_TON3H 0x08 +#define MAX25014_TON3L 0x09 +#define MAX25014_TON4H 0x0A +#define MAX25014_TON4L 0x0B +#define MAX25014_TON_1_4_LSB 0x0C +#define MAX25014_SETTING 0x12 +#define MAX25014_DISABLE 0x13 +#define MAX25014_BSTMON 0x14 +#define MAX25014_IOUT1 0x15 +#define MAX25014_IOUT2 0x16 +#define MAX25014_IOUT3 0x17 +#define MAX25014_IOUT4 0x18 +#define MAX25014_OPEN 0x1B +#define MAX25014_SHORT_GND 0x1C +#define MAX25014_SHORT_LED 0x1D +#define MAX25014_MASK 0x1E +#define MAX25014_DIAG 0x1F + +#define MAX25014_IMODE_HDIM BIT(2) +#define MAX25014_ISET_ENABLE BIT(5) +#define MAX25014_ISET_PSEN BIT(4) +#define MAX25014_DIAG_HW_RST BIT(2) +#define MAX25014_SETTING_FPWM GENMASK(6, 4) + +struct max25014 { + struct i2c_client *client; + struct backlight_device *bl; + struct regmap *regmap; + struct gpio_desc *enable; + struct regulator *vin; /* regulator for boost converter Vin rail */ + uint32_t iset; + uint8_t strings_mask; +}; + +static const struct regmap_config max25014_regmap_config = { + .reg_bits = 8, + .val_bits = 8, + .max_register = MAX25014_DIAG, +}; + +/** + * @brief control the brightness with i2c registers + * + * @param regmap trivial + * @param brt brightness + * @return int + */ +static int max25014_register_control(struct regmap *regmap, uint32_t brt) +{ + uint32_t reg = TON_STEP * brt; + int ret; + /* + * 18 bit number lowest, 2 bits in first register, + * next lowest 8 in the L register, next 8 in the H register + * Seemingly setting the strength of only one string controls all of + * them, individual settings don't affect the outcome. + */ + + ret = regmap_write(regmap, MAX25014_TON_1_4_LSB, reg & 0b00000011); + if (ret != 0) + return ret; + ret = regmap_write(regmap, MAX25014_TON1L, (reg >> 2) & 0b11111111); + if (ret != 0) + return ret; + return regmap_write(regmap, MAX25014_TON1H, (reg >> 10) & 0b11111111); +} + +static int max25014_check_errors(struct max25014 *maxim) +{ + uint8_t i; + int ret; + uint32_t val; + + ret = regmap_read(maxim->regmap, MAX25014_OPEN, &val); + if (ret != 0) + return ret; + if (val > 0) { + dev_err(&maxim->client->dev, "Open led strings detected on:\n"); + for (i = 0; i < 4; i++) { + if (val & 1 << i) + dev_err(&maxim->client->dev, "string %d\n", i + 1); + } + return -EIO; + } + + ret = regmap_read(maxim->regmap, MAX25014_SHORT_GND, &val); + if (ret != 0) + return ret; + if (val > 0) { + dev_err(&maxim->client->dev, "Short to ground detected on:\n"); + for (i = 0; i < 4; i++) { + if (val & 1 << i) + dev_err(&maxim->client->dev, "string %d\n", i + 1); + } + return -EIO; + } + + ret = regmap_read(maxim->regmap, MAX25014_SHORT_GND, &val); + if (ret != 0) + return ret; + if (val > 0) { + dev_err(&maxim->client->dev, "Shorted led detected on:\n"); + for (i = 0; i < 4; i++) { + if (val & 1 << i) + dev_err(&maxim->client->dev, "string %d\n", i + 1); + } + return -EIO; + } + + ret = regmap_read(maxim->regmap, MAX25014_DIAG, &val); + if (ret != 0) + return ret; + /* + * The HW_RST bit always starts at 1 after power up. + * It is reset on first read, does not indicate an error. + */ + if (val > 0 && val != MAX25014_DIAG_HW_RST) { + if (val & 0b1) + dev_err(&maxim->client->dev, + "Overtemperature shutdown\n"); + if (val & 0b10) + dev_err(&maxim->client->dev, + "Chip is getting too hot (>125C)\n"); + if (val & 0b1000) + dev_err(&maxim->client->dev, + "Boost converter overvoltage\n"); + if (val & 0b10000) + dev_err(&maxim->client->dev, + "Boost converter undervoltage\n"); + if (val & 0b100000) + dev_err(&maxim->client->dev, "IREF out of range\n"); + return -EIO; + } + return 0; +} + +/* + * 1. disable unused strings + * 2. set dim mode + * 3. set initial brightness + * 4. set setting register + * 5. enable the backlight + */ +static int max25014_configure(struct max25014 *maxim) +{ + int ret; + uint32_t val; + + ret = regmap_write(maxim->regmap, MAX25014_DISABLE, + maxim->strings_mask); + if (ret != 0) + return ret; + + ret = regmap_write(maxim->regmap, MAX25014_IMODE, MAX25014_IMODE_HDIM); + if (ret != 0) + return ret; + + ret = regmap_read(maxim->regmap, MAX25014_SETTING, &val); + if (ret != 0) + return ret; + + ret = regmap_write(maxim->regmap, MAX25014_SETTING, + val & ~MAX25014_SETTING_FPWM); + if (ret != 0) + return ret; + + ret = regmap_write(maxim->regmap, MAX25014_ISET, + maxim->iset | MAX25014_ISET_ENABLE | + MAX25014_ISET_PSEN); + return ret; +} + +static int max25014_update_status(struct backlight_device *bl_dev) +{ + struct max25014 *maxim = bl_get_data(bl_dev); + + if (backlight_is_blank(maxim->bl)) + bl_dev->props.brightness = 0; + + return max25014_register_control(maxim->regmap, + bl_dev->props.brightness); +} + +static const struct backlight_ops max25014_bl_ops = { + .options = BL_CORE_SUSPENDRESUME, + .update_status = max25014_update_status, +}; + +static int max25014_parse_dt(struct max25014 *maxim, + uint32_t *initial_brightness) +{ + struct device *dev = &maxim->client->dev; + struct device_node *node = dev->of_node; + struct fwnode_handle *child; + uint32_t strings[4]; + int res, i; + + if (!node) { + dev_err(dev, "no platform data\n"); + return -EINVAL; + } + + child = device_get_next_child_node(dev, NULL); + if (child) { + res = fwnode_property_count_u32(child, "led-sources"); + if (res > 0) { + fwnode_property_read_u32_array(child, "led-sources", + strings, res); + + /* set all strings as disabled, then enable those in led-sources*/ + maxim->strings_mask = 0xf; + for (i = 0; i < res; i++) { + if (strings[i] <= 4) + maxim->strings_mask &= ~BIT(strings[i]); + } + } + + fwnode_property_read_u32(child, "default-brightness", + initial_brightness); + + fwnode_handle_put(child); + } + + maxim->iset = MAX25014_ISET_DEFAULT_100; + of_property_read_u32(node, "maxim,iset", &maxim->iset); + + if (maxim->iset < 0 || maxim->iset > 15) { + dev_err(dev, + "Invalid iset, should be a value from 0-15, entered was %d\n", + maxim->iset); + return -EINVAL; + } + + if (*initial_brightness < 0 || *initial_brightness > 100) { + dev_err(dev, + "Invalid initial brightness, should be a value from 0-100, entered was %d\n", + *initial_brightness); + return -EINVAL; + } + + return 0; +} + +static int max25014_probe(struct i2c_client *cl) +{ + struct backlight_device *bl; + const struct i2c_device_id *id = i2c_client_get_device_id(cl); + struct max25014 *maxim; + struct backlight_properties props; + int ret; + uint32_t initial_brightness = 50; + + maxim = devm_kzalloc(&cl->dev, sizeof(struct max25014), GFP_KERNEL); + if (!maxim) + return -ENOMEM; + + maxim->client = cl; + + ret = max25014_parse_dt(maxim, &initial_brightness); + if (ret < 0) + return ret; + + maxim->vin = devm_regulator_get_optional(&maxim->client->dev, "power"); + if (IS_ERR(maxim->vin)) { + if (PTR_ERR(maxim->vin) == -EPROBE_DEFER) + return -EPROBE_DEFER; + maxim->vin = NULL; + } + + if (maxim->vin) { + ret = regulator_enable(maxim->vin); + if (ret < 0) { + dev_err(&maxim->client->dev, + "failed to enable Vin: %d\n", ret); + return ret; + } + } + + maxim->enable = devm_gpiod_get_optional(&maxim->client->dev, "enable", + GPIOD_ASIS); + if (IS_ERR(maxim->enable)) { + ret = PTR_ERR(maxim->enable); + dev_err(&maxim->client->dev, "failed to get enable gpio: %d\n", + ret); + goto disable_vin; + } + + if (maxim->enable) + gpiod_set_value_cansleep(maxim->enable, 1); + + /* Enable can be tied to vin rail wait if either is available */ + if (maxim->enable || maxim->vin) { + /* Datasheet Electrical Characteristics tSTARTUP 2ms */ + usleep_range(2000, 2500); + } + + maxim->regmap = devm_regmap_init_i2c(cl, &max25014_regmap_config); + if (IS_ERR(maxim->regmap)) { + ret = PTR_ERR(maxim->regmap); + dev_err(&maxim->client->dev, + "failed to initialize the i2c regmap: %d\n", ret); + goto disable_full; + } + + i2c_set_clientdata(cl, maxim); + + ret = max25014_check_errors(maxim); + if (ret) { /* error is already reported in the above function */ + goto disable_full; + } + + ret = max25014_configure(maxim); + if (ret) { + dev_err(&maxim->client->dev, "device config err: %d", ret); + goto disable_full; + } + + memset(&props, 0, sizeof(props)); + props.type = BACKLIGHT_PLATFORM; + props.max_brightness = MAX_BRIGHTNESS; + props.brightness = initial_brightness; + props.scale = BACKLIGHT_SCALE_LINEAR; + + bl = devm_backlight_device_register(&maxim->client->dev, id->name, + &maxim->client->dev, maxim, + &max25014_bl_ops, &props); + if (IS_ERR(bl)) + return PTR_ERR(bl); + + maxim->bl = bl; + + return 0; + +disable_full: + if (maxim->enable) + gpiod_set_value_cansleep(maxim->enable, 0); +disable_vin: + if (maxim->vin) + regulator_disable(maxim->vin); + return ret; +} + +static void max25014_remove(struct i2c_client *cl) +{ + struct max25014 *maxim = i2c_get_clientdata(cl); + + maxim->bl->props.brightness = 0; + max25014_update_status(maxim->bl); + if (maxim->enable) + gpiod_set_value_cansleep(maxim->enable, 0); + if (maxim->vin) + regulator_disable(maxim->vin); +} + +static const struct of_device_id max25014_dt_ids[] = { + { .compatible = "maxim,max25014", }, + { } +}; +MODULE_DEVICE_TABLE(of, max25014_dt_ids); + +static const struct i2c_device_id max25014_ids[] = { + { "max25014" }, + { } +}; +MODULE_DEVICE_TABLE(i2c, max25014_ids); + +static struct i2c_driver max25014_driver = { + .driver = { + .name = KBUILD_MODNAME, + .of_match_table = of_match_ptr(max25014_dt_ids), + }, + .probe = max25014_probe, + .remove = max25014_remove, + .id_table = max25014_ids, +}; +module_i2c_driver(max25014_driver); + +MODULE_DESCRIPTION("Maxim MAX25014 backlight driver"); +MODULE_AUTHOR("Maud Spierings <maudspierings@gocontroll.com>"); +MODULE_LICENSE("GPL"); -- 2.51.2 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v5 2/4] backlight: add max25014atg backlighty 2025-11-07 12:49 ` [PATCH v5 2/4] backlight: add max25014atg backlight Maud Spierings via B4 Relay @ 2025-11-07 15:51 ` Frank Li 2025-11-07 16:22 ` Daniel Thompson 2025-11-10 8:25 ` Maud Spierings 2025-11-07 16:14 ` [PATCH v5 2/4] backlight: add max25014atg backlight Daniel Thompson 1 sibling, 2 replies; 19+ messages in thread From: Frank Li @ 2025-11-07 15:51 UTC (permalink / raw) To: maudspierings Cc: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Helge Deller, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Liam Girdwood, Mark Brown, dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, imx, linux-arm-kernel On Fri, Nov 07, 2025 at 01:49:59PM +0100, Maud Spierings via B4 Relay wrote: > From: Maud Spierings <maudspierings@gocontroll.com> > > The Maxim MAX25014 is a 4-channel automotive grade backlight driver IC > with integrated boost controller. > > Signed-off-by: Maud Spierings <maudspierings@gocontroll.com> > --- > MAINTAINERS | 1 + > drivers/video/backlight/Kconfig | 7 + > drivers/video/backlight/Makefile | 1 + > drivers/video/backlight/max25014.c | 409 +++++++++++++++++++++++++++++++++++++ > 4 files changed, 418 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 606ce086f758..d082d3f8cfae 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -15265,6 +15265,7 @@ MAX25014 BACKLIGHT DRIVER > M: Maud Spierings <maudspierings@gocontroll.com> > S: Maintained > F: Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml > +F: drivers/video/backlight/max25014.c > > MAX31335 RTC DRIVER > M: Antoniu Miclaus <antoniu.miclaus@analog.com> > diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig > index d9374d208cee..d3bb6ccd4185 100644 > --- a/drivers/video/backlight/Kconfig > +++ b/drivers/video/backlight/Kconfig > @@ -262,6 +262,13 @@ config BACKLIGHT_DA9052 > help > Enable the Backlight Driver for DA9052-BC and DA9053-AA/Bx PMICs. > > +config BACKLIGHT_MAX25014 > + tristate "Backlight driver for the Maxim MAX25014 chip" > + depends on I2C > + select REGMAP_I2C > + help > + If you are using a MAX25014 chip as a backlight driver say Y to enable it. > + > config BACKLIGHT_MAX8925 > tristate "Backlight driver for MAX8925" > depends on MFD_MAX8925 > diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile > index dfbb169bf6ea..1170d9ec40b8 100644 > --- a/drivers/video/backlight/Makefile > +++ b/drivers/video/backlight/Makefile > @@ -45,6 +45,7 @@ obj-$(CONFIG_BACKLIGHT_LOCOMO) += locomolcd.o > obj-$(CONFIG_BACKLIGHT_LP855X) += lp855x_bl.o > obj-$(CONFIG_BACKLIGHT_LP8788) += lp8788_bl.o > obj-$(CONFIG_BACKLIGHT_LV5207LP) += lv5207lp.o > +obj-$(CONFIG_BACKLIGHT_MAX25014) += max25014.o > obj-$(CONFIG_BACKLIGHT_MAX8925) += max8925_bl.o > obj-$(CONFIG_BACKLIGHT_MP3309C) += mp3309c.o > obj-$(CONFIG_BACKLIGHT_MT6370) += mt6370-backlight.o > diff --git a/drivers/video/backlight/max25014.c b/drivers/video/backlight/max25014.c > new file mode 100644 > index 000000000000..36bae508697e > --- /dev/null ... > + > +struct max25014 { > + struct i2c_client *client; > + struct backlight_device *bl; > + struct regmap *regmap; > + struct gpio_desc *enable; > + struct regulator *vin; /* regulator for boost converter Vin rail */ > + uint32_t iset; > + uint8_t strings_mask; > +}; > + > +static const struct regmap_config max25014_regmap_config = { > + .reg_bits = 8, > + .val_bits = 8, > + .max_register = MAX25014_DIAG, > +}; > + > +/** > + * @brief control the brightness with i2c registers > + * > + * @param regmap trivial > + * @param brt brightness > + * @return int > + */ > +static int max25014_register_control(struct regmap *regmap, uint32_t brt) > +{ > + uint32_t reg = TON_STEP * brt; > + int ret; > + /* > + * 18 bit number lowest, 2 bits in first register, > + * next lowest 8 in the L register, next 8 in the H register > + * Seemingly setting the strength of only one string controls all of > + * them, individual settings don't affect the outcome. > + */ > + > + ret = regmap_write(regmap, MAX25014_TON_1_4_LSB, reg & 0b00000011); > + if (ret != 0) if (ret), check others regmap_*() > + return ret; > + ret = regmap_write(regmap, MAX25014_TON1L, (reg >> 2) & 0b11111111); > + if (ret != 0) > + return ret; > + return regmap_write(regmap, MAX25014_TON1H, (reg >> 10) & 0b11111111); > +} > + > +static int max25014_check_errors(struct max25014 *maxim) > +{ > + uint8_t i; > + int ret; > + uint32_t val; > + > + ret = regmap_read(maxim->regmap, MAX25014_OPEN, &val); > + if (ret != 0) > + return ret; > + if (val > 0) { uint32 always >= 0 So if (val) > + dev_err(&maxim->client->dev, "Open led strings detected on:\n"); > + for (i = 0; i < 4; i++) { > + if (val & 1 << i) > + dev_err(&maxim->client->dev, "string %d\n", i + 1); > + } > + return -EIO; > + } > + > + ret = regmap_read(maxim->regmap, MAX25014_SHORT_GND, &val); > + if (ret != 0) > + return ret; > + if (val > 0) { > + dev_err(&maxim->client->dev, "Short to ground detected on:\n"); > + for (i = 0; i < 4; i++) { > + if (val & 1 << i) > + dev_err(&maxim->client->dev, "string %d\n", i + 1); > + } > + return -EIO; > + } > + > + ret = regmap_read(maxim->regmap, MAX25014_SHORT_GND, &val); > + if (ret != 0) > + return ret; > + if (val > 0) { > + dev_err(&maxim->client->dev, "Shorted led detected on:\n"); > + for (i = 0; i < 4; i++) { > + if (val & 1 << i) > + dev_err(&maxim->client->dev, "string %d\n", i + 1); > + } > + return -EIO; > + } > + > + ret = regmap_read(maxim->regmap, MAX25014_DIAG, &val); > + if (ret != 0) > + return ret; > + /* > + * The HW_RST bit always starts at 1 after power up. > + * It is reset on first read, does not indicate an error. > + */ > + if (val > 0 && val != MAX25014_DIAG_HW_RST) { > + if (val & 0b1) BIT(0) > + dev_err(&maxim->client->dev, > + "Overtemperature shutdown\n"); > + if (val & 0b10) > + dev_err(&maxim->client->dev, > + "Chip is getting too hot (>125C)\n"); > + if (val & 0b1000) > + dev_err(&maxim->client->dev, > + "Boost converter overvoltage\n"); > + if (val & 0b10000) > + dev_err(&maxim->client->dev, > + "Boost converter undervoltage\n"); > + if (val & 0b100000) > + dev_err(&maxim->client->dev, "IREF out of range\n"); > + return -EIO; > + } > + return 0; > +} > + ... > +static int max25014_parse_dt(struct max25014 *maxim, > + uint32_t *initial_brightness) > +{ > + struct device *dev = &maxim->client->dev; > + struct device_node *node = dev->of_node; > + struct fwnode_handle *child; > + uint32_t strings[4]; > + int res, i; > + > + if (!node) { > + dev_err(dev, "no platform data\n"); > + return -EINVAL; > + } call from probe, check other place return dev_err_probe() > + > + child = device_get_next_child_node(dev, NULL); > + if (child) { > + res = fwnode_property_count_u32(child, "led-sources"); > + if (res > 0) { > + fwnode_property_read_u32_array(child, "led-sources", > + strings, res); > + > + /* set all strings as disabled, then enable those in led-sources*/ > + maxim->strings_mask = 0xf; > + for (i = 0; i < res; i++) { > + if (strings[i] <= 4) > + maxim->strings_mask &= ~BIT(strings[i]); > + } > + } > + > + fwnode_property_read_u32(child, "default-brightness", > + initial_brightness); > + > + fwnode_handle_put(child); > + } > + > + maxim->iset = MAX25014_ISET_DEFAULT_100; > + of_property_read_u32(node, "maxim,iset", &maxim->iset); > + > + if (maxim->iset < 0 || maxim->iset > 15) { > + dev_err(dev, > + "Invalid iset, should be a value from 0-15, entered was %d\n", > + maxim->iset); > + return -EINVAL; > + } > + > + if (*initial_brightness < 0 || *initial_brightness > 100) { > + dev_err(dev, > + "Invalid initial brightness, should be a value from 0-100, entered was %d\n", > + *initial_brightness); > + return -EINVAL; > + } > + > + return 0; > +} > + > +static int max25014_probe(struct i2c_client *cl) > +{ > + struct backlight_device *bl; > + const struct i2c_device_id *id = i2c_client_get_device_id(cl); > + struct max25014 *maxim; > + struct backlight_properties props; > + int ret; > + uint32_t initial_brightness = 50; try keep reverise christmas order > + > + maxim = devm_kzalloc(&cl->dev, sizeof(struct max25014), GFP_KERNEL); > + if (!maxim) > + return -ENOMEM; > + > + maxim->client = cl; > + > + ret = max25014_parse_dt(maxim, &initial_brightness); > + if (ret < 0) > + return ret; > + > + maxim->vin = devm_regulator_get_optional(&maxim->client->dev, "power"); > + if (IS_ERR(maxim->vin)) { > + if (PTR_ERR(maxim->vin) == -EPROBE_DEFER) > + return -EPROBE_DEFER; > + maxim->vin = NULL; > + } > + > + if (maxim->vin) { > + ret = regulator_enable(maxim->vin); > + if (ret < 0) { > + dev_err(&maxim->client->dev, > + "failed to enable Vin: %d\n", ret); > + return ret; > + } > + } use devm_regulator_get_enable_optional() to combine devm_regulator_get_optional() and regulator_enable() to one call. > + > + maxim->enable = devm_gpiod_get_optional(&maxim->client->dev, "enable", > + GPIOD_ASIS); > + if (IS_ERR(maxim->enable)) { > + ret = PTR_ERR(maxim->enable); > + dev_err(&maxim->client->dev, "failed to get enable gpio: %d\n", > + ret); > + goto disable_vin; > + } > + > + if (maxim->enable) > + gpiod_set_value_cansleep(maxim->enable, 1); gpiod_set_value_cansleep() tolerate NULL, so needn't if check here and if you pass GPIOD_OUT_HIGH at devm_gpiod_get_optional(), needn't call this function. > + > + /* Enable can be tied to vin rail wait if either is available */ > + if (maxim->enable || maxim->vin) { > + /* Datasheet Electrical Characteristics tSTARTUP 2ms */ > + usleep_range(2000, 2500); now perfer use fsleep() Frank > + } > + > + maxim->regmap = devm_regmap_init_i2c(cl, &max25014_regmap_config); > + if (IS_ERR(maxim->regmap)) { > + ret = PTR_ERR(maxim->regmap); > + dev_err(&maxim->client->dev, > + "failed to initialize the i2c regmap: %d\n", ret); > + goto disable_full; > + } > + > + i2c_set_clientdata(cl, maxim); > + > + ret = max25014_check_errors(maxim); > + if (ret) { /* error is already reported in the above function */ > + goto disable_full; > + } > + > + ret = max25014_configure(maxim); > + if (ret) { > + dev_err(&maxim->client->dev, "device config err: %d", ret); > + goto disable_full; > + } > + > + memset(&props, 0, sizeof(props)); > + props.type = BACKLIGHT_PLATFORM; > + props.max_brightness = MAX_BRIGHTNESS; > + props.brightness = initial_brightness; > + props.scale = BACKLIGHT_SCALE_LINEAR; > + > + bl = devm_backlight_device_register(&maxim->client->dev, id->name, > + &maxim->client->dev, maxim, > + &max25014_bl_ops, &props); > + if (IS_ERR(bl)) > + return PTR_ERR(bl); > + > + maxim->bl = bl; > + > + return 0; > + > +disable_full: > + if (maxim->enable) > + gpiod_set_value_cansleep(maxim->enable, 0); > +disable_vin: > + if (maxim->vin) > + regulator_disable(maxim->vin); > + return ret; > +} > + > +static void max25014_remove(struct i2c_client *cl) > +{ > + struct max25014 *maxim = i2c_get_clientdata(cl); > + > + maxim->bl->props.brightness = 0; > + max25014_update_status(maxim->bl); > + if (maxim->enable) > + gpiod_set_value_cansleep(maxim->enable, 0); > + if (maxim->vin) > + regulator_disable(maxim->vin); > +} > + > +static const struct of_device_id max25014_dt_ids[] = { > + { .compatible = "maxim,max25014", }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, max25014_dt_ids); > + > +static const struct i2c_device_id max25014_ids[] = { > + { "max25014" }, > + { } > +}; > +MODULE_DEVICE_TABLE(i2c, max25014_ids); > + > +static struct i2c_driver max25014_driver = { > + .driver = { > + .name = KBUILD_MODNAME, > + .of_match_table = of_match_ptr(max25014_dt_ids), > + }, > + .probe = max25014_probe, > + .remove = max25014_remove, > + .id_table = max25014_ids, > +}; > +module_i2c_driver(max25014_driver); > + > +MODULE_DESCRIPTION("Maxim MAX25014 backlight driver"); > +MODULE_AUTHOR("Maud Spierings <maudspierings@gocontroll.com>"); > +MODULE_LICENSE("GPL"); > > -- > 2.51.2 > > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v5 2/4] backlight: add max25014atg backlighty 2025-11-07 15:51 ` [PATCH v5 2/4] backlight: add max25014atg backlighty Frank Li @ 2025-11-07 16:22 ` Daniel Thompson 2025-11-07 20:42 ` Frank Li 2025-11-10 8:25 ` Maud Spierings 1 sibling, 1 reply; 19+ messages in thread From: Daniel Thompson @ 2025-11-07 16:22 UTC (permalink / raw) To: Frank Li Cc: maudspierings, Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Helge Deller, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Liam Girdwood, Mark Brown, dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, imx, linux-arm-kernel On Fri, Nov 07, 2025 at 10:51:11AM -0500, Frank Li wrote: > On Fri, Nov 07, 2025 at 01:49:59PM +0100, Maud Spierings via B4 Relay wrote: > > From: Maud Spierings <maudspierings@gocontroll.com> > > > > The Maxim MAX25014 is a 4-channel automotive grade backlight driver IC > > with integrated boost controller. > > > > Signed-off-by: Maud Spierings <maudspierings@gocontroll.com> <snip> > > +static int max25014_probe(struct i2c_client *cl) > > +{ > > + struct backlight_device *bl; > > + const struct i2c_device_id *id = i2c_client_get_device_id(cl); > > + struct max25014 *maxim; > > + struct backlight_properties props; > > + int ret; > > + uint32_t initial_brightness = 50; > > try keep reverise christmas order I thought reverse christmas tree order only applied to code where the maintainers called it out in Development/process (e.g. netdev and tip). Daniel. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v5 2/4] backlight: add max25014atg backlighty 2025-11-07 16:22 ` Daniel Thompson @ 2025-11-07 20:42 ` Frank Li 0 siblings, 0 replies; 19+ messages in thread From: Frank Li @ 2025-11-07 20:42 UTC (permalink / raw) To: Daniel Thompson Cc: maudspierings, Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Helge Deller, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Liam Girdwood, Mark Brown, dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, imx, linux-arm-kernel On Fri, Nov 07, 2025 at 04:22:33PM +0000, Daniel Thompson wrote: > On Fri, Nov 07, 2025 at 10:51:11AM -0500, Frank Li wrote: > > On Fri, Nov 07, 2025 at 01:49:59PM +0100, Maud Spierings via B4 Relay wrote: > > > From: Maud Spierings <maudspierings@gocontroll.com> > > > > > > The Maxim MAX25014 is a 4-channel automotive grade backlight driver IC > > > with integrated boost controller. > > > > > > Signed-off-by: Maud Spierings <maudspierings@gocontroll.com> > > <snip> > > > > +static int max25014_probe(struct i2c_client *cl) > > > +{ > > > + struct backlight_device *bl; > > > + const struct i2c_device_id *id = i2c_client_get_device_id(cl); > > > + struct max25014 *maxim; > > > + struct backlight_properties props; > > > + int ret; > > > + uint32_t initial_brightness = 50; > > > > try keep reverise christmas order > > I thought reverse christmas tree order only applied to code where the > maintainers called it out in Development/process (e.g. netdev and tip). But, but it is small change when you update next version. Frank > > > Daniel. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v5 2/4] backlight: add max25014atg backlighty 2025-11-07 15:51 ` [PATCH v5 2/4] backlight: add max25014atg backlighty Frank Li 2025-11-07 16:22 ` Daniel Thompson @ 2025-11-10 8:25 ` Maud Spierings 1 sibling, 0 replies; 19+ messages in thread From: Maud Spierings @ 2025-11-10 8:25 UTC (permalink / raw) To: Frank Li Cc: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Helge Deller, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Liam Girdwood, Mark Brown, dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, imx, linux-arm-kernel On 11/7/25 16:51, Frank Li wrote: > On Fri, Nov 07, 2025 at 01:49:59PM +0100, Maud Spierings via B4 Relay wrote: >> From: Maud Spierings <maudspierings@gocontroll.com> >> >> The Maxim MAX25014 is a 4-channel automotive grade backlight driver IC >> with integrated boost controller. >> >> Signed-off-by: Maud Spierings <maudspierings@gocontroll.com> >> --- >> MAINTAINERS | 1 + >> drivers/video/backlight/Kconfig | 7 + >> drivers/video/backlight/Makefile | 1 + >> drivers/video/backlight/max25014.c | 409 +++++++++++++++++++++++++++++++++++++ >> 4 files changed, 418 insertions(+) >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index 606ce086f758..d082d3f8cfae 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -15265,6 +15265,7 @@ MAX25014 BACKLIGHT DRIVER >> M: Maud Spierings <maudspierings@gocontroll.com> >> S: Maintained >> F: Documentation/devicetree/bindings/leds/backlight/maxim,max25014.yaml >> +F: drivers/video/backlight/max25014.c >> >> MAX31335 RTC DRIVER >> M: Antoniu Miclaus <antoniu.miclaus@analog.com> >> diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig >> index d9374d208cee..d3bb6ccd4185 100644 >> --- a/drivers/video/backlight/Kconfig >> +++ b/drivers/video/backlight/Kconfig >> @@ -262,6 +262,13 @@ config BACKLIGHT_DA9052 >> help >> Enable the Backlight Driver for DA9052-BC and DA9053-AA/Bx PMICs. >> >> +config BACKLIGHT_MAX25014 >> + tristate "Backlight driver for the Maxim MAX25014 chip" >> + depends on I2C >> + select REGMAP_I2C >> + help >> + If you are using a MAX25014 chip as a backlight driver say Y to enable it. >> + >> config BACKLIGHT_MAX8925 >> tristate "Backlight driver for MAX8925" >> depends on MFD_MAX8925 >> diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile >> index dfbb169bf6ea..1170d9ec40b8 100644 >> --- a/drivers/video/backlight/Makefile >> +++ b/drivers/video/backlight/Makefile >> @@ -45,6 +45,7 @@ obj-$(CONFIG_BACKLIGHT_LOCOMO) += locomolcd.o >> obj-$(CONFIG_BACKLIGHT_LP855X) += lp855x_bl.o >> obj-$(CONFIG_BACKLIGHT_LP8788) += lp8788_bl.o >> obj-$(CONFIG_BACKLIGHT_LV5207LP) += lv5207lp.o >> +obj-$(CONFIG_BACKLIGHT_MAX25014) += max25014.o >> obj-$(CONFIG_BACKLIGHT_MAX8925) += max8925_bl.o >> obj-$(CONFIG_BACKLIGHT_MP3309C) += mp3309c.o >> obj-$(CONFIG_BACKLIGHT_MT6370) += mt6370-backlight.o >> diff --git a/drivers/video/backlight/max25014.c b/drivers/video/backlight/max25014.c >> new file mode 100644 >> index 000000000000..36bae508697e >> --- /dev/null > ... >> + >> +struct max25014 { >> + struct i2c_client *client; >> + struct backlight_device *bl; >> + struct regmap *regmap; >> + struct gpio_desc *enable; >> + struct regulator *vin; /* regulator for boost converter Vin rail */ >> + uint32_t iset; >> + uint8_t strings_mask; >> +}; >> + >> +static const struct regmap_config max25014_regmap_config = { >> + .reg_bits = 8, >> + .val_bits = 8, >> + .max_register = MAX25014_DIAG, >> +}; >> + >> +/** >> + * @brief control the brightness with i2c registers >> + * >> + * @param regmap trivial >> + * @param brt brightness >> + * @return int >> + */ >> +static int max25014_register_control(struct regmap *regmap, uint32_t brt) >> +{ >> + uint32_t reg = TON_STEP * brt; >> + int ret; >> + /* >> + * 18 bit number lowest, 2 bits in first register, >> + * next lowest 8 in the L register, next 8 in the H register >> + * Seemingly setting the strength of only one string controls all of >> + * them, individual settings don't affect the outcome. >> + */ >> + >> + ret = regmap_write(regmap, MAX25014_TON_1_4_LSB, reg & 0b00000011); >> + if (ret != 0) > > if (ret), check others regmap_*() > >> + return ret; >> + ret = regmap_write(regmap, MAX25014_TON1L, (reg >> 2) & 0b11111111); >> + if (ret != 0) >> + return ret; >> + return regmap_write(regmap, MAX25014_TON1H, (reg >> 10) & 0b11111111); >> +} >> + >> +static int max25014_check_errors(struct max25014 *maxim) >> +{ >> + uint8_t i; >> + int ret; >> + uint32_t val; >> + >> + ret = regmap_read(maxim->regmap, MAX25014_OPEN, &val); >> + if (ret != 0) >> + return ret; >> + if (val > 0) { > > uint32 always >= 0 > > So > if (val) > >> + dev_err(&maxim->client->dev, "Open led strings detected on:\n"); >> + for (i = 0; i < 4; i++) { >> + if (val & 1 << i) >> + dev_err(&maxim->client->dev, "string %d\n", i + 1); >> + } >> + return -EIO; >> + } >> + >> + ret = regmap_read(maxim->regmap, MAX25014_SHORT_GND, &val); >> + if (ret != 0) >> + return ret; >> + if (val > 0) { >> + dev_err(&maxim->client->dev, "Short to ground detected on:\n"); >> + for (i = 0; i < 4; i++) { >> + if (val & 1 << i) >> + dev_err(&maxim->client->dev, "string %d\n", i + 1); >> + } >> + return -EIO; >> + } >> + >> + ret = regmap_read(maxim->regmap, MAX25014_SHORT_GND, &val); >> + if (ret != 0) >> + return ret; >> + if (val > 0) { >> + dev_err(&maxim->client->dev, "Shorted led detected on:\n"); >> + for (i = 0; i < 4; i++) { >> + if (val & 1 << i) >> + dev_err(&maxim->client->dev, "string %d\n", i + 1); >> + } >> + return -EIO; >> + } >> + >> + ret = regmap_read(maxim->regmap, MAX25014_DIAG, &val); >> + if (ret != 0) >> + return ret; >> + /* >> + * The HW_RST bit always starts at 1 after power up. >> + * It is reset on first read, does not indicate an error. >> + */ >> + if (val > 0 && val != MAX25014_DIAG_HW_RST) { >> + if (val & 0b1) > > BIT(0) > >> + dev_err(&maxim->client->dev, >> + "Overtemperature shutdown\n"); >> + if (val & 0b10) >> + dev_err(&maxim->client->dev, >> + "Chip is getting too hot (>125C)\n"); >> + if (val & 0b1000) >> + dev_err(&maxim->client->dev, >> + "Boost converter overvoltage\n"); >> + if (val & 0b10000) >> + dev_err(&maxim->client->dev, >> + "Boost converter undervoltage\n"); >> + if (val & 0b100000) >> + dev_err(&maxim->client->dev, "IREF out of range\n"); >> + return -EIO; >> + } >> + return 0; >> +} >> + > ... >> +static int max25014_parse_dt(struct max25014 *maxim, >> + uint32_t *initial_brightness) >> +{ >> + struct device *dev = &maxim->client->dev; >> + struct device_node *node = dev->of_node; >> + struct fwnode_handle *child; >> + uint32_t strings[4]; >> + int res, i; >> + >> + if (!node) { >> + dev_err(dev, "no platform data\n"); >> + return -EINVAL; >> + } > > call from probe, check other place > > return dev_err_probe() > >> + >> + child = device_get_next_child_node(dev, NULL); >> + if (child) { >> + res = fwnode_property_count_u32(child, "led-sources"); >> + if (res > 0) { >> + fwnode_property_read_u32_array(child, "led-sources", >> + strings, res); >> + >> + /* set all strings as disabled, then enable those in led-sources*/ >> + maxim->strings_mask = 0xf; >> + for (i = 0; i < res; i++) { >> + if (strings[i] <= 4) >> + maxim->strings_mask &= ~BIT(strings[i]); >> + } >> + } >> + >> + fwnode_property_read_u32(child, "default-brightness", >> + initial_brightness); >> + >> + fwnode_handle_put(child); >> + } >> + >> + maxim->iset = MAX25014_ISET_DEFAULT_100; >> + of_property_read_u32(node, "maxim,iset", &maxim->iset); >> + >> + if (maxim->iset < 0 || maxim->iset > 15) { >> + dev_err(dev, >> + "Invalid iset, should be a value from 0-15, entered was %d\n", >> + maxim->iset); >> + return -EINVAL; >> + } >> + >> + if (*initial_brightness < 0 || *initial_brightness > 100) { >> + dev_err(dev, >> + "Invalid initial brightness, should be a value from 0-100, entered was %d\n", >> + *initial_brightness); >> + return -EINVAL; >> + } >> + >> + return 0; >> +} >> + >> +static int max25014_probe(struct i2c_client *cl) >> +{ >> + struct backlight_device *bl; >> + const struct i2c_device_id *id = i2c_client_get_device_id(cl); >> + struct max25014 *maxim; >> + struct backlight_properties props; >> + int ret; >> + uint32_t initial_brightness = 50; > > try keep reverise christmas order > >> + >> + maxim = devm_kzalloc(&cl->dev, sizeof(struct max25014), GFP_KERNEL); >> + if (!maxim) >> + return -ENOMEM; >> + >> + maxim->client = cl; >> + >> + ret = max25014_parse_dt(maxim, &initial_brightness); >> + if (ret < 0) >> + return ret; >> + >> + maxim->vin = devm_regulator_get_optional(&maxim->client->dev, "power"); >> + if (IS_ERR(maxim->vin)) { >> + if (PTR_ERR(maxim->vin) == -EPROBE_DEFER) >> + return -EPROBE_DEFER; >> + maxim->vin = NULL; >> + } >> + >> + if (maxim->vin) { >> + ret = regulator_enable(maxim->vin); >> + if (ret < 0) { >> + dev_err(&maxim->client->dev, >> + "failed to enable Vin: %d\n", ret); >> + return ret; >> + } >> + } > > use devm_regulator_get_enable_optional() to combine devm_regulator_get_optional() > and regulator_enable() to one call. This however does not return the regulator and will not allow further potential power management, I did look into using that one but decided against it. There is no runtime PM implemented right now so it wouldn't really matter at this point. If it is desirable I will switch it, and it will have to be switched back when PM gets implemented. >> + >> + maxim->enable = devm_gpiod_get_optional(&maxim->client->dev, "enable", >> + GPIOD_ASIS); >> + if (IS_ERR(maxim->enable)) { >> + ret = PTR_ERR(maxim->enable); >> + dev_err(&maxim->client->dev, "failed to get enable gpio: %d\n", >> + ret); >> + goto disable_vin; >> + } >> + >> + if (maxim->enable) >> + gpiod_set_value_cansleep(maxim->enable, 1); > > gpiod_set_value_cansleep() tolerate NULL, so needn't if check here > > and if you pass GPIOD_OUT_HIGH at devm_gpiod_get_optional(), needn't call > this function. got it, changed. >> + >> + /* Enable can be tied to vin rail wait if either is available */ >> + if (maxim->enable || maxim->vin) { >> + /* Datasheet Electrical Characteristics tSTARTUP 2ms */ >> + usleep_range(2000, 2500); > > now perfer use fsleep() Ah didn't know of that, thanks! Other small remarks have also been resolved. Kind regards, Maud >> + } >> + >> + maxim->regmap = devm_regmap_init_i2c(cl, &max25014_regmap_config); >> + if (IS_ERR(maxim->regmap)) { >> + ret = PTR_ERR(maxim->regmap); >> + dev_err(&maxim->client->dev, >> + "failed to initialize the i2c regmap: %d\n", ret); >> + goto disable_full; >> + } >> + >> + i2c_set_clientdata(cl, maxim); >> + >> + ret = max25014_check_errors(maxim); >> + if (ret) { /* error is already reported in the above function */ >> + goto disable_full; >> + } >> + >> + ret = max25014_configure(maxim); >> + if (ret) { >> + dev_err(&maxim->client->dev, "device config err: %d", ret); >> + goto disable_full; >> + } >> + >> + memset(&props, 0, sizeof(props)); >> + props.type = BACKLIGHT_PLATFORM; >> + props.max_brightness = MAX_BRIGHTNESS; >> + props.brightness = initial_brightness; >> + props.scale = BACKLIGHT_SCALE_LINEAR; >> + >> + bl = devm_backlight_device_register(&maxim->client->dev, id->name, >> + &maxim->client->dev, maxim, >> + &max25014_bl_ops, &props); >> + if (IS_ERR(bl)) >> + return PTR_ERR(bl); >> + >> + maxim->bl = bl; >> + >> + return 0; >> + >> +disable_full: >> + if (maxim->enable) >> + gpiod_set_value_cansleep(maxim->enable, 0); >> +disable_vin: >> + if (maxim->vin) >> + regulator_disable(maxim->vin); >> + return ret; >> +} >> + >> +static void max25014_remove(struct i2c_client *cl) >> +{ >> + struct max25014 *maxim = i2c_get_clientdata(cl); >> + >> + maxim->bl->props.brightness = 0; >> + max25014_update_status(maxim->bl); >> + if (maxim->enable) >> + gpiod_set_value_cansleep(maxim->enable, 0); >> + if (maxim->vin) >> + regulator_disable(maxim->vin); >> +} >> + >> +static const struct of_device_id max25014_dt_ids[] = { >> + { .compatible = "maxim,max25014", }, >> + { } >> +}; >> +MODULE_DEVICE_TABLE(of, max25014_dt_ids); >> + >> +static const struct i2c_device_id max25014_ids[] = { >> + { "max25014" }, >> + { } >> +}; >> +MODULE_DEVICE_TABLE(i2c, max25014_ids); >> + >> +static struct i2c_driver max25014_driver = { >> + .driver = { >> + .name = KBUILD_MODNAME, >> + .of_match_table = of_match_ptr(max25014_dt_ids), >> + }, >> + .probe = max25014_probe, >> + .remove = max25014_remove, >> + .id_table = max25014_ids, >> +}; >> +module_i2c_driver(max25014_driver); >> + >> +MODULE_DESCRIPTION("Maxim MAX25014 backlight driver"); >> +MODULE_AUTHOR("Maud Spierings <maudspierings@gocontroll.com>"); >> +MODULE_LICENSE("GPL"); >> >> -- >> 2.51.2 >> >> ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v5 2/4] backlight: add max25014atg backlight 2025-11-07 12:49 ` [PATCH v5 2/4] backlight: add max25014atg backlight Maud Spierings via B4 Relay 2025-11-07 15:51 ` [PATCH v5 2/4] backlight: add max25014atg backlighty Frank Li @ 2025-11-07 16:14 ` Daniel Thompson 2025-11-10 8:40 ` Maud Spierings 1 sibling, 1 reply; 19+ messages in thread From: Daniel Thompson @ 2025-11-07 16:14 UTC (permalink / raw) To: maudspierings Cc: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Helge Deller, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Liam Girdwood, Mark Brown, dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, imx, linux-arm-kernel On Fri, Nov 07, 2025 at 01:49:59PM +0100, Maud Spierings via B4 Relay wrote: > From: Maud Spierings <maudspierings@gocontroll.com> > > The Maxim MAX25014 is a 4-channel automotive grade backlight driver IC > with integrated boost controller. > > Signed-off-by: Maud Spierings <maudspierings@gocontroll.com> > diff --git a/drivers/video/backlight/max25014.c b/drivers/video/backlight/max25014.c > new file mode 100644 > index 000000000000..36bae508697e > --- /dev/null > +++ b/drivers/video/backlight/max25014.c > @@ -0,0 +1,409 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Backlight driver for Maxim MAX25014 > + * > + * Copyright (C) 2025 GOcontroll B.V. > + * Author: Maud Spierings <maudspierings@gocontroll.com> > + */ > + > +#include <linux/backlight.h> > +#include <linux/gpio/consumer.h> > +#include <linux/i2c.h> > +#include <linux/regmap.h> > +#include <linux/slab.h> > + > +#define MAX25014_ISET_DEFAULT_100 11 > +#define MAX_BRIGHTNESS 100 > +#define MIN_BRIGHTNESS 0 > +#define TON_MAX 130720 /* @153Hz */ > +#define TON_STEP 1307 /* @153Hz */ > +#define TON_MIN 0 > + > +#define MAX25014_DEV_ID 0x00 > +#define MAX25014_REV_ID 0x01 > +#define MAX25014_ISET 0x02 > +#define MAX25014_IMODE 0x03 > +#define MAX25014_TON1H 0x04 > +#define MAX25014_TON1L 0x05 > +#define MAX25014_TON2H 0x06 > +#define MAX25014_TON2L 0x07 > +#define MAX25014_TON3H 0x08 > +#define MAX25014_TON3L 0x09 > +#define MAX25014_TON4H 0x0A > +#define MAX25014_TON4L 0x0B > +#define MAX25014_TON_1_4_LSB 0x0C > +#define MAX25014_SETTING 0x12 > +#define MAX25014_DISABLE 0x13 > +#define MAX25014_BSTMON 0x14 > +#define MAX25014_IOUT1 0x15 > +#define MAX25014_IOUT2 0x16 > +#define MAX25014_IOUT3 0x17 > +#define MAX25014_IOUT4 0x18 > +#define MAX25014_OPEN 0x1B > +#define MAX25014_SHORT_GND 0x1C > +#define MAX25014_SHORT_LED 0x1D > +#define MAX25014_MASK 0x1E > +#define MAX25014_DIAG 0x1F > + > +#define MAX25014_IMODE_HDIM BIT(2) > +#define MAX25014_ISET_ENABLE BIT(5) > +#define MAX25014_ISET_PSEN BIT(4) > +#define MAX25014_DIAG_HW_RST BIT(2) > +#define MAX25014_SETTING_FPWM GENMASK(6, 4) > + > +struct max25014 { > + struct i2c_client *client; > + struct backlight_device *bl; > + struct regmap *regmap; > + struct gpio_desc *enable; > + struct regulator *vin; /* regulator for boost converter Vin rail */ > + uint32_t iset; > + uint8_t strings_mask; > +}; > + > +static const struct regmap_config max25014_regmap_config = { > + .reg_bits = 8, > + .val_bits = 8, > + .max_register = MAX25014_DIAG, > +}; > + > +/** > + * @brief control the brightness with i2c registers > + * > + * @param regmap trivial > + * @param brt brightness > + * @return int > + */ > +static int max25014_register_control(struct regmap *regmap, uint32_t brt) This isn't a good name for a function. It doesn't really say what it does. Please find a more descriptive name. > +{ > + uint32_t reg = TON_STEP * brt; > + int ret; > + /* > + * 18 bit number lowest, 2 bits in first register, > + * next lowest 8 in the L register, next 8 in the H register > + * Seemingly setting the strength of only one string controls all of > + * them, individual settings don't affect the outcome. > + */ > + > + ret = regmap_write(regmap, MAX25014_TON_1_4_LSB, reg & 0b00000011); > + if (ret != 0) > + return ret; > + ret = regmap_write(regmap, MAX25014_TON1L, (reg >> 2) & 0b11111111); > + if (ret != 0) > + return ret; > + return regmap_write(regmap, MAX25014_TON1H, (reg >> 10) & 0b11111111); > +} > + > +static int max25014_check_errors(struct max25014 *maxim) > +{ > + uint8_t i; > + int ret; > + uint32_t val; > + > + ret = regmap_read(maxim->regmap, MAX25014_OPEN, &val); > + if (ret != 0) > + return ret; > + if (val > 0) { > + dev_err(&maxim->client->dev, "Open led strings detected on:\n"); > + for (i = 0; i < 4; i++) { > + if (val & 1 << i) > + dev_err(&maxim->client->dev, "string %d\n", i + 1); > + } > + return -EIO; > + } > + > + ret = regmap_read(maxim->regmap, MAX25014_SHORT_GND, &val); > + if (ret != 0) > + return ret; > + if (val > 0) { > + dev_err(&maxim->client->dev, "Short to ground detected on:\n"); > + for (i = 0; i < 4; i++) { > + if (val & 1 << i) > + dev_err(&maxim->client->dev, "string %d\n", i + 1); > + } > + return -EIO; > + } > + > + ret = regmap_read(maxim->regmap, MAX25014_SHORT_GND, &val); > + if (ret != 0) > + return ret; > + if (val > 0) { > + dev_err(&maxim->client->dev, "Shorted led detected on:\n"); > + for (i = 0; i < 4; i++) { > + if (val & 1 << i) > + dev_err(&maxim->client->dev, "string %d\n", i + 1); > + } > + return -EIO; > + } > + > + ret = regmap_read(maxim->regmap, MAX25014_DIAG, &val); > + if (ret != 0) > + return ret; > + /* > + * The HW_RST bit always starts at 1 after power up. > + * It is reset on first read, does not indicate an error. > + */ > + if (val > 0 && val != MAX25014_DIAG_HW_RST) { > + if (val & 0b1) > + dev_err(&maxim->client->dev, > + "Overtemperature shutdown\n"); > + if (val & 0b10) > + dev_err(&maxim->client->dev, > + "Chip is getting too hot (>125C)\n"); > + if (val & 0b1000) > + dev_err(&maxim->client->dev, > + "Boost converter overvoltage\n"); > + if (val & 0b10000) > + dev_err(&maxim->client->dev, > + "Boost converter undervoltage\n"); > + if (val & 0b100000) > + dev_err(&maxim->client->dev, "IREF out of range\n"); > + return -EIO; > + } > + return 0; > +} > + > +/* > + * 1. disable unused strings > + * 2. set dim mode > + * 3. set initial brightness How does this code set the initial brightness? It doens't set the MAX25014_TON* registers. > + * 4. set setting register > + * 5. enable the backlight > + */ > +static int max25014_configure(struct max25014 *maxim) > +{ > + int ret; > + uint32_t val; > + > + ret = regmap_write(maxim->regmap, MAX25014_DISABLE, > + maxim->strings_mask); > + if (ret != 0) > + return ret; > + > + ret = regmap_write(maxim->regmap, MAX25014_IMODE, MAX25014_IMODE_HDIM); > + if (ret != 0) > + return ret; > + > + ret = regmap_read(maxim->regmap, MAX25014_SETTING, &val); > + if (ret != 0) > + return ret; > + > + ret = regmap_write(maxim->regmap, MAX25014_SETTING, > + val & ~MAX25014_SETTING_FPWM); > + if (ret != 0) > + return ret; > + > + ret = regmap_write(maxim->regmap, MAX25014_ISET, > + maxim->iset | MAX25014_ISET_ENABLE | > + MAX25014_ISET_PSEN); > + return ret; > +} > + > +static int max25014_update_status(struct backlight_device *bl_dev) > +{ > + struct max25014 *maxim = bl_get_data(bl_dev); > + > + if (backlight_is_blank(maxim->bl)) > + bl_dev->props.brightness = 0; > + > + return max25014_register_control(maxim->regmap, > + bl_dev->props.brightness); > +} > + > +static const struct backlight_ops max25014_bl_ops = { > + .options = BL_CORE_SUSPENDRESUME, > + .update_status = max25014_update_status, > +}; > + > +static int max25014_parse_dt(struct max25014 *maxim, > + uint32_t *initial_brightness) > +{ > + struct device *dev = &maxim->client->dev; > + struct device_node *node = dev->of_node; > + struct fwnode_handle *child; > + uint32_t strings[4]; > + int res, i; > + > + if (!node) { > + dev_err(dev, "no platform data\n"); > + return -EINVAL; > + } > + > + child = device_get_next_child_node(dev, NULL); > + if (child) { > + res = fwnode_property_count_u32(child, "led-sources"); > + if (res > 0) { > + fwnode_property_read_u32_array(child, "led-sources", > + strings, res); > + > + /* set all strings as disabled, then enable those in led-sources*/ > + maxim->strings_mask = 0xf; > + for (i = 0; i < res; i++) { > + if (strings[i] <= 4) > + maxim->strings_mask &= ~BIT(strings[i]); > + } > + } > + > + fwnode_property_read_u32(child, "default-brightness", > + initial_brightness); > + > + fwnode_handle_put(child); > + } > + > + maxim->iset = MAX25014_ISET_DEFAULT_100; > + of_property_read_u32(node, "maxim,iset", &maxim->iset); > + > + if (maxim->iset < 0 || maxim->iset > 15) { > + dev_err(dev, > + "Invalid iset, should be a value from 0-15, entered was %d\n", > + maxim->iset); > + return -EINVAL; > + } > + > + if (*initial_brightness < 0 || *initial_brightness > 100) { > + dev_err(dev, > + "Invalid initial brightness, should be a value from 0-100, entered was %d\n", > + *initial_brightness); > + return -EINVAL; > + } > + > + return 0; > +} > + > +static int max25014_probe(struct i2c_client *cl) > +{ > + struct backlight_device *bl; > + const struct i2c_device_id *id = i2c_client_get_device_id(cl); > + struct max25014 *maxim; > + struct backlight_properties props; > + int ret; > + uint32_t initial_brightness = 50; > + > + maxim = devm_kzalloc(&cl->dev, sizeof(struct max25014), GFP_KERNEL); > + if (!maxim) > + return -ENOMEM; > + > + maxim->client = cl; > + > + ret = max25014_parse_dt(maxim, &initial_brightness); > + if (ret < 0) > + return ret; > + > + maxim->vin = devm_regulator_get_optional(&maxim->client->dev, "power"); I would have expected to see devm_regulator_get() here. Why do you care whether you get a real regulator or a dummy if you just NULL check maxim->vin everywhere? > + if (IS_ERR(maxim->vin)) { > + if (PTR_ERR(maxim->vin) == -EPROBE_DEFER) > + return -EPROBE_DEFER; > + maxim->vin = NULL; > + } > + > + if (maxim->vin) { If you had called devm_regulator_get() there would be no need for a NULL check here. > + ret = regulator_enable(maxim->vin); > + if (ret < 0) { > + dev_err(&maxim->client->dev, > + "failed to enable Vin: %d\n", ret); > + return ret; > + } > + } > + > + maxim->enable = devm_gpiod_get_optional(&maxim->client->dev, "enable", > + GPIOD_ASIS); > + if (IS_ERR(maxim->enable)) { > + ret = PTR_ERR(maxim->enable); > + dev_err(&maxim->client->dev, "failed to get enable gpio: %d\n", > + ret); > + goto disable_vin; > + } > + > + if (maxim->enable) > + gpiod_set_value_cansleep(maxim->enable, 1); No need for NULL pointer check here (see https://elixir.bootlin.com/linux/v6.18-rc4/source/drivers/gpio/gpiolib.c#L358-L363 ). > + > + /* Enable can be tied to vin rail wait if either is available */ > + if (maxim->enable || maxim->vin) { > + /* Datasheet Electrical Characteristics tSTARTUP 2ms */ > + usleep_range(2000, 2500); > + } If you really want to keep the devm_regulator_get_optional() I guess maybe you could persuade me it's need to avoid this sleep... although I'd be fairly happy to remove the NULL checks here too! > + > + maxim->regmap = devm_regmap_init_i2c(cl, &max25014_regmap_config); > + if (IS_ERR(maxim->regmap)) { > + ret = PTR_ERR(maxim->regmap); > + dev_err(&maxim->client->dev, > + "failed to initialize the i2c regmap: %d\n", ret); > + goto disable_full; > + } > + > + i2c_set_clientdata(cl, maxim); > + > + ret = max25014_check_errors(maxim); > + if (ret) { /* error is already reported in the above function */ > + goto disable_full; > + } > + > + ret = max25014_configure(maxim); > + if (ret) { > + dev_err(&maxim->client->dev, "device config err: %d", ret); > + goto disable_full; > + } > + > + memset(&props, 0, sizeof(props)); > + props.type = BACKLIGHT_PLATFORM; > + props.max_brightness = MAX_BRIGHTNESS; > + props.brightness = initial_brightness; > + props.scale = BACKLIGHT_SCALE_LINEAR; > + > + bl = devm_backlight_device_register(&maxim->client->dev, id->name, > + &maxim->client->dev, maxim, > + &max25014_bl_ops, &props); > + if (IS_ERR(bl)) > + return PTR_ERR(bl); > + > + maxim->bl = bl; > + > + return 0; > + > +disable_full: > + if (maxim->enable) > + gpiod_set_value_cansleep(maxim->enable, 0); Again, NULL check isn't needed. > +disable_vin: > + if (maxim->vin) > + regulator_disable(maxim->vin); > + return ret; > +} > + > +static void max25014_remove(struct i2c_client *cl) > +{ > + struct max25014 *maxim = i2c_get_clientdata(cl); > + > + maxim->bl->props.brightness = 0; > + max25014_update_status(maxim->bl); > + if (maxim->enable) > + gpiod_set_value_cansleep(maxim->enable, 0); Lose the NULL check. > + if (maxim->vin) > + regulator_disable(maxim->vin); > +} > + > +static const struct of_device_id max25014_dt_ids[] = { > + { .compatible = "maxim,max25014", }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, max25014_dt_ids); > + > +static const struct i2c_device_id max25014_ids[] = { > + { "max25014" }, > + { } > +}; > +MODULE_DEVICE_TABLE(i2c, max25014_ids); > + > +static struct i2c_driver max25014_driver = { > + .driver = { > + .name = KBUILD_MODNAME, > + .of_match_table = of_match_ptr(max25014_dt_ids), > + }, > + .probe = max25014_probe, > + .remove = max25014_remove, > + .id_table = max25014_ids, > +}; > +module_i2c_driver(max25014_driver); > + > +MODULE_DESCRIPTION("Maxim MAX25014 backlight driver"); > +MODULE_AUTHOR("Maud Spierings <maudspierings@gocontroll.com>"); > +MODULE_LICENSE("GPL"); Daniel. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v5 2/4] backlight: add max25014atg backlight 2025-11-07 16:14 ` [PATCH v5 2/4] backlight: add max25014atg backlight Daniel Thompson @ 2025-11-10 8:40 ` Maud Spierings 2025-11-10 10:01 ` Daniel Thompson 0 siblings, 1 reply; 19+ messages in thread From: Maud Spierings @ 2025-11-10 8:40 UTC (permalink / raw) To: Daniel Thompson Cc: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Helge Deller, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Liam Girdwood, Mark Brown, dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, imx, linux-arm-kernel On 11/7/25 17:14, Daniel Thompson wrote: > On Fri, Nov 07, 2025 at 01:49:59PM +0100, Maud Spierings via B4 Relay wrote: >> From: Maud Spierings <maudspierings@gocontroll.com> >> >> The Maxim MAX25014 is a 4-channel automotive grade backlight driver IC >> with integrated boost controller. >> >> Signed-off-by: Maud Spierings <maudspierings@gocontroll.com> >> diff --git a/drivers/video/backlight/max25014.c b/drivers/video/backlight/max25014.c >> new file mode 100644 >> index 000000000000..36bae508697e >> --- /dev/null >> +++ b/drivers/video/backlight/max25014.c >> @@ -0,0 +1,409 @@ >> +// SPDX-License-Identifier: GPL-2.0-only >> +/* >> + * Backlight driver for Maxim MAX25014 >> + * >> + * Copyright (C) 2025 GOcontroll B.V. >> + * Author: Maud Spierings <maudspierings@gocontroll.com> >> + */ >> + >> +#include <linux/backlight.h> >> +#include <linux/gpio/consumer.h> >> +#include <linux/i2c.h> >> +#include <linux/regmap.h> >> +#include <linux/slab.h> >> + >> +#define MAX25014_ISET_DEFAULT_100 11 >> +#define MAX_BRIGHTNESS 100 >> +#define MIN_BRIGHTNESS 0 >> +#define TON_MAX 130720 /* @153Hz */ >> +#define TON_STEP 1307 /* @153Hz */ >> +#define TON_MIN 0 >> + >> +#define MAX25014_DEV_ID 0x00 >> +#define MAX25014_REV_ID 0x01 >> +#define MAX25014_ISET 0x02 >> +#define MAX25014_IMODE 0x03 >> +#define MAX25014_TON1H 0x04 >> +#define MAX25014_TON1L 0x05 >> +#define MAX25014_TON2H 0x06 >> +#define MAX25014_TON2L 0x07 >> +#define MAX25014_TON3H 0x08 >> +#define MAX25014_TON3L 0x09 >> +#define MAX25014_TON4H 0x0A >> +#define MAX25014_TON4L 0x0B >> +#define MAX25014_TON_1_4_LSB 0x0C >> +#define MAX25014_SETTING 0x12 >> +#define MAX25014_DISABLE 0x13 >> +#define MAX25014_BSTMON 0x14 >> +#define MAX25014_IOUT1 0x15 >> +#define MAX25014_IOUT2 0x16 >> +#define MAX25014_IOUT3 0x17 >> +#define MAX25014_IOUT4 0x18 >> +#define MAX25014_OPEN 0x1B >> +#define MAX25014_SHORT_GND 0x1C >> +#define MAX25014_SHORT_LED 0x1D >> +#define MAX25014_MASK 0x1E >> +#define MAX25014_DIAG 0x1F >> + >> +#define MAX25014_IMODE_HDIM BIT(2) >> +#define MAX25014_ISET_ENABLE BIT(5) >> +#define MAX25014_ISET_PSEN BIT(4) >> +#define MAX25014_DIAG_HW_RST BIT(2) >> +#define MAX25014_SETTING_FPWM GENMASK(6, 4) >> + >> +struct max25014 { >> + struct i2c_client *client; >> + struct backlight_device *bl; >> + struct regmap *regmap; >> + struct gpio_desc *enable; >> + struct regulator *vin; /* regulator for boost converter Vin rail */ >> + uint32_t iset; >> + uint8_t strings_mask; >> +}; >> + >> +static const struct regmap_config max25014_regmap_config = { >> + .reg_bits = 8, >> + .val_bits = 8, >> + .max_register = MAX25014_DIAG, >> +}; >> + >> +/** >> + * @brief control the brightness with i2c registers >> + * >> + * @param regmap trivial >> + * @param brt brightness >> + * @return int >> + */ >> +static int max25014_register_control(struct regmap *regmap, uint32_t brt) > > This isn't a good name for a function. It doesn't really say what it > does. Please find a more descriptive name. Having a lot of difficulties find a succinct name that fits better, max25014_register_brightness_control()? max25014_i2c_brightness_control()? > >> +{ >> + uint32_t reg = TON_STEP * brt; >> + int ret; >> + /* >> + * 18 bit number lowest, 2 bits in first register, >> + * next lowest 8 in the L register, next 8 in the H register >> + * Seemingly setting the strength of only one string controls all of >> + * them, individual settings don't affect the outcome. >> + */ >> + >> + ret = regmap_write(regmap, MAX25014_TON_1_4_LSB, reg & 0b00000011); >> + if (ret != 0) >> + return ret; >> + ret = regmap_write(regmap, MAX25014_TON1L, (reg >> 2) & 0b11111111); >> + if (ret != 0) >> + return ret; >> + return regmap_write(regmap, MAX25014_TON1H, (reg >> 10) & 0b11111111); >> +} >> + >> +static int max25014_check_errors(struct max25014 *maxim) >> +{ >> + uint8_t i; >> + int ret; >> + uint32_t val; >> + >> + ret = regmap_read(maxim->regmap, MAX25014_OPEN, &val); >> + if (ret != 0) >> + return ret; >> + if (val > 0) { >> + dev_err(&maxim->client->dev, "Open led strings detected on:\n"); >> + for (i = 0; i < 4; i++) { >> + if (val & 1 << i) >> + dev_err(&maxim->client->dev, "string %d\n", i + 1); >> + } >> + return -EIO; >> + } >> + >> + ret = regmap_read(maxim->regmap, MAX25014_SHORT_GND, &val); >> + if (ret != 0) >> + return ret; >> + if (val > 0) { >> + dev_err(&maxim->client->dev, "Short to ground detected on:\n"); >> + for (i = 0; i < 4; i++) { >> + if (val & 1 << i) >> + dev_err(&maxim->client->dev, "string %d\n", i + 1); >> + } >> + return -EIO; >> + } >> + >> + ret = regmap_read(maxim->regmap, MAX25014_SHORT_GND, &val); >> + if (ret != 0) >> + return ret; >> + if (val > 0) { >> + dev_err(&maxim->client->dev, "Shorted led detected on:\n"); >> + for (i = 0; i < 4; i++) { >> + if (val & 1 << i) >> + dev_err(&maxim->client->dev, "string %d\n", i + 1); >> + } >> + return -EIO; >> + } >> + >> + ret = regmap_read(maxim->regmap, MAX25014_DIAG, &val); >> + if (ret != 0) >> + return ret; >> + /* >> + * The HW_RST bit always starts at 1 after power up. >> + * It is reset on first read, does not indicate an error. >> + */ >> + if (val > 0 && val != MAX25014_DIAG_HW_RST) { >> + if (val & 0b1) >> + dev_err(&maxim->client->dev, >> + "Overtemperature shutdown\n"); >> + if (val & 0b10) >> + dev_err(&maxim->client->dev, >> + "Chip is getting too hot (>125C)\n"); >> + if (val & 0b1000) >> + dev_err(&maxim->client->dev, >> + "Boost converter overvoltage\n"); >> + if (val & 0b10000) >> + dev_err(&maxim->client->dev, >> + "Boost converter undervoltage\n"); >> + if (val & 0b100000) >> + dev_err(&maxim->client->dev, "IREF out of range\n"); >> + return -EIO; >> + } >> + return 0; >> +} >> + >> +/* >> + * 1. disable unused strings >> + * 2. set dim mode >> + * 3. set initial brightness > > How does this code set the initial brightness? It doens't set the > MAX25014_TON* registers. Yep forgot to remove that, I discovered the backlight core takes care of the default brightness, so I removed it from here. > >> + * 4. set setting register >> + * 5. enable the backlight >> + */ >> +static int max25014_configure(struct max25014 *maxim) >> +{ >> + int ret; >> + uint32_t val; >> + >> + ret = regmap_write(maxim->regmap, MAX25014_DISABLE, >> + maxim->strings_mask); >> + if (ret != 0) >> + return ret; >> + >> + ret = regmap_write(maxim->regmap, MAX25014_IMODE, MAX25014_IMODE_HDIM); >> + if (ret != 0) >> + return ret; >> + >> + ret = regmap_read(maxim->regmap, MAX25014_SETTING, &val); >> + if (ret != 0) >> + return ret; >> + >> + ret = regmap_write(maxim->regmap, MAX25014_SETTING, >> + val & ~MAX25014_SETTING_FPWM); >> + if (ret != 0) >> + return ret; >> + >> + ret = regmap_write(maxim->regmap, MAX25014_ISET, >> + maxim->iset | MAX25014_ISET_ENABLE | >> + MAX25014_ISET_PSEN); >> + return ret; >> +} >> + >> +static int max25014_update_status(struct backlight_device *bl_dev) >> +{ >> + struct max25014 *maxim = bl_get_data(bl_dev); >> + >> + if (backlight_is_blank(maxim->bl)) >> + bl_dev->props.brightness = 0; >> + >> + return max25014_register_control(maxim->regmap, >> + bl_dev->props.brightness); >> +} >> + >> +static const struct backlight_ops max25014_bl_ops = { >> + .options = BL_CORE_SUSPENDRESUME, >> + .update_status = max25014_update_status, >> +}; >> + >> +static int max25014_parse_dt(struct max25014 *maxim, >> + uint32_t *initial_brightness) >> +{ >> + struct device *dev = &maxim->client->dev; >> + struct device_node *node = dev->of_node; >> + struct fwnode_handle *child; >> + uint32_t strings[4]; >> + int res, i; >> + >> + if (!node) { >> + dev_err(dev, "no platform data\n"); >> + return -EINVAL; >> + } >> + >> + child = device_get_next_child_node(dev, NULL); >> + if (child) { >> + res = fwnode_property_count_u32(child, "led-sources"); >> + if (res > 0) { >> + fwnode_property_read_u32_array(child, "led-sources", >> + strings, res); >> + >> + /* set all strings as disabled, then enable those in led-sources*/ >> + maxim->strings_mask = 0xf; >> + for (i = 0; i < res; i++) { >> + if (strings[i] <= 4) >> + maxim->strings_mask &= ~BIT(strings[i]); >> + } >> + } >> + >> + fwnode_property_read_u32(child, "default-brightness", >> + initial_brightness); >> + >> + fwnode_handle_put(child); >> + } >> + >> + maxim->iset = MAX25014_ISET_DEFAULT_100; >> + of_property_read_u32(node, "maxim,iset", &maxim->iset); >> + >> + if (maxim->iset < 0 || maxim->iset > 15) { >> + dev_err(dev, >> + "Invalid iset, should be a value from 0-15, entered was %d\n", >> + maxim->iset); >> + return -EINVAL; >> + } >> + >> + if (*initial_brightness < 0 || *initial_brightness > 100) { >> + dev_err(dev, >> + "Invalid initial brightness, should be a value from 0-100, entered was %d\n", >> + *initial_brightness); >> + return -EINVAL; >> + } >> + >> + return 0; >> +} >> + >> +static int max25014_probe(struct i2c_client *cl) >> +{ >> + struct backlight_device *bl; >> + const struct i2c_device_id *id = i2c_client_get_device_id(cl); >> + struct max25014 *maxim; >> + struct backlight_properties props; >> + int ret; >> + uint32_t initial_brightness = 50; >> + >> + maxim = devm_kzalloc(&cl->dev, sizeof(struct max25014), GFP_KERNEL); >> + if (!maxim) >> + return -ENOMEM; >> + >> + maxim->client = cl; >> + >> + ret = max25014_parse_dt(maxim, &initial_brightness); >> + if (ret < 0) >> + return ret; >> + >> + maxim->vin = devm_regulator_get_optional(&maxim->client->dev, "power"); > > I would have expected to see devm_regulator_get() here. Why do you care > whether you get a real regulator or a dummy if you just NULL check > maxim->vin everywhere? > > >> + if (IS_ERR(maxim->vin)) { >> + if (PTR_ERR(maxim->vin) == -EPROBE_DEFER) >> + return -EPROBE_DEFER; >> + maxim->vin = NULL; >> + } >> + >> + if (maxim->vin) { > > If you had called devm_regulator_get() there would be no need for a NULL > check here. > > >> + ret = regulator_enable(maxim->vin); >> + if (ret < 0) { >> + dev_err(&maxim->client->dev, >> + "failed to enable Vin: %d\n", ret); >> + return ret; >> + } >> + } >> + >> + maxim->enable = devm_gpiod_get_optional(&maxim->client->dev, "enable", >> + GPIOD_ASIS); >> + if (IS_ERR(maxim->enable)) { >> + ret = PTR_ERR(maxim->enable); >> + dev_err(&maxim->client->dev, "failed to get enable gpio: %d\n", >> + ret); >> + goto disable_vin; >> + } >> + >> + if (maxim->enable) >> + gpiod_set_value_cansleep(maxim->enable, 1); > > No need for NULL pointer check here (see > https://elixir.bootlin.com/linux/v6.18-rc4/source/drivers/gpio/gpiolib.c#L358-L363 ). > > >> + >> + /* Enable can be tied to vin rail wait if either is available */ >> + if (maxim->enable || maxim->vin) { >> + /* Datasheet Electrical Characteristics tSTARTUP 2ms */ >> + usleep_range(2000, 2500); >> + } > > If you really want to keep the devm_regulator_get_optional() I guess > maybe you could persuade me it's need to avoid this sleep... although > I'd be fairly happy to remove the NULL checks here too! Just wait unconditionally? kind regards, Maud > >> + >> + maxim->regmap = devm_regmap_init_i2c(cl, &max25014_regmap_config); >> + if (IS_ERR(maxim->regmap)) { >> + ret = PTR_ERR(maxim->regmap); >> + dev_err(&maxim->client->dev, >> + "failed to initialize the i2c regmap: %d\n", ret); >> + goto disable_full; >> + } >> + >> + i2c_set_clientdata(cl, maxim); >> + >> + ret = max25014_check_errors(maxim); >> + if (ret) { /* error is already reported in the above function */ >> + goto disable_full; >> + } >> + >> + ret = max25014_configure(maxim); >> + if (ret) { >> + dev_err(&maxim->client->dev, "device config err: %d", ret); >> + goto disable_full; >> + } >> + >> + memset(&props, 0, sizeof(props)); >> + props.type = BACKLIGHT_PLATFORM; >> + props.max_brightness = MAX_BRIGHTNESS; >> + props.brightness = initial_brightness; >> + props.scale = BACKLIGHT_SCALE_LINEAR; >> + >> + bl = devm_backlight_device_register(&maxim->client->dev, id->name, >> + &maxim->client->dev, maxim, >> + &max25014_bl_ops, &props); >> + if (IS_ERR(bl)) >> + return PTR_ERR(bl); >> + >> + maxim->bl = bl; >> + >> + return 0; >> + >> +disable_full: >> + if (maxim->enable) >> + gpiod_set_value_cansleep(maxim->enable, 0); > > Again, NULL check isn't needed. > >> +disable_vin: >> + if (maxim->vin) >> + regulator_disable(maxim->vin); >> + return ret; >> +} >> + >> +static void max25014_remove(struct i2c_client *cl) >> +{ >> + struct max25014 *maxim = i2c_get_clientdata(cl); >> + >> + maxim->bl->props.brightness = 0; >> + max25014_update_status(maxim->bl); >> + if (maxim->enable) >> + gpiod_set_value_cansleep(maxim->enable, 0); > > Lose the NULL check. > >> + if (maxim->vin) >> + regulator_disable(maxim->vin); >> +} >> + >> +static const struct of_device_id max25014_dt_ids[] = { >> + { .compatible = "maxim,max25014", }, >> + { } >> +}; >> +MODULE_DEVICE_TABLE(of, max25014_dt_ids); >> + >> +static const struct i2c_device_id max25014_ids[] = { >> + { "max25014" }, >> + { } >> +}; >> +MODULE_DEVICE_TABLE(i2c, max25014_ids); >> + >> +static struct i2c_driver max25014_driver = { >> + .driver = { >> + .name = KBUILD_MODNAME, >> + .of_match_table = of_match_ptr(max25014_dt_ids), >> + }, >> + .probe = max25014_probe, >> + .remove = max25014_remove, >> + .id_table = max25014_ids, >> +}; >> +module_i2c_driver(max25014_driver); >> + >> +MODULE_DESCRIPTION("Maxim MAX25014 backlight driver"); >> +MODULE_AUTHOR("Maud Spierings <maudspierings@gocontroll.com>"); >> +MODULE_LICENSE("GPL"); > > > Daniel. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v5 2/4] backlight: add max25014atg backlight 2025-11-10 8:40 ` Maud Spierings @ 2025-11-10 10:01 ` Daniel Thompson 2025-11-10 10:03 ` Maud Spierings 0 siblings, 1 reply; 19+ messages in thread From: Daniel Thompson @ 2025-11-10 10:01 UTC (permalink / raw) To: Maud Spierings Cc: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Helge Deller, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Liam Girdwood, Mark Brown, dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, imx, linux-arm-kernel On Mon, Nov 10, 2025 at 09:40:07AM +0100, Maud Spierings wrote: > On 11/7/25 17:14, Daniel Thompson wrote: > > On Fri, Nov 07, 2025 at 01:49:59PM +0100, Maud Spierings via B4 Relay wrote: > > > +/** > > > + * @brief control the brightness with i2c registers > > > + * > > > + * @param regmap trivial > > > + * @param brt brightness > > > + * @return int > > > + */ > > > +static int max25014_register_control(struct regmap *regmap, uint32_t brt) > > > > This isn't a good name for a function. It doesn't really say what it > > does. Please find a more descriptive name. > > Having a lot of difficulties find a succinct name that fits better, > max25014_register_brightness_control()? > max25014_i2c_brightness_control()? I'd focus on what it does rather than how it does it meaning something like max25014_update_brightness() would work. However, at present, this code is only called from max25014_update_status() so the simplest thing to do is to move the code into max25014_update_status() and remove this function entirely (then it doesn't matter what it is called ;-) ). > > > +/* > > > + * 1. disable unused strings > > > + * 2. set dim mode > > > + * 3. set initial brightness > > > > How does this code set the initial brightness? It doens't set the > > MAX25014_TON* registers. > > Yep forgot to remove that, I discovered the backlight core takes care of the > default brightness, so I removed it from here. What do you mean by this? Are you sure you aren't relying on another driver to enable the backlight rather than the backlight core? > > > + * 4. set setting register > > > + * 5. enable the backlight > > > + */ > > > +static int max25014_configure(struct max25014 *maxim) > > > +static int max25014_probe(struct i2c_client *cl) > > > <snip> > > > + > > > + /* Enable can be tied to vin rail wait if either is available */ > > > + if (maxim->enable || maxim->vin) { > > > + /* Datasheet Electrical Characteristics tSTARTUP 2ms */ > > > + usleep_range(2000, 2500); > > > + } > > > > If you really want to keep the devm_regulator_get_optional() I guess > > maybe you could persuade me it's need to avoid this sleep... although > > I'd be fairly happy to remove the NULL checks here too! > > Just wait unconditionally? If you think it will be unusual for the driver to be used without enable or regulator then it's ok to wait unconditionally (all examples you have added so far have an enable pin). Daniel. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v5 2/4] backlight: add max25014atg backlight 2025-11-10 10:01 ` Daniel Thompson @ 2025-11-10 10:03 ` Maud Spierings 2025-11-10 10:19 ` Daniel Thompson 0 siblings, 1 reply; 19+ messages in thread From: Maud Spierings @ 2025-11-10 10:03 UTC (permalink / raw) To: Daniel Thompson Cc: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Helge Deller, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Liam Girdwood, Mark Brown, dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, imx, linux-arm-kernel On 11/10/25 11:01, Daniel Thompson wrote: > On Mon, Nov 10, 2025 at 09:40:07AM +0100, Maud Spierings wrote: >> On 11/7/25 17:14, Daniel Thompson wrote: >>> On Fri, Nov 07, 2025 at 01:49:59PM +0100, Maud Spierings via B4 Relay wrote: >>>> +/** >>>> + * @brief control the brightness with i2c registers >>>> + * >>>> + * @param regmap trivial >>>> + * @param brt brightness >>>> + * @return int >>>> + */ >>>> +static int max25014_register_control(struct regmap *regmap, uint32_t brt) >>> >>> This isn't a good name for a function. It doesn't really say what it >>> does. Please find a more descriptive name. >> >> Having a lot of difficulties find a succinct name that fits better, >> max25014_register_brightness_control()? >> max25014_i2c_brightness_control()? > > I'd focus on what it does rather than how it does it meaning something > like max25014_update_brightness() would work. > > However, at present, this code is only called from > max25014_update_status() so the simplest thing to do is to move the > code into max25014_update_status() and remove this function entirely > (then it doesn't matter what it is called ;-) ). > Perhaps this could be seperated out if/when pwm functionality is implemented. I believe the brightness may also be controlled that way in hybrid mode, but I am not entirely sure. > >>>> +/* >>>> + * 1. disable unused strings >>>> + * 2. set dim mode >>>> + * 3. set initial brightness >>> >>> How does this code set the initial brightness? It doens't set the >>> MAX25014_TON* registers. >> >> Yep forgot to remove that, I discovered the backlight core takes care of the >> default brightness, so I removed it from here. > > What do you mean by this? Are you sure you aren't relying on another > driver to enable the backlight rather than the backlight core? Not that I know of, there is the systemd backlight service, but I am pretty sure I can see it first turn on, then get switched to the old value by the systemd service. Unless the simple-panel driver controls it? The backlight is linked to that. >>>> + * 4. set setting register >>>> + * 5. enable the backlight >>>> + */ >>>> +static int max25014_configure(struct max25014 *maxim) > > >>>> +static int max25014_probe(struct i2c_client *cl) >>>> <snip> >>>> + >>>> + /* Enable can be tied to vin rail wait if either is available */ >>>> + if (maxim->enable || maxim->vin) { >>>> + /* Datasheet Electrical Characteristics tSTARTUP 2ms */ >>>> + usleep_range(2000, 2500); >>>> + } >>> >>> If you really want to keep the devm_regulator_get_optional() I guess >>> maybe you could persuade me it's need to avoid this sleep... although >>> I'd be fairly happy to remove the NULL checks here too! >> >> Just wait unconditionally? > > If you think it will be unusual for the driver to be used without enable > or regulator then it's ok to wait unconditionally (all examples you > have added so far have an enable pin). I think it may actually be a very common implementation to have the enable pin attached to Vin, we don't have it set up that way. But it is displayed that way in an example schematic in the datasheet. Kind regards, Maud ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v5 2/4] backlight: add max25014atg backlight 2025-11-10 10:03 ` Maud Spierings @ 2025-11-10 10:19 ` Daniel Thompson 0 siblings, 0 replies; 19+ messages in thread From: Daniel Thompson @ 2025-11-10 10:19 UTC (permalink / raw) To: Maud Spierings Cc: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Helge Deller, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Liam Girdwood, Mark Brown, dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, imx, linux-arm-kernel On Mon, Nov 10, 2025 at 11:03:27AM +0100, Maud Spierings wrote: > On 11/10/25 11:01, Daniel Thompson wrote: > > On Mon, Nov 10, 2025 at 09:40:07AM +0100, Maud Spierings wrote: > > > On 11/7/25 17:14, Daniel Thompson wrote: > > > > On Fri, Nov 07, 2025 at 01:49:59PM +0100, Maud Spierings via B4 Relay wrote: > > > > > +/* > > > > > + * 1. disable unused strings > > > > > + * 2. set dim mode > > > > > + * 3. set initial brightness > > > > > > > > How does this code set the initial brightness? It doens't set the > > > > MAX25014_TON* registers. > > > > > > Yep forgot to remove that, I discovered the backlight core takes care of the > > > default brightness, so I removed it from here. > > > > What do you mean by this? Are you sure you aren't relying on another > > driver to enable the backlight rather than the backlight core? > > Not that I know of, there is the systemd backlight service, but I am pretty > sure I can see it first turn on, then get switched to the old value by the > systemd service. Unless the simple-panel driver controls it? The backlight > is linked to that. I think you should look at the code. I think it's likely the backlight is only coming on due to the link to simple-panel. Normal way to handle that case (if you want to avoid the backlight turning on "too early") is to set the power mode to BACKLIGHT_POWER_OFF if (and only off) the backlight is linked to a panel. See pwm_backlight_initial_power_state() for an example. If you are relying on "the backlight core [to take] care of the default brightness" then you have to request it in the driver (by calling backlight_update_status() after registering the backlight). > > > > > + * 4. set setting register > > > > > + * 5. enable the backlight > > > > > + */ > > > > > +static int max25014_configure(struct max25014 *maxim) > > > > > > > > > +static int max25014_probe(struct i2c_client *cl) > > > > > <snip> > > > > > + > > > > > + /* Enable can be tied to vin rail wait if either is available */ > > > > > + if (maxim->enable || maxim->vin) { > > > > > + /* Datasheet Electrical Characteristics tSTARTUP 2ms */ > > > > > + usleep_range(2000, 2500); > > > > > + } > > > > > > > > If you really want to keep the devm_regulator_get_optional() I guess > > > > maybe you could persuade me it's need to avoid this sleep... although > > > > I'd be fairly happy to remove the NULL checks here too! > > > > > > Just wait unconditionally? > > > > If you think it will be unusual for the driver to be used without enable > > or regulator then it's ok to wait unconditionally (all examples you > > have added so far have an enable pin). > > I think it may actually be a very common implementation to have the enable > pin attached to Vin, we don't have it set up that way. But it is displayed > that way in an example schematic in the datasheet. Your call. Daniel. ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v5 3/4] arm64: dts: freescale: moduline-display-av101hdt-a10: add backlight 2025-11-07 12:49 [PATCH v5 0/4] backlight: add new max25014 backlight driver Maud Spierings via B4 Relay 2025-11-07 12:49 ` [PATCH v5 1/4] dt-bindings: backlight: Add max25014 support Maud Spierings via B4 Relay 2025-11-07 12:49 ` [PATCH v5 2/4] backlight: add max25014atg backlight Maud Spierings via B4 Relay @ 2025-11-07 12:50 ` Maud Spierings via B4 Relay 2025-11-07 12:50 ` [PATCH v5 4/4] arm64: dts: freescale: moduline-display-av123z7m-n17: " Maud Spierings via B4 Relay 3 siblings, 0 replies; 19+ messages in thread From: Maud Spierings via B4 Relay @ 2025-11-07 12:50 UTC (permalink / raw) To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Helge Deller, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Liam Girdwood, Mark Brown Cc: dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, imx, linux-arm-kernel, Maud Spierings From: Maud Spierings <maudspierings@gocontroll.com> Add the missing backlight driver. Signed-off-by: Maud Spierings <maudspierings@gocontroll.com> --- ...x8p-ml81-moduline-display-106-av101hdt-a10.dtso | 30 ++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/arch/arm64/boot/dts/freescale/imx8mp-tx8p-ml81-moduline-display-106-av101hdt-a10.dtso b/arch/arm64/boot/dts/freescale/imx8mp-tx8p-ml81-moduline-display-106-av101hdt-a10.dtso index e3965caca6be..0e2914861154 100644 --- a/arch/arm64/boot/dts/freescale/imx8mp-tx8p-ml81-moduline-display-106-av101hdt-a10.dtso +++ b/arch/arm64/boot/dts/freescale/imx8mp-tx8p-ml81-moduline-display-106-av101hdt-a10.dtso @@ -17,6 +17,7 @@ panel { compatible = "boe,av101hdt-a10"; + backlight = <&backlight>; enable-gpios = <&gpio1 7 GPIO_ACTIVE_HIGH>; pinctrl-0 = <&pinctrl_panel>; pinctrl-names = "default"; @@ -40,7 +41,36 @@ reg_vbus: regulator-vbus { }; }; +&i2c4 { + #address-cells = <1>; + #size-cells = <0>; + + backlight: backlight@6f { + compatible = "maxim,max25014"; + reg = <0x6f>; + #address-cells = <1>; + #size-cells = <0>; + enable-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_backlight>; + maxim,iset = <7>; + + led@0 { + reg = <0>; + led-sources = <0 1 2>; + default-brightness = <50>; + }; + }; +}; + &iomuxc { + pinctrl_backlight: backlightgrp { + fsl,pins = < + MX8MP_IOMUXC_GPIO1_IO04__GPIO1_IO04 + (MX8MP_PULL_UP | MX8MP_PULL_ENABLE) + >; + }; + pinctrl_panel: panelgrp { fsl,pins = < MX8MP_IOMUXC_GPIO1_IO07__GPIO1_IO07 -- 2.51.2 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v5 4/4] arm64: dts: freescale: moduline-display-av123z7m-n17: add backlight 2025-11-07 12:49 [PATCH v5 0/4] backlight: add new max25014 backlight driver Maud Spierings via B4 Relay ` (2 preceding siblings ...) 2025-11-07 12:50 ` [PATCH v5 3/4] arm64: dts: freescale: moduline-display-av101hdt-a10: add backlight Maud Spierings via B4 Relay @ 2025-11-07 12:50 ` Maud Spierings via B4 Relay 3 siblings, 0 replies; 19+ messages in thread From: Maud Spierings via B4 Relay @ 2025-11-07 12:50 UTC (permalink / raw) To: Lee Jones, Daniel Thompson, Jingoo Han, Pavel Machek, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Helge Deller, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Liam Girdwood, Mark Brown Cc: dri-devel, linux-leds, devicetree, linux-kernel, linux-fbdev, imx, linux-arm-kernel, Maud Spierings From: Maud Spierings <maudspierings@gocontroll.com> Add the missing backlight. Signed-off-by: Maud Spierings <maudspierings@gocontroll.com> --- ...x8p-ml81-moduline-display-106-av123z7m-n17.dtso | 25 +++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/arch/arm64/boot/dts/freescale/imx8mp-tx8p-ml81-moduline-display-106-av123z7m-n17.dtso b/arch/arm64/boot/dts/freescale/imx8mp-tx8p-ml81-moduline-display-106-av123z7m-n17.dtso index 3eb665ce9d5d..786a04ef40c8 100644 --- a/arch/arm64/boot/dts/freescale/imx8mp-tx8p-ml81-moduline-display-106-av123z7m-n17.dtso +++ b/arch/arm64/boot/dts/freescale/imx8mp-tx8p-ml81-moduline-display-106-av123z7m-n17.dtso @@ -16,6 +16,7 @@ panel { compatible = "boe,av123z7m-n17"; + backlight = <&backlight>; enable-gpios = <&gpio1 7 GPIO_ACTIVE_HIGH>; pinctrl-0 = <&pinctrl_panel>; pinctrl-names = "default"; @@ -91,10 +92,32 @@ lvds1_out: endpoint { }; }; - /* max25014 @ 0x6f */ + backlight: backlight@6f { + compatible = "maxim,max25014"; + reg = <0x6f>; + #address-cells = <1>; + #size-cells = <0>; + enable-gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_backlight>; + maxim,iset = <7>; + + led@0 { + reg = <0>; + led-sources = <0 1 2 3>; + default-brightness = <50>; + }; + }; }; &iomuxc { + pinctrl_backlight: backlightgrp { + fsl,pins = < + MX8MP_IOMUXC_GPIO1_IO04__GPIO1_IO04 + (MX8MP_PULL_UP | MX8MP_PULL_ENABLE) + >; + }; + pinctrl_lvds_bridge: lvdsbridgegrp { fsl,pins = < MX8MP_IOMUXC_SAI1_TXD2__GPIO4_IO14 -- 2.51.2 ^ permalink raw reply related [flat|nested] 19+ messages in thread
end of thread, other threads:[~2025-11-10 10:17 UTC | newest] Thread overview: 19+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-11-07 12:49 [PATCH v5 0/4] backlight: add new max25014 backlight driver Maud Spierings via B4 Relay 2025-11-07 12:49 ` [PATCH v5 1/4] dt-bindings: backlight: Add max25014 support Maud Spierings via B4 Relay 2025-11-07 15:35 ` [PATCH v5 1/4] dt-bindings: backlight: Add max25014 supporty Frank Li 2025-11-07 17:14 ` Conor Dooley 2025-11-10 7:59 ` Maud Spierings 2025-11-07 18:16 ` [PATCH v5 1/4] dt-bindings: backlight: Add max25014 support Conor Dooley 2025-11-10 7:55 ` Maud Spierings 2025-11-07 12:49 ` [PATCH v5 2/4] backlight: add max25014atg backlight Maud Spierings via B4 Relay 2025-11-07 15:51 ` [PATCH v5 2/4] backlight: add max25014atg backlighty Frank Li 2025-11-07 16:22 ` Daniel Thompson 2025-11-07 20:42 ` Frank Li 2025-11-10 8:25 ` Maud Spierings 2025-11-07 16:14 ` [PATCH v5 2/4] backlight: add max25014atg backlight Daniel Thompson 2025-11-10 8:40 ` Maud Spierings 2025-11-10 10:01 ` Daniel Thompson 2025-11-10 10:03 ` Maud Spierings 2025-11-10 10:19 ` Daniel Thompson 2025-11-07 12:50 ` [PATCH v5 3/4] arm64: dts: freescale: moduline-display-av101hdt-a10: add backlight Maud Spierings via B4 Relay 2025-11-07 12:50 ` [PATCH v5 4/4] arm64: dts: freescale: moduline-display-av123z7m-n17: " Maud Spierings via B4 Relay
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).