Hello Mani,
On Sat, May 04, 2024 at 11:02:01PM +0530, Manivannan Sadhasivam wrote:
> On Tue, Apr 30, 2024 at 02:01:08PM +0200, Niklas Cassel wrote:
> > The PCIe controller in rk3568 and rk3588 can operate in endpoint mode.
> > This endpoint mode support heavily leverages the existing code in
> > pcie-designware-ep.c.
> >
> > Add support for endpoint mode to the existing pcie-dw-rockchip glue
> > driver.
> >
> > Signed-off-by: Niklas Cassel <cassel@xxxxxxxxxx>
> > ---
> > drivers/pci/controller/dwc/Kconfig | 17 ++-
> > drivers/pci/controller/dwc/pcie-dw-rockchip.c | 177 ++++++++++++++++++++++++++
> > 2 files changed, 191 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> > index 8afacc90c63b..9fae0d977271 100644
> > --- a/drivers/pci/controller/dwc/Kconfig
> > +++ b/drivers/pci/controller/dwc/Kconfig
> > @@ -311,16 +311,27 @@ config PCIE_RCAR_GEN4_EP
> > SoCs. To compile this driver as a module, choose M here: the module
> > will be called pcie-rcar-gen4.ko. This uses the DesignWare core.
> >
> > +config PCIE_ROCKCHIP_DW
> > + bool
> > +
> > config PCIE_ROCKCHIP_DW_HOST
> > - bool "Rockchip DesignWare PCIe controller"
> > - select PCIE_DW
> > + bool "Rockchip DesignWare PCIe controller (host mode)"
> > select PCIE_DW_HOST
> > depends on PCI_MSI
> > depends on ARCH_ROCKCHIP || COMPILE_TEST
> > depends on OF
> > help
> > Enables support for the DesignWare PCIe controller in the
> > - Rockchip SoC except RK3399.
> > + Rockchip SoC (except RK3399) to work in host mode.
>
> Just curious. RK3399 is an exception because lack of driver support or it
> doesn't support EP mode at all?
RK3399 is an exception because it uses a non-DWC PCIe controller.
RK3399 has support for both RC and EP mode, see:
CONFIG_PCIE_ROCKCHIP_HOST
drivers/pci/controller/pcie-rockchip.c
and
CONFIG_PCIE_ROCKCHIP_EP
drivers/pci/controller/pcie-rockchip-ep.c
>
> > +
> > +config PCIE_ROCKCHIP_DW_EP
> > + bool "Rockchip DesignWare PCIe controller (endpoint mode)"
> > + select PCIE_DW_EP
> > + depends on ARCH_ROCKCHIP || COMPILE_TEST
> > + depends on OF
> > + help
> > + Enables support for the DesignWare PCIe controller in the
> > + Rockchip SoC (except RK3399) to work in endpoint mode.
> >
> > config PCI_EXYNOS
> > tristate "Samsung Exynos PCIe controller"
> > diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> > index f38d267e4e64..7614c20c7112 100644
> > --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> > +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> > @@ -34,10 +34,16 @@
> > #define to_rockchip_pcie(x) dev_get_drvdata((x)->dev)
> >
> > #define PCIE_CLIENT_RC_MODE HIWORD_UPDATE_BIT(0x40)
> > +#define PCIE_CLIENT_EP_MODE HIWORD_UPDATE(0xf0, 0x0)
> > #define PCIE_CLIENT_ENABLE_LTSSM HIWORD_UPDATE_BIT(0xc)
> > +#define PCIE_CLIENT_DISABLE_LTSSM HIWORD_UPDATE(0x0c, 0x8)
> > +#define PCIE_CLIENT_INTR_STATUS_MISC 0x10
> > +#define PCIE_CLIENT_INTR_MASK_MISC 0x24
> > #define PCIE_SMLH_LINKUP BIT(16)
> > #define PCIE_RDLH_LINKUP BIT(17)
> > #define PCIE_LINKUP (PCIE_SMLH_LINKUP | PCIE_RDLH_LINKUP)
> > +#define PCIE_RDLH_LINK_UP_CHGED BIT(1)
> > +#define PCIE_LINK_REQ_RST_NOT_INT BIT(2)
> > #define PCIE_L0S_ENTRY 0x11
> > #define PCIE_CLIENT_GENERAL_CONTROL 0x0
> > #define PCIE_CLIENT_INTR_STATUS_LEGACY 0x8
> > @@ -159,6 +165,12 @@ static void rockchip_pcie_enable_ltssm(struct rockchip_pcie *rockchip)
> > PCIE_CLIENT_GENERAL_CONTROL);
> > }
> >
> > +static void rockchip_pcie_disable_ltssm(struct rockchip_pcie *rockchip)
> > +{
> > + rockchip_pcie_writel_apb(rockchip, PCIE_CLIENT_DISABLE_LTSSM,
> > + PCIE_CLIENT_GENERAL_CONTROL);
> > +}
> > +
> > static int rockchip_pcie_link_up(struct dw_pcie *pci)
> > {
> > struct rockchip_pcie *rockchip = to_rockchip_pcie(pci);
> > @@ -195,6 +207,13 @@ static int rockchip_pcie_start_link(struct dw_pcie *pci)
> > return 0;
> > }
> >
> > +static void rockchip_pcie_stop_link(struct dw_pcie *pci)
> > +{
> > + struct rockchip_pcie *rockchip = to_rockchip_pcie(pci);
> > +
> > + rockchip_pcie_disable_ltssm(rockchip);
> > +}
> > +
> > static int rockchip_pcie_host_init(struct dw_pcie_rp *pp)
> > {
> > struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > @@ -220,6 +239,59 @@ static const struct dw_pcie_host_ops rockchip_pcie_host_ops = {
> > .init = rockchip_pcie_host_init,
> > };
> >
> > +static void rockchip_pcie_ep_init(struct dw_pcie_ep *ep)
> > +{
> > + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > + enum pci_barno bar;
> > +
> > + for (bar = 0; bar < PCI_STD_NUM_BARS; bar++)
> > + dw_pcie_ep_reset_bar(pci, bar);
> > +};
> > +
> > +static int rockchip_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
> > + unsigned int type, u16 interrupt_num)
> > +{
> > + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > +
> > + switch (type) {
> > + case PCI_IRQ_INTX:
> > + return dw_pcie_ep_raise_intx_irq(ep, func_no);
> > + case PCI_IRQ_MSI:
> > + return dw_pcie_ep_raise_msi_irq(ep, func_no, interrupt_num);
> > + case PCI_IRQ_MSIX:
> > + return dw_pcie_ep_raise_msix_irq(ep, func_no, interrupt_num);
> > + default:
> > + dev_err(pci->dev, "UNKNOWN IRQ type\n");
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static const struct pci_epc_features rockchip_pcie_epc_features = {
> > + .linkup_notifier = true,
> > + .msi_capable = true,
> > + .msix_capable = true,
> > + .align = SZ_64K,
> > + .bar[BAR_0] = { .type = BAR_FIXED, .fixed_size = SZ_1M, },
> > + .bar[BAR_1] = { .type = BAR_FIXED, .fixed_size = SZ_1M, },
> > + .bar[BAR_2] = { .type = BAR_FIXED, .fixed_size = SZ_1M, },
> > + .bar[BAR_3] = { .type = BAR_FIXED, .fixed_size = SZ_1M, },
> > + .bar[BAR_4] = { .type = BAR_RESERVED, },
>
> You have documented the reason for this in cover letter. But it'd be good if you
> do the same in commit message also.
Will do!
It appears that rk3568 does not have ATU and DMA regs mapped to BAR4
(or any other BAR).
I will create a separate "static const struct pci_epc_features"
for rk3568.
>
> > + .bar[BAR_5] = { .type = BAR_FIXED, .fixed_size = SZ_1M, },
> > +};
> > +
> > +static const struct pci_epc_features *
> > +rockchip_pcie_get_features(struct dw_pcie_ep *ep)
> > +{
> > + return &rockchip_pcie_epc_features;
> > +}
> > +
> > +static const struct dw_pcie_ep_ops rockchip_pcie_ep_ops = {
> > + .init = rockchip_pcie_ep_init,
> > + .raise_irq = rockchip_pcie_raise_irq,
> > + .get_features = rockchip_pcie_get_features,
> > +};
> > +
> > static int rockchip_pcie_clk_init(struct rockchip_pcie *rockchip)
> > {
> > struct device *dev = rockchip->pci.dev;
> > @@ -284,8 +356,39 @@ static void rockchip_pcie_phy_deinit(struct rockchip_pcie *rockchip)
> > static const struct dw_pcie_ops dw_pcie_ops = {
> > .link_up = rockchip_pcie_link_up,
> > .start_link = rockchip_pcie_start_link,
> > + .stop_link = rockchip_pcie_stop_link,
> > };
> >
> > +static irqreturn_t rockchip_pcie_ep_sys_irq_thread(int irq, void *arg)
> > +{
> > + struct rockchip_pcie *rockchip = arg;
> > + struct dw_pcie *pci = &rockchip->pci;
> > + struct device *dev = pci->dev;
> > + u32 reg, val;
> > +
> > + reg = rockchip_pcie_readl_apb(rockchip, PCIE_CLIENT_INTR_STATUS_MISC);
> > +
> > + dev_dbg(dev, "PCIE_CLIENT_INTR_STATUS_MISC: %#x\n", reg);
> > + dev_dbg(dev, "LTSSM_STATUS: %#x\n", rockchip_pcie_ltssm(rockchip));
> > +
> > + if (reg & PCIE_LINK_REQ_RST_NOT_INT) {
> > + dev_dbg(dev, "hot reset or link-down reset\n");
>
> 'hot reset' means the host doing a hot reset?
Hot reset means that LTSSM state machine is in state "Hot Reset".
I don't know all the reasons why the state machine can go to this state
by heart, but from the Databook:
"A Downstream Port (DSP) can hot reset an Upstream Port (USP) by sending
two consecutive TS1 ordered sets with the hot reset bit asserted"
"There is no difference in the handling of a link-down reset or a hot reset;
the controller asserts the link_req_rst_not output requesting the
DWC_pcie_clkrst.v module to reset the controller."
Kind regards,
Niklas
