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 <h...@denx.de>
>> Cc: Lukasz Majewski <l.majew...@samsung.com>
>> Cc: Kyungmin Park <kyungmin.p...@samsung.com>
>> Cc: Marek Vasut <ma...@denx.de>
>> Cc: Pantelis Antoniou <pa...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
