[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAGdLNWEuur0VeyPXRdf_+xh-BHA0ZOKy38E5T0HRQPxnGg20ug@mail.gmail.com>
Date: Mon, 29 Aug 2016 09:32:17 -0600
From: Azael Avalos <coproscefalo@...il.com>
To: Darren Hart <dvhart@...radead.org>
Cc: "platform-driver-x86@...r.kernel.org"
<platform-driver-x86@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/3] toshiba_acpi: Change error checking logic from TCI functions
Hi Darren,
2016-08-28 10:56 GMT-06:00 Darren Hart <dvhart@...radead.org>:
> On Tue, Aug 16, 2016 at 12:06:16PM -0600, Azael Avalos wrote:
>> Currently the success/error checking logic is intermixed, making the
>> code a bit cumbersome to understand.
>>
>> This patch changes the affected functions to first check for errors
>> and take appropriate actions, then check for the supported features.
>>
>> This patch also separates the error check from the acpi_status and
>> the tci_raw function call error check, as those two are completely
>> unrelated and were nested in if/else statements.
>
> Thanks, this is a good improvement. One questions below...
>
>>
>> Signed-off-by: Azael Avalos <coproscefalo@...il.com>
>> ---
>> drivers/platform/x86/toshiba_acpi.c | 222 ++++++++++++++++++++++--------------
>> 1 file changed, 135 insertions(+), 87 deletions(-)
>>
>> diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
>> index c6fc5cc..2256cf5 100644
>> --- a/drivers/platform/x86/toshiba_acpi.c
>> +++ b/drivers/platform/x86/toshiba_acpi.c
>> @@ -476,10 +476,15 @@ static void toshiba_illumination_available(struct toshiba_acpi_dev *dev)
>>
>> status = tci_raw(dev, in, out);
>> sci_close(dev);
>> - if (ACPI_FAILURE(status))
>> + if (ACPI_FAILURE(status)) {
>> pr_err("ACPI call to query Illumination support failed\n");
>> - else if (out[0] == TOS_SUCCESS)
>> - dev->illumination_supported = 1;
>> + return;
>> + }
>> +
>> + if (out[0] != TOS_SUCCESS)
>
> Does this condition not merit a pr_err message? It reads like an error...
>
> There are several similar situations below which are equally silent. Is this a
> deliberate decision?
This was on purpose, since we are querying for all the supported features,
the kernel log will contain a lot of error strings for not supported features,
as not all laptop models support all of them.
>
> --
> Darren Hart
> Intel Open Source Technology Center
> --
> To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
-- El mundo apesta y vosotros apestais tambien --
Powered by blists - more mailing lists