On Mon, 21 Aug 2023, Marek Behún wrote:
> On Fri, 18 Aug 2023 10:09:21 +0100
> Lee Jones <lee@xxxxxxxxxx> wrote:
>
> > > + if (!err) {
> > > + /* put the LED into MCU controlled mode */
> >
> > Nit: You improved the comment above to be more grammatically correct by
> > starting with an uppercase character. Please continue with this
> > improvement for all comments there in.
>
> OK.
>
> > > +static struct led_trigger omnia_hw_trigger = {
> > > + .name = "omnia-mcu",
> > > + .activate = omnia_hwtrig_activate,
> > > + .deactivate = omnia_hwtrig_deactivate,
> > > + .trigger_type = &omnia_hw_trigger_type,
> >
> > Not sure I understand this interface.
> >
> > Why not just set a bool instead of carrying around an empty struct?
>
> Let me explain:
>
> So, if a LED class device has the same trigger type as a LED trigger,
> the trigger will be available for that LED (it is listed in the sysfs
> trigger file and can be chosen).
>
> So we have a mechanism to "pair" a LED with a given trigger, to make it
> possible for the LED core to distinguish whether a given trigger is
> available for the LED.
>
> A boolean information would not be enough: if we used a bool, we would
> know that the trigger is private. But the LED core would not know for
> which LEDs the trigger should be avaiable.
>
> In pseudocode:
>
> list_triggers_for_led(led) {
> for (trigger in trigger_list) {
> if (!trigger.trigger_type || trigger.trigger_type ==
> led.trigger_type)
> trigger is available for led
> else
> trigger is not available for led
> }
> }
>
> Is this explaination good enough?
Yes, thank you.
--
Lee Jones [李琼斯]
