[PATCH v2] remoteproc: qcom_wcnss: Fix handling the lack of PD regulators in v3

[PATCH v2] remoteproc: qcom_wcnss: Fix handling the lack of PD regulators in v3

[Val Packett]

The changes introduced to handle single power domain platforms have
swapped the info pointer increment from num_pd_vregs to num_pds, which
would shift the info pointer past the end of the array for pronto-v3,
which does not list power domain regulators in vregs.

This showed up as a difference between GCC- and LLVM-compiled kernels
on SDM632 devices, where only with LLVM one would get the
"regulator request with no identifier" error, because the out-of-bounds
memory ended up being zeroed. Fix by skipping the increment when there
are more power domains than regulators.

Signed-off-by: Val Packett <val@packett.cool>
---
v2: changed to detect the >= condition suggested by Konrad
v1: https://lore.kernel.org/all/20260126235018.969140-1-val@packett.cool/

"possible_pds" is the best name I could come up with (as "num" is already
taken by the number of *successfully attached* PDs and "max" is the constant
for the array length) for the count we're checking against. Maybe the "num"
could be changed to "attached" but that feels like too much diff.

~val
---

 drivers/remoteproc/qcom_wcnss.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/remoteproc/qcom_wcnss.c b/drivers/remoteproc/qcom_wcnss.c
index ee18bf2e8054..60f629b5bbed 100644
--- a/drivers/remoteproc/qcom_wcnss.c
+++ b/drivers/remoteproc/qcom_wcnss.c
@@ -441,25 +441,31 @@ static void wcnss_release_pds(struct qcom_wcnss *wcnss)
 }
 
 static int wcnss_init_regulators(struct qcom_wcnss *wcnss,
-				 const struct wcnss_vreg_info *info,
-				 int num_vregs, int num_pd_vregs)
+				 const struct wcnss_data *data)
 {
+	const struct wcnss_vreg_info *info = data->vregs;
 	struct regulator_bulk_data *bulk;
+	size_t i, possible_pds = 0, num_vregs = data->num_vregs;
 	int ret;
-	int i;
+
+	for (i = 0; i < WCNSS_MAX_PDS; i++)
+		if (data->pd_names[i])
+			possible_pds++;
 
 	/*
 	 * If attaching the power domains suceeded we can skip requesting
 	 * the regulators for the power domains. For old device trees we need to
 	 * reserve extra space to manage them through the regulator interface.
 	 */
-	if (wcnss->num_pds) {
+	if (possible_pds >= num_vregs) {
+		/* Do nothing if vregs do not include PD regulators (pronto-v3) */
+	} else if (wcnss->num_pds) {
 		info += wcnss->num_pds;
 		/* Handle single power domain case */
-		if (wcnss->num_pds < num_pd_vregs)
-			num_vregs += num_pd_vregs - wcnss->num_pds;
+		if (wcnss->num_pds < data->num_pd_vregs)
+			num_vregs += data->num_pd_vregs - wcnss->num_pds;
 	} else {
-		num_vregs += num_pd_vregs;
+		num_vregs += data->num_pd_vregs;
 	}
 
 	bulk = devm_kcalloc(wcnss->dev,
@@ -607,8 +613,7 @@ static int wcnss_probe(struct platform_device *pdev)
 	if (ret && (ret != -ENODATA || !data->num_pd_vregs))
 		return ret;
 
-	ret = wcnss_init_regulators(wcnss, data->vregs, data->num_vregs,
-				    data->num_pd_vregs);
+	ret = wcnss_init_regulators(wcnss, data);
 	if (ret)
 		goto detach_pds;
 
-- 
2.52.0

Re: [PATCH v2] remoteproc: qcom_wcnss: Fix handling the lack of PD regulators in v3

[Konrad Dybcio]

On 2/1/26 9:55 PM, Val Packett wrote:
> The changes introduced to handle single power domain platforms have
> swapped the info pointer increment from num_pd_vregs to num_pds, which
> would shift the info pointer past the end of the array for pronto-v3,
> which does not list power domain regulators in vregs.
> 
> This showed up as a difference between GCC- and LLVM-compiled kernels
> on SDM632 devices, where only with LLVM one would get the
> "regulator request with no identifier" error, because the out-of-bounds
> memory ended up being zeroed. Fix by skipping the increment when there
> are more power domains than regulators.
> 
> Signed-off-by: Val Packett <val@packett.cool>
> ---

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad

Re: [PATCH v2] remoteproc: qcom_wcnss: Fix handling the lack of PD regulators in v3

[Yoann Lecuyer]

> The changes introduced to handle single power domain platforms have
> swapped the info pointer increment from num_pd_vregs to num_pds, which
> would shift the info pointer past the end of the array for pronto-v3,
> which does not list power domain regulators in vregs.
>
> This showed up as a difference between GCC- and LLVM-compiled kernels
> on SDM632 devices, where only with LLVM one would get the
> "regulator request with no identifier" error, because the out-of-bounds
> memory ended up being zeroed. Fix by skipping the increment when there
> are more power domains than regulators.
>
> Signed-off-by: Val Packett <val@packett.cool>

Tested-by: Yoann Lecuyer <yoann.lecuyer@gmail.com>

Yoann

Re: [PATCH v2] remoteproc: qcom_wcnss: Fix handling the lack of PD regulators in v3

[Bjorn Andersson]

On Sun, Feb 01, 2026 at 05:55:03PM -0300, Val Packett wrote:
> The changes introduced to handle single power domain platforms have
> swapped the info pointer increment from num_pd_vregs to num_pds, which
> would shift the info pointer past the end of the array for pronto-v3,
> which does not list power domain regulators in vregs.
> 
> This showed up as a difference between GCC- and LLVM-compiled kernels
> on SDM632 devices, where only with LLVM one would get the
> "regulator request with no identifier" error, because the out-of-bounds
> memory ended up being zeroed. Fix by skipping the increment when there
> are more power domains than regulators.
> 

Is the error only an error print, or did the thing stop working as well?

Should we no longer carry
Fixes: 65991ea8a6d1 ("remoteproc: qcom_wcnss: Handle platforms with only single power domain")

Regards,
Bjorn

> Signed-off-by: Val Packett <val@packett.cool>
> ---
> v2: changed to detect the >= condition suggested by Konrad
> v1: https://lore.kernel.org/all/20260126235018.969140-1-val@packett.cool/
> 
> "possible_pds" is the best name I could come up with (as "num" is already
> taken by the number of *successfully attached* PDs and "max" is the constant
> for the array length) for the count we're checking against. Maybe the "num"
> could be changed to "attached" but that feels like too much diff.
> 
> ~val
> ---
> 
>  drivers/remoteproc/qcom_wcnss.c | 23 ++++++++++++++---------
>  1 file changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/remoteproc/qcom_wcnss.c b/drivers/remoteproc/qcom_wcnss.c
> index ee18bf2e8054..60f629b5bbed 100644
> --- a/drivers/remoteproc/qcom_wcnss.c
> +++ b/drivers/remoteproc/qcom_wcnss.c
> @@ -441,25 +441,31 @@ static void wcnss_release_pds(struct qcom_wcnss *wcnss)
>  }
>  
>  static int wcnss_init_regulators(struct qcom_wcnss *wcnss,
> -				 const struct wcnss_vreg_info *info,
> -				 int num_vregs, int num_pd_vregs)
> +				 const struct wcnss_data *data)
>  {
> +	const struct wcnss_vreg_info *info = data->vregs;
>  	struct regulator_bulk_data *bulk;
> +	size_t i, possible_pds = 0, num_vregs = data->num_vregs;
>  	int ret;
> -	int i;
> +
> +	for (i = 0; i < WCNSS_MAX_PDS; i++)
> +		if (data->pd_names[i])
> +			possible_pds++;
>  
>  	/*
>  	 * If attaching the power domains suceeded we can skip requesting
>  	 * the regulators for the power domains. For old device trees we need to
>  	 * reserve extra space to manage them through the regulator interface.
>  	 */
> -	if (wcnss->num_pds) {
> +	if (possible_pds >= num_vregs) {
> +		/* Do nothing if vregs do not include PD regulators (pronto-v3) */
> +	} else if (wcnss->num_pds) {
>  		info += wcnss->num_pds;
>  		/* Handle single power domain case */
> -		if (wcnss->num_pds < num_pd_vregs)
> -			num_vregs += num_pd_vregs - wcnss->num_pds;
> +		if (wcnss->num_pds < data->num_pd_vregs)
> +			num_vregs += data->num_pd_vregs - wcnss->num_pds;
>  	} else {
> -		num_vregs += num_pd_vregs;
> +		num_vregs += data->num_pd_vregs;
>  	}
>  
>  	bulk = devm_kcalloc(wcnss->dev,
> @@ -607,8 +613,7 @@ static int wcnss_probe(struct platform_device *pdev)
>  	if (ret && (ret != -ENODATA || !data->num_pd_vregs))
>  		return ret;
>  
> -	ret = wcnss_init_regulators(wcnss, data->vregs, data->num_vregs,
> -				    data->num_pd_vregs);
> +	ret = wcnss_init_regulators(wcnss, data);
>  	if (ret)
>  		goto detach_pds;
>  
> -- 
> 2.52.0
> 

Re: [PATCH v2] remoteproc: qcom_wcnss: Fix handling the lack of PD regulators in v3

[Val Packett]

On 2/23/26 5:03 PM, Bjorn Andersson wrote:
> On Sun, Feb 01, 2026 at 05:55:03PM -0300, Val Packett wrote:
>> The changes introduced to handle single power domain platforms have
>> swapped the info pointer increment from num_pd_vregs to num_pds, which
>> would shift the info pointer past the end of the array for pronto-v3,
>> which does not list power domain regulators in vregs.
>>
>> This showed up as a difference between GCC- and LLVM-compiled kernels
>> on SDM632 devices, where only with LLVM one would get the
>> "regulator request with no identifier" error, because the out-of-bounds
>> memory ended up being zeroed. Fix by skipping the increment when there
>> are more power domains than regulators.
>>
> Is the error only an error print, or did the thing stop working as well?

It's a real error. Likely no one would've bothered debugging it if it 
weren't breaking everything :)

It was a blocker for allowing LLVM/clang builds of the msm8953 kernel in 
postmarketOS, the whole reason to dig into this was "switching to clang 
breaks the modem, WTF?!"

~val

Re: [PATCH v2] remoteproc: qcom_wcnss: Fix handling the lack of PD regulators in v3

[Bjorn Andersson]

On Tue, Feb 24, 2026 at 03:45:05PM -0300, Val Packett wrote:
> 
> On 2/23/26 5:03 PM, Bjorn Andersson wrote:
> > On Sun, Feb 01, 2026 at 05:55:03PM -0300, Val Packett wrote:
> > > The changes introduced to handle single power domain platforms have
> > > swapped the info pointer increment from num_pd_vregs to num_pds, which
> > > would shift the info pointer past the end of the array for pronto-v3,
> > > which does not list power domain regulators in vregs.
> > > 
> > > This showed up as a difference between GCC- and LLVM-compiled kernels
> > > on SDM632 devices, where only with LLVM one would get the
> > > "regulator request with no identifier" error, because the out-of-bounds
> > > memory ended up being zeroed. Fix by skipping the increment when there
> > > are more power domains than regulators.
> > > 
> > Is the error only an error print, or did the thing stop working as well?
> 
> It's a real error. Likely no one would've bothered debugging it if it
> weren't breaking everything :)
> 
> It was a blocker for allowing LLVM/clang builds of the msm8953 kernel in
> postmarketOS, the whole reason to dig into this was "switching to clang
> breaks the modem, WTF?!"
> 

Can you provide me with a Fixes: line please?

Regards,
Bjorn

> ~val
> 

Re: [PATCH v2] remoteproc: qcom_wcnss: Fix handling the lack of PD regulators in v3

[Val Packett]

On 2/27/26 12:52 AM, Bjorn Andersson wrote:
> On Tue, Feb 24, 2026 at 03:45:05PM -0300, Val Packett wrote:
>> On 2/23/26 5:03 PM, Bjorn Andersson wrote:
>>> On Sun, Feb 01, 2026 at 05:55:03PM -0300, Val Packett wrote:
>>>> The changes introduced to handle single power domain platforms have
>>>> swapped the info pointer increment from num_pd_vregs to num_pds, which
>>>> would shift the info pointer past the end of the array for pronto-v3,
>>>> which does not list power domain regulators in vregs.
>>>>
>>>> This showed up as a difference between GCC- and LLVM-compiled kernels
>>>> on SDM632 devices, where only with LLVM one would get the
>>>> "regulator request with no identifier" error, because the out-of-bounds
>>>> memory ended up being zeroed. Fix by skipping the increment when there
>>>> are more power domains than regulators.
>>>>
>>> Is the error only an error print, or did the thing stop working as well?
>> It's a real error. Likely no one would've bothered debugging it if it
>> weren't breaking everything :)
>>
>> It was a blocker for allowing LLVM/clang builds of the msm8953 kernel in
>> postmarketOS, the whole reason to dig into this was "switching to clang
>> breaks the modem, WTF?!"
>>
> Can you provide me with a Fixes: line please?
>
Oops, did I lose it at some point? o.0 Sorry!

Fixes: 65991ea8a6d1 ("remoteproc: qcom_wcnss: Handle platforms with only 
single power domain")


~val