Re: New brcmfmac errors in 4.12: brcmf_sdio_rxglom: sublen ... not multiple of 8

[Arend Van Spriel <arend.vanspriel@broadcom.com>]

On 19-5-2017 21:07, Heiner Kallweit wrote:
> Am 19.05.2017 um 20:03 schrieb Arend Van Spriel:
>> On 14-5-2017 13:45, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 14-05-17 10:21, Arend Van Spriel wrote:
>>>> On 13-5-2017 16:55, Hans de Goede wrote:
>>>>> Hi,
>>>>>
>>>>> On 13-05-17 15:39, Heiner Kallweit wrote:
>>>>>> Am 13.05.2017 um 14:35 schrieb Hans de Goede:
>>>>>>> <resend with the author of the commit causing this added>
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> On 13-05-17 14:19, Hans de Goede wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> I've just rebased my personal kernel tree to what will soon be
>>>>>>>> 4.12-rc1
>>>>>>>> and I'm getting my dmesg log filled with the following errors:
>>>>>>>>
>>>>>>>> [   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
>>>>>>>>
>>>>>> After a brief look at the code I'm not sure that the check actually
>>>>>> checks
>>>>>> for an error condition. Apart from the error messages:
>>>>>> Do you face issues with the functionality of the driver?
>>>>>
>>>>> Yes after a while I get -ETIMEOUT errors for any sdio transfers
>>>>> to the device. But I'm not sure if this is caused by this commit,
>>>>> I think I've seen this once with 4.11 too.
>>>>>
>>>>> I've reverted the commit for now, but I'm fine with instead of
>>>>> doing the revert dropping the error check if the brcmfmac developers
>>>>> think that is ok. Currently the ETIMEOUT seems to be gone, so
>>>>> if dropping the revert causes it to re-appear then we know more.
>>>>
>>>> Instead of reverting please give this patch a try and let me know if it
>>>> works for you.
>>>>
>>>> Regards,
>>>> Arend
>>>> ---
>>>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
>>>> b/drivers/n
>>>> index fc64b89..e034500 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));
>>>>          }
>>>>
>>>
>>> I can confirm that the above fix fixes the messages.
>>
>> Heiner,
>>
>> Could I get your ack or even better your test verdict as well?
>>
> Unfortunately I don't have hardware to test this (as sdio isn't available
> on Odroid C2). However the patch looks logical and good to me.
> 
> I add Helmut as he gave his Tested-by for commit 5ef1ecf060f28e
> "mmc: sdio: fix alignment issue in struct sdio_func".
> Maybe he can test your patch as well.

Ok. Thanks,
Arend

> Rgds, Heiner
> 
>> Regards,
>> Arend
>>
>