Search Linux Wireless

Re: [PATCH v2 1/7] brcmfmac: Add support for host platform NVRAM loading.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 20 August 2015 at 18:06, Arend van Spriel <arend@xxxxxxxxxxxx> wrote:
> On 08/20/2015 05:53 PM, Rafał Miłecki wrote:
>>
>> On 19 August 2015 at 23:43, Arend van Spriel <arend@xxxxxxxxxxxx> wrote:
>>>
>>> On 08/19/2015 11:21 PM, Arend van Spriel wrote:
>>>>
>>>>
>>>> subject changed to v2. So let's go over your beef.
>>>>
>>>> On 07/11/2015 07:09 PM, Rafał Miłecki wrote:
>>>>>
>>>>>
>>>>> On 10 July 2015 at 20:31, Arend van Spriel <arend@xxxxxxxxxxxx> wrote:
>>>>>>
>>>>>>
>>>>>> @@ -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")
>>>>
>>>>
>>>>
>>>> done
>>>>
>>>>>> @@ -426,19 +428,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;
>>>>>
>>>>>
>>>>>
>>>>> This can be const.
>>>>
>>>>
>>>>
>>>> done
>>>
>>>
>>>
>>> Actually 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);

Oh, I feel silly. Yeah, you're right. In OpenWrt I was using two
separated variables and it was what basically let it work. Just ignore
that noise from me :|

-- 
Rafał
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Wireless Personal Area Network]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Hiking]     [MIPS Linux]     [ARM Linux]     [Linux RAID]

  Powered by Linux