On 19 August 2015 at 23:21, Arend van Spriel <[email protected]> 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 <[email protected]> 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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
