[U-Boot] [PATCH v4 1/1] board: arm: Add support for Broadcom BCM7445

[Bin Meng]

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?

> 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.

> 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.

Regards,
Bin