subject:"svn commit\: r192027 \- head\/sys\/arm\/at91"

Re: svn commit: r192027 - head/sys/arm/at91

2009-05-15 Thread Stanislav Sedov

On Thu, 14 May 2009 23:35:36 -0600 (MDT)
M. Warner Losh i...@bsdimp.com mentioned:

 In message: 20090515092205.6f6d06fa.s...@freebsd.org
 Stanislav Sedov s...@freebsd.org writes:
 : On Thu, 14 May 2009 21:37:12 -0600 (MDT)
 : M. Warner Losh i...@bsdimp.com mentioned:
 : 
 :  In message: 200905122114.n4cleag9033...@svn.freebsd.org
 :  Stanislav Sedov s...@freebsd.org writes:
 :  : @@ -926,6 +937,7 @@ atestart_locked(struct ifnet *ifp)
 :  :  * tell the hardware to xmit the packet.
 :  :  */
 :  : WR4(sc, ETH_TAR, segs[0].ds_addr);
 :  : +   BARRIER(sc, ETH_TAR, 8, BUS_SPACE_BARRIER_WRITE);
 :  : WR4(sc, ETH_TCR, segs[0].ds_len);
 :  
 :  Why is a barrier needed here?
 :  
 : Writing the TCR register triggers the transmit, so it had to be written
 : strongly after the TAR register. That's why I added the barrier here.
 
 Then shouldn't the barrier be after TCR write?  Or does this ensure
 that the write is before TCR?
 

Yeah, this barrier is to ensure that the TCR register gets written after the
TAR register has been written, not before. I don't think an additional barrier
is needed after the TCR write.

-- 
Stanislav Sedov
ST4096-RIPE

!DSPAM:4a0d7945994291484520249!


___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org

Re: svn commit: r192027 - head/sys/arm/at91

2009-05-15 Thread M. Warner Losh

In message: 20090515141642.ebc06b59.s...@freebsd.org
Stanislav Sedov s...@freebsd.org writes:
: On Thu, 14 May 2009 23:35:36 -0600 (MDT)
: M. Warner Losh i...@bsdimp.com mentioned:
: 
:  In message: 20090515092205.6f6d06fa.s...@freebsd.org
:  Stanislav Sedov s...@freebsd.org writes:
:  : On Thu, 14 May 2009 21:37:12 -0600 (MDT)
:  : M. Warner Losh i...@bsdimp.com mentioned:
:  : 
:  :  In message: 200905122114.n4cleag9033...@svn.freebsd.org
:  :  Stanislav Sedov s...@freebsd.org writes:
:  :  : @@ -926,6 +937,7 @@ atestart_locked(struct ifnet *ifp)
:  :  :* tell the hardware to xmit the packet.
:  :  :*/
:  :  :   WR4(sc, ETH_TAR, segs[0].ds_addr);
:  :  : + BARRIER(sc, ETH_TAR, 8, BUS_SPACE_BARRIER_WRITE);
:  :  :   WR4(sc, ETH_TCR, segs[0].ds_len);
:  :  
:  :  Why is a barrier needed here?
:  :  
:  : Writing the TCR register triggers the transmit, so it had to be written
:  : strongly after the TAR register. That's why I added the barrier here.
:  
:  Then shouldn't the barrier be after TCR write?  Or does this ensure
:  that the write is before TCR?
:  
: 
: Yeah, this barrier is to ensure that the TCR register gets written after the
: TAR register has been written, not before. I don't think an additional barrier
: is needed after the TCR write.

Did this fix an observed bug, or is it theoretical?  None of Atmel's
code does this, but maybe we turn on some flag that reorders writes.
On the other hand, I've seen some minor flakiness from time to time
that could be explained by reordering

There's likely a bunch of other places where something like this may
be needed.  The PDC has size/address information, followed by an
enable bit.  The MCI device has some similar weirdness as well...

Warner
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org

Re: svn commit: r192027 - head/sys/arm/at91

2009-05-15 Thread Stanislav Sedov

On Fri, 15 May 2009 09:05:31 -0600 (MDT)
M. Warner Losh i...@bsdimp.com mentioned:

 In message: 20090515141642.ebc06b59.s...@freebsd.org
 Stanislav Sedov s...@freebsd.org writes:
 : On Thu, 14 May 2009 23:35:36 -0600 (MDT)
 : M. Warner Losh i...@bsdimp.com mentioned:
 : 
 :  In message: 20090515092205.6f6d06fa.s...@freebsd.org
 :  Stanislav Sedov s...@freebsd.org writes:
 :  : On Thu, 14 May 2009 21:37:12 -0600 (MDT)
 :  : M. Warner Losh i...@bsdimp.com mentioned:
 :  : 
 :  :  In message: 200905122114.n4cleag9033...@svn.freebsd.org
 :  :  Stanislav Sedov s...@freebsd.org writes:
 :  :  : @@ -926,6 +937,7 @@ atestart_locked(struct ifnet *ifp)
 :  :  :  * tell the hardware to xmit the packet.
 :  :  :  */
 :  :  : WR4(sc, ETH_TAR, segs[0].ds_addr);
 :  :  : +   BARRIER(sc, ETH_TAR, 8, BUS_SPACE_BARRIER_WRITE);
 :  :  : WR4(sc, ETH_TCR, segs[0].ds_len);
 :  :  
 :  :  Why is a barrier needed here?
 :  :  
 :  : Writing the TCR register triggers the transmit, so it had to be written
 :  : strongly after the TAR register. That's why I added the barrier here.
 :  
 :  Then shouldn't the barrier be after TCR write?  Or does this ensure
 :  that the write is before TCR?
 :  
 : 
 : Yeah, this barrier is to ensure that the TCR register gets written after the
 : TAR register has been written, not before. I don't think an additional 
 barrier
 : is needed after the TCR write.
 
 Did this fix an observed bug, or is it theoretical?  None of Atmel's
 code does this, but maybe we turn on some flag that reorders writes.
 On the other hand, I've seen some minor flakiness from time to time
 that could be explained by reordering
 
 There's likely a bunch of other places where something like this may
 be needed.  The PDC has size/address information, followed by an
 enable bit.  The MCI device has some similar weirdness as well...
 

I don't think there're any reordering possible on at91 platform,
though I need to check first. The bus_space_barrier call is currently
a no-op on arm platforms, so this modifications were mostly to make
the code more correct theoretically then fixing any possible real-world
issues.

PDC is the entirely another thing, so it need to be checked separately.
EMAC doesn't use PDC but a real DMA implementation.

-- 
Stanislav Sedov
ST4096-RIPE

!DSPAM:4a0dc041994294531918519!


___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org

Re: svn commit: r192027 - head/sys/arm/at91

2009-05-15 Thread M. Warner Losh

In message: 20090515231922.fb760af4.s...@freebsd.org
Stanislav Sedov s...@freebsd.org writes:
: On Fri, 15 May 2009 09:05:31 -0600 (MDT)
: M. Warner Losh i...@bsdimp.com mentioned:
: 
:  In message: 20090515141642.ebc06b59.s...@freebsd.org
:  Stanislav Sedov s...@freebsd.org writes:
:  : On Thu, 14 May 2009 23:35:36 -0600 (MDT)
:  : M. Warner Losh i...@bsdimp.com mentioned:
:  : 
:  :  In message: 20090515092205.6f6d06fa.s...@freebsd.org
:  :  Stanislav Sedov s...@freebsd.org writes:
:  :  : On Thu, 14 May 2009 21:37:12 -0600 (MDT)
:  :  : M. Warner Losh i...@bsdimp.com mentioned:
:  :  : 
:  :  :  In message: 200905122114.n4cleag9033...@svn.freebsd.org
:  :  :  Stanislav Sedov s...@freebsd.org writes:
:  :  :  : @@ -926,6 +937,7 @@ atestart_locked(struct ifnet *ifp)
:  :  :  :* tell the hardware to xmit the packet.
:  :  :  :*/
:  :  :  :   WR4(sc, ETH_TAR, segs[0].ds_addr);
:  :  :  : + BARRIER(sc, ETH_TAR, 8, BUS_SPACE_BARRIER_WRITE);
:  :  :  :   WR4(sc, ETH_TCR, segs[0].ds_len);
:  :  :  
:  :  :  Why is a barrier needed here?
:  :  :  
:  :  : Writing the TCR register triggers the transmit, so it had to be 
written
:  :  : strongly after the TAR register. That's why I added the barrier here.
:  :  
:  :  Then shouldn't the barrier be after TCR write?  Or does this ensure
:  :  that the write is before TCR?
:  :  
:  : 
:  : Yeah, this barrier is to ensure that the TCR register gets written after 
the
:  : TAR register has been written, not before. I don't think an additional 
barrier
:  : is needed after the TCR write.
:  
:  Did this fix an observed bug, or is it theoretical?  None of Atmel's
:  code does this, but maybe we turn on some flag that reorders writes.
:  On the other hand, I've seen some minor flakiness from time to time
:  that could be explained by reordering
:  
:  There's likely a bunch of other places where something like this may
:  be needed.  The PDC has size/address information, followed by an
:  enable bit.  The MCI device has some similar weirdness as well...
:  
: 
: I don't think there're any reordering possible on at91 platform,
: though I need to check first. The bus_space_barrier call is currently
: a no-op on arm platforms, so this modifications were mostly to make
: the code more correct theoretically then fixing any possible real-world
: issues.

True.

: PDC is the entirely another thing, so it need to be checked separately.
: EMAC doesn't use PDC but a real DMA implementation.

Yes.  Understood.  Just thinking of other places this might matter.

Any idea if this matters on the AVR32?

Then again, the built-in devices are mapped into uncached memory, so
maybe it just doesn't matter :).

Warner
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org

Re: svn commit: r192027 - head/sys/arm/at91

2009-05-14 Thread M. Warner Losh

In message: 200905122114.n4cleag9033...@svn.freebsd.org
Stanislav Sedov s...@freebsd.org writes:
: @@ -926,6 +937,7 @@ atestart_locked(struct ifnet *ifp)
:* tell the hardware to xmit the packet.
:*/
:   WR4(sc, ETH_TAR, segs[0].ds_addr);
: + BARRIER(sc, ETH_TAR, 8, BUS_SPACE_BARRIER_WRITE);
:   WR4(sc, ETH_TCR, segs[0].ds_len);

Why is a barrier needed here?

Warner
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org

Re: svn commit: r192027 - head/sys/arm/at91

2009-05-14 Thread Stanislav Sedov

On Thu, 14 May 2009 21:37:12 -0600 (MDT)
M. Warner Losh i...@bsdimp.com mentioned:

 In message: 200905122114.n4cleag9033...@svn.freebsd.org
 Stanislav Sedov s...@freebsd.org writes:
 : @@ -926,6 +937,7 @@ atestart_locked(struct ifnet *ifp)
 :  * tell the hardware to xmit the packet.
 :  */
 : WR4(sc, ETH_TAR, segs[0].ds_addr);
 : +   BARRIER(sc, ETH_TAR, 8, BUS_SPACE_BARRIER_WRITE);
 : WR4(sc, ETH_TCR, segs[0].ds_len);
 
 Why is a barrier needed here?
 
Writing the TCR register triggers the transmit, so it had to be written
strongly after the TAR register. That's why I added the barrier here.

-- 
Stanislav Sedov
ST4096-RIPE

!DSPAM:4a0cfbe4994295595297431!


___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org

Re: svn commit: r192027 - head/sys/arm/at91

2009-05-14 Thread M. Warner Losh

In message: 20090515092205.6f6d06fa.s...@freebsd.org
Stanislav Sedov s...@freebsd.org writes:
: On Thu, 14 May 2009 21:37:12 -0600 (MDT)
: M. Warner Losh i...@bsdimp.com mentioned:
: 
:  In message: 200905122114.n4cleag9033...@svn.freebsd.org
:  Stanislav Sedov s...@freebsd.org writes:
:  : @@ -926,6 +937,7 @@ atestart_locked(struct ifnet *ifp)
:  :* tell the hardware to xmit the packet.
:  :*/
:  :   WR4(sc, ETH_TAR, segs[0].ds_addr);
:  : + BARRIER(sc, ETH_TAR, 8, BUS_SPACE_BARRIER_WRITE);
:  :   WR4(sc, ETH_TCR, segs[0].ds_len);
:  
:  Why is a barrier needed here?
:  
: Writing the TCR register triggers the transmit, so it had to be written
: strongly after the TAR register. That's why I added the barrier here.

Then shouldn't the barrier be after TCR write?  Or does this ensure
that the write is before TCR?

Warner
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org

svn commit: r192027 - head/sys/arm/at91

2009-05-12 Thread Stanislav Sedov

Author: stas
Date: Tue May 12 21:14:36 2009
New Revision: 192027
URL: http://svn.freebsd.org/changeset/base/192027

Log:
  - Eliminate extra register reads by using a variable to store
registers contents.
  - Use memory barriers to preserve the order of buffer space operations.
This might be needed if we'll ever use this driver on architectures
where ordering is not guaranteed.

Modified:
  head/sys/arm/at91/if_ate.c

Modified: head/sys/arm/at91/if_ate.c
==
--- head/sys/arm/at91/if_ate.c  Tue May 12 20:56:34 2009(r192026)
+++ head/sys/arm/at91/if_ate.c  Tue May 12 21:14:36 2009(r192027)
@@ -118,6 +118,13 @@ WR4(struct ate_softc *sc, bus_size_t off
bus_write_4(sc-mem_res, off, val);
 }
 
+static inline void
+BARRIER(struct ate_softc *sc, bus_size_t off, bus_size_t len, int flags)
+{
+
+   bus_barrier(sc-mem_res, off, len, flags);
+}
+
 #define ATE_LOCK(_sc)  mtx_lock((_sc)-sc_mtx)
 #defineATE_UNLOCK(_sc) mtx_unlock((_sc)-sc_mtx)
 #define ATE_LOCK_INIT(_sc) \
@@ -586,18 +593,19 @@ ate_ifmedia_sts(struct ifnet *ifp, struc
 static void
 ate_stat_update(struct ate_softc *sc, int active)
 {
+   uint32_t reg;
+
/*
 * The speed and full/half-duplex state needs to be reflected
 * in the ETH_CFG register.
 */
-   if (IFM_SUBTYPE(active) == IFM_10_T)
-   WR4(sc, ETH_CFG, RD4(sc, ETH_CFG)  ~ETH_CFG_SPD);
-   else
-   WR4(sc, ETH_CFG, RD4(sc, ETH_CFG) | ETH_CFG_SPD);
+   reg = RD4(sc, ETH_CFG);
+   reg = ~(ETH_CFG_SPD | ETH_CFG_FD);
+   if (IFM_SUBTYPE(active) != IFM_10_T)
+   reg |= ETH_CFG_SPD;
if (active  IFM_FDX)
-   WR4(sc, ETH_CFG, RD4(sc, ETH_CFG) | ETH_CFG_FD);
-   else
-   WR4(sc, ETH_CFG, RD4(sc, ETH_CFG)  ~ETH_CFG_FD);
+   reg |= ETH_CFG_FD;
+   WR4(sc, ETH_CFG, reg);
 }
 
 static void
@@ -709,11 +717,10 @@ ate_intr(void *xsc)
 {
struct ate_softc *sc = xsc;
struct ifnet *ifp = sc-ifp;
-   int status;
-   int i;
-   void *bp;
struct mbuf *mb;
-   uint32_t rx_stat;
+   void *bp;
+   uint32_t status, reg, rx_stat;
+   int i;
 
status = RD4(sc, ETH_ISR);
if (status == 0)
@@ -800,10 +807,11 @@ ate_intr(void *xsc)
ATE_UNLOCK(sc);
}
if (status  ETH_ISR_RBNA) {
-   printf(RBNA workaround\n);
/* Workaround Errata #11 */
-   WR4(sc, ETH_CTL, RD4(sc, ETH_CTL) ~ ETH_CTL_RE);
-   WR4(sc, ETH_CTL, RD4(sc, ETH_CTL) | ETH_CTL_RE);
+   reg = RD4(sc, ETH_CTL);
+   WR4(sc, ETH_CTL, reg  ~ETH_CTL_RE);
+   BARRIER(sc, ETH_CTL, 4, BUS_SPACE_BARRIER_WRITE);
+   WR4(sc, ETH_CTL, reg | ETH_CTL_RE);
}
 }
 
@@ -816,6 +824,7 @@ ateinit_locked(void *xsc)
struct ate_softc *sc = xsc;
struct ifnet *ifp = sc-ifp;
struct mii_data *mii;
+   uint32_t reg;
 
ATE_ASSERT_LOCKED(sc);
 
@@ -834,10 +843,12 @@ ateinit_locked(void *xsc)
 * to this chip.  Select the right one based on a compile-time
 * option.
 */
+   reg = RD4(sc, ETH_CFG);
if (sc-use_rmii)
-   WR4(sc, ETH_CFG, RD4(sc, ETH_CFG) | ETH_CFG_RMII);
+   reg |= ETH_CFG_RMII;
else
-   WR4(sc, ETH_CFG, RD4(sc, ETH_CFG)  ~ETH_CFG_RMII);
+   reg = ~ETH_CFG_RMII;
+   WR4(sc, ETH_CFG, reg);
 
ate_rxfilter(sc);
 
@@ -926,6 +937,7 @@ atestart_locked(struct ifnet *ifp)
 * tell the hardware to xmit the packet.
 */
WR4(sc, ETH_TAR, segs[0].ds_addr);
+   BARRIER(sc, ETH_TAR, 8, BUS_SPACE_BARRIER_WRITE);
WR4(sc, ETH_TCR, segs[0].ds_len);

/*
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org

Re: svn commit: r192027 - head/sys/arm/at91

Re: svn commit: r192027 - head/sys/arm/at91

Re: svn commit: r192027 - head/sys/arm/at91

Re: svn commit: r192027 - head/sys/arm/at91

Re: svn commit: r192027 - head/sys/arm/at91

Re: svn commit: r192027 - head/sys/arm/at91

Re: svn commit: r192027 - head/sys/arm/at91

svn commit: r192027 - head/sys/arm/at91

8 matches

Site Navigation

Mail list logo

Footer information