| Message ID | 20230612-topic-rcg2_ro-v1-2-e7d824aeb628@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Add read-only RCG2 ops | expand |
On 13.06.2023 19:56, Bjorn Andersson wrote: > On Mon, Jun 12, 2023 at 11:22:48AM +0200, Konrad Dybcio wrote: >> The config/periph/system NoC clocks are wholly controlled by the >> RPM firmware and Linux should never ever alter their configuration. >> > > Does Linux need to know about them? Not really, but it allows us to get rates of their children. We can get rid of them if one can argue debugcc is enough. Unless we need clk_get_rate for some reason. Konrad > > Regards, > Bjorn > >> Switch them over to read-only ops to avoid that. >> >> Fixes: b1e010c0730a ("clk: qcom: Add MSM8996 Global Clock Control (GCC) driver") >> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> >> --- >> drivers/clk/qcom/gcc-msm8996.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/clk/qcom/gcc-msm8996.c b/drivers/clk/qcom/gcc-msm8996.c >> index 5e44d1bcca9e..588e3b67657a 100644 >> --- a/drivers/clk/qcom/gcc-msm8996.c >> +++ b/drivers/clk/qcom/gcc-msm8996.c >> @@ -264,7 +264,7 @@ static struct clk_rcg2 system_noc_clk_src = { >> .name = "system_noc_clk_src", >> .parent_data = gcc_xo_gpll0_gpll0_early_div, >> .num_parents = ARRAY_SIZE(gcc_xo_gpll0_gpll0_early_div), >> - .ops = &clk_rcg2_ops, >> + .ops = &clk_rcg2_ro_ops, >> }, >> }; >> >> @@ -284,7 +284,7 @@ static struct clk_rcg2 config_noc_clk_src = { >> .name = "config_noc_clk_src", >> .parent_data = gcc_xo_gpll0, >> .num_parents = ARRAY_SIZE(gcc_xo_gpll0), >> - .ops = &clk_rcg2_ops, >> + .ops = &clk_rcg2_ro_ops, >> }, >> }; >> >> @@ -306,7 +306,7 @@ static struct clk_rcg2 periph_noc_clk_src = { >> .name = "periph_noc_clk_src", >> .parent_data = gcc_xo_gpll0, >> .num_parents = ARRAY_SIZE(gcc_xo_gpll0), >> - .ops = &clk_rcg2_ops, >> + .ops = &clk_rcg2_ro_ops, >> }, >> }; >> >> >> -- >> 2.41.0 >>
On Mon, Jun 12, 2023 at 11:22:48AM +0200, Konrad Dybcio wrote: > The config/periph/system NoC clocks are wholly controlled by the > RPM firmware and Linux should never ever alter their configuration. > Does Linux need to know about them? Regards, Bjorn > Switch them over to read-only ops to avoid that. > > Fixes: b1e010c0730a ("clk: qcom: Add MSM8996 Global Clock Control (GCC) driver") > Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> > --- > drivers/clk/qcom/gcc-msm8996.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/clk/qcom/gcc-msm8996.c b/drivers/clk/qcom/gcc-msm8996.c > index 5e44d1bcca9e..588e3b67657a 100644 > --- a/drivers/clk/qcom/gcc-msm8996.c > +++ b/drivers/clk/qcom/gcc-msm8996.c > @@ -264,7 +264,7 @@ static struct clk_rcg2 system_noc_clk_src = { > .name = "system_noc_clk_src", > .parent_data = gcc_xo_gpll0_gpll0_early_div, > .num_parents = ARRAY_SIZE(gcc_xo_gpll0_gpll0_early_div), > - .ops = &clk_rcg2_ops, > + .ops = &clk_rcg2_ro_ops, > }, > }; > > @@ -284,7 +284,7 @@ static struct clk_rcg2 config_noc_clk_src = { > .name = "config_noc_clk_src", > .parent_data = gcc_xo_gpll0, > .num_parents = ARRAY_SIZE(gcc_xo_gpll0), > - .ops = &clk_rcg2_ops, > + .ops = &clk_rcg2_ro_ops, > }, > }; > > @@ -306,7 +306,7 @@ static struct clk_rcg2 periph_noc_clk_src = { > .name = "periph_noc_clk_src", > .parent_data = gcc_xo_gpll0, > .num_parents = ARRAY_SIZE(gcc_xo_gpll0), > - .ops = &clk_rcg2_ops, > + .ops = &clk_rcg2_ro_ops, > }, > }; > > > -- > 2.41.0 >
On 13.06.2023 19:54, Konrad Dybcio wrote: > > > On 13.06.2023 19:56, Bjorn Andersson wrote: >> On Mon, Jun 12, 2023 at 11:22:48AM +0200, Konrad Dybcio wrote: >>> The config/periph/system NoC clocks are wholly controlled by the >>> RPM firmware and Linux should never ever alter their configuration. >>> >> >> Does Linux need to know about them? > Not really, but it allows us to get rates of their children. > > We can get rid of them if one can argue debugcc is enough. Unless > we need clk_get_rate for some reason. > Any opinions? Konrad
On Thu, 24 Aug 2023 at 13:28, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: > > On 31.07.2023 13:01, Konrad Dybcio wrote: > > On 13.06.2023 19:54, Konrad Dybcio wrote: > >> > >> > >> On 13.06.2023 19:56, Bjorn Andersson wrote: > >>> On Mon, Jun 12, 2023 at 11:22:48AM +0200, Konrad Dybcio wrote: > >>>> The config/periph/system NoC clocks are wholly controlled by the > >>>> RPM firmware and Linux should never ever alter their configuration. > >>>> > >>> > >>> Does Linux need to know about them? > >> Not really, but it allows us to get rates of their children. > >> > >> We can get rid of them if one can argue debugcc is enough. Unless > >> we need clk_get_rate for some reason. > >> > > Any opinions? > So, do we drop them? My opinion would be to drop the NoC clocks unless there is any extra value (like child clocks).
diff --git a/drivers/clk/qcom/gcc-msm8996.c b/drivers/clk/qcom/gcc-msm8996.c index 5e44d1bcca9e..588e3b67657a 100644 --- a/drivers/clk/qcom/gcc-msm8996.c +++ b/drivers/clk/qcom/gcc-msm8996.c @@ -264,7 +264,7 @@ static struct clk_rcg2 system_noc_clk_src = { .name = "system_noc_clk_src", .parent_data = gcc_xo_gpll0_gpll0_early_div, .num_parents = ARRAY_SIZE(gcc_xo_gpll0_gpll0_early_div), - .ops = &clk_rcg2_ops, + .ops = &clk_rcg2_ro_ops, }, }; @@ -284,7 +284,7 @@ static struct clk_rcg2 config_noc_clk_src = { .name = "config_noc_clk_src", .parent_data = gcc_xo_gpll0, .num_parents = ARRAY_SIZE(gcc_xo_gpll0), - .ops = &clk_rcg2_ops, + .ops = &clk_rcg2_ro_ops, }, }; @@ -306,7 +306,7 @@ static struct clk_rcg2 periph_noc_clk_src = { .name = "periph_noc_clk_src", .parent_data = gcc_xo_gpll0, .num_parents = ARRAY_SIZE(gcc_xo_gpll0), - .ops = &clk_rcg2_ops, + .ops = &clk_rcg2_ro_ops, }, };
The config/periph/system NoC clocks are wholly controlled by the RPM firmware and Linux should never ever alter their configuration. Switch them over to read-only ops to avoid that. Fixes: b1e010c0730a ("clk: qcom: Add MSM8996 Global Clock Control (GCC) driver") Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> --- drivers/clk/qcom/gcc-msm8996.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)