Re: [PATCH 1/4] toshiba_acpi: Rename hci_raw to tci_raw

[Azael Avalos]

2014-09-25 21:01 GMT-06:00 Darren Hart <dvhart@xxxxxxxxxxxxx>:
> On Tue, Sep 23, 2014 at 06:24:25PM -0600, Azael Avalos wrote:
>> The function name hci_raw was used before to reflect
>> a raw (read/write) call to the Toshiba's Hardware
>> Configuration Interface (HCI), however, since the
>> introduction of the System Configuration Interface
>> (SCI), that "name" no longer applies.
>>
>> This patch changes the name of that function to
>> tci_raw (for Toshiba Configuration Interface), and
>> change the comments about it.
>
> I'm not following the motivation for this change. The HCI clearly still exists,
> at least on the platforms this driver was written to support. When/Where was the
> "SCI" "introduced" in a way that requires a change here? Was this a change by
> Toshiba you are refering to?

The HCI and SCI form an integral part of the Toshiba configuration scheme,
the SCI has been present on any Toshiba branded laptop to date, what
was not present (at least on toshiba_acpi) was (propper) SCI support
until the patches I sent.

Take a look at Jonathan Buzzard documentation [1], and also the toshiba
module under char, the documentation was written in 1999, and the
module has a (starting) date of 1996.

By the time toshiba_acpi was written, only HCI functions were accessed,
as well as the configuration handle was (and still is, depending on model)
named GHCI, and so the comments refer to it, now that we have SCI
(driver support) on board, the naming seems obsolete, or at least
misguiding, as the HCI is not the only configuration interface.

Hope this clears things a bit.

[1] http://www.buzzard.me.uk/toshiba/docs.html

>
>
>>
>> Signed-off-by: Azael Avalos <coproscefalo@xxxxxxxxx>
>> ---
>>  drivers/platform/x86/toshiba_acpi.c | 40 ++++++++++++++++++-------------------
>>  1 file changed, 20 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
>> index edd8f3d..b7030dc 100644
>> --- a/drivers/platform/x86/toshiba_acpi.c
>> +++ b/drivers/platform/x86/toshiba_acpi.c
>> @@ -71,7 +71,7 @@ MODULE_LICENSE("GPL");
>>  /* Toshiba ACPI method paths */
>>  #define METHOD_VIDEO_OUT     "\\_SB_.VALX.DSSX"
>>
>> -/* Toshiba HCI interface definitions
>> +/* Toshiba configuration interface definitions
>>   *
>>   * HCI is Toshiba's "Hardware Control Interface" which is supposed to
>
> I'm not sure this patch is appropriate/necessary, but if so, you missed a spot
> here :)

Oh no, this is intentional, what I probably should have done here is be more
verbose about it, perhaps something like:

/*
 * The Toshiba configuration interface is composed of the HCI and the
 * SCI, which are defined as
...

>
> --
> 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/