Re: [PATCH v12 7/8] PCI: dwc: ep: Call dw_pcie_ep_init_registers() API directly from all glue drivers

Frank Li
2024-03-29T19:16:39Z

On Wed, Mar 27, 2024 at 02:43:36PM +0530, Manivannan Sadhasivam wrote:
> Currently, dw_pcie_ep_init_registers() API is directly called by the glue
> drivers requiring active refclk from host. But for the other drivers, it is
> getting called implicitly by dw_pcie_ep_init(). This is due to the fact
> that this API initializes DWC EP specific registers and that requires an
> active refclk (either from host or generated locally by endpoint itsef).
>
> But, this causes a discrepancy among the glue drivers. So to avoid this
> confusion, let's call this API directly from all glue drivers irrespective
> of refclk dependency. Only difference here is that the drivers requiring
> refclk from host will call this API only after the refclk is received and
> other drivers without refclk dependency will call this API right after
> dw_pcie_ep_init().
>
> With this change, the check for 'core_init_notifier' flag can now be
> dropped from dw_pcie_ep_init() API. This will also allow us to remove the
> 'core_init_notifier' flag completely in the later commits.
>
> Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Reviewed-by: Niklas Cassel <cassel@kernel.org>

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
> drivers/pci/controller/dwc/pci-dra7xx.c | 7 +++++++
> drivers/pci/controller/dwc/pci-imx6.c | 8 ++++++++
> drivers/pci/controller/dwc/pci-keystone.c | 9 +++++++++
> drivers/pci/controller/dwc/pci-layerscape-ep.c | 7 +++++++
> drivers/pci/controller/dwc/pcie-artpec6.c | 13 ++++++++++++-
> drivers/pci/controller/dwc/pcie-designware-ep.c | 22 ----------------------
> drivers/pci/controller/dwc/pcie-designware-plat.c | 9 +++++++++
> drivers/pci/controller/dwc/pcie-keembay.c | 16 +++++++++++++++-
> drivers/pci/controller/dwc/pcie-rcar-gen4.c | 12 +++++++++++-
> drivers/pci/controller/dwc/pcie-uniphier-ep.c | 13 ++++++++++++-
> 10 files changed, 90 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pci-dra7xx.c b/drivers/pci/controller/dwc/pci-dra7xx.c
> index 0e406677060d..395042b29ffc 100644
> --- a/drivers/pci/controller/dwc/pci-dra7xx.c
> +++ b/drivers/pci/controller/dwc/pci-dra7xx.c
> @@ -467,6 +467,13 @@ static int dra7xx_add_pcie_ep(struct dra7xx_pcie *dra7xx,
> return ret;
> }
>
> + ret = dw_pcie_ep_init_registers(ep);
> + if (ret) {
> + dev_err(dev, "Failed to initialize DWC endpoint registers\n");
> + dw_pcie_ep_deinit(ep);
> + return ret;
> + }
> +
> return 0;
> }
>
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 99a60270b26c..8d28ecc381bc 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -1123,6 +1123,14 @@ static int imx6_add_pcie_ep(struct imx6_pcie *imx6_pcie,
> dev_err(dev, "failed to initialize endpoint\n");
> return ret;
> }
> +
> + ret = dw_pcie_ep_init_registers(ep);
> + if (ret) {
> + dev_err(dev, "Failed to initialize DWC endpoint registers\n");
> + dw_pcie_ep_deinit(ep);
> + return ret;
> + }
> +
> /* Start LTSSM. */
> imx6_pcie_ltssm_enable(dev);
>
> diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
> index 844de4418724..81ebac520650 100644
> --- a/drivers/pci/controller/dwc/pci-keystone.c
> +++ b/drivers/pci/controller/dwc/pci-keystone.c
> @@ -1286,6 +1286,13 @@ static int ks_pcie_probe(struct platform_device *pdev)
> ret = dw_pcie_ep_init(&pci->ep);
> if (ret < 0)
> goto err_get_sync;
> +
> + ret = dw_pcie_ep_init_registers(&pci->ep);
> + if (ret) {
> + dev_err(dev, "Failed to initialize DWC endpoint registers\n");
> + goto err_ep_init;
> + }
> +
> break;
> default:
> dev_err(dev, "INVALID device type %d\n", mode);
> @@ -1295,6 +1302,8 @@ static int ks_pcie_probe(struct platform_device *pdev)
>
> return 0;
>
> +err_ep_init:
> + dw_pcie_ep_deinit(&pci->ep);
> err_get_sync:
> pm_runtime_put(dev);
> pm_runtime_disable(dev);
> diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> index 1f6ee1460ec2..9eb2233e3d7f 100644
> --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> @@ -279,6 +279,13 @@ static int __init ls_pcie_ep_probe(struct platform_device *pdev)
> if (ret)
> return ret;
>
> + ret = dw_pcie_ep_init_registers(&pci->ep);
> + if (ret) {
> + dev_err(dev, "Failed to initialize DWC endpoint registers\n");
> + dw_pcie_ep_deinit(&pci->ep);
> + return ret;
> + }
> +
> return ls_pcie_ep_interrupt_init(pcie, pdev);
> }
>
> diff --git a/drivers/pci/controller/dwc/pcie-artpec6.c b/drivers/pci/controller/dwc/pcie-artpec6.c
> index 9ed0a9ba7619..a6095561db4a 100644
> --- a/drivers/pci/controller/dwc/pcie-artpec6.c
> +++ b/drivers/pci/controller/dwc/pcie-artpec6.c
> @@ -441,7 +441,18 @@ static int artpec6_pcie_probe(struct platform_device *pdev)
>
> pci->ep.ops = &pcie_ep_ops;
>
> - return dw_pcie_ep_init(&pci->ep);
> + ret = dw_pcie_ep_init(&pci->ep);
> + if (ret)
> + return ret;
> +
> + ret = dw_pcie_ep_init_registers(&pci->ep);
> + if (ret) {
> + dev_err(dev, "Failed to initialize DWC endpoint registers\n");
> + dw_pcie_ep_deinit(&pci->ep);
> + return ret;
> + }
> +
> + break;
> default:
> dev_err(dev, "INVALID device type %d\n", artpec6_pcie->mode);
> }
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index 761d3012a073..2063cf2049e5 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -821,7 +821,6 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> struct device *dev = pci->dev;
> struct platform_device *pdev = to_platform_device(dev);
> struct device_node *np = dev->of_node;
> - const struct pci_epc_features *epc_features;
>
> INIT_LIST_HEAD(&ep->func_list);
>
> @@ -867,29 +866,8 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> goto err_exit_epc_mem;
> }
>
> - if (ep->ops->get_features) {
> - epc_features = ep->ops->get_features(ep);
> - if (epc_features->core_init_notifier)
> - return 0;
> - }
> -
> - /*
> - * NOTE:- Avoid accessing the hardware (Ex:- DBI space) before this
> - * step as platforms that implement 'core_init_notifier' feature may
> - * not have the hardware ready (i.e. core initialized) for access
> - * (Ex: tegra194). Any hardware access on such platforms result
> - * in system hang.
> - */
> - ret = dw_pcie_ep_init_registers(ep);
> - if (ret)
> - goto err_free_epc_mem;
> -
> return 0;
>
> -err_free_epc_mem:
> - pci_epc_mem_free_addr(epc, ep->msi_mem_phys, ep->msi_mem,
> - epc->mem->window.page_size);
> -
> err_exit_epc_mem:
> pci_epc_mem_exit(epc);
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-plat.c b/drivers/pci/controller/dwc/pcie-designware-plat.c
> index 778588b4be70..ca9b22e654cd 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-plat.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-plat.c
> @@ -145,6 +145,15 @@ static int dw_plat_pcie_probe(struct platform_device *pdev)
>
> pci->ep.ops = &pcie_ep_ops;
> ret = dw_pcie_ep_init(&pci->ep);
> + if (ret)
> + return ret;
> +
> + ret = dw_pcie_ep_init_registers(&pci->ep);
> + if (ret) {
> + dev_err(dev, "Failed to initialize DWC endpoint registers\n");
> + dw_pcie_ep_deinit(&pci->ep);
> + }
> +
> break;
> default:
> dev_err(dev, "INVALID device type %d\n", dw_plat_pcie->mode);
> diff --git a/drivers/pci/controller/dwc/pcie-keembay.c b/drivers/pci/controller/dwc/pcie-keembay.c
> index 5e8e54f597dd..b2556dbcffb5 100644
> --- a/drivers/pci/controller/dwc/pcie-keembay.c
> +++ b/drivers/pci/controller/dwc/pcie-keembay.c
> @@ -396,6 +396,7 @@ static int keembay_pcie_probe(struct platform_device *pdev)
> struct keembay_pcie *pcie;
> struct dw_pcie *pci;
> enum dw_pcie_device_mode mode;
> + int ret;
>
> data = device_get_match_data(dev);
> if (!data)
> @@ -430,11 +431,24 @@ static int keembay_pcie_probe(struct platform_device *pdev)
> return -ENODEV;
>
> pci->ep.ops = &keembay_pcie_ep_ops;
> - return dw_pcie_ep_init(&pci->ep);
> + ret = dw_pcie_ep_init(&pci->ep);
> + if (ret)
> + return ret;
> +
> + ret = dw_pcie_ep_init_registers(&pci->ep);
> + if (ret) {
> + dev_err(dev, "Failed to initialize DWC endpoint registers\n");
> + dw_pcie_ep_deinit(&pci->ep);
> + return ret;
> + }
> +
> + break;
> default:
> dev_err(dev, "Invalid device type %d\n", pcie->mode);
> return -ENODEV;
> }
> +
> + return 0;
> }
>
> static const struct keembay_pcie_of_data keembay_pcie_rc_of_data = {
> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> index de4bdfaecab3..e155a905fb4f 100644
> --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> @@ -416,6 +416,7 @@ static const struct dw_pcie_ep_ops pcie_ep_ops = {
> static int rcar_gen4_add_dw_pcie_ep(struct rcar_gen4_pcie *rcar)
> {
> struct dw_pcie_ep *ep = &rcar->dw.ep;
> + struct device *dev = rcar->dw.dev;
> int ret;
>
> if (!IS_ENABLED(CONFIG_PCIE_RCAR_GEN4_EP))
> @@ -424,8 +425,17 @@ static int rcar_gen4_add_dw_pcie_ep(struct rcar_gen4_pcie *rcar)
> ep->ops = &pcie_ep_ops;
>
> ret = dw_pcie_ep_init(ep);
> - if (ret)
> + if (ret) {
> rcar_gen4_pcie_ep_deinit(rcar);
> + return ret;
> + }
> +
> + ret = dw_pcie_ep_init_registers(ep);
> + if (ret) {
> + dev_err(dev, "Failed to initialize DWC endpoint registers\n");
> + dw_pcie_ep_deinit(ep);
> + rcar_gen4_pcie_ep_deinit(rcar);
> + }
>
> return ret;
> }
> diff --git a/drivers/pci/controller/dwc/pcie-uniphier-ep.c b/drivers/pci/controller/dwc/pcie-uniphier-ep.c
> index 639bc2e12476..0e5e7344de48 100644
> --- a/drivers/pci/controller/dwc/pcie-uniphier-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-uniphier-ep.c
> @@ -399,7 +399,18 @@ 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;
> +
> + ret = dw_pcie_ep_init_registers(&priv->pci.ep);
> + if (ret) {
> + dev_err(dev, "Failed to initialize DWC endpoint registers\n");
> + dw_pcie_ep_deinit(&priv->pci.ep);
> + return ret;
> + }
> +
> + return 0;
> }
>
> static const struct uniphier_pcie_ep_soc_data uniphier_pro5_data = {
>
> --
> 2.25.1
>