[U-Boot] [PATCH 3/5] smart-gateway-mt7688: Correct build time check for overwriting factory data

From: Tom Rini <trini@konsulko.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 3/5] smart-gateway-mt7688: Correct build time check for overwriting factory data
Date: Thu, 14 Nov 2019 11:59:46 -0500	[thread overview]
Message-ID: <20191114165946.GB19317@bill-the-cat> (raw)
In-Reply-To: <0796e235-b817-8971-d932-8e795d81faa7@denx.de>

On Thu, Nov 14, 2019 at 05:50:19PM +0100, Stefan Roese wrote:
> Hi Tom,
> 
> On 14.11.19 15:54, Tom Rini wrote:
> > The board file has a build time check to ensure that we do not have the
> > redundant environment overwriting the factory data.  However,
> > multiplying the redundant offset by two isn't correct.  Since we
> > correctly confirm that U-Boot itself will not go in to the primary
> > environment we only need to next make sure that the redundant
> > environment + the size of that does not exceed where the factory data
> > is.
> > 
> > Cc: Stefan Roese <sr@denx.de>
> > Signed-off-by: Tom Rini <trini@konsulko.com>
> > ---
> >   board/gardena/smart-gateway-mt7688/board.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/board/gardena/smart-gateway-mt7688/board.c b/board/gardena/smart-gateway-mt7688/board.c
> > index 7b9570714342..e9ffd42a8a68 100644
> > --- a/board/gardena/smart-gateway-mt7688/board.c
> > +++ b/board/gardena/smart-gateway-mt7688/board.c
> > @@ -18,7 +18,7 @@
> >   #define FACTORY_DATA_OFFS	0xc0000
> >   #define FACTORY_DATA_SECT_SIZE	0x10000
> > -#if ((CONFIG_ENV_OFFSET_REDUND * 2) > FACTORY_DATA_OFFS)
> > +#if ((CONFIG_ENV_OFFSET_REDUND + CONFIG_ENV_SIZE) > FACTORY_DATA_OFFS)
> >   #error "U-Boot image with environment too big (overlapping with factory-data)!"
> >   #endif
> >   #define FACTORY_DATA_USER_OFFS	0x140
> 
> Hmmm, the change looks good from looking at your patch and I was
> wondering why I did insert such a broken check here. But when looking
> at the current mainline code, this is what the code looks like:
> 
> #define FACTORY_DATA_OFFS	0xc0000
> #define FACTORY_DATA_SECT_SIZE	0x10000
> #if ((CONFIG_ENV_OFFSET_REDUND + CONFIG_ENV_SIZE_REDUND) > FACTORY_DATA_OFFS)
> #error "U-Boot image with environment too big (overlapping with factory-data)!"
> #endif
> 
> So its already correct in mainline (current master). I'm wondering on
> which git repository you based your patch upon?

Ah, OK, I see better now, thanks for the review.  Patch 2/5 changes this
there's no case (nor way for, logically) CONFIG_ENV_SIZE !=
CONFIG_ENV_SIZE_REDUND so it drops CONFIG_ENV_SIZE_REDUND from the
codebase.  So a v2 of the series should change this board first to use
CONFIG_ENV_SIZE and then drop the symbol.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20191114/18c92521/attachment.sig>