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
