[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Thu, 9 Sep 2021 09:15:48 +0200
From: Hans de Goede <hdegoede@...hat.com>
To: "K Naduvalath, Sumesh" <sumesh.k.naduvalath@...el.com>,
Randy Dunlap <rdunlap@...radead.org>,
"mgross@...ux.intel.com" <mgross@...ux.intel.com>,
"srinivas.pandruvada@...ux.intel.com"
<srinivas.pandruvada@...ux.intel.com>
Cc: "Pandruvada, Srinivas" <srinivas.pandruvada@...el.com>,
"platform-driver-x86@...r.kernel.org"
<platform-driver-x86@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"Chinnu, Ganapathi" <ganapathi.chinnu@...el.com>,
"Kumar, Nachiketa" <nachiketa.kumar@...el.com>
Subject: Re: [PATCH v3 1/1] ishtp: Add support for Intel ishtp eclite driver
Hi,
On 9/2/21 2:07 PM, K Naduvalath, Sumesh wrote:
> Thank you Randy for the review. Please find my comments inline -
>
>> -----Original Message-----
>> From: Randy Dunlap <rdunlap@...radead.org>
>> Sent: Thursday, September 2, 2021 1:02 AM
>> To: K Naduvalath, Sumesh <sumesh.k.naduvalath@...el.com>;
>> hdegoede@...hat.com; mgross@...ux.intel.com;
>> srinivas.pandruvada@...ux.intel.com
>> Cc: Pandruvada, Srinivas <srinivas.pandruvada@...el.com>; platform-driver-
>> x86@...r.kernel.org; linux-kernel@...r.kernel.org; Chinnu, Ganapathi
>> <ganapathi.chinnu@...el.com>; Kumar, Nachiketa
>> <nachiketa.kumar@...el.com>
>> Subject: Re: [PATCH v3 1/1] ishtp: Add support for Intel ishtp eclite driver
>>
>> On 9/1/21 5:43 AM, sumesh.k.naduvalath@...el.com wrote:
>>> diff --git a/drivers/platform/x86/Kconfig
>>> b/drivers/platform/x86/Kconfig index d12db6c316ea..8a0945ed1182
>> 100644
>>> --- a/drivers/platform/x86/Kconfig
>>> +++ b/drivers/platform/x86/Kconfig
>>> @@ -1176,6 +1176,22 @@ config INTEL_CHTDC_TI_PWRBTN
>>> To compile this driver as a module, choose M here: the module
>>> will be called intel_chtdc_ti_pwrbtn.
>>>
>>> +config INTEL_ISHTP_ECLITE
>>> + tristate "Intel ISHTP eclite controller"
>>> + depends on INTEL_ISH_HID
>>> + depends on ACPI
>>> + help
>>> + This driver is for accessing the PSE (Programmable Service Engine),
>>> + an Embedded Controller like IP, using ISHTP (Integrated Sensor Hub
>>
>> Which "IP" is that? "an Embedded Controller like IP" doesn't make sense
>> IMO.
>
> This is Intel Elkhartlake's PSE IP. This IP is a dedicated low power real-time ARM
> based microcontroller within the SoC. More information here:
> https://www.intel.in/content/www/in/en/products/platforms/details/elkhart-lake.html
>
> Do you suggest to reword like below?
>
> This driver is for accessing the PSE (Programmable Service Engine) - an Embedded
> Controller like IP - using ISHTP...
That seems fine to me, can send a v4 with this and the other thing Randy
pointed out fixed please ?
Regards,
Hans
>
> Or reword it with more details?
>
>>
>>
>>> + Transport Protocol) to get battery, thermal and UCSI (USB Type-C
>>> + Connector System Software Interface) related data from the
>> platform.
>>
>> Indent above line with tab + 2 spaces instead of many spaces.
>>
> Sure. Will fix in V4.
>
>>> + Users who don't want to use discrete Embedded Controller on
>> Intel's
>>> + Elkhartlake platform, can leverage this integrated solution of
>>
>> Drop this comma ^
>>
> Sure. I'll fix it.
>
>>> + ECLite which is part of PSE subsystem.
>>> +
>>> + To compile this driver as a module, choose M here: the module
>>> + will be called intel_ishtp_eclite
>>
>> End the last sentence with a period ('.').
>>
>>
> Sure. I'll fix it, thanks.
>
>> thanks.
>> --
>> ~Randy
>
Powered by blists - more mailing lists