[U-Boot] [RFC] Proposal on changing raw boot mode

[U-Boot] [RFC] Proposal on changing raw boot mode

[Sam Protsenko]

Hi guys,

I'd like to make two changes on how raw MMC address and size of U-Boot
are represented. But I think it's better to discuss it first, so we
are on the same page about it.

Basically I want to review two config options here.

1. CONFIG_SYS_U_BOOT_MAX_SIZE_SECTORS

    Simple grep shows us that noone actually uses this constant
(despite it's being defined for multiple boards). So I'm thinking to
remove it altogether. What do you think about that?

2. CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR

    For TI boards it's defined in common file:
include/configs/ti_armv7_common.h . So if some board has another
U-Boot partition address (on MMC), it must redefine that option after
including ti_armv7_common.h.

    Historical background: actually I tried to change this address
before, because DRA7 EVM and AM57x EVM / X15 boards are broken in
mainline U-Boot right now. My attempt [1] turned out to be
ill-designed, as it broke other boards (IIRC, it was BeagleBone
Black): [2]. Which further led to reverting my patch: [3].

    It remains to be a problem, though. So I see 2 possible ways how to fix it:

    (a) Just re-define this address in corresponding board configs (headers).
    (b) Convert this option to Kconfig and define it correctly in each
board's defconfig.

It would be great to hear your thoughts about those two items, so we
can avoid resending/discarding of patches due to under-communication.

Thanks!

[1] http://lists.denx.de/pipermail/u-boot/2016-April/252060.html
[2] http://lists.denx.de/pipermail/u-boot/2016-May/253407.html
[3] http://lists.denx.de/pipermail/u-boot/2016-May/253408.html

[U-Boot] [RFC] Proposal on changing raw boot mode

[Alexander Graf]

On 20/10/2016 19:07, Sam Protsenko wrote:
> Hi guys,
> 
> I'd like to make two changes on how raw MMC address and size of U-Boot
> are represented. But I think it's better to discuss it first, so we
> are on the same page about it.
> 
> Basically I want to review two config options here.
> 
> 1. CONFIG_SYS_U_BOOT_MAX_SIZE_SECTORS
> 
>     Simple grep shows us that noone actually uses this constant
> (despite it's being defined for multiple boards). So I'm thinking to
> remove it altogether. What do you think about that?
> 
> 2. CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR
> 
>     For TI boards it's defined in common file:
> include/configs/ti_armv7_common.h . So if some board has another
> U-Boot partition address (on MMC), it must redefine that option after
> including ti_armv7_common.h.

I don't understand this part. If you're actually using partitions, just
use the partition code rather than the sector code. The main purpose of
the sector SPL loading code is to allow loading from somewhere that is
*not* a partition, no?

>     Historical background: actually I tried to change this address
> before, because DRA7 EVM and AM57x EVM / X15 boards are broken in
> mainline U-Boot right now. My attempt [1] turned out to be
> ill-designed, as it broke other boards (IIRC, it was BeagleBone
> Black): [2]. Which further led to reverting my patch: [3].
> 
>     It remains to be a problem, though. So I see 2 possible ways how to fix it:
> 
>     (a) Just re-define this address in corresponding board configs (headers).
>     (b) Convert this option to Kconfig and define it correctly in each
> board's defconfig.

Converting to Kconfig is always a good idea. So that one's a no-brainer
"yes, just do it".

As for the overall logic, I find it very hard to follow and terribly
crude. We currently mangle "sector offset u-boot starts" with "partition
number u-boot is on" with various intransparent combinations of those.

Can't we just split CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION and
CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR into

  CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR
  CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION
  CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION_OFFSET

Then just model the whole "TI usually sets SECTOR to x" logic in Kconfig
and override it in your board's defconfig.


Alex

[U-Boot] [RFC] Proposal on changing raw boot mode

[Anatolij Gustschin]

Hi,

On Thu, 20 Oct 2016 20:07:24 +0300
Sam Protsenko semen.protsenko at linaro.org wrote:
...
> 1. CONFIG_SYS_U_BOOT_MAX_SIZE_SECTORS
> 
>     Simple grep shows us that noone actually uses this constant
> (despite it's being defined for multiple boards). So I'm thinking to
> remove it altogether. What do you think about that?

it is not directly used, but config files including imx6_spl.h define
CONFIG_SYS_MONITOR_LEN depending on CONFIG_SYS_U_BOOT_MAX_SIZE_SECTORS.
When removing, take care to update CONFIG_SYS_MONITOR_LEN there.

--
Anatolij

[U-Boot] [RFC] Proposal on changing raw boot mode

[Tom Rini]

On Thu, Oct 20, 2016 at 08:07:24PM +0300, Sam Protsenko wrote:
> Hi guys,
> 
> I'd like to make two changes on how raw MMC address and size of U-Boot
> are represented. But I think it's better to discuss it first, so we
> are on the same page about it.
> 
> Basically I want to review two config options here.
> 
> 1. CONFIG_SYS_U_BOOT_MAX_SIZE_SECTORS
> 
>     Simple grep shows us that noone actually uses this constant
> (despite it's being defined for multiple boards). So I'm thinking to
> remove it altogether. What do you think about that?

With the one exception that's been pointed out, yes.

> 2. CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR
> 
>     For TI boards it's defined in common file:
> include/configs/ti_armv7_common.h . So if some board has another
> U-Boot partition address (on MMC), it must redefine that option after
> including ti_armv7_common.h.
> 
>     Historical background: actually I tried to change this address
> before, because DRA7 EVM and AM57x EVM / X15 boards are broken in
> mainline U-Boot right now. My attempt [1] turned out to be
> ill-designed, as it broke other boards (IIRC, it was BeagleBone
> Black): [2]. Which further led to reverting my patch: [3].
> 
>     It remains to be a problem, though. So I see 2 possible ways how to fix it:
> 
>     (a) Just re-define this address in corresponding board configs (headers).
>     (b) Convert this option to Kconfig and define it correctly in each
> board's defconfig.

This, and a few other options, are SPL-specific things that didn't have
SPL in the prefix and weren't converted along with Simon's series that
covered almost everything else.  So yes, please convert this to Kconfig,
put it in common/spl/Kconfig, and have sane defaults for various
platforms.

But also note that when we modify the TI platforms we will have to be
careful to not break other use-cases as, iirc, part of the problem was
that you want make the Android values the default values for platforms
that quite often also run not-Android.

Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20161021/9b1fcdd9/attachment.sig>