Add new functions to manage recovery of configuration for endpoint controller
and restart the controller when asserting bus-reset from root complex (RC).
This feature is only available if bus-reset (PERST#) line is physically
routed between RC and endpoint, and the signal from RC also resets
the endpoint controller.
This series is only for UniPhier PCIe endpoint controller at this point.
Changes since v1:
- Update the patches to rebase onto the latest tree
Kunihiko Hayashi (3):
PCI: endpoint: Add 'started' to pci_epc to set whether the controller
is started
PCI: endpoint: Add endpoint restart management
PCI: uniphier-ep: Add EPC restart management support
drivers/pci/controller/dwc/Kconfig | 1 +
drivers/pci/controller/dwc/pcie-uniphier-ep.c | 44 +++++++++-
drivers/pci/endpoint/Kconfig | 9 ++
drivers/pci/endpoint/Makefile | 1 +
drivers/pci/endpoint/pci-epc-core.c | 2 +
drivers/pci/endpoint/pci-epc-restart.c | 114 ++++++++++++++++++++++++++
include/linux/pci-epc.h | 22 +++++
7 files changed, 192 insertions(+), 1 deletion(-)
create mode 100644 drivers/pci/endpoint/pci-epc-restart.c
--
2.7.4
Add new functions to manage recovery of configuration parameters and
restart the controller when asserting bus-reset from root-complex (RC).
This feature is only available if bus-reset (PERST#) line is physically
routed between RC and endpoint and the signal from RC affects endpoint.
This feature works as follows.
- This adds a polling thread, that polls and detects the bus-reset signal,
and recovers configuration parameters for endpoint. The polling period
is fixed at EPC_RESTART_POLL_PERIOD_MS.
- After the endpoint controller starts and the user sets configuration
parameters using sysfs or function drivers, once RC asserts bus-reset
and endpoint can receive it, the endpoint controller will also reset
and initialize configuration parameters.
- If the thread detects bus-reset signal, the thread stops the controller,
unbinds it, quickly binds it and restart it. The configuration
paremters are restored to the user's values.
Signed-off-by: Kunihiko Hayashi <[email protected]>
---
drivers/pci/endpoint/Kconfig | 9 +++
drivers/pci/endpoint/Makefile | 1 +
drivers/pci/endpoint/pci-epc-restart.c | 114 +++++++++++++++++++++++++++++++++
include/linux/pci-epc.h | 15 +++++
4 files changed, 139 insertions(+)
create mode 100644 drivers/pci/endpoint/pci-epc-restart.c
diff --git a/drivers/pci/endpoint/Kconfig b/drivers/pci/endpoint/Kconfig
index 17bbdc9..016c82a 100644
--- a/drivers/pci/endpoint/Kconfig
+++ b/drivers/pci/endpoint/Kconfig
@@ -28,6 +28,15 @@ config PCI_ENDPOINT_CONFIGFS
configure the endpoint function and used to bind the
function with a endpoint controller.
+config PCI_ENDPOINT_RESTART
+ bool "PCI Endpoint Restart Management Support"
+ depends on PCI_ENDPOINT
+ help
+ Enable restart management functions, which detects bus-reset
+ from root complex, and recover endpoint configuration.
+ This is only available if bus-reset line is physically routed
+ between root complex and endpoint.
+
source "drivers/pci/endpoint/functions/Kconfig"
endmenu
diff --git a/drivers/pci/endpoint/Makefile b/drivers/pci/endpoint/Makefile
index 95b2fe4..7301aea 100644
--- a/drivers/pci/endpoint/Makefile
+++ b/drivers/pci/endpoint/Makefile
@@ -4,5 +4,6 @@
#
obj-$(CONFIG_PCI_ENDPOINT_CONFIGFS) += pci-ep-cfs.o
+obj-$(CONFIG_PCI_ENDPOINT_RESTART) += pci-epc-restart.o
obj-$(CONFIG_PCI_ENDPOINT) += pci-epc-core.o pci-epf-core.o\
pci-epc-mem.o functions/
diff --git a/drivers/pci/endpoint/pci-epc-restart.c b/drivers/pci/endpoint/pci-epc-restart.c
new file mode 100644
index 0000000..ab956be
--- /dev/null
+++ b/drivers/pci/endpoint/pci-epc-restart.c
@@ -0,0 +1,114 @@
+// SPDX-License-Identifier: GPL-2.0
+/**
+ * PCI Endpoint Controller Restart Management
+ *
+ * Copyright (C) 2019-2020 Socionext Inc.
+ * Author: Kunihiko Hayashi <[email protected]>
+ */
+
+#include <linux/completion.h>
+#include <linux/delay.h>
+#include <linux/kthread.h>
+#include <linux/pci-epc.h>
+#include <linux/pci-epf.h>
+
+#define EPC_RESTART_POLL_PERIOD_MS 80
+
+static int pci_epc_restart_thread(void *dev_id)
+{
+ struct pci_epc *epc = dev_id;
+ struct pci_epf *epf;
+ int ret = 0;
+
+ dev_info(&epc->dev, "[epc-restart] thread start\n");
+
+ while (!kthread_should_stop()) {
+ if (epc->restart_poll) {
+ /* call polling function */
+ ret = epc->restart_poll(epc->restart_poll_data);
+ if (!ret) {
+ msleep(EPC_RESTART_POLL_PERIOD_MS);
+ continue;
+ } else if (ret < 0) {
+ break;
+ }
+ } else {
+ /* wait for interrupt */
+ ret = wait_for_completion_interruptible(&epc->restart_comp);
+ if (ret <= 0)
+ break;
+ }
+
+ if (!pci_epc_is_started(epc))
+ continue;
+
+ /*
+ * After stop linkup, call unbind() once and call bind() again.
+ * to reload config parameters to the controller.
+ */
+ pci_epc_stop(epc);
+ list_for_each_entry(epf, &epc->pci_epf, list) {
+ pci_epf_unbind(epf);
+ pci_epf_bind(epf);
+ }
+ pci_epc_start(epc);
+
+ dev_info(&epc->dev, "[epc-restart] assert\n");
+ }
+
+ dev_info(&epc->dev, "[epc-restart] thread exit\n");
+
+ return ret;
+}
+
+/**
+ * pci_epc_restart_init() - initialize epc-restart thread
+ * @epc: the EPC device
+ */
+int pci_epc_restart_init(struct pci_epc *epc)
+{
+ init_completion(&epc->restart_comp);
+
+ epc->restart_task = kthread_run(pci_epc_restart_thread, epc,
+ "pci-epc-restart");
+ if (IS_ERR(epc->restart_task))
+ return PTR_ERR(epc->restart_task);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(pci_epc_restart_init);
+
+/**
+ * pci_epc_restart_cleanup() - clean up epc-restart thread
+ * @epc: the EPC device
+ */
+void pci_epc_restart_cleanup(struct pci_epc *epc)
+{
+ if (epc->restart_task)
+ kthread_stop(epc->restart_task);
+}
+EXPORT_SYMBOL_GPL(pci_epc_restart_cleanup);
+
+/**
+ * pci_epc_restart_notify() - notify to epc-restart thread
+ * @epc: the EPC device
+ */
+void pci_epc_restart_notify(struct pci_epc *epc)
+{
+ complete(&epc->restart_comp);
+}
+EXPORT_SYMBOL_GPL(pci_epc_restart_notify);
+
+/**
+ * pci_epc_restart_register_poll_func() - register polling method for restart thread
+ * @epc: the EPC device
+ * @func: polling function
+ * @data: data for polling function
+ */
+void pci_epc_restart_register_poll_func(struct pci_epc *epc, bool (*func)(void *),
+ void *data)
+{
+ epc->restart_poll = func;
+ epc->restart_poll_data = data;
+}
+EXPORT_SYMBOL_GPL(pci_epc_restart_register_poll_func);
diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
index 5808952..45d2592 100644
--- a/include/linux/pci-epc.h
+++ b/include/linux/pci-epc.h
@@ -132,6 +132,10 @@ struct pci_epc_mem {
* @function_num_map: bitmap to manage physical function number
* @notifier: used to notify EPF of any EPC events (like linkup)
* @started: true if this EPC is started
+ * @restart_task: pointer to task for restart thread
+ * @restart_comp: completion object for receiving reset interrupts from RC
+ * @restart_poll: function to poll reset detection from RC
+ * @restart_poll_data: an argument that restart_poll function gets
*/
struct pci_epc {
struct device dev;
@@ -147,6 +151,11 @@ struct pci_epc {
unsigned long function_num_map;
struct atomic_notifier_head notifier;
bool started;
+ /* for epc-restart */
+ struct task_struct *restart_task;
+ struct completion restart_comp;
+ bool (*restart_poll)(void *func);
+ void *restart_poll_data;
};
/**
@@ -254,4 +263,10 @@ void __iomem *pci_epc_mem_alloc_addr(struct pci_epc *epc,
phys_addr_t *phys_addr, size_t size);
void pci_epc_mem_free_addr(struct pci_epc *epc, phys_addr_t phys_addr,
void __iomem *virt_addr, size_t size);
+int pci_epc_restart_init(struct pci_epc *epc);
+void pci_epc_restart_cleanup(struct pci_epc *epc);
+void pci_epc_restart_notify(struct pci_epc *epc);
+void pci_epc_restart_register_poll_func(struct pci_epc *epc,
+ bool (*func)(void *), void *data);
+
#endif /* __LINUX_PCI_EPC_H */
--
2.7.4
This adds a member 'started' as a boolean value to struct pci_epc to set
whether the controller is started, and also adds a function to get the
value.
Signed-off-by: Kunihiko Hayashi <[email protected]>
---
drivers/pci/endpoint/pci-epc-core.c | 2 ++
include/linux/pci-epc.h | 7 +++++++
2 files changed, 9 insertions(+)
diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
index cc8f9eb..2904175 100644
--- a/drivers/pci/endpoint/pci-epc-core.c
+++ b/drivers/pci/endpoint/pci-epc-core.c
@@ -174,6 +174,7 @@ void pci_epc_stop(struct pci_epc *epc)
mutex_lock(&epc->lock);
epc->ops->stop(epc);
+ epc->started = false;
mutex_unlock(&epc->lock);
}
EXPORT_SYMBOL_GPL(pci_epc_stop);
@@ -196,6 +197,7 @@ int pci_epc_start(struct pci_epc *epc)
mutex_lock(&epc->lock);
ret = epc->ops->start(epc);
+ epc->started = true;
mutex_unlock(&epc->lock);
return ret;
diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
index b82c9b1..5808952 100644
--- a/include/linux/pci-epc.h
+++ b/include/linux/pci-epc.h
@@ -131,6 +131,7 @@ struct pci_epc_mem {
* @lock: mutex to protect pci_epc ops
* @function_num_map: bitmap to manage physical function number
* @notifier: used to notify EPF of any EPC events (like linkup)
+ * @started: true if this EPC is started
*/
struct pci_epc {
struct device dev;
@@ -145,6 +146,7 @@ struct pci_epc {
struct mutex lock;
unsigned long function_num_map;
struct atomic_notifier_head notifier;
+ bool started;
};
/**
@@ -191,6 +193,11 @@ pci_epc_register_notifier(struct pci_epc *epc, struct notifier_block *nb)
return atomic_notifier_chain_register(&epc->notifier, nb);
}
+static inline bool pci_epc_is_started(struct pci_epc *epc)
+{
+ return epc->started;
+}
+
struct pci_epc *
__devm_pci_epc_create(struct device *dev, const struct pci_epc_ops *ops,
struct module *owner);
--
2.7.4
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 <[email protected]>
---
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;
+
+ 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 = {
--
2.7.4
Hi Kunihiko,
On 24/01/21 8:39 pm, Kunihiko Hayashi wrote:
> This adds a member 'started' as a boolean value to struct pci_epc to set
> whether the controller is started, and also adds a function to get the
> value.
>
> Signed-off-by: Kunihiko Hayashi <[email protected]>
> ---
> drivers/pci/endpoint/pci-epc-core.c | 2 ++
> include/linux/pci-epc.h | 7 +++++++
> 2 files changed, 9 insertions(+)
>
> diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
> index cc8f9eb..2904175 100644
> --- a/drivers/pci/endpoint/pci-epc-core.c
> +++ b/drivers/pci/endpoint/pci-epc-core.c
> @@ -174,6 +174,7 @@ void pci_epc_stop(struct pci_epc *epc)
>
> mutex_lock(&epc->lock);
> epc->ops->stop(epc);
> + epc->started = false;
> mutex_unlock(&epc->lock);
> }
> EXPORT_SYMBOL_GPL(pci_epc_stop);
> @@ -196,6 +197,7 @@ int pci_epc_start(struct pci_epc *epc)
>
> mutex_lock(&epc->lock);
> ret = epc->ops->start(epc);
> + epc->started = true;
> mutex_unlock(&epc->lock);
>
> return ret;
> diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
> index b82c9b1..5808952 100644
> --- a/include/linux/pci-epc.h
> +++ b/include/linux/pci-epc.h
> @@ -131,6 +131,7 @@ struct pci_epc_mem {
> * @lock: mutex to protect pci_epc ops
> * @function_num_map: bitmap to manage physical function number
> * @notifier: used to notify EPF of any EPC events (like linkup)
> + * @started: true if this EPC is started
> */
> struct pci_epc {
> struct device dev;
> @@ -145,6 +146,7 @@ struct pci_epc {
> struct mutex lock;
> unsigned long function_num_map;
> struct atomic_notifier_head notifier;
> + bool started;
> };
>
> /**
> @@ -191,6 +193,11 @@ pci_epc_register_notifier(struct pci_epc *epc, struct notifier_block *nb)
> return atomic_notifier_chain_register(&epc->notifier, nb);
> }
>
> +static inline bool pci_epc_is_started(struct pci_epc *epc)
> +{
> + return epc->started;
> +}
This should also be protected.
Thanks
Kishon
> +
> struct pci_epc *
> __devm_pci_epc_create(struct device *dev, const struct pci_epc_ops *ops,
> struct module *owner);
>
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 <[email protected]>
> ---
> 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 <[email protected]>
>> ---
>> 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
Hi Kishon,
On 2021/01/28 23:11, Kishon Vijay Abraham I wrote:
> Hi Kunihiko,
>
> On 24/01/21 8:39 pm, Kunihiko Hayashi wrote:
>> This adds a member 'started' as a boolean value to struct pci_epc to set
>> whether the controller is started, and also adds a function to get the
>> value.
>>
>> Signed-off-by: Kunihiko Hayashi <[email protected]>
>> ---
>> drivers/pci/endpoint/pci-epc-core.c | 2 ++
>> include/linux/pci-epc.h | 7 +++++++
>> 2 files changed, 9 insertions(+)
>>
>> diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
>> index cc8f9eb..2904175 100644
>> --- a/drivers/pci/endpoint/pci-epc-core.c
>> +++ b/drivers/pci/endpoint/pci-epc-core.c
>> @@ -174,6 +174,7 @@ void pci_epc_stop(struct pci_epc *epc)
>>
>> mutex_lock(&epc->lock);
>> epc->ops->stop(epc);
>> + epc->started = false;
>> mutex_unlock(&epc->lock);
>> }
>> EXPORT_SYMBOL_GPL(pci_epc_stop);
>> @@ -196,6 +197,7 @@ int pci_epc_start(struct pci_epc *epc)
>>
>> mutex_lock(&epc->lock);
>> ret = epc->ops->start(epc);
>> + epc->started = true;
>> mutex_unlock(&epc->lock);
>>
>> return ret;
>> diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
>> index b82c9b1..5808952 100644
>> --- a/include/linux/pci-epc.h
>> +++ b/include/linux/pci-epc.h
>> @@ -131,6 +131,7 @@ struct pci_epc_mem {
>> * @lock: mutex to protect pci_epc ops
>> * @function_num_map: bitmap to manage physical function number
>> * @notifier: used to notify EPF of any EPC events (like linkup)
>> + * @started: true if this EPC is started
>> */
>> struct pci_epc {
>> struct device dev;
>> @@ -145,6 +146,7 @@ struct pci_epc {
>> struct mutex lock;
>> unsigned long function_num_map;
>> struct atomic_notifier_head notifier;
>> + bool started;
>> };
>>
>> /**
>> @@ -191,6 +193,11 @@ pci_epc_register_notifier(struct pci_epc *epc, struct notifier_block *nb)
>> return atomic_notifier_chain_register(&epc->notifier, nb);
>> }
>>
>> +static inline bool pci_epc_is_started(struct pci_epc *epc)
>> +{
>> + return epc->started;
>> +}
>
> This should also be protected.
Ok, I prepared this function for restart management in patch 2/3.
This also needs to be reconsidered.
Thank you,
---
Best Regards
Kunihiko Hayashi