* Re: [PATCH] b43: support new RX header, noticed to be used in 598.314+ fw @ 2011-12-26 15:29 Guennadi Liakhovetski 2011-12-26 17:18 ` Larry Finger 2011-12-26 17:28 ` [PATCH] b43: fix regression in PIO case Guennadi Liakhovetski 0 siblings, 2 replies; 12+ messages in thread From: Guennadi Liakhovetski @ 2011-12-26 15:29 UTC (permalink / raw) To: Rafał Miłecki; +Cc: linux-wireless, John W. Linville, linux-kernel 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 <zajec5@gmail.com> 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/ ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] b43: support new RX header, noticed to be used in 598.314+ fw 2011-12-26 15:29 [PATCH] b43: support new RX header, noticed to be used in 598.314+ fw Guennadi Liakhovetski @ 2011-12-26 17:18 ` Larry Finger 2011-12-26 17:28 ` [PATCH] b43: fix regression in PIO case Guennadi Liakhovetski 1 sibling, 0 replies; 12+ messages in thread From: Larry Finger @ 2011-12-26 17:18 UTC (permalink / raw) To: Guennadi Liakhovetski Cc: Rafał Miłecki, linux-wireless, John W. Linville, linux-kernel 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<zajec5@gmail.com> > 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] b43: fix regression in PIO case 2011-12-26 15:29 [PATCH] b43: support new RX header, noticed to be used in 598.314+ fw Guennadi Liakhovetski 2011-12-26 17:18 ` Larry Finger @ 2011-12-26 17:28 ` Guennadi Liakhovetski 2011-12-26 17:51 ` Larry Finger 2011-12-27 18:47 ` Rafał Miłecki 1 sibling, 2 replies; 12+ messages in thread From: Guennadi Liakhovetski @ 2011-12-26 17:28 UTC (permalink / raw) To: Rafał Miłecki Cc: linux-wireless, John W. Linville, linux-kernel, Linus Torvalds This patch fixes the regression, introduced by commit 17030f48e31adde5b043741c91ba143f5f7db0fd From: Rafał Miłecki <zajec5@gmail.com> 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 <g.liakhovetski@gmx.de> --- 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)); } ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] b43: fix regression in PIO case 2011-12-26 17:28 ` [PATCH] b43: fix regression in PIO case Guennadi Liakhovetski @ 2011-12-26 17:51 ` Larry Finger 2011-12-26 18:17 ` Guennadi Liakhovetski 2011-12-27 18:47 ` Rafał Miłecki 1 sibling, 1 reply; 12+ messages in thread From: Larry Finger @ 2011-12-26 17:51 UTC (permalink / raw) To: Guennadi Liakhovetski Cc: Rafał Miłecki, linux-wireless, John W. Linville, linux-kernel, Linus Torvalds, b43-dev On 12/26/2011 11:28 AM, Guennadi Liakhovetski wrote: > This patch fixes the regression, introduced by > > commit 17030f48e31adde5b043741c91ba143f5f7db0fd > From: Rafał Miłecki<zajec5@gmail.com> > 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<g.liakhovetski@gmx.de> > --- > 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] b43: fix regression in PIO case 2011-12-26 17:51 ` Larry Finger @ 2011-12-26 18:17 ` Guennadi Liakhovetski 2011-12-26 18:32 ` Larry Finger 0 siblings, 1 reply; 12+ messages in thread From: Guennadi Liakhovetski @ 2011-12-26 18:17 UTC (permalink / raw) To: Larry Finger Cc: Rafał Miłecki, linux-wireless, John W. Linville, linux-kernel, Linus Torvalds, b43-dev 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<zajec5@gmail.com> > > 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<g.liakhovetski@gmx.de> > > --- > > 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/ ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] b43: fix regression in PIO case 2011-12-26 18:17 ` Guennadi Liakhovetski @ 2011-12-26 18:32 ` Larry Finger 0 siblings, 0 replies; 12+ messages in thread From: Larry Finger @ 2011-12-26 18:32 UTC (permalink / raw) To: Guennadi Liakhovetski Cc: Rafał Miłecki, linux-wireless, John W. Linville, linux-kernel, Linus Torvalds, b43-dev 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<zajec5@gmail.com> >>> 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<g.liakhovetski@gmx.de> >>> --- >>> 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] b43: fix regression in PIO case 2011-12-26 17:28 ` [PATCH] b43: fix regression in PIO case Guennadi Liakhovetski 2011-12-26 17:51 ` Larry Finger @ 2011-12-27 18:47 ` Rafał Miłecki 2011-12-27 23:05 ` Guennadi Liakhovetski 1 sibling, 1 reply; 12+ messages in thread From: Rafał Miłecki @ 2011-12-27 18:47 UTC (permalink / raw) To: Guennadi Liakhovetski Cc: linux-wireless, John W. Linville, linux-kernel, Linus Torvalds W dniu 26 grudnia 2011 18:28 użytkownik Guennadi Liakhovetski <g.liakhovetski@gmx.de> napisał: > This patch fixes the regression, introduced by > > commit 17030f48e31adde5b043741c91ba143f5f7db0fd > From: Rafał Miłecki <zajec5@gmail.com> > 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 <g.liakhovetski@gmx.de> > --- > 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ł ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] b43: fix regression in PIO case 2011-12-27 18:47 ` Rafał Miłecki @ 2011-12-27 23:05 ` Guennadi Liakhovetski 2011-12-27 23:47 ` Larry Finger 0 siblings, 1 reply; 12+ messages in thread From: Guennadi Liakhovetski @ 2011-12-27 23:05 UTC (permalink / raw) To: Rafał Miłecki Cc: linux-wireless, John W. Linville, linux-kernel, Linus Torvalds On Tue, 27 Dec 2011, Rafał Miłecki wrote: > W dniu 26 grudnia 2011 18:28 użytkownik Guennadi Liakhovetski > <g.liakhovetski@gmx.de> napisał: > > This patch fixes the regression, introduced by > > > > commit 17030f48e31adde5b043741c91ba143f5f7db0fd > > From: Rafał Miłecki <zajec5@gmail.com> > > 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 <g.liakhovetski@gmx.de> > > --- > > 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/ ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] b43: fix regression in PIO case 2011-12-27 23:05 ` Guennadi Liakhovetski @ 2011-12-27 23:47 ` Larry Finger 2011-12-28 0:00 ` Guennadi Liakhovetski 0 siblings, 1 reply; 12+ messages in thread From: Larry Finger @ 2011-12-27 23:47 UTC (permalink / raw) To: Guennadi Liakhovetski Cc: Rafał Miłecki, linux-wireless, John W. Linville, linux-kernel, Linus Torvalds 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 >> <g.liakhovetski@gmx.de> napisał: >>> This patch fixes the regression, introduced by >>> >>> commit 17030f48e31adde5b043741c91ba143f5f7db0fd >>> From: Rafał Miłecki<zajec5@gmail.com> >>> 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<g.liakhovetski@gmx.de> >>> --- >>> 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] b43: fix regression in PIO case 2011-12-27 23:47 ` Larry Finger @ 2011-12-28 0:00 ` Guennadi Liakhovetski 2011-12-28 0:11 ` Larry Finger 0 siblings, 1 reply; 12+ messages in thread From: Guennadi Liakhovetski @ 2011-12-28 0:00 UTC (permalink / raw) To: Larry Finger Cc: Rafał Miłecki, linux-wireless, John W. Linville, linux-kernel, Linus Torvalds 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 > > > <g.liakhovetski@gmx.de> napisał: > > > > This patch fixes the regression, introduced by > > > > > > > > commit 17030f48e31adde5b043741c91ba143f5f7db0fd > > > > From: Rafał Miłecki<zajec5@gmail.com> > > > > 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<g.liakhovetski@gmx.de> > > > > --- > > > > 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/ ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] b43: fix regression in PIO case 2011-12-28 0:00 ` Guennadi Liakhovetski @ 2011-12-28 0:11 ` Larry Finger 2011-12-28 16:37 ` Guennadi Liakhovetski 0 siblings, 1 reply; 12+ messages in thread From: Larry Finger @ 2011-12-28 0:11 UTC (permalink / raw) To: Guennadi Liakhovetski Cc: Rafał Miłecki, linux-wireless, John W. Linville, linux-kernel, Linus Torvalds 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 >>>> <g.liakhovetski@gmx.de> napisał: >>>>> This patch fixes the regression, introduced by >>>>> >>>>> commit 17030f48e31adde5b043741c91ba143f5f7db0fd >>>>> From: Rafał Miłecki<zajec5@gmail.com> >>>>> 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<g.liakhovetski@gmx.de> >>>>> --- >>>>> 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] b43: fix regression in PIO case 2011-12-28 0:11 ` Larry Finger @ 2011-12-28 16:37 ` Guennadi Liakhovetski 0 siblings, 0 replies; 12+ messages in thread From: Guennadi Liakhovetski @ 2011-12-28 16:37 UTC (permalink / raw) To: Larry Finger Cc: Rafał Miłecki, linux-wireless, John W. Linville, linux-kernel, Linus Torvalds 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/ ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2011-12-28 16:37 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-12-26 15:29 [PATCH] b43: support new RX header, noticed to be used in 598.314+ fw Guennadi Liakhovetski 2011-12-26 17:18 ` Larry Finger 2011-12-26 17:28 ` [PATCH] b43: fix regression in PIO case Guennadi Liakhovetski 2011-12-26 17:51 ` Larry Finger 2011-12-26 18:17 ` Guennadi Liakhovetski 2011-12-26 18:32 ` Larry Finger 2011-12-27 18:47 ` Rafał Miłecki 2011-12-27 23:05 ` Guennadi Liakhovetski 2011-12-27 23:47 ` Larry Finger 2011-12-28 0:00 ` Guennadi Liakhovetski 2011-12-28 0:11 ` Larry Finger 2011-12-28 16:37 ` Guennadi Liakhovetski
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).