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

[Tom Rini <trini@konsulko.com>]

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@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.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200917/680405e1/attachment.sig>