Hi Tom,
On 5/6/20 11:20, Tom Rini wrote:
On Thu, Jun 04, 2020 at 09:13:06PM -0600, Simon Glass wrote:
Hi Tom,
On Tue, 2 Jun 2020 at 07:53, Tom Rini <tr...@konsulko.com> wrote:
On Mon, Jun 01, 2020 at 05:55:31PM -0300, Walter Lozano wrote:
Hi Simon,
On 26/5/20 15:39, Walter Lozano wrote:
Hi Simon,
On 25/5/20 18:40, Simon Glass wrote:
Hi Tom,
On Mon, 25 May 2020 at 14:57, Tom Rini <tr...@konsulko.com> wrote:
On Mon, May 25, 2020 at 02:34:20PM -0600, Simon Glass wrote:
Hi Tom,
On Mon, 25 May 2020 at 13:47, Tom Rini <tr...@konsulko.com> wrote:
On Mon, May 25, 2020 at 09:35:44AM -0600, Simon Glass wrote:
This patch provides the documentation for a proposed
enhancement to driver
model to reduce overhead in SPL.
The actual patches are not included here because
they are based on some
pending work by Walter Lozano which is not in final form.
For now, the source tree is available at:
https://gitlab.denx.de/u-boot/custodians/u-boot-dm/-/tree/dtoc-working
Comments welcome!
So here's my worry. It's not clear, aside from the device tree, how
much re-use of existing code we get with this. It feels
like it might
be fairly minimal. And at that point, are we not perhaps making too
much work for ourselves compared with just excepting that there will
need to be a place for non-abstracted-framework drivers?
What do we do
about TPL, when we get to the point of everything being
converted to DM
and as-needed tiny-DM but there's still TPL drivers?
The reason we have
SPL_FRAMEWORK as a symbol today is that we already had some
SoCs/architectures (primarily PowerPC) that had "SPL" but it was very
centric to the SoCs in question.
The interface for example for mmc is:
int spl_mmc_load_image(struct spl_image_info *spl_image, struct
spl_boot_device *bootdev) and neither part of that is
inherently DM. So
let it be MMC_TINY for the non-DM case and regular DM_MMC for the DM
case. I wonder if we could clean that up code a little
if we let it be
separate.
The interface for example for spi is:
int spl_spi_load_image(struct spl_image_info *spl_image,
struct spl_boot_device *bootdev) and well, the same
thing. Or maybe we
can even push that up to the spi_flash_load() call.
But my worry is that a different set of abstractions here are still
going to bring us in more overhead than writing drivers for the
functionality we need directly, and if we define what's
allowed in this
limited case well, that might be good enough.
Some boards (e.g. x86) Need to read multiple things from the SPI flash
(such as FSP binaries), so I still think we will want a generic
reading interface.
You could be right, but my hunch is that there is value in having
things more generic and the cost should be minimal. The value is that
hopefully writing a few C functions in the SPI driver will be enough
to enable tiny SPI on an SoC, reusing much of the code in the driver
(only the reading bits!). We won't need as much special-case code and
an entirely different way of configuring these devices for TPL/SPL.
It has been interesting digging into the Zephyr model. It's drivers
are very basic and thus small. But there is still value in using the
device tree to assemble things.
Anyway I'm not really sure at this point. It is just a hunch. I don't
think we can know all this until we have a bit more information.
Perhaps with a board with SPI, MMC and serial converted we would get a
better picture?
I think it's absolutely the case that we'll have to convert something
and see how it looks, then convert something else and see if it still
looks good enough. At a high enough level there's not really too much
of a difference between what it sounds like you're proposing and what
I'm proposing. Possibly even in a progmatic way too. We have (I think
anyhow) fairly static board configurations in this case so we don't so
much need to "probe" for possible drivers be told what our device
hierarchy is and to initialize what we're going to use.
Yes, we may end up with special, separate code anyway, since if you
end up refactoring the driver so much (and putting tiny-dm tentacles
into it) that it becomes harder to maintain, it isn't a win.
Basically I started out similar to what you are saying, with the idea
of just direct calls into the driver (e.g. the driver implements
serial_putc() and spi_read_flash()). But then I figured it is a very
small overhead to retain some sort of driver model, so I thought I'd
try that.
I'll fiddle with this again in a week or so...
Thanks for this proposal.
I'm very interested in see where this implementation leads us, as I
always felt that some work could be done in order to reduce the overhead
of DM support in TPL/SPL. I'll review this work and hopefully come back
to you with some comments.
In the same sense, I feel that maybe we can add some additional
intelligence to dtoc in order to produce a more customized code for
TPL/SPL, maybe relaying in some custom stuff in u-boot.dtsi, but this is
only a feeling.
I've been checking your work and found it very interesting. Let me share
some comments
I really like the trade-off between size and features in this implementation
and the way you get rid of not very useful data, such as strings and class
overhead.
I see that you parse drivers in order to build the relationship between
drivers and compatible strings among other things. I faced a similar
requirement in the series I proposed, however, I proposed the use of a
U_BOOT_DRIVER_ALIAS in order to explicitly declare the alias. Then main
reason behind this were to make code cleaner, avoid a lot of parsing and
having to take into account possible valid C code which is not cover by the
parsing.
I have no problem with your approach, on the contrary I like the idea of
improving dtoc, but I think that if we take this path, we should put some
constrains and to document them to avoid unexpected behavior. Most of the
constrains maybe already used by all the drivers, like the way we declare
compatible strings, however any limitation in the parsing should be
documented.
The same goes for parsing config files which is also a nice improvement. I
feel that every step we give adding intelligence to dtoc is a step towards
footprint improvement.
Would this allow us to make progress towards the idea of being able to
discard compatibles (and referenced data) that won't be used at run time
given the dts files for the boards we'll support at run time?
Do you mean just the compatible strings, or do you mean whole drivers?
Compatible and data. One of the "this is ugly but works" patches to
reduce size on i.MX families was to guard compatible string + data (see
drivers/mmc/fsl_esdhc_imx.c for example) with compile time checks. If
we could automate that somehow, that would be good.
Could you point to the "ugly but works" patch in order to have better
context?
Regarding drivers/mmc/fsl_esdhc_imx.c I have been working in adding
OF_PLATDATA support to it in order to reduce the SPL footprint on iMX6,
which I think it could be a first step for further improvements.
https://patchwork.ozlabs.org/project/uboot/list/?series=167495&state=*
Regards,
Walter
