Re: [PATCH v3 5/6] leds: turris-omnia: support HW controlled mode via private trigger

["Marek Behún" <kabel@kernel.org>]

On Wed,  2 Aug 2023 18:07:47 +0200
Marek Behún <kabel@kernel.org> wrote:

> +static int omnia_hwtrig_activate(struct led_classdev *cdev)
> +{
> +	struct led_classdev_mc *mc_cdev = lcdev_to_mccdev(cdev);
> +	struct omnia_leds *leds = dev_get_drvdata(cdev->dev->parent);
> +	struct omnia_led *led = to_omnia_led(mc_cdev);
> +	int err = 0;
> +
> +	mutex_lock(&leds->lock);
> +
> +	if (!led->on) {
> +		/*
> +		 * If the LED is off (brightness was set to 0), the last
> +		 * configured color was not necessarily sent to the MCU.
> +		 * Recompute with max_brightness and send if needed.
> +		 */
> +		led_mc_calc_color_components(mc_cdev, cdev->max_brightness);
> +
> +		if (omnia_led_channels_changed(led))
> +			err = omnia_led_send_color_cmd(leds->client, led);
> +	}
> +
> +	if (!err) {
> +		/* put the LED into MCU controlled mode */
> +		err = omnia_cmd_write(leds->client, CMD_LED_MODE,
> +				      CMD_LED_MODE_LED(led->reg));
> +		if (!err)
> +			led->hwtrig = true;
> +	}
> +
> +	mutex_unlock(&leds->lock);
> +
> +	return err;
> +}

Pavel, Lee, here I wanted to ask: can the .activate() method of a LED
trigger sleep? The .brightness_set() method of a LED cannot sleep, and
therefore we have .brightness_set_blocking() method, which schedules
the potentially sleeping call into a work. But there is no such
mechanism for the LED trigger .activate() method.

I looked at the LED core code, and it does not seem to me that
.activate() sleeping would cause issues, besides keeping trigger list
lock locked...

Note that previously none of the LED triggers in drivers/leds/trigger
sleeped in .activate(), but recently the ethernet PHY subsystem was
wired into the netdev trigger, which may cause the .activate() method
to do a PHY transfer, which AFAIK may sleep...

Marek