On Sat, Mar 25, 2023 at 12:10 AM Armin Wolf <W_Armin@xxxxxx> wrote: > > Hi, > > is it really necessary to allow userspace to read/write ec_trigger? The ACPI device > used for triggering the EC is only initialized during probing, so changing the value > of ec_trigger will make no difference in such cases. > > Maybe you could change module_param(ec_trigger, bool, 0644) to module_param(ec_trigger, bool, 0)? > > Armin Wolf Great point, this is actually a regression from Gergo's original patch that I didn't realize I caused. I believe the intention was that if the quirk detection code doesn't have full coverage users can set the parameter themselves. In Gergo's code it used the acpi_device from ideapad-laptop.c which is always loaded if it exists. Now I only load it if ec_trigger is true at probe time, I think I should update it to load the acpi device always if it exists so that the user can set this parameter at any time. I suppose I would just remove the if (ec_trigger) (and the debug print) in the probe code when I load it. That is, unless you think it is best to just patch in more models to the quirk detection later and not provide a parameter. Barnabás actually suggested removing the ec_trigger flag completely because right now the code isn't relying on it, but I think that is a bug.
