[PATCH v2] serial: ns16550: Enable clocks during probe

[Johan Jonker]

Hi Samuel,

Could you provide an example and or details for which SoC type this is in mind?

For current Rockchip we don't need this.
Everything works out of the box and is not touched in the clock driver.
The setting is done with serial_rockchip.c

Your clk_enable() function makes it a requirement for a enable ops function on all SoCs with
compatible "snps,dw-apb-uart" nodes.

The S clock is on position 0 and is set with "clock-frequency" property.
The P clock is on position 1 and is not touched.
> +	ret = clk_get_by_index(dev, 0, &clk);

This tries to enable the S clock instead of the P clock.

So NAK for now.

Johan

===

>From Linux clk-rk3188.c:
	GATE(PCLK_UART2, "pclk_uart2", "pclk_peri", 0, RK2928_CLKGATE_CON(8), 2, GFLAGS),

static struct rockchip_clk_branch common_uart2_fracmux __initdata =
	MUX(SCLK_UART2, "sclk_uart2", mux_sclk_uart2_p, CLK_SET_RATE_PARENT,
			RK2928_CLKSEL_CON(15), 8, 2, MFLAGS);
===

>From serial_rockchip.c:

static int rockchip_serial_probe(struct udevice *dev)
{
#if CONFIG_IS_ENABLED(OF_PLATDATA)
	struct rockchip_uart_plat *plat = dev_get_plat(dev);

	/* Create some new platform data for the standard driver */
	plat->plat.base = plat->dtplat.reg[0];
	plat->plat.reg_shift = plat->dtplat.reg_shift;

////
	plat->plat.clock = plat->dtplat.clock_frequency;
////

	plat->plat.fcr = UART_FCR_DEFVAL;
	dev_set_plat(dev, &plat->plat);

	return ns16550_serial_probe(dev);
#else
	return -ENODEV;
#endif
}

===

>From clk_rk3066.c:

static int rk3066_clk_enable(struct clk *clk)
{
	struct rk3066_clk_priv *priv = dev_get_priv(clk->dev);

	switch (clk->id) {
	case HCLK_NANDC0:
		rk_clrreg(&priv->cru->cru_clkgate_con[5], BIT(9));
		break;
	case HCLK_SDMMC:
		rk_clrreg(&priv->cru->cru_clkgate_con[5], BIT(10));
		break;
	case HCLK_SDIO:
		rk_clrreg(&priv->cru->cru_clkgate_con[5], BIT(11));
		break;
	}

	return 0;
}
static struct clk_ops rk3066_clk_ops = {
	.disable	= rk3066_clk_disable,
	.enable	= rk3066_clk_enable,
	.get_rate	= rk3066_clk_get_rate,
	.set_rate	= rk3066_clk_set_rate,
};

===

For rk3xxx.dtsi:

	uart2: serial at 20064000 {
		compatible = "snps,dw-apb-uart";
		reg = <0x20064000 0x400>;
		interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
		reg-shift = <2>;
		reg-io-width = <1>;
		clock-names = "baudclk", "apb_pclk";
		clocks = <&cru SCLK_UART2>, <&cru PCLK_UART2>;
		status = "disabled";
	};

On 12/13/22 02:50, Samuel Holland wrote:
> If the UART bus clock has a gate, it must be enabled before the UART
> can be used.
> 
> Signed-off-by: Samuel Holland <samuel at sholland.org>
> ---
> 
> Changes in v2:
>  - Only enable the first clock, as using the clk_get_bulk() API pushes
>    a board (phycore-rk3288) over its SPL size limit.
> 
>  drivers/serial/ns16550.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
> index 7592979cab5..072419343a3 100644
> --- a/drivers/serial/ns16550.c
> +++ b/drivers/serial/ns16550.c
> @@ -507,6 +507,7 @@ int ns16550_serial_probe(struct udevice *dev)
>  	struct ns16550 *const com_port = dev_get_priv(dev);
>  	struct reset_ctl_bulk reset_bulk;
>  	fdt_addr_t addr;
> +	struct clk clk;
>  	int ret;
>  
>  	/*
> @@ -524,6 +525,10 @@ int ns16550_serial_probe(struct udevice *dev)
>  	if (!ret)
>  		reset_deassert_bulk(&reset_bulk);
>  
> +	ret = clk_get_by_index(dev, 0, &clk);
> +	if (!ret)
> +		clk_enable(&clk);
> +
>  	com_port->plat = dev_get_plat(dev);
>  	ns16550_init(com_port, -1);
>