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

[Bjorn Helgaas <helgaas@kernel.org>]

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?

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

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()?

>  		polarity = IOAPIC_POL_HIGH;
>  
>  		/* Special treatment for IRQ0 */
> -- 
> 2.27.0
>