| Message ID | 20170526110255.21342-1-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 1dbf647f31751a4e94fa0435c34f0f5ad5ce0adc |
| Delegated to: | Kalle Valo |
| Headers | show |
Hans de Goede <hdegoede@redhat.com> writes: > From: Arend Van Spriel <arend.vanspriel@broadcom.com> > > This fixes the following errors showing up in dmesg: > > [ 32.528271] brcmfmac: brcmf_sdio_rxglom: sublen 524 not multiple of 8 > [ 32.528296] brcmfmac: brcmf_sdio_rxglom: sublen 84 not multiple of 8 > [ 33.063241] brcmfmac: brcmf_sdio_rxglom: sublen 1492 not multiple of 8 > > Fixes: 6e84ab604bde ("properly align buffers ... with 64 bit DMA") > Suggested-by: Arend van Spriel <arend.vanspriel@broadcom.com> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> I'll queue this to 4.12.
Hi, On 26-05-17 13:15, Kalle Valo wrote: > Hans de Goede <hdegoede@redhat.com> writes: > >> From: Arend Van Spriel <arend.vanspriel@broadcom.com> Ah I see I set the Author to Arend when I added this to my tree a while back, that is fine as he did all the work for this one. I was under the impression Arend would submit this himself, but since I did not see a submission yet I decided to go ahead and submit this. >> This fixes the following errors showing up in dmesg: >> >> [ 32.528271] brcmfmac: brcmf_sdio_rxglom: sublen 524 not multiple of 8 >> [ 32.528296] brcmfmac: brcmf_sdio_rxglom: sublen 84 not multiple of 8 >> [ 33.063241] brcmfmac: brcmf_sdio_rxglom: sublen 1492 not multiple of 8 >> >> Fixes: 6e84ab604bde ("properly align buffers ... with 64 bit DMA") >> Suggested-by: Arend van Spriel <arend.vanspriel@broadcom.com> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> > > I'll queue this to 4.12. Thank you, given that Arend is set as the Author you can add my: Tested-by: Hans de Goede <hdegoede@redhat.com> And maybe drop the Suggested-by: Arend van Spriel ? Regards, Hans
On 26-05-17 13:18, Hans de Goede wrote: > Hi, > > On 26-05-17 13:15, Kalle Valo wrote: >> Hans de Goede <hdegoede@redhat.com> writes: >> >>> From: Arend Van Spriel <arend.vanspriel@broadcom.com> > > Ah I see I set the Author to Arend when I added this to my > tree a while back, that is fine as he did all the work > for this one. I was under the impression Arend would submit > this himself, but since I did not see a submission yet > I decided to go ahead and submit this. I did not get to sending it last week. My work hours for open-source have been reduced by 80% so time is limited. I have it queued. >>> This fixes the following errors showing up in dmesg: >>> >>> [ 32.528271] brcmfmac: brcmf_sdio_rxglom: sublen 524 not multiple of 8 >>> [ 32.528296] brcmfmac: brcmf_sdio_rxglom: sublen 84 not multiple of 8 >>> [ 33.063241] brcmfmac: brcmf_sdio_rxglom: sublen 1492 not multiple >>> of 8 >>> >>> Fixes: 6e84ab604bde ("properly align buffers ... with 64 bit DMA") >>> Suggested-by: Arend van Spriel <arend.vanspriel@broadcom.com> >>> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> >> I'll queue this to 4.12. > > Thank you, given that Arend is set as the Author you can add my: > > Tested-by: Hans de Goede <hdegoede@redhat.com> > > And maybe drop the Suggested-by: Arend van Spriel ? Seems to me you need my Signed-off-by: as I do not see that in this patch. Regards, Arend
Arend van Spriel <arend.vanspriel@broadcom.com> writes: >>>> Fixes: 6e84ab604bde ("properly align buffers ... with 64 bit DMA") >>>> Suggested-by: Arend van Spriel <arend.vanspriel@broadcom.com> >>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >>> >>> I'll queue this to 4.12. >> >> Thank you, given that Arend is set as the Author you can add my: >> >> Tested-by: Hans de Goede <hdegoede@redhat.com> >> >> And maybe drop the Suggested-by: Arend van Spriel ? Sure, I can add that. > Seems to me you need my Signed-off-by: as I do not see that in this patch. Yes, that is needed if you are the author. If you reply to this message I can add that.
On 26-05-17 16:53, Kalle Valo wrote: > Arend van Spriel <arend.vanspriel@broadcom.com> writes: > >>>>> Fixes: 6e84ab604bde ("properly align buffers ... with 64 bit DMA") >>>>> Suggested-by: Arend van Spriel <arend.vanspriel@broadcom.com> >>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >>>> >>>> I'll queue this to 4.12. >>> >>> Thank you, given that Arend is set as the Author you can add my: >>> >>> Tested-by: Hans de Goede <hdegoede@redhat.com> >>> >>> And maybe drop the Suggested-by: Arend van Spriel ? > > Sure, I can add that. > >> Seems to me you need my Signed-off-by: as I do not see that in this patch. > > Yes, that is needed if you are the author. If you reply to this message > I can add that. Ok. Here it is. Please feel free to add Signed-off-by tag for me authoring this change. The Signed-off-by tag for Hans can stay as well as this patch went through his hands (and he came up with the commit message ;-) ). Regards, Arend
On 26-05-17 19:11, Arend van Spriel wrote: > On 26-05-17 16:53, Kalle Valo wrote: >> Arend van Spriel <arend.vanspriel@broadcom.com> writes: >> >>>>>> Fixes: 6e84ab604bde ("properly align buffers ... with 64 bit DMA") >>>>>> Suggested-by: Arend van Spriel <arend.vanspriel@broadcom.com> >>>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >>>>> >>>>> I'll queue this to 4.12. >>>> >>>> Thank you, given that Arend is set as the Author you can add my: >>>> >>>> Tested-by: Hans de Goede <hdegoede@redhat.com> >>>> >>>> And maybe drop the Suggested-by: Arend van Spriel ? >> >> Sure, I can add that. >> >>> Seems to me you need my Signed-off-by: as I do not see that in this patch. >> >> Yes, that is needed if you are the author. If you reply to this message >> I can add that. > > Ok. Here it is. Please feel free to add Signed-off-by tag for me > authoring this change. The Signed-off-by tag for Hans can stay as well > as this patch went through his hands (and he came up with the commit > message ;-) ). Hi Kalle, Here is the commit message I came up with which may explain the issue better: """ brcmfmac: fix alignment configuration on host using 64-bit DMA For SDIO the alignment requirement for transfers from device to host is configured in firmware. This configuration is limited to minimum of 4-byte alignment. However, this is not correct for platforms using 64-bit DMA when the minimum alignment should be 8 bytes. This issue appeared when the ALIGNMENT definition was set according the DMA configuration. The configuration in firmware was not using that macro defintion, but a hardcoded value of 4. Hence the driver reported alignment failures for data coming from the device and causing transfers to fail. Fixes: 6e84ab604bde ("brcmfmac: properly align buffers on certain platforms Reported-by: Hans de Goede <hdegoede@redhat.com> Tested-by: Hans de Goede <hdegoede@redhat.com> Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com> Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com> Reviewed-by: Franky Lin <franky.lin@broadcom.com> Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com> """ I leave it at your discretion how to deal with this. If needed I can submit the complete patch. Regards, Arend
On 26-05-17 19:11, Arend van Spriel wrote: > On 26-05-17 16:53, Kalle Valo wrote: >> Arend van Spriel <arend.vanspriel@broadcom.com> writes: >> >>>>>> Fixes: 6e84ab604bde ("properly align buffers ... with 64 bit DMA") >>>>>> Suggested-by: Arend van Spriel <arend.vanspriel@broadcom.com> >>>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >>>>> >>>>> I'll queue this to 4.12. >>>> >>>> Thank you, given that Arend is set as the Author you can add my: >>>> >>>> Tested-by: Hans de Goede <hdegoede@redhat.com> >>>> >>>> And maybe drop the Suggested-by: Arend van Spriel ? >> >> Sure, I can add that. >> >>> Seems to me you need my Signed-off-by: as I do not see that in this patch. >> >> Yes, that is needed if you are the author. If you reply to this message >> I can add that. > > Ok. Here it is. Please feel free to add Signed-off-by tag for me > authoring this change. The Signed-off-by tag for Hans can stay as well > as this patch went through his hands (and he came up with the commit > message ;-) ). Hi Kalle, Here is the commit message I came up with which may explain the issue better: """ brcmfmac: fix alignment configuration on host using 64-bit DMA For SDIO the alignment requirement for transfers from device to host is configured in firmware. This configuration is limited to minimum of 4-byte alignment. However, this is not correct for platforms using 64-bit DMA when the minimum alignment should be 8 bytes. This issue appeared when the ALIGNMENT definition was set according the DMA configuration. The configuration in firmware was not using that macro defintion, but a hardcoded value of 4. Hence the driver reported alignment failures for data coming from the device and causing transfers to fail. Fixes: 6e84ab604bde ("brcmfmac: properly align buffers on certain platforms Reported-by: Hans de Goede <hdegoede@redhat.com> Tested-by: Hans de Goede <hdegoede@redhat.com> Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com> Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com> Reviewed-by: Franky Lin <franky.lin@broadcom.com> Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com> """ I leave it at your discretion how to deal with this. If needed I can submit the complete patch. Regards, Arend
Hans de Goede <hdegoede@redhat.com> wrote: > From: Arend Van Spriel <arend.vanspriel@broadcom.com> > > For SDIO the alignment requirement for transfers from device to host > is configured in firmware. This configuration is limited to minimum > of 4-byte alignment. However, this is not correct for platforms using > 64-bit DMA when the minimum alignment should be 8 bytes. This issue > appeared when the ALIGNMENT definition was set according the DMA > configuration. The configuration in firmware was not using that macro > defintion, but a hardcoded value of 4. Hence the driver reported > alignment failures for data coming from the device and causing > transfers to fail. > > Fixes: 6e84ab604bde ("brcmfmac: properly align buffers on certain platforms > Reported-by: Hans de Goede <hdegoede@redhat.com> > Tested-by: Hans de Goede <hdegoede@redhat.com> > Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com> > Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com> > Reviewed-by: Franky Lin <franky.lin@broadcom.com> > Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> Patch applied to wireless-drivers.git, thanks. 1dbf647f3175 brcmfmac: fix alignment configuration on host using 64-bit DMA
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c index fc64b8913aa6..e03450059b06 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c @@ -3422,7 +3422,7 @@ static int brcmf_sdio_bus_preinit(struct device *dev) /* otherwise, set txglomalign */ value = sdiodev->settings->bus.sdio.sd_sgentry_align; /* SDIO ADMA requires at least 32 bit alignment */ - value = max_t(u32, value, 4); + value = max_t(u32, value, ALIGNMENT); err = brcmf_iovar_data_set(dev, "bus:txglomalign", &value, sizeof(u32)); }