Am 25.03.23 um 16:16 schrieb Andrew Kallmeyer:
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.
I think it is best to still provide this module param for people who need
it, but only allow enabling it when loading the module. This way userspace
should not be able to read/write ec_trigger after the module has been loaded.
Because of this, i believe the ACPI device should only be initialized when
the DMI table says its necessary or ec_trigger was set. So the current solution
is fine for me, except for ec_trigger being accessible to userspace after
the module was loaded.
Armin Wolf
