linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] PCI: imx6:don't sleep in atomic context
       [not found] <1447066080-5859-1-git-send-email-sanjeev_sharma@mentor.com>
@ 2015-11-10  8:41 ` Lucas Stach
  2015-11-10  9:28   ` Arnd Bergmann
       [not found] ` <1449042196-25710-1-git-send-email-sanjeev_sharma@mentor.com>
  2016-01-06 22:04 ` [PATCH] " Bjorn Helgaas
  2 siblings, 1 reply; 11+ messages in thread
From: Lucas Stach @ 2015-11-10  8:41 UTC (permalink / raw)
  To: linux-arm-kernel

Am Montag, den 09.11.2015, 16:18 +0530 schrieb Sanjeev Sharma:
> If additional PCIe switch get connected between the
> host and the NIC,the kernel crashes with "BUG:
> scheduling while atomic". To handle this we need to
> call mdelay() instead of usleep_range().
> 
> For more detail please refer bugzilla.kernel.org, Bug
> 100031
> 
> Signed-off-by: Sanjeev Sharma <sanjeev_sharma@mentor.com>

This is wrong. You are not the author of this patch and this should be
reflected both in the From: line as well as in the order of signoffs.

> Signed-off-by: David Mueller <dave.mueller@gmx.ch>
> ---
>  drivers/pci/host/pci-imx6.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
> index 233a196..9769b13 100644
> --- a/drivers/pci/host/pci-imx6.c
> +++ b/drivers/pci/host/pci-imx6.c
> @@ -499,7 +499,7 @@ static int imx6_pcie_link_up(struct pcie_port *pp)
>  		 * Wait a little bit, then re-check if the link finished
>  		 * the training.
>  		 */
> -		usleep_range(1000, 2000);
> +		mdelay(1000);

A mdelay(1000) is a whole different timescale than a usleep(1000). If
this patch works for you with mdelay(1) or maybe mdelay(2) I would be
fine with it.

Regards,
Lucas

>  	}
>  	/*
>  	 * From L0, initiate MAC entry to gen2 if EP/RC supports gen2.

-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] PCI: imx6:don't sleep in atomic context
  2015-11-10  8:41 ` [PATCH] PCI: imx6:don't sleep in atomic context Lucas Stach
@ 2015-11-10  9:28   ` Arnd Bergmann
  2015-11-10  9:35     ` Lucas Stach
  0 siblings, 1 reply; 11+ messages in thread
From: Arnd Bergmann @ 2015-11-10  9:28 UTC (permalink / raw)
  To: linux-arm-kernel

On Tuesday 10 November 2015 09:41:18 Lucas Stach wrote:
> > diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
> > index 233a196..9769b13 100644
> > --- a/drivers/pci/host/pci-imx6.c
> > +++ b/drivers/pci/host/pci-imx6.c
> > @@ -499,7 +499,7 @@ static int imx6_pcie_link_up(struct pcie_port *pp)
> >                * Wait a little bit, then re-check if the link finished
> >                * the training.
> >                */
> > -             usleep_range(1000, 2000);
> > +             mdelay(1000);
> 
> A mdelay(1000) is a whole different timescale than a usleep(1000). If
> this patch works for you with mdelay(1) or maybe mdelay(2) I would be
> fine with it.

mdelay(1) is still a really long time to block the CPU for, on potentially
every config space access.

Everybody else just returns the link status here, which seems to be
the better alternative. If you need to delay the startup, better have
a msleep(1) loop in the initial probe function where you are allowed to
sleep.

	Arnd

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] PCI: imx6:don't sleep in atomic context
  2015-11-10  9:28   ` Arnd Bergmann
@ 2015-11-10  9:35     ` Lucas Stach
  2015-11-10  9:45       ` Arnd Bergmann
  0 siblings, 1 reply; 11+ messages in thread
From: Lucas Stach @ 2015-11-10  9:35 UTC (permalink / raw)
  To: linux-arm-kernel

Am Dienstag, den 10.11.2015, 10:28 +0100 schrieb Arnd Bergmann:
> On Tuesday 10 November 2015 09:41:18 Lucas Stach wrote:
> > > diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
> > > index 233a196..9769b13 100644
> > > --- a/drivers/pci/host/pci-imx6.c
> > > +++ b/drivers/pci/host/pci-imx6.c
> > > @@ -499,7 +499,7 @@ static int imx6_pcie_link_up(struct pcie_port *pp)
> > >                * Wait a little bit, then re-check if the link finished
> > >                * the training.
> > >                */
> > > -             usleep_range(1000, 2000);
> > > +             mdelay(1000);
> > 
> > A mdelay(1000) is a whole different timescale than a usleep(1000). If
> > this patch works for you with mdelay(1) or maybe mdelay(2) I would be
> > fine with it.
> 
> mdelay(1) is still a really long time to block the CPU for, on potentially
> every config space access.
> 
> Everybody else just returns the link status here, which seems to be
> the better alternative. If you need to delay the startup, better have
> a msleep(1) loop in the initial probe function where you are allowed to
> sleep.
> 
Yes, it's somewhere on my TODO list to rework the link-up handling here,
but as there are quite a few timing and ordering implications in that
code, this needs a good thought and a good deal of testing. So I'm
inclined to ACK the current patch to get rid of the obvious bug and sort
things out properly in a follow on patchset.

Regards,
Lucas

-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] PCI: imx6:don't sleep in atomic context
  2015-11-10  9:35     ` Lucas Stach
@ 2015-11-10  9:45       ` Arnd Bergmann
  2015-11-16  9:36         ` Sharma, Sanjeev
  0 siblings, 1 reply; 11+ messages in thread
From: Arnd Bergmann @ 2015-11-10  9:45 UTC (permalink / raw)
  To: linux-arm-kernel

On Tuesday 10 November 2015 10:35:10 Lucas Stach wrote:
> Am Dienstag, den 10.11.2015, 10:28 +0100 schrieb Arnd Bergmann:
> > On Tuesday 10 November 2015 09:41:18 Lucas Stach wrote:
> > > > diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
> > > > index 233a196..9769b13 100644
> > > > --- a/drivers/pci/host/pci-imx6.c
> > > > +++ b/drivers/pci/host/pci-imx6.c
> > > > @@ -499,7 +499,7 @@ static int imx6_pcie_link_up(struct pcie_port *pp)
> > > >                * Wait a little bit, then re-check if the link finished
> > > >                * the training.
> > > >                */
> > > > -             usleep_range(1000, 2000);
> > > > +             mdelay(1000);
> > > 
> > > A mdelay(1000) is a whole different timescale than a usleep(1000). If
> > > this patch works for you with mdelay(1) or maybe mdelay(2) I would be
> > > fine with it.
> > 
> > mdelay(1) is still a really long time to block the CPU for, on potentially
> > every config space access.
> > 
> > Everybody else just returns the link status here, which seems to be
> > the better alternative. If you need to delay the startup, better have
> > a msleep(1) loop in the initial probe function where you are allowed to
> > sleep.
> > 
> Yes, it's somewhere on my TODO list to rework the link-up handling here,
> but as there are quite a few timing and ordering implications in that
> code, this needs a good thought and a good deal of testing. So I'm
> inclined to ACK the current patch to get rid of the obvious bug and sort
> things out properly in a follow on patchset.

Maybe use that patch with some modifications then:

* add a comment to explain that this is currently called from possibly
  atomic context through pci_config_{read,write} and that the link
  state handling never belonged here.

* instead of looping five times for up to 2ms each, loop 100 times
  around a udelay(20) to hopefully be done earlier. I was going to
  suggest using time_before(timeout, jiffies) as the condition to
  wait for, but that doesn't work if called with interrupts disabled.

	Arnd

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] PCI: imx6:don't sleep in atomic context
  2015-11-10  9:45       ` Arnd Bergmann
@ 2015-11-16  9:36         ` Sharma, Sanjeev
  2015-11-24 13:57           ` Lucas Stach
  0 siblings, 1 reply; 11+ messages in thread
From: Sharma, Sanjeev @ 2015-11-16  9:36 UTC (permalink / raw)
  To: linux-arm-kernel

On Tuesday 10 November 2015 10:35:10 Lucas Stach wrote:
> Am Dienstag, den 10.11.2015, 10:28 +0100 schrieb Arnd Bergmann:
> > On Tuesday 10 November 2015 09:41:18 Lucas Stach wrote:
> > > > diff --git a/drivers/pci/host/pci-imx6.c 
> > > > b/drivers/pci/host/pci-imx6.c index 233a196..9769b13 100644
> > > > --- a/drivers/pci/host/pci-imx6.c
> > > > +++ b/drivers/pci/host/pci-imx6.c
> > > > @@ -499,7 +499,7 @@ static int imx6_pcie_link_up(struct pcie_port *pp)
> > > >                * Wait a little bit, then re-check if the link finished
> > > >                * the training.
> > > >                */
> > > > -             usleep_range(1000, 2000);
> > > > +             mdelay(1000);
> > > 
> > > A mdelay(1000) is a whole different timescale than a usleep(1000). 
> > > If this patch works for you with mdelay(1) or maybe mdelay(2) I 
> > > would be fine with it.
> > 
> > mdelay(1) is still a really long time to block the CPU for, on 
> > potentially every config space access.
> > 
> > Everybody else just returns the link status here, which seems to be 
> > the better alternative. If you need to delay the startup, better 
> > have a msleep(1) loop in the initial probe function where you are 
> > allowed to sleep.
> > 
> Yes, it's somewhere on my TODO list to rework the link-up handling 
> here, but as there are quite a few timing and ordering implications in 
> that code, this needs a good thought and a good deal of testing. So 
> I'm inclined to ACK the current patch to get rid of the obvious bug 
> and sort things out properly in a follow on patchset.

Maybe use that patch with some modifications then:

* add a comment to explain that this is currently called from possibly
  atomic context through pci_config_{read,write} and that the link
  state handling never belonged here.

* instead of looping five times for up to 2ms each, loop 100 times
  around a udelay(20) to hopefully be done earlier. I was going to
  suggest using time_before(timeout, jiffies) as the condition to
  wait for, but that doesn't work if called with interrupts disabled.

	Arnd
Shall I go ahead by changing only current patch to mdelay(1). I will also
Incorporate comment  #1 given by Arnd above. 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] PCI: imx6:don't sleep in atomic context
  2015-11-16  9:36         ` Sharma, Sanjeev
@ 2015-11-24 13:57           ` Lucas Stach
  0 siblings, 0 replies; 11+ messages in thread
From: Lucas Stach @ 2015-11-24 13:57 UTC (permalink / raw)
  To: linux-arm-kernel

Am Montag, den 16.11.2015, 09:36 +0000 schrieb Sharma, Sanjeev:
> On Tuesday 10 November 2015 10:35:10 Lucas Stach wrote:
> > Am Dienstag, den 10.11.2015, 10:28 +0100 schrieb Arnd Bergmann:
> > > On Tuesday 10 November 2015 09:41:18 Lucas Stach wrote:
> > > > > diff --git a/drivers/pci/host/pci-imx6.c 
> > > > > b/drivers/pci/host/pci-imx6.c index 233a196..9769b13 100644
> > > > > --- a/drivers/pci/host/pci-imx6.c
> > > > > +++ b/drivers/pci/host/pci-imx6.c
> > > > > @@ -499,7 +499,7 @@ static int imx6_pcie_link_up(struct pcie_port *pp)
> > > > >                * Wait a little bit, then re-check if the link finished
> > > > >                * the training.
> > > > >                */
> > > > > -             usleep_range(1000, 2000);
> > > > > +             mdelay(1000);
> > > > 
> > > > A mdelay(1000) is a whole different timescale than a usleep(1000). 
> > > > If this patch works for you with mdelay(1) or maybe mdelay(2) I 
> > > > would be fine with it.
> > > 
> > > mdelay(1) is still a really long time to block the CPU for, on 
> > > potentially every config space access.
> > > 
> > > Everybody else just returns the link status here, which seems to be 
> > > the better alternative. If you need to delay the startup, better 
> > > have a msleep(1) loop in the initial probe function where you are 
> > > allowed to sleep.
> > > 
> > Yes, it's somewhere on my TODO list to rework the link-up handling 
> > here, but as there are quite a few timing and ordering implications in 
> > that code, this needs a good thought and a good deal of testing. So 
> > I'm inclined to ACK the current patch to get rid of the obvious bug 
> > and sort things out properly in a follow on patchset.
> 
> Maybe use that patch with some modifications then:
> 
> * add a comment to explain that this is currently called from possibly
>   atomic context through pci_config_{read,write} and that the link
>   state handling never belonged here.
> 
> * instead of looping five times for up to 2ms each, loop 100 times
>   around a udelay(20) to hopefully be done earlier. I was going to
>   suggest using time_before(timeout, jiffies) as the condition to
>   wait for, but that doesn't work if called with interrupts disabled.
> 
> 	Arnd
> Shall I go ahead by changing only current patch to mdelay(1). I will also
> Incorporate comment  #1 given by Arnd above. 

Yes, please go ahead. Smaller delay loops make sense, but please ensure
that the total timeouts stay the same.

Regards,
Lucas
-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2] PCI: imx6:don't sleep in atomic context
       [not found] ` <1449042196-25710-1-git-send-email-sanjeev_sharma@mentor.com>
@ 2016-01-06  2:13   ` Bjorn Helgaas
  0 siblings, 0 replies; 11+ messages in thread
From: Bjorn Helgaas @ 2016-01-06  2:13 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Dec 02, 2015 at 01:13:16PM +0530, Sanjeev Sharma wrote:
> From: David Mueller <dave.mueller@gmx.ch>
> 
> If additional PCIe switch get connected between the
> host and the NIC,the kernel crashes with "BUG:
> scheduling while atomic". To handle this we need to
> call mdelay() instead of usleep_range().
> 
> This is currently called from atomic context through
> pci_config_{read,write).
> 
> For more detail please refer bugzilla.kernel.org, Bug
> 100031

Bug reports have URLs; please use them.  I'll save you the trouble:

  Link: https://bugzilla.kernel.org/show_bug.cgi?id=100031

In that bug report, I said: "My first question on the list will be:
Besides imx6, there are five other DesignWare-based drivers, and
imx6_pcie_link_up() looks nothing like the other
pcie_host_ops.link_up() methods.  We need to explain why imx6 is
special, and whether all the .link_up() methods can be made similar."

The only response was that i.MX6 is special because of a hardware bug,
so we need to start at Gen1, wait, try to change to Gen2, etc.  That
all sounds like work that should be done in
imx6_pcie_establish_link().

This patch doesn't seem to make anything *worse*, so I might take it
anyway, but I'm a little disappointed that we didn't make any progress
toward making imx6 more like the other drivers.  I know this sounds
pedantic, but it's a lot easier to let things slowly diverge than it
is to keep them uniform, and bugs hide in differences like this.

> Signed-off-by: David Mueller <dave.mueller@gmx.ch>
> Signed-off-by: Sanjeev Sharma <sanjeev_sharma@mentor.com>
> 
> Changes in v2:
> 	-order of signoff has been change
> 	-Author of patch has been change
> 	-A mdelay(1000) is different timescale than
> a usleep(1000).change it to correct scale i.e mdelay(1)
> 	-updated comment
> ---
>  drivers/pci/host/pci-imx6.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
> index 233a196..c03527f 100644
> --- a/drivers/pci/host/pci-imx6.c
> +++ b/drivers/pci/host/pci-imx6.c
> @@ -499,7 +499,7 @@ static int imx6_pcie_link_up(struct pcie_port *pp)
>  		 * Wait a little bit, then re-check if the link finished
>  		 * the training.
>  		 */
> -		usleep_range(1000, 2000);
> +		mdelay(1);
>  	}
>  	/*
>  	 * From L0, initiate MAC entry to gen2 if EP/RC supports gen2.
> -- 
> 1.7.11.7
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] PCI: imx6:don't sleep in atomic context
       [not found] <1447066080-5859-1-git-send-email-sanjeev_sharma@mentor.com>
  2015-11-10  8:41 ` [PATCH] PCI: imx6:don't sleep in atomic context Lucas Stach
       [not found] ` <1449042196-25710-1-git-send-email-sanjeev_sharma@mentor.com>
@ 2016-01-06 22:04 ` Bjorn Helgaas
  2016-02-18  7:17   ` Sharma, Sanjeev
  2 siblings, 1 reply; 11+ messages in thread
From: Bjorn Helgaas @ 2016-01-06 22:04 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Sanjeev,

On Mon, Nov 09, 2015 at 04:18:00PM +0530, Sanjeev Sharma wrote:
> If additional PCIe switch get connected between the
> host and the NIC,the kernel crashes with "BUG:
> scheduling while atomic". To handle this we need to
> call mdelay() instead of usleep_range().
> 
> For more detail please refer bugzilla.kernel.org, Bug
> 100031
> 
> Signed-off-by: Sanjeev Sharma <sanjeev_sharma@mentor.com>
> Signed-off-by: David Mueller <dave.mueller@gmx.ch>

I'm dropping this for now because we've been kicking around the same
solution (with tweaks to the mdelay amount) since June, but no
progress on the *real* issue, which is that imx6_pcie_link_up() should
never wait; it should simply return the link status.

I'm pretty sure the amount of time I've spent looking into this would
have been enough to make some progress on that underlying issue.

Bjorn

> ---
>  drivers/pci/host/pci-imx6.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
> index 233a196..9769b13 100644
> --- a/drivers/pci/host/pci-imx6.c
> +++ b/drivers/pci/host/pci-imx6.c
> @@ -499,7 +499,7 @@ static int imx6_pcie_link_up(struct pcie_port *pp)
>  		 * Wait a little bit, then re-check if the link finished
>  		 * the training.
>  		 */
> -		usleep_range(1000, 2000);
> +		mdelay(1000);
>  	}
>  	/*
>  	 * From L0, initiate MAC entry to gen2 if EP/RC supports gen2.
> -- 
> 1.7.11.7
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] PCI: imx6:don't sleep in atomic context
  2016-01-06 22:04 ` [PATCH] " Bjorn Helgaas
@ 2016-02-18  7:17   ` Sharma, Sanjeev
  2016-02-18 15:08     ` Bjorn Helgaas
  0 siblings, 1 reply; 11+ messages in thread
From: Sharma, Sanjeev @ 2016-02-18  7:17 UTC (permalink / raw)
  To: linux-arm-kernel

-----Original Message-----
From: Bjorn Helgaas [mailto:helgaas at kernel.org] 
Sent: Thursday, January 07, 2016 3:35 AM
To: Sharma, Sanjeev
Cc: Richard.Zhu at freescale.com; l.stach at pengutronix.de; bhelgaas at google.com; linux-pci at vger.kernel.org; linux-arm-kernel at lists.infradead.org; linux-kernel at vger.kernel.org; David Mueller
Subject: Re: [PATCH] PCI: imx6:don't sleep in atomic context

Hi Sanjeev,

On Mon, Nov 09, 2015 at 04:18:00PM +0530, Sanjeev Sharma wrote:
> If additional PCIe switch get connected between the host and the 
> NIC,the kernel crashes with "BUG:
> scheduling while atomic". To handle this we need to call mdelay() 
> instead of usleep_range().
> 
> For more detail please refer bugzilla.kernel.org, Bug
> 100031
> 
> Signed-off-by: Sanjeev Sharma <sanjeev_sharma@mentor.com>
> Signed-off-by: David Mueller <dave.mueller@gmx.ch>

I'm dropping this for now because we've been kicking around the same solution (with tweaks to the mdelay amount) since June, but no progress on the *real* issue, which is that imx6_pcie_link_up() should never wait; it should simply return the link status.

I'm pretty sure the amount of time I've spent looking into this would have been enough to make some progress on that underlying issue.

Bjorn

Ok ! please share the change you are planning to implement. 

Sanjeev Sharma 
> ---
>  drivers/pci/host/pci-imx6.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c 
> index 233a196..9769b13 100644
> --- a/drivers/pci/host/pci-imx6.c
> +++ b/drivers/pci/host/pci-imx6.c
> @@ -499,7 +499,7 @@ static int imx6_pcie_link_up(struct pcie_port *pp)
>  		 * Wait a little bit, then re-check if the link finished
>  		 * the training.
>  		 */
> -		usleep_range(1000, 2000);
> +		mdelay(1000);
>  	}
>  	/*
>  	 * From L0, initiate MAC entry to gen2 if EP/RC supports gen2.
> --
> 1.7.11.7
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" 
> in the body of a message to majordomo at vger.kernel.org More majordomo 
> info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] PCI: imx6:don't sleep in atomic context
  2016-02-18  7:17   ` Sharma, Sanjeev
@ 2016-02-18 15:08     ` Bjorn Helgaas
  2016-02-19  9:18       ` Sharma, Sanjeev
  0 siblings, 1 reply; 11+ messages in thread
From: Bjorn Helgaas @ 2016-02-18 15:08 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Feb 18, 2016 at 07:17:41AM +0000, Sharma, Sanjeev wrote:
> -----Original Message-----
> From: Bjorn Helgaas [mailto:helgaas at kernel.org] 
> Sent: Thursday, January 07, 2016 3:35 AM
> To: Sharma, Sanjeev
> Cc: Richard.Zhu at freescale.com; l.stach at pengutronix.de; bhelgaas at google.com; linux-pci at vger.kernel.org; linux-arm-kernel at lists.infradead.org; linux-kernel at vger.kernel.org; David Mueller
> Subject: Re: [PATCH] PCI: imx6:don't sleep in atomic context
> 
> Hi Sanjeev,
> 
> On Mon, Nov 09, 2015 at 04:18:00PM +0530, Sanjeev Sharma wrote:
> > If additional PCIe switch get connected between the host and the 
> > NIC,the kernel crashes with "BUG:
> > scheduling while atomic". To handle this we need to call mdelay() 
> > instead of usleep_range().
> > 
> > For more detail please refer bugzilla.kernel.org, Bug
> > 100031
> > 
> > Signed-off-by: Sanjeev Sharma <sanjeev_sharma@mentor.com>
> > Signed-off-by: David Mueller <dave.mueller@gmx.ch>
> 
> I'm dropping this for now because we've been kicking around the same solution (with tweaks to the mdelay amount) since June, but no progress on the *real* issue, which is that imx6_pcie_link_up() should never wait; it should simply return the link status.
> 
> I'm pretty sure the amount of time I've spent looking into this would have been enough to make some progress on that underlying issue.
> 
> Bjorn
> 
> Ok ! please share the change you are planning to implement. 

I didn't mean I personally was going to do this.  But I think Lucas
stepped up and did it:

https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?h=pci/host-imx6&id=4d107d3b5a686b5834e533a00b73bf7b1cf59df7

> > ---
> >  drivers/pci/host/pci-imx6.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c 
> > index 233a196..9769b13 100644
> > --- a/drivers/pci/host/pci-imx6.c
> > +++ b/drivers/pci/host/pci-imx6.c
> > @@ -499,7 +499,7 @@ static int imx6_pcie_link_up(struct pcie_port *pp)
> >  		 * Wait a little bit, then re-check if the link finished
> >  		 * the training.
> >  		 */
> > -		usleep_range(1000, 2000);
> > +		mdelay(1000);
> >  	}
> >  	/*
> >  	 * From L0, initiate MAC entry to gen2 if EP/RC supports gen2.
> > --
> > 1.7.11.7
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-pci" 
> > in the body of a message to majordomo at vger.kernel.org More majordomo 
> > info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] PCI: imx6:don't sleep in atomic context
  2016-02-18 15:08     ` Bjorn Helgaas
@ 2016-02-19  9:18       ` Sharma, Sanjeev
  0 siblings, 0 replies; 11+ messages in thread
From: Sharma, Sanjeev @ 2016-02-19  9:18 UTC (permalink / raw)
  To: linux-arm-kernel

-----Original Message-----
From: Bjorn Helgaas [mailto:helgaas at kernel.org] 
Sent: Thursday, February 18, 2016 8:39 PM
To: Sharma, Sanjeev
Cc: Richard.Zhu at freescale.com; l.stach at pengutronix.de; bhelgaas at google.com; linux-pci at vger.kernel.org; linux-arm-kernel at lists.infradead.org; linux-kernel at vger.kernel.org; David Mueller
Subject: Re: [PATCH] PCI: imx6:don't sleep in atomic context

On Thu, Feb 18, 2016 at 07:17:41AM +0000, Sharma, Sanjeev wrote:
> -----Original Message-----
> From: Bjorn Helgaas [mailto:helgaas at kernel.org]
> Sent: Thursday, January 07, 2016 3:35 AM
> To: Sharma, Sanjeev
> Cc: Richard.Zhu at freescale.com; l.stach at pengutronix.de; 
> bhelgaas at google.com; linux-pci at vger.kernel.org; 
> linux-arm-kernel at lists.infradead.org; linux-kernel at vger.kernel.org; 
> David Mueller
> Subject: Re: [PATCH] PCI: imx6:don't sleep in atomic context
> 
> Hi Sanjeev,
> 
> On Mon, Nov 09, 2015 at 04:18:00PM +0530, Sanjeev Sharma wrote:
> > If additional PCIe switch get connected between the host and the 
> > NIC,the kernel crashes with "BUG:
> > scheduling while atomic". To handle this we need to call mdelay() 
> > instead of usleep_range().
> > 
> > For more detail please refer bugzilla.kernel.org, Bug
> > 100031
> > 
> > Signed-off-by: Sanjeev Sharma <sanjeev_sharma@mentor.com>
> > Signed-off-by: David Mueller <dave.mueller@gmx.ch>
> 
> I'm dropping this for now because we've been kicking around the same solution (with tweaks to the mdelay amount) since June, but no progress on the *real* issue, which is that imx6_pcie_link_up() should never wait; it should simply return the link status.
> 
> I'm pretty sure the amount of time I've spent looking into this would have been enough to make some progress on that underlying issue.
> 
> Bjorn
> 
> Ok ! please share the change you are planning to implement. 

I didn't mean I personally was going to do this.  But I think Lucas stepped up and did it:

https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?h=pci/host-imx6&id=4d107d3b5a686b5834e533a00b73bf7b1cf59df7

Thanks Bjorn 

code look ok !

> > ---
> >  drivers/pci/host/pci-imx6.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/pci/host/pci-imx6.c 
> > b/drivers/pci/host/pci-imx6.c index 233a196..9769b13 100644
> > --- a/drivers/pci/host/pci-imx6.c
> > +++ b/drivers/pci/host/pci-imx6.c
> > @@ -499,7 +499,7 @@ static int imx6_pcie_link_up(struct pcie_port *pp)
> >  		 * Wait a little bit, then re-check if the link finished
> >  		 * the training.
> >  		 */
> > -		usleep_range(1000, 2000);
> > +		mdelay(1000);
> >  	}
> >  	/*
> >  	 * From L0, initiate MAC entry to gen2 if EP/RC supports gen2.
> > --
> > 1.7.11.7
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-pci" 
> > in the body of a message to majordomo at vger.kernel.org More majordomo 
> > info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" 
> in the body of a message to majordomo at vger.kernel.org More majordomo 
> info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2016-02-19  9:18 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <1447066080-5859-1-git-send-email-sanjeev_sharma@mentor.com>
2015-11-10  8:41 ` [PATCH] PCI: imx6:don't sleep in atomic context Lucas Stach
2015-11-10  9:28   ` Arnd Bergmann
2015-11-10  9:35     ` Lucas Stach
2015-11-10  9:45       ` Arnd Bergmann
2015-11-16  9:36         ` Sharma, Sanjeev
2015-11-24 13:57           ` Lucas Stach
     [not found] ` <1449042196-25710-1-git-send-email-sanjeev_sharma@mentor.com>
2016-01-06  2:13   ` [PATCH v2] " Bjorn Helgaas
2016-01-06 22:04 ` [PATCH] " Bjorn Helgaas
2016-02-18  7:17   ` Sharma, Sanjeev
2016-02-18 15:08     ` Bjorn Helgaas
2016-02-19  9:18       ` Sharma, Sanjeev

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).