| Message ID | 1429035033-14076-11-git-send-email-arend@broadcom.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Kalle Valo |
| Headers | show |
On 14 April 2015 at 20:10, Arend van Spriel <arend@broadcom.com> wrote: > From: Hante Meuleman <meuleman@broadcom.com> > > With PCIE it is possible to support multiple devices with the > same device type. They all load the same nvram file. In order to > support this the nvram can specify which part of the nvram is > for which pcie device. This patch adds support for these new > types of nvram files. I'm a bit unsure about this patch. Does it support NVRAM files that are already in use by any platform? Or are you trying to solve problem we hit with ARM routers like Netgear R8000? If you developed this patch to support e.g. Netgear R8000 I'm afraid it won't be really helpful. You won't be able to provide a single NVRAM file for all devices because of many differences between devices. I don't only mean different models, but even units. I believe these PCIe devices have to receive NVRAM with e.g. MAC address so you don't want to provide one file for all users. It means we'll need to develop some extra layer that will fetch *system* (SoC) NVRAM and translate it into a NVRAM file that will be stored in user space. Then this file will be loaded back into the kernel. So if this is about adding support for embedded devices, I'd prefer much more to simply use Linux's NVRAM driver to get all wanted entries. Other than that I'm a bit confused by the "pcie/" prefix you decided to use. Every Broadcom NVRAM I've seen was using pci/ prefix. Shouldn't you stick to this standard format? Switching to "pcie/" would require another translation when reading entries from system NVRAM. -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Huh, why dropping linux-wireless (and top posting btw)? Please let everyone follow the discussion :) On 15 April 2015 at 21:20, Hante Meuleman <meuleman@broadcom.com> wrote: > As I wrote to you in a mail and on the openwrt forum, this patch is indeed an attempt to support more complex nvram files. I also wrote, that in order to be able to use it, the nvram contents of the device (r8000) needs to be put a specific file. Now for your concerns, we can perhaps add something which will read the nvram contents directly from an nvram store. But that is irrelevant to this patch. The parsing is still needed, and all we would need to add is something which is reading the nvram contents from some other place So it makes me wonder if we need this patch in its current form. I think getting NVRAM directly from the platform is much user friendly. It doesn't require user to install some extra tools for dumping NVRAM and putting it in a specific file. One extra layer less. With that said I think it's hard to review your code for parsing NVRAM. We don't know how it's going to be fetched in the first place. > though it would have to be put under some kernel config flag as this would not be supported in non-router systems. The contents of the nvram would however still need to be parsed in exactly the same way as the nvram files we read from disk. Again, it's hard to say for me. Are you going to use bcm47xx_nvram_getenv? Are you going to use MTD subsystem? Are you going to develop different solution? When using e.g. bcm47xx_nvram_getenv you won't want all this parsing stuff at all. It seems this patch provides some end-support for NVRAM parsing while we still miss something between. Something for getting NVRAM from platform and providing it to the brcmfmac somehow. > As to your concern regarding pci/ versus pcie/: pci/ is old type and will never be used/supported by brcmfmac. All new routers will use either the compressed format like the r8000 does or the pcie/ (uncompressed) format depending on the size of the nvram store. Oh, I didn't notice that. OK, thanks for pointing this. -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 14 April 2015 at 20:10, Arend van Spriel <arend@broadcom.com> wrote: > @@ -23,6 +23,10 @@ > #include "debug.h" > #include "firmware.h" > > +#define BRCMF_FW_MAX_NVRAM_SIZE 64000 > +#define BRCMF_FW_NVRAM_DEVPATH_LEN 19 /* devpath0=pcie/1/4/ */ > +#define BRCMF_FW_NVRAM_PCIEDEV_LEN 9 /* pcie/1/4/ */ You most likely know what you're doing, but just for sure: are there any devices using "pcie/" prefix *without* a devpath? I got an impression that Broadcom first started using devpath-s and then (later) started using "pcie/" prefix (instead of "pci/"). > @@ -192,12 +214,136 @@ static int brcmf_init_nvram_parser(struct nvram_parser *nvp, > return 0; > } > > +/* brcmf_fw_strip_multi_v1 :Some nvram files contain settings for multiple > + * devices. Strip it down for one device, use domain_nr/bus_nr to determine > + * which data is to be returned. v1 is the version where nvram is stored > + * compressed and "devpath" maps to index for valid entries. > + */ > +static void brcmf_fw_strip_multi_v1(struct nvram_parser *nvp, u16 domain_nr, > + u16 bus_nr) > +{ > + u32 i, j; > + bool found; > + u8 *nvram; > + u8 id; > + > + nvram = kzalloc(nvp->nvram_len + 1 + 3 + sizeof(u32), GFP_KERNEL); > + if (!nvram) > + goto fail; > + > + /* min length: devpath0=pcie/1/4/ + 0:x=y */ > + if (nvp->nvram_len < BRCMF_FW_NVRAM_DEVPATH_LEN + 6) > + goto fail; > + > + /* First search for the devpathX and see if it is the configuration > + * for domain_nr/bus_nr. Search complete nvp > + */ > + found = false; > + i = 0; > + while (i < nvp->nvram_len - BRCMF_FW_NVRAM_DEVPATH_LEN) { > + /* Format: devpathX=pcie/Y/Z/ > + * Y = domain_nr, Z = bus_nr, X = virtual ID > + */ > + if ((strncmp(&nvp->nvram[i], "devpath", 7) == 0) && > + (strncmp(&nvp->nvram[i + 8], "=pcie/", 6) == 0)) { > + if (((nvp->nvram[i + 14] - '0') == domain_nr) && > + ((nvp->nvram[i + 16] - '0') == bus_nr)) { > + id = nvp->nvram[i + 7] - '0'; > + found = true; > + break; > + } > + } > + while (nvp->nvram[i] != 0) > + i++; I guess while should also check nvp->nvram_len to avoid crashing with malformed NVRAM content. Same in _v2 function. > + i++; > + } > + if (!found) > + goto fail; > + > + /* Now copy all valid entries, release old nvram and assign new one */ > + i = 0; > + j = 0; > + while (i < nvp->nvram_len) { > + if ((nvp->nvram[i] - '0' == id) && (nvp->nvram[i + 1] == ':')) { > + i += 2; > + while (nvp->nvram[i] != 0) { > + nvram[j] = nvp->nvram[i]; > + i++; > + j++; > + } > + nvram[j] = 0; > + j++; > + } > + while (nvp->nvram[i] != 0) > + i++; Same here. > +/* brcmf_fw_strip_multi_v2 :Some nvram files contain settings for multiple > + * devices. Strip it down for one device, use domain_nr/bus_nr to determine > + * which data is to be returned. v2 is the version where nvram is stored > + * uncompressed, all relevant valid entries are identified by > + * pcie/domain_nr/bus_nr: > + */ > +static void brcmf_fw_strip_multi_v2(struct nvram_parser *nvp, u16 domain_nr, > + u16 bus_nr) > +{ > + u32 i, j; > + u8 *nvram; > + > + nvram = kzalloc(nvp->nvram_len + 1 + 3 + sizeof(u32), GFP_KERNEL); > + if (!nvram) > + goto fail; > + > + /* Copy all valid entries, release old nvram and assign new one. > + * Valid entries are of type pcie/X/Y/ where X = domain_nr and > + * Y = bus_nr. > + */ > + i = 0; > + j = 0; > + while (i < nvp->nvram_len - BRCMF_FW_NVRAM_PCIEDEV_LEN) { > + if ((strncmp(&nvp->nvram[i], "pcie/", 5) == 0) && > + (nvp->nvram[i + 6] == '/') && (nvp->nvram[i + 8] == '/') && > + ((nvp->nvram[i + 5] - '0') == domain_nr) && > + ((nvp->nvram[i + 7] - '0') == bus_nr)) { > + i += BRCMF_FW_NVRAM_PCIEDEV_LEN; > + while (nvp->nvram[i] != 0) { > + nvram[j] = nvp->nvram[i]; > + i++; > + j++; > + } > + nvram[j] = 0; > + j++; > + } > + while (nvp->nvram[i] != 0) > + i++; > + i++; > + } > + kfree(nvp->nvram); > + nvp->nvram = nvram; > + nvp->nvram_len = j; > + return; > +fail: > + kfree(nvram); > + nvp->nvram_len = 0; > +} I guess you could consider implementing a one generic function that will accept sth like a "char *prefix" argument. First check for a devpathX=$prefix entry and use X: instead of prefix: or not. -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
+ openwrt-devel list On 04/17/15 09:45, Rafa? Mi?ecki wrote: > Huh, why dropping linux-wireless (and top posting btw)? Please let > everyone follow the discussion :) > > On 15 April 2015 at 21:20, Hante Meuleman<meuleman@broadcom.com> wrote: >> As I wrote to you in a mail and on the openwrt forum, this patch is indeed an attempt to support more complex nvram files. I also wrote, that in order to be able to use it, the nvram contents of the device (r8000) needs to be put a specific file. Now for your concerns, we can perhaps add something which will read the nvram contents directly from an nvram store. But that is irrelevant to this patch. The parsing is still needed, and all we would need to add is something which is reading the nvram contents from some other place > > So it makes me wonder if we need this patch in its current form. I > think getting NVRAM directly from the platform is much user friendly. > It doesn't require user to install some extra tools for dumping NVRAM > and putting it in a specific file. One extra layer less. > With that said I think it's hard to review your code for parsing > NVRAM. We don't know how it's going to be fetched in the first place. You already made that point and we agreed to look for a solution. >> though it would have to be put under some kernel config flag as this would not be supported in non-router systems. The contents of the nvram would however still need to be parsed in exactly the same way as the nvram files we read from disk. > > Again, it's hard to say for me. Are you going to use > bcm47xx_nvram_getenv? Are you going to use MTD subsystem? Are you > going to develop different solution? When using e.g. > bcm47xx_nvram_getenv you won't want all this parsing stuff at all. Please look at the usage scenario here. The brcmfmac driver is not needing a few key,value pairs. It needs a portion of NVRAM to download to the device. The patch provides the functionality to do just that. Get the appropriate portion, strip comments and whitespace, and send it to the device. Using bcm47xx_nvram_getenv is not a useful api as it would mean we need brcmfmac to know which key ids to ask for, reassemble it to key=value string and send it to the fullmac device. In bcm47xx_nvram_getenv() it does have the whole nvram content available in nvram_buf which is filled by nvram_init(). So if something similar is made available on r8000 (or ARM routers in general) build target all we need is an api to get the nvram_buf. Another option is to add the parsing stuff in that nvram code and have an api to get the appropriate portion based on pcie domain and bus number as used in brcmf_fw_get_firmwares_pcie(). However, I would prefer to have this in the driver and not in arch specific code as there may be other platforms like our set-top boxes needing this. Regards, Arend -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 17 April 2015 at 10:50, Arend van Spriel <arend@broadcom.com> wrote: > On 04/17/15 09:45, Rafa? Mi?ecki wrote: >> >> Huh, why dropping linux-wireless (and top posting btw)? Please let >> everyone follow the discussion :) >> >> On 15 April 2015 at 21:20, Hante Meuleman<meuleman@broadcom.com> wrote: >>> >>> As I wrote to you in a mail and on the openwrt forum, this patch is >>> indeed an attempt to support more complex nvram files. I also wrote, that in >>> order to be able to use it, the nvram contents of the device (r8000) needs >>> to be put a specific file. Now for your concerns, we can perhaps add >>> something which will read the nvram contents directly from an nvram store. >>> But that is irrelevant to this patch. The parsing is still needed, and all >>> we would need to add is something which is reading the nvram contents from >>> some other place >> >> >> So it makes me wonder if we need this patch in its current form. I >> think getting NVRAM directly from the platform is much user friendly. >> It doesn't require user to install some extra tools for dumping NVRAM >> and putting it in a specific file. One extra layer less. >> With that said I think it's hard to review your code for parsing >> NVRAM. We don't know how it's going to be fetched in the first place. > > You already made that point and we agreed to look for a solution. OK, it wasn't supposed to be rude or anything :) Thanks. >>> though it would have to be put under some kernel config flag as this >>> would not be supported in non-router systems. The contents of the nvram >>> would however still need to be parsed in exactly the same way as the nvram >>> files we read from disk. >> >> >> Again, it's hard to say for me. Are you going to use >> bcm47xx_nvram_getenv? Are you going to use MTD subsystem? Are you >> going to develop different solution? When using e.g. >> bcm47xx_nvram_getenv you won't want all this parsing stuff at all. > > > Please look at the usage scenario here. The brcmfmac driver is not needing a > few key,value pairs. It needs a portion of NVRAM to download to the device. > The patch provides the functionality to do just that. Get the appropriate > portion, strip comments and whitespace, and send it to the device. Using > bcm47xx_nvram_getenv is not a useful api as it would mean we need brcmfmac > to know which key ids to ask for, reassemble it to key=value string and send > it to the fullmac device. > > In bcm47xx_nvram_getenv() it does have the whole nvram content available in > nvram_buf which is filled by nvram_init(). So if something similar is made > available on r8000 (or ARM routers in general) build target all we need is > an api to get the nvram_buf. > > Another option is to add the parsing stuff in that nvram code and have an > api to get the appropriate portion based on pcie domain and bus number as > used in brcmf_fw_get_firmwares_pcie(). However, I would prefer to have this > in the driver and not in arch specific code as there may be other platforms > like our set-top boxes needing this. This is some plan for the future I was lacking from the beginning. It makes things more clear now, thanks.
On 04/20/15 13:26, Rafa? Mi?ecki wrote: > On 17 April 2015 at 10:50, Arend van Spriel<arend@broadcom.com> wrote: >> On 04/17/15 09:45, Rafa? Mi?ecki wrote: >>> >>> Huh, why dropping linux-wireless (and top posting btw)? Please let >>> everyone follow the discussion :) >>> >>> On 15 April 2015 at 21:20, Hante Meuleman<meuleman@broadcom.com> wrote: >>>> >>>> As I wrote to you in a mail and on the openwrt forum, this patch is >>>> indeed an attempt to support more complex nvram files. I also wrote, that in >>>> order to be able to use it, the nvram contents of the device (r8000) needs >>>> to be put a specific file. Now for your concerns, we can perhaps add >>>> something which will read the nvram contents directly from an nvram store. >>>> But that is irrelevant to this patch. The parsing is still needed, and all >>>> we would need to add is something which is reading the nvram contents from >>>> some other place >>> >>> >>> So it makes me wonder if we need this patch in its current form. I >>> think getting NVRAM directly from the platform is much user friendly. >>> It doesn't require user to install some extra tools for dumping NVRAM >>> and putting it in a specific file. One extra layer less. >>> With that said I think it's hard to review your code for parsing >>> NVRAM. We don't know how it's going to be fetched in the first place. >> >> You already made that point and we agreed to look for a solution. > > OK, it wasn't supposed to be rude or anything :) Thanks. No problem. Just wanted to make sure we are moving forward. >>>> though it would have to be put under some kernel config flag as this >>>> would not be supported in non-router systems. The contents of the nvram >>>> would however still need to be parsed in exactly the same way as the nvram >>>> files we read from disk. >>> >>> >>> Again, it's hard to say for me. Are you going to use >>> bcm47xx_nvram_getenv? Are you going to use MTD subsystem? Are you >>> going to develop different solution? When using e.g. >>> bcm47xx_nvram_getenv you won't want all this parsing stuff at all. >> >> >> Please look at the usage scenario here. The brcmfmac driver is not needing a >> few key,value pairs. It needs a portion of NVRAM to download to the device. >> The patch provides the functionality to do just that. Get the appropriate >> portion, strip comments and whitespace, and send it to the device. Using >> bcm47xx_nvram_getenv is not a useful api as it would mean we need brcmfmac >> to know which key ids to ask for, reassemble it to key=value string and send >> it to the fullmac device. >> >> In bcm47xx_nvram_getenv() it does have the whole nvram content available in >> nvram_buf which is filled by nvram_init(). So if something similar is made >> available on r8000 (or ARM routers in general) build target all we need is >> an api to get the nvram_buf. >> >> Another option is to add the parsing stuff in that nvram code and have an >> api to get the appropriate portion based on pcie domain and bus number as >> used in brcmf_fw_get_firmwares_pcie(). However, I would prefer to have this >> in the driver and not in arch specific code as there may be other platforms >> like our set-top boxes needing this. > > This is some plan for the future I was lacking from the beginning. It > makes things more clear now, thanks. You're welcome. Do you want to see this clarification in the commit message? Regards, Arend -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 20 April 2015 at 19:12, Arend van Spriel <arend@broadcom.com> wrote: > On 04/20/15 13:26, Rafa? Mi?ecki wrote: >> >> On 17 April 2015 at 10:50, Arend van Spriel<arend@broadcom.com> wrote: >>> Another option is to add the parsing stuff in that nvram code and have an >>> api to get the appropriate portion based on pcie domain and bus number as >>> used in brcmf_fw_get_firmwares_pcie(). However, I would prefer to have >>> this >>> in the driver and not in arch specific code as there may be other >>> platforms >>> like our set-top boxes needing this. >> >> >> This is some plan for the future I was lacking from the beginning. It >> makes things more clear now, thanks. > > You're welcome. Do you want to see this clarification in the commit message? I don't really need that, I'm leaving it up to you :) The last remaining question from me is if about this NVRAM validation function (if it exists).
On 04/20/15 20:49, Rafa? Mi?ecki wrote: > On 20 April 2015 at 19:12, Arend van Spriel<arend@broadcom.com> wrote: >> On 04/20/15 13:26, Rafa? Mi?ecki wrote: >>> >>> On 17 April 2015 at 10:50, Arend van Spriel<arend@broadcom.com> wrote: >>>> Another option is to add the parsing stuff in that nvram code and have an >>>> api to get the appropriate portion based on pcie domain and bus number as >>>> used in brcmf_fw_get_firmwares_pcie(). However, I would prefer to have >>>> this >>>> in the driver and not in arch specific code as there may be other >>>> platforms >>>> like our set-top boxes needing this. >>> >>> >>> This is some plan for the future I was lacking from the beginning. It >>> makes things more clear now, thanks. >> >> You're welcome. Do you want to see this clarification in the commit message? > > I don't really need that, I'm leaving it up to you :) > > The last remaining question from me is if about this NVRAM validation > function (if it exists). Ok. I can try to answer that one. The nvram parsing code in firmware.c intends to do just that. So apart from stripping comments and whitespace it validates each line and gives off a warning if a wrongly formatted entry is found. Regards, Arend -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 20 April 2015 at 22:16, Arend van Spriel <arend@broadcom.com> wrote: > On 04/20/15 20:49, Rafa? Mi?ecki wrote: >> >> On 20 April 2015 at 19:12, Arend van Spriel<arend@broadcom.com> wrote: >>> >>> On 04/20/15 13:26, Rafa? Mi?ecki wrote: >>>> >>>> >>>> On 17 April 2015 at 10:50, Arend van Spriel<arend@broadcom.com> wrote: >>>>> >>>>> Another option is to add the parsing stuff in that nvram code and have >>>>> an >>>>> api to get the appropriate portion based on pcie domain and bus number >>>>> as >>>>> used in brcmf_fw_get_firmwares_pcie(). However, I would prefer to have >>>>> this >>>>> in the driver and not in arch specific code as there may be other >>>>> platforms >>>>> like our set-top boxes needing this. >>>> >>>> >>>> >>>> This is some plan for the future I was lacking from the beginning. It >>>> makes things more clear now, thanks. >>> >>> >>> You're welcome. Do you want to see this clarification in the commit >>> message? >> >> >> I don't really need that, I'm leaving it up to you :) >> >> The last remaining question from me is if about this NVRAM validation >> function (if it exists). > > > Ok. I can try to answer that one. The nvram parsing code in firmware.c > intends to do just that. So apart from stripping comments and whitespace it > validates each line and gives off a warning if a wrongly formatted entry is > found. OK, thanks. So I guess this patch is OK after all?
On 04/21/15 12:16, Rafa? Mi?ecki wrote: > On 20 April 2015 at 22:16, Arend van Spriel<arend@broadcom.com> wrote: >> On 04/20/15 20:49, Rafa? Mi?ecki wrote: >>> >>> On 20 April 2015 at 19:12, Arend van Spriel<arend@broadcom.com> wrote: >>>> >>>> On 04/20/15 13:26, Rafa? Mi?ecki wrote: >>>>> >>>>> >>>>> On 17 April 2015 at 10:50, Arend van Spriel<arend@broadcom.com> wrote: >>>>>> >>>>>> Another option is to add the parsing stuff in that nvram code and have >>>>>> an >>>>>> api to get the appropriate portion based on pcie domain and bus number >>>>>> as >>>>>> used in brcmf_fw_get_firmwares_pcie(). However, I would prefer to have >>>>>> this >>>>>> in the driver and not in arch specific code as there may be other >>>>>> platforms >>>>>> like our set-top boxes needing this. >>>>> >>>>> >>>>> >>>>> This is some plan for the future I was lacking from the beginning. It >>>>> makes things more clear now, thanks. >>>> >>>> >>>> You're welcome. Do you want to see this clarification in the commit >>>> message? >>> >>> >>> I don't really need that, I'm leaving it up to you :) >>> >>> The last remaining question from me is if about this NVRAM validation >>> function (if it exists). >> >> >> Ok. I can try to answer that one. The nvram parsing code in firmware.c >> intends to do just that. So apart from stripping comments and whitespace it >> validates each line and gives off a warning if a wrongly formatted entry is >> found. > > OK, thanks. So I guess this patch is OK after all? Until proven otherwise. I tested it with a number of format violations, but there could be more creative people out there coming up with a pattern uncovering some issue. Regards, Arend -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 14 April 2015 at 20:10, Arend van Spriel <arend@broadcom.com> wrote: > +int brcmf_fw_get_firmwares(struct device *dev, u16 flags, > + const char *code, const char *nvram, > + void (*fw_cb)(struct device *dev, > + const struct firmware *fw, > + void *nvram_image, u32 nvram_len)) > +{ > + return brcmf_fw_get_firmwares_pcie(dev, flags, code, nvram, fw_cb, 0, > + 0); > +} > + git complains about adding an extra (2nd) empty line at EOF -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/firmware.c b/drivers/net/wireless/brcm80211/brcmfmac/firmware.c index 9cb9915..8ff31ff 100644 --- a/drivers/net/wireless/brcm80211/brcmfmac/firmware.c +++ b/drivers/net/wireless/brcm80211/brcmfmac/firmware.c @@ -23,6 +23,10 @@ #include "debug.h" #include "firmware.h" +#define BRCMF_FW_MAX_NVRAM_SIZE 64000 +#define BRCMF_FW_NVRAM_DEVPATH_LEN 19 /* devpath0=pcie/1/4/ */ +#define BRCMF_FW_NVRAM_PCIEDEV_LEN 9 /* pcie/1/4/ */ + char brcmf_firmware_path[BRCMF_FW_PATH_LEN]; module_param_string(firmware_path, brcmf_firmware_path, BRCMF_FW_PATH_LEN, 0440); @@ -46,6 +50,8 @@ enum nvram_parser_state { * @column: current column in line. * @pos: byte offset in input buffer. * @entry: start position of key,value entry. + * @multi_dev_v1: detect pcie multi device v1 (compressed). + * @multi_dev_v2: detect pcie multi device v2. */ struct nvram_parser { enum nvram_parser_state state; @@ -56,6 +62,8 @@ struct nvram_parser { u32 column; u32 pos; u32 entry; + bool multi_dev_v1; + bool multi_dev_v2; }; static bool is_nvram_char(char c) @@ -108,6 +116,10 @@ static enum nvram_parser_state brcmf_nvram_handle_key(struct nvram_parser *nvp) st = COMMENT; else st = VALUE; + if (strncmp(&nvp->fwnv->data[nvp->entry], "devpath", 7) == 0) + nvp->multi_dev_v1 = true; + if (strncmp(&nvp->fwnv->data[nvp->entry], "pcie/", 5) == 0) + nvp->multi_dev_v2 = true; } else if (!is_nvram_char(c)) { brcmf_dbg(INFO, "warning: ln=%d:col=%d: '=' expected, skip invalid key entry\n", nvp->line, nvp->column); @@ -133,6 +145,8 @@ brcmf_nvram_handle_value(struct nvram_parser *nvp) ekv = (u8 *)&nvp->fwnv->data[nvp->pos]; skv = (u8 *)&nvp->fwnv->data[nvp->entry]; cplen = ekv - skv; + if (nvp->nvram_len + cplen + 1 >= BRCMF_FW_MAX_NVRAM_SIZE) + return END; /* copy to output buffer */ memcpy(&nvp->nvram[nvp->nvram_len], skv, cplen); nvp->nvram_len += cplen; @@ -180,10 +194,18 @@ static enum nvram_parser_state static int brcmf_init_nvram_parser(struct nvram_parser *nvp, const struct firmware *nv) { + size_t size; + memset(nvp, 0, sizeof(*nvp)); nvp->fwnv = nv; + /* Limit size to MAX_NVRAM_SIZE, some files contain lot of comment */ + if (nv->size > BRCMF_FW_MAX_NVRAM_SIZE) + size = BRCMF_FW_MAX_NVRAM_SIZE; + else + size = nv->size; /* Alloc for extra 0 byte + roundup by 4 + length field */ - nvp->nvram = kzalloc(nv->size + 1 + 3 + sizeof(u32), GFP_KERNEL); + size += 1 + 3 + sizeof(u32); + nvp->nvram = kzalloc(size, GFP_KERNEL); if (!nvp->nvram) return -ENOMEM; @@ -192,12 +214,136 @@ static int brcmf_init_nvram_parser(struct nvram_parser *nvp, return 0; } +/* brcmf_fw_strip_multi_v1 :Some nvram files contain settings for multiple + * devices. Strip it down for one device, use domain_nr/bus_nr to determine + * which data is to be returned. v1 is the version where nvram is stored + * compressed and "devpath" maps to index for valid entries. + */ +static void brcmf_fw_strip_multi_v1(struct nvram_parser *nvp, u16 domain_nr, + u16 bus_nr) +{ + u32 i, j; + bool found; + u8 *nvram; + u8 id; + + nvram = kzalloc(nvp->nvram_len + 1 + 3 + sizeof(u32), GFP_KERNEL); + if (!nvram) + goto fail; + + /* min length: devpath0=pcie/1/4/ + 0:x=y */ + if (nvp->nvram_len < BRCMF_FW_NVRAM_DEVPATH_LEN + 6) + goto fail; + + /* First search for the devpathX and see if it is the configuration + * for domain_nr/bus_nr. Search complete nvp + */ + found = false; + i = 0; + while (i < nvp->nvram_len - BRCMF_FW_NVRAM_DEVPATH_LEN) { + /* Format: devpathX=pcie/Y/Z/ + * Y = domain_nr, Z = bus_nr, X = virtual ID + */ + if ((strncmp(&nvp->nvram[i], "devpath", 7) == 0) && + (strncmp(&nvp->nvram[i + 8], "=pcie/", 6) == 0)) { + if (((nvp->nvram[i + 14] - '0') == domain_nr) && + ((nvp->nvram[i + 16] - '0') == bus_nr)) { + id = nvp->nvram[i + 7] - '0'; + found = true; + break; + } + } + while (nvp->nvram[i] != 0) + i++; + i++; + } + if (!found) + goto fail; + + /* Now copy all valid entries, release old nvram and assign new one */ + i = 0; + j = 0; + while (i < nvp->nvram_len) { + if ((nvp->nvram[i] - '0' == id) && (nvp->nvram[i + 1] == ':')) { + i += 2; + while (nvp->nvram[i] != 0) { + nvram[j] = nvp->nvram[i]; + i++; + j++; + } + nvram[j] = 0; + j++; + } + while (nvp->nvram[i] != 0) + i++; + i++; + } + kfree(nvp->nvram); + nvp->nvram = nvram; + nvp->nvram_len = j; + return; + +fail: + kfree(nvram); + nvp->nvram_len = 0; +} + +/* brcmf_fw_strip_multi_v2 :Some nvram files contain settings for multiple + * devices. Strip it down for one device, use domain_nr/bus_nr to determine + * which data is to be returned. v2 is the version where nvram is stored + * uncompressed, all relevant valid entries are identified by + * pcie/domain_nr/bus_nr: + */ +static void brcmf_fw_strip_multi_v2(struct nvram_parser *nvp, u16 domain_nr, + u16 bus_nr) +{ + u32 i, j; + u8 *nvram; + + nvram = kzalloc(nvp->nvram_len + 1 + 3 + sizeof(u32), GFP_KERNEL); + if (!nvram) + goto fail; + + /* Copy all valid entries, release old nvram and assign new one. + * Valid entries are of type pcie/X/Y/ where X = domain_nr and + * Y = bus_nr. + */ + i = 0; + j = 0; + while (i < nvp->nvram_len - BRCMF_FW_NVRAM_PCIEDEV_LEN) { + if ((strncmp(&nvp->nvram[i], "pcie/", 5) == 0) && + (nvp->nvram[i + 6] == '/') && (nvp->nvram[i + 8] == '/') && + ((nvp->nvram[i + 5] - '0') == domain_nr) && + ((nvp->nvram[i + 7] - '0') == bus_nr)) { + i += BRCMF_FW_NVRAM_PCIEDEV_LEN; + while (nvp->nvram[i] != 0) { + nvram[j] = nvp->nvram[i]; + i++; + j++; + } + nvram[j] = 0; + j++; + } + while (nvp->nvram[i] != 0) + i++; + i++; + } + kfree(nvp->nvram); + nvp->nvram = nvram; + nvp->nvram_len = j; + return; +fail: + kfree(nvram); + nvp->nvram_len = 0; +} + /* brcmf_nvram_strip :Takes a buffer of "<var>=<value>\n" lines read from a fil * and ending in a NUL. Removes carriage returns, empty lines, comment lines, * and converts newlines to NULs. Shortens buffer as needed and pads with NULs. * End of buffer is completed with token identifying length of buffer. */ -static void *brcmf_fw_nvram_strip(const struct firmware *nv, u32 *new_length) +static void *brcmf_fw_nvram_strip(const struct firmware *nv, u32 *new_length, + u16 domain_nr, u16 bus_nr) { struct nvram_parser nvp; u32 pad; @@ -212,6 +358,16 @@ static void *brcmf_fw_nvram_strip(const struct firmware *nv, u32 *new_length) if (nvp.state == END) break; } + if (nvp.multi_dev_v1) + brcmf_fw_strip_multi_v1(&nvp, domain_nr, bus_nr); + else if (nvp.multi_dev_v2) + brcmf_fw_strip_multi_v2(&nvp, domain_nr, bus_nr); + + if (nvp.nvram_len == 0) { + kfree(nvp.nvram); + return NULL; + } + pad = nvp.nvram_len; *new_length = roundup(nvp.nvram_len + 1, 4); while (pad != *new_length) { @@ -239,6 +395,8 @@ struct brcmf_fw { u16 flags; const struct firmware *code; const char *nvram_name; + u16 domain_nr; + u16 bus_nr; void (*done)(struct device *dev, const struct firmware *fw, void *nvram_image, u32 nvram_len); }; @@ -254,7 +412,8 @@ static void brcmf_fw_request_nvram_done(const struct firmware *fw, void *ctx) goto fail; if (fw) { - nvram = brcmf_fw_nvram_strip(fw, &nvram_length); + nvram = brcmf_fw_nvram_strip(fw, &nvram_length, + fwctx->domain_nr, fwctx->bus_nr); release_firmware(fw); if (!nvram && !(fwctx->flags & BRCMF_FW_REQ_NV_OPTIONAL)) goto fail; @@ -309,11 +468,12 @@ fail: kfree(fwctx); } -int brcmf_fw_get_firmwares(struct device *dev, u16 flags, - const char *code, const char *nvram, - void (*fw_cb)(struct device *dev, - const struct firmware *fw, - void *nvram_image, u32 nvram_len)) +int brcmf_fw_get_firmwares_pcie(struct device *dev, u16 flags, + const char *code, const char *nvram, + void (*fw_cb)(struct device *dev, + const struct firmware *fw, + void *nvram_image, u32 nvram_len), + u16 domain_nr, u16 bus_nr) { struct brcmf_fw *fwctx; @@ -333,8 +493,21 @@ int brcmf_fw_get_firmwares(struct device *dev, u16 flags, fwctx->done = fw_cb; if (flags & BRCMF_FW_REQUEST_NVRAM) fwctx->nvram_name = nvram; + fwctx->domain_nr = domain_nr; + fwctx->bus_nr = bus_nr; return request_firmware_nowait(THIS_MODULE, true, code, dev, GFP_KERNEL, fwctx, brcmf_fw_request_code_done); } + +int brcmf_fw_get_firmwares(struct device *dev, u16 flags, + const char *code, const char *nvram, + void (*fw_cb)(struct device *dev, + const struct firmware *fw, + void *nvram_image, u32 nvram_len)) +{ + return brcmf_fw_get_firmwares_pcie(dev, flags, code, nvram, fw_cb, 0, + 0); +} + diff --git a/drivers/net/wireless/brcm80211/brcmfmac/firmware.h b/drivers/net/wireless/brcm80211/brcmfmac/firmware.h index 4d34823..604dd48 100644 --- a/drivers/net/wireless/brcm80211/brcmfmac/firmware.h +++ b/drivers/net/wireless/brcm80211/brcmfmac/firmware.h @@ -32,6 +32,12 @@ void brcmf_fw_nvram_free(void *nvram); * fails it will not use the callback, but call device_release_driver() * instead which will call the driver .remove() callback. */ +int brcmf_fw_get_firmwares_pcie(struct device *dev, u16 flags, + const char *code, const char *nvram, + void (*fw_cb)(struct device *dev, + const struct firmware *fw, + void *nvram_image, u32 nvram_len), + u16 domain_nr, u16 bus_nr); int brcmf_fw_get_firmwares(struct device *dev, u16 flags, const char *code, const char *nvram, void (*fw_cb)(struct device *dev, diff --git a/drivers/net/wireless/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/brcm80211/brcmfmac/pcie.c index 5e97a97..0453d10 100644 --- a/drivers/net/wireless/brcm80211/brcmfmac/pcie.c +++ b/drivers/net/wireless/brcm80211/brcmfmac/pcie.c @@ -1649,8 +1649,13 @@ brcmf_pcie_probe(struct pci_dev *pdev, const struct pci_device_id *id) struct brcmf_pciedev_info *devinfo; struct brcmf_pciedev *pcie_bus_dev; struct brcmf_bus *bus; + u16 domain_nr; + u16 bus_nr; - brcmf_dbg(PCIE, "Enter %x:%x\n", pdev->vendor, pdev->device); + domain_nr = pci_domain_nr(pdev->bus) + 1; + bus_nr = pdev->bus->number; + brcmf_dbg(PCIE, "Enter %x:%x (%d/%d)\n", pdev->vendor, pdev->device, + domain_nr, bus_nr); ret = -ENOMEM; devinfo = kzalloc(sizeof(*devinfo), GFP_KERNEL); @@ -1699,10 +1704,10 @@ brcmf_pcie_probe(struct pci_dev *pdev, const struct pci_device_id *id) if (ret) goto fail_bus; - ret = brcmf_fw_get_firmwares(bus->dev, BRCMF_FW_REQUEST_NVRAM | - BRCMF_FW_REQ_NV_OPTIONAL, - devinfo->fw_name, devinfo->nvram_name, - brcmf_pcie_setup); + ret = brcmf_fw_get_firmwares_pcie(bus->dev, BRCMF_FW_REQUEST_NVRAM | + BRCMF_FW_REQ_NV_OPTIONAL, + devinfo->fw_name, devinfo->nvram_name, + brcmf_pcie_setup, domain_nr, bus_nr); if (ret == 0) return 0; fail_bus: