[PATCH 0/2] qcom: gracefully handle some errors better

[PATCH 0/2] qcom: gracefully handle some errors better

[Casey Connolly via B4 Relay]

Adjust the serial driver to not bail out if we can't find the clock,
this could happen pre-relocation if the clock driver isn't bound or if
there is no clock driver yet. In most cases this isn't an issue since
the clocks are left enabled by the previous bootloader stage.

Additionally adjust the rpmh driver to gracefully bail out for
non-active write requests, sleep/wake votes are meaningless in U-Boot
since we will always be in the active state. It's possible that the
caller may send an active rpmh write after the sleep request but might
not if we return an error, so let's play it safe.

---
Casey Connolly (2):
      serial: msm-geni: allow invalid clock
      qcom: rpmh: don't error for SLEEP requests

 drivers/serial/serial_msm_geni.c | 15 +++++----------
 drivers/soc/qcom/rpmh-rsc.c      |  8 ++++----
 drivers/soc/qcom/rpmh.c          |  4 ++--
 3 files changed, 11 insertions(+), 16 deletions(-)
---
base-commit: ba7bf918dafcd093ad733b07ba490baeb20cf5da

// Casey (she/they)

[PATCH 1/2] serial: msm-geni: allow invalid clock

[Casey Connolly via B4 Relay]

From: Casey Connolly <casey.connolly@linaro.org>

Pre-relocation we probably won't have a clock but it's usually
been enabled for us already, or worst case we will enable it after
relocation.

Signed-off-by: Casey Connolly <casey.connolly@linaro.org>
---
 drivers/serial/serial_msm_geni.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/serial/serial_msm_geni.c b/drivers/serial/serial_msm_geni.c
index bb5a2cb4d2cd..c37255f57d05 100644
--- a/drivers/serial/serial_msm_geni.c
+++ b/drivers/serial/serial_msm_geni.c
@@ -211,9 +211,9 @@ static int msm_serial_setbrg(struct udevice *dev, int baud)
 	}
 	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;
+		return 0;
 	}
 	geni_serial_baud(priv->base, clk_div, baud);
 
 	return 0;
@@ -516,15 +516,14 @@ static int msm_serial_probe(struct udevice *dev)
 	int ret;
 	u32 proto;
 	struct clk *clk;
 
-	clk = devm_clk_get(dev, NULL);
-	if (IS_ERR(clk))
-		return PTR_ERR(clk);
-	priv->se = clk;
+	clk = devm_clk_get_optional(dev, NULL);
+	if (!IS_ERR(clk))
+		priv->se = clk;
 
 	/* Try enable clock */
-	ret = clk_enable(clk);
+	clk_enable(clk);
 
 	/* Check if firmware loading is needed (BT UART) */
 	proto = readl(priv->base + GENI_FW_REVISION_RO);
 	proto &= FW_REV_PROTOCOL_MSK;
@@ -546,12 +545,8 @@ static int msm_serial_probe(struct udevice *dev)
 	/* Don't actually probe non-debug UARTs */
 	if (ofnode_device_is_compatible(dev_ofnode(dev), "qcom,geni-uart"))
 		return -ENOENT;
 
-	/* Now handle clock enable return value */
-	if (ret)
-		return ret;
-
 	ret = geni_set_oversampling(dev);
 	if (ret < 0)
 		return ret;
 

-- 
2.51.0

[PATCH 2/2] qcom: rpmh: don't error for SLEEP requests

[Casey Connolly via B4 Relay]

From: Casey Connolly <casey.connolly@linaro.org>

Just stub them out since the caller might also send active votes. Since
we don't suspend there's no risk of missing votes here, we never enter
the sleep state.

Signed-off-by: Casey Connolly <casey.connolly@linaro.org>
---
 drivers/soc/qcom/rpmh-rsc.c | 8 ++++----
 drivers/soc/qcom/rpmh.c     | 4 ++--
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
index dce61f262292..0b821cc6f9d8 100644
--- a/drivers/soc/qcom/rpmh-rsc.c
+++ b/drivers/soc/qcom/rpmh-rsc.c
@@ -260,10 +260,10 @@ static struct tcs_group *get_tcs_for_msg(struct rsc_drv *drv,
 	 * U-Boot: since we're single threaded and running synchronously we can
 	 * just always used the first active TCS.
 	 */
 	if (msg->state != RPMH_ACTIVE_ONLY_STATE) {
-		log_err("WARN: only ACTIVE_ONLY state supported\n");
-		return ERR_PTR(-EINVAL);
+		log_debug("WARN: only ACTIVE_ONLY state supported\n");
+		return NULL;
 	}
 
 	return &drv->tcs[ACTIVE_TCS];
 }
@@ -389,10 +389,10 @@ int rpmh_rsc_send_data(struct rsc_drv *drv, const struct tcs_request *msg)
 	int tcs_id, i = 0;
 	u32 val;
 
 	tcs = get_tcs_for_msg(drv, msg);
-	if (IS_ERR(tcs))
-		return PTR_ERR(tcs);
+	if (IS_ERR_OR_NULL(tcs))
+		return 0;
 
 	/* U-Boot is single-threaded, always use the first TCS as we'll never conflict */
 	tcs_id = tcs->offset;
 	if (!read_tcs_reg(drv, drv->regs[RSC_DRV_STATUS], tcs_id)) {
diff --git a/drivers/soc/qcom/rpmh.c b/drivers/soc/qcom/rpmh.c
index 8c222324c66d..b55e23c4417d 100644
--- a/drivers/soc/qcom/rpmh.c
+++ b/drivers/soc/qcom/rpmh.c
@@ -59,10 +59,10 @@ static int __rpmh_write(const struct udevice *dev, enum rpmh_state state,
 {
 	struct rpmh_ctrlr *ctrlr = get_rpmh_ctrlr(dev);
 
 	if (state != RPMH_ACTIVE_ONLY_STATE) {
-		log_err("only ACTIVE_ONLY state supported\n");
-		return -EINVAL;
+		log_debug("WARN: Only ACTIVE_ONLY state supported\n");
+		return 0;
 	}
 
 	return rpmh_rsc_send_data(ctrlr_to_drv(ctrlr), &rpm_msg->msg);
 }

-- 
2.51.0

Re: [PATCH 0/2] qcom: gracefully handle some errors better

[Casey Connolly]

On Fri, 20 Mar 2026 16:45:39 +0100, Casey Connolly wrote:
> Adjust the serial driver to not bail out if we can't find the clock,
> this could happen pre-relocation if the clock driver isn't bound or if
> there is no clock driver yet. In most cases this isn't an issue since
> the clocks are left enabled by the previous bootloader stage.
> 
> Additionally adjust the rpmh driver to gracefully bail out for
> non-active write requests, sleep/wake votes are meaningless in U-Boot
> since we will always be in the active state. It's possible that the
> caller may send an active rpmh write after the sleep request but might
> not if we return an error, so let's play it safe.
> 
> [...]

Applied, thanks!

[1/2] serial: msm-geni: allow invalid clock
      https://source.denx.de/u-boot/custodians/u-boot-snapdragon/-/commit/7f09aff39924
[2/2] qcom: rpmh: don't error for SLEEP requests
      https://source.denx.de/u-boot/custodians/u-boot-snapdragon/-/commit/4577a672ecea

Best regards,
-- 
// Casey (she/they)

Re: [PATCH 1/2] serial: msm-geni: allow invalid clock

[Sumit Garg]

On Fri, Mar 20, 2026 at 04:45:40PM +0100, Casey Connolly via B4 Relay wrote:
> From: Casey Connolly <casey.connolly@linaro.org>
> 
> Pre-relocation we probably won't have a clock but it's usually
> been enabled for us already, or worst case we will enable it after
> relocation.
> 

Not sure if this is the right approach, since the UART driver might just
hang without clock being enabled. What's the motivation for this change?

-Sumit

> Signed-off-by: Casey Connolly <casey.connolly@linaro.org>
> ---
>  drivers/serial/serial_msm_geni.c | 15 +++++----------
>  1 file changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/serial/serial_msm_geni.c b/drivers/serial/serial_msm_geni.c
> index bb5a2cb4d2cd..c37255f57d05 100644
> --- a/drivers/serial/serial_msm_geni.c
> +++ b/drivers/serial/serial_msm_geni.c
> @@ -211,9 +211,9 @@ static int msm_serial_setbrg(struct udevice *dev, int baud)
>  	}
>  	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;
> +		return 0;
>  	}
>  	geni_serial_baud(priv->base, clk_div, baud);
>  
>  	return 0;
> @@ -516,15 +516,14 @@ static int msm_serial_probe(struct udevice *dev)
>  	int ret;
>  	u32 proto;
>  	struct clk *clk;
>  
> -	clk = devm_clk_get(dev, NULL);
> -	if (IS_ERR(clk))
> -		return PTR_ERR(clk);
> -	priv->se = clk;
> +	clk = devm_clk_get_optional(dev, NULL);
> +	if (!IS_ERR(clk))
> +		priv->se = clk;
>  
>  	/* Try enable clock */
> -	ret = clk_enable(clk);
> +	clk_enable(clk);
>  
>  	/* Check if firmware loading is needed (BT UART) */
>  	proto = readl(priv->base + GENI_FW_REVISION_RO);
>  	proto &= FW_REV_PROTOCOL_MSK;
> @@ -546,12 +545,8 @@ static int msm_serial_probe(struct udevice *dev)
>  	/* Don't actually probe non-debug UARTs */
>  	if (ofnode_device_is_compatible(dev_ofnode(dev), "qcom,geni-uart"))
>  		return -ENOENT;
>  
> -	/* Now handle clock enable return value */
> -	if (ret)
> -		return ret;
> -
>  	ret = geni_set_oversampling(dev);
>  	if (ret < 0)
>  		return ret;
>  
> 
> -- 
> 2.51.0
> 
> 

Re: [PATCH 1/2] serial: msm-geni: allow invalid clock

[Casey Connolly]

Hi Sumit,

On 07/04/2026 13:38, Sumit Garg wrote:
> On Fri, Mar 20, 2026 at 04:45:40PM +0100, Casey Connolly via B4 Relay wrote:
>> From: Casey Connolly <casey.connolly@linaro.org>
>>
>> Pre-relocation we probably won't have a clock but it's usually
>> been enabled for us already, or worst case we will enable it after
>> relocation.
>>
> 
> Not sure if this is the right approach, since the UART driver might just
> hang without clock being enabled. What's the motivation for this change?

Hmm, I didn't consider that. The motivation here is to handle CCF, since
we don't want to support clock pre-relocation. In pretty much all cases
today the serial clocks are already configured but you're right that we
ought not to depend on that.

I can't really think of another good solution than hardcoding this clock
configuration somewhere :/

Any ideas?

> 
> -Sumit
> 
>> Signed-off-by: Casey Connolly <casey.connolly@linaro.org>
>> ---
>>  drivers/serial/serial_msm_geni.c | 15 +++++----------
>>  1 file changed, 5 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/serial/serial_msm_geni.c b/drivers/serial/serial_msm_geni.c
>> index bb5a2cb4d2cd..c37255f57d05 100644
>> --- a/drivers/serial/serial_msm_geni.c
>> +++ b/drivers/serial/serial_msm_geni.c
>> @@ -211,9 +211,9 @@ static int msm_serial_setbrg(struct udevice *dev, int baud)
>>  	}
>>  	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;
>> +		return 0;
>>  	}
>>  	geni_serial_baud(priv->base, clk_div, baud);
>>  
>>  	return 0;
>> @@ -516,15 +516,14 @@ static int msm_serial_probe(struct udevice *dev)
>>  	int ret;
>>  	u32 proto;
>>  	struct clk *clk;
>>  
>> -	clk = devm_clk_get(dev, NULL);
>> -	if (IS_ERR(clk))
>> -		return PTR_ERR(clk);
>> -	priv->se = clk;
>> +	clk = devm_clk_get_optional(dev, NULL);
>> +	if (!IS_ERR(clk))
>> +		priv->se = clk;
>>  
>>  	/* Try enable clock */
>> -	ret = clk_enable(clk);
>> +	clk_enable(clk);
>>  
>>  	/* Check if firmware loading is needed (BT UART) */
>>  	proto = readl(priv->base + GENI_FW_REVISION_RO);
>>  	proto &= FW_REV_PROTOCOL_MSK;
>> @@ -546,12 +545,8 @@ static int msm_serial_probe(struct udevice *dev)
>>  	/* Don't actually probe non-debug UARTs */
>>  	if (ofnode_device_is_compatible(dev_ofnode(dev), "qcom,geni-uart"))
>>  		return -ENOENT;
>>  
>> -	/* Now handle clock enable return value */
>> -	if (ret)
>> -		return ret;
>> -
>>  	ret = geni_set_oversampling(dev);
>>  	if (ret < 0)
>>  		return ret;
>>  
>>
>> -- 
>> 2.51.0
>>
>>

-- 
// Casey (she/her)

Re: [PATCH 1/2] serial: msm-geni: allow invalid clock

[Sumit Garg]

On Wed, Apr 08, 2026 at 11:16:24AM +0200, Casey Connolly wrote:
> Hi Sumit,
> 
> On 07/04/2026 13:38, Sumit Garg wrote:
> > On Fri, Mar 20, 2026 at 04:45:40PM +0100, Casey Connolly via B4 Relay wrote:
> >> From: Casey Connolly <casey.connolly@linaro.org>
> >>
> >> Pre-relocation we probably won't have a clock but it's usually
> >> been enabled for us already, or worst case we will enable it after
> >> relocation.
> >>
> > 
> > Not sure if this is the right approach, since the UART driver might just
> > hang without clock being enabled. What's the motivation for this change?
> 
> Hmm, I didn't consider that. The motivation here is to handle CCF, since
> we don't want to support clock pre-relocation. In pretty much all cases
> today the serial clocks are already configured but you're right that we
> ought not to depend on that.

I think this again comes down to that how CCF framework can enable
U-Boot SPL use-cases or where in U-Boot proper you get to work with an
uninitialized UART port for an OEM board variant.

> 
> I can't really think of another good solution than hardcoding this clock
> configuration somewhere :/
> 

That's true since we rely totally on DT based driver probing in U-Boot.

> Any ideas?
> 

Have you tried working with DT properties like bootph-pre-ram etc. that
allows only say UART driver to probe before relocation with CCF?

-Sumit