[PATCH 5/7] drivers: pci: add pcie support for fu740

[Green Wan]

On Thu, Mar 11, 2021 at 10:40 PM Bin Meng <bmeng.cn at gmail.com> wrote:

> On Thu, Mar 11, 2021 at 10:25 PM Green Wan <green.wan at sifive.com> wrote:
> >
> >
> >
> > Bin Meng <bmeng.cn at gmail.com>於 2021年3月11日 週四，下午10:14寫道：
> >>
> >> On Thu, Mar 11, 2021 at 9:50 PM Green Wan <green.wan at 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.wan at 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?

- Green


> >
> >
> >>
> >> > + *
> >> > + * Copyright (C) 2020-2021 SiFive, Inc.
> >> > + *
> >> > + * Based in early part on the i.MX6 PCIe host controller shim which
> is:
> >> > + *
> >> > + * Copyright (C) 2013 Kosagi
> >> > + *             http://www.kosagi.com
> >> > + *
> >> > + * Based on driver from author: Alan Mikhak <
> amikhak at wirelessfabric.com>
> >> > + */
> >> > +#include "pcie_sifive.h"
> >> > +#include <common.h>
> >> > +#include <dm.h>
> >> > +
> >> > +/* Host Bridge Identification */
> >> > +#define DEVICE_NAME    "SiFive FU740 PCIe Host Controller"
> >> > +#define VENDOR_ID      0x51fe
> >> > +#define DEVICE_ID      0x51fe
> >> > +
> >>
> >> [snip]
>
> Regards,
> Bin
>