-----Original Message----- From: Fabio Estevam [mailto:festevam@gmail.com] Sent: 2019年1月28日 22:24 To: Stefano Babic sbabic@denx.de Cc: Peng Fan peng.fan@nxp.com; Fabio Estevam fabio.estevam@nxp.com; u-boot@lists.denx.de; dl-uboot-imx uboot-imx@nxp.com; Lukasz Majewski lukma@denx.de Subject: Re: [U-Boot] [PATCH 07/10] clk: imx8: add i.MX8QM clk driver
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.
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 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://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.p engutronix.de%2Fcgit%2Fbarebox%2Ftree%2Fdrivers%2Fclk%2Fimx%2Fclk-i mx8mq.c&data=02%7C01%7Cpeng.fan%40nxp.com%7C124a7ae228794 2551ea208d6852c3c0a%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C 0%7C636842822364000869&sdata=6xwmlVrr2MGKkRCNUFZTBcec9KP RNoID%2FBWTU5HFKcg%3D&reserved=0
i.MX6Q clock driver: https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.p engutronix.de%2Fcgit%2Fbarebox%2Ftree%2Fdrivers%2Fclk%2Fimx%2Fclk-i mx6.c&data=02%7C01%7Cpeng.fan%40nxp.com%7C124a7ae22879425 51ea208d6852c3c0a%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0% 7C636842822364000869&sdata=m6%2FRa8HY9M1aPXy45%2Bp0aa9Xit 6cMUVqBw%2F4UVTSOz0%3D&reserved=0
It allows syncing with the kernel clock driver more easily and it makes it easier to maintain several i.MX SoCs in the long term.
I thought about this before, but this will increase SPL code side a lot when SPL DM clk used.
Regards, Peng.