Re: [NEEDS-REVIEW] [PATCH v1 1/2] x86/PCI: Get rid of custom x86 model comparison

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Mon, Jul 13, 2020 at 01:59:55PM -0700, Dave Hansen wrote:
> On 7/13/20 12:44 PM, Andy Shevchenko wrote:
> > -	switch (intel_mid_identify_cpu()) {
> > -	case INTEL_MID_CPU_CHIP_TANGIER:
> > +	id = x86_match_cpu(intel_mid_cpu_ids);
> > +	if (id)
> > +		model = id->model;
> > +
> > +	switch (model) {
> > +	case INTEL_FAM6_ATOM_SILVERMONT_MID:
> >  		polarity = IOAPIC_POL_HIGH;
> 
> The diff makes it _look_ like there's a behavior change, swapping
> silvermont and tangier.  It appears from intel_mid_arch_setup() that
> INTEL_FAM6_ATOM_SILVERMONT_MID and tangier are related:
> 
> #define INTEL_FAM6_ATOM_SILVERMONT_MID  0x4A /* Merriefield */
> 
> ...
>         case 0x3C:
>         case 0x4A:
>                 __intel_mid_cpu_chip = INTEL_MID_CPU_CHIP_TANGIER;
>                 x86_platform.legacy.rtc = 1;
>                 break;

Yes. The original code is too old and was submitted without relying on existing
kernel helpers.

> But that's probably worth a note in the changelog.  If you added a patch
>  to intel_mid_arch_setup() to this series to use the intel-family.h
> #defines, this would be much more self explanatory.

No, I'm not going to do that at all. The idea behind is to get rid of this ugly
customisation.

> This also all makes me wonder if intel_mid_identify_cpu() is really even
> necessary.

Exactly!

> Does this fix any kinds of problems?  It's a diffstat-challenged cleanup
> if not:

>  arch/x86/pci/intel_mid_pci.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> That's not usually how cleanups look. :)

I understand. This will help to do a real clean up in the future.

-- 
With Best Regards,
Andy Shevchenko