| Message ID | 20230714134938.2124861-1-m.felsch@pengutronix.de (mailing list archive) |
|---|---|
| State | RFC, archived |
| Headers | show |
| Series | clk: imx: pll14xx: align pdiv with reference manual | expand |
On 23-07-14 15:49:38, Marco Felsch wrote: > The PLL14xx hardware can be found on i.MX8M{M,N,P} SoCs and always come > with a 6-bit pre-divider. Neither the reference manuals nor the > datasheets of these SoCs do mention any restrictions. Furthermore the > current code doesn't respect the restrictions from the comment too. > > Therefore drop the restriction and align the max pre-divider (pdiv) > value to 63 to get more accurate frequencies. > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> I'm OK with this: Reviewed-by: Abel Vesa <abel.vesa@linaro.org> > --- > Hi Adam, > > here is the patch I made for setting the exact video-pll settings. > > Regards, > Marco > > > drivers/clk/imx/clk-pll14xx.c | 11 +++++------ > 1 file changed, 5 insertions(+), 6 deletions(-) > > diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c > index 7150c59bbfc95..dc6bc21dff41f 100644 > --- a/drivers/clk/imx/clk-pll14xx.c > +++ b/drivers/clk/imx/clk-pll14xx.c > @@ -139,11 +139,10 @@ static void imx_pll14xx_calc_settings(struct clk_pll14xx *pll, unsigned long rat > /* > * Fractional PLL constrains: > * > - * a) 6MHz <= prate <= 25MHz > - * b) 1 <= p <= 63 (1 <= p <= 4 prate = 24MHz) > - * c) 64 <= m <= 1023 > - * d) 0 <= s <= 6 > - * e) -32768 <= k <= 32767 > + * a) 1 <= p <= 63 > + * b) 64 <= m <= 1023 > + * c) 0 <= s <= 6 > + * d) -32768 <= k <= 32767 > * > * fvco = (m * 65536 + k) * prate / (p * 65536) > */ > @@ -186,7 +185,7 @@ static void imx_pll14xx_calc_settings(struct clk_pll14xx *pll, unsigned long rat > } > > /* Finally calculate best values */ > - for (pdiv = 1; pdiv <= 7; pdiv++) { > + for (pdiv = 1; pdiv <= 63; pdiv++) { > for (sdiv = 0; sdiv <= 6; sdiv++) { > /* calc mdiv = round(rate * pdiv * 2^sdiv) / prate) */ > mdiv = DIV_ROUND_CLOSEST(rate * (pdiv << sdiv), prate); > -- > 2.39.2 >
On Tue, Jul 25, 2023 at 2:38 AM Abel Vesa <abel.vesa@linaro.org> wrote: > > On 23-07-14 15:49:38, Marco Felsch wrote: > > The PLL14xx hardware can be found on i.MX8M{M,N,P} SoCs and always come > > with a 6-bit pre-divider. Neither the reference manuals nor the > > datasheets of these SoCs do mention any restrictions. Furthermore the > > current code doesn't respect the restrictions from the comment too. > > > > Therefore drop the restriction and align the max pre-divider (pdiv) > > value to 63 to get more accurate frequencies. I like this. > > > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> > > I'm OK with this: > > Reviewed-by: Abel Vesa <abel.vesa@linaro.org> > Reviewed-by: Adam Ford <aford173@gmail.com> > > --- > > Hi Adam, > > > > here is the patch I made for setting the exact video-pll settings. > > > > Regards, > > Marco > > > > > > drivers/clk/imx/clk-pll14xx.c | 11 +++++------ > > 1 file changed, 5 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c > > index 7150c59bbfc95..dc6bc21dff41f 100644 > > --- a/drivers/clk/imx/clk-pll14xx.c > > +++ b/drivers/clk/imx/clk-pll14xx.c > > @@ -139,11 +139,10 @@ static void imx_pll14xx_calc_settings(struct clk_pll14xx *pll, unsigned long rat > > /* > > * Fractional PLL constrains: > > * > > - * a) 6MHz <= prate <= 25MHz > > - * b) 1 <= p <= 63 (1 <= p <= 4 prate = 24MHz) > > - * c) 64 <= m <= 1023 > > - * d) 0 <= s <= 6 > > - * e) -32768 <= k <= 32767 > > + * a) 1 <= p <= 63 > > + * b) 64 <= m <= 1023 > > + * c) 0 <= s <= 6 > > + * d) -32768 <= k <= 32767 > > * > > * fvco = (m * 65536 + k) * prate / (p * 65536) > > */ > > @@ -186,7 +185,7 @@ static void imx_pll14xx_calc_settings(struct clk_pll14xx *pll, unsigned long rat > > } > > > > /* Finally calculate best values */ > > - for (pdiv = 1; pdiv <= 7; pdiv++) { > > + for (pdiv = 1; pdiv <= 63; pdiv++) { > > for (sdiv = 0; sdiv <= 6; sdiv++) { > > /* calc mdiv = round(rate * pdiv * 2^sdiv) / prate) */ > > mdiv = DIV_ROUND_CLOSEST(rate * (pdiv << sdiv), prate); > > -- > > 2.39.2 > >
On Fri, Jul 14, 2023 at 8:49 AM Marco Felsch <m.felsch@pengutronix.de> wrote: > > The PLL14xx hardware can be found on i.MX8M{M,N,P} SoCs and always come > with a 6-bit pre-divider. Neither the reference manuals nor the > datasheets of these SoCs do mention any restrictions. Furthermore the > current code doesn't respect the restrictions from the comment too. > > Therefore drop the restriction and align the max pre-divider (pdiv) > value to 63 to get more accurate frequencies. > Should this get a fixes tag since it appears to fix a bug? > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> > --- > Hi Adam, > > here is the patch I made for setting the exact video-pll settings. > > Regards, > Marco > > > drivers/clk/imx/clk-pll14xx.c | 11 +++++------ > 1 file changed, 5 insertions(+), 6 deletions(-) > > diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c > index 7150c59bbfc95..dc6bc21dff41f 100644 > --- a/drivers/clk/imx/clk-pll14xx.c > +++ b/drivers/clk/imx/clk-pll14xx.c > @@ -139,11 +139,10 @@ static void imx_pll14xx_calc_settings(struct clk_pll14xx *pll, unsigned long rat > /* > * Fractional PLL constrains: > * > - * a) 6MHz <= prate <= 25MHz > - * b) 1 <= p <= 63 (1 <= p <= 4 prate = 24MHz) > - * c) 64 <= m <= 1023 > - * d) 0 <= s <= 6 > - * e) -32768 <= k <= 32767 > + * a) 1 <= p <= 63 > + * b) 64 <= m <= 1023 > + * c) 0 <= s <= 6 > + * d) -32768 <= k <= 32767 > * > * fvco = (m * 65536 + k) * prate / (p * 65536) > */ > @@ -186,7 +185,7 @@ static void imx_pll14xx_calc_settings(struct clk_pll14xx *pll, unsigned long rat > } > > /* Finally calculate best values */ > - for (pdiv = 1; pdiv <= 7; pdiv++) { > + for (pdiv = 1; pdiv <= 63; pdiv++) { > for (sdiv = 0; sdiv <= 6; sdiv++) { > /* calc mdiv = round(rate * pdiv * 2^sdiv) / prate) */ > mdiv = DIV_ROUND_CLOSEST(rate * (pdiv << sdiv), prate); > -- > 2.39.2 >
On 23-08-06, Adam Ford wrote: > On Fri, Jul 14, 2023 at 8:49 AM Marco Felsch <m.felsch@pengutronix.de> wrote: > > > > The PLL14xx hardware can be found on i.MX8M{M,N,P} SoCs and always come > > with a 6-bit pre-divider. Neither the reference manuals nor the > > datasheets of these SoCs do mention any restrictions. Furthermore the > > current code doesn't respect the restrictions from the comment too. > > > > Therefore drop the restriction and align the max pre-divider (pdiv) > > value to 63 to get more accurate frequencies. > > > > Should this get a fixes tag since it appears to fix a bug? Good question, I don't see it as a bug instead it has limited support. Ahmad, prepared a bugfix since he found out that a value comming from the pre defined pll-table is not valid. His bugfix need this patch to be able to calc the correct value, so I think add a fixes tag would be okay to get his bugfix backported as well. > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> > > --- > > Hi Adam, > > > > here is the patch I made for setting the exact video-pll settings. > > > > Regards, > > Marco > > > > > > drivers/clk/imx/clk-pll14xx.c | 11 +++++------ > > 1 file changed, 5 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c > > index 7150c59bbfc95..dc6bc21dff41f 100644 > > --- a/drivers/clk/imx/clk-pll14xx.c > > +++ b/drivers/clk/imx/clk-pll14xx.c > > @@ -139,11 +139,10 @@ static void imx_pll14xx_calc_settings(struct clk_pll14xx *pll, unsigned long rat > > /* > > * Fractional PLL constrains: > > * > > - * a) 6MHz <= prate <= 25MHz > > - * b) 1 <= p <= 63 (1 <= p <= 4 prate = 24MHz) > > - * c) 64 <= m <= 1023 > > - * d) 0 <= s <= 6 > > - * e) -32768 <= k <= 32767 > > + * a) 1 <= p <= 63 > > + * b) 64 <= m <= 1023 > > + * c) 0 <= s <= 6 > > + * d) -32768 <= k <= 32767 > > * > > * fvco = (m * 65536 + k) * prate / (p * 65536) > > */ > > @@ -186,7 +185,7 @@ static void imx_pll14xx_calc_settings(struct clk_pll14xx *pll, unsigned long rat > > } > > > > /* Finally calculate best values */ > > - for (pdiv = 1; pdiv <= 7; pdiv++) { > > + for (pdiv = 1; pdiv <= 63; pdiv++) { > > for (sdiv = 0; sdiv <= 6; sdiv++) { > > /* calc mdiv = round(rate * pdiv * 2^sdiv) / prate) */ > > mdiv = DIV_ROUND_CLOSEST(rate * (pdiv << sdiv), prate); > > -- > > 2.39.2 > > >
diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c index 7150c59bbfc95..dc6bc21dff41f 100644 --- a/drivers/clk/imx/clk-pll14xx.c +++ b/drivers/clk/imx/clk-pll14xx.c @@ -139,11 +139,10 @@ static void imx_pll14xx_calc_settings(struct clk_pll14xx *pll, unsigned long rat /* * Fractional PLL constrains: * - * a) 6MHz <= prate <= 25MHz - * b) 1 <= p <= 63 (1 <= p <= 4 prate = 24MHz) - * c) 64 <= m <= 1023 - * d) 0 <= s <= 6 - * e) -32768 <= k <= 32767 + * a) 1 <= p <= 63 + * b) 64 <= m <= 1023 + * c) 0 <= s <= 6 + * d) -32768 <= k <= 32767 * * fvco = (m * 65536 + k) * prate / (p * 65536) */ @@ -186,7 +185,7 @@ static void imx_pll14xx_calc_settings(struct clk_pll14xx *pll, unsigned long rat } /* Finally calculate best values */ - for (pdiv = 1; pdiv <= 7; pdiv++) { + for (pdiv = 1; pdiv <= 63; pdiv++) { for (sdiv = 0; sdiv <= 6; sdiv++) { /* calc mdiv = round(rate * pdiv * 2^sdiv) / prate) */ mdiv = DIV_ROUND_CLOSEST(rate * (pdiv << sdiv), prate);
The PLL14xx hardware can be found on i.MX8M{M,N,P} SoCs and always come with a 6-bit pre-divider. Neither the reference manuals nor the datasheets of these SoCs do mention any restrictions. Furthermore the current code doesn't respect the restrictions from the comment too. Therefore drop the restriction and align the max pre-divider (pdiv) value to 63 to get more accurate frequencies. Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> --- Hi Adam, here is the patch I made for setting the exact video-pll settings. Regards, Marco drivers/clk/imx/clk-pll14xx.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)