[U-Boot] [PATCH v4 2/2] common: Generic firmware loader for file system

[Marek Vasut]

On 12/19/2017 11:31 AM, Chee, Tien Fong wrote:
> On Isn, 2017-12-18 at 08:39 +0100, Lothar Waßmann wrote:
>> Hi,
>>
>> On Mon, 18 Dec 2017 13:10:56 +0800 tien.fong.chee at intel.com wrote:
>>>
>>> From: Tien Fong Chee <tien.fong.chee at intel.com>
>>>
>>> This is file system generic loader which can be used to load
>>> the file image from the storage into target such as memory.
>>> The consumer driver would then use this loader to program whatever,
>>> ie. the FPGA device.
>>>
>>> Signed-off-by: Tien Fong Chee <tien.fong.chee at intel.com>
>>> ---
>>>  common/Makefile            |   1 +
>>>  common/fs_loader.c         | 311
>>> +++++++++++++++++++++++++++++++++++++++++++++
>>>  doc/README.firmware_loader |  77 +++++++++++
>>>  include/fs_loader.h        |  28 ++++
>>>  4 files changed, 417 insertions(+)
>>>  create mode 100644 common/fs_loader.c
>>>  create mode 100644 doc/README.firmware_loader
>>>  create mode 100644 include/fs_loader.h
>>>
>> [...]
>>>
>>> diff --git a/common/fs_loader.c b/common/fs_loader.c
>>> new file mode 100644
>>> index 0000000..81cf5d6
>>> --- /dev/null
>>> +++ b/common/fs_loader.c
>> [...]
>>>
>>> +/*
>>> + * Prepare firmware struct;
>>> + * return -ve if fail.
>>> + */
>>> +static int _request_firmware_prepare(struct firmware **firmware_p,
>>> +				     const char *name, void *dbuf,
>>> +				     size_t size, u32 offset)
>>> +{
>>> +	struct firmware *firmware = NULL;
>>> +	struct firmware_priv *fw_priv = NULL;
>>> +
>>> +	*firmware_p = NULL;
>>> +
>>> +	if (!name || name[0] == '\0')
>>> +		return -EINVAL;
>>> +
>>> +	*firmware_p = firmware = calloc(1, sizeof(*firmware));
>>> +
>>> +	if (!firmware) {
>>> +		printf("%s: calloc(struct firmware) failed\n",
>>> __func__);
>>> +		return -ENOMEM;
>>> +	}
>>> +
>>> +	fw_priv = calloc(1, sizeof(*fw_priv));
>>> +
>>> +	if (!fw_priv) {
>>> +		printf("%s: calloc(struct fw_priv) failed\n",
>>> __func__);
>>> +		return -ENOMEM;
>> What about freeing 'firmware' and NULLing *firmware_p here?
> There is no "freeing" support in U-Boot. I can assign NULL
> to *firmware_p.

Try git grep free maybe ?

>> Or better, do the assignment of *firmware_p at the end.
> Are you means switch the location between *firmware_p and fw_priv in
> calloc?

[...

-- 
Best regards,
Marek Vasut