* [PATCH 0/2] leds: pwm: Add default-brightness property @ 2024-10-15 15:14 George Stark 2024-10-15 15:14 ` [PATCH 1/2] dt-bindings: " George Stark 2024-10-15 15:14 ` [PATCH 2/2] leds: pwm: Add optional DT property default-brightness George Stark 0 siblings, 2 replies; 6+ messages in thread From: George Stark @ 2024-10-15 15:14 UTC (permalink / raw) To: pavel, lee, robh, krzk+dt, conor+dt Cc: linux-leds, devicetree, linux-kernel, kernel, George Stark led-pwm driver supports default-state DT property and if that state is on then the driver during initialization turns on the LED setting maximum brightness. Sometimes it's desirable to use lower initial brightness. This patch series adds support for DT property default-brightness. Patch series is prepared for linux-next Things to discuss: If such a property is acceptable it could be moved to leds/common.yaml due to several drivers support multiple brightness levels and could support the property too. George Stark (2): dt-bindings: leds: pwm: Add default-brightness property leds: pwm: Add optional DT property default-brightness .../devicetree/bindings/leds/leds-pwm.yaml | 6 ++++++ drivers/leds/leds-pwm.c | 13 ++++++++++--- 2 files changed, 16 insertions(+), 3 deletions(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] dt-bindings: leds: pwm: Add default-brightness property 2024-10-15 15:14 [PATCH 0/2] leds: pwm: Add default-brightness property George Stark @ 2024-10-15 15:14 ` George Stark 2024-10-15 22:11 ` Rob Herring (Arm) 2024-10-15 15:14 ` [PATCH 2/2] leds: pwm: Add optional DT property default-brightness George Stark 1 sibling, 1 reply; 6+ messages in thread From: George Stark @ 2024-10-15 15:14 UTC (permalink / raw) To: pavel, lee, robh, krzk+dt, conor+dt Cc: linux-leds, devicetree, linux-kernel, kernel, George Stark Optional default-brightness property specifies brightness value to be used if default LED state is on. Signed-off-by: George Stark <gnstark@salutedevices.com> --- Documentation/devicetree/bindings/leds/leds-pwm.yaml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Documentation/devicetree/bindings/leds/leds-pwm.yaml b/Documentation/devicetree/bindings/leds/leds-pwm.yaml index 113b7c218303..61b97e8bc36d 100644 --- a/Documentation/devicetree/bindings/leds/leds-pwm.yaml +++ b/Documentation/devicetree/bindings/leds/leds-pwm.yaml @@ -34,6 +34,12 @@ patternProperties: Maximum brightness possible for the LED $ref: /schemas/types.yaml#/definitions/uint32 + default-brightness: + description: + Brightness to be set if LED's default state is on. Used only during + initialization. If the option is not set then max brightness is used. + $ref: /schemas/types.yaml#/definitions/uint32 + required: - pwms - max-brightness -- 2.25.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] dt-bindings: leds: pwm: Add default-brightness property 2024-10-15 15:14 ` [PATCH 1/2] dt-bindings: " George Stark @ 2024-10-15 22:11 ` Rob Herring (Arm) 0 siblings, 0 replies; 6+ messages in thread From: Rob Herring (Arm) @ 2024-10-15 22:11 UTC (permalink / raw) To: George Stark Cc: conor+dt, pavel, linux-kernel, kernel, linux-leds, devicetree, krzk+dt, lee On Tue, 15 Oct 2024 18:14:09 +0300, George Stark wrote: > Optional default-brightness property specifies brightness value to be > used if default LED state is on. > > Signed-off-by: George Stark <gnstark@salutedevices.com> > --- > Documentation/devicetree/bindings/leds/leds-pwm.yaml | 6 ++++++ > 1 file changed, 6 insertions(+) > Acked-by: Rob Herring (Arm) <robh@kernel.org> ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] leds: pwm: Add optional DT property default-brightness 2024-10-15 15:14 [PATCH 0/2] leds: pwm: Add default-brightness property George Stark 2024-10-15 15:14 ` [PATCH 1/2] dt-bindings: " George Stark @ 2024-10-15 15:14 ` George Stark 2024-10-31 14:32 ` Lee Jones 1 sibling, 1 reply; 6+ messages in thread From: George Stark @ 2024-10-15 15:14 UTC (permalink / raw) To: pavel, lee, robh, krzk+dt, conor+dt Cc: linux-leds, devicetree, linux-kernel, kernel, George Stark When probing if default LED state is on then default brightness will be applied instead of max brightness. Signed-off-by: George Stark <gnstark@salutedevices.com> --- drivers/leds/leds-pwm.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c index 7961dca0db2f..514fc8ca3e80 100644 --- a/drivers/leds/leds-pwm.c +++ b/drivers/leds/leds-pwm.c @@ -65,7 +65,8 @@ static int led_pwm_set(struct led_classdev *led_cdev, __attribute__((nonnull)) static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv, - struct led_pwm *led, struct fwnode_handle *fwnode) + struct led_pwm *led, struct fwnode_handle *fwnode, + unsigned int default_brightness) { struct led_pwm_data *led_data = &priv->leds[priv->num_leds]; struct led_init_data init_data = { .fwnode = fwnode }; @@ -104,7 +105,7 @@ static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv, /* set brightness */ switch (led->default_state) { case LEDS_DEFSTATE_ON: - led_data->cdev.brightness = led->max_brightness; + led_data->cdev.brightness = default_brightness; break; case LEDS_DEFSTATE_KEEP: { @@ -141,6 +142,7 @@ static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv, static int led_pwm_create_fwnode(struct device *dev, struct led_pwm_priv *priv) { struct led_pwm led; + unsigned int default_brightness; int ret; device_for_each_child_node_scoped(dev, fwnode) { @@ -160,7 +162,12 @@ static int led_pwm_create_fwnode(struct device *dev, struct led_pwm_priv *priv) led.default_state = led_init_default_state_get(fwnode); - ret = led_pwm_add(dev, priv, &led, fwnode); + ret = fwnode_property_read_u32(fwnode, "default-brightness", + &default_brightness); + if (ret < 0 || default_brightness > led.max_brightness) + default_brightness = led.max_brightness; + + ret = led_pwm_add(dev, priv, &led, fwnode, default_brightness); if (ret) return ret; } -- 2.25.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] leds: pwm: Add optional DT property default-brightness 2024-10-15 15:14 ` [PATCH 2/2] leds: pwm: Add optional DT property default-brightness George Stark @ 2024-10-31 14:32 ` Lee Jones 2024-11-01 15:42 ` George Stark 0 siblings, 1 reply; 6+ messages in thread From: Lee Jones @ 2024-10-31 14:32 UTC (permalink / raw) To: George Stark Cc: pavel, robh, krzk+dt, conor+dt, linux-leds, devicetree, linux-kernel, kernel On Tue, 15 Oct 2024, George Stark wrote: > When probing if default LED state is on then default brightness will be > applied instead of max brightness. > > Signed-off-by: George Stark <gnstark@salutedevices.com> > --- > drivers/leds/leds-pwm.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c > index 7961dca0db2f..514fc8ca3e80 100644 > --- a/drivers/leds/leds-pwm.c > +++ b/drivers/leds/leds-pwm.c > @@ -65,7 +65,8 @@ static int led_pwm_set(struct led_classdev *led_cdev, > > __attribute__((nonnull)) > static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv, > - struct led_pwm *led, struct fwnode_handle *fwnode) > + struct led_pwm *led, struct fwnode_handle *fwnode, > + unsigned int default_brightness) > { > struct led_pwm_data *led_data = &priv->leds[priv->num_leds]; > struct led_init_data init_data = { .fwnode = fwnode }; > @@ -104,7 +105,7 @@ static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv, > /* set brightness */ > switch (led->default_state) { > case LEDS_DEFSTATE_ON: > - led_data->cdev.brightness = led->max_brightness; > + led_data->cdev.brightness = default_brightness; > break; > case LEDS_DEFSTATE_KEEP: > { > @@ -141,6 +142,7 @@ static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv, > static int led_pwm_create_fwnode(struct device *dev, struct led_pwm_priv *priv) > { > struct led_pwm led; > + unsigned int default_brightness; > int ret; > > device_for_each_child_node_scoped(dev, fwnode) { > @@ -160,7 +162,12 @@ static int led_pwm_create_fwnode(struct device *dev, struct led_pwm_priv *priv) > > led.default_state = led_init_default_state_get(fwnode); > > - ret = led_pwm_add(dev, priv, &led, fwnode); > + ret = fwnode_property_read_u32(fwnode, "default-brightness", > + &default_brightness); > + if (ret < 0 || default_brightness > led.max_brightness) > + default_brightness = led.max_brightness; > + > + ret = led_pwm_add(dev, priv, &led, fwnode, default_brightness); This creates a lot more hopping around than is necessary. Since led_pwm_add() already has access to the fwnode, why not look up the property in there instead, thus massively simplifying things. > if (ret) > return ret; > } > -- > 2.25.1 > -- Lee Jones [李琼斯] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] leds: pwm: Add optional DT property default-brightness 2024-10-31 14:32 ` Lee Jones @ 2024-11-01 15:42 ` George Stark 0 siblings, 0 replies; 6+ messages in thread From: George Stark @ 2024-11-01 15:42 UTC (permalink / raw) To: Lee Jones Cc: pavel, robh, krzk+dt, conor+dt, linux-leds, devicetree, linux-kernel, kernel Hello Lee Thanks for the review! On 10/31/24 17:32, Lee Jones wrote: > On Tue, 15 Oct 2024, George Stark wrote: > >> When probing if default LED state is on then default brightness will be >> applied instead of max brightness. >> >> Signed-off-by: George Stark <gnstark@salutedevices.com> >> --- >> drivers/leds/leds-pwm.c | 13 ++++++++++--- >> 1 file changed, 10 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c >> index 7961dca0db2f..514fc8ca3e80 100644 >> --- a/drivers/leds/leds-pwm.c >> +++ b/drivers/leds/leds-pwm.c >> @@ -65,7 +65,8 @@ static int led_pwm_set(struct led_classdev *led_cdev, >> >> __attribute__((nonnull)) >> static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv, >> - struct led_pwm *led, struct fwnode_handle *fwnode) >> + struct led_pwm *led, struct fwnode_handle *fwnode, >> + unsigned int default_brightness) >> { >> struct led_pwm_data *led_data = &priv->leds[priv->num_leds]; >> struct led_init_data init_data = { .fwnode = fwnode }; >> @@ -104,7 +105,7 @@ static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv, >> /* set brightness */ >> switch (led->default_state) { >> case LEDS_DEFSTATE_ON: >> - led_data->cdev.brightness = led->max_brightness; >> + led_data->cdev.brightness = default_brightness; >> break; >> case LEDS_DEFSTATE_KEEP: >> { >> @@ -141,6 +142,7 @@ static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv, >> static int led_pwm_create_fwnode(struct device *dev, struct led_pwm_priv *priv) >> { >> struct led_pwm led; >> + unsigned int default_brightness; >> int ret; >> >> device_for_each_child_node_scoped(dev, fwnode) { >> @@ -160,7 +162,12 @@ static int led_pwm_create_fwnode(struct device *dev, struct led_pwm_priv *priv) >> >> led.default_state = led_init_default_state_get(fwnode); >> >> - ret = led_pwm_add(dev, priv, &led, fwnode); >> + ret = fwnode_property_read_u32(fwnode, "default-brightness", >> + &default_brightness); >> + if (ret < 0 || default_brightness > led.max_brightness) >> + default_brightness = led.max_brightness; >> + >> + ret = led_pwm_add(dev, priv, &led, fwnode, default_brightness); > > This creates a lot more hopping around than is necessary. > > Since led_pwm_add() already has access to the fwnode, why not look up > the property in there instead, thus massively simplifying things. I looked up the new property here to be near to led_init_default_state_get (both props are from the same group) and led_pwm_add is big enough already. And you're absolutely right that the patch can be optimized. Please take a look at the v2 > > >> if (ret) >> return ret; >> } >> -- >> 2.25.1 >> > -- Best regards George ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-11-01 15:42 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-10-15 15:14 [PATCH 0/2] leds: pwm: Add default-brightness property George Stark 2024-10-15 15:14 ` [PATCH 1/2] dt-bindings: " George Stark 2024-10-15 22:11 ` Rob Herring (Arm) 2024-10-15 15:14 ` [PATCH 2/2] leds: pwm: Add optional DT property default-brightness George Stark 2024-10-31 14:32 ` Lee Jones 2024-11-01 15:42 ` George Stark
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).