[U-Boot] [PATCH 2/2] dfu, nand: add medium specific polltimeout function

[Pantelis Antoniou]

Hi Marek,

On Apr 10, 2014, at 10:54 AM, Marek Vasut wrote:

> On Thursday, April 10, 2014 at 07:08:06 AM, Heiko Schocher wrote:
>> add a possibility to add a medium specific polltimeout
>> function. So it is possible to define different
>> poll timeouts.
>> 
>> Used on nand medium, for setting the DFU_MANIFEST_POLL_TIMEOUT
>> only on nand ubi partitions, which is currently the only
>> usecase.
>> 
>> Signed-off-by: Heiko Schocher <hs at denx.de>
>> Cc: Lukasz Majewski <l.majewski at samsung.com>
>> Cc: Kyungmin Park <kyungmin.park at samsung.com>
>> Cc: Marek Vasut <marex at denx.de>
>> Cc: Pantelis Antoniou <panto at antoniou-consulting.com>
> 
> [...]
> 
>> @@ -174,6 +174,17 @@ static void dnload_request_flush(struct usb_ep *ep,
>> struct usb_request *req) req->length, f_dfu->blk_seq_num);
>> }
>> 
>> +static void dfu_set_poll_timeout_manifest(struct dfu_status *dstat,
>> +					  struct f_dfu *f_dfu)
>> +{
>> +	struct dfu_entity *dfu = dfu_get_entity(f_dfu->altsetting);
>> +
>> +	if (dfu->poll_timeout)
>> +		dfu_set_poll_timeout(dstat, dfu->poll_timeout(dfu));
>> +	else
>> +		dfu_set_poll_timeout(dstat, DFU_MANIFEST_POLL_TIMEOUT);
>> +}
> 
> Don't you think it'd be better (yet more intrusive) to have all the DFU users 
> have default implementation of dfu->poll_timeout() ? Then you'd be able to avoid 
> this if and even get rid of this dfu_set_poll_timeout_manifest() function.
> 

Could work, but why not a simple accessor like this:

static inline unsigned int dfu_get_poll_timeout(struct dfu_entity *dfu)
{

	return dfu->poll_timeout ? dfu->poll_timeout(dfu); 
		DFU_MANIFEST_POLL_TIMEOUT);
}

and  dfu_set_poll_timeout(dstat, dfu_get_poll_timeout(dfu));

You even get the benefit of have a method to read the timeout value if we ever
needed sometime in the future.

> 
> [...]
> 
> Best regards,
> Marek Vasut

Regards

-- Pantelis