| Message ID | 20181009124755.25402-1-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | [1/6] brcmfmac: Remove firmware-loading code duplication | expand |
On 10/9/2018 2:47 PM, Hans de Goede wrote: > brcmf_fw_request_next_item and brcmf_fw_request_done both have identical > code to complete the fw-request depending on the item-type. > > This commit adds a new brcmf_fw_complete_request helper removing this code > duplication. Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > .../broadcom/brcm80211/brcmfmac/firmware.c | 62 +++++++++---------- > 1 file changed, 31 insertions(+), 31 deletions(-)
Arend van Spriel <arend.vanspriel@broadcom.com> writes: > On 10/9/2018 2:47 PM, Hans de Goede wrote: >> brcmf_fw_request_next_item and brcmf_fw_request_done both have identical >> code to complete the fw-request depending on the item-type. >> >> This commit adds a new brcmf_fw_complete_request helper removing this code >> duplication. > > Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com> For some reason you commented on v1 but there was v2 posted already: https://patchwork.kernel.org/patch/10634355/ I guess I can take v2 still?
Hi, On 05-11-18 16:05, Kalle Valo wrote: > Arend van Spriel <arend.vanspriel@broadcom.com> writes: > >> On 10/9/2018 2:47 PM, Hans de Goede wrote: >>> brcmf_fw_request_next_item and brcmf_fw_request_done both have identical >>> code to complete the fw-request depending on the item-type. >>> >>> This commit adds a new brcmf_fw_complete_request helper removing this code >>> duplication. >> >> Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com> > > For some reason you commented on v1 but there was v2 posted already: > > https://patchwork.kernel.org/patch/10634355/ > > I guess I can take v2 still? Yes the only thing different in v2 was dropping the SPDX license header for the new drivers/net/wireless/broadcom/brcm80211/brcmfmac/dmi.c file and replacing it with the full ISC license text as seen in other brcmfmac files. Nothing else was changed, so the review of v1 is valid for v2 too. Arend had one very minor comment on the name of a variable in the fifth patch, but that is not important so if you want to pick up v2 as is go for it. Note the ISC license text is now in Torvald's tree as: LICENSES/other/ISC So could even go with v1, but I guess you prefer to move all files of a driver over to the SPDX headers in one go ... Regards, Hans
Hans de Goede <hdegoede@redhat.com> writes: > Hi, > > On 05-11-18 16:05, Kalle Valo wrote: >> Arend van Spriel <arend.vanspriel@broadcom.com> writes: >> >>> On 10/9/2018 2:47 PM, Hans de Goede wrote: >>>> brcmf_fw_request_next_item and brcmf_fw_request_done both have identical >>>> code to complete the fw-request depending on the item-type. >>>> >>>> This commit adds a new brcmf_fw_complete_request helper removing this code >>>> duplication. >>> >>> Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com> >> >> For some reason you commented on v1 but there was v2 posted already: >> >> https://patchwork.kernel.org/patch/10634355/ >> >> I guess I can take v2 still? > > Yes the only thing different in v2 was dropping the SPDX license header > for the new drivers/net/wireless/broadcom/brcm80211/brcmfmac/dmi.c > file and replacing it with the full ISC license text as seen in other > brcmfmac files. Nothing else was changed, so the review of v1 is > valid for v2 too. > > Arend had one very minor comment on the name of a variable in the fifth > patch, but that is not important so if you want to pick up v2 as is > go for it. > > Note the ISC license text is now in Torvald's tree as: > LICENSES/other/ISC > > So could even go with v1, but I guess you prefer to move all files of > a driver over to the SPDX headers in one go ... Correct, I really would prefer move to SPDX headers in one go.
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c index 9095b830ae4d..784c84f0e9e7 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c @@ -504,6 +504,34 @@ static int brcmf_fw_request_nvram_done(const struct firmware *fw, void *ctx) return -ENOENT; } +static int brcmf_fw_complete_request(const struct firmware *fw, + struct brcmf_fw *fwctx) +{ + struct brcmf_fw_item *cur = &fwctx->req->items[fwctx->curpos]; + int ret = 0; + + brcmf_dbg(TRACE, "firmware %s %sfound\n", cur->path, fw ? "" : "not "); + + switch (cur->type) { + case BRCMF_FW_TYPE_NVRAM: + ret = brcmf_fw_request_nvram_done(fw, fwctx); + break; + case BRCMF_FW_TYPE_BINARY: + if (fw) + cur->binary = fw; + else + ret = -ENOENT; + break; + default: + /* something fishy here so bail out early */ + brcmf_err("unknown fw type: %d\n", cur->type); + release_firmware(fw); + ret = -EINVAL; + } + + return (cur->flags & BRCMF_FW_REQF_OPTIONAL) ? 0 : ret; +} + static int brcmf_fw_request_next_item(struct brcmf_fw *fwctx, bool async) { struct brcmf_fw_item *cur; @@ -525,15 +553,7 @@ static int brcmf_fw_request_next_item(struct brcmf_fw *fwctx, bool async) if (ret < 0) { brcmf_fw_request_done(NULL, fwctx); } else if (!async && fw) { - brcmf_dbg(TRACE, "firmware %s %sfound\n", cur->path, - fw ? "" : "not "); - if (cur->type == BRCMF_FW_TYPE_BINARY) - cur->binary = fw; - else if (cur->type == BRCMF_FW_TYPE_NVRAM) - brcmf_fw_request_nvram_done(fw, fwctx); - else - release_firmware(fw); - + brcmf_fw_complete_request(fw, fwctx); return -EAGAIN; } return 0; @@ -547,28 +567,8 @@ static void brcmf_fw_request_done(const struct firmware *fw, void *ctx) cur = &fwctx->req->items[fwctx->curpos]; - brcmf_dbg(TRACE, "enter: firmware %s %sfound\n", cur->path, - fw ? "" : "not "); - - if (!fw) - ret = -ENOENT; - - switch (cur->type) { - case BRCMF_FW_TYPE_NVRAM: - ret = brcmf_fw_request_nvram_done(fw, fwctx); - break; - case BRCMF_FW_TYPE_BINARY: - cur->binary = fw; - break; - default: - /* something fishy here so bail out early */ - brcmf_err("unknown fw type: %d\n", cur->type); - release_firmware(fw); - ret = -EINVAL; - goto fail; - } - - if (ret < 0 && !(cur->flags & BRCMF_FW_REQF_OPTIONAL)) + ret = brcmf_fw_complete_request(fw, fwctx); + if (ret < 0) goto fail; do {
brcmf_fw_request_next_item and brcmf_fw_request_done both have identical code to complete the fw-request depending on the item-type. This commit adds a new brcmf_fw_complete_request helper removing this code duplication. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- .../broadcom/brcm80211/brcmfmac/firmware.c | 62 +++++++++---------- 1 file changed, 31 insertions(+), 31 deletions(-)