[PATCH 0/2] Support single-PD in wcnss driver

[PATCH 0/2] Support single-PD in wcnss driver

[Luca Weiss]

Both MSM8974 and MSM8226 only model CX as power domain, leaving MX & PX
as regulators. Add some patches to support this use case.

Signed-off-by: Luca Weiss <luca@lucaweiss.eu>
---
Matti Lehtimäki (2):
      dt-bindings: remoteproc: qcom,wcnss-pil: Add support for single power-domain platforms
      remoteproc: qcom_wcnss: Handle platforms with only single power domain

 .../bindings/remoteproc/qcom,wcnss-pil.yaml        |  2 ++
 drivers/remoteproc/qcom_wcnss.c                    | 32 +++++++++++++++++-----
 2 files changed, 27 insertions(+), 7 deletions(-)
---
base-commit: 0b58e108042b0ed28a71cd7edf5175999955b233
change-id: 20250129-wcnss-singlepd-f9e6854257ed

Best regards,
-- 
Luca Weiss <luca@lucaweiss.eu>

[PATCH 1/2] dt-bindings: remoteproc: qcom,wcnss-pil: Add support for single power-domain platforms

[Luca Weiss]

From: Matti Lehtimäki <matti.lehtimaki@gmail.com>

Support platforms such as MSM8226 and MSM8974 with only one power rail
(CX) modelled as power domain while MX and PX are regulators.

Signed-off-by: Matti Lehtimäki <matti.lehtimaki@gmail.com>
[luca: reword commit message]
Signed-off-by: Luca Weiss <luca@lucaweiss.eu>
---
 Documentation/devicetree/bindings/remoteproc/qcom,wcnss-pil.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,wcnss-pil.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,wcnss-pil.yaml
index 8e033b22d28cfa8203234f744b3b408e976e20c3..d3c71bcf0f02122eb0dae214f135d8d7f71a9600 100644
--- a/Documentation/devicetree/bindings/remoteproc/qcom,wcnss-pil.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/qcom,wcnss-pil.yaml
@@ -69,9 +69,11 @@ properties:
       CX regulator to be held on behalf of the booting of the WCNSS core.
 
   power-domains:
+    minItems: 1
     maxItems: 2
 
   power-domain-names:
+    minItems: 1
     items:
       - const: cx
       - const: mx

-- 
2.48.1

Re: [PATCH 1/2] dt-bindings: remoteproc: qcom,wcnss-pil: Add support for single power-domain platforms

[Krzysztof Kozlowski]

On 29/01/2025 18:51, Luca Weiss wrote:
> From: Matti Lehtimäki <matti.lehtimaki@gmail.com>
> 
> Support platforms such as MSM8226 and MSM8974 with only one power rail
> (CX) modelled as power domain while MX and PX are regulators.
> 
> Signed-off-by: Matti Lehtimäki <matti.lehtimaki@gmail.com>
> [luca: reword commit message]
> Signed-off-by: Luca Weiss <luca@lucaweiss.eu>
> ---
>  Documentation/devicetree/bindings/remoteproc/qcom,wcnss-pil.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,wcnss-pil.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,wcnss-pil.yaml
> index 8e033b22d28cfa8203234f744b3b408e976e20c3..d3c71bcf0f02122eb0dae214f135d8d7f71a9600 100644
> --- a/Documentation/devicetree/bindings/remoteproc/qcom,wcnss-pil.yaml
> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,wcnss-pil.yaml
> @@ -69,9 +69,11 @@ properties:
>        CX regulator to be held on behalf of the booting of the WCNSS core.
>  
>    power-domains:
> +    minItems: 1
>      maxItems: 2
>  
>    power-domain-names:
> +    minItems: 1


This should be further narrowed in allOf:if:then per each variant,
because now you say that all devices here can have only one power
domain... unless the compatibles do not allow that, but then explain in
commit msg.

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: remoteproc: qcom,wcnss-pil: Add support for single power-domain platforms

[Luca Weiss]

On woensdag 29 januari 2025 19:44:54 Midden-Europese standaardtijd Krzysztof 
Kozlowski wrote:
> On 29/01/2025 18:51, Luca Weiss wrote:
> > From: Matti Lehtimäki <matti.lehtimaki@gmail.com>
> > 
> > Support platforms such as MSM8226 and MSM8974 with only one power rail
> > (CX) modelled as power domain while MX and PX are regulators.
> > 
> > Signed-off-by: Matti Lehtimäki <matti.lehtimaki@gmail.com>
> > [luca: reword commit message]
> > Signed-off-by: Luca Weiss <luca@lucaweiss.eu>
> > ---
> >  Documentation/devicetree/bindings/remoteproc/qcom,wcnss-pil.yaml | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,wcnss-
pil.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,wcnss-pil.yaml
> > index 
8e033b22d28cfa8203234f744b3b408e976e20c3..d3c71bcf0f02122eb0dae214f135d8d7f71a9600 
100644
> > --- a/Documentation/devicetree/bindings/remoteproc/qcom,wcnss-pil.yaml
> > +++ b/Documentation/devicetree/bindings/remoteproc/qcom,wcnss-pil.yaml
> > @@ -69,9 +69,11 @@ properties:
> >        CX regulator to be held on behalf of the booting of the WCNSS core.
> >  
> >    power-domains:
> > +    minItems: 1
> >      maxItems: 2
> >  
> >    power-domain-names:
> > +    minItems: 1
> 
> 
> This should be further narrowed in allOf:if:then per each variant,
> because now you say that all devices here can have only one power
> domain... unless the compatibles do not allow that, but then explain in
> commit msg.

Yes, the compatibles are so broad that they cannot be used to narrow this 
down. I can add this information in v2.

I'd add something like the following. Let me know if that isn't clear enough.

"Due to the compatibles qcom,pronto-vN-pil being so broad we cannot narrow 
this down by SoC without introducing new compatibles."

Regards
Luca

> 
> Best regards,
> Krzysztof
> 

Re: [PATCH 1/2] dt-bindings: remoteproc: qcom,wcnss-pil: Add support for single power-domain platforms

[Stephan Gerhold]

On Wed, Jan 29, 2025 at 10:42:22PM +0100, Luca Weiss wrote:
> On woensdag 29 januari 2025 19:44:54 Midden-Europese standaardtijd Krzysztof 
> Kozlowski wrote:
> > On 29/01/2025 18:51, Luca Weiss wrote:
> > > From: Matti Lehtimäki <matti.lehtimaki@gmail.com>
> > > 
> > > Support platforms such as MSM8226 and MSM8974 with only one power rail
> > > (CX) modelled as power domain while MX and PX are regulators.
> > > 
> > > Signed-off-by: Matti Lehtimäki <matti.lehtimaki@gmail.com>
> > > [luca: reword commit message]
> > > Signed-off-by: Luca Weiss <luca@lucaweiss.eu>
> > > ---
> > >  Documentation/devicetree/bindings/remoteproc/qcom,wcnss-pil.yaml | 2 ++
> > >  1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,wcnss-
> pil.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,wcnss-pil.yaml
> > > index 
> 8e033b22d28cfa8203234f744b3b408e976e20c3..d3c71bcf0f02122eb0dae214f135d8d7f71a9600 
> 100644
> > > --- a/Documentation/devicetree/bindings/remoteproc/qcom,wcnss-pil.yaml
> > > +++ b/Documentation/devicetree/bindings/remoteproc/qcom,wcnss-pil.yaml
> > > @@ -69,9 +69,11 @@ properties:
> > >        CX regulator to be held on behalf of the booting of the WCNSS core.
> > >  
> > >    power-domains:
> > > +    minItems: 1
> > >      maxItems: 2
> > >  
> > >    power-domain-names:
> > > +    minItems: 1
> > 
> > 
> > This should be further narrowed in allOf:if:then per each variant,
> > because now you say that all devices here can have only one power
> > domain... unless the compatibles do not allow that, but then explain in
> > commit msg.
> 
> Yes, the compatibles are so broad that they cannot be used to narrow this 
> down. I can add this information in v2.
> 
> I'd add something like the following. Let me know if that isn't clear enough.
> 
> "Due to the compatibles qcom,pronto-vN-pil being so broad we cannot narrow 
> this down by SoC without introducing new compatibles."
> 

The qcom,pronto-v2-pil compatible isn't too broad here IMO. It describes
a specific hardware block that needs the CX and MX power domains to
operate. The hardware block doesn't care if the power rail is being
turned on via a regulator or power domain. At the end, this is a
firmware difference "leaking" into the hardware description of the DT.

We can describe this in the bindings. Both CX and MX must be present,
either as actual power domains or regulators. For the old setup (=
either PDs OR regulators, but not mixed) it's already there further
below. You need to adjust the part where vddmx-supply is marked as
deprecated, undeprecate it, and then try something like the following
(untested):

  - if:
      properties:
        compatible:
          contains:
            enum:
              - qcom,pronto-v1-pil
              - qcom,pronto-v2-pil
      # ... drop deprecations ...

      # CX and MX must be present either as power domains or regulators
      oneOf:
        # Both CX and MX represented as power domains
        - required:
            - power-domains
            - power-domain-names
          properties:
            vddmx-supply: false
            vddcx-supply: false
        # CX represented as power domain, MX as regulator
        - required:
            - power-domains
            - power-domain-names
            - vddmx-supply
          properties:
            # Not sure if this works here, might need to put minItems: 1
            # into top-level and restrict entry above to minItems: 2
            power-domains:
              minItems: 1
              maxItems: 1
            power-domain-names:
              minItems: 1
              maxItems: 1
            vddcx-supply: false
        # Both CX and MX represented as regulators
        - required:
            - vddmx-supply
            - vddcx-supply
          properties:
            power-domains: false
            power-domain-names: false

Thanks,
Stephan

[PATCH 2/2] remoteproc: qcom_wcnss: Handle platforms with only single power domain

[Luca Weiss]

From: Matti Lehtimäki <matti.lehtimaki@gmail.com>

Both MSM8974 and MSM8226 have only CX as power domain with MX & PX being
handled as regulators. Handle this case by reodering pd_names to have CX
first, and handling that the driver core will already attach a single
power domain internally.

Signed-off-by: Matti Lehtimäki <matti.lehtimaki@gmail.com>
[luca: minor changes]
Signed-off-by: Luca Weiss <luca@lucaweiss.eu>
---
 drivers/remoteproc/qcom_wcnss.c | 32 +++++++++++++++++++++++++-------
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/drivers/remoteproc/qcom_wcnss.c b/drivers/remoteproc/qcom_wcnss.c
index a7bb9da27029db23f3759b19e423fab11b8430e4..4658ffb9bb13dfd52ecb23e85e0ad2d36af0cc80 100644
--- a/drivers/remoteproc/qcom_wcnss.c
+++ b/drivers/remoteproc/qcom_wcnss.c
@@ -117,10 +117,10 @@ static const struct wcnss_data pronto_v1_data = {
 	.pmu_offset = 0x1004,
 	.spare_offset = 0x1088,
 
-	.pd_names = { "mx", "cx" },
+	.pd_names = { "cx", "mx" },
 	.vregs = (struct wcnss_vreg_info[]) {
-		{ "vddmx", 950000, 1150000, 0 },
 		{ "vddcx", .super_turbo = true},
+		{ "vddmx", 950000, 1150000, 0 },
 		{ "vddpx", 1800000, 1800000, 0 },
 	},
 	.num_pd_vregs = 2,
@@ -131,10 +131,10 @@ static const struct wcnss_data pronto_v2_data = {
 	.pmu_offset = 0x1004,
 	.spare_offset = 0x1088,
 
-	.pd_names = { "mx", "cx" },
+	.pd_names = { "cx", "mx" },
 	.vregs = (struct wcnss_vreg_info[]) {
-		{ "vddmx", 1287500, 1287500, 0 },
 		{ "vddcx", .super_turbo = true },
+		{ "vddmx", 1287500, 1287500, 0 },
 		{ "vddpx", 1800000, 1800000, 0 },
 	},
 	.num_pd_vregs = 2,
@@ -397,8 +397,17 @@ static irqreturn_t wcnss_stop_ack_interrupt(int irq, void *dev)
 static int wcnss_init_pds(struct qcom_wcnss *wcnss,
 			  const char * const pd_names[WCNSS_MAX_PDS])
 {
+	struct device *dev = wcnss->dev;
 	int i, ret;
 
+	/* Handle single power domain */
+	if (dev->pm_domain) {
+		wcnss->pds[0] = dev;
+		wcnss->num_pds = 1;
+		pm_runtime_enable(dev);
+		return 0;
+	}
+
 	for (i = 0; i < WCNSS_MAX_PDS; i++) {
 		if (!pd_names[i])
 			break;
@@ -418,8 +427,15 @@ static int wcnss_init_pds(struct qcom_wcnss *wcnss,
 
 static void wcnss_release_pds(struct qcom_wcnss *wcnss)
 {
+	struct device *dev = wcnss->dev;
 	int i;
 
+	/* Handle single power domain */
+	if (wcnss->num_pds == 1 && dev->pm_domain) {
+		pm_runtime_disable(dev);
+		return;
+	}
+
 	for (i = 0; i < wcnss->num_pds; i++)
 		dev_pm_domain_detach(wcnss->pds[i], false);
 }
@@ -437,9 +453,11 @@ static int wcnss_init_regulators(struct qcom_wcnss *wcnss,
 	 * 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)
-		info += num_pd_vregs;
-	else
+	if (wcnss->num_pds) {
+		info += wcnss->num_pds;
+		/* Handle single power domain case */
+		num_vregs += num_pd_vregs - wcnss->num_pds;
+	} else
 		num_vregs += num_pd_vregs;
 
 	bulk = devm_kcalloc(wcnss->dev,

-- 
2.48.1