| Message ID | 20220121210340.32362-1-ansuelsmth@gmail.com |
|---|---|
| Headers | show |
| Series | Multiple addition and improvement to ipq8064 gcc | expand |
Quoting Ansuel Smith (2022-01-21 13:03:35) > Add additional freq supported for the sdc table. > > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com> > --- > drivers/clk/qcom/gcc-ipq806x.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/clk/qcom/gcc-ipq806x.c b/drivers/clk/qcom/gcc-ipq806x.c > index 77bc3d94f580..dbd61e4844b0 100644 > --- a/drivers/clk/qcom/gcc-ipq806x.c > +++ b/drivers/clk/qcom/gcc-ipq806x.c > @@ -1292,6 +1292,7 @@ static const struct freq_tbl clk_tbl_sdc[] = { > { 20210000, P_PLL8, 1, 1, 19 }, > { 24000000, P_PLL8, 4, 1, 4 }, > { 48000000, P_PLL8, 4, 1, 2 }, > + { 52000000, P_PLL8, 1, 2, 15 }, /* 51.2 Mhz */ Why the comment and fake rate? Can it be 51200000 instead and drop the comment?
On Tue, Jan 25, 2022 at 12:45:53PM -0800, Stephen Boyd wrote: > Quoting Ansuel Smith (2022-01-21 13:03:35) > > Add additional freq supported for the sdc table. > > > > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com> > > --- > > drivers/clk/qcom/gcc-ipq806x.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/clk/qcom/gcc-ipq806x.c b/drivers/clk/qcom/gcc-ipq806x.c > > index 77bc3d94f580..dbd61e4844b0 100644 > > --- a/drivers/clk/qcom/gcc-ipq806x.c > > +++ b/drivers/clk/qcom/gcc-ipq806x.c > > @@ -1292,6 +1292,7 @@ static const struct freq_tbl clk_tbl_sdc[] = { > > { 20210000, P_PLL8, 1, 1, 19 }, > > { 24000000, P_PLL8, 4, 1, 4 }, > > { 48000000, P_PLL8, 4, 1, 2 }, > > + { 52000000, P_PLL8, 1, 2, 15 }, /* 51.2 Mhz */ > > Why the comment and fake rate? Can it be 51200000 instead and drop the > comment? I will add the related reason in the commit. We cannot achieve exact 52Mhz(jitter free) clock using PLL8. As per the MND calculator the closest possible jitter free clock using PLL8 is 51.2Mhz. This patch adds the values, which will provide jitter free 51.2Mhz when the requested frequency is 52mhz.
Quoting Ansuel Smith (2022-01-25 13:03:52) > On Tue, Jan 25, 2022 at 12:45:53PM -0800, Stephen Boyd wrote: > > Quoting Ansuel Smith (2022-01-21 13:03:35) > > > Add additional freq supported for the sdc table. > > > > > > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com> > > > --- > > > drivers/clk/qcom/gcc-ipq806x.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/drivers/clk/qcom/gcc-ipq806x.c b/drivers/clk/qcom/gcc-ipq806x.c > > > index 77bc3d94f580..dbd61e4844b0 100644 > > > --- a/drivers/clk/qcom/gcc-ipq806x.c > > > +++ b/drivers/clk/qcom/gcc-ipq806x.c > > > @@ -1292,6 +1292,7 @@ static const struct freq_tbl clk_tbl_sdc[] = { > > > { 20210000, P_PLL8, 1, 1, 19 }, > > > { 24000000, P_PLL8, 4, 1, 4 }, > > > { 48000000, P_PLL8, 4, 1, 2 }, > > > + { 52000000, P_PLL8, 1, 2, 15 }, /* 51.2 Mhz */ > > > > Why the comment and fake rate? Can it be 51200000 instead and drop the > > comment? > > I will add the related reason in the commit. > > We cannot achieve exact 52Mhz(jitter free) clock using PLL8. > As per the MND calculator the closest possible jitter free clock > using PLL8 is 51.2Mhz. This patch adds the values, which will provide > jitter free 51.2Mhz when the requested frequency is 52mhz. Sounds like this clk should use the round down clk_ops instead of the round up ones. Then the actual frequency can be in the table.
On Fri 21 Jan 15:03 CST 2022, Ansuel Smith wrote: Please drop the "drivers: " prefix in $subject on all these patches to align it with (most) other entried in the git log. Thanks, Bjorn > Parent gcc_pxo_pll8_pll0 had the parent definition and parent map > swapped. Fix this naming error. > > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com> > --- > drivers/clk/qcom/gcc-ipq806x.c | 20 ++++++++++---------- > 1 file changed, 10 insertions(+), 10 deletions(-) > > diff --git a/drivers/clk/qcom/gcc-ipq806x.c b/drivers/clk/qcom/gcc-ipq806x.c > index d6b7adb4be38..34cddf461dba 100644 > --- a/drivers/clk/qcom/gcc-ipq806x.c > +++ b/drivers/clk/qcom/gcc-ipq806x.c > @@ -291,13 +291,13 @@ static const char * const gcc_pxo_pll3[] = { > "pll3", > }; > > -static const struct parent_map gcc_pxo_pll8_pll0[] = { > +static const struct parent_map gcc_pxo_pll8_pll0_map[] = { > { P_PXO, 0 }, > { P_PLL8, 3 }, > { P_PLL0, 2 } > }; > > -static const char * const gcc_pxo_pll8_pll0_map[] = { > +static const char * const gcc_pxo_pll8_pll0[] = { > "pxo", > "pll8_vote", > "pll0_vote", > @@ -1993,7 +1993,7 @@ static struct clk_rcg usb30_master_clk_src = { > }, > .s = { > .src_sel_shift = 0, > - .parent_map = gcc_pxo_pll8_pll0, > + .parent_map = gcc_pxo_pll8_pll0_map, > }, > .freq_tbl = clk_tbl_usb30_master, > .clkr = { > @@ -2001,7 +2001,7 @@ static struct clk_rcg usb30_master_clk_src = { > .enable_mask = BIT(11), > .hw.init = &(struct clk_init_data){ > .name = "usb30_master_ref_src", > - .parent_names = gcc_pxo_pll8_pll0_map, > + .parent_names = gcc_pxo_pll8_pll0, > .num_parents = 3, > .ops = &clk_rcg_ops, > .flags = CLK_SET_RATE_GATE, > @@ -2063,7 +2063,7 @@ static struct clk_rcg usb30_utmi_clk = { > }, > .s = { > .src_sel_shift = 0, > - .parent_map = gcc_pxo_pll8_pll0, > + .parent_map = gcc_pxo_pll8_pll0_map, > }, > .freq_tbl = clk_tbl_usb30_utmi, > .clkr = { > @@ -2071,7 +2071,7 @@ static struct clk_rcg usb30_utmi_clk = { > .enable_mask = BIT(11), > .hw.init = &(struct clk_init_data){ > .name = "usb30_utmi_clk", > - .parent_names = gcc_pxo_pll8_pll0_map, > + .parent_names = gcc_pxo_pll8_pll0, > .num_parents = 3, > .ops = &clk_rcg_ops, > .flags = CLK_SET_RATE_GATE, > @@ -2133,7 +2133,7 @@ static struct clk_rcg usb_hs1_xcvr_clk_src = { > }, > .s = { > .src_sel_shift = 0, > - .parent_map = gcc_pxo_pll8_pll0, > + .parent_map = gcc_pxo_pll8_pll0_map, > }, > .freq_tbl = clk_tbl_usb, > .clkr = { > @@ -2141,7 +2141,7 @@ static struct clk_rcg usb_hs1_xcvr_clk_src = { > .enable_mask = BIT(11), > .hw.init = &(struct clk_init_data){ > .name = "usb_hs1_xcvr_src", > - .parent_names = gcc_pxo_pll8_pll0_map, > + .parent_names = gcc_pxo_pll8_pll0, > .num_parents = 3, > .ops = &clk_rcg_ops, > .flags = CLK_SET_RATE_GATE, > @@ -2197,7 +2197,7 @@ static struct clk_rcg usb_fs1_xcvr_clk_src = { > }, > .s = { > .src_sel_shift = 0, > - .parent_map = gcc_pxo_pll8_pll0, > + .parent_map = gcc_pxo_pll8_pll0_map, > }, > .freq_tbl = clk_tbl_usb, > .clkr = { > @@ -2205,7 +2205,7 @@ static struct clk_rcg usb_fs1_xcvr_clk_src = { > .enable_mask = BIT(11), > .hw.init = &(struct clk_init_data){ > .name = "usb_fs1_xcvr_src", > - .parent_names = gcc_pxo_pll8_pll0_map, > + .parent_names = gcc_pxo_pll8_pll0, > .num_parents = 3, > .ops = &clk_rcg_ops, > .flags = CLK_SET_RATE_GATE, > -- > 2.33.1 >
On Tue, Jan 25, 2022 at 02:18:24PM -0800, Stephen Boyd wrote: > Quoting Ansuel Smith (2022-01-25 13:03:52) > > On Tue, Jan 25, 2022 at 12:45:53PM -0800, Stephen Boyd wrote: > > > Quoting Ansuel Smith (2022-01-21 13:03:35) > > > > Add additional freq supported for the sdc table. > > > > > > > > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com> > > > > --- > > > > drivers/clk/qcom/gcc-ipq806x.c | 1 + > > > > 1 file changed, 1 insertion(+) > > > > > > > > diff --git a/drivers/clk/qcom/gcc-ipq806x.c b/drivers/clk/qcom/gcc-ipq806x.c > > > > index 77bc3d94f580..dbd61e4844b0 100644 > > > > --- a/drivers/clk/qcom/gcc-ipq806x.c > > > > +++ b/drivers/clk/qcom/gcc-ipq806x.c > > > > @@ -1292,6 +1292,7 @@ static const struct freq_tbl clk_tbl_sdc[] = { > > > > { 20210000, P_PLL8, 1, 1, 19 }, > > > > { 24000000, P_PLL8, 4, 1, 4 }, > > > > { 48000000, P_PLL8, 4, 1, 2 }, > > > > + { 52000000, P_PLL8, 1, 2, 15 }, /* 51.2 Mhz */ > > > > > > Why the comment and fake rate? Can it be 51200000 instead and drop the > > > comment? > > > > I will add the related reason in the commit. > > > > We cannot achieve exact 52Mhz(jitter free) clock using PLL8. > > As per the MND calculator the closest possible jitter free clock > > using PLL8 is 51.2Mhz. This patch adds the values, which will provide > > jitter free 51.2Mhz when the requested frequency is 52mhz. > > Sounds like this clk should use the round down clk_ops instead of the > round up ones. Then the actual frequency can be in the table. Some hint on how to do that? This use the rcg generic ops that doesn't use any round. Should I crate some special ops in the rcg driver to implement the round ops?
Quoting Ansuel Smith (2022-02-01 14:01:40) > On Tue, Jan 25, 2022 at 02:18:24PM -0800, Stephen Boyd wrote: > > Quoting Ansuel Smith (2022-01-25 13:03:52) > > > On Tue, Jan 25, 2022 at 12:45:53PM -0800, Stephen Boyd wrote: > > > > Quoting Ansuel Smith (2022-01-21 13:03:35) > > > > > Add additional freq supported for the sdc table. > > > > > > > > > > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com> > > > > > --- > > > > > drivers/clk/qcom/gcc-ipq806x.c | 1 + > > > > > 1 file changed, 1 insertion(+) > > > > > > > > > > diff --git a/drivers/clk/qcom/gcc-ipq806x.c b/drivers/clk/qcom/gcc-ipq806x.c > > > > > index 77bc3d94f580..dbd61e4844b0 100644 > > > > > --- a/drivers/clk/qcom/gcc-ipq806x.c > > > > > +++ b/drivers/clk/qcom/gcc-ipq806x.c > > > > > @@ -1292,6 +1292,7 @@ static const struct freq_tbl clk_tbl_sdc[] = { > > > > > { 20210000, P_PLL8, 1, 1, 19 }, > > > > > { 24000000, P_PLL8, 4, 1, 4 }, > > > > > { 48000000, P_PLL8, 4, 1, 2 }, > > > > > + { 52000000, P_PLL8, 1, 2, 15 }, /* 51.2 Mhz */ > > > > > > > > Why the comment and fake rate? Can it be 51200000 instead and drop the > > > > comment? > > > > > > I will add the related reason in the commit. > > > > > > We cannot achieve exact 52Mhz(jitter free) clock using PLL8. > > > As per the MND calculator the closest possible jitter free clock > > > using PLL8 is 51.2Mhz. This patch adds the values, which will provide > > > jitter free 51.2Mhz when the requested frequency is 52mhz. > > > > Sounds like this clk should use the round down clk_ops instead of the > > round up ones. Then the actual frequency can be in the table. > > Some hint on how to do that? This use the rcg generic ops that doesn't > use any round. Should I crate some special ops in the rcg driver to > implement the round ops? > Use the clk_rcg2_floor_ops, or if this isn't an rcg2 clk, then make a duplicate clk_rcg_floor_ops that does the same thing.
This is an attempt in making the ipq8064 SoC actually usable. Currently many feature are missing for this SoC and devs user off-the-tree patches to make it work (example patch for missing clock, patch for cpufreq driver, patch to add missing node in the dts) I notice there was some work in modernizing the gcc driver for other qcom target but this wasn't done for ipq806x. This does exactly this, we drop any parent_names stuff and we switch to the parent_data way. We also drop the pxo and cxo source clk from gcc driver and we refer to the dts for it. This also add all the missing feature for the nss cores and the cryptoengine in them. It does also introduce the required flags to make the RPM actually work and NOT reject any command. There was an attempt in declaring these clock as core clock in the dts but this ends up in no serial as the kernel makes these clock not accessible. We just want to make the kernel NOT disable them if unused nothing more. At the end we update the ipq8064 dtsi to add the pxo and cxo tag and declare them in gcc and also fix a problem with tsens probe. v3: - Rework Documentation with Rob suggestions v2: - Fix error from Rob bot. - Add additional commits to make qcom,gcc.yaml a template - Squash parent_hws patch with the modernize patch - Create gcc_pxo instead of using long define. Ansuel Smith (15): dt-bindings: clock: split qcom,gcc.yaml to common and specific schema dt-bindings: clock: simplify qcom,gcc-apq8064 Documentation dt-bindings: clock: Document qcom,gcc-ipq8064 binding drivers: clk: qcom: gcc-ipq806x: fix wrong naming for gcc_pxo_pll8_pll0 drivers: clk: qcom: gcc-ipq806x: convert parent_names to parent_data drivers: clk: qcom: gcc-ipq806x: use ARRAY_SIZE for num_parents drivers: clk: qcom: gcc-ipq806x: drop hardcoded pxo and cxo source clk drivers: clk: qcom: gcc-ipq806x: add additional freq nss cores drivers: clk: qcom: gcc-ipq806x: add unusued flag for critical clock drivers: clk: qcom: gcc-ipq806x: add additional freq for sdc table dt-bindings: clock: add ipq8064 ce5 clk define drivers: clk: qcom: gcc-ipq806x: add CryptoEngine clocks dt-bindings: reset: add ipq8064 ce5 resets drivers: clk: qcom: gcc-ipq806x: add CryptoEngine resets ARM: dts: qcom: Add syscon and cxo/pxo clock to gcc node for ipq8064 .../bindings/clock/qcom,gcc-apq8064.yaml | 29 +- .../bindings/clock/qcom,gcc-common.yaml | 42 ++ .../bindings/clock/qcom,gcc-ipq8064.yaml | 76 +++ .../devicetree/bindings/clock/qcom,gcc.yaml | 31 +- arch/arm/boot/dts/qcom-ipq8064.dtsi | 8 +- drivers/clk/qcom/gcc-ipq806x.c | 638 +++++++++++++----- include/dt-bindings/clock/qcom,gcc-ipq806x.h | 5 +- include/dt-bindings/reset/qcom,gcc-ipq806x.h | 5 + 8 files changed, 615 insertions(+), 219 deletions(-) create mode 100644 Documentation/devicetree/bindings/clock/qcom,gcc-common.yaml create mode 100644 Documentation/devicetree/bindings/clock/qcom,gcc-ipq8064.yaml