On Tue, Aug 01, 2023 at 05:16:23PM +0200, Marek Beh'un wrote:
> At the time we call
> BUG_ON(props.color == LED_COLOR_ID_MULTI);
> the props variable is still initialized to zero.
>
> Call the BUG_ON only after we parse fwnode into props.
>
> Fixes: 77dce3a22e89 ("leds: disallow /sys/class/leds/*:multi:* for now")
> Signed-off-by: Marek Beh'un <kabel@xxxxxxxxxx>
I've just discovered this has broken boot on my Libre Computer
AML-A311D-CC-V0.2, which was working just fine with Debian 12's stock kernel:
mark@stor-flodeboller:~$ uname -a
Linux stor-flodeboller 6.1.0-12-arm64 #1 SMP Debian 6.1.52-1 (2023-09-07) aarch64 GNU/Linux
When upgrading to v6.6-rc3 for testing, the board dies at boot time due to the
BUG_ON() moved by this commit. That BUG_ON() provides no useful context for
solving the issue, and it's *distinctly* unhelpful.
I decompiled the DTB provided by the firmware, and it has:
| leds {
| compatible = "gpio-leds";
|
| led-yellow-green {
| color = <0x08>;
| function = "status";
| gpios = <0x16 0x44 0x01>;
| linux,default-trigger = "default-on";
| panic-indicator;
| };
|
| led-blue {
| color = <0x03>;
| function = "activity";
| gpios = <0x16 0x48 0x01>;
| linux,default-trigger = "activity";
| };
| };
... and IIUC LED_COLOR_ID_MULTI is 8, so the problem is the led-yellow-green
subnode. I presume that could use LED_COLOR_ID_RGB.
Can we please do something so that this doesn't prevent the board from booting?
Is there some reason we don't transparently convert LED_COLOR_ID_MULTI to
LED_COLOR_ID_RGB?
If that's not viable, could we skip the LED *without* crashing the kernel, so
that the board remains usable?
Thanks,
Mark.
> ---
> drivers/leds/led-core.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
> index b9b1295833c9..04f9ea675f2c 100644
> --- a/drivers/leds/led-core.c
> +++ b/drivers/leds/led-core.c
> @@ -474,15 +474,15 @@ int led_compose_name(struct device *dev, struct led_init_data *init_data,
> struct fwnode_handle *fwnode = init_data->fwnode;
> const char *devicename = init_data->devicename;
>
> - /* We want to label LEDs that can produce full range of colors
> - * as RGB, not multicolor */
> - BUG_ON(props.color == LED_COLOR_ID_MULTI);
> -
> if (!led_classdev_name)
> return -EINVAL;
>
> led_parse_fwnode_props(dev, fwnode, &props);
>
> + /* We want to label LEDs that can produce full range of colors
> + * as RGB, not multicolor */
> + BUG_ON(props.color == LED_COLOR_ID_MULTI);
> +
> if (props.label) {
> /*
> * If init_data.devicename is NULL, then it indicates that
> --
> 2.41.0
>
>
