[U-Boot] [linux-sunxi] Re: [PATCH v4 13/19] sunxi: DT: A64: update board .dts files from Linux

Sun Apr 1 02:41:35 UTC 2018

On Sun, Apr 1, 2018 at 9:28 AM, André Przywara <andre.przywara at arm.com> wrote:
> On 30/03/18 05:25, Chen-Yu Tsai wrote:
>
> ....
>>> OK. So meanwhile I have something almost(TM) working:
>>> - drivers/clk/sunxi/clk-a64.c, which is a UCLASS_CLK implementation of
>>> the clock IDs from allwinner,sun50i-a64-ccu that we need: CLK_BUS_UARTx,
>>> CLK_BUS_MMCx, CLK_MMCx. Their implementation is fairly simple, actually
>>> (I did it the U-Boot way, not pulling in any super-fancy Linux code).
>>> Porting this over to H3/H5 and other SoCs should be trivial: copy/paste
>>> for now. We can look at how to unify this later.
>>> - drivers/mmc/sunxi_mmc.c extended to use UCLASS_CLK clocks. This is
>>> also not too bad, but I seem to miss a bit in here, as it times out.
>>> Will debug this tonight.
>>> - Cowardly dodging a proper UCLASS_RESET driver for now, instead hacking
>>> the single bit in :-(
>>>
>>> That looks tight to still get into this merge window, though, at least
>>> if we follow the usual process.
>>
>> You could post an initial version during the merge window, and refine it
>> in the following two weeks? AFAIK the rules allow for it to be merged for
>> the coming release, instead of the next.
>
> Is that so? I had the impression that this was tightened in the last few
> releases, so no features would be allowed beyond the merge window
> anymore. I will try to send something ASAP, but ...
>
>> Curiously, U-boot repositories don't have -next branches. Is that a
>> maintainer preference? Having one would help developers in a way as
>> to not have to hunt down prerequisites and try to figure out whether
>> they have passed review and will be merged or not, and also avoid
>> conflicts with other to-be-queued patches. I know this takes extra
>> effort from the maintainer to possibly rebase or manage conflicts
>> after a new merge window has opened, as I personally manage sunxi-next
>> for the Linux kernel to serve as sort of an integration branch for
>> others. I'm asking is it possible to have -next, even just for sunxi.
>>
>>>
>>> Keep you posted.
>>
>> I'm interested. IMHO we don't need full blown drivers like in Linux.
>> We should be able to get away with selectively supporting only the
>> resources needed for the peripherals supported / actively used in U-boot.
>> This goes for clocks, resets, pinctrl, regulators, etc..
>
> So I have something(TM) working now. This is a bit like a can of worms:
> - As mentioned above, we need a UCLASS_CLK driver. This is pretty
> straightforward, one driver per SoC, then something like:
> int sunxi_clk_enable(clk)
> {
>         switch (clk->id) {
>         case CLK_MMC0:
>                 addr = priv->base + 0x88;
>                 setbits_le32(clk_base, BIT(31));
> (plus get_rate/set_rate)
> As you guessed, we just list the clocks we need, in the moment this is
> UART and MMC. Adding new clocks is easy, other SoCs can be copy&pasted
> for now, we might find a clever way of code sharing later.
> One nasty property is the marriage of RESET and CLK in the sunxi-ng
> clock binding. So we also need a DM reset driver. I need to wrap my head
> around how to instantiate those at the same time from only one compatible.

We could look at how the DM gpio driver currently does it: The compatible
matches the driver directly, and the DM bind function creates many child
devices using platform data and binds it to the same driver. The device
node is also assigned to the same one. AFAIK you have to figure out how
to lookup a different driver by name for the child device, e.g. a reset
driver to bind to the child device of the clk device.

In addition, Philipp from Theobroma Systems posted a series some time ago
for sunxi DM conversion, which included some patches that involved creating
multiple uclass devices for the same device node.

>
> - Also I realised two days ago that we need a DM pinctrl driver. As this
> was on my list anyway, I just bit the bullet. Eventually this isn't as
> bad as it sounds, as I resorted to the "pinmux" property to give me the
> mux value, so don't need the huge table Linux uses.
> But: a similar problem as above, as we need to marry this to the already
> existing DM_GPIO driver, because they share a DT node.

Same as the above I guess? And having the pinctrl driver as the base device
might work out better. It looks like we won't have gpio/pinctrl exclusivity
like we do in Linux, so people should try to avoid shooting themselves in
the foot. We could try denying requests based on whether the pinmux value
in the register is not the default GPIO / disconnected value.

>
> So the current status is:
> - UCLASS_CLK works and looks fairly reasonable.
> - UCLASS_PINCTRL works, just requires adding a pinmux property to each
> pinctrl pin group node (just a few), as I proposed last year for Linux[1].

IIRC this didn't go well? We could have a simplified table to cover the
use cases we need (again it's probably just UART + MMC). We don't need
to declare every single pin. Since a function tends to have the same
pinmux value for each used pin within the same pingroup, we could just
have a table that maps [SoC, pingroup, function] to pinmux value. And
you could just ignore the gpio_{in,out} pinmux nodes.

> - no UCLASS_RESET for the sunxi-ng resets yet. Hacked badly atm.
> - The existing UCLASS_GPIO driver clashes with UCLASS_PINCTRL, so I
> disabled the former for now.
> - The existing UCLASS_MMC driver got amended to use all of those.
>
> This boots on the Pine64, at least via FEL, with USB, MMC and Ethernet
> working in U-Boot proper.
>
> Just in case someone gets impatient:
> https://github.com/apritzel/u-boot/commits/sunxi-dm-WIP
>
> I will try to get rid of the hacks and post an RFC.
>
> But, as Jagan mentioned already: eventually the outcome is quite
> questionable. For the near future we need the non-DM bits (UART + MMC)
> for the SPL still, so we can't get rid of this code. So technically we
> support DM_MMC/DM_BLK, but it's not clear what the actual benefit is.

My thoughts exactly. We end up with either two drivers, one DM and one not,
or we have a whole bunch of #ifdefs in the DM driver to trim it down for
SPL.

Regards
ChenYu

>
> Cheers,
> Andre.
>
> [1]
> http://archive.armlinux.org.uk/lurker/message/20171113.012520.b50dc300.en.html
>