From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heiko Schocher Date: Thu, 10 Apr 2014 12:21:24 +0200 Subject: [U-Boot] [PATCH 2/2] dfu, nand: add medium specific polltimeout function In-Reply-To: <20140410120844.768cd853@amdc2363> References: <1397106486-1233-1-git-send-email-hs@denx.de> <1397106486-1233-2-git-send-email-hs@denx.de> <201404100954.30588.marex@denx.de> <22E0B499-3CF1-4809-8F4A-AAF720924411@antoniou-consulting.com> <20140410120844.768cd853@amdc2363> Message-ID: <534670A4.5060907@denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 >>>> Cc: Lukasz Majewski >>>> Cc: Kyungmin Park >>>> Cc: Marek Vasut >>>> Cc: Pantelis Antoniou >>> >>> [...] >>> >>>> @@ -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