[v2,3/3] PCI: uniphier-ep: Add EPC restart management support
diff mbox series

Message ID 1611500977-24816-4-git-send-email-hayashi.kunihiko@socionext.com
State New, archived
Headers show
Series
  • PCI: endpoint: Add endpoint restart management support
Related show

Commit Message

Kunihiko Hayashi Jan. 24, 2021, 3:09 p.m. UTC
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(-)

Comments

Kishon Vijay Abraham I Jan. 28, 2021, 2:29 p.m. UTC | #1
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 = {
>
Kunihiko Hayashi Feb. 2, 2021, 4:13 p.m. UTC | #2
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

Patch
diff mbox series

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 = {