2023-06-25 12:43:07

by Peng Fan (OSS)

[permalink] [raw]
Subject: [PATCH 1/2] clk: imx: pllv4: Fix SPLL2 MULT range

From: Ye Li <[email protected]>

The SPLL2 on iMX8ULP is different with other frac PLLs, it can
support VCO from 650Mhz to 1Ghz. According to RM, the MULT is
using a range from 27 to 54, not some fixed values. If using
current PLL implementation, some clock rate can't be supported.

Fix the issue by adding new type for the SPLL2 and use MULT range
to replace MULT table

Fixes: 5f0601c47c33 ("clk: imx: Update the pllv4 to support imx8ulp")
Reviewed-by: Peng Fan <[email protected]>
Reviewed-by: Jacky Bai <[email protected]>
Signed-off-by: Ye Li <[email protected]>
Signed-off-by: Peng Fan <[email protected]>
---
drivers/clk/imx/clk-pllv4.c | 46 +++++++++++++++++++++++++++++--------
drivers/clk/imx/clk.h | 1 +
2 files changed, 37 insertions(+), 10 deletions(-)

diff --git a/drivers/clk/imx/clk-pllv4.c b/drivers/clk/imx/clk-pllv4.c
index 6e7e34571fc8..9b136c951762 100644
--- a/drivers/clk/imx/clk-pllv4.c
+++ b/drivers/clk/imx/clk-pllv4.c
@@ -44,11 +44,15 @@ struct clk_pllv4 {
u32 cfg_offset;
u32 num_offset;
u32 denom_offset;
+ bool use_mult_range;
};

/* Valid PLL MULT Table */
static const int pllv4_mult_table[] = {33, 27, 22, 20, 17, 16};

+/* Valid PLL MULT range, (max, min) */
+static const int pllv4_mult_range[] = {54, 27};
+
#define to_clk_pllv4(__hw) container_of(__hw, struct clk_pllv4, hw)

#define LOCK_TIMEOUT_US USEC_PER_MSEC
@@ -94,17 +98,30 @@ static unsigned long clk_pllv4_recalc_rate(struct clk_hw *hw,
static long clk_pllv4_round_rate(struct clk_hw *hw, unsigned long rate,
unsigned long *prate)
{
+ struct clk_pllv4 *pll = to_clk_pllv4(hw);
unsigned long parent_rate = *prate;
unsigned long round_rate, i;
u32 mfn, mfd = DEFAULT_MFD;
bool found = false;
u64 temp64;
-
- for (i = 0; i < ARRAY_SIZE(pllv4_mult_table); i++) {
- round_rate = parent_rate * pllv4_mult_table[i];
- if (rate >= round_rate) {
+ u32 mult;
+
+ if (pll->use_mult_range) {
+ temp64 = (u64)rate;
+ do_div(temp64, parent_rate);
+ mult = temp64;
+ if (mult >= pllv4_mult_range[1] &&
+ mult <= pllv4_mult_range[0]) {
+ round_rate = parent_rate * mult;
found = true;
- break;
+ }
+ } else {
+ for (i = 0; i < ARRAY_SIZE(pllv4_mult_table); i++) {
+ round_rate = parent_rate * pllv4_mult_table[i];
+ if (rate >= round_rate) {
+ found = true;
+ break;
+ }
}
}

@@ -138,14 +155,20 @@ static long clk_pllv4_round_rate(struct clk_hw *hw, unsigned long rate,
return round_rate + (u32)temp64;
}

-static bool clk_pllv4_is_valid_mult(unsigned int mult)
+static bool clk_pllv4_is_valid_mult(struct clk_pllv4 *pll, unsigned int mult)
{
int i;

/* check if mult is in valid MULT table */
- for (i = 0; i < ARRAY_SIZE(pllv4_mult_table); i++) {
- if (pllv4_mult_table[i] == mult)
+ if (pll->use_mult_range) {
+ if (mult >= pllv4_mult_range[1] &&
+ mult <= pllv4_mult_range[0])
return true;
+ } else {
+ for (i = 0; i < ARRAY_SIZE(pllv4_mult_table); i++) {
+ if (pllv4_mult_table[i] == mult)
+ return true;
+ }
}

return false;
@@ -160,7 +183,7 @@ static int clk_pllv4_set_rate(struct clk_hw *hw, unsigned long rate,

mult = rate / parent_rate;

- if (!clk_pllv4_is_valid_mult(mult))
+ if (!clk_pllv4_is_valid_mult(pll, mult))
return -EINVAL;

if (parent_rate <= MAX_MFD)
@@ -227,10 +250,13 @@ struct clk_hw *imx_clk_hw_pllv4(enum imx_pllv4_type type, const char *name,

pll->base = base;

- if (type == IMX_PLLV4_IMX8ULP) {
+ if (type == IMX_PLLV4_IMX8ULP ||
+ type == IMX_PLLV4_IMX8ULP_1GHZ) {
pll->cfg_offset = IMX8ULP_PLL_CFG_OFFSET;
pll->num_offset = IMX8ULP_PLL_NUM_OFFSET;
pll->denom_offset = IMX8ULP_PLL_DENOM_OFFSET;
+ if (type == IMX_PLLV4_IMX8ULP_1GHZ)
+ pll->use_mult_range = true;
} else {
pll->cfg_offset = PLL_CFG_OFFSET;
pll->num_offset = PLL_NUM_OFFSET;
diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h
index af19d9f6aed0..adb7ad649a0d 100644
--- a/drivers/clk/imx/clk.h
+++ b/drivers/clk/imx/clk.h
@@ -45,6 +45,7 @@ enum imx_pll14xx_type {
enum imx_pllv4_type {
IMX_PLLV4_IMX7ULP,
IMX_PLLV4_IMX8ULP,
+ IMX_PLLV4_IMX8ULP_1GHZ,
};

enum imx_pfdv2_type {
--
2.37.1



2023-07-20 01:53:53

by Peng Fan

[permalink] [raw]
Subject: RE: [PATCH 1/2] clk: imx: pllv4: Fix SPLL2 MULT range

Hi Abel, Stephen,

> Subject: [PATCH 1/2] clk: imx: pllv4: Fix SPLL2 MULT range

Would you give a look at this patchset?

Thanks,
Peng.

>
> From: Ye Li <[email protected]>
>
> The SPLL2 on iMX8ULP is different with other frac PLLs, it can support VCO
> from 650Mhz to 1Ghz. According to RM, the MULT is using a range from 27
> to 54, not some fixed values. If using current PLL implementation, some
> clock rate can't be supported.
>
> Fix the issue by adding new type for the SPLL2 and use MULT range to
> replace MULT table
>
> Fixes: 5f0601c47c33 ("clk: imx: Update the pllv4 to support imx8ulp")
> Reviewed-by: Peng Fan <[email protected]>
> Reviewed-by: Jacky Bai <[email protected]>
> Signed-off-by: Ye Li <[email protected]>
> Signed-off-by: Peng Fan <[email protected]>
> ---
> drivers/clk/imx/clk-pllv4.c | 46 +++++++++++++++++++++++++++++--------
> drivers/clk/imx/clk.h | 1 +
> 2 files changed, 37 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/clk/imx/clk-pllv4.c b/drivers/clk/imx/clk-pllv4.c index
> 6e7e34571fc8..9b136c951762 100644
> --- a/drivers/clk/imx/clk-pllv4.c
> +++ b/drivers/clk/imx/clk-pllv4.c
> @@ -44,11 +44,15 @@ struct clk_pllv4 {
> u32 cfg_offset;
> u32 num_offset;
> u32 denom_offset;
> + bool use_mult_range;
> };
>
> /* Valid PLL MULT Table */
> static const int pllv4_mult_table[] = {33, 27, 22, 20, 17, 16};
>
> +/* Valid PLL MULT range, (max, min) */
> +static const int pllv4_mult_range[] = {54, 27};
> +
> #define to_clk_pllv4(__hw) container_of(__hw, struct clk_pllv4, hw)
>
> #define LOCK_TIMEOUT_US USEC_PER_MSEC
> @@ -94,17 +98,30 @@ static unsigned long clk_pllv4_recalc_rate(struct
> clk_hw *hw, static long clk_pllv4_round_rate(struct clk_hw *hw, unsigned
> long rate,
> unsigned long *prate)
> {
> + struct clk_pllv4 *pll = to_clk_pllv4(hw);
> unsigned long parent_rate = *prate;
> unsigned long round_rate, i;
> u32 mfn, mfd = DEFAULT_MFD;
> bool found = false;
> u64 temp64;
> -
> - for (i = 0; i < ARRAY_SIZE(pllv4_mult_table); i++) {
> - round_rate = parent_rate * pllv4_mult_table[i];
> - if (rate >= round_rate) {
> + u32 mult;
> +
> + if (pll->use_mult_range) {
> + temp64 = (u64)rate;
> + do_div(temp64, parent_rate);
> + mult = temp64;
> + if (mult >= pllv4_mult_range[1] &&
> + mult <= pllv4_mult_range[0]) {
> + round_rate = parent_rate * mult;
> found = true;
> - break;
> + }
> + } else {
> + for (i = 0; i < ARRAY_SIZE(pllv4_mult_table); i++) {
> + round_rate = parent_rate * pllv4_mult_table[i];
> + if (rate >= round_rate) {
> + found = true;
> + break;
> + }
> }
> }
>
> @@ -138,14 +155,20 @@ static long clk_pllv4_round_rate(struct clk_hw
> *hw, unsigned long rate,
> return round_rate + (u32)temp64;
> }
>
> -static bool clk_pllv4_is_valid_mult(unsigned int mult)
> +static bool clk_pllv4_is_valid_mult(struct clk_pllv4 *pll, unsigned int
> +mult)
> {
> int i;
>
> /* check if mult is in valid MULT table */
> - for (i = 0; i < ARRAY_SIZE(pllv4_mult_table); i++) {
> - if (pllv4_mult_table[i] == mult)
> + if (pll->use_mult_range) {
> + if (mult >= pllv4_mult_range[1] &&
> + mult <= pllv4_mult_range[0])
> return true;
> + } else {
> + for (i = 0; i < ARRAY_SIZE(pllv4_mult_table); i++) {
> + if (pllv4_mult_table[i] == mult)
> + return true;
> + }
> }
>
> return false;
> @@ -160,7 +183,7 @@ static int clk_pllv4_set_rate(struct clk_hw *hw,
> unsigned long rate,
>
> mult = rate / parent_rate;
>
> - if (!clk_pllv4_is_valid_mult(mult))
> + if (!clk_pllv4_is_valid_mult(pll, mult))
> return -EINVAL;
>
> if (parent_rate <= MAX_MFD)
> @@ -227,10 +250,13 @@ struct clk_hw *imx_clk_hw_pllv4(enum
> imx_pllv4_type type, const char *name,
>
> pll->base = base;
>
> - if (type == IMX_PLLV4_IMX8ULP) {
> + if (type == IMX_PLLV4_IMX8ULP ||
> + type == IMX_PLLV4_IMX8ULP_1GHZ) {
> pll->cfg_offset = IMX8ULP_PLL_CFG_OFFSET;
> pll->num_offset = IMX8ULP_PLL_NUM_OFFSET;
> pll->denom_offset = IMX8ULP_PLL_DENOM_OFFSET;
> + if (type == IMX_PLLV4_IMX8ULP_1GHZ)
> + pll->use_mult_range = true;
> } else {
> pll->cfg_offset = PLL_CFG_OFFSET;
> pll->num_offset = PLL_NUM_OFFSET;
> diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h index
> af19d9f6aed0..adb7ad649a0d 100644
> --- a/drivers/clk/imx/clk.h
> +++ b/drivers/clk/imx/clk.h
> @@ -45,6 +45,7 @@ enum imx_pll14xx_type { enum imx_pllv4_type {
> IMX_PLLV4_IMX7ULP,
> IMX_PLLV4_IMX8ULP,
> + IMX_PLLV4_IMX8ULP_1GHZ,
> };
>
> enum imx_pfdv2_type {
> --
> 2.37.1


2023-07-25 09:19:00

by Abel Vesa

[permalink] [raw]
Subject: Re: [PATCH 1/2] clk: imx: pllv4: Fix SPLL2 MULT range

On 23-06-25 20:33:39, Peng Fan (OSS) wrote:
> From: Ye Li <[email protected]>
>
> The SPLL2 on iMX8ULP is different with other frac PLLs, it can
> support VCO from 650Mhz to 1Ghz. According to RM, the MULT is
> using a range from 27 to 54, not some fixed values. If using
> current PLL implementation, some clock rate can't be supported.
>
> Fix the issue by adding new type for the SPLL2 and use MULT range
> to replace MULT table
>
> Fixes: 5f0601c47c33 ("clk: imx: Update the pllv4 to support imx8ulp")
> Reviewed-by: Peng Fan <[email protected]>
> Reviewed-by: Jacky Bai <[email protected]>
> Signed-off-by: Ye Li <[email protected]>
> Signed-off-by: Peng Fan <[email protected]>

LGTM.

Reviewed-by: Abel Vesa <[email protected]>

> ---
> drivers/clk/imx/clk-pllv4.c | 46 +++++++++++++++++++++++++++++--------
> drivers/clk/imx/clk.h | 1 +
> 2 files changed, 37 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/clk/imx/clk-pllv4.c b/drivers/clk/imx/clk-pllv4.c
> index 6e7e34571fc8..9b136c951762 100644
> --- a/drivers/clk/imx/clk-pllv4.c
> +++ b/drivers/clk/imx/clk-pllv4.c
> @@ -44,11 +44,15 @@ struct clk_pllv4 {
> u32 cfg_offset;
> u32 num_offset;
> u32 denom_offset;
> + bool use_mult_range;
> };
>
> /* Valid PLL MULT Table */
> static const int pllv4_mult_table[] = {33, 27, 22, 20, 17, 16};
>
> +/* Valid PLL MULT range, (max, min) */
> +static const int pllv4_mult_range[] = {54, 27};
> +
> #define to_clk_pllv4(__hw) container_of(__hw, struct clk_pllv4, hw)
>
> #define LOCK_TIMEOUT_US USEC_PER_MSEC
> @@ -94,17 +98,30 @@ static unsigned long clk_pllv4_recalc_rate(struct clk_hw *hw,
> static long clk_pllv4_round_rate(struct clk_hw *hw, unsigned long rate,
> unsigned long *prate)
> {
> + struct clk_pllv4 *pll = to_clk_pllv4(hw);
> unsigned long parent_rate = *prate;
> unsigned long round_rate, i;
> u32 mfn, mfd = DEFAULT_MFD;
> bool found = false;
> u64 temp64;
> -
> - for (i = 0; i < ARRAY_SIZE(pllv4_mult_table); i++) {
> - round_rate = parent_rate * pllv4_mult_table[i];
> - if (rate >= round_rate) {
> + u32 mult;
> +
> + if (pll->use_mult_range) {
> + temp64 = (u64)rate;
> + do_div(temp64, parent_rate);
> + mult = temp64;
> + if (mult >= pllv4_mult_range[1] &&
> + mult <= pllv4_mult_range[0]) {
> + round_rate = parent_rate * mult;
> found = true;
> - break;
> + }
> + } else {
> + for (i = 0; i < ARRAY_SIZE(pllv4_mult_table); i++) {
> + round_rate = parent_rate * pllv4_mult_table[i];
> + if (rate >= round_rate) {
> + found = true;
> + break;
> + }
> }
> }
>
> @@ -138,14 +155,20 @@ static long clk_pllv4_round_rate(struct clk_hw *hw, unsigned long rate,
> return round_rate + (u32)temp64;
> }
>
> -static bool clk_pllv4_is_valid_mult(unsigned int mult)
> +static bool clk_pllv4_is_valid_mult(struct clk_pllv4 *pll, unsigned int mult)
> {
> int i;
>
> /* check if mult is in valid MULT table */
> - for (i = 0; i < ARRAY_SIZE(pllv4_mult_table); i++) {
> - if (pllv4_mult_table[i] == mult)
> + if (pll->use_mult_range) {
> + if (mult >= pllv4_mult_range[1] &&
> + mult <= pllv4_mult_range[0])
> return true;
> + } else {
> + for (i = 0; i < ARRAY_SIZE(pllv4_mult_table); i++) {
> + if (pllv4_mult_table[i] == mult)
> + return true;
> + }
> }
>
> return false;
> @@ -160,7 +183,7 @@ static int clk_pllv4_set_rate(struct clk_hw *hw, unsigned long rate,
>
> mult = rate / parent_rate;
>
> - if (!clk_pllv4_is_valid_mult(mult))
> + if (!clk_pllv4_is_valid_mult(pll, mult))
> return -EINVAL;
>
> if (parent_rate <= MAX_MFD)
> @@ -227,10 +250,13 @@ struct clk_hw *imx_clk_hw_pllv4(enum imx_pllv4_type type, const char *name,
>
> pll->base = base;
>
> - if (type == IMX_PLLV4_IMX8ULP) {
> + if (type == IMX_PLLV4_IMX8ULP ||
> + type == IMX_PLLV4_IMX8ULP_1GHZ) {
> pll->cfg_offset = IMX8ULP_PLL_CFG_OFFSET;
> pll->num_offset = IMX8ULP_PLL_NUM_OFFSET;
> pll->denom_offset = IMX8ULP_PLL_DENOM_OFFSET;
> + if (type == IMX_PLLV4_IMX8ULP_1GHZ)
> + pll->use_mult_range = true;
> } else {
> pll->cfg_offset = PLL_CFG_OFFSET;
> pll->num_offset = PLL_NUM_OFFSET;
> diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h
> index af19d9f6aed0..adb7ad649a0d 100644
> --- a/drivers/clk/imx/clk.h
> +++ b/drivers/clk/imx/clk.h
> @@ -45,6 +45,7 @@ enum imx_pll14xx_type {
> enum imx_pllv4_type {
> IMX_PLLV4_IMX7ULP,
> IMX_PLLV4_IMX8ULP,
> + IMX_PLLV4_IMX8ULP_1GHZ,
> };
>
> enum imx_pfdv2_type {
> --
> 2.37.1
>

2023-08-14 11:01:18

by Abel Vesa

[permalink] [raw]
Subject: Re: [PATCH 1/2] clk: imx: pllv4: Fix SPLL2 MULT range


On Sun, 25 Jun 2023 20:33:39 +0800, Peng Fan (OSS) wrote:
> The SPLL2 on iMX8ULP is different with other frac PLLs, it can
> support VCO from 650Mhz to 1Ghz. According to RM, the MULT is
> using a range from 27 to 54, not some fixed values. If using
> current PLL implementation, some clock rate can't be supported.
>
> Fix the issue by adding new type for the SPLL2 and use MULT range
> to replace MULT table
>
> [...]

Applied, thanks!

[1/2] clk: imx: pllv4: Fix SPLL2 MULT range
commit: 3f0cdb945471f1abd1cf4d172190e9c489c5052a
[2/2] clk: imx: imx8ulp: update SPLL2 type
commit: 7653a59be8af043adc4c09473975a860e6055ff9

Best regards,
--
Abel Vesa <[email protected]>