* [PATCH v2] clk: Show clock rate and error value when fail to set clock rate @ 2015-04-20 11:08 Chanwoo Choi 2015-04-21 8:37 ` Sylwester Nawrocki 0 siblings, 1 reply; 6+ messages in thread From: Chanwoo Choi @ 2015-04-20 11:08 UTC (permalink / raw) To: mturquette, sboyd, s.nawrocki; +Cc: linux-kernel, Chanwoo Choi This patch shows the current clock rate nad error value when clk_set_rate() fail to set the clock rate because log messgae means only the clock rate. Cc: Mike Turquette <mturquette@linaro.org> Cc: Stephen Boyd <sboyd@codeaurora.org> Cc: Sylwester Nawrocki <s.nawrocki@samsung.com> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com> --- Changes from v1: - Add error value of clk-set_rate() with current clock rate drivers/clk/clk-conf.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/clk/clk-conf.c b/drivers/clk/clk-conf.c index aad4796..2b6375f 100644 --- a/drivers/clk/clk-conf.c +++ b/drivers/clk/clk-conf.c @@ -107,8 +107,9 @@ static int __set_clk_rates(struct device_node *node, bool clk_supplier) rc = clk_set_rate(clk, rate); if (rc < 0) - pr_err("clk: couldn't set %s clock rate: %d\n", - __clk_get_name(clk), rc); + pr_err("clk: couldn't set %s rate to %ld (%d)\n", + __clk_get_name(clk), clk_get_rate(clk), + rc); clk_put(clk); } index++; -- 1.8.5.5 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] clk: Show clock rate and error value when fail to set clock rate 2015-04-20 11:08 [PATCH v2] clk: Show clock rate and error value when fail to set clock rate Chanwoo Choi @ 2015-04-21 8:37 ` Sylwester Nawrocki 2015-04-27 8:46 ` Chanwoo Choi 0 siblings, 1 reply; 6+ messages in thread From: Sylwester Nawrocki @ 2015-04-21 8:37 UTC (permalink / raw) To: Chanwoo Choi; +Cc: mturquette, sboyd, linux-kernel Hi Chanwoo, On 20/04/15 13:08, Chanwoo Choi wrote: > This patch shows the current clock rate nad error value when clk_set_rate() > fail to set the clock rate because log messgae means only the clock rate. > > Cc: Mike Turquette <mturquette@linaro.org> > Cc: Stephen Boyd <sboyd@codeaurora.org> > Cc: Sylwester Nawrocki <s.nawrocki@samsung.com> > Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com> > --- > Changes from v1: > - Add error value of clk-set_rate() with current clock rate > > drivers/clk/clk-conf.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/clk/clk-conf.c b/drivers/clk/clk-conf.c > index aad4796..2b6375f 100644 > --- a/drivers/clk/clk-conf.c > +++ b/drivers/clk/clk-conf.c > @@ -107,8 +107,9 @@ static int __set_clk_rates(struct device_node *node, bool clk_supplier) > > rc = clk_set_rate(clk, rate); > if (rc < 0) > - pr_err("clk: couldn't set %s clock rate: %d\n", > - __clk_get_name(clk), rc); > + pr_err("clk: couldn't set %s rate to %ld (%d)\n", > + __clk_get_name(clk), clk_get_rate(clk), > + rc); Apologies for my previous misleading comment, with this patch you log current clock rate as the rate which was attempted to be set. And these two will likely be different when clk_set_rate() failed. So it should rather be: pr_err("clk: couldn't set %s clk rate to %ld (%d)\n", __clk_get_name(clk), rate, rc); If you really need to log current rate, then it could be changed to: pr_err("clk: couldn't set %s clk rate to %ld (%d), current rate: %ld\n", __clk_get_name(clk), rate, rc, clk_get_rate(clk)); But I doubt we need to log all these parameters. The clock rate changes could be logged with the ftrace functionality added recently to the clk core. > clk_put(clk); > } > index++; -- Regards, Sylwester ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] clk: Show clock rate and error value when fail to set clock rate 2015-04-21 8:37 ` Sylwester Nawrocki @ 2015-04-27 8:46 ` Chanwoo Choi 2015-04-27 8:51 ` Sylwester Nawrocki 0 siblings, 1 reply; 6+ messages in thread From: Chanwoo Choi @ 2015-04-27 8:46 UTC (permalink / raw) To: Sylwester Nawrocki; +Cc: mturquette, sboyd, linux-kernel Hi Sylwester, On 04/21/2015 05:37 PM, Sylwester Nawrocki wrote: > Hi Chanwoo, > > On 20/04/15 13:08, Chanwoo Choi wrote: >> This patch shows the current clock rate nad error value when clk_set_rate() >> fail to set the clock rate because log messgae means only the clock rate. >> >> Cc: Mike Turquette <mturquette@linaro.org> >> Cc: Stephen Boyd <sboyd@codeaurora.org> >> Cc: Sylwester Nawrocki <s.nawrocki@samsung.com> >> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com> >> --- >> Changes from v1: >> - Add error value of clk-set_rate() with current clock rate >> >> drivers/clk/clk-conf.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/clk/clk-conf.c b/drivers/clk/clk-conf.c >> index aad4796..2b6375f 100644 >> --- a/drivers/clk/clk-conf.c >> +++ b/drivers/clk/clk-conf.c >> @@ -107,8 +107,9 @@ static int __set_clk_rates(struct device_node *node, bool clk_supplier) >> >> rc = clk_set_rate(clk, rate); >> if (rc < 0) >> - pr_err("clk: couldn't set %s clock rate: %d\n", >> - __clk_get_name(clk), rc); >> + pr_err("clk: couldn't set %s rate to %ld (%d)\n", >> + __clk_get_name(clk), clk_get_rate(clk), >> + rc); > > Apologies for my previous misleading comment, with this patch > you log current clock rate as the rate which was attempted to > be set. And these two will likely be different when clk_set_rate() > failed. So it should rather be: > > pr_err("clk: couldn't set %s clk rate to %ld (%d)\n", > __clk_get_name(clk), rate, rc); I'll modify it as following without error value. If I add the error value, the number of character are over 80. pr_err("clk: couldn't set %s clk rate to %ld\n", __clk_get_name(clk), rate); Thanks, Chanwoo Choi ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] clk: Show clock rate and error value when fail to set clock rate 2015-04-27 8:46 ` Chanwoo Choi @ 2015-04-27 8:51 ` Sylwester Nawrocki 2015-04-27 9:03 ` Chanwoo Choi 0 siblings, 1 reply; 6+ messages in thread From: Sylwester Nawrocki @ 2015-04-27 8:51 UTC (permalink / raw) To: Chanwoo Choi; +Cc: mturquette, sboyd, linux-kernel Hi Chanwoo, On 27/04/15 10:46, Chanwoo Choi wrote: >>> diff --git a/drivers/clk/clk-conf.c b/drivers/clk/clk-conf.c >>> >> index aad4796..2b6375f 100644 >>> >> --- a/drivers/clk/clk-conf.c >>> >> +++ b/drivers/clk/clk-conf.c >>> >> @@ -107,8 +107,9 @@ static int __set_clk_rates(struct device_node *node, bool clk_supplier) >>> >> >>> >> rc = clk_set_rate(clk, rate); >>> >> if (rc < 0) >>> >> - pr_err("clk: couldn't set %s clock rate: %d\n", >>> >> - __clk_get_name(clk), rc); >>> >> + pr_err("clk: couldn't set %s rate to %ld (%d)\n", >>> >> + __clk_get_name(clk), clk_get_rate(clk), >>> >> + rc); >> > >> > Apologies for my previous misleading comment, with this patch >> > you log current clock rate as the rate which was attempted to >> > be set. And these two will likely be different when clk_set_rate() >> > failed. So it should rather be: >> > >> > pr_err("clk: couldn't set %s clk rate to %ld (%d)\n", >> > __clk_get_name(clk), rate, rc); > > I'll modify it as following without error value. > If I add the error value, the number of character are over 80. > pr_err("clk: couldn't set %s clk rate to %ld\n", > __clk_get_name(clk), rate); I wouldn't worry about line length over 80 characters, it not a hard rule. Let's leave the error code as it might be useful for debugging. -- Regards, Sylwester ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] clk: Show clock rate and error value when fail to set clock rate 2015-04-27 8:51 ` Sylwester Nawrocki @ 2015-04-27 9:03 ` Chanwoo Choi 2015-04-27 9:06 ` Chanwoo Choi 0 siblings, 1 reply; 6+ messages in thread From: Chanwoo Choi @ 2015-04-27 9:03 UTC (permalink / raw) To: Sylwester Nawrocki; +Cc: mturquette, sboyd, linux-kernel Hi Sylwester, On 04/27/2015 05:51 PM, Sylwester Nawrocki wrote: > Hi Chanwoo, > > On 27/04/15 10:46, Chanwoo Choi wrote: >>>> diff --git a/drivers/clk/clk-conf.c b/drivers/clk/clk-conf.c >>>>>> index aad4796..2b6375f 100644 >>>>>> --- a/drivers/clk/clk-conf.c >>>>>> +++ b/drivers/clk/clk-conf.c >>>>>> @@ -107,8 +107,9 @@ static int __set_clk_rates(struct device_node *node, bool clk_supplier) >>>>>> >>>>>> rc = clk_set_rate(clk, rate); >>>>>> if (rc < 0) >>>>>> - pr_err("clk: couldn't set %s clock rate: %d\n", >>>>>> - __clk_get_name(clk), rc); >>>>>> + pr_err("clk: couldn't set %s rate to %ld (%d)\n", >>>>>> + __clk_get_name(clk), clk_get_rate(clk), >>>>>> + rc); >>>> >>>> Apologies for my previous misleading comment, with this patch >>>> you log current clock rate as the rate which was attempted to >>>> be set. And these two will likely be different when clk_set_rate() >>>> failed. So it should rather be: >>>> >>>> pr_err("clk: couldn't set %s clk rate to %ld (%d)\n", >>>> __clk_get_name(clk), rate, rc); >> >> I'll modify it as following without error value. >> If I add the error value, the number of character are over 80. >> pr_err("clk: couldn't set %s clk rate to %ld\n", >> __clk_get_name(clk), rate); > > I wouldn't worry about line length over 80 characters, it not a hard rule. > Let's leave the error code as it might be useful for debugging. > OK, I'll add error value again. Thanks, Chanwoo Choi ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] clk: Show clock rate and error value when fail to set clock rate 2015-04-27 9:03 ` Chanwoo Choi @ 2015-04-27 9:06 ` Chanwoo Choi 0 siblings, 0 replies; 6+ messages in thread From: Chanwoo Choi @ 2015-04-27 9:06 UTC (permalink / raw) To: Sylwester Nawrocki; +Cc: mturquette, sboyd, linux-kernel On 04/27/2015 06:03 PM, Chanwoo Choi wrote: > Hi Sylwester, > > On 04/27/2015 05:51 PM, Sylwester Nawrocki wrote: >> Hi Chanwoo, >> >> On 27/04/15 10:46, Chanwoo Choi wrote: >>>>> diff --git a/drivers/clk/clk-conf.c b/drivers/clk/clk-conf.c >>>>>>> index aad4796..2b6375f 100644 >>>>>>> --- a/drivers/clk/clk-conf.c >>>>>>> +++ b/drivers/clk/clk-conf.c >>>>>>> @@ -107,8 +107,9 @@ static int __set_clk_rates(struct device_node *node, bool clk_supplier) >>>>>>> >>>>>>> rc = clk_set_rate(clk, rate); >>>>>>> if (rc < 0) >>>>>>> - pr_err("clk: couldn't set %s clock rate: %d\n", >>>>>>> - __clk_get_name(clk), rc); >>>>>>> + pr_err("clk: couldn't set %s rate to %ld (%d)\n", >>>>>>> + __clk_get_name(clk), clk_get_rate(clk), >>>>>>> + rc); >>>>> >>>>> Apologies for my previous misleading comment, with this patch >>>>> you log current clock rate as the rate which was attempted to >>>>> be set. And these two will likely be different when clk_set_rate() >>>>> failed. So it should rather be: >>>>> >>>>> pr_err("clk: couldn't set %s clk rate to %ld (%d)\n", >>>>> __clk_get_name(clk), rate, rc); >>> >>> I'll modify it as following without error value. >>> If I add the error value, the number of character are over 80. >>> pr_err("clk: couldn't set %s clk rate to %ld\n", >>> __clk_get_name(clk), rate); >> >> I wouldn't worry about line length over 80 characters, it not a hard rule. >> Let's leave the error code as it might be useful for debugging. >> > > OK, I'll add error value again. As you commented, I'll add all information as following: pr_err("clk: couldn't set %s clk rate to %ld (%d), current rate: %ld\n", __clk_get_name(clk), rate, rc, clk_get_rate(clk)); Thanks, Chanwoo Choi ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-04-27 9:06 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-04-20 11:08 [PATCH v2] clk: Show clock rate and error value when fail to set clock rate Chanwoo Choi 2015-04-21 8:37 ` Sylwester Nawrocki 2015-04-27 8:46 ` Chanwoo Choi 2015-04-27 8:51 ` Sylwester Nawrocki 2015-04-27 9:03 ` Chanwoo Choi 2015-04-27 9:06 ` Chanwoo Choi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox