[PATCH v4 07/28] serial: msm-geni: Enable SE clk in probe

[Casey Connolly]

Hi Stephen,

On 5/24/25 00:58, Stephen Boyd wrote:
> Enable the serial engine clk in probe so that this driver can work on
> platforms that don't already initialize the clk for this device before
> this driver runs. This fixes a problem I see on Coreboot platforms like
> Trogdor where the UART hardware isn't enabled by coreboot unless the
> serial console build is used.
> 
> Signed-off-by: Stephen Boyd <swboyd at chromium.org>
> ---
>   drivers/serial/serial_msm_geni.c | 26 ++++++++++++--------------
>   1 file changed, 12 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/serial/serial_msm_geni.c b/drivers/serial/serial_msm_geni.c
> index cb6c09fdd09e..620cb9a59343 100644
> --- a/drivers/serial/serial_msm_geni.c
> +++ b/drivers/serial/serial_msm_geni.c
> @@ -131,6 +131,7 @@ struct msm_serial_data {
>   	phys_addr_t base;
>   	u32 baud;
>   	u32 oversampling;
> +	struct clk *se;
>   };
>   
>   unsigned long root_freq[] = {7372800,  14745600, 19200000, 29491200,
> @@ -181,19 +182,6 @@ static int get_clk_div_rate(u32 baud, u64 sampling_rate, u32 *clk_div)
>   	return ser_clk;
>   }
>   
> -static int geni_serial_set_clock_rate(struct udevice *dev, u64 rate)
> -{
> -	struct clk *clk;
> -	int ret;
> -
> -	clk = devm_clk_get(dev, NULL);
> -	if (IS_ERR(clk))
> -		return PTR_ERR(clk);
> -
> -	ret = clk_set_rate(clk, rate);
> -	return ret;
> -}
> -
>   /**
>    * geni_se_get_tx_fifo_depth() - Get the TX fifo depth of the serial engine
>    * @base:	Pointer to the concerned serial engine.
> @@ -252,7 +240,7 @@ static int msm_serial_setbrg(struct udevice *dev, int baud)
>   	priv->baud = baud;
>   
>   	clk_rate = get_clk_div_rate(baud, priv->oversampling, &clk_div);
> -	ret = geni_serial_set_clock_rate(dev, clk_rate);
> +	ret = clk_set_rate(priv->se, clk_rate);
>   	if (ret < 0) {
>   		pr_err("%s: Couldn't set clock rate: %d\n", __func__, ret);
>   		return ret;
> @@ -561,6 +549,16 @@ static int msm_serial_probe(struct udevice *dev)
>   {
>   	struct msm_serial_data *priv = dev_get_priv(dev);
>   	int ret;
> +	struct clk *clk;
> +
> +	clk = devm_clk_get(dev, NULL);
> +	if (IS_ERR(clk))
> +		return PTR_ERR(clk);
> +	priv->se = clk;
> +
> +	ret = clk_enable(clk);
> +	if (ret)
> +		return ret;

There's a subtle change in behaviour here, since we'll now fail probe if 
the clock isn't found. I think it would be best to treat this clock 
being missing as non-fatal since it can make bringup a little easier (no 
need to stub the clock).

Kind regards,>
>   	ret = geni_set_oversampling(dev);
>   	if (ret < 0)
-- 
Casey (she/they)