[PATCH v3 00/10] Qualcomm AOSS QMP driver and modem dts

[PATCH v3 00/10] Qualcomm AOSS QMP driver and modem dts

[Bjorn Andersson]

Update the reserved memory map for SDM845, add the ADSP and CDSP nodes,
introduce a communication driver for the AOSS and a PD driver for this and
finally add the modem remoteproc driver.

Bjorn Andersson (8):
  arm64: dts: qcom: sdm845: Update PIL region memory map
  arm64: dts: qcom: sdm845: Define rmtfs memory
  arm64: dts: sdm845: Introduce ADSP and CDSP PAS nodes
  dt-bindings: soc: qcom: Add AOSS QMP binding
  soc: qcom: Add AOSS QMP communication driver
  soc: qcom: Add AOSS QMP genpd provider
  remoteproc: q6v5-mss: Active powerdomain for SDM845
  arm64: dts: qcom: Add AOSS QMP node

Rajendra Nayak (1):
  remoteproc: q6v5-mss: Vote for rpmh power domains

Sibi Sankar (1):
  arm64: dts: qcom: sdm845: Add Q6V5 MSS node

 .../bindings/soc/qcom/qcom,aoss-qmp.txt       |  75 +++++
 arch/arm64/boot/dts/qcom/sdm845-mtp.dts       |   8 +
 arch/arm64/boot/dts/qcom/sdm845.dtsi          | 196 ++++++++++-
 drivers/remoteproc/qcom_q6v5_mss.c            | 144 +++++++-
 drivers/soc/qcom/Kconfig                      |  18 +
 drivers/soc/qcom/Makefile                     |   2 +
 drivers/soc/qcom/aoss-qmp-pd.c                | 138 ++++++++
 drivers/soc/qcom/aoss-qmp.c                   | 317 ++++++++++++++++++
 include/dt-bindings/power/qcom-aoss-qmp.h     |  15 +
 include/linux/soc/qcom/aoss-qmp.h             |  14 +
 10 files changed, 920 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,aoss-qmp.txt
 create mode 100644 drivers/soc/qcom/aoss-qmp-pd.c
 create mode 100644 drivers/soc/qcom/aoss-qmp.c
 create mode 100644 include/dt-bindings/power/qcom-aoss-qmp.h
 create mode 100644 include/linux/soc/qcom/aoss-qmp.h

-- 
2.18.0

[PATCH v3 01/10] arm64: dts: qcom: sdm845: Update PIL region memory map

[Bjorn Andersson]

Update existing and add all missing PIL regions to the reserved memory
map, as described in version 10.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

Changes since v2:
- New patch

 arch/arm64/boot/dts/qcom/sdm845.dtsi | 61 ++++++++++++++++++++++++++--
 1 file changed, 58 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 0ec827394e92..cdcac3704c13 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -89,12 +89,47 @@
 		};
 
 		memory@86200000 {
-			reg = <0 0x86200000 0 0x2d00000>;
+			reg = <0 0x86200000 0 0x100000>;
 			no-map;
 		};
 
-		wlan_msa_mem: memory@96700000 {
-			reg = <0 0x96700000 0 0x100000>;
+		memory@86300000 {
+			reg = <0 0x86300000 0 0x4800000>;
+			no-map;
+		};
+
+		memory@8ab00000 {
+			reg = <0 0x8ab00000 0 0x1400000>;
+			no-map;
+		};
+
+		memory@8bf00000 {
+			reg = <0 0x8bf00000 0 0x500000>;
+			no-map;
+		};
+
+		ipa_fw_mem: memory@8c400000 {
+			reg = <0 0x8c400000 0 0x10000>;
+			no-map;
+		};
+
+		ipa_gsi_mem: memory@8c410000 {
+			reg = <0 0x8c410000 0 0x5000>;
+			no-map;
+		};
+
+		memory@8c415000 {
+			reg = <0 0x8c415000 0 0x2000>;
+			no-map;
+		};
+
+		adsp_mem: memory@8c500000 {
+			reg = <0 0x8c500000 0 0x1a00000>;
+			no-map;
+		};
+
+		wlan_msa_mem: memory@8df00000 {
+			reg = <0 0x8df00000 0 0x100000>;
 			no-map;
 		};
 
@@ -103,10 +138,30 @@
 			no-map;
 		};
 
+		venus_mem: memory@95800000 {
+			reg = <0 0x95800000 0 0x500000>;
+			no-map;
+		};
+
+		cdsp_mem: memory@95d00000 {
+			reg = <0 0x95d00000 0 0x800000>;
+			no-map;
+		};
+
 		mba_region: memory@96500000 {
 			reg = <0 0x96500000 0 0x200000>;
 			no-map;
 		};
+
+		slpi_mem: memory@96700000 {
+			reg = <0 0x96700000 0 0x1400000>;
+			no-map;
+		};
+
+		spss_mem: memory@97b00000 {
+			reg = <0 0x97b00000 0 0x100000>;
+			no-map;
+		};
 	};
 
 	cpus {
-- 
2.18.0

Re: [PATCH v3 01/10] arm64: dts: qcom: sdm845: Update PIL region memory map

[Stephen Boyd]

Quoting Bjorn Andersson (2019-01-21 21:51:03)
> @@ -103,10 +138,30 @@
>                         no-map;
>                 };
>  
> +               venus_mem: memory@95800000 {
> +                       reg = <0 0x95800000 0 0x500000>;
> +                       no-map;
> +               };
> +
> +               cdsp_mem: memory@95d00000 {
> +                       reg = <0 0x95d00000 0 0x800000>;
> +                       no-map;
> +               };
> +
>                 mba_region: memory@96500000 {
>                         reg = <0 0x96500000 0 0x200000>;
>                         no-map;
>                 };
> +
> +               slpi_mem: memory@96700000 {
> +                       reg = <0 0x96700000 0 0x1400000>;
> +                       no-map;
> +               };
> +
> +               spss_mem: memory@97b00000 {
> +                       reg = <0 0x97b00000 0 0x100000>;
> +                       no-map;
> +               };
>         };
>  

What's the plan if certain configurations don't use all these carveouts?
Can we mark the reservation nodes as status = "disabled", or the reverse
and mark them as status = "ok" in all boards, and then reclaim the
memory for peripherals we don't care to use?

Re: [PATCH v3 01/10] arm64: dts: qcom: sdm845: Update PIL region memory map

[Bjorn Andersson]

On Tue 22 Jan 10:58 PST 2019, Stephen Boyd wrote:

> Quoting Bjorn Andersson (2019-01-21 21:51:03)
> > @@ -103,10 +138,30 @@
> >                         no-map;
> >                 };
> >  
> > +               venus_mem: memory@95800000 {
> > +                       reg = <0 0x95800000 0 0x500000>;
> > +                       no-map;
> > +               };
> > +
> > +               cdsp_mem: memory@95d00000 {
> > +                       reg = <0 0x95d00000 0 0x800000>;
> > +                       no-map;
> > +               };
> > +
> >                 mba_region: memory@96500000 {
> >                         reg = <0 0x96500000 0 0x200000>;
> >                         no-map;
> >                 };
> > +
> > +               slpi_mem: memory@96700000 {
> > +                       reg = <0 0x96700000 0 0x1400000>;
> > +                       no-map;
> > +               };
> > +
> > +               spss_mem: memory@97b00000 {
> > +                       reg = <0 0x97b00000 0 0x100000>;
> > +                       no-map;
> > +               };
> >         };
> >  
> 
> What's the plan if certain configurations don't use all these carveouts?
> Can we mark the reservation nodes as status = "disabled", or the reverse
> and mark them as status = "ok" in all boards, and then reclaim the
> memory for peripherals we don't care to use?
> 

The code path that picks these up does look for "status", so I suggest
that we leave them all enabled in the platform dtsi and then let the
device's reclaim them as needed.

Regards,
Bjorn

Re: [PATCH v3 01/10] arm64: dts: qcom: sdm845: Update PIL region memory map

[Doug Anderson]

Hi,

On Tue, Jan 22, 2019 at 11:24 AM Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>
> On Tue 22 Jan 10:58 PST 2019, Stephen Boyd wrote:
>
> > Quoting Bjorn Andersson (2019-01-21 21:51:03)
> > > @@ -103,10 +138,30 @@
> > >                         no-map;
> > >                 };
> > >
> > > +               venus_mem: memory@95800000 {
> > > +                       reg = <0 0x95800000 0 0x500000>;
> > > +                       no-map;
> > > +               };
> > > +
> > > +               cdsp_mem: memory@95d00000 {
> > > +                       reg = <0 0x95d00000 0 0x800000>;
> > > +                       no-map;
> > > +               };
> > > +
> > >                 mba_region: memory@96500000 {
> > >                         reg = <0 0x96500000 0 0x200000>;
> > >                         no-map;
> > >                 };
> > > +
> > > +               slpi_mem: memory@96700000 {
> > > +                       reg = <0 0x96700000 0 0x1400000>;
> > > +                       no-map;
> > > +               };
> > > +
> > > +               spss_mem: memory@97b00000 {
> > > +                       reg = <0 0x97b00000 0 0x100000>;
> > > +                       no-map;
> > > +               };
> > >         };
> > >
> >
> > What's the plan if certain configurations don't use all these carveouts?
> > Can we mark the reservation nodes as status = "disabled", or the reverse
> > and mark them as status = "ok" in all boards, and then reclaim the
> > memory for peripherals we don't care to use?
> >
>
> The code path that picks these up does look for "status", so I suggest
> that we leave them all enabled in the platform dtsi and then let the
> device's reclaim them as needed.

Does that mean we should add labels for all of the sub-nodes so that
boards can easily mark them "disabled"?

-Doug

Re: [PATCH v3 01/10] arm64: dts: qcom: sdm845: Update PIL region memory map

[Bjorn Andersson]

On Tue 22 Jan 15:10 PST 2019, Doug Anderson wrote:

> Hi,
> 
> On Tue, Jan 22, 2019 at 11:24 AM Bjorn Andersson
> <bjorn.andersson@linaro.org> wrote:
> >
> > On Tue 22 Jan 10:58 PST 2019, Stephen Boyd wrote:
> >
> > > Quoting Bjorn Andersson (2019-01-21 21:51:03)
> > > > @@ -103,10 +138,30 @@
> > > >                         no-map;
> > > >                 };
> > > >
> > > > +               venus_mem: memory@95800000 {
> > > > +                       reg = <0 0x95800000 0 0x500000>;
> > > > +                       no-map;
> > > > +               };
> > > > +
> > > > +               cdsp_mem: memory@95d00000 {
> > > > +                       reg = <0 0x95d00000 0 0x800000>;
> > > > +                       no-map;
> > > > +               };
> > > > +
> > > >                 mba_region: memory@96500000 {
> > > >                         reg = <0 0x96500000 0 0x200000>;
> > > >                         no-map;
> > > >                 };
> > > > +
> > > > +               slpi_mem: memory@96700000 {
> > > > +                       reg = <0 0x96700000 0 0x1400000>;
> > > > +                       no-map;
> > > > +               };
> > > > +
> > > > +               spss_mem: memory@97b00000 {
> > > > +                       reg = <0 0x97b00000 0 0x100000>;
> > > > +                       no-map;
> > > > +               };
> > > >         };
> > > >
> > >
> > > What's the plan if certain configurations don't use all these carveouts?
> > > Can we mark the reservation nodes as status = "disabled", or the reverse
> > > and mark them as status = "ok" in all boards, and then reclaim the
> > > memory for peripherals we don't care to use?
> > >
> >
> > The code path that picks these up does look for "status", so I suggest
> > that we leave them all enabled in the platform dtsi and then let the
> > device's reclaim them as needed.
> 
> Does that mean we should add labels for all of the sub-nodes so that
> boards can easily mark them "disabled"?
> 

That sounds reasonable, I'll dig up some labels for the unlabeled nodes
as well.

Thanks,
Bjorn

Re: [PATCH v3 01/10] arm64: dts: qcom: sdm845: Update PIL region memory map

[Doug Anderson]

Hi,

On Mon, Jan 21, 2019 at 9:52 PM Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>
> Update existing and add all missing PIL regions to the reserved memory
> map, as described in version 10.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
>
> Changes since v2:
> - New patch
>
>  arch/arm64/boot/dts/qcom/sdm845.dtsi | 61 ++++++++++++++++++++++++++--
>  1 file changed, 58 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index 0ec827394e92..cdcac3704c13 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -89,12 +89,47 @@
>                 };
>
>                 memory@86200000 {
> -                       reg = <0 0x86200000 0 0x2d00000>;
> +                       reg = <0 0x86200000 0 0x100000>;
>                         no-map;
>                 };
>
> -               wlan_msa_mem: memory@96700000 {
> -                       reg = <0 0x96700000 0 0x100000>;
> +               memory@86300000 {
> +                       reg = <0 0x86300000 0 0x4800000>;
> +                       no-map;
> +               };

I know it's not a problem upstream (yet), but downstream this collides
with a memory region in the cheza board.  We have:

rmtfs@88f00000 {
  compatible = "qcom,rmtfs-mem";
  reg = <0x0 0x88f00000 0x0 0x800000>;
  no-map;

  qcom,client-id = <1>;
};

...and the above region overlays it since it goes till 0x8ab00000


> +
> +               memory@8ab00000 {
> +                       reg = <0 0x8ab00000 0 0x1400000>;
> +                       no-map;
> +               };
> +
> +               memory@8bf00000 {
> +                       reg = <0 0x8bf00000 0 0x500000>;
> +                       no-map;
> +               };
> +
> +               ipa_fw_mem: memory@8c400000 {
> +                       reg = <0 0x8c400000 0 0x10000>;
> +                       no-map;
> +               };
> +
> +               ipa_gsi_mem: memory@8c410000 {
> +                       reg = <0 0x8c410000 0 0x5000>;
> +                       no-map;
> +               };
> +
> +               memory@8c415000 {
> +                       reg = <0 0x8c415000 0 0x2000>;
> +                       no-map;
> +               };
> +
> +               adsp_mem: memory@8c500000 {
> +                       reg = <0 0x8c500000 0 0x1a00000>;
> +                       no-map;
> +               };
> +
> +               wlan_msa_mem: memory@8df00000 {

Your patch moves 'wlan_msa_mem' from 0x96700000 to 0x8df00000.  Is
that OK?  I haven't been involved in all of the previous discussions
but if everything is all OK w/ the device tree just moving this chunk
around (without any other coordination w/ firmware) it seems really
weird that we even need to specify it in the device tree.  ...but
maybe I shouldn't open this can of worms.  You can pretend I didn't
say anything.

-Doug

Re: [PATCH v3 01/10] arm64: dts: qcom: sdm845: Update PIL region memory map

[Bjorn Andersson]

On Tue 22 Jan 15:16 PST 2019, Doug Anderson wrote:

> Hi,
> 
> On Mon, Jan 21, 2019 at 9:52 PM Bjorn Andersson
> <bjorn.andersson@linaro.org> wrote:
> >
> > Update existing and add all missing PIL regions to the reserved memory
> > map, as described in version 10.
> >
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > ---
> >
> > Changes since v2:
> > - New patch
> >
> >  arch/arm64/boot/dts/qcom/sdm845.dtsi | 61 ++++++++++++++++++++++++++--
> >  1 file changed, 58 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > index 0ec827394e92..cdcac3704c13 100644
> > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > @@ -89,12 +89,47 @@
> >                 };
> >
> >                 memory@86200000 {
> > -                       reg = <0 0x86200000 0 0x2d00000>;
> > +                       reg = <0 0x86200000 0 0x100000>;
> >                         no-map;
> >                 };
> >
> > -               wlan_msa_mem: memory@96700000 {
> > -                       reg = <0 0x96700000 0 0x100000>;
> > +               memory@86300000 {
> > +                       reg = <0 0x86300000 0 0x4800000>;
> > +                       no-map;
> > +               };
> 
> I know it's not a problem upstream (yet), but downstream this collides
> with a memory region in the cheza board.  We have:
> 
> rmtfs@88f00000 {
>   compatible = "qcom,rmtfs-mem";
>   reg = <0x0 0x88f00000 0x0 0x800000>;
>   no-map;
> 
>   qcom,client-id = <1>;
> };
> 
> ...and the above region overlays it since it goes till 0x8ab00000
> 

Digging through the table again I see that there's another level here,
so it seems only the first 44MB of these 78MB are reserved for non-APSS
things. So this should actually be 0x2c00000 long.

I will update this and we'll have one conflict less.

> 
> > +
> > +               memory@8ab00000 {
> > +                       reg = <0 0x8ab00000 0 0x1400000>;
> > +                       no-map;
> > +               };
> > +
> > +               memory@8bf00000 {
> > +                       reg = <0 0x8bf00000 0 0x500000>;
> > +                       no-map;
> > +               };
> > +
> > +               ipa_fw_mem: memory@8c400000 {
> > +                       reg = <0 0x8c400000 0 0x10000>;
> > +                       no-map;
> > +               };
> > +
> > +               ipa_gsi_mem: memory@8c410000 {
> > +                       reg = <0 0x8c410000 0 0x5000>;
> > +                       no-map;
> > +               };
> > +
> > +               memory@8c415000 {
> > +                       reg = <0 0x8c415000 0 0x2000>;
> > +                       no-map;
> > +               };
> > +
> > +               adsp_mem: memory@8c500000 {
> > +                       reg = <0 0x8c500000 0 0x1a00000>;
> > +                       no-map;
> > +               };
> > +
> > +               wlan_msa_mem: memory@8df00000 {
> 
> Your patch moves 'wlan_msa_mem' from 0x96700000 to 0x8df00000.  Is
> that OK?  I haven't been involved in all of the previous discussions
> but if everything is all OK w/ the device tree just moving this chunk
> around (without any other coordination w/ firmware) it seems really
> weird that we even need to specify it in the device tree.  ...but
> maybe I shouldn't open this can of worms.  You can pretend I didn't
> say anything.
> 

0x96700000 seems to be reserved for the sensor core, so either WiFi
wasn't actually tested before, or more likely its firmware is position
independent.

Most (all?) firmware is position independent, but the security
configuration prevents us from relocating it. One such example is that
the ADSP in the newer firmware versions are not allowed to execute from
the old memory region.

Regards,
Bjorn

Re: [PATCH v3 01/10] arm64: dts: qcom: sdm845: Update PIL region memory map

[Sibi Sankar]

Hey Bjorn,

On 2019-01-22 11:21, Bjorn Andersson wrote:
> Update existing and add all missing PIL regions to the reserved memory
> map, as described in version 10.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
> 
> Changes since v2:
> - New patch
> 
>  arch/arm64/boot/dts/qcom/sdm845.dtsi | 61 ++++++++++++++++++++++++++--
>  1 file changed, 58 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index 0ec827394e92..cdcac3704c13 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -89,12 +89,47 @@
>  		};

Shouldn't we add hyp and xbl regions
as well?

> 
>  		memory@86200000 {
> -			reg = <0 0x86200000 0 0x2d00000>;
> +			reg = <0 0x86200000 0 0x100000>;
>  			no-map;
>  		};
> 
> -		wlan_msa_mem: memory@96700000 {
> -			reg = <0 0x96700000 0 0x100000>;
> +		memory@86300000 {
> +			reg = <0 0x86300000 0 0x4800000>;

from v10 I see we don't need to reserve the
last 28M it just needs to be

reg = <0 0x86300000 0 0x2c00000>;

> +			no-map;
> +		};
> +
> +		memory@8ab00000 {
> +			reg = <0 0x8ab00000 0 0x1400000>;
> +			no-map;
> +		};
> +
> +		memory@8bf00000 {
> +			reg = <0 0x8bf00000 0 0x500000>;
> +			no-map;
> +		};
> +
> +		ipa_fw_mem: memory@8c400000 {
> +			reg = <0 0x8c400000 0 0x10000>;
> +			no-map;
> +		};
> +
> +		ipa_gsi_mem: memory@8c410000 {
> +			reg = <0 0x8c410000 0 0x5000>;
> +			no-map;
> +		};
> +
> +		memory@8c415000 {
> +			reg = <0 0x8c415000 0 0x2000>;
> +			no-map;
> +		};
> +
> +		adsp_mem: memory@8c500000 {
> +			reg = <0 0x8c500000 0 0x1a00000>;
> +			no-map;
> +		};
> +
> +		wlan_msa_mem: memory@8df00000 {
> +			reg = <0 0x8df00000 0 0x100000>;
>  			no-map;
>  		};
> 
> @@ -103,10 +138,30 @@
>  			no-map;
>  		};
> 
> +		venus_mem: memory@95800000 {
> +			reg = <0 0x95800000 0 0x500000>;
> +			no-map;
> +		};
> +
> +		cdsp_mem: memory@95d00000 {
> +			reg = <0 0x95d00000 0 0x800000>;
> +			no-map;
> +		};
> +
>  		mba_region: memory@96500000 {

should we re-name mba_region/mpss_region
to mba_mem and mpss_mem for consistency.

>  			reg = <0 0x96500000 0 0x200000>;
>  			no-map;
>  		};
> +
> +		slpi_mem: memory@96700000 {
> +			reg = <0 0x96700000 0 0x1400000>;
> +			no-map;
> +		};
> +
> +		spss_mem: memory@97b00000 {
> +			reg = <0 0x97b00000 0 0x100000>;
> +			no-map;
> +		};
>  	};
> 
>  	cpus {

-- 
-- Sibi Sankar --
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project.

[PATCH v3 02/10] arm64: dts: qcom: sdm845: Define rmtfs memory

[Bjorn Andersson]

Define the rmtfs memory node, as described in version 10 of the memory
map.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

Changes since v2:
- New patch

 arch/arm64/boot/dts/qcom/sdm845.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index cdcac3704c13..64f57cc5c61a 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -72,6 +72,15 @@
 		#size-cells = <2>;
 		ranges;
 
+		rmtfs@85d00000 {
+			compatible = "qcom,rmtfs-mem";
+			reg = <0 0x85d00000 0 0x200000>;
+			no-map;
+
+			qcom,client-id = <1>;
+			qcom,vmid = <15>;
+		};
+
 		memory@85fc0000 {
 			reg = <0 0x85fc0000 0 0x20000>;
 			no-map;
-- 
2.18.0

Re: [PATCH v3 02/10] arm64: dts: qcom: sdm845: Define rmtfs memory

[Doug Anderson]

Hi,

On Mon, Jan 21, 2019 at 9:51 PM Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>
> Define the rmtfs memory node, as described in version 10 of the memory
> map.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
>
> Changes since v2:
> - New patch
>
>  arch/arm64/boot/dts/qcom/sdm845.dtsi | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index cdcac3704c13..64f57cc5c61a 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -72,6 +72,15 @@
>                 #size-cells = <2>;
>                 ranges;
>
> +               rmtfs@85d00000 {
> +                       compatible = "qcom,rmtfs-mem";
> +                       reg = <0 0x85d00000 0 0x200000>;
> +                       no-map;
> +
> +                       qcom,client-id = <1>;
> +                       qcom,vmid = <15>;
> +               };

Ah, I saw this after I posted my comments to patch #1.  I guess this
is the same as this node we have in our cheza board file downstream
(need to get that posted upstream soon):

rmtfs@88f00000 {
  compatible = "qcom,rmtfs-mem";
  reg = <0x0 0x88f00000 0x0 0x800000>;
  no-map;

  qcom,client-id = <1>;
};

That brings up a few things:

1. You should add a node label here.  This allows us to act on the
node more easily from board files, like setting it to disabled or
changing it.

2. In https://crrev.com/c/1119572, the argument was made that the size
of this carveout is board-specific.  That makes it hard to put it in
sdm845.dts.


-Doug

Re: [PATCH v3 02/10] arm64: dts: qcom: sdm845: Define rmtfs memory

[Brian Norris]

On Tue, Jan 22, 2019 at 3:26 PM Doug Anderson <dianders@chromium.org> wrote:
> 2. In https://crrev.com/c/1119572, the argument was made that the size
> of this carveout is board-specific.  That makes it hard to put it in
> sdm845.dts.

And we've increased the size of the memory carve-out since then,
because the memory requirements depend on the SW features you're using
on the modem side.

So that's definitely a reason for giving it a label we can modify it
with, at a minimum (as Doug suggested). I'm somewhat ambivalent as to
whether this should or shouldn't go in the base sdm845.dtsi -- it's
still a feature where most of the definitions (e.g., compatible;
client ID) should be applicable to all boards.

Brian

Re: [PATCH v3 02/10] arm64: dts: qcom: sdm845: Define rmtfs memory

[Bjorn Andersson]

On Tue 22 Jan 15:26 PST 2019, Doug Anderson wrote:

> Hi,
> 
> On Mon, Jan 21, 2019 at 9:51 PM Bjorn Andersson
> <bjorn.andersson@linaro.org> wrote:
> >
> > Define the rmtfs memory node, as described in version 10 of the memory
> > map.
> >
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > ---
> >
> > Changes since v2:
> > - New patch
> >
> >  arch/arm64/boot/dts/qcom/sdm845.dtsi | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > index cdcac3704c13..64f57cc5c61a 100644
> > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > @@ -72,6 +72,15 @@
> >                 #size-cells = <2>;
> >                 ranges;
> >
> > +               rmtfs@85d00000 {
> > +                       compatible = "qcom,rmtfs-mem";
> > +                       reg = <0 0x85d00000 0 0x200000>;
> > +                       no-map;
> > +
> > +                       qcom,client-id = <1>;
> > +                       qcom,vmid = <15>;
> > +               };
> 
> Ah, I saw this after I posted my comments to patch #1.  I guess this
> is the same as this node we have in our cheza board file downstream
> (need to get that posted upstream soon):
> 
> rmtfs@88f00000 {
>   compatible = "qcom,rmtfs-mem";
>   reg = <0x0 0x88f00000 0x0 0x800000>;
>   no-map;
> 
>   qcom,client-id = <1>;
> };
> 
> That brings up a few things:
> 
> 1. You should add a node label here.  This allows us to act on the
> node more easily from board files, like setting it to disabled or
> changing it.
> 

I'll make sure to label it.

> 2. In https://crrev.com/c/1119572, the argument was made that the size
> of this carveout is board-specific.  That makes it hard to put it in
> sdm845.dts.
> 

I don't think I've seen a modern platform where this isn't 2MB, so I
think it's safe to add it to the platform. But a label sounds good, if
someone out there has a custom modem firmware with some odd changes in
this area.

I'll label it, to make it possible to move, resize or reclaim in boards.

Regards,
Bjorn

[PATCH v3 03/10] arm64: dts: sdm845: Introduce ADSP and CDSP PAS nodes

[Bjorn Andersson]

Add the ADSP and CDSP nodes for PAS-based remoteproc, supporting booting
these cores on e.g. the MTP, and enable the same for the MTP.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

Changes since v2:
- New patch

 arch/arm64/boot/dts/qcom/sdm845-mtp.dts |  8 ++++
 arch/arm64/boot/dts/qcom/sdm845.dtsi    | 58 +++++++++++++++++++++++++
 2 files changed, 66 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
index af8c6a2445a2..02b8357c8ce8 100644
--- a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
+++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
@@ -48,6 +48,10 @@
 	};
 };
 
+&adsp_pas {
+	status = "okay";
+};
+
 &apps_rsc {
 	pm8998-rpmh-regulators {
 		compatible = "qcom,pm8998-rpmh-regulators";
@@ -344,6 +348,10 @@
 	};
 };
 
+&cdsp_pas {
+	status = "okay";
+};
+
 &gcc {
 	protected-clocks = <GCC_QSPI_CORE_CLK>,
 			   <GCC_QSPI_CORE_CLK_SRC>,
diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 64f57cc5c61a..1033b77856e6 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -319,6 +319,64 @@
 		};
 	};
 
+	adsp_pas: remoteproc-adsp {
+		compatible = "qcom,sdm845-adsp-pas";
+
+		interrupts-extended = <&intc GIC_SPI 162 IRQ_TYPE_EDGE_RISING>,
+				      <&adsp_smp2p_in 0 IRQ_TYPE_EDGE_RISING>,
+				      <&adsp_smp2p_in 1 IRQ_TYPE_EDGE_RISING>,
+				      <&adsp_smp2p_in 2 IRQ_TYPE_EDGE_RISING>,
+				      <&adsp_smp2p_in 3 IRQ_TYPE_EDGE_RISING>;
+		interrupt-names = "wdog", "fatal", "ready",
+				  "handover", "stop-ack";
+
+		clocks = <&xo_board>;
+		clock-names = "xo";
+
+		memory-region = <&adsp_mem>;
+
+		qcom,smem-states = <&adsp_smp2p_out 0>;
+		qcom,smem-state-names = "stop";
+
+		status = "disabled";
+
+		glink-edge {
+			interrupts = <GIC_SPI 156 IRQ_TYPE_EDGE_RISING>;
+			label = "lpass";
+			qcom,remote-pid = <2>;
+			mboxes = <&apss_shared 8>;
+		};
+	};
+
+	cdsp_pas: remoteproc-cdsp {
+		compatible = "qcom,sdm845-cdsp-pas";
+
+		interrupts-extended = <&intc GIC_SPI 578 IRQ_TYPE_EDGE_RISING>,
+				      <&cdsp_smp2p_in 0 IRQ_TYPE_EDGE_RISING>,
+				      <&cdsp_smp2p_in 1 IRQ_TYPE_EDGE_RISING>,
+				      <&cdsp_smp2p_in 2 IRQ_TYPE_EDGE_RISING>,
+				      <&cdsp_smp2p_in 3 IRQ_TYPE_EDGE_RISING>;
+		interrupt-names = "wdog", "fatal", "ready",
+				  "handover", "stop-ack";
+
+		clocks = <&xo_board>;
+		clock-names = "xo";
+
+		memory-region = <&cdsp_mem>;
+
+		qcom,smem-states = <&cdsp_smp2p_out 0>;
+		qcom,smem-state-names = "stop";
+
+		status = "disabled";
+
+		glink-edge {
+			interrupts = <GIC_SPI 574 IRQ_TYPE_EDGE_RISING>;
+			label = "turing";
+			qcom,remote-pid = <5>;
+			mboxes = <&apss_shared 4>;
+		};
+	};
+
 	tcsr_mutex: hwlock {
 		compatible = "qcom,tcsr-mutex";
 		syscon = <&tcsr_mutex_regs 0 0x1000>;
-- 
2.18.0

Re: [PATCH v3 03/10] arm64: dts: sdm845: Introduce ADSP and CDSP PAS nodes

[Doug Anderson]

Hi,

On Mon, Jan 21, 2019 at 9:52 PM Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>
> Add the ADSP and CDSP nodes for PAS-based remoteproc, supporting booting
> these cores on e.g. the MTP, and enable the same for the MTP.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
>
> Changes since v2:
> - New patch
>
>  arch/arm64/boot/dts/qcom/sdm845-mtp.dts |  8 ++++
>  arch/arm64/boot/dts/qcom/sdm845.dtsi    | 58 +++++++++++++++++++++++++
>  2 files changed, 66 insertions(+)

It's a bit of a nit of mine that if it's not totally obvious what
acronyms mean that they should be spelled out in places that use them.

In this case I believe ADSP is the Audio DSP.  Is CDSP the Camera DSP?  ...or ?


> +       adsp_pas: remoteproc-adsp {
> +               compatible = "qcom,sdm845-adsp-pas";
> +
> +               interrupts-extended = <&intc GIC_SPI 162 IRQ_TYPE_EDGE_RISING>,
> +                                     <&adsp_smp2p_in 0 IRQ_TYPE_EDGE_RISING>,
> +                                     <&adsp_smp2p_in 1 IRQ_TYPE_EDGE_RISING>,
> +                                     <&adsp_smp2p_in 2 IRQ_TYPE_EDGE_RISING>,
> +                                     <&adsp_smp2p_in 3 IRQ_TYPE_EDGE_RISING>;
> +               interrupt-names = "wdog", "fatal", "ready",
> +                                 "handover", "stop-ack";
> +
> +               clocks = <&xo_board>;
> +               clock-names = "xo";

I've found that nearly all the places that refer to xo_board are wrong
and should actually point to '<&rpmhcc RPMH_CXO_CLK>'.  Maybe yours
should too?

-Doug

Re: [PATCH v3 03/10] arm64: dts: sdm845: Introduce ADSP and CDSP PAS nodes

[Bjorn Andersson]

On Tue 22 Jan 15:46 PST 2019, Doug Anderson wrote:

> Hi,
> 
> On Mon, Jan 21, 2019 at 9:52 PM Bjorn Andersson
> <bjorn.andersson@linaro.org> wrote:
> >
> > Add the ADSP and CDSP nodes for PAS-based remoteproc, supporting booting
> > these cores on e.g. the MTP, and enable the same for the MTP.
> >
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > ---
> >
> > Changes since v2:
> > - New patch
> >
> >  arch/arm64/boot/dts/qcom/sdm845-mtp.dts |  8 ++++
> >  arch/arm64/boot/dts/qcom/sdm845.dtsi    | 58 +++++++++++++++++++++++++
> >  2 files changed, 66 insertions(+)
> 
> It's a bit of a nit of mine that if it's not totally obvious what
> acronyms mean that they should be spelled out in places that use them.
> 
> In this case I believe ADSP is the Audio DSP.  Is CDSP the Camera DSP?  ...or ?
> 

C as in Compute. I'll spell these out as I respin the series.

> 
> > +       adsp_pas: remoteproc-adsp {
> > +               compatible = "qcom,sdm845-adsp-pas";
> > +
> > +               interrupts-extended = <&intc GIC_SPI 162 IRQ_TYPE_EDGE_RISING>,
> > +                                     <&adsp_smp2p_in 0 IRQ_TYPE_EDGE_RISING>,
> > +                                     <&adsp_smp2p_in 1 IRQ_TYPE_EDGE_RISING>,
> > +                                     <&adsp_smp2p_in 2 IRQ_TYPE_EDGE_RISING>,
> > +                                     <&adsp_smp2p_in 3 IRQ_TYPE_EDGE_RISING>;
> > +               interrupt-names = "wdog", "fatal", "ready",
> > +                                 "handover", "stop-ack";
> > +
> > +               clocks = <&xo_board>;
> > +               clock-names = "xo";
> 
> I've found that nearly all the places that refer to xo_board are wrong
> and should actually point to '<&rpmhcc RPMH_CXO_CLK>'.  Maybe yours
> should too?
> 

Yes, xo_board is a fake clock representing the 19.2MHz clock feeding the
cxo (or cxo2) pad of the SoC. So you're definitely right in that this
should be referencing the actual 19.2MHz clock.

We've kept referring to this as xo_board, as we don't handle probe
deferral when gcc will probe earlier than rpmcc in the boot and for
other non-clock drivers the fear of actually hitting 0 on the refcounter
for this (you don't want to disable the cxo while running the system).

I'll give it a spin with appropriate reference and see what happens, I
think this should either be changed or documented in the commit message.

Thanks,
Bjorn

Re: [PATCH v3 03/10] arm64: dts: sdm845: Introduce ADSP and CDSP PAS nodes

[Doug Anderson]

Hi,

On Tue, Jan 22, 2019 at 4:26 PM Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
> > > +               clocks = <&xo_board>;
> > > +               clock-names = "xo";
> >
> > I've found that nearly all the places that refer to xo_board are wrong
> > and should actually point to '<&rpmhcc RPMH_CXO_CLK>'.  Maybe yours
> > should too?
> >
>
> Yes, xo_board is a fake clock representing the 19.2MHz clock feeding the
> cxo (or cxo2) pad of the SoC. So you're definitely right in that this
> should be referencing the actual 19.2MHz clock.
>
> We've kept referring to this as xo_board, as we don't handle probe
> deferral when gcc will probe earlier than rpmcc in the boot and for
> other non-clock drivers the fear of actually hitting 0 on the refcounter
> for this (you don't want to disable the cxo while running the system).

Note that, as defined in the device tree, "xo_board" is actually 38.4.
IIUC that is not actually a fake/bogus clock but represents the actual
crystal on the board.  There's a divide by 2 in the CPU though so most
peripherals consider "xo" as 19.2.

...OK, confirmed.  The actual RF_XO_CLK pin on the board is truly
connected to 38.4.

-Doug

Re: [PATCH v3 03/10] arm64: dts: sdm845: Introduce ADSP and CDSP PAS nodes

[Bjorn Andersson]

On Tue 22 Jan 16:40 PST 2019, Doug Anderson wrote:

> Hi,
> 
> On Tue, Jan 22, 2019 at 4:26 PM Bjorn Andersson
> <bjorn.andersson@linaro.org> wrote:
> > > > +               clocks = <&xo_board>;
> > > > +               clock-names = "xo";
> > >
> > > I've found that nearly all the places that refer to xo_board are wrong
> > > and should actually point to '<&rpmhcc RPMH_CXO_CLK>'.  Maybe yours
> > > should too?
> > >
> >
> > Yes, xo_board is a fake clock representing the 19.2MHz clock feeding the
> > cxo (or cxo2) pad of the SoC. So you're definitely right in that this
> > should be referencing the actual 19.2MHz clock.
> >
> > We've kept referring to this as xo_board, as we don't handle probe
> > deferral when gcc will probe earlier than rpmcc in the boot and for
> > other non-clock drivers the fear of actually hitting 0 on the refcounter
> > for this (you don't want to disable the cxo while running the system).
> 
> Note that, as defined in the device tree, "xo_board" is actually 38.4.
> IIUC that is not actually a fake/bogus clock but represents the actual
> crystal on the board.  There's a divide by 2 in the CPU though so most
> peripherals consider "xo" as 19.2.
> 

There's the 38.4MHz XO connected to the PMIC, but the signal going into
the CXO_IN pad of the SoC is supposed to come from LNBBCLK1 and be
19.2MHz.

> ...OK, confirmed.  The actual RF_XO_CLK pin on the board is truly
> connected to 38.4.
> 

And the three RF clocks from the PMIC are all ticking at 38.4MHz.


The "xo" I need here is the LNBBCLK1 (RPMH_CXO_CLK in clk-rpmh), for the
purpose of preventing the root clock to be turned off if apps goes to
suspend while the modem is booting, before it has had a chance to tell
RPM(h) that it needs it to be on.

Regards,
Bjorn

Re: [PATCH v3 03/10] arm64: dts: sdm845: Introduce ADSP and CDSP PAS nodes

[Doug Anderson]

Hi,

On Tue, Jan 22, 2019 at 5:09 PM Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>
> On Tue 22 Jan 16:40 PST 2019, Doug Anderson wrote:
>
> > Hi,
> >
> > On Tue, Jan 22, 2019 at 4:26 PM Bjorn Andersson
> > <bjorn.andersson@linaro.org> wrote:
> > > > > +               clocks = <&xo_board>;
> > > > > +               clock-names = "xo";
> > > >
> > > > I've found that nearly all the places that refer to xo_board are wrong
> > > > and should actually point to '<&rpmhcc RPMH_CXO_CLK>'.  Maybe yours
> > > > should too?
> > > >
> > >
> > > Yes, xo_board is a fake clock representing the 19.2MHz clock feeding the
> > > cxo (or cxo2) pad of the SoC. So you're definitely right in that this
> > > should be referencing the actual 19.2MHz clock.
> > >
> > > We've kept referring to this as xo_board, as we don't handle probe
> > > deferral when gcc will probe earlier than rpmcc in the boot and for
> > > other non-clock drivers the fear of actually hitting 0 on the refcounter
> > > for this (you don't want to disable the cxo while running the system).
> >
> > Note that, as defined in the device tree, "xo_board" is actually 38.4.
> > IIUC that is not actually a fake/bogus clock but represents the actual
> > crystal on the board.  There's a divide by 2 in the CPU though so most
> > peripherals consider "xo" as 19.2.
> >
>
> There's the 38.4MHz XO connected to the PMIC, but the signal going into
> the CXO_IN pad of the SoC is supposed to come from LNBBCLK1 and be
> 19.2MHz.

Ah, thanks for pointing me to the right clock!  :-)

OK, so something is definitely wonky then.  "xo_board" is definitely
38.4 currently in the device tree.  That's my fault due to commit
5ea3939cf51f ("arm64: dts: sdm845: Fix xo_board clock name and
speed").  ...but, in my defense:

A) The hardcoded "divide by 2" for "bi_tcxo" in "clk-rpmh.c" came from Qualcomm.

B) The parent of "bi_tcxo" has always been "xo_board"

C) Children of "bi_tcxo" have always been only correct if "bi_tcxo" was 19.2


...so if "cxo" is really 19.2 then we need to update clk-rpmh to get
rid of the hardcoded divide by 2 I think?


-Doug

Re: [PATCH v3 03/10] arm64: dts: sdm845: Introduce ADSP and CDSP PAS nodes

[Stephen Boyd]

Quoting Doug Anderson (2019-01-23 15:24:36)
> Hi,
> 
> On Tue, Jan 22, 2019 at 5:09 PM Bjorn Andersson
> <bjorn.andersson@linaro.org> wrote:
> >
> > On Tue 22 Jan 16:40 PST 2019, Doug Anderson wrote:
> >
> > > Hi,
> > >
> > > On Tue, Jan 22, 2019 at 4:26 PM Bjorn Andersson
> > > <bjorn.andersson@linaro.org> wrote:
> > > > > > +               clocks = <&xo_board>;
> > > > > > +               clock-names = "xo";
> > > > >
> > > > > I've found that nearly all the places that refer to xo_board are wrong
> > > > > and should actually point to '<&rpmhcc RPMH_CXO_CLK>'.  Maybe yours
> > > > > should too?
> > > > >
> > > >
> > > > Yes, xo_board is a fake clock representing the 19.2MHz clock feeding the
> > > > cxo (or cxo2) pad of the SoC. So you're definitely right in that this
> > > > should be referencing the actual 19.2MHz clock.
> > > >
> > > > We've kept referring to this as xo_board, as we don't handle probe
> > > > deferral when gcc will probe earlier than rpmcc in the boot and for
> > > > other non-clock drivers the fear of actually hitting 0 on the refcounter
> > > > for this (you don't want to disable the cxo while running the system).
> > >
> > > Note that, as defined in the device tree, "xo_board" is actually 38.4.
> > > IIUC that is not actually a fake/bogus clock but represents the actual
> > > crystal on the board.  There's a divide by 2 in the CPU though so most
> > > peripherals consider "xo" as 19.2.
> > >
> >
> > There's the 38.4MHz XO connected to the PMIC, but the signal going into
> > the CXO_IN pad of the SoC is supposed to come from LNBBCLK1 and be
> > 19.2MHz.
> 
> Ah, thanks for pointing me to the right clock!  :-)
> 
> OK, so something is definitely wonky then.  "xo_board" is definitely
> 38.4 currently in the device tree.  That's my fault due to commit
> 5ea3939cf51f ("arm64: dts: sdm845: Fix xo_board clock name and
> speed").  ...but, in my defense:
> 
> A) The hardcoded "divide by 2" for "bi_tcxo" in "clk-rpmh.c" came from Qualcomm.
> 
> B) The parent of "bi_tcxo" has always been "xo_board"
> 
> C) Children of "bi_tcxo" have always been only correct if "bi_tcxo" was 19.2
> 
> 
> ...so if "cxo" is really 19.2 then we need to update clk-rpmh to get
> rid of the hardcoded divide by 2 I think?
> 

It looks OK to me the way it is. Here's why. xo_board is the crystal on
the board. That is 38.4 MHz. The lnbbclk1 signal is 19.2MHz (really? I
thought it was still 38.4 and there was a divider on the CXO pin inside
the SoC). The RPMh driver design to do a div-by-2 on the parent clk is
there to model how the PMIC or the SoC is dividing down that crystal
frequency to be 19.2 MHz for the CXO that the rest of the clk tree in
the SoC sees. So if devices still want to use something that isn't the
rpmh 'bi_tcxo' clk, they should be referencing something like lnbbclk1
as their input clk, and that lnbbclk1 should be a new fixed factor clk
specified in DT that takes the xo_board and creates the lnbbclk1
frequency out of it. Or if we want to get really fancy we can populate
that from the PMIC clk div module itself and then show how the PMIC is
doing the division from the buffer. I don't know if anyone really cares
about this level of detail though.

The reason that certain devices may not want to specify the RPMh clk
'bi_tcxo' is because they may not want the RPM to keep the XO buffer
enabled across low power modes. I don't think this is very common, but
it may be that there isn't any need to do anything besides calculate
some frequency based on the CXO frequency at the SoC's pin and thus
specifying the "raw" XO clk works just as well. I have no idea if that's
the case here.

[PATCH v3 04/10] dt-bindings: soc: qcom: Add AOSS QMP binding

[Bjorn Andersson]

Add binding for the QMP based side-channel communication mechanism to
the AOSS, which is used to control resources not exposed through the
RPMh interface.

Reviewed-by: Rob Herring <robh@kernel.org>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

Changes since v2:
- Update indentation of example
- Add colling device subnodes to the description

 .../bindings/soc/qcom/qcom,aoss-qmp.txt       | 75 +++++++++++++++++++
 include/dt-bindings/power/qcom-aoss-qmp.h     | 15 ++++
 2 files changed, 90 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,aoss-qmp.txt
 create mode 100644 include/dt-bindings/power/qcom-aoss-qmp.h

diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,aoss-qmp.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,aoss-qmp.txt
new file mode 100644
index 000000000000..881dc8c7907a
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,aoss-qmp.txt
@@ -0,0 +1,75 @@
+Qualcomm Always-On Subsystem side channel binding
+
+This binding describes the hardware component responsible for side channel
+requests to the always-on subsystem (AOSS), used for certain power management
+requests that is not handled by the standard RPMh interface. Each client in the
+SoC has it's own block of message RAM and IRQ for communication with the AOSS.
+The protocol used to communicate in the message RAM is known as QMP.
+
+The AOSS side channel exposes control over a set of resources, used to control
+a set of debug related clocks and to affect the low power state of resources
+related to the secondary subsystems. These resources are exposed as a set of
+power-domains.
+
+- compatible:
+	Usage: required
+	Value type: <string>
+	Definition: must be "qcom,sdm845-aoss-qmp"
+
+- reg:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: the base address and size of the message RAM for this
+		    client's communication with the AOSS
+
+- interrupts:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: should specify the AOSS message IRQ for this client
+
+- mboxes:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: reference to the mailbox representing the outgoing doorbell
+		    in APCS for this client, as described in mailbox/mailbox.txt
+
+- #power-domain-cells:
+	Usage: optional
+	Value type: <u32>
+	Definition: must be 1
+		    The provided power-domains are:
+		    QDSS clock-domain (0), CDSP state (1), LPASS state (2),
+		    modem state (3), SLPI state (4), SPSS state (5) and Venus
+		    state (6).
+
+= SUBNODES
+The AOSS side channel also provides the controls for three cooling devices,
+these are expressed as subnodes of the QMP node. The name of the node is used
+to identify the resource and must therefor be "cx", "mx" or "ebi".
+
+- #cooling-cells:
+	Usage: optional
+	Value type: <u32>
+	Definition: must be 2
+
+= EXAMPLE
+
+The following example represents the AOSS side-channel message RAM and the
+mechanism exposing the power-domains, as found in SDM845.
+
+  aoss_qmp: qmp@c300000 {
+	  compatible = "qcom,sdm845-aoss-qmp";
+	  reg = <0x0c300000 0x100000>;
+	  interrupts = <GIC_SPI 389 IRQ_TYPE_EDGE_RISING>;
+	  mboxes = <&apss_shared 0>;
+
+	  #power-domain-cells = <1>;
+
+	  cx_cdev: cx {
+		#cooling-cells = <2>;
+	  };
+
+	  mx_cdev: mx {
+		#cooling-cells = <2>;
+	  };
+  };
diff --git a/include/dt-bindings/power/qcom-aoss-qmp.h b/include/dt-bindings/power/qcom-aoss-qmp.h
new file mode 100644
index 000000000000..7d8ac1a4f90c
--- /dev/null
+++ b/include/dt-bindings/power/qcom-aoss-qmp.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (c) 2018, Linaro Ltd. */
+
+#ifndef __DT_BINDINGS_POWER_QCOM_AOSS_QMP_H
+#define __DT_BINDINGS_POWER_QCOM_AOSS_QMP_H
+
+#define AOSS_QMP_QDSS_CLK	0
+#define AOSS_QMP_LS_CDSP		1
+#define AOSS_QMP_LS_LPASS	2
+#define AOSS_QMP_LS_MODEM	3
+#define AOSS_QMP_LS_SLPI		4
+#define AOSS_QMP_LS_SPSS		5
+#define AOSS_QMP_LS_VENUS	6
+
+#endif
-- 
2.18.0

Re: [PATCH v3 04/10] dt-bindings: soc: qcom: Add AOSS QMP binding

[Stephen Boyd]

Quoting Bjorn Andersson (2019-01-21 21:51:06)
> Add binding for the QMP based side-channel communication mechanism to
> the AOSS, which is used to control resources not exposed through the
> RPMh interface.
> 
> Reviewed-by: Rob Herring <robh@kernel.org>
> Reviewed-by: Stephen Boyd <swboyd@chromium.org>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
> 
> Changes since v2:
> - Update indentation of example
> - Add colling device subnodes to the description
> 
>  .../bindings/soc/qcom/qcom,aoss-qmp.txt       | 75 +++++++++++++++++++
>  include/dt-bindings/power/qcom-aoss-qmp.h     | 15 ++++
>  2 files changed, 90 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,aoss-qmp.txt
>  create mode 100644 include/dt-bindings/power/qcom-aoss-qmp.h
> 
> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,aoss-qmp.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,aoss-qmp.txt
> new file mode 100644
> index 000000000000..881dc8c7907a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,aoss-qmp.txt
> @@ -0,0 +1,75 @@
> +Qualcomm Always-On Subsystem side channel binding
> +
> +This binding describes the hardware component responsible for side channel
> +requests to the always-on subsystem (AOSS), used for certain power management
> +requests that is not handled by the standard RPMh interface. Each client in the
> +SoC has it's own block of message RAM and IRQ for communication with the AOSS.
> +The protocol used to communicate in the message RAM is known as QMP.

Just curious if QMP is "Qualcomm Messaging Protocol"? I don't think it's
ever spelled out in this patchset.

Re: [PATCH v3 04/10] dt-bindings: soc: qcom: Add AOSS QMP binding

[Bjorn Andersson]

On Tue 22 Jan 11:04 PST 2019, Stephen Boyd wrote:

> Quoting Bjorn Andersson (2019-01-21 21:51:06)
> > Add binding for the QMP based side-channel communication mechanism to
> > the AOSS, which is used to control resources not exposed through the
> > RPMh interface.
> > 
> > Reviewed-by: Rob Herring <robh@kernel.org>
> > Reviewed-by: Stephen Boyd <swboyd@chromium.org>
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > ---
> > 
> > Changes since v2:
> > - Update indentation of example
> > - Add colling device subnodes to the description
> > 
> >  .../bindings/soc/qcom/qcom,aoss-qmp.txt       | 75 +++++++++++++++++++
> >  include/dt-bindings/power/qcom-aoss-qmp.h     | 15 ++++
> >  2 files changed, 90 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,aoss-qmp.txt
> >  create mode 100644 include/dt-bindings/power/qcom-aoss-qmp.h
> > 
> > diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,aoss-qmp.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,aoss-qmp.txt
> > new file mode 100644
> > index 000000000000..881dc8c7907a
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,aoss-qmp.txt
> > @@ -0,0 +1,75 @@
> > +Qualcomm Always-On Subsystem side channel binding
> > +
> > +This binding describes the hardware component responsible for side channel
> > +requests to the always-on subsystem (AOSS), used for certain power management
> > +requests that is not handled by the standard RPMh interface. Each client in the
> > +SoC has it's own block of message RAM and IRQ for communication with the AOSS.
> > +The protocol used to communicate in the message RAM is known as QMP.
> 
> Just curious if QMP is "Qualcomm Messaging Protocol"? I don't think it's
> ever spelled out in this patchset.
> 

I believe that is the case, would you like me to spell it out?

Re: [PATCH v3 04/10] dt-bindings: soc: qcom: Add AOSS QMP binding

[Stephen Boyd]

Quoting Bjorn Andersson (2019-01-22 11:25:37)
> On Tue 22 Jan 11:04 PST 2019, Stephen Boyd wrote:
> 
> > Quoting Bjorn Andersson (2019-01-21 21:51:06)
> > > Add binding for the QMP based side-channel communication mechanism to
> > > the AOSS, which is used to control resources not exposed through the
> > > RPMh interface.
> > > 
> > > Reviewed-by: Rob Herring <robh@kernel.org>
> > > Reviewed-by: Stephen Boyd <swboyd@chromium.org>
> > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > > ---
> > > 
> > > Changes since v2:
> > > - Update indentation of example
> > > - Add colling device subnodes to the description
> > > 
> > >  .../bindings/soc/qcom/qcom,aoss-qmp.txt       | 75 +++++++++++++++++++
> > >  include/dt-bindings/power/qcom-aoss-qmp.h     | 15 ++++
> > >  2 files changed, 90 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,aoss-qmp.txt
> > >  create mode 100644 include/dt-bindings/power/qcom-aoss-qmp.h
> > > 
> > > diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,aoss-qmp.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,aoss-qmp.txt
> > > new file mode 100644
> > > index 000000000000..881dc8c7907a
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,aoss-qmp.txt
> > > @@ -0,0 +1,75 @@
> > > +Qualcomm Always-On Subsystem side channel binding
> > > +
> > > +This binding describes the hardware component responsible for side channel
> > > +requests to the always-on subsystem (AOSS), used for certain power management
> > > +requests that is not handled by the standard RPMh interface. Each client in the
> > > +SoC has it's own block of message RAM and IRQ for communication with the AOSS.
> > > +The protocol used to communicate in the message RAM is known as QMP.
> > 
> > Just curious if QMP is "Qualcomm Messaging Protocol"? I don't think it's
> > ever spelled out in this patchset.
> > 
> 
> I believe that is the case, would you like me to spell it out?

Yes, at least once would be great. Thanks.

[PATCH v3 05/10] soc: qcom: Add AOSS QMP communication driver

[Bjorn Andersson]

The AOSS QMP driver is used to communicate with the AOSS for certain
side-channel requests, that are not enabled through the RPMh interface.

The communication is a very simple synchronous mechanism of messages
being written in message RAM and a doorbell in the AOSS is rung. As the
AOSS has processed the message length is cleared and an interrupt is
fired by the AOSS as acknowledgment.

Reviewed-by: Arun Kumar Neelakantam <aneela@codeaurora.org>
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

Changes since v2:
- Free mbox channel on failure and remove

 drivers/soc/qcom/Kconfig          |   9 +
 drivers/soc/qcom/Makefile         |   1 +
 drivers/soc/qcom/aoss-qmp.c       | 317 ++++++++++++++++++++++++++++++
 include/linux/soc/qcom/aoss-qmp.h |  14 ++
 4 files changed, 341 insertions(+)
 create mode 100644 drivers/soc/qcom/aoss-qmp.c
 create mode 100644 include/linux/soc/qcom/aoss-qmp.h

diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index 6241d3e3b115..e2c859121b88 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -3,6 +3,15 @@
 #
 menu "Qualcomm SoC drivers"
 
+config QCOM_AOSS_QMP
+	tristate "Qualcomm AOSS Messaging Driver"
+	depends on ARCH_QCOM || COMPILE_TEST
+	depends on MAILBOX
+	help
+	  This driver provides the means for communicating with the
+	  micro-controller in the AOSS, using QMP, to control certain resource
+	  that are not exposed through RPMh.
+
 config QCOM_COMMAND_DB
 	bool "Qualcomm Command DB"
 	depends on ARCH_QCOM || COMPILE_TEST
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index ffe519b0cb66..2c04d27fbf9e 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 CFLAGS_rpmh-rsc.o := -I$(src)
+obj-$(CONFIG_QCOM_AOSS_QMP) +=	aoss-qmp.o
 obj-$(CONFIG_QCOM_GENI_SE) +=	qcom-geni-se.o
 obj-$(CONFIG_QCOM_COMMAND_DB) += cmd-db.o
 obj-$(CONFIG_QCOM_GLINK_SSR) +=	glink_ssr.o
diff --git a/drivers/soc/qcom/aoss-qmp.c b/drivers/soc/qcom/aoss-qmp.c
new file mode 100644
index 000000000000..86ee622cdadf
--- /dev/null
+++ b/drivers/soc/qcom/aoss-qmp.c
@@ -0,0 +1,317 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018, Linaro Ltd
+ */
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/mailbox_client.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/soc/qcom/aoss-qmp.h>
+
+#define QMP_DESC_MAGIC			0x0
+#define QMP_DESC_VERSION		0x4
+#define QMP_DESC_FEATURES		0x8
+
+#define QMP_DESC_UCORE_LINK_STATE	0xc
+#define QMP_DESC_UCORE_LINK_STATE_ACK	0x10
+#define QMP_DESC_UCORE_CH_STATE		0x14
+#define QMP_DESC_UCORE_CH_STATE_ACK	0x18
+#define QMP_DESC_UCORE_MBOX_SIZE	0x1c
+#define QMP_DESC_UCORE_MBOX_OFFSET	0x20
+
+#define QMP_DESC_MCORE_LINK_STATE	0x24
+#define QMP_DESC_MCORE_LINK_STATE_ACK	0x28
+#define QMP_DESC_MCORE_CH_STATE		0x2c
+#define QMP_DESC_MCORE_CH_STATE_ACK	0x30
+#define QMP_DESC_MCORE_MBOX_SIZE	0x34
+#define QMP_DESC_MCORE_MBOX_OFFSET	0x38
+
+#define QMP_STATE_UP	0x0000ffff
+#define QMP_STATE_DOWN	0xffff0000
+
+#define QMP_MAGIC	0x4d41494c
+#define QMP_VERSION	1
+
+/**
+ * struct qmp - driver state for QMP implementation
+ * @msgram: iomem referencing the message RAM used for communication
+ * @dev: reference to QMP device
+ * @mbox_client: mailbox client used to ring the doorbell on transmit
+ * @mbox_chan: mailbox channel used to ring the doorbell on transmit
+ * @offset: offset within @msgram where messages should be written
+ * @size: maximum size of the messages to be transmitted
+ * @event: wait_queue for synchronization with the IRQ
+ * @tx_lock: provides syncrhonization between multiple callers of qmp_send()
+ * @pd_pdev: platform device for the power-domain child device
+ */
+struct qmp {
+	void __iomem *msgram;
+	struct device *dev;
+
+	struct mbox_client mbox_client;
+	struct mbox_chan *mbox_chan;
+
+	size_t offset;
+	size_t size;
+
+	wait_queue_head_t event;
+
+	struct mutex tx_lock;
+
+	struct platform_device *pd_pdev;
+};
+
+static void qmp_kick(struct qmp *qmp)
+{
+	mbox_send_message(qmp->mbox_chan, NULL);
+	mbox_client_txdone(qmp->mbox_chan, 0);
+}
+
+static bool qmp_magic_valid(struct qmp *qmp)
+{
+	return readl(qmp->msgram + QMP_DESC_MAGIC) == QMP_MAGIC;
+}
+
+static bool qmp_link_acked(struct qmp *qmp)
+{
+	return readl(qmp->msgram + QMP_DESC_MCORE_LINK_STATE_ACK) == QMP_STATE_UP;
+}
+
+static bool qmp_mcore_channel_acked(struct qmp *qmp)
+{
+	return readl(qmp->msgram + QMP_DESC_MCORE_CH_STATE_ACK) == QMP_STATE_UP;
+}
+
+static bool qmp_ucore_channel_up(struct qmp *qmp)
+{
+	return readl(qmp->msgram + QMP_DESC_UCORE_CH_STATE) == QMP_STATE_UP;
+}
+
+static int qmp_open(struct qmp *qmp)
+{
+	int ret;
+	u32 val;
+
+	ret = wait_event_timeout(qmp->event, qmp_magic_valid(qmp), HZ);
+	if (!ret) {
+		dev_err(qmp->dev, "QMP magic doesn't match\n");
+		return -ETIMEDOUT;
+	}
+
+	val = readl(qmp->msgram + QMP_DESC_VERSION);
+	if (val != QMP_VERSION) {
+		dev_err(qmp->dev, "unsupported QMP version %d\n", val);
+		return -EINVAL;
+	}
+
+	qmp->offset = readl(qmp->msgram + QMP_DESC_MCORE_MBOX_OFFSET);
+	qmp->size = readl(qmp->msgram + QMP_DESC_MCORE_MBOX_SIZE);
+	if (!qmp->size) {
+		dev_err(qmp->dev, "invalid mailbox size 0x%zx\n", qmp->size);
+		return -EINVAL;
+	}
+
+	/* Ack remote core's link state */
+	val = readl(qmp->msgram + QMP_DESC_UCORE_LINK_STATE);
+	writel(val, qmp->msgram + QMP_DESC_UCORE_LINK_STATE_ACK);
+
+	/* Set local core's link state to up */
+	writel(QMP_STATE_UP, qmp->msgram + QMP_DESC_MCORE_LINK_STATE);
+
+	qmp_kick(qmp);
+
+	ret = wait_event_timeout(qmp->event, qmp_link_acked(qmp), HZ);
+	if (!ret) {
+		dev_err(qmp->dev, "ucore didn't ack link\n");
+		goto timeout_close_link;
+	}
+
+	writel(QMP_STATE_UP, qmp->msgram + QMP_DESC_MCORE_CH_STATE);
+
+	ret = wait_event_timeout(qmp->event, qmp_ucore_channel_up(qmp), HZ);
+	if (!ret) {
+		dev_err(qmp->dev, "ucore didn't open channel\n");
+		goto timeout_close_channel;
+	}
+
+	/* Ack remote core's channel state */
+	val = readl(qmp->msgram + QMP_DESC_UCORE_CH_STATE);
+	writel(val, qmp->msgram + QMP_DESC_UCORE_CH_STATE_ACK);
+
+	qmp_kick(qmp);
+
+	ret = wait_event_timeout(qmp->event, qmp_mcore_channel_acked(qmp), HZ);
+	if (!ret) {
+		dev_err(qmp->dev, "ucore didn't ack channel\n");
+		goto timeout_close_channel;
+	}
+
+	return 0;
+
+timeout_close_channel:
+	writel(QMP_STATE_DOWN, qmp->msgram + QMP_DESC_MCORE_CH_STATE);
+
+timeout_close_link:
+	writel(QMP_STATE_DOWN, qmp->msgram + QMP_DESC_MCORE_LINK_STATE);
+	qmp_kick(qmp);
+
+	return -ETIMEDOUT;
+}
+
+static void qmp_close(struct qmp *qmp)
+{
+	writel(QMP_STATE_DOWN, qmp->msgram + QMP_DESC_MCORE_CH_STATE);
+	writel(QMP_STATE_DOWN, qmp->msgram + QMP_DESC_MCORE_LINK_STATE);
+	qmp_kick(qmp);
+}
+
+static irqreturn_t qmp_intr(int irq, void *data)
+{
+	struct qmp *qmp = data;
+
+	wake_up_interruptible_all(&qmp->event);
+
+	return IRQ_HANDLED;
+}
+
+static bool qmp_message_empty(struct qmp *qmp)
+{
+	return readl(qmp->msgram + qmp->offset) == 0;
+}
+
+/**
+ * qmp_send() - send a message to the AOSS
+ * @qmp: qmp context
+ * @data: message to be sent
+ * @len: length of the message
+ *
+ * Transmit @data to AOSS and wait for the AOSS to acknowledge the message.
+ * @len must be a multiple of 4 and not longer than the mailbox size. Access is
+ * synchronized by this implementation.
+ *
+ * Return: 0 on success, negative errno on failure
+ */
+int qmp_send(struct qmp *qmp, const void *data, size_t len)
+{
+	int ret;
+
+	if (WARN_ON(len + sizeof(u32) > qmp->size))
+		return -EINVAL;
+
+	if (WARN_ON(len % sizeof(u32)))
+		return -EINVAL;
+
+	mutex_lock(&qmp->tx_lock);
+
+	/* The message RAM only implements 32-bit accesses */
+	__iowrite32_copy(qmp->msgram + qmp->offset + sizeof(u32),
+			 data, len / sizeof(u32));
+	writel(len, qmp->msgram + qmp->offset);
+	qmp_kick(qmp);
+
+	ret = wait_event_interruptible_timeout(qmp->event,
+					       qmp_message_empty(qmp), HZ);
+	if (!ret) {
+		dev_err(qmp->dev, "ucore did not ack channel\n");
+		ret = -ETIMEDOUT;
+
+		/* Clear message from buffer */
+		writel(0, qmp->msgram + qmp->offset);
+	} else {
+		ret = 0;
+	}
+
+	mutex_unlock(&qmp->tx_lock);
+
+	return ret;
+}
+EXPORT_SYMBOL(qmp_send);
+
+static int qmp_probe(struct platform_device *pdev)
+{
+	struct resource *res;
+	struct qmp *qmp;
+	int irq;
+	int ret;
+
+	qmp = devm_kzalloc(&pdev->dev, sizeof(*qmp), GFP_KERNEL);
+	if (!qmp)
+		return -ENOMEM;
+
+	qmp->dev = &pdev->dev;
+	init_waitqueue_head(&qmp->event);
+	mutex_init(&qmp->tx_lock);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	qmp->msgram = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(qmp->msgram))
+		return PTR_ERR(qmp->msgram);
+
+	qmp->mbox_client.dev = &pdev->dev;
+	qmp->mbox_client.knows_txdone = true;
+	qmp->mbox_chan = mbox_request_channel(&qmp->mbox_client, 0);
+	if (IS_ERR(qmp->mbox_chan)) {
+		dev_err(&pdev->dev, "failed to acquire ipc mailbox\n");
+		return PTR_ERR(qmp->mbox_chan);
+	}
+
+	irq = platform_get_irq(pdev, 0);
+	ret = devm_request_irq(&pdev->dev, irq, qmp_intr, IRQF_ONESHOT,
+			       "aoss-qmp", qmp);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to request interrupt\n");
+		mbox_free_channel(qmp->mbox_chan);
+		return ret;
+	}
+
+	ret = qmp_open(qmp);
+	if (ret < 0) {
+		mbox_free_channel(qmp->mbox_chan);
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, qmp);
+
+	if (of_property_read_bool(pdev->dev.of_node, "#power-domain-cells")) {
+		qmp->pd_pdev = platform_device_register_data(&pdev->dev,
+							     "aoss_qmp_pd",
+							     PLATFORM_DEVID_NONE,
+							     NULL, 0);
+		if (IS_ERR(qmp->pd_pdev))
+			dev_err(&pdev->dev, "failed to register AOSS PD\n");
+	}
+
+	return 0;
+}
+
+static int qmp_remove(struct platform_device *pdev)
+{
+	struct qmp *qmp = platform_get_drvdata(pdev);
+
+	platform_device_unregister(qmp->pd_pdev);
+
+	mbox_free_channel(qmp->mbox_chan);
+	qmp_close(qmp);
+
+	return 0;
+}
+
+static const struct of_device_id qmp_dt_match[] = {
+	{ .compatible = "qcom,sdm845-aoss-qmp", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, qmp_dt_match);
+
+static struct platform_driver qmp_driver = {
+	.driver = {
+		.name		= "aoss_qmp",
+		.of_match_table	= qmp_dt_match,
+	},
+	.probe = qmp_probe,
+	.remove	= qmp_remove,
+};
+module_platform_driver(qmp_driver);
+
+MODULE_DESCRIPTION("Qualcomm AOSS QMP driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/soc/qcom/aoss-qmp.h b/include/linux/soc/qcom/aoss-qmp.h
new file mode 100644
index 000000000000..a2ac891d7fd4
--- /dev/null
+++ b/include/linux/soc/qcom/aoss-qmp.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2018, Linaro Ltd
+ */
+#ifndef __AOP_QMP_H__
+#define __AOP_QMP_H__
+
+#include <linux/types.h>
+
+struct qmp;
+
+int qmp_send(struct qmp *qmp, const void *data, size_t len);
+
+#endif
-- 
2.18.0

[PATCH v3 06/10] soc: qcom: Add AOSS QMP genpd provider

[Bjorn Andersson]

The AOSS QMP genpd provider implements control over power-related
resources related to low-power state associated with the remoteprocs in
the system as well as control over a set of clocks related to debug
hardware in the SoC.

Tested-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

Changes since v2:
- Add define for request array size

 drivers/soc/qcom/Kconfig       |   9 +++
 drivers/soc/qcom/Makefile      |   1 +
 drivers/soc/qcom/aoss-qmp-pd.c | 138 +++++++++++++++++++++++++++++++++
 3 files changed, 148 insertions(+)
 create mode 100644 drivers/soc/qcom/aoss-qmp-pd.c

diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index e2c859121b88..9d71e23005ad 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -12,6 +12,15 @@ config QCOM_AOSS_QMP
 	  micro-controller in the AOSS, using QMP, to control certain resource
 	  that are not exposed through RPMh.
 
+config QCOM_AOSS_QMP_PD
+	tristate "Qualcomm AOSS Messaging Power Domain driver"
+	depends on QCOM_AOSS_QMP
+	select PM_GENERIC_DOMAINS
+	help
+	  This driver provides the means of controlling the AOSS's handling of
+	  low-power state for resources related to the remoteproc subsystems as
+	  well as controlling the debug clocks.
+
 config QCOM_COMMAND_DB
 	bool "Qualcomm Command DB"
 	depends on ARCH_QCOM || COMPILE_TEST
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index 2c04d27fbf9e..16913e73fddf 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 CFLAGS_rpmh-rsc.o := -I$(src)
 obj-$(CONFIG_QCOM_AOSS_QMP) +=	aoss-qmp.o
+obj-$(CONFIG_QCOM_AOSS_QMP_PD) += aoss-qmp-pd.o
 obj-$(CONFIG_QCOM_GENI_SE) +=	qcom-geni-se.o
 obj-$(CONFIG_QCOM_COMMAND_DB) += cmd-db.o
 obj-$(CONFIG_QCOM_GLINK_SSR) +=	glink_ssr.o
diff --git a/drivers/soc/qcom/aoss-qmp-pd.c b/drivers/soc/qcom/aoss-qmp-pd.c
new file mode 100644
index 000000000000..82dd569a2bc9
--- /dev/null
+++ b/drivers/soc/qcom/aoss-qmp-pd.c
@@ -0,0 +1,138 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018, Linaro Ltd
+ */
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+#include <linux/soc/qcom/aoss-qmp.h>
+#include <dt-bindings/power/qcom-aoss-qmp.h>
+
+/* Requests are expected to be 96 bytes long */
+#define AOSS_QMP_PD_MSG_LEN	96
+
+struct qmp_pd {
+	struct qmp *qmp;
+
+	struct generic_pm_domain pd;
+
+	const char *name;
+};
+
+#define to_qmp_pd_resource(res) container_of(res, struct qmp_pd, pd)
+
+struct qmp_pd_resource {
+	const char *name;
+	int (*on)(struct generic_pm_domain *domain);
+	int (*off)(struct generic_pm_domain *domain);
+};
+
+static int qmp_pd_clock_toggle(struct qmp_pd *res, bool enable)
+{
+	char buf[AOSS_QMP_PD_MSG_LEN];
+
+	snprintf(buf, sizeof(buf), "{class: clock, res: %s, val: %d}",
+		 res->name, !!enable);
+	return qmp_send(res->qmp, buf, sizeof(buf));
+}
+
+static int qmp_pd_clock_on(struct generic_pm_domain *domain)
+{
+	return qmp_pd_clock_toggle(to_qmp_pd_resource(domain), true);
+}
+
+static int qmp_pd_clock_off(struct generic_pm_domain *domain)
+{
+	return qmp_pd_clock_toggle(to_qmp_pd_resource(domain), false);
+}
+
+static int qmp_pd_image_toggle(struct qmp_pd *res, bool enable)
+{
+	char buf[AOSS_QMP_PD_MSG_LEN];
+
+	snprintf(buf, sizeof(buf),
+		 "{class: image, res: load_state, name: %s, val: %s}",
+		 res->name, enable ? "on" : "off");
+	return qmp_send(res->qmp, buf, sizeof(buf));
+}
+
+static int qmp_pd_image_on(struct generic_pm_domain *domain)
+{
+	return qmp_pd_image_toggle(to_qmp_pd_resource(domain), true);
+}
+
+static int qmp_pd_image_off(struct generic_pm_domain *domain)
+{
+	return qmp_pd_image_toggle(to_qmp_pd_resource(domain), false);
+}
+
+static const struct qmp_pd_resource sdm845_resources[] = {
+	[AOSS_QMP_QDSS_CLK] = { "qdss", qmp_pd_clock_on, qmp_pd_clock_off },
+	[AOSS_QMP_LS_CDSP] = { "cdsp", qmp_pd_image_on, qmp_pd_image_off },
+	[AOSS_QMP_LS_LPASS] = { "adsp", qmp_pd_image_on, qmp_pd_image_off },
+	[AOSS_QMP_LS_MODEM] = { "modem", qmp_pd_image_on, qmp_pd_image_off },
+	[AOSS_QMP_LS_SLPI] = { "slpi", qmp_pd_image_on, qmp_pd_image_off },
+	[AOSS_QMP_LS_SPSS] = { "spss", qmp_pd_image_on, qmp_pd_image_off },
+	[AOSS_QMP_LS_VENUS] = { "venus", qmp_pd_image_on, qmp_pd_image_off },
+};
+
+static int qmp_pd_probe(struct platform_device *pdev)
+{
+	struct genpd_onecell_data *data;
+	struct device *parent = pdev->dev.parent;
+	struct qmp_pd *res;
+	struct qmp *qmp;
+	size_t num = ARRAY_SIZE(sdm845_resources);
+	int i;
+
+	qmp = dev_get_drvdata(pdev->dev.parent);
+	if (!qmp)
+		return -EINVAL;
+
+	res = devm_kcalloc(&pdev->dev, num, sizeof(*res), GFP_KERNEL);
+	if (!res)
+		return -ENOMEM;
+
+	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	data->domains = devm_kcalloc(&pdev->dev, num, sizeof(*data->domains),
+				     GFP_KERNEL);
+
+	for (i = 0; i < num; i++) {
+		pm_genpd_init(&res[i].pd, NULL, true);
+		res[i].qmp = qmp;
+		res[i].name = sdm845_resources[i].name;
+
+		res[i].pd.name = sdm845_resources[i].name;
+		res[i].pd.power_on = sdm845_resources[i].on;
+		res[i].pd.power_off = sdm845_resources[i].off;
+
+		data->domains[data->num_domains++] = &res[i].pd;
+	}
+
+	return of_genpd_add_provider_onecell(parent->of_node, data);
+}
+
+static int qmp_pd_remove(struct platform_device *pdev)
+{
+	struct device *parent = pdev->dev.parent;
+
+	of_genpd_del_provider(parent->of_node);
+
+	return 0;
+}
+
+static struct platform_driver qmp_pd_driver = {
+	.driver = {
+		.name		= "aoss_qmp_pd",
+	},
+	.probe = qmp_pd_probe,
+	.remove = qmp_pd_remove,
+};
+module_platform_driver(qmp_pd_driver);
+
+MODULE_ALIAS("platform:aoss_qmp_pd");
+MODULE_DESCRIPTION("Qualcomm AOSS QMP load-state driver");
+MODULE_LICENSE("GPL v2");
-- 
2.18.0

[PATCH v3 07/10] remoteproc: q6v5-mss: Vote for rpmh power domains

[Bjorn Andersson]

From: Rajendra Nayak <rnayak@codeaurora.org>

With rpmh ARC resources being modelled as power domains with performance
state, we need to proxy vote on these for SDM845.
Add support to vote on multiple of them, now that genpd supports
associating mutliple power domains to a device.

Tested-by: Sibi Sankar <sibis@codeaurora.org>
Reviewed-by: Sibi Sankar <sibis@codeaurora.org>
Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
[bjorn: Drop device link, improve error handling, name things "proxy"]
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

Changes since v2:
- Disable proxy pds if handover did not arrived before stop

 drivers/remoteproc/qcom_q6v5_mss.c | 117 ++++++++++++++++++++++++++++-
 1 file changed, 113 insertions(+), 4 deletions(-)

diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
index 01be7314e176..003186ce56c7 100644
--- a/drivers/remoteproc/qcom_q6v5_mss.c
+++ b/drivers/remoteproc/qcom_q6v5_mss.c
@@ -25,6 +25,8 @@
 #include <linux/of_address.h>
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+#include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 #include <linux/remoteproc.h>
@@ -131,6 +133,7 @@ struct rproc_hexagon_res {
 	char **proxy_clk_names;
 	char **reset_clk_names;
 	char **active_clk_names;
+	char **proxy_pd_names;
 	int version;
 	bool need_mem_protection;
 	bool has_alt_reset;
@@ -156,9 +159,11 @@ struct q6v5 {
 	struct clk *active_clks[8];
 	struct clk *reset_clks[4];
 	struct clk *proxy_clks[4];
+	struct device *proxy_pds[3];
 	int active_clk_count;
 	int reset_clk_count;
 	int proxy_clk_count;
+	int proxy_pd_count;
 
 	struct reg_info active_regs[1];
 	struct reg_info proxy_regs[3];
@@ -321,6 +326,41 @@ static void q6v5_clk_disable(struct device *dev,
 		clk_disable_unprepare(clks[i]);
 }
 
+static int q6v5_pds_enable(struct q6v5 *qproc, struct device **pds,
+			   size_t pd_count)
+{
+	int ret;
+	int i;
+
+	for (i = 0; i < pd_count; i++) {
+		dev_pm_genpd_set_performance_state(pds[i], INT_MAX);
+		ret = pm_runtime_get_sync(pds[i]);
+		if (ret < 0)
+			goto unroll_pd_votes;
+	}
+
+	return 0;
+
+unroll_pd_votes:
+	for (i--; i >= 0; i--) {
+		dev_pm_genpd_set_performance_state(pds[i], 0);
+		pm_runtime_put(pds[i]);
+	}
+
+	return ret;
+};
+
+static void q6v5_pds_disable(struct q6v5 *qproc, struct device **pds,
+			     size_t pd_count)
+{
+	int i;
+
+	for (i = 0; i < pd_count; i++) {
+		dev_pm_genpd_set_performance_state(pds[i], 0);
+		pm_runtime_put(pds[i]);
+	}
+}
+
 static int q6v5_xfer_mem_ownership(struct q6v5 *qproc, int *current_perm,
 				   bool remote_owner, phys_addr_t addr,
 				   size_t size)
@@ -690,11 +730,17 @@ static int q6v5_mba_load(struct q6v5 *qproc)
 
 	qcom_q6v5_prepare(&qproc->q6v5);
 
+	ret = q6v5_pds_enable(qproc, qproc->proxy_pds, qproc->proxy_pd_count);
+	if (ret < 0) {
+		dev_err(qproc->dev, "failed to enable proxy power domains\n");
+		goto disable_irqs;
+	}
+
 	ret = q6v5_regulator_enable(qproc, qproc->proxy_regs,
 				    qproc->proxy_reg_count);
 	if (ret) {
 		dev_err(qproc->dev, "failed to enable proxy supplies\n");
-		goto disable_irqs;
+		goto disable_proxy_pds;
 	}
 
 	ret = q6v5_clk_enable(qproc->dev, qproc->proxy_clks,
@@ -791,6 +837,8 @@ static int q6v5_mba_load(struct q6v5 *qproc)
 disable_proxy_reg:
 	q6v5_regulator_disable(qproc, qproc->proxy_regs,
 			       qproc->proxy_reg_count);
+disable_proxy_pds:
+	q6v5_pds_disable(qproc, qproc->proxy_pds, qproc->proxy_pd_count);
 disable_irqs:
 	qcom_q6v5_unprepare(&qproc->q6v5);
 
@@ -841,6 +889,8 @@ static void q6v5_mba_reclaim(struct q6v5 *qproc)
 
 	ret = qcom_q6v5_unprepare(&qproc->q6v5);
 	if (ret) {
+		q6v5_pds_disable(qproc, qproc->proxy_pds,
+				 qproc->proxy_pd_count);
 		q6v5_clk_disable(qproc->dev, qproc->proxy_clks,
 				 qproc->proxy_clk_count);
 		q6v5_regulator_disable(qproc, qproc->proxy_regs,
@@ -1121,6 +1171,7 @@ static void qcom_msa_handover(struct qcom_q6v5 *q6v5)
 			 qproc->proxy_clk_count);
 	q6v5_regulator_disable(qproc, qproc->proxy_regs,
 			       qproc->proxy_reg_count);
+	q6v5_pds_disable(qproc, qproc->proxy_pds, qproc->proxy_pd_count);
 }
 
 static int q6v5_init_mem(struct q6v5 *qproc, struct platform_device *pdev)
@@ -1181,6 +1232,45 @@ static int q6v5_init_clocks(struct device *dev, struct clk **clks,
 	return i;
 }
 
+static int q6v5_pds_attach(struct device *dev, struct device **devs,
+			   char **pd_names)
+{
+	size_t num_pds = 0;
+	int ret;
+	int i;
+
+	if (!pd_names)
+		return 0;
+
+	while (pd_names[num_pds])
+		num_pds++;
+
+	for (i = 0; i < num_pds; i++) {
+		devs[i] = dev_pm_domain_attach_by_name(dev, pd_names[i]);
+		if (IS_ERR(devs[i])) {
+			ret = PTR_ERR(devs[i]);
+			goto unroll_attach;
+		}
+	}
+
+	return num_pds;
+
+unroll_attach:
+	for (i--; i >= 0; i--)
+		dev_pm_domain_detach(devs[i], false);
+
+	return ret;
+};
+
+static void q6v5_pds_detach(struct q6v5 *qproc, struct device **pds,
+			    size_t pd_count)
+{
+	int i;
+
+	for (i = 0; i < pd_count; i++)
+		dev_pm_domain_detach(pds[i], false);
+}
+
 static int q6v5_init_reset(struct q6v5 *qproc)
 {
 	qproc->mss_restart = devm_reset_control_get_exclusive(qproc->dev,
@@ -1322,10 +1412,18 @@ static int q6v5_probe(struct platform_device *pdev)
 	}
 	qproc->active_reg_count = ret;
 
+	ret = q6v5_pds_attach(&pdev->dev, qproc->proxy_pds,
+			      desc->proxy_pd_names);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Failed to init power domains\n");
+		goto free_rproc;
+	}
+	qproc->proxy_pd_count = ret;
+
 	qproc->has_alt_reset = desc->has_alt_reset;
 	ret = q6v5_init_reset(qproc);
 	if (ret)
-		goto free_rproc;
+		goto detach_proxy_pds;
 
 	qproc->version = desc->version;
 	qproc->need_mem_protection = desc->need_mem_protection;
@@ -1333,7 +1431,7 @@ static int q6v5_probe(struct platform_device *pdev)
 	ret = qcom_q6v5_init(&qproc->q6v5, pdev, rproc, MPSS_CRASH_REASON_SMEM,
 			     qcom_msa_handover);
 	if (ret)
-		goto free_rproc;
+		goto detach_proxy_pds;
 
 	qproc->mpss_perm = BIT(QCOM_SCM_VMID_HLOS);
 	qproc->mba_perm = BIT(QCOM_SCM_VMID_HLOS);
@@ -1344,10 +1442,12 @@ static int q6v5_probe(struct platform_device *pdev)
 
 	ret = rproc_add(rproc);
 	if (ret)
-		goto free_rproc;
+		goto detach_proxy_pds;
 
 	return 0;
 
+detach_proxy_pds:
+	q6v5_pds_detach(qproc, qproc->proxy_pds, qproc->proxy_pd_count);
 free_rproc:
 	rproc_free(rproc);
 
@@ -1364,6 +1464,9 @@ static int q6v5_remove(struct platform_device *pdev)
 	qcom_remove_glink_subdev(qproc->rproc, &qproc->glink_subdev);
 	qcom_remove_smd_subdev(qproc->rproc, &qproc->smd_subdev);
 	qcom_remove_ssr_subdev(qproc->rproc, &qproc->ssr_subdev);
+
+	q6v5_pds_detach(qproc, qproc->proxy_pds, qproc->proxy_pd_count);
+
 	rproc_free(qproc->rproc);
 
 	return 0;
@@ -1388,6 +1491,12 @@ static const struct rproc_hexagon_res sdm845_mss = {
 			"mnoc_axi",
 			NULL
 	},
+	.proxy_pd_names = (char*[]){
+			"cx",
+			"mx",
+			"mss",
+			NULL
+	},
 	.need_mem_protection = true,
 	.has_alt_reset = true,
 	.version = MSS_SDM845,
-- 
2.18.0

Re: [PATCH v3 07/10] remoteproc: q6v5-mss: Vote for rpmh power domains

[Doug Anderson]

Hi,

On Mon, Jan 21, 2019 at 9:52 PM Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
> @@ -1333,7 +1431,7 @@ static int q6v5_probe(struct platform_device *pdev)
>         ret = qcom_q6v5_init(&qproc->q6v5, pdev, rproc, MPSS_CRASH_REASON_SMEM,
>                              qcom_msa_handover);
>         if (ret)
> -               goto free_rproc;
> +               goto detach_proxy_pds;
>
>         qproc->mpss_perm = BIT(QCOM_SCM_VMID_HLOS);
>         qproc->mba_perm = BIT(QCOM_SCM_VMID_HLOS);
> @@ -1344,10 +1442,12 @@ static int q6v5_probe(struct platform_device *pdev)
>
>         ret = rproc_add(rproc);
>         if (ret)
> -               goto free_rproc;
> +               goto detach_proxy_pds;

I can't comment on the patch overall since I haven't spent any time in
remoteproc, but as previously pointed out by Sibi, now that you've
landed commit 027045a6e2b7 ("remoteproc: qcom: Add shutdown-ack irq"),
you need to adjust the "goto"s in your patch.  Specifically
(whitespace damaged) atop your whole series:

@@ -1488,7 +1488,7 @@ static int q6v5_probe(struct platform_device *pdev)
        qproc->sysmon = qcom_add_sysmon_subdev(rproc, "modem", 0x12);
        if (IS_ERR(qproc->sysmon)) {
                ret = PTR_ERR(qproc->sysmon);
-               goto free_rproc;
+               goto detach_proxy_pds;
        }

-Doug

[PATCH v3 08/10] remoteproc: q6v5-mss: Active powerdomain for SDM845

[Bjorn Andersson]

The SDM845 MSS needs the load_state powerdomain voted for during the
duration of the MSS being powered on, to let the AOSS know that it may
not perform certain power save measures. So vote for this.

Tested-by: Sibi Sankar <sibis@codeaurora.org>
Reviewed-by: Sibi Sankar <sibis@codeaurora.org>
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

Changes since v2:
- None

 drivers/remoteproc/qcom_q6v5_mss.c | 31 ++++++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
index 003186ce56c7..3e25016954d9 100644
--- a/drivers/remoteproc/qcom_q6v5_mss.c
+++ b/drivers/remoteproc/qcom_q6v5_mss.c
@@ -133,6 +133,7 @@ struct rproc_hexagon_res {
 	char **proxy_clk_names;
 	char **reset_clk_names;
 	char **active_clk_names;
+	char **active_pd_names;
 	char **proxy_pd_names;
 	int version;
 	bool need_mem_protection;
@@ -159,10 +160,12 @@ struct q6v5 {
 	struct clk *active_clks[8];
 	struct clk *reset_clks[4];
 	struct clk *proxy_clks[4];
+	struct device *active_pds[1];
 	struct device *proxy_pds[3];
 	int active_clk_count;
 	int reset_clk_count;
 	int proxy_clk_count;
+	int active_pd_count;
 	int proxy_pd_count;
 
 	struct reg_info active_regs[1];
@@ -730,10 +733,16 @@ static int q6v5_mba_load(struct q6v5 *qproc)
 
 	qcom_q6v5_prepare(&qproc->q6v5);
 
+	ret = q6v5_pds_enable(qproc, qproc->active_pds, qproc->active_pd_count);
+	if (ret < 0) {
+		dev_err(qproc->dev, "failed to enable active power domains\n");
+		goto disable_irqs;
+	}
+
 	ret = q6v5_pds_enable(qproc, qproc->proxy_pds, qproc->proxy_pd_count);
 	if (ret < 0) {
 		dev_err(qproc->dev, "failed to enable proxy power domains\n");
-		goto disable_irqs;
+		goto disable_active_pds;
 	}
 
 	ret = q6v5_regulator_enable(qproc, qproc->proxy_regs,
@@ -839,6 +848,8 @@ static int q6v5_mba_load(struct q6v5 *qproc)
 			       qproc->proxy_reg_count);
 disable_proxy_pds:
 	q6v5_pds_disable(qproc, qproc->proxy_pds, qproc->proxy_pd_count);
+disable_active_pds:
+	q6v5_pds_disable(qproc, qproc->active_pds, qproc->active_pd_count);
 disable_irqs:
 	qcom_q6v5_unprepare(&qproc->q6v5);
 
@@ -878,6 +889,7 @@ static void q6v5_mba_reclaim(struct q6v5 *qproc)
 			 qproc->active_clk_count);
 	q6v5_regulator_disable(qproc, qproc->active_regs,
 			       qproc->active_reg_count);
+	q6v5_pds_disable(qproc, qproc->active_pds, qproc->active_pd_count);
 
 	/* In case of failure or coredump scenario where reclaiming MBA memory
 	 * could not happen reclaim it here.
@@ -1412,11 +1424,19 @@ static int q6v5_probe(struct platform_device *pdev)
 	}
 	qproc->active_reg_count = ret;
 
+	ret = q6v5_pds_attach(&pdev->dev, qproc->active_pds,
+			      desc->active_pd_names);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Failed to attach active power domains\n");
+		goto free_rproc;
+	}
+	qproc->active_pd_count = ret;
+
 	ret = q6v5_pds_attach(&pdev->dev, qproc->proxy_pds,
 			      desc->proxy_pd_names);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "Failed to init power domains\n");
-		goto free_rproc;
+		goto detach_active_pds;
 	}
 	qproc->proxy_pd_count = ret;
 
@@ -1448,6 +1468,8 @@ static int q6v5_probe(struct platform_device *pdev)
 
 detach_proxy_pds:
 	q6v5_pds_detach(qproc, qproc->proxy_pds, qproc->proxy_pd_count);
+detach_active_pds:
+	q6v5_pds_detach(qproc, qproc->active_pds, qproc->active_pd_count);
 free_rproc:
 	rproc_free(rproc);
 
@@ -1465,6 +1487,7 @@ static int q6v5_remove(struct platform_device *pdev)
 	qcom_remove_smd_subdev(qproc->rproc, &qproc->smd_subdev);
 	qcom_remove_ssr_subdev(qproc->rproc, &qproc->ssr_subdev);
 
+	q6v5_pds_detach(qproc, qproc->active_pds, qproc->active_pd_count);
 	q6v5_pds_detach(qproc, qproc->proxy_pds, qproc->proxy_pd_count);
 
 	rproc_free(qproc->rproc);
@@ -1491,6 +1514,10 @@ static const struct rproc_hexagon_res sdm845_mss = {
 			"mnoc_axi",
 			NULL
 	},
+	.active_pd_names = (char*[]){
+			"load_state",
+			NULL
+	},
 	.proxy_pd_names = (char*[]){
 			"cx",
 			"mx",
-- 
2.18.0

[PATCH v3 09/10] arm64: dts: qcom: Add AOSS QMP node

[Bjorn Andersson]

The AOSS QMP provides a number of power domains, used for QDSS and
PIL, add the node for this.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

Changes since v2:
- New patch

 arch/arm64/boot/dts/qcom/sdm845.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 1033b77856e6..5cc2615461da 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -13,6 +13,7 @@
 #include <dt-bindings/clock/qcom,videocc-sdm845.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/phy/phy-qcom-qusb2.h>
+#include <dt-bindings/power/qcom-aoss-qmp.h>
 #include <dt-bindings/power/qcom-rpmpd.h>
 #include <dt-bindings/reset/qcom,sdm845-aoss.h>
 #include <dt-bindings/reset/qcom,sdm845-pdc.h>
@@ -2070,6 +2071,15 @@
 			#reset-cells = <1>;
 		};
 
+		aoss_qmp: qmp@c300000 {
+			compatible = "qcom,sdm845-aoss-qmp";
+			reg = <0 0x0c300000 0 0x100000>;
+			interrupts = <GIC_SPI 389 IRQ_TYPE_EDGE_RISING>;
+			mboxes = <&apss_shared 0>;
+
+			#power-domain-cells = <1>;
+		};
+
 		spmi_bus: spmi@c440000 {
 			compatible = "qcom,spmi-pmic-arb";
 			reg = <0 0x0c440000 0 0x1100>,
-- 
2.18.0

[PATCH v3 10/10] arm64: dts: qcom: sdm845: Add Q6V5 MSS node

[Bjorn Andersson]

From: Sibi Sankar <sibis@codeaurora.org>

This patch adds Q6V5 MSS remoteproc node for SDM845 SoCs.

Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

Changes since v2:
- Picked up Sibi's patch
- Fixed reg to work with address/size-cells as 2

 arch/arm64/boot/dts/qcom/sdm845.dtsi | 58 ++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 5cc2615461da..78df5f1bce2d 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -1617,6 +1617,64 @@
 			clock-names = "xo";
 		};
 
+		mss_pil: remoteproc@4080000 {
+			compatible = "qcom,sdm845-mss-pil";
+			reg = <0 0x04080000 0 0x408>, <0 0x04180000 0 0x48>;
+			reg-names = "qdsp6", "rmb";
+
+			interrupts-extended =
+				<&intc GIC_SPI 266 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>,
+				<&modem_smp2p_in 7 IRQ_TYPE_EDGE_RISING>;
+			interrupt-names = "wdog", "fatal", "ready",
+					  "handover", "stop-ack",
+					  "shutdown-ack";
+
+			clocks = <&gcc GCC_MSS_CFG_AHB_CLK>,
+				 <&gcc GCC_MSS_Q6_MEMNOC_AXI_CLK>,
+				 <&gcc GCC_BOOT_ROM_AHB_CLK>,
+				 <&gcc GCC_MSS_GPLL0_DIV_CLK_SRC>,
+				 <&gcc GCC_MSS_SNOC_AXI_CLK>,
+				 <&gcc GCC_MSS_MFAB_AXIS_CLK>,
+				 <&gcc GCC_PRNG_AHB_CLK>,
+				 <&rpmhcc RPMH_CXO_CLK>;
+			clock-names = "iface", "bus", "mem", "gpll0_mss",
+				      "snoc_axi", "mnoc_axi", "prng", "xo";
+
+			qcom,smem-states = <&modem_smp2p_out 0>;
+			qcom,smem-state-names = "stop";
+
+			resets = <&aoss_reset AOSS_CC_MSS_RESTART>,
+				 <&pdc_reset PDC_MODEM_SYNC_RESET>;
+			reset-names = "mss_restart", "pdc_reset";
+
+			qcom,halt-regs = <&tcsr_mutex_regs 0x23000 0x25000 0x24000>;
+
+			power-domains = <&aoss_qmp AOSS_QMP_LS_MODEM>,
+					<&rpmhpd SDM845_CX>,
+					<&rpmhpd SDM845_MX>,
+					<&rpmhpd SDM845_MSS>;
+			power-domain-names = "load_state", "cx", "mx", "mss";
+
+			mba {
+				memory-region = <&mba_region>;
+			};
+
+			mpss {
+				memory-region = <&mpss_region>;
+			};
+
+			glink-edge {
+				interrupts = <GIC_SPI 449 IRQ_TYPE_EDGE_RISING>;
+				label = "modem";
+				qcom,remote-pid = <1>;
+				mboxes = <&apss_shared 12>;
+			};
+		};
+
 		sdhc_2: sdhci@8804000 {
 			compatible = "qcom,sdm845-sdhci", "qcom,sdhci-msm-v5";
 			reg = <0 0x08804000 0 0x1000>;
-- 
2.18.0

Re: [PATCH v3 10/10] arm64: dts: qcom: sdm845: Add Q6V5 MSS node

[Doug Anderson]

Hi,

On Mon, Jan 21, 2019 at 9:52 PM Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>
> From: Sibi Sankar <sibis@codeaurora.org>
>
> This patch adds Q6V5 MSS remoteproc node for SDM845 SoCs.
>
> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
>
> Changes since v2:
> - Picked up Sibi's patch
> - Fixed reg to work with address/size-cells as 2
>
>  arch/arm64/boot/dts/qcom/sdm845.dtsi | 58 ++++++++++++++++++++++++++++
>  1 file changed, 58 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index 5cc2615461da..78df5f1bce2d 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -1617,6 +1617,64 @@
>                         clock-names = "xo";
>                 };
>
> +               mss_pil: remoteproc@4080000 {
> +                       compatible = "qcom,sdm845-mss-pil";
> +                       reg = <0 0x04080000 0 0x408>, <0 0x04180000 0 0x48>;
> +                       reg-names = "qdsp6", "rmb";
> +
> +                       interrupts-extended =
> +                               <&intc GIC_SPI 266 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>,
> +                               <&modem_smp2p_in 7 IRQ_TYPE_EDGE_RISING>;
> +                       interrupt-names = "wdog", "fatal", "ready",
> +                                         "handover", "stop-ack",
> +                                         "shutdown-ack";
> +
> +                       clocks = <&gcc GCC_MSS_CFG_AHB_CLK>,
> +                                <&gcc GCC_MSS_Q6_MEMNOC_AXI_CLK>,
> +                                <&gcc GCC_BOOT_ROM_AHB_CLK>,
> +                                <&gcc GCC_MSS_GPLL0_DIV_CLK_SRC>,
> +                                <&gcc GCC_MSS_SNOC_AXI_CLK>,
> +                                <&gcc GCC_MSS_MFAB_AXIS_CLK>,
> +                                <&gcc GCC_PRNG_AHB_CLK>,
> +                                <&rpmhcc RPMH_CXO_CLK>;
> +                       clock-names = "iface", "bus", "mem", "gpll0_mss",
> +                                     "snoc_axi", "mnoc_axi", "prng", "xo";
> +
> +                       qcom,smem-states = <&modem_smp2p_out 0>;
> +                       qcom,smem-state-names = "stop";
> +
> +                       resets = <&aoss_reset AOSS_CC_MSS_RESTART>,
> +                                <&pdc_reset PDC_MODEM_SYNC_RESET>;
> +                       reset-names = "mss_restart", "pdc_reset";
> +
> +                       qcom,halt-regs = <&tcsr_mutex_regs 0x23000 0x25000 0x24000>;
> +
> +                       power-domains = <&aoss_qmp AOSS_QMP_LS_MODEM>,
> +                                       <&rpmhpd SDM845_CX>,
> +                                       <&rpmhpd SDM845_MX>,
> +                                       <&rpmhpd SDM845_MSS>;
> +                       power-domain-names = "load_state", "cx", "mx", "mss";
> +
> +                       mba {
> +                               memory-region = <&mba_region>;
> +                       };
> +
> +                       mpss {
> +                               memory-region = <&mpss_region>;
> +                       };
> +
> +                       glink-edge {
> +                               interrupts = <GIC_SPI 449 IRQ_TYPE_EDGE_RISING>;
> +                               label = "modem";
> +                               qcom,remote-pid = <1>;
> +                               mboxes = <&apss_shared 12>;
> +                       };
> +               };
> +
>                 sdhc_2: sdhci@8804000 {

Can you please sort by unit address now that you have a device tree
that has more stuff?

-Doug

Re: [PATCH v3 10/10] arm64: dts: qcom: sdm845: Add Q6V5 MSS node

[Bjorn Andersson]

On Tue 22 Jan 16:28 PST 2019, Doug Anderson wrote:

> Hi,
> 
> On Mon, Jan 21, 2019 at 9:52 PM Bjorn Andersson
> <bjorn.andersson@linaro.org> wrote:
> >
> > From: Sibi Sankar <sibis@codeaurora.org>
> >
> > This patch adds Q6V5 MSS remoteproc node for SDM845 SoCs.
> >
> > Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> > Reviewed-by: Douglas Anderson <dianders@chromium.org>
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > ---
> >
> > Changes since v2:
> > - Picked up Sibi's patch
> > - Fixed reg to work with address/size-cells as 2
> >
> >  arch/arm64/boot/dts/qcom/sdm845.dtsi | 58 ++++++++++++++++++++++++++++
> >  1 file changed, 58 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > index 5cc2615461da..78df5f1bce2d 100644
> > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > @@ -1617,6 +1617,64 @@
> >                         clock-names = "xo";
> >                 };
> >
> > +               mss_pil: remoteproc@4080000 {
> > +                       compatible = "qcom,sdm845-mss-pil";
> > +                       reg = <0 0x04080000 0 0x408>, <0 0x04180000 0 0x48>;
> > +                       reg-names = "qdsp6", "rmb";
> > +
> > +                       interrupts-extended =
> > +                               <&intc GIC_SPI 266 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>,
> > +                               <&modem_smp2p_in 7 IRQ_TYPE_EDGE_RISING>;
> > +                       interrupt-names = "wdog", "fatal", "ready",
> > +                                         "handover", "stop-ack",
> > +                                         "shutdown-ack";
> > +
> > +                       clocks = <&gcc GCC_MSS_CFG_AHB_CLK>,
> > +                                <&gcc GCC_MSS_Q6_MEMNOC_AXI_CLK>,
> > +                                <&gcc GCC_BOOT_ROM_AHB_CLK>,
> > +                                <&gcc GCC_MSS_GPLL0_DIV_CLK_SRC>,
> > +                                <&gcc GCC_MSS_SNOC_AXI_CLK>,
> > +                                <&gcc GCC_MSS_MFAB_AXIS_CLK>,
> > +                                <&gcc GCC_PRNG_AHB_CLK>,
> > +                                <&rpmhcc RPMH_CXO_CLK>;
> > +                       clock-names = "iface", "bus", "mem", "gpll0_mss",
> > +                                     "snoc_axi", "mnoc_axi", "prng", "xo";
> > +
> > +                       qcom,smem-states = <&modem_smp2p_out 0>;
> > +                       qcom,smem-state-names = "stop";
> > +
> > +                       resets = <&aoss_reset AOSS_CC_MSS_RESTART>,
> > +                                <&pdc_reset PDC_MODEM_SYNC_RESET>;
> > +                       reset-names = "mss_restart", "pdc_reset";
> > +
> > +                       qcom,halt-regs = <&tcsr_mutex_regs 0x23000 0x25000 0x24000>;
> > +
> > +                       power-domains = <&aoss_qmp AOSS_QMP_LS_MODEM>,
> > +                                       <&rpmhpd SDM845_CX>,
> > +                                       <&rpmhpd SDM845_MX>,
> > +                                       <&rpmhpd SDM845_MSS>;
> > +                       power-domain-names = "load_state", "cx", "mx", "mss";
> > +
> > +                       mba {
> > +                               memory-region = <&mba_region>;
> > +                       };
> > +
> > +                       mpss {
> > +                               memory-region = <&mpss_region>;
> > +                       };
> > +
> > +                       glink-edge {
> > +                               interrupts = <GIC_SPI 449 IRQ_TYPE_EDGE_RISING>;
> > +                               label = "modem";
> > +                               qcom,remote-pid = <1>;
> > +                               mboxes = <&apss_shared 12>;
> > +                       };
> > +               };
> > +
> >                 sdhc_2: sdhci@8804000 {
> 
> Can you please sort by unit address now that you have a device tree
> that has more stuff?
> 

Of course, sorry for missing that.

Regards,
Bjorn