Hi Lukasz, On Thu, 25 Apr 2019 at 04:30, Lukasz Majewski <lu...@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot