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? > + > +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. > + .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? Rest LGTM! - Mani -- மணிவண்ணன் சதாசிவம்