Re: [PATCH 3/4] toshiba_acpi: Change HCI/SCI functions return code type

[Azael Avalos]

2014-09-25 21:11 GMT-06:00 Darren Hart <dvhart@xxxxxxxxxxxxx>:
> On Tue, Sep 23, 2014 at 06:24:27PM -0600, Azael Avalos wrote:
>> Currently the HCI/SCI read/write functions are returning
>> the status of the ACPI call and also assigning the
>> returned value of the HCI/SCI function.
>>
>> This patch changes such functions, returning the value
>> of the HCI/SCI function instead of the ACPI call status.
>>
>> The next patch will change all the HCI/SCI functions
>> to reflect the change made in this patch.
>
> If you are changing what the functions return in this patch, you also need to
> update the call sites at the same time (same patch).

Ok

>
>>
>> Signed-off-by: Azael Avalos <coproscefalo@xxxxxxxxx>
>> ---
>>  drivers/platform/x86/toshiba_acpi.c | 51 ++++++++++++++++++++-----------------
>>  1 file changed, 27 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
>> index 5b16d11..43385f7 100644
>> --- a/drivers/platform/x86/toshiba_acpi.c
>> +++ b/drivers/platform/x86/toshiba_acpi.c
>> @@ -316,47 +316,49 @@ static acpi_status tci_raw(struct toshiba_acpi_dev *dev,
>>   * may be useful (such as "not supported").
>>   */
>
> The full text of the comment above is:
>
> /* common hci tasks (get or set one or two value)
>  *
>  * In addition to the ACPI status, the HCI system returns a result which
>  * may be useful (such as "not supported").
>  */
>
> Is this no longer relevant?

On the contrary, the "result" parameter is the one being returned by the
modified read/write functions now, and was (and still is) the only one
being checked for support, error, or otherwise, depending on what the
Toshiba method returns.

>
> I agree that the return and status approach seems suboptimal, but I'm not clear on the motivation for the change. Is there something besides cleanup you're attempting to work toward with this series?

Cleanup mostly, what's the purpose of returning a value,
if that value is never checked? Better return a value that
indeed is being checked, and contains useful info about
the status of the queried function (such as "not supported")
:-)


>
> --
> Darren Hart
> Intel Open Source Technology Center

Cheers
Azael


-- 
-- El mundo apesta y vosotros apestais tambien --
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/