From mboxrd@z Thu Jan  1 00:00:00 1970
From: Lukasz Majewski <lukma@denx.de>
Date: Mon, 22 Jun 2020 09:19:04 +0200
Subject: [PATCH v2 03/17] dfu: rename dfu_tftp_write() to
 dfu_write_by_name()
In-Reply-To: <20200622004149.GA6534@laputa>
References: <20200617025515.23585-1-takahiro.akashi@linaro.org>
 <20200617025515.23585-4-takahiro.akashi@linaro.org>
 <CADg8p96fwH3FSzOkQCa71V6y8Dc84r+XKMOMdQ2V4B+=aSu85Q@mail.gmail.com>
 <20200621093832.2228ba41@jawa> <20200622004149.GA6534@laputa>
Message-ID: <20200622091904.32a7b7cd@jawa>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi AKASHI,

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

Ok. Thanks for the explanation.

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




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/20200622/54350bff/attachment.sig>