Hi Fabio,
On 28/01/19 15:23, Fabio Estevam wrote:
Hi Stefano,
On Mon, Jan 28, 2019 at 12:04 PM Stefano Babic sbabic@denx.de wrote:
Ok - I see you decide to split the code on depend of the SOC type. Nevertheless, even if defines are different, it seems to me that there is still a lot of common code (_imx8_clk_enable is very similar). Do we need really a separate file for each MX8 variant ? Which are the blocking points that avoid to do this ? I am looking at the patch, but I do not find such as issue. Can you explain it ?
I am also concerned about the current i.MX clock driver status in U-Boot and I expressed this during my review of Lukasz's patch that added a imx6 clock driver.
Nice we are sharing the sa me opinion. I am currently checking all patches from oldest to neweset, I have not yet catched Lukasz', but they are on my sight, too.
It does not scale well as we can see by the huge switch statement in this patch. Of course this would need to grow when new peripherals clocks are added.
I fully agree. Yes, it is simple now to do in this way, but it does not scale very well.
I think a better approach would be to use the same scheme as done in the kernel with common clock framework.
Barebox folows this same approach. For example:
i.MX8M clock driver: https://git.pengutronix.de/cgit/barebox/tree/drivers/clk/imx/clk-imx8mq.c
i.MX6Q clock driver: https://git.pengutronix.de/cgit/barebox/tree/drivers/clk/imx/clk-imx6.c
It allows syncing with the kernel clock driver more easily
This is also a great benefit.
and it makes it easier to maintain several i.MX SoCs in the long term.
Best regards, Stefano