2011-07-19 21:33:44

by Rafał Miłecki

[permalink] [raw]
Subject: [PATCH 0/3] b43 & bcma: DMA special (translation) bits

Thanks to Pavel's comments I digged into translation bits and found out
the hackish way we currently use to handle 32-bit and 64-bit DMA.

I've replaced out bit shifting hacks with just a one, documented in the
comment. This workaround ideally should be just fixed, but I'm not going
to blidly change the current behaviour of ssb_dma_translation without
testing that with b43legacy and b44. As I don't have such a hardware,
we will need to find some testers.

In any case, the way we changed the code should be correct. When ssb
function will be fixed, b43 will only require deletion of 2 code lines.

I've tested this code for regressions on my following cards:
1) 14e4:432b (64-bit DMA)
2) 14e4:4315 (64-bit DMA)
3) 14e4:4312 (32-bit DMA)
Transmission is still possible on all of them, no errors were noticed.


Pavel: sorry for CC-ing you with my previous serie. Fortunately this
lead to the much nicer solution, thanks for your help.
I let myself yo CC you once more, as this patch serie tries to fix
issues you pointed to me.

Rafał Miłecki (3):
b43: replace DMA translation workarounds with just a one, commented
bcma: inform drivers about translation bits needed for the core
b43: bcma: get DMA translation bits

drivers/bcma/core.c | 16 ++++++++++++++++
drivers/net/wireless/b43/dma.c | 17 ++++++++++++++---
drivers/net/wireless/b43/dma.h | 4 ++++
include/linux/bcma/bcma.h | 5 +++++
4 files changed, 39 insertions(+), 3 deletions(-)

--
1.7.3.4



2011-07-19 21:33:50

by Rafał Miłecki

[permalink] [raw]
Subject: [PATCH 2/3] bcma: inform drivers about translation bits needed for the core

When using DMA, drivers need to pass special translation info to the
hardware.

Signed-off-by: Rafał Miłecki <[email protected]>
---
drivers/bcma/core.c | 16 ++++++++++++++++
include/linux/bcma/bcma.h | 5 +++++
2 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/drivers/bcma/core.c b/drivers/bcma/core.c
index 2d8506d..4a04a49 100644
--- a/drivers/bcma/core.c
+++ b/drivers/bcma/core.c
@@ -106,3 +106,19 @@ void bcma_core_pll_ctl(struct bcma_device *core, u32 req, u32 status, bool on)
}
}
EXPORT_SYMBOL_GPL(bcma_core_pll_ctl);
+
+u32 bcma_core_dma_translation(struct bcma_device *core)
+{
+ switch (core->bus->hosttype) {
+ case BCMA_HOSTTYPE_PCI:
+ if (bcma_aread32(core, BCMA_IOST) & BCMA_IOST_DMA64)
+ return BCMA_DMA_TRANSLATION_DMA64_CMT;
+ else
+ return BCMA_DMA_TRANSLATION_DMA32_CMT;
+ default:
+ pr_err("DMA translation unknown for host %d\n",
+ core->bus->hosttype);
+ }
+ return BCMA_DMA_TRANSLATION_NONE;
+}
+EXPORT_SYMBOL(bcma_core_dma_translation);
diff --git a/include/linux/bcma/bcma.h b/include/linux/bcma/bcma.h
index cc1582d..8c96654 100644
--- a/include/linux/bcma/bcma.h
+++ b/include/linux/bcma/bcma.h
@@ -262,5 +262,10 @@ extern void bcma_core_set_clockmode(struct bcma_device *core,
enum bcma_clkmode clkmode);
extern void bcma_core_pll_ctl(struct bcma_device *core, u32 req, u32 status,
bool on);
+#define BCMA_DMA_TRANSLATION_MASK 0xC0000000
+#define BCMA_DMA_TRANSLATION_NONE 0x00000000
+#define BCMA_DMA_TRANSLATION_DMA32_CMT 0x40000000 /* Client Mode Translation for 32-bit DMA */
+#define BCMA_DMA_TRANSLATION_DMA64_CMT 0x80000000 /* Client Mode Translation for 64-bit DMA */
+extern u32 bcma_core_dma_translation(struct bcma_device *core);

#endif /* LINUX_BCMA_H_ */
--
1.7.3.4


2011-07-19 22:53:17

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH 1/3] b43: replace DMA translation workarounds with just a one, commented

On 07/19/2011 05:12 PM, Rafał Miłecki wrote:
> Routing bits are always placed in 0xC0000000, there is no any special
> bit shifting for 64-bit DMA. We just workaround wrong value (passed by
> ssb) in this way. Replace that with just a one workaround and comment
> it. Real fix for this requires testing with all ssb drivers.

If you wish to supply the real patch, I can test b43legacy.

Larry

2011-07-19 23:15:16

by Michael Büsch

[permalink] [raw]
Subject: Re: [PATCH 1/3] b43: replace DMA translation workarounds with just a one, commented

On Wed, 20 Jul 2011 00:12:20 +0200
Rafał Miłecki <[email protected]> wrote:
> + * be fixed on ssb side, but requires testing with b43,
> + * b43legacy and b44. */

No it doesn't. b44 and b43legacy don't use 64bit DMA.
Just fix it in ssb, please.

2011-07-20 08:30:16

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH 1/3] b43: replace DMA translation workarounds with just a one, commented

On 07/20/2011 01:16 AM, Rafał Miłecki wrote:
> W dniu 20 lipca 2011 01:15 użytkownik Michael Büsch<[email protected]> napisał:
>> On Wed, 20 Jul 2011 00:12:20 +0200
>> Rafał Miłecki<[email protected]> wrote:
>>> + * be fixed on ssb side, but requires testing with b43,
>>> + * b43legacy and b44. */
>>
>> No it doesn't. b44 and b43legacy don't use 64bit DMA.
>> Just fix it in ssb, please.
>
> They (drivers) don't, but what if we start giving them routing for
> 64-bit DMA? AFAIU they treat 64-bit DMA as 32-bit one (according to
> specs: "If 64 Bit isn't an option, Silicon Backplane and PCI-E buses
> can use 32 bit DMA.").
>
> Won't they start using 64-bit DMA in the 32-bit way but with 64-bit
> routing bits?

The BCM4311 was the first device that supported 64-bit DMA. I had the first of
those, and I got to debug the 64-bit stuff that had been written by Michael, but
there had been no hardware for testing. All earlier devices were restricted to
32-bit paths.

Larry

2011-07-20 06:16:09

by Rafał Miłecki

[permalink] [raw]
Subject: Re: [PATCH 1/3] b43: replace DMA translation workarounds with just a one, commented

W dniu 20 lipca 2011 01:15 użytkownik Michael Büsch <[email protected]> napisał:
> On Wed, 20 Jul 2011 00:12:20 +0200
> Rafał Miłecki <[email protected]> wrote:
>> +              * be fixed on ssb side, but requires testing with b43,
>> +              * b43legacy and b44. */
>
> No it doesn't. b44 and b43legacy don't use 64bit DMA.
> Just fix it in ssb, please.

They (drivers) don't, but what if we start giving them routing for
64-bit DMA? AFAIU they treat 64-bit DMA as 32-bit one (according to
specs: "If 64 Bit isn't an option, Silicon Backplane and PCI-E buses
can use 32 bit DMA.").

Won't they start using 64-bit DMA in the 32-bit way but with 64-bit
routing bits?

--
Rafał

2011-07-19 21:33:53

by Rafał Miłecki

[permalink] [raw]
Subject: [PATCH 3/3] b43: bcma: get DMA translation bits

Add some missing defines by the way.

Signed-off-by: Rafał Miłecki <[email protected]>
---
drivers/net/wireless/b43/dma.c | 5 +++++
drivers/net/wireless/b43/dma.h | 4 ++++
2 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/b43/dma.c b/drivers/net/wireless/b43/dma.c
index 530a153..0a5004b 100644
--- a/drivers/net/wireless/b43/dma.c
+++ b/drivers/net/wireless/b43/dma.c
@@ -1057,6 +1057,11 @@ int b43_dma_init(struct b43_wldev *dev)
return err;

switch (dev->dev->bus_type) {
+#ifdef CONFIG_B43_BCMA
+ case B43_BUS_BCMA:
+ dma->translation = bcma_core_dma_translation(dev->dev->bdev);
+ break;
+#endif
#ifdef CONFIG_B43_SSB
case B43_BUS_SSB:
dma->translation = ssb_dma_translation(dev->dev->sdev);
diff --git a/drivers/net/wireless/b43/dma.h b/drivers/net/wireless/b43/dma.h
index e8a80a1..cdf8709 100644
--- a/drivers/net/wireless/b43/dma.h
+++ b/drivers/net/wireless/b43/dma.h
@@ -20,6 +20,7 @@
#define B43_DMA32_TXSUSPEND 0x00000002
#define B43_DMA32_TXLOOPBACK 0x00000004
#define B43_DMA32_TXFLUSH 0x00000010
+#define B43_DMA32_TXPARITYDISABLE 0x00000800
#define B43_DMA32_TXADDREXT_MASK 0x00030000
#define B43_DMA32_TXADDREXT_SHIFT 16
#define B43_DMA32_TXRING 0x04
@@ -44,6 +45,7 @@
#define B43_DMA32_RXFROFF_MASK 0x000000FE
#define B43_DMA32_RXFROFF_SHIFT 1
#define B43_DMA32_RXDIRECTFIFO 0x00000100
+#define B43_DMA32_RXPARITYDISABLE 0x00000800
#define B43_DMA32_RXADDREXT_MASK 0x00030000
#define B43_DMA32_RXADDREXT_SHIFT 16
#define B43_DMA32_RXRING 0x14
@@ -84,6 +86,7 @@ struct b43_dmadesc32 {
#define B43_DMA64_TXSUSPEND 0x00000002
#define B43_DMA64_TXLOOPBACK 0x00000004
#define B43_DMA64_TXFLUSH 0x00000010
+#define B43_DMA64_TXPARITYDISABLE 0x00000800
#define B43_DMA64_TXADDREXT_MASK 0x00030000
#define B43_DMA64_TXADDREXT_SHIFT 16
#define B43_DMA64_TXINDEX 0x04
@@ -111,6 +114,7 @@ struct b43_dmadesc32 {
#define B43_DMA64_RXFROFF_MASK 0x000000FE
#define B43_DMA64_RXFROFF_SHIFT 1
#define B43_DMA64_RXDIRECTFIFO 0x00000100
+#define B43_DMA64_RXPARITYDISABLE 0x00000800
#define B43_DMA64_RXADDREXT_MASK 0x00030000
#define B43_DMA64_RXADDREXT_SHIFT 16
#define B43_DMA64_RXINDEX 0x24
--
1.7.3.4


2011-07-20 11:10:55

by Rafał Miłecki

[permalink] [raw]
Subject: [PATCH 1/3] b43: replace DMA translation workarounds with just a one, commented

On Jul 20, 2011 10:55 AM, "Michael Büsch" <[email protected]> wrote:
>
> On Wed, 20 Jul 2011 08:16:08 +0200
> Rafał Miłecki <[email protected]> wrote:
>
> > W dniu 20 lipca 2011 01:15 użytkownik Michael Büsch <[email protected]> napisał:
> > > On Wed, 20 Jul 2011 00:12:20 +0200
> > > Rafał Miłecki <[email protected]> wrote:
> > >> +              * be fixed on ssb side, but requires testing with b43,
> > >> +              * b43legacy and b44. */
> > >
> > > No it doesn't. b44 and b43legacy don't use 64bit DMA.
> > > Just fix it in ssb, please.
> >
> > They (drivers) don't, but what if we start giving them routing for
> > 64-bit DMA? AFAIU they treat 64-bit DMA as 32-bit one (according to
> > specs: "If 64 Bit isn't an option, Silicon Backplane and PCI-E buses
> > can use 32 bit DMA.").
> >
> > Won't they start using 64-bit DMA in the 32-bit way but with 64-bit
> > routing bits?
> >
>
> I have no idea what you're talking about.
> The fact is: These "temporary workarounds" tend to stay in the driver forever
> if we don't fix it _now_. So please fix it now.
> We know whether we are on 64bit DMA or not. So if we are on 64bit DMA, use the 64bit mask.
> Simply pass the "32 or 64 bit" boolean flag to ssb_dma_translation() as parameter.
> There's nothing that can go wrong here with older drivers.

Sorry, for not explaining this well eno
ugh. I'm afraid of situation where b44 driver supports 64-bit DMA
device *without* handling this as such a device. It treats it as
32-bit one and it works thanks to kind of ssb's fallback described in
the specs. Now if I clean ssb_dma_translation we will detect that DMA
is 64-bit and we will pass different routing bit.

So as the result b44 will still treat DMA as 32-bit one, but it will
start using routing for 64-bit one.
Alternative is to do not detect kind of DMA in the ssb_dma_translation
but take it as a parameter (as you suggested). A *little* more to care
about on the driver side but would make a trick.

Or... is there any b44 supported device with 64-bit DMA at all?


--
Rafał

2011-07-20 08:55:47

by Michael Büsch

[permalink] [raw]
Subject: Re: [PATCH 1/3] b43: replace DMA translation workarounds with just a one, commented

On Wed, 20 Jul 2011 08:16:08 +0200
Rafał Miłecki <[email protected]> wrote:

> W dniu 20 lipca 2011 01:15 użytkownik Michael Büsch <[email protected]> napisał:
> > On Wed, 20 Jul 2011 00:12:20 +0200
> > Rafał Miłecki <[email protected]> wrote:
> >> +              * be fixed on ssb side, but requires testing with b43,
> >> +              * b43legacy and b44. */
> >
> > No it doesn't. b44 and b43legacy don't use 64bit DMA.
> > Just fix it in ssb, please.
>
> They (drivers) don't, but what if we start giving them routing for
> 64-bit DMA? AFAIU they treat 64-bit DMA as 32-bit one (according to
> specs: "If 64 Bit isn't an option, Silicon Backplane and PCI-E buses
> can use 32 bit DMA.").
>
> Won't they start using 64-bit DMA in the 32-bit way but with 64-bit
> routing bits?
>

I have no idea what you're talking about.
The fact is: These "temporary workarounds" tend to stay in the driver forever
if we don't fix it _now_. So please fix it now.
We know whether we are on 64bit DMA or not. So if we are on 64bit DMA, use the 64bit mask.
Simply pass the "32 or 64 bit" boolean flag to ssb_dma_translation() as parameter.
There's nothing that can go wrong here with older drivers.

2011-07-20 15:31:38

by Michael Büsch

[permalink] [raw]
Subject: Re: [PATCH 1/3] b43: replace DMA translation workarounds with just a one, commented

On Wed, 20 Jul 2011 13:10:55 +0200
Rafał Miłecki <[email protected]> wrote:

> Sorry, for not explaining this well eno
> ugh. I'm afraid of situation where b44 driver supports 64-bit DMA
> device *without* handling this as such a device.

A b44 or b43legacy device with 64bit DMA engine does not exist.

2011-07-19 21:33:47

by Rafał Miłecki

[permalink] [raw]
Subject: [PATCH 1/3] b43: replace DMA translation workarounds with just a one, commented

Routing bits are always placed in 0xC0000000, there is no any special
bit shifting for 64-bit DMA. We just workaround wrong value (passed by
ssb) in this way. Replace that with just a one workaround and comment
it. Real fix for this requires testing with all ssb drivers.

Signed-off-by: Rafał Miłecki <[email protected]>
---
drivers/net/wireless/b43/dma.c | 12 +++++++++---
1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/b43/dma.c b/drivers/net/wireless/b43/dma.c
index ce572ae..530a153 100644
--- a/drivers/net/wireless/b43/dma.c
+++ b/drivers/net/wireless/b43/dma.c
@@ -174,7 +174,7 @@ static void op64_fill_descriptor(struct b43_dmaring *ring,
addrhi = (((u64) dmaaddr >> 32) & ~SSB_DMA_TRANSLATION_MASK);
addrext = (((u64) dmaaddr >> 32) & SSB_DMA_TRANSLATION_MASK)
>> SSB_DMA_TRANSLATION_SHIFT;
- addrhi |= (ring->dev->dma.translation << 1);
+ addrhi |= ring->dev->dma.translation;
if (slot == ring->nr_slots - 1)
ctl0 |= B43_DMA64_DCTL0_DTABLEEND;
if (start)
@@ -675,7 +675,7 @@ static int dmacontroller_setup(struct b43_dmaring *ring)
b43_dma_write(ring, B43_DMA64_TXRINGHI,
((ringbase >> 32) &
~SSB_DMA_TRANSLATION_MASK)
- | (trans << 1));
+ | trans);
} else {
u32 ringbase = (u32) (ring->dmabase);

@@ -708,7 +708,7 @@ static int dmacontroller_setup(struct b43_dmaring *ring)
b43_dma_write(ring, B43_DMA64_RXRINGHI,
((ringbase >> 32) &
~SSB_DMA_TRANSLATION_MASK)
- | (trans << 1));
+ | trans);
b43_dma_write(ring, B43_DMA64_RXINDEX, ring->nr_slots *
sizeof(struct b43_dmadesc64));
} else {
@@ -1060,6 +1060,12 @@ int b43_dma_init(struct b43_wldev *dev)
#ifdef CONFIG_B43_SSB
case B43_BUS_SSB:
dma->translation = ssb_dma_translation(dev->dev->sdev);
+ /* ssb does not handle 64-bit DMA case, where Client Mode
+ * Translation is set with value 0x2 instead of 0x1. This should
+ * be fixed on ssb side, but requires testing with b43,
+ * b43legacy and b44. */
+ if (b43_read32(dev, SSB_TMSHIGH) & SSB_TMSHIGH_DMA64)
+ dma->translation <<= 1;
break;
#endif
}
--
1.7.3.4