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

[Lee Jones <lee@kernel.org>]

On Wed, 02 Aug 2023, Marek Behún wrote:

> 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...

I suspect you know more than I do at this point.  I could take a
deep-dive into the code however a) I'm presently swamped with incoming
reviews and b) I do not have any additional resources at my disposable
than you do - it would consist of reading through and brain-grepping the
code.

Pavel or Jacek may have more knowledge at-hand though.

-- 
Lee Jones [李琼斯]