[U-Boot] [PATCH v4 1/1] board: arm: Add support for Broadcom BCM7445
Thomas Fitzsimmons
fitzsim at fitzsim.org
Wed Aug 28 17:24:28 UTC 2019
Hi Bin,
Bin Meng <bmeng.cn at gmail.com> writes:
> Hi Thomas,
>
> On Wed, Aug 28, 2019 at 6:31 AM Thomas Fitzsimmons <fitzsim at fitzsim.org> wrote:
>>
>> Hi Bin,
>>
>> Bin Meng <bmeng.cn at gmail.com> writes:
>>
>> > Hi Thomas,
>> >
>> > On Sat, Jun 9, 2018 at 6:06 AM Thomas Fitzsimmons <fitzsim at fitzsim.org> wrote:
>> >>
>> >> Add support for loading U-Boot on the Broadcom 7445 SoC. This port
>> >> assumes Broadcom's BOLT bootloader is acting as the second stage
>> >> bootloader, and U-Boot is acting as the third stage bootloader, loaded
>> >> as an ELF program by BOLT.
>> >>
>> >> Signed-off-by: Thomas Fitzsimmons <fitzsim at fitzsim.org>
>> >> Cc: Stefan Roese <sr at denx.de>
>> >> Cc: Tom Rini <trini at konsulko.com>
>> >> Cc: Florian Fainelli <f.fainelli at gmail.com>
>> >> ---
>> >> Changes for v4:
>> >> - Use high timer register for get_ticks
>> >> - Move hard-coded register addresses from Kconfig to header
>> >> - Document I-cache/D-cache expectation
>> >>
>> >> MAINTAINERS | 10 +
>> >> arch/arm/Kconfig | 12 +
>> >> arch/arm/Makefile | 1 +
>> >> arch/arm/mach-bcmstb/Kconfig | 36 ++
>> >> arch/arm/mach-bcmstb/Makefile | 8 +
>> >> arch/arm/mach-bcmstb/include/mach/gpio.h | 11 +
>> >> arch/arm/mach-bcmstb/include/mach/hardware.h | 11 +
>> >> arch/arm/mach-bcmstb/include/mach/prior_stage.h | 30 ++
>> >> arch/arm/mach-bcmstb/include/mach/sdhci.h | 15 +
>> >> arch/arm/mach-bcmstb/include/mach/timer.h | 13 +
>> >> arch/arm/mach-bcmstb/lowlevel_init.S | 21 ++
>> >> board/broadcom/bcmstb/MAINTAINERS | 7 +
>> >> board/broadcom/bcmstb/Makefile | 8 +
>> >> board/broadcom/bcmstb/bcmstb.c | 194 +++++++++++
>> >> configs/bcm7445_defconfig | 27 ++
>> >> doc/README.bcm7xxx | 150 ++++++++
>> >> drivers/mmc/Kconfig | 11 +
>> >> drivers/mmc/Makefile | 1 +
>> >> drivers/mmc/bcmstb_sdhci.c | 67 ++++
>> >> drivers/spi/Kconfig | 7 +
>> >> drivers/spi/Makefile | 1 +
>> >> drivers/spi/bcmstb_spi.c | 439 ++++++++++++++++++++++++
>> >> drivers/spi/spi-uclass.c | 2 +-
>> >> dts/Kconfig | 7 +
>> >> include/configs/bcm7445.h | 26 ++
>> >> include/configs/bcmstb.h | 183 ++++++++++
>> >> include/fdtdec.h | 4 +
>> >> lib/fdtdec.c | 4 +
>> >> 28 files changed, 1305 insertions(+), 1 deletion(-)
>> >> create mode 100644 arch/arm/mach-bcmstb/Kconfig
>> >> create mode 100644 arch/arm/mach-bcmstb/Makefile
>> >> create mode 100644 arch/arm/mach-bcmstb/include/mach/gpio.h
>> >> create mode 100644 arch/arm/mach-bcmstb/include/mach/hardware.h
>> >> create mode 100644 arch/arm/mach-bcmstb/include/mach/prior_stage.h
>> >> create mode 100644 arch/arm/mach-bcmstb/include/mach/sdhci.h
>> >> create mode 100644 arch/arm/mach-bcmstb/include/mach/timer.h
>> >> create mode 100644 arch/arm/mach-bcmstb/lowlevel_init.S
>> >> create mode 100644 board/broadcom/bcmstb/MAINTAINERS
>> >> create mode 100644 board/broadcom/bcmstb/Makefile
>> >> create mode 100644 board/broadcom/bcmstb/bcmstb.c
>> >> create mode 100644 configs/bcm7445_defconfig
>> >> create mode 100644 doc/README.bcm7xxx
>> >> create mode 100644 drivers/mmc/bcmstb_sdhci.c
>> >> create mode 100644 drivers/spi/bcmstb_spi.c
>> >> create mode 100644 include/configs/bcm7445.h
>> >> create mode 100644 include/configs/bcmstb.h
>> >>
>> >
>> > [snip]
>> >
>> >> diff --git a/drivers/spi/spi-uclass.c b/drivers/spi/spi-uclass.c
>> >> index d2d091f..c517d06 100644
>> >> --- a/drivers/spi/spi-uclass.c
>> >> +++ b/drivers/spi/spi-uclass.c
>> >> @@ -273,7 +273,7 @@ int spi_get_bus_and_cs(int busnum, int cs, int speed, int mode,
>> >> bool created = false;
>> >> int ret;
>> >>
>> >> -#if CONFIG_IS_ENABLED(OF_PLATDATA)
>> >> +#if CONFIG_IS_ENABLED(OF_PLATDATA) || CONFIG_IS_ENABLED(OF_PRIOR_STAGE)
>> >
>> > Could you please explain this change a little bit? Why should we call
>> > uclass_first_device_err() for OF_PRIOR_STAGE?
>> >
>> >> ret = uclass_first_device_err(UCLASS_SPI, &bus);
>> >> #else
>> >> ret = uclass_get_device_by_seq(UCLASS_SPI, busnum, &bus);
>>
>> On the BCM7445 eval board, the prior-stage-provided device tree does not
>> have an alias for spi0, though it has aliases for other device types; I
>> don't know why this is the case, but I don't have control over what the
>> prior stage bootloader (Broadcom's BOLT) provides. Without that alias,
>> uclass_get_device_by_seq fails to find the SPI bus (and so U-Boot can't
>> find the SPI flash device that stores its environment).
>>
>
> I checked uclass_get_device_by_seq() and did not find any codes that
> are trying to read aliases. Am I missing anything?
Alias handling is not in the direct uclass_get_device_by_seq call chain,
and it took some debugging to find this. The requested sequence number
is handled earlier in the boot, in device_bind_common:
dev->seq = -1;
dev->req_seq = -1;
if (CONFIG_IS_ENABLED(DM_SEQ_ALIAS) &&
(uc->uc_drv->flags & DM_UC_FLAG_SEQ_ALIAS)) {
/*
* Some devices, such as a SPI bus, I2C bus and serial ports
* are numbered using aliases.
*
* This is just a 'requested' sequence, and will be
* resolved (and ->seq updated) when the device is probed.
*/
if (CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)) {
if (uc->uc_drv->name && ofnode_valid(node))
dev_read_alias_seq(dev, &dev->req_seq);
} else {
dev->req_seq = uclass_find_next_free_req_seq(drv->id);
}
}
If the prior stage bootloader provides no SPI bus alias, then
dev_read_alias_seq fails, dev->req_seq stays unset (-1), and later when
an attempt is made to access the SPI bus, the call to
uclass_get_device_by_seq fails.
>> At the time, I checked other ARM device trees in the Linux kernel and
>> not many set an alias for spi0, so I wrote the patch to choose the first
>> SPI bus. Doing so was in line with what CONFIG_OF_PLATDATA did on
>> boards that wanted to avoid device tree accesses.
>>
>
> Based on what you said, the adding of OF_PRIOR_STAGE here sounds like a hack.
Yeah, it makes an assumption about which SPI bus to use, that may or may
not be valid on other boards.
>> I see that since I introduced CONFIG_OF_PRIOR_STAGE, several other ports
>> have started using it, so there's probably a need to generalize this; if
>> other prior stage bootloaders provide SPI aliases then there should be a
>> way for this code to use them.
>>
>
> I think the correct way is to call uclass_get_device_by_seq().
> CONFIG_OF_PRIOR_STAGE should not imply such behavior.
OK, but without other changes this would break boards that rely on the
above assumption (for example BCM7445).
I'm experimenting with patches that work on BCM7445 and eliminate the
SPI bus assumption but I don't know what effect they might have on other
ports that use OF_PRIOR_STAGE.
In general it seems like a good idea to use the prior-stage-provided
aliases, but the question is what to do when an alias is missing; maybe
always fall back to calling uclass_find_next_free_req_seq?
Have you found a board for which uclass_get_device_by_seq works for SPI
and uclass_first_device_err does the wrong thing? Is it a publicly
available port that I can have a look at?
Thanks,
Thomas
More information about the U-Boot
mailing list