On Mon, Mar 02, 2020 at 08:12:09AM +0100, Heinrich Schuchardt wrote: > On 3/2/20 1:11 AM, AKASHI Takahiro wrote: > > On Fri, Feb 28, 2020 at 07:26:25PM +0100, Heinrich Schuchardt wrote: > > > On 2/28/20 1:06 AM, AKASHI Takahiro wrote: > > > > Logically, the current update_tftp() should and does compile and work > > > > correctly even without satisfying the following condition: > > > > > > > > > #if defined(CONFIG_UPDATE_TFTP) && !defined(CONFIG_MTD_NOR_FLASH) > > > > > #error "CONFIG_UPDATE_TFTP and !CONFIG_MTD_NOR_FLASH needed for > > > > > legacy behaviour" > > > > > #endif > > > > > > > > It would be better to just drop it so that this function will be > > > > used on wider range of platforms. > > > > > > Function update_tftp() calls update_flash(). update_flash() does nothing > > > if CONFIG_MTD_NOR_FLASH=n. > > > > > > Please, describe which configuration would be additionally usable if > > > your patch is applied. > > > > update_tftp() takes additional two parameters, interface and devstring, > > since the commit c7ff5528439a ("update: tftp: dfu: Extend update_tftp() > > function to support DFU"). > > CONFIG_DFU and COFIG_DFU_XXX, without MTD_NOR_FLASH, does work. > > But would you use CONFIG_UPDATE_TFTP in conjunction with CONFIG_DFU*?
common/update.c is only compiled under CONFIG_UPDATE_TFTP. So without CNFIG_DFU*, the commit c7ff5528439a doens't make any sense. > common/Kconfig describes UPDATE_TFTP as: "This option allows performing > update of NOR with data in fitImage sent via TFTP boot." > > doc/README.dfutftp says: > "* Update TFTP (CONFIG_UPDATE_TFTP) only supports writing > code to NAND memory via TFTP. > * DFU supports writing data to the variety of mediums (NAND, > eMMC, SD, partitions, RAM, etc) via USB." > > I assume README.dfutftp saying "NAND" and not "NOR" is a documentation > error. I believe that the document should be overhauled. > So why do you specifically need to allow > CONFIG_UPDATE_TFTP=y && CONFIG_MTD_NOR_FLASH=n ? Do you remember that I'm now working on capsule update? That is why I included you in Cc. Anyhow, Lukasz should have a comment here. Thanks, -Takahiro Akashi > Best regards > > Heinrich > > > > > Thanks, > > -Takahiro Akashi > > > > > > > Best regards > > > > > > Heinrich > > > > > > > > > > > Signed-off-by: AKASHI Takahiro <takahiro.aka...@linaro.org> > > > > --- > > > > common/update.c | 7 ++----- > > > > 1 file changed, 2 insertions(+), 5 deletions(-) > > > > > > > > diff --git a/common/update.c b/common/update.c > > > > index c8dd346a0956..ade029851dbd 100644 > > > > --- a/common/update.c > > > > +++ b/common/update.c > > > > @@ -14,10 +14,6 @@ > > > > #error "CONFIG_FIT and CONFIG_OF_LIBFDT are required for auto-update > > > > feature" > > > > #endif > > > > > > > > -#if defined(CONFIG_UPDATE_TFTP) && !defined(CONFIG_MTD_NOR_FLASH) > > > > -#error "CONFIG_UPDATE_TFTP and !CONFIG_MTD_NOR_FLASH needed for legacy > > > > behaviour" > > > > -#endif > > > > - > > > > #include <command.h> > > > > #include <env.h> > > > > #include <flash.h> > > > > @@ -210,8 +206,9 @@ static int update_flash(ulong addr_source, ulong > > > > addr_first, ulong size) > > > > printf("Error: could not protect flash sectors\n"); > > > > return 1; > > > > } > > > > +#else > > > > + return -1; > > > > #endif > > > > - return 0; > > > > } > > > > > > > > static int update_fit_getparams(const void *fit, int noffset, ulong > > > > *addr, > > > > > > > >