| Message ID | 1432839849-1432-1-git-send-email-geert+renesas@glider.be (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On 05/28/2015 10:04 PM, Geert Uytterhoeven wrote: > Currently the con_id of the acquired clock is printed for debugging > purposes. But in several cases, the con_id is NULL, which doesn't > provide much debugging information when printed. These cases are: > - When explicitly passing a NULL con_id (which means the first clock > tied to the device, if available), > - When not using pm_clk_add(), but pm_clk_add_clk() (which takes a > "struct clk *" directly). > > Hence print the actual clock name instead of the con_id. > > As the clock name is not available with legacy clock frameworks, and a > (non-NULL) con_id is more useful than a hex address, keep printing the > con_id if the Common Clock Framework is not enabled. overall: Reviewed-by: Grygorii Strashko <grygorii.strashko@linaro.org> But, It seems case !CONFIG_COMMON_CLK is handled by vsprintf, may we can assume that printk will handle invalid input parameters values correctly: like dev_dbg(dev, "Clock %pC con_id:%s managed by runtime PM.\n", ce->clk, ce->con_id); > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- > drivers/base/power/clock_ops.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/base/power/clock_ops.c b/drivers/base/power/clock_ops.c > index 442ce010559bf531..6d56577903bd01e1 100644 > --- a/drivers/base/power/clock_ops.c > +++ b/drivers/base/power/clock_ops.c > @@ -68,7 +68,11 @@ static void pm_clk_acquire(struct device *dev, struct pm_clock_entry *ce) > } else { > clk_prepare(ce->clk); > ce->status = PCE_STATUS_ACQUIRED; > +#ifdef CONFIG_COMMON_CLK > + dev_dbg(dev, "Clock %pC managed by runtime PM.\n", ce->clk); > +#else > dev_dbg(dev, "Clock %s managed by runtime PM.\n", ce->con_id); > +#endif > } > } > >
Hi Grygorii, On Fri, May 29, 2015 at 12:40 PM, Grygorii.Strashko@linaro.org <grygorii.strashko@linaro.org> wrote: > On 05/28/2015 10:04 PM, Geert Uytterhoeven wrote: >> Currently the con_id of the acquired clock is printed for debugging >> purposes. But in several cases, the con_id is NULL, which doesn't >> provide much debugging information when printed. These cases are: >> - When explicitly passing a NULL con_id (which means the first clock >> tied to the device, if available), >> - When not using pm_clk_add(), but pm_clk_add_clk() (which takes a >> "struct clk *" directly). >> >> Hence print the actual clock name instead of the con_id. >> >> As the clock name is not available with legacy clock frameworks, and a >> (non-NULL) con_id is more useful than a hex address, keep printing the >> con_id if the Common Clock Framework is not enabled. > > overall: > Reviewed-by: Grygorii Strashko <grygorii.strashko@linaro.org> > > But, It seems case !CONFIG_COMMON_CLK is handled by vsprintf, may we can assume Yes it is, it will print the hex pointer value. > that printk will handle invalid input parameters values correctly: > > like > dev_dbg(dev, "Clock %pC con_id:%s managed by runtime PM.\n", ce->clk, ce->con_id); Thanks, I didn't think about printing both, as I was too focused on printing a single perfect value ;-) BTW, what I wanted to do was something like dev_dbg(dev, "Clock %s managed by runtime PM.\n", ce->con_id ? ce->con_id : __clk_get_name(ce->clk)); but __clk_get_name() doesn't exist if !CONFIG_COMMON_CLK. Having the con_id too, is useful if it's non-NULL, so I'll follow your suggestion. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/base/power/clock_ops.c b/drivers/base/power/clock_ops.c index 442ce010559bf531..6d56577903bd01e1 100644 --- a/drivers/base/power/clock_ops.c +++ b/drivers/base/power/clock_ops.c @@ -68,7 +68,11 @@ static void pm_clk_acquire(struct device *dev, struct pm_clock_entry *ce) } else { clk_prepare(ce->clk); ce->status = PCE_STATUS_ACQUIRED; +#ifdef CONFIG_COMMON_CLK + dev_dbg(dev, "Clock %pC managed by runtime PM.\n", ce->clk); +#else dev_dbg(dev, "Clock %s managed by runtime PM.\n", ce->con_id); +#endif } }
Currently the con_id of the acquired clock is printed for debugging purposes. But in several cases, the con_id is NULL, which doesn't provide much debugging information when printed. These cases are: - When explicitly passing a NULL con_id (which means the first clock tied to the device, if available), - When not using pm_clk_add(), but pm_clk_add_clk() (which takes a "struct clk *" directly). Hence print the actual clock name instead of the con_id. As the clock name is not available with legacy clock frameworks, and a (non-NULL) con_id is more useful than a hex address, keep printing the con_id if the Common Clock Framework is not enabled. Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> --- drivers/base/power/clock_ops.c | 4 ++++ 1 file changed, 4 insertions(+)