Hi
Sorry for "simulating" a reply - I'm not subscribed to linux-wireless, so,
cannot (easily) reply.
The commit in subject line
commit 17030f48e31adde5b043741c91ba143f5f7db0fd
From: Rafa? Mi?ecki <[email protected]>
Date: Thu, 11 Aug 2011 17:16:27 +0200
Subject: [PATCH] b43: support new RX header, noticed to be used in 598.314+ fw
breaks my b43 SDIO card in 3.2-ish kernels. Given the approaching
3.2-final release, would be nice to have it fixed before that. The card
I've got is marked with some "SD-Link11g" from some "C-guys, Inc." from
Taiwan:-) Using firmware version 410.31754. Do I really _have_ to update
or is the driver supposed to handle "all" firmware versions, or at least
not drop ones, it used to support before?
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
W dniu 26 grudnia 2011 18:28 użytkownik Guennadi Liakhovetski
<[email protected]> napisał:
> This patch fixes the regression, introduced by
>
> commit 17030f48e31adde5b043741c91ba143f5f7db0fd
> From: Rafał Miłecki <[email protected]>
> Date: Thu, 11 Aug 2011 17:16:27 +0200
> Subject: [PATCH] b43: support new RX header, noticed to be used in 598.314+ fw
>
> in PIO case.
>
> Signed-off-by: Guennadi Liakhovetski <[email protected]>
> ---
> diff --git a/drivers/net/wireless/b43/pio.c b/drivers/net/wireless/b43/pio.c
> index ce8a4bd..b64b64c 100644
> --- a/drivers/net/wireless/b43/pio.c
> +++ b/drivers/net/wireless/b43/pio.c
> @@ -617,9 +617,19 @@ static bool pio_rx_frame(struct b43_pio_rxqueue *q)
> const char *err_msg = NULL;
> struct b43_rxhdr_fw4 *rxhdr =
> (struct b43_rxhdr_fw4 *)wl->pio_scratchspace;
> + size_t rxhdr_size = sizeof(*rxhdr);
>
> BUILD_BUG_ON(sizeof(wl->pio_scratchspace) < sizeof(*rxhdr));
> - memset(rxhdr, 0, sizeof(*rxhdr));
> + switch (dev->fw.hdr_format) {
> + case B43_FW_HDR_410:
> + case B43_FW_HDR_351:
> + rxhdr_size -= sizeof(rxhdr->format_598) -
> + sizeof(rxhdr->format_351);
> + break;
> + case B43_FW_HDR_598:
> + break;
> + }
> + memset(rxhdr, 0, rxhdr_size);
Huuh, that's really tricky. Can you just do "normal" conditions as
Larry suggested, please?
--
Rafał
On Tue, 27 Dec 2011, Larry Finger wrote:
[snip]
> No, you are right. I misread the code. Your patch above would work and is
> probably as clean as one can expect.
John, Linus, please ack / pull the original patch from this thread into
3.2.
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
This patch fixes the regression, introduced by
commit 17030f48e31adde5b043741c91ba143f5f7db0fd
From: Rafa? Mi?ecki <[email protected]>
Date: Thu, 11 Aug 2011 17:16:27 +0200
Subject: [PATCH] b43: support new RX header, noticed to be used in 598.314+ fw
in PIO case.
Signed-off-by: Guennadi Liakhovetski <[email protected]>
---
diff --git a/drivers/net/wireless/b43/pio.c b/drivers/net/wireless/b43/pio.c
index ce8a4bd..b64b64c 100644
--- a/drivers/net/wireless/b43/pio.c
+++ b/drivers/net/wireless/b43/pio.c
@@ -617,9 +617,19 @@ static bool pio_rx_frame(struct b43_pio_rxqueue *q)
const char *err_msg = NULL;
struct b43_rxhdr_fw4 *rxhdr =
(struct b43_rxhdr_fw4 *)wl->pio_scratchspace;
+ size_t rxhdr_size = sizeof(*rxhdr);
BUILD_BUG_ON(sizeof(wl->pio_scratchspace) < sizeof(*rxhdr));
- memset(rxhdr, 0, sizeof(*rxhdr));
+ switch (dev->fw.hdr_format) {
+ case B43_FW_HDR_410:
+ case B43_FW_HDR_351:
+ rxhdr_size -= sizeof(rxhdr->format_598) -
+ sizeof(rxhdr->format_351);
+ break;
+ case B43_FW_HDR_598:
+ break;
+ }
+ memset(rxhdr, 0, rxhdr_size);
/* Check if we have data and wait for it to get ready. */
if (q->rev >= 8) {
@@ -657,11 +667,11 @@ data_ready:
/* Get the preamble (RX header) */
if (q->rev >= 8) {
- b43_block_read(dev, rxhdr, sizeof(*rxhdr),
+ b43_block_read(dev, rxhdr, rxhdr_size,
q->mmio_base + B43_PIO8_RXDATA,
sizeof(u32));
} else {
- b43_block_read(dev, rxhdr, sizeof(*rxhdr),
+ b43_block_read(dev, rxhdr, rxhdr_size,
q->mmio_base + B43_PIO_RXDATA,
sizeof(u16));
}
On 12/27/2011 06:00 PM, Guennadi Liakhovetski wrote:
> On Tue, 27 Dec 2011, Larry Finger wrote:
>
>> On 12/27/2011 05:05 PM, Guennadi Liakhovetski wrote:
>>> On Tue, 27 Dec 2011, Rafał Miłecki wrote:
>>>
>>>> W dniu 26 grudnia 2011 18:28 użytkownik Guennadi Liakhovetski
>>>> <[email protected]> napisał:
>>>>> This patch fixes the regression, introduced by
>>>>>
>>>>> commit 17030f48e31adde5b043741c91ba143f5f7db0fd
>>>>> From: Rafał Miłecki<[email protected]>
>>>>> Date: Thu, 11 Aug 2011 17:16:27 +0200
>>>>> Subject: [PATCH] b43: support new RX header, noticed to be used in
>>>>> 598.314+ fw
>>>>>
>>>>> in PIO case.
>>>>>
>>>>> Signed-off-by: Guennadi Liakhovetski<[email protected]>
>>>>> ---
>>>>> diff --git a/drivers/net/wireless/b43/pio.c
>>>>> b/drivers/net/wireless/b43/pio.c
>>>>> index ce8a4bd..b64b64c 100644
>>>>> --- a/drivers/net/wireless/b43/pio.c
>>>>> +++ b/drivers/net/wireless/b43/pio.c
>>>>> @@ -617,9 +617,19 @@ static bool pio_rx_frame(struct b43_pio_rxqueue *q)
>>>>> const char *err_msg = NULL;
>>>>> struct b43_rxhdr_fw4 *rxhdr =
>>>>> (struct b43_rxhdr_fw4 *)wl->pio_scratchspace;
>>>>> + size_t rxhdr_size = sizeof(*rxhdr);
>>>>>
>>>>> BUILD_BUG_ON(sizeof(wl->pio_scratchspace)< sizeof(*rxhdr));
>>>>> - memset(rxhdr, 0, sizeof(*rxhdr));
>>>>> + switch (dev->fw.hdr_format) {
>>>>> + case B43_FW_HDR_410:
>>>>> + case B43_FW_HDR_351:
>>>>> + rxhdr_size -= sizeof(rxhdr->format_598) -
>>>>> + sizeof(rxhdr->format_351);
>>>>> + break;
>>>>> + case B43_FW_HDR_598:
>>>>> + break;
>>>>> + }
>>>>> + memset(rxhdr, 0, rxhdr_size);
>>>>
>>>> Huuh, that's really tricky. Can you just do "normal" conditions as
>>>> Larry suggested, please?
>>>
>>> Sorry? I absolutely see nothing tricky there. Do you think this would look
>>> "less tricky" to you:
>>>
>>> switch (dev->fw.hdr_format) {
>>> case B43_FW_HDR_410:
>>> case B43_FW_HDR_351:
>>> rxhdr_size = offsetof(struct b43_rxhdr_fw4,
>>> format_351) +
>>> sizeof(rxhdr_size->format_351);
>>> break;
>>> case B43_FW_HDR_598:
>>> rxhdr_size = sizeof(*rxhdr);
>>> break;
>>> }
>>>
>>
>> How about this?
>>
>> Index: wireless-testing-new/drivers/net/wireless/b43/pio.c
>> ===================================================================
>> --- wireless-testing-new.orig/drivers/net/wireless/b43/pio.c
>> +++ wireless-testing-new/drivers/net/wireless/b43/pio.c
>> @@ -617,9 +617,20 @@ static bool pio_rx_frame(struct b43_pio_
>> const char *err_msg = NULL;
>> struct b43_rxhdr_fw4 *rxhdr =
>> (struct b43_rxhdr_fw4 *)wl->pio_scratchspace;
>> + size_t rxhdr_size;
>>
>> BUILD_BUG_ON(sizeof(wl->pio_scratchspace)< sizeof(*rxhdr));
>> - memset(rxhdr, 0, sizeof(*rxhdr));
>> + switch (dev->fw.hdr_format) {
>> + case B43_FW_HDR_410:
>> + case B43_FW_HDR_351:
>> + rxhdr_size = sizeof(rxhdr->format_351);
>> + break;
>> + case B43_FW_HDR_598:
>> + default:
>> + rxhdr_size = sizeof(rxhdr->format_598);
>> + break;
>> + }
>> + memset(rxhdr, 0, rxhdr_size);
>>
>> /* Check if we have data and wait for it to get ready. */
>> if (q->rev>= 8) {
>
> I am sorry, I'm either being blind and stupid or you're trying to do
> something quite wrong there. struct b43_rxhdr_fw4 has a bunch of fields
> first, then at the end it has a union of two fields: format_598 and
> format_351, right? rxhdr is pointing at the struct itself. Before the
> offending patch memset() used to clean the whole struct. Now in your above
> version you calculate the size of one of those union fields and nullify
> that many bytes from the _beginning_ of the whole struct.
>
> I've seen myself being wrong before, but here... I'll let you judge
> though.
No, you are right. I misread the code. Your patch above would work and is
probably as clean as one can expect.
Larry
On 12/26/2011 12:17 PM, Guennadi Liakhovetski wrote:
> On Mon, 26 Dec 2011, Larry Finger wrote:
>
>> On 12/26/2011 11:28 AM, Guennadi Liakhovetski wrote:
>>> This patch fixes the regression, introduced by
>>>
>>> commit 17030f48e31adde5b043741c91ba143f5f7db0fd
>>> From: Rafa? Mi?ecki<[email protected]>
>>> Date: Thu, 11 Aug 2011 17:16:27 +0200
>>> Subject: [PATCH] b43: support new RX header, noticed to be used in 598.314+
>>> fw
>>>
>>> in PIO case.
>>>
>>> Signed-off-by: Guennadi Liakhovetski<[email protected]>
>>> ---
>>> diff --git a/drivers/net/wireless/b43/pio.c b/drivers/net/wireless/b43/pio.c
>>> index ce8a4bd..b64b64c 100644
>>> --- a/drivers/net/wireless/b43/pio.c
>>> +++ b/drivers/net/wireless/b43/pio.c
>>> @@ -617,9 +617,19 @@ static bool pio_rx_frame(struct b43_pio_rxqueue *q)
>>> const char *err_msg = NULL;
>>> struct b43_rxhdr_fw4 *rxhdr =
>>> (struct b43_rxhdr_fw4 *)wl->pio_scratchspace;
>>> + size_t rxhdr_size = sizeof(*rxhdr);
>>>
>>> BUILD_BUG_ON(sizeof(wl->pio_scratchspace)< sizeof(*rxhdr));
>>> - memset(rxhdr, 0, sizeof(*rxhdr));
>>> + switch (dev->fw.hdr_format) {
>>> + case B43_FW_HDR_410:
>>> + case B43_FW_HDR_351:
>>> + rxhdr_size -= sizeof(rxhdr->format_598) -
>>> + sizeof(rxhdr->format_351);
>>> + break;
>>> + case B43_FW_HDR_598:
>>> + break;
>>> + }
>>
>> I do not think the above will work for format_410. By my count, the format_410
>> struct is 4 bytes longer than the format_351 struct.
>
> I think you're looking at struct b43_txhdr, whereas the problem is in
> struct b43_rxhdr_fw4.
>
> Thanks
> Guennadi
>
>>
>> Even if it does work, I suggest using the following:
>>
>> size_t rxhdr_size;
>>
>> BUILD_BUG_ON(sizeof(wl->pio_scratchspace)< sizeof(*rxhdr));
>> switch (dev->fw.hdr_format) {
>> case B43_FW_HDR_351:
>> rxhdr_size = sizeof(rxhdr->format_351);
>> break;
>> case B43_FW_HDR_410:
You are correct - I was looking in the wrong place. Even so, I prefer setting
rxhdr_size in the case branches, and not adjust one preset earlier.
Larry
On 12/26/2011 09:29 AM, Guennadi Liakhovetski wrote:
> Hi
>
> Sorry for "simulating" a reply - I'm not subscribed to linux-wireless, so,
> cannot (easily) reply.
>
> The commit in subject line
>
> commit 17030f48e31adde5b043741c91ba143f5f7db0fd
> From: Rafa? Mi?ecki<[email protected]>
> Date: Thu, 11 Aug 2011 17:16:27 +0200
> Subject: [PATCH] b43: support new RX header, noticed to be used in 598.314+ fw
>
> breaks my b43 SDIO card in 3.2-ish kernels. Given the approaching
> 3.2-final release, would be nice to have it fixed before that. The card
> I've got is marked with some "SD-Link11g" from some "C-guys, Inc." from
> Taiwan:-) Using firmware version 410.31754. Do I really _have_ to update
> or is the driver supposed to handle "all" firmware versions, or at least
> not drop ones, it used to support before?
The only cards that *need* to use the new-style firmware are those for which
firmware does not exist in the older versions. As this is not the case for your
device, something else is wrong.
Please post the details for your card. If you had a PCI system, I would ask for
'lspci -n'. Please do the same for the SDIO card.
From your message, it appears that 3.1 works OK. If that is true, would it be
possible for you to bisect this problem?
Larry
On Tue, 27 Dec 2011, Larry Finger wrote:
> On 12/27/2011 05:05 PM, Guennadi Liakhovetski wrote:
> > On Tue, 27 Dec 2011, Rafał Miłecki wrote:
> >
> > > W dniu 26 grudnia 2011 18:28 użytkownik Guennadi Liakhovetski
> > > <[email protected]> napisał:
> > > > This patch fixes the regression, introduced by
> > > >
> > > > commit 17030f48e31adde5b043741c91ba143f5f7db0fd
> > > > From: Rafał Miłecki<[email protected]>
> > > > Date: Thu, 11 Aug 2011 17:16:27 +0200
> > > > Subject: [PATCH] b43: support new RX header, noticed to be used in
> > > > 598.314+ fw
> > > >
> > > > in PIO case.
> > > >
> > > > Signed-off-by: Guennadi Liakhovetski<[email protected]>
> > > > ---
> > > > diff --git a/drivers/net/wireless/b43/pio.c
> > > > b/drivers/net/wireless/b43/pio.c
> > > > index ce8a4bd..b64b64c 100644
> > > > --- a/drivers/net/wireless/b43/pio.c
> > > > +++ b/drivers/net/wireless/b43/pio.c
> > > > @@ -617,9 +617,19 @@ static bool pio_rx_frame(struct b43_pio_rxqueue *q)
> > > > const char *err_msg = NULL;
> > > > struct b43_rxhdr_fw4 *rxhdr =
> > > > (struct b43_rxhdr_fw4 *)wl->pio_scratchspace;
> > > > + size_t rxhdr_size = sizeof(*rxhdr);
> > > >
> > > > BUILD_BUG_ON(sizeof(wl->pio_scratchspace)< sizeof(*rxhdr));
> > > > - memset(rxhdr, 0, sizeof(*rxhdr));
> > > > + switch (dev->fw.hdr_format) {
> > > > + case B43_FW_HDR_410:
> > > > + case B43_FW_HDR_351:
> > > > + rxhdr_size -= sizeof(rxhdr->format_598) -
> > > > + sizeof(rxhdr->format_351);
> > > > + break;
> > > > + case B43_FW_HDR_598:
> > > > + break;
> > > > + }
> > > > + memset(rxhdr, 0, rxhdr_size);
> > >
> > > Huuh, that's really tricky. Can you just do "normal" conditions as
> > > Larry suggested, please?
> >
> > Sorry? I absolutely see nothing tricky there. Do you think this would look
> > "less tricky" to you:
> >
> > switch (dev->fw.hdr_format) {
> > case B43_FW_HDR_410:
> > case B43_FW_HDR_351:
> > rxhdr_size = offsetof(struct b43_rxhdr_fw4,
> > format_351) +
> > sizeof(rxhdr_size->format_351);
> > break;
> > case B43_FW_HDR_598:
> > rxhdr_size = sizeof(*rxhdr);
> > break;
> > }
> >
>
> How about this?
>
> Index: wireless-testing-new/drivers/net/wireless/b43/pio.c
> ===================================================================
> --- wireless-testing-new.orig/drivers/net/wireless/b43/pio.c
> +++ wireless-testing-new/drivers/net/wireless/b43/pio.c
> @@ -617,9 +617,20 @@ static bool pio_rx_frame(struct b43_pio_
> const char *err_msg = NULL;
> struct b43_rxhdr_fw4 *rxhdr =
> (struct b43_rxhdr_fw4 *)wl->pio_scratchspace;
> + size_t rxhdr_size;
>
> BUILD_BUG_ON(sizeof(wl->pio_scratchspace) < sizeof(*rxhdr));
> - memset(rxhdr, 0, sizeof(*rxhdr));
> + switch (dev->fw.hdr_format) {
> + case B43_FW_HDR_410:
> + case B43_FW_HDR_351:
> + rxhdr_size = sizeof(rxhdr->format_351);
> + break;
> + case B43_FW_HDR_598:
> + default:
> + rxhdr_size = sizeof(rxhdr->format_598);
> + break;
> + }
> + memset(rxhdr, 0, rxhdr_size);
>
> /* Check if we have data and wait for it to get ready. */
> if (q->rev >= 8) {
I am sorry, I'm either being blind and stupid or you're trying to do
something quite wrong there. struct b43_rxhdr_fw4 has a bunch of fields
first, then at the end it has a union of two fields: format_598 and
format_351, right? rxhdr is pointing at the struct itself. Before the
offending patch memset() used to clean the whole struct. Now in your above
version you calculate the size of one of those union fields and nullify
that many bytes from the _beginning_ of the whole struct.
I've seen myself being wrong before, but here... I'll let you judge
though.
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
On 12/26/2011 11:28 AM, Guennadi Liakhovetski wrote:
> This patch fixes the regression, introduced by
>
> commit 17030f48e31adde5b043741c91ba143f5f7db0fd
> From: Rafa? Mi?ecki<[email protected]>
> Date: Thu, 11 Aug 2011 17:16:27 +0200
> Subject: [PATCH] b43: support new RX header, noticed to be used in 598.314+ fw
>
> in PIO case.
>
> Signed-off-by: Guennadi Liakhovetski<[email protected]>
> ---
> diff --git a/drivers/net/wireless/b43/pio.c b/drivers/net/wireless/b43/pio.c
> index ce8a4bd..b64b64c 100644
> --- a/drivers/net/wireless/b43/pio.c
> +++ b/drivers/net/wireless/b43/pio.c
> @@ -617,9 +617,19 @@ static bool pio_rx_frame(struct b43_pio_rxqueue *q)
> const char *err_msg = NULL;
> struct b43_rxhdr_fw4 *rxhdr =
> (struct b43_rxhdr_fw4 *)wl->pio_scratchspace;
> + size_t rxhdr_size = sizeof(*rxhdr);
>
> BUILD_BUG_ON(sizeof(wl->pio_scratchspace)< sizeof(*rxhdr));
> - memset(rxhdr, 0, sizeof(*rxhdr));
> + switch (dev->fw.hdr_format) {
> + case B43_FW_HDR_410:
> + case B43_FW_HDR_351:
> + rxhdr_size -= sizeof(rxhdr->format_598) -
> + sizeof(rxhdr->format_351);
> + break;
> + case B43_FW_HDR_598:
> + break;
> + }
I do not think the above will work for format_410. By my count, the format_410
struct is 4 bytes longer than the format_351 struct.
Even if it does work, I suggest using the following:
size_t rxhdr_size;
BUILD_BUG_ON(sizeof(wl->pio_scratchspace)< sizeof(*rxhdr));
switch (dev->fw.hdr_format) {
case B43_FW_HDR_351:
rxhdr_size = sizeof(rxhdr->format_351);
break;
case B43_FW_HDR_410:
etc.
Larry
On Mon, 26 Dec 2011, Larry Finger wrote:
> On 12/26/2011 11:28 AM, Guennadi Liakhovetski wrote:
> > This patch fixes the regression, introduced by
> >
> > commit 17030f48e31adde5b043741c91ba143f5f7db0fd
> > From: Rafa? Mi?ecki<[email protected]>
> > Date: Thu, 11 Aug 2011 17:16:27 +0200
> > Subject: [PATCH] b43: support new RX header, noticed to be used in 598.314+
> > fw
> >
> > in PIO case.
> >
> > Signed-off-by: Guennadi Liakhovetski<[email protected]>
> > ---
> > diff --git a/drivers/net/wireless/b43/pio.c b/drivers/net/wireless/b43/pio.c
> > index ce8a4bd..b64b64c 100644
> > --- a/drivers/net/wireless/b43/pio.c
> > +++ b/drivers/net/wireless/b43/pio.c
> > @@ -617,9 +617,19 @@ static bool pio_rx_frame(struct b43_pio_rxqueue *q)
> > const char *err_msg = NULL;
> > struct b43_rxhdr_fw4 *rxhdr =
> > (struct b43_rxhdr_fw4 *)wl->pio_scratchspace;
> > + size_t rxhdr_size = sizeof(*rxhdr);
> >
> > BUILD_BUG_ON(sizeof(wl->pio_scratchspace)< sizeof(*rxhdr));
> > - memset(rxhdr, 0, sizeof(*rxhdr));
> > + switch (dev->fw.hdr_format) {
> > + case B43_FW_HDR_410:
> > + case B43_FW_HDR_351:
> > + rxhdr_size -= sizeof(rxhdr->format_598) -
> > + sizeof(rxhdr->format_351);
> > + break;
> > + case B43_FW_HDR_598:
> > + break;
> > + }
>
> I do not think the above will work for format_410. By my count, the format_410
> struct is 4 bytes longer than the format_351 struct.
I think you're looking at struct b43_txhdr, whereas the problem is in
struct b43_rxhdr_fw4.
Thanks
Guennadi
>
> Even if it does work, I suggest using the following:
>
> size_t rxhdr_size;
>
> BUILD_BUG_ON(sizeof(wl->pio_scratchspace)< sizeof(*rxhdr));
> switch (dev->fw.hdr_format) {
> case B43_FW_HDR_351:
> rxhdr_size = sizeof(rxhdr->format_351);
> break;
> case B43_FW_HDR_410:
> etc.
>
>
> Larry
>
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
On Tue, 27 Dec 2011, Rafał Miłecki wrote:
> W dniu 26 grudnia 2011 18:28 użytkownik Guennadi Liakhovetski
> <[email protected]> napisał:
> > This patch fixes the regression, introduced by
> >
> > commit 17030f48e31adde5b043741c91ba143f5f7db0fd
> > From: Rafał Miłecki <[email protected]>
> > Date: Thu, 11 Aug 2011 17:16:27 +0200
> > Subject: [PATCH] b43: support new RX header, noticed to be used in 598.314+ fw
> >
> > in PIO case.
> >
> > Signed-off-by: Guennadi Liakhovetski <[email protected]>
> > ---
> > diff --git a/drivers/net/wireless/b43/pio.c b/drivers/net/wireless/b43/pio.c
> > index ce8a4bd..b64b64c 100644
> > --- a/drivers/net/wireless/b43/pio.c
> > +++ b/drivers/net/wireless/b43/pio.c
> > @@ -617,9 +617,19 @@ static bool pio_rx_frame(struct b43_pio_rxqueue *q)
> > const char *err_msg = NULL;
> > struct b43_rxhdr_fw4 *rxhdr =
> > (struct b43_rxhdr_fw4 *)wl->pio_scratchspace;
> > + size_t rxhdr_size = sizeof(*rxhdr);
> >
> > BUILD_BUG_ON(sizeof(wl->pio_scratchspace) < sizeof(*rxhdr));
> > - memset(rxhdr, 0, sizeof(*rxhdr));
> > + switch (dev->fw.hdr_format) {
> > + case B43_FW_HDR_410:
> > + case B43_FW_HDR_351:
> > + rxhdr_size -= sizeof(rxhdr->format_598) -
> > + sizeof(rxhdr->format_351);
> > + break;
> > + case B43_FW_HDR_598:
> > + break;
> > + }
> > + memset(rxhdr, 0, rxhdr_size);
>
> Huuh, that's really tricky. Can you just do "normal" conditions as
> Larry suggested, please?
Sorry? I absolutely see nothing tricky there. Do you think this would look
"less tricky" to you:
switch (dev->fw.hdr_format) {
case B43_FW_HDR_410:
case B43_FW_HDR_351:
rxhdr_size = offsetof(struct b43_rxhdr_fw4,
format_351) +
sizeof(rxhdr_size->format_351);
break;
case B43_FW_HDR_598:
rxhdr_size = sizeof(*rxhdr);
break;
}
?
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
On 12/27/2011 05:05 PM, Guennadi Liakhovetski wrote:
> On Tue, 27 Dec 2011, Rafał Miłecki wrote:
>
>> W dniu 26 grudnia 2011 18:28 użytkownik Guennadi Liakhovetski
>> <[email protected]> napisał:
>>> This patch fixes the regression, introduced by
>>>
>>> commit 17030f48e31adde5b043741c91ba143f5f7db0fd
>>> From: Rafał Miłecki<[email protected]>
>>> Date: Thu, 11 Aug 2011 17:16:27 +0200
>>> Subject: [PATCH] b43: support new RX header, noticed to be used in 598.314+ fw
>>>
>>> in PIO case.
>>>
>>> Signed-off-by: Guennadi Liakhovetski<[email protected]>
>>> ---
>>> diff --git a/drivers/net/wireless/b43/pio.c b/drivers/net/wireless/b43/pio.c
>>> index ce8a4bd..b64b64c 100644
>>> --- a/drivers/net/wireless/b43/pio.c
>>> +++ b/drivers/net/wireless/b43/pio.c
>>> @@ -617,9 +617,19 @@ static bool pio_rx_frame(struct b43_pio_rxqueue *q)
>>> const char *err_msg = NULL;
>>> struct b43_rxhdr_fw4 *rxhdr =
>>> (struct b43_rxhdr_fw4 *)wl->pio_scratchspace;
>>> + size_t rxhdr_size = sizeof(*rxhdr);
>>>
>>> BUILD_BUG_ON(sizeof(wl->pio_scratchspace)< sizeof(*rxhdr));
>>> - memset(rxhdr, 0, sizeof(*rxhdr));
>>> + switch (dev->fw.hdr_format) {
>>> + case B43_FW_HDR_410:
>>> + case B43_FW_HDR_351:
>>> + rxhdr_size -= sizeof(rxhdr->format_598) -
>>> + sizeof(rxhdr->format_351);
>>> + break;
>>> + case B43_FW_HDR_598:
>>> + break;
>>> + }
>>> + memset(rxhdr, 0, rxhdr_size);
>>
>> Huuh, that's really tricky. Can you just do "normal" conditions as
>> Larry suggested, please?
>
> Sorry? I absolutely see nothing tricky there. Do you think this would look
> "less tricky" to you:
>
> switch (dev->fw.hdr_format) {
> case B43_FW_HDR_410:
> case B43_FW_HDR_351:
> rxhdr_size = offsetof(struct b43_rxhdr_fw4,
> format_351) +
> sizeof(rxhdr_size->format_351);
> break;
> case B43_FW_HDR_598:
> rxhdr_size = sizeof(*rxhdr);
> break;
> }
>
How about this?
Index: wireless-testing-new/drivers/net/wireless/b43/pio.c
===================================================================
--- wireless-testing-new.orig/drivers/net/wireless/b43/pio.c
+++ wireless-testing-new/drivers/net/wireless/b43/pio.c
@@ -617,9 +617,20 @@ static bool pio_rx_frame(struct b43_pio_
const char *err_msg = NULL;
struct b43_rxhdr_fw4 *rxhdr =
(struct b43_rxhdr_fw4 *)wl->pio_scratchspace;
+ size_t rxhdr_size;
BUILD_BUG_ON(sizeof(wl->pio_scratchspace) < sizeof(*rxhdr));
- memset(rxhdr, 0, sizeof(*rxhdr));
+ switch (dev->fw.hdr_format) {
+ case B43_FW_HDR_410:
+ case B43_FW_HDR_351:
+ rxhdr_size = sizeof(rxhdr->format_351);
+ break;
+ case B43_FW_HDR_598:
+ default:
+ rxhdr_size = sizeof(rxhdr->format_598);
+ break;
+ }
+ memset(rxhdr, 0, rxhdr_size);
/* Check if we have data and wait for it to get ready. */
if (q->rev >= 8) {
Larry