On Tue, Jan 29, 2019 at 1:35 PM Lukasz Majewski <lu...@denx.de> wrote: > > Hi Jagan, > > > On Tue, Jan 29, 2019 at 12:46 PM Lukasz Majewski <lu...@denx.de> > > wrote: > > > > > > Hi Stefano, Fabio, > > > > > > > Hi Lukasz, > > > > > > > > On 21/01/19 15:19, Lukasz Majewski wrote: > > > > > Hi Fabio, > > > > > > > > > >> Hi Lukasz, > > > > >> > > > > >> On Sat, Jan 19, 2019 at 7:15 AM Lukasz Majewski <lu...@denx.de> > > > > >> wrote: > > > > >>> +static ulong imx6q_clk_get_rate(struct clk *clk) > > > > >>> +{ > > > > >>> + ulong rate = 0; > > > > >>> + > > > > >>> + debug("%s(#%lu)\n", __func__, clk->id); > > > > >>> + > > > > >>> + switch (clk->id) { > > > > >>> + case IMX6QDL_CLK_ECSPI1: > > > > >>> + case IMX6QDL_CLK_ECSPI2: > > > > >>> + case IMX6QDL_CLK_ECSPI3: > > > > >>> + case IMX6QDL_CLK_ECSPI4: > > > > >>> + return imx6_get_cspi_clk(); > > > > >>> + > > > > >>> + case IMX6QDL_CLK_USDHC1: > > > > >>> + case IMX6QDL_CLK_USDHC2: > > > > >>> + case IMX6QDL_CLK_USDHC3: > > > > >>> + case IMX6QDL_CLK_USDHC4: > > > > >>> + return imx6_get_usdhc_clk(clk->id - > > > > >>> IMX6QDL_CLK_USDHC1); > > > > >> > > > > >> I don't think this scales well as this needs to grow for all > > > > >> other peripherals and for each port instance. > > > > > > > > > > > > > I am hiiting the same doubts as Fabio when I reviewed Peng's > > > > patches with clocks for i.MX8M. The current approach was > > > > introduced some years ago with i.MX5, but it does not fit well > > > > now and it does not scale. > > > > > > > > > > > > > The rationale regarding this approach: > > > > > > > > > > 1. Reuse the clock.c code for iMX6Q as much as possible. > > > > > > > > > > 2, This code is based on the clk-imx8q.c file - hence the > > > > > question why the Linux clock API was not ported for this new > > > > > SoC?. > > > > > > > > Many reasons, first there was already a set of functions to get / > > > > set clocks coming from previos i.MX platforms, and this API was > > > > simply reused. And nobody was in charge to port the clock API. > > > > > > > > > > > > > >> > > > > >> If we are adding a clock driver for mx6, why don't we add it > > > > >> just like the kernel one? > > > > > > > > > > I can try to port the Linux code, but IMHO it would be feasible > > > > > to port only relevant (ECSPI, USDHC) parts of it (not all as I > > > > > cannot test it all properly). > > > > > > > > Fully agree. Further parts will be added on demand. Less is > > > > better. I disagree to add not tested code. > > > > > > The work is in progress - I will add the same directory structure as > > > Linux's Common Clock Framework [CCF]. I shall finish in 2-3 days. > > > > > > There are a few problems to tackle (when porting code from Linux): > > > > Just to add my experience with previous CLK support series[1]. Do we > > really need to port it from Linux, because I'm thinking we can manage > > it as simple as other SoC support in U-Boot.(I have few clocks > > addition on this series other than ENET) > > > > What do you think? > > This patch series seems to me like the one which reuses the "large" > switch/case approach with "fsl,imx6q-ccm" driver [*]. > > The ENET shall also be handled by the CCF from Linux. > > Another problem - the U-boot's clock DM API is not supporting passing > parent clock rate, which is important in the hierarchical clock tree. > The Linux CCF is a hierarchical one built in the driver's [*] probe.
Understand, we need to taken care the parent clock rates as well for set and get rates. In fact we tried similar thing with sunxi, patch[2] implements get_rate by taking parent clock rates in to account by using switch statements. and this change[3] do add same but with recursive calls. Since U-Boot need minimal clocks and most of them run with default parents it better to implement as smooth as possible otherwise it would ended-up size and complex issues, IMHO. I think other platforms like mediatek clocks are also following same. > > > > Do we need to port the (trimmed) Common Clock Framework [CCF] from > Linux? > > With the first version of this patch I also wanted to re-use the code > from clock.c imx6 file. This would be good enough for now. > > However, in the long term (and taking into consideration the fact that > imx6 needs to be converted to DM anyway) it may be beneficial to re-use > CCF. > > The _real_ problem is to fit it and reuse in SPL (to avoid footprint > size regression). The CCF port will work correctly in u-boot proper, > but for SPL we would need some hacks (as e.g. by default we now strip > clock properties from DTS when appending to SPL). > > Now SPLs for IMX6 use solid code without any unneeded stuff (and > hence we didn't need TPL...) . Yes, all these size issues might occur even in future if we port large things from Linux. [2] https://patchwork.ozlabs.org/patch/1019673/ [3] https://gist.github.com/apritzel/db93dd06b4defb46504bccbfe4fc2c20#file-sunxi_clk-c-L86-L112 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot