Hi Green, On Fri, Mar 12, 2021 at 4:05 PM Green Wan <green....@sifive.com> wrote: > > > > On Thu, Mar 11, 2021 at 10:40 PM Bin Meng <bmeng...@gmail.com> wrote: >> >> On Thu, Mar 11, 2021 at 10:25 PM Green Wan <green....@sifive.com> wrote: >> > >> > >> > >> > Bin Meng <bmeng...@gmail.com>於 2021年3月11日 週四，下午10:14寫道： >> >> >> >> On Thu, Mar 11, 2021 at 9:50 PM Green Wan <green....@sifive.com> wrote: >> >> > >> >> > Add pcie driver for SiFive fu740, the driver depends on >> >> > fu740 gpio, clk and reset driver to do init. Force running at Gen1 >> >> > for better capatible enumeration. >> >> > >> >> > Several devices are tested: >> >> > a) M.2 NVMe SSD >> >> > b) USB-to-PCI adapter >> >> > c) Ethernet adapter (E1000 compatible) >> >> > >> >> > Signed-off-by: Green Wan <green....@sifive.com> >> >> > --- >> >> > drivers/pci/Kconfig | 9 + >> >> > drivers/pci/Makefile | 1 + >> >> > drivers/pci/pcie_sifive.c | 797 >> >> > ++++++++++++++++++++++++++++++++++++++++++++++ >> >> > drivers/pci/pcie_sifive.h | 374 ++++++++++++++++++++++ >> >> > 4 files changed, 1181 insertions(+) >> >> > create mode 100644 drivers/pci/pcie_sifive.c >> >> > create mode 100644 drivers/pci/pcie_sifive.h >> >> > >> >> > diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig >> >> > index ba41787..b078e76 100644 >> >> > --- a/drivers/pci/Kconfig >> >> > +++ b/drivers/pci/Kconfig >> >> > @@ -97,6 +97,15 @@ config PCIE_DW_MVEBU >> >> > Armada-8K SoCs. The PCIe controller on Armada-8K is based on >> >> > DesignWare hardware. >> >> > >> >> > +config PCIE_SIFIVE_FU740 >> >> > + bool "Enable SiFive FU740 PCIe" >> >> > + depends on CLK_SIFIVE_PRCI >> >> > + depends on RESET_SIFIVE >> >> > + depends on SIFIVE_GPIO >> >> > + help >> >> > + Say Y here if you want to enable PCIe controller support on >> >> > + FU740. >> >> > + >> >> > config PCIE_FSL >> >> > bool "FSL PowerPC PCIe support" >> >> > depends on DM_PCI >> >> > diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile >> >> > index 5ed94bc..5400d59 100644 >> >> > --- a/drivers/pci/Makefile >> >> > +++ b/drivers/pci/Makefile >> >> > @@ -51,3 +51,4 @@ obj-$(CONFIG_PCIE_ROCKCHIP) += pcie_rockchip.o >> >> > obj-$(CONFIG_PCIE_DW_ROCKCHIP) += pcie_dw_rockchip.o >> >> > obj-$(CONFIG_PCI_BRCMSTB) += pcie_brcmstb.o >> >> > obj-$(CONFIG_PCI_OCTEONTX) += pci_octeontx.o >> >> > +obj-$(CONFIG_PCIE_SIFIVE_FU740) += pcie_sifive.o >> >> > diff --git a/drivers/pci/pcie_sifive.c b/drivers/pci/pcie_sifive.c >> >> > new file mode 100644 >> >> > index 0000000..ada6087 >> >> > --- /dev/null >> >> > +++ b/drivers/pci/pcie_sifive.c >> >> > @@ -0,0 +1,797 @@ >> >> > +// SPDX-License-Identifier: GPL-2.0+ >> >> > +/* >> >> > + * SiFive FU740 DesignWare PCIe Controller >> >> >> >> Should we call this driver a DesgnWare PCIe controller (e.g.: >> >> pcie_designware.c), instead of pcie_sifive.c? >> > >> > >> > The driver currently depends on fu740 drivers such as prci, clk, gpio and >> > reset. probably rename it to pcie_dw_sifive.c like other designware based >> > driver. What do we think of it? >> >> Is it possible to update the existing dw pcie driver to support fu740? >> >> Depending on prci, clk and reset is not a problem. We can use dm_ APIs >> to hide device specific (prci, sifive-gpio) stuff. >> >> Unless SiFive made significant changes to the designware PCIe IP, we >> should not create a new driver for it. > > > Hi Bin, > > So far, we'd like to maintain a sifive driver to handle the different > sequences and changes. For example, we don't have a memory region start for > phy registers but other platforms might have. Those differences and details > might need info from other maintainers. > > But I do agree with you that we should have certain level common code cross > designware based drivers. Actually, I did give a try on merging them. But I > lack the detail in the first attempt. Every driver looks has its own > specialty. I wasn't 100% sure what the common we should have at the time. > > Maybe we can create a "pcie_dw_common.c" or "pcie_designware.c" for all DWC > IP and try to move common functions into it as a start. We can migrate some > dw_pcie_xxx() and data structure from Linux. Then, I can do it on top of this > patch as a RFC so it doesn't break current implementation. What do you think > of it?
I think this approach makes sense. Regards, Bin
