No DMA RX on some BCM4321, on BCM43224 and BCM43225

No DMA RX on some BCM4321, on BCM43224 and BCM43225

[Rafał Miłecki]

When using DMA on one of my BCM4321s and BCM43224 or BCM43225 I don't
get a single RX IRQ.

Doing scanning requests sends TX packets to the hardware, and the
ucode generates IRQs for confirming transmitted packets. RX does not
seem to work however. I expected to see "native" beacons, or response
to the scanning request.

Do you have idea what may be wrong, where should I look at? So far
I've discovered (from looking at MMIO dumps) that:
1) wl uses 38 as B43_DMA0_RX_FRAMEOFFSET (instead of our 30)
2) wl enables channel with 0x801, not just 0x1 (specs say 0x800 is
Parity Disable - Not on all units)

I've implemented both changes in b43, it didn't help.

-- 
Rafa?

No DMA RX on some BCM4321, on BCM43224 and BCM43225

[Larry Finger]

On 07/18/2011 03:14 PM, Rafa? Mi?ecki wrote:
> When using DMA on one of my BCM4321s and BCM43224 or BCM43225 I don't
> get a single RX IRQ.
>
> Doing scanning requests sends TX packets to the hardware, and the
> ucode generates IRQs for confirming transmitted packets. RX does not
> seem to work however. I expected to see "native" beacons, or response
> to the scanning request.
>
> Do you have idea what may be wrong, where should I look at? So far
> I've discovered (from looking at MMIO dumps) that:
> 1) wl uses 38 as B43_DMA0_RX_FRAMEOFFSET (instead of our 30)
> 2) wl enables channel with 0x801, not just 0x1 (specs say 0x800 is
> Parity Disable - Not on all units)
>
> I've implemented both changes in b43, it didn't help.

Does your MMIO dump from wl show the setup of the RX ring and the RX 
descriptors? There might be something special there.

What are the differences between the BCM4321 that works, and the one that 
doesn't? That might provide a clue for me to check in the reference driver.

Larry

No DMA RX on some BCM4321, on BCM43224 and BCM43225

[Rafał Miłecki]

W dniu 18 lipca 2011 22:29 u?ytkownik Larry Finger
<Larry.Finger@lwfinger.net> napisa?:
> On 07/18/2011 03:14 PM, Rafa? Mi?ecki wrote:
>>
>> When using DMA on one of my BCM4321s and BCM43224 or BCM43225 I don't
>> get a single RX IRQ.
>>
>> Doing scanning requests sends TX packets to the hardware, and the
>> ucode generates IRQs for confirming transmitted packets. RX does not
>> seem to work however. I expected to see "native" beacons, or response
>> to the scanning request.
>>
>> Do you have idea what may be wrong, where should I look at? So far
>> I've discovered (from looking at MMIO dumps) that:
>> 1) wl uses 38 as B43_DMA0_RX_FRAMEOFFSET (instead of our 30)
>> 2) wl enables channel with 0x801, not just 0x1 (specs say 0x800 is
>> Parity Disable - Not on all units)
>>
>> I've implemented both changes in b43, it didn't help.
>
> Does your MMIO dump from wl show the setup of the RX ring and the RX
> descriptors? There might be something special there.
>
> What are the differences between the BCM4321 that works, and the one that
> doesn't? That might provide a clue for me to check in the reference driver.

14e4:4329 [no dma :(]
PCI, N / 1

14e4:4328 [dma :)]
PCIe, N / 2

Unfortunately, I haven't took that cards with myself. I can not test
them for now.

-- 
Rafa?

No DMA RX on some BCM4321, on BCM43224 and BCM43225

[Rafał Miłecki]

W dniu 18 lipca 2011 22:29 u?ytkownik Larry Finger
<Larry.Finger@lwfinger.net> napisa?:
> On 07/18/2011 03:14 PM, Rafa? Mi?ecki wrote:
>>
>> When using DMA on one of my BCM4321s and BCM43224 or BCM43225 I don't
>> get a single RX IRQ.
>>
>> Doing scanning requests sends TX packets to the hardware, and the
>> ucode generates IRQs for confirming transmitted packets. RX does not
>> seem to work however. I expected to see "native" beacons, or response
>> to the scanning request.
>>
>> Do you have idea what may be wrong, where should I look at? So far
>> I've discovered (from looking at MMIO dumps) that:
>> 1) wl uses 38 as B43_DMA0_RX_FRAMEOFFSET (instead of our 30)
>> 2) wl enables channel with 0x801, not just 0x1 (specs say 0x800 is
>> Parity Disable - Not on all units)
>>
>> I've implemented both changes in b43, it didn't help.
>
> Does your MMIO dump from wl show the setup of the RX ring and the RX
> descriptors? There might be something special there.
>
> What are the differences between the BCM4321 that works, and the one that
> doesn't? That might provide a clue for me to check in the reference driver.

Larry, for now, can you check:

1) When wl does set parity (0x800) bit
2) Does newer wl always use 38 offset for RX header?

-- 
Rafa?

No DMA RX on some BCM4321, on BCM43224 and BCM43225

[Rafał Miłecki]

W dniu 19 lipca 2011 01:32 u?ytkownik Rafa? Mi?ecki <zajec5@gmail.com> napisa?:
> W dniu 18 lipca 2011 22:29 u?ytkownik Larry Finger
> <Larry.Finger@lwfinger.net> napisa?:
>> On 07/18/2011 03:14 PM, Rafa? Mi?ecki wrote:
>>>
>>> When using DMA on one of my BCM4321s and BCM43224 or BCM43225 I don't
>>> get a single RX IRQ.
>>>
>>> Doing scanning requests sends TX packets to the hardware, and the
>>> ucode generates IRQs for confirming transmitted packets. RX does not
>>> seem to work however. I expected to see "native" beacons, or response
>>> to the scanning request.
>>>
>>> Do you have idea what may be wrong, where should I look at? So far
>>> I've discovered (from looking at MMIO dumps) that:
>>> 1) wl uses 38 as B43_DMA0_RX_FRAMEOFFSET (instead of our 30)
>>> 2) wl enables channel with 0x801, not just 0x1 (specs say 0x800 is
>>> Parity Disable - Not on all units)
>>>
>>> I've implemented both changes in b43, it didn't help.
>>
>> Does your MMIO dump from wl show the setup of the RX ring and the RX
>> descriptors? There might be something special there.
>>
>> What are the differences between the BCM4321 that works, and the one that
>> doesn't? That might provide a clue for me to check in the reference driver.
>
> Larry, for now, can you check:
>
> 1) When wl does set parity (0x800) bit
> 2) Does newer wl always use 38 offset for RX header?

After adding routing (translation) address, using parity bit (0x800)
and cold booting, DMA seems to work =)

-- 
Rafa?

No DMA RX on some BCM4321, on BCM43224 and BCM43225

[Larry Finger]

On 07/19/2011 01:56 AM, Rafa? Mi?ecki wrote:
> W dniu 19 lipca 2011 01:32 u?ytkownik Rafa? Mi?ecki<zajec5@gmail.com>  napisa?:
>> Larry, for now, can you check:
>>
>> 1) When wl does set parity (0x800) bit
>> 2) Does newer wl always use 38 offset for RX header?
>
> After adding routing (translation) address, using parity bit (0x800)
> and cold booting, DMA seems to work =)

Good to hear. I have not found anything about the offset of 38, but I have some 
confusing stuff on the parity enable bit.

One routine in the reference driver does the following:

1. If bit 1 is set in the DMA control flags (this bit was set to 1 in the init)
  a. Read DMA control
  a. OR result with 0x800 and write to control
  a. Read DMA control
  a. If bit 0x800 is set
   1. Write original contents back to control
  a. Otherwise
   1. Clear bit 1 in DMA control flags

The above routine makes it look as if they have tested whether the device 
implements the parity bit. The funny part is that I cannot find any other place 
where they test the DMA control flags for bit 1 set. There is a spot where they 
unconditionally set bit 0x800 in the DMA control, which seems to match what you see.

I guess it will be OK to unconditionally set bit 0x800. Any device that does not 
implement the parity enable bit will not care, and those that have it work with wl.

Larry

No DMA RX on some BCM4321, on BCM43224 and BCM43225

[Jonas Gorski]

2011/7/19 Larry Finger <Larry.Finger@lwfinger.net>:
> On 07/19/2011 01:56 AM, Rafa? Mi?ecki wrote:
>>
>> W dniu 19 lipca 2011 01:32 u?ytkownik Rafa? Mi?ecki<zajec5@gmail.com>
>> ?napisa?:
>>>
>>> Larry, for now, can you check:
>>>
>>> 1) When wl does set parity (0x800) bit
>>> 2) Does newer wl always use 38 offset for RX header?
>>
>> After adding routing (translation) address, using parity bit (0x800)
>> and cold booting, DMA seems to work =)
>
> Good to hear. I have not found anything about the offset of 38, but I have
> some confusing stuff on the parity enable bit.
>
> One routine in the reference driver does the following:
>
> 1. If bit 1 is set in the DMA control flags (this bit was set to 1 in the
> init)
> ?a. Read DMA control
> ?a. OR result with 0x800 and write to control
> ?a. Read DMA control
> ?a. If bit 0x800 is set
> ?1. Write original contents back to control
> ?a. Otherwise
> ?1. Clear bit 1 in DMA control flags
>
> The above routine makes it look as if they have tested whether the device
> implements the parity bit. The funny part is that I cannot find any other
> place where they test the DMA control flags for bit 1 set. There is a spot
> where they unconditionally set bit 0x800 in the DMA control, which seems to
> match what you see.

The brcm80211 sources agree:
<http://git.kernel.org/?p=linux/kernel/git/gregkh/staging-2.6.git;a=blob;f=drivers/staging/brcm80211/brcmsmac/dma.c;h=ea17671efb63aa0c9556c8e46c842734ba398f86;hb=refs/heads/staging-next#l1147>


Regards,
Jonas

No DMA RX on some BCM4321, on BCM43224 and BCM43225

[Michael Büsch]

On Mon, 18 Jul 2011 22:14:50 +0200
Rafa? Mi?ecki <zajec5@gmail.com> wrote:

> When using DMA on one of my BCM4321s and BCM43224 or BCM43225 I don't
> get a single RX IRQ.
> 
> Doing scanning requests sends TX packets to the hardware, and the
> ucode generates IRQs for confirming transmitted packets. RX does not
> seem to work however. I expected to see "native" beacons, or response
> to the scanning request.
> 
> Do you have idea what may be wrong, where should I look at? 

Is wl still using the same dma engine for 
RX as older versions did? (what was it? the first?)

> I've discovered (from looking at MMIO dumps) that:
> 1) wl uses 38 as B43_DMA0_RX_FRAMEOFFSET (instead of our 30)

That could probably mean that the rx header format changed. or it's just an alignment fix of some sort. Not sure...

No DMA RX on some BCM4321, on BCM43224 and BCM43225

[Rafał Miłecki]

W dniu 18 lipca 2011 22:41 u?ytkownik Michael B?sch <m@bues.ch> napisa?:
> On Mon, 18 Jul 2011 22:14:50 +0200
> Rafa? Mi?ecki <zajec5@gmail.com> wrote:
>
>> When using DMA on one of my BCM4321s and BCM43224 or BCM43225 I don't
>> get a single RX IRQ.
>>
>> Doing scanning requests sends TX packets to the hardware, and the
>> ucode generates IRQs for confirming transmitted packets. RX does not
>> seem to work however. I expected to see "native" beacons, or response
>> to the scanning request.
>>
>> Do you have idea what may be wrong, where should I look at?
>
> Is wl still using the same dma engine for
> RX as older versions did? (what was it? the first?)

Yes. That's what wl does:

1) Disabling DMA engines (RX part)
write32 0xfaafc220 <- 0x00000000
 read32 0xfaafc230 -> 0x00000000

2) Enabling DMA engines (RX part)
 read32 0xfaafc220 -> 0x00000040
write32 0xfaafc220 <- 0x0000084d CTL: [38 << 1 (OFFSET) | 0x1 (EN) |
0x800 (PARITY)]
write32 0xfaafc228 <- 0x18948000 LO: [lo addr & ~routing_bits]
write32 0xfaafc22c <- 0x80000000 HI: [routing & ~ROUT_MASK | routing]

(improved) b43 does:

write32 0xfaafc220 <- 0x0000084d CTL
write32 0xfaafc228 <- 0x17b70000 LO
write32 0xfaafc22c <- 0x80000000 HI
write32 0xfaafc224 <- 0x00000400 STOP IDX

-- 
Rafa?

No DMA RX on some BCM4321, on BCM43224 and BCM43225

[Michael Büsch]

On Tue, 19 Jul 2011 00:39:10 +0200
Rafa? Mi?ecki <zajec5@gmail.com> wrote:

> W dniu 18 lipca 2011 22:41 u?ytkownik Michael B?sch <m@bues.ch> napisa?:
> > On Mon, 18 Jul 2011 22:14:50 +0200
> > Rafa? Mi?ecki <zajec5@gmail.com> wrote:
> >
> >> When using DMA on one of my BCM4321s and BCM43224 or BCM43225 I don't
> >> get a single RX IRQ.
> >>
> >> Doing scanning requests sends TX packets to the hardware, and the
> >> ucode generates IRQs for confirming transmitted packets. RX does not
> >> seem to work however. I expected to see "native" beacons, or response
> >> to the scanning request.
> >>
> >> Do you have idea what may be wrong, where should I look at?
> >
> > Is wl still using the same dma engine for
> > RX as older versions did? (what was it? the first?)
> 
> Yes. That's what wl does:
> 
> 1) Disabling DMA engines (RX part)
> write32 0xfaafc220 <- 0x00000000
>  read32 0xfaafc230 -> 0x00000000
> 
> 2) Enabling DMA engines (RX part)
>  read32 0xfaafc220 -> 0x00000040
> write32 0xfaafc220 <- 0x0000084d CTL: [38 << 1 (OFFSET) | 0x1 (EN) |
> 0x800 (PARITY)]
> write32 0xfaafc228 <- 0x18948000 LO: [lo addr & ~routing_bits]
> write32 0xfaafc22c <- 0x80000000 HI: [routing & ~ROUT_MASK | routing]
> 
> (improved) b43 does:
> 
> write32 0xfaafc220 <- 0x0000084d CTL
> write32 0xfaafc228 <- 0x17b70000 LO
> write32 0xfaafc22c <- 0x80000000 HI
> write32 0xfaafc224 <- 0x00000400 STOP IDX

What about the DMA interrupt masks?

No DMA RX on some BCM4321, on BCM43224 and BCM43225

[Rafał Miłecki]

W dniu 19 lipca 2011 00:59 u?ytkownik Michael B?sch <m@bues.ch> napisa?:
> On Tue, 19 Jul 2011 00:39:10 +0200
> Rafa? Mi?ecki <zajec5@gmail.com> wrote:
>
>> W dniu 18 lipca 2011 22:41 u?ytkownik Michael B?sch <m@bues.ch> napisa?:
>> > On Mon, 18 Jul 2011 22:14:50 +0200
>> > Rafa? Mi?ecki <zajec5@gmail.com> wrote:
>> >
>> >> When using DMA on one of my BCM4321s and BCM43224 or BCM43225 I don't
>> >> get a single RX IRQ.
>> >>
>> >> Doing scanning requests sends TX packets to the hardware, and the
>> >> ucode generates IRQs for confirming transmitted packets. RX does not
>> >> seem to work however. I expected to see "native" beacons, or response
>> >> to the scanning request.
>> >>
>> >> Do you have idea what may be wrong, where should I look at?
>> >
>> > Is wl still using the same dma engine for
>> > RX as older versions did? (what was it? the first?)
>>
>> Yes. That's what wl does:
>>
>> 1) Disabling DMA engines (RX part)
>> write32 0xfaafc220 <- 0x00000000
>> ?read32 0xfaafc230 -> 0x00000000
>>
>> 2) Enabling DMA engines (RX part)
>> ?read32 0xfaafc220 -> 0x00000040
>> write32 0xfaafc220 <- 0x0000084d CTL: [38 << 1 (OFFSET) | 0x1 (EN) |
>> 0x800 (PARITY)]
>> write32 0xfaafc228 <- 0x18948000 LO: [lo addr & ~routing_bits]
>> write32 0xfaafc22c <- 0x80000000 HI: [routing & ~ROUT_MASK | routing]
>>
>> (improved) b43 does:
>>
>> write32 0xfaafc220 <- 0x0000084d CTL
>> write32 0xfaafc228 <- 0x17b70000 LO
>> write32 0xfaafc22c <- 0x80000000 HI
>> write32 0xfaafc224 <- 0x00000400 STOP IDX
>
> What about the DMA interrupt masks?

Well, in b43 we ask for all IRQs:
b43_write32(dev, B43_MMIO_GEN_IRQ_REASON, B43_IRQ_ALL);
write32 0xfaafc128 <- 0xffffffff

wl also does:
write32 0xfaafc128 <- 0xffffffff

(All the dumps I provide in this thread come from BCM43224)

-- 
Rafa?

No DMA RX on some BCM4321, on BCM43224 and BCM43225

[Michael Büsch]

On Tue, 19 Jul 2011 01:07:20 +0200
Rafa? Mi?ecki <zajec5@gmail.com> wrote:

> W dniu 19 lipca 2011 00:59 u?ytkownik Michael B?sch <m@bues.ch> napisa?:
> > On Tue, 19 Jul 2011 00:39:10 +0200
> > Rafa? Mi?ecki <zajec5@gmail.com> wrote:
> >
> >> W dniu 18 lipca 2011 22:41 u?ytkownik Michael B?sch <m@bues.ch> napisa?:
> >> > On Mon, 18 Jul 2011 22:14:50 +0200
> >> > Rafa? Mi?ecki <zajec5@gmail.com> wrote:
> >> >
> >> >> When using DMA on one of my BCM4321s and BCM43224 or BCM43225 I don't
> >> >> get a single RX IRQ.
> >> >>
> >> >> Doing scanning requests sends TX packets to the hardware, and the
> >> >> ucode generates IRQs for confirming transmitted packets. RX does not
> >> >> seem to work however. I expected to see "native" beacons, or response
> >> >> to the scanning request.
> >> >>
> >> >> Do you have idea what may be wrong, where should I look at?
> >> >
> >> > Is wl still using the same dma engine for
> >> > RX as older versions did? (what was it? the first?)
> >>
> >> Yes. That's what wl does:
> >>
> >> 1) Disabling DMA engines (RX part)
> >> write32 0xfaafc220 <- 0x00000000
> >> ?read32 0xfaafc230 -> 0x00000000
> >>
> >> 2) Enabling DMA engines (RX part)
> >> ?read32 0xfaafc220 -> 0x00000040
> >> write32 0xfaafc220 <- 0x0000084d CTL: [38 << 1 (OFFSET) | 0x1 (EN) |
> >> 0x800 (PARITY)]
> >> write32 0xfaafc228 <- 0x18948000 LO: [lo addr & ~routing_bits]
> >> write32 0xfaafc22c <- 0x80000000 HI: [routing & ~ROUT_MASK | routing]
> >>
> >> (improved) b43 does:
> >>
> >> write32 0xfaafc220 <- 0x0000084d CTL
> >> write32 0xfaafc228 <- 0x17b70000 LO
> >> write32 0xfaafc22c <- 0x80000000 HI
> >> write32 0xfaafc224 <- 0x00000400 STOP IDX
> >
> > What about the DMA interrupt masks?
> 
> Well, in b43 we ask for all IRQs:
> b43_write32(dev, B43_MMIO_GEN_IRQ_REASON, B43_IRQ_ALL);
> write32 0xfaafc128 <- 0xffffffff
> 
> wl also does:
> write32 0xfaafc128 <- 0xffffffff
> 
> (All the dumps I provide in this thread come from BCM43224)
> 

No. The _DMA_ interrupt masks. Each DMA engine has its interrupt mask register.

No DMA RX on some BCM4321, on BCM43224 and BCM43225

[Rafał Miłecki]

W dniu 19 lipca 2011 01:08 u?ytkownik Michael B?sch <m@bues.ch> napisa?:
> On Tue, 19 Jul 2011 01:07:20 +0200
> Rafa? Mi?ecki <zajec5@gmail.com> wrote:
>
>> W dniu 19 lipca 2011 00:59 u?ytkownik Michael B?sch <m@bues.ch> napisa?:
>> > On Tue, 19 Jul 2011 00:39:10 +0200
>> > Rafa? Mi?ecki <zajec5@gmail.com> wrote:
>> >
>> >> W dniu 18 lipca 2011 22:41 u?ytkownik Michael B?sch <m@bues.ch> napisa?:
>> >> > On Mon, 18 Jul 2011 22:14:50 +0200
>> >> > Rafa? Mi?ecki <zajec5@gmail.com> wrote:
>> >> >
>> >> >> When using DMA on one of my BCM4321s and BCM43224 or BCM43225 I don't
>> >> >> get a single RX IRQ.
>> >> >>
>> >> >> Doing scanning requests sends TX packets to the hardware, and the
>> >> >> ucode generates IRQs for confirming transmitted packets. RX does not
>> >> >> seem to work however. I expected to see "native" beacons, or response
>> >> >> to the scanning request.
>> >> >>
>> >> >> Do you have idea what may be wrong, where should I look at?
>> >> >
>> >> > Is wl still using the same dma engine for
>> >> > RX as older versions did? (what was it? the first?)
>> >>
>> >> Yes. That's what wl does:
>> >>
>> >> 1) Disabling DMA engines (RX part)
>> >> write32 0xfaafc220 <- 0x00000000
>> >> ?read32 0xfaafc230 -> 0x00000000
>> >>
>> >> 2) Enabling DMA engines (RX part)
>> >> ?read32 0xfaafc220 -> 0x00000040
>> >> write32 0xfaafc220 <- 0x0000084d CTL: [38 << 1 (OFFSET) | 0x1 (EN) |
>> >> 0x800 (PARITY)]
>> >> write32 0xfaafc228 <- 0x18948000 LO: [lo addr & ~routing_bits]
>> >> write32 0xfaafc22c <- 0x80000000 HI: [routing & ~ROUT_MASK | routing]
>> >>
>> >> (improved) b43 does:
>> >>
>> >> write32 0xfaafc220 <- 0x0000084d CTL
>> >> write32 0xfaafc228 <- 0x17b70000 LO
>> >> write32 0xfaafc22c <- 0x80000000 HI
>> >> write32 0xfaafc224 <- 0x00000400 STOP IDX
>> >
>> > What about the DMA interrupt masks?
>>
>> Well, in b43 we ask for all IRQs:
>> b43_write32(dev, B43_MMIO_GEN_IRQ_REASON, B43_IRQ_ALL);
>> write32 0xfaafc128 <- 0xffffffff
>>
>> wl also does:
>> write32 0xfaafc128 <- 0xffffffff
>>
>> (All the dumps I provide in this thread come from BCM43224)
>>
>
> No. The _DMA_ interrupt masks. Each DMA engine has its interrupt mask register.

You must mean descriptors, sorry, I misunderstood you.

AFAIK descriptors are placed in system memory and pointed to the
hardware. They are not written by MMIO operations.

I still need to create a hack detecting passing descriptor address to
the hardware and dumping content from the pointer.

-- 
Rafa?

No DMA RX on some BCM4321, on BCM43224 and BCM43225

[Michael Büsch]

On Tue, 19 Jul 2011 01:15:27 +0200
Rafa? Mi?ecki <zajec5@gmail.com> wrote:

> > No. The _DMA_ interrupt masks. Each DMA engine has its interrupt mask register.
> 
> You must mean descriptors, sorry, I misunderstood you.

Ehm, no?
I mean the DMA interrupt mask registers:

#define B43_MMIO_DMA0_IRQ_MASK          0x24
#define B43_MMIO_DMA1_IRQ_MASK          0x2C
#define B43_MMIO_DMA2_IRQ_MASK          0x34
#define B43_MMIO_DMA3_IRQ_MASK          0x3C
#define B43_MMIO_DMA4_IRQ_MASK          0x44
#define B43_MMIO_DMA5_IRQ_MASK          0x4C

No DMA RX on some BCM4321, on BCM43224 and BCM43225

[Rafał Miłecki]

W dniu 19 lipca 2011 01:24 u?ytkownik Michael B?sch <m@bues.ch> napisa?:
> On Tue, 19 Jul 2011 01:15:27 +0200
> Rafa? Mi?ecki <zajec5@gmail.com> wrote:
>
>> > No. The _DMA_ interrupt masks. Each DMA engine has its interrupt mask register.
>>
>> You must mean descriptors, sorry, I misunderstood you.
>
> Ehm, no?
> I mean the DMA interrupt mask registers:
>
> #define B43_MMIO_DMA0_IRQ_MASK ? ? ? ? ?0x24
> #define B43_MMIO_DMA1_IRQ_MASK ? ? ? ? ?0x2C
> #define B43_MMIO_DMA2_IRQ_MASK ? ? ? ? ?0x34
> #define B43_MMIO_DMA3_IRQ_MASK ? ? ? ? ?0x3C
> #define B43_MMIO_DMA4_IRQ_MASK ? ? ? ? ?0x44
> #define B43_MMIO_DMA5_IRQ_MASK ? ? ? ? ?0x4C

Whoops, sorry and thanks for your patience.

I'm falling asleep now, will check this later. Unfortunately today
I've quite busy day, that will have to wait probably.

-- 
Rafa?

No DMA RX on some BCM4321, on BCM43224 and BCM43225

[Rafał Miłecki]

W dniu 19 lipca 2011 00:39 u?ytkownik Rafa? Mi?ecki <zajec5@gmail.com> napisa?:
> W dniu 18 lipca 2011 22:41 u?ytkownik Michael B?sch <m@bues.ch> napisa?:
>> On Mon, 18 Jul 2011 22:14:50 +0200
>> Rafa? Mi?ecki <zajec5@gmail.com> wrote:
>>
>>> When using DMA on one of my BCM4321s and BCM43224 or BCM43225 I don't
>>> get a single RX IRQ.
>>>
>>> Doing scanning requests sends TX packets to the hardware, and the
>>> ucode generates IRQs for confirming transmitted packets. RX does not
>>> seem to work however. I expected to see "native" beacons, or response
>>> to the scanning request.
>>>
>>> Do you have idea what may be wrong, where should I look at?
>>
>> Is wl still using the same dma engine for
>> RX as older versions did? (what was it? the first?)
>
> Yes. That's what wl does:
>
> 1) Disabling DMA engines (RX part)
> write32 0xfaafc220 <- 0x00000000
> ?read32 0xfaafc230 -> 0x00000000
>
> 2) Enabling DMA engines (RX part)
> ?read32 0xfaafc220 -> 0x00000040
> write32 0xfaafc220 <- 0x0000084d CTL: [38 << 1 (OFFSET) | 0x1 (EN) |
> 0x800 (PARITY)]
> write32 0xfaafc228 <- 0x18948000 LO: [lo addr & ~routing_bits]
> write32 0xfaafc22c <- 0x80000000 HI: [routing & ~ROUT_MASK | routing]

As you may now, wl does a lot of weird operations. Before it really
starts working, it resets everything 10 times.

For DMA it looks like this:
1) Check if DMA is 30 or 32 bit (by setting and reading back "Address
Extension"). wl does it even when TM State High clearly says 64 bit
DMA is availale (and wl knows it!):
ssb_read32(0xf9c) -> 0x500c0000
write32 0xfaafc200 <- 0x00030000
 read32 0xfaafc200 -> 0x00030000
2) Disable every DMA engine (SUSPEND for TX, zero ctl register for TX)
3) Disable every DMA engine (SUSPEND for TX, zero ctl register for TX) (yeah...)
4) Setup rings (pass routing, addresses, set enable, parity, RX header
length for RX)

Maybe disabling engines before setting them up is needed?

-- 
Rafa?

No DMA RX on some BCM4321, on BCM43224 and BCM43225

[David Woodhouse]

On Mon, 2011-07-18 at 22:14 +0200, Rafa? Mi?ecki wrote:
> When using DMA on one of my BCM4321s and BCM43224 or BCM43225 I don't
> get a single RX IRQ.
> 
> Doing scanning requests sends TX packets to the hardware, and the
> ucode generates IRQs for confirming transmitted packets. RX does not
> seem to work however. I expected to see "native" beacons, or response
> to the scanning request.
> 
> Do you have idea what may be wrong, where should I look at? So far
> I've discovered (from looking at MMIO dumps) that:
> 1) wl uses 38 as B43_DMA0_RX_FRAMEOFFSET (instead of our 30)
> 2) wl enables channel with 0x801, not just 0x1 (specs say 0x800 is
> Parity Disable - Not on all units)
> 
> I've implemented both changes in b43, it didn't help.

The other thing I observed that under ndiswrapper, the Windows driver
(for BCM4331) was writing the full physical address to the TX_INDEX
register, not an index at all.

Setup...
W 4 159.866820 1 0xb06002c8 0x1f86c000 0x0 0
W 4 159.866822 1 0xb06002cc 0x80000000 0x0 0
R 4 159.866827 1 0xb06002c0 0x0 0x0 0
W 4 159.866829 1 0xb06002c0 0x801 0x0 0

... and use:
W 4 197.207151 1 0xb06002c4 0x1f86c010 0x0 0

... and again later:
W 4 197.228596 1 0xb06002c4 0x1f86c020 0x0 0

Is that behaviour understood?

Btw, Rafa?, if you want to publish a git tree on git.infradead.org I can
give you an account there). Do you prefer username 'rafal' or 'zajec'...
or something different?

-- 
dwmw2

No DMA RX on some BCM4321, on BCM43224 and BCM43225

[David Woodhouse]

On Tue, 2011-07-19 at 15:33 -0700, David Woodhouse wrote:
> The other thing I observed that under ndiswrapper, the Windows driver
> (for BCM4331) was writing the full physical address to the TX_INDEX
> register, not an index at all.
> 
> Setup...
> W 4 159.866820 1 0xb06002c8 0x1f86c000 0x0 0
> W 4 159.866822 1 0xb06002cc 0x80000000 0x0 0
> R 4 159.866827 1 0xb06002c0 0x0 0x0 0
> W 4 159.866829 1 0xb06002c0 0x801 0x0 0
> 
> ... and use:
> W 4 197.207151 1 0xb06002c4 0x1f86c010 0x0 0
> 
> ... and again later:
> W 4 197.228596 1 0xb06002c4 0x1f86c020 0x0 0
> 
> Is that behaviour understood?

Having fixed b43 to do the same (and thanks to all of Rafa?'s work), I
now have my BCM4331 working with b43.

Without this change, if the TX descriptor ring was not aligned to 8KiB,
the hardware would get confused when we send the 256th DMA frame and
wrap back to slot zero.

I have to include the higher bits of the address in what I write to the
TXINDEX register, *and* I have to enable the DMA engine by writing to
TXCTL *after* setting the TXRING{HI,LO} registers when setting it up.

The brcmsmac driver seems to do this only if the TXRINGLO register
actually latches the low bits (write 0xff0, read back and see if you get
zero).

I also changed the values of B43_DMA64_[RT]XSTATDPTR to 0xfff, for
similar reasons ? sometimes bit 12 is set when it arguably shouldn't be,
because the hardware works strangely if the table isn't aligned.

-- 
dwmw2

[PATCH] b43: Mask out unwanted bits of RX slot address

[David Woodhouse]

On some 64-bit DMA engines (my BCM4331, at least) with a ring buffer
that is not aligned to 8KiB, we often see bit 12 spuriously being set in
the B43_DMA64_RXSTATUS register. This will happen the first time the
ring fills and loops back to zero:

[35755.186080] Read RXSTATUS 3f0 (makes offs 3f0, divided by 10 gives slot 3f
[35755.288366] Read RXSTATUS 1000 (makes offs 1000, divided by 10 gives slot 100
[35755.291007] ------------[ cut here ]------------
[35755.291041] WARNING: at drivers/net/wireless/b43/dma.c:1700 b43_dma_rx+0x5e/0x2df [b43]()
 ...
[35766.745374] b43-phy41 debug: DMA RX: Dropping poisoned buffer.

All we need to do is mask the offending bit out. The packets *are* where
they should be; it's just the high bit of the reported offset which is
weird.

Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>

diff --git a/drivers/net/wireless/b43/dma.c b/drivers/net/wireless/b43/dma.c
index 82168f8..92dd6d9 100644
--- a/drivers/net/wireless/b43/dma.c
+++ b/drivers/net/wireless/b43/dma.c
@@ -247,6 +249,10 @@ static int op64_get_current_rxslot(struct b43_dmaring *ring)
 
 	val = b43_dma_read(ring, B43_DMA64_RXSTATUS);
 	val &= B43_DMA64_RXSTATDPTR;
+	/* When the ring is not aligned to 8KiB, we sometimes get
+	   bit 12 (0x1000) set in the address. Mask it out; we
+	   only use a 4KiB ring. */
+	val &= B43_DMA_RINGMEMSIZE-1;
 
 	return (val / sizeof(struct b43_dmadesc64));
 }

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse at intel.com                              Intel Corporation

[PATCH] b43: Mask out unwanted bits of RX slot address

[Rafał Miłecki]

W dniu 12 sierpnia 2011 12:15 u?ytkownik David Woodhouse
<dwmw2@infradead.org> napisa?:
> On some 64-bit DMA engines (my BCM4331, at least) with a ring buffer
> that is not aligned to 8KiB, we often see bit 12 spuriously being set in
> the B43_DMA64_RXSTATUS register. This will happen the first time the
> ring fills and loops back to zero:
>
> [35755.186080] Read RXSTATUS 3f0 (makes offs 3f0, divided by 10 gives slot 3f
> [35755.288366] Read RXSTATUS 1000 (makes offs 1000, divided by 10 gives slot 100
> [35755.291007] ------------[ cut here ]------------
> [35755.291041] WARNING: at drivers/net/wireless/b43/dma.c:1700 b43_dma_rx+0x5e/0x2df [b43]()
> ?...
> [35766.745374] b43-phy41 debug: DMA RX: Dropping poisoned buffer.
>
> All we need to do is mask the offending bit out. The packets *are* where
> they should be; it's just the high bit of the reported offset which is
> weird.
>
> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
>
> diff --git a/drivers/net/wireless/b43/dma.c b/drivers/net/wireless/b43/dma.c
> index 82168f8..92dd6d9 100644
> --- a/drivers/net/wireless/b43/dma.c
> +++ b/drivers/net/wireless/b43/dma.c
> @@ -247,6 +249,10 @@ static int op64_get_current_rxslot(struct b43_dmaring *ring)
>
> ? ? ? ?val = b43_dma_read(ring, B43_DMA64_RXSTATUS);
> ? ? ? ?val &= B43_DMA64_RXSTATDPTR;
> + ? ? ? /* When the ring is not aligned to 8KiB, we sometimes get
> + ? ? ? ? ?bit 12 (0x1000) set in the address. Mask it out; we
> + ? ? ? ? ?only use a 4KiB ring. */
> + ? ? ? val &= B43_DMA_RINGMEMSIZE-1;

I still have to understand that B43_DMA_RINGMEMSIZE (it's equal to
PAGE_SIZE, which is calculated in some magic way).

However there should be spaces around minus.

-- 
Rafa?

[PATCH] b43: Mask out unwanted bits of RX slot address

[Michael Büsch]

On Sat, 13 Aug 2011 21:10:34 +0200
Rafa? Mi?ecki <zajec5@gmail.com> wrote:

> I still have to understand that B43_DMA_RINGMEMSIZE (it's equal to
> PAGE_SIZE, which is calculated in some magic way).

Well, I guess there's some kind of legacy involved here.
In the early days we used to allocate the ring memory
with the page allocator. We simply allocated one page.
So the size of the memory would always be equal to PAGE_SIZE.

-- 
Greetings, Michael.

[PATCH] b43: Mask out unwanted bits of RX slot address

[Rafał Miłecki]

W dniu 12 sierpnia 2011 12:15 u?ytkownik David Woodhouse
<dwmw2@infradead.org> napisa?:
> On some 64-bit DMA engines (my BCM4331, at least) with a ring buffer
> that is not aligned to 8KiB, we often see bit 12 spuriously being set in
> the B43_DMA64_RXSTATUS register. This will happen the first time the
> ring fills and loops back to zero:
>
> [35755.186080] Read RXSTATUS 3f0 (makes offs 3f0, divided by 10 gives slot 3f
> [35755.288366] Read RXSTATUS 1000 (makes offs 1000, divided by 10 gives slot 100
> [35755.291007] ------------[ cut here ]------------
> [35755.291041] WARNING: at drivers/net/wireless/b43/dma.c:1700 b43_dma_rx+0x5e/0x2df [b43]()
> ?...
> [35766.745374] b43-phy41 debug: DMA RX: Dropping poisoned buffer.

Interesting... below is the dump from ndiswrapper on BCM4331:

R 4 1431.714843 2 0xb0600230 0x10000fb0 0x0 0
R 4 1431.740035 2 0xb0600230 0x10000fc0 0x0 0
R 4 1431.740050 2 0xb0600230 0x10000fc0 0x0 0
R 4 1431.850216 2 0xb0600230 0x10000fd0 0x0 0
R 4 1431.850229 2 0xb0600230 0x10000fd0 0x0 0
R 4 1431.882151 2 0xb0600230 0x10000fe0 0x0 0
R 4 1431.882164 2 0xb0600230 0x10000fe0 0x0 0
R 4 1431.885822 2 0xb0600230 0x10000ff0 0x0 0
R 4 1431.885835 2 0xb0600230 0x10000ff0 0x0 0
R 4 1431.896775 2 0xb0600230 0x10000000 0x0 0
R 4 1431.896790 2 0xb0600230 0x10000000 0x0 0
R 4 1431.915842 2 0xb0600230 0x10000010 0x0 0
R 4 1431.915857 2 0xb0600230 0x10000010 0x0 0
R 4 1431.917399 2 0xb0600230 0x10000020 0x0 0
R 4 1431.917413 2 0xb0600230 0x10000020 0x0 0

Broadcom uses bigger ring and they don't suffer from 0x1000 being ever
set. I've filled the ring few times, 0x1000 was never set on going
back to slot 0.

-- 
Rafa?

[PATCH] b43: Mask out unwanted bits of RX slot address

[David Woodhouse]

On Sun, 2011-08-14 at 00:13 +0200, Rafa? Mi?ecki wrote:
> Interesting... below is the dump from ndiswrapper on BCM4331:
> 
> R 4 1431.714843 2 0xb0600230 0x10000fb0 0x0 0
> R 4 1431.740035 2 0xb0600230 0x10000fc0 0x0 0
> R 4 1431.740050 2 0xb0600230 0x10000fc0 0x0 0
> R 4 1431.850216 2 0xb0600230 0x10000fd0 0x0 0
> R 4 1431.850229 2 0xb0600230 0x10000fd0 0x0 0
> R 4 1431.882151 2 0xb0600230 0x10000fe0 0x0 0
> R 4 1431.882164 2 0xb0600230 0x10000fe0 0x0 0
> R 4 1431.885822 2 0xb0600230 0x10000ff0 0x0 0
> R 4 1431.885835 2 0xb0600230 0x10000ff0 0x0 0
> R 4 1431.896775 2 0xb0600230 0x10000000 0x0 0
> R 4 1431.896790 2 0xb0600230 0x10000000 0x0 0
> R 4 1431.915842 2 0xb0600230 0x10000010 0x0 0
> R 4 1431.915857 2 0xb0600230 0x10000010 0x0 0
> R 4 1431.917399 2 0xb0600230 0x10000020 0x0 0
> R 4 1431.917413 2 0xb0600230 0x10000020 0x0 0
> 
> Broadcom uses bigger ring and they don't suffer from 0x1000 being ever
> set. I've filled the ring few times, 0x1000 was never set on going
> back to slot 0. 

What was the physical address of the ring for this test run?

-- 
dwmw2

[PATCH] b43: Mask out unwanted bits of RX slot address

[Rafał Miłecki]

W dniu 14 sierpnia 2011 00:56 u?ytkownik David Woodhouse
<dwmw2@infradead.org> napisa?:
> On Sun, 2011-08-14 at 00:13 +0200, Rafa? Mi?ecki wrote:
>> Interesting... below is the dump from ndiswrapper on BCM4331:
>>
>> R 4 1431.714843 2 0xb0600230 0x10000fb0 0x0 0
>> R 4 1431.740035 2 0xb0600230 0x10000fc0 0x0 0
>> R 4 1431.740050 2 0xb0600230 0x10000fc0 0x0 0
>> R 4 1431.850216 2 0xb0600230 0x10000fd0 0x0 0
>> R 4 1431.850229 2 0xb0600230 0x10000fd0 0x0 0
>> R 4 1431.882151 2 0xb0600230 0x10000fe0 0x0 0
>> R 4 1431.882164 2 0xb0600230 0x10000fe0 0x0 0
>> R 4 1431.885822 2 0xb0600230 0x10000ff0 0x0 0
>> R 4 1431.885835 2 0xb0600230 0x10000ff0 0x0 0
>> R 4 1431.896775 2 0xb0600230 0x10000000 0x0 0
>> R 4 1431.896790 2 0xb0600230 0x10000000 0x0 0
>> R 4 1431.915842 2 0xb0600230 0x10000010 0x0 0
>> R 4 1431.915857 2 0xb0600230 0x10000010 0x0 0
>> R 4 1431.917399 2 0xb0600230 0x10000020 0x0 0
>> R 4 1431.917413 2 0xb0600230 0x10000020 0x0 0
>>
>> Broadcom uses bigger ring and they don't suffer from 0x1000 being ever
>> set. I've filled the ring few times, 0x1000 was never set on going
>> back to slot 0.
>
> What was the physical address of the ring for this test run?

W 4 1279.247106 2 0xb0600228 0x1f932000 0x0 0
W 4 1279.247107 2 0xb060022c 0x80000000 0x0 0
R 4 1279.247112 2 0xb0600220 0x40 0x0 0
W 4 1279.247114 2 0xb0600220 0x84d 0x0 0
W 4 1279.259637 2 0xb0600224 0x1f932200 0x0 0

I read from the dump it was 0x000000001f932000

-- 
Rafa?

[PATCH] b43: Mask out unwanted bits of RX slot address

[David Woodhouse]

On Sun, 2011-08-14 at 10:18 +0200, Rafa? Mi?ecki wrote:
> 
> > What was the physical address of the ring for this test run?
> 
> W 4 1279.247106 2 0xb0600228 0x1f932000 0x0 0
> W 4 1279.247107 2 0xb060022c 0x80000000 0x0 0
> R 4 1279.247112 2 0xb0600220 0x40 0x0 0
> W 4 1279.247114 2 0xb0600220 0x84d 0x0 0
> W 4 1279.259637 2 0xb0600224 0x1f932200 0x0 0
> 
> I read from the dump it was 0x000000001f932000

The interesting case is when it's *not* aligned to 8KiB.

-- 
dwmw2

[PATCH] b43: Mask out unwanted bits of RX slot address

[Rafał Miłecki]

W dniu 14 sierpnia 2011 00:13 u?ytkownik Rafa? Mi?ecki
<zajec5@gmail.com> napisa?:
> W dniu 12 sierpnia 2011 12:15 u?ytkownik David Woodhouse
> <dwmw2@infradead.org> napisa?:
>> On some 64-bit DMA engines (my BCM4331, at least) with a ring buffer
>> that is not aligned to 8KiB, we often see bit 12 spuriously being set in
>> the B43_DMA64_RXSTATUS register. This will happen the first time the
>> ring fills and loops back to zero:
>>
>> [35755.186080] Read RXSTATUS 3f0 (makes offs 3f0, divided by 10 gives slot 3f
>> [35755.288366] Read RXSTATUS 1000 (makes offs 1000, divided by 10 gives slot 100
>> [35755.291007] ------------[ cut here ]------------
>> [35755.291041] WARNING: at drivers/net/wireless/b43/dma.c:1700 b43_dma_rx+0x5e/0x2df [b43]()
>> ?...
>> [35766.745374] b43-phy41 debug: DMA RX: Dropping poisoned buffer.
>
> Interesting... below is the dump from ndiswrapper on BCM4331:
>
> R 4 1431.714843 2 0xb0600230 0x10000fb0 0x0 0
> R 4 1431.740035 2 0xb0600230 0x10000fc0 0x0 0
> R 4 1431.740050 2 0xb0600230 0x10000fc0 0x0 0
> R 4 1431.850216 2 0xb0600230 0x10000fd0 0x0 0
> R 4 1431.850229 2 0xb0600230 0x10000fd0 0x0 0
> R 4 1431.882151 2 0xb0600230 0x10000fe0 0x0 0
> R 4 1431.882164 2 0xb0600230 0x10000fe0 0x0 0
> R 4 1431.885822 2 0xb0600230 0x10000ff0 0x0 0
> R 4 1431.885835 2 0xb0600230 0x10000ff0 0x0 0
> R 4 1431.896775 2 0xb0600230 0x10000000 0x0 0
> R 4 1431.896790 2 0xb0600230 0x10000000 0x0 0
> R 4 1431.915842 2 0xb0600230 0x10000010 0x0 0
> R 4 1431.915857 2 0xb0600230 0x10000010 0x0 0
> R 4 1431.917399 2 0xb0600230 0x10000020 0x0 0
> R 4 1431.917413 2 0xb0600230 0x10000020 0x0 0
>
> Broadcom uses bigger ring and they don't suffer from 0x1000 being ever
> set. I've filled the ring few times, 0x1000 was never set on going
> back to slot 0.

I've creased RX ring size to 256 (0xFF) from 64 (0x3F):

[40452.825252] Read RXSTATUS 0x10000f00 (makes offs 0xf00, divided by
0x10 gives slot 0xf0
[40452.838688] Read RXSTATUS 0x10000f10 (makes offs 0xf10, divided by
0x10 gives slot 0xf1
[40452.841106] Read RXSTATUS 0x10000f20 (makes offs 0xf20, divided by
0x10 gives slot 0xf2
[40452.843893] Read RXSTATUS 0x10000f30 (makes offs 0xf30, divided by
0x10 gives slot 0xf3
[40452.846386] Read RXSTATUS 0x10000f40 (makes offs 0xf40, divided by
0x10 gives slot 0xf4
[40452.848881] Read RXSTATUS 0x10000f50 (makes offs 0xf50, divided by
0x10 gives slot 0xf5
[40452.853260] Read RXSTATUS 0x10000f60 (makes offs 0xf60, divided by
0x10 gives slot 0xf6
[40452.855144] Read RXSTATUS 0x10000f70 (makes offs 0xf70, divided by
0x10 gives slot 0xf7
[40452.857195] Read RXSTATUS 0x10000f80 (makes offs 0xf80, divided by
0x10 gives slot 0xf8
[40452.859106] Read RXSTATUS 0x10000f90 (makes offs 0xf90, divided by
0x10 gives slot 0xf9
[40452.861587] Read RXSTATUS 0x10000fa0 (makes offs 0xfa0, divided by
0x10 gives slot 0xfa
[40452.865788] Read RXSTATUS 0x10000fb0 (makes offs 0xfb0, divided by
0x10 gives slot 0xfb
[40452.877345] Read RXSTATUS 0x10000fc0 (makes offs 0xfc0, divided by
0x10 gives slot 0xfc
[40454.693234] Read RXSTATUS 0x10000fd0 (makes offs 0xfd0, divided by
0x10 gives slot 0xfd
[40454.694962] Read RXSTATUS 0x10000fe0 (makes offs 0xfe0, divided by
0x10 gives slot 0xfe
[40455.430057] Read RXSTATUS 0x10000ff0 (makes offs 0xff0, divided by
0x10 gives slot 0xff
[40455.449115] Read RXSTATUS 0x10000000 (makes offs 0x0, divided by
0x10 gives slot 0x0
[40455.471817] Read RXSTATUS 0x10000010 (makes offs 0x10, divided by
0x10 gives slot 0x1
[40455.502368] Read RXSTATUS 0x10000020 (makes offs 0x20, divided by
0x10 gives slot 0x2
[40455.884527] Read RXSTATUS 0x10000030 (makes offs 0x30, divided by
0x10 gives slot 0x3
[40455.904890] Read RXSTATUS 0x10000040 (makes offs 0x40, divided by
0x10 gives slot 0x4

No more 0x1000 bit. Will test with other values.

-- 
Rafa?

[PATCH] b43: Mask out unwanted bits of RX slot address

[Rafał Miłecki]

W dniu 14 sierpnia 2011 11:02 u?ytkownik Rafa? Mi?ecki
<zajec5@gmail.com> napisa?:
> No more 0x1000 bit. Will test with other values.

RX ring size 128:

[40628.194250] Read RXSTATUS 0x100007a0 (makes offs 0x7a0, divided by
0x10 gives slot 0x7a
[40628.262661] Read RXSTATUS 0x100007b0 (makes offs 0x7b0, divided by
0x10 gives slot 0x7b
[40628.264430] Read RXSTATUS 0x100007c0 (makes offs 0x7c0, divided by
0x10 gives slot 0x7c
[40628.266273] Read RXSTATUS 0x100007d0 (makes offs 0x7d0, divided by
0x10 gives slot 0x7d
[40628.267963] Read RXSTATUS 0x100007e0 (makes offs 0x7e0, divided by
0x10 gives slot 0x7e
[40628.269738] Read RXSTATUS 0x100007f0 (makes offs 0x7f0, divided by
0x10 gives slot 0x7f
[40628.332119] Read RXSTATUS 0x10001000 (makes offs 0x1000, divided by
0x10 gives slot 0x100
[40628.424611] Read RXSTATUS 0x10001010 (makes offs 0x1010, divided by
0x10 gives slot 0x101
[40628.471523] Read RXSTATUS 0x10001020 (makes offs 0x1020, divided by
0x10 gives slot 0x102

RX ring size 32:

[40693.574982] Read RXSTATUS 0x100001a0 (makes offs 0x1a0, divided by
0x10 gives slot 0x1a
[40693.577424] Read RXSTATUS 0x100001b0 (makes offs 0x1b0, divided by
0x10 gives slot 0x1b
[40693.579987] Read RXSTATUS 0x100001c0 (makes offs 0x1c0, divided by
0x10 gives slot 0x1c
[40693.585268] Read RXSTATUS 0x100001d0 (makes offs 0x1d0, divided by
0x10 gives slot 0x1d
[40693.587825] Read RXSTATUS 0x100001e0 (makes offs 0x1e0, divided by
0x10 gives slot 0x1e
[40693.601323] Read RXSTATUS 0x100001f0 (makes offs 0x1f0, divided by
0x10 gives slot 0x1f
[40695.763842] Read RXSTATUS 0x10001000 (makes offs 0x1000, divided by
0x10 gives slot 0x100
[40695.784039] Read RXSTATUS 0x10001010 (makes offs 0x1010, divided by
0x10 gives slot 0x101
[40695.806865] Read RXSTATUS 0x10001020 (makes offs 0x1020, divided by
0x10 gives slot 0x102

I guess we should just increase RX ring size instead hacking
(stripping) 0x1000 bit. We just need to check  on which hardware wl
uses 256 ring size.

-- 
Rafa?

[PATCH] b43: Mask out unwanted bits of RX slot address

[Rafał Miłecki]

W dniu 14 sierpnia 2011 11:07 u?ytkownik Rafa? Mi?ecki
<zajec5@gmail.com> napisa?:
> I guess we should just increase RX ring size instead hacking
> (stripping) 0x1000 bit. We just need to check ?on which hardware wl
> uses 256 ring size.

Ignore that. I just got 8 KiB aligned ring when testing 256 ring size.

I agree with David, that 0x1000 comes from ring address. Depending on
address alignment it's 0 or 1.

-- 
Rafa?

[PATCH] b43: Mask out unwanted bits of RX slot address

[Rafał Miłecki]

W dniu 14 sierpnia 2011 11:24 u?ytkownik Rafa? Mi?ecki
<zajec5@gmail.com> napisa?:
> W dniu 14 sierpnia 2011 11:07 u?ytkownik Rafa? Mi?ecki
> <zajec5@gmail.com> napisa?:
>> I guess we should just increase RX ring size instead hacking
>> (stripping) 0x1000 bit. We just need to check ?on which hardware wl
>> uses 256 ring size.
>
> Ignore that. I just got 8 KiB aligned ring when testing 256 ring size.
>
> I agree with David, that 0x1000 comes from ring address. Depending on
> address alignment it's 0 or 1.

The question I now have is:

1) Should be keep using 4 KiB aligned rings (sometimes, depending on
the luck, 8 KiB) with ignoring 0x1000 bit
2) Switch to 8 KiB aligned rings

Broadcom seems to use second method, it never uses just 4 KiB aligned ring.

[PATCH] b43: Mask out unwanted bits of RX slot address

[Rafał Miłecki]

W dniu 14 sierpnia 2011 11:24 u?ytkownik Rafa? Mi?ecki
<zajec5@gmail.com> napisa?:
> W dniu 14 sierpnia 2011 11:07 u?ytkownik Rafa? Mi?ecki
> <zajec5@gmail.com> napisa?:
>> I guess we should just increase RX ring size instead hacking
>> (stripping) 0x1000 bit. We just need to check ?on which hardware wl
>> uses 256 ring size.
>
> Ignore that. I just got 8 KiB aligned ring when testing 256 ring size.
>
> I agree with David, that 0x1000 comes from ring address. Depending on
> address alignment it's 0 or 1.

I wanted to check if firmware ever uses 0x1000 for addressing
purposes. To test that I've increased RX ring size to 257 and waited
for 8 KiB aligned ring address (to avoid copying 0x1000 bit from
address).

[ 3963.448699] RX ring at 24661e000
(...)
[ 4041.766608] Read RXSTATUS 0x10000fc0 (makes offs 0xfc0, divided by
0x10 gives slot 0xfc
[ 4042.066683] Read RXSTATUS 0x10000fd0 (makes offs 0xfd0, divided by
0x10 gives slot 0xfd
[ 4042.070585] Read RXSTATUS 0x10000fe0 (makes offs 0xfe0, divided by
0x10 gives slot 0xfe
[ 4042.141030] Read RXSTATUS 0x10000ff0 (makes offs 0xff0, divided by
0x10 gives slot 0xff
[ 4042.206599] b43-phy0 ERROR: Fatal DMA error: 0x00011000,
0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000
[ 4042.208438] b43-phy0 ERROR: This device does not support DMA on
your system. It will now be switched to PIO.
[ 4042.210463] b43-phy0: Controller RESET (DMA error) ...

Maybe we should just change:
#define		B43_DMA64_TXSTATDPTR		0x00001FFF

-- 
Rafa?

[PATCH] b43: Mask out unwanted bits of RX slot address

[Rafał Miłecki]

W dniu 14 sierpnia 2011 13:17 u?ytkownik Rafa? Mi?ecki
<zajec5@gmail.com> napisa?:
> W dniu 14 sierpnia 2011 11:24 u?ytkownik Rafa? Mi?ecki
> <zajec5@gmail.com> napisa?:
>> W dniu 14 sierpnia 2011 11:07 u?ytkownik Rafa? Mi?ecki
>> <zajec5@gmail.com> napisa?:
>>> I guess we should just increase RX ring size instead hacking
>>> (stripping) 0x1000 bit. We just need to check ?on which hardware wl
>>> uses 256 ring size.
>>
>> Ignore that. I just got 8 KiB aligned ring when testing 256 ring size.
>>
>> I agree with David, that 0x1000 comes from ring address. Depending on
>> address alignment it's 0 or 1.
>
> I wanted to check if firmware ever uses 0x1000 for addressing
> purposes. To test that I've increased RX ring size to 257 and waited
> for 8 KiB aligned ring address (to avoid copying 0x1000 bit from
> address).
>
> [ 3963.448699] RX ring at 24661e000
> (...)
> [ 4041.766608] Read RXSTATUS 0x10000fc0 (makes offs 0xfc0, divided by
> 0x10 gives slot 0xfc
> [ 4042.066683] Read RXSTATUS 0x10000fd0 (makes offs 0xfd0, divided by
> 0x10 gives slot 0xfd
> [ 4042.070585] Read RXSTATUS 0x10000fe0 (makes offs 0xfe0, divided by
> 0x10 gives slot 0xfe
> [ 4042.141030] Read RXSTATUS 0x10000ff0 (makes offs 0xff0, divided by
> 0x10 gives slot 0xff
> [ 4042.206599] b43-phy0 ERROR: Fatal DMA error: 0x00011000,
> 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000
> [ 4042.208438] b43-phy0 ERROR: This device does not support DMA on
> your system. It will now be switched to PIO.
> [ 4042.210463] b43-phy0: Controller RESET (DMA error) ...
>
> Maybe we should just change:
> #define ? ? ? ? B43_DMA64_TXSTATDPTR ? ? ? ? ? ?0x00001FFF

I meant
#define		B43_DMA64_RXSTATDPTR		0x00001FFF
ofc

-- 
Rafa?

[PATCH] b43: Mask out unwanted bits of RX slot address

[David Woodhouse]

On Sun, 2011-08-14 at 13:17 +0200, Rafa? Mi?ecki wrote:
> I wanted to check if firmware ever uses 0x1000 for addressing
> purposes. To test that I've increased RX ring size to 257 and waited
> for 8 KiB aligned ring address (to avoid copying 0x1000 bit from
> address). 

Erm, if you increase the ring size to 257, don't you have to allocate
8KiB for the ring buffer? If you're still allocating only 4KiB you'll be
scribbling off the end of it. And if you allocate 8KiB there'll be no
waiting; it'll *always* be aligned to 8KiB.

-- 
dwmw2

[PATCH] b43: Mask out unwanted bits of RX slot address

[Rafał Miłecki]

W dniu 14 sierpnia 2011 13:35 u?ytkownik David Woodhouse
<dwmw2@infradead.org> napisa?:
> On Sun, 2011-08-14 at 13:17 +0200, Rafa? Mi?ecki wrote:
>> I wanted to check if firmware ever uses 0x1000 for addressing
>> purposes. To test that I've increased RX ring size to 257 and waited
>> for 8 KiB aligned ring address (to avoid copying 0x1000 bit from
>> address).
>
> Erm, if you increase the ring size to 257, don't you have to allocate
> 8KiB for the ring buffer? If you're still allocating only 4KiB you'll be
> scribbling off the end of it. And if you allocate 8KiB there'll be no
> waiting; it'll *always* be aligned to 8KiB.

That's right. I've now modified B43_DMA_RINGMEMSIZE to be 8 KiB and I
don't get DMA errors anymore. Tested with 5 b43 reloads, the last time
I even used 384 slots in RX ring. Seems to work stable.

-- 
Rafa?

[PATCH] b43: Mask out unwanted bits of RX slot address

[Michael Büsch]

On Sun, 14 Aug 2011 17:02:27 +0200
Rafa? Mi?ecki <zajec5@gmail.com> wrote:
> That's right. I've now modified B43_DMA_RINGMEMSIZE to be 8 KiB and I
> don't get DMA errors anymore. Tested with 5 b43 reloads, the last time
> I even used 384 slots in RX ring. Seems to work stable.

I'm OK with changing the ringsize to a hardcoded 8kiB, btw.
As I said, the current PAGE_SIZE dependency is mere legacy.

-- 
Greetings, Michael.

[PATCH] b43: Mask out unwanted bits of RX slot address

[Rafał Miłecki]

W dniu 14 sierpnia 2011 13:17 u?ytkownik Rafa? Mi?ecki
<zajec5@gmail.com> napisa?:
> W dniu 14 sierpnia 2011 11:24 u?ytkownik Rafa? Mi?ecki
> <zajec5@gmail.com> napisa?:
>> W dniu 14 sierpnia 2011 11:07 u?ytkownik Rafa? Mi?ecki
>> <zajec5@gmail.com> napisa?:
>>> I guess we should just increase RX ring size instead hacking
>>> (stripping) 0x1000 bit. We just need to check ?on which hardware wl
>>> uses 256 ring size.
>>
>> Ignore that. I just got 8 KiB aligned ring when testing 256 ring size.
>>
>> I agree with David, that 0x1000 comes from ring address. Depending on
>> address alignment it's 0 or 1.
>
> I wanted to check if firmware ever uses 0x1000 for addressing
> purposes. To test that I've increased RX ring size to 257 and waited
> for 8 KiB aligned ring address (to avoid copying 0x1000 bit from
> address).
>
> [ 3963.448699] RX ring at 24661e000
> (...)
> [ 4041.766608] Read RXSTATUS 0x10000fc0 (makes offs 0xfc0, divided by
> 0x10 gives slot 0xfc
> [ 4042.066683] Read RXSTATUS 0x10000fd0 (makes offs 0xfd0, divided by
> 0x10 gives slot 0xfd
> [ 4042.070585] Read RXSTATUS 0x10000fe0 (makes offs 0xfe0, divided by
> 0x10 gives slot 0xfe
> [ 4042.141030] Read RXSTATUS 0x10000ff0 (makes offs 0xff0, divided by
> 0x10 gives slot 0xff
> [ 4042.206599] b43-phy0 ERROR: Fatal DMA error: 0x00011000,
> 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000
> [ 4042.208438] b43-phy0 ERROR: This device does not support DMA on
> your system. It will now be switched to PIO.
> [ 4042.210463] b43-phy0: Controller RESET (DMA error) ...

This is just random. I've done additional tests with ring size 258. I
always test with 8 KiB aligned ring address and I randomly get DMA
errors or such a nice:

[ 5264.389572] RXERROR: 0x00000fe0 RXSTATUS: 0x10000fd0 (makes offs
0xfd0, divided by 0x10 gives slot 0xfd
[ 5264.391571] RXERROR: 0x00000ff0 RXSTATUS: 0x10000fe0 (makes offs
0xfe0, divided by 0x10 gives slot 0xfe
[ 5264.632352] RXERROR: 0x00001000 RXSTATUS: 0x10000ff0 (makes offs
0xff0, divided by 0x10 gives slot 0xff
[ 5264.751025] RXERROR: 0x00001010 RXSTATUS: 0x10001000 (makes offs
0x1000, divided by 0x10 gives slot 0x100
[ 5264.837564] RXERROR: 0x00000000 RXSTATUS: 0x10001010 (makes offs
0x1010, divided by 0x10 gives slot 0x101
[ 5264.853311] RXERROR: 0x00000010 RXSTATUS: 0x10000000 (makes offs
0x0, divided by 0x10 gives slot 0x0
[ 5264.906550] RXERROR: 0x00000020 RXSTATUS: 0x10000010 (makes offs
0x10, divided by 0x10 gives slot 0x1
[ 5265.047389] RXERROR: 0x00000030 RXSTATUS: 0x10000020 (makes offs
0x20, divided by 0x10 gives slot 0x2

In any case we see it can happen firmware is using that 0x1000 for indexing.

We just need to keep the original mask and start making rings 8 KiB
aligned to workaround firmware bug.

-- 
Rafa?

[PATCH] b43: Mask out unwanted bits of RX slot address

[Michael Büsch]

On Sun, 14 Aug 2011 13:38:59 +0200
Rafa? Mi?ecki <zajec5@gmail.com> wrote:
> In any case we see it can happen firmware is using that 0x1000 for indexing.
> 
> We just need to keep the original mask and start making rings 8 KiB
> aligned to workaround firmware bug.

The firmware does not control DMA. This is pure hardware feature.

-- 
Greetings, Michael.

[PATCH] b43: Mask out unwanted bits of RX slot address

[David Woodhouse]

On Sun, 2011-08-14 at 13:38 +0200, Rafa? Mi?ecki wrote:
> In any case we see it can happen firmware is using that 0x1000 for
> indexing. 

Well yes, but that won't happen if we limit ourselves to 4KiB of ring.

So continuing to use 4KiB and masking out the 'top' bit works too.

-- 
dwmw2

[PATCH] b43: Mask out unwanted bits of RX slot address

[Rafał Miłecki]

W dniu 14 sierpnia 2011 14:06 u?ytkownik David Woodhouse
<dwmw2@infradead.org> napisa?:
> On Sun, 2011-08-14 at 13:38 +0200, Rafa? Mi?ecki wrote:
>> In any case we see it can happen firmware is using that 0x1000 for
>> indexing.
>
> Well yes, but that won't happen if we limit ourselves to 4KiB of ring.
>
> So continuing to use 4KiB and masking out the 'top' bit works too.

Well, currently we use just 1 KiB for RX ring:
#define B43_RXRING_SLOTS		64
(to remind others: sizeof(struct b43_dmadesc64) equals 0x10)

Setting 0x1000 bit with value taken from address sounds like pure bug,
I can not image any real use of that. Why we should ever want to read
single bit of ring base address from descriptor...

In the future, if Broadcom will use more than 256 slots (more than 4
KiB) for RX ring, we will need that bit. I prefer to add workaround
for this hardware bug now, rather than fixing this little hack in the
future.

-- 
Rafa?

[PATCH] Fix alignment issues with DMA TX on BCM4331

[David Woodhouse]

When the TX descriptor ring is not aligned to 8KiB on BCM4331, we have
to write the full address to the TXINDEX register or the DMA engine gets
confused after the first time we wrap round to slot zero.

[ 7438.538945] Poked TX with address fe0 for slot 254
[ 7438.539077] Acking gen reason 20000000
[ 7438.539177] irq xmitstat 20fc1001 0000007f
[ 7438.607861] Poked TX with address 0 for slot 0
[ 7438.608567] Acking gen reason 20000000
[ 7438.608668] irq xmitstat 20fe1001 00000080
[ 7438.608709] irq xmitstat 20000011 00000080
[ 7438.608724] b43-phy2 debug: Out of order TX status report on DMA ring 1. Expected 256, but got 0
[ 7438.608739] irq xmitstat 20020011 00000080
[ 7438.608750] b43-phy2 debug: Out of order TX status report on DMA ring 1. Expected 256, but got 2
[ 7438.608765] irq xmitstat 20040011 00000080

We write 0xff0 to the TXADDRLO register to see if the DMA engine is
capable of unaligned operation. If it *is*, then it'll have this problem
and we have to write the full address to TXINDEX. Comments in brcmsmac
indicate that the low 13 bits are required.

If we're doing this, we *also* have to write to TXCTL to enable the DMA
engine *after* setting up the ring address.

Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
--
I've made that change to the initialisation order of TXCTL vs.
TXADDR{HI,LO} unconditional; is there a reason not to?

diff --git a/drivers/net/wireless/b43/dma.c b/drivers/net/wireless/b43/dma.c
index 82168f8..92dd6d9 100644
--- a/drivers/net/wireless/b43/dma.c
+++ b/drivers/net/wireless/b43/dma.c
@@ -225,8 +225,10 @@ static void op64_fill_descriptor(struct b43_dmaring *ring,
 
 static void op64_poke_tx(struct b43_dmaring *ring, int slot)
 {
-	b43_dma_write(ring, B43_DMA64_TXINDEX,
-		      (u32) (slot * sizeof(struct b43_dmadesc64)));
+	u32 indexval = slot * sizeof(struct b43_dmadesc64);
+	if (ring->unaligned)
+		indexval |= (u32)ring->dmabase;
+	b43_dma_write(ring, B43_DMA64_TXINDEX, indexval);
 }
 
 static void op64_tx_suspend(struct b43_dmaring *ring)
@@ -704,9 +710,14 @@ static int dmacontroller_setup(struct b43_dmaring *ring)
 			    & B43_DMA64_TXADDREXT_MASK;
 			if (!parity)
 				value |= B43_DMA64_TXPARITYDISABLE;
-			b43_dma_write(ring, B43_DMA64_TXCTL, value);
+
+			b43_dma_write(ring, B43_DMA64_TXRINGLO, 0xff0);
+			if (b43_dma_read(ring, B43_DMA64_TXRINGLO))
+				ring->unaligned = 1;
+
 			b43_dma_write(ring, B43_DMA64_TXRINGLO, addrlo);
 			b43_dma_write(ring, B43_DMA64_TXRINGHI, addrhi);
+			b43_dma_write(ring, B43_DMA64_TXCTL, value);
 		} else {
 			u32 ringbase = (u32) (ring->dmabase);
 			addrext = b43_dma_address(&ring->dev->dma, ringbase, B43_DMA_ADDR_EXT);
diff --git a/drivers/net/wireless/b43/dma.h b/drivers/net/wireless/b43/dma.h
index 7e20b04f..16dc565 100644
--- a/drivers/net/wireless/b43/dma.h
+++ b/drivers/net/wireless/b43/dma.h
@@ -251,6 +251,8 @@ struct b43_dmaring {
 	int index;
 	/* Boolean. Is this a TX ring? */
 	bool tx;
+	/* Boolean. Is this ring capable of 16-byte alignment? */
+	bool unaligned;
 	/* The type of DMA engine used. */
 	enum b43_dmatype type;
 	/* Boolean. Is this ring stopped at ieee80211 level? */

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse at intel.com                              Intel Corporation

[PATCH] Fix alignment issues with DMA TX on BCM4331

[Rafał Miłecki]

W dniu 12 sierpnia 2011 12:27 u?ytkownik David Woodhouse
<dwmw2@infradead.org> napisa?:
> When the TX descriptor ring is not aligned to 8KiB on BCM4331, we have
> to write the full address to the TXINDEX register or the DMA engine gets
> confused after the first time we wrap round to slot zero.
>
> [ 7438.538945] Poked TX with address fe0 for slot 254
> [ 7438.539077] Acking gen reason 20000000
> [ 7438.539177] irq xmitstat 20fc1001 0000007f
> [ 7438.607861] Poked TX with address 0 for slot 0
> [ 7438.608567] Acking gen reason 20000000
> [ 7438.608668] irq xmitstat 20fe1001 00000080
> [ 7438.608709] irq xmitstat 20000011 00000080
> [ 7438.608724] b43-phy2 debug: Out of order TX status report on DMA ring 1. Expected 256, but got 0
> [ 7438.608739] irq xmitstat 20020011 00000080
> [ 7438.608750] b43-phy2 debug: Out of order TX status report on DMA ring 1. Expected 256, but got 2
> [ 7438.608765] irq xmitstat 20040011 00000080
>
> We write 0xff0 to the TXADDRLO register to see if the DMA engine is
> capable of unaligned operation. If it *is*, then it'll have this problem
> and we have to write the full address to TXINDEX. Comments in brcmsmac
> indicate that the low 13 bits are required.
>
> If we're doing this, we *also* have to write to TXCTL to enable the DMA
> engine *after* setting up the ring address.
>
> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
> --
> I've made that change to the initialisation order of TXCTL vs.
> TXADDR{HI,LO} unconditional; is there a reason not to?
>
> diff --git a/drivers/net/wireless/b43/dma.c b/drivers/net/wireless/b43/dma.c
> index 82168f8..92dd6d9 100644
> --- a/drivers/net/wireless/b43/dma.c
> +++ b/drivers/net/wireless/b43/dma.c
> @@ -225,8 +225,10 @@ static void op64_fill_descriptor(struct b43_dmaring *ring,
>
> ?static void op64_poke_tx(struct b43_dmaring *ring, int slot)
> ?{
> - ? ? ? b43_dma_write(ring, B43_DMA64_TXINDEX,
> - ? ? ? ? ? ? ? ? ? ? (u32) (slot * sizeof(struct b43_dmadesc64)));
> + ? ? ? u32 indexval = slot * sizeof(struct b43_dmadesc64);
> + ? ? ? if (ring->unaligned)
> + ? ? ? ? ? ? ? indexval |= (u32)ring->dmabase;
> + ? ? ? b43_dma_write(ring, B43_DMA64_TXINDEX, indexval);

Shouldn't this bit or be a numerical sum? I mean: +=
Imagine you get dmabase like 0x1f310123 instead of some nice 0x1f310000


> @@ -704,9 +710,14 @@ static int dmacontroller_setup(struct b43_dmaring *ring)
> ? ? ? ? ? ? ? ? ? ? ? ? ? ?& B43_DMA64_TXADDREXT_MASK;
> ? ? ? ? ? ? ? ? ? ? ? ?if (!parity)
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?value |= B43_DMA64_TXPARITYDISABLE;
> - ? ? ? ? ? ? ? ? ? ? ? b43_dma_write(ring, B43_DMA64_TXCTL, value);
> +
> + ? ? ? ? ? ? ? ? ? ? ? b43_dma_write(ring, B43_DMA64_TXRINGLO, 0xff0);
> + ? ? ? ? ? ? ? ? ? ? ? if (b43_dma_read(ring, B43_DMA64_TXRINGLO))
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ring->unaligned = 1;
> +
> ? ? ? ? ? ? ? ? ? ? ? ?b43_dma_write(ring, B43_DMA64_TXRINGLO, addrlo);
> ? ? ? ? ? ? ? ? ? ? ? ?b43_dma_write(ring, B43_DMA64_TXRINGHI, addrhi);
> + ? ? ? ? ? ? ? ? ? ? ? b43_dma_write(ring, B43_DMA64_TXCTL, value);

This needs testing on all the old hardware. Not sure if it's OK on
older cards to change the order.

John: please give this patch some more time for testing & discussion.

-- 
Rafa?

[PATCH] Fix alignment issues with DMA TX on BCM4331

[David Woodhouse]

On Sat, 2011-08-13 at 21:06 +0200, Rafa? Mi?ecki wrote:
> W dniu 12 sierpnia 2011 12:27 u?ytkownik David Woodhouse
> <dwmw2@infradead.org> napisa?:
> > When the TX descriptor ring is not aligned to 8KiB on BCM4331, we have
> > to write the full address to the TXINDEX register or the DMA engine gets
> > confused after the first time we wrap round to slot zero.
> >
> > [ 7438.538945] Poked TX with address fe0 for slot 254
> > [ 7438.539077] Acking gen reason 20000000
> > [ 7438.539177] irq xmitstat 20fc1001 0000007f
> > [ 7438.607861] Poked TX with address 0 for slot 0
> > [ 7438.608567] Acking gen reason 20000000
> > [ 7438.608668] irq xmitstat 20fe1001 00000080
> > [ 7438.608709] irq xmitstat 20000011 00000080
> > [ 7438.608724] b43-phy2 debug: Out of order TX status report on DMA ring 1. Expected 256, but got 0
> > [ 7438.608739] irq xmitstat 20020011 00000080
> > [ 7438.608750] b43-phy2 debug: Out of order TX status report on DMA ring 1. Expected 256, but got 2
> > [ 7438.608765] irq xmitstat 20040011 00000080
> >
> > We write 0xff0 to the TXADDRLO register to see if the DMA engine is
> > capable of unaligned operation. If it *is*, then it'll have this problem
> > and we have to write the full address to TXINDEX. Comments in brcmsmac
> > indicate that the low 13 bits are required.
> >
> > If we're doing this, we *also* have to write to TXCTL to enable the DMA
> > engine *after* setting up the ring address.
> >
> > Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
> > --
> > I've made that change to the initialisation order of TXCTL vs.
> > TXADDR{HI,LO} unconditional; is there a reason not to?
> >
> > diff --git a/drivers/net/wireless/b43/dma.c b/drivers/net/wireless/b43/dma.c
> > index 82168f8..92dd6d9 100644
> > --- a/drivers/net/wireless/b43/dma.c
> > +++ b/drivers/net/wireless/b43/dma.c
> > @@ -225,8 +225,10 @@ static void op64_fill_descriptor(struct b43_dmaring *ring,
> >
> >  static void op64_poke_tx(struct b43_dmaring *ring, int slot)
> >  {
> > -       b43_dma_write(ring, B43_DMA64_TXINDEX,
> > -                     (u32) (slot * sizeof(struct b43_dmadesc64)));
> > +       u32 indexval = slot * sizeof(struct b43_dmadesc64);
> > +       if (ring->unaligned)
> > +               indexval |= (u32)ring->dmabase;
> > +       b43_dma_write(ring, B43_DMA64_TXINDEX, indexval);
> 
> Shouldn't this bit or be a numerical sum? I mean: +=
> Imagine you get dmabase like 0x1f310123 instead of some nice 0x1f310000

You won't. The ring is always going to be aligned to its own size, so a
simple mask is perfectly sufficient. Even if we go to an 8KiB ring
that'll still be true.

If we ever tried to use a ring that *wasn't* so aligned, I strongly
suspect we'd get other hardware problems like the ones that this patch
addresses.

> > @@ -704,9 +710,14 @@ static int dmacontroller_setup(struct b43_dmaring *ring)
> >                            & B43_DMA64_TXADDREXT_MASK;
> >                        if (!parity)
> >                                value |= B43_DMA64_TXPARITYDISABLE;
> > -                       b43_dma_write(ring, B43_DMA64_TXCTL, value);
> > +
> > +                       b43_dma_write(ring, B43_DMA64_TXRINGLO, 0xff0);
> > +                       if (b43_dma_read(ring, B43_DMA64_TXRINGLO))
> > +                               ring->unaligned = 1;
> > +
> >                        b43_dma_write(ring, B43_DMA64_TXRINGLO, addrlo);
> >                        b43_dma_write(ring, B43_DMA64_TXRINGHI, addrhi);
> > +                       b43_dma_write(ring, B43_DMA64_TXCTL, value);
> 
> This needs testing on all the old hardware. Not sure if it's OK on
> older cards to change the order.

Definitely.

> John: please give this patch some more time for testing & discussion.

Does John normally pick up patches from the b43 list before they're sent
directly to him? If so, I'll be more careful in future...

-- 
dwmw2

[PATCH] Fix alignment issues with DMA TX on BCM4331

[Rafał Miłecki]

W dniu 13 sierpnia 2011 22:18 u?ytkownik David Woodhouse
<dwmw2@infradead.org> napisa?:
> On Sat, 2011-08-13 at 21:06 +0200, Rafa? Mi?ecki wrote:
>> John: please give this patch some more time for testing & discussion.
>
> Does John normally pick up patches from the b43 list before they're sent
> directly to him? If so, I'll be more careful in future...

Ah, no, I missed that. We should be _safe_ here, on our ML ;)

-- 
Rafa?

[PATCH] Fix alignment issues with DMA TX on BCM4331

[David Woodhouse]

On Sat, 2011-08-13 at 22:22 +0200, Rafa? Mi?ecki wrote:
> > Does John normally pick up patches from the b43 list before they're sent
> > directly to him? If so, I'll be more careful in future...
> 
> Ah, no, I missed that. We should be _safe_ here, on our ML ;)

Even if he was watching, I'd hope he wouldn't pick up a patch which
said:

> I've made that change to the initialisation order of TXCTL vs.
> TXADDR{HI,LO} unconditional; is there a reason not to? 

-- 
dwmw2

[PATCH] Fix alignment issues with DMA TX on BCM4331

[Rafał Miłecki]

W dniu 13 sierpnia 2011 22:18 u?ytkownik David Woodhouse
<dwmw2@infradead.org> napisa?:
> On Sat, 2011-08-13 at 21:06 +0200, Rafa? Mi?ecki wrote:
>> W dniu 12 sierpnia 2011 12:27 u?ytkownik David Woodhouse
>> <dwmw2@infradead.org> napisa?:
>> > When the TX descriptor ring is not aligned to 8KiB on BCM4331, we have
>> > to write the full address to the TXINDEX register or the DMA engine gets
>> > confused after the first time we wrap round to slot zero.
>> >
>> > [ 7438.538945] Poked TX with address fe0 for slot 254
>> > [ 7438.539077] Acking gen reason 20000000
>> > [ 7438.539177] irq xmitstat 20fc1001 0000007f
>> > [ 7438.607861] Poked TX with address 0 for slot 0
>> > [ 7438.608567] Acking gen reason 20000000
>> > [ 7438.608668] irq xmitstat 20fe1001 00000080
>> > [ 7438.608709] irq xmitstat 20000011 00000080
>> > [ 7438.608724] b43-phy2 debug: Out of order TX status report on DMA ring 1. Expected 256, but got 0
>> > [ 7438.608739] irq xmitstat 20020011 00000080
>> > [ 7438.608750] b43-phy2 debug: Out of order TX status report on DMA ring 1. Expected 256, but got 2
>> > [ 7438.608765] irq xmitstat 20040011 00000080
>> >
>> > We write 0xff0 to the TXADDRLO register to see if the DMA engine is
>> > capable of unaligned operation. If it *is*, then it'll have this problem
>> > and we have to write the full address to TXINDEX. Comments in brcmsmac
>> > indicate that the low 13 bits are required.
>> >
>> > If we're doing this, we *also* have to write to TXCTL to enable the DMA
>> > engine *after* setting up the ring address.
>> >
>> > Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
>> > --
>> > I've made that change to the initialisation order of TXCTL vs.
>> > TXADDR{HI,LO} unconditional; is there a reason not to?
>> >
>> > diff --git a/drivers/net/wireless/b43/dma.c b/drivers/net/wireless/b43/dma.c
>> > index 82168f8..92dd6d9 100644
>> > --- a/drivers/net/wireless/b43/dma.c
>> > +++ b/drivers/net/wireless/b43/dma.c
>> > @@ -225,8 +225,10 @@ static void op64_fill_descriptor(struct b43_dmaring *ring,
>> >
>> > ?static void op64_poke_tx(struct b43_dmaring *ring, int slot)
>> > ?{
>> > - ? ? ? b43_dma_write(ring, B43_DMA64_TXINDEX,
>> > - ? ? ? ? ? ? ? ? ? ? (u32) (slot * sizeof(struct b43_dmadesc64)));
>> > + ? ? ? u32 indexval = slot * sizeof(struct b43_dmadesc64);
>> > + ? ? ? if (ring->unaligned)
>> > + ? ? ? ? ? ? ? indexval |= (u32)ring->dmabase;
>> > + ? ? ? b43_dma_write(ring, B43_DMA64_TXINDEX, indexval);
>>
>> Shouldn't this bit or be a numerical sum? I mean: +=
>> Imagine you get dmabase like 0x1f310123 instead of some nice 0x1f310000
>
> You won't. The ring is always going to be aligned to its own size, so a
> simple mask is perfectly sufficient. Even if we go to an 8KiB ring
> that'll still be true.
>
> If we ever tried to use a ring that *wasn't* so aligned, I strongly
> suspect we'd get other hardware problems like the ones that this patch
> addresses.

I didn't know about such aligning. Of course my example 0x1f310123 was
silly, it was more realistic to take for ex. 0x1f310100. But if you
say kernel will always nicely align rings, OK then. Thanks for
teaching me sth new.

-- 
Rafa?

[PATCH] Fix alignment issues with DMA TX on BCM4331

[Larry Finger]

On 08/12/2011 05:27 AM, David Woodhouse wrote:
> When the TX descriptor ring is not aligned to 8KiB on BCM4331, we have
> to write the full address to the TXINDEX register or the DMA engine gets
> confused after the first time we wrap round to slot zero.
>
> [ 7438.538945] Poked TX with address fe0 for slot 254
> [ 7438.539077] Acking gen reason 20000000
> [ 7438.539177] irq xmitstat 20fc1001 0000007f
> [ 7438.607861] Poked TX with address 0 for slot 0
> [ 7438.608567] Acking gen reason 20000000
> [ 7438.608668] irq xmitstat 20fe1001 00000080
> [ 7438.608709] irq xmitstat 20000011 00000080
> [ 7438.608724] b43-phy2 debug: Out of order TX status report on DMA ring 1. Expected 256, but got 0
> [ 7438.608739] irq xmitstat 20020011 00000080
> [ 7438.608750] b43-phy2 debug: Out of order TX status report on DMA ring 1. Expected 256, but got 2
> [ 7438.608765] irq xmitstat 20040011 00000080
>
> We write 0xff0 to the TXADDRLO register to see if the DMA engine is
> capable of unaligned operation. If it *is*, then it'll have this problem
> and we have to write the full address to TXINDEX. Comments in brcmsmac
> indicate that the low 13 bits are required.
>
> If we're doing this, we *also* have to write to TXCTL to enable the DMA
> engine *after* setting up the ring address.
>
> Signed-off-by: David Woodhouse<David.Woodhouse@intel.com>
> --
> I've made that change to the initialisation order of TXCTL vs.
> TXADDR{HI,LO} unconditional; is there a reason not to?
>
> diff --git a/drivers/net/wireless/b43/dma.c b/drivers/net/wireless/b43/dma.c
> index 82168f8..92dd6d9 100644
> --- a/drivers/net/wireless/b43/dma.c
> +++ b/drivers/net/wireless/b43/dma.c
> @@ -225,8 +225,10 @@ static void op64_fill_descriptor(struct b43_dmaring *ring,
>
>   static void op64_poke_tx(struct b43_dmaring *ring, int slot)
>   {
> -	b43_dma_write(ring, B43_DMA64_TXINDEX,
> -		      (u32) (slot * sizeof(struct b43_dmadesc64)));
> +	u32 indexval = slot * sizeof(struct b43_dmadesc64);
> +	if (ring->unaligned)
> +		indexval |= (u32)ring->dmabase;
> +	b43_dma_write(ring, B43_DMA64_TXINDEX, indexval);
>   }
>
>   static void op64_tx_suspend(struct b43_dmaring *ring)
> @@ -704,9 +710,14 @@ static int dmacontroller_setup(struct b43_dmaring *ring)
>   			&  B43_DMA64_TXADDREXT_MASK;
>   			if (!parity)
>   				value |= B43_DMA64_TXPARITYDISABLE;
> -			b43_dma_write(ring, B43_DMA64_TXCTL, value);
> +
> +			b43_dma_write(ring, B43_DMA64_TXRINGLO, 0xff0);

Should there be a new symbol rather than this magic number?

> +			if (b43_dma_read(ring, B43_DMA64_TXRINGLO))
> +				ring->unaligned = 1;
> +
>   			b43_dma_write(ring, B43_DMA64_TXRINGLO, addrlo);
>   			b43_dma_write(ring, B43_DMA64_TXRINGHI, addrhi);
> +			b43_dma_write(ring, B43_DMA64_TXCTL, value);
>   		} else {
>   			u32 ringbase = (u32) (ring->dmabase);
>   			addrext = b43_dma_address(&ring->dev->dma, ringbase, B43_DMA_ADDR_EXT);
> diff --git a/drivers/net/wireless/b43/dma.h b/drivers/net/wireless/b43/dma.h
> index 7e20b04f..16dc565 100644
> --- a/drivers/net/wireless/b43/dma.h
> +++ b/drivers/net/wireless/b43/dma.h
> @@ -251,6 +251,8 @@ struct b43_dmaring {
>   	int index;
>   	/* Boolean. Is this a TX ring? */
>   	bool tx;
> +	/* Boolean. Is this ring capable of 16-byte alignment? */
> +	bool unaligned;
>   	/* The type of DMA engine used. */
>   	enum b43_dmatype type;
>   	/* Boolean. Is this ring stopped at ieee80211 level? */
>

Tested on BCM4318 on 32-bit system.

Larry

[PATCH] Fix alignment issues with DMA TX on BCM4331

[Rafał Miłecki]

W dniu 13 sierpnia 2011 23:57 u?ytkownik Larry Finger
<Larry.Finger@lwfinger.net> napisa?:
> On 08/12/2011 05:27 AM, David Woodhouse wrote:
>>
>> When the TX descriptor ring is not aligned to 8KiB on BCM4331, we have
>> to write the full address to the TXINDEX register or the DMA engine gets
>> confused after the first time we wrap round to slot zero.
>>
>> [ 7438.538945] Poked TX with address fe0 for slot 254
>> [ 7438.539077] Acking gen reason 20000000
>> [ 7438.539177] irq xmitstat 20fc1001 0000007f
>> [ 7438.607861] Poked TX with address 0 for slot 0
>> [ 7438.608567] Acking gen reason 20000000
>> [ 7438.608668] irq xmitstat 20fe1001 00000080
>> [ 7438.608709] irq xmitstat 20000011 00000080
>> [ 7438.608724] b43-phy2 debug: Out of order TX status report on DMA ring
>> 1. Expected 256, but got 0
>> [ 7438.608739] irq xmitstat 20020011 00000080
>> [ 7438.608750] b43-phy2 debug: Out of order TX status report on DMA ring
>> 1. Expected 256, but got 2
>> [ 7438.608765] irq xmitstat 20040011 00000080
>>
>> We write 0xff0 to the TXADDRLO register to see if the DMA engine is
>> capable of unaligned operation. If it *is*, then it'll have this problem
>> and we have to write the full address to TXINDEX. Comments in brcmsmac
>> indicate that the low 13 bits are required.
>>
>> If we're doing this, we *also* have to write to TXCTL to enable the DMA
>> engine *after* setting up the ring address.
>>
>> Signed-off-by: David Woodhouse<David.Woodhouse@intel.com>
>> --
>> I've made that change to the initialisation order of TXCTL vs.
>> TXADDR{HI,LO} unconditional; is there a reason not to?
>>
>> diff --git a/drivers/net/wireless/b43/dma.c
>> b/drivers/net/wireless/b43/dma.c
>> index 82168f8..92dd6d9 100644
>> --- a/drivers/net/wireless/b43/dma.c
>> +++ b/drivers/net/wireless/b43/dma.c
>> @@ -225,8 +225,10 @@ static void op64_fill_descriptor(struct b43_dmaring
>> *ring,
>>
>> ?static void op64_poke_tx(struct b43_dmaring *ring, int slot)
>> ?{
>> - ? ? ? b43_dma_write(ring, B43_DMA64_TXINDEX,
>> - ? ? ? ? ? ? ? ? ? ? (u32) (slot * sizeof(struct b43_dmadesc64)));
>> + ? ? ? u32 indexval = slot * sizeof(struct b43_dmadesc64);
>> + ? ? ? if (ring->unaligned)
>> + ? ? ? ? ? ? ? indexval |= (u32)ring->dmabase;
>> + ? ? ? b43_dma_write(ring, B43_DMA64_TXINDEX, indexval);
>> ?}
>>
>> ?static void op64_tx_suspend(struct b43_dmaring *ring)
>> @@ -704,9 +710,14 @@ static int dmacontroller_setup(struct b43_dmaring
>> *ring)
>> ? ? ? ? ? ? ? ? ? ? ? ?& ?B43_DMA64_TXADDREXT_MASK;
>> ? ? ? ? ? ? ? ? ? ? ? ?if (!parity)
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?value |= B43_DMA64_TXPARITYDISABLE;
>> - ? ? ? ? ? ? ? ? ? ? ? b43_dma_write(ring, B43_DMA64_TXCTL, value);
>> +
>> + ? ? ? ? ? ? ? ? ? ? ? b43_dma_write(ring, B43_DMA64_TXRINGLO, 0xff0);
>
> Should there be a new symbol rather than this magic number?
>
>> + ? ? ? ? ? ? ? ? ? ? ? if (b43_dma_read(ring, B43_DMA64_TXRINGLO))
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ring->unaligned = 1;
>> +
>> ? ? ? ? ? ? ? ? ? ? ? ?b43_dma_write(ring, B43_DMA64_TXRINGLO, addrlo);
>> ? ? ? ? ? ? ? ? ? ? ? ?b43_dma_write(ring, B43_DMA64_TXRINGHI, addrhi);
>> + ? ? ? ? ? ? ? ? ? ? ? b43_dma_write(ring, B43_DMA64_TXCTL, value);
>> ? ? ? ? ? ? ? ?} else {
>> ? ? ? ? ? ? ? ? ? ? ? ?u32 ringbase = (u32) (ring->dmabase);
>> ? ? ? ? ? ? ? ? ? ? ? ?addrext = b43_dma_address(&ring->dev->dma,
>> ringbase, B43_DMA_ADDR_EXT);
>> diff --git a/drivers/net/wireless/b43/dma.h
>> b/drivers/net/wireless/b43/dma.h
>> index 7e20b04f..16dc565 100644
>> --- a/drivers/net/wireless/b43/dma.h
>> +++ b/drivers/net/wireless/b43/dma.h
>> @@ -251,6 +251,8 @@ struct b43_dmaring {
>> ? ? ? ?int index;
>> ? ? ? ?/* Boolean. Is this a TX ring? */
>> ? ? ? ?bool tx;
>> + ? ? ? /* Boolean. Is this ring capable of 16-byte alignment? */
>> + ? ? ? bool unaligned;
>> ? ? ? ?/* The type of DMA engine used. */
>> ? ? ? ?enum b43_dmatype type;
>> ? ? ? ?/* Boolean. Is this ring stopped at ieee80211 level? */
>>
>
> Tested on BCM4318 on 32-bit system.

The question is if your card supports unaligned (and as result, if you
tested it at all)

-- 
Rafa?

[PATCH] Fix alignment issues with DMA TX on BCM4331

[David Woodhouse]

On Sun, 2011-08-14 at 00:02 +0200, Rafa? Mi?ecki wrote:
> W dniu 13 sierpnia 2011 23:57 u?ytkownik Larry Finger
> <Larry.Finger@lwfinger.net> napisa?:
> >> +
> >> +                       b43_dma_write(ring, B43_DMA64_TXRINGLO, 0xff0);
> >
> > Should there be a new symbol rather than this magic number?

Probably not. We're testing whether it stores certain bits of the
address, or if the low 12 bits of the register are just hard-coded to
zero. The raw value 0xff0 conveys that as well as any macro name that I
could think of.

> > Tested on BCM4318 on 32-bit system.
> 
> The question is if your card supports unaligned (and as result, if you
> tested it at all)

Either way, he confirmed that writing TXCTL last works.

-- 
dwmw2

[PATCH] Fix alignment issues with DMA TX on BCM4331

[Rafał Miłecki]

W dniu 14 sierpnia 2011 00:07 u?ytkownik David Woodhouse
<dwmw2@infradead.org> napisa?:
> On Sun, 2011-08-14 at 00:02 +0200, Rafa? Mi?ecki wrote:
>> W dniu 13 sierpnia 2011 23:57 u?ytkownik Larry Finger
>> <Larry.Finger@lwfinger.net> napisa?:
>> >> +
>> >> + ? ? ? ? ? ? ? ? ? ? ? b43_dma_write(ring, B43_DMA64_TXRINGLO, 0xff0);
>> >
>> > Should there be a new symbol rather than this magic number?
>
> Probably not. We're testing whether it stores certain bits of the
> address, or if the low 12 bits of the register are just hard-coded to
> zero. The raw value 0xff0 conveys that as well as any macro name that I
> could think of.
>
>> > Tested on BCM4318 on 32-bit system.
>>
>> The question is if your card supports unaligned (and as result, if you
>> tested it at all)
>
> Either way, he confirmed that writing TXCTL last works.

I've just checked wl and they don't do that anyway. The keep order
depending on the alignment bool.

-- 
Rafa?

[PATCH] Fix alignment issues with DMA TX on BCM4331

[David Woodhouse]

On Sun, 2011-08-14 at 00:12 +0200, Rafa? Mi?ecki wrote:
> I've just checked wl and they don't do that anyway. The keep order
> depending on the alignment bool. 

Ah, OK. So they still write TXCTL first, if the device can't support
unaligned descriptors?

The brcm80211 driver doesn't; it writes TXCTL last, unconditionally.

Do you want me to send another patch which makes it conditional? It
complicates the code with something that might be unnecessary legacy
baggage... but means we don't have to be worried about regressions on
older hardware.

-- 
dwmw2

[PATCH] Fix alignment issues with DMA TX on BCM4331

[Rafał Miłecki]

W dniu 14 sierpnia 2011 00:17 u?ytkownik David Woodhouse
<dwmw2@infradead.org> napisa?:
> On Sun, 2011-08-14 at 00:12 +0200, Rafa? Mi?ecki wrote:
>> I've just checked wl and they don't do that anyway. The keep order
>> depending on the alignment bool.
>
> Ah, OK. So they still write TXCTL first, if the device can't support
> unaligned descriptors?
>
> The brcm80211 driver doesn't; it writes TXCTL last, unconditionally.
>
> Do you want me to send another patch which makes it conditional? It
> complicates the code with something that might be unnecessary legacy
> baggage... but means we don't have to be worried about regressions on
> older hardware.

Here you have a log from wl from 14e4:432b:

write32 0xfaafc208 <- 0x00000ff0
 read32 0xfaafc208 -> 0x00000000
[that means no unaligned support]
...
 read32 0xfaafc200 -> 0x00000000
write32 0xfaafc200 <- 0x00000801
write32 0xfaafc208 <- 0x1b550000
write32 0xfaafc20c <- 0x80000000
[when no unaligned, first enable, then set addresses]

-- 
Rafa?

[PATCH] Fix alignment issues with DMA TX on BCM4331

[Rafał Miłecki]

W dniu 14 sierpnia 2011 00:17 u?ytkownik David Woodhouse
<dwmw2@infradead.org> napisa?:
> On Sun, 2011-08-14 at 00:12 +0200, Rafa? Mi?ecki wrote:
>> I've just checked wl and they don't do that anyway. The keep order
>> depending on the alignment bool.
>
> Ah, OK. So they still write TXCTL first, if the device can't support
> unaligned descriptors?
>
> The brcm80211 driver doesn't; it writes TXCTL last, unconditionally.

Uh, you mead me read that code... OK:

if (!di->aligndesc_4k)
	_dma_ddtable_init(di, DMA_TX, di->txdpa);

if ((di->dma.dmactrlflags & DMA_CTRL_PEN) == 0)
	control |= D64_XC_PD;
OR_REG(&di->d64txregs->control, control);

if (di->aligndesc_4k)
	_dma_ddtable_init(di, DMA_TX, di->txdpa);

I stripped the comments, which are simply wrong (copy&paste gone too fast).

Following is RX part, with the sane comment this time:

/* DMA engine with out alignment requirement requires table to be inited
 * before enabling the engine
 */
if (!di->aligndesc_4k)
	_dma_ddtable_init(di, DMA_RX, di->rxdpa);

_dma_rxenable(di);

if (di->aligndesc_4k)
	_dma_ddtable_init(di, DMA_RX, di->rxdpa);

-- 
Rafa?

[PATCH] Fix alignment issues with DMA TX on BCM4331

[David Woodhouse]

On Sun, 2011-08-14 at 09:35 +0200, Rafa? Mi?ecki wrote:
> 
> > The brcm80211 driver doesn't; it writes TXCTL last, unconditionally.
> 
> Uh, you mead me read that code... OK:
> 
> if (!di->aligndesc_4k)
>         _dma_ddtable_init(di, DMA_TX, di->txdpa);
> 
> if ((di->dma.dmactrlflags & DMA_CTRL_PEN) == 0)
>         control |= D64_XC_PD;
> OR_REG(&di->d64txregs->control, control);
> 
> if (di->aligndesc_4k)
>         _dma_ddtable_init(di, DMA_TX, di->txdpa); 

Hm, so I lied. Sorry about that. Will send a new patch.

I'll also have another look at the RX path. I had tried enabling RX
after setting the addresses, but that had failed.

-- 
dwmw2

[PATCH] Fix alignment issues with DMA TX on BCM4331

[Rafał Miłecki]

W dniu 14 sierpnia 2011 10:04 u?ytkownik David Woodhouse
<dwmw2@infradead.org> napisa?:
> On Sun, 2011-08-14 at 09:35 +0200, Rafa? Mi?ecki wrote:
>>
>> > The brcm80211 driver doesn't; it writes TXCTL last, unconditionally.
>>
>> Uh, you mead me read that code... OK:
>>
>> if (!di->aligndesc_4k)
>> ? ? ? ? _dma_ddtable_init(di, DMA_TX, di->txdpa);
>>
>> if ((di->dma.dmactrlflags & DMA_CTRL_PEN) == 0)
>> ? ? ? ? control |= D64_XC_PD;
>> OR_REG(&di->d64txregs->control, control);
>>
>> if (di->aligndesc_4k)
>> ? ? ? ? _dma_ddtable_init(di, DMA_TX, di->txdpa);
>
> Hm, so I lied. Sorry about that. Will send a new patch.
>
> I'll also have another look at the RX path. I had tried enabling RX
> after setting the addresses, but that had failed.

Dump from ndis on BCM4331 for RX ring:
write32 0xb0600228 <- 0x1f85e000	ADDR_LO
write32 0xb060022c <- 0x80000000	ADDR_HI
 read32 0xb0600220 -> 0x00000040	CTL (Frame Offset: 0x20)
write32 0xb0600220 <- 0x0000084d	CTL (Frame Offset: 0x26 | Parity Disable)
write32 0xb0600224 <- 0x1f85e200	DESC_STOP

-- 
Rafa?

[PATCH] Fix alignment issues with DMA TX on BCM4331

[Larry Finger]

On 08/13/2011 05:02 PM, Rafa? Mi?ecki wrote:
> W dniu 13 sierpnia 2011 23:57 u?ytkownik Larry Finger
> <Larry.Finger@lwfinger.net>  napisa?:
>> On 08/12/2011 05:27 AM, David Woodhouse wrote:
>>>
>>> When the TX descriptor ring is not aligned to 8KiB on BCM4331, we have
>>> to write the full address to the TXINDEX register or the DMA engine gets
>>> confused after the first time we wrap round to slot zero.
>>>
>>> [ 7438.538945] Poked TX with address fe0 for slot 254
>>> [ 7438.539077] Acking gen reason 20000000
>>> [ 7438.539177] irq xmitstat 20fc1001 0000007f
>>> [ 7438.607861] Poked TX with address 0 for slot 0
>>> [ 7438.608567] Acking gen reason 20000000
>>> [ 7438.608668] irq xmitstat 20fe1001 00000080
>>> [ 7438.608709] irq xmitstat 20000011 00000080
>>> [ 7438.608724] b43-phy2 debug: Out of order TX status report on DMA ring
>>> 1. Expected 256, but got 0
>>> [ 7438.608739] irq xmitstat 20020011 00000080
>>> [ 7438.608750] b43-phy2 debug: Out of order TX status report on DMA ring
>>> 1. Expected 256, but got 2
>>> [ 7438.608765] irq xmitstat 20040011 00000080
>>>
>>> We write 0xff0 to the TXADDRLO register to see if the DMA engine is
>>> capable of unaligned operation. If it *is*, then it'll have this problem
>>> and we have to write the full address to TXINDEX. Comments in brcmsmac
>>> indicate that the low 13 bits are required.
>>>
>>> If we're doing this, we *also* have to write to TXCTL to enable the DMA
>>> engine *after* setting up the ring address.
>>>
>>> Signed-off-by: David Woodhouse<David.Woodhouse@intel.com>
>>> --
>>> I've made that change to the initialisation order of TXCTL vs.
>>> TXADDR{HI,LO} unconditional; is there a reason not to?
>>>
>>> diff --git a/drivers/net/wireless/b43/dma.c
>>> b/drivers/net/wireless/b43/dma.c
>>> index 82168f8..92dd6d9 100644
>>> --- a/drivers/net/wireless/b43/dma.c
>>> +++ b/drivers/net/wireless/b43/dma.c
>>> @@ -225,8 +225,10 @@ static void op64_fill_descriptor(struct b43_dmaring
>>> *ring,
>>>
>>>   static void op64_poke_tx(struct b43_dmaring *ring, int slot)
>>>   {
>>> -       b43_dma_write(ring, B43_DMA64_TXINDEX,
>>> -                     (u32) (slot * sizeof(struct b43_dmadesc64)));
>>> +       u32 indexval = slot * sizeof(struct b43_dmadesc64);
>>> +       if (ring->unaligned)
>>> +               indexval |= (u32)ring->dmabase;
>>> +       b43_dma_write(ring, B43_DMA64_TXINDEX, indexval);
>>>   }
>>>
>>>   static void op64_tx_suspend(struct b43_dmaring *ring)
>>> @@ -704,9 +710,14 @@ static int dmacontroller_setup(struct b43_dmaring
>>> *ring)
>>>                         &    B43_DMA64_TXADDREXT_MASK;
>>>                         if (!parity)
>>>                                 value |= B43_DMA64_TXPARITYDISABLE;
>>> -                       b43_dma_write(ring, B43_DMA64_TXCTL, value);
>>> +
>>> +                       b43_dma_write(ring, B43_DMA64_TXRINGLO, 0xff0);
>>
>> Should there be a new symbol rather than this magic number?
>>
>>> +                       if (b43_dma_read(ring, B43_DMA64_TXRINGLO))
>>> +                               ring->unaligned = 1;
>>> +
>>>                         b43_dma_write(ring, B43_DMA64_TXRINGLO, addrlo);
>>>                         b43_dma_write(ring, B43_DMA64_TXRINGHI, addrhi);
>>> +                       b43_dma_write(ring, B43_DMA64_TXCTL, value);
>>>                 } else {
>>>                         u32 ringbase = (u32) (ring->dmabase);
>>>                         addrext = b43_dma_address(&ring->dev->dma,
>>> ringbase, B43_DMA_ADDR_EXT);
>>> diff --git a/drivers/net/wireless/b43/dma.h
>>> b/drivers/net/wireless/b43/dma.h
>>> index 7e20b04f..16dc565 100644
>>> --- a/drivers/net/wireless/b43/dma.h
>>> +++ b/drivers/net/wireless/b43/dma.h
>>> @@ -251,6 +251,8 @@ struct b43_dmaring {
>>>         int index;
>>>         /* Boolean. Is this a TX ring? */
>>>         bool tx;
>>> +       /* Boolean. Is this ring capable of 16-byte alignment? */
>>> +       bool unaligned;
>>>         /* The type of DMA engine used. */
>>>         enum b43_dmatype type;
>>>         /* Boolean. Is this ring stopped at ieee80211 level? */
>>>
>>
>> Tested on BCM4318 on 32-bit system.
>
> The question is if your card supports unaligned (and as result, if you
> tested it at all)

I should have made it clearer. My card does 32-bit DMA, thus this code does not 
change anything in its code. What I tested is that the patch did not break 
operations on my card due to some unforeseen complication.

Larry

[PATCH] Fix alignment issues with DMA TX on BCM4331

[David Woodhouse]

On Sat, 2011-08-13 at 17:14 -0500, Larry Finger wrote:
> I should have made it clearer. My card does 32-bit DMA, thus this code does not 
> change anything in its code. What I tested is that the patch did not break 
> operations on my card due to some unforeseen complication. 

Oh, OK. I thought BCM4318 was new enough to do 64-bit DMA. Which devices
do we need to test on, then?

-- 
dwmw2

[PATCH] Fix alignment issues with DMA TX on BCM4331

[Larry Finger]

On 08/13/2011 05:17 PM, David Woodhouse wrote:
> On Sat, 2011-08-13 at 17:14 -0500, Larry Finger wrote:
>> I should have made it clearer. My card does 32-bit DMA, thus this code does not
>> change anything in its code. What I tested is that the patch did not break
>> operations on my card due to some unforeseen complication.
>
> Oh, OK. I thought BCM4318 was new enough to do 64-bit DMA. Which devices
> do we need to test on, then?

I need to test with my BCM4311, which was the first card to do 64-bit DMA. I'll 
have to reboot to do that, which is not convenient now. I'll do it within the 
next 18 hours.

Larry