From: Manivannan Sadhasivam <mani@kernel.org>
To: Vidya Sagar <vidyas@nvidia.com>
Cc: jingoohan1@gmail.com, gustavo.pimentel@synopsys.com,
lpieralisi@kernel.org, robh@kernel.org, kw@linux.com,
bhelgaas@google.com, mani@kernel.org,
Sergey.Semin@baikalelectronics.ru, dmitry.baryshkov@linaro.org,
linmq006@gmail.com, ffclaire1224@gmail.com,
thierry.reding@gmail.com, jonathanh@nvidia.com,
linux-pci@vger.kernel.org, linux-arm-msm@vger.kernel.org,
linux-kernel@vger.kernel.org, kthota@nvidia.com,
mmaddireddy@nvidia.com, sagar.tv@gmail.com
Subject: Re: [PATCH V5 1/3] PCI: designware-ep: Fix DBI access before core init
Date: Wed, 26 Oct 2022 10:40:19 +0530 [thread overview]
Message-ID: <20221026051019.GA5179@thinkpad> (raw)
In-Reply-To: <20221013175712.7539-2-vidyas@nvidia.com>
On Thu, Oct 13, 2022 at 11:27:10PM +0530, Vidya Sagar wrote:
> Platforms that cannot support their core initialization without the
> reference clock from the host, implement the feature 'core_init_notifier'
> to indicate the DesignWare sub-system about when their core is getting
> initialized. Any accesses to the core (Ex:- DBI) would the core being
> ready result in system hang in such systems (Ex:- tegra194).
> This patch moves any access to the core to dw_pcie_ep_init_complete() API
dw_pcie_ep_init_complete() got renamed to dw_pcie_ep_init_late()
> which is effectively called only after the core initialization.
> It also introduces .ep_init_late() ops hook to be used for any post init
> work that platform drivers may have to do.
>
> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
> ---
> V5:
> * Changed dw_pcie_ep_init_complete() to dw_pcie_ep_init_late()
> * Skipped memory allocation if done already. This is to avoid freeing and then
> allocating again during PERST# toggles from the host.
>
> V4:
> * Addressed review comments from Bjorn and Manivannan
> * Moved dw_pcie_ep_init_complete() inside dw_pcie_ep_init_notify()
> * Added .ep_init_late() ops to perform late init tasks
>
> .../pci/controller/dwc/pcie-designware-ep.c | 125 +++++++++++-------
> drivers/pci/controller/dwc/pcie-designware.h | 10 +-
> 2 files changed, 80 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index 83ddb190292e..f300ea2f7bf7 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
[...]
> +int dw_pcie_ep_init_notify(struct dw_pcie_ep *ep)
> +{
> + struct pci_epc *epc = ep->epc;
> + int ret;
> +
> + ret = dw_pcie_ep_init_late(ep);
> + if (ret)
> + return ret;
> +
> + if (ep->ops->ep_init_late)
> + ep->ops->ep_init_late(ep);
I think you introduced this callback for the sake of Qcom driver I believe.
But it is not really required. I'll share more comments in patch 2/3.
Thanks,
Mani
> +
> + pci_epc_init_notify(epc);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(dw_pcie_ep_init_notify);
>
> int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> {
> int ret;
> - void *addr;
> - u8 func_no;
> struct resource *res;
> struct pci_epc *epc;
> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> @@ -690,7 +747,6 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> struct platform_device *pdev = to_platform_device(dev);
> struct device_node *np = dev->of_node;
> const struct pci_epc_features *epc_features;
> - struct dw_pcie_ep_func *ep_func;
>
> INIT_LIST_HEAD(&ep->func_list);
>
> @@ -719,26 +775,6 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> ep->phys_base = res->start;
> ep->addr_size = resource_size(res);
>
> - dw_pcie_version_detect(pci);
> -
> - dw_pcie_iatu_detect(pci);
> -
> - ep->ib_window_map = devm_bitmap_zalloc(dev, pci->num_ib_windows,
> - GFP_KERNEL);
> - if (!ep->ib_window_map)
> - return -ENOMEM;
> -
> - ep->ob_window_map = devm_bitmap_zalloc(dev, pci->num_ob_windows,
> - GFP_KERNEL);
> - if (!ep->ob_window_map)
> - return -ENOMEM;
> -
> - addr = devm_kcalloc(dev, pci->num_ob_windows, sizeof(phys_addr_t),
> - GFP_KERNEL);
> - if (!addr)
> - return -ENOMEM;
> - ep->outbound_addr = addr;
> -
> if (pci->link_gen < 1)
> pci->link_gen = of_pci_get_max_link_speed(np);
>
> @@ -755,20 +791,6 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> if (ret < 0)
> epc->max_functions = 1;
>
> - for (func_no = 0; func_no < epc->max_functions; func_no++) {
> - ep_func = devm_kzalloc(dev, sizeof(*ep_func), GFP_KERNEL);
> - if (!ep_func)
> - return -ENOMEM;
> -
> - ep_func->func_no = func_no;
> - ep_func->msi_cap = dw_pcie_ep_find_capability(ep, func_no,
> - PCI_CAP_ID_MSI);
> - ep_func->msix_cap = dw_pcie_ep_find_capability(ep, func_no,
> - PCI_CAP_ID_MSIX);
> -
> - list_add_tail(&ep_func->list, &ep->func_list);
> - }
> -
> if (ep->ops->ep_init)
> ep->ops->ep_init(ep);
>
> @@ -793,7 +815,14 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> return 0;
> }
>
> - ret = dw_pcie_ep_init_complete(ep);
> + /*
> + * 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 hard hang.
> + */
> + ret = dw_pcie_ep_init_late(ep);
> if (ret)
> goto err_free_epc_mem;
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 45fcdfc8c035..7252513956b7 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -253,6 +253,7 @@ struct dw_pcie_rp {
>
> struct dw_pcie_ep_ops {
> void (*ep_init)(struct dw_pcie_ep *ep);
> + void (*ep_init_late)(struct dw_pcie_ep *ep);
> int (*raise_irq)(struct dw_pcie_ep *ep, u8 func_no,
> enum pci_epc_irq_type type, u16 interrupt_num);
> const struct pci_epc_features* (*get_features)(struct dw_pcie_ep *ep);
> @@ -467,8 +468,7 @@ static inline void __iomem *dw_pcie_own_conf_map_bus(struct pci_bus *bus,
> #ifdef CONFIG_PCIE_DW_EP
> void dw_pcie_ep_linkup(struct dw_pcie_ep *ep);
> int dw_pcie_ep_init(struct dw_pcie_ep *ep);
> -int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep);
> -void dw_pcie_ep_init_notify(struct dw_pcie_ep *ep);
> +int dw_pcie_ep_init_notify(struct dw_pcie_ep *ep);
> void dw_pcie_ep_exit(struct dw_pcie_ep *ep);
> int dw_pcie_ep_raise_legacy_irq(struct dw_pcie_ep *ep, u8 func_no);
> int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
> @@ -490,15 +490,11 @@ static inline int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> return 0;
> }
>
> -static inline int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep)
> +static inline int dw_pcie_ep_init_notify(struct dw_pcie_ep *ep)
> {
> return 0;
> }
>
> -static inline void dw_pcie_ep_init_notify(struct dw_pcie_ep *ep)
> -{
> -}
> -
> static inline void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
> {
> }
> --
> 2.17.1
>
--
மணிவண்ணன் சதாசிவம்
next prev parent reply other threads:[~2022-10-26 5:10 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-13 17:57 [PATCH V5 0/3] PCI: designware-ep: Fix DBI access before core init Vidya Sagar
2022-10-13 17:57 ` [PATCH V5 1/3] " Vidya Sagar
2022-10-15 22:00 ` Han Jingoo
2022-10-26 5:10 ` Manivannan Sadhasivam [this message]
2022-10-13 17:57 ` [PATCH V5 2/3] PCI: qcom-ep: Refactor EP initialization completion Vidya Sagar
2022-10-26 5:30 ` Manivannan Sadhasivam
2022-10-13 17:57 ` [PATCH V5 3/3] PCI: tegra194: " Vidya Sagar
2022-10-14 6:10 ` [PATCH V5 0/3] PCI: designware-ep: Fix DBI access before core init Manivannan Sadhasivam
2023-02-14 13:03 ` Manivannan Sadhasivam
2023-02-14 13:57 ` Vidya Sagar
2023-03-07 15:18 ` Manivannan Sadhasivam
2023-05-15 7:40 ` Manivannan Sadhasivam
2023-08-25 12:38 ` Manivannan Sadhasivam
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20221026051019.GA5179@thinkpad \
--to=mani@kernel.org \
--cc=Sergey.Semin@baikalelectronics.ru \
--cc=bhelgaas@google.com \
--cc=dmitry.baryshkov@linaro.org \
--cc=ffclaire1224@gmail.com \
--cc=gustavo.pimentel@synopsys.com \
--cc=jingoohan1@gmail.com \
--cc=jonathanh@nvidia.com \
--cc=kthota@nvidia.com \
--cc=kw@linux.com \
--cc=linmq006@gmail.com \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=lpieralisi@kernel.org \
--cc=mmaddireddy@nvidia.com \
--cc=robh@kernel.org \
--cc=sagar.tv@gmail.com \
--cc=thierry.reding@gmail.com \
--cc=vidyas@nvidia.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).