[PATCH] RFC: tiny-dm: Proposal for using driver model in SPL

[Walter Lozano]

Hi Simon,

On 25/5/20 18:40, Simon Glass wrote:
> Hi Tom,
>
> On Mon, 25 May 2020 at 14:57, Tom Rini <trini at 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 <trini at 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.

Regards.

Water