| Message ID | 9c81251b-bc87-4ca3-bb86-843dc85e5145@moroto.mountain (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | platform/x86: wmi: Fix wmi_dev_probe() | expand |
Am 05.01.24 um 14:47 schrieb Dan Carpenter: > This has a reversed if statement so it accidentally disables the wmi > method before returning. Good catch, you are absolutely right! And on top of that it also breaks WMI event drivers since the WMI_PROBED flag will not be set when the driver successfully probes and instead will be set when the driver fails to probe. For the patch: Reviewed-by: Armin Wolf <W_Armin@gmx.de> Thanks, Armin Wolf > > Fixes: 704af3a40747 ("platform/x86: wmi: Remove chardev interface") > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> > --- > drivers/platform/x86/wmi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c > index 157f1ce8ac0a..e6f6fa2fd080 100644 > --- a/drivers/platform/x86/wmi.c > +++ b/drivers/platform/x86/wmi.c > @@ -868,7 +868,7 @@ static int wmi_dev_probe(struct device *dev) > if (wdriver->probe) { > ret = wdriver->probe(dev_to_wdev(dev), > find_guid_context(wblock, wdriver)); > - if (!ret) { > + if (ret) { > if (ACPI_FAILURE(wmi_method_enable(wblock, false))) > dev_warn(dev, "Failed to disable device\n"); >
Hi, On 1/5/24 14:47, Dan Carpenter wrote: > This has a reversed if statement so it accidentally disables the wmi > method before returning. > > Fixes: 704af3a40747 ("platform/x86: wmi: Remove chardev interface") > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> Thank you for your patch, I've applied this patch to my review-hans branch: https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans Note it will show up in my review-hans branch once I've pushed my local branch there, which might take a while. I will include this patch in my next fixes pull-req to Linus for the current kernel development cycle. Regards, Hans > --- > drivers/platform/x86/wmi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c > index 157f1ce8ac0a..e6f6fa2fd080 100644 > --- a/drivers/platform/x86/wmi.c > +++ b/drivers/platform/x86/wmi.c > @@ -868,7 +868,7 @@ static int wmi_dev_probe(struct device *dev) > if (wdriver->probe) { > ret = wdriver->probe(dev_to_wdev(dev), > find_guid_context(wblock, wdriver)); > - if (!ret) { > + if (ret) { > if (ACPI_FAILURE(wmi_method_enable(wblock, false))) > dev_warn(dev, "Failed to disable device\n"); >
diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index 157f1ce8ac0a..e6f6fa2fd080 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -868,7 +868,7 @@ static int wmi_dev_probe(struct device *dev) if (wdriver->probe) { ret = wdriver->probe(dev_to_wdev(dev), find_guid_context(wblock, wdriver)); - if (!ret) { + if (ret) { if (ACPI_FAILURE(wmi_method_enable(wblock, false))) dev_warn(dev, "Failed to disable device\n");
This has a reversed if statement so it accidentally disables the wmi method before returning. Fixes: 704af3a40747 ("platform/x86: wmi: Remove chardev interface") Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> --- drivers/platform/x86/wmi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)