| Message ID | 20201230024726.7861-1-markpearson@lenovo.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | platform/x86: thinkpad_acpi: correct palmsensor error checking | expand |
Hi, On 12/30/20 3:47 AM, Mark Pearson wrote: > The previous commit adding functionality for the palm sensor had a > mistake which meant the error conditions on initialisation was not checked > correctly. On some older platforms this meant that if the sensor wasn't > available an error would be returned and the driver would fail to load. > > This commit corrects the error condition. Many thanks to Mario Oenning > for reporting and determining the issue > > Signed-off-by: Mark Pearson <markpearson@lenovo.com> 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. Once I've run some tests on this branch the patches there will be added to the platform-drivers-x86/for-next branch and eventually will be included in the pdx86 pull-request to Linus for the next merge-window. Since this is a bugfix I will also cherry-pick this patch to the fixes branch and include it in an upcoming fixes pull-req for 5.11. Regards, Hans > --- > drivers/platform/x86/thinkpad_acpi.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c > index e03df2881dc6..c102657b3eb3 100644 > --- a/drivers/platform/x86/thinkpad_acpi.c > +++ b/drivers/platform/x86/thinkpad_acpi.c > @@ -9951,9 +9951,9 @@ static int tpacpi_proxsensor_init(struct ibm_init_struct *iibm) > if ((palm_err == -ENODEV) && (lap_err == -ENODEV)) > return 0; > /* Otherwise, if there was an error return it */ > - if (palm_err && (palm_err != ENODEV)) > + if (palm_err && (palm_err != -ENODEV)) > return palm_err; > - if (lap_err && (lap_err != ENODEV)) > + if (lap_err && (lap_err != -ENODEV)) > return lap_err; > > if (has_palmsensor) { >
On Wed, Dec 30, 2020 at 4:54 AM Mark Pearson <markpearson@lenovo.com> wrote: > > The previous commit adding functionality for the palm sensor had a > mistake which meant the error conditions on initialisation was not checked > correctly. On some older platforms this meant that if the sensor wasn't > available an error would be returned and the driver would fail to load. > This commit corrects the error condition. Many thanks to Mario Oenning > for reporting and determining the issue Hint to the future (maybe Hans will fix this while it's in his review queue): we have a Reported-by tag. Of course if a person would like to avoid it, then it's fine.
Hi, On 1/4/21 4:17 PM, Andy Shevchenko wrote: > On Wed, Dec 30, 2020 at 4:54 AM Mark Pearson <markpearson@lenovo.com> wrote: >> >> The previous commit adding functionality for the palm sensor had a >> mistake which meant the error conditions on initialisation was not checked >> correctly. On some older platforms this meant that if the sensor wasn't >> available an error would be returned and the driver would fail to load. > >> This commit corrects the error condition. Many thanks to Mario Oenning >> for reporting and determining the issue > > Hint to the future (maybe Hans will fix this while it's in his review queue): > we have a Reported-by tag. Of course if a person would like to avoid > it, then it's fine. I did notice this too, but I did not fix it. For future patches the right thing to do is to ask the reporter if he is ok with a Reported-by tag being added (which will expose their email to the world) and then proceed depending on their answer, at least that is what I usually do. Although sometimes I do just add the Reported-by tag, esp. if the email has been send to a (couple of) lists, so the email already has been exposed to a large audience. Regards, Hans
On 04/01/2021 10:32, Hans de Goede wrote: > Hi, > > On 1/4/21 4:17 PM, Andy Shevchenko wrote: >> On Wed, Dec 30, 2020 at 4:54 AM Mark Pearson >> <markpearson@lenovo.com> wrote: >>> >>> The previous commit adding functionality for the palm sensor had >>> a mistake which meant the error conditions on initialisation was >>> not checked correctly. On some older platforms this meant that if >>> the sensor wasn't available an error would be returned and the >>> driver would fail to load. >> >>> This commit corrects the error condition. Many thanks to Mario >>> Oenning for reporting and determining the issue >> >> Hint to the future (maybe Hans will fix this while it's in his >> review queue): we have a Reported-by tag. Of course if a person >> would like to avoid it, then it's fine. > > I did notice this too, but I did not fix it. For future patches the > right thing to do is to ask the reporter if he is ok with a > Reported-by tag being added (which will expose their email to the > world) and then proceed depending on their answer, at least that is > what I usually do. Although sometimes I do just add the Reported-by > tag, esp. if the email has been send to a (couple of) lists, so the > email already has been exposed to a large audience. > Ack - thanks all. I wasn't aware of the reported-by etiquette :) Probably a good one to know about....I'm sure this won't be the last issue reported against a Lenovo system! Mark
diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c index e03df2881dc6..c102657b3eb3 100644 --- a/drivers/platform/x86/thinkpad_acpi.c +++ b/drivers/platform/x86/thinkpad_acpi.c @@ -9951,9 +9951,9 @@ static int tpacpi_proxsensor_init(struct ibm_init_struct *iibm) if ((palm_err == -ENODEV) && (lap_err == -ENODEV)) return 0; /* Otherwise, if there was an error return it */ - if (palm_err && (palm_err != ENODEV)) + if (palm_err && (palm_err != -ENODEV)) return palm_err; - if (lap_err && (lap_err != ENODEV)) + if (lap_err && (lap_err != -ENODEV)) return lap_err; if (has_palmsensor) {
The previous commit adding functionality for the palm sensor had a mistake which meant the error conditions on initialisation was not checked correctly. On some older platforms this meant that if the sensor wasn't available an error would be returned and the driver would fail to load. This commit corrects the error condition. Many thanks to Mario Oenning for reporting and determining the issue Signed-off-by: Mark Pearson <markpearson@lenovo.com> --- drivers/platform/x86/thinkpad_acpi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)