2017-05-26 11:02:58

by Hans de Goede

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

From: Arend Van Spriel <[email protected]>

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 <[email protected]>
Signed-off-by: Hans de Goede <[email protected]>
---
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


2017-05-26 11:15:58

by Kalle Valo

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

Hans de Goede <[email protected]> writes:

> From: Arend Van Spriel <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Hans de Goede <[email protected]>

I'll queue this to 4.12.

--
Kalle Valo

2017-05-30 12:26:08

by Arend Van Spriel

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

On 26-05-17 19:11, Arend van Spriel wrote:
> On 26-05-17 16:53, Kalle Valo wrote:
>> Arend van Spriel <[email protected]> writes:
>>
>>>>>> Fixes: 6e84ab604bde ("properly align buffers ... with 64 bit DMA")
>>>>>> Suggested-by: Arend van Spriel <[email protected]>
>>>>>> Signed-off-by: Hans de Goede <[email protected]>
>>>>>
>>>>> 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 <[email protected]>
>>>>
>>>> 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 <[email protected]>
Tested-by: Hans de Goede <[email protected]>
Reviewed-by: Hante Meuleman <[email protected]>
Reviewed-by: Pieter-Paul Giesberts <[email protected]>
Reviewed-by: Franky Lin <[email protected]>
Signed-off-by: Arend van Spriel <[email protected]>
"""

I leave it at your discretion how to deal with this. If needed I can
submit the complete patch.

Regards,
Arend

2017-05-26 17:11:37

by Arend Van Spriel

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

On 26-05-17 16:53, Kalle Valo wrote:
> Arend van Spriel <[email protected]> writes:
>
>>>>> Fixes: 6e84ab604bde ("properly align buffers ... with 64 bit DMA")
>>>>> Suggested-by: Arend van Spriel <[email protected]>
>>>>> Signed-off-by: Hans de Goede <[email protected]>
>>>>
>>>> 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 <[email protected]>
>>>
>>> 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

2017-05-26 14:54:08

by Kalle Valo

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

Arend van Spriel <[email protected]> writes:

>>>> Fixes: 6e84ab604bde ("properly align buffers ... with 64 bit DMA")
>>>> Suggested-by: Arend van Spriel <[email protected]>
>>>> Signed-off-by: Hans de Goede <[email protected]>
>>>
>>> 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 <[email protected]>
>>
>> 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

2017-05-30 12:26:00

by Arend Van Spriel

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

On 26-05-17 19:11, Arend van Spriel wrote:
> On 26-05-17 16:53, Kalle Valo wrote:
>> Arend van Spriel <[email protected]> writes:
>>
>>>>>> Fixes: 6e84ab604bde ("properly align buffers ... with 64 bit DMA")
>>>>>> Suggested-by: Arend van Spriel <[email protected]>
>>>>>> Signed-off-by: Hans de Goede <[email protected]>
>>>>>
>>>>> 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 <[email protected]>
>>>>
>>>> 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 <[email protected]>
Tested-by: Hans de Goede <[email protected]>
Reviewed-by: Hante Meuleman <[email protected]>
Reviewed-by: Pieter-Paul Giesberts <[email protected]>
Reviewed-by: Franky Lin <[email protected]>
Signed-off-by: Arend van Spriel <[email protected]>
"""

I leave it at your discretion how to deal with this. If needed I can
submit the complete patch.

Regards,
Arend

2017-05-26 11:18:55

by Hans de Goede

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

Hi,

On 26-05-17 13:15, Kalle Valo wrote:
> Hans de Goede <[email protected]> writes:
>
>> From: Arend Van Spriel <[email protected]>

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 <[email protected]>
>> Signed-off-by: Hans de Goede <[email protected]>
>
> 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 <[email protected]>

And maybe drop the Suggested-by: Arend van Spriel ?

Regards,

Hans

2017-05-26 11:35:47

by Arend Van Spriel

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

On 26-05-17 13:18, Hans de Goede wrote:
> Hi,
>
> On 26-05-17 13:15, Kalle Valo wrote:
>> Hans de Goede <[email protected]> writes:
>>
>>> From: Arend Van Spriel <[email protected]>
>
> 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 <[email protected]>
>>> Signed-off-by: Hans de Goede <[email protected]>
>>
>> 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 <[email protected]>
>
> 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

2017-06-01 09:03:41

by Kalle Valo

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

Hans de Goede <[email protected]> wrote:

> From: Arend Van Spriel <[email protected]>
>
> 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 <[email protected]>
> Tested-by: Hans de Goede <[email protected]>
> Reviewed-by: Hante Meuleman <[email protected]>
> Reviewed-by: Pieter-Paul Giesberts <[email protected]>
> Reviewed-by: Franky Lin <[email protected]>
> Signed-off-by: Arend van Spriel <[email protected]>
> Signed-off-by: Hans de Goede <[email protected]>

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