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

[Bin Meng]

Hi Green,

On Fri, Mar 12, 2021 at 4:05 PM Green Wan <green.wan at sifive.com> wrote:
>
>
>
> 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?

I think this approach makes sense.

Regards,
Bin