[U-Boot] [PATCH 4/4] davinci: omapl138-lcdk: enable driver-model in SPL

Bartosz Golaszewski
2019-05-29T14:15:37Z

śr., 29 maj 2019 o 15:47 Adam Ford <aford173@gmail.com> napisał(a):
>
> On Wed, May 29, 2019 at 8:23 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> >
> > śr., 29 maj 2019 o 15:04 Adam Ford <aford173@gmail.com> napisał(a):
> > >
> > > On Wed, May 29, 2019 at 4:40 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > > >
> > > > wt., 28 maj 2019 o 17:05 Adam Ford <aford173@gmail.com> napisał(a):
> > > > >
> > > > > On Tue, May 28, 2019 at 2:58 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > > > > >
> > > > > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > > > > >
> > > > > > Enable CONFIG_SPL_DM and enable the driver model for serial by defining
> > > > > > an appropriate device in the board file for da850-lcdk.
> > > > > >
> > > > > > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > > > > > ---
> > > > > > board/davinci/da8xxevm/omapl138_lcdk.c | 16 ++++++++++++++++
> > > > > > configs/omapl138_lcdk_defconfig | 5 +++++
> > > > > > 2 files changed, 21 insertions(+)
> > > > > >
> > > > > > diff --git a/board/davinci/da8xxevm/omapl138_lcdk.c b/board/davinci/da8xxevm/omapl138_lcdk.c
> > > > > > index 156d61d9fb..b20747ba01 100644
> > > > > > --- a/board/davinci/da8xxevm/omapl138_lcdk.c
> > > > > > +++ b/board/davinci/da8xxevm/omapl138_lcdk.c
> > > > > > @@ -15,6 +15,8 @@
> > > > > > #include <asm/arch/hardware.h>
> > > > > > #include <asm/ti-common/davinci_nand.h>
> > > > > > #include <asm/io.h>
> > > > > > +#include <ns16550.h>
> > > > > > +#include <dm/platdata.h>
> > > > > > #include <linux/errno.h>
> > > > > > #include <asm/mach-types.h>
> > > > > > #include <asm/arch/davinci_misc.h>
> > > > > > @@ -369,3 +371,17 @@ int board_mmc_init(bd_t *bis)
> > > > > > }
> > > > > > #endif
> > > > > > #endif
> > > > > > +
> > > > > > +#ifdef CONFIG_SPL_BUILD
> > > > > > +static const struct ns16550_platdata serial_pdata = {
> > > > > > + .base = DAVINCI_UART2_BASE,
> > > > > > + .reg_shift = 2,
> > > > > > + .clock = 228000000,
> > > > > > + .fcr = UART_FCR_DEFVAL,
> > > > > > +};
> > > > > > +
> > > > > > +U_BOOT_DEVICE(omapl138_uart) = {
> > > > > > + .name = "ns16550_serial",
> > > > > > + .platdata = &serial_pdata,
> > > > > > +};
> > > > > > +#endif
> > > > > > diff --git a/configs/omapl138_lcdk_defconfig b/configs/omapl138_lcdk_defconfig
> > > > > > index b375da50df..f53a51321c 100644
> > > > > > --- a/configs/omapl138_lcdk_defconfig
> > > > > > +++ b/configs/omapl138_lcdk_defconfig
> > > > > > @@ -33,10 +33,13 @@ CONFIG_CMD_MTDPARTS=y
> > > > > > CONFIG_CMD_DIAG=y
> > > > > > CONFIG_CMD_UBI=y
> > > > > > CONFIG_OF_CONTROL=y
> > > > > > +CONFIG_SPL_OF_CONTROL=y
> > > > >
> > > > > If SPL_OF_CONTROL is being added, why do you need to manually add the
> > > > > stuff above for the serial port? Checkout the da850-evm-u-boot.dtsi
> > > > > file. I adds the device tree entries for the serial port to appear in
> > > > > the SPL DTB.
> > > > >
> > > > > See http://git.denx.de/?p=u-boot.git;a=commit;h=f7c1d53605d9ec528abacda9ba1763c67221fc88
> > > > >
> > > >
> > > > I tried to follow the example from this patch but for some reason
> > > > adding u-boot,dm-spl; to the soc at 1c00000 node in
> > > > da850-lcdk-u-boot.dtsi results in the final image being reported as
> > > > corrupted by the aisimage tool:
> > > >
> > > > [...]
> > > > COPY spl/u-boot-spl.dtb
> > > > CC spl/common/spl/spl.o
> > > > LD spl/common/spl/built-in.o
> > > > CC spl/lib/display_options.o
> > > > LD spl/lib/built-in.o
> > > > LD spl/u-boot-spl
> > > > OBJCOPY spl/u-boot-spl-nodtb.bin
> > > > CAT spl/u-boot-spl-dtb.bin
> > > > COPY spl/u-boot-spl.bin
> > > > MKIMAGE spl/u-boot-spl.ais
> > > > Error: - AIS Image corrupted
> > > > CAT u-boot.ais
> > > > CFGCHK u-boot.cfg
> > > >
> > > > The image doen't boot (or at least there's nothing on the console).
> > > >
> > > > I'm not sure what's causing it. Any idea?
> > >
> > > I got the same 'AIS Image corrupted' message if the SPL + DTB image is
> > > too large.
> > >
> > > I enabled SPL_SYS_MALLOC_SIMPLE along with USE_TINY_PRINTF reduced my code size:
> > > (see commit c4fa049a121457ff38b74daac32e18d7afbd4538)
> > > Original:
> > > text data bss dec hex filename
> > > 20760 1216 80 22056 5628 spl/u-boot-spl
> > >
> > > Tiny Printf
> > > text data bss dec hex filename
> > > 17947 1216 80 19243 4b2b spl/u-boot-spl
> > >
> > > Malloc Simple + Tiny Printf
> > > text data bss dec hex filename
> > > 15187 176 28 15391 3c1f spl/u-boot-spl
> > >
> > > I think you can also setup CONFIG_SPL_SYS_THUMB_BUILD to also reduce
> > > size. The DA850 evm has that and CONFIG_SYS_THUMB_BUILD set to shrink
> > > U-Boot.
> > > I just ran some checks and disabling CONFIG_SPL_SYS_THUMB_BUILD makes
> > > the u-boot-spl file overflow the space requirements. With it enabled,
> > > it builds
> > > text data bss dec hex filename
> > > 24356 1144 24 25524 63b4 spl/u-boot-spl
> > >
> > > For U-Boot proper, the difference between CONFIG_SYS_THUMB_BUILD and
> > > not using it is
> > >
> > > with CONFIG_SYS_THUMB_BUILD
> > > text data bss dec hex filename
> > > 324147 19700 58256 402103 622b7 u-boot
> > >
> > > (not using it)
> > > text data bss dec hex filename
> > > 423167 19700 58248 501115 7a57b u-boot
> > >
> >
> > Even with THUMB and other code shrinking options I still get the same
> > error. In fact I even disabled MMC support in SPL just to see if it'll
> > work and I still get the same error despite the size of the resulting
> > SPL image being well within limit:
> >
> > text data bss dec hex filename
> > 19269 3692 1368 24329 5f09 spl/u-boot-spl
> >
> > as opposed to
> >
> > text data bss dec hex filename
> > 27766 4008 1368 33142 8176 spl/u-boot-spl
> >
> > with MMC in SPL.
> >
> > Also this error pops up only when extending the soc node of the device
> > tree. If I do this:
> >
> > / {
> > aliases {
> > i2c0 = &i2c0;
> > };
> >
> > foo {
> > compatible = "foo";
> > };
> >
> > bar {
> > compatible = "bar";
> > };
> > };
> >
> > It builds fine. I think the culprit is somewhere else here.
>
> Maybe keeping the DM chunks of C code as starting point is better.
> Once it's all merged, it can always be improved later. I threw out
> the idea as a suggestion, but it's clearly not as simple as I original
> thought.
>

Thanks, I take it as an Ack. I'll try to get back to it once I convert
the nand driver to using DM which won't be easy either as the nand
node on the device-tree is a sub-node of the aemif device. On linux
there's an actual aemif driver that populates its child nodes, but on
u-boot I'll have to come up with some workaround.

Bart

> adam
> >
> > Bart
> >
> > > So the size is about 25% smaller with it enabled.
> > >
> > > Unrelated to size, but something you want to consider is to increase
> > > your available malloc pool size to accommodate the device tree stuff
> > > once you've managed to shrink the code. For da850evm,
> > > SPL_SYS_MALLOC_F_LEN = 0x800
> > >
> > > adam
> > >
> > > >
> > > > Bart
> > > >
> > > > >
> > > > > > CONFIG_DEFAULT_DEVICE_TREE="da850-lcdk"
> > > > > > +CONFIG_SPL_OF_PLATDATA=y
> > > > > > CONFIG_ENV_IS_IN_NAND=y
> > > > > > CONFIG_NET_RANDOM_ETHADDR=y
> > > > > > CONFIG_DM=y
> > > > > > +CONFIG_SPL_DM=y
> > > > > > CONFIG_DM_I2C=y
> > > > > > CONFIG_DM_I2C_COMPAT=y
> > > > > > CONFIG_SYS_I2C_DAVINCI=y
> > > > > > @@ -53,8 +56,10 @@ CONFIG_SPI_FLASH_STMICRO=y
> > > > > > CONFIG_SPI_FLASH_WINBOND=y
> > > > > > CONFIG_MII=y
> > > > > > CONFIG_DRIVER_TI_EMAC=y
> > > > > > +CONFIG_SPECIFY_CONSOLE_INDEX=y
> > > > > > CONFIG_DM_SERIAL=y
> > > > > > CONFIG_SYS_NS16550=y
> > > > > > CONFIG_SPI=y
> > > > > > CONFIG_DAVINCI_SPI=y
> > > > > > CONFIG_USE_TINY_PRINTF=y
> > > > > > +# CONFIG_SPL_OF_LIBFDT is not set
> > > > > > --
> > > > > > 2.21.0
> > > > > >