Re: [PATCH v4 6/8] cmd: clk: Use dump function from clk_ops

[Sean Anderson <seanga2@gmail.com>]

On 10/17/23 12:56, Igor Prusov wrote:
> Add another loop to dump additional info from clock providers that
> implement dump operation.
> 
> Signed-off-by: Igor Prusov <ivprusov@sberdevices.ru>
> Reviewed-by: Patrice Chotard <patrice.chotard@foss.st.com>
> Tested-by: Patrice Chotard <patrice.chotard@foss.st.com>
> ---
>   cmd/clk.c | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/cmd/clk.c b/cmd/clk.c
> index c7c379d7a6..90cc6fa906 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");
> +			ops->dump(dev);
> +		}
> +	}
> +
>   	return 0;
>   }
>   #else

So this produces output like

=> clk dump
  Rate               Usecnt      Name
------------------------------------------
  26000000             0        |-- osc
--------------------------
  Rate      Enabled Name
------------------------
  26000000  y       osc
  780000000 y           pll0
  390000000 -               aclk
  390000000 y                   cpu
  390000000 y                   sram0
  390000000 y                   sram1
  7800000   -                   clint
  195000000 y                   apb0
  195000000 y                       gpio
  195000000 y                       uart1
  195000000 y                       uart2
  195000000 y                       uart3
  195000000 y                       fpioa
  195000000 y                       sha
  195000000 y                   apb1
  195000000 y                       aes
  195000000 y                       otp
  195000000 y                   apb2
  195000000 y                   rom
  390000000 y                   dma
  390000000 y                   dvp
  390000000 y                   fft
  390000000 y               spi0
  390000000 y               spi1
  390000000 y               spi2
  97500000  y               spi3
  390000000 y               i2c0
  390000000 y               i2c1
  390000000 y               i2c2
  390000000 y               timer0
  390000000 y               timer1
  390000000 y               timer2
  390000000 y           pll1
  390000000 y               ai
  0         n           pll2
  0         y               i2s0
  0         y               i2s1
  0         y               i2s2
  0         -               i2s0_m
  0         -               i2s1_m
  0         -               i2s2_m
  13000000  y           wdt0
  13000000  n           wdt1
  26000000  n           rtc

And TBH I don't think it's particularly clear (at least at a glance) where
one clock ends and another begins. I think something like

diff --git i/cmd/clk.c w/cmd/clk.c
index f55911db7a3..7bbcbfeda33 100644
--- i/cmd/clk.c
+++ w/cmd/clk.c
@@ -73,7 +73,7 @@ static int soc_clk_dump(void)
         uclass_foreach_dev_probe(UCLASS_CLK, dev) {
                 ops = dev_get_driver_ops(dev);
                 if (ops && ops->dump) {
-                       printf("--------------------------\n");
+                       printf("\n%s %s:\n", dev->driver->name, dev->name);
                         ops->dump(dev);
                 }
         }

would work a lot better. This produces an output like

=> clk dump
  Rate               Usecnt      Name
------------------------------------------
  26000000             0        |-- osc

k210_clk clock-controller:
  Rate      Enabled Name
------------------------
  26000000  y       osc
  780000000 y           pll0
  390000000 -               aclk
  390000000 y                   cpu
  390000000 y                   sram0
  390000000 y                   sram1
</snip>

which I think makes it clearer that we have a new clock tree getting dumped.

This also doesn't really address multiple interacting clock trees (such as
e.g. if I had another clock derived from a k210_clk in the above example)
but at least it's a start.

--Sean