| Message ID | 1432813063-30922-1-git-send-email-zajec5@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Kalle Valo |
| Headers | show |
On 05/28/15 13:37, Rafa? Mi?ecki wrote: > As we plan to add support for platform NVRAM we should store direct > data pointer without the extra struct firmware layer. This will allow > us to support other sources with the only requirement being u8 buffer. > > Signed-off-by: Rafa? Mi?ecki<zajec5@gmail.com> > Signed-off-by: Hante Meuleman<meuleman@broadcom.com> > Signed-off-by: Arend van Spriel<arend@broadcom.com> > --- > Tested on router with BCM43602-s using /lib/firmware/brcm/brcmfmac43602-pcie.txt > > I've written this patch from scratch, it's inspired by the dropped: > [PATCH 6/6] brcmfmac: Add support for host platform NVRAM loading. Hi Rafa?, So what is your goal here. The inspirational patch was dropped so it can be resubmitted when the mips change it relies on has made its way upstream. So I have to rebase the patch over here and your patch will just give me conflicts during that rebase. So can we please wait or do you need this change right now. Regards, Arend > --- > drivers/net/wireless/brcm80211/brcmfmac/firmware.c | 40 +++++++++++----------- > 1 file changed, 20 insertions(+), 20 deletions(-) > > diff --git a/drivers/net/wireless/brcm80211/brcmfmac/firmware.c b/drivers/net/wireless/brcm80211/brcmfmac/firmware.c > index 7ae6461..b72df87 100644 > --- a/drivers/net/wireless/brcm80211/brcmfmac/firmware.c > +++ b/drivers/net/wireless/brcm80211/brcmfmac/firmware.c > @@ -43,7 +43,7 @@ enum nvram_parser_state { > * struct nvram_parser - internal info for parser. > * > * @state: current parser state. > - * @fwnv: input buffer being parsed. > + * @data: input buffer being parsed. > * @nvram: output buffer with parse result. > * @nvram_len: lenght of parse result. > * @line: current line. > @@ -55,7 +55,7 @@ enum nvram_parser_state { > */ > struct nvram_parser { > enum nvram_parser_state state; > - const struct firmware *fwnv; > + const u8 *data; > u8 *nvram; > u32 nvram_len; > u32 line; > @@ -91,7 +91,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)) > @@ -115,16 +115,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) || c == ' ') { > brcmf_dbg(INFO, "warning: ln=%d:col=%d: '=' expected, skip invalid key entry\n", > @@ -145,11 +145,11 @@ brcmf_nvram_handle_value(struct nvram_parser *nvp) > char *ekv; > u32 cplen; > > - c = nvp->fwnv->data[nvp->pos]; > + c = nvp->data[nvp->pos]; > if (!is_nvram_char(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; > @@ -170,7 +170,7 @@ brcmf_nvram_handle_comment(struct nvram_parser *nvp) > { > char *eoc, *sol; > > - sol = (char *)&nvp->fwnv->data[nvp->pos]; > + sol = (char *)&nvp->data[nvp->pos]; > eoc = strchr(sol, '\n'); > if (!eoc) { > eoc = strchr(sol, '\0'); > @@ -201,17 +201,17 @@ static enum nvram_parser_state > }; > > static int brcmf_init_nvram_parser(struct nvram_parser *nvp, > - const struct firmware *nv) > + const u8 *data, size_t data_len) > { > size_t size; > > memset(nvp, 0, sizeof(*nvp)); > - nvp->fwnv = nv; > + nvp->data = data; > /* 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); > @@ -356,18 +356,18 @@ 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, > - u16 domain_nr, u16 bus_nr) > +static void *brcmf_fw_nvram_strip(const u8 *data, size_t data_len, > + u32 *new_length, u16 domain_nr, u16 bus_nr) > { > struct nvram_parser nvp; > u32 pad; > 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; > @@ -426,7 +426,7 @@ 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->data, fw->size,&nvram_length, > fwctx->domain_nr, fwctx->bus_nr); > release_firmware(fw); > if (!nvram&& !(fwctx->flags& BRCMF_FW_REQ_NV_OPTIONAL)) -- 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 28 May 2015 at 13:54, Arend van Spriel <arend@broadcom.com> wrote: > On 05/28/15 13:37, Rafa? Mi?ecki wrote: >> >> As we plan to add support for platform NVRAM we should store direct >> data pointer without the extra struct firmware layer. This will allow >> us to support other sources with the only requirement being u8 buffer. >> >> Signed-off-by: Rafa? Mi?ecki<zajec5@gmail.com> >> Signed-off-by: Hante Meuleman<meuleman@broadcom.com> >> Signed-off-by: Arend van Spriel<arend@broadcom.com> >> --- >> Tested on router with BCM43602-s using >> /lib/firmware/brcm/brcmfmac43602-pcie.txt >> >> I've written this patch from scratch, it's inspired by the dropped: >> [PATCH 6/6] brcmfmac: Add support for host platform NVRAM loading. > > > Hi Rafa?, > > So what is your goal here. The inspirational patch was dropped so it can be > resubmitted when the mips change it relies on has made its way upstream. So > I have to rebase the patch over here and your patch will just give me > conflicts during that rebase. So can we please wait or do you need this > change right now. The dropped patch will require rebasing/rewriting anyway. There were few changes to firmware.c already, I've few more planned, you'll have to drop some code form your patch (parts that will go into MIPS tree) and probably apply few changes as requested in comments. I also don't think it makes much sense to pause any development because of having some out-of-tree patch queued for later submitting. And after all, hey, look at the bright side! :) With this patch you'll have to maintain smaller amount of out-of-tree(-for-now) code :) So my goals are: 1) Have all required cleanups pushed mainline early. 2) Make it easier to main out-of-tree changes. The personal reason behind that is to add OpenWrt support for BCM43602 as early as possible. Having clean backports + tiny NVRAM patch make it much easier to do now, maintain and update in the future. -- 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/28/15 14:34, Rafa? Mi?ecki wrote: > On 28 May 2015 at 13:54, Arend van Spriel<arend@broadcom.com> wrote: >> On 05/28/15 13:37, Rafa? Mi?ecki wrote: >>> >>> As we plan to add support for platform NVRAM we should store direct >>> data pointer without the extra struct firmware layer. This will allow >>> us to support other sources with the only requirement being u8 buffer. >>> >>> Signed-off-by: Rafa? Mi?ecki<zajec5@gmail.com> >>> Signed-off-by: Hante Meuleman<meuleman@broadcom.com> >>> Signed-off-by: Arend van Spriel<arend@broadcom.com> >>> --- >>> Tested on router with BCM43602-s using >>> /lib/firmware/brcm/brcmfmac43602-pcie.txt >>> >>> I've written this patch from scratch, it's inspired by the dropped: >>> [PATCH 6/6] brcmfmac: Add support for host platform NVRAM loading. >> >> >> Hi Rafa?, >> >> So what is your goal here. The inspirational patch was dropped so it can be >> resubmitted when the mips change it relies on has made its way upstream. So >> I have to rebase the patch over here and your patch will just give me >> conflicts during that rebase. So can we please wait or do you need this >> change right now. > > The dropped patch will require rebasing/rewriting anyway. There were > few changes to firmware.c already, I've few more planned, you'll have > to drop some code form your patch (parts that will go into MIPS tree) > and probably apply few changes as requested in comments. I already did that and submitted the mips part to Ralf. I am sorry to say this but what annoys me is that since then you started submitting patches that seem to be taken from the dropped patch. So I have a problem seeing the bright side. If Ralf takes the mips part it ends up in linux-next and we can submit the brcmfmac part. Regards, Arend > I also don't think it makes much sense to pause any development > because of having some out-of-tree patch queued for later submitting. > > And after all, hey, look at the bright side! :) With this patch you'll > have to maintain smaller amount of out-of-tree(-for-now) code :) > > So my goals are: > 1) Have all required cleanups pushed mainline early. > 2) Make it easier to main out-of-tree changes. > The personal reason behind that is to add OpenWrt support for BCM43602 > as early as possible. Having clean backports + tiny NVRAM patch make > it much easier to do now, maintain and update in the future. -- 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 28 May 2015 at 23:24, Arend van Spriel <arend@broadcom.com> wrote: > On 05/28/15 14:34, Rafa? Mi?ecki wrote: >> >> On 28 May 2015 at 13:54, Arend van Spriel<arend@broadcom.com> wrote: >>> >>> On 05/28/15 13:37, Rafa? Mi?ecki wrote: >>>> >>>> >>>> As we plan to add support for platform NVRAM we should store direct >>>> data pointer without the extra struct firmware layer. This will allow >>>> us to support other sources with the only requirement being u8 buffer. >>>> >>>> Signed-off-by: Rafa? Mi?ecki<zajec5@gmail.com> >>>> Signed-off-by: Hante Meuleman<meuleman@broadcom.com> >>>> Signed-off-by: Arend van Spriel<arend@broadcom.com> >>>> --- >>>> Tested on router with BCM43602-s using >>>> /lib/firmware/brcm/brcmfmac43602-pcie.txt >>>> >>>> I've written this patch from scratch, it's inspired by the dropped: >>>> [PATCH 6/6] brcmfmac: Add support for host platform NVRAM loading. >>> >>> >>> >>> Hi Rafa?, >>> >>> So what is your goal here. The inspirational patch was dropped so it can >>> be >>> resubmitted when the mips change it relies on has made its way upstream. >>> So >>> I have to rebase the patch over here and your patch will just give me >>> conflicts during that rebase. So can we please wait or do you need this >>> change right now. >> >> >> The dropped patch will require rebasing/rewriting anyway. There were >> few changes to firmware.c already, I've few more planned, you'll have >> to drop some code form your patch (parts that will go into MIPS tree) >> and probably apply few changes as requested in comments. > > > I already did that and submitted the mips part to Ralf. I am sorry to say > this but what annoys me is that since then you started submitting patches > that seem to be taken from the dropped patch. So I have a problem seeing the > bright side. If Ralf takes the mips part it ends up in linux-next and we can > submit the brcmfmac part. This is truly the first patch based on your dropped one. All other 5 patches were addressing problems I noticed by myself. One of them fixed the same issue you /silently/ did in the dropped one, but I wasn't even aware of that until I started rebasing your patch. We simply noticed the same problem and fixed it on our owns. So I think this patch is the only one that could annoy you, but *honestly* my intention was exactly the opposite. As already said, I just wanted to do possible cleanup early and let you maintain 50% of your original patch instead the whole one. I really don't want to have you annoyed because the need of rebasing your patch. What you said about Ralf's tree and linux-next isn't exactly true. I do believe Kalle won't merge linux-next into wireless-driver-next, so we have to wait for 4.2-rc1, then for Dave merging Linus's tree, then Kalle merging Dave's tree. We really shouldn't stop development for weeks because of having some patch prepared for later submitting. I think the only think you can do to make your life easier is to submit cleanup part of your prepared patch. This is exactly what I tried to do for you.
On 05/29/15 07:20, Rafa? Mi?ecki wrote: > On 28 May 2015 at 23:24, Arend van Spriel<arend@broadcom.com> wrote: >> On 05/28/15 14:34, Rafa? Mi?ecki wrote: >>> >>> On 28 May 2015 at 13:54, Arend van Spriel<arend@broadcom.com> wrote: >>>> >>>> On 05/28/15 13:37, Rafa? Mi?ecki wrote: >>>>> >>>>> >>>>> As we plan to add support for platform NVRAM we should store direct >>>>> data pointer without the extra struct firmware layer. This will allow >>>>> us to support other sources with the only requirement being u8 buffer. >>>>> >>>>> Signed-off-by: Rafa? Mi?ecki<zajec5@gmail.com> >>>>> Signed-off-by: Hante Meuleman<meuleman@broadcom.com> >>>>> Signed-off-by: Arend van Spriel<arend@broadcom.com> >>>>> --- >>>>> Tested on router with BCM43602-s using >>>>> /lib/firmware/brcm/brcmfmac43602-pcie.txt >>>>> >>>>> I've written this patch from scratch, it's inspired by the dropped: >>>>> [PATCH 6/6] brcmfmac: Add support for host platform NVRAM loading. >>>> >>>> >>>> >>>> Hi Rafa?, >>>> >>>> So what is your goal here. The inspirational patch was dropped so it can >>>> be >>>> resubmitted when the mips change it relies on has made its way upstream. >>>> So >>>> I have to rebase the patch over here and your patch will just give me >>>> conflicts during that rebase. So can we please wait or do you need this >>>> change right now. >>> >>> >>> The dropped patch will require rebasing/rewriting anyway. There were >>> few changes to firmware.c already, I've few more planned, you'll have >>> to drop some code form your patch (parts that will go into MIPS tree) >>> and probably apply few changes as requested in comments. >> >> >> I already did that and submitted the mips part to Ralf. I am sorry to say >> this but what annoys me is that since then you started submitting patches >> that seem to be taken from the dropped patch. So I have a problem seeing the >> bright side. If Ralf takes the mips part it ends up in linux-next and we can >> submit the brcmfmac part. > > This is truly the first patch based on your dropped one. All other 5 > patches were addressing problems I noticed by myself. One of them > fixed the same issue you /silently/ did in the dropped one, but I > wasn't even aware of that until I started rebasing your patch. We > simply noticed the same problem and fixed it on our owns. > > So I think this patch is the only one that could annoy you, but > *honestly* my intention was exactly the opposite. As already said, I > just wanted to do possible cleanup early and let you maintain 50% of > your original patch instead the whole one. > > I really don't want to have you annoyed because the need of rebasing > your patch. What you said about Ralf's tree and linux-next isn't > exactly true. I do believe Kalle won't merge linux-next into > wireless-driver-next, so we have to wait for 4.2-rc1, then for Dave > merging Linus's tree, then Kalle merging Dave's tree. Well, it is an integration issue. So yes, wireless-drivers-next (and net-next) would not build for CONFIG_BCM47XX with the brcmfmac patch, because of the missing mips patch. However, the linux-next tree will have no build issue and consequently 4.2-rc1 will have no build issue. I think this is acceptable although I would like to hear the opinion of Kalle on this. Regards, Arend > We really > shouldn't stop development for weeks because of having some patch > prepared for later submitting. > I think the only think you can do to make your life easier is to > submit cleanup part of your prepared patch. This is exactly what I > tried to do for you. > -- 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 29 May 2015 at 10:03, Arend van Spriel <arend@broadcom.com> wrote: > On 05/29/15 07:20, Rafa? Mi?ecki wrote: >> On 28 May 2015 at 23:24, Arend van Spriel<arend@broadcom.com> wrote: >>> On 05/28/15 14:34, Rafa? Mi?ecki wrote: >>>> On 28 May 2015 at 13:54, Arend van Spriel<arend@broadcom.com> wrote: >>>>> On 05/28/15 13:37, Rafa? Mi?ecki wrote: >>>>>> As we plan to add support for platform NVRAM we should store direct >>>>>> data pointer without the extra struct firmware layer. This will allow >>>>>> us to support other sources with the only requirement being u8 buffer. >>>>>> >>>>>> Signed-off-by: Rafa? Mi?ecki<zajec5@gmail.com> >>>>>> Signed-off-by: Hante Meuleman<meuleman@broadcom.com> >>>>>> Signed-off-by: Arend van Spriel<arend@broadcom.com> >>>>>> --- >>>>>> Tested on router with BCM43602-s using >>>>>> /lib/firmware/brcm/brcmfmac43602-pcie.txt >>>>>> >>>>>> I've written this patch from scratch, it's inspired by the dropped: >>>>>> [PATCH 6/6] brcmfmac: Add support for host platform NVRAM loading. >>>>> >>>>> So what is your goal here. The inspirational patch was dropped so it >>>>> can >>>>> be >>>>> resubmitted when the mips change it relies on has made its way >>>>> upstream. >>>>> So >>>>> I have to rebase the patch over here and your patch will just give me >>>>> conflicts during that rebase. So can we please wait or do you need this >>>>> change right now. >>>> >>>> The dropped patch will require rebasing/rewriting anyway. There were >>>> few changes to firmware.c already, I've few more planned, you'll have >>>> to drop some code form your patch (parts that will go into MIPS tree) >>>> and probably apply few changes as requested in comments. >>> >>> I already did that and submitted the mips part to Ralf. I am sorry to say >>> this but what annoys me is that since then you started submitting patches >>> that seem to be taken from the dropped patch. So I have a problem seeing >>> the >>> bright side. If Ralf takes the mips part it ends up in linux-next and we >>> can >>> submit the brcmfmac part. >> >> This is truly the first patch based on your dropped one. All other 5 >> patches were addressing problems I noticed by myself. One of them >> fixed the same issue you /silently/ did in the dropped one, but I >> wasn't even aware of that until I started rebasing your patch. We >> simply noticed the same problem and fixed it on our owns. >> >> So I think this patch is the only one that could annoy you, but >> *honestly* my intention was exactly the opposite. As already said, I >> just wanted to do possible cleanup early and let you maintain 50% of >> your original patch instead the whole one. >> >> I really don't want to have you annoyed because the need of rebasing >> your patch. What you said about Ralf's tree and linux-next isn't >> exactly true. I do believe Kalle won't merge linux-next into >> wireless-driver-next, so we have to wait for 4.2-rc1, then for Dave >> merging Linus's tree, then Kalle merging Dave's tree. > > Well, it is an integration issue. So yes, wireless-drivers-next (and > net-next) would not build for CONFIG_BCM47XX with the brcmfmac patch, > because of the missing mips patch. However, the linux-next tree will have no > build issue and consequently 4.2-rc1 will have no build issue. I think this > is acceptable although I would like to hear the opinion of Kalle on this. First, I don't really like the idea of breaking building of a tree, even if it affects a one architecture only. Secondly, limiting breakage to one arch only will require you to use some magic #if-s we want to avoid. And then (after having 4.2 in wireless-drivers-next) you'll have to drop them anyway (just to clean the code). So I think leaving this stuff for 4.3 is the best choice.
On 05/29/15 10:03, Arend van Spriel wrote: > On 05/29/15 07:20, Rafa? Mi?ecki wrote: >> On 28 May 2015 at 23:24, Arend van Spriel<arend@broadcom.com> wrote: >>> On 05/28/15 14:34, Rafa? Mi?ecki wrote: >>>> >>>> On 28 May 2015 at 13:54, Arend van Spriel<arend@broadcom.com> wrote: >>>>> >>>>> On 05/28/15 13:37, Rafa? Mi?ecki wrote: >>>>>> >>>>>> >>>>>> As we plan to add support for platform NVRAM we should store direct >>>>>> data pointer without the extra struct firmware layer. This will allow >>>>>> us to support other sources with the only requirement being u8 >>>>>> buffer. >>>>>> >>>>>> Signed-off-by: Rafa? Mi?ecki<zajec5@gmail.com> >>>>>> Signed-off-by: Hante Meuleman<meuleman@broadcom.com> >>>>>> Signed-off-by: Arend van Spriel<arend@broadcom.com> >>>>>> --- >>>>>> Tested on router with BCM43602-s using >>>>>> /lib/firmware/brcm/brcmfmac43602-pcie.txt >>>>>> >>>>>> I've written this patch from scratch, it's inspired by the dropped: >>>>>> [PATCH 6/6] brcmfmac: Add support for host platform NVRAM loading. >>>>> >>>>> >>>>> >>>>> Hi Rafa?, >>>>> >>>>> So what is your goal here. The inspirational patch was dropped so >>>>> it can >>>>> be >>>>> resubmitted when the mips change it relies on has made its way >>>>> upstream. >>>>> So >>>>> I have to rebase the patch over here and your patch will just give me >>>>> conflicts during that rebase. So can we please wait or do you need >>>>> this >>>>> change right now. >>>> >>>> >>>> The dropped patch will require rebasing/rewriting anyway. There were >>>> few changes to firmware.c already, I've few more planned, you'll have >>>> to drop some code form your patch (parts that will go into MIPS tree) >>>> and probably apply few changes as requested in comments. >>> >>> >>> I already did that and submitted the mips part to Ralf. I am sorry to >>> say >>> this but what annoys me is that since then you started submitting >>> patches >>> that seem to be taken from the dropped patch. So I have a problem >>> seeing the >>> bright side. If Ralf takes the mips part it ends up in linux-next and >>> we can >>> submit the brcmfmac part. >> >> This is truly the first patch based on your dropped one. All other 5 >> patches were addressing problems I noticed by myself. One of them >> fixed the same issue you /silently/ did in the dropped one, but I >> wasn't even aware of that until I started rebasing your patch. We >> simply noticed the same problem and fixed it on our owns. >> >> So I think this patch is the only one that could annoy you, but >> *honestly* my intention was exactly the opposite. As already said, I >> just wanted to do possible cleanup early and let you maintain 50% of >> your original patch instead the whole one. >> >> I really don't want to have you annoyed because the need of rebasing >> your patch. What you said about Ralf's tree and linux-next isn't >> exactly true. I do believe Kalle won't merge linux-next into >> wireless-driver-next, so we have to wait for 4.2-rc1, then for Dave >> merging Linus's tree, then Kalle merging Dave's tree. > > Well, it is an integration issue. So yes, wireless-drivers-next (and > net-next) would not build for CONFIG_BCM47XX with the brcmfmac patch, > because of the missing mips patch. However, the linux-next tree will > have no build issue and consequently 4.2-rc1 will have no build issue. I > think this is acceptable although I would like to hear the opinion of > Kalle on this. Hi Kalle, Do you have an opinion on this? mips-next and linux-next now have the function brcmfmac needs for CONFIG_BCM47XX so I could submit brcmfmac patch, but that means wireless-drivers-next (and net-next) will not build for CONFIG_BCM47XX until after the 4.2 merge window. Regards, Arend > Regards, > Arend > >> We really >> shouldn't stop development for weeks because of having some patch >> prepared for later submitting. >> I think the only think you can do to make your life easier is to >> submit cleanup part of your prepared patch. This is exactly what I >> tried to do for you. >> > -- 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
Hi Arend, Arend van Spriel <arend@broadcom.com> writes: >>> I really don't want to have you annoyed because the need of rebasing >>> your patch. What you said about Ralf's tree and linux-next isn't >>> exactly true. I do believe Kalle won't merge linux-next into >>> wireless-driver-next, so we have to wait for 4.2-rc1, then for Dave >>> merging Linus's tree, then Kalle merging Dave's tree. >> >> Well, it is an integration issue. So yes, wireless-drivers-next (and >> net-next) would not build for CONFIG_BCM47XX with the brcmfmac patch, >> because of the missing mips patch. However, the linux-next tree will >> have no build issue and consequently 4.2-rc1 will have no build issue. I >> think this is acceptable although I would like to hear the opinion of >> Kalle on this. > > Hi Kalle, > > Do you have an opinion on this? mips-next and linux-next now have the > function brcmfmac needs for CONFIG_BCM47XX so I could submit brcmfmac > patch, but that means wireless-drivers-next (and net-next) will not > build for CONFIG_BCM47XX until after the 4.2 merge window. The answer is clear: we must not ever break the build deliberately, in under any circumstances. Mistakes of course always happen but if we know a patch will break the build it should not be applied. So if patch A has a build dependency on patch B in another tree, we have to wait for that patch B to trickle down to wireless-drivers-next before I can commit patch A. There are other ways to speed this up if really needed but I don't see this case is important enough for the extra hurdle. I haven't followed all the details so I might be missing something but I think it would be good just to wait for 4.2-rc1, it's not that far anyway, and go with the original Arend's plan.
On 06/04/15 16:23, Kalle Valo wrote: > Hi Arend, > > Arend van Spriel<arend@broadcom.com> writes: > >>>> I really don't want to have you annoyed because the need of rebasing >>>> your patch. What you said about Ralf's tree and linux-next isn't >>>> exactly true. I do believe Kalle won't merge linux-next into >>>> wireless-driver-next, so we have to wait for 4.2-rc1, then for Dave >>>> merging Linus's tree, then Kalle merging Dave's tree. >>> >>> Well, it is an integration issue. So yes, wireless-drivers-next (and >>> net-next) would not build for CONFIG_BCM47XX with the brcmfmac patch, >>> because of the missing mips patch. However, the linux-next tree will >>> have no build issue and consequently 4.2-rc1 will have no build issue. I >>> think this is acceptable although I would like to hear the opinion of >>> Kalle on this. >> >> Hi Kalle, >> >> Do you have an opinion on this? mips-next and linux-next now have the >> function brcmfmac needs for CONFIG_BCM47XX so I could submit brcmfmac >> patch, but that means wireless-drivers-next (and net-next) will not >> build for CONFIG_BCM47XX until after the 4.2 merge window. > > The answer is clear: we must not ever break the build deliberately, in > under any circumstances. Mistakes of course always happen but if we know > a patch will break the build it should not be applied. > > So if patch A has a build dependency on patch B in another tree, we have > to wait for that patch B to trickle down to wireless-drivers-next before > I can commit patch A. There are other ways to speed this up if really > needed but I don't see this case is important enough for the extra > hurdle. > > I haven't followed all the details so I might be missing something but I > think it would be good just to wait for 4.2-rc1, it's not that far > anyway, and go with the original Arend's plan. Ok. I will submit the brcmfmac patch relying on the mips change to wireless-drivers-next for 4.3 after 4.2-rc1 merge window. I will ack the patch from Rafal in seperate email. 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/28/15 13:37, Rafa? Mi?ecki wrote: > As we plan to add support for platform NVRAM we should store direct > data pointer without the extra struct firmware layer. This will allow > us to support other sources with the only requirement being u8 buffer. > > Signed-off-by: Rafa? Mi?ecki<zajec5@gmail.com> > Signed-off-by: Hante Meuleman<meuleman@broadcom.com> > Signed-off-by: Arend van Spriel<arend@broadcom.com> This look a bit contradictory. You mention below you written this from 'scratch' so it seems odd that it is signed off by me and Hante. You may add: Acked-by: Arend van Spriel <arend@broadcom.com> > --- > Tested on router with BCM43602-s using /lib/firmware/brcm/brcmfmac43602-pcie.txt > > I've written this patch from scratch, it's inspired by the dropped: > [PATCH 6/6] brcmfmac: Add support for host platform NVRAM loading. > --- > drivers/net/wireless/brcm80211/brcmfmac/firmware.c | 40 +++++++++++----------- > 1 file changed, 20 insertions(+), 20 deletions(-) > > diff --git a/drivers/net/wireless/brcm80211/brcmfmac/firmware.c b/drivers/net/wireless/brcm80211/brcmfmac/firmware.c > index 7ae6461..b72df87 100644 > --- a/drivers/net/wireless/brcm80211/brcmfmac/firmware.c > +++ b/drivers/net/wireless/brcm80211/brcmfmac/firmware.c > @@ -43,7 +43,7 @@ enum nvram_parser_state { > * struct nvram_parser - internal info for parser. > * > * @state: current parser state. > - * @fwnv: input buffer being parsed. > + * @data: input buffer being parsed. So you are not adding the data_len to the structure, which means you found it is not needed? Just checking as it is different from the aforementioned patch. Regards, Arend > * @nvram: output buffer with parse result. > * @nvram_len: lenght of parse result. > * @line: current line. > @@ -55,7 +55,7 @@ enum nvram_parser_state { > */ > struct nvram_parser { > enum nvram_parser_state state; > - const struct firmware *fwnv; > + const u8 *data; > u8 *nvram; > u32 nvram_len; > u32 line; > @@ -91,7 +91,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)) > @@ -115,16 +115,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) || c == ' ') { > brcmf_dbg(INFO, "warning: ln=%d:col=%d: '=' expected, skip invalid key entry\n", > @@ -145,11 +145,11 @@ brcmf_nvram_handle_value(struct nvram_parser *nvp) > char *ekv; > u32 cplen; > > - c = nvp->fwnv->data[nvp->pos]; > + c = nvp->data[nvp->pos]; > if (!is_nvram_char(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; > @@ -170,7 +170,7 @@ brcmf_nvram_handle_comment(struct nvram_parser *nvp) > { > char *eoc, *sol; > > - sol = (char *)&nvp->fwnv->data[nvp->pos]; > + sol = (char *)&nvp->data[nvp->pos]; > eoc = strchr(sol, '\n'); > if (!eoc) { > eoc = strchr(sol, '\0'); > @@ -201,17 +201,17 @@ static enum nvram_parser_state > }; > > static int brcmf_init_nvram_parser(struct nvram_parser *nvp, > - const struct firmware *nv) > + const u8 *data, size_t data_len) > { > size_t size; > > memset(nvp, 0, sizeof(*nvp)); > - nvp->fwnv = nv; > + nvp->data = data; > /* 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); > @@ -356,18 +356,18 @@ 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, > - u16 domain_nr, u16 bus_nr) > +static void *brcmf_fw_nvram_strip(const u8 *data, size_t data_len, > + u32 *new_length, u16 domain_nr, u16 bus_nr) > { > struct nvram_parser nvp; > u32 pad; > 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; > @@ -426,7 +426,7 @@ 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->data, fw->size,&nvram_length, > fwctx->domain_nr, fwctx->bus_nr); > release_firmware(fw); > if (!nvram&& !(fwctx->flags& BRCMF_FW_REQ_NV_OPTIONAL)) -- 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 4 June 2015 at 17:02, Arend van Spriel <arend@broadcom.com> wrote: > On 05/28/15 13:37, Rafa? Mi?ecki wrote: >> >> As we plan to add support for platform NVRAM we should store direct >> data pointer without the extra struct firmware layer. This will allow >> us to support other sources with the only requirement being u8 buffer. >> >> Signed-off-by: Rafa? Mi?ecki<zajec5@gmail.com> >> Signed-off-by: Hante Meuleman<meuleman@broadcom.com> >> Signed-off-by: Arend van Spriel<arend@broadcom.com> > > > This look a bit contradictory. You mention below you written this from > 'scratch' so it seems odd that it is signed off by me and Hante. > > You may add: > > Acked-by: Arend van Spriel <arend@broadcom.com> Sorry, I wasn't sure how to handle it. Will fix it. >> --- >> Tested on router with BCM43602-s using >> /lib/firmware/brcm/brcmfmac43602-pcie.txt >> >> I've written this patch from scratch, it's inspired by the dropped: >> [PATCH 6/6] brcmfmac: Add support for host platform NVRAM loading. >> --- >> drivers/net/wireless/brcm80211/brcmfmac/firmware.c | 40 >> +++++++++++----------- >> 1 file changed, 20 insertions(+), 20 deletions(-) >> >> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/firmware.c >> b/drivers/net/wireless/brcm80211/brcmfmac/firmware.c >> index 7ae6461..b72df87 100644 >> --- a/drivers/net/wireless/brcm80211/brcmfmac/firmware.c >> +++ b/drivers/net/wireless/brcm80211/brcmfmac/firmware.c >> @@ -43,7 +43,7 @@ enum nvram_parser_state { >> * struct nvram_parser - internal info for parser. >> * >> * @state: current parser state. >> - * @fwnv: input buffer being parsed. >> + * @data: input buffer being parsed. > > > So you are not adding the data_len to the structure, which means you found > it is not needed? Just checking as it is different from the aforementioned > patch. That's right.
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/firmware.c b/drivers/net/wireless/brcm80211/brcmfmac/firmware.c index 7ae6461..b72df87 100644 --- a/drivers/net/wireless/brcm80211/brcmfmac/firmware.c +++ b/drivers/net/wireless/brcm80211/brcmfmac/firmware.c @@ -43,7 +43,7 @@ enum nvram_parser_state { * struct nvram_parser - internal info for parser. * * @state: current parser state. - * @fwnv: input buffer being parsed. + * @data: input buffer being parsed. * @nvram: output buffer with parse result. * @nvram_len: lenght of parse result. * @line: current line. @@ -55,7 +55,7 @@ enum nvram_parser_state { */ struct nvram_parser { enum nvram_parser_state state; - const struct firmware *fwnv; + const u8 *data; u8 *nvram; u32 nvram_len; u32 line; @@ -91,7 +91,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)) @@ -115,16 +115,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) || c == ' ') { brcmf_dbg(INFO, "warning: ln=%d:col=%d: '=' expected, skip invalid key entry\n", @@ -145,11 +145,11 @@ brcmf_nvram_handle_value(struct nvram_parser *nvp) char *ekv; u32 cplen; - c = nvp->fwnv->data[nvp->pos]; + c = nvp->data[nvp->pos]; if (!is_nvram_char(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; @@ -170,7 +170,7 @@ brcmf_nvram_handle_comment(struct nvram_parser *nvp) { char *eoc, *sol; - sol = (char *)&nvp->fwnv->data[nvp->pos]; + sol = (char *)&nvp->data[nvp->pos]; eoc = strchr(sol, '\n'); if (!eoc) { eoc = strchr(sol, '\0'); @@ -201,17 +201,17 @@ static enum nvram_parser_state }; static int brcmf_init_nvram_parser(struct nvram_parser *nvp, - const struct firmware *nv) + const u8 *data, size_t data_len) { size_t size; memset(nvp, 0, sizeof(*nvp)); - nvp->fwnv = nv; + nvp->data = data; /* 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); @@ -356,18 +356,18 @@ 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, - u16 domain_nr, u16 bus_nr) +static void *brcmf_fw_nvram_strip(const u8 *data, size_t data_len, + u32 *new_length, u16 domain_nr, u16 bus_nr) { struct nvram_parser nvp; u32 pad; 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; @@ -426,7 +426,7 @@ 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->data, fw->size, &nvram_length, fwctx->domain_nr, fwctx->bus_nr); release_firmware(fw); if (!nvram && !(fwctx->flags & BRCMF_FW_REQ_NV_OPTIONAL))