| Message ID | 20230111114059.6553-3-manivannan.sadhasivam@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Lorenzo Pieralisi |
| Headers | show |
| Series | PCI: endpoint: Rework the EPC to EPF notification | expand |
On Wed, Jan 11, 2023 at 05:10:56PM +0530, Manivannan Sadhasivam wrote: > dw_pcie_ep_linkup() may take more time to execute depending on the EPF > driver implementation. Calling this API in the hard IRQ handler is not > encouraged since the hard IRQ handlers are supposed to complete quickly. > > So move the dw_pcie_ep_linkup() call to threaded IRQ handler. > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> Hi Vidya, Could you please review this patch? This patch is blocking the merge since last release. And there are other series pending to be submitted/merged due to this (including one from you). Thanks, Mani > --- > drivers/pci/controller/dwc/pcie-tegra194.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c > index 02d78a12b6e7..09825b4a075e 100644 > --- a/drivers/pci/controller/dwc/pcie-tegra194.c > +++ b/drivers/pci/controller/dwc/pcie-tegra194.c > @@ -286,6 +286,7 @@ struct tegra_pcie_dw { > struct gpio_desc *pex_refclk_sel_gpiod; > unsigned int pex_rst_irq; > int ep_state; > + long link_status; > }; > > static inline struct tegra_pcie_dw *to_tegra_pcie(struct dw_pcie *pci) > @@ -449,9 +450,13 @@ static void pex_ep_event_hot_rst_done(struct tegra_pcie_dw *pcie) > static irqreturn_t tegra_pcie_ep_irq_thread(int irq, void *arg) > { > struct tegra_pcie_dw *pcie = arg; > + struct dw_pcie_ep *ep = &pcie->pci.ep; > struct dw_pcie *pci = &pcie->pci; > u32 val, speed; > > + if (test_and_clear_bit(0, &pcie->link_status)) > + dw_pcie_ep_linkup(ep); > + > speed = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_LNKSTA) & > PCI_EXP_LNKSTA_CLS; > clk_set_rate(pcie->core_clk, pcie_gen_freq[speed - 1]); > @@ -498,7 +503,6 @@ static irqreturn_t tegra_pcie_ep_irq_thread(int irq, void *arg) > static irqreturn_t tegra_pcie_ep_hard_irq(int irq, void *arg) > { > struct tegra_pcie_dw *pcie = arg; > - struct dw_pcie_ep *ep = &pcie->pci.ep; > int spurious = 1; > u32 status_l0, status_l1, link_status; > > @@ -514,7 +518,8 @@ static irqreturn_t tegra_pcie_ep_hard_irq(int irq, void *arg) > link_status = appl_readl(pcie, APPL_LINK_STATUS); > if (link_status & APPL_LINK_STATUS_RDLH_LINK_UP) { > dev_dbg(pcie->dev, "Link is up with Host\n"); > - dw_pcie_ep_linkup(ep); > + set_bit(0, &pcie->link_status); > + return IRQ_WAKE_THREAD; > } > } > > -- > 2.25.1 >
Thanks for pushing this change. Reviewed-by: Vidya Sagar <vidyas@nvidia.com> On 1/11/2023 5:10 PM, Manivannan Sadhasivam wrote: > External email: Use caution opening links or attachments > > > dw_pcie_ep_linkup() may take more time to execute depending on the EPF > driver implementation. Calling this API in the hard IRQ handler is not > encouraged since the hard IRQ handlers are supposed to complete quickly. > > So move the dw_pcie_ep_linkup() call to threaded IRQ handler. > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > --- > drivers/pci/controller/dwc/pcie-tegra194.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c > index 02d78a12b6e7..09825b4a075e 100644 > --- a/drivers/pci/controller/dwc/pcie-tegra194.c > +++ b/drivers/pci/controller/dwc/pcie-tegra194.c > @@ -286,6 +286,7 @@ struct tegra_pcie_dw { > struct gpio_desc *pex_refclk_sel_gpiod; > unsigned int pex_rst_irq; > int ep_state; > + long link_status; > }; > > static inline struct tegra_pcie_dw *to_tegra_pcie(struct dw_pcie *pci) > @@ -449,9 +450,13 @@ static void pex_ep_event_hot_rst_done(struct tegra_pcie_dw *pcie) > static irqreturn_t tegra_pcie_ep_irq_thread(int irq, void *arg) > { > struct tegra_pcie_dw *pcie = arg; > + struct dw_pcie_ep *ep = &pcie->pci.ep; > struct dw_pcie *pci = &pcie->pci; > u32 val, speed; > > + if (test_and_clear_bit(0, &pcie->link_status)) > + dw_pcie_ep_linkup(ep); > + > speed = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_LNKSTA) & > PCI_EXP_LNKSTA_CLS; > clk_set_rate(pcie->core_clk, pcie_gen_freq[speed - 1]); > @@ -498,7 +503,6 @@ static irqreturn_t tegra_pcie_ep_irq_thread(int irq, void *arg) > static irqreturn_t tegra_pcie_ep_hard_irq(int irq, void *arg) > { > struct tegra_pcie_dw *pcie = arg; > - struct dw_pcie_ep *ep = &pcie->pci.ep; > int spurious = 1; > u32 status_l0, status_l1, link_status; > > @@ -514,7 +518,8 @@ static irqreturn_t tegra_pcie_ep_hard_irq(int irq, void *arg) > link_status = appl_readl(pcie, APPL_LINK_STATUS); > if (link_status & APPL_LINK_STATUS_RDLH_LINK_UP) { > dev_dbg(pcie->dev, "Link is up with Host\n"); > - dw_pcie_ep_linkup(ep); > + set_bit(0, &pcie->link_status); > + return IRQ_WAKE_THREAD; > } > } > > -- > 2.25.1 >
On Mon, Jan 23, 2023 at 12:52:18PM +0530, Vidya Sagar wrote: > Thanks for pushing this change. > > Reviewed-by: Vidya Sagar <vidyas@nvidia.com> > Thanks Vidya! I will push a next version with a fix for an issue identified by kbot. Hopefully that will get merged for 6.3. Thanks, Mani > On 1/11/2023 5:10 PM, Manivannan Sadhasivam wrote: > > External email: Use caution opening links or attachments > > > > > > dw_pcie_ep_linkup() may take more time to execute depending on the EPF > > driver implementation. Calling this API in the hard IRQ handler is not > > encouraged since the hard IRQ handlers are supposed to complete quickly. > > > > So move the dw_pcie_ep_linkup() call to threaded IRQ handler. > > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > --- > > drivers/pci/controller/dwc/pcie-tegra194.c | 9 +++++++-- > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c > > index 02d78a12b6e7..09825b4a075e 100644 > > --- a/drivers/pci/controller/dwc/pcie-tegra194.c > > +++ b/drivers/pci/controller/dwc/pcie-tegra194.c > > @@ -286,6 +286,7 @@ struct tegra_pcie_dw { > > struct gpio_desc *pex_refclk_sel_gpiod; > > unsigned int pex_rst_irq; > > int ep_state; > > + long link_status; > > }; > > > > static inline struct tegra_pcie_dw *to_tegra_pcie(struct dw_pcie *pci) > > @@ -449,9 +450,13 @@ static void pex_ep_event_hot_rst_done(struct tegra_pcie_dw *pcie) > > static irqreturn_t tegra_pcie_ep_irq_thread(int irq, void *arg) > > { > > struct tegra_pcie_dw *pcie = arg; > > + struct dw_pcie_ep *ep = &pcie->pci.ep; > > struct dw_pcie *pci = &pcie->pci; > > u32 val, speed; > > > > + if (test_and_clear_bit(0, &pcie->link_status)) > > + dw_pcie_ep_linkup(ep); > > + > > speed = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_LNKSTA) & > > PCI_EXP_LNKSTA_CLS; > > clk_set_rate(pcie->core_clk, pcie_gen_freq[speed - 1]); > > @@ -498,7 +503,6 @@ static irqreturn_t tegra_pcie_ep_irq_thread(int irq, void *arg) > > static irqreturn_t tegra_pcie_ep_hard_irq(int irq, void *arg) > > { > > struct tegra_pcie_dw *pcie = arg; > > - struct dw_pcie_ep *ep = &pcie->pci.ep; > > int spurious = 1; > > u32 status_l0, status_l1, link_status; > > > > @@ -514,7 +518,8 @@ static irqreturn_t tegra_pcie_ep_hard_irq(int irq, void *arg) > > link_status = appl_readl(pcie, APPL_LINK_STATUS); > > if (link_status & APPL_LINK_STATUS_RDLH_LINK_UP) { > > dev_dbg(pcie->dev, "Link is up with Host\n"); > > - dw_pcie_ep_linkup(ep); > > + set_bit(0, &pcie->link_status); > > + return IRQ_WAKE_THREAD; > > } > > } > > > > -- > > 2.25.1 > >
diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c index 02d78a12b6e7..09825b4a075e 100644 --- a/drivers/pci/controller/dwc/pcie-tegra194.c +++ b/drivers/pci/controller/dwc/pcie-tegra194.c @@ -286,6 +286,7 @@ struct tegra_pcie_dw { struct gpio_desc *pex_refclk_sel_gpiod; unsigned int pex_rst_irq; int ep_state; + long link_status; }; static inline struct tegra_pcie_dw *to_tegra_pcie(struct dw_pcie *pci) @@ -449,9 +450,13 @@ static void pex_ep_event_hot_rst_done(struct tegra_pcie_dw *pcie) static irqreturn_t tegra_pcie_ep_irq_thread(int irq, void *arg) { struct tegra_pcie_dw *pcie = arg; + struct dw_pcie_ep *ep = &pcie->pci.ep; struct dw_pcie *pci = &pcie->pci; u32 val, speed; + if (test_and_clear_bit(0, &pcie->link_status)) + dw_pcie_ep_linkup(ep); + speed = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_LNKSTA) & PCI_EXP_LNKSTA_CLS; clk_set_rate(pcie->core_clk, pcie_gen_freq[speed - 1]); @@ -498,7 +503,6 @@ static irqreturn_t tegra_pcie_ep_irq_thread(int irq, void *arg) static irqreturn_t tegra_pcie_ep_hard_irq(int irq, void *arg) { struct tegra_pcie_dw *pcie = arg; - struct dw_pcie_ep *ep = &pcie->pci.ep; int spurious = 1; u32 status_l0, status_l1, link_status; @@ -514,7 +518,8 @@ static irqreturn_t tegra_pcie_ep_hard_irq(int irq, void *arg) link_status = appl_readl(pcie, APPL_LINK_STATUS); if (link_status & APPL_LINK_STATUS_RDLH_LINK_UP) { dev_dbg(pcie->dev, "Link is up with Host\n"); - dw_pcie_ep_linkup(ep); + set_bit(0, &pcie->link_status); + return IRQ_WAKE_THREAD; } }
dw_pcie_ep_linkup() may take more time to execute depending on the EPF driver implementation. Calling this API in the hard IRQ handler is not encouraged since the hard IRQ handlers are supposed to complete quickly. So move the dw_pcie_ep_linkup() call to threaded IRQ handler. Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> --- drivers/pci/controller/dwc/pcie-tegra194.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)