Re: [PATCH 4/5] clk: sunxi-ng: a64: Add constraints on PLL-VIDEO0's n/m ratio
Dne nedelja, 31. december 2023 ob 10:10:40 CET je Frank Oltmanns napisal(a): > > On 2023-12-19 at 17:54:19 +0100, Jernej Škrabec > wrote: > > Dne ponedeljek, 18. december 2023 ob 14:35:22 CET je Frank Oltmanns > > napisal(a): > >> The Allwinner A64 manual lists the following constraint for the > >> PLL-VIDEO0 clock: 8 <= N/M <= 25 > >> > >> Use this constraint. > >> > >> Signed-off-by: Frank Oltmanns > >> --- > >> drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 8 ++-- > >> 1 file changed, 6 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c > >> b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c > >> index c034ac027d1c..75d839da446c 100644 > >> --- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c > >> +++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c > >> @@ -68,7 +68,8 @@ static > >> SUNXI_CCU_NM_WITH_SDM_GATE_LOCK(pll_audio_base_clk, "pll-audio-base", > >> BIT(28), /* lock */ > >> CLK_SET_RATE_UNGATE); > >> > >> -static SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_CLOSEST(pll_video0_clk, > >> "pll-video0", > >> +static > >> SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_FEAT_NM_RATIO(pll_video0_clk, > >> + "pll-video0", > >>"osc24M", 0x010, > >>19200, /* Minimum rate > >> */ > >>100800, /* Maximum rate > >> */ > > I just realized that adding the whole ratio limits for ccu_nm is > superfluous as you could just as well express them in for of a minimum > and maximum range: > Since 8 <= N/M <= 25 and parent_rate = 24 MHz, therefore > 192 MHz <= rate <= 600 MHz. Good point! > > These absolute limits are also listed in Allwinner's A64 manual. > > BUT, here the upper limit was raised to 1008 MHz: > 5de39acaf34604bd04834f092479cf4dcc946dd "clk: sunxi-ng: a64: Add max. > rate constraint to video PLL" > > With this upper limit the ratio limitation is effectively: > 8 <= N/M <= 42 > > Icenowy Zheng (added to CC) had the reasonable explanation that this was > used in the BSP kernel, so we should probably stick to that and ditch > the two PLL-VIDEO0 related patches. What are your thoughts on that? Ok, it seems that these patches are really superfluous. Remove them for v2. Best regards, Jernej > > >> @@ -80,7 +81,10 @@ static > >> SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_CLOSEST(pll_video0_clk, "pll-vid > >>29700, /* frac rate 1 > >> */ > >>BIT(31),/* gate */ > >>BIT(28),/* lock */ > >> - CLK_SET_RATE_UNGATE); > >> + CLK_SET_RATE_UNGATE, > >> + CCU_FEATURE_FRACTIONAL | > >> + CCU_FEATURE_CLOSEST_RATE, > > > > Above flags are unrelated change, put them in new patch if needed. > > > > Best regards, > > Jernej > > > >> + 8, 25); /* min/max nm > >> ratio */ > >> > >> static SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK(pll_ve_clk, "pll-ve", > >>"osc24M", 0x018, > >> > >> >
Re: [PATCH 4/5] clk: sunxi-ng: a64: Add constraints on PLL-VIDEO0's n/m ratio
On 2023-12-19 at 17:54:19 +0100, Jernej Škrabec wrote: > Dne ponedeljek, 18. december 2023 ob 14:35:22 CET je Frank Oltmanns > napisal(a): >> The Allwinner A64 manual lists the following constraint for the >> PLL-VIDEO0 clock: 8 <= N/M <= 25 >> >> Use this constraint. >> >> Signed-off-by: Frank Oltmanns >> --- >> drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 8 ++-- >> 1 file changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c >> b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c >> index c034ac027d1c..75d839da446c 100644 >> --- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c >> +++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c >> @@ -68,7 +68,8 @@ static SUNXI_CCU_NM_WITH_SDM_GATE_LOCK(pll_audio_base_clk, >> "pll-audio-base", >> BIT(28), /* lock */ >> CLK_SET_RATE_UNGATE); >> >> -static SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_CLOSEST(pll_video0_clk, >> "pll-video0", >> +static >> SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_FEAT_NM_RATIO(pll_video0_clk, >> +"pll-video0", >> "osc24M", 0x010, >> 19200, /* Minimum rate >> */ >> 100800, /* Maximum rate >> */ I just realized that adding the whole ratio limits for ccu_nm is superfluous as you could just as well express them in for of a minimum and maximum range: Since 8 <= N/M <= 25 and parent_rate = 24 MHz, therefore 192 MHz <= rate <= 600 MHz. These absolute limits are also listed in Allwinner's A64 manual. BUT, here the upper limit was raised to 1008 MHz: 5de39acaf34604bd04834f092479cf4dcc946dd "clk: sunxi-ng: a64: Add max. rate constraint to video PLL" With this upper limit the ratio limitation is effectively: 8 <= N/M <= 42 Icenowy Zheng (added to CC) had the reasonable explanation that this was used in the BSP kernel, so we should probably stick to that and ditch the two PLL-VIDEO0 related patches. What are your thoughts on that? >> @@ -80,7 +81,10 @@ static >> SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_CLOSEST(pll_video0_clk, "pll-vid >> 29700, /* frac rate 1 >> */ >> BIT(31),/* gate */ >> BIT(28),/* lock */ >> -CLK_SET_RATE_UNGATE); >> +CLK_SET_RATE_UNGATE, >> +CCU_FEATURE_FRACTIONAL | >> +CCU_FEATURE_CLOSEST_RATE, > > Above flags are unrelated change, put them in new patch if needed. > > Best regards, > Jernej > >> +8, 25); /* min/max nm >> ratio */ >> >> static SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK(pll_ve_clk, "pll-ve", >> "osc24M", 0x018, >> >>
Re: [PATCH 4/5] clk: sunxi-ng: a64: Add constraints on PLL-VIDEO0's n/m ratio
On 2023-12-20 at 16:12:42 +0100, Jernej Škrabec wrote: > Dne sreda, 20. december 2023 ob 08:09:28 CET je Frank Oltmanns napisal(a): >> >> On 2023-12-19 at 17:54:19 +0100, Jernej Škrabec >> wrote: >> > Dne ponedeljek, 18. december 2023 ob 14:35:22 CET je Frank Oltmanns >> > napisal(a): >> >> The Allwinner A64 manual lists the following constraint for the >> >> PLL-VIDEO0 clock: 8 <= N/M <= 25 >> >> >> >> Use this constraint. >> >> >> >> Signed-off-by: Frank Oltmanns >> >> --- >> >> drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 8 ++-- >> >> 1 file changed, 6 insertions(+), 2 deletions(-) >> >> >> >> diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c >> >> b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c >> >> index c034ac027d1c..75d839da446c 100644 >> >> --- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c >> >> +++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c >> >> @@ -68,7 +68,8 @@ static >> >> SUNXI_CCU_NM_WITH_SDM_GATE_LOCK(pll_audio_base_clk, "pll-audio-base", >> >> BIT(28), /* lock */ >> >> CLK_SET_RATE_UNGATE); >> >> >> >> -static SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_CLOSEST(pll_video0_clk, >> >> "pll-video0", >> >> +static >> >> SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_FEAT_NM_RATIO(pll_video0_clk, >> >> + "pll-video0", >> >> "osc24M", 0x010, >> >> 19200, /* Minimum rate >> >> */ >> >> 100800, /* Maximum rate >> >> */ >> >> @@ -80,7 +81,10 @@ static >> >> SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_CLOSEST(pll_video0_clk, "pll-vid >> >> 29700, /* frac rate 1 >> >> */ >> >> BIT(31),/* gate */ >> >> BIT(28),/* lock */ >> >> - CLK_SET_RATE_UNGATE); >> >> + CLK_SET_RATE_UNGATE, >> >> + CCU_FEATURE_FRACTIONAL | >> >> + CCU_FEATURE_CLOSEST_RATE, >> > >> > Above flags are unrelated change, put them in new patch if needed. >> >> You might notice that I am using a new macro for initializing the >> pll_video0_clk struct: >> New: SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_FEAT_NM_RATIO >> Old: SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_CLOSEST >> >> Setting the two CCU_FEATURE flags is part of the old initialization >> macro. >> >> I'll add SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_NM_RATIO_CLOSEST which >> hopefully resolves the confusion. > > I'm in doubt if we need so many macros. How many users of these macro we'll > have? > I see that R40 SoC would also need same ratio limits, but other that that, > none? Ok, IIUC no additional macro and we keep this part of the patch as is. Best regards, Frank > > Best regards, > Jernej > >> >> Thanks, >> Frank >> >> > >> > Best regards, >> > Jernej >> > >> >> + 8, 25); /* min/max nm >> >> ratio */ >> >> >> >> static SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK(pll_ve_clk, "pll-ve", >> >> "osc24M", 0x018, >> >> >> >> >>
Re: [PATCH 4/5] clk: sunxi-ng: a64: Add constraints on PLL-VIDEO0's n/m ratio
Dne sreda, 20. december 2023 ob 08:09:28 CET je Frank Oltmanns napisal(a): > > On 2023-12-19 at 17:54:19 +0100, Jernej Škrabec > wrote: > > Dne ponedeljek, 18. december 2023 ob 14:35:22 CET je Frank Oltmanns > > napisal(a): > >> The Allwinner A64 manual lists the following constraint for the > >> PLL-VIDEO0 clock: 8 <= N/M <= 25 > >> > >> Use this constraint. > >> > >> Signed-off-by: Frank Oltmanns > >> --- > >> drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 8 ++-- > >> 1 file changed, 6 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c > >> b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c > >> index c034ac027d1c..75d839da446c 100644 > >> --- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c > >> +++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c > >> @@ -68,7 +68,8 @@ static > >> SUNXI_CCU_NM_WITH_SDM_GATE_LOCK(pll_audio_base_clk, "pll-audio-base", > >> BIT(28), /* lock */ > >> CLK_SET_RATE_UNGATE); > >> > >> -static SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_CLOSEST(pll_video0_clk, > >> "pll-video0", > >> +static > >> SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_FEAT_NM_RATIO(pll_video0_clk, > >> + "pll-video0", > >>"osc24M", 0x010, > >>19200, /* Minimum rate > >> */ > >>100800, /* Maximum rate > >> */ > >> @@ -80,7 +81,10 @@ static > >> SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_CLOSEST(pll_video0_clk, "pll-vid > >>29700, /* frac rate 1 > >> */ > >>BIT(31),/* gate */ > >>BIT(28),/* lock */ > >> - CLK_SET_RATE_UNGATE); > >> + CLK_SET_RATE_UNGATE, > >> + CCU_FEATURE_FRACTIONAL | > >> + CCU_FEATURE_CLOSEST_RATE, > > > > Above flags are unrelated change, put them in new patch if needed. > > You might notice that I am using a new macro for initializing the > pll_video0_clk struct: > New: SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_FEAT_NM_RATIO > Old: SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_CLOSEST > > Setting the two CCU_FEATURE flags is part of the old initialization > macro. > > I'll add SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_NM_RATIO_CLOSEST which > hopefully resolves the confusion. I'm in doubt if we need so many macros. How many users of these macro we'll have? I see that R40 SoC would also need same ratio limits, but other that that, none? Best regards, Jernej > > Thanks, > Frank > > > > > Best regards, > > Jernej > > > >> + 8, 25); /* min/max nm > >> ratio */ > >> > >> static SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK(pll_ve_clk, "pll-ve", > >>"osc24M", 0x018, > >> > >> >
Re: [PATCH 4/5] clk: sunxi-ng: a64: Add constraints on PLL-VIDEO0's n/m ratio
On 2023-12-19 at 17:54:19 +0100, Jernej Škrabec wrote: > Dne ponedeljek, 18. december 2023 ob 14:35:22 CET je Frank Oltmanns > napisal(a): >> The Allwinner A64 manual lists the following constraint for the >> PLL-VIDEO0 clock: 8 <= N/M <= 25 >> >> Use this constraint. >> >> Signed-off-by: Frank Oltmanns >> --- >> drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 8 ++-- >> 1 file changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c >> b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c >> index c034ac027d1c..75d839da446c 100644 >> --- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c >> +++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c >> @@ -68,7 +68,8 @@ static SUNXI_CCU_NM_WITH_SDM_GATE_LOCK(pll_audio_base_clk, >> "pll-audio-base", >> BIT(28), /* lock */ >> CLK_SET_RATE_UNGATE); >> >> -static SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_CLOSEST(pll_video0_clk, >> "pll-video0", >> +static >> SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_FEAT_NM_RATIO(pll_video0_clk, >> +"pll-video0", >> "osc24M", 0x010, >> 19200, /* Minimum rate >> */ >> 100800, /* Maximum rate >> */ >> @@ -80,7 +81,10 @@ static >> SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_CLOSEST(pll_video0_clk, "pll-vid >> 29700, /* frac rate 1 >> */ >> BIT(31),/* gate */ >> BIT(28),/* lock */ >> -CLK_SET_RATE_UNGATE); >> +CLK_SET_RATE_UNGATE, >> +CCU_FEATURE_FRACTIONAL | >> +CCU_FEATURE_CLOSEST_RATE, > > Above flags are unrelated change, put them in new patch if needed. You might notice that I am using a new macro for initializing the pll_video0_clk struct: New: SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_FEAT_NM_RATIO Old: SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_CLOSEST Setting the two CCU_FEATURE flags is part of the old initialization macro. I'll add SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_NM_RATIO_CLOSEST which hopefully resolves the confusion. Thanks, Frank > > Best regards, > Jernej > >> +8, 25); /* min/max nm >> ratio */ >> >> static SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK(pll_ve_clk, "pll-ve", >> "osc24M", 0x018, >> >>
Re: [PATCH 4/5] clk: sunxi-ng: a64: Add constraints on PLL-VIDEO0's n/m ratio
Dne ponedeljek, 18. december 2023 ob 14:35:22 CET je Frank Oltmanns napisal(a): > The Allwinner A64 manual lists the following constraint for the > PLL-VIDEO0 clock: 8 <= N/M <= 25 > > Use this constraint. > > Signed-off-by: Frank Oltmanns > --- > drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 8 ++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c > b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c > index c034ac027d1c..75d839da446c 100644 > --- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c > +++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c > @@ -68,7 +68,8 @@ static SUNXI_CCU_NM_WITH_SDM_GATE_LOCK(pll_audio_base_clk, > "pll-audio-base", > BIT(28), /* lock */ > CLK_SET_RATE_UNGATE); > > -static SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_CLOSEST(pll_video0_clk, > "pll-video0", > +static SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_FEAT_NM_RATIO(pll_video0_clk, > + "pll-video0", > "osc24M", 0x010, > 19200, /* Minimum rate > */ > 100800, /* Maximum rate > */ > @@ -80,7 +81,10 @@ static > SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_CLOSEST(pll_video0_clk, "pll-vid > 29700, /* frac rate 1 > */ > BIT(31),/* gate */ > BIT(28),/* lock */ > - CLK_SET_RATE_UNGATE); > + CLK_SET_RATE_UNGATE, > + CCU_FEATURE_FRACTIONAL | > + CCU_FEATURE_CLOSEST_RATE, Above flags are unrelated change, put them in new patch if needed. Best regards, Jernej > + 8, 25); /* min/max nm > ratio */ > > static SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK(pll_ve_clk, "pll-ve", > "osc24M", 0x018, > >
[PATCH 4/5] clk: sunxi-ng: a64: Add constraints on PLL-VIDEO0's n/m ratio
The Allwinner A64 manual lists the following constraint for the PLL-VIDEO0 clock: 8 <= N/M <= 25 Use this constraint. Signed-off-by: Frank Oltmanns --- drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c index c034ac027d1c..75d839da446c 100644 --- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c +++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c @@ -68,7 +68,8 @@ static SUNXI_CCU_NM_WITH_SDM_GATE_LOCK(pll_audio_base_clk, "pll-audio-base", BIT(28), /* lock */ CLK_SET_RATE_UNGATE); -static SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_CLOSEST(pll_video0_clk, "pll-video0", +static SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_FEAT_NM_RATIO(pll_video0_clk, + "pll-video0", "osc24M", 0x010, 19200, /* Minimum rate */ 100800, /* Maximum rate */ @@ -80,7 +81,10 @@ static SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK_MIN_MAX_CLOSEST(pll_video0_clk, "pll-vid 29700, /* frac rate 1 */ BIT(31),/* gate */ BIT(28),/* lock */ - CLK_SET_RATE_UNGATE); + CLK_SET_RATE_UNGATE, + CCU_FEATURE_FRACTIONAL | + CCU_FEATURE_CLOSEST_RATE, + 8, 25); /* min/max nm ratio */ static SUNXI_CCU_NM_WITH_FRAC_GATE_LOCK(pll_ve_clk, "pll-ve", "osc24M", 0x018, -- 2.43.0
