On Tue, Mar 26, 2019 at 01:24:41PM +0000, David Woodhouse wrote: > On Tue, 2019-03-26 at 12:17 +0000, Lorenzo Pieralisi wrote: > > [+Zhou, Gustavo] > > > > On Tue, Mar 26, 2019 at 12:00:55PM +0200, Jonathan Chocron wrote: > > > Adding support for Amazon's Annapurna Labs PCIe driver. > > > The HW controller is based on DesignWare's IP. > > > > > > The HW doesn't support accessing the Root Port's config space via > > > ECAM, so we obtain its base address via an AMZN0001 device. > > > > > > Furthermore, the DesignWare PCIe controller doesn't filter out > > > config transactions sent to devices 1 and up on its bus, so they > > > are filtered by the driver. > > > All subordinate buses do support ECAM access. > > > > > > Implementing specific PCI config access functions involves: > > > - Adding an init function to obtain the Root Port's base address > > > from an AMZN0001 device. > > > - Adding a new entry in the mcfg quirk array > > > > > > Co-developed-by: Vladimir Aerov <vaerov@xxxxxxxxxx> > > > Signed-off-by: Jonathan Chocron <jonnyc@xxxxxxxxxx> > > > Signed-off-by: Vladimir Aerov <vaerov@xxxxxxxxxx> > > > Reviewed-by: Benjamin Herrenschmidt <benh@xxxxxxxxxxxxxxxxxxx> > > > Reviewed-by: David Woodhouse <dwmw@xxxxxxxxxxxx> > > > > Review tags should be given on public mailing lists for public > > review and I have not seen them (they were already there in v1) so > > you should drop them. > > We did that internally. You really don't want me telling engineers to > post to the list *first* without running things by me to get the basics > right. Not to start with, at least. Hi David, I am obviously in favour of internal review and I do not question it was carried out internally, I just kindly ask developers to drop review tags given internally when going to public mailing lists - I understand it is churn for you but I prefer them to be given explicitly. Thanks ! Lorenzo > Reviewed-by: David Woodhouse <dwmw@xxxxxxxxxxxx> > > > > > Changes from v1: > > > - Fix commit message comments (incl. using AMZN0001 > > > instead of PNP0C02) > > > - Use the usual multi-line comment style > > > > > > MAINTAINERS | 6 +++ > > > drivers/acpi/pci_mcfg.c | 12 +++++ > > > drivers/pci/controller/dwc/Makefile | 1 + > > > drivers/pci/controller/dwc/pcie-al.c | 93 ++++++++++++++++++++++++++++++++++++ > > > include/linux/pci-ecam.h | 1 + > > > 5 files changed, 113 insertions(+) > > > create mode 100644 drivers/pci/controller/dwc/pcie-al.c > > > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > > index 32d444476a90..7a17017f9f82 100644 > > > --- a/MAINTAINERS > > > +++ b/MAINTAINERS > > > @@ -11769,6 +11769,12 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/pci.git/ > > > S: Supported > > > F: drivers/pci/controller/ > > > > > > +PCIE DRIVER FOR ANNAPURNA LABS > > > +M: Jonathan Chocron <jonnyc@xxxxxxxxxx> > > > +L: linux-pci@xxxxxxxxxxxxxxx > > > +S: Maintained > > > +F: drivers/pci/controller/dwc/pcie-al.c > > > > I do not think we need a maintainer file for that see below, and > > actually this quirk should be handled by DWC maintainers since it is a > > DWC quirk, not a platform one. > > Many of the others already have this, it seems. > > It's also fine to drop it, and include it when we add the rest of the > Alpine SOC support and a MAINTAINERS entry for that. >