| Message ID | 20240713075940.80073-3-luke@ljones.dev (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | platform/x86: asus-wmi: extra debugging and a fix | expand |
On Sat, 13 Jul 2024, Luke D. Jones wrote: > On some newer laptops it appears that an AMD driver can register a > platform_profile handler. If this happens then the asus_wmi driver would > error with -EEXIST when trying to register its own handler leaving the > user with a possibly unusable system - this is especially true for > laptops with an MCU that emit a stream of HID packets, some of which can > be misinterpreted as shutdown signals. > > We can safely continue loading the driver instead of bombing out. > > Signed-off-by: Luke D. Jones <luke@ljones.dev> > --- > drivers/platform/x86/asus-wmi.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c > index 4c129881ce28..7d87ff68f418 100644 > --- a/drivers/platform/x86/asus-wmi.c > +++ b/drivers/platform/x86/asus-wmi.c > @@ -3836,8 +3836,13 @@ static int platform_profile_setup(struct asus_wmi *asus) > asus->platform_profile_handler.choices); > > err = platform_profile_register(&asus->platform_profile_handler); > - if (err) > + if (err == -EEXIST) { > + pr_warn("%s, a platform_profile handler is already registered\n", __func__); > + return 0; > + } else if (err) { > + pr_err("%s, failed at platform_profile_register: %d\n", __func__, err); Please don't print __func__ to user in warnings or errors, and try to write in English what is the reason (instead of resorting to use function names).
On Tue, 30 Jul 2024, at 3:45 AM, Ilpo Järvinen wrote: > On Sat, 13 Jul 2024, Luke D. Jones wrote: > > > On some newer laptops it appears that an AMD driver can register a > > platform_profile handler. If this happens then the asus_wmi driver would > > error with -EEXIST when trying to register its own handler leaving the > > user with a possibly unusable system - this is especially true for > > laptops with an MCU that emit a stream of HID packets, some of which can > > be misinterpreted as shutdown signals. > > > > We can safely continue loading the driver instead of bombing out. > > > > Signed-off-by: Luke D. Jones <luke@ljones.dev> > > --- > > drivers/platform/x86/asus-wmi.c | 9 +++++++-- > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c > > index 4c129881ce28..7d87ff68f418 100644 > > --- a/drivers/platform/x86/asus-wmi.c > > +++ b/drivers/platform/x86/asus-wmi.c > > @@ -3836,8 +3836,13 @@ static int platform_profile_setup(struct asus_wmi *asus) > > asus->platform_profile_handler.choices); > > > > err = platform_profile_register(&asus->platform_profile_handler); > > - if (err) > > + if (err == -EEXIST) { > > + pr_warn("%s, a platform_profile handler is already registered\n", __func__); > > + return 0; > > + } else if (err) { > > + pr_err("%s, failed at platform_profile_register: %d\n", __func__, err); > > Please don't print __func__ to user in warnings or errors, and try to > write in English what is the reason (instead of resorting to use function > names). Sorry, I do know this. The patch must have been submitted after your advice on it last time. Not sure. In either case v2 incoming. I'll skip cover letter due to the tiny size. > > return err; > > + } > > > > asus->platform_profile_support = true; > > return 0; > > @@ -4713,7 +4718,7 @@ static int asus_wmi_add(struct platform_device *pdev) > > throttle_thermal_policy_set_default(asus); > > > > err = platform_profile_setup(asus); > > - if (err) > > + if (err && err != -EEXIST) > > goto fail_platform_profile_setup; > > > > err = asus_wmi_sysfs_init(asus->platform_device); > > > >
diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c index 4c129881ce28..7d87ff68f418 100644 --- a/drivers/platform/x86/asus-wmi.c +++ b/drivers/platform/x86/asus-wmi.c @@ -3836,8 +3836,13 @@ static int platform_profile_setup(struct asus_wmi *asus) asus->platform_profile_handler.choices); err = platform_profile_register(&asus->platform_profile_handler); - if (err) + if (err == -EEXIST) { + pr_warn("%s, a platform_profile handler is already registered\n", __func__); + return 0; + } else if (err) { + pr_err("%s, failed at platform_profile_register: %d\n", __func__, err); return err; + } asus->platform_profile_support = true; return 0; @@ -4713,7 +4718,7 @@ static int asus_wmi_add(struct platform_device *pdev) throttle_thermal_policy_set_default(asus); err = platform_profile_setup(asus); - if (err) + if (err && err != -EEXIST) goto fail_platform_profile_setup; err = asus_wmi_sysfs_init(asus->platform_device);
On some newer laptops it appears that an AMD driver can register a platform_profile handler. If this happens then the asus_wmi driver would error with -EEXIST when trying to register its own handler leaving the user with a possibly unusable system - this is especially true for laptops with an MCU that emit a stream of HID packets, some of which can be misinterpreted as shutdown signals. We can safely continue loading the driver instead of bombing out. Signed-off-by: Luke D. Jones <luke@ljones.dev> --- drivers/platform/x86/asus-wmi.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)