* [PATCH 1/2] ssb: fix DMA translation for some specific boards
@ 2011-08-10 16:11 Rafał Miłecki
2011-08-10 16:11 ` [PATCH 2/2] b43: fix DMA on some bugged hardware Rafał Miłecki
2011-08-10 16:40 ` [PATCH 1/2] ssb: fix DMA translation for some specific boards Michael Büsch
0 siblings, 2 replies; 11+ messages in thread
From: Rafał Miłecki @ 2011-08-10 16:11 UTC (permalink / raw)
To: linux-wireless, John W. Linville
Cc: b43-dev, Rafał Miłecki, Michael Buesch
Signed-off-by: Michael Buesch <m@bues.ch>
Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
---
drivers/ssb/main.c | 22 +++++++++++++++++++---
1 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/drivers/ssb/main.c b/drivers/ssb/main.c
index 6ec6e09..dac2c0b 100644
--- a/drivers/ssb/main.c
+++ b/drivers/ssb/main.c
@@ -1260,16 +1260,32 @@ void ssb_device_disable(struct ssb_device *dev, u32 core_specific_flags)
}
EXPORT_SYMBOL(ssb_device_disable);
+/* Some chipsets need routing known for PCIe and 64-bit DMA */
+static bool ssb_dma_translation_special_bit(struct ssb_device *dev)
+{
+ u16 chip_id = dev->bus->chip_id;
+
+ if (dev->id.coreid == SSB_DEV_80211)
+ return (chip_id == 0x4322 || chip_id == 43221 ||
+ chip_id == 43231 || chip_id == 43222);
+ return 0;
+}
+
u32 ssb_dma_translation(struct ssb_device *dev)
{
switch (dev->bus->bustype) {
case SSB_BUSTYPE_SSB:
return 0;
case SSB_BUSTYPE_PCI:
- if (ssb_read32(dev, SSB_TMSHIGH) & SSB_TMSHIGH_DMA64)
+ if (pci_is_pcie(dev->bus->host_pci) &&
+ ssb_read32(dev, SSB_TMSHIGH) & SSB_TMSHIGH_DMA64) {
return SSB_PCIE_DMA_H32;
- else
- return SSB_PCI_DMA;
+ } else {
+ if (ssb_dma_translation_special_bit(dev))
+ return SSB_PCIE_DMA_H32;
+ else
+ return SSB_PCI_DMA;
+ }
default:
__ssb_dma_not_implemented(dev);
}
--
1.7.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/2] b43: fix DMA on some bugged hardware
2011-08-10 16:11 [PATCH 1/2] ssb: fix DMA translation for some specific boards Rafał Miłecki
@ 2011-08-10 16:11 ` Rafał Miłecki
2011-08-10 16:33 ` Michael Büsch
2011-08-10 16:40 ` [PATCH 1/2] ssb: fix DMA translation for some specific boards Michael Büsch
1 sibling, 1 reply; 11+ messages in thread
From: Rafał Miłecki @ 2011-08-10 16:11 UTC (permalink / raw)
To: linux-wireless, John W. Linville; +Cc: b43-dev, Rafał Miłecki
Some hardware with 64-bit DMA uses lower address word for setting
routing (translation) bit. Add workaround for such boards.
Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
---
drivers/net/wireless/b43/b43.h | 1 +
drivers/net/wireless/b43/dma.c | 52 ++++++++++++++++++++++++++++++----------
2 files changed, 40 insertions(+), 13 deletions(-)
diff --git a/drivers/net/wireless/b43/b43.h b/drivers/net/wireless/b43/b43.h
index c818b0b..3aee322 100644
--- a/drivers/net/wireless/b43/b43.h
+++ b/drivers/net/wireless/b43/b43.h
@@ -594,6 +594,7 @@ struct b43_dma {
struct b43_dmaring *rx_ring;
u32 translation; /* Routing bits */
+ bool translation_in_low; /* Should translation bit go into low addr? */
bool parity; /* Check for parity */
};
diff --git a/drivers/net/wireless/b43/dma.c b/drivers/net/wireless/b43/dma.c
index 0953ce1..9a2b678 100644
--- a/drivers/net/wireless/b43/dma.c
+++ b/drivers/net/wireless/b43/dma.c
@@ -174,7 +174,10 @@ 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;
+ if (ring->dev->dma.translation_in_low)
+ addrlo |= ring->dev->dma.translation;
+ else
+ addrhi |= ring->dev->dma.translation;
if (slot == ring->nr_slots - 1)
ctl0 |= B43_DMA64_DCTL0_DTABLEEND;
if (start)
@@ -656,10 +659,12 @@ static int alloc_initial_descbuffers(struct b43_dmaring *ring)
static int dmacontroller_setup(struct b43_dmaring *ring)
{
int err = 0;
+ int tmp;
u32 value;
u32 addrext;
u32 trans = ring->dev->dma.translation;
bool parity = ring->dev->dma.parity;
+ u32 addrs[2];
if (ring->tx) {
if (ring->type == B43_DMA_64BIT) {
@@ -673,12 +678,14 @@ static int dmacontroller_setup(struct b43_dmaring *ring)
if (!parity)
value |= B43_DMA64_TXPARITYDISABLE;
b43_dma_write(ring, B43_DMA64_TXCTL, value);
- b43_dma_write(ring, B43_DMA64_TXRINGLO,
- (ringbase & 0xFFFFFFFF));
- b43_dma_write(ring, B43_DMA64_TXRINGHI,
- ((ringbase >> 32) &
- ~SSB_DMA_TRANSLATION_MASK)
- | trans);
+
+ addrs[0] = ringbase & 0xFFFFFFFF;
+ addrs[1] = ringbase >> 32;
+ tmp = ring->dev->dma.translation_in_low ? 0 : 1;
+ addrs[tmp] &= ~SSB_DMA_TRANSLATION_MASK;
+ addrs[tmp] |= trans;
+ b43_dma_write(ring, B43_DMA64_TXRINGLO, addrs[0]);
+ b43_dma_write(ring, B43_DMA64_TXRINGHI, addrs[1]);
} else {
u32 ringbase = (u32) (ring->dmabase);
@@ -710,12 +717,15 @@ static int dmacontroller_setup(struct b43_dmaring *ring)
if (!parity)
value |= B43_DMA64_RXPARITYDISABLE;
b43_dma_write(ring, B43_DMA64_RXCTL, value);
- b43_dma_write(ring, B43_DMA64_RXRINGLO,
- (ringbase & 0xFFFFFFFF));
- b43_dma_write(ring, B43_DMA64_RXRINGHI,
- ((ringbase >> 32) &
- ~SSB_DMA_TRANSLATION_MASK)
- | trans);
+
+ addrs[0] = ringbase & 0xFFFFFFFF;
+ addrs[1] = ringbase >> 32;
+ tmp = ring->dev->dma.translation_in_low ? 0 : 1;
+ addrs[tmp] &= ~SSB_DMA_TRANSLATION_MASK;
+ addrs[tmp] |= trans;
+ b43_dma_write(ring, B43_DMA64_RXRINGLO, addrs[0]);
+ b43_dma_write(ring, B43_DMA64_RXRINGHI, addrs[1]);
+
b43_dma_write(ring, B43_DMA64_RXINDEX, ring->nr_slots *
sizeof(struct b43_dmadesc64));
} else {
@@ -1052,6 +1062,21 @@ static int b43_dma_set_mask(struct b43_wldev *dev, u64 mask)
return 0;
}
+/* Some hardware with 64-bit DMA seems to be bugged and looks for translation
+ * bit in low address word instead of high one.
+ */
+static bool b43_dma_translation_in_low_word(struct b43_wldev *dev)
+{
+#ifdef CONFIG_B43_SSB
+ if (dev->dev->bus_type == B43_BUS_SSB &&
+ dev->dev->sdev->bus->bustype == SSB_BUSTYPE_PCI &&
+ !(dev->dev->sdev->bus->host_pci->is_pcie &&
+ ssb_read32(dev->dev->sdev, SSB_TMSHIGH) & SSB_TMSHIGH_DMA64))
+ return 1;
+#endif
+ return 0;
+}
+
int b43_dma_init(struct b43_wldev *dev)
{
struct b43_dma *dma = &dev->dma;
@@ -1077,6 +1102,7 @@ int b43_dma_init(struct b43_wldev *dev)
break;
#endif
}
+ dma->translation_in_low = b43_dma_translation_in_low_word(dev);
dma->parity = true;
#ifdef CONFIG_B43_BCMA
--
1.7.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] b43: fix DMA on some bugged hardware
2011-08-10 16:11 ` [PATCH 2/2] b43: fix DMA on some bugged hardware Rafał Miłecki
@ 2011-08-10 16:33 ` Michael Büsch
2011-08-10 16:45 ` Rafał Miłecki
0 siblings, 1 reply; 11+ messages in thread
From: Michael Büsch @ 2011-08-10 16:33 UTC (permalink / raw)
To: Rafał Miłecki; +Cc: linux-wireless, John W. Linville, b43-dev
On Wed, 10 Aug 2011 18:11:28 +0200
> diff --git a/drivers/net/wireless/b43/dma.c b/drivers/net/wireless/b43/dma.c
> index 0953ce1..9a2b678 100644
> --- a/drivers/net/wireless/b43/dma.c
> +++ b/drivers/net/wireless/b43/dma.c
> @@ -174,7 +174,10 @@ 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;
> + if (ring->dev->dma.translation_in_low)
> + addrlo |= ring->dev->dma.translation;
> + else
> + addrhi |= ring->dev->dma.translation;
> if (slot == ring->nr_slots - 1)
> ctl0 |= B43_DMA64_DCTL0_DTABLEEND;
> if (start)
> @@ -656,10 +659,12 @@ static int alloc_initial_descbuffers(struct b43_dmaring *ring)
> static int dmacontroller_setup(struct b43_dmaring *ring)
> {
> int err = 0;
> + int tmp;
> u32 value;
> u32 addrext;
> u32 trans = ring->dev->dma.translation;
> bool parity = ring->dev->dma.parity;
> + u32 addrs[2];
>
> if (ring->tx) {
> if (ring->type == B43_DMA_64BIT) {
> @@ -673,12 +678,14 @@ static int dmacontroller_setup(struct b43_dmaring *ring)
> if (!parity)
> value |= B43_DMA64_TXPARITYDISABLE;
> b43_dma_write(ring, B43_DMA64_TXCTL, value);
> - b43_dma_write(ring, B43_DMA64_TXRINGLO,
> - (ringbase & 0xFFFFFFFF));
> - b43_dma_write(ring, B43_DMA64_TXRINGHI,
> - ((ringbase >> 32) &
> - ~SSB_DMA_TRANSLATION_MASK)
> - | trans);
> +
> + addrs[0] = ringbase & 0xFFFFFFFF;
> + addrs[1] = ringbase >> 32;
> + tmp = ring->dev->dma.translation_in_low ? 0 : 1;
> + addrs[tmp] &= ~SSB_DMA_TRANSLATION_MASK;
> + addrs[tmp] |= trans;
> + b43_dma_write(ring, B43_DMA64_TXRINGLO, addrs[0]);
> + b43_dma_write(ring, B43_DMA64_TXRINGHI, addrs[1]);
> } else {
> u32 ringbase = (u32) (ring->dmabase);
>
> @@ -710,12 +717,15 @@ static int dmacontroller_setup(struct b43_dmaring *ring)
> if (!parity)
> value |= B43_DMA64_RXPARITYDISABLE;
> b43_dma_write(ring, B43_DMA64_RXCTL, value);
> - b43_dma_write(ring, B43_DMA64_RXRINGLO,
> - (ringbase & 0xFFFFFFFF));
> - b43_dma_write(ring, B43_DMA64_RXRINGHI,
> - ((ringbase >> 32) &
> - ~SSB_DMA_TRANSLATION_MASK)
> - | trans);
> +
> + addrs[0] = ringbase & 0xFFFFFFFF;
> + addrs[1] = ringbase >> 32;
> + tmp = ring->dev->dma.translation_in_low ? 0 : 1;
> + addrs[tmp] &= ~SSB_DMA_TRANSLATION_MASK;
> + addrs[tmp] |= trans;
> + b43_dma_write(ring, B43_DMA64_RXRINGLO, addrs[0]);
> + b43_dma_write(ring, B43_DMA64_RXRINGHI, addrs[1]);
> +
> b43_dma_write(ring, B43_DMA64_RXINDEX, ring->nr_slots *
> sizeof(struct b43_dmadesc64));
> } else {
> @@ -1052,6 +1062,21 @@ static int b43_dma_set_mask(struct b43_wldev *dev, u64 mask)
> return 0;
> }
This doesn't look correct to me for several reasons:
In the fill-op the address is not masked correctly with the translation mask.
In both the fill-op and both ring setups, the actual address extension bits
are always taken from the address's high word. I guess the extension should
be the low word bits for devices where we use the low word. That's the only
thing that would make sense. But hey, it's not that we have sane hardware here.
So this has to be checked.
--
Greetings, Michael.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] ssb: fix DMA translation for some specific boards
2011-08-10 16:11 [PATCH 1/2] ssb: fix DMA translation for some specific boards Rafał Miłecki
2011-08-10 16:11 ` [PATCH 2/2] b43: fix DMA on some bugged hardware Rafał Miłecki
@ 2011-08-10 16:40 ` Michael Büsch
2011-08-10 16:42 ` Rafał Miłecki
1 sibling, 1 reply; 11+ messages in thread
From: Michael Büsch @ 2011-08-10 16:40 UTC (permalink / raw)
To: Rafał Miłecki; +Cc: linux-wireless, John W. Linville, b43-dev
On Wed, 10 Aug 2011 18:11:27 +0200
> +/* Some chipsets need routing known for PCIe and 64-bit DMA */
> +static bool ssb_dma_translation_special_bit(struct ssb_device *dev)
> +{
> + u16 chip_id = dev->bus->chip_id;
> +
> + if (dev->id.coreid == SSB_DEV_80211)
> + return (chip_id == 0x4322 || chip_id == 43221 ||
> + chip_id == 43231 || chip_id == 43222);
> + return 0;
> +}
ACK, apart from this coding style violation (missing curly braces).
--
Greetings, Michael.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] ssb: fix DMA translation for some specific boards
2011-08-10 16:40 ` [PATCH 1/2] ssb: fix DMA translation for some specific boards Michael Büsch
@ 2011-08-10 16:42 ` Rafał Miłecki
2011-08-10 16:44 ` Michael Büsch
0 siblings, 1 reply; 11+ messages in thread
From: Rafał Miłecki @ 2011-08-10 16:42 UTC (permalink / raw)
To: Michael Büsch; +Cc: linux-wireless, John W. Linville, b43-dev
W dniu 10 sierpnia 2011 18:40 użytkownik Michael Büsch <m@bues.ch> napisał:
> On Wed, 10 Aug 2011 18:11:27 +0200
>> +/* Some chipsets need routing known for PCIe and 64-bit DMA */
>> +static bool ssb_dma_translation_special_bit(struct ssb_device *dev)
>> +{
>> + u16 chip_id = dev->bus->chip_id;
>> +
>> + if (dev->id.coreid == SSB_DEV_80211)
>> + return (chip_id == 0x4322 || chip_id == 43221 ||
>> + chip_id == 43231 || chip_id == 43222);
>> + return 0;
>> +}
>
> ACK, apart from this coding style violation (missing curly braces).
>From kernel CodingStyle:
> Do not unnecessarily use braces where a single statement will do.
>
> if (condition)
> action();
I guess simple "return" (even when using line break) is still like
single statement...
Am I wrong?
--
Rafał
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] ssb: fix DMA translation for some specific boards
2011-08-10 16:42 ` Rafał Miłecki
@ 2011-08-10 16:44 ` Michael Büsch
2011-08-10 17:27 ` Johannes Berg
0 siblings, 1 reply; 11+ messages in thread
From: Michael Büsch @ 2011-08-10 16:44 UTC (permalink / raw)
To: Rafał Miłecki; +Cc: linux-wireless, John W. Linville, b43-dev
On Wed, 10 Aug 2011 18:42:59 +0200
Rafał Miłecki <zajec5@gmail.com> wrote:
> W dniu 10 sierpnia 2011 18:40 użytkownik Michael Büsch <m@bues.ch> napisał:
> > On Wed, 10 Aug 2011 18:11:27 +0200
> >> +/* Some chipsets need routing known for PCIe and 64-bit DMA */
> >> +static bool ssb_dma_translation_special_bit(struct ssb_device *dev)
> >> +{
> >> + u16 chip_id = dev->bus->chip_id;
> >> +
> >> + if (dev->id.coreid == SSB_DEV_80211)
> >> + return (chip_id == 0x4322 || chip_id == 43221 ||
> >> + chip_id == 43231 || chip_id == 43222);
> >> + return 0;
> >> +}
> >
> > ACK, apart from this coding style violation (missing curly braces).
>
> From kernel CodingStyle:
> > Do not unnecessarily use braces where a single statement will do.
> >
> > if (condition)
> > action();
>
> I guess simple "return" (even when using line break) is still like
> single statement...
> Am I wrong?
>
Last time I checked this only applied to single-line-statements.
Maybe it changed, though.
--
Greetings, Michael.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] b43: fix DMA on some bugged hardware
2011-08-10 16:33 ` Michael Büsch
@ 2011-08-10 16:45 ` Rafał Miłecki
2011-08-10 16:55 ` Michael Büsch
0 siblings, 1 reply; 11+ messages in thread
From: Rafał Miłecki @ 2011-08-10 16:45 UTC (permalink / raw)
To: Michael Büsch; +Cc: linux-wireless, John W. Linville, b43-dev
VyBkbml1IDEwIHNpZXJwbmlhIDIwMTEgMTg6MzMgdcW8eXRrb3duaWsgTWljaGFlbCBCw7xzY2gg
PG1AYnVlcy5jaD4gbmFwaXNhxYI6Cj4gT24gV2VkLCAxMCBBdWcgMjAxMSAxODoxMToyOCArMDIw
MAo+PiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9uZXQvd2lyZWxlc3MvYjQzL2RtYS5jIGIvZHJpdmVy
cy9uZXQvd2lyZWxlc3MvYjQzL2RtYS5jCj4+IGluZGV4IDA5NTNjZTEuLjlhMmI2NzggMTAwNjQ0
Cj4+IC0tLSBhL2RyaXZlcnMvbmV0L3dpcmVsZXNzL2I0My9kbWEuYwo+PiArKysgYi9kcml2ZXJz
L25ldC93aXJlbGVzcy9iNDMvZG1hLmMKPj4gQEAgLTE3NCw3ICsxNzQsMTAgQEAgc3RhdGljIHZv
aWQgb3A2NF9maWxsX2Rlc2NyaXB0b3Ioc3RydWN0IGI0M19kbWFyaW5nICpyaW5nLAo+PiDCoCDC
oCDCoCBhZGRyaGkgPSAoKCh1NjQpIGRtYWFkZHIgPj4gMzIpICYgflNTQl9ETUFfVFJBTlNMQVRJ
T05fTUFTSyk7Cj4+IMKgIMKgIMKgIGFkZHJleHQgPSAoKCh1NjQpIGRtYWFkZHIgPj4gMzIpICYg
U1NCX0RNQV9UUkFOU0xBVElPTl9NQVNLKQo+PiDCoCDCoCDCoCDCoCDCoCA+PiBTU0JfRE1BX1RS
QU5TTEFUSU9OX1NISUZUOwo+PiAtIMKgIMKgIGFkZHJoaSB8PSByaW5nLT5kZXYtPmRtYS50cmFu
c2xhdGlvbjsKPj4gKyDCoCDCoCBpZiAocmluZy0+ZGV2LT5kbWEudHJhbnNsYXRpb25faW5fbG93
KQo+PiArIMKgIMKgIMKgIMKgIMKgIMKgIGFkZHJsbyB8PSByaW5nLT5kZXYtPmRtYS50cmFuc2xh
dGlvbjsKPj4gKyDCoCDCoCBlbHNlCj4+ICsgwqAgwqAgwqAgwqAgwqAgwqAgYWRkcmhpIHw9IHJp
bmctPmRldi0+ZG1hLnRyYW5zbGF0aW9uOwo+PiDCoCDCoCDCoCBpZiAoc2xvdCA9PSByaW5nLT5u
cl9zbG90cyAtIDEpCj4+IMKgIMKgIMKgIMKgIMKgIMKgIMKgIGN0bDAgfD0gQjQzX0RNQTY0X0RD
VEwwX0RUQUJMRUVORDsKPj4gwqAgwqAgwqAgaWYgKHN0YXJ0KQo+PiBAQCAtNjU2LDEwICs2NTks
MTIgQEAgc3RhdGljIGludCBhbGxvY19pbml0aWFsX2Rlc2NidWZmZXJzKHN0cnVjdCBiNDNfZG1h
cmluZyAqcmluZykKPj4gwqBzdGF0aWMgaW50IGRtYWNvbnRyb2xsZXJfc2V0dXAoc3RydWN0IGI0
M19kbWFyaW5nICpyaW5nKQo+PiDCoHsKPj4gwqAgwqAgwqAgaW50IGVyciA9IDA7Cj4+ICsgwqAg
wqAgaW50IHRtcDsKPj4gwqAgwqAgwqAgdTMyIHZhbHVlOwo+PiDCoCDCoCDCoCB1MzIgYWRkcmV4
dDsKPj4gwqAgwqAgwqAgdTMyIHRyYW5zID0gcmluZy0+ZGV2LT5kbWEudHJhbnNsYXRpb247Cj4+
IMKgIMKgIMKgIGJvb2wgcGFyaXR5ID0gcmluZy0+ZGV2LT5kbWEucGFyaXR5Owo+PiArIMKgIMKg
IHUzMiBhZGRyc1syXTsKPj4KPj4gwqAgwqAgwqAgaWYgKHJpbmctPnR4KSB7Cj4+IMKgIMKgIMKg
IMKgIMKgIMKgIMKgIGlmIChyaW5nLT50eXBlID09IEI0M19ETUFfNjRCSVQpIHsKPj4gQEAgLTY3
MywxMiArNjc4LDE0IEBAIHN0YXRpYyBpbnQgZG1hY29udHJvbGxlcl9zZXR1cChzdHJ1Y3QgYjQz
X2RtYXJpbmcgKnJpbmcpCj4+IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIGlmICgh
cGFyaXR5KQo+PiDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCB2
YWx1ZSB8PSBCNDNfRE1BNjRfVFhQQVJJVFlESVNBQkxFOwo+PiDCoCDCoCDCoCDCoCDCoCDCoCDC
oCDCoCDCoCDCoCDCoCBiNDNfZG1hX3dyaXRlKHJpbmcsIEI0M19ETUE2NF9UWENUTCwgdmFsdWUp
Owo+PiAtIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIGI0M19kbWFfd3JpdGUocmluZywg
QjQzX0RNQTY0X1RYUklOR0xPLAo+PiAtIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKg
IMKgIMKgIMKgIMKgIMKgIMKgIChyaW5nYmFzZSAmIDB4RkZGRkZGRkYpKTsKPj4gLSDCoCDCoCDC
oCDCoCDCoCDCoCDCoCDCoCDCoCDCoCBiNDNfZG1hX3dyaXRlKHJpbmcsIEI0M19ETUE2NF9UWFJJ
TkdISSwKPj4gLSDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDC
oCDCoCAoKHJpbmdiYXNlID4+IDMyKSAmCj4+IC0gwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAg
wqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqB+U1NCX0RNQV9UUkFOU0xBVElPTl9NQVNLKQo+PiAt
IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIHwgdHJh
bnMpOwo+PiArCj4+ICsgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgYWRkcnNbMF0gPSBy
aW5nYmFzZSAmIDB4RkZGRkZGRkY7Cj4+ICsgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAg
YWRkcnNbMV0gPSByaW5nYmFzZSA+PiAzMjsKPj4gKyDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDC
oCDCoCB0bXAgPSByaW5nLT5kZXYtPmRtYS50cmFuc2xhdGlvbl9pbl9sb3cgPyAwIDogMTsKPj4g
KyDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCBhZGRyc1t0bXBdICY9IH5TU0JfRE1BX1RS
QU5TTEFUSU9OX01BU0s7Cj4+ICsgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgYWRkcnNb
dG1wXSB8PSB0cmFuczsKPj4gKyDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCBiNDNfZG1h
X3dyaXRlKHJpbmcsIEI0M19ETUE2NF9UWFJJTkdMTywgYWRkcnNbMF0pOwo+PiArIMKgIMKgIMKg
IMKgIMKgIMKgIMKgIMKgIMKgIMKgIGI0M19kbWFfd3JpdGUocmluZywgQjQzX0RNQTY0X1RYUklO
R0hJLCBhZGRyc1sxXSk7Cj4+IMKgIMKgIMKgIMKgIMKgIMKgIMKgIH0gZWxzZSB7Cj4+IMKgIMKg
IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIHUzMiByaW5nYmFzZSA9ICh1MzIpIChyaW5nLT5k
bWFiYXNlKTsKPj4KPj4gQEAgLTcxMCwxMiArNzE3LDE1IEBAIHN0YXRpYyBpbnQgZG1hY29udHJv
bGxlcl9zZXR1cChzdHJ1Y3QgYjQzX2RtYXJpbmcgKnJpbmcpCj4+IMKgIMKgIMKgIMKgIMKgIMKg
IMKgIMKgIMKgIMKgIMKgIGlmICghcGFyaXR5KQo+PiDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDC
oCDCoCDCoCDCoCDCoCDCoCDCoCB2YWx1ZSB8PSBCNDNfRE1BNjRfUlhQQVJJVFlESVNBQkxFOwo+
PiDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCBiNDNfZG1hX3dyaXRlKHJpbmcsIEI0
M19ETUE2NF9SWENUTCwgdmFsdWUpOwo+PiAtIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKg
IGI0M19kbWFfd3JpdGUocmluZywgQjQzX0RNQTY0X1JYUklOR0xPLAo+PiAtIMKgIMKgIMKgIMKg
IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIChyaW5nYmFzZSAmIDB4RkZG
RkZGRkYpKTsKPj4gLSDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCBiNDNfZG1hX3dyaXRl
KHJpbmcsIEI0M19ETUE2NF9SWFJJTkdISSwKPj4gLSDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDC
oCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCAoKHJpbmdiYXNlID4+IDMyKSAmCj4+IC0gwqAgwqAg
wqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqB+U1NCX0RNQV9U
UkFOU0xBVElPTl9NQVNLKQo+PiAtIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKg
IMKgIMKgIMKgIMKgIMKgIHwgdHJhbnMpOwo+PiArCj4+ICsgwqAgwqAgwqAgwqAgwqAgwqAgwqAg
wqAgwqAgwqAgYWRkcnNbMF0gPSByaW5nYmFzZSAmIDB4RkZGRkZGRkY7Cj4+ICsgwqAgwqAgwqAg
wqAgwqAgwqAgwqAgwqAgwqAgwqAgYWRkcnNbMV0gPSByaW5nYmFzZSA+PiAzMjsKPj4gKyDCoCDC
oCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCB0bXAgPSByaW5nLT5kZXYtPmRtYS50cmFuc2xhdGlv
bl9pbl9sb3cgPyAwIDogMTsKPj4gKyDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCBhZGRy
c1t0bXBdICY9IH5TU0JfRE1BX1RSQU5TTEFUSU9OX01BU0s7Cj4+ICsgwqAgwqAgwqAgwqAgwqAg
wqAgwqAgwqAgwqAgwqAgYWRkcnNbdG1wXSB8PSB0cmFuczsKPj4gKyDCoCDCoCDCoCDCoCDCoCDC
oCDCoCDCoCDCoCDCoCBiNDNfZG1hX3dyaXRlKHJpbmcsIEI0M19ETUE2NF9SWFJJTkdMTywgYWRk
cnNbMF0pOwo+PiArIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIGI0M19kbWFfd3JpdGUo
cmluZywgQjQzX0RNQTY0X1JYUklOR0hJLCBhZGRyc1sxXSk7Cj4+ICsKPj4gwqAgwqAgwqAgwqAg
wqAgwqAgwqAgwqAgwqAgwqAgwqAgYjQzX2RtYV93cml0ZShyaW5nLCBCNDNfRE1BNjRfUlhJTkRF
WCwgcmluZy0+bnJfc2xvdHMgKgo+PiDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDC
oCDCoCDCoCDCoCDCoCDCoCDCoCBzaXplb2Yoc3RydWN0IGI0M19kbWFkZXNjNjQpKTsKPj4gwqAg
wqAgwqAgwqAgwqAgwqAgwqAgfSBlbHNlIHsKPj4gQEAgLTEwNTIsNiArMTA2MiwyMSBAQCBzdGF0
aWMgaW50IGI0M19kbWFfc2V0X21hc2soc3RydWN0IGI0M193bGRldiAqZGV2LCB1NjQgbWFzaykK
Pj4gwqAgwqAgwqAgcmV0dXJuIDA7Cj4+IMKgfQo+Cj4KPiBUaGlzIGRvZXNuJ3QgbG9vayBjb3Jy
ZWN0IHRvIG1lIGZvciBzZXZlcmFsIHJlYXNvbnM6Cj4KPiBJbiB0aGUgZmlsbC1vcCB0aGUgYWRk
cmVzcyBpcyBub3QgbWFza2VkIGNvcnJlY3RseSB3aXRoIHRoZSB0cmFuc2xhdGlvbiBtYXNrLgo+
IEluIGJvdGggdGhlIGZpbGwtb3AgYW5kIGJvdGggcmluZyBzZXR1cHMsIHRoZSBhY3R1YWwgYWRk
cmVzcyBleHRlbnNpb24gYml0cwo+IGFyZSBhbHdheXMgdGFrZW4gZnJvbSB0aGUgYWRkcmVzcydz
IGhpZ2ggd29yZC4gSSBndWVzcyB0aGUgZXh0ZW5zaW9uIHNob3VsZAo+IGJlIHRoZSBsb3cgd29y
ZCBiaXRzIGZvciBkZXZpY2VzIHdoZXJlIHdlIHVzZSB0aGUgbG93IHdvcmQuIFRoYXQncyB0aGUg
b25seQo+IHRoaW5nIHRoYXQgd291bGQgbWFrZSBzZW5zZS4gQnV0IGhleSwgaXQncyBub3QgdGhh
dCB3ZSBoYXZlIHNhbmUgaGFyZHdhcmUgaGVyZS4KPiBTbyB0aGlzIGhhcyB0byBiZSBjaGVja2Vk
LgoKT3VjaCwgeWVhaCwgeW91IHNob3VsZCBiZSByaWdodCAoYWNjb3JkaW5nIHRvIGNvbW1vbiBz
ZW5zZSBvZiBkZXNpZ24pLgoKVW5mb3J0dW5hdGVseSwgb24gbXkgbWFjaGluZSwga2VybmVsIHBy
b3ZpZGVzIGxvdyBhZGRyZXNzZXMgZm9yIERNQSBwdXJwb3NlczoKMHgxZjMxMDAwMAoweDFmMzE4
MDAwCjB4MWYzMWMwMDAKCkNhbiBJIGFzay9oYWNrIGtlcm5lbCB0byBvZmZlciBiNDMgYWRkcmVz
c2VzIHN0YXJ0aW5nIHdpdGggMHg0Li4uIG9yCjB4OC4uLiAob3IgMHhjLi4uKT8KCi0tIApSYWZh
xYIK
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] b43: fix DMA on some bugged hardware
2011-08-10 16:45 ` Rafał Miłecki
@ 2011-08-10 16:55 ` Michael Büsch
2011-08-10 17:10 ` Rafał Miłecki
0 siblings, 1 reply; 11+ messages in thread
From: Michael Büsch @ 2011-08-10 16:55 UTC (permalink / raw)
To: Rafał Miłecki; +Cc: linux-wireless, John W. Linville, b43-dev
On Wed, 10 Aug 2011 18:45:57 +0200
Rafał Miłecki <zajec5@gmail.com> wrote:
> W dniu 10 sierpnia 2011 18:33 użytkownik Michael Büsch <m@bues.ch> napisał:
> > On Wed, 10 Aug 2011 18:11:28 +0200
> >> diff --git a/drivers/net/wireless/b43/dma.c b/drivers/net/wireless/b43/dma.c
> >> index 0953ce1..9a2b678 100644
> >> --- a/drivers/net/wireless/b43/dma.c
> >> +++ b/drivers/net/wireless/b43/dma.c
> >> @@ -174,7 +174,10 @@ 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;
> >> + if (ring->dev->dma.translation_in_low)
> >> + addrlo |= ring->dev->dma.translation;
> >> + else
> >> + addrhi |= ring->dev->dma.translation;
> >> if (slot == ring->nr_slots - 1)
> >> ctl0 |= B43_DMA64_DCTL0_DTABLEEND;
> >> if (start)
> >> @@ -656,10 +659,12 @@ static int alloc_initial_descbuffers(struct b43_dmaring *ring)
> >> static int dmacontroller_setup(struct b43_dmaring *ring)
> >> {
> >> int err = 0;
> >> + int tmp;
> >> u32 value;
> >> u32 addrext;
> >> u32 trans = ring->dev->dma.translation;
> >> bool parity = ring->dev->dma.parity;
> >> + u32 addrs[2];
> >>
> >> if (ring->tx) {
> >> if (ring->type == B43_DMA_64BIT) {
> >> @@ -673,12 +678,14 @@ static int dmacontroller_setup(struct b43_dmaring *ring)
> >> if (!parity)
> >> value |= B43_DMA64_TXPARITYDISABLE;
> >> b43_dma_write(ring, B43_DMA64_TXCTL, value);
> >> - b43_dma_write(ring, B43_DMA64_TXRINGLO,
> >> - (ringbase & 0xFFFFFFFF));
> >> - b43_dma_write(ring, B43_DMA64_TXRINGHI,
> >> - ((ringbase >> 32) &
> >> - ~SSB_DMA_TRANSLATION_MASK)
> >> - | trans);
> >> +
> >> + addrs[0] = ringbase & 0xFFFFFFFF;
> >> + addrs[1] = ringbase >> 32;
> >> + tmp = ring->dev->dma.translation_in_low ? 0 : 1;
> >> + addrs[tmp] &= ~SSB_DMA_TRANSLATION_MASK;
> >> + addrs[tmp] |= trans;
> >> + b43_dma_write(ring, B43_DMA64_TXRINGLO, addrs[0]);
> >> + b43_dma_write(ring, B43_DMA64_TXRINGHI, addrs[1]);
> >> } else {
> >> u32 ringbase = (u32) (ring->dmabase);
> >>
> >> @@ -710,12 +717,15 @@ static int dmacontroller_setup(struct b43_dmaring *ring)
> >> if (!parity)
> >> value |= B43_DMA64_RXPARITYDISABLE;
> >> b43_dma_write(ring, B43_DMA64_RXCTL, value);
> >> - b43_dma_write(ring, B43_DMA64_RXRINGLO,
> >> - (ringbase & 0xFFFFFFFF));
> >> - b43_dma_write(ring, B43_DMA64_RXRINGHI,
> >> - ((ringbase >> 32) &
> >> - ~SSB_DMA_TRANSLATION_MASK)
> >> - | trans);
> >> +
> >> + addrs[0] = ringbase & 0xFFFFFFFF;
> >> + addrs[1] = ringbase >> 32;
> >> + tmp = ring->dev->dma.translation_in_low ? 0 : 1;
> >> + addrs[tmp] &= ~SSB_DMA_TRANSLATION_MASK;
> >> + addrs[tmp] |= trans;
> >> + b43_dma_write(ring, B43_DMA64_RXRINGLO, addrs[0]);
> >> + b43_dma_write(ring, B43_DMA64_RXRINGHI, addrs[1]);
> >> +
> >> b43_dma_write(ring, B43_DMA64_RXINDEX, ring->nr_slots *
> >> sizeof(struct b43_dmadesc64));
> >> } else {
> >> @@ -1052,6 +1062,21 @@ static int b43_dma_set_mask(struct b43_wldev *dev, u64 mask)
> >> return 0;
> >> }
> >
> >
> > This doesn't look correct to me for several reasons:
> >
> > In the fill-op the address is not masked correctly with the translation mask.
> > In both the fill-op and both ring setups, the actual address extension bits
> > are always taken from the address's high word. I guess the extension should
> > be the low word bits for devices where we use the low word. That's the only
> > thing that would make sense. But hey, it's not that we have sane hardware here.
> > So this has to be checked.
>
> Ouch, yeah, you should be right (according to common sense of design).
>
> Unfortunately, on my machine, kernel provides low addresses for DMA purposes:
> 0x1f310000
> 0x1f318000
> 0x1f31c000
>
> Can I ask/hack kernel to offer b43 addresses starting with 0x4... or
> 0x8... (or 0xc...)?
I'm not sure I understand..
Are you actually saying that those shiny new devices are total crap
in that their 64bit DMA engine can only do real-life-30bit, because they
completely fucked up the extension bits?
Did you try to fix the extension according to my suggestion? Does that
break stuff?
--
Greetings, Michael.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] b43: fix DMA on some bugged hardware
2011-08-10 16:55 ` Michael Büsch
@ 2011-08-10 17:10 ` Rafał Miłecki
2011-08-10 17:15 ` Michael Büsch
0 siblings, 1 reply; 11+ messages in thread
From: Rafał Miłecki @ 2011-08-10 17:10 UTC (permalink / raw)
To: Michael Büsch; +Cc: linux-wireless, John W. Linville, b43-dev
W dniu 10 sierpnia 2011 18:55 użytkownik Michael Büsch <m@bues.ch> napisał:
> On Wed, 10 Aug 2011 18:45:57 +0200
> Rafał Miłecki <zajec5@gmail.com> wrote:
>
>> W dniu 10 sierpnia 2011 18:33 użytkownik Michael Büsch <m@bues.ch> napisał:
>> > On Wed, 10 Aug 2011 18:11:28 +0200
>> >> diff --git a/drivers/net/wireless/b43/dma.c b/drivers/net/wireless/b43/dma.c
>> >> index 0953ce1..9a2b678 100644
>> >> --- a/drivers/net/wireless/b43/dma.c
>> >> +++ b/drivers/net/wireless/b43/dma.c
>> >> @@ -174,7 +174,10 @@ 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;
>> >> + if (ring->dev->dma.translation_in_low)
>> >> + addrlo |= ring->dev->dma.translation;
>> >> + else
>> >> + addrhi |= ring->dev->dma.translation;
>> >> if (slot == ring->nr_slots - 1)
>> >> ctl0 |= B43_DMA64_DCTL0_DTABLEEND;
>> >> if (start)
>> >> @@ -656,10 +659,12 @@ static int alloc_initial_descbuffers(struct b43_dmaring *ring)
>> >> static int dmacontroller_setup(struct b43_dmaring *ring)
>> >> {
>> >> int err = 0;
>> >> + int tmp;
>> >> u32 value;
>> >> u32 addrext;
>> >> u32 trans = ring->dev->dma.translation;
>> >> bool parity = ring->dev->dma.parity;
>> >> + u32 addrs[2];
>> >>
>> >> if (ring->tx) {
>> >> if (ring->type == B43_DMA_64BIT) {
>> >> @@ -673,12 +678,14 @@ static int dmacontroller_setup(struct b43_dmaring *ring)
>> >> if (!parity)
>> >> value |= B43_DMA64_TXPARITYDISABLE;
>> >> b43_dma_write(ring, B43_DMA64_TXCTL, value);
>> >> - b43_dma_write(ring, B43_DMA64_TXRINGLO,
>> >> - (ringbase & 0xFFFFFFFF));
>> >> - b43_dma_write(ring, B43_DMA64_TXRINGHI,
>> >> - ((ringbase >> 32) &
>> >> - ~SSB_DMA_TRANSLATION_MASK)
>> >> - | trans);
>> >> +
>> >> + addrs[0] = ringbase & 0xFFFFFFFF;
>> >> + addrs[1] = ringbase >> 32;
>> >> + tmp = ring->dev->dma.translation_in_low ? 0 : 1;
>> >> + addrs[tmp] &= ~SSB_DMA_TRANSLATION_MASK;
>> >> + addrs[tmp] |= trans;
>> >> + b43_dma_write(ring, B43_DMA64_TXRINGLO, addrs[0]);
>> >> + b43_dma_write(ring, B43_DMA64_TXRINGHI, addrs[1]);
>> >> } else {
>> >> u32 ringbase = (u32) (ring->dmabase);
>> >>
>> >> @@ -710,12 +717,15 @@ static int dmacontroller_setup(struct b43_dmaring *ring)
>> >> if (!parity)
>> >> value |= B43_DMA64_RXPARITYDISABLE;
>> >> b43_dma_write(ring, B43_DMA64_RXCTL, value);
>> >> - b43_dma_write(ring, B43_DMA64_RXRINGLO,
>> >> - (ringbase & 0xFFFFFFFF));
>> >> - b43_dma_write(ring, B43_DMA64_RXRINGHI,
>> >> - ((ringbase >> 32) &
>> >> - ~SSB_DMA_TRANSLATION_MASK)
>> >> - | trans);
>> >> +
>> >> + addrs[0] = ringbase & 0xFFFFFFFF;
>> >> + addrs[1] = ringbase >> 32;
>> >> + tmp = ring->dev->dma.translation_in_low ? 0 : 1;
>> >> + addrs[tmp] &= ~SSB_DMA_TRANSLATION_MASK;
>> >> + addrs[tmp] |= trans;
>> >> + b43_dma_write(ring, B43_DMA64_RXRINGLO, addrs[0]);
>> >> + b43_dma_write(ring, B43_DMA64_RXRINGHI, addrs[1]);
>> >> +
>> >> b43_dma_write(ring, B43_DMA64_RXINDEX, ring->nr_slots *
>> >> sizeof(struct b43_dmadesc64));
>> >> } else {
>> >> @@ -1052,6 +1062,21 @@ static int b43_dma_set_mask(struct b43_wldev *dev, u64 mask)
>> >> return 0;
>> >> }
>> >
>> >
>> > This doesn't look correct to me for several reasons:
>> >
>> > In the fill-op the address is not masked correctly with the translation mask.
>> > In both the fill-op and both ring setups, the actual address extension bits
>> > are always taken from the address's high word. I guess the extension should
>> > be the low word bits for devices where we use the low word. That's the only
>> > thing that would make sense. But hey, it's not that we have sane hardware here.
>> > So this has to be checked.
>>
>> Ouch, yeah, you should be right (according to common sense of design).
>>
>> Unfortunately, on my machine, kernel provides low addresses for DMA purposes:
>> 0x1f310000
>> 0x1f318000
>> 0x1f31c000
>>
>> Can I ask/hack kernel to offer b43 addresses starting with 0x4... or
>> 0x8... (or 0xc...)?
>
> I'm not sure I understand..
> Are you actually saying that those shiny new devices are total crap
> in that their 64bit DMA engine can only do real-life-30bit, because they
> completely fucked up the extension bits?
> Did you try to fix the extension according to my suggestion? Does that
> break stuff?
Sorry, I was thinking about some additional tests without really
sharing my thoughts.
kernel offers me addresses like:
0x000000001f310000
0x000000001f318000
0x000000001f31c000
However I wanted to hack kernel to receive something conflicting with:
#define SSB_DMA_TRANSLATION_MASK 0xC0000000
For example I wanted to receive from kernel address like:
0x000000005f318000
or
0x000000009f318000
However that are just some additional tests. It sounds pretty obvious
that we should take addrext from the "correct" part of ringbase (AKA
ring->dmabase).
--
Rafał
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] b43: fix DMA on some bugged hardware
2011-08-10 17:10 ` Rafał Miłecki
@ 2011-08-10 17:15 ` Michael Büsch
0 siblings, 0 replies; 11+ messages in thread
From: Michael Büsch @ 2011-08-10 17:15 UTC (permalink / raw)
To: Rafał Miłecki; +Cc: linux-wireless, John W. Linville, b43-dev
> kernel offers me addresses like:
> 0x000000001f310000
Yeah and that's why you didn't trigger the bug in the patch.
Extension is zero for both cases (low and high).
--
Greetings, Michael.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] ssb: fix DMA translation for some specific boards
2011-08-10 16:44 ` Michael Büsch
@ 2011-08-10 17:27 ` Johannes Berg
0 siblings, 0 replies; 11+ messages in thread
From: Johannes Berg @ 2011-08-10 17:27 UTC (permalink / raw)
To: Michael Büsch
Cc: Rafał Miłecki, linux-wireless, John W. Linville,
b43-dev
On Wed, 2011-08-10 at 18:44 +0200, Michael Büsch wrote:
> > >> + if (dev->id.coreid == SSB_DEV_80211)
> > >> + return (chip_id == 0x4322 || chip_id == 43221 ||
> > >> + chip_id == 43231 || chip_id == 43222);
> > >> + return 0;
> > >> +}
> > >
> > > ACK, apart from this coding style violation (missing curly braces).
> >
> > From kernel CodingStyle:
> > > Do not unnecessarily use braces where a single statement will do.
> > >
> > > if (condition)
> > > action();
> >
> > I guess simple "return" (even when using line break) is still like
> > single statement...
> > Am I wrong?
> >
>
> Last time I checked this only applied to single-line-statements.
> Maybe it changed, though.
Just sidestep the issue and invert the test ;-)
(However, CodingStyle says nothing about single lines, only statements,
and I tend to agree)
johannes
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2011-08-10 17:27 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-08-10 16:11 [PATCH 1/2] ssb: fix DMA translation for some specific boards Rafał Miłecki
2011-08-10 16:11 ` [PATCH 2/2] b43: fix DMA on some bugged hardware Rafał Miłecki
2011-08-10 16:33 ` Michael Büsch
2011-08-10 16:45 ` Rafał Miłecki
2011-08-10 16:55 ` Michael Büsch
2011-08-10 17:10 ` Rafał Miłecki
2011-08-10 17:15 ` Michael Büsch
2011-08-10 16:40 ` [PATCH 1/2] ssb: fix DMA translation for some specific boards Michael Büsch
2011-08-10 16:42 ` Rafał Miłecki
2011-08-10 16:44 ` Michael Büsch
2011-08-10 17:27 ` Johannes Berg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).