[U-Boot] [Patch V7 04/18] net/fm/eth: Use mb() to be compatible for both ARM and PowerPC

[Gong Q.Y.]

> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Friday, October 23, 2015 9:42 PM
> To: Hou Zhiqiang-B48286
> Cc: Gong Qianyu-B52263; u-boot at lists.denx.de; Hu Mingkai-B21284; Sun
> York-R58495; Xie Shaohui-B21989; Song Wenbin-B53747; Kushwaha Prabhakar-
> B32579; Wang Huan-B18965
> Subject: Re: [Patch V7 04/18] net/fm/eth: Use mb() to be compatible for
> both ARM and PowerPC
> 
> On Fri, 2015-10-23 at 06:01 -0500, Hou Zhiqiang-B48286 wrote:
> >
> > > -----Original Message-----
> > > From: Wood Scott-B07421
> > > Sent: 2015年10月23日 7:11
> > > To: Gong Qianyu-B52263
> > > Cc: u-boot at lists.denx.de; Hu Mingkai-B21284; Sun York-R58495; Hou
> > > Zhiqiang-B48286; Xie Shaohui-B21989; Song Wenbin-B53747; Wood Scott-
> > > B07421; Kushwaha Prabhakar-B32579; Wang Huan-B18965
> > > Subject: Re: [Patch V7 04/18] net/fm/eth: Use mb() to be compatible
> > > for both ARM and PowerPC
> > >
> > > On Thu, 2015-10-22 at 18:46 +0800, Gong Qianyu wrote:
> > > > From: Shaohui Xie <Shaohui.Xie at freescale.com>
> > > >
> > > > Use mb() instead of sync() to be compatible for both ARM and
> PowerPC.
> > > >
> > > > Signed-off-by: Shaohui Xie <Shaohui.Xie at freescale.com>
> > > > Signed-off-by: Mingkai Hu <Mingkai.Hu at freescale.com>
> > > > Signed-off-by: Gong Qianyu <Qianyu.Gong at freescale.com>
> > > > ---
> > > > V7:
> > > >  - No change.
> > > > V6:
> > > >  - No change.
> > > > V5:
> > > >  - No change.
> > > > V4:
> > > >  - No change.
> > > > V3:
> > > >  - New patch. Separated from patch 'net: Move some header files to
> > > include/'
> > > >
> > > >  drivers/net/fm/eth.c | 14 +++++++-------
> > > >  1 file changed, 7 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/drivers/net/fm/eth.c b/drivers/net/fm/eth.c index
> > > > 368d554..ad02c66 100644
> > > > --- a/drivers/net/fm/eth.c
> > > > +++ b/drivers/net/fm/eth.c
> > > > @@ -371,7 +371,7 @@ static void
> > > > fmc_tx_port_graceful_stop_enable(struct
> > > > fm_eth *fm_eth)
> > > >       pram = fm_eth->tx_pram;
> > > >       /* graceful stop transmission of frames */
> > > >       setbits_be32(&pram->mode, PRAM_MODE_GRACEFUL_STOP);
> > > > -     sync();
> > > > +     mb();
> > > >  }
> > > >
> > > >  static void fmc_tx_port_graceful_stop_disable(struct fm_eth
> > > > *fm_eth) @@ -381,7 +381,7 @@ static void
> > > > fmc_tx_port_graceful_stop_disable(struct
> > > > fm_eth *fm_eth)
> > > >       pram = fm_eth->tx_pram;
> > > >       /* re-enable transmission of frames */
> > > >       clrbits_be32(&pram->mode, PRAM_MODE_GRACEFUL_STOP);
> > > > -     sync();
> > > > +     mb();
> > > >  }
> > >
> > > Why is it needed at all?  The I/O accessors should include the
> > > necessary barriers.
> >
> > The I/O accessors of powerpc does include the barrier, but it absents
> > from ARMs'.
> 
> Then fix ARM's accessors if they're missing barriers that are required.
> But first, consider whether the barrier is required on ARM.  What is
> special about FM that it needs it where all other drivers don't?
> 
> -Scott

I tested on both LS1043ARDB and T2080RDB board just now.
FM still works well without the barrier.