[PATCH] platform/x86: thinkpad_acpi: correct palmsensor error checking

[PATCH] platform/x86: thinkpad_acpi: correct palmsensor error checking

[Mark Pearson]

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(-)

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) {
-- 
2.28.0

Re: [PATCH] platform/x86: thinkpad_acpi: correct palmsensor error checking

[Hans de Goede]

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) {
> 

Re: [PATCH] platform/x86: thinkpad_acpi: correct palmsensor error checking

[Andy Shevchenko]

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.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] platform/x86: thinkpad_acpi: correct palmsensor error checking

[Hans de Goede]

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

Re: [External] Re: [PATCH] platform/x86: thinkpad_acpi: correct palmsensor error checking

[Mark Pearson]

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