[PATCH v6 01/17] dfu: rename dfu_tftp_write() to dfu_write_by_name()

[Lukasz Majewski]

On Thu, 17 Sep 2020 21:32:25 -0400
Tom Rini <trini at konsulko.com> wrote:

> On Fri, Sep 18, 2020 at 10:28:57AM +0900, AKASHI Takahiro wrote:
> > Tom, Lukasz,
> > 
> > On Thu, Sep 10, 2020 at 10:41:39PM -0400, Tom Rini wrote:  
> > > On Fri, Sep 11, 2020 at 09:46:14AM +0900, AKASHI Takahiro wrote:  
> > > > On Wed, Sep 09, 2020 at 10:26:34PM -0400, Tom Rini wrote:  
> > > > > On Thu, Sep 10, 2020 at 10:54:18AM +0900, AKASHI Takahiro
> > > > > wrote:  
> > > > > > On Wed, Sep 09, 2020 at 02:30:53PM +0200, Heinrich
> > > > > > Schuchardt wrote:  
> > > > > > > On 07.09.20 07:34, AKASHI Takahiro wrote:  
> > > > > > > > This function is essentially independent from tftp, and
> > > > > > > > will also be utilised in implementing UEFI capsule
> > > > > > > > update in a later commit. So just give it a more
> > > > > > > > generic name. In addition, a new configuration option,
> > > > > > > > CONFIG_DFU_ALT, was introduced so that the file will be
> > > > > > > > compiled with different options, particularly one added
> > > > > > > > in a later commit.
> > > > > > > >
> > > > > > > > Signed-off-by: AKASHI Takahiro
> > > > > > > > <takahiro.akashi at linaro.org> ---
> > > > > > > >  common/update.c                       |  5 +++--
> > > > > > > >  drivers/dfu/Kconfig                   |  5 +++++
> > > > > > > >  drivers/dfu/Makefile                  |  2 +-
> > > > > > > >  drivers/dfu/{dfu_tftp.c => dfu_alt.c} | 17
> > > > > > > > ++++++++++++-- include/dfu.h                         |
> > > > > > > > 32 +++++++++++++-------------- 5 files changed, 40
> > > > > > > > insertions(+), 21 deletions(-) rename
> > > > > > > > drivers/dfu/{dfu_tftp.c => dfu_alt.c} (67%)
> > > > > > > >
> > > > > > > > diff --git a/common/update.c b/common/update.c
> > > > > > > > index 36b6b7523d50..39946776d74f 100644
> > > > > > > > --- a/common/update.c
> > > > > > > > +++ b/common/update.c
> > > > > > > > @@ -324,8 +324,9 @@ got_update_file:
> > > > > > > >  			}
> > > > > > > >  		} else if (fit_image_check_type(fit,
> > > > > > > > noffset, IH_TYPE_FIRMWARE)) {
> > > > > > > > -			ret =
> > > > > > > > dfu_tftp_write(fit_image_name, update_addr,
> > > > > > > > -
> > > > > > > > update_size, interface, devstring);
> > > > > > > > +			ret =
> > > > > > > > dfu_write_by_name(fit_image_name, update_addr,
> > > > > > > > +
> > > > > > > > update_size, interface,
> > > > > > > > +
> > > > > > > > devstring); if (ret)
> > > > > > > >  				return ret;
> > > > > > > >  		}
> > > > > > > > diff --git a/drivers/dfu/Kconfig b/drivers/dfu/Kconfig
> > > > > > > > index 0eec00ba734d..78f901ff348a 100644
> > > > > > > > --- a/drivers/dfu/Kconfig
> > > > > > > > +++ b/drivers/dfu/Kconfig
> > > > > > > > @@ -14,8 +14,13 @@ config DFU_OVER_TFTP
> > > > > > > >  	depends on NET
> > > > > > > >
> > > > > > > >  if DFU
> > > > > > > > +config DFU_ALT
> > > > > > > > +	bool  
> > > > > > > 
> > > > > > > You cannot define CONFIG_DFU_ALT as bool. This symbol
> > > > > > > already exists as string in include/configs/odroid.h,
> > > > > > > include/configs/s5p_goni.h, et.al. It is used to
> > > > > > > initialize environment variable dfu_alt_info. Your patch
> > > > > > > leads to an error on Travis:
> > > > > > > 
> > > > > > >        arm:  +   odroid  
> > > > > > 
> > > > > > Okay, will fix it.
> > > > > > DFU_WRITE_ALT? Or, as Heinrich suggested, move them
> > > > > > to dfu.c if Lukasz agrees.
> > > > > > 
> > > > > > # I haven't seen any comments from him yet though.  
> > > > > 
> > > > > I think what you're introducing as "CONFIG_DFU_ALT" and
> > > > > renaming "drivers/dfu/dfu_tftp.c" to "drivers/dfu/dfu_alt.c"
> > > > > is what's confusing. That I think needs a different name than
> > > > > "alt" to convey that it's a generic "from DRAM to flash/etc"
> > > > > hook.  
> > > > 
> > > > Whatever the name is, I need reviews on all of my DFU patches
> > > > from Lukasz in the first place.  
> > > 
> > > The lower impact on the rest of the DFU subsytem and other
> > > automated testing that can be done, the easier it can be for
> > > someone else to review the DFU parts if Lukasz isn't able to find
> > > time to do it.  
> > 
> > It sounds nice, but who will take that role?  
> 
> Me.
> 

I thought that there is some ongoing discussion between Heinrich and
Takahiro regarding those patches ...

And yes, I'm busy with other work and my time budget is
unfortunately reduced since March 2020.

It would be good to have some help to not block ongoing DFU
development.


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200918/abdf0e8c/attachment.sig>