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

[AKASHI Takahiro]

Heinrich,

On Fri, Jul 10, 2020 at 07:45:30AM +0200, Heinrich Schuchardt wrote:
> On 7/10/20 3:25 AM, AKASHI Takahiro 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
> 
> _ALT does not convey any meaning to me. Why don't you simply move the
> function to drivers/dfu/dfu.c?

Please read my succeeding patches carefully.
I added a variant function later.

> If it is not needed the linker will drop it.

CONFIG_DFU_ALT is NOT a user-selectable configuration.
So unless it is really necessary, it won't compile 'automatically'.
We don't have to rely on linkers here.

> dfu_write_buffer() might be a good name for the function.

See my comment above.
There is a good reason for the name.

-Takahiro Akashi

> Best regards
> 
> Heinrich
> 
> > 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%)
> >
> > diff --git a/drivers/dfu/Kconfig b/drivers/dfu/Kconfig
> > index cafb6a34090e..1bddaef45532 100644
> > --- a/drivers/dfu/Kconfig
> > +++ b/drivers/dfu/Kconfig
> > @@ -15,8 +15,13 @@ config DFU_OVER_TFTP
> >  	select UPDATE_TFTP
> >
> >  if DFU
> > +config DFU_ALT
> > +	bool
> > +	default n
> > +
> >  config DFU_TFTP
> >  	bool "DFU via TFTP"
> > +	select DFU_ALT
> >  	select DFU_OVER_TFTP
> >  	help
> >  	  This option allows performing update of DFU-managed medium with data
> > diff --git a/drivers/dfu/Makefile b/drivers/dfu/Makefile
> > index 0d7925c083ef..cc7de1d3ed9b 100644
> > --- a/drivers/dfu/Makefile
> > +++ b/drivers/dfu/Makefile
> > @@ -9,5 +9,5 @@ obj-$(CONFIG_$(SPL_)DFU_MTD) += dfu_mtd.o
> >  obj-$(CONFIG_$(SPL_)DFU_NAND) += dfu_nand.o
> >  obj-$(CONFIG_$(SPL_)DFU_RAM) += dfu_ram.o
> >  obj-$(CONFIG_$(SPL_)DFU_SF) += dfu_sf.o
> > -obj-$(CONFIG_$(SPL_)DFU_TFTP) += dfu_tftp.o
> > +obj-$(CONFIG_$(SPL_)DFU_ALT) += dfu_alt.o
> >  obj-$(CONFIG_$(SPL_)DFU_VIRT) += dfu_virt.o
> > diff --git a/drivers/dfu/dfu_tftp.c b/drivers/dfu/dfu_alt.c
> > similarity index 67%
> > rename from drivers/dfu/dfu_tftp.c
> > rename to drivers/dfu/dfu_alt.c
> > 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)
> >  {
> >  	char *s, *sb;
> >  	int alt_setting_num, ret;
> > diff --git a/include/dfu.h b/include/dfu.h
> > index 6fa450593605..94b0a9e68317 100644
> > --- a/include/dfu.h
> > +++ b/include/dfu.h
> > @@ -494,27 +494,27 @@ static inline int dfu_fill_entity_virt(struct dfu_entity *dfu, char *devstr,
> >  #endif
> >
> >  /**
> > - * dfu_tftp_write() - write TFTP data to DFU medium
> > + * 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 via TFTP on DFU supported medium.
> > + * This function is storing data received on DFU supported medium which
> > + * is specified by @dfu_entity_name.
> >   *
> > - * @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")
> > - *
> > - * Return:		0 on success, otherwise error code
> > + * Return:		0 - on success, error code - otherwise
> >   */
> > -#if CONFIG_IS_ENABLED(DFU_TFTP)
> > -int dfu_tftp_write(char *dfu_entity_name, unsigned int addr, unsigned int len,
> > -		   char *interface, char *devstring);
> > +#if CONFIG_IS_ENABLED(DFU_ALT)
> > +int dfu_write_by_name(char *dfu_entity_name, unsigned int addr,
> > +		      unsigned int len, char *interface, char *devstring);
> >  #else
> > -static inline int dfu_tftp_write(char *dfu_entity_name, unsigned int addr,
> > -				 unsigned int len, char *interface,
> > -				 char *devstring)
> > +static inline int dfu_write_by_name(char *dfu_entity_name, unsigned int addr,
> > +				    unsigned int len, char *interface,
> > +				    char *devstring)
> >  {
> > -	puts("TFTP write support for DFU not available!\n");
> > +	puts("write support for DFU not available!\n");
> >  	return -ENOSYS;
> >  }
> >  #endif
> >
>