[U-Boot] [PATCH v4 1/1] board: arm: Add support for Broadcom BCM7445
Bin Meng
bmeng.cn at gmail.com
Thu Aug 29 15:24:22 UTC 2019
+Simon
On Thu, Aug 29, 2019 at 1:24 AM Thomas Fitzsimmons <fitzsim at fitzsim.org> wrote:
>
> 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.
Ah, I see. Thanks!
>
> >> 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?
I tend to agree. As I see lots of places in U-Boot sources, like below:
drivers/i2c/intel_i2c.c::intel_i2c_bind()
if (device_is_on_pci_bus(dev)) {
/*
* ToDo:
* Setting req_seq in the driver is probably not recommended.
* But without a DT alias the number is not configured. And
* using this driver is impossible for PCIe I2C devices.
* This can be removed, once a better (correct) way for this
* is found and implemented.
*/
dev->req_seq = num_cards;
And another example:
drivers/usb/host/ehci-vf.c::vf_usb_bind()
/*
* Without this hack, if we return ENODEV for USB Controller 0, on
* probe for the next controller, USB Controller 1 will be given a
* sequence number of 0. This conflicts with our requirement of
* sequence numbers while initialising the peripherals.
*/
dev->req_seq = num_controllers;
Simon, do you think we should change the behavior of dev->req_seq in
the device bind for uclass drivers with DM_UC_FLAG_SEQ_ALIAS flag?
>
> 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?
No, I was not indicating an error like uclass_get_device_by_seq works
for SPI and uclass_first_device_err does the wrong thing. The issue
with your implementation is that it forces probing the first SPI
controller and ignore the "busnum" completely. This makes a board with
multiple SPI controllers unable to probe other controllers than the
very first one.
Regards,
Bin
More information about the U-Boot
mailing list