[PATCH] common: update_tftp: remove unnecessary build check

[AKASHI Takahiro]

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.akashi at 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,
> > > > 
> > > 
>