[prev in list] [next in list] [prev in thread] [next in thread] 

List:       uclinux-dev
Subject:    Re: [uClinux-dev] [PATCH] m68knommu: Coldfire QSPI platform support
From:       Greg Ungerer <gerg () snapgear ! com>
Date:       2010-09-28 5:37:49
Message-ID: 4CA17F2D.7000407 () snapgear ! com
[Download RAW message or body]

Hi Jate,

On 25/09/10 00:00, Jate Sujjavanich wrote:
> I tried out the&msg->queue, and it's running fine. It's better than my fix because \
> it's more clear than "mcfqspi->msgq.next". 
> 
> After grabbing a msg from the msgq, the mcfqspi_work function calls
> list_del_init on the mcfqspi->msgq which unintentionally deletes the rest
> of the list before it can be processed. If qspi call was made using
> spi_sync, this can result in a process hang.
> 
> Signed-off by Jate Sujjavanich<jsujjavanich@syntech-fuelmaster.com>

Seeing as this is spi related you should send this to the spi
list as well (or at least to Grant Likley as well).

Regards
Greg



> -----------
> diff --git a/drivers/spi/coldfire_qspi.c b/drivers/spi/coldfire_qspi.c
> index 59be3ef..0aa00af 100644
> --- a/drivers/spi/coldfire_qspi.c
> +++ b/drivers/spi/coldfire_qspi.c
> @@ -316,7 +316,7 @@ static void mcfqspi_work(struct work_struct *work)
> msg = container_of(mcfqspi->msgq.next, struct spi_message,
> queue);
> 
> -               list_del_init(&mcfqspi->msgq);
> +               list_del_init(&msg->queue);
> spin_unlock_irqrestore(&mcfqspi->lock, flags);
> 
> spi = msg->spi;
> 
> -----Original Message-----
> From: Steven King [mailto:sfking@fdwdc.com]
> Sent: Thursday, September 23, 2010 11:43 AM
> To: Jate Sujjavanich
> Cc: uclinux-dev@uclinux.org; 'Greg Ungerer'
> Subject: Re: [uClinux-dev] [PATCH] m68knommu: Coldfire QSPI platform support
> 
> On Wednesday 22 September 2010 09:03:24 Jate Sujjavanich wrote:
> > Steven,
> > 
> > I have multiple qspi devices on my system, and I was getting a hang on
> > certain processes that were making spi_sync calls. There completion
> > callbacks were never being called because the messages weren't being
> > processed. See the patch.
> > 
> > - Jate
> > 
> > ---------
> > After grabbing a msg from the msgq, the mcfqspi_work function calls
> > list_del_init on the mcfqspi->msgq which unintentionally deletes the rest
> > of the list before it can be processed. If qspi call was made using
> > spi_sync, this can result in a process hang.
> > 
> > Signed-off by Jate Sujjavanich<jsujjavanich@syntech-fuelmaster.com>
> > ---------
> > diff --git a/drivers/spi/coldfire_qspi.c b/drivers/spi/coldfire_qspi.c
> > index 05d4cfa..23822bb 100644
> > --- a/drivers/spi/coldfire_qspi.c
> > +++ b/drivers/spi/coldfire_qspi.c
> > @@ -317,7 +317,7 @@ static void mcfqspi_work(struct work_struct *work)
> > msg = container_of(mcfqspi->msgq.next, struct spi_message,
> > queue);
> > 
> > -               list_del_init(&mcfqspi->msgq);
> > +               list_del_init(mcfqspi->msgq.next);
> > spin_unlock_irqrestore(&mcfqspi->lock, flags);
> > 
> > spi = msg->spi;
> 
> Hi Jate!
> 
> Actually, After looking to see what the other work queue based spi drivers
> were doing, I think that list_del_init in my original is total wrong!  Can you try
> this patch?
> 
> diff --git a/drivers/spi/coldfire_qspi.c b/drivers/spi/coldfire_qspi.c
> index 59be3ef..0aa00af 100644
> --- a/drivers/spi/coldfire_qspi.c
> +++ b/drivers/spi/coldfire_qspi.c
> @@ -316,7 +316,7 @@ static void mcfqspi_work(struct work_struct *work)
> msg = container_of(mcfqspi->msgq.next, struct spi_message,
> queue);
> 
> -               list_del_init(&mcfqspi->msgq);
> +               list_del_init(&msg->queue);
> spin_unlock_irqrestore(&mcfqspi->lock, flags);
> 
> spi = msg->spi;
> 
> 
> 
> 

_______________________________________________
uClinux-dev mailing list
uClinux-dev@uclinux.org
http://mailman.uclinux.org/mailman/listinfo/uclinux-dev
This message was resent by uclinux-dev@uclinux.org
To unsubscribe see:
http://mailman.uclinux.org/mailman/options/uclinux-dev


[prev in list] [next in list] [prev in thread] [next in thread]