| Message ID | 1611500977-24816-4-git-send-email-hayashi.kunihiko@socionext.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Lorenzo Pieralisi |
| Headers | show |
| Series | PCI: endpoint: Add endpoint restart management support | expand |
Hi Kunihiko, On 24/01/21 8:39 pm, Kunihiko Hayashi wrote: > Set the polling function and call the init function to enable EPC restart > management. The polling function detects that the bus-reset signal is a > rising edge. > > Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com> > --- > drivers/pci/controller/dwc/Kconfig | 1 + > drivers/pci/controller/dwc/pcie-uniphier-ep.c | 44 ++++++++++++++++++++++++++- > 2 files changed, 44 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig > index 22c5529..90d400a 100644 > --- a/drivers/pci/controller/dwc/Kconfig > +++ b/drivers/pci/controller/dwc/Kconfig > @@ -302,6 +302,7 @@ config PCIE_UNIPHIER_EP > depends on OF && HAS_IOMEM > depends on PCI_ENDPOINT > select PCIE_DW_EP > + select PCI_ENDPOINT_RESTART > help > Say Y here if you want PCIe endpoint controller support on > UniPhier SoCs. This driver supports Pro5 SoC. > diff --git a/drivers/pci/controller/dwc/pcie-uniphier-ep.c b/drivers/pci/controller/dwc/pcie-uniphier-ep.c > index 69810c6..9d83850 100644 > --- a/drivers/pci/controller/dwc/pcie-uniphier-ep.c > +++ b/drivers/pci/controller/dwc/pcie-uniphier-ep.c > @@ -26,6 +26,7 @@ > #define PCL_RSTCTRL_PIPE3 BIT(0) > > #define PCL_RSTCTRL1 0x0020 > +#define PCL_RSTCTRL_PERST_MON BIT(16) > #define PCL_RSTCTRL_PERST BIT(0) > > #define PCL_RSTCTRL2 0x0024 > @@ -60,6 +61,7 @@ struct uniphier_pcie_ep_priv { > struct clk *clk, *clk_gio; > struct reset_control *rst, *rst_gio; > struct phy *phy; > + bool bus_reset_status; > const struct pci_epc_features *features; > }; > > @@ -212,6 +214,41 @@ uniphier_pcie_get_features(struct dw_pcie_ep *ep) > return priv->features; > } > > +static bool uniphier_pcie_ep_poll_reset(void *data) > +{ > + struct uniphier_pcie_ep_priv *priv = data; > + bool ret, status; > + > + if (!priv) > + return false; > + > + status = !(readl(priv->base + PCL_RSTCTRL1) & PCL_RSTCTRL_PERST_MON); > + > + /* return true if the rising edge of bus reset is detected */ > + ret = !!(status == false && priv->bus_reset_status == true); > + priv->bus_reset_status = status; I'm still not convinced about having a separate library for restart management but shouldn't we reset the function driver on falling edge? After the rising edge the host expects the endpoint to be ready. Why not use the CORE_INIT (core_init_notifier) infrastructure? Thanks Kishon > + > + return ret; > +} > + > +static int uniphier_pcie_ep_init_complete(struct dw_pcie_ep *ep) > +{ > + struct dw_pcie *pci = to_dw_pcie_from_ep(ep); > + struct uniphier_pcie_ep_priv *priv = to_uniphier_pcie(pci); > + int ret; > + > + /* Set up epc-restart thread */ > + pci_epc_restart_register_poll_func(ep->epc, > + uniphier_pcie_ep_poll_reset, priv); > + /* With call of poll_reset() directly, initialize internal state */ > + uniphier_pcie_ep_poll_reset(priv); > + ret = pci_epc_restart_init(ep->epc); > + if (ret) > + dev_err(pci->dev, "Failed to initialize epc-restart (%d)\n", ret); > + > + return ret; > +} > + > static const struct dw_pcie_ep_ops uniphier_pcie_ep_ops = { > .ep_init = uniphier_pcie_ep_init, > .raise_irq = uniphier_pcie_ep_raise_irq, > @@ -318,7 +355,12 @@ static int uniphier_pcie_ep_probe(struct platform_device *pdev) > return ret; > > priv->pci.ep.ops = &uniphier_pcie_ep_ops; > - return dw_pcie_ep_init(&priv->pci.ep); > + > + ret = dw_pcie_ep_init(&priv->pci.ep); > + if (ret) > + return ret; > + > + return uniphier_pcie_ep_init_complete(&priv->pci.ep); > } > > static const struct pci_epc_features uniphier_pro5_data = { >
Hi Kishon, Thank you for your comment. On 2021/01/28 23:29, Kishon Vijay Abraham I wrote: > Hi Kunihiko, > > On 24/01/21 8:39 pm, Kunihiko Hayashi wrote: >> Set the polling function and call the init function to enable EPC restart >> management. The polling function detects that the bus-reset signal is a >> rising edge. >> >> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com> >> --- >> drivers/pci/controller/dwc/Kconfig | 1 + >> drivers/pci/controller/dwc/pcie-uniphier-ep.c | 44 ++++++++++++++++++++++++++- >> 2 files changed, 44 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig >> index 22c5529..90d400a 100644 >> --- a/drivers/pci/controller/dwc/Kconfig >> +++ b/drivers/pci/controller/dwc/Kconfig >> @@ -302,6 +302,7 @@ config PCIE_UNIPHIER_EP >> depends on OF && HAS_IOMEM >> depends on PCI_ENDPOINT >> select PCIE_DW_EP >> + select PCI_ENDPOINT_RESTART >> help >> Say Y here if you want PCIe endpoint controller support on >> UniPhier SoCs. This driver supports Pro5 SoC. >> diff --git a/drivers/pci/controller/dwc/pcie-uniphier-ep.c b/drivers/pci/controller/dwc/pcie-uniphier-ep.c >> index 69810c6..9d83850 100644 >> --- a/drivers/pci/controller/dwc/pcie-uniphier-ep.c >> +++ b/drivers/pci/controller/dwc/pcie-uniphier-ep.c >> @@ -26,6 +26,7 @@ >> #define PCL_RSTCTRL_PIPE3 BIT(0) >> >> #define PCL_RSTCTRL1 0x0020 >> +#define PCL_RSTCTRL_PERST_MON BIT(16) >> #define PCL_RSTCTRL_PERST BIT(0) >> >> #define PCL_RSTCTRL2 0x0024 >> @@ -60,6 +61,7 @@ struct uniphier_pcie_ep_priv { >> struct clk *clk, *clk_gio; >> struct reset_control *rst, *rst_gio; >> struct phy *phy; >> + bool bus_reset_status; >> const struct pci_epc_features *features; >> }; >> >> @@ -212,6 +214,41 @@ uniphier_pcie_get_features(struct dw_pcie_ep *ep) >> return priv->features; >> } >> >> +static bool uniphier_pcie_ep_poll_reset(void *data) >> +{ >> + struct uniphier_pcie_ep_priv *priv = data; >> + bool ret, status; >> + >> + if (!priv) >> + return false; >> + >> + status = !(readl(priv->base + PCL_RSTCTRL1) & PCL_RSTCTRL_PERST_MON); >> + >> + /* return true if the rising edge of bus reset is detected */ >> + ret = !!(status == false && priv->bus_reset_status == true); >> + priv->bus_reset_status = status; > > I'm still not convinced about having a separate library for restart > management but shouldn't we reset the function driver on falling edge? I understand your opnion well. There might not be enough way to give controller-specific features to handle "restart" as a common function. > After the rising edge the host expects the endpoint to be ready. I see. I didn't consider that restart was completed just after the rising edge. > Why not use the CORE_INIT (core_init_notifier) infrastructure? I don't follow the CORE_INIT yet, so I'll try to introduce it into the driver. However, our current controller doesn't have an interrupt that detects PERST like pcie-tegra194. I think the driver needs a thread for polling PERST like patch 2/3. Thank you, --- Best Regards Kunihiko Hayashi
diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig index 22c5529..90d400a 100644 --- a/drivers/pci/controller/dwc/Kconfig +++ b/drivers/pci/controller/dwc/Kconfig @@ -302,6 +302,7 @@ config PCIE_UNIPHIER_EP depends on OF && HAS_IOMEM depends on PCI_ENDPOINT select PCIE_DW_EP + select PCI_ENDPOINT_RESTART help Say Y here if you want PCIe endpoint controller support on UniPhier SoCs. This driver supports Pro5 SoC. diff --git a/drivers/pci/controller/dwc/pcie-uniphier-ep.c b/drivers/pci/controller/dwc/pcie-uniphier-ep.c index 69810c6..9d83850 100644 --- a/drivers/pci/controller/dwc/pcie-uniphier-ep.c +++ b/drivers/pci/controller/dwc/pcie-uniphier-ep.c @@ -26,6 +26,7 @@ #define PCL_RSTCTRL_PIPE3 BIT(0) #define PCL_RSTCTRL1 0x0020 +#define PCL_RSTCTRL_PERST_MON BIT(16) #define PCL_RSTCTRL_PERST BIT(0) #define PCL_RSTCTRL2 0x0024 @@ -60,6 +61,7 @@ struct uniphier_pcie_ep_priv { struct clk *clk, *clk_gio; struct reset_control *rst, *rst_gio; struct phy *phy; + bool bus_reset_status; const struct pci_epc_features *features; }; @@ -212,6 +214,41 @@ uniphier_pcie_get_features(struct dw_pcie_ep *ep) return priv->features; } +static bool uniphier_pcie_ep_poll_reset(void *data) +{ + struct uniphier_pcie_ep_priv *priv = data; + bool ret, status; + + if (!priv) + return false; + + status = !(readl(priv->base + PCL_RSTCTRL1) & PCL_RSTCTRL_PERST_MON); + + /* return true if the rising edge of bus reset is detected */ + ret = !!(status == false && priv->bus_reset_status == true); + priv->bus_reset_status = status; + + return ret; +} + +static int uniphier_pcie_ep_init_complete(struct dw_pcie_ep *ep) +{ + struct dw_pcie *pci = to_dw_pcie_from_ep(ep); + struct uniphier_pcie_ep_priv *priv = to_uniphier_pcie(pci); + int ret; + + /* Set up epc-restart thread */ + pci_epc_restart_register_poll_func(ep->epc, + uniphier_pcie_ep_poll_reset, priv); + /* With call of poll_reset() directly, initialize internal state */ + uniphier_pcie_ep_poll_reset(priv); + ret = pci_epc_restart_init(ep->epc); + if (ret) + dev_err(pci->dev, "Failed to initialize epc-restart (%d)\n", ret); + + return ret; +} + static const struct dw_pcie_ep_ops uniphier_pcie_ep_ops = { .ep_init = uniphier_pcie_ep_init, .raise_irq = uniphier_pcie_ep_raise_irq, @@ -318,7 +355,12 @@ static int uniphier_pcie_ep_probe(struct platform_device *pdev) return ret; priv->pci.ep.ops = &uniphier_pcie_ep_ops; - return dw_pcie_ep_init(&priv->pci.ep); + + ret = dw_pcie_ep_init(&priv->pci.ep); + if (ret) + return ret; + + return uniphier_pcie_ep_init_complete(&priv->pci.ep); } static const struct pci_epc_features uniphier_pro5_data = {
Set the polling function and call the init function to enable EPC restart management. The polling function detects that the bus-reset signal is a rising edge. Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com> --- drivers/pci/controller/dwc/Kconfig | 1 + drivers/pci/controller/dwc/pcie-uniphier-ep.c | 44 ++++++++++++++++++++++++++- 2 files changed, 44 insertions(+), 1 deletion(-)