| Message ID | 1432123792-4155-7-git-send-email-arend@broadcom.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
On 20 May 2015 at 14:09, Arend van Spriel <arend@broadcom.com> wrote: > From: Hante Meuleman <meuleman@broadcom.com> > > Host platforms such as routers supported by OpenWRT can > support NVRAM reading directly from internal NVRAM store. > With this patch the nvram load routines will fall back to > this method when there is no nvram file and support is > available in the kernel. FWIW it's OpenWrt :) > This patch relies on a change which has been submitted to the > linux-mips maintainer [1]. However, the brcmfmac code that relies on > it is under CONFIG_BCM47XX flag. Still need to know whether that > patch will be accepted or not before applying this one. Yeah, lets give this patch a few days at least, so we can be sure it'll be properly synced with MIPS work. I'm OK with brcmfmac internal changes (you may send them now as separated patch if you want to), but lets be careful with bcm47xx_nvram.h part. > @@ -19,6 +19,9 @@ > #include <linux/device.h> > #include <linux/firmware.h> > #include <linux/module.h> > +#if IS_ENABLED(CONFIG_BCM47XX) > +#include <linux/bcm47xx_nvram.h> > +#endif This header is safe to include on any arch, as well as all functions are. Drop the #if please. > @@ -66,6 +71,27 @@ struct nvram_parser { > bool multi_dev_v2; > }; > > +#if IS_ENABLED(CONFIG_BCM47XX) > +static char *brcmf_nvram_get_contents(size_t *nvram_size) > +{ > + return bcm47xx_nvram_get_contents(nvram_size); > +} > + > +static void brcmf_nvram_release_contents(char *nvram) > +{ > + bcm47xx_nvram_release_contents(nvram); > +} > +#else > +static char *brcmf_nvram_get_contents(size_t *nvram_size) > +{ > + return NULL; > +} > + > +static void brcmf_nvram_release_contents(char *nvram) > +{ > +} > +#endif Everything you put in #else simply re-implements what you sent for MIPS tree. We don't want to duplicate that code. Also applying above code will break building wireless-drivers-next on MIPS, we can't push this patch. I can understand you are looking for a way to get this patch into current -next and to somehow sync work across trees. I'm against anything like merging MIPS tree to wireless-driver-next, but I may have some idea. I think the best way for achieving this is to rework your patch to modify include/linux/bcm47xx_nvram.h. You could modify it the same way you did in your patch for MIPS tree, except for bcm47xx_nvram_get_contents. Don't implement this function for real (in .c file), but instead make in dummy inline in a bcm47xx_nvram.h like: static inline char *bcm47xx_nvram_get_contents(size_t *val_len) { /* TODO: Implement in .c file */ return NULL; }
On 20 May 2015 at 14:09, Arend van Spriel <arend@broadcom.com> wrote: > @@ -139,11 +165,11 @@ brcmf_nvram_handle_value(struct nvram_parser *nvp) > char *ekv; > u32 cplen; > > - c = nvp->fwnv->data[nvp->pos]; > - if (!is_nvram_char(c)) { > + c = nvp->data[nvp->pos]; > + if (!is_nvram_char(c) && (c != ' ')) { Don't smuggle behavior changes in patches doing something else! > @@ -406,19 +434,34 @@ static void brcmf_fw_request_nvram_done(const struct firmware *fw, void *ctx) > struct brcmf_fw *fwctx = ctx; > u32 nvram_length = 0; > void *nvram = NULL; > + u8 *data = NULL; > + size_t data_len; > + bool raw_nvram; > > brcmf_dbg(TRACE, "enter: dev=%s\n", dev_name(fwctx->dev)); > - if (!fw && !(fwctx->flags & BRCMF_FW_REQ_NV_OPTIONAL)) > - goto fail; > + if ((fw) && (fw->data)) { if (fw && fw->data) will work just fine, I'm surprised checkpatch doesn't complain. -- 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 May 2015 at 16:33, Rafa? Mi?ecki <zajec5@gmail.com> wrote: > I think the best way for achieving this is to rework your patch to > modify include/linux/bcm47xx_nvram.h. You could modify it the same way > you did in your patch for MIPS tree, except for > bcm47xx_nvram_get_contents. Don't implement this function for real (in > .c file), but instead make in dummy inline in a bcm47xx_nvram.h like: > static inline char *bcm47xx_nvram_get_contents(size_t *val_len) > { > /* TODO: Implement in .c file */ > return NULL; > } One more note. This of course will lead to conflict at some point, but I believe Linus will handle it.
On 05/21/15 10:28, Rafa? Mi?ecki wrote: > On 20 May 2015 at 16:33, Rafa? Mi?ecki<zajec5@gmail.com> wrote: >> I think the best way for achieving this is to rework your patch to >> modify include/linux/bcm47xx_nvram.h. You could modify it the same way >> you did in your patch for MIPS tree, except for >> bcm47xx_nvram_get_contents. Don't implement this function for real (in >> .c file), but instead make in dummy inline in a bcm47xx_nvram.h like: >> static inline char *bcm47xx_nvram_get_contents(size_t *val_len) >> { >> /* TODO: Implement in .c file */ >> return NULL; >> } > > One more note. > This of course will lead to conflict at some point, but I believe > Linus will handle it. I prefer to avoid tricks so I will ask to drop this patch and wait for it to land in the next kernel, ie. 4.2, and resubmit this patch for 4.3. I am not in a hurry. 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 05/21/15 11:28, Arend van Spriel wrote: > On 05/21/15 10:28, Rafa? Mi?ecki wrote: >> On 20 May 2015 at 16:33, Rafa? Mi?ecki<zajec5@gmail.com> wrote: >>> I think the best way for achieving this is to rework your patch to >>> modify include/linux/bcm47xx_nvram.h. You could modify it the same way >>> you did in your patch for MIPS tree, except for >>> bcm47xx_nvram_get_contents. Don't implement this function for real (in >>> .c file), but instead make in dummy inline in a bcm47xx_nvram.h like: >>> static inline char *bcm47xx_nvram_get_contents(size_t *val_len) >>> { >>> /* TODO: Implement in .c file */ >>> return NULL; >>> } >> >> One more note. >> This of course will lead to conflict at some point, but I believe >> Linus will handle it. > > I prefer to avoid tricks so I will ask to drop this patch and wait for > it to land in the next kernel, ie. 4.2, and resubmit this patch for 4.3. > I am not in a hurry. The 'it' in 'wait for it to land' being the mips patch providing the new api function. I will submit a v2 for that one. Regards, Arend > 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 21 May 2015 at 11:28, Arend van Spriel <arend@broadcom.com> wrote: > On 05/21/15 10:28, Rafa? Mi?ecki wrote: >> >> On 20 May 2015 at 16:33, Rafa? Mi?ecki<zajec5@gmail.com> wrote: >>> >>> I think the best way for achieving this is to rework your patch to >>> modify include/linux/bcm47xx_nvram.h. You could modify it the same way >>> you did in your patch for MIPS tree, except for >>> bcm47xx_nvram_get_contents. Don't implement this function for real (in >>> .c file), but instead make in dummy inline in a bcm47xx_nvram.h like: >>> static inline char *bcm47xx_nvram_get_contents(size_t *val_len) >>> { >>> /* TODO: Implement in .c file */ >>> return NULL; >>> } >> >> >> One more note. >> This of course will lead to conflict at some point, but I believe >> Linus will handle it. > > > I prefer to avoid tricks so I will ask to drop this patch and wait for it to > land in the next kernel, ie. 4.2, and resubmit this patch for 4.3. I am not > in a hurry. OK :)
Arend van Spriel <arend@broadcom.com> writes: > On 05/21/15 10:28, Rafa? Mi?ecki wrote: >> On 20 May 2015 at 16:33, Rafa? Mi?ecki<zajec5@gmail.com> wrote: >>> I think the best way for achieving this is to rework your patch to >>> modify include/linux/bcm47xx_nvram.h. You could modify it the same way >>> you did in your patch for MIPS tree, except for >>> bcm47xx_nvram_get_contents. Don't implement this function for real (in >>> .c file), but instead make in dummy inline in a bcm47xx_nvram.h like: >>> static inline char *bcm47xx_nvram_get_contents(size_t *val_len) >>> { >>> /* TODO: Implement in .c file */ >>> return NULL; >>> } >> >> One more note. >> This of course will lead to conflict at some point, but I believe >> Linus will handle it. > > I prefer to avoid tricks so I will ask to drop this patch and wait for > it to land in the next kernel, ie. 4.2, and resubmit this patch for > 4.3. I am not in a hurry. Yes, please :) Let's try to avoid conflicts as much as possible.
On 05/21/15 12:16, Kalle Valo wrote: > Arend van Spriel<arend@broadcom.com> writes: > >> On 05/21/15 10:28, Rafa? Mi?ecki wrote: >>> On 20 May 2015 at 16:33, Rafa? Mi?ecki<zajec5@gmail.com> wrote: >>>> I think the best way for achieving this is to rework your patch to >>>> modify include/linux/bcm47xx_nvram.h. You could modify it the same way >>>> you did in your patch for MIPS tree, except for >>>> bcm47xx_nvram_get_contents. Don't implement this function for real (in >>>> .c file), but instead make in dummy inline in a bcm47xx_nvram.h like: >>>> static inline char *bcm47xx_nvram_get_contents(size_t *val_len) >>>> { >>>> /* TODO: Implement in .c file */ >>>> return NULL; >>>> } >>> >>> One more note. >>> This of course will lead to conflict at some point, but I believe >>> Linus will handle it. >> >> I prefer to avoid tricks so I will ask to drop this patch and wait for >> it to land in the next kernel, ie. 4.2, and resubmit this patch for >> 4.3. I am not in a hurry. > > Yes, please :) Let's try to avoid conflicts as much as possible. Ok. Let's make it official. Kalle, can you drop patch 6 from the series (if not already done so ;-) ). 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
Arend van Spriel <arend@broadcom.com> writes: >>> I prefer to avoid tricks so I will ask to drop this patch and wait for >>> it to land in the next kernel, ie. 4.2, and resubmit this patch for >>> 4.3. I am not in a hurry. >> >> Yes, please :) Let's try to avoid conflicts as much as possible. > > Ok. Let's make it official. Kalle, can you drop patch 6 from the > series (if not already done so ;-) ). Ok, dropped.
The patch is dropped, but I still owe you a response to this. I missed it because you sent 2 or 3 replies confusing me. On 05/20/15 17:02, Rafa? Mi?ecki wrote: > On 20 May 2015 at 14:09, Arend van Spriel<arend@broadcom.com> wrote: >> @@ -139,11 +165,11 @@ brcmf_nvram_handle_value(struct nvram_parser *nvp) >> char *ekv; >> u32 cplen; >> >> - c = nvp->fwnv->data[nvp->pos]; >> - if (!is_nvram_char(c)) { >> + c = nvp->data[nvp->pos]; >> + if (!is_nvram_char(c)&& (c != ' ')) { > > Don't smuggle behavior changes in patches doing something else! The subject is "Add support for host platform NVRAM loading" and guess what. That type of NVRAM turned out to have spaces in the entries so in my opinion it is related to this patch. I can split it up if you feel strongly about this. >> @@ -406,19 +434,34 @@ static void brcmf_fw_request_nvram_done(const struct firmware *fw, void *ctx) >> struct brcmf_fw *fwctx = ctx; >> u32 nvram_length = 0; >> void *nvram = NULL; >> + u8 *data = NULL; >> + size_t data_len; >> + bool raw_nvram; >> >> brcmf_dbg(TRACE, "enter: dev=%s\n", dev_name(fwctx->dev)); >> - if (!fw&& !(fwctx->flags& BRCMF_FW_REQ_NV_OPTIONAL)) >> - goto fail; >> + if ((fw)&& (fw->data)) { > > if (fw&& fw->data) > will work just fine, I'm surprised checkpatch doesn't complain. I ran checkpatch.pl --strict and did not get complaint about this change. 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 22 May 2015 at 10:31, Arend van Spriel <arend@broadcom.com> wrote: > On 05/20/15 17:02, Rafa? Mi?ecki wrote: >> >> On 20 May 2015 at 14:09, Arend van Spriel<arend@broadcom.com> wrote: >>> >>> @@ -139,11 +165,11 @@ brcmf_nvram_handle_value(struct nvram_parser *nvp) >>> char *ekv; >>> u32 cplen; >>> >>> - c = nvp->fwnv->data[nvp->pos]; >>> - if (!is_nvram_char(c)) { >>> + c = nvp->data[nvp->pos]; >>> + if (!is_nvram_char(c)&& (c != ' ')) { >> >> >> Don't smuggle behavior changes in patches doing something else! > > > The subject is "Add support for host platform NVRAM loading" and guess what. > That type of NVRAM turned out to have spaces in the entries so in my opinion > it is related to this patch. I can split it up if you feel strongly about > this. I'd expect such patch to just implement *loading* from different source and nothing else. If there are additional changes needed, I think they should go in separated patch if possible. I noticed the same problem with parsing NVRAM values and sent [PATCH] brcmfmac: allow NVRAM values to contain space and '#' chars , so you should be able to drop this patch of your patch anyway. You may give me an Ack if you have a moment :) >>> @@ -406,19 +434,34 @@ static void brcmf_fw_request_nvram_done(const >>> struct firmware *fw, void *ctx) >>> struct brcmf_fw *fwctx = ctx; >>> u32 nvram_length = 0; >>> void *nvram = NULL; >>> + u8 *data = NULL; >>> + size_t data_len; >>> + bool raw_nvram; >>> >>> brcmf_dbg(TRACE, "enter: dev=%s\n", dev_name(fwctx->dev)); >>> - if (!fw&& !(fwctx->flags& BRCMF_FW_REQ_NV_OPTIONAL)) >>> - goto fail; >>> + if ((fw)&& (fw->data)) { >> >> >> if (fw&& fw->data) >> will work just fine, I'm surprised checkpatch doesn't complain. > > I ran checkpatch.pl --strict and did not get complaint about this change. I know, it's weird. Maybe I'll report this an improvement idea to checkpatch maintainer.
On 05/22/15 11:05, Rafa? Mi?ecki wrote: > On 22 May 2015 at 10:31, Arend van Spriel<arend@broadcom.com> wrote: >> On 05/20/15 17:02, Rafa? Mi?ecki wrote: >>> >>> On 20 May 2015 at 14:09, Arend van Spriel<arend@broadcom.com> wrote: >>>> >>>> @@ -139,11 +165,11 @@ brcmf_nvram_handle_value(struct nvram_parser *nvp) >>>> char *ekv; >>>> u32 cplen; >>>> >>>> - c = nvp->fwnv->data[nvp->pos]; >>>> - if (!is_nvram_char(c)) { >>>> + c = nvp->data[nvp->pos]; >>>> + if (!is_nvram_char(c)&& (c != ' ')) { >>> >>> >>> Don't smuggle behavior changes in patches doing something else! >> >> >> The subject is "Add support for host platform NVRAM loading" and guess what. >> That type of NVRAM turned out to have spaces in the entries so in my opinion >> it is related to this patch. I can split it up if you feel strongly about >> this. > > I'd expect such patch to just implement *loading* from different > source and nothing else. If there are additional changes needed, I > think they should go in separated patch if possible. > > I noticed the same problem with parsing NVRAM values and sent > [PATCH] brcmfmac: allow NVRAM values to contain space and '#' chars > , so you should be able to drop this patch of your patch anyway. > You may give me an Ack if you have a moment :) Whoops. I did not :-p I don't want to deal with '#' in value field as it is either invalid or irrelevant to firmware on the device. Regards, Arend >>>> @@ -406,19 +434,34 @@ static void brcmf_fw_request_nvram_done(const >>>> struct firmware *fw, void *ctx) >>>> struct brcmf_fw *fwctx = ctx; >>>> u32 nvram_length = 0; >>>> void *nvram = NULL; >>>> + u8 *data = NULL; >>>> + size_t data_len; >>>> + bool raw_nvram; >>>> >>>> brcmf_dbg(TRACE, "enter: dev=%s\n", dev_name(fwctx->dev)); >>>> - if (!fw&& !(fwctx->flags& BRCMF_FW_REQ_NV_OPTIONAL)) >>>> - goto fail; >>>> + if ((fw)&& (fw->data)) { >>> >>> >>> if (fw&& fw->data) >>> will work just fine, I'm surprised checkpatch doesn't complain. >> >> I ran checkpatch.pl --strict and did not get complaint about this change. > > I know, it's weird. Maybe I'll report this an improvement idea to > checkpatch maintainer. > -- 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 8ff31ff..bf8928d 100644 --- a/drivers/net/wireless/brcm80211/brcmfmac/firmware.c +++ b/drivers/net/wireless/brcm80211/brcmfmac/firmware.c @@ -19,6 +19,9 @@ #include <linux/device.h> #include <linux/firmware.h> #include <linux/module.h> +#if IS_ENABLED(CONFIG_BCM47XX) +#include <linux/bcm47xx_nvram.h> +#endif #include "debug.h" #include "firmware.h" @@ -43,7 +46,8 @@ enum nvram_parser_state { * struct nvram_parser - internal info for parser. * * @state: current parser state. - * @fwnv: input buffer being parsed. + * @data: input buffer data pointer. + * @data_len : len of input buffer. * @nvram: output buffer with parse result. * @nvram_len: lenght of parse result. * @line: current line. @@ -55,7 +59,8 @@ enum nvram_parser_state { */ struct nvram_parser { enum nvram_parser_state state; - const struct firmware *fwnv; + u8 *data; + u32 data_len; u8 *nvram; u32 nvram_len; u32 line; @@ -66,6 +71,27 @@ struct nvram_parser { bool multi_dev_v2; }; +#if IS_ENABLED(CONFIG_BCM47XX) +static char *brcmf_nvram_get_contents(size_t *nvram_size) +{ + return bcm47xx_nvram_get_contents(nvram_size); +} + +static void brcmf_nvram_release_contents(char *nvram) +{ + bcm47xx_nvram_release_contents(nvram); +} +#else +static char *brcmf_nvram_get_contents(size_t *nvram_size) +{ + return NULL; +} + +static void brcmf_nvram_release_contents(char *nvram) +{ +} +#endif + static bool is_nvram_char(char c) { /* comment marker excluded */ @@ -85,7 +111,7 @@ static enum nvram_parser_state brcmf_nvram_handle_idle(struct nvram_parser *nvp) { char c; - c = nvp->fwnv->data[nvp->pos]; + c = nvp->data[nvp->pos]; if (c == '\n') return COMMENT; if (is_whitespace(c)) @@ -109,16 +135,16 @@ static enum nvram_parser_state brcmf_nvram_handle_key(struct nvram_parser *nvp) enum nvram_parser_state st = nvp->state; char c; - c = nvp->fwnv->data[nvp->pos]; + c = nvp->data[nvp->pos]; if (c == '=') { /* ignore RAW1 by treating as comment */ - if (strncmp(&nvp->fwnv->data[nvp->entry], "RAW1", 4) == 0) + if (strncmp(&nvp->data[nvp->entry], "RAW1", 4) == 0) st = COMMENT; else st = VALUE; - if (strncmp(&nvp->fwnv->data[nvp->entry], "devpath", 7) == 0) + if (strncmp(&nvp->data[nvp->entry], "devpath", 7) == 0) nvp->multi_dev_v1 = true; - if (strncmp(&nvp->fwnv->data[nvp->entry], "pcie/", 5) == 0) + if (strncmp(&nvp->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", @@ -139,11 +165,11 @@ brcmf_nvram_handle_value(struct nvram_parser *nvp) char *ekv; u32 cplen; - c = nvp->fwnv->data[nvp->pos]; - if (!is_nvram_char(c)) { + c = nvp->data[nvp->pos]; + if (!is_nvram_char(c) && (c != ' ')) { /* key,value pair complete */ - ekv = (u8 *)&nvp->fwnv->data[nvp->pos]; - skv = (u8 *)&nvp->fwnv->data[nvp->entry]; + ekv = (u8 *)&nvp->data[nvp->pos]; + skv = (u8 *)&nvp->data[nvp->entry]; cplen = ekv - skv; if (nvp->nvram_len + cplen + 1 >= BRCMF_FW_MAX_NVRAM_SIZE) return END; @@ -164,7 +190,7 @@ brcmf_nvram_handle_comment(struct nvram_parser *nvp) { char *eol, *sol; - sol = (char *)&nvp->fwnv->data[nvp->pos]; + sol = (char *)&nvp->data[nvp->pos]; eol = strchr(sol, '\n'); if (eol == NULL) return END; @@ -191,18 +217,20 @@ static enum nvram_parser_state brcmf_nvram_handle_end }; -static int brcmf_init_nvram_parser(struct nvram_parser *nvp, - const struct firmware *nv) +static int brcmf_init_nvram_parser(struct nvram_parser *nvp, u8 *data, + u32 data_len) { size_t size; memset(nvp, 0, sizeof(*nvp)); - nvp->fwnv = nv; + nvp->data = data; + nvp->data_len = data_len; + /* Limit size to MAX_NVRAM_SIZE, some files contain lot of comment */ - if (nv->size > BRCMF_FW_MAX_NVRAM_SIZE) + if (data_len > BRCMF_FW_MAX_NVRAM_SIZE) size = BRCMF_FW_MAX_NVRAM_SIZE; else - size = nv->size; + size = data_len; /* Alloc for extra 0 byte + roundup by 4 + length field */ size += 1 + 3 + sizeof(u32); nvp->nvram = kzalloc(size, GFP_KERNEL); @@ -342,7 +370,7 @@ fail: * 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(u8 *data, u32 data_len, u32 *new_length, u16 domain_nr, u16 bus_nr) { struct nvram_parser nvp; @@ -350,10 +378,10 @@ static void *brcmf_fw_nvram_strip(const struct firmware *nv, u32 *new_length, u32 token; __le32 token_le; - if (brcmf_init_nvram_parser(&nvp, nv) < 0) + if (brcmf_init_nvram_parser(&nvp, data, data_len) < 0) return NULL; - while (nvp.pos < nv->size) { + while (nvp.pos < data_len) { nvp.state = nv_parser_states[nvp.state](&nvp); if (nvp.state == END) break; @@ -406,19 +434,34 @@ static void brcmf_fw_request_nvram_done(const struct firmware *fw, void *ctx) struct brcmf_fw *fwctx = ctx; u32 nvram_length = 0; void *nvram = NULL; + u8 *data = NULL; + size_t data_len; + bool raw_nvram; brcmf_dbg(TRACE, "enter: dev=%s\n", dev_name(fwctx->dev)); - if (!fw && !(fwctx->flags & BRCMF_FW_REQ_NV_OPTIONAL)) - goto fail; + if ((fw) && (fw->data)) { + data = (u8 *)fw->data; + data_len = fw->size; + raw_nvram = false; + } else { + data = brcmf_nvram_get_contents(&data_len); + if (!data && !(fwctx->flags & BRCMF_FW_REQ_NV_OPTIONAL)) + goto fail; + raw_nvram = true; + } - if (fw) { - nvram = brcmf_fw_nvram_strip(fw, &nvram_length, + if (data) { + nvram = brcmf_fw_nvram_strip(data, (u32)data_len, &nvram_length, fwctx->domain_nr, fwctx->bus_nr); - release_firmware(fw); - if (!nvram && !(fwctx->flags & BRCMF_FW_REQ_NV_OPTIONAL)) - goto fail; + if (raw_nvram) + brcmf_nvram_release_contents(data); } + if (fw) + release_firmware(fw); + if (!nvram && !(fwctx->flags & BRCMF_FW_REQ_NV_OPTIONAL)) + goto fail; + fwctx->done(fwctx->dev, fwctx->code, nvram, nvram_length); kfree(fwctx); return; @@ -453,15 +496,9 @@ static void brcmf_fw_request_code_done(const struct firmware *fw, void *ctx) if (!ret) return; - /* when nvram is optional call .done() callback here */ - if (fwctx->flags & BRCMF_FW_REQ_NV_OPTIONAL) { - fwctx->done(fwctx->dev, fw, NULL, 0); - kfree(fwctx); - return; - } + brcmf_fw_request_nvram_done(NULL, fwctx); + return; - /* failed nvram request */ - release_firmware(fw); fail: brcmf_dbg(TRACE, "failed: dev=%s\n", dev_name(fwctx->dev)); device_release_driver(fwctx->dev);