Re: [RFC v2 6/6] platform/x86: intel_pmc_ipc: Use generic Intel IPC device calls

["Kuppuswamy, Sathyanarayanan" <sathyaosid@gmail.com>]

Hi Lee,


On 9/5/2017 12:38 AM, Lee Jones wrote:
> On Sat, 02 Sep 2017, sathyanarayanan.kuppuswamy@linux.intel.com wrote:
>
>> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
>>
>> Removed redundant IPC helper functions and refactored the driver to use
>> generic IPC device driver APIs.
>>
>> This patch also cleans-up PMC IPC user drivers to use APIs provided
>> by generic IPC driver.
>>
>> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
>> ---
>>   arch/x86/include/asm/intel_pmc_ipc.h          |  37 +--
>>   drivers/mfd/intel_soc_pmic_bxtwc.c            |  24 +-
>>   include/linux/mfd/intel_soc_pmic.h            |   2 +
> I'm a bit concerned by the API.
This is not a new change. Even before refactoring this driver, we have 
been using u32 bit variable to pass the DPTR and SPTR address.
>    Any reason why you're not using
> pointers for addresses?
I think the main reason is,  this is the expected format defined by 
SCU/PMC spec. According to the spec document, SPTR and DPTR registers 
are used to program the 32 bit SRAM address from which the PMC/SCU 
firmware can read/write the data of an IPC command. if we are not using 
SPTR or DPTR, we need to leave the value at zero.
> pointers, you should be using NULL, instead of 0.
>
>>   drivers/platform/x86/intel_pmc_ipc.c          | 364 +++++++++-----------------
>>   drivers/platform/x86/intel_telemetry_pltdrv.c | 114 ++++----
>>   5 files changed, 215 insertions(+), 326 deletions(-)
>>
>> Changes since v1:
>>   * Removed custom APIs.
>>   * Cleaned up PMC IPC user drivers to use APIs provided by generic
>>     IPC driver.