Re: [PATCH v2 1/7] brcmfmac: Add support for host platform NVRAM loading.
From: Arend van Spriel <hidden>
Date: 2015-08-20 16:07:04
On 08/20/2015 05:53 PM, Rafał Miłecki wrote:
On 19 August 2015 at 23:43, Arend van Spriel [off-list ref] wrote:quoted
On 08/19/2015 11:21 PM, Arend van Spriel wrote:quoted
subject changed to v2. So let's go over your beef. On 07/11/2015 07:09 PM, Rafał Miłecki wrote:quoted
On 10 July 2015 at 20:31, Arend van Spriel [off-list ref] wrote:quoted
@@ -146,7 +147,7 @@ brcmf_nvram_handle_value(struct nvram_parser *nvp) u32 cplen; c = nvp->data[nvp->pos]; - if (!is_nvram_char(c)) { + if (!is_nvram_char(c) && (c != ' ')) {This is redundant, please drop this change. See fc23e81eb8f4 ("brcmfmac: allow NVRAM values to contain spaces")donequoted
quoted
@@ -426,19 +428,34 @@ static void brcmf_fw_request_nvram_done(conststruct firmware *fw, void *ctx) struct brcmf_fw *fwctx = ctx; u32 nvram_length = 0; void *nvram = NULL; + u8 *data = NULL;This can be const.doneActually this is not done, but either way will require a cast because bcm47xx_nvram_release_contents expects char* so there is nothing gained. Unless someone will change bcm47xx_nvram_get/release_contents api to const char*.Passing non-const pointer to function taking const one is OK. You don't need casting, compiler won't complain about this.
bcm47xx_nvram_release_contents expect a non-const pointer so the const
data pointer needs to be cast to non-const. Which you claim is hacky.
Here is what happens when I make data pointer const:
CC [M] drivers/net/wireless/brcm80211/brcmfmac/firmware.o
drivers/net/wireless/brcm80211/brcmfmac/firmware.c: In function
���brcmf_fw_request_nvram_done���:
drivers/net/wireless/brcm80211/brcmfmac/firmware.c:450:4: warning:
passing argument 1 of ���bcm47xx_nvram_release_contents��� discards
���const��� qualifier from pointer target type [enabled by default]
bcm47xx_nvram_release_contents(data);
^
In file included from
drivers/net/wireless/brcm80211/brcmfmac/firmware.c:22:0:
include/linux/bcm47xx_nvram.h:44:20: note: expected ���char *��� but
argument is of type ���const u8 *���
static inline void bcm47xx_nvram_release_contents(char *nvram)
^On the other hand casing const pointer to the non-const one is hacky and I believe you should avoid that.
Either way you have to do a cast from const to non-const. u8 *data => data = (u8 *)fw->data; const u8 *data => bcm47xx_nvram_release_contents((char *)data); Regards, Arend