| Message ID | 1541476188-75475-4-git-send-email-chi-hsien.lin@cypress.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | chip related changes | expand |
On 11/6/2018 4:50 AM, Chi-Hsien Lin wrote: > From: Madhan Mohan R <MadhanMohan.R@cypress.com> > > Along with F2 watermark (existing) configuration, F1 MesBusyCtrl > should be enabled & configured to avoid overflow errors. Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com> > Signed-off-by: Madhan Mohan R <madhanmohan.r@cypress.com> > Signed-off-by: Chi-Hsien Lin <chi-hsien.lin@cypress.com> > --- > 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.
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@cypress.com> >> >> Along with F2 watermark (existing) configuration, F1 MesBusyCtrl >> should be enabled & configured to avoid overflow errors. > > Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com> >> Signed-off-by: Madhan Mohan R <madhanmohan.r@cypress.com> >> Signed-off-by: Chi-Hsien Lin <chi-hsien.lin@cypress.com> >> --- >> 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); > > . >
On 11/9/2018 8:34 AM, Chi-Hsien Lin wrote: > > > 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@cypress.com> >>> >>> Along with F2 watermark (existing) configuration, F1 MesBusyCtrl >>> should be enabled & configured to avoid overflow errors. >> >> Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com> >>> Signed-off-by: Madhan Mohan R <madhanmohan.r@cypress.com> >>> Signed-off-by: Chi-Hsien Lin <chi-hsien.lin@cypress.com> >>> --- >>> 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); > The second argument should just be SBSDIO_FUNC1_MESBUSYCTRL so that will make it a bit less messy. Look okay to me. Regards, Arend
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) #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); break; default: brcmf_sdiod_writeb(sdiod, SBSDIO_WATERMARK, diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h index 7faed831f07d..8aaabca1eb0e 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h @@ -104,6 +104,9 @@ #define SBSDIO_FUNC1_RFRAMEBCHI 0x1001C /* MesBusyCtl (rev 11) */ #define SBSDIO_FUNC1_MESBUSYCTRL 0x1001D +/* Enable busy capability for MES access */ +#define SBSDIO_MESBUSYCTRL_ENAB 0x80 + /* Sdio Core Rev 12 */ #define SBSDIO_FUNC1_WAKEUPCTRL 0x1001E #define SBSDIO_FUNC1_WCTRL_ALPWAIT_MASK 0x1