[PATCH 4.12 REGRESSION fix] brcmfmac: Use ALIGNMENT rather then hardcoded "4" for bus:txglomalign

[PATCH 4.12 REGRESSION fix] brcmfmac: Use ALIGNMENT rather then hardcoded "4" for bus:txglomalign

[Hans de Goede]

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>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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));
 	}
-- 
2.13.0

Re: [PATCH 4.12 REGRESSION fix] brcmfmac: Use ALIGNMENT rather then hardcoded "4" for bus:txglomalign

[Kalle Valo]

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.

-- 
Kalle Valo

Re: [PATCH 4.12 REGRESSION fix] brcmfmac: Use ALIGNMENT rather then hardcoded "4" for bus:txglomalign

[Hans de Goede]

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

Re: [PATCH 4.12 REGRESSION fix] brcmfmac: Use ALIGNMENT rather then hardcoded "4" for bus:txglomalign

[Arend van Spriel]

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

Re: [PATCH 4.12 REGRESSION fix] brcmfmac: Use ALIGNMENT rather then hardcoded "4" for bus:txglomalign

[Kalle Valo]

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.

-- 
Kalle Valo

Re: [PATCH 4.12 REGRESSION fix] brcmfmac: Use ALIGNMENT rather then hardcoded "4" for bus:txglomalign

[Arend van Spriel]

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

Re: [PATCH 4.12 REGRESSION fix] brcmfmac: Use ALIGNMENT rather then hardcoded "4" for bus:txglomalign

[Arend van Spriel]

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

Re: [PATCH 4.12 REGRESSION fix] brcmfmac: Use ALIGNMENT rather then hardcoded "4" for bus:txglomalign

[Arend van Spriel]

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

Re: [4.12, REGRESSION, fix] brcmfmac: fix alignment configuration on host using 64-bit DMA

[Kalle Valo]

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

-- 
https://patchwork.kernel.org/patch/9750347/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches