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

[Heiko Schocher]

Hello Lukasz,

Am 10.04.2014 12:08, schrieb Lukasz Majewski:
> Hi Pantelis,
>
>> 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.
>
> Seems reasonable for me: +1

Yep, good idea, I change this.

> Some comment:
>
> Guys, please be consistent with CCing people. I didn't receive this
> thread. Also this original reply from Pantelis was not CCed to Heiko.

Hmm.. I lloked in my received EMails, and I see you always on cc ... ?

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany