Re: [PATCH] thermal: intel_pch_thermal: Support skylake PCH 100 series thermal

[OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>]

"Pandruvada, Srinivas" <srinivas.pandruvada@intel.com> writes:

> On Tue, 2016-10-18 at 06:44 +0900, OGAWA Hirofumi wrote:
>> Using registers are compatible with current driver.
> This description is not enough. Without going through the whole code, I
> was not able to find out what this patch is doing.
>
> Basically you want to tie the themal_zone name and device operations to
> a board, rather than to PCI devices ids. In this way multiple PCI ids
> for the same board can be accounted for.

Exactly. I thought the patch is enough easy to read, and changelog would
be better to not explain what code itself, and wrote note of code
(registers are same with current).

Well, ok. I will explain what patch does. Your explain looks like good,
so I will borrow it :)

> Also need to consider another patch sent to mailing list for supporting
> Haswell PCH.

Sounds good. I think the patch makes it easier.

> One comment below in the code.
>
> [...]
>  
>>  static struct pci_device_id intel_pch_thermal_id[] = {
>> -	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCH_THERMAL_DID_WPT) },
>> -	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCH_THERMAL_DID_SKL) },
>> +	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCH_THERMAL_DID_WPT),
>> +	  .driver_data = board_wpt, },
>> +	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCH_THERMAL_DID_SKL),
>> +	  .driver_data = board_skl, },
>> +	/* skylake PCH 100 series */
>> +	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0xa131),
> Change a131 to a define (PCH_THERMAL_DID_SKL_H)

I don't think it helps anything actually, because no users of define
anymore. Well, anyway, ok, I don't care either case.

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>