Re: [PATCHv2 3/3] fpga_mgr: Add streaming support through the new driver_data API

From: Alan Tull
Date: Tue May 23 2017 - 11:26:10 EST


On Tue, May 23, 2017 at 10:21 AM, Alan Tull <atull@xxxxxxxxxx> wrote:

>>>> +
>>>> + mgr->state = FPGA_MGR_STATE_FIRMWARE_REQ;
>>>> + while (length > 0) {
>>>
>>> This could be "do { ... } while ((params.fw_size >= FIRMWARE_CHUNK_SZ)
>>> && (length > 0));" since that's what it's really doing.
>>
>> Yes, this is better, thanks.
>>
>>>
>>>> + ret = driver_data_request_sync(image_name, &req_params,
>>>> dev);
>>>> + if (ret) {
>>>> + dev_err(dev, "Error reading firmware %d\n", ret);
>>>> + mgr->state = FPGA_MGR_STATE_FIRMWARE_REQ_ERR;
>
> Also need: kfree(buf);
>
>>>> + return ret;

Or this could be a break instead of a return.

>>>> + }
>>>> +
>>>> + length -= params.fw_size;
>>>> + params.offset += params.fw_size;
>>>> + if (params.fw_size < SZ_4K)
>>>> + break;
>>>> + }
>>>> +
>>>> + kfree(buf);
>>>
>>> Please skip a line before return.
>>>
>>>> + return ret;
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(fpga_mgr_firmware_stream);