[PATCH v2 03/17] dfu: rename dfu_tftp_write() to dfu_write_by_name()

[AKASHI Takahiro]

Lukasz,

On Sun, Jun 21, 2020 at 09:38:32AM +0200, Lukasz Majewski wrote:
> Hi Sughosh,
> 
> > On Wed, 17 Jun 2020 at 08:25, AKASHI Takahiro
> > <takahiro.akashi at linaro.org> wrote:
> > 
> > > This function is essentially independent from tffp, 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>
> > > ---
> > >  drivers/dfu/Kconfig                   |  5 +++++
> > >  drivers/dfu/Makefile                  |  2 +-
> > >  drivers/dfu/{dfu_tftp.c => dfu_alt.c} | 17 ++++++++++++--
> > >  include/dfu.h                         | 32
> > > +++++++++++++-------------- 4 files changed, 37 insertions(+), 19
> > > deletions(-) rename drivers/dfu/{dfu_tftp.c => dfu_alt.c} (67%)
> > >  
> > 
> > You are renaming dfu_tftp.c to dfu_alt.c. Can the two api's not be
> > added to dfu.c directly. Is there a need for a separate dfu_alt.c. It
> > finally depends on what Lukasz thinks.
> 
> If this rename is going to provide a common code to be used in both
> use cases - then I'm fine with it.
> 
> UEFI seems to be different from tftp.

UEFI subsystem doesn't utilize tftp.

> And dfu_tftp.c name indicates
> that it shall be used with tftp protocol.

Well, the name does, but in fact, as my commit message indicated,
it has no dependency on tftp and UEFI can and will utilize this function.

> However, I don't know what rationale is behind dfu_alt.c name?

See my patch in my series:
https://lists.denx.de/pipermail/u-boot/2020-June/416550.html

We will need one more variant, especially, almost the same logic
without FIT handling.

Thanks,
-Takahiro Akashi

> 
> > 
> > <snip>
> > 
> > 
> > > index ffae4bb54f80..5b1b13d7170d 100644
> > > --- a/drivers/dfu/dfu_tftp.c
> > > +++ b/drivers/dfu/dfu_alt.c
> > > @@ -10,8 +10,21 @@
> > >  #include <errno.h>
> > >  #include <dfu.h>
> > >
> > > -int dfu_tftp_write(char *dfu_entity_name, unsigned int addr,
> > > unsigned int len,
> > > -                  char *interface, char *devstring)
> > > +/**
> > > + * dfu_write_by_name() - write data to DFU medium
> > > + * @dfu_entity_name:    Name of DFU entity to write
> > > + * @addr:               Address of data buffer to write
> > > + * @len:                Number of bytes
> > > + * @interface:          Destination DFU medium (e.g. "mmc")
> > > + * @devstring:          Instance number of destination DFU medium
> > > (e.g. "1")
> > > + *
> > > + * This function is storing data received on DFU supported medium
> > > which
> > > + * is specified by @dfu_entity_name.
> > > + *
> > > + * Return:              0 - on success, error code - otherwise
> > > + */
> > > +int dfu_write_by_name(char *dfu_entity_name, unsigned int addr,
> > > +                     unsigned int len, char *interface, char
> > > *devstring) {
> > >  
> > 
> > I know that this has been taken from the original dfu_tftp.c, but the
> > addr parameter needs to be a void pointer. The unsigned int does not
> > work when addr is greater than 4GB. This needs to be changed for both
> > the api's that are being added.
> > 
> > -sughosh
> 
> 
> 
> 
> 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