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

[Lukasz Majewski]

Hi Heiko,

> Hello Lukasz,
> 
> Am 10.04.2014 16:31, schrieb Lukasz Majewski:
> > Hi Heiko,
> >
> >> 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 ... ?
> >
> > I wasn't added to CC in the original patch 2/2.
> >
> > I was only added to Cc below the Signed-of-by, but then I was
> > missing in the CC of the message itself.
> 
> Yes, I see ... Hmm.. I sent patches always with git send-mail ...
> 
> Header I got:
> 
>  From - Thu Apr 10 07:12:33 2014
> X-Account-Key: account2
> X-UIDL: 1130185039.262989
> X-Mozilla-Status: 0001
> X-Mozilla-Status2: 00000000
> X-Mozilla-Keys:
> Return-Path: <hs at denx.de>
> Received: from murder ([192.168.8.180])
> 	 by backend11 (Cyrus v2.2.12) with LMTPA;
> 	 Thu, 10 Apr 2014 07:08:28 +0200
> X-Sieve: CMU Sieve 2.2
> Received: from mail.m-online.net (localhost [127.0.0.1])
> 	 by frontend1.mail.m-online.net (Cyrus v2.2.12) with LMTPA;
> 	 Thu, 10 Apr 2014 07:08:27 +0200
> [...]
> Received: from pollux.denx.de (pollux [192.168.1.1])
> 	by mail.denx.de (Postfix) with ESMTP id D0851342155;
> 	Thu, 10 Apr 2014 07:08:08 +0200 (CEST)
> Received: by pollux.denx.de (Postfix, from userid 515)
> 	id 98C9BF6E; Thu, 10 Apr 2014 07:08:08 +0200 (CEST)
> From: Heiko Schocher <hs at denx.de>
> To: u-boot at lists.denx.de
> Cc: Heiko Schocher <hs at denx.de>,
> 	Lukasz Majewski <l.majewski at samsung.com>,
> 	Kyungmin Park <kyungmin.park at samsung.com>,
> 	Marek Vasut <marex at denx.de>,
> 	Pantelis Antoniou <panto at antoniou-consulting.com>
> Subject: [PATCH 2/2] dfu, nand: add medium specific polltimeout
> function Date: Thu, 10 Apr 2014 07:08:06 +0200
> Message-Id: <1397106486-1233-2-git-send-email-hs at denx.de>
> X-Mailer: git-send-email 1.8.3.1
> In-Reply-To: <1397106486-1233-1-git-send-email-hs at denx.de>
> References: <1397106486-1233-1-git-send-email-hs at denx.de>
> 
> There you are in the cc list ... but I see, in patchwork:
> 
> http://patchwork.ozlabs.org/patch/337981/
> 
> (click on "Headers show")
> [...]
> Message-Id: <1397106486-1233-2-git-send-email-hs at denx.de>
> X-Mailer: git-send-email 1.8.3.1
> In-Reply-To: <1397106486-1233-1-git-send-email-hs at denx.de>
> References: <1397106486-1233-1-git-send-email-hs at denx.de>
> Cc: Marek Vasut <marex at denx.de>,
> 	Pantelis Antoniou <panto at antoniou-consulting.com>,
> 	Kyungmin Park <kyungmin.park at samsung.com>
> Subject: [U-Boot] [PATCH 2/2] dfu,
> 	nand: add medium specific polltimeout function
> 
> There you are missing ... ?

Yes. Here is the problem. When you reply to patch, then Cc from the
commit message is not taken into account.

This is why I've asked for being consistent with CC list.

For this reason (also the cover letter apply to this), when I send
patches with git-send-email, I add all concerned people with explicit
--cc/--to option.

> 
> bye,
> Heiko



-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group