diff mbox series

[v3,10/15] drivers: clk: qcom: gcc-ipq806x: add additional freq for sdc table

Message ID 20220121210340.32362-11-ansuelsmth@gmail.com (mailing list archive)
State Superseded
Headers show
Series Multiple addition and improvement to ipq8064 gcc | expand

Commit Message

Christian Marangi Jan. 21, 2022, 9:03 p.m. UTC
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(+)

Comments

Stephen Boyd Jan. 25, 2022, 8:45 p.m. UTC | #1
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?
Christian Marangi Jan. 25, 2022, 9:03 p.m. UTC | #2
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.
Stephen Boyd Jan. 25, 2022, 10:18 p.m. UTC | #3
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.
Christian Marangi Feb. 1, 2022, 10:01 p.m. UTC | #4
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?
Stephen Boyd Feb. 5, 2022, 3:03 a.m. UTC | #5
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.
diff mbox series

Patch

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 */
 	{  64000000, P_PLL8,  3, 1,   2 },
 	{  96000000, P_PLL8,  4, 0,   0 },
 	{ 192000000, P_PLL8,  2, 0,   0 },