Message-ID:
 <CWLP123MB54739F38EF9CFA057021BC2DC5C22@CWLP123MB5473.GBRP123.PROD.OUTLOOK.COM>
Date: Wed, 26 Feb 2025 17:04:40 +0000
From: Manuel Fombuena <fombuena@...look.com>
To: pavel@....cz,
	lee@...nel.org,
	linux-leds@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: [PATCH 0/3] leds-st1202 improvements

Following the feedback received on the set of patches for leds-st1202 that
I sent previously, I am sending separately for your consideration those
that are not fixes with an improved cover letter that hopefully is more
aligned with the guidelines.

[PATCH 1/3] leds: leds-st1202: initialize hardware before DT node

The purpose of this change is to make leds-st1202 initialization more
robust. The underlying idea is that a hardware initialization
failure is somewhat more likely to happen than merely filling a
led_classdev struct.

Currently, st1202_dt_init() is called first to fill the led_classdev
struct. Afterwards, st1202_setup() is called to initialize the hardware,
but it doesn't require any return from st1202_dt_init() or depends on
anything done by it to do that. This has been inferred reviewing the code
and corroborated with testing on an actual device switching the order
of the calls.

The present calling order can lead to a situation in which the hardware
fails to initialize while the led_classdev struct has been filled.

It is important to note that this situation would be more severe if a
patch that I submitted previously [1] was not applied, as in that case
devm_led_classdev_register_ext would be in st1202_dt_init and a later
failure on st1202_setup would mean that the kobjects are available in user
space but the hardware is in failed state.

While I think this change makes an improvement and I can't foresee any
issue, not applying the patch would have no consequences under normal
circumstances.

[1] https://lore.kernel.org/all/CWLP123MB54732771AC0CE5491B3C84DCC5C32@CWLP123MB5473.GBRP123.PROD.OUTLOOK.COM/

[PATCH 2/3] leds: leds-st1202: spacing and proofreading editing

These are minor changes to polish the format of a comment for consistency,
correct a typo that comes straight from the datasheet, and add punctuation
for readability.

There would be no consequences if the patch was not applied.

[PATCH 3/3] leds: Kconfig: leds-st1202: add select for required
LEDS_TRIGGER_PATTERN

leds-st1202 requires the LED pattern trigger to work. Without it, there
would be no /sys/class/leds/<led>/hw_pattern to write the patterns to and
no way to interact with the LEDs.

With such requirement, it doesn't feel prudent to leave the selection
of LEDS_TRIGGER_PATTERN effectively on it being already selected,
since nothing on leds-st1202's Kconfig does it.

To reproduce the potential issue that I am trying to explain:

- make menuconfig KCONFIG_CONFIG=
- select LEDS_ST1202 dependencies OF, I2C and LEDS_CLASS.
- select LEDS_ST1202.
- LEDS_TRIGGERS is selected but LEDS_TRIGGER_PATTERN isn't.

Without this requirement explicitly selected, the resulting kernel won't
include the LED pattern trigger so the consequence of not applying
this patch would be that downstream projects and users would need to
figure the requirement out, if their starting base .config file doesn't
have LEDS_TRIGGER_PATTERN already selected.

Manuel Fombuena (3):
  leds: leds-st1202: initialize hardware before DT node child operations
  leds: leds-st1202: spacing and proofreading editing
  leds: Kconfig: leds-st1202: add select for required
    LEDS_TRIGGER_PATTERN

 drivers/leds/Kconfig       |  1 +
 drivers/leds/leds-st1202.c | 14 +++++++-------
 2 files changed, 8 insertions(+), 7 deletions(-)

-- 
2.48.1

Powered by blists - more mailing lists