Re: [PATCH V16 7/7] PCI: Add quirk for multifunction devices of LS7A

From: Huacai Chen <chenhuacai@gmail.com>
To: Bjorn Helgaas <helgaas@kernel.org>
Cc: "Jianmin Lv" <lvjianmin@loongson.cn>,
	"Huacai Chen" <chenhuacai@loongson.cn>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	"Lorenzo Pieralisi" <lorenzo.pieralisi@arm.com>,
	"Rob Herring" <robh@kernel.org>,
	"Krzysztof Wilczyński" <kw@linux.com>,
	linux-pci <linux-pci@vger.kernel.org>,
	"Xuefeng Li" <lixuefeng@loongson.cn>,
	"Jiaxun Yang" <jiaxun.yang@flygoat.com>
Subject: Re: [PATCH V16 7/7] PCI: Add quirk for multifunction devices of LS7A
Date: Fri, 22 Jul 2022 12:11:18 +0800	[thread overview]
Message-ID: <CAAhV-H6XimeV1yf3LpQrixUu-rhSM=j02XKxF=+GtNDV4MESOQ@mail.gmail.com> (raw)
In-Reply-To: <20220721175048.GA1738677@bhelgaas>

Hi, Bjorn,

On Fri, Jul 22, 2022 at 1:50 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Thu, Jul 21, 2022 at 12:47:18PM +0800, Huacai Chen wrote:
>
> > Unfortunately, this patch only lists devices in LS7A1000, but some of
> > LS7A2000 (GNET and HDMI) also need to quirk, can they be squashed in
> > this patch? If not, we will add them in a new patch.
> >
> >  #define DEV_LS7A_CONF  0x7a10
> >  #define DEV_LS7A_LPC   0x7a0c
> >  #define DEV_LS7A_GMAC  0x7a03
> > +#define DEV_LS7A_GNET  0x7a13
> >  #define DEV_LS7A_DC1   0x7a06
> >  #define DEV_LS7A_DC2   0x7a36
> >  #define DEV_LS7A_GPU   0x7a15
> >  #define DEV_LS7A_AHCI  0x7a08
> >  #define DEV_LS7A_EHCI  0x7a14
> >  #define DEV_LS7A_OHCI  0x7a24
> > +#define DEV_LS7A_HDMI  0x7a37
>
> I squashed these in.  Let me know if I did anything wrong:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?id=930c6074d7dd
The logic is surely correct. But what is the principle of device list
order? If the order is value ascending, then LPC should be after AHCI;
if not, I prefer to group them with functions as below. :)
---

diff --git a/drivers/pci/controller/pci-loongson.c
b/drivers/pci/controller/pci-loongson.c
index 05997b51c86d..a7c3d5db3be8 100644
--- a/drivers/pci/controller/pci-loongson.c
+++ b/drivers/pci/controller/pci-loongson.c
@@ -22,6 +22,13 @@
 #define DEV_LS2K_APB   0x7a02
 #define DEV_LS7A_CONF  0x7a10
 #define DEV_LS7A_LPC   0x7a0c
+#define DEV_LS7A_DC1   0x7a06
+#define DEV_LS7A_DC2   0x7a36
+#define DEV_LS7A_HDMI  0x7a37
+#define DEV_LS7A_AHCI  0x7a08
+#define DEV_LS7A_EHCI  0x7a14
+#define DEV_LS7A_GMAC  0x7a03
+#define DEV_LS7A_GNET  0x7a13

 #define FLAG_CFG0      BIT(0)
 #define FLAG_CFG1      BIT(1)
@@ -103,6 +110,25 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
 DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
                        DEV_PCIE_PORT_2, loongson_bmaster_quirk);

+static void loongson_pci_pin_quirk(struct pci_dev *pdev)
+{
+       pdev->pin = 1 + (PCI_FUNC(pdev->devfn) & 3);
+}
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LOONGSON,
+                       DEV_LS7A_DC1, loongson_pci_pin_quirk);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LOONGSON,
+                       DEV_LS7A_DC2, loongson_pci_pin_quirk);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LOONGSON,
+                       DEV_LS7A_HDMI, loongson_pci_pin_quirk);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LOONGSON,
+                       DEV_LS7A_AHCI, loongson_pci_pin_quirk);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LOONGSON,
+                       DEV_LS7A_EHCI, loongson_pci_pin_quirk);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LOONGSON,
+                       DEV_LS7A_GMAC, loongson_pci_pin_quirk);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LOONGSON,
+                       DEV_LS7A_GNET, loongson_pci_pin_quirk);
+
 static struct loongson_pci *pci_bus_to_loongson_pci(struct pci_bus *bus)
 {
        struct pci_config_window *cfg;

---
Huacai
