Message ID | 20200710012537.6264-4-takahiro.akashi@linaro.org |
---|---|
State | Superseded, archived |
Delegated to: | Heinrich Schuchardt |
Headers | show |
Series | efi_loader: add capsule update support | expand |
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? If it is not needed the linker will drop it. dfu_write_buffer() might be a good name for the function. 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@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 >
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@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 > > >
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
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%)