[PATCH v7 0/8] clk: Switch from soc_clk_dump to clk_ops function

Currently clock providers may override default implementation of soc_clk_dump function to replace clk dump command output. This causes confusing behaviour when u-boot is built with one of such drivers enabled but still has clocks defined using CCF. For example, enabling CMD_CLK and using clk dump on sandbox target will not show CCF clocks because k210 driver overrides common soc_clk_dump.
Changelog: v1 -> v2: - Add missing static to dump functions
v2 -> v3: - Make soc_clk_dump in cmd/clk.c static instead of removing __weak
v3 -> v4: - Rebase and refactor dump for new Amlogic A1 clock controller driver
v4 -> v5: - Add docs for dump() function in clk_ops - Print driver and device names before calling corresponding dump()
v5 -> v6: - dump() return type changed to void - meson_clk_dump() and helper functions moved under CONFIG_CMD_CLK to fix unused-function diagnostic
v6 -> v7: - fix return type of k210_clk_dump() - fix clk_ops dump() docs since it returns void now
Igor Prusov (8): clk: zynq: Move soc_clk_dump to Zynq clock driver clk: ast2600: Move soc_clk_dump function clk: k210: Move soc_clk_dump function clk: amlogic: Move driver and ops structs clk: Add dump operation to clk_ops cmd: clk: Use dump function from clk_ops clk: treewide: switch to clock dump from clk_ops cmd: clk: Make soc_clk_dump static
arch/arm/mach-zynq/clk.c | 57 -------------- arch/mips/mach-pic32/cpu.c | 23 ------ cmd/clk.c | 13 +++- drivers/clk/aspeed/clk_ast2600.c | 83 ++++++++++---------- drivers/clk/clk_k210.c | 104 ++++++++++++------------- drivers/clk/clk_pic32.c | 37 +++++++++ drivers/clk/clk_versal.c | 9 ++- drivers/clk/clk_zynq.c | 52 +++++++++++++ drivers/clk/clk_zynqmp.c | 22 +++--- drivers/clk/imx/clk-imx8.c | 13 +--- drivers/clk/meson/a1.c | 58 ++++++-------- drivers/clk/mvebu/armada-37xx-periph.c | 20 +++-- drivers/clk/stm32/clk-stm32mp1.c | 31 ++------ include/clk-uclass.h | 13 ++++ include/clk.h | 2 - 15 files changed, 269 insertions(+), 268 deletions(-)

Move clock dump function in preparation for switching to dump function in clk_ops.
Acked-by: Michal Simek michal.simek@amd.com Signed-off-by: Igor Prusov ivprusov@sberdevices.ru --- arch/arm/mach-zynq/clk.c | 57 --------------------------------------- drivers/clk/clk_zynq.c | 58 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 57 deletions(-)
diff --git a/arch/arm/mach-zynq/clk.c b/arch/arm/mach-zynq/clk.c index 1945f60e08..e6a67326dd 100644 --- a/arch/arm/mach-zynq/clk.c +++ b/arch/arm/mach-zynq/clk.c @@ -13,20 +13,6 @@
DECLARE_GLOBAL_DATA_PTR;
-static const char * const clk_names[clk_max] = { - "armpll", "ddrpll", "iopll", - "cpu_6or4x", "cpu_3or2x", "cpu_2x", "cpu_1x", - "ddr2x", "ddr3x", "dci", - "lqspi", "smc", "pcap", "gem0", "gem1", - "fclk0", "fclk1", "fclk2", "fclk3", "can0", "can1", - "sdio0", "sdio1", "uart0", "uart1", "spi0", "spi1", "dma", - "usb0_aper", "usb1_aper", "gem0_aper", "gem1_aper", - "sdio0_aper", "sdio1_aper", "spi0_aper", "spi1_aper", - "can0_aper", "can1_aper", "i2c0_aper", "i2c1_aper", - "uart0_aper", "uart1_aper", "gpio_aper", "lqspi_aper", - "smc_aper", "swdt", "dbg_trc", "dbg_apb" -}; - /** * set_cpu_clk_info() - Setup clock information * @@ -65,46 +51,3 @@ int set_cpu_clk_info(void)
return 0; } - -/** - * soc_clk_dump() - Print clock frequencies - * Returns zero on success - * - * Implementation for the clk dump command. - */ -int soc_clk_dump(void) -{ - struct udevice *dev; - int i, ret; - - ret = uclass_get_device_by_driver(UCLASS_CLK, - DM_DRIVER_GET(zynq_clk), &dev); - if (ret) - return ret; - - printf("clk\t\tfrequency\n"); - for (i = 0; i < clk_max; i++) { - const char *name = clk_names[i]; - if (name) { - struct clk clk; - unsigned long rate; - - clk.id = i; - ret = clk_request(dev, &clk); - if (ret < 0) - return ret; - - rate = clk_get_rate(&clk); - - clk_free(&clk); - - if ((rate == (unsigned long)-ENOSYS) || - (rate == (unsigned long)-ENXIO)) - printf("%10s%20s\n", name, "unknown"); - else - printf("%10s%20lu\n", name, rate); - } - } - - return 0; -} diff --git a/drivers/clk/clk_zynq.c b/drivers/clk/clk_zynq.c index e80500e382..be5226175f 100644 --- a/drivers/clk/clk_zynq.c +++ b/drivers/clk/clk_zynq.c @@ -454,6 +454,64 @@ static int dummy_enable(struct clk *clk) return 0; }
+static const char * const clk_names[clk_max] = { + "armpll", "ddrpll", "iopll", + "cpu_6or4x", "cpu_3or2x", "cpu_2x", "cpu_1x", + "ddr2x", "ddr3x", "dci", + "lqspi", "smc", "pcap", "gem0", "gem1", + "fclk0", "fclk1", "fclk2", "fclk3", "can0", "can1", + "sdio0", "sdio1", "uart0", "uart1", "spi0", "spi1", "dma", + "usb0_aper", "usb1_aper", "gem0_aper", "gem1_aper", + "sdio0_aper", "sdio1_aper", "spi0_aper", "spi1_aper", + "can0_aper", "can1_aper", "i2c0_aper", "i2c1_aper", + "uart0_aper", "uart1_aper", "gpio_aper", "lqspi_aper", + "smc_aper", "swdt", "dbg_trc", "dbg_apb" +}; + +/** + * soc_clk_dump() - Print clock frequencies + * Returns zero on success + * + * Implementation for the clk dump command. + */ +int soc_clk_dump(void) +{ + struct udevice *dev; + int i, ret; + + ret = uclass_get_device_by_driver(UCLASS_CLK, + DM_DRIVER_GET(zynq_clk), &dev); + if (ret) + return ret; + + printf("clk\t\tfrequency\n"); + for (i = 0; i < clk_max; i++) { + const char *name = clk_names[i]; + + if (name) { + struct clk clk; + unsigned long rate; + + clk.id = i; + ret = clk_request(dev, &clk); + if (ret < 0) + return ret; + + rate = clk_get_rate(&clk); + + clk_free(&clk); + + if ((rate == (unsigned long)-ENOSYS) || + (rate == (unsigned long)-ENXIO)) + printf("%10s%20s\n", name, "unknown"); + else + printf("%10s%20lu\n", name, rate); + } + } + + return 0; +} + static struct clk_ops zynq_clk_ops = { .get_rate = zynq_clk_get_rate, #ifndef CONFIG_SPL_BUILD

Move clock dump function to avoid forward declaration after switching to dump in clk_ops.
Signed-off-by: Igor Prusov ivprusov@sberdevices.ru --- drivers/clk/aspeed/clk_ast2600.c | 70 ++++++++++++++++---------------- 1 file changed, 35 insertions(+), 35 deletions(-)
diff --git a/drivers/clk/aspeed/clk_ast2600.c b/drivers/clk/aspeed/clk_ast2600.c index e5ada5b6d4..b3cc8392fa 100644 --- a/drivers/clk/aspeed/clk_ast2600.c +++ b/drivers/clk/aspeed/clk_ast2600.c @@ -1104,41 +1104,6 @@ static int ast2600_clk_enable(struct clk *clk) return 0; }
-struct clk_ops ast2600_clk_ops = { - .get_rate = ast2600_clk_get_rate, - .set_rate = ast2600_clk_set_rate, - .enable = ast2600_clk_enable, -}; - -static int ast2600_clk_probe(struct udevice *dev) -{ - struct ast2600_clk_priv *priv = dev_get_priv(dev); - - priv->scu = devfdt_get_addr_ptr(dev); - if (IS_ERR(priv->scu)) - return PTR_ERR(priv->scu); - - ast2600_init_rgmii_clk(priv->scu, &rgmii_clk_defconfig); - ast2600_init_rmii_clk(priv->scu, &rmii_clk_defconfig); - ast2600_configure_mac12_clk(priv->scu); - ast2600_configure_mac34_clk(priv->scu); - ast2600_configure_rsa_ecc_clk(priv->scu); - - return 0; -} - -static int ast2600_clk_bind(struct udevice *dev) -{ - int ret; - - /* The reset driver does not have a device node, so bind it here */ - ret = device_bind_driver(gd->dm_root, "ast_sysreset", "reset", &dev); - if (ret) - debug("Warning: No reset driver: ret=%d\n", ret); - - return 0; -} - struct aspeed_clks { ulong id; const char *name; @@ -1203,6 +1168,41 @@ int soc_clk_dump(void) return 0; }
+struct clk_ops ast2600_clk_ops = { + .get_rate = ast2600_clk_get_rate, + .set_rate = ast2600_clk_set_rate, + .enable = ast2600_clk_enable, +}; + +static int ast2600_clk_probe(struct udevice *dev) +{ + struct ast2600_clk_priv *priv = dev_get_priv(dev); + + priv->scu = devfdt_get_addr_ptr(dev); + if (IS_ERR(priv->scu)) + return PTR_ERR(priv->scu); + + ast2600_init_rgmii_clk(priv->scu, &rgmii_clk_defconfig); + ast2600_init_rmii_clk(priv->scu, &rmii_clk_defconfig); + ast2600_configure_mac12_clk(priv->scu); + ast2600_configure_mac34_clk(priv->scu); + ast2600_configure_rsa_ecc_clk(priv->scu); + + return 0; +} + +static int ast2600_clk_bind(struct udevice *dev) +{ + int ret; + + /* The reset driver does not have a device node, so bind it here */ + ret = device_bind_driver(gd->dm_root, "ast_sysreset", "reset", &dev); + if (ret) + debug("Warning: No reset driver: ret=%d\n", ret); + + return 0; +} + static const struct udevice_id ast2600_clk_ids[] = { { .compatible = "aspeed,ast2600-scu", }, { },

Move clock dump function to avoid forward declaration after switching to dump in clk_ops.
Reviewed-by: Sean Anderson seanga2@gmail.com Signed-off-by: Igor Prusov ivprusov@sberdevices.ru --- drivers/clk/clk_k210.c | 92 +++++++++++++++++++++--------------------- 1 file changed, 46 insertions(+), 46 deletions(-)
diff --git a/drivers/clk/clk_k210.c b/drivers/clk/clk_k210.c index c534cc07e0..2f17152021 100644 --- a/drivers/clk/clk_k210.c +++ b/drivers/clk/clk_k210.c @@ -1238,52 +1238,6 @@ static int k210_clk_request(struct clk *clk) return 0; }
-static const struct clk_ops k210_clk_ops = { - .request = k210_clk_request, - .set_rate = k210_clk_set_rate, - .get_rate = k210_clk_get_rate, - .set_parent = k210_clk_set_parent, - .enable = k210_clk_enable, - .disable = k210_clk_disable, -}; - -static int k210_clk_probe(struct udevice *dev) -{ - int ret; - struct k210_clk_priv *priv = dev_get_priv(dev); - - priv->base = dev_read_addr_ptr(dev_get_parent(dev)); - if (!priv->base) - return -EINVAL; - - ret = clk_get_by_index(dev, 0, &priv->in0); - if (ret) - return ret; - - /* - * Force setting defaults, even before relocation. This is so we can - * set the clock rate for PLL1 before we relocate into aisram. - */ - if (!(gd->flags & GD_FLG_RELOC)) - clk_set_defaults(dev, CLK_DEFAULTS_POST_FORCE); - - return 0; -} - -static const struct udevice_id k210_clk_ids[] = { - { .compatible = "canaan,k210-clk" }, - { }, -}; - -U_BOOT_DRIVER(k210_clk) = { - .name = "k210_clk", - .id = UCLASS_CLK, - .of_match = k210_clk_ids, - .ops = &k210_clk_ops, - .probe = k210_clk_probe, - .priv_auto = sizeof(struct k210_clk_priv), -}; - #if IS_ENABLED(CONFIG_CMD_CLK) static char show_enabled(struct k210_clk_priv *priv, int id) { @@ -1342,3 +1296,49 @@ int soc_clk_dump(void) return 0; } #endif + +static const struct clk_ops k210_clk_ops = { + .request = k210_clk_request, + .set_rate = k210_clk_set_rate, + .get_rate = k210_clk_get_rate, + .set_parent = k210_clk_set_parent, + .enable = k210_clk_enable, + .disable = k210_clk_disable, +}; + +static int k210_clk_probe(struct udevice *dev) +{ + int ret; + struct k210_clk_priv *priv = dev_get_priv(dev); + + priv->base = dev_read_addr_ptr(dev_get_parent(dev)); + if (!priv->base) + return -EINVAL; + + ret = clk_get_by_index(dev, 0, &priv->in0); + if (ret) + return ret; + + /* + * Force setting defaults, even before relocation. This is so we can + * set the clock rate for PLL1 before we relocate into aisram. + */ + if (!(gd->flags & GD_FLG_RELOC)) + clk_set_defaults(dev, CLK_DEFAULTS_POST_FORCE); + + return 0; +} + +static const struct udevice_id k210_clk_ids[] = { + { .compatible = "canaan,k210-clk" }, + { }, +}; + +U_BOOT_DRIVER(k210_clk) = { + .name = "k210_clk", + .id = UCLASS_CLK, + .of_match = k210_clk_ids, + .ops = &k210_clk_ops, + .probe = k210_clk_probe, + .priv_auto = sizeof(struct k210_clk_priv), +};

Move driver and ops structs to avoid forward declaration after switching to dump in clk_ops.
Reviewed-by: Neil Armstrong neil.armstrong@linaro.org Signed-off-by: Igor Prusov ivprusov@sberdevices.ru --- drivers/clk/meson/a1.c | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/drivers/clk/meson/a1.c b/drivers/clk/meson/a1.c index d0f5bb3753..c91357ec27 100644 --- a/drivers/clk/meson/a1.c +++ b/drivers/clk/meson/a1.c @@ -607,14 +607,6 @@ static int meson_clk_set_parent(struct clk *clk, struct clk *parent_clk) return meson_mux_set_parent_by_id(clk, parent_clk->id); }
-static struct clk_ops meson_clk_ops = { - .disable = meson_clk_disable, - .enable = meson_clk_enable, - .get_rate = meson_clk_get_rate, - .set_rate = meson_clk_set_rate, - .set_parent = meson_clk_set_parent, -}; - static int meson_clk_probe(struct udevice *dev) { struct meson_clk *priv = dev_get_priv(dev); @@ -644,15 +636,6 @@ static const struct udevice_id meson_clk_ids[] = { { } };
-U_BOOT_DRIVER(meson_clk) = { - .name = "meson-clk-a1", - .id = UCLASS_CLK, - .of_match = meson_clk_ids, - .priv_auto = sizeof(struct meson_clk), - .ops = &meson_clk_ops, - .probe = meson_clk_probe, -}; - static const char *meson_clk_get_name(struct clk *clk, int id) { const struct meson_clk_info *info; @@ -733,3 +716,20 @@ int soc_clk_dump(void)
return 0; } + +static struct clk_ops meson_clk_ops = { + .disable = meson_clk_disable, + .enable = meson_clk_enable, + .get_rate = meson_clk_get_rate, + .set_rate = meson_clk_set_rate, + .set_parent = meson_clk_set_parent, +}; + +U_BOOT_DRIVER(meson_clk) = { + .name = "meson-clk-a1", + .id = UCLASS_CLK, + .of_match = meson_clk_ids, + .priv_auto = sizeof(struct meson_clk), + .ops = &meson_clk_ops, + .probe = meson_clk_probe, +};

This adds dump function to struct clk_ops which should replace soc_clk_dump. It allows clock drivers to provide custom dump implementation without overriding generic CCF dump function.
Reviewed-by: Patrice Chotard patrice.chotard@foss.st.com Tested-by: Patrice Chotard patrice.chotard@foss.st.com Reviewed-by: Sean Anderson seanga2@gmail.com Signed-off-by: Igor Prusov ivprusov@sberdevices.ru --- include/clk-uclass.h | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/include/clk-uclass.h b/include/clk-uclass.h index a22f1a5d84..f10dd213ff 100644 --- a/include/clk-uclass.h +++ b/include/clk-uclass.h @@ -25,6 +25,7 @@ struct ofnode_phandle_args; * @set_parent: Set current clock parent * @enable: Enable a clock. * @disable: Disable a clock. + * @dump: Print clock information. * * The individual methods are described more fully below. */ @@ -39,6 +40,9 @@ struct clk_ops { int (*set_parent)(struct clk *clk, struct clk *parent); int (*enable)(struct clk *clk); int (*disable)(struct clk *clk); +#if IS_ENABLED(CONFIG_CMD_CLK) + void (*dump)(struct udevice *dev); +#endif };
#if 0 /* For documentation only */ @@ -135,6 +139,15 @@ int enable(struct clk *clk); * Return: zero on success, or -ve error code. */ int disable(struct clk *clk); + +/** + * dump() - Print clock information. + * @clk: The clock device to dump. + * + * If present, this function is called by "clk dump" command for each + * bound device. + */ +void dump(struct udevice *dev); #endif
#endif

On 11/9/23 05:55, Igor Prusov wrote:
This adds dump function to struct clk_ops which should replace soc_clk_dump. It allows clock drivers to provide custom dump implementation without overriding generic CCF dump function.
Reviewed-by: Patrice Chotard patrice.chotard@foss.st.com Tested-by: Patrice Chotard patrice.chotard@foss.st.com Reviewed-by: Sean Anderson seanga2@gmail.com Signed-off-by: Igor Prusov ivprusov@sberdevices.ru
include/clk-uclass.h | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/include/clk-uclass.h b/include/clk-uclass.h index a22f1a5d84..f10dd213ff 100644 --- a/include/clk-uclass.h +++ b/include/clk-uclass.h @@ -25,6 +25,7 @@ struct ofnode_phandle_args;
- @set_parent: Set current clock parent
- @enable: Enable a clock.
- @disable: Disable a clock.
*/
- @dump: Print clock information.
- The individual methods are described more fully below.
@@ -39,6 +40,9 @@ struct clk_ops { int (*set_parent)(struct clk *clk, struct clk *parent); int (*enable)(struct clk *clk); int (*disable)(struct clk *clk); +#if IS_ENABLED(CONFIG_CMD_CLK)
- void (*dump)(struct udevice *dev);
+#endif };
#if 0 /* For documentation only */ @@ -135,6 +139,15 @@ int enable(struct clk *clk);
- Return: zero on success, or -ve error code.
*/ int disable(struct clk *clk);
+/**
- dump() - Print clock information.
- @clk: The clock device to dump.
The correct member here is @dev. I fixed this when applying.
--Sean
- If present, this function is called by "clk dump" command for each
- bound device.
- */
+void dump(struct udevice *dev); #endif
#endif

Add another loop to dump additional info from clock providers that implement dump operation.
Reviewed-by: Patrice Chotard patrice.chotard@foss.st.com Tested-by: Patrice Chotard patrice.chotard@foss.st.com Reviewed-by: Sean Anderson seanga2@gmail.com Signed-off-by: Igor Prusov ivprusov@sberdevices.ru --- cmd/clk.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/cmd/clk.c b/cmd/clk.c index c7c379d7a6..4b9709d3ff 100644 --- a/cmd/clk.c +++ b/cmd/clk.c @@ -62,6 +62,7 @@ static void show_clks(struct udevice *dev, int depth, int last_flag) int __weak soc_clk_dump(void) { struct udevice *dev; + const struct clk_ops *ops;
printf(" Rate Usecnt Name\n"); printf("------------------------------------------\n"); @@ -69,6 +70,14 @@ int __weak soc_clk_dump(void) uclass_foreach_dev_probe(UCLASS_CLK, dev) show_clks(dev, -1, 0);
+ uclass_foreach_dev_probe(UCLASS_CLK, dev) { + ops = dev_get_driver_ops(dev); + if (ops && ops->dump) { + printf("\n%s %s:\n", dev->driver->name, dev->name); + ops->dump(dev); + } + } + return 0; } #else

Switch to using new dump operation in clock provider drivers instead of overriding soc_clk_dump.
Tested-by: Patrice Chotard patrice.chotard@foss.st.com Reviewed-by: Sean Anderson seanga2@gmail.com Signed-off-by: Igor Prusov ivprusov@sberdevices.ru --- arch/mips/mach-pic32/cpu.c | 23 ---------------- drivers/clk/aspeed/clk_ast2600.c | 13 +++++---- drivers/clk/clk_k210.c | 12 +++------ drivers/clk/clk_pic32.c | 37 ++++++++++++++++++++++++++ drivers/clk/clk_versal.c | 9 ++++--- drivers/clk/clk_zynq.c | 28 ++++++++----------- drivers/clk/clk_zynqmp.c | 22 +++++++-------- drivers/clk/imx/clk-imx8.c | 13 +++------ drivers/clk/meson/a1.c | 28 ++++++------------- drivers/clk/mvebu/armada-37xx-periph.c | 20 +++++++++----- drivers/clk/stm32/clk-stm32mp1.c | 31 ++++++--------------- 11 files changed, 108 insertions(+), 128 deletions(-)
diff --git a/arch/mips/mach-pic32/cpu.c b/arch/mips/mach-pic32/cpu.c index dbf8c9cd22..3181a946a2 100644 --- a/arch/mips/mach-pic32/cpu.c +++ b/arch/mips/mach-pic32/cpu.c @@ -143,26 +143,3 @@ const char *get_core_name(void) return str; } #endif -#ifdef CONFIG_CMD_CLK - -int soc_clk_dump(void) -{ - int i; - - printf("PLL Speed: %lu MHz\n", - CLK_MHZ(rate(PLLCLK))); - - printf("CPU Speed: %lu MHz\n", CLK_MHZ(rate(PB7CLK))); - - printf("MPLL Speed: %lu MHz\n", CLK_MHZ(rate(MPLL))); - - for (i = PB1CLK; i <= PB7CLK; i++) - printf("PB%d Clock Speed: %lu MHz\n", i - PB1CLK + 1, - CLK_MHZ(rate(i))); - - for (i = REF1CLK; i <= REF5CLK; i++) - printf("REFO%d Clock Speed: %lu MHz\n", i - REF1CLK + 1, - CLK_MHZ(rate(i))); - return 0; -} -#endif diff --git a/drivers/clk/aspeed/clk_ast2600.c b/drivers/clk/aspeed/clk_ast2600.c index b3cc8392fa..eecfacd7fc 100644 --- a/drivers/clk/aspeed/clk_ast2600.c +++ b/drivers/clk/aspeed/clk_ast2600.c @@ -1109,6 +1109,7 @@ struct aspeed_clks { const char *name; };
+#if IS_ENABLED(CONFIG_CMD_CLK) static struct aspeed_clks aspeed_clk_names[] = { { ASPEED_CLK_HPLL, "hpll" }, { ASPEED_CLK_MPLL, "mpll" }, @@ -1123,18 +1124,12 @@ static struct aspeed_clks aspeed_clk_names[] = { { ASPEED_CLK_HUARTX, "huxclk" }, };
-int soc_clk_dump(void) +static void ast2600_clk_dump(struct udevice *dev) { - struct udevice *dev; struct clk clk; unsigned long rate; int i, ret;
- ret = uclass_get_device_by_driver(UCLASS_CLK, DM_DRIVER_GET(aspeed_scu), - &dev); - if (ret) - return ret; - printf("Clk\t\tHz\n");
for (i = 0; i < ARRAY_SIZE(aspeed_clk_names); i++) { @@ -1167,11 +1162,15 @@ int soc_clk_dump(void)
return 0; } +#endif
struct clk_ops ast2600_clk_ops = { .get_rate = ast2600_clk_get_rate, .set_rate = ast2600_clk_set_rate, .enable = ast2600_clk_enable, +#if IS_ENABLED(CONFIG_CMD_CLK) + .dump = ast2600_clk_dump, +#endif };
static int ast2600_clk_probe(struct udevice *dev) diff --git a/drivers/clk/clk_k210.c b/drivers/clk/clk_k210.c index 2f17152021..0d266bd1aa 100644 --- a/drivers/clk/clk_k210.c +++ b/drivers/clk/clk_k210.c @@ -1276,16 +1276,10 @@ static void show_clks(struct k210_clk_priv *priv, int id, int depth) } }
-int soc_clk_dump(void) +static void k210_clk_dump(struct udevice *dev) { - int ret; - struct udevice *dev; struct k210_clk_priv *priv;
- ret = uclass_get_device_by_driver(UCLASS_CLK, DM_DRIVER_GET(k210_clk), - &dev); - if (ret) - return ret; priv = dev_get_priv(dev);
puts(" Rate Enabled Name\n"); @@ -1293,7 +1287,6 @@ int soc_clk_dump(void) printf(" %-9lu %-7c %*s%s\n", clk_get_rate(&priv->in0), 'y', 0, "", priv->in0.dev->name); show_clks(priv, K210_CLK_IN0, 1); - return 0; } #endif
@@ -1304,6 +1297,9 @@ static const struct clk_ops k210_clk_ops = { .set_parent = k210_clk_set_parent, .enable = k210_clk_enable, .disable = k210_clk_disable, +#if IS_ENABLED(CONFIG_CMD_CLK) + .dump = k210_clk_dump, +#endif };
static int k210_clk_probe(struct udevice *dev) diff --git a/drivers/clk/clk_pic32.c b/drivers/clk/clk_pic32.c index ef06a7fb9f..a77d0e7419 100644 --- a/drivers/clk/clk_pic32.c +++ b/drivers/clk/clk_pic32.c @@ -20,6 +20,8 @@
DECLARE_GLOBAL_DATA_PTR;
+#define CLK_MHZ(x) ((x) / 1000000) + /* Primary oscillator */ #define SYS_POSC_CLK_HZ 24000000
@@ -385,9 +387,44 @@ static ulong pic32_set_rate(struct clk *clk, ulong rate) return rate; }
+#if IS_ENABLED(CONFIG_CMD_CLK) +static void pic32_dump(struct udevice *dev) +{ + int i; + struct clk clk; + + clk.dev = dev; + + clk.id = PLLCLK; + printf("PLL Speed: %lu MHz\n", + CLK_MHZ(pic32_get_rate(&clk))); + + clk.id = PB7CLK; + printf("CPU Speed: %lu MHz\n", CLK_MHZ(pic32_get_rate(&clk))); + + clk.id = MPLL; + printf("MPLL Speed: %lu MHz\n", CLK_MHZ(pic32_get_rate(&clk))); + + for (i = PB1CLK; i <= PB7CLK; i++) { + clk.id = i; + printf("PB%d Clock Speed: %lu MHz\n", i - PB1CLK + 1, + CLK_MHZ(pic32_get_rate(&clk))); + } + + for (i = REF1CLK; i <= REF5CLK; i++) { + clk.id = i; + printf("REFO%d Clock Speed: %lu MHz\n", i - REF1CLK + 1, + CLK_MHZ(pic32_get_rate(&clk))); + } +} +#endif + static struct clk_ops pic32_pic32_clk_ops = { .set_rate = pic32_set_rate, .get_rate = pic32_get_rate, +#if IS_ENABLED(CONFIG_CMD_CLK) + .dump = pic32_dump, +#endif };
static int pic32_clk_probe(struct udevice *dev) diff --git a/drivers/clk/clk_versal.c b/drivers/clk/clk_versal.c index c473643603..42ab032bf7 100644 --- a/drivers/clk/clk_versal.c +++ b/drivers/clk/clk_versal.c @@ -555,7 +555,8 @@ static int versal_clock_get_rate(u32 clk_id, u64 *clk_rate) return 0; }
-int soc_clk_dump(void) +#if IS_ENABLED(CONFIG_CMD_CLK) +static void versal_clk_dump(struct udevice __always_unused *dev) { u64 clk_rate = 0; u32 type, ret, i = 0; @@ -575,9 +576,8 @@ int soc_clk_dump(void) printf("clk: %s freq:%lld\n", clock[i].clk_name, clk_rate); } - - return 0; } +#endif
static void versal_get_clock_info(void) { @@ -769,6 +769,9 @@ static struct clk_ops versal_clk_ops = { .set_rate = versal_clk_set_rate, .get_rate = versal_clk_get_rate, .enable = versal_clk_enable, +#if IS_ENABLED(CONFIG_CMD_CLK) + .dump = versal_clk_dump, +#endif };
static const struct udevice_id versal_clk_ids[] = { diff --git a/drivers/clk/clk_zynq.c b/drivers/clk/clk_zynq.c index be5226175f..34f964d72a 100644 --- a/drivers/clk/clk_zynq.c +++ b/drivers/clk/clk_zynq.c @@ -454,6 +454,7 @@ static int dummy_enable(struct clk *clk) return 0; }
+#if IS_ENABLED(CONFIG_CMD_CLK) static const char * const clk_names[clk_max] = { "armpll", "ddrpll", "iopll", "cpu_6or4x", "cpu_3or2x", "cpu_2x", "cpu_1x", @@ -468,22 +469,10 @@ static const char * const clk_names[clk_max] = { "smc_aper", "swdt", "dbg_trc", "dbg_apb" };
-/** - * soc_clk_dump() - Print clock frequencies - * Returns zero on success - * - * Implementation for the clk dump command. - */ -int soc_clk_dump(void) +static void zynq_clk_dump(struct udevice *dev) { - struct udevice *dev; int i, ret;
- ret = uclass_get_device_by_driver(UCLASS_CLK, - DM_DRIVER_GET(zynq_clk), &dev); - if (ret) - return ret; - printf("clk\t\tfrequency\n"); for (i = 0; i < clk_max; i++) { const char *name = clk_names[i]; @@ -494,8 +483,11 @@ int soc_clk_dump(void)
clk.id = i; ret = clk_request(dev, &clk); - if (ret < 0) - return ret; + if (ret < 0) { + printf("%s clk_request() failed: %d\n", + __func__, ret); + break; + }
rate = clk_get_rate(&clk);
@@ -508,9 +500,8 @@ int soc_clk_dump(void) printf("%10s%20lu\n", name, rate); } } - - return 0; } +#endif
static struct clk_ops zynq_clk_ops = { .get_rate = zynq_clk_get_rate, @@ -518,6 +509,9 @@ static struct clk_ops zynq_clk_ops = { .set_rate = zynq_clk_set_rate, #endif .enable = dummy_enable, +#if IS_ENABLED(CONFIG_CMD_CLK) + .dump = zynq_clk_dump, +#endif };
static int zynq_clk_probe(struct udevice *dev) diff --git a/drivers/clk/clk_zynqmp.c b/drivers/clk/clk_zynqmp.c index 1cfe0e25b1..a334a77b79 100644 --- a/drivers/clk/clk_zynqmp.c +++ b/drivers/clk/clk_zynqmp.c @@ -735,16 +735,11 @@ static ulong zynqmp_clk_set_rate(struct clk *clk, ulong rate) } }
-int soc_clk_dump(void) +#if IS_ENABLED(CONFIG_CMD_CLK) +static void zynqmp_clk_dump(struct udevice *dev) { - struct udevice *dev; int i, ret;
- ret = uclass_get_device_by_driver(UCLASS_CLK, - DM_DRIVER_GET(zynqmp_clk), &dev); - if (ret) - return ret; - printf("clk\t\tfrequency\n"); for (i = 0; i < clk_max; i++) { const char *name = clk_names[i]; @@ -754,8 +749,11 @@ int soc_clk_dump(void)
clk.id = i; ret = clk_request(dev, &clk); - if (ret < 0) - return ret; + if (ret < 0) { + printf("%s clk_request() failed: %d\n", + __func__, ret); + break; + }
rate = clk_get_rate(&clk);
@@ -769,9 +767,8 @@ int soc_clk_dump(void) printf("%10s%20lu\n", name, rate); } } - - return 0; } +#endif
static int zynqmp_get_freq_by_name(char *name, struct udevice *dev, ulong *freq) { @@ -871,6 +868,9 @@ static struct clk_ops zynqmp_clk_ops = { .set_rate = zynqmp_clk_set_rate, .get_rate = zynqmp_clk_get_rate, .enable = zynqmp_clk_enable, +#if IS_ENABLED(CONFIG_CMD_CLK) + .dump = zynqmp_clk_dump, +#endif };
static const struct udevice_id zynqmp_clk_ids[] = { diff --git a/drivers/clk/imx/clk-imx8.c b/drivers/clk/imx/clk-imx8.c index ceeead3434..9600672e07 100644 --- a/drivers/clk/imx/clk-imx8.c +++ b/drivers/clk/imx/clk-imx8.c @@ -43,18 +43,12 @@ static int imx8_clk_enable(struct clk *clk) }
#if IS_ENABLED(CONFIG_CMD_CLK) -int soc_clk_dump(void) +static void imx8_clk_dump(struct udevice *dev) { - struct udevice *dev; struct clk clk; unsigned long rate; int i, ret;
- ret = uclass_get_device_by_driver(UCLASS_CLK, - DM_DRIVER_GET(imx8_clk), &dev); - if (ret) - return ret; - printf("Clk\t\tHz\n");
for (i = 0; i < num_clks; i++) { @@ -84,8 +78,6 @@ int soc_clk_dump(void) printf("%s(%3lu):\t%lu\n", imx8_clk_names[i].name, imx8_clk_names[i].id, rate); } - - return 0; } #endif
@@ -94,6 +86,9 @@ static struct clk_ops imx8_clk_ops = { .get_rate = imx8_clk_get_rate, .enable = imx8_clk_enable, .disable = imx8_clk_disable, +#if IS_ENABLED(CONFIG_CMD_CLK) + .dump = imx8_clk_dump, +#endif };
static int imx8_clk_probe(struct udevice *dev) diff --git a/drivers/clk/meson/a1.c b/drivers/clk/meson/a1.c index c91357ec27..5220a337a8 100644 --- a/drivers/clk/meson/a1.c +++ b/drivers/clk/meson/a1.c @@ -636,6 +636,7 @@ static const struct udevice_id meson_clk_ids[] = { { } };
+#if IS_ENABLED(CONFIG_CMD_CLK) static const char *meson_clk_get_name(struct clk *clk, int id) { const struct meson_clk_info *info; @@ -645,7 +646,7 @@ static const char *meson_clk_get_name(struct clk *clk, int id) return IS_ERR(info) ? "unknown" : info->name; }
-static int meson_clk_dump(struct clk *clk) +static int meson_clk_dump_single(struct clk *clk) { const struct meson_clk_info *info; struct meson_clk *priv; @@ -680,7 +681,7 @@ static int meson_clk_dump(struct clk *clk) return 0; }
-static int meson_clk_dump_dev(struct udevice *dev) +static void meson_clk_dump(struct udevice *dev) { int i; struct meson_clk_data *data; @@ -693,29 +694,13 @@ static int meson_clk_dump_dev(struct udevice *dev)
data = (struct meson_clk_data *)dev_get_driver_data(dev); for (i = 0; i < data->num_clocks; i++) { - meson_clk_dump(&(struct clk){ + meson_clk_dump_single(&(struct clk){ .dev = dev, .id = i }); } - - return 0; -} - -int soc_clk_dump(void) -{ - struct udevice *dev; - int i = 0; - - while (!uclass_get_device(UCLASS_CLK, i++, &dev)) { - if (dev->driver == DM_DRIVER_GET(meson_clk)) { - meson_clk_dump_dev(dev); - printf("\n"); - } - } - - return 0; } +#endif
static struct clk_ops meson_clk_ops = { .disable = meson_clk_disable, @@ -723,6 +708,9 @@ static struct clk_ops meson_clk_ops = { .get_rate = meson_clk_get_rate, .set_rate = meson_clk_set_rate, .set_parent = meson_clk_set_parent, +#if IS_ENABLED(CONFIG_CMD_CLK) + .dump = meson_clk_dump, +#endif };
U_BOOT_DRIVER(meson_clk) = { diff --git a/drivers/clk/mvebu/armada-37xx-periph.c b/drivers/clk/mvebu/armada-37xx-periph.c index e75052f383..1a7097029a 100644 --- a/drivers/clk/mvebu/armada-37xx-periph.c +++ b/drivers/clk/mvebu/armada-37xx-periph.c @@ -488,33 +488,36 @@ static int armada_37xx_periph_clk_dump(struct udevice *dev) static int clk_dump(const char *name, int (*func)(struct udevice *)) { struct udevice *dev; + int ret;
if (uclass_get_device_by_name(UCLASS_CLK, name, &dev)) { printf("Cannot find device %s\n", name); return -ENODEV; }
- return func(dev); + ret = func(dev); + if (ret) + printf("Dump failed for %s: %d\n", name, ret); + + return ret; }
int armada_37xx_tbg_clk_dump(struct udevice *);
-int soc_clk_dump(void) +static void armada37xx_clk_dump(struct udevice __always_unused *dev) { printf(" xtal at %u000000 Hz\n\n", get_ref_clk());
if (clk_dump("tbg@13200", armada_37xx_tbg_clk_dump)) - return 1; + return;
if (clk_dump("nb-periph-clk@13000", armada_37xx_periph_clk_dump)) - return 1; + return;
if (clk_dump("sb-periph-clk@18000", armada_37xx_periph_clk_dump)) - return 1; - - return 0; + return; } #endif
@@ -605,6 +608,9 @@ static const struct clk_ops armada_37xx_periph_clk_ops = { .set_parent = armada_37xx_periph_clk_set_parent, .enable = armada_37xx_periph_clk_enable, .disable = armada_37xx_periph_clk_disable, +#if IS_ENABLED(CONFIG_CMD_CLK) + .dump = armada37xx_clk_dump, +#endif };
static const struct udevice_id armada_37xx_periph_clk_ids[] = { diff --git a/drivers/clk/stm32/clk-stm32mp1.c b/drivers/clk/stm32/clk-stm32mp1.c index f3ac8c7583..6f000c8e44 100644 --- a/drivers/clk/stm32/clk-stm32mp1.c +++ b/drivers/clk/stm32/clk-stm32mp1.c @@ -2225,10 +2225,13 @@ static void stm32mp1_osc_init(struct udevice *dev) } }
-static void __maybe_unused stm32mp1_clk_dump(struct stm32mp1_clk_priv *priv) +static void __maybe_unused stm32mp1_clk_dump(struct udevice *dev) { char buf[32]; int i, s, p; + struct stm32mp1_clk_priv *priv; + + priv = dev_get_priv(dev);
printf("Clocks:\n"); for (i = 0; i < _PARENT_NB; i++) { @@ -2252,27 +2255,6 @@ static void __maybe_unused stm32mp1_clk_dump(struct stm32mp1_clk_priv *priv) } }
-#ifdef CONFIG_CMD_CLK -int soc_clk_dump(void) -{ - struct udevice *dev; - struct stm32mp1_clk_priv *priv; - int ret; - - ret = uclass_get_device_by_driver(UCLASS_CLK, - DM_DRIVER_GET(stm32mp1_clock), - &dev); - if (ret) - return ret; - - priv = dev_get_priv(dev); - - stm32mp1_clk_dump(priv); - - return 0; -} -#endif - static int stm32mp1_clk_probe(struct udevice *dev) { int result = 0; @@ -2302,7 +2284,7 @@ static int stm32mp1_clk_probe(struct udevice *dev) #if defined(VERBOSE_DEBUG) /* display debug information for probe after relocation */ if (gd->flags & GD_FLG_RELOC) - stm32mp1_clk_dump(priv); + stm32mp1_clk_dump(dev); #endif
gd->cpu_clk = stm32mp1_clk_get(priv, _CK_MPU); @@ -2333,6 +2315,9 @@ static const struct clk_ops stm32mp1_clk_ops = { .disable = stm32mp1_clk_disable, .get_rate = stm32mp1_clk_get_rate, .set_rate = stm32mp1_clk_set_rate, +#if IS_ENABLED(CONFIG_CMD_CLK) && !IS_ENABLED(CONFIG_SPL_BUILD) + .dump = stm32mp1_clk_dump, +#endif };
U_BOOT_DRIVER(stm32mp1_clock) = {

After introducing dump to clk_ops there is no need to override or expose this symbol anymore.
Reviewed-by: Patrice Chotard patrice.chotard@foss.st.com Tested-by: Patrice Chotard patrice.chotard@foss.st.com Reviewed-by: Sean Anderson seanga2@gmail.com Signed-off-by: Igor Prusov ivprusov@sberdevices.ru --- cmd/clk.c | 4 ++-- include/clk.h | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/cmd/clk.c b/cmd/clk.c index 4b9709d3ff..7bbcbfeda3 100644 --- a/cmd/clk.c +++ b/cmd/clk.c @@ -59,7 +59,7 @@ static void show_clks(struct udevice *dev, int depth, int last_flag) } }
-int __weak soc_clk_dump(void) +static int soc_clk_dump(void) { struct udevice *dev; const struct clk_ops *ops; @@ -81,7 +81,7 @@ int __weak soc_clk_dump(void) return 0; } #else -int __weak soc_clk_dump(void) +static int soc_clk_dump(void) { puts("Not implemented\n"); return 1; diff --git a/include/clk.h b/include/clk.h index 249c0e0ab4..3d6394477b 100644 --- a/include/clk.h +++ b/include/clk.h @@ -676,8 +676,6 @@ static inline bool clk_valid(struct clk *clk) return clk && !!clk->dev; }
-int soc_clk_dump(void); - #endif
#define clk_prepare_enable(clk) clk_enable(clk)

Hi Igor, Sean,
On 09/11/2023 11:55, Igor Prusov wrote:
Currently clock providers may override default implementation of soc_clk_dump function to replace clk dump command output. This causes confusing behaviour when u-boot is built with one of such drivers enabled but still has clocks defined using CCF. For example, enabling CMD_CLK and using clk dump on sandbox target will not show CCF clocks because k210 driver overrides common soc_clk_dump.
What's the state of this serie ?
Thanks, Neil
Changelog: v1 -> v2:
- Add missing static to dump functions
v2 -> v3:
- Make soc_clk_dump in cmd/clk.c static instead of removing __weak
v3 -> v4:
- Rebase and refactor dump for new Amlogic A1 clock controller driver
v4 -> v5:
- Add docs for dump() function in clk_ops
- Print driver and device names before calling corresponding dump()
v5 -> v6:
- dump() return type changed to void
- meson_clk_dump() and helper functions moved under CONFIG_CMD_CLK to fix unused-function diagnostic
v6 -> v7:
- fix return type of k210_clk_dump()
- fix clk_ops dump() docs since it returns void now
Igor Prusov (8): clk: zynq: Move soc_clk_dump to Zynq clock driver clk: ast2600: Move soc_clk_dump function clk: k210: Move soc_clk_dump function clk: amlogic: Move driver and ops structs clk: Add dump operation to clk_ops cmd: clk: Use dump function from clk_ops clk: treewide: switch to clock dump from clk_ops cmd: clk: Make soc_clk_dump static
arch/arm/mach-zynq/clk.c | 57 -------------- arch/mips/mach-pic32/cpu.c | 23 ------ cmd/clk.c | 13 +++- drivers/clk/aspeed/clk_ast2600.c | 83 ++++++++++---------- drivers/clk/clk_k210.c | 104 ++++++++++++------------- drivers/clk/clk_pic32.c | 37 +++++++++ drivers/clk/clk_versal.c | 9 ++- drivers/clk/clk_zynq.c | 52 +++++++++++++ drivers/clk/clk_zynqmp.c | 22 +++--- drivers/clk/imx/clk-imx8.c | 13 +--- drivers/clk/meson/a1.c | 58 ++++++-------- drivers/clk/mvebu/armada-37xx-periph.c | 20 +++-- drivers/clk/stm32/clk-stm32mp1.c | 31 ++------ include/clk-uclass.h | 13 ++++ include/clk.h | 2 - 15 files changed, 269 insertions(+), 268 deletions(-)

On 12/12/23 09:03, Neil Armstrong wrote:
Hi Igor, Sean,
On 09/11/2023 11:55, Igor Prusov wrote:
Currently clock providers may override default implementation of soc_clk_dump function to replace clk dump command output. This causes confusing behaviour when u-boot is built with one of such drivers enabled but still has clocks defined using CCF. For example, enabling CMD_CLK and using clk dump on sandbox target will not show CCF clocks because k210 driver overrides common soc_clk_dump.
What's the state of this serie ?
Awaiting upstream. I will make a PR with it for next.
--Sean
Thanks, Neil
Changelog: v1 -> v2: - Add missing static to dump functions
v2 -> v3: - Make soc_clk_dump in cmd/clk.c static instead of removing __weak
v3 -> v4: - Rebase and refactor dump for new Amlogic A1 clock controller driver
v4 -> v5: - Add docs for dump() function in clk_ops - Print driver and device names before calling corresponding dump()
v5 -> v6: - dump() return type changed to void - meson_clk_dump() and helper functions moved under CONFIG_CMD_CLK to fix unused-function diagnostic
v6 -> v7: - fix return type of k210_clk_dump() - fix clk_ops dump() docs since it returns void now
Igor Prusov (8): clk: zynq: Move soc_clk_dump to Zynq clock driver clk: ast2600: Move soc_clk_dump function clk: k210: Move soc_clk_dump function clk: amlogic: Move driver and ops structs clk: Add dump operation to clk_ops cmd: clk: Use dump function from clk_ops clk: treewide: switch to clock dump from clk_ops cmd: clk: Make soc_clk_dump static
arch/arm/mach-zynq/clk.c | 57 -------------- arch/mips/mach-pic32/cpu.c | 23 ------ cmd/clk.c | 13 +++- drivers/clk/aspeed/clk_ast2600.c | 83 ++++++++++---------- drivers/clk/clk_k210.c | 104 ++++++++++++------------- drivers/clk/clk_pic32.c | 37 +++++++++ drivers/clk/clk_versal.c | 9 ++- drivers/clk/clk_zynq.c | 52 +++++++++++++ drivers/clk/clk_zynqmp.c | 22 +++--- drivers/clk/imx/clk-imx8.c | 13 +--- drivers/clk/meson/a1.c | 58 ++++++-------- drivers/clk/mvebu/armada-37xx-periph.c | 20 +++-- drivers/clk/stm32/clk-stm32mp1.c | 31 ++------ include/clk-uclass.h | 13 ++++ include/clk.h | 2 - 15 files changed, 269 insertions(+), 268 deletions(-)

On Thu, 9 Nov 2023 13:55:08 +0300, Igor Prusov wrote:
Currently clock providers may override default implementation of soc_clk_dump function to replace clk dump command output. This causes confusing behaviour when u-boot is built with one of such drivers enabled but still has clocks defined using CCF. For example, enabling CMD_CLK and using clk dump on sandbox target will not show CCF clocks because k210 driver overrides common soc_clk_dump.
[...]
Applied, thanks!
[1/8] clk: zynq: Move soc_clk_dump to Zynq clock driver https://source.denx.de/u-boot/custodians/u-boot-clk/-/commit/bdac75511411 [2/8] clk: ast2600: Move soc_clk_dump function https://source.denx.de/u-boot/custodians/u-boot-clk/-/commit/3f192541df79 [3/8] clk: k210: Move soc_clk_dump function https://source.denx.de/u-boot/custodians/u-boot-clk/-/commit/77beaad3d1d2 [4/8] clk: amlogic: Move driver and ops structs https://source.denx.de/u-boot/custodians/u-boot-clk/-/commit/4f48202ba82e [5/8] clk: Add dump operation to clk_ops https://source.denx.de/u-boot/custodians/u-boot-clk/-/commit/505ef5f627d8 [6/8] cmd: clk: Use dump function from clk_ops https://source.denx.de/u-boot/custodians/u-boot-clk/-/commit/258c1002383e [7/8] clk: treewide: switch to clock dump from clk_ops https://source.denx.de/u-boot/custodians/u-boot-clk/-/commit/bc3e313ff6af [8/8] cmd: clk: Make soc_clk_dump static https://source.denx.de/u-boot/custodians/u-boot-clk/-/commit/5666558a6cb0
Best regards,
participants (3)
-
Igor Prusov
-
Neil Armstrong
-
Sean Anderson