[PATCH v4 02/16] dfu: modify an argument type for an address

Heinrich Schuchardt xypron.glpk at gmx.de
Wed Jul 22 17:50:36 CEST 2020


On 22.07.20 14:43, Heinrich Schuchardt wrote:
> On 22.07.20 08:05, AKASHI Takahiro wrote:
>> The range of an addressable pointer can go beyond 'integer'.
>> So change the argument type to a void pointer.
>>
>> Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
>> ---
>>  common/update.c       | 3 ++-
>>  drivers/dfu/dfu_alt.c | 4 ++--
>>  include/dfu.h         | 4 ++--
>>  3 files changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/common/update.c b/common/update.c
>> index 7f73c6372da0..f82d77cc0be9 100644
>> --- a/common/update.c
>> +++ b/common/update.c
>> @@ -181,7 +181,8 @@ got_update_file:
>>  		}
>>
>>  		if (fit_image_check_type(fit, noffset, IH_TYPE_FIRMWARE)) {
>> -			ret = dfu_write_by_name(fit_image_name, update_addr,
>> +			ret = dfu_write_by_name(fit_image_name,
>> +						(void *)update_addr,
>>  						update_size, interface,
>>  						devstring);
>>  			if (ret)
>> diff --git a/drivers/dfu/dfu_alt.c b/drivers/dfu/dfu_alt.c
>> index 5b1b13d7170d..f6b87c51ed30 100644
>> --- a/drivers/dfu/dfu_alt.c
>> +++ b/drivers/dfu/dfu_alt.c
>> @@ -23,14 +23,14 @@
>>   *
>>   * Return:              0 - on success, error code - otherwise
>>   */
>> -int dfu_write_by_name(char *dfu_entity_name, unsigned int addr,
>> +int dfu_write_by_name(char *dfu_entity_name, void *addr,
>>  		      unsigned int len, char *interface, char *devstring)
>>  {
>>  	char *s, *sb;
>>  	int alt_setting_num, ret;
>>  	struct dfu_entity *dfu;
>>
>> -	debug("%s: name: %s addr: 0x%x len: %d device: %s:%s\n", __func__,
>> +	debug("%s: name: %s addr: 0x%p len: %d device: %s:%s\n", __func__,
>>  	      dfu_entity_name, addr, len, interface, devstring);
>>
>>  	ret = dfu_init_env_entities(interface, devstring);
>> diff --git a/include/dfu.h b/include/dfu.h
>> index 94b0a9e68317..327fffc0dba6 100644
>> --- a/include/dfu.h
>> +++ b/include/dfu.h
>> @@ -507,10 +507,10 @@ static inline int dfu_fill_entity_virt(struct dfu_entity *dfu, char *devstr,
>>   * Return:		0 - on success, error code - otherwise
>>   */
>>  #if CONFIG_IS_ENABLED(DFU_ALT)
>> -int dfu_write_by_name(char *dfu_entity_name, unsigned int addr,
>> +int dfu_write_by_name(char *dfu_entity_name, void *addr,
>>  		      unsigned int len, char *interface, char *devstring);
>>  #else
>> -static inline int dfu_write_by_name(char *dfu_entity_name, unsigned int addr,
>> +static inline int dfu_write_by_name(char *dfu_entity_name, void *addr,
>
> update_tftp() takes the value of this address from environment variable
> loadaddr. So this is not a pointer. It is an address in the virtual
> address space of the sandbox. You will have to call map_sysmem() to make
> it a pointer.
>
> To be strict the correct type for addr is phys_addr_t. But as we use
> simple_strtoul() to convert the loadaddr string using ulong as type is
> also fine. I suggest to use ulong as in update_tftp.
>
> We need to add a call to map_sysmem() to convert to the address pointer
> needed by dfu_write_from_mem_addr().

My first analysis was wrong. The missing address conversions for the
sandbox are in common/update.c and driver/dfu/dfu_ram.c. I have created
patch

https://lists.denx.de/pipermail/u-boot/2020-July/421060.html
[PATCH 1/1] dfu: fix dfu tftp on sandbox

The only change needed for the current patch is to remove the now
superfluous conversion when calling dfu_write_from_mem_addr().

Otherwise:

Reviewed-by: Heinrich Schuchardt <xypron.glpk at gmx.de>


More information about the U-Boot mailing list