| Message ID | 20170726202557.15632-32-ian@mnementh.co.uk (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
On 26-07-17 22:25, Ian Molton wrote: > Linux doesnt pass you func0 as a function when probing - instead > providing specific access functions to read/write it. > > This prepares for a patch to remove the actual array entry itself. Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com> > Signed-off-by: Ian Molton <ian@mnementh.co.uk> > --- > drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 5 +---- > drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 6 +++--- > drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h | 13 ++++++------- > 3 files changed, 10 insertions(+), 14 deletions(-) [...] > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h > index 8a976c89cf63..227c90198a8e 100644 > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h > @@ -21,7 +21,9 @@ > #include <linux/firmware.h> > #include "firmware.h" > > -#define SDIO_FUNC_0 0 > +/* Maximum number of I/O funcs */ > +#define NUM_SDIO_FUNCS 3 > + > #define SDIO_FUNC_1 1 > #define SDIO_FUNC_2 2 > > @@ -39,9 +41,6 @@ > #define INTR_STATUS_FUNC1 0x2 > #define INTR_STATUS_FUNC2 0x4 > > -/* Maximum number of I/O funcs */ > -#define SDIOD_MAX_IOFUNCS 7 > - Good riddance, because ... > /* mask of register map */ > #define REG_F0_REG_MASK 0x7FF > #define REG_F1_MISC_MASK 0x1FFFF > @@ -175,7 +174,7 @@ struct brcmf_sdio; > struct brcmf_sdiod_freezer; > > struct brcmf_sdio_dev { > - struct sdio_func *func[SDIO_MAX_FUNCS]; ... it was not used anyway as this definition is in <linux/mmc/card.h>. > + struct sdio_func *func[NUM_SDIO_FUNCS]; > u8 num_funcs; /* Supported funcs on client */ > u32 sbwad; /* Save backplane window address */ > struct brcmf_core *cc_core; /* chipcommon core info struct */ > @@ -297,10 +296,10 @@ void brcmf_sdiod_intr_unregister(struct brcmf_sdio_dev *sdiodev); > /* SDIO device register access interface */ > /* Functions for accessing SDIO Function 0 */ > #define brcmf_sdiod_func0_rb(sdiodev, addr, r) \ > - sdio_f0_readb((sdiodev)->func[0], (addr), (r)) > + sdio_f0_readb((sdiodev)->func[1], (addr), (r)) There is no reason to keep these any longer as these do not provide any functionality over the core sdio function unless you consider the sdiodev dereference.
On 08/08/17 12:19, Arend van Spriel wrote: >> #define brcmf_sdiod_func0_rb(sdiodev, addr, r) \ >> - sdio_f0_readb((sdiodev)->func[0], (addr), (r)) >> + sdio_f0_readb((sdiodev)->func[1], (addr), (r)) > > There is no reason to keep these any longer as these do not provide any > functionality over the core sdio function unless you consider the > sdiodev dereference. > I tend to agree, although they're fairly readable anyway. I was trying to keep the changes small and incremental. I'll knock up a patch and see how it looks with them converted to the actual functions when I get back from hoiliday. -Ian
On 08-08-17 13:27, Ian Molton wrote: > On 08/08/17 12:19, Arend van Spriel wrote: > >>> #define brcmf_sdiod_func0_rb(sdiodev, addr, r) \ >>> - sdio_f0_readb((sdiodev)->func[0], (addr), (r)) >>> + sdio_f0_readb((sdiodev)->func[1], (addr), (r)) >> >> There is no reason to keep these any longer as these do not provide any >> functionality over the core sdio function unless you consider the >> sdiodev dereference. >> > > I tend to agree, although they're fairly readable anyway. I was trying > to keep the changes small and incremental. > > I'll knock up a patch and see how it looks with them converted to the > actual functions when I get back from hoiliday. Enjoy the holiday. Glad to see I am not the only one checking email when he is not supposed to work ;-) Regards, Arend
On 08/08/17 12:19, Arend van Spriel wrote: >> - sdio_f0_readb((sdiodev)->func[0], (addr), (r)) >> + sdio_f0_readb((sdiodev)->func[1], (addr), (r)) > > There is no reason to keep these any longer as these do not provide any > functionality over the core sdio function unless you consider the > sdiodev dereference. I'm happy to submit an incremental patch to these that gets us right down to the linux mmc core functions. Just seemed like too big a change to do in one hit. -Ian
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c index 95149c686c5f..da0654c50db9 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c @@ -1021,8 +1021,7 @@ static int brcmf_ops_sdio_probe(struct sdio_func *func, /* store refs to functions used. mmc_card does * not hold the F0 function pointer. */ - sdiodev->func[0] = kmemdup(func, sizeof(*func), GFP_KERNEL); - sdiodev->func[0]->num = 0; + sdiodev->func[0] = NULL; sdiodev->func[1] = func->card->sdio_func[0]; sdiodev->func[2] = func; @@ -1048,7 +1047,6 @@ static int brcmf_ops_sdio_probe(struct sdio_func *func, fail: dev_set_drvdata(&func->dev, NULL); dev_set_drvdata(&sdiodev->func[1]->dev, NULL); - kfree(sdiodev->func[0]); kfree(sdiodev); kfree(bus_if); return err; @@ -1081,7 +1079,6 @@ static void brcmf_ops_sdio_remove(struct sdio_func *func) dev_set_drvdata(&sdiodev->func[2]->dev, NULL); kfree(bus_if); - kfree(sdiodev->func[0]); kfree(sdiodev); } diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c index 91a17ef63a6b..230a24cb6c0a 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c @@ -3766,9 +3766,9 @@ static u32 brcmf_sdio_buscore_read32(void *ctx, u32 addr) /* Force 4339 chips over rev2 to use the same ID */ /* This is borderline tolerable whilst there is only two exceptions */ /* But could be handled better */ - if ((sdiodev->func[0]->device == SDIO_DEVICE_ID_BROADCOM_4335_4339 || - sdiodev->func[0]->device == SDIO_DEVICE_ID_BROADCOM_4339) && - addr == CORE_CC_REG(SI_ENUM_BASE, chipid)) { + if ((sdiodev->func[1]->device == SDIO_DEVICE_ID_BROADCOM_4335_4339 || + sdiodev->func[1]->device == SDIO_DEVICE_ID_BROADCOM_4339) && + addr == CORE_CC_REG(SI_ENUM_BASE, chipid)) { rev = (val & CID_REV_MASK) >> CID_REV_SHIFT; diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h index 8a976c89cf63..227c90198a8e 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h @@ -21,7 +21,9 @@ #include <linux/firmware.h> #include "firmware.h" -#define SDIO_FUNC_0 0 +/* Maximum number of I/O funcs */ +#define NUM_SDIO_FUNCS 3 + #define SDIO_FUNC_1 1 #define SDIO_FUNC_2 2 @@ -39,9 +41,6 @@ #define INTR_STATUS_FUNC1 0x2 #define INTR_STATUS_FUNC2 0x4 -/* Maximum number of I/O funcs */ -#define SDIOD_MAX_IOFUNCS 7 - /* mask of register map */ #define REG_F0_REG_MASK 0x7FF #define REG_F1_MISC_MASK 0x1FFFF @@ -175,7 +174,7 @@ struct brcmf_sdio; struct brcmf_sdiod_freezer; struct brcmf_sdio_dev { - struct sdio_func *func[SDIO_MAX_FUNCS]; + struct sdio_func *func[NUM_SDIO_FUNCS]; u8 num_funcs; /* Supported funcs on client */ u32 sbwad; /* Save backplane window address */ struct brcmf_core *cc_core; /* chipcommon core info struct */ @@ -297,10 +296,10 @@ void brcmf_sdiod_intr_unregister(struct brcmf_sdio_dev *sdiodev); /* SDIO device register access interface */ /* Functions for accessing SDIO Function 0 */ #define brcmf_sdiod_func0_rb(sdiodev, addr, r) \ - sdio_f0_readb((sdiodev)->func[0], (addr), (r)) + sdio_f0_readb((sdiodev)->func[1], (addr), (r)) #define brcmf_sdiod_func0_wb(sdiodev, addr, v, ret) \ - sdio_f0_writeb((sdiodev)->func[0], (v), (addr), (ret)) + sdio_f0_writeb((sdiodev)->func[1], (v), (addr), (ret)) /* Functions for accessing SDIO Function 1 */ #define brcmf_sdiod_readb(sdiodev, addr, r) \
Linux doesnt pass you func0 as a function when probing - instead providing specific access functions to read/write it. This prepares for a patch to remove the actual array entry itself. Signed-off-by: Ian Molton <ian@mnementh.co.uk> --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 5 +---- drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 6 +++--- drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h | 13 ++++++------- 3 files changed, 10 insertions(+), 14 deletions(-)