[PATCH] rockchip: clk: add UART0 clock getter/setter
Add dedicated getter and setter for SCLK_UART0_PMU. This allows the driver to correctly handle UART0 clocks, and thus it fixes the issues with UART0 not working in case DEBUG_UART is disabled. Unlike other Rockchip SoCs, i.e. rk3399, in the PX30 the default clock source for UART is GPLL, instead of external oscillator. If the DEBUG_UART is enabled, the clock source is changed in board_debug_uart_init function to 24Mhz oscillator, which also matches the fallback value obtained from DT node. In case the DEBUG_UART is disabled, the UART clock source remains default, and the DM serial driver wrongly configures the baud rate, resulting in broken communication. By implementing the UART clock getter/setter, the serial driver can probe the actual configuration and corectly configure itself. The DEBUG_UART settings now should not affect it.
The driver supports GPLL and 24M oscillator. NPLL and USBPHY480M sources, that are managed by CRU, are not yet handled, as likely they won't be used in real scenarios.
Signed-off-by: Lukasz Czechowski lukasz.czechowski@thaumatec.com --- arch/arm/include/asm/arch-rockchip/cru_px30.h | 7 ++ drivers/clk/rockchip/clk_px30.c | 107 ++++++++++++++++++ 2 files changed, 114 insertions(+)
diff --git a/arch/arm/include/asm/arch-rockchip/cru_px30.h b/arch/arm/include/asm/arch-rockchip/cru_px30.h index b66277fc7f3..504459bd93d 100644 --- a/arch/arm/include/asm/arch-rockchip/cru_px30.h +++ b/arch/arm/include/asm/arch-rockchip/cru_px30.h @@ -464,5 +464,12 @@ enum { UART0_CLK_SEL_UART0_FRAC, UART0_DIVNP5_SHIFT = 0, UART0_DIVNP5_MASK = 0x1f << UART0_DIVNP5_SHIFT, + + /* CRU_PMU_CLKSEL5_CON */ + CLK_UART_FRAC_NUMERATOR_SHIFT = 16, + CLK_UART_FRAC_NUMERATOR_MASK = 0xffff << CLK_UART_FRAC_NUMERATOR_SHIFT, + CLK_UART_FRAC_DENOMINATOR_SHIFT = 0, + CLK_UART_FRAC_DENOMINATOR_MASK = + 0xffff << CLK_UART_FRAC_DENOMINATOR_SHIFT, }; #endif diff --git a/drivers/clk/rockchip/clk_px30.c b/drivers/clk/rockchip/clk_px30.c index 2875c152b20..64f1a335cb0 100644 --- a/drivers/clk/rockchip/clk_px30.c +++ b/drivers/clk/rockchip/clk_px30.c @@ -1589,6 +1589,107 @@ static ulong px30_pmuclk_set_gpll_rate(struct px30_pmuclk_priv *priv, ulong hz) return priv->gpll_hz; }
+static ulong px30_pmu_uart0_get_clk(struct px30_pmuclk_priv *priv) +{ + struct px30_pmucru *pmucru = priv->pmucru; + u32 clk_uart0_div_con; + u32 clk_uart0_pll_sel; + u32 clk_uart0_sel; + u32 con; + ulong pll_rate; + ulong clk_uart0; + + con = readl(&pmucru->pmu_clksel_con[3]); + clk_uart0_div_con = bitfield_extract_by_mask(con, UART0_DIV_CON_MASK); + clk_uart0_pll_sel = bitfield_extract_by_mask(con, UART0_PLL_SEL_MASK); + + switch (clk_uart0_pll_sel) { + case UART0_PLL_SEL_GPLL: + pll_rate = px30_pmuclk_get_gpll_rate(priv); + break; + case UART0_PLL_SEL_24M: + pll_rate = OSC_HZ; + break; + case UART0_PLL_SEL_480M: + case UART0_PLL_SEL_NPLL: + /* usbphy480M and NPLL clocks, generated by CRU, are not supported yet */ + default: + return -ENOENT; + } + + clk_uart0 = DIV_TO_RATE(pll_rate, clk_uart0_div_con); + con = readl(&pmucru->pmu_clksel_con[4]); + clk_uart0_sel = bitfield_extract_by_mask(con, UART0_CLK_SEL_MASK); + + switch (clk_uart0_sel) { + case UART0_CLK_SEL_UART0: + return clk_uart0; + case UART0_CLK_SEL_UART0_NP5:{ + u32 clk_uart0_divnp5_div_con; + + clk_uart0_divnp5_div_con = + bitfield_extract_by_mask(con, UART0_DIVNP5_MASK); + return DIV_TO_RATE((2 * clk_uart0), + (2 * clk_uart0_divnp5_div_con + 3)); + } + case UART0_CLK_SEL_UART0_FRAC:{ + u32 fracdiv, n, m; + + fracdiv = readl(&pmucru->pmu_clksel_con[5]); + n = bitfield_extract_by_mask(fracdiv, + CLK_UART_FRAC_NUMERATOR_MASK); + m = bitfield_extract_by_mask(fracdiv, + CLK_UART_FRAC_DENOMINATOR_MASK); + return (u64) clk_uart0 * n / m; + } + default: + return -ENOENT; + } +} + +static ulong px30_pmu_uart0_set_clk(struct px30_pmuclk_priv *priv, ulong rate) +{ + struct px30_pmucru *pmucru = priv->pmucru; + ulong gpll_rate; + u32 clk_uart0_div_con; + u32 clk_uart0_pll_sel; + u32 clk_uart0_sel; + ulong m = 0, n = 0; + + gpll_rate = px30_pmuclk_get_gpll_rate(priv); + if (gpll_rate % rate == 0) { + clk_uart0_pll_sel = UART0_PLL_SEL_GPLL; + clk_uart0_sel = UART0_CLK_SEL_UART0; + clk_uart0_div_con = DIV_ROUND_UP(priv->gpll_hz, rate); + } else if (rate == OSC_HZ) { + clk_uart0_pll_sel = UART0_PLL_SEL_24M; + clk_uart0_sel = UART0_CLK_SEL_UART0; + clk_uart0_div_con = 1; + } else { + clk_uart0_pll_sel = UART0_PLL_SEL_GPLL; + clk_uart0_sel = UART0_CLK_SEL_UART0_FRAC; + clk_uart0_div_con = 1; + rational_best_approximation(rate, priv->gpll_hz, + GENMASK(16 - 1, 0), + GENMASK(16 - 1, 0), &m, &n); + } + + rk_clrsetreg(&pmucru->pmu_clksel_con[3], + UART0_PLL_SEL_MASK | UART0_DIV_CON_MASK, + clk_uart0_pll_sel << UART0_PLL_SEL_SHIFT | + (clk_uart0_div_con - 1)); + rk_clrsetreg(&pmucru->pmu_clksel_con[4], UART0_CLK_SEL_MASK, + clk_uart0_sel << UART0_CLK_SEL_SHIFT); + if (m && n) { + u32 fracdiv; + + fracdiv = m << CLK_UART_FRAC_NUMERATOR_SHIFT | n; + writel(fracdiv, &pmucru->pmu_clksel_con[5]); + } + + return px30_pmu_uart0_get_clk(priv); +} + static ulong px30_pmuclk_get_rate(struct clk *clk) { struct px30_pmuclk_priv *priv = dev_get_priv(clk->dev); @@ -1602,6 +1703,9 @@ static ulong px30_pmuclk_get_rate(struct clk *clk) case PCLK_PMU_PRE: rate = px30_pclk_pmu_get_pmuclk(priv); break; + case SCLK_UART0_PMU: + rate = px30_pmu_uart0_get_clk(priv); + break; default: return -ENOENT; } @@ -1622,6 +1726,9 @@ static ulong px30_pmuclk_set_rate(struct clk *clk, ulong rate) case PCLK_PMU_PRE: ret = px30_pclk_pmu_set_pmuclk(priv, rate); break; + case SCLK_UART0_PMU: + ret = px30_pmu_uart0_set_clk(priv, rate); + break; default: return -ENOENT; }
Hi Lukasz,
On 7/31/24 11:43 AM, Lukasz Czechowski wrote:
Add dedicated getter and setter for SCLK_UART0_PMU. This allows the driver to correctly handle UART0 clocks, and thus it fixes the issues with UART0 not working in case DEBUG_UART is disabled. Unlike other Rockchip SoCs, i.e. rk3399, in the PX30 the default clock source for UART is GPLL, instead of external oscillator. If the DEBUG_UART is enabled, the clock source is changed in board_debug_uart_init function to 24Mhz oscillator, which also matches the fallback value obtained from DT node. In case the DEBUG_UART is disabled, the UART clock source remains default, and the DM serial driver wrongly configures the baud rate, resulting in broken communication. By implementing the UART clock getter/setter, the serial driver can probe the actual configuration and corectly configure itself. The DEBUG_UART settings now should not affect it.
The driver supports GPLL and 24M oscillator. NPLL and USBPHY480M sources, that are managed by CRU, are not yet handled, as likely they won't be used in real scenarios.
Signed-off-by: Lukasz Czechowski lukasz.czechowski@thaumatec.com
arch/arm/include/asm/arch-rockchip/cru_px30.h | 7 ++ drivers/clk/rockchip/clk_px30.c | 107 ++++++++++++++++++ 2 files changed, 114 insertions(+)
diff --git a/arch/arm/include/asm/arch-rockchip/cru_px30.h b/arch/arm/include/asm/arch-rockchip/cru_px30.h index b66277fc7f3..504459bd93d 100644 --- a/arch/arm/include/asm/arch-rockchip/cru_px30.h +++ b/arch/arm/include/asm/arch-rockchip/cru_px30.h @@ -464,5 +464,12 @@ enum { UART0_CLK_SEL_UART0_FRAC, UART0_DIVNP5_SHIFT = 0, UART0_DIVNP5_MASK = 0x1f << UART0_DIVNP5_SHIFT,
- /* CRU_PMU_CLKSEL5_CON */
- CLK_UART_FRAC_NUMERATOR_SHIFT = 16,
- CLK_UART_FRAC_NUMERATOR_MASK = 0xffff << CLK_UART_FRAC_NUMERATOR_SHIFT,
- CLK_UART_FRAC_DENOMINATOR_SHIFT = 0,
- CLK_UART_FRAC_DENOMINATOR_MASK =
0xffff << CLK_UART_FRAC_DENOMINATOR_SHIFT,diff --git a/drivers/clk/rockchip/clk_px30.c b/drivers/clk/rockchip/clk_px30.c index 2875c152b20..64f1a335cb0 100644 --- a/drivers/clk/rockchip/clk_px30.c +++ b/drivers/clk/rockchip/clk_px30.c @@ -1589,6 +1589,107 @@ static ulong px30_pmuclk_set_gpll_rate(struct px30_pmuclk_priv *priv, ulong hz) return priv->gpll_hz; }
+static ulong px30_pmu_uart0_get_clk(struct px30_pmuclk_priv *priv) +{
- struct px30_pmucru *pmucru = priv->pmucru;
- u32 clk_uart0_div_con;
- u32 clk_uart0_pll_sel;
- u32 clk_uart0_sel;
- u32 con;
- ulong pll_rate;
- ulong clk_uart0;
You could group all declarations of the same type on the same line and also respect reverse xmas tree ordering (https://docs.kernel.org/process/maintainer-netdev.html#local-variable-orderi...) (though I don't think we are that strict in U-Boot).
- con = readl(&pmucru->pmu_clksel_con[3]);
- clk_uart0_div_con = bitfield_extract_by_mask(con, UART0_DIV_CON_MASK);
- clk_uart0_pll_sel = bitfield_extract_by_mask(con, UART0_PLL_SEL_MASK);
- switch (clk_uart0_pll_sel) {
- case UART0_PLL_SEL_GPLL:
pll_rate = px30_pmuclk_get_gpll_rate(priv);break;- case UART0_PLL_SEL_24M:
pll_rate = OSC_HZ;break;- case UART0_PLL_SEL_480M:
- case UART0_PLL_SEL_NPLL:
/* usbphy480M and NPLL clocks, generated by CRU, are not supported yet */- default:
return -ENOENT;- }
- clk_uart0 = DIV_TO_RATE(pll_rate, clk_uart0_div_con);
- con = readl(&pmucru->pmu_clksel_con[4]);
- clk_uart0_sel = bitfield_extract_by_mask(con, UART0_CLK_SEL_MASK);
- switch (clk_uart0_sel) {
- case UART0_CLK_SEL_UART0:
return clk_uart0;- case UART0_CLK_SEL_UART0_NP5:{
u32 clk_uart0_divnp5_div_con;clk_uart0_divnp5_div_con =bitfield_extract_by_mask(con, UART0_DIVNP5_MASK);return DIV_TO_RATE((2 * clk_uart0),(2 * clk_uart0_divnp5_div_con + 3));
Are you sure about this DIV_TO_RATE here?
The TRM states:
clk_uart0_divnp5_div_con clk_uart0_np5=2*clk_uart0/(2*div_con+3)
but this would make it clk_uart0_np5=2*clk_uart0/((2*div_con+3) + 1)
I believe?
The kernel seems to be handling it like the TRM says (but I wouldn't be 100% certain here as the clock subsystem and Rockchip drivers have many indirections and I may have gotten lost at some point :) ).
I think we're fine also regarding u32 overflows as we would hit this if the rates were abore 2.1GHz and I think GPLL and NPLL aren't that fast.
} > + case UART0_CLK_SEL_UART0_FRAC:{u32 fracdiv, n, m;fracdiv = readl(&pmucru->pmu_clksel_con[5]);n = bitfield_extract_by_mask(fracdiv,CLK_UART_FRAC_NUMERATOR_MASK);m = bitfield_extract_by_mask(fracdiv,CLK_UART_FRAC_DENOMINATOR_MASK);return (u64) clk_uart0 * n / m;}- default:
return -ENOENT;- }
+}
+static ulong px30_pmu_uart0_set_clk(struct px30_pmuclk_priv *priv, ulong rate) +{
- struct px30_pmucru *pmucru = priv->pmucru;
- ulong gpll_rate;
- u32 clk_uart0_div_con;
- u32 clk_uart0_pll_sel;
- u32 clk_uart0_sel;
- ulong m = 0, n = 0;
- gpll_rate = px30_pmuclk_get_gpll_rate(priv);
- if (gpll_rate % rate == 0) {
clk_uart0_pll_sel = UART0_PLL_SEL_GPLL;clk_uart0_sel = UART0_CLK_SEL_UART0;clk_uart0_div_con = DIV_ROUND_UP(priv->gpll_hz, rate);- } else if (rate == OSC_HZ) {
clk_uart0_pll_sel = UART0_PLL_SEL_24M;clk_uart0_sel = UART0_CLK_SEL_UART0;clk_uart0_div_con = 1;- } else {
clk_uart0_pll_sel = UART0_PLL_SEL_GPLL;clk_uart0_sel = UART0_CLK_SEL_UART0_FRAC;clk_uart0_div_con = 1;rational_best_approximation(rate, priv->gpll_hz,GENMASK(16 - 1, 0),GENMASK(16 - 1, 0), &m, &n);
I guess we could try to find whether OSC_HZ could produce a closer approximation than GPLL but I'm not sure it's worth it right now.
Otherwise looking good, Cheers, Quentin
Hi Lukasz,
Maybe also make it explicit in the commit title that this is for px30 only :)
On 8/1/24 12:26 PM, Quentin Schulz wrote:
Hi Lukasz,
On 7/31/24 11:43 AM, Lukasz Czechowski wrote:
Add dedicated getter and setter for SCLK_UART0_PMU. This allows the driver to correctly handle UART0 clocks, and thus it fixes the issues with UART0 not working in case DEBUG_UART is disabled. Unlike other Rockchip SoCs, i.e. rk3399, in the PX30 the default clock source for UART is GPLL, instead of external oscillator. If the DEBUG_UART is enabled, the clock source is changed in board_debug_uart_init function to 24Mhz oscillator, which also matches the fallback value obtained from DT node. In case the DEBUG_UART is disabled, the UART clock source remains default, and the DM serial driver wrongly configures the baud rate, resulting in broken communication. By implementing the UART clock getter/setter, the serial driver can probe the actual configuration and corectly configure itself. The DEBUG_UART settings now should not affect it.
The driver supports GPLL and 24M oscillator. NPLL and USBPHY480M sources, that are managed by CRU, are not yet handled, as likely they won't be used in real scenarios.
Signed-off-by: Lukasz Czechowski lukasz.czechowski@thaumatec.com
arch/arm/include/asm/arch-rockchip/cru_px30.h | 7 ++ drivers/clk/rockchip/clk_px30.c | 107 ++++++++++++++++++ 2 files changed, 114 insertions(+)
diff --git a/arch/arm/include/asm/arch-rockchip/cru_px30.h b/arch/arm/include/asm/arch-rockchip/cru_px30.h index b66277fc7f3..504459bd93d 100644 --- a/arch/arm/include/asm/arch-rockchip/cru_px30.h +++ b/arch/arm/include/asm/arch-rockchip/cru_px30.h @@ -464,5 +464,12 @@ enum { UART0_CLK_SEL_UART0_FRAC, UART0_DIVNP5_SHIFT = 0, UART0_DIVNP5_MASK = 0x1f << UART0_DIVNP5_SHIFT,
+ /* CRU_PMU_CLKSEL5_CON */ + CLK_UART_FRAC_NUMERATOR_SHIFT = 16, + CLK_UART_FRAC_NUMERATOR_MASK = 0xffff << CLK_UART_FRAC_NUMERATOR_SHIFT, + CLK_UART_FRAC_DENOMINATOR_SHIFT = 0, + CLK_UART_FRAC_DENOMINATOR_MASK = + 0xffff << CLK_UART_FRAC_DENOMINATOR_SHIFT, }; #endif diff --git a/drivers/clk/rockchip/clk_px30.c b/drivers/clk/rockchip/clk_px30.c index 2875c152b20..64f1a335cb0 100644 --- a/drivers/clk/rockchip/clk_px30.c +++ b/drivers/clk/rockchip/clk_px30.c @@ -1589,6 +1589,107 @@ static ulong px30_pmuclk_set_gpll_rate(struct px30_pmuclk_priv *priv, ulong hz) return priv->gpll_hz; } +static ulong px30_pmu_uart0_get_clk(struct px30_pmuclk_priv *priv) +{ + struct px30_pmucru *pmucru = priv->pmucru; + u32 clk_uart0_div_con; + u32 clk_uart0_pll_sel; + u32 clk_uart0_sel; + u32 con; + ulong pll_rate; + ulong clk_uart0;
You could group all declarations of the same type on the same line and also respect reverse xmas tree ordering (https://docs.kernel.org/process/maintainer-netdev.html#local-variable-orderi...) (though I don't think we are that strict in U-Boot).
+ con = readl(&pmucru->pmu_clksel_con[3]); + clk_uart0_div_con = bitfield_extract_by_mask(con, UART0_DIV_CON_MASK); + clk_uart0_pll_sel = bitfield_extract_by_mask(con, UART0_PLL_SEL_MASK);
+ switch (clk_uart0_pll_sel) { + case UART0_PLL_SEL_GPLL: + pll_rate = px30_pmuclk_get_gpll_rate(priv); + break; + case UART0_PLL_SEL_24M: + pll_rate = OSC_HZ; + break; + case UART0_PLL_SEL_480M: + case UART0_PLL_SEL_NPLL: + /* usbphy480M and NPLL clocks, generated by CRU, are not supported yet */ + default: + return -ENOENT; + }
+ clk_uart0 = DIV_TO_RATE(pll_rate, clk_uart0_div_con); + con = readl(&pmucru->pmu_clksel_con[4]); + clk_uart0_sel = bitfield_extract_by_mask(con, UART0_CLK_SEL_MASK);
+ switch (clk_uart0_sel) { + case UART0_CLK_SEL_UART0: + return clk_uart0; + case UART0_CLK_SEL_UART0_NP5:{ + u32 clk_uart0_divnp5_div_con;
+ clk_uart0_divnp5_div_con = + bitfield_extract_by_mask(con, UART0_DIVNP5_MASK); + return DIV_TO_RATE((2 * clk_uart0), + (2 * clk_uart0_divnp5_div_con + 3));
Are you sure about this DIV_TO_RATE here?
The TRM states:
clk_uart0_divnp5_div_con clk_uart0_np5=2*clk_uart0/(2*div_con+3)
but this would make it clk_uart0_np5=2*clk_uart0/((2*div_con+3) + 1)
I believe?
The kernel seems to be handling it like the TRM says (but I wouldn't be 100% certain here as the clock subsystem and Rockchip drivers have many indirections and I may have gotten lost at some point :) ).
I think we're fine also regarding u32 overflows as we would hit this if the rates were abore 2.1GHz and I think GPLL and NPLL aren't that fast.
+ } > + case UART0_CLK_SEL_UART0_FRAC:{ + u32 fracdiv, n, m;
+ fracdiv = readl(&pmucru->pmu_clksel_con[5]); + n = bitfield_extract_by_mask(fracdiv, + CLK_UART_FRAC_NUMERATOR_MASK); + m = bitfield_extract_by_mask(fracdiv, + CLK_UART_FRAC_DENOMINATOR_MASK); + return (u64) clk_uart0 * n / m; + } + default: + return -ENOENT; + } +}
+static ulong px30_pmu_uart0_set_clk(struct px30_pmuclk_priv *priv, ulong rate) +{ + struct px30_pmucru *pmucru = priv->pmucru; + ulong gpll_rate; + u32 clk_uart0_div_con; + u32 clk_uart0_pll_sel; + u32 clk_uart0_sel; + ulong m = 0, n = 0;
+ gpll_rate = px30_pmuclk_get_gpll_rate(priv); + if (gpll_rate % rate == 0) { + clk_uart0_pll_sel = UART0_PLL_SEL_GPLL; + clk_uart0_sel = UART0_CLK_SEL_UART0; + clk_uart0_div_con = DIV_ROUND_UP(priv->gpll_hz, rate); + } else if (rate == OSC_HZ) { + clk_uart0_pll_sel = UART0_PLL_SEL_24M; + clk_uart0_sel = UART0_CLK_SEL_UART0; + clk_uart0_div_con = 1; + } else { + clk_uart0_pll_sel = UART0_PLL_SEL_GPLL; + clk_uart0_sel = UART0_CLK_SEL_UART0_FRAC; + clk_uart0_div_con = 1; + rational_best_approximation(rate, priv->gpll_hz, + GENMASK(16 - 1, 0), + GENMASK(16 - 1, 0), &m, &n);
I guess we could try to find whether OSC_HZ could produce a closer approximation than GPLL but I'm not sure it's worth it right now.
Otherwise looking good, Cheers, Quentin
Hi Quentin, I've submitted the updated version of the patch. Indeed the calculation for UART0_CLK_SEL_UART0_NP5 seemed wrong - I've corrected it. Added also casting to u64, as according to TRM, PLL can reach up to 3,2GHz. Declaration order and commit message are updated. As for the clock source for UART0_CLK_SEL_UART0_FRAC, I'd like to keep things simple, so I left the GPLL configured as clock source. Similarly is already done in the rk3588 clock driver.
Best regards, Lukasz
czw., 1 sie 2024 o 12:27 Quentin Schulz quentin.schulz@cherry.de napisał(a):
Hi Lukasz,
Maybe also make it explicit in the commit title that this is for px30 only :)
On 8/1/24 12:26 PM, Quentin Schulz wrote:
Hi Lukasz,
On 7/31/24 11:43 AM, Lukasz Czechowski wrote:
Add dedicated getter and setter for SCLK_UART0_PMU. This allows the driver to correctly handle UART0 clocks, and thus it fixes the issues with UART0 not working in case DEBUG_UART is disabled. Unlike other Rockchip SoCs, i.e. rk3399, in the PX30 the default clock source for UART is GPLL, instead of external oscillator. If the DEBUG_UART is enabled, the clock source is changed in board_debug_uart_init function to 24Mhz oscillator, which also matches the fallback value obtained from DT node. In case the DEBUG_UART is disabled, the UART clock source remains default, and the DM serial driver wrongly configures the baud rate, resulting in broken communication. By implementing the UART clock getter/setter, the serial driver can probe the actual configuration and corectly configure itself. The DEBUG_UART settings now should not affect it.
The driver supports GPLL and 24M oscillator. NPLL and USBPHY480M sources, that are managed by CRU, are not yet handled, as likely they won't be used in real scenarios.
Signed-off-by: Lukasz Czechowski lukasz.czechowski@thaumatec.com
arch/arm/include/asm/arch-rockchip/cru_px30.h | 7 ++ drivers/clk/rockchip/clk_px30.c | 107 ++++++++++++++++++ 2 files changed, 114 insertions(+)
diff --git a/arch/arm/include/asm/arch-rockchip/cru_px30.h b/arch/arm/include/asm/arch-rockchip/cru_px30.h index b66277fc7f3..504459bd93d 100644 --- a/arch/arm/include/asm/arch-rockchip/cru_px30.h +++ b/arch/arm/include/asm/arch-rockchip/cru_px30.h @@ -464,5 +464,12 @@ enum { UART0_CLK_SEL_UART0_FRAC, UART0_DIVNP5_SHIFT = 0, UART0_DIVNP5_MASK = 0x1f << UART0_DIVNP5_SHIFT,
- /* CRU_PMU_CLKSEL5_CON */
- CLK_UART_FRAC_NUMERATOR_SHIFT = 16,
- CLK_UART_FRAC_NUMERATOR_MASK = 0xffff <<
CLK_UART_FRAC_NUMERATOR_SHIFT,
- CLK_UART_FRAC_DENOMINATOR_SHIFT = 0,
- CLK_UART_FRAC_DENOMINATOR_MASK =
0xffff << CLK_UART_FRAC_DENOMINATOR_SHIFT,diff --git a/drivers/clk/rockchip/clk_px30.c b/drivers/clk/rockchip/clk_px30.c index 2875c152b20..64f1a335cb0 100644 --- a/drivers/clk/rockchip/clk_px30.c +++ b/drivers/clk/rockchip/clk_px30.c @@ -1589,6 +1589,107 @@ static ulong px30_pmuclk_set_gpll_rate(struct px30_pmuclk_priv *priv, ulong hz) return priv->gpll_hz; } +static ulong px30_pmu_uart0_get_clk(struct px30_pmuclk_priv *priv) +{
- struct px30_pmucru *pmucru = priv->pmucru;
- u32 clk_uart0_div_con;
- u32 clk_uart0_pll_sel;
- u32 clk_uart0_sel;
- u32 con;
- ulong pll_rate;
- ulong clk_uart0;
You could group all declarations of the same type on the same line and also respect reverse xmas tree ordering (https://docs.kernel.org/process/maintainer-netdev.html#local-variable-orderi...) (though I don't think we are that strict in U-Boot).
- con = readl(&pmucru->pmu_clksel_con[3]);
- clk_uart0_div_con = bitfield_extract_by_mask(con,
UART0_DIV_CON_MASK);
- clk_uart0_pll_sel = bitfield_extract_by_mask(con,
UART0_PLL_SEL_MASK);
- switch (clk_uart0_pll_sel) {
- case UART0_PLL_SEL_GPLL:
pll_rate = px30_pmuclk_get_gpll_rate(priv);break;- case UART0_PLL_SEL_24M:
pll_rate = OSC_HZ;break;- case UART0_PLL_SEL_480M:
- case UART0_PLL_SEL_NPLL:
/* usbphy480M and NPLL clocks, generated by CRU, are notsupported yet */
- default:
return -ENOENT;- }
- clk_uart0 = DIV_TO_RATE(pll_rate, clk_uart0_div_con);
- con = readl(&pmucru->pmu_clksel_con[4]);
- clk_uart0_sel = bitfield_extract_by_mask(con, UART0_CLK_SEL_MASK);
- switch (clk_uart0_sel) {
- case UART0_CLK_SEL_UART0:
return clk_uart0;- case UART0_CLK_SEL_UART0_NP5:{
u32 clk_uart0_divnp5_div_con;clk_uart0_divnp5_div_con =bitfield_extract_by_mask(con, UART0_DIVNP5_MASK);return DIV_TO_RATE((2 * clk_uart0),(2 * clk_uart0_divnp5_div_con + 3));Are you sure about this DIV_TO_RATE here?
The TRM states:
clk_uart0_divnp5_div_con clk_uart0_np5=2*clk_uart0/(2*div_con+3)
but this would make it clk_uart0_np5=2*clk_uart0/((2*div_con+3) + 1)
I believe?
The kernel seems to be handling it like the TRM says (but I wouldn't be 100% certain here as the clock subsystem and Rockchip drivers have many indirections and I may have gotten lost at some point :) ).
I think we're fine also regarding u32 overflows as we would hit this if the rates were abore 2.1GHz and I think GPLL and NPLL aren't that fast.
} > + case UART0_CLK_SEL_UART0_FRAC:{u32 fracdiv, n, m;fracdiv = readl(&pmucru->pmu_clksel_con[5]);n = bitfield_extract_by_mask(fracdiv,CLK_UART_FRAC_NUMERATOR_MASK);m = bitfield_extract_by_mask(fracdiv,CLK_UART_FRAC_DENOMINATOR_MASK);return (u64) clk_uart0 * n / m;}- default:
return -ENOENT;- }
+}
+static ulong px30_pmu_uart0_set_clk(struct px30_pmuclk_priv *priv, ulong rate) +{
- struct px30_pmucru *pmucru = priv->pmucru;
- ulong gpll_rate;
- u32 clk_uart0_div_con;
- u32 clk_uart0_pll_sel;
- u32 clk_uart0_sel;
- ulong m = 0, n = 0;
- gpll_rate = px30_pmuclk_get_gpll_rate(priv);
- if (gpll_rate % rate == 0) {
clk_uart0_pll_sel = UART0_PLL_SEL_GPLL;clk_uart0_sel = UART0_CLK_SEL_UART0;clk_uart0_div_con = DIV_ROUND_UP(priv->gpll_hz, rate);- } else if (rate == OSC_HZ) {
clk_uart0_pll_sel = UART0_PLL_SEL_24M;clk_uart0_sel = UART0_CLK_SEL_UART0;clk_uart0_div_con = 1;- } else {
clk_uart0_pll_sel = UART0_PLL_SEL_GPLL;clk_uart0_sel = UART0_CLK_SEL_UART0_FRAC;clk_uart0_div_con = 1;rational_best_approximation(rate, priv->gpll_hz,GENMASK(16 - 1, 0),GENMASK(16 - 1, 0), &m, &n);I guess we could try to find whether OSC_HZ could produce a closer approximation than GPLL but I'm not sure it's worth it right now.
Otherwise looking good, Cheers, Quentin
Add dedicated getter and setter for SCLK_UART0_PMU. This allows the driver to correctly handle UART0 clocks, and thus it fixes the issues with UART0 not working in case DEBUG_UART is disabled. Unlike other Rockchip SoCs, i.e. rk3399, in the PX30 the default clock source for UART is GPLL, instead of external oscillator. If the DEBUG_UART is enabled, the clock source is changed in board_debug_uart_init function to 24Mhz oscillator, which also matches the fallback value obtained from DT node. In case the DEBUG_UART is disabled, the UART clock source remains default, and the DM serial driver wrongly configures the baud rate, resulting in broken communication. By implementing the UART clock getter/setter, the serial driver can probe the actual configuration and corectly configure itself. The DEBUG_UART settings now should not affect it.
The driver supports GPLL and 24M oscillator. NPLL and USBPHY480M sources, that are managed by CRU, are not yet handled, as likely they won't be used in real scenarios.
Signed-off-by: Lukasz Czechowski lukasz.czechowski@thaumatec.com --- arch/arm/include/asm/arch-rockchip/cru_px30.h | 7 ++ drivers/clk/rockchip/clk_px30.c | 108 ++++++++++++++++++ 2 files changed, 115 insertions(+)
diff --git a/arch/arm/include/asm/arch-rockchip/cru_px30.h b/arch/arm/include/asm/arch-rockchip/cru_px30.h index b66277fc7f3..504459bd93d 100644 --- a/arch/arm/include/asm/arch-rockchip/cru_px30.h +++ b/arch/arm/include/asm/arch-rockchip/cru_px30.h @@ -464,5 +464,12 @@ enum { UART0_CLK_SEL_UART0_FRAC, UART0_DIVNP5_SHIFT = 0, UART0_DIVNP5_MASK = 0x1f << UART0_DIVNP5_SHIFT, + + /* CRU_PMU_CLKSEL5_CON */ + CLK_UART_FRAC_NUMERATOR_SHIFT = 16, + CLK_UART_FRAC_NUMERATOR_MASK = 0xffff << CLK_UART_FRAC_NUMERATOR_SHIFT, + CLK_UART_FRAC_DENOMINATOR_SHIFT = 0, + CLK_UART_FRAC_DENOMINATOR_MASK = + 0xffff << CLK_UART_FRAC_DENOMINATOR_SHIFT, }; #endif diff --git a/drivers/clk/rockchip/clk_px30.c b/drivers/clk/rockchip/clk_px30.c index 2875c152b20..6768ef21b68 100644 --- a/drivers/clk/rockchip/clk_px30.c +++ b/drivers/clk/rockchip/clk_px30.c @@ -1589,6 +1589,108 @@ static ulong px30_pmuclk_set_gpll_rate(struct px30_pmuclk_priv *priv, ulong hz) return priv->gpll_hz; }
+static ulong px30_pmu_uart0_get_clk(struct px30_pmuclk_priv *priv) +{ + struct px30_pmucru *pmucru = priv->pmucru; + u32 clk_uart0_div_con; + u32 clk_uart0_pll_sel; + u32 clk_uart0_sel; + ulong clk_uart0; + ulong pll_rate; + u32 con; + + con = readl(&pmucru->pmu_clksel_con[3]); + clk_uart0_div_con = bitfield_extract_by_mask(con, UART0_DIV_CON_MASK); + clk_uart0_pll_sel = bitfield_extract_by_mask(con, UART0_PLL_SEL_MASK); + + switch (clk_uart0_pll_sel) { + case UART0_PLL_SEL_GPLL: + pll_rate = px30_pmuclk_get_gpll_rate(priv); + break; + case UART0_PLL_SEL_24M: + pll_rate = OSC_HZ; + break; + case UART0_PLL_SEL_480M: + case UART0_PLL_SEL_NPLL: + /* usbphy480M and NPLL clocks, generated by CRU, are not supported yet */ + default: + return -ENOENT; + } + + clk_uart0 = DIV_TO_RATE(pll_rate, clk_uart0_div_con); + con = readl(&pmucru->pmu_clksel_con[4]); + clk_uart0_sel = bitfield_extract_by_mask(con, UART0_CLK_SEL_MASK); + + switch (clk_uart0_sel) { + case UART0_CLK_SEL_UART0: + return clk_uart0; + case UART0_CLK_SEL_UART0_NP5:{ + u32 clk_uart0_divnp5_div_con; + + clk_uart0_divnp5_div_con = + bitfield_extract_by_mask(con, UART0_DIVNP5_MASK); + return 2 * (u64) clk_uart0 / (2 * + clk_uart0_divnp5_div_con + + 3); + } + case UART0_CLK_SEL_UART0_FRAC:{ + u32 fracdiv, n, m; + + fracdiv = readl(&pmucru->pmu_clksel_con[5]); + n = bitfield_extract_by_mask(fracdiv, + CLK_UART_FRAC_NUMERATOR_MASK); + m = bitfield_extract_by_mask(fracdiv, + CLK_UART_FRAC_DENOMINATOR_MASK); + return (u64) clk_uart0 * n / m; + } + default: + return -ENOENT; + } +} + +static ulong px30_pmu_uart0_set_clk(struct px30_pmuclk_priv *priv, ulong rate) +{ + struct px30_pmucru *pmucru = priv->pmucru; + u32 clk_uart0_div_con; + u32 clk_uart0_pll_sel; + u32 clk_uart0_sel; + ulong gpll_rate; + ulong m = 0, n = 0; + + gpll_rate = px30_pmuclk_get_gpll_rate(priv); + if (gpll_rate % rate == 0) { + clk_uart0_pll_sel = UART0_PLL_SEL_GPLL; + clk_uart0_sel = UART0_CLK_SEL_UART0; + clk_uart0_div_con = DIV_ROUND_UP(priv->gpll_hz, rate); + } else if (rate == OSC_HZ) { + clk_uart0_pll_sel = UART0_PLL_SEL_24M; + clk_uart0_sel = UART0_CLK_SEL_UART0; + clk_uart0_div_con = 1; + } else { + clk_uart0_pll_sel = UART0_PLL_SEL_GPLL; + clk_uart0_sel = UART0_CLK_SEL_UART0_FRAC; + clk_uart0_div_con = 1; + rational_best_approximation(rate, priv->gpll_hz, + GENMASK(16 - 1, 0), + GENMASK(16 - 1, 0), &m, &n); + } + + rk_clrsetreg(&pmucru->pmu_clksel_con[3], + UART0_PLL_SEL_MASK | UART0_DIV_CON_MASK, + clk_uart0_pll_sel << UART0_PLL_SEL_SHIFT | + (clk_uart0_div_con - 1)); + rk_clrsetreg(&pmucru->pmu_clksel_con[4], UART0_CLK_SEL_MASK, + clk_uart0_sel << UART0_CLK_SEL_SHIFT); + if (m && n) { + u32 fracdiv; + + fracdiv = m << CLK_UART_FRAC_NUMERATOR_SHIFT | n; + writel(fracdiv, &pmucru->pmu_clksel_con[5]); + } + + return px30_pmu_uart0_get_clk(priv); +} + static ulong px30_pmuclk_get_rate(struct clk *clk) { struct px30_pmuclk_priv *priv = dev_get_priv(clk->dev); @@ -1602,6 +1704,9 @@ static ulong px30_pmuclk_get_rate(struct clk *clk) case PCLK_PMU_PRE: rate = px30_pclk_pmu_get_pmuclk(priv); break; + case SCLK_UART0_PMU: + rate = px30_pmu_uart0_get_clk(priv); + break; default: return -ENOENT; } @@ -1622,6 +1727,9 @@ static ulong px30_pmuclk_set_rate(struct clk *clk, ulong rate) case PCLK_PMU_PRE: ret = px30_pclk_pmu_set_pmuclk(priv, rate); break; + case SCLK_UART0_PMU: + ret = px30_pmu_uart0_set_clk(priv, rate); + break; default: return -ENOENT; }
Hi Lukasz,
On 8/12/24 6:27 PM, Lukasz Czechowski wrote:
Add dedicated getter and setter for SCLK_UART0_PMU. This allows the driver to correctly handle UART0 clocks, and thus it fixes the issues with UART0 not working in case DEBUG_UART is disabled. Unlike other Rockchip SoCs, i.e. rk3399, in the PX30 the default clock source for UART is GPLL, instead of external oscillator. If the DEBUG_UART is enabled, the clock source is changed in board_debug_uart_init function to 24Mhz oscillator, which also matches the fallback value obtained from DT node. In case the DEBUG_UART is disabled, the UART clock source remains default, and the DM serial driver wrongly configures the baud rate, resulting in broken communication. By implementing the UART clock getter/setter, the serial driver can probe the actual configuration and corectly configure itself. The DEBUG_UART settings now should not affect it.
The driver supports GPLL and 24M oscillator. NPLL and USBPHY480M sources, that are managed by CRU, are not yet handled, as likely they won't be used in real scenarios.
Signed-off-by: Lukasz Czechowski lukasz.czechowski@thaumatec.com
[...]
+static ulong px30_pmu_uart0_get_clk(struct px30_pmuclk_priv *priv) +{
- struct px30_pmucru *pmucru = priv->pmucru;
- u32 clk_uart0_div_con;
- u32 clk_uart0_pll_sel;
- u32 clk_uart0_sel;
- ulong clk_uart0;
- ulong pll_rate;
- u32 con;
- con = readl(&pmucru->pmu_clksel_con[3]);
- clk_uart0_div_con = bitfield_extract_by_mask(con, UART0_DIV_CON_MASK);
- clk_uart0_pll_sel = bitfield_extract_by_mask(con, UART0_PLL_SEL_MASK);
- switch (clk_uart0_pll_sel) {
- case UART0_PLL_SEL_GPLL:
pll_rate = px30_pmuclk_get_gpll_rate(priv);break;- case UART0_PLL_SEL_24M:
pll_rate = OSC_HZ;break;- case UART0_PLL_SEL_480M:
- case UART0_PLL_SEL_NPLL:
/* usbphy480M and NPLL clocks, generated by CRU, are not supported yet */- default:
return -ENOENT;- }
- clk_uart0 = DIV_TO_RATE(pll_rate, clk_uart0_div_con);
- con = readl(&pmucru->pmu_clksel_con[4]);
- clk_uart0_sel = bitfield_extract_by_mask(con, UART0_CLK_SEL_MASK);
- switch (clk_uart0_sel) {
- case UART0_CLK_SEL_UART0:
return clk_uart0;- case UART0_CLK_SEL_UART0_NP5:{
u32 clk_uart0_divnp5_div_con;clk_uart0_divnp5_div_con =bitfield_extract_by_mask(con, UART0_DIVNP5_MASK);return 2 * (u64) clk_uart0 / (2 *clk_uart0_divnp5_div_con +3);}- case UART0_CLK_SEL_UART0_FRAC:{
u32 fracdiv, n, m;fracdiv = readl(&pmucru->pmu_clksel_con[5]);n = bitfield_extract_by_mask(fracdiv,CLK_UART_FRAC_NUMERATOR_MASK);m = bitfield_extract_by_mask(fracdiv,CLK_UART_FRAC_DENOMINATOR_MASK);return (u64) clk_uart0 * n / m;}
Considering the function is named px30_pmu_uart0_get_clk(), it's implied everything in that function is for uart0 so you could have save those 6 characters (_uart0) from each variable name which probably could help with having less odd line-wrapping.
Reviewed-by: Quentin Schulz quentin.schulz@cherry.de
Thanks! Quentin
Add dedicated getter and setter for SCLK_UART0_PMU. This allows the driver to correctly handle UART0 clocks, and thus it fixes the issues with UART0 not working in case DEBUG_UART is disabled. Unlike other Rockchip SoCs, i.e. rk3399, in the PX30 the default clock source for UART is GPLL, instead of external oscillator. If the DEBUG_UART is enabled, the clock source is changed in board_debug_uart_init function to 24Mhz oscillator, which also matches the fallback value obtained from DT node. In case the DEBUG_UART is disabled, the UART clock source remains default, and the DM serial driver wrongly configures the baud rate, resulting in broken communication. By implementing the UART clock getter/setter, the serial driver can probe the actual configuration and corectly configure itself. The DEBUG_UART settings now should not affect it.
The driver supports GPLL and 24M oscillator. NPLL and USBPHY480M sources, that are managed by CRU, are not yet handled, as likely they won't be used in real scenarios.
Signed-off-by: Lukasz Czechowski lukasz.czechowski@thaumatec.com
Reviewed-by: Quentin Schulz quentin.schulz@cherry.de --- arch/arm/include/asm/arch-rockchip/cru_px30.h | 7 ++ drivers/clk/rockchip/clk_px30.c | 105 ++++++++++++++++++ 2 files changed, 112 insertions(+)
diff --git a/arch/arm/include/asm/arch-rockchip/cru_px30.h b/arch/arm/include/asm/arch-rockchip/cru_px30.h index b66277fc7f3..504459bd93d 100644 --- a/arch/arm/include/asm/arch-rockchip/cru_px30.h +++ b/arch/arm/include/asm/arch-rockchip/cru_px30.h @@ -464,5 +464,12 @@ enum { UART0_CLK_SEL_UART0_FRAC, UART0_DIVNP5_SHIFT = 0, UART0_DIVNP5_MASK = 0x1f << UART0_DIVNP5_SHIFT, + + /* CRU_PMU_CLKSEL5_CON */ + CLK_UART_FRAC_NUMERATOR_SHIFT = 16, + CLK_UART_FRAC_NUMERATOR_MASK = 0xffff << CLK_UART_FRAC_NUMERATOR_SHIFT, + CLK_UART_FRAC_DENOMINATOR_SHIFT = 0, + CLK_UART_FRAC_DENOMINATOR_MASK = + 0xffff << CLK_UART_FRAC_DENOMINATOR_SHIFT, }; #endif diff --git a/drivers/clk/rockchip/clk_px30.c b/drivers/clk/rockchip/clk_px30.c index 2875c152b20..e5c004cfcc8 100644 --- a/drivers/clk/rockchip/clk_px30.c +++ b/drivers/clk/rockchip/clk_px30.c @@ -1589,6 +1589,105 @@ static ulong px30_pmuclk_set_gpll_rate(struct px30_pmuclk_priv *priv, ulong hz) return priv->gpll_hz; }
+static ulong px30_pmu_uart0_get_clk(struct px30_pmuclk_priv *priv) +{ + struct px30_pmucru *pmucru = priv->pmucru; + u32 clk_div_con; + u32 clk_pll_sel; + ulong pll_rate; + u32 clk_sel; + ulong clk; + u32 con; + + con = readl(&pmucru->pmu_clksel_con[3]); + clk_div_con = bitfield_extract_by_mask(con, UART0_DIV_CON_MASK); + clk_pll_sel = bitfield_extract_by_mask(con, UART0_PLL_SEL_MASK); + + switch (clk_pll_sel) { + case UART0_PLL_SEL_GPLL: + pll_rate = px30_pmuclk_get_gpll_rate(priv); + break; + case UART0_PLL_SEL_24M: + pll_rate = OSC_HZ; + break; + case UART0_PLL_SEL_480M: + case UART0_PLL_SEL_NPLL: + /* usbphy480M and NPLL clocks, generated by CRU, are not supported yet */ + default: + return -ENOENT; + } + + clk = DIV_TO_RATE(pll_rate, clk_div_con); + con = readl(&pmucru->pmu_clksel_con[4]); + clk_sel = bitfield_extract_by_mask(con, UART0_CLK_SEL_MASK); + + switch (clk_sel) { + case UART0_CLK_SEL_UART0: + return clk; + case UART0_CLK_SEL_UART0_NP5:{ + u32 clk_divnp5_div_con; + + clk_divnp5_div_con = + bitfield_extract_by_mask(con, UART0_DIVNP5_MASK); + return 2 * (u64) clk / (2 * clk_divnp5_div_con + 3); + } + case UART0_CLK_SEL_UART0_FRAC:{ + u32 fracdiv, n, m; + + fracdiv = readl(&pmucru->pmu_clksel_con[5]); + n = bitfield_extract_by_mask(fracdiv, + CLK_UART_FRAC_NUMERATOR_MASK); + m = bitfield_extract_by_mask(fracdiv, + CLK_UART_FRAC_DENOMINATOR_MASK); + return (u64) clk * n / m; + } + default: + return -ENOENT; + } +} + +static ulong px30_pmu_uart0_set_clk(struct px30_pmuclk_priv *priv, ulong rate) +{ + struct px30_pmucru *pmucru = priv->pmucru; + ulong m = 0, n = 0; + ulong gpll_rate; + u32 clk_div_con; + u32 clk_pll_sel; + u32 clk_sel; + + gpll_rate = px30_pmuclk_get_gpll_rate(priv); + if (gpll_rate % rate == 0) { + clk_pll_sel = UART0_PLL_SEL_GPLL; + clk_sel = UART0_CLK_SEL_UART0; + clk_div_con = DIV_ROUND_UP(priv->gpll_hz, rate); + } else if (rate == OSC_HZ) { + clk_pll_sel = UART0_PLL_SEL_24M; + clk_sel = UART0_CLK_SEL_UART0; + clk_div_con = 1; + } else { + clk_pll_sel = UART0_PLL_SEL_GPLL; + clk_sel = UART0_CLK_SEL_UART0_FRAC; + clk_div_con = 1; + rational_best_approximation(rate, priv->gpll_hz, + GENMASK(16 - 1, 0), + GENMASK(16 - 1, 0), &m, &n); + } + + rk_clrsetreg(&pmucru->pmu_clksel_con[3], + UART0_PLL_SEL_MASK | UART0_DIV_CON_MASK, + clk_pll_sel << UART0_PLL_SEL_SHIFT | (clk_div_con - 1)); + rk_clrsetreg(&pmucru->pmu_clksel_con[4], UART0_CLK_SEL_MASK, + clk_sel << UART0_CLK_SEL_SHIFT); + if (m && n) { + u32 fracdiv; + + fracdiv = m << CLK_UART_FRAC_NUMERATOR_SHIFT | n; + writel(fracdiv, &pmucru->pmu_clksel_con[5]); + } + + return px30_pmu_uart0_get_clk(priv); +} + static ulong px30_pmuclk_get_rate(struct clk *clk) { struct px30_pmuclk_priv *priv = dev_get_priv(clk->dev); @@ -1602,6 +1701,9 @@ static ulong px30_pmuclk_get_rate(struct clk *clk) case PCLK_PMU_PRE: rate = px30_pclk_pmu_get_pmuclk(priv); break; + case SCLK_UART0_PMU: + rate = px30_pmu_uart0_get_clk(priv); + break; default: return -ENOENT; } @@ -1622,6 +1724,9 @@ static ulong px30_pmuclk_set_rate(struct clk *clk, ulong rate) case PCLK_PMU_PRE: ret = px30_pclk_pmu_set_pmuclk(priv, rate); break; + case SCLK_UART0_PMU: + ret = px30_pmu_uart0_set_clk(priv, rate); + break; default: return -ENOENT; }
On 2024/8/22 18:33, Lukasz Czechowski wrote:
Add dedicated getter and setter for SCLK_UART0_PMU. This allows the driver to correctly handle UART0 clocks, and thus it fixes the issues with UART0 not working in case DEBUG_UART is disabled. Unlike other Rockchip SoCs, i.e. rk3399, in the PX30 the default clock source for UART is GPLL, instead of external oscillator. If the DEBUG_UART is enabled, the clock source is changed in board_debug_uart_init function to 24Mhz oscillator, which also matches the fallback value obtained from DT node. In case the DEBUG_UART is disabled, the UART clock source remains default, and the DM serial driver wrongly configures the baud rate, resulting in broken communication. By implementing the UART clock getter/setter, the serial driver can probe the actual configuration and corectly configure itself. The DEBUG_UART settings now should not affect it.
The driver supports GPLL and 24M oscillator. NPLL and USBPHY480M sources, that are managed by CRU, are not yet handled, as likely they won't be used in real scenarios.
Signed-off-by: Lukasz Czechowski lukasz.czechowski@thaumatec.com
Reviewed-by: Quentin Schulz quentin.schulz@cherry.de
Reviewed-by: Kever Yang kever.yang@rock-chips.com
Thanks, - Kever
arch/arm/include/asm/arch-rockchip/cru_px30.h | 7 ++ drivers/clk/rockchip/clk_px30.c | 105 ++++++++++++++++++ 2 files changed, 112 insertions(+)
diff --git a/arch/arm/include/asm/arch-rockchip/cru_px30.h b/arch/arm/include/asm/arch-rockchip/cru_px30.h index b66277fc7f3..504459bd93d 100644 --- a/arch/arm/include/asm/arch-rockchip/cru_px30.h +++ b/arch/arm/include/asm/arch-rockchip/cru_px30.h @@ -464,5 +464,12 @@ enum { UART0_CLK_SEL_UART0_FRAC, UART0_DIVNP5_SHIFT = 0, UART0_DIVNP5_MASK = 0x1f << UART0_DIVNP5_SHIFT,
- /* CRU_PMU_CLKSEL5_CON */
- CLK_UART_FRAC_NUMERATOR_SHIFT = 16,
- CLK_UART_FRAC_NUMERATOR_MASK = 0xffff << CLK_UART_FRAC_NUMERATOR_SHIFT,
- CLK_UART_FRAC_DENOMINATOR_SHIFT = 0,
- CLK_UART_FRAC_DENOMINATOR_MASK =
0xffff << CLK_UART_FRAC_DENOMINATOR_SHIFT,diff --git a/drivers/clk/rockchip/clk_px30.c b/drivers/clk/rockchip/clk_px30.c index 2875c152b20..e5c004cfcc8 100644 --- a/drivers/clk/rockchip/clk_px30.c +++ b/drivers/clk/rockchip/clk_px30.c @@ -1589,6 +1589,105 @@ static ulong px30_pmuclk_set_gpll_rate(struct px30_pmuclk_priv *priv, ulong hz) return priv->gpll_hz; }
+static ulong px30_pmu_uart0_get_clk(struct px30_pmuclk_priv *priv) +{
- struct px30_pmucru *pmucru = priv->pmucru;
- u32 clk_div_con;
- u32 clk_pll_sel;
- ulong pll_rate;
- u32 clk_sel;
- ulong clk;
- u32 con;
- con = readl(&pmucru->pmu_clksel_con[3]);
- clk_div_con = bitfield_extract_by_mask(con, UART0_DIV_CON_MASK);
- clk_pll_sel = bitfield_extract_by_mask(con, UART0_PLL_SEL_MASK);
- switch (clk_pll_sel) {
- case UART0_PLL_SEL_GPLL:
pll_rate = px30_pmuclk_get_gpll_rate(priv);break;- case UART0_PLL_SEL_24M:
pll_rate = OSC_HZ;break;- case UART0_PLL_SEL_480M:
- case UART0_PLL_SEL_NPLL:
/* usbphy480M and NPLL clocks, generated by CRU, are not supported yet */- default:
return -ENOENT;- }
- clk = DIV_TO_RATE(pll_rate, clk_div_con);
- con = readl(&pmucru->pmu_clksel_con[4]);
- clk_sel = bitfield_extract_by_mask(con, UART0_CLK_SEL_MASK);
- switch (clk_sel) {
- case UART0_CLK_SEL_UART0:
return clk;- case UART0_CLK_SEL_UART0_NP5:{
u32 clk_divnp5_div_con;clk_divnp5_div_con =bitfield_extract_by_mask(con, UART0_DIVNP5_MASK);return 2 * (u64) clk / (2 * clk_divnp5_div_con + 3);}- case UART0_CLK_SEL_UART0_FRAC:{
u32 fracdiv, n, m;fracdiv = readl(&pmucru->pmu_clksel_con[5]);n = bitfield_extract_by_mask(fracdiv,CLK_UART_FRAC_NUMERATOR_MASK);m = bitfield_extract_by_mask(fracdiv,CLK_UART_FRAC_DENOMINATOR_MASK);return (u64) clk * n / m;}- default:
return -ENOENT;- }
+}
+static ulong px30_pmu_uart0_set_clk(struct px30_pmuclk_priv *priv, ulong rate) +{
- struct px30_pmucru *pmucru = priv->pmucru;
- ulong m = 0, n = 0;
- ulong gpll_rate;
- u32 clk_div_con;
- u32 clk_pll_sel;
- u32 clk_sel;
- gpll_rate = px30_pmuclk_get_gpll_rate(priv);
- if (gpll_rate % rate == 0) {
clk_pll_sel = UART0_PLL_SEL_GPLL;clk_sel = UART0_CLK_SEL_UART0;clk_div_con = DIV_ROUND_UP(priv->gpll_hz, rate);- } else if (rate == OSC_HZ) {
clk_pll_sel = UART0_PLL_SEL_24M;clk_sel = UART0_CLK_SEL_UART0;clk_div_con = 1;- } else {
clk_pll_sel = UART0_PLL_SEL_GPLL;clk_sel = UART0_CLK_SEL_UART0_FRAC;clk_div_con = 1;rational_best_approximation(rate, priv->gpll_hz,GENMASK(16 - 1, 0),GENMASK(16 - 1, 0), &m, &n);- }
- rk_clrsetreg(&pmucru->pmu_clksel_con[3],
UART0_PLL_SEL_MASK | UART0_DIV_CON_MASK,clk_pll_sel << UART0_PLL_SEL_SHIFT | (clk_div_con - 1));- rk_clrsetreg(&pmucru->pmu_clksel_con[4], UART0_CLK_SEL_MASK,
clk_sel << UART0_CLK_SEL_SHIFT);- if (m && n) {
u32 fracdiv;fracdiv = m << CLK_UART_FRAC_NUMERATOR_SHIFT | n;writel(fracdiv, &pmucru->pmu_clksel_con[5]);- }
- return px30_pmu_uart0_get_clk(priv);
+}
- static ulong px30_pmuclk_get_rate(struct clk *clk) { struct px30_pmuclk_priv *priv = dev_get_priv(clk->dev);
@@ -1602,6 +1701,9 @@ static ulong px30_pmuclk_get_rate(struct clk *clk) case PCLK_PMU_PRE: rate = px30_pclk_pmu_get_pmuclk(priv); break;
- case SCLK_UART0_PMU:
rate = px30_pmu_uart0_get_clk(priv);break;@@ -1622,6 +1724,9 @@ static ulong px30_pmuclk_set_rate(struct clk *clk, ulong rate) case PCLK_PMU_PRE: ret = px30_pclk_pmu_set_pmuclk(priv, rate); break;
- case SCLK_UART0_PMU:
ret = px30_pmu_uart0_set_clk(priv, rate);break;
participants (4)
-
Kever Yang -
Lukasz Czechowski -
Quentin Schulz -
Łukasz Czechowski