| Message ID | 20170713133224.753-1-georgi.djakov@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On 07/13, Georgi Djakov wrote: > As there is no way to actually query the hardware for the current clock > rate, now racalc_rate() just returns the last rate that was previously > set. But if the rate was not set yet, we return the bogus rate of 1000Hz. > > The branch clocks actually have the same rate as their parent (xo_board), > so just return this rate. > > Reported-by: Archit Taneja <architt@codeaurora.org> > Fixes: 00f64b58874e ("clk: qcom: Add support for SMD-RPM Clocks") > Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org> > --- > drivers/clk/qcom/clk-smd-rpm.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c > index d990fe44aef3..b45782657ca9 100644 > --- a/drivers/clk/qcom/clk-smd-rpm.c > +++ b/drivers/clk/qcom/clk-smd-rpm.c > @@ -364,6 +364,10 @@ static unsigned long clk_smd_rpm_recalc_rate(struct clk_hw *hw, > { > struct clk_smd_rpm *r = to_clk_smd_rpm(hw); > > + /* Return the parent rate for branches */ > + if (r->branch) > + return parent_rate; > + What's parent_rate here though? 0? I don't see where we parent the branch clks to anything. And we should really just remove the recalc_rate() op for branches entirely so that we don't have to call down into the driver to find out something we could have known in the core.
On 07/14/2017 12:56 AM, Stephen Boyd wrote: > On 07/13, Georgi Djakov wrote: >> As there is no way to actually query the hardware for the current clock >> rate, now racalc_rate() just returns the last rate that was previously >> set. But if the rate was not set yet, we return the bogus rate of 1000Hz. >> >> The branch clocks actually have the same rate as their parent (xo_board), >> so just return this rate. >> >> Reported-by: Archit Taneja <architt@codeaurora.org> >> Fixes: 00f64b58874e ("clk: qcom: Add support for SMD-RPM Clocks") >> Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org> >> --- >> drivers/clk/qcom/clk-smd-rpm.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c >> index d990fe44aef3..b45782657ca9 100644 >> --- a/drivers/clk/qcom/clk-smd-rpm.c >> +++ b/drivers/clk/qcom/clk-smd-rpm.c >> @@ -364,6 +364,10 @@ static unsigned long clk_smd_rpm_recalc_rate(struct clk_hw *hw, >> { >> struct clk_smd_rpm *r = to_clk_smd_rpm(hw); >> >> + /* Return the parent rate for branches */ >> + if (r->branch) >> + return parent_rate; >> + > > What's parent_rate here though? 0? I don't see where we parent > the branch clks to anything. Its 19.2MHz as xo_board is a fixed-clock in DT. We use the __DEFINE_CLK_SMD_RPM_BRANCH macro to define all the branch clks and this macro sets their parent name to "xo_board". Sorry for not explaining this earlier. > And we should really just remove the recalc_rate() op for > branches entirely so that we don't have to call down into the > driver to find out something we could have known in the core. Yes this would be better. I agree. Will remove recalc/round_rate ops for the branch clks. Thanks, Georgi -- To unsubscribe from this list: send the line "unsubscribe linux-clk" 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/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c index d990fe44aef3..b45782657ca9 100644 --- a/drivers/clk/qcom/clk-smd-rpm.c +++ b/drivers/clk/qcom/clk-smd-rpm.c @@ -364,6 +364,10 @@ static unsigned long clk_smd_rpm_recalc_rate(struct clk_hw *hw, { struct clk_smd_rpm *r = to_clk_smd_rpm(hw); + /* Return the parent rate for branches */ + if (r->branch) + return parent_rate; + /* * RPM handles rate rounding and we don't have a way to * know what the rate will be, so just return whatever
As there is no way to actually query the hardware for the current clock rate, now racalc_rate() just returns the last rate that was previously set. But if the rate was not set yet, we return the bogus rate of 1000Hz. The branch clocks actually have the same rate as their parent (xo_board), so just return this rate. Reported-by: Archit Taneja <architt@codeaurora.org> Fixes: 00f64b58874e ("clk: qcom: Add support for SMD-RPM Clocks") Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org> --- drivers/clk/qcom/clk-smd-rpm.c | 4 ++++ 1 file changed, 4 insertions(+) -- To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html