[PATCH v2 0/9] Modem support for MSM8226

[PATCH v2 0/9] Modem support for MSM8226

[Luca Weiss]

This series adds support for modem remoteproc found on MSM8226.
It also adds needed device tree nodes and enables modem used for
location service on matisse-wifi.

Also bam-dmux node is added for mobile data which is used on
3G/4G-enabled devices.

Signed-off-by: Luca Weiss <luca@lucaweiss.eu>
---
Changes in v2:
- Drop applied tcsr-msm8226 dt-bindings patch
- Replace qcom,ipc-* with mboxes in smsm node
- Replace qcom,ipc with mboxes in smp2p nodes
- Drop mba & mpss subnodes in modem dts
- Disable added modem in apq8026 smartwatches due to lack of any useful
  modem firmware
- Add patch for bam-dmux
- See individual patches for more!

---
Luca Weiss (4):
      remoteproc: qcom_q6v5_mss: Handle platforms with one power domain
      remoteproc: qcom_q6v5_mss: Add modem support on MSM8226
      ARM: dts: qcom: msm8226: Add modem remoteproc node
      ARM: dts: qcom: msm8226: Add BAM DMUX Ethernet/IP device

Matti Lehtimäki (5):
      dt-bindings: remoteproc: qcom,msm8916-mss-pil: Support platforms with one power domain
      dt-bindings: remoteproc: qcom,msm8916-mss-pil: Add MSM8226
      ARM: dts: qcom: msm8226: Add node for TCSR halt regs
      ARM: dts: qcom: msm8226: Add smsm node
      ARM: dts: qcom: msm8226-samsung-matisse-common: Enable modem

 .../bindings/remoteproc/qcom,msm8916-mss-pil.yaml  |  56 +++++++-
 .../boot/dts/qcom/qcom-apq8026-asus-sparrow.dts    |   7 +
 .../boot/dts/qcom/qcom-apq8026-huawei-sturgeon.dts |   7 +
 arch/arm/boot/dts/qcom/qcom-apq8026-lg-lenok.dts   |   7 +
 .../dts/qcom/qcom-apq8026-samsung-milletwifi.dts   |   2 +
 .../qcom/qcom-msm8226-samsung-matisse-common.dtsi  |  13 +-
 arch/arm/boot/dts/qcom/qcom-msm8226.dtsi           | 145 +++++++++++++++++++++
 arch/arm/boot/dts/qcom/qcom-msm8926-htc-memul.dts  |   2 +
 drivers/remoteproc/qcom_q6v5_mss.c                 | 137 ++++++++++++++++++-
 9 files changed, 367 insertions(+), 9 deletions(-)
---
base-commit: 15fbefd533c38e72315c25ae3746005fddb4b8e3
change-id: 20250107-msm8226-modem-f15a1e6f8d84

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

[PATCH v2 1/9] dt-bindings: remoteproc: qcom,msm8916-mss-pil: Support platforms with one power domain

[Luca Weiss]

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

Add support for platforms such as MSM8974 which have the mx voltage rail
exposed as regulator and only cx voltage rail as power domain.

Signed-off-by: Matti Lehtimäki <matti.lehtimaki@gmail.com>
Signed-off-by: Luca Weiss <luca@lucaweiss.eu>
---
New patch in v2 split from previous first patch
  - Improve description of changes
  - Add one power domain changes to existing MSM8974
  - Add missing minItems to other platforms
  - Require mx-supply for MSM8974
---
 .../bindings/remoteproc/qcom,msm8916-mss-pil.yaml  | 34 +++++++++++++++++++---
 1 file changed, 30 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,msm8916-mss-pil.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,msm8916-mss-pil.yaml
index 588b010b2a9e53946a615219fc1468bc700deef8..c81364daa34b545e99261bf1f94ae2ad28d48ea7 100644
--- a/Documentation/devicetree/bindings/remoteproc/qcom,msm8916-mss-pil.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/qcom,msm8916-mss-pil.yaml
@@ -70,16 +70,17 @@ properties:
     items:
       - description: CX proxy power domain (control handed over after startup)
       - description: MX proxy power domain (control handed over after startup)
+                     (not valid for qcom,msm8974-mss-pil)
       - description: MSS proxy power domain (control handed over after startup)
                      (only valid for qcom,msm8953-mss-pil)
-    minItems: 2
+    minItems: 1
 
   power-domain-names:
     items:
       - const: cx
-      - const: mx
+      - const: mx # not valid for qcom,msm8974-mss-pil
       - const: mss # only valid for qcom,msm8953-mss-pil
-    minItems: 2
+    minItems: 1
 
   pll-supply:
     description: PLL proxy supply (control handed over after startup)
@@ -207,13 +208,38 @@ allOf:
       required:
         - power-domains
         - power-domain-names
-    else:
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,msm8909-mss-pil
+              - qcom,msm8916-mss-pil
+    then:
       properties:
         power-domains:
+          minItems: 2
           maxItems: 2
         power-domain-names:
+          minItems: 2
           maxItems: 2
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,msm8974-mss-pil
+    then:
+      properties:
+        power-domains:
+          maxItems: 1
+        power-domain-names:
+          maxItems: 1
+      required:
+        - mx-supply
+
   - if:
       properties:
         compatible:

-- 
2.48.1

Re: [PATCH v2 1/9] dt-bindings: remoteproc: qcom,msm8916-mss-pil: Support platforms with one power domain

[Krzysztof Kozlowski]

On Sun, Jan 26, 2025 at 09:57:20PM +0100, Luca Weiss wrote:
> From: Matti Lehtimäki <matti.lehtimaki@gmail.com>
> 
> Add support for platforms such as MSM8974 which have the mx voltage rail
> exposed as regulator and only cx voltage rail as power domain.
> 
> Signed-off-by: Matti Lehtimäki <matti.lehtimaki@gmail.com>
> Signed-off-by: Luca Weiss <luca@lucaweiss.eu>
> ---
> New patch in v2 split from previous first patch
>   - Improve description of changes
>   - Add one power domain changes to existing MSM8974
>   - Add missing minItems to other platforms
>   - Require mx-supply for MSM8974
> ---
>  .../bindings/remoteproc/qcom,msm8916-mss-pil.yaml  | 34 +++++++++++++++++++---
>  1 file changed, 30 insertions(+), 4 deletions(-)

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

[PATCH v2 2/9] dt-bindings: remoteproc: qcom,msm8916-mss-pil: Add MSM8226

[Luca Weiss]

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

Add support for external power block headswitch (BHS) registers.
Similarly to MSM8974 also MSM8226 has the mx voltage rail exposed as
regulator and only cx voltage rail as power domain.

Signed-off-by: Matti Lehtimäki <matti.lehtimaki@gmail.com>
Signed-off-by: Luca Weiss <luca@lucaweiss.eu>
---
Changes in v2:
  - Improve description of changes
  - Require mx-supply for MSM8226
  - Disallow qcom,ext-bhs-reg for other platforms
---
 .../bindings/remoteproc/qcom,msm8916-mss-pil.yaml  | 26 ++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,msm8916-mss-pil.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,msm8916-mss-pil.yaml
index c81364daa34b545e99261bf1f94ae2ad28d48ea7..0a3e39c55ec3dcfc9a4d4ac28f5debd77dc49b99 100644
--- a/Documentation/devicetree/bindings/remoteproc/qcom,msm8916-mss-pil.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/qcom,msm8916-mss-pil.yaml
@@ -17,6 +17,7 @@ properties:
   compatible:
     oneOf:
       - enum:
+          - qcom,msm8226-mss-pil
           - qcom,msm8909-mss-pil
           - qcom,msm8916-mss-pil
           - qcom,msm8953-mss-pil
@@ -70,7 +71,7 @@ properties:
     items:
       - description: CX proxy power domain (control handed over after startup)
       - description: MX proxy power domain (control handed over after startup)
-                     (not valid for qcom,msm8974-mss-pil)
+                     (not valid for qcom,msm8226-mss-pil or qcom,msm8974-mss-pil)
       - description: MSS proxy power domain (control handed over after startup)
                      (only valid for qcom,msm8953-mss-pil)
     minItems: 1
@@ -78,7 +79,7 @@ properties:
   power-domain-names:
     items:
       - const: cx
-      - const: mx # not valid for qcom,msm8974-mss-pil
+      - const: mx # not valid for qcom,msm8226-mss-pil or qcom,msm8974-mss-pil
       - const: mss # only valid for qcom,msm8953-mss-pil
     minItems: 1
 
@@ -107,6 +108,15 @@ properties:
     items:
       - const: stop
 
+  qcom,ext-bhs-reg:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    description: External power block headswitch (BHS) register
+                 (only valid for qcom,msm8226-mss-pil)
+    items:
+      - items:
+          - description: phandle to external BHS syscon region
+          - description: offset to the external BHS register
+
   qcom,halt-regs:
     $ref: /schemas/types.yaml#/definitions/phandle-array
     description:
@@ -230,6 +240,7 @@ allOf:
         compatible:
           contains:
             enum:
+              - qcom,msm8226-mss-pil
               - qcom,msm8974-mss-pil
     then:
       properties:
@@ -240,6 +251,17 @@ allOf:
       required:
         - mx-supply
 
+  - if:
+      properties:
+        compatible:
+          const: qcom,msm8226-mss-pil
+    then:
+      required:
+        - qcom,ext-bhs-reg
+    else:
+      properties:
+        qcom,ext-bhs-reg: false
+
   - if:
       properties:
         compatible:

-- 
2.48.1

Re: [PATCH v2 2/9] dt-bindings: remoteproc: qcom,msm8916-mss-pil: Add MSM8226

[Krzysztof Kozlowski]

On Sun, Jan 26, 2025 at 09:57:21PM +0100, Luca Weiss wrote:
> From: Matti Lehtimäki <matti.lehtimaki@gmail.com>
> 
> Add support for external power block headswitch (BHS) registers.
> Similarly to MSM8974 also MSM8226 has the mx voltage rail exposed as
> regulator and only cx voltage rail as power domain.
> 
> Signed-off-by: Matti Lehtimäki <matti.lehtimaki@gmail.com>
> Signed-off-by: Luca Weiss <luca@lucaweiss.eu>
> ---
> Changes in v2:
>   - Improve description of changes
>   - Require mx-supply for MSM8226
>   - Disallow qcom,ext-bhs-reg for other platforms
> ---
>  .../bindings/remoteproc/qcom,msm8916-mss-pil.yaml  | 26 ++++++++++++++++++++--
>  1 file changed, 24 insertions(+), 2 deletions(-)

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

[PATCH v2 3/9] remoteproc: qcom_q6v5_mss: Handle platforms with one power domain

[Luca Weiss]

For example MSM8974 has mx voltage rail exposed as regulator and only cx
voltage rail is exposed as power domain. This power domain (cx) is
attached internally in power domain and cannot be attached in this driver.

Fixes: 8750cf392394 ("remoteproc: qcom_q6v5_mss: Allow replacing regulators with power domains")
Co-developed-by: Matti Lehtimäki <matti.lehtimaki@gmail.com>
Signed-off-by: Matti Lehtimäki <matti.lehtimaki@gmail.com>
Signed-off-by: Luca Weiss <luca@lucaweiss.eu>
---
Changes in v2:
  - Move MSM8974 mx-supply from "fallback_proxy_supply" to
    "proxy_supply" to match updated DT schema
  - Add fixes tag
---
 drivers/remoteproc/qcom_q6v5_mss.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
index e78bd986dc3f256effce4470222c0a5faeea86ec..e2523b01febf393abfe50740a68b85a04011293c 100644
--- a/drivers/remoteproc/qcom_q6v5_mss.c
+++ b/drivers/remoteproc/qcom_q6v5_mss.c
@@ -1828,6 +1828,13 @@ static int q6v5_pds_attach(struct device *dev, struct device **devs,
 	if (!pd_names)
 		return 0;
 
+	/* Handle single power domain */
+	if (dev->pm_domain) {
+		devs[0] = dev;
+		pm_runtime_enable(dev);
+		return 1;
+	}
+
 	while (pd_names[num_pds])
 		num_pds++;
 
@@ -1851,8 +1858,15 @@ static int q6v5_pds_attach(struct device *dev, struct device **devs,
 static void q6v5_pds_detach(struct q6v5 *qproc, struct device **pds,
 			    size_t pd_count)
 {
+	struct device *dev = qproc->dev;
 	int i;
 
+	/* Handle single power domain */
+	if (dev->pm_domain && pd_count) {
+		pm_runtime_disable(dev);
+		return;
+	}
+
 	for (i = 0; i < pd_count; i++)
 		dev_pm_domain_detach(pds[i], false);
 }
@@ -2449,13 +2463,13 @@ static const struct rproc_hexagon_res msm8974_mss = {
 			.supply = "pll",
 			.uA = 100000,
 		},
-		{}
-	},
-	.fallback_proxy_supply = (struct qcom_mss_reg_res[]) {
 		{
 			.supply = "mx",
 			.uV = 1050000,
 		},
+		{}
+	},
+	.fallback_proxy_supply = (struct qcom_mss_reg_res[]) {
 		{
 			.supply = "cx",
 			.uA = 100000,

-- 
2.48.1

Re: [PATCH v2 3/9] remoteproc: qcom_q6v5_mss: Handle platforms with one power domain

[Stephan Gerhold]

On Sun, Jan 26, 2025 at 09:57:22PM +0100, Luca Weiss wrote:
> For example MSM8974 has mx voltage rail exposed as regulator and only cx
> voltage rail is exposed as power domain. This power domain (cx) is
> attached internally in power domain and cannot be attached in this driver.
> 
> Fixes: 8750cf392394 ("remoteproc: qcom_q6v5_mss: Allow replacing regulators with power domains")
> Co-developed-by: Matti Lehtimäki <matti.lehtimaki@gmail.com>
> Signed-off-by: Matti Lehtimäki <matti.lehtimaki@gmail.com>
> Signed-off-by: Luca Weiss <luca@lucaweiss.eu>
> ---
> Changes in v2:
>   - Move MSM8974 mx-supply from "fallback_proxy_supply" to
>     "proxy_supply" to match updated DT schema
>   - Add fixes tag
> ---
>  drivers/remoteproc/qcom_q6v5_mss.c | 20 +++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
> index e78bd986dc3f256effce4470222c0a5faeea86ec..e2523b01febf393abfe50740a68b85a04011293c 100644
> --- a/drivers/remoteproc/qcom_q6v5_mss.c
> +++ b/drivers/remoteproc/qcom_q6v5_mss.c
> @@ -1828,6 +1828,13 @@ static int q6v5_pds_attach(struct device *dev, struct device **devs,
>  	if (!pd_names)
>  		return 0;
>  
> +	/* Handle single power domain */
> +	if (dev->pm_domain) {
> +		devs[0] = dev;
> +		pm_runtime_enable(dev);
> +		return 1;
> +	}
> +
>  	while (pd_names[num_pds])
>  		num_pds++;

Hmm, I think you should put the above if condition below this loop and
verify that num_pds == 1. Otherwise this would hide the error condition
if the platform needs multple PDs, but someone only specifies one of
them in the DT. i.e.

	if (num_pds == 1 && dev->pm_domain) {
		// ...
	}

>  
> @@ -1851,8 +1858,15 @@ static int q6v5_pds_attach(struct device *dev, struct device **devs,
>  static void q6v5_pds_detach(struct q6v5 *qproc, struct device **pds,
>  			    size_t pd_count)
>  {
> +	struct device *dev = qproc->dev;
>  	int i;
>  
> +	/* Handle single power domain */
> +	if (dev->pm_domain && pd_count) {

Maybe if (pd_count == 1 && dev->pm_domain) for consistency with the
above then.

> +		pm_runtime_disable(dev);

I'm guessing it does, but just to make sure: Have you verified that the
power domain is indeed voted for off after this call to
pm_runtime_disable()? Start the remoteproc and when it's booted inspect
/sys/kernel/debug/pm_genpd/pm_genpd_summary to see if the PD/remoteproc
dev is suspended.

Thanks,
Stephan

Re: [PATCH v2 3/9] remoteproc: qcom_q6v5_mss: Handle platforms with one power domain

[Luca Weiss]

On maandag 27 januari 2025 09:58:45 Midden-Europese standaardtijd Stephan Gerhold wrote:
> On Sun, Jan 26, 2025 at 09:57:22PM +0100, Luca Weiss wrote:
> > For example MSM8974 has mx voltage rail exposed as regulator and only cx
> > voltage rail is exposed as power domain. This power domain (cx) is
> > attached internally in power domain and cannot be attached in this driver.
> > 
> > Fixes: 8750cf392394 ("remoteproc: qcom_q6v5_mss: Allow replacing regulators with power domains")
> > Co-developed-by: Matti Lehtimäki <matti.lehtimaki@gmail.com>
> > Signed-off-by: Matti Lehtimäki <matti.lehtimaki@gmail.com>
> > Signed-off-by: Luca Weiss <luca@lucaweiss.eu>
> > ---
> > Changes in v2:
> >   - Move MSM8974 mx-supply from "fallback_proxy_supply" to
> >     "proxy_supply" to match updated DT schema
> >   - Add fixes tag
> > ---
> >  drivers/remoteproc/qcom_q6v5_mss.c | 20 +++++++++++++++++---
> >  1 file changed, 17 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
> > index e78bd986dc3f256effce4470222c0a5faeea86ec..e2523b01febf393abfe50740a68b85a04011293c 100644
> > --- a/drivers/remoteproc/qcom_q6v5_mss.c
> > +++ b/drivers/remoteproc/qcom_q6v5_mss.c
> > @@ -1828,6 +1828,13 @@ static int q6v5_pds_attach(struct device *dev, struct device **devs,
> >  	if (!pd_names)
> >  		return 0;
> >  
> > +	/* Handle single power domain */
> > +	if (dev->pm_domain) {
> > +		devs[0] = dev;
> > +		pm_runtime_enable(dev);
> > +		return 1;
> > +	}
> > +
> >  	while (pd_names[num_pds])
> >  		num_pds++;
> 
> Hmm, I think you should put the above if condition below this loop and
> verify that num_pds == 1. Otherwise this would hide the error condition
> if the platform needs multple PDs, but someone only specifies one of
> them in the DT. i.e.
> 
> 	if (num_pds == 1 && dev->pm_domain) {
> 		// ...
> 	}
> 
> >  
> > @@ -1851,8 +1858,15 @@ static int q6v5_pds_attach(struct device *dev, struct device **devs,
> >  static void q6v5_pds_detach(struct q6v5 *qproc, struct device **pds,
> >  			    size_t pd_count)
> >  {
> > +	struct device *dev = qproc->dev;
> >  	int i;
> >  
> > +	/* Handle single power domain */
> > +	if (dev->pm_domain && pd_count) {
> 
> Maybe if (pd_count == 1 && dev->pm_domain) for consistency with the
> above then.

Good suggestions, thanks!

> 
> > +		pm_runtime_disable(dev);
> 
> I'm guessing it does, but just to make sure: Have you verified that the
> power domain is indeed voted for off after this call to
> pm_runtime_disable()? Start the remoteproc and when it's booted inspect
> /sys/kernel/debug/pm_genpd/pm_genpd_summary to see if the PD/remoteproc
> dev is suspended.

Looks sane I think (modem: remoteproc@fc880000)

htc-memul:~$ sudo cat /sys/kernel/debug/pm_genpd/pm_genpd_summary
domain                          status          children        performance
    /device                         runtime status                  managed by
------------------------------------------------------------------------------
oxili_cx                        off-0                           0
    fdb00000.gpu                    suspended                   0           SW
camss_vfe                       off-0                           0
camss_jpeg                      off-0                           0
mdss                            on                              0
    fd900000.display-subsystem      active                      0           SW
venus0                          off-0                           0
cx_vfc                          off-0                           0
cx_ao                           off-0                           0
cx                              on                              0
    fc880000.remoteproc             suspended                   0           SW
    fe200000.remoteproc             unsupported                 0           SW
    fb204000.remoteproc             suspended                   0           SW
usb_hs_hsic                     off-0                           0


Regards
Luca

> 
> Thanks,
> Stephan
> 

Re: [PATCH v2 3/9] remoteproc: qcom_q6v5_mss: Handle platforms with one power domain

[Stephan Gerhold]

On Mon, Jan 27, 2025 at 11:21:04PM +0100, Luca Weiss wrote:
> On maandag 27 januari 2025 09:58:45 Midden-Europese standaardtijd Stephan Gerhold wrote:
> > On Sun, Jan 26, 2025 at 09:57:22PM +0100, Luca Weiss wrote:
> > > For example MSM8974 has mx voltage rail exposed as regulator and only cx
> > > voltage rail is exposed as power domain. This power domain (cx) is
> > > attached internally in power domain and cannot be attached in this driver.
> > > 
> > > Fixes: 8750cf392394 ("remoteproc: qcom_q6v5_mss: Allow replacing regulators with power domains")
> > > Co-developed-by: Matti Lehtimäki <matti.lehtimaki@gmail.com>
> > > Signed-off-by: Matti Lehtimäki <matti.lehtimaki@gmail.com>
> > > Signed-off-by: Luca Weiss <luca@lucaweiss.eu>
> > > ---
> > > Changes in v2:
> > >   - Move MSM8974 mx-supply from "fallback_proxy_supply" to
> > >     "proxy_supply" to match updated DT schema
> > >   - Add fixes tag
> > > ---
> > >  drivers/remoteproc/qcom_q6v5_mss.c | 20 +++++++++++++++++---
> > >  1 file changed, 17 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
> > > index e78bd986dc3f256effce4470222c0a5faeea86ec..e2523b01febf393abfe50740a68b85a04011293c 100644
> > > --- a/drivers/remoteproc/qcom_q6v5_mss.c
> > > +++ b/drivers/remoteproc/qcom_q6v5_mss.c
> > > @@ -1828,6 +1828,13 @@ static int q6v5_pds_attach(struct device *dev, struct device **devs,
> > >  	if (!pd_names)
> > >  		return 0;
> > >  
> > > +	/* Handle single power domain */
> > > +	if (dev->pm_domain) {
> > > +		devs[0] = dev;
> > > +		pm_runtime_enable(dev);
> > > +		return 1;
> > > +	}
> > > +
> > >  	while (pd_names[num_pds])
> > >  		num_pds++;
> > 
> > Hmm, I think you should put the above if condition below this loop and
> > verify that num_pds == 1. Otherwise this would hide the error condition
> > if the platform needs multple PDs, but someone only specifies one of
> > them in the DT. i.e.
> > 
> > 	if (num_pds == 1 && dev->pm_domain) {
> > 		// ...
> > 	}
> > 
> > >  
> > > @@ -1851,8 +1858,15 @@ static int q6v5_pds_attach(struct device *dev, struct device **devs,
> > >  static void q6v5_pds_detach(struct q6v5 *qproc, struct device **pds,
> > >  			    size_t pd_count)
> > >  {
> > > +	struct device *dev = qproc->dev;
> > >  	int i;
> > >  
> > > +	/* Handle single power domain */
> > > +	if (dev->pm_domain && pd_count) {
> > 
> > Maybe if (pd_count == 1 && dev->pm_domain) for consistency with the
> > above then.
> 
> Good suggestions, thanks!
> 
> > 
> > > +		pm_runtime_disable(dev);
> > 
> > I'm guessing it does, but just to make sure: Have you verified that the
> > power domain is indeed voted for off after this call to
> > pm_runtime_disable()? Start the remoteproc and when it's booted inspect
> > /sys/kernel/debug/pm_genpd/pm_genpd_summary to see if the PD/remoteproc
> > dev is suspended.
> 
> Looks sane I think (modem: remoteproc@fc880000)
> 

The modem does look sane yeah...

> htc-memul:~$ sudo cat /sys/kernel/debug/pm_genpd/pm_genpd_summary
> domain                          status          children        performance
>     /device                         runtime status                  managed by
> ------------------------------------------------------------------------------
> oxili_cx                        off-0                           0
>     fdb00000.gpu                    suspended                   0           SW
> camss_vfe                       off-0                           0
> camss_jpeg                      off-0                           0
> mdss                            on                              0
>     fd900000.display-subsystem      active                      0           SW
> venus0                          off-0                           0
> cx_vfc                          off-0                           0
> cx_ao                           off-0                           0
> cx                              on                              0
>     fc880000.remoteproc             suspended                   0           SW
>     fe200000.remoteproc             unsupported                 0           SW
>     fb204000.remoteproc             suspended                   0           SW

... but "unsupported" for ADSP and the end result for cx ("on") does
look suspicious. Looking at qcom_q6v5_pas.c, it uses almost the same
code for single power domain support as here...

What state is the ADSP in here at this point? Did it boot sucessfully?

If I'm reading the code correctly, the pm_runtime_enable() in
adsp_pds_attach() should get rid of the "unsupported" at probe time,
so I'm a bit confused how this can happen..

[5 minutes of staring later...]

Um, qcom,msm8226-adsp-pil uses &adsp_resource_init, which doesn't have
any "proxy_pd_names". It should use &msm8996_adsp_resource so that it
actually manages the CX power domain. Same for MSM8974, but that was
never converted to use power domains... 🙈

Can you submit a patch that changes at least msm8226 to use
&msm8996_adsp_resource? Would be also good to make the same changes
I suggested here (check num_pds == 1 / pd_count == 1).

Thanks,
Stephan

Re: [PATCH v2 3/9] remoteproc: qcom_q6v5_mss: Handle platforms with one power domain

[Luca Weiss]

On dinsdag 28 januari 2025 08:30:25 Midden-Europese standaardtijd Stephan Gerhold wrote:
> On Mon, Jan 27, 2025 at 11:21:04PM +0100, Luca Weiss wrote:
> > On maandag 27 januari 2025 09:58:45 Midden-Europese standaardtijd Stephan Gerhold wrote:
> > > On Sun, Jan 26, 2025 at 09:57:22PM +0100, Luca Weiss wrote:
> > > > For example MSM8974 has mx voltage rail exposed as regulator and only cx
> > > > voltage rail is exposed as power domain. This power domain (cx) is
> > > > attached internally in power domain and cannot be attached in this driver.
> > > > 
> > > > Fixes: 8750cf392394 ("remoteproc: qcom_q6v5_mss: Allow replacing regulators with power domains")
> > > > Co-developed-by: Matti Lehtimäki <matti.lehtimaki@gmail.com>
> > > > Signed-off-by: Matti Lehtimäki <matti.lehtimaki@gmail.com>
> > > > Signed-off-by: Luca Weiss <luca@lucaweiss.eu>
> > > > ---
> > > > Changes in v2:
> > > >   - Move MSM8974 mx-supply from "fallback_proxy_supply" to
> > > >     "proxy_supply" to match updated DT schema
> > > >   - Add fixes tag
> > > > ---
> > > >  drivers/remoteproc/qcom_q6v5_mss.c | 20 +++++++++++++++++---
> > > >  1 file changed, 17 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
> > > > index e78bd986dc3f256effce4470222c0a5faeea86ec..e2523b01febf393abfe50740a68b85a04011293c 100644
> > > > --- a/drivers/remoteproc/qcom_q6v5_mss.c
> > > > +++ b/drivers/remoteproc/qcom_q6v5_mss.c
> > > > @@ -1828,6 +1828,13 @@ static int q6v5_pds_attach(struct device *dev, struct device **devs,
> > > >  	if (!pd_names)
> > > >  		return 0;
> > > >  
> > > > +	/* Handle single power domain */
> > > > +	if (dev->pm_domain) {
> > > > +		devs[0] = dev;
> > > > +		pm_runtime_enable(dev);
> > > > +		return 1;
> > > > +	}
> > > > +
> > > >  	while (pd_names[num_pds])
> > > >  		num_pds++;
> > > 
> > > Hmm, I think you should put the above if condition below this loop and
> > > verify that num_pds == 1. Otherwise this would hide the error condition
> > > if the platform needs multple PDs, but someone only specifies one of
> > > them in the DT. i.e.
> > > 
> > > 	if (num_pds == 1 && dev->pm_domain) {
> > > 		// ...
> > > 	}
> > > 
> > > >  
> > > > @@ -1851,8 +1858,15 @@ static int q6v5_pds_attach(struct device *dev, struct device **devs,
> > > >  static void q6v5_pds_detach(struct q6v5 *qproc, struct device **pds,
> > > >  			    size_t pd_count)
> > > >  {
> > > > +	struct device *dev = qproc->dev;
> > > >  	int i;
> > > >  
> > > > +	/* Handle single power domain */
> > > > +	if (dev->pm_domain && pd_count) {
> > > 
> > > Maybe if (pd_count == 1 && dev->pm_domain) for consistency with the
> > > above then.
> > 
> > Good suggestions, thanks!
> > 
> > > 
> > > > +		pm_runtime_disable(dev);
> > > 
> > > I'm guessing it does, but just to make sure: Have you verified that the
> > > power domain is indeed voted for off after this call to
> > > pm_runtime_disable()? Start the remoteproc and when it's booted inspect
> > > /sys/kernel/debug/pm_genpd/pm_genpd_summary to see if the PD/remoteproc
> > > dev is suspended.
> > 
> > Looks sane I think (modem: remoteproc@fc880000)
> > 
> 
> The modem does look sane yeah...
> 
> > htc-memul:~$ sudo cat /sys/kernel/debug/pm_genpd/pm_genpd_summary
> > domain                          status          children        performance
> >     /device                         runtime status                  managed by
> > ------------------------------------------------------------------------------
> > oxili_cx                        off-0                           0
> >     fdb00000.gpu                    suspended                   0           SW
> > camss_vfe                       off-0                           0
> > camss_jpeg                      off-0                           0
> > mdss                            on                              0
> >     fd900000.display-subsystem      active                      0           SW
> > venus0                          off-0                           0
> > cx_vfc                          off-0                           0
> > cx_ao                           off-0                           0
> > cx                              on                              0
> >     fc880000.remoteproc             suspended                   0           SW
> >     fe200000.remoteproc             unsupported                 0           SW
> >     fb204000.remoteproc             suspended                   0           SW
> 
> ... but "unsupported" for ADSP and the end result for cx ("on") does
> look suspicious. Looking at qcom_q6v5_pas.c, it uses almost the same
> code for single power domain support as here...
> 
> What state is the ADSP in here at this point? Did it boot sucessfully?
> 
> If I'm reading the code correctly, the pm_runtime_enable() in
> adsp_pds_attach() should get rid of the "unsupported" at probe time,
> so I'm a bit confused how this can happen..
> 
> [5 minutes of staring later...]
> 
> Um, qcom,msm8226-adsp-pil uses &adsp_resource_init, which doesn't have
> any "proxy_pd_names". It should use &msm8996_adsp_resource so that it
> actually manages the CX power domain. Same for MSM8974, but that was
> never converted to use power domains... 🙈

Oh wow, thanks for catching this!

> 
> Can you submit a patch that changes at least msm8226 to use
> &msm8996_adsp_resource? Would be also good to make the same changes
> I suggested here (check num_pds == 1 / pd_count == 1).

Sent a series to fix the above and this:
https://lore.kernel.org/linux-arm-msm/20250128-pas-singlepd-v1-0-85d9ae4b0093@lucaweiss.eu/T/

Regards
Luca

> 
> Thanks,
> Stephan
> 

[PATCH v2 4/9] remoteproc: qcom_q6v5_mss: Add modem support on MSM8226

[Luca Weiss]

Add support for the external power block headswitch register needed by
MSM8226 and some other qcom platforms.

Co-developed-by: Matti Lehtimäki <matti.lehtimaki@gmail.com>
Signed-off-by: Matti Lehtimäki <matti.lehtimaki@gmail.com>
Signed-off-by: Luca Weiss <luca@lucaweiss.eu>
---
Changes in v2:
  - Remove cx-supply from MSM8226 "fallback_proxy_supply" because it's
    always used as power domain
---
 drivers/remoteproc/qcom_q6v5_mss.c | 117 +++++++++++++++++++++++++++++++++++++
 1 file changed, 117 insertions(+)

diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
index e2523b01febf393abfe50740a68b85a04011293c..ef85c9843a12acb0e4e9ee6167c34d4981d4dc8a 100644
--- a/drivers/remoteproc/qcom_q6v5_mss.c
+++ b/drivers/remoteproc/qcom_q6v5_mss.c
@@ -134,6 +134,11 @@
 #define BOOT_FSM_TIMEOUT                10000
 #define BHS_CHECK_MAX_LOOPS             200
 
+/* External power block headswitch */
+#define EXTERNAL_BHS_ON			BIT(0)
+#define EXTERNAL_BHS_STATUS		BIT(4)
+#define EXTERNAL_BHS_TIMEOUT_US		50
+
 struct reg_info {
 	struct regulator *reg;
 	int uV;
@@ -161,6 +166,7 @@ struct rproc_hexagon_res {
 	bool has_mba_logs;
 	bool has_spare_reg;
 	bool has_qaccept_regs;
+	bool has_ext_bhs_reg;
 	bool has_ext_cntl_regs;
 	bool has_vq6;
 };
@@ -180,6 +186,7 @@ struct q6v5 {
 	u32 halt_nc;
 	u32 halt_vq6;
 	u32 conn_box;
+	u32 ext_bhs;
 
 	u32 qaccept_mdm;
 	u32 qaccept_cx;
@@ -237,6 +244,7 @@ struct q6v5 {
 	bool has_mba_logs;
 	bool has_spare_reg;
 	bool has_qaccept_regs;
+	bool has_ext_bhs_reg;
 	bool has_ext_cntl_regs;
 	bool has_vq6;
 	u64 mpss_perm;
@@ -246,6 +254,7 @@ struct q6v5 {
 };
 
 enum {
+	MSS_MSM8226,
 	MSS_MSM8909,
 	MSS_MSM8916,
 	MSS_MSM8953,
@@ -1750,6 +1759,23 @@ static int q6v5_init_mem(struct q6v5 *qproc, struct platform_device *pdev)
 		qproc->qaccept_axi = args.args[2];
 	}
 
+	if (qproc->has_ext_bhs_reg) {
+		ret = of_parse_phandle_with_fixed_args(pdev->dev.of_node,
+						       "qcom,ext-bhs-reg",
+						       1, 0, &args);
+		if (ret < 0) {
+			dev_err(&pdev->dev, "failed to parse ext-bhs-reg index 0\n");
+			return -EINVAL;
+		}
+
+		qproc->conn_map = syscon_node_to_regmap(args.np);
+		of_node_put(args.np);
+		if (IS_ERR(qproc->conn_map))
+			return PTR_ERR(qproc->conn_map);
+
+		qproc->ext_bhs = args.args[0];
+	}
+
 	if (qproc->has_ext_cntl_regs) {
 		ret = of_parse_phandle_with_fixed_args(pdev->dev.of_node,
 						       "qcom,ext-regs",
@@ -1871,6 +1897,36 @@ static void q6v5_pds_detach(struct q6v5 *qproc, struct device **pds,
 		dev_pm_domain_detach(pds[i], false);
 }
 
+static int q6v5_external_bhs_enable(struct q6v5 *qproc)
+{
+	u32 val;
+	int ret = 0;
+
+	/*
+	 * Enable external power block headswitch and wait for it to
+	 * stabilize
+	 */
+	regmap_update_bits(qproc->conn_map, qproc->ext_bhs,
+			   EXTERNAL_BHS_ON, 1);
+
+	ret = regmap_read_poll_timeout(qproc->conn_map, qproc->ext_bhs,
+				       val, val & EXTERNAL_BHS_STATUS,
+				       1, EXTERNAL_BHS_TIMEOUT_US);
+
+	if (ret) {
+		dev_err(qproc->dev, "External BHS timed out\n");
+		ret = -ETIMEDOUT;
+	}
+
+	return ret;
+}
+
+static void q6v5_external_bhs_disable(struct q6v5 *qproc)
+{
+	regmap_update_bits(qproc->conn_map, qproc->ext_bhs,
+			   EXTERNAL_BHS_ON, 0);
+}
+
 static int q6v5_init_reset(struct q6v5 *qproc)
 {
 	qproc->mss_restart = devm_reset_control_get_exclusive(qproc->dev,
@@ -2021,6 +2077,7 @@ static int q6v5_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, qproc);
 
 	qproc->has_qaccept_regs = desc->has_qaccept_regs;
+	qproc->has_ext_bhs_reg = desc->has_ext_bhs_reg;
 	qproc->has_ext_cntl_regs = desc->has_ext_cntl_regs;
 	qproc->has_vq6 = desc->has_vq6;
 	qproc->has_spare_reg = desc->has_spare_reg;
@@ -2079,6 +2136,14 @@ static int q6v5_probe(struct platform_device *pdev)
 		qproc->proxy_pd_count = ret;
 	}
 
+	if (qproc->has_ext_bhs_reg) {
+		ret = q6v5_external_bhs_enable(qproc);
+		if (ret < 0) {
+			dev_err(&pdev->dev, "Failed to enable external BHS.\n");
+			goto detach_proxy_pds;
+		}
+	}
+
 	qproc->has_alt_reset = desc->has_alt_reset;
 	ret = q6v5_init_reset(qproc);
 	if (ret)
@@ -2143,6 +2208,9 @@ static void q6v5_remove(struct platform_device *pdev)
 	qcom_remove_smd_subdev(rproc, &qproc->smd_subdev);
 	qcom_remove_glink_subdev(rproc, &qproc->glink_subdev);
 
+	if (qproc->has_ext_bhs_reg)
+		q6v5_external_bhs_disable(qproc);
+
 	q6v5_pds_detach(qproc, qproc->proxy_pds, qproc->proxy_pd_count);
 }
 
@@ -2174,6 +2242,7 @@ static const struct rproc_hexagon_res sc7180_mss = {
 	.has_mba_logs = true,
 	.has_spare_reg = true,
 	.has_qaccept_regs = false,
+	.has_ext_bhs_reg = false,
 	.has_ext_cntl_regs = false,
 	.has_vq6 = false,
 	.version = MSS_SC7180,
@@ -2202,6 +2271,7 @@ static const struct rproc_hexagon_res sc7280_mss = {
 	.has_mba_logs = true,
 	.has_spare_reg = false,
 	.has_qaccept_regs = true,
+	.has_ext_bhs_reg = false,
 	.has_ext_cntl_regs = true,
 	.has_vq6 = true,
 	.version = MSS_SC7280,
@@ -2233,6 +2303,7 @@ static const struct rproc_hexagon_res sdm660_mss = {
 	.has_mba_logs = false,
 	.has_spare_reg = false,
 	.has_qaccept_regs = false,
+	.has_ext_bhs_reg = false,
 	.has_ext_cntl_regs = false,
 	.has_vq6 = false,
 	.version = MSS_SDM660,
@@ -2268,6 +2339,7 @@ static const struct rproc_hexagon_res sdm845_mss = {
 	.has_mba_logs = false,
 	.has_spare_reg = false,
 	.has_qaccept_regs = false,
+	.has_ext_bhs_reg = false,
 	.has_ext_cntl_regs = false,
 	.has_vq6 = false,
 	.version = MSS_SDM845,
@@ -2299,6 +2371,7 @@ static const struct rproc_hexagon_res msm8998_mss = {
 	.has_mba_logs = false,
 	.has_spare_reg = false,
 	.has_qaccept_regs = false,
+	.has_ext_bhs_reg = false,
 	.has_ext_cntl_regs = false,
 	.has_vq6 = false,
 	.version = MSS_MSM8998,
@@ -2337,6 +2410,7 @@ static const struct rproc_hexagon_res msm8996_mss = {
 	.has_mba_logs = false,
 	.has_spare_reg = false,
 	.has_qaccept_regs = false,
+	.has_ext_bhs_reg = false,
 	.has_ext_cntl_regs = false,
 	.has_vq6 = false,
 	.version = MSS_MSM8996,
@@ -2371,6 +2445,7 @@ static const struct rproc_hexagon_res msm8909_mss = {
 	.has_mba_logs = false,
 	.has_spare_reg = false,
 	.has_qaccept_regs = false,
+	.has_ext_bhs_reg = false,
 	.has_ext_cntl_regs = false,
 	.has_vq6 = false,
 	.version = MSS_MSM8909,
@@ -2416,6 +2491,7 @@ static const struct rproc_hexagon_res msm8916_mss = {
 	.has_mba_logs = false,
 	.has_spare_reg = false,
 	.has_qaccept_regs = false,
+	.has_ext_bhs_reg = false,
 	.has_ext_cntl_regs = false,
 	.has_vq6 = false,
 	.version = MSS_MSM8916,
@@ -2451,6 +2527,7 @@ static const struct rproc_hexagon_res msm8953_mss = {
 	.has_mba_logs = false,
 	.has_spare_reg = false,
 	.has_qaccept_regs = false,
+	.has_ext_bhs_reg = false,
 	.has_ext_cntl_regs = false,
 	.has_vq6 = false,
 	.version = MSS_MSM8953,
@@ -2504,13 +2581,53 @@ static const struct rproc_hexagon_res msm8974_mss = {
 	.has_mba_logs = false,
 	.has_spare_reg = false,
 	.has_qaccept_regs = false,
+	.has_ext_bhs_reg = false,
 	.has_ext_cntl_regs = false,
 	.has_vq6 = false,
 	.version = MSS_MSM8974,
 };
 
+static const struct rproc_hexagon_res msm8226_mss = {
+	.hexagon_mba_image = "mba.b00",
+	.proxy_supply = (struct qcom_mss_reg_res[]) {
+		{
+			.supply = "pll",
+			.uA = 100000,
+		},
+		{
+			.supply = "mx",
+			.uV = 1050000,
+		},
+		{}
+	},
+	.proxy_clk_names = (char*[]){
+		"xo",
+		NULL
+	},
+	.active_clk_names = (char*[]){
+		"iface",
+		"bus",
+		"mem",
+		NULL
+	},
+	.proxy_pd_names = (char*[]){
+		"cx",
+		NULL
+	},
+	.need_mem_protection = false,
+	.has_alt_reset = false,
+	.has_mba_logs = false,
+	.has_spare_reg = false,
+	.has_qaccept_regs = false,
+	.has_ext_bhs_reg = true,
+	.has_ext_cntl_regs = false,
+	.has_vq6 = false,
+	.version = MSS_MSM8226,
+};
+
 static const struct of_device_id q6v5_of_match[] = {
 	{ .compatible = "qcom,q6v5-pil", .data = &msm8916_mss},
+	{ .compatible = "qcom,msm8226-mss-pil", .data = &msm8226_mss},
 	{ .compatible = "qcom,msm8909-mss-pil", .data = &msm8909_mss},
 	{ .compatible = "qcom,msm8916-mss-pil", .data = &msm8916_mss},
 	{ .compatible = "qcom,msm8953-mss-pil", .data = &msm8953_mss},

-- 
2.48.1

Re: [PATCH v2 4/9] remoteproc: qcom_q6v5_mss: Add modem support on MSM8226

[Stephan Gerhold]

On Sun, Jan 26, 2025 at 09:57:23PM +0100, Luca Weiss wrote:
> Add support for the external power block headswitch register needed by
> MSM8226 and some other qcom platforms.
> 
> Co-developed-by: Matti Lehtimäki <matti.lehtimaki@gmail.com>
> Signed-off-by: Matti Lehtimäki <matti.lehtimaki@gmail.com>
> Signed-off-by: Luca Weiss <luca@lucaweiss.eu>
> ---
> Changes in v2:
>   - Remove cx-supply from MSM8226 "fallback_proxy_supply" because it's
>     always used as power domain
> ---
>  drivers/remoteproc/qcom_q6v5_mss.c | 117 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 117 insertions(+)
> 
> diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
> index e2523b01febf393abfe50740a68b85a04011293c..ef85c9843a12acb0e4e9ee6167c34d4981d4dc8a 100644
> --- a/drivers/remoteproc/qcom_q6v5_mss.c
> +++ b/drivers/remoteproc/qcom_q6v5_mss.c
> @@ -134,6 +134,11 @@
>  #define BOOT_FSM_TIMEOUT                10000
>  #define BHS_CHECK_MAX_LOOPS             200
>  
> +/* External power block headswitch */
> +#define EXTERNAL_BHS_ON			BIT(0)
> +#define EXTERNAL_BHS_STATUS		BIT(4)
> +#define EXTERNAL_BHS_TIMEOUT_US		50
> +
>  struct reg_info {
>  	struct regulator *reg;
>  	int uV;
> @@ -161,6 +166,7 @@ struct rproc_hexagon_res {
> [...]
> @@ -1871,6 +1897,36 @@ static void q6v5_pds_detach(struct q6v5 *qproc, struct device **pds,
>  		dev_pm_domain_detach(pds[i], false);
>  }
>  
> +static int q6v5_external_bhs_enable(struct q6v5 *qproc)
> +{
> +	u32 val;
> +	int ret = 0;
> +
> +	/*
> +	 * Enable external power block headswitch and wait for it to
> +	 * stabilize
> +	 */
> +	regmap_update_bits(qproc->conn_map, qproc->ext_bhs,
> +			   EXTERNAL_BHS_ON, 1);

Nitpick: regmap_update_bits() doesn't take a bool (0/1) as last value,
it's the actual value. The 1 as last argument is essentially a magic
number here that happens to be equal to EXTERNAL_BHS_ON = BIT(0).

This should be

	regmap_update_bits(..., EXTERNAL_BHS_ON, EXTERNAL_BHS_ON);

or even better just

	regmap_set_bits(..., EXTERNAL_BHS_ON);

> +
> +	ret = regmap_read_poll_timeout(qproc->conn_map, qproc->ext_bhs,
> +				       val, val & EXTERNAL_BHS_STATUS,
> +				       1, EXTERNAL_BHS_TIMEOUT_US);
> +
> +	if (ret) {
> +		dev_err(qproc->dev, "External BHS timed out\n");
> +		ret = -ETIMEDOUT;
> +	}
> +
> +	return ret;
> +}
> +
> +static void q6v5_external_bhs_disable(struct q6v5 *qproc)
> +{
> +	regmap_update_bits(qproc->conn_map, qproc->ext_bhs,
> +			   EXTERNAL_BHS_ON, 0);

	regmap_clear_bits(..., EXTERNAL_BHS_ON);

> +}
> +
>  static int q6v5_init_reset(struct q6v5 *qproc)
>  {
>  	qproc->mss_restart = devm_reset_control_get_exclusive(qproc->dev,
> @@ -2021,6 +2077,7 @@ static int q6v5_probe(struct platform_device *pdev)
>  	platform_set_drvdata(pdev, qproc);
>  
>  	qproc->has_qaccept_regs = desc->has_qaccept_regs;
> +	qproc->has_ext_bhs_reg = desc->has_ext_bhs_reg;
>  	qproc->has_ext_cntl_regs = desc->has_ext_cntl_regs;
>  	qproc->has_vq6 = desc->has_vq6;
>  	qproc->has_spare_reg = desc->has_spare_reg;
> @@ -2079,6 +2136,14 @@ static int q6v5_probe(struct platform_device *pdev)
>  		qproc->proxy_pd_count = ret;
>  	}
>  
> +	if (qproc->has_ext_bhs_reg) {
> +		ret = q6v5_external_bhs_enable(qproc);
> +		if (ret < 0) {
> +			dev_err(&pdev->dev, "Failed to enable external BHS.\n");

Since the only possible error condition inside
q6v5_external_bhs_enable() is the timeout condition that is already
logged, you might as well drop this error message.

Thanks,
Stephan

[PATCH v2 5/9] ARM: dts: qcom: msm8226: Add node for TCSR halt regs

[Luca Weiss]

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

Add a syscon device node for the TCSR halt regs needed by
modem and other remoteprocs.

Signed-off-by: Matti Lehtimäki <matti.lehtimaki@gmail.com>
Signed-off-by: Luca Weiss <luca@lucaweiss.eu>
---
 arch/arm/boot/dts/qcom/qcom-msm8226.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/qcom/qcom-msm8226.dtsi b/arch/arm/boot/dts/qcom/qcom-msm8226.dtsi
index 64c8ac94f352e46dc4a18f902d2c30114ecd91d2..caaeeadf289f78d29403312738bbb714eaad74d2 100644
--- a/arch/arm/boot/dts/qcom/qcom-msm8226.dtsi
+++ b/arch/arm/boot/dts/qcom/qcom-msm8226.dtsi
@@ -851,6 +851,11 @@ tcsr_mutex: hwlock@fd484000 {
 			#hwlock-cells = <1>;
 		};
 
+		tcsr_regs_1: syscon@fd485000 {
+			compatible = "qcom,tcsr-msm8226", "syscon";
+			reg = <0xfd485000 0x1000>;
+		};
+
 		tlmm: pinctrl@fd510000 {
 			compatible = "qcom,msm8226-pinctrl";
 			reg = <0xfd510000 0x4000>;

-- 
2.48.1

Re: [PATCH v2 5/9] ARM: dts: qcom: msm8226: Add node for TCSR halt regs

[Konrad Dybcio]

On 26.01.2025 9:57 PM, Luca Weiss wrote:
> From: Matti Lehtimäki <matti.lehtimaki@gmail.com>
> 
> Add a syscon device node for the TCSR halt regs needed by
> modem and other remoteprocs.
> 
> Signed-off-by: Matti Lehtimäki <matti.lehtimaki@gmail.com>
> Signed-off-by: Luca Weiss <luca@lucaweiss.eu>
> ---

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

Konrad

[PATCH v2 6/9] ARM: dts: qcom: msm8226: Add smsm node

[Luca Weiss]

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

Add smsm node for remoteproc cores.

Signed-off-by: Matti Lehtimäki <matti.lehtimaki@gmail.com>
Co-developed-by: Luca Weiss <luca@lucaweiss.eu>
Signed-off-by: Luca Weiss <luca@lucaweiss.eu>
---
 arch/arm/boot/dts/qcom/qcom-msm8226.dtsi | 34 ++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/arch/arm/boot/dts/qcom/qcom-msm8226.dtsi b/arch/arm/boot/dts/qcom/qcom-msm8226.dtsi
index caaeeadf289f78d29403312738bbb714eaad74d2..7e0838af6c2d0dc26e0082471b70578bc627491d 100644
--- a/arch/arm/boot/dts/qcom/qcom-msm8226.dtsi
+++ b/arch/arm/boot/dts/qcom/qcom-msm8226.dtsi
@@ -253,6 +253,40 @@ adsp_smp2p_in: slave-kernel {
 		};
 	};
 
+	smsm {
+		compatible = "qcom,smsm";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		mboxes = <0>, <&apcs 13>, <&apcs 9>, <&apcs 19>;
+
+		apps_smsm: apps@0 {
+			reg = <0>;
+			#qcom,smem-state-cells = <1>;
+		};
+
+		modem_smsm: modem@1 {
+			reg = <1>;
+			interrupts = <GIC_SPI 26 IRQ_TYPE_EDGE_RISING>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+		};
+
+		adsp_smsm: adsp@2 {
+			reg = <2>;
+			interrupts = <GIC_SPI 157 IRQ_TYPE_EDGE_RISING>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+		};
+
+		wcnss_smsm: wcnss@7 {
+			reg = <7>;
+			interrupts = <GIC_SPI 144 IRQ_TYPE_EDGE_RISING>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+		};
+	};
+
 	soc: soc {
 		compatible = "simple-bus";
 		#address-cells = <1>;

-- 
2.48.1

Re: [PATCH v2 6/9] ARM: dts: qcom: msm8226: Add smsm node

[Stephan Gerhold]

On Sun, Jan 26, 2025 at 09:57:25PM +0100, Luca Weiss wrote:
> From: Matti Lehtimäki <matti.lehtimaki@gmail.com>
> 
> Add smsm node for remoteproc cores.
> 
> Signed-off-by: Matti Lehtimäki <matti.lehtimaki@gmail.com>
> Co-developed-by: Luca Weiss <luca@lucaweiss.eu>
> Signed-off-by: Luca Weiss <luca@lucaweiss.eu>

Reviewed-by: Stephan Gerhold <stephan.gerhold@linaro.org>

> ---
>  arch/arm/boot/dts/qcom/qcom-msm8226.dtsi | 34 ++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/qcom/qcom-msm8226.dtsi b/arch/arm/boot/dts/qcom/qcom-msm8226.dtsi
> index caaeeadf289f78d29403312738bbb714eaad74d2..7e0838af6c2d0dc26e0082471b70578bc627491d 100644
> --- a/arch/arm/boot/dts/qcom/qcom-msm8226.dtsi
> +++ b/arch/arm/boot/dts/qcom/qcom-msm8226.dtsi
> @@ -253,6 +253,40 @@ adsp_smp2p_in: slave-kernel {
>  		};
>  	};
>  
> +	smsm {
> +		compatible = "qcom,smsm";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		mboxes = <0>, <&apcs 13>, <&apcs 9>, <&apcs 19>;
> +
> +		apps_smsm: apps@0 {
> +			reg = <0>;
> +			#qcom,smem-state-cells = <1>;
> +		};
> +
> +		modem_smsm: modem@1 {
> +			reg = <1>;
> +			interrupts = <GIC_SPI 26 IRQ_TYPE_EDGE_RISING>;
> +			interrupt-controller;
> +			#interrupt-cells = <2>;
> +		};
> +
> +		adsp_smsm: adsp@2 {
> +			reg = <2>;
> +			interrupts = <GIC_SPI 157 IRQ_TYPE_EDGE_RISING>;
> +			interrupt-controller;
> +			#interrupt-cells = <2>;
> +		};
> +
> +		wcnss_smsm: wcnss@7 {
> +			reg = <7>;
> +			interrupts = <GIC_SPI 144 IRQ_TYPE_EDGE_RISING>;
> +			interrupt-controller;
> +			#interrupt-cells = <2>;
> +		};
> +	};
> +
>  	soc: soc {
>  		compatible = "simple-bus";
>  		#address-cells = <1>;
> 
> -- 
> 2.48.1
> 

[PATCH v2 7/9] ARM: dts: qcom: msm8226: Add modem remoteproc node

[Luca Weiss]

Add a node for the modem remoteproc found on MSM8226.

For the apq8026 smartwatches, delete the modem reserved-memory again
since they have very little RAM in the first place anyways, and all of
these devices don't have functional modem firmware anyways - these
watches don't even have GPS.

Co-developed-by: Matti Lehtimäki <matti.lehtimaki@gmail.com>
Signed-off-by: Matti Lehtimäki <matti.lehtimaki@gmail.com>
Signed-off-by: Luca Weiss <luca@lucaweiss.eu>
---
 .../boot/dts/qcom/qcom-apq8026-asus-sparrow.dts    |  7 ++
 .../boot/dts/qcom/qcom-apq8026-huawei-sturgeon.dts |  7 ++
 arch/arm/boot/dts/qcom/qcom-apq8026-lg-lenok.dts   |  7 ++
 .../dts/qcom/qcom-apq8026-samsung-milletwifi.dts   |  2 +
 .../qcom/qcom-msm8226-samsung-matisse-common.dtsi  |  6 +-
 arch/arm/boot/dts/qcom/qcom-msm8226.dtsi           | 80 ++++++++++++++++++++++
 arch/arm/boot/dts/qcom/qcom-msm8926-htc-memul.dts  |  2 +
 7 files changed, 109 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/qcom/qcom-apq8026-asus-sparrow.dts b/arch/arm/boot/dts/qcom/qcom-apq8026-asus-sparrow.dts
index a2ca456012f1a070afc2805ae71ddbf6f5aac607..816b61ea504ee80d86dd4ccd1a15db687f6a91ff 100644
--- a/arch/arm/boot/dts/qcom/qcom-apq8026-asus-sparrow.dts
+++ b/arch/arm/boot/dts/qcom/qcom-apq8026-asus-sparrow.dts
@@ -9,6 +9,8 @@
 #include "pm8226.dtsi"
 
 /delete-node/ &adsp_region;
+/delete-node/ &mba_region;
+/delete-node/ &mpss_region;
 
 / {
 	model = "ASUS ZenWatch 2";
@@ -82,6 +84,11 @@ bluetooth {
 	};
 };
 
+/* No (useful) modem on this device */
+&modem {
+	/delete-property/ memory-region;
+};
+
 &pm8226_vib {
 	status = "okay";
 };
diff --git a/arch/arm/boot/dts/qcom/qcom-apq8026-huawei-sturgeon.dts b/arch/arm/boot/dts/qcom/qcom-apq8026-huawei-sturgeon.dts
index ac228965a48559cd0e54671b1b80773a34595d1e..06c4dca6c9ea2da4ef0be877d25830c172757fa2 100644
--- a/arch/arm/boot/dts/qcom/qcom-apq8026-huawei-sturgeon.dts
+++ b/arch/arm/boot/dts/qcom/qcom-apq8026-huawei-sturgeon.dts
@@ -10,6 +10,8 @@
 #include <dt-bindings/input/ti-drv260x.h>
 
 /delete-node/ &adsp_region;
+/delete-node/ &mba_region;
+/delete-node/ &mpss_region;
 
 / {
 	model = "Huawei Watch";
@@ -141,6 +143,11 @@ bluetooth {
 	};
 };
 
+/* No (useful) modem on this device */
+&modem {
+	/delete-property/ memory-region;
+};
+
 &rpm_requests {
 	regulators {
 		compatible = "qcom,rpm-pm8226-regulators";
diff --git a/arch/arm/boot/dts/qcom/qcom-apq8026-lg-lenok.dts b/arch/arm/boot/dts/qcom/qcom-apq8026-lg-lenok.dts
index a70de21bf139bd3f67d9e848f75cac20839335fb..1c3e2d2cd54979d4859196c7003d9543971e53bc 100644
--- a/arch/arm/boot/dts/qcom/qcom-apq8026-lg-lenok.dts
+++ b/arch/arm/boot/dts/qcom/qcom-apq8026-lg-lenok.dts
@@ -10,6 +10,8 @@
 #include <dt-bindings/clock/qcom,mmcc-msm8974.h>
 
 /delete-node/ &adsp_region;
+/delete-node/ &mba_region;
+/delete-node/ &mpss_region;
 
 / {
 	model = "LG G Watch R";
@@ -149,6 +151,11 @@ bluetooth {
 	};
 };
 
+/* No (useful) modem on this device */
+&modem {
+	/delete-property/ memory-region;
+};
+
 &rpm_requests {
 	regulators {
 		compatible = "qcom,rpm-pm8226-regulators";
diff --git a/arch/arm/boot/dts/qcom/qcom-apq8026-samsung-milletwifi.dts b/arch/arm/boot/dts/qcom/qcom-apq8026-samsung-milletwifi.dts
index 7d519156d91d156d801c477711751aba66cc3224..a8543ca7b5569549418683b6be320784eba1768c 100644
--- a/arch/arm/boot/dts/qcom/qcom-apq8026-samsung-milletwifi.dts
+++ b/arch/arm/boot/dts/qcom/qcom-apq8026-samsung-milletwifi.dts
@@ -12,6 +12,8 @@
 #include "pm8226.dtsi"
 
 /delete-node/ &adsp_region;
+/delete-node/ &mba_region;
+/delete-node/ &mpss_region;
 /delete-node/ &smem_region;
 
 / {
diff --git a/arch/arm/boot/dts/qcom/qcom-msm8226-samsung-matisse-common.dtsi b/arch/arm/boot/dts/qcom/qcom-msm8226-samsung-matisse-common.dtsi
index a15a44fc0181bf23e7531ecdc8978505ca48189d..fbd568c7d6b7415d240aa1a2329d07cf9135274c 100644
--- a/arch/arm/boot/dts/qcom/qcom-msm8226-samsung-matisse-common.dtsi
+++ b/arch/arm/boot/dts/qcom/qcom-msm8226-samsung-matisse-common.dtsi
@@ -8,6 +8,8 @@
 #include "pm8226.dtsi"
 
 /delete-node/ &adsp_region;
+/delete-node/ &mba_region;
+/delete-node/ &mpss_region;
 /delete-node/ &smem_region;
 
 / {
@@ -145,12 +147,12 @@ framebuffer@3200000 {
 			no-map;
 		};
 
-		mpss@8400000 {
+		mpss_region: mpss@8400000 {
 			reg = <0x08400000 0x1f00000>;
 			no-map;
 		};
 
-		mba@a300000 {
+		mba_region: mba@a300000 {
 			reg = <0x0a300000 0x100000>;
 			no-map;
 		};
diff --git a/arch/arm/boot/dts/qcom/qcom-msm8226.dtsi b/arch/arm/boot/dts/qcom/qcom-msm8226.dtsi
index 7e0838af6c2d0dc26e0082471b70578bc627491d..f7f57c3f583557830c0a6dc9f0eebc2ddebbbe3c 100644
--- a/arch/arm/boot/dts/qcom/qcom-msm8226.dtsi
+++ b/arch/arm/boot/dts/qcom/qcom-msm8226.dtsi
@@ -213,6 +213,16 @@ smem_region: smem@3000000 {
 			no-map;
 		};
 
+		mpss_region: mpss@8000000 {
+			reg = <0x08000000 0x5100000>;
+			no-map;
+		};
+
+		mba_region: mba@d100000 {
+			reg = <0x0d100000 0x100000>;
+			no-map;
+		};
+
 		adsp_region: adsp@dc00000 {
 			reg = <0x0dc00000 0x1900000>;
 			no-map;
@@ -253,6 +263,31 @@ adsp_smp2p_in: slave-kernel {
 		};
 	};
 
+	smp2p-modem {
+		compatible = "qcom,smp2p";
+		qcom,smem = <435>, <428>;
+
+		interrupt-parent = <&intc>;
+		interrupts = <GIC_SPI 27 IRQ_TYPE_EDGE_RISING>;
+
+		mboxes = <&apcs 14>;
+
+		qcom,local-pid = <0>;
+		qcom,remote-pid = <1>;
+
+		modem_smp2p_out: master-kernel {
+			qcom,entry-name = "master-kernel";
+			#qcom,smem-state-cells = <1>;
+		};
+
+		modem_smp2p_in: slave-kernel {
+			qcom,entry-name = "slave-kernel";
+
+			interrupt-controller;
+			#interrupt-cells = <2>;
+		};
+	};
+
 	smsm {
 		compatible = "qcom,smsm";
 		#address-cells = <1>;
@@ -879,6 +914,51 @@ spmi_bus: spmi@fc4cf000 {
 			#interrupt-cells = <4>;
 		};
 
+		modem: remoteproc@fc880000 {
+			compatible = "qcom,msm8226-mss-pil";
+			reg = <0xfc880000 0x100>,
+			      <0xfc820000 0x020>;
+			reg-names = "qdsp6", "rmb";
+
+			interrupts-extended = <&intc GIC_SPI 24 IRQ_TYPE_EDGE_RISING>,
+					      <&modem_smp2p_in 0 IRQ_TYPE_EDGE_RISING>,
+					      <&modem_smp2p_in 1 IRQ_TYPE_EDGE_RISING>,
+					      <&modem_smp2p_in 2 IRQ_TYPE_EDGE_RISING>,
+					      <&modem_smp2p_in 3 IRQ_TYPE_EDGE_RISING>;
+			interrupt-names = "wdog", "fatal", "ready", "handover", "stop-ack";
+
+			clocks = <&gcc GCC_MSS_Q6_BIMC_AXI_CLK>,
+				 <&gcc GCC_MSS_CFG_AHB_CLK>,
+				 <&gcc GCC_BOOT_ROM_AHB_CLK>,
+				 <&xo_board>;
+			clock-names = "iface", "bus", "mem", "xo";
+
+			resets = <&gcc GCC_MSS_RESTART>;
+			reset-names = "mss_restart";
+
+			power-domains = <&rpmpd MSM8226_VDDCX>;
+			power-domain-names = "cx";
+
+			qcom,ext-bhs-reg = <&tcsr_regs_1 0x194>;
+			qcom,halt-regs = <&tcsr_regs_1 0x180 0x200 0x280>;
+
+			qcom,smem-states = <&modem_smp2p_out 0>;
+			qcom,smem-state-names = "stop";
+
+			memory-region = <&mba_region>, <&mpss_region>;
+
+			status = "disabled";
+
+			smd-edge {
+				interrupts = <GIC_SPI 25 IRQ_TYPE_EDGE_RISING>;
+
+				mboxes = <&apcs 12>;
+				qcom,smd-edge = <0>;
+
+				label = "modem";
+			};
+		};
+
 		tcsr_mutex: hwlock@fd484000 {
 			compatible = "qcom,msm8226-tcsr-mutex", "qcom,tcsr-mutex";
 			reg = <0xfd484000 0x1000>;
diff --git a/arch/arm/boot/dts/qcom/qcom-msm8926-htc-memul.dts b/arch/arm/boot/dts/qcom/qcom-msm8926-htc-memul.dts
index 3037344eb24055071cc77c9cfce0f1d0f95de921..18396623a91dd4bf4b66f01c1d983c391722f3b5 100644
--- a/arch/arm/boot/dts/qcom/qcom-msm8926-htc-memul.dts
+++ b/arch/arm/boot/dts/qcom/qcom-msm8926-htc-memul.dts
@@ -9,6 +9,8 @@
 #include "pm8226.dtsi"
 
 /delete-node/ &adsp_region;
+/delete-node/ &mba_region;
+/delete-node/ &mpss_region;
 /delete-node/ &smem_region;
 
 / {

-- 
2.48.1

Re: [PATCH v2 7/9] ARM: dts: qcom: msm8226: Add modem remoteproc node

[Stephan Gerhold]

On Sun, Jan 26, 2025 at 09:57:26PM +0100, Luca Weiss wrote:
> Add a node for the modem remoteproc found on MSM8226.
> 
> For the apq8026 smartwatches, delete the modem reserved-memory again
> since they have very little RAM in the first place anyways, and all of
> these devices don't have functional modem firmware anyways - these
> watches don't even have GPS.
> 
> Co-developed-by: Matti Lehtimäki <matti.lehtimaki@gmail.com>
> Signed-off-by: Matti Lehtimäki <matti.lehtimaki@gmail.com>
> Signed-off-by: Luca Weiss <luca@lucaweiss.eu>
> ---
>  .../boot/dts/qcom/qcom-apq8026-asus-sparrow.dts    |  7 ++
>  .../boot/dts/qcom/qcom-apq8026-huawei-sturgeon.dts |  7 ++
>  arch/arm/boot/dts/qcom/qcom-apq8026-lg-lenok.dts   |  7 ++
>  .../dts/qcom/qcom-apq8026-samsung-milletwifi.dts   |  2 +
>  .../qcom/qcom-msm8226-samsung-matisse-common.dtsi  |  6 +-
>  arch/arm/boot/dts/qcom/qcom-msm8226.dtsi           | 80 ++++++++++++++++++++++
>  arch/arm/boot/dts/qcom/qcom-msm8926-htc-memul.dts  |  2 +
>  7 files changed, 109 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/qcom/qcom-apq8026-asus-sparrow.dts b/arch/arm/boot/dts/qcom/qcom-apq8026-asus-sparrow.dts
> index a2ca456012f1a070afc2805ae71ddbf6f5aac607..816b61ea504ee80d86dd4ccd1a15db687f6a91ff 100644
> --- a/arch/arm/boot/dts/qcom/qcom-apq8026-asus-sparrow.dts
> +++ b/arch/arm/boot/dts/qcom/qcom-apq8026-asus-sparrow.dts
> @@ -9,6 +9,8 @@
>  #include "pm8226.dtsi"
>  
>  /delete-node/ &adsp_region;
> +/delete-node/ &mba_region;
> +/delete-node/ &mpss_region;
>  
>  / {
>  	model = "ASUS ZenWatch 2";
> @@ -82,6 +84,11 @@ bluetooth {
>  	};
>  };
>  
> +/* No (useful) modem on this device */
> +&modem {
> +	/delete-property/ memory-region;
> +};

Rather than putting /delete-node/ and /delete-property/ on all devices
that do not need/use the modem, I think it would be cleaner if you make
the remoteproc related reserved-memory status = "disabled" by default
and enable it only where necessary together with the modem. The &modem
itself is status = "disabled" by default, so it makes sense to use the
same for the reserved memory as well. See e.g. the existing usage of
this pattern for &mpss_mem/&wcnss_mem/&venus_mem/&mba_mem in
msm8916.dtsi. Then you can drop the /delete-property/ here.

It would be even nicer to use the dynamic allocation
(size/alignment/alloc-ranges instead of reg), but I'm skeptical if this
will work properly on such an old platform...

Thanks,
Stephan

Re: [PATCH v2 7/9] ARM: dts: qcom: msm8226: Add modem remoteproc node

[Luca Weiss]

On maandag 27 januari 2025 10:19:21 Midden-Europese standaardtijd Stephan 
Gerhold wrote:
> On Sun, Jan 26, 2025 at 09:57:26PM +0100, Luca Weiss wrote:
> > Add a node for the modem remoteproc found on MSM8226.
> > 
> > For the apq8026 smartwatches, delete the modem reserved-memory again
> > since they have very little RAM in the first place anyways, and all of
> > these devices don't have functional modem firmware anyways - these
> > watches don't even have GPS.
> > 
> > Co-developed-by: Matti Lehtimäki <matti.lehtimaki@gmail.com>
> > Signed-off-by: Matti Lehtimäki <matti.lehtimaki@gmail.com>
> > Signed-off-by: Luca Weiss <luca@lucaweiss.eu>
> > ---
> >  .../boot/dts/qcom/qcom-apq8026-asus-sparrow.dts    |  7 ++
> >  .../boot/dts/qcom/qcom-apq8026-huawei-sturgeon.dts |  7 ++
> >  arch/arm/boot/dts/qcom/qcom-apq8026-lg-lenok.dts   |  7 ++
> >  .../dts/qcom/qcom-apq8026-samsung-milletwifi.dts   |  2 +
> >  .../qcom/qcom-msm8226-samsung-matisse-common.dtsi  |  6 +-
> >  arch/arm/boot/dts/qcom/qcom-msm8226.dtsi           | 80 +++++++++++++++++
+++++
> >  arch/arm/boot/dts/qcom/qcom-msm8926-htc-memul.dts  |  2 +
> >  7 files changed, 109 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/qcom/qcom-apq8026-asus-sparrow.dts b/arch/
arm/boot/dts/qcom/qcom-apq8026-asus-sparrow.dts
> > index 
a2ca456012f1a070afc2805ae71ddbf6f5aac607..816b61ea504ee80d86dd4ccd1a15db687f6a91ff 
100644
> > --- a/arch/arm/boot/dts/qcom/qcom-apq8026-asus-sparrow.dts
> > +++ b/arch/arm/boot/dts/qcom/qcom-apq8026-asus-sparrow.dts
> > @@ -9,6 +9,8 @@
> >  #include "pm8226.dtsi"
> >  
> >  /delete-node/ &adsp_region;
> > +/delete-node/ &mba_region;
> > +/delete-node/ &mpss_region;
> >  
> >  / {
> >  	model = "ASUS ZenWatch 2";
> > @@ -82,6 +84,11 @@ bluetooth {
> >  	};
> >  };
> >  
> > +/* No (useful) modem on this device */
> > +&modem {
> > +	/delete-property/ memory-region;
> > +};
> 
> Rather than putting /delete-node/ and /delete-property/ on all devices
> that do not need/use the modem, I think it would be cleaner if you make
> the remoteproc related reserved-memory status = "disabled" by default
> and enable it only where necessary together with the modem. The &modem
> itself is status = "disabled" by default, so it makes sense to use the
> same for the reserved memory as well. See e.g. the existing usage of
> this pattern for &mpss_mem/&wcnss_mem/&venus_mem/&mba_mem in
> msm8916.dtsi. Then you can drop the /delete-property/ here.

That's a good point, I didn't realize you can disable a reserved-memory node 
like that. And since often even APQ variants have useful modem firmware (e.g. 
GPS) I didn't find something quickly I guess.

> 
> It would be even nicer to use the dynamic allocation
> (size/alignment/alloc-ranges instead of reg), but I'm skeptical if this
> will work properly on such an old platform...

IIRC I've read in msm8974 docs that dynamic allocation should be possible, I 
can give it a try some day, both on 8974 and 8226.

Regards
Luca

> 
> Thanks,
> Stephan
> 

[PATCH v2 8/9] ARM: dts: qcom: msm8226: Add BAM DMUX Ethernet/IP device

[Luca Weiss]

BAM DMUX is used as the network interface to the modem.

Signed-off-by: Luca Weiss <luca@lucaweiss.eu>
---
 arch/arm/boot/dts/qcom/qcom-msm8226.dtsi | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/arch/arm/boot/dts/qcom/qcom-msm8226.dtsi b/arch/arm/boot/dts/qcom/qcom-msm8226.dtsi
index f7f57c3f583557830c0a6dc9f0eebc2ddebbbe3c..5e82d871b81019d2f4f84d1228e8e9278ac44be3 100644
--- a/arch/arm/boot/dts/qcom/qcom-msm8226.dtsi
+++ b/arch/arm/boot/dts/qcom/qcom-msm8226.dtsi
@@ -914,6 +914,18 @@ spmi_bus: spmi@fc4cf000 {
 			#interrupt-cells = <4>;
 		};
 
+		bam_dmux_dma: dma-controller@fc834000 {
+			compatible = "qcom,bam-v1.4.0";
+			reg = <0xfc834000 0x7000>;
+			interrupts = <GIC_SPI 29 IRQ_TYPE_LEVEL_HIGH>;
+			#dma-cells = <1>;
+			qcom,ee = <0>;
+
+			num-channels = <6>;
+			qcom,num-ees = <1>;
+			qcom,powered-remotely;
+		};
+
 		modem: remoteproc@fc880000 {
 			compatible = "qcom,msm8226-mss-pil";
 			reg = <0xfc880000 0x100>,
@@ -949,6 +961,20 @@ modem: remoteproc@fc880000 {
 
 			status = "disabled";
 
+			bam_dmux: bam-dmux {
+				compatible = "qcom,bam-dmux";
+
+				interrupt-parent = <&modem_smsm>;
+				interrupts = <1 IRQ_TYPE_EDGE_BOTH>, <11 IRQ_TYPE_EDGE_BOTH>;
+				interrupt-names = "pc", "pc-ack";
+
+				qcom,smem-states = <&apps_smsm 1>, <&apps_smsm 11>;
+				qcom,smem-state-names = "pc", "pc-ack";
+
+				dmas = <&bam_dmux_dma 4>, <&bam_dmux_dma 5>;
+				dma-names = "tx", "rx";
+			};
+
 			smd-edge {
 				interrupts = <GIC_SPI 25 IRQ_TYPE_EDGE_RISING>;
 

-- 
2.48.1

Re: [PATCH v2 8/9] ARM: dts: qcom: msm8226: Add BAM DMUX Ethernet/IP device

[Stephan Gerhold]

On Sun, Jan 26, 2025 at 09:57:27PM +0100, Luca Weiss wrote:
> BAM DMUX is used as the network interface to the modem.
> 
> Signed-off-by: Luca Weiss <luca@lucaweiss.eu>

Reviewed-by: Stephan Gerhold <stephan.gerhold@linaro.org>

> ---
>  arch/arm/boot/dts/qcom/qcom-msm8226.dtsi | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/qcom/qcom-msm8226.dtsi b/arch/arm/boot/dts/qcom/qcom-msm8226.dtsi
> index f7f57c3f583557830c0a6dc9f0eebc2ddebbbe3c..5e82d871b81019d2f4f84d1228e8e9278ac44be3 100644
> --- a/arch/arm/boot/dts/qcom/qcom-msm8226.dtsi
> +++ b/arch/arm/boot/dts/qcom/qcom-msm8226.dtsi
> @@ -914,6 +914,18 @@ spmi_bus: spmi@fc4cf000 {
>  			#interrupt-cells = <4>;
>  		};
>  
> +		bam_dmux_dma: dma-controller@fc834000 {
> +			compatible = "qcom,bam-v1.4.0";
> +			reg = <0xfc834000 0x7000>;
> +			interrupts = <GIC_SPI 29 IRQ_TYPE_LEVEL_HIGH>;
> +			#dma-cells = <1>;
> +			qcom,ee = <0>;
> +
> +			num-channels = <6>;
> +			qcom,num-ees = <1>;
> +			qcom,powered-remotely;
> +		};
> +
>  		modem: remoteproc@fc880000 {
>  			compatible = "qcom,msm8226-mss-pil";
>  			reg = <0xfc880000 0x100>,
> @@ -949,6 +961,20 @@ modem: remoteproc@fc880000 {
>  
>  			status = "disabled";
>  
> +			bam_dmux: bam-dmux {
> +				compatible = "qcom,bam-dmux";
> +
> +				interrupt-parent = <&modem_smsm>;
> +				interrupts = <1 IRQ_TYPE_EDGE_BOTH>, <11 IRQ_TYPE_EDGE_BOTH>;
> +				interrupt-names = "pc", "pc-ack";
> +
> +				qcom,smem-states = <&apps_smsm 1>, <&apps_smsm 11>;
> +				qcom,smem-state-names = "pc", "pc-ack";
> +
> +				dmas = <&bam_dmux_dma 4>, <&bam_dmux_dma 5>;
> +				dma-names = "tx", "rx";
> +			};
> +
>  			smd-edge {
>  				interrupts = <GIC_SPI 25 IRQ_TYPE_EDGE_RISING>;
>  
> 
> -- 
> 2.48.1
> 

[PATCH v2 9/9] ARM: dts: qcom: msm8226-samsung-matisse-common: Enable modem

[Luca Weiss]

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

Enable modem remoteproc on samsung,matisse-wifi & matisselte.

Signed-off-by: Matti Lehtimäki <matti.lehtimaki@gmail.com>
Signed-off-by: Luca Weiss <luca@lucaweiss.eu>
---
 arch/arm/boot/dts/qcom/qcom-msm8226-samsung-matisse-common.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/boot/dts/qcom/qcom-msm8226-samsung-matisse-common.dtsi b/arch/arm/boot/dts/qcom/qcom-msm8226-samsung-matisse-common.dtsi
index fbd568c7d6b7415d240aa1a2329d07cf9135274c..4155bfb2136022f2a85d69451c34f06ed2a700ac 100644
--- a/arch/arm/boot/dts/qcom/qcom-msm8226-samsung-matisse-common.dtsi
+++ b/arch/arm/boot/dts/qcom/qcom-msm8226-samsung-matisse-common.dtsi
@@ -225,6 +225,13 @@ &blsp1_uart3 {
 	status = "okay";
 };
 
+&modem {
+	mx-supply = <&pm8226_l3>;
+	pll-supply = <&pm8226_l8>;
+
+	status = "okay";
+};
+
 &rpm_requests {
 	regulators {
 		compatible = "qcom,rpm-pm8226-regulators";

-- 
2.48.1

Re: [PATCH v2 9/9] ARM: dts: qcom: msm8226-samsung-matisse-common: Enable modem

[Stephan Gerhold]

On Sun, Jan 26, 2025 at 09:57:28PM +0100, Luca Weiss wrote:
> From: Matti Lehtimäki <matti.lehtimaki@gmail.com>
> 
> Enable modem remoteproc on samsung,matisse-wifi & matisselte.
> 
> Signed-off-by: Matti Lehtimäki <matti.lehtimaki@gmail.com>
> Signed-off-by: Luca Weiss <luca@lucaweiss.eu>
> ---
>  arch/arm/boot/dts/qcom/qcom-msm8226-samsung-matisse-common.dtsi | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/qcom/qcom-msm8226-samsung-matisse-common.dtsi b/arch/arm/boot/dts/qcom/qcom-msm8226-samsung-matisse-common.dtsi
> index fbd568c7d6b7415d240aa1a2329d07cf9135274c..4155bfb2136022f2a85d69451c34f06ed2a700ac 100644
> --- a/arch/arm/boot/dts/qcom/qcom-msm8226-samsung-matisse-common.dtsi
> +++ b/arch/arm/boot/dts/qcom/qcom-msm8226-samsung-matisse-common.dtsi
> @@ -225,6 +225,13 @@ &blsp1_uart3 {
>  	status = "okay";
>  };
>  
> +&modem {
> +	mx-supply = <&pm8226_l3>;
> +	pll-supply = <&pm8226_l8>;

Hmmmmm, so I was looking at msm8926.dtsi downstream and it seems to
override the msm8226.dtsi modem/mss definition [1]:

 1. ext-bhs-reg is dropped
 2. vdd_mss-supply is added

This common include seems to cover both apq8026 (matissewifi) and
msm8926 (matisselte).

Do we need to handle this difference?

First time I see such a difference between variants of a single SoC.
Looking at all the overrides in msm8926.dtsi, it seems like they made
quite some rework for the "LTE variant".

Thanks,
Stephan

[1]: https://github.com/GalaxyTab4/android_kernel_samsung_matissewifi/blob/45f97bab323176fead18c8ddc20dc57d979904a3/arch/arm/boot/dts/msm8226/msm8926.dtsi#L32-L40

Re: [PATCH v2 9/9] ARM: dts: qcom: msm8226-samsung-matisse-common: Enable modem

[Luca Weiss]

On maandag 27 januari 2025 10:44:19 Midden-Europese standaardtijd Stephan 
Gerhold wrote:
> On Sun, Jan 26, 2025 at 09:57:28PM +0100, Luca Weiss wrote:
> > From: Matti Lehtimäki <matti.lehtimaki@gmail.com>
> > 
> > Enable modem remoteproc on samsung,matisse-wifi & matisselte.
> > 
> > Signed-off-by: Matti Lehtimäki <matti.lehtimaki@gmail.com>
> > Signed-off-by: Luca Weiss <luca@lucaweiss.eu>
> > ---
> >  arch/arm/boot/dts/qcom/qcom-msm8226-samsung-matisse-common.dtsi | 7 +++++
++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/qcom/qcom-msm8226-samsung-matisse-
common.dtsi b/arch/arm/boot/dts/qcom/qcom-msm8226-samsung-matisse-common.dtsi
> > index fbd568c7d6b7415d240aa1a2329d07cf9135274c..
4155bfb2136022f2a85d69451c34f06ed2a700ac 100644
> > --- a/arch/arm/boot/dts/qcom/qcom-msm8226-samsung-matisse-common.dtsi
> > +++ b/arch/arm/boot/dts/qcom/qcom-msm8226-samsung-matisse-common.dtsi
> > @@ -225,6 +225,13 @@ &blsp1_uart3 {
> >  	status = "okay";
> >  };
> >  
> > +&modem {
> > +	mx-supply = <&pm8226_l3>;
> > +	pll-supply = <&pm8226_l8>;
> 
> Hmmmmm, so I was looking at msm8926.dtsi downstream and it seems to
> override the msm8226.dtsi modem/mss definition [1]:
> 
>  1. ext-bhs-reg is dropped
>  2. vdd_mss-supply is added

That's the same what I found, and have working with more (currently hacky) 
patches on msm8926-htc-memul. I decided against introducing it with this 
series since these patches have been pending for too long and initially when I 
was doing 8926, I wasn't sure how to make this into a proper patch. Now my 
idea is to introduce a "static const struct rproc_hexagon_res msm8926_mss" 
with the compatible qcom,msm8926-mss-pil which has these two differences.
I think that's a better idea than adding some extra conditionals around to 
make the mss-supply optional and ext-bhs somehow optional in the driver based 
on dt.

> 
> This common include seems to cover both apq8026 (matissewifi) and
> msm8926 (matisselte).

That's a very good point. I didn't think much about the matisse patches since 
they were introduced by Matti but yes... Either I add the msm8926 support in 
the same series already, or we just enable modem for matissewifi for now. I 
think I'll opt for the latter since the series is already 9 patches long and I 
don't want to make it too complicated.

> 
> Do we need to handle this difference?
> 
> First time I see such a difference between variants of a single SoC.
> Looking at all the overrides in msm8926.dtsi, it seems like they made
> quite some rework for the "LTE variant".

Yeah, no clue what they did here. But also I think Qualcomm stopped making a 
thousand different variants of the same SoC (*cough* msm8974), nowadays they 
"only" make different variants which get different letters and numbers 
(sc7280=qcm6490=sm7325 etc).

> 
> Thanks,
> Stephan
> 
> [1]: https://github.com/GalaxyTab4/android_kernel_samsung_matissewifi/blob/
45f97bab323176fead18c8ddc20dc57d979904a3/arch/arm/boot/dts/msm8226/
msm8926.dtsi#L32-L40
> 

Re: [PATCH v2 9/9] ARM: dts: qcom: msm8226-samsung-matisse-common: Enable modem

[Stephan Gerhold]

On Mon, Jan 27, 2025 at 02:34:08PM +0100, Luca Weiss wrote:
> On maandag 27 januari 2025 10:44:19 Midden-Europese standaardtijd Stephan 
> Gerhold wrote:
> > On Sun, Jan 26, 2025 at 09:57:28PM +0100, Luca Weiss wrote:
> > > From: Matti Lehtimäki <matti.lehtimaki@gmail.com>
> > > 
> > > Enable modem remoteproc on samsung,matisse-wifi & matisselte.
> > > 
> > > Signed-off-by: Matti Lehtimäki <matti.lehtimaki@gmail.com>
> > > Signed-off-by: Luca Weiss <luca@lucaweiss.eu>
> > > ---
> > >  arch/arm/boot/dts/qcom/qcom-msm8226-samsung-matisse-common.dtsi | 7 +++++
> ++
> > >  1 file changed, 7 insertions(+)
> > > 
> > > diff --git a/arch/arm/boot/dts/qcom/qcom-msm8226-samsung-matisse-
> common.dtsi b/arch/arm/boot/dts/qcom/qcom-msm8226-samsung-matisse-common.dtsi
> > > index fbd568c7d6b7415d240aa1a2329d07cf9135274c..
> 4155bfb2136022f2a85d69451c34f06ed2a700ac 100644
> > > --- a/arch/arm/boot/dts/qcom/qcom-msm8226-samsung-matisse-common.dtsi
> > > +++ b/arch/arm/boot/dts/qcom/qcom-msm8226-samsung-matisse-common.dtsi
> > > @@ -225,6 +225,13 @@ &blsp1_uart3 {
> > >  	status = "okay";
> > >  };
> > >  
> > > +&modem {
> > > +	mx-supply = <&pm8226_l3>;
> > > +	pll-supply = <&pm8226_l8>;
> > 
> > Hmmmmm, so I was looking at msm8926.dtsi downstream and it seems to
> > override the msm8226.dtsi modem/mss definition [1]:
> > 
> >  1. ext-bhs-reg is dropped
> >  2. vdd_mss-supply is added
> 
> That's the same what I found, and have working with more (currently hacky) 
> patches on msm8926-htc-memul. I decided against introducing it with this 
> series since these patches have been pending for too long and initially when I 
> was doing 8926, I wasn't sure how to make this into a proper patch. Now my 
> idea is to introduce a "static const struct rproc_hexagon_res msm8926_mss" 
> with the compatible qcom,msm8926-mss-pil which has these two differences.
> I think that's a better idea than adding some extra conditionals around to 
> make the mss-supply optional and ext-bhs somehow optional in the driver based 
> on dt.
> 

Yeah, that sounds good. Could add a new qcom-msm8926.dtsi include that
overrides the compatible and /delete-property/ the ext-bhs-reg. Then
just #include that only for matisselte and add the mss-supply there.

> > 
> > This common include seems to cover both apq8026 (matissewifi) and
> > msm8926 (matisselte).
> 
> That's a very good point. I didn't think much about the matisse patches since 
> they were introduced by Matti but yes... Either I add the msm8926 support in 
> the same series already, or we just enable modem for matissewifi for now. I 
> think I'll opt for the latter since the series is already 9 patches long and I 
> don't want to make it too complicated.
> 

Either way is fine for me. I guess I'd personally throw it in one
series, just because it fits well together and it can take quite some
time to get patches applied. :-)

Thanks,
Stephan

Re: [PATCH v2 9/9] ARM: dts: qcom: msm8226-samsung-matisse-common: Enable modem

[Konrad Dybcio]

On 27.01.2025 2:34 PM, Luca Weiss wrote:
> On maandag 27 januari 2025 10:44:19 Midden-Europese standaardtijd Stephan 
> Gerhold wrote:
>> On Sun, Jan 26, 2025 at 09:57:28PM +0100, Luca Weiss wrote:
>>> From: Matti Lehtimäki <matti.lehtimaki@gmail.com>
>>>
>>> Enable modem remoteproc on samsung,matisse-wifi & matisselte.
>>>
>>> Signed-off-by: Matti Lehtimäki <matti.lehtimaki@gmail.com>
>>> Signed-off-by: Luca Weiss <luca@lucaweiss.eu>
>>> ---
>>>  arch/arm/boot/dts/qcom/qcom-msm8226-samsung-matisse-common.dtsi | 7 +++++
> ++
>>>  1 file changed, 7 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/qcom/qcom-msm8226-samsung-matisse-
> common.dtsi b/arch/arm/boot/dts/qcom/qcom-msm8226-samsung-matisse-common.dtsi
>>> index fbd568c7d6b7415d240aa1a2329d07cf9135274c..
> 4155bfb2136022f2a85d69451c34f06ed2a700ac 100644
>>> --- a/arch/arm/boot/dts/qcom/qcom-msm8226-samsung-matisse-common.dtsi
>>> +++ b/arch/arm/boot/dts/qcom/qcom-msm8226-samsung-matisse-common.dtsi
>>> @@ -225,6 +225,13 @@ &blsp1_uart3 {
>>>  	status = "okay";
>>>  };
>>>  
>>> +&modem {
>>> +	mx-supply = <&pm8226_l3>;
>>> +	pll-supply = <&pm8226_l8>;
>>
>> Hmmmmm, so I was looking at msm8926.dtsi downstream and it seems to
>> override the msm8226.dtsi modem/mss definition [1]:
>>
>>  1. ext-bhs-reg is dropped
>>  2. vdd_mss-supply is added
> 
> That's the same what I found, and have working with more (currently hacky) 
> patches on msm8926-htc-memul. I decided against introducing it with this 
> series since these patches have been pending for too long and initially when I 
> was doing 8926, I wasn't sure how to make this into a proper patch. Now my 
> idea is to introduce a "static const struct rproc_hexagon_res msm8926_mss" 
> with the compatible qcom,msm8926-mss-pil which has these two differences.
> I think that's a better idea than adding some extra conditionals around to 
> make the mss-supply optional and ext-bhs somehow optional in the driver based 
> on dt.

Sounds good. It would then live in msm8926.dtsi which already exists

Konrad