[PATCH v2] rockchip: px30: clk: add UART0 clock getter/setter

[Quentin Schulz]

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 at 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 at cherry.de>

Thanks!
Quentin