On 11/08/2018 7:53, Arend van Spriel wrote:
> On 11/6/2018 4:50 AM, Chi-Hsien Lin wrote:
>> From: Madhan Mohan R <MadhanMohan.R@xxxxxxxxxxx>
>>
>> Along with F2 watermark (existing) configuration, F1 MesBusyCtrl
>> should be enabled & configured to avoid overflow errors.
>
> Reviewed-by: Arend van Spriel <arend.vanspriel@xxxxxxxxxxxx>
>> Signed-off-by: Madhan Mohan R <madhanmohan.r@xxxxxxxxxxx>
>> Signed-off-by: Chi-Hsien Lin <chi-hsien.lin@xxxxxxxxxxx>
>> ---
>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 3 +++
>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h | 3 +++
>> 2 files changed, 6 insertions(+)
>>
>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
>> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
>> index 541d54661c9e..34a838fcc319 100644
>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
>> @@ -51,6 +51,7 @@
>>
>> #define DEFAULT_F2_WATERMARK 0x8
>> #define CY_4373_F2_WATERMARK 0x40
>> +#define CY_4373_F1_MESBUSYCTRL (CY_4373_F2_WATERMARK |
>> SBSDIO_MESBUSYCTRL_ENAB)
>
> I don't see much value for this define. It is use once below so just or
> it there. That way you can "directly" see what is written to the register.
>
>> #ifdef DEBUG
>>
>> @@ -4118,6 +4119,8 @@ static void brcmf_sdio_firmware_callback(struct
>> device *dev, int err,
>> devctl |= SBSDIO_DEVCTL_F2WM_ENAB;
>> brcmf_sdiod_writeb(sdiod, SBSDIO_DEVICE_CTL, devctl,
>> &err);
>> + brcmf_sdiod_writeb(sdiod, SBSDIO_FUNC1_MESBUSYCTRL,
>> + CY_4373_F1_MESBUSYCTRL, &err);
>
> just use 'CY_4373_F2_WATERMARK | SBSDIO_MESBUSYCTRL_ENAB' here. No
> braces needed.
Thanks for the input. The biggest difference is to prevent a 4-line
function call like below so it's more readable. I'll make this change in
V2. Please let me know if below looks too messy then I'll move back to V1:
brcmf_sdiod_writeb(sdiod, CY_4373_F2_WATERMARK |
SBSDIO_MESBUSYCTRL_ENAB,
CY_4373_F2_WATERMARK |
SBSDIO_MESBUSYCTRL_ENAB, &err);
>
> .
>
