[U-Boot] [PATCH v3 0/9] sunxi: DM-based CLK, RESET and PINCTRL

[Dr. Philipp Tomsich]

Andre,

> On 06 Mar 2017, at 03:08, André Przywara <andre.przywara at arm.com> wrote:
> 
> On 01/03/17 21:19, Philipp Tomsich wrote:
>> Hi everyone,
>> 
>> here's the the new version of CLK, RESET and PINCTRL drivers to
>> configure sunxi from the device-tree.  This adds support for the
>> upstream CCU node (for reset and pinctrl) and tries to address the
>> various concerns people had.
>> 
>> Note that (to stay in sync with the Linux files), some of the
>> patches may not fully adhere to the style (e.g. some of the code
>> reused verbatim from Linux and in the config tables).
>> 
>> This has been tested with Ethernet, I2C, SPI and MMC on the A64-uQ7.
>> See my separate patchsets for the conversion of these drivers over
>> to support DM-based CLK, RESET and PINCTRL configuration.
>> 
>> Changes is v3:
>> * add support for the 'new-style' clock subsystem (CCU) by porting
>>   from Linux and adding the necessary glue implementation
> 
> Gah, is this really necessary? Do we really need to pull in the whole of
> the complex Linux clock driver?

It’s not necessary and I’d be more than happy to reduce the list of the
clocks in the table, but reusing the Linux driver (and it’s infrastructure)
allows us to add new SoCs by reusing the table from Linux.  However,
reducing the number of table entries will not reduce the “bulk” much,
as these are only config entries (and the code to support the various
types of clocks should be present nonetheless).

Given that these tables are fully tested on the Linux side, I believe this
to be of more value than to keep this small for U-Boot… after all, the
full CCU-support will only be used for the final U-Boot stage (which runs
from DRAM) and never considered for SPL/TPL.

> In the end all that U-Boot needs to do is to program a few simple
> clocks, a task which it easily did so far with just some register
> writes. I don't think it's appropriate for U-Boot to get the whole
> complex clock driver from Linux for just that purpose.

I beg to differ on the “a few simple clocks” aspect of this.
A full-featured U-Boot should require (including reparenting) clocks for
SPI, I2C, MMC, EMAC, USB (incl. PHYs) and video (on the SoCs where
simplefb is supported).

It’s best to keep all of this in sync with Linux, as this will simplify the
long-term maintenance.

> One indication of this being too much is that the mailing list seemed to
> have blocked patch 8/9, probably because it's too big ;-)
> I am especially scared when it comes to adding all of the other SoC's
> clock drivers to the code base as well.

It was just a couple bytes over the limit to require moderation ;-)

The benefit of this approach is that all the ccu_*.c clock implementations
and the configuration table can be reused almost verbatim from Linux.
The only change necessary will be in the list of headers included and
the change of the _ops datatype.

> So can't we do a much simpler (because limited) implementation?
> If a peripheral driver gets the clock index from the DT and asks the
> clock driver to program (or just enable) clock "75", can't we just have
> a simple function which redirects this clock number to our already
> existing code?
> Something like (rough sketch):
> int sun50i_clk_set_freq(int nr, int freq)
> {
> 	int idx;
> 
> 	switch (nr) {
> 	case CLK_MMC0 ... CLK_MMC2:
> 		idx = nr - CLK_MMC0;
> 		return mmc_set_mod_clk(idx,
> 				       clkreg_addr[MMC_CLK + idx],
> 				       freq);
> 	}
> }
> ... with mmc_set_mod_clk() being a slightly changed version of the
> existing implementation?

This will not help with many of the use-cases, such as reparenting clocks
dynamically as we change clocks on SPI.

It seems to be a good alternative implementation for the SPL use-case,
though and I’d be happy to add this in as the SPL-alternative.

> I think that would seriously reduce the bloat and be a better fit for
> U-Boot. And it might even allow the SPL to reuse this by just calling
> into the dispatch function with hardcoded parameters.

The total bulk added is not trivial, but appears to be be reasonable (for the
final-stage U-Boot):
1. code to support the various clocks: +33KB
2. worst-case config-table (i.e. what we have on the sun50iw1p1): +15KB

And the config-table can easily be reduced to a more manageable size
to removing clocks that we’ll never need (e.g. GPU, audio, …).

Cheers,
Philipp.