[U-Boot] [PATCH v3 00/11] clk: Port Linux common clock framework [CCF] to U-boot (tag: 5.0-rc3)

[Simon Glass]

Hi Lukasz,

On Thu, 25 Apr 2019 at 04:30, Lukasz Majewski <lukma at denx.de> wrote:
>
> This patch series brings the files from Linux kernel to provide clocks support
> as it is used on the Linux kernel with common clock framework [CCF] setup.
>
> This series also fixes several problems with current clocks and provides
> sandbox tests for functions addded to clk-uclass.c file.
>
> Design decisions/issues:
> =========================
>
> - U-boot's DM for clk differs from Linux CCF. The most notably difference
> is the lack of support for hierarchical clocks and "clock as a manager
> driver" (single clock DTS node acts as a starting point for all other
> clocks).
>
> - The clk_get_rate() now caches the previously read data (no need for
> recursive access.
>
> - On purpose the "manager" clk driver (clk-imx6q.c) is not using large
> table to store pointers to clocks - e.g. clk[IMX6QDL_CLK_USDHC2_SEL] = ....
> Instead we use udevice's linked list for the same class (UCLASS_CLK).
> The rationale - when porting the code as is from Linux, one would need
> ~1KiB of RAM to store it. This is way too much if we do plan to use this
> driver in SPL.
>
> - The "central" structure of this patch series is struct udevice and its
> driver_data field contains the struct clk pointer (to the originally created
> one).
>
> - Up till now U-boot's driver model's CLK operates on udevice (main access to
>   clock is by udevice ops)
>   In the CCF the access to struct clk (comprising pointer to *dev) is
>   possible via dev_get_driver_data()
>
>   Storing back pointer (from udevice to struct clk) as driver_data is a
>   convention for CCF.

Ick. Why not use uclass-private data to store this, since every
UCLASS_CLK device can have a parent.

>
> - I could use *private_alloc_size to allocate driver's 'private"
>   structures (dev->priv) for e.g. divider (struct clk_divider *divider)
>   for IMX6Q clock, but this would change the original structure of the CCF code.
>
> The question is if it would be better to use private_alloc_size (and
> dev->private) or stay with driver_data.
> The former requires some rewritting in CCF original code (to remove
> (c)malloc, etc), but comply with u-boot DM. The latter allows re-using the
> CCF code as is, but introduces some convention special for CCF (I'm not
> sure thought if dev->priv is NOT another convention as well).

Yes I would like to avoid malloc() calls in drivers and use the
in-built mechanism.

>
> - I've added the clk_get_parent(), which reads parent's dev->driver_data to
>   provide parent's struct clk pointer. This seems the easiest way to get
>   child/parent relationship for struct clk in U-boot's udevice
>   based clocks.
>
> - For tests I had to "emulate" CCF code structure to test functionality of clk_get_parent_rate()
>   and clk_get_by_id(). Those functions will not work properly with
>   "standard" (i.e. non CCF) clock setup(with not set dev->driver_data to
>   struct clk).

Well I think we need a better approach for that anywat. driver_data is
used for getting something from the DT.

Regards,
Simon