Patchwork Using GPMI-NAND driver on iMX28 using 3.4-rc1?

login
register
mail settings
Submitter Vinod Koul
Date April 5, 2012, 12:27 p.m.
Message ID <1333628861.31825.22.camel@vkoul-udesk3>
Download mbox | patch
Permalink /patch/150945/
State New
Headers show

Comments

Vinod Koul - April 5, 2012, 12:27 p.m.
On Thu, 2012-04-05 at 05:03 -0700, Sam Gandhi wrote:
> 2012/4/4 Huang Shijie <b32955@freescale.com>:
> > Hi All:
> >> On Wed, Apr 4, 2012 at 7:07 PM, Sam Gandhi<samgandhi9@gmail.com>  wrote:
> >>
> >>> I reverted that commit and I still see following  error!
> >> Huang,
> >>
> >> Are you able to use gpmi on mx28 running 3.4-rc1?
> >>
> > I also meet the same problem today.
> >
> >
> >>> flash_erase /dev/mtd1 0 0
> >>> Erasing 1------------[ cut here ]------------
> >>> kernel BUG at /home/sam/linux/drivers/dma/dmaengine.h:53!
> > the mxs-dma has added some patches about the cookie.
> > The bug is in the dmaengine.h.
> >
> > So let more people know this bug.
> >
> > BR
> > Huang Shijie
> >
> >
> FWIW, Just a data point.
> 
> I coverted BUG_ON in dmaengine.h to printk as shown below. With this
> change I was able to format nand, create UBI partition. I have been
> running UBI torture test called integck on my board that does lot of
> I/O, mounting/unmounting of filesystem for close to 8 hour now without
> crash. But I do see those printks. I haven't followed logic of
> tx->cookie well enough to figure out what the appropriate change
> should be. Note this is with commit
> 00292bbf769620dea923dbd906afd88955f7ea19 reverted in my tree.
> 
> Cookie 0  completed 102118268 DMA_MIN 1
> Cookie 0  completed 102120401 DMA_MIN 1
> Cookie 0  completed 102237726 DMA_MIN 1
> 
> git diff drivers/dma/dmaengine.h
> diff --git a/drivers/dma/dmaengine.h b/drivers/dma/dmaengine.h
> index 17f983a..3d10a70 100644
> --- a/drivers/dma/dmaengine.h
> +++ b/drivers/dma/dmaengine.h
> @@ -50,7 +50,11 @@ static inline dma_cookie_t dma_cookie_assign(struct
> dma_async_tx_descriptor *tx)
>   */
>  static inline void dma_cookie_complete(struct dma_async_tx_descriptor *tx)
>  {
> -       BUG_ON(tx->cookie < DMA_MIN_COOKIE);
> +       if ( tx->cookie < DMA_MIN_COOKIE)
> +               printk(KERN_ERR "Cookie %d,  completed %d DMA_MIN %d
> ",tx->cookie,
> +               tx->chan->completed_cookie,
> +               DMA_MIN_COOKIE);
> +       /* BUG_ON(tx->cookie < DMA_MIN_COOKIE); */
>         tx->chan->completed_cookie = tx->cookie;
>         tx->cookie = 0;
>  }
This means you are trying to mark a cookie complete when it is already
marked so!, hence dmaengine screams.
The bug is is mxs-dma.
Let me know if below fixes it (assuming you are not using cyclic API,
that would need fixup as well)

 	if (direction == DMA_TRANS_NONE) {
Sam Gandhi - April 5, 2012, 1:02 p.m.
Vinod,

On Thu, Apr 5, 2012 at 5:27 AM, Vinod Koul <vinod.koul@linux.intel.com> wrote:
> On Thu, 2012-04-05 at 05:03 -0700, Sam Gandhi wrote:
>> 2012/4/4 Huang Shijie <b32955@freescale.com>:
>> > Hi All:
>> >> On Wed, Apr 4, 2012 at 7:07 PM, Sam Gandhi<samgandhi9@gmail.com>  wrote:
>> >>
>> >>> I reverted that commit and I still see following  error!
>> >> Huang,
>> >>
>> >> Are you able to use gpmi on mx28 running 3.4-rc1?
>> >>
>> > I also meet the same problem today.
>> >
>> >
>> >>> flash_erase /dev/mtd1 0 0
>> >>> Erasing 1------------[ cut here ]------------
>> >>> kernel BUG at /home/sam/linux/drivers/dma/dmaengine.h:53!
>> > the mxs-dma has added some patches about the cookie.
>> > The bug is in the dmaengine.h.
>> >
>> > So let more people know this bug.
>> >
>> > BR
>> > Huang Shijie
>> >
>> >
>> FWIW, Just a data point.
>>
>> I coverted BUG_ON in dmaengine.h to printk as shown below. With this
>> change I was able to format nand, create UBI partition. I have been
>> running UBI torture test called integck on my board that does lot of
>> I/O, mounting/unmounting of filesystem for close to 8 hour now without
>> crash. But I do see those printks. I haven't followed logic of
>> tx->cookie well enough to figure out what the appropriate change
>> should be. Note this is with commit
>> 00292bbf769620dea923dbd906afd88955f7ea19 reverted in my tree.
>>
>> Cookie 0  completed 102118268 DMA_MIN 1
>> Cookie 0  completed 102120401 DMA_MIN 1
>> Cookie 0  completed 102237726 DMA_MIN 1
>>
>> git diff drivers/dma/dmaengine.h
>> diff --git a/drivers/dma/dmaengine.h b/drivers/dma/dmaengine.h
>> index 17f983a..3d10a70 100644
>> --- a/drivers/dma/dmaengine.h
>> +++ b/drivers/dma/dmaengine.h
>> @@ -50,7 +50,11 @@ static inline dma_cookie_t dma_cookie_assign(struct
>> dma_async_tx_descriptor *tx)
>>   */
>>  static inline void dma_cookie_complete(struct dma_async_tx_descriptor *tx)
>>  {
>> -       BUG_ON(tx->cookie < DMA_MIN_COOKIE);
>> +       if ( tx->cookie < DMA_MIN_COOKIE)
>> +               printk(KERN_ERR "Cookie %d,  completed %d DMA_MIN %d
>> ",tx->cookie,
>> +               tx->chan->completed_cookie,
>> +               DMA_MIN_COOKIE);
>> +       /* BUG_ON(tx->cookie < DMA_MIN_COOKIE); */
>>         tx->chan->completed_cookie = tx->cookie;
>>         tx->cookie = 0;
>>  }
> This means you are trying to mark a cookie complete when it is already
> marked so!, hence dmaengine screams.
> The bug is is mxs-dma.
> Let me know if below fixes it (assuming you are not using cyclic API,
> that would need fixup as well)
>
> diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
> index c81ef7e..5ddd84e 100644
> --- a/drivers/dma/mxs-dma.c
> +++ b/drivers/dma/mxs-dma.c
> @@ -399,6 +399,10 @@ static struct dma_async_tx_descriptor
> *mxs_dma_prep_slave_sg(
>                ccw->bits &= ~CCW_DEC_SEM;
>        } else {
>                idx = 0;
> +               /* assign cookie here,
> +                * hopefully for above case we dont need it
> +                */
> +               dma_cookie_assign(&mxs_chan->desc);
>        }
>
>        if (direction == DMA_TRANS_NONE) {
>
I applied your suggested change and don't hit the BUG_ON in
dmaengine.h with this change UBI torture test has run for last 30 min
or so I will let it run for a day. [ I will let mxs-dma authors
comment if this is a right change.. ]

-Sam
Vinod Koul - April 5, 2012, 1:48 p.m.
On Thu, 2012-04-05 at 06:02 -0700, Sam Gandhi wrote:
> Vinod,
> 
> On Thu, Apr 5, 2012 at 5:27 AM, Vinod Koul <vinod.koul@linux.intel.com> wrote:
> > On Thu, 2012-04-05 at 05:03 -0700, Sam Gandhi wrote:
> >> 2012/4/4 Huang Shijie <b32955@freescale.com>:
> >> > Hi All:
> >> >> On Wed, Apr 4, 2012 at 7:07 PM, Sam Gandhi<samgandhi9@gmail.com>  wrote:
> >> >>
> >> >>> I reverted that commit and I still see following  error!
> >> >> Huang,
> >> >>
> >> >> Are you able to use gpmi on mx28 running 3.4-rc1?
> >> >>
> >> > I also meet the same problem today.
> >> >
> >> >
> >> >>> flash_erase /dev/mtd1 0 0
> >> >>> Erasing 1------------[ cut here ]------------
> >> >>> kernel BUG at /home/sam/linux/drivers/dma/dmaengine.h:53!
> >> > the mxs-dma has added some patches about the cookie.
> >> > The bug is in the dmaengine.h.
> >> >
> >> > So let more people know this bug.
> >> >
> >> > BR
> >> > Huang Shijie
> >> >
> >> >
> >> FWIW, Just a data point.
> >>
> >> I coverted BUG_ON in dmaengine.h to printk as shown below. With this
> >> change I was able to format nand, create UBI partition. I have been
> >> running UBI torture test called integck on my board that does lot of
> >> I/O, mounting/unmounting of filesystem for close to 8 hour now without
> >> crash. But I do see those printks. I haven't followed logic of
> >> tx->cookie well enough to figure out what the appropriate change
> >> should be. Note this is with commit
> >> 00292bbf769620dea923dbd906afd88955f7ea19 reverted in my tree.
> >>
> >> Cookie 0  completed 102118268 DMA_MIN 1
> >> Cookie 0  completed 102120401 DMA_MIN 1
> >> Cookie 0  completed 102237726 DMA_MIN 1
> >>
> >> git diff drivers/dma/dmaengine.h
> >> diff --git a/drivers/dma/dmaengine.h b/drivers/dma/dmaengine.h
> >> index 17f983a..3d10a70 100644
> >> --- a/drivers/dma/dmaengine.h
> >> +++ b/drivers/dma/dmaengine.h
> >> @@ -50,7 +50,11 @@ static inline dma_cookie_t dma_cookie_assign(struct
> >> dma_async_tx_descriptor *tx)
> >>   */
> >>  static inline void dma_cookie_complete(struct dma_async_tx_descriptor *tx)
> >>  {
> >> -       BUG_ON(tx->cookie < DMA_MIN_COOKIE);
> >> +       if ( tx->cookie < DMA_MIN_COOKIE)
> >> +               printk(KERN_ERR "Cookie %d,  completed %d DMA_MIN %d
> >> ",tx->cookie,
> >> +               tx->chan->completed_cookie,
> >> +               DMA_MIN_COOKIE);
> >> +       /* BUG_ON(tx->cookie < DMA_MIN_COOKIE); */
> >>         tx->chan->completed_cookie = tx->cookie;
> >>         tx->cookie = 0;
> >>  }
> > This means you are trying to mark a cookie complete when it is already
> > marked so!, hence dmaengine screams.
> > The bug is is mxs-dma.
> > Let me know if below fixes it (assuming you are not using cyclic API,
> > that would need fixup as well)
> >
> > diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
> > index c81ef7e..5ddd84e 100644
> > --- a/drivers/dma/mxs-dma.c
> > +++ b/drivers/dma/mxs-dma.c
> > @@ -399,6 +399,10 @@ static struct dma_async_tx_descriptor
> > *mxs_dma_prep_slave_sg(
> >                ccw->bits &= ~CCW_DEC_SEM;
> >        } else {
> >                idx = 0;
> > +               /* assign cookie here,
> > +                * hopefully for above case we dont need it
> > +                */
> > +               dma_cookie_assign(&mxs_chan->desc);
> >        }
> >
> >        if (direction == DMA_TRANS_NONE) {
> >
> I applied your suggested change and don't hit the BUG_ON in
> dmaengine.h with this change UBI torture test has run for last 30 min
> or so I will let it run for a day. [ I will let mxs-dma authors
> comment if this is a right change.. ]
Good I will apply this and send to Linus.
Care to give your tested-by.
Sam Gandhi - April 5, 2012, 2:38 p.m.
On Thu, Apr 5, 2012 at 6:48 AM, Vinod Koul <vinod.koul@linux.intel.com> wrote:
> On Thu, 2012-04-05 at 06:02 -0700, Sam Gandhi wrote:
>> Vinod,
>>
>> On Thu, Apr 5, 2012 at 5:27 AM, Vinod Koul <vinod.koul@linux.intel.com> wrote:
>> > On Thu, 2012-04-05 at 05:03 -0700, Sam Gandhi wrote:
>> >> 2012/4/4 Huang Shijie <b32955@freescale.com>:
>> >> > Hi All:
>> >> >> On Wed, Apr 4, 2012 at 7:07 PM, Sam Gandhi<samgandhi9@gmail.com>  wrote:
>> >> >>
>> >> >>> I reverted that commit and I still see following  error!
>> >> >> Huang,
>> >> >>
>> >> >> Are you able to use gpmi on mx28 running 3.4-rc1?
>> >> >>
>> >> > I also meet the same problem today.
>> >> >
>> >> >
>> >> >>> flash_erase /dev/mtd1 0 0
>> >> >>> Erasing 1------------[ cut here ]------------
>> >> >>> kernel BUG at /home/sam/linux/drivers/dma/dmaengine.h:53!
>> >> > the mxs-dma has added some patches about the cookie.
>> >> > The bug is in the dmaengine.h.
>> >> >
>> >> > So let more people know this bug.
>> >> >
>> >> > BR
>> >> > Huang Shijie
>> >> >
>> >> >
>> >> FWIW, Just a data point.
>> >>
>> >> I coverted BUG_ON in dmaengine.h to printk as shown below. With this
>> >> change I was able to format nand, create UBI partition. I have been
>> >> running UBI torture test called integck on my board that does lot of
>> >> I/O, mounting/unmounting of filesystem for close to 8 hour now without
>> >> crash. But I do see those printks. I haven't followed logic of
>> >> tx->cookie well enough to figure out what the appropriate change
>> >> should be. Note this is with commit
>> >> 00292bbf769620dea923dbd906afd88955f7ea19 reverted in my tree.
>> >>
>> >> Cookie 0  completed 102118268 DMA_MIN 1
>> >> Cookie 0  completed 102120401 DMA_MIN 1
>> >> Cookie 0  completed 102237726 DMA_MIN 1
>> >>
>> >> git diff drivers/dma/dmaengine.h
>> >> diff --git a/drivers/dma/dmaengine.h b/drivers/dma/dmaengine.h
>> >> index 17f983a..3d10a70 100644
>> >> --- a/drivers/dma/dmaengine.h
>> >> +++ b/drivers/dma/dmaengine.h
>> >> @@ -50,7 +50,11 @@ static inline dma_cookie_t dma_cookie_assign(struct
>> >> dma_async_tx_descriptor *tx)
>> >>   */
>> >>  static inline void dma_cookie_complete(struct dma_async_tx_descriptor *tx)
>> >>  {
>> >> -       BUG_ON(tx->cookie < DMA_MIN_COOKIE);
>> >> +       if ( tx->cookie < DMA_MIN_COOKIE)
>> >> +               printk(KERN_ERR "Cookie %d,  completed %d DMA_MIN %d
>> >> ",tx->cookie,
>> >> +               tx->chan->completed_cookie,
>> >> +               DMA_MIN_COOKIE);
>> >> +       /* BUG_ON(tx->cookie < DMA_MIN_COOKIE); */
>> >>         tx->chan->completed_cookie = tx->cookie;
>> >>         tx->cookie = 0;
>> >>  }
>> > This means you are trying to mark a cookie complete when it is already
>> > marked so!, hence dmaengine screams.
>> > The bug is is mxs-dma.
>> > Let me know if below fixes it (assuming you are not using cyclic API,
>> > that would need fixup as well)
>> >
>> > diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
>> > index c81ef7e..5ddd84e 100644
>> > --- a/drivers/dma/mxs-dma.c
>> > +++ b/drivers/dma/mxs-dma.c
>> > @@ -399,6 +399,10 @@ static struct dma_async_tx_descriptor
>> > *mxs_dma_prep_slave_sg(
>> >                ccw->bits &= ~CCW_DEC_SEM;
>> >        } else {
>> >                idx = 0;
>> > +               /* assign cookie here,
>> > +                * hopefully for above case we dont need it
>> > +                */
>> > +               dma_cookie_assign(&mxs_chan->desc);
>> >        }
>> >
>> >        if (direction == DMA_TRANS_NONE) {
>> >
>> I applied your suggested change and don't hit the BUG_ON in
>> dmaengine.h with this change UBI torture test has run for last 30 min
>> or so I will let it run for a day. [ I will let mxs-dma authors
>> comment if this is a right change.. ]
> Good I will apply this and send to Linus.
> Care to give your tested-by.

Sure.

-Sam (samgandhi9@gmail.com )
Shawn Guo - April 6, 2012, 2:23 a.m.
On Thu, Apr 05, 2012 at 05:57:41PM +0530, Vinod Koul wrote:
> On Thu, 2012-04-05 at 05:03 -0700, Sam Gandhi wrote:
> > 2012/4/4 Huang Shijie <b32955@freescale.com>:
...
> > >>> flash_erase /dev/mtd1 0 0
> > >>> Erasing 1------------[ cut here ]------------
> > >>> kernel BUG at /home/sam/linux/drivers/dma/dmaengine.h:53!
> > > the mxs-dma has added some patches about the cookie.
> > > The bug is in the dmaengine.h.
> > >
> > > So let more people know this bug.
> > >

I'm still trying to understand why I did run into the BUG_ON when I
test it with mxs mmc and sound drivers.

...

> This means you are trying to mark a cookie complete when it is already
> marked so!, hence dmaengine screams.
> The bug is is mxs-dma.
> Let me know if below fixes it (assuming you are not using cyclic API,
> that would need fixup as well)
> 
> diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
> index c81ef7e..5ddd84e 100644
> --- a/drivers/dma/mxs-dma.c
> +++ b/drivers/dma/mxs-dma.c
> @@ -399,6 +399,10 @@ static struct dma_async_tx_descriptor
> *mxs_dma_prep_slave_sg(
>  		ccw->bits &= ~CCW_DEC_SEM;
>  	} else {
>  		idx = 0;
> +		/* assign cookie here,
> +		 * hopefully for above case we dont need it
> +		 */
> +		dma_cookie_assign(&mxs_chan->desc);

Isn't it done in mxs_dma_tx_submit() already? The gpmi driver somehow
misses the call to dmaengine_submit() for some case?

Regards,
Shawn

>  	}
>  
>  	if (direction == DMA_TRANS_NONE) {
> 
> --
Huang Shijie - April 6, 2012, 2:45 a.m.
hi,
> Isn't it done in mxs_dma_tx_submit() already? The gpmi driver somehow
> misses the call to dmaengine_submit() for some case?
>
I checked the code again. It seems I do not miss the call to 
dmaengine_submit().

There are five places where call the dmaengine_submit() by 
start_dma_with_bch_irq()/start_dma_without_bch_irq():
         gpmi_send_command(),gpmi_send_data(),gpmi_read_data(), 
gpmi_send_page(), gpmi_read_page().

The gpmi nand code runs well in previous mxs-dma code.

BR
Huang Shijie
Shawn Guo - April 6, 2012, 3:45 a.m.
On Fri, Apr 06, 2012 at 10:45:07AM +0800, Huang Shijie wrote:
> hi,
> >Isn't it done in mxs_dma_tx_submit() already? The gpmi driver somehow
> >misses the call to dmaengine_submit() for some case?
> >
> I checked the code again. It seems I do not miss the call to
> dmaengine_submit().
> 
I do not have nand device to play.  Maybe you want to have a check
if dma_cookie_assign (dmaengine_submit) and dma_cookie_complete
(via mxs_dma_int_handler) are being called as a pair for gpmi.

Patch

diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
index c81ef7e..5ddd84e 100644
--- a/drivers/dma/mxs-dma.c
+++ b/drivers/dma/mxs-dma.c
@@ -399,6 +399,10 @@  static struct dma_async_tx_descriptor
*mxs_dma_prep_slave_sg(
 		ccw->bits &= ~CCW_DEC_SEM;
 	} else {
 		idx = 0;
+		/* assign cookie here,
+		 * hopefully for above case we dont need it
+		 */
+		dma_cookie_assign(&mxs_chan->desc);
 	}