| Message ID | 20221213145708.v2.1.Ia0bc6b272f1e2e3f37873c61d79138c2663c4055@changeid |
|---|---|
| State | Accepted |
| Commit | c40251c120fac5a85fb445b6b2c7db5d18792e2a |
| Delegated to: | Sean Anderson |
| Headers | show |
| Series | [v2] cmd: clk: probe the clock before dump them | expand |
On 12/13/22 08:57, Patrick Delaunay wrote: > The clock UCLASS need to be probed to allow availability of the > private data (struct clk *), get in show_clks() with dev_get_clk_ptr() > before use them. > > Without this patch the clock dump can cause crash because all the > private data are not available before calling the API clk_get_rate(). > > It is the case for the SCMI clocks, priv->channel is needed for > scmi_clk_get_rate() and it is initialized only in scmi_clk_probe(). > This issue causes a crash for "clk dump" command on STM32MP135F-DK board > for SCMI clock not yet probed. > > Fixes: 1a725e229096 ("clk: fix clock tree dump to properly dump out every registered clock") > Signed-off-by: Patrick Delaunay <patrick.delaunay@foss.st.com> > --- > > Changes in v2: > - simplify the patch after Simon review of V1: always probe the clk device > before call to show_clks(), by using device_foreach_child_probe() and > uclass_foreach_dev_probe() > - test UCLASS_CLK only when show_clks is called for child device > > cmd/clk.c | 15 +++++---------- > 1 file changed, 5 insertions(+), 10 deletions(-) > > diff --git a/cmd/clk.c b/cmd/clk.c > index a483fd898122..ff7c7649a159 100644 > --- a/cmd/clk.c > +++ b/cmd/clk.c > @@ -22,7 +22,7 @@ static void show_clks(struct udevice *dev, int depth, int last_flag) > u32 rate; > > clkp = dev_get_clk_ptr(dev); > - if (device_get_uclass_id(dev) == UCLASS_CLK && clkp) { > + if (clkp) { > parent = clk_get_parent(clkp); > if (!IS_ERR(parent) && depth == -1) > return; > @@ -49,10 +49,11 @@ static void show_clks(struct udevice *dev, int depth, int last_flag) > printf("%s\n", dev->name); > } > > - list_for_each_entry(child, &dev->child_head, sibling_node) { > + device_foreach_child_probe(child, dev) { > + if (device_get_uclass_id(child) != UCLASS_CLK) > + continue; > if (child == dev) > continue; > - > is_last = list_is_last(&child->sibling_node, &dev->child_head); > show_clks(child, depth, (last_flag << 1) | is_last); > } > @@ -61,17 +62,11 @@ static void show_clks(struct udevice *dev, int depth, int last_flag) > int __weak soc_clk_dump(void) > { > struct udevice *dev; > - struct uclass *uc; > - int ret; > - > - ret = uclass_get(UCLASS_CLK, &uc); > - if (ret) > - return ret; > > printf(" Rate Usecnt Name\n"); > printf("------------------------------------------\n"); > > - uclass_foreach_dev(dev, uc) > + uclass_foreach_dev_probe(UCLASS_CLK, dev) > show_clks(dev, -1, 0); > > return 0; Reviewed-by: Sean Anderson <seanga2@gmail.com>
On Tue, 13 Dec 2022 14:57:10 +0100, Patrick Delaunay wrote: > The clock UCLASS need to be probed to allow availability of the > private data (struct clk *), get in show_clks() with dev_get_clk_ptr() > before use them. > > Without this patch the clock dump can cause crash because all the > private data are not available before calling the API clk_get_rate(). > > [...] Applied, thanks! [1/1] cmd: clk: probe the clock before dump them commit: dbd5ad09d2625cc631c1a98ec4517bde59f49b0d Best regards,
diff --git a/cmd/clk.c b/cmd/clk.c index a483fd898122..ff7c7649a159 100644 --- a/cmd/clk.c +++ b/cmd/clk.c @@ -22,7 +22,7 @@ static void show_clks(struct udevice *dev, int depth, int last_flag) u32 rate; clkp = dev_get_clk_ptr(dev); - if (device_get_uclass_id(dev) == UCLASS_CLK && clkp) { + if (clkp) { parent = clk_get_parent(clkp); if (!IS_ERR(parent) && depth == -1) return; @@ -49,10 +49,11 @@ static void show_clks(struct udevice *dev, int depth, int last_flag) printf("%s\n", dev->name); } - list_for_each_entry(child, &dev->child_head, sibling_node) { + device_foreach_child_probe(child, dev) { + if (device_get_uclass_id(child) != UCLASS_CLK) + continue; if (child == dev) continue; - is_last = list_is_last(&child->sibling_node, &dev->child_head); show_clks(child, depth, (last_flag << 1) | is_last); } @@ -61,17 +62,11 @@ static void show_clks(struct udevice *dev, int depth, int last_flag) int __weak soc_clk_dump(void) { struct udevice *dev; - struct uclass *uc; - int ret; - - ret = uclass_get(UCLASS_CLK, &uc); - if (ret) - return ret; printf(" Rate Usecnt Name\n"); printf("------------------------------------------\n"); - uclass_foreach_dev(dev, uc) + uclass_foreach_dev_probe(UCLASS_CLK, dev) show_clks(dev, -1, 0); return 0;
The clock UCLASS need to be probed to allow availability of the private data (struct clk *), get in show_clks() with dev_get_clk_ptr() before use them. Without this patch the clock dump can cause crash because all the private data are not available before calling the API clk_get_rate(). It is the case for the SCMI clocks, priv->channel is needed for scmi_clk_get_rate() and it is initialized only in scmi_clk_probe(). This issue causes a crash for "clk dump" command on STM32MP135F-DK board for SCMI clock not yet probed. Fixes: 1a725e229096 ("clk: fix clock tree dump to properly dump out every registered clock") Signed-off-by: Patrick Delaunay <patrick.delaunay@foss.st.com> --- Changes in v2: - simplify the patch after Simon review of V1: always probe the clk device before call to show_clks(), by using device_foreach_child_probe() and uclass_foreach_dev_probe() - test UCLASS_CLK only when show_clks is called for child device cmd/clk.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-)