RE: [PATCH V5 2/2] PCI/ACPI: hisi: Add ACPI support for HiSilicon SoCs Host Controllers

From: Gabriele Paoloni
Date: Tue Nov 22 2016 - 02:44:17 EST


> -----Original Message-----
> From: Bjorn Helgaas [mailto:helgaas@xxxxxxxxxx]
> Sent: 21 November 2016 22:32
> To: Gabriele Paoloni
> Cc: liudongdong (C); arnd@xxxxxxxx; rafael@xxxxxxxxxx;
> Lorenzo.Pieralisi@xxxxxxx; tn@xxxxxxxxxxxx; Wangzhou (B);
> pratyush.anand@xxxxxxxxx; linux-pci@xxxxxxxxxxxxxxx; linux-
> acpi@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; jcm@xxxxxxxxxx;
> Chenxin (Charles); hanjun.guo@xxxxxxxxxx; Linuxarm
> Subject: Re: [PATCH V5 2/2] PCI/ACPI: hisi: Add ACPI support for
> HiSilicon SoCs Host Controllers
>
> On Mon, Nov 21, 2016 at 09:09:28AM +0000, Gabriele Paoloni wrote:
>
> > > > +config PCI_HISI_ACPI
> > > > + depends on ACPI && ARM64
> > > > + bool "HiSilicon Hip05 and Hip06 and Hip07 SoCs ACPI PCIe
> > > controllers"
> > > > + select PNP
> > > > + help
> > > > + Say Y here if you want ACPI PCIe controller support on
> > > HiSilicon
> > > > + Hip05 and Hip06 and Hip07 SoCs
> > >
> > > I'm not sure about this Kconfig setup. Do we really want to force
> > > people to enable a special config option to get this support?
> > >
> > > I'm comparing it in my mind with other PCI quirks. They're all
> > > enabled as a group by CONFIG_PCI_QUIRKS. Ultimately we want an
> ACPI
> > > kernel to work without requiring any platform-specific config
> options.
> > >
> > > I'm wondering if we should consolidate all the ECAM quirk code in a
> > > single place (maybe pci/ecam-quirks.c, pci/ecam.c, or pci/pci-
> acpi.c),
> > > under a config option like CONFIG_PCI_ECAM_QUIRKS or maybe even
> plain
> > > CONFIG_PCI_QUIRKS (of course, it could still be qualified by
> > > CONFIG_ACPI and CONFIG_ARM64).
> >
> > What about having a single config options but keeping separate files
> > for each vendors (at least as first step)?
>
> That sounds fine. The main thing is that we're trying to build a
> generic kernel that can run on any ACPI arm64 platform, so we really
> shouldn't have to turn on platform-specific config options.

Ok great we'll do so then

Thanks

Gab

>
> > Maybe if we see that we can consolidate all the vendors in one file
> > we can do it as a second step...