On 19 August 2015 at 23:21, Arend van Spriel <arend@xxxxxxxxxxxx> wrote:
> subject changed to v2. So let's go over your beef.
I really hope none of my comment was mean or anything :)
> On 07/11/2015 07:09 PM, Rafał Miłecki wrote:
>>
>> On 10 July 2015 at 20:31, Arend van Spriel <arend@xxxxxxxxxxxx> wrote:
>>> + data_len = fw->size;
>>> + raw_nvram = false;
>>> + } else {
>>> + data = bcm47xx_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->data, fw->size,
>>> &nvram_length,
>>> + if (data) {
>>> + nvram = brcmf_fw_nvram_strip(data, 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)
>>> + bcm47xx_nvram_release_contents(data);
>>
>>
>> This is cosmetical but maybe you could move above 2 lines next to the
>> release_firmware? So we have all freeing code at one please? Do you
>> think it would improve readability?
>> Nothing important thought. Feel free to ignore me here.
>
>
> confused! The release_firmware call is removed here, right?
Yes, you removed it from the "if (data) {" condition body but also
re-added right after it. AFAIR I got an impression it may make more
sense to have something like:
if (raw_nvram)
bcm47xx_nvram_release_contents(data);
if (fw)
release_firmware(fw);
but you can just ignore it if it doesn't sound clear.
--
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
