Hi Peng,
On 24/12/18 05:04, Peng Fan wrote:
> Add i.MX8QM clk driver, SDHC/FEC/UART/I2C supported.
>
> Signed-off-by: Peng Fan <peng....@nxp.com>
> ---
> drivers/clk/imx/Makefile | 1 +
> drivers/clk/imx/clk-imx8.c | 1 +
> drivers/clk/imx/clk-imx8qm.c | 307
> +++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 309 insertions(+)
> create mode 100644 drivers/clk/imx/clk-imx8qm.c
>
> diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile
> index d07d91b88f..eb379c188a 100644
> --- a/drivers/clk/imx/Makefile
> +++ b/drivers/clk/imx/Makefile
> @@ -6,4 +6,5 @@ obj-$(CONFIG_CLK_IMX8) += clk-imx8.o
>
> ifdef CONFIG_CLK_IMX8
> obj-$(CONFIG_IMX8QXP) += clk-imx8qxp.o
> +obj-$(CONFIG_IMX8QM) += clk-imx8qm.o
> endif
> diff --git a/drivers/clk/imx/clk-imx8.c b/drivers/clk/imx/clk-imx8.c
> index c69a9ed867..a755e26501 100644
> --- a/drivers/clk/imx/clk-imx8.c
> +++ b/drivers/clk/imx/clk-imx8.c
> @@ -101,6 +101,7 @@ static int imx8_clk_probe(struct udevice *dev)
>
> static const struct udevice_id imx8_clk_ids[] = {
> { .compatible = "fsl,imx8qxp-clk" },
> + { .compatible = "fsl,imx8qm-clk" },
> { },
> };
>
> diff --git a/drivers/clk/imx/clk-imx8qm.c b/drivers/clk/imx/clk-imx8qm.c
> new file mode 100644
> index 0000000000..6b5561e178
> --- /dev/null
> +++ b/drivers/clk/imx/clk-imx8qm.c
> @@ -0,0 +1,307 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2018 NXP
> + * Peng Fan <peng....@nxp.com>
> + */
> +
> +#include <common.h>
> +#include <clk-uclass.h>
> +#include <dm.h>
> +#include <asm/arch/sci/sci.h>
> +#include <asm/arch/clock.h>
> +#include <dt-bindings/clock/imx8qm-clock.h>
> +#include <dt-bindings/soc/imx_rsrc.h>
> +#include <misc.h>
> +
> +#include "clk-imx8.h"
> +
> +#if CONFIG_IS_ENABLED(CMD_CLK)
> +struct imx8_clks imx8_clk_names[] = {
> + { IMX8QM_A53_DIV, "A53_DIV" },
> + { IMX8QM_UART0_CLK, "UART0" },
> + { IMX8QM_UART1_CLK, "UART1" },
> + { IMX8QM_UART2_CLK, "UART2" },
> + { IMX8QM_UART3_CLK, "UART3" },
> + { IMX8QM_SDHC0_CLK, "SDHC0" },
> + { IMX8QM_SDHC1_CLK, "SDHC1" },
> + { IMX8QM_SDHC2_CLK, "SDHC2" },
> + { IMX8QM_ENET0_AHB_CLK, "ENET0_AHB" },
> + { IMX8QM_ENET0_IPG_CLK, "ENET0_IPG" },
> + { IMX8QM_ENET0_REF_DIV, "ENET0_REF" },
> + { IMX8QM_ENET0_PTP_CLK, "ENET0_PTP" },
> + { IMX8QM_ENET1_AHB_CLK, "ENET1_AHB" },
> + { IMX8QM_ENET1_IPG_CLK, "ENET1_IPG" },
> + { IMX8QM_ENET1_REF_DIV, "ENET1_REF" },
> + { IMX8QM_ENET1_PTP_CLK, "ENET1_PTP" },
> +};
> +
> +int num_clks = ARRAY_SIZE(imx8_clk_names);
> +#endif
> +
> +ulong imx8_clk_get_rate(struct clk *clk)
> +{
> + sc_pm_clk_t pm_clk;
> + ulong rate;
> + u16 resource;
> + int ret;
> +
> + debug("%s(#%lu)\n", __func__, clk->id);
> +
> + switch (clk->id) {
> + case IMX8QM_A53_DIV:
> + resource = SC_R_A53;
> + pm_clk = SC_PM_CLK_CPU;
> + break;
> + case IMX8QM_I2C0_CLK:
> + resource = SC_R_I2C_0;
> + pm_clk = SC_PM_CLK_PER;
> + break;
> + case IMX8QM_I2C1_CLK:
> + resource = SC_R_I2C_1;
> + pm_clk = SC_PM_CLK_PER;
> + break;
> + case IMX8QM_I2C2_CLK:
> + resource = SC_R_I2C_2;
> + pm_clk = SC_PM_CLK_PER;
> + break;
> + case IMX8QM_I2C3_CLK:
> + resource = SC_R_I2C_3;
> + pm_clk = SC_PM_CLK_PER;
> + break;
> + case IMX8QM_SDHC0_IPG_CLK:
> + case IMX8QM_SDHC0_CLK:
> + case IMX8QM_SDHC0_DIV:
> + resource = SC_R_SDHC_0;
> + pm_clk = SC_PM_CLK_PER;
> + break;
> + case IMX8QM_SDHC1_IPG_CLK:
> + case IMX8QM_SDHC1_CLK:
> + case IMX8QM_SDHC1_DIV:
> + resource = SC_R_SDHC_1;
> + pm_clk = SC_PM_CLK_PER;
> + break;
> + case IMX8QM_UART0_IPG_CLK:
> + case IMX8QM_UART0_CLK:
> + resource = SC_R_UART_0;
> + pm_clk = SC_PM_CLK_PER;
> + break;
> + case IMX8QM_UART1_CLK:
> + resource = SC_R_UART_1;
> + pm_clk = SC_PM_CLK_PER;
> + break;
> + case IMX8QM_UART2_CLK:
> + resource = SC_R_UART_2;
> + pm_clk = SC_PM_CLK_PER;
> + break;
> + case IMX8QM_UART3_CLK:
> + resource = SC_R_UART_3;
> + pm_clk = SC_PM_CLK_PER;
> + break;
> + case IMX8QM_ENET0_IPG_CLK:
> + case IMX8QM_ENET0_AHB_CLK:
> + case IMX8QM_ENET0_REF_DIV:
> + case IMX8QM_ENET0_PTP_CLK:
> + resource = SC_R_ENET_0;
> + pm_clk = SC_PM_CLK_PER;
> + break;
> + case IMX8QM_ENET1_IPG_CLK:
> + case IMX8QM_ENET1_AHB_CLK:
> + case IMX8QM_ENET1_REF_DIV:
> + case IMX8QM_ENET1_PTP_CLK:
> + resource = SC_R_ENET_1;
> + pm_clk = SC_PM_CLK_PER;
> + break;
> + default:
> + if (clk->id < IMX8QM_UART0_IPG_CLK ||
> + clk->id >= IMX8QM_CLK_END) {
> + printf("%s(Invalid clk ID #%lu)\n",
> + __func__, clk->id);
> + return -EINVAL;
> + }
> + return -ENOTSUPP;
> + };
> +
> + ret = sc_pm_get_clock_rate(-1, resource, pm_clk,
> + (sc_pm_clock_rate_t *)&rate);
> + if (ret) {
> + printf("%s err %d\n", __func__, ret);
> + return ret;
> + }
> +
> + return rate;
> +}
> +
> +ulong imx8_clk_set_rate(struct clk *clk, unsigned long rate)
> +{
> + sc_pm_clk_t pm_clk;
> + u32 new_rate = rate;
> + u16 resource;
> + int ret;
> +
> + debug("%s(#%lu), rate: %lu\n", __func__, clk->id, rate);
> +
> + switch (clk->id) {
> + case IMX8QM_I2C0_CLK:
> + resource = SC_R_I2C_0;
> + pm_clk = SC_PM_CLK_PER;
> + break;
> + case IMX8QM_I2C1_CLK:
> + resource = SC_R_I2C_1;
> + pm_clk = SC_PM_CLK_PER;
> + break;
> + case IMX8QM_I2C2_CLK:
> + resource = SC_R_I2C_2;
> + pm_clk = SC_PM_CLK_PER;
> + break;
> + case IMX8QM_I2C3_CLK:
> + resource = SC_R_I2C_3;
> + pm_clk = SC_PM_CLK_PER;
> + break;
> + case IMX8QM_UART0_CLK:
> + resource = SC_R_UART_0;
> + pm_clk = SC_PM_CLK_PER;
> + break;
> + case IMX8QM_UART1_CLK:
> + resource = SC_R_UART_1;
> + pm_clk = SC_PM_CLK_PER;
> + break;
> + case IMX8QM_UART2_CLK:
> + resource = SC_R_UART_2;
> + pm_clk = SC_PM_CLK_PER;
> + break;
> + case IMX8QM_UART3_CLK:
> + resource = SC_R_UART_3;
> + pm_clk = SC_PM_CLK_PER;
> + break;
> + case IMX8QM_SDHC0_IPG_CLK:
> + case IMX8QM_SDHC0_CLK:
> + case IMX8QM_SDHC0_DIV:
> + resource = SC_R_SDHC_0;
> + pm_clk = SC_PM_CLK_PER;
> + break;
> + case IMX8QM_SDHC1_IPG_CLK:
> + case IMX8QM_SDHC1_CLK:
> + case IMX8QM_SDHC1_DIV:
> + resource = SC_R_SDHC_1;
> + pm_clk = SC_PM_CLK_PER;
> + break;
> + case IMX8QM_ENET0_IPG_CLK:
> + case IMX8QM_ENET0_AHB_CLK:
> + case IMX8QM_ENET0_REF_DIV:
> + case IMX8QM_ENET0_PTP_CLK:
> + case IMX8QM_ENET0_ROOT_DIV:
> + resource = SC_R_ENET_0;
> + pm_clk = SC_PM_CLK_PER;
> + break;
> + case IMX8QM_ENET1_IPG_CLK:
> + case IMX8QM_ENET1_AHB_CLK:
> + case IMX8QM_ENET1_REF_DIV:
> + case IMX8QM_ENET1_PTP_CLK:
> + case IMX8QM_ENET1_ROOT_DIV:
> + resource = SC_R_ENET_1;
> + pm_clk = SC_PM_CLK_PER;
> + break;
> + default:
> + if (clk->id < IMX8QM_UART0_IPG_CLK ||
> + clk->id >= IMX8QM_CLK_END) {
> + printf("%s(Invalid clk ID #%lu)\n",
> + __func__, clk->id);
> + return -EINVAL;
> + }
> + return -ENOTSUPP;
> + };
> +
> + ret = sc_pm_set_clock_rate(-1, resource, pm_clk, &new_rate);
> + if (ret) {
> + printf("%s err %d\n", __func__, ret);
> + return ret;
> + }
> +
> + return new_rate;
> +}
> +
> +int __imx8_clk_enable(struct clk *clk, bool enable)
> +{
> + sc_pm_clk_t pm_clk;
> + u16 resource;
> + int ret;
> +
> + debug("%s(#%lu)\n", __func__, clk->id);
> +
> + switch (clk->id) {
> + case IMX8QM_I2C0_CLK:
> + resource = SC_R_I2C_0;
> + pm_clk = SC_PM_CLK_PER;
> + break;
> + case IMX8QM_I2C1_CLK:
> + resource = SC_R_I2C_1;
> + pm_clk = SC_PM_CLK_PER;
> + break;
> + case IMX8QM_I2C2_CLK:
> + resource = SC_R_I2C_2;
> + pm_clk = SC_PM_CLK_PER;
> + break;
> + case IMX8QM_I2C3_CLK:
> + resource = SC_R_I2C_3;
> + pm_clk = SC_PM_CLK_PER;
> + break;
> + case IMX8QM_UART0_CLK:
> + resource = SC_R_UART_0;
> + pm_clk = SC_PM_CLK_PER;
> + break;
> + case IMX8QM_UART1_CLK:
> + resource = SC_R_UART_1;
> + pm_clk = SC_PM_CLK_PER;
> + break;
> + case IMX8QM_UART2_CLK:
> + resource = SC_R_UART_2;
> + pm_clk = SC_PM_CLK_PER;
> + break;
> + case IMX8QM_UART3_CLK:
> + resource = SC_R_UART_3;
> + pm_clk = SC_PM_CLK_PER;
> + break;
> + case IMX8QM_SDHC0_IPG_CLK:
> + case IMX8QM_SDHC0_CLK:
> + case IMX8QM_SDHC0_DIV:
> + resource = SC_R_SDHC_0;
> + pm_clk = SC_PM_CLK_PER;
> + break;
> + case IMX8QM_SDHC1_IPG_CLK:
> + case IMX8QM_SDHC1_CLK:
> + case IMX8QM_SDHC1_DIV:
> + resource = SC_R_SDHC_1;
> + pm_clk = SC_PM_CLK_PER;
> + break;
> + case IMX8QM_ENET0_IPG_CLK:
> + case IMX8QM_ENET0_AHB_CLK:
> + case IMX8QM_ENET0_REF_DIV:
> + case IMX8QM_ENET0_PTP_CLK:
> + resource = SC_R_ENET_0;
> + pm_clk = SC_PM_CLK_PER;
> + break;
> + case IMX8QM_ENET1_IPG_CLK:
> + case IMX8QM_ENET1_AHB_CLK:
> + case IMX8QM_ENET1_REF_DIV:
> + case IMX8QM_ENET1_PTP_CLK:
> + resource = SC_R_ENET_1;
> + pm_clk = SC_PM_CLK_PER;
> + break;
> + default:
> + if (clk->id < IMX8QM_UART0_IPG_CLK ||
> + clk->id >= IMX8QM_CLK_END) {
> + printf("%s(Invalid clk ID #%lu)\n",
> + __func__, clk->id);
> + return -EINVAL;
> + }
> + return -ENOTSUPP;
> + }
> +
> + ret = sc_pm_clock_enable(-1, resource, pm_clk, enable, 0);
> + if (ret) {
> + printf("%s err %d\n", __func__, ret);
> + return ret;
> + }
> +
> + return 0;
> +}
>
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 ?
Regards,
Stefano
--
=====================================================================
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de
=====================================================================
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
