[PATCH v2] board: freescale: p1_p2_rdb_pc: Simplify SPL offset macros

[Tom Rini]

On Tue, Jun 28, 2022 at 08:58:48PM +0200, Pali Rohár wrote:
> On Tuesday 28 June 2022 14:53:26 Tom Rini wrote:
> > On Tue, Jun 28, 2022 at 08:41:44PM +0200, Pali Rohár wrote:
> > > On Tuesday 28 June 2022 14:39:11 Tom Rini wrote:
> > > > On Tue, Jun 28, 2022 at 08:34:47PM +0200, Pali Rohár wrote:
> > > > > On Tuesday 28 June 2022 14:28:52 Tom Rini wrote:
> > > > > > On Tue, Jun 28, 2022 at 08:22:38PM +0200, Pali Rohár wrote:
> > > > > > > On Tuesday 28 June 2022 14:17:42 Tom Rini wrote:
> > > > > > > > On Tue, Jun 28, 2022 at 07:53:55PM +0200, Pali Rohár wrote:
> > > > > > > > 
> > > > > > > > > Now when CONFIG_SYS_TEXT_BASE has sane value, use it for calculation of
> > > > > > > > > other SPL offset values: CONFIG_SPL_MAX_SIZE, CONFIG_SYS_MMC_U_BOOT_* and
> > > > > > > > > CONFIG_SYS_SPI_FLASH_U_BOOT_* macros.
> > > > > > > > > 
> > > > > > > > > No functional change.
> > > > > > > > > 
> > > > > > > > > Signed-off-by: Pali Rohár <pali at kernel.org>
> > > > > > > > > ---
> > > > > > > > > Changes in v2:
> > > > > > > > > * Rebased on top of the U-Boot next branch, commit d61c11b8c894fad517677dc51ee82d1eade39c01
> > > > > > > > > ---
> > > > > > > > >  include/configs/p1_p2_rdb_pc.h | 12 ++++++------
> > > > > > > > >  1 file changed, 6 insertions(+), 6 deletions(-)
> > > > > > > > > 
> > > > > > > > > diff --git a/include/configs/p1_p2_rdb_pc.h b/include/configs/p1_p2_rdb_pc.h
> > > > > > > > > index 56a16502dcc7..7237e3c1a63c 100644
> > > > > > > > > --- a/include/configs/p1_p2_rdb_pc.h
> > > > > > > > > +++ b/include/configs/p1_p2_rdb_pc.h
> > > > > > > > > @@ -78,15 +78,15 @@
> > > > > > > > >  
> > > > > > > > >  #ifdef CONFIG_SDCARD
> > > > > > > > >  #define CONFIG_SYS_MMC_U_BOOT_SIZE	(768 << 10)
> > > > > > > > > -#define CONFIG_SYS_MMC_U_BOOT_DST	(0x11000000)
> > > > > > > > > -#define CONFIG_SYS_MMC_U_BOOT_START	(0x11000000)
> > > > > > > > > -#define CONFIG_SYS_MMC_U_BOOT_OFFS	(128 << 10)
> > > > > > > > > +#define CONFIG_SYS_MMC_U_BOOT_DST	CONFIG_SYS_TEXT_BASE
> > > > > > > > > +#define CONFIG_SYS_MMC_U_BOOT_START	CONFIG_SYS_TEXT_BASE
> > > > > > > > > +#define CONFIG_SYS_MMC_U_BOOT_OFFS	CONFIG_SPL_PAD_TO
> > > > > > > > >  #define CONFIG_SYS_MPC85XX_NO_RESETVEC
> > > > > > > > >  #elif defined(CONFIG_SPIFLASH)
> > > > > > > > >  #define CONFIG_SYS_SPI_FLASH_U_BOOT_SIZE	(768 << 10)
> > > > > > > > > -#define CONFIG_SYS_SPI_FLASH_U_BOOT_DST		(0x11000000)
> > > > > > > > > -#define CONFIG_SYS_SPI_FLASH_U_BOOT_START	(0x11000000)
> > > > > > > > > -#define CONFIG_SYS_SPI_FLASH_U_BOOT_OFFS	(128 << 10)
> > > > > > > > > +#define CONFIG_SYS_SPI_FLASH_U_BOOT_DST		CONFIG_SYS_TEXT_BASE
> > > > > > > > > +#define CONFIG_SYS_SPI_FLASH_U_BOOT_START	CONFIG_SYS_TEXT_BASE
> > > > > > > > > +#define CONFIG_SYS_SPI_FLASH_U_BOOT_OFFS	CONFIG_SPL_PAD_TO
> > > > > > > > >  #define CONFIG_SYS_MPC85XX_NO_RESETVEC
> > > > > > > > >  #elif defined(CONFIG_MTD_RAW_NAND)
> > > > > > > > >  #ifdef CONFIG_TPL_BUILD
> > > > > > > > 
> > > > > > > > So, this is entirely a PowerPC spl set of CONFIG variables.  Can we move
> > > > > > > > them to Kconfig, and use default xxx as you're changing them to above?
> > > > > > > > The other platforms using this look to be doing the same at first
> > > > > > > > glance.
> > > > > > > > 
> > > > > > > > -- 
> > > > > > > > Tom
> > > > > > > 
> > > > > > > Not sure how to do it right now. CONFIG_SYS_MMC_U_BOOT_OFFS would need
> > > > > > > to be adjusted by size of boot sector, as is in this patch:
> > > > > > > https://patchwork.ozlabs.org/project/uboot/patch/20220405134032.704-3-pali@kernel.org/
> > > > > > > 
> > > > > > > So I just sent patch in current form which simplify definitions of
> > > > > > > CONFIG_SYS_MMC_U_BOOT_* and CONFIG_SYS_SPI_FLASH_U_BOOT_* macros.
> > > > > > 
> > > > > > Given the limited places we use CONFIG_SYS_MMC_U_BOOT_OFFS (two spots,
> > > > > > one file), remove it from CONFIG space, make it file local, and
> > > > > > FSL_PREPBL_ESDHC_BOOT_SECTOR will be in Kconfig to start with?  I'm
> > > > > > taking a harder look at what CONFIG_SYS_* can be moved to Kconfig, and
> > > > > > what needs to be just something else, and what can be cleaned up /
> > > > > > removed, so I really want to figure out a solution here that migrates
> > > > > > the symbols or removes them from CONFIG namespace.
> > > > > > 
> > > > > > -- 
> > > > > > Tom
> > > > > 
> > > > > More than month ago I proposed different solution, prepare codebase to
> > > > > completely drop CONFIG_SYS_MMC_U_BOOT_OFFS option. But nobody reacted to
> > > > > it and now I see on patchwork just "Changes Requested".
> > > > > https://patchwork.ozlabs.org/project/uboot/patch/20220511183332.1362-2-pali@kernel.org/
> > > > 
> > > > Er, I see
> > > > https://patchwork.ozlabs.org/project/uboot/patch/20220511183332.1362-1-pali@kernel.org/
> > > > notes it fails to build, so that's probably the changes requested.
> > > > 
> > > > -- 
> > > > Tom
> > > 
> > > V2 for that build failure already exists and half hour ago I sent
> > > another reminder for v2:
> > > https://lore.kernel.org/u-boot/20220628181436.iwmwvvoithgwp6pu@pali/
> > > but it has nothing with CONFIG_SYS_MMC_U_BOOT_OFFS...
> > 
> > Patch 1/3 in the series causes fails to build errors, the whole series
> > gets changes requested, that's normal to me and expecting the whole
> > series to be reposted.
> 
> See the previous link. I have already rebased and resent first patch as v2.

Yes, that's my point.  You should have resent the whole series, not just
that first patch.  Why?  Well, this feels like we're getting back to the
state much of the N900 patches got to where it was really hard for
Lokesh, or myself, to figure out the correct order to apply N patches
in, with some of them having been reposted / rebased and others
depending on those patches.  The v2 you link above isn't "v2 1/3", but
just "v2".  So, reposting the whole series is preferred.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20220628/ecaec8dd/attachment.sig>