Re: [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 04:02:01PM -0500, Bjorn Helgaas wrote:
> On Mon, Jul 13, 2020 at 10:44:36PM +0300, Andy Shevchenko wrote:
> > Switch the platform code to use x86_id_table and accompanying API
> > instead of custom comparison against x86 CPU model.
> > 
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> >  arch/x86/pci/intel_mid_pci.c | 17 +++++++++++++++--
> >  1 file changed, 15 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/x86/pci/intel_mid_pci.c b/arch/x86/pci/intel_mid_pci.c
> > index 00c62115f39c..d8af4787e616 100644
> > --- a/arch/x86/pci/intel_mid_pci.c
> > +++ b/arch/x86/pci/intel_mid_pci.c
> > @@ -28,10 +28,12 @@
> >  #include <linux/io.h>
> >  #include <linux/smp.h>
> >  
> > +#include <asm/cpu_device_id.h>
> >  #include <asm/segment.h>
> >  #include <asm/pci_x86.h>
> >  #include <asm/hw_irq.h>
> >  #include <asm/io_apic.h>
> > +#include <asm/intel-family.h>
> >  #include <asm/intel-mid.h>
> >  
> >  #define PCIE_CAP_OFFSET	0x100
> > @@ -211,9 +213,16 @@ static int pci_write(struct pci_bus *bus, unsigned int devfn, int where,
> >  			       where, size, value);
> >  }
> >  
> > +static const struct x86_cpu_id intel_mid_cpu_ids[] = {
> > +	X86_MATCH_INTEL_FAM6_MODEL(ATOM_SILVERMONT_MID, NULL),
> > +	{}
> > +};
> > +
> >  static int intel_mid_pci_irq_enable(struct pci_dev *dev)
> >  {
> > +	const struct x86_cpu_id *id;
> >  	struct irq_alloc_info info;
> > +	u16 model = 0;
> >  	int polarity;
> >  	int ret;
> >  	u8 gsi;
> > @@ -227,8 +236,12 @@ static int intel_mid_pci_irq_enable(struct pci_dev *dev)
> >  		return ret;
> >  	}
> >  
> > -	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:
> 
> Is there a magic decoder ring somewhere that connects
> INTEL_MID_CPU_CHIP_TANGIER and INTEL_FAM6_ATOM_SILVERMONT_MID?

Yes. And the idea is to get rid of it.

> I don't know how to verify that the new code is equivalent to the old.
> 
> Or maybe the new code is *better* than the old, in which case the
> subject/commit log should mention that it's fixing or improving
> something.
> 
> Also, there are a number of other places that check for
> "intel_mid_identify_cpu() == INTEL_MID_CPU_CHIP_TANGIER":
> 
>   mrfld_pinctrl_init
>   register_mrfld_power_btn
>   mrfld_legacy_rtc_init
>   mrfld_sd_init
>   spidev_platform_data
>   register_mid_wdt
>   sfi_parse_devs

>   atomisp_css_input_set_mode

This has been pending in Mauro's tree.

> Maybe they should all be changed together?  Or maybe this needs an
> explanation about why some places need intel_mid_identify_cpu() and
> others need x86_match_cpu()?

No. The rest is subject to huge clean up (complete removal) in the future.
I don't want to waste time on something which I will remove for sure.

-- 
With Best Regards,
Andy Shevchenko