diff mbox series

[v4,2/5] PCI: tegra194: Move dw_pcie_ep_linkup() to threaded IRQ handler

Message ID 20221025145101.116393-3-manivannan.sadhasivam@linaro.org
State New
Headers show
Series PCI: endpoint: Rework the EPC to EPF notification | expand

Commit Message

Manivannan Sadhasivam Oct. 25, 2022, 2:50 p.m. UTC 
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(-)

Comments

Manivannan Sadhasivam Nov. 14, 2022, 11:06 a.m. UTC | #1 
On Tue, Oct 25, 2022 at 08:20:58PM +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>
> ---
>  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 1b6b437823d2..a0d231b7a435 100644
> --- a/drivers/pci/controller/dwc/pcie-tegra194.c
> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
> @@ -287,6 +287,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)
> @@ -450,9 +451,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]);
> @@ -499,7 +504,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;
>  
> @@ -515,7 +519,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
>
Manivannan Sadhasivam Nov. 14, 2022, 11:08 a.m. UTC | #2 
On Mon, Nov 14, 2022 at 04:37:00PM +0530, Manivannan Sadhasivam wrote:
> On Tue, Oct 25, 2022 at 08:20:58PM +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>

Sorry for resending it (something messed up with my email client).

Vidya, can you please review this patch?

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 1b6b437823d2..a0d231b7a435 100644
> > --- a/drivers/pci/controller/dwc/pcie-tegra194.c
> > +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
> > @@ -287,6 +287,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)
> > @@ -450,9 +451,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]);
> > @@ -499,7 +504,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;
> >  
> > @@ -515,7 +519,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
> > 
> 
> -- 
> மணிவண்ணன் சதாசிவம்
Manivannan Sadhasivam Nov. 22, 2022, 1:49 p.m. UTC | #3 
On Mon, Nov 14, 2022 at 04:38:20PM +0530, Manivannan Sadhasivam wrote:
> On Mon, Nov 14, 2022 at 04:37:00PM +0530, Manivannan Sadhasivam wrote:
> > On Tue, Oct 25, 2022 at 08:20:58PM +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>
> 
> Sorry for resending it (something messed up with my email client).
> 
> Vidya, can you please review this patch?
> 

Ping!

Thanks,
Mani

> 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 1b6b437823d2..a0d231b7a435 100644
> > > --- a/drivers/pci/controller/dwc/pcie-tegra194.c
> > > +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
> > > @@ -287,6 +287,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)
> > > @@ -450,9 +451,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]);
> > > @@ -499,7 +504,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;
> > >  
> > > @@ -515,7 +519,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
> > > 
> > 
> > -- 
> > மணிவண்ணன் சதாசிவம்
> 
> -- 
> மணிவண்ணன் சதாசிவம்
Manivannan Sadhasivam Dec. 5, 2022, 6:49 a.m. UTC | #4 
On Tue, Nov 22, 2022 at 07:19:41PM +0530, Manivannan Sadhasivam wrote:
> On Mon, Nov 14, 2022 at 04:38:20PM +0530, Manivannan Sadhasivam wrote:
> > On Mon, Nov 14, 2022 at 04:37:00PM +0530, Manivannan Sadhasivam wrote:
> > > On Tue, Oct 25, 2022 at 08:20:58PM +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>
> > 
> > Sorry for resending it (something messed up with my email client).
> > 
> > Vidya, can you please review this patch?
> > 
> 
> Ping!
> 

Ping again. Vidya, only your t-b tag is missing to get this series merged. Can
you please test and report back.

There are few series that depends on this including a couple from you.

Thanks,
Mani

> Thanks,
> Mani
> 
> > 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 1b6b437823d2..a0d231b7a435 100644
> > > > --- a/drivers/pci/controller/dwc/pcie-tegra194.c
> > > > +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
> > > > @@ -287,6 +287,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)
> > > > @@ -450,9 +451,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]);
> > > > @@ -499,7 +504,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;
> > > >  
> > > > @@ -515,7 +519,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 mbox series

Patch

diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
index 1b6b437823d2..a0d231b7a435 100644
--- a/drivers/pci/controller/dwc/pcie-tegra194.c
+++ b/drivers/pci/controller/dwc/pcie-tegra194.c
@@ -287,6 +287,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)
@@ -450,9 +451,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]);
@@ -499,7 +504,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;
 
@@ -515,7 +519,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;
 			}
 		}