Hi Marek,
On 8/2/23 18:13, Marek Behún wrote:
On Wed, 2 Aug 2023 18:07:47 +0200
Marek Behún <kabel@xxxxxxxxxx> 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...
In general led_set_brightness() can't sleep because it is called from
triggers in atomic contexts, which shouldn't be the case for activate().
.activate() is called from led_trigger_{set|remove}() which is called
from led_classdev_{register|unregister}) and from sysfs trigger attr's
led_trigger_write() handler, so no risk here.
Triggers already call e.g. kzalloc() in .activate() which may sleep.
--
Best regards,
Jacek Anaszewski
