From mboxrd@z Thu Jan 1 00:00:00 1970 From: Walter Lozano Date: Tue, 26 May 2020 15:39:57 -0300 Subject: [PATCH] RFC: tiny-dm: Proposal for using driver model in SPL In-Reply-To: References: <20200525093539.1.Ibf2d19439cde35e39192a9d4a8dad23539fae2e6@changeid> <20200525194737.GS12717@bill-the-cat> <20200525205714.GV12717@bill-the-cat> Message-ID: <9b018eaa-4b57-acdd-9ec3-02017badcdca@collabora.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Simon, On 25/5/20 18:40, Simon Glass wrote: > Hi Tom, > > On Mon, 25 May 2020 at 14:57, Tom Rini 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 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