linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC/T] b43: Implement the BCM94311MCG rev 02 card with a rev 13 802.11 core
@ 2007-11-15  5:55 Larry Finger
  2007-11-15 10:38 ` Michael Buesch
  0 siblings, 1 reply; 5+ messages in thread
From: Larry Finger @ 2007-11-15  5:55 UTC (permalink / raw)
  To: Michael Buesch; +Cc: seandarcy2, Bcm43xx-dev, linux-wireless

This patch file will enable the usage of the b43 driver with the
BCM94311MCG wlan mini-PCI (rev 02), which has not been supported.
This PCIe card uses 64-bit DMA. Most of the changes were needed
to implement this mode. It has been tested on the x86_64 architecture,
but should work on all platforms. FYI, full 64-bit DMA addressing
is implemented and the driver should work with a full 2^(64) bytes
of RAM. No, I have not tested that feature!

This patch is intended to be applied to the everything branch of
the wireless-2.6 git tree. For it to work, the set of 6 patches to
modify the SPROM handling code of ssb that I recently submitted must
be applied as well.

There is one anomaly with the code. When it initializes the DMA, and
enables interrupts for the first time, a single "PHY TX Error" is
generated. I have code to suppress that error message; however, it was
in the bottom-half interrupt handler, and I'm still trying to fix it
in less critical code.

The commitment text is shown below:

Larry
==================================================================

The BCM94311MCG rev 02 chip has an 802.11 core with revision 13 and
has not been supported until now. The changes include the following:

(1) Adding the 802.11 rev 13 device to the ssb_device_id table to load b43.
(2) Change the descriptor ring buffers allocation to 8K to force alignment
    on 8K boundary.
(3) Added PHY revision 9 to the supported list.
(4) Fixed 64-bit addressing errors.
(5) Removed some magic numbers in the DMA setup.

Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
---

 dma.c  |   39 ++++++++++++++++-----------------------
 dma.h  |    7 +++++++
 main.c |    3 ++-
 3 files changed, 25 insertions(+), 24 deletions(-)

Index: wireless-2.6/drivers/net/wireless/b43/main.c
===================================================================
--- wireless-2.6.orig/drivers/net/wireless/b43/main.c
+++ wireless-2.6/drivers/net/wireless/b43/main.c
@@ -93,6 +93,7 @@ static const struct ssb_device_id b43_ss
 	SSB_DEVICE(SSB_VENDOR_BROADCOM, SSB_DEV_80211, 7),
 	SSB_DEVICE(SSB_VENDOR_BROADCOM, SSB_DEV_80211, 9),
 	SSB_DEVICE(SSB_VENDOR_BROADCOM, SSB_DEV_80211, 10),
+	SSB_DEVICE(SSB_VENDOR_BROADCOM, SSB_DEV_80211, 13),
 	SSB_DEVTABLE_END
 };
 
@@ -3061,7 +3062,7 @@ static int b43_phy_versioning(struct b43
 			unsupported = 1;
 		break;
 	case B43_PHYTYPE_G:
-		if (phy_rev > 8)
+		if (phy_rev > 9)
 			unsupported = 1;
 		break;
 	default:
Index: wireless-2.6/drivers/net/wireless/b43/dma.c
===================================================================
--- wireless-2.6.orig/drivers/net/wireless/b43/dma.c
+++ wireless-2.6/drivers/net/wireless/b43/dma.c
@@ -165,7 +165,7 @@ static void op64_fill_descriptor(struct 
 	addrhi = (((u64) dmaaddr >> 32) & ~SSB_DMA_TRANSLATION_MASK);
 	addrext = (((u64) dmaaddr >> 32) & SSB_DMA_TRANSLATION_MASK)
 	    >> SSB_DMA_TRANSLATION_SHIFT;
-	addrhi |= ssb_dma_translation(ring->dev->dev);
+	addrhi |= (ssb_dma_translation(ring->dev->dev) << 1);
 	if (slot == ring->nr_slots - 1)
 		ctl0 |= B43_DMA64_DCTL0_DTABLEEND;
 	if (start)
@@ -426,14 +426,15 @@ static inline
 static int alloc_ringmemory(struct b43_dmaring *ring)
 {
 	struct device *dev = ring->dev->dev->dev;
+	int size = (ring->dma64) ? 8192 : B43_DMA_RINGMEMSIZE;
 
-	ring->descbase = dma_alloc_coherent(dev, B43_DMA_RINGMEMSIZE,
+	ring->descbase = dma_alloc_coherent(dev, size,
 					    &(ring->dmabase), GFP_KERNEL);
 	if (!ring->descbase) {
 		b43err(ring->dev->wl, "DMA ringmemory allocation failed\n");
 		return -ENOMEM;
 	}
-	memset(ring->descbase, 0, B43_DMA_RINGMEMSIZE);
+	memset(ring->descbase, 0, size);
 
 	return 0;
 }
@@ -483,7 +484,7 @@ int b43_dmacontroller_rx_reset(struct b4
 	return 0;
 }
 
-/* Reset the RX DMA channel */
+/* Reset the TX DMA channel */
 int b43_dmacontroller_tx_reset(struct b43_wldev *dev, u16 mmio_base, int dma64)
 {
 	int i;
@@ -636,18 +637,13 @@ static int dmacontroller_setup(struct b4
 		if (ring->dma64) {
 			u64 ringbase = (u64) (ring->dmabase);
 
-			addrext = ((ringbase >> 32) & SSB_DMA_TRANSLATION_MASK)
-			    >> SSB_DMA_TRANSLATION_SHIFT;
-			value = B43_DMA64_TXENABLE;
-			value |= (addrext << B43_DMA64_TXADDREXT_SHIFT)
-			    & B43_DMA64_TXADDREXT_MASK;
-			b43_dma_write(ring, B43_DMA64_TXCTL, value);
+			b43_dma_write(ring, B43_DMA64_TXCTL,
+				      B43_DMA64_TXENABLE);
 			b43_dma_write(ring, B43_DMA64_TXRINGLO,
 				      (ringbase & 0xFFFFFFFF));
 			b43_dma_write(ring, B43_DMA64_TXRINGHI,
 				      ((ringbase >> 32) &
-				       ~SSB_DMA_TRANSLATION_MASK)
-				      | trans);
+				       0xFFFFFFFF));
 		} else {
 			u32 ringbase = (u32) (ring->dmabase);
 
@@ -668,20 +664,16 @@ static int dmacontroller_setup(struct b4
 		if (ring->dma64) {
 			u64 ringbase = (u64) (ring->dmabase);
 
-			addrext = ((ringbase >> 32) & SSB_DMA_TRANSLATION_MASK)
-			    >> SSB_DMA_TRANSLATION_SHIFT;
-			value = (ring->frameoffset << B43_DMA64_RXFROFF_SHIFT);
-			value |= B43_DMA64_RXENABLE;
-			value |= (addrext << B43_DMA64_RXADDREXT_SHIFT)
-			    & B43_DMA64_RXADDREXT_MASK;
+			value = (ring->frameoffset << B43_DMA64_RXFROFF_SHIFT)
+				| B43_DMA64_RXENABLE;
 			b43_dma_write(ring, B43_DMA64_RXCTL, value);
 			b43_dma_write(ring, B43_DMA64_RXRINGLO,
 				      (ringbase & 0xFFFFFFFF));
 			b43_dma_write(ring, B43_DMA64_RXRINGHI,
 				      ((ringbase >> 32) &
-				       ~SSB_DMA_TRANSLATION_MASK)
-				      | trans);
-			b43_dma_write(ring, B43_DMA64_RXINDEX, 200);
+				       0xFFFFFFFF));
+			b43_dma_write(ring, B43_DMA64_RXINDEX, ring->nr_slots *
+				      sizeof(struct b43_dmadesc64));
 		} else {
 			u32 ringbase = (u32) (ring->dmabase);
 
@@ -695,11 +687,12 @@ static int dmacontroller_setup(struct b4
 			b43_dma_write(ring, B43_DMA32_RXRING,
 				      (ringbase & ~SSB_DMA_TRANSLATION_MASK)
 				      | trans);
-			b43_dma_write(ring, B43_DMA32_RXINDEX, 200);
+			b43_dma_write(ring, B43_DMA32_RXINDEX, ring->nr_slots *
+				      sizeof(struct b43_dmadesc32));
 		}
 	}
 
-      out:
+out:
 	return err;
 }
 
Index: wireless-2.6/drivers/net/wireless/b43/dma.h
===================================================================
--- wireless-2.6.orig/drivers/net/wireless/b43/dma.h
+++ wireless-2.6/drivers/net/wireless/b43/dma.h
@@ -260,6 +260,13 @@ static inline u32 b43_dma_read(struct b4
 static inline
     void b43_dma_write(struct b43_dmaring *ring, u16 offset, u32 value)
 {
+	/* temporary debugging code */
+	if (((offset == 8) || (offset == 0x28)) && ring->dma64 &&
+	     ((value & 0x1FFF) != 0)) {
+		printk(KERN_ERR "b43: bad desc ring address for 64-bit DMA"
+		       " - offset, value: 0x%.2X 0x%.4X\n", offset, value);
+		dump_stack();
+	}
 	b43_write32(ring->dev, ring->mmio_base + offset, value);
 }
 

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

* Re: [RFC/T] b43: Implement the BCM94311MCG rev 02 card with a rev 13 802.11 core
  2007-11-15  5:55 [RFC/T] b43: Implement the BCM94311MCG rev 02 card with a rev 13 802.11 core Larry Finger
@ 2007-11-15 10:38 ` Michael Buesch
  2007-11-15 15:02   ` Larry Finger
  0 siblings, 1 reply; 5+ messages in thread
From: Michael Buesch @ 2007-11-15 10:38 UTC (permalink / raw)
  To: Larry Finger; +Cc: seandarcy2, Bcm43xx-dev, linux-wireless

On Thursday 15 November 2007 06:55:42 Larry Finger wrote:
> This patch file will enable the usage of the b43 driver with the
> BCM94311MCG wlan mini-PCI (rev 02), which has not been supported.
> This PCIe card uses 64-bit DMA. Most of the changes were needed
> to implement this mode. It has been tested on the x86_64 architecture,
> but should work on all platforms. FYI, full 64-bit DMA addressing
> is implemented and the driver should work with a full 2^(64) bytes
> of RAM. No, I have not tested that feature!
> 
> This patch is intended to be applied to the everything branch of
> the wireless-2.6 git tree. For it to work, the set of 6 patches to
> modify the SPROM handling code of ssb that I recently submitted must
> be applied as well.
> 
> There is one anomaly with the code. When it initializes the DMA, and
> enables interrupts for the first time, a single "PHY TX Error" is
> generated. I have code to suppress that error message; however, it was
> in the bottom-half interrupt handler, and I'm still trying to fix it
> in less critical code.
> 
> The commitment text is shown below:

> Index: wireless-2.6/drivers/net/wireless/b43/dma.c
> ===================================================================
> --- wireless-2.6.orig/drivers/net/wireless/b43/dma.c
> +++ wireless-2.6/drivers/net/wireless/b43/dma.c
> @@ -165,7 +165,7 @@ static void op64_fill_descriptor(struct 
>  	addrhi = (((u64) dmaaddr >> 32) & ~SSB_DMA_TRANSLATION_MASK);
>  	addrext = (((u64) dmaaddr >> 32) & SSB_DMA_TRANSLATION_MASK)
>  	    >> SSB_DMA_TRANSLATION_SHIFT;
> -	addrhi |= ssb_dma_translation(ring->dev->dev);
> +	addrhi |= (ssb_dma_translation(ring->dev->dev) << 1);
>  	if (slot == ring->nr_slots - 1)
>  		ctl0 |= B43_DMA64_DCTL0_DTABLEEND;
>  	if (start)

Wait, this looks broken to me. It looks like the bug is more
inside of the ssb_dma_translation function, rather than here.

> @@ -426,14 +426,15 @@ static inline
>  static int alloc_ringmemory(struct b43_dmaring *ring)
>  {
>  	struct device *dev = ring->dev->dev->dev;
> +	int size = (ring->dma64) ? 8192 : B43_DMA_RINGMEMSIZE;

Uhm, a page is also 4k in x86_64?
Why doesn't using a page here work? What does happen?

> -	ring->descbase = dma_alloc_coherent(dev, B43_DMA_RINGMEMSIZE,
> +	ring->descbase = dma_alloc_coherent(dev, size,
>  					    &(ring->dmabase), GFP_KERNEL);
>  	if (!ring->descbase) {
>  		b43err(ring->dev->wl, "DMA ringmemory allocation failed\n");
>  		return -ENOMEM;
>  	}
> -	memset(ring->descbase, 0, B43_DMA_RINGMEMSIZE);
> +	memset(ring->descbase, 0, size);
>  
>  	return 0;
>  }
> @@ -483,7 +484,7 @@ int b43_dmacontroller_rx_reset(struct b4
>  	return 0;
>  }
>  
> -/* Reset the RX DMA channel */
> +/* Reset the TX DMA channel */
>  int b43_dmacontroller_tx_reset(struct b43_wldev *dev, u16 mmio_base, int dma64)
>  {
>  	int i;
> @@ -636,18 +637,13 @@ static int dmacontroller_setup(struct b4
>  		if (ring->dma64) {
>  			u64 ringbase = (u64) (ring->dmabase);
>  
> -			addrext = ((ringbase >> 32) & SSB_DMA_TRANSLATION_MASK)
> -			    >> SSB_DMA_TRANSLATION_SHIFT;
> -			value = B43_DMA64_TXENABLE;
> -			value |= (addrext << B43_DMA64_TXADDREXT_SHIFT)
> -			    & B43_DMA64_TXADDREXT_MASK;
> -			b43_dma_write(ring, B43_DMA64_TXCTL, value);
> +			b43_dma_write(ring, B43_DMA64_TXCTL,
> +				      B43_DMA64_TXENABLE);

Ehm, why are you removing this?

>  			b43_dma_write(ring, B43_DMA64_TXRINGLO,
>  				      (ringbase & 0xFFFFFFFF));
>  			b43_dma_write(ring, B43_DMA64_TXRINGHI,
>  				      ((ringbase >> 32) &
> -				       ~SSB_DMA_TRANSLATION_MASK)
> -				      | trans);
> +				       0xFFFFFFFF));

Huh?

>  		} else {
>  			u32 ringbase = (u32) (ring->dmabase);
>  
> @@ -668,20 +664,16 @@ static int dmacontroller_setup(struct b4
>  		if (ring->dma64) {
>  			u64 ringbase = (u64) (ring->dmabase);
>  
> -			addrext = ((ringbase >> 32) & SSB_DMA_TRANSLATION_MASK)
> -			    >> SSB_DMA_TRANSLATION_SHIFT;
> -			value = (ring->frameoffset << B43_DMA64_RXFROFF_SHIFT);
> -			value |= B43_DMA64_RXENABLE;
> -			value |= (addrext << B43_DMA64_RXADDREXT_SHIFT)
> -			    & B43_DMA64_RXADDREXT_MASK;
> +			value = (ring->frameoffset << B43_DMA64_RXFROFF_SHIFT)
> +				| B43_DMA64_RXENABLE;
>  			b43_dma_write(ring, B43_DMA64_RXCTL, value);
>  			b43_dma_write(ring, B43_DMA64_RXRINGLO,
>  				      (ringbase & 0xFFFFFFFF));
>  			b43_dma_write(ring, B43_DMA64_RXRINGHI,
>  				      ((ringbase >> 32) &
> -				       ~SSB_DMA_TRANSLATION_MASK)
> -				      | trans);
> -			b43_dma_write(ring, B43_DMA64_RXINDEX, 200);
> +				       0xFFFFFFFF));
> +			b43_dma_write(ring, B43_DMA64_RXINDEX, ring->nr_slots *
> +				      sizeof(struct b43_dmadesc64));

Same here.

>  		} else {
>  			u32 ringbase = (u32) (ring->dmabase);
>  
> @@ -695,11 +687,12 @@ static int dmacontroller_setup(struct b4
>  			b43_dma_write(ring, B43_DMA32_RXRING,
>  				      (ringbase & ~SSB_DMA_TRANSLATION_MASK)
>  				      | trans);
> -			b43_dma_write(ring, B43_DMA32_RXINDEX, 200);
> +			b43_dma_write(ring, B43_DMA32_RXINDEX, ring->nr_slots *
> +				      sizeof(struct b43_dmadesc32));

I'm not sure why you do this change.

>  		}
>  	}
>  
> -      out:
> +out:
>  	return err;
>  }
>  
> Index: wireless-2.6/drivers/net/wireless/b43/dma.h
> ===================================================================
> --- wireless-2.6.orig/drivers/net/wireless/b43/dma.h
> +++ wireless-2.6/drivers/net/wireless/b43/dma.h
> @@ -260,6 +260,13 @@ static inline u32 b43_dma_read(struct b4
>  static inline
>      void b43_dma_write(struct b43_dmaring *ring, u16 offset, u32 value)
>  {
> +	/* temporary debugging code */
> +	if (((offset == 8) || (offset == 0x28)) && ring->dma64 &&
> +	     ((value & 0x1FFF) != 0)) {
> +		printk(KERN_ERR "b43: bad desc ring address for 64-bit DMA"
> +		       " - offset, value: 0x%.2X 0x%.4X\n", offset, value);
> +		dump_stack();
> +	}

Ok, temporary. You know what that means :)

>  	b43_write32(ring->dev, ring->mmio_base + offset, value);
>  }
>  
> 
> 



-- 
Greetings Michael.

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

* Re: [RFC/T] b43: Implement the BCM94311MCG rev 02 card with a rev 13 802.11 core
  2007-11-15 10:38 ` Michael Buesch
@ 2007-11-15 15:02   ` Larry Finger
  2007-11-15 21:49     ` Michael Buesch
  0 siblings, 1 reply; 5+ messages in thread
From: Larry Finger @ 2007-11-15 15:02 UTC (permalink / raw)
  To: Michael Buesch; +Cc: seandarcy2, Bcm43xx-dev, linux-wireless

Michael Buesch wrote:
> On Thursday 15 November 2007 06:55:42 Larry Finger wrote:
>> --- wireless-2.6.orig/drivers/net/wireless/b43/dma.c
>> +++ wireless-2.6/drivers/net/wireless/b43/dma.c
>> @@ -165,7 +165,7 @@ static void op64_fill_descriptor(struct 
>>  	addrhi = (((u64) dmaaddr >> 32) & ~SSB_DMA_TRANSLATION_MASK);
>>  	addrext = (((u64) dmaaddr >> 32) & SSB_DMA_TRANSLATION_MASK)
>>  	    >> SSB_DMA_TRANSLATION_SHIFT;
>> -	addrhi |= ssb_dma_translation(ring->dev->dev);
>> +	addrhi |= (ssb_dma_translation(ring->dev->dev) << 1);
>>  	if (slot == ring->nr_slots - 1)
>>  		ctl0 |= B43_DMA64_DCTL0_DTABLEEND;
>>  	if (start)
> 
> Wait, this looks broken to me. It looks like the bug is more
> inside of the ssb_dma_translation function, rather than here.

Yes and no. In the DMA specs (http://bcm-v4.sipsolutions.net/802.11/DMA), a 2-bit "routing code" is
used for the address portion of a descriptor. For 32-bit, a value of 1 indicates "Client Mode
Translation". With a 64-bit device, a value of 2 is the valid routing code and 1 leads to DMA
Errors. It seemed easier to take a 0/1 output from ssb_dma_translation and shift it for the 64-bit
case than to teach ssb to distinguish between 32- and 64-bit DMA. BTW, we do have one other option -
we could ignore the routing and address extension bits and _limit_ the user to a maximum of 0.5
Zettabytes! What is the probability that any machine will exceed that amount of memory within our
lifetimes? But then, I never expected to see Terrabyte disk drives in a 3.5 inch package!! Have you
noted that Broadcom uses a 2-bit routing code _AND_ a 2-bit address extension? Why they didn't use a
flat 64-bit address is a mystery.

I can, however, go either way - your drivers and your call.

>> @@ -426,14 +426,15 @@ static inline
>>  static int alloc_ringmemory(struct b43_dmaring *ring)
>>  {
>>  	struct device *dev = ring->dev->dev->dev;
>> +	int size = (ring->dma64) ? 8192 : B43_DMA_RINGMEMSIZE;
> 
> Uhm, a page is also 4k in x86_64?
> Why doesn't using a page here work? What does happen?

You get random 4K/8K alignment and the driver fails with a Fatal DMA error for those with 4K alignment.

>> @@ -636,18 +637,13 @@ static int dmacontroller_setup(struct b4
>>  		if (ring->dma64) {
>>  			u64 ringbase = (u64) (ring->dmabase);
>>  
>> -			addrext = ((ringbase >> 32) & SSB_DMA_TRANSLATION_MASK)
>> -			    >> SSB_DMA_TRANSLATION_SHIFT;
>> -			value = B43_DMA64_TXENABLE;
>> -			value |= (addrext << B43_DMA64_TXADDREXT_SHIFT)
>> -			    & B43_DMA64_TXADDREXT_MASK;
>> -			b43_dma_write(ring, B43_DMA64_TXCTL, value);
>> +			b43_dma_write(ring, B43_DMA64_TXCTL,
>> +				      B43_DMA64_TXENABLE);
> 
> Ehm, why are you removing this?

Unlike 32-bit DMA and the 64-bit descriptors, the 64-bit case uses two full 32-bit words to store
the Descriptor Ring Address. No fancy packing of the upper bits into the control word are needed.

>>  			b43_dma_write(ring, B43_DMA64_TXRINGLO,
>>  				      (ringbase & 0xFFFFFFFF));
>>  			b43_dma_write(ring, B43_DMA64_TXRINGHI,
>>  				      ((ringbase >> 32) &
>> -				       ~SSB_DMA_TRANSLATION_MASK)
>> -				      | trans);
>> +				       0xFFFFFFFF));
> 
> Huh?

This mask is not needed. It is a carryover from when I was having a problem. This will become
b43_dma_write(ring, B43_DMA64_TXRINGHI,(ringbase >> 32)); in the next version.

>>  		} else {
>>  			u32 ringbase = (u32) (ring->dmabase);
>>  
>> @@ -668,20 +664,16 @@ static int dmacontroller_setup(struct b4
>>  		if (ring->dma64) {
>>  			u64 ringbase = (u64) (ring->dmabase);
>>  
>> -			addrext = ((ringbase >> 32) & SSB_DMA_TRANSLATION_MASK)
>> -			    >> SSB_DMA_TRANSLATION_SHIFT;
>> -			value = (ring->frameoffset << B43_DMA64_RXFROFF_SHIFT);
>> -			value |= B43_DMA64_RXENABLE;
>> -			value |= (addrext << B43_DMA64_RXADDREXT_SHIFT)
>> -			    & B43_DMA64_RXADDREXT_MASK;
>> +			value = (ring->frameoffset << B43_DMA64_RXFROFF_SHIFT)
>> +				| B43_DMA64_RXENABLE;
>>  			b43_dma_write(ring, B43_DMA64_RXCTL, value);
>>  			b43_dma_write(ring, B43_DMA64_RXRINGLO,
>>  				      (ringbase & 0xFFFFFFFF));
>>  			b43_dma_write(ring, B43_DMA64_RXRINGHI,
>>  				      ((ringbase >> 32) &
>> -				       ~SSB_DMA_TRANSLATION_MASK)
>> -				      | trans);
>> -			b43_dma_write(ring, B43_DMA64_RXINDEX, 200);
>> +				       0xFFFFFFFF));
>> +			b43_dma_write(ring, B43_DMA64_RXINDEX, ring->nr_slots *
>> +				      sizeof(struct b43_dmadesc64));
> 
> Same here.

Same change for the RX Descriptor Ring Address.

>> @@ -695,11 +687,12 @@ static int dmacontroller_setup(struct b4
>>  			b43_dma_write(ring, B43_DMA32_RXRING,
>>  				      (ringbase & ~SSB_DMA_TRANSLATION_MASK)
>>  				      | trans);
>> -			b43_dma_write(ring, B43_DMA32_RXINDEX, 200);
>> +			b43_dma_write(ring, B43_DMA32_RXINDEX, ring->nr_slots *
>> +				      sizeof(struct b43_dmadesc32));
> 
> I'm not sure why you do this change.

It took me a while to figure out where the magic number of 200 came from, and what I needed for the
64-bit case. In fact I think the 200 is a bug and should be 0x200. To me, this change makes it clearer.

>> Index: wireless-2.6/drivers/net/wireless/b43/dma.h
>> ===================================================================
>> --- wireless-2.6.orig/drivers/net/wireless/b43/dma.h
>> +++ wireless-2.6/drivers/net/wireless/b43/dma.h
>> @@ -260,6 +260,13 @@ static inline u32 b43_dma_read(struct b4
>>  static inline
>>      void b43_dma_write(struct b43_dmaring *ring, u16 offset, u32 value)
>>  {
>> +	/* temporary debugging code */
>> +	if (((offset == 8) || (offset == 0x28)) && ring->dma64 &&
>> +	     ((value & 0x1FFF) != 0)) {
>> +		printk(KERN_ERR "b43: bad desc ring address for 64-bit DMA"
>> +		       " - offset, value: 0x%.2X 0x%.4X\n", offset, value);
>> +		dump_stack();
>> +	}
> 
> Ok, temporary. You know what that means :)

In this case, it disappears before the patch goes to John!

Larry



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

* Re: [RFC/T] b43: Implement the BCM94311MCG rev 02 card with a rev 13 802.11 core
  2007-11-15 15:02   ` Larry Finger
@ 2007-11-15 21:49     ` Michael Buesch
  2007-11-15 23:11       ` Larry Finger
  0 siblings, 1 reply; 5+ messages in thread
From: Michael Buesch @ 2007-11-15 21:49 UTC (permalink / raw)
  To: Larry Finger; +Cc: seandarcy2, Bcm43xx-dev, linux-wireless

On Thursday 15 November 2007 16:02:48 Larry Finger wrote:
> >> @@ -695,11 +687,12 @@ static int dmacontroller_setup(struct b4
> >>  			b43_dma_write(ring, B43_DMA32_RXRING,
> >>  				      (ringbase & ~SSB_DMA_TRANSLATION_MASK)
> >>  				      | trans);
> >> -			b43_dma_write(ring, B43_DMA32_RXINDEX, 200);
> >> +			b43_dma_write(ring, B43_DMA32_RXINDEX, ring->nr_slots *
> >> +				      sizeof(struct b43_dmadesc32));
> > 
> > I'm not sure why you do this change.
> 
> It took me a while to figure out where the magic number of 200 came from, and what I needed for the
> 64-bit case. In fact I think the 200 is a bug and should be 0x200. To me, this change makes it clearer.

The 200 is just a random number.
I think we don't really care what the value is. (Except zero, which doesn't
work on some devices).

-- 
Greetings Michael.

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

* Re: [RFC/T] b43: Implement the BCM94311MCG rev 02 card with a rev 13 802.11 core
  2007-11-15 21:49     ` Michael Buesch
@ 2007-11-15 23:11       ` Larry Finger
  0 siblings, 0 replies; 5+ messages in thread
From: Larry Finger @ 2007-11-15 23:11 UTC (permalink / raw)
  To: Michael Buesch; +Cc: seandarcy2, Bcm43xx-dev, linux-wireless

Michael Buesch wrote:
> On Thursday 15 November 2007 16:02:48 Larry Finger wrote:
>> It took me a while to figure out where the magic number of 200 came from, and what I needed for the
>> 64-bit case. In fact I think the 200 is a bug and should be 0x200. To me, this change makes it clearer.
> 
> The 200 is just a random number.
> I think we don't really care what the value is. (Except zero, which doesn't
> work on some devices).

I think the specs are missing one word and that this quantity should be called "Descriptor Stop
Index" for RX. My understanding is that the RX process uses the descriptors in order until this
particular value is reached. At this point, it restarts at the beginning making a ring buffer. As we
have just allocated 64 slots, it didn't quite seem right to use only the first 25 of them, which is
what the 200 accomplishes. Of course, 25 DMA RX buffers are quite likely to be more than enough. We
might reconsider cutting the allocation number and not take so much DMAable memory. With 64-bit DMA,
all memory is suitable, but not so for the 30-bit cards.

Larry



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

end of thread, other threads:[~2007-11-15 23:11 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-11-15  5:55 [RFC/T] b43: Implement the BCM94311MCG rev 02 card with a rev 13 802.11 core Larry Finger
2007-11-15 10:38 ` Michael Buesch
2007-11-15 15:02   ` Larry Finger
2007-11-15 21:49     ` Michael Buesch
2007-11-15 23:11       ` Larry Finger

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