Re: [PATCH 02/11] brcmfmac: set F2 watermark to 256 for 4373

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

On 11/6/2018 4:50 AM, Chi-Hsien Lin wrote:
> From: Wright Feng <wright.feng@cypress.com>
>
> We got SDIO_CRC_ERROR with 4373 on SDR104 when doing bi-directional
> throughput test. Enable watermark to 256 to guarantee the operation
> stability.

Maybe it would be better to clarify the unit of the watermark. Here you 
use bytes, but in the register it needs number of words (word being 4 
bytes). So is this really only applicable to 4373? I have had question 
about SDIO crc errors before for other chips. Other than that....

Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Signed-off-by: Wright Feng <wright.feng@cypress.com>
> Signed-off-by: Chi-Hsien Lin <chi-hsien.lin@cypress.com>
> ---
>  .../wireless/broadcom/brcm80211/brcmfmac/sdio.c    | 25 ++++++++++++++++++++--
>  1 file changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> index b2e1ab5adb64..541d54661c9e 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
> @@ -49,6 +49,9 @@
>  #define DCMD_RESP_TIMEOUT	msecs_to_jiffies(2500)
>  #define CTL_DONE_TIMEOUT	msecs_to_jiffies(2500)

Mention the unit so:

+/* watermark expressed in number of words */
> +#define DEFAULT_F2_WATERMARK    0x8
> +#define CY_4373_F2_WATERMARK    0x40

or

+#define DEFAULT_F2_WATERMARK	(32 >> 2)
+@define CY_4373_F2_WATERMARK   (256 >> 2)