2023-02-03 17:05:32

by Mukesh Ojha

[permalink] [raw]
Subject: [PATCH 1/2] dt-bindings: sram: qcom,imem: document sm8450

Add compatible for sm8450 IMEM.

Signed-off-by: Mukesh Ojha <[email protected]>
---
Documentation/devicetree/bindings/sram/qcom,imem.yaml | 1 +
1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/sram/qcom,imem.yaml b/Documentation/devicetree/bindings/sram/qcom,imem.yaml
index 665c06e..ba694ce 100644
--- a/Documentation/devicetree/bindings/sram/qcom,imem.yaml
+++ b/Documentation/devicetree/bindings/sram/qcom,imem.yaml
@@ -26,6 +26,7 @@ properties:
- qcom,sdm845-imem
- qcom,sdx55-imem
- qcom,sdx65-imem
+ - qcom,sm8450-imem
- const: syscon
- const: simple-mfd

--
2.7.4



2023-02-03 17:05:37

by Mukesh Ojha

[permalink] [raw]
Subject: [PATCH 2/2] arm64: dts: qcom: sm8450: Add IMEM and PIL info region

Add a simple-mfd representing IMEM on SM8450 and define the PIL
relocation info region, so that post mortem tools will be able
to locate the loaded remoteprocs.

Signed-off-by: Mukesh Ojha <[email protected]>
---
arch/arm64/boot/dts/qcom/sm8450.dtsi | 15 +++++++++++++++
1 file changed, 15 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
index 5704750..474ea1b 100644
--- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
@@ -3536,6 +3536,21 @@
};
};

+ sram@146aa000 {
+ compatible = "qcom,sm8450-imem", "syscon", "simple-mfd";
+ reg = <0 0x146aa000 0 0x1000>;
+
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ ranges = <0 0 0x146aa000 0x1000>;
+
+ pil-reloc@94c {
+ compatible = "qcom,pil-reloc-info";
+ reg = <0x94c 0xc8>;
+ };
+ };
+
apps_rsc: rsc@17a00000 {
label = "apps_rsc";
compatible = "qcom,rpmh-rsc";
--
2.7.4


2023-02-03 17:51:46

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 2/2] arm64: dts: qcom: sm8450: Add IMEM and PIL info region

On 03/02/2023 18:04, Mukesh Ojha wrote:
> Add a simple-mfd representing IMEM on SM8450 and define the PIL
> relocation info region, so that post mortem tools will be able
> to locate the loaded remoteprocs.
>
> Signed-off-by: Mukesh Ojha <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/sm8450.dtsi | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> index 5704750..474ea1b 100644
> --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> @@ -3536,6 +3536,21 @@
> };
> };
>
> + sram@146aa000 {

I don't think it's correct location - sorted.

> + compatible = "qcom,sm8450-imem", "syscon", "simple-mfd";
> + reg = <0 0x146aa000 0 0x1000>;
> +
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + ranges = <0 0 0x146aa000 0x1000>;

Put ranges after reg.


Best regards,
Krzysztof


2023-02-03 17:52:37

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: sram: qcom,imem: document sm8450

On 03/02/2023 18:04, Mukesh Ojha wrote:
> Add compatible for sm8450 IMEM.
>
> Signed-off-by: Mukesh Ojha <[email protected]>
> ---


Acked-by: Krzysztof Kozlowski <[email protected]>

Best regards,
Krzysztof


2023-02-03 21:37:55

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH 2/2] arm64: dts: qcom: sm8450: Add IMEM and PIL info region



On 3.02.2023 18:04, Mukesh Ojha wrote:
> Add a simple-mfd representing IMEM on SM8450 and define the PIL
> relocation info region, so that post mortem tools will be able
> to locate the loaded remoteprocs.
>
> Signed-off-by: Mukesh Ojha <[email protected]>
> ---
From XBL:

0x14680000, 0x0002A000, "IMEM Base"

Is there anything in that wider address range that would interest
us? I recall Alex once dug into that when diving into IPA, but
I can not recall the conclusion..

Konrad
> arch/arm64/boot/dts/qcom/sm8450.dtsi | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> index 5704750..474ea1b 100644
> --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> @@ -3536,6 +3536,21 @@
> };
> };
>
> + sram@146aa000 {
> + compatible = "qcom,sm8450-imem", "syscon", "simple-mfd";
> + reg = <0 0x146aa000 0 0x1000>;
> +
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + ranges = <0 0 0x146aa000 0x1000>;
> +
> + pil-reloc@94c {
> + compatible = "qcom,pil-reloc-info";
> + reg = <0x94c 0xc8>;
> + };
> + };
> +
> apps_rsc: rsc@17a00000 {
> label = "apps_rsc";
> compatible = "qcom,rpmh-rsc";

2023-02-06 14:36:31

by Mukesh Ojha

[permalink] [raw]
Subject: Re: [PATCH 2/2] arm64: dts: qcom: sm8450: Add IMEM and PIL info region



On 2/4/2023 3:07 AM, Konrad Dybcio wrote:
>
>
> On 3.02.2023 18:04, Mukesh Ojha wrote:
>> Add a simple-mfd representing IMEM on SM8450 and define the PIL
>> relocation info region, so that post mortem tools will be able
>> to locate the loaded remoteprocs.
>>
>> Signed-off-by: Mukesh Ojha <[email protected]>
>> ---
> From XBL:
>
> 0x14680000, 0x0002A000, "IMEM Base"
>
> Is there anything in that wider address range that would interest
> us? I recall Alex once dug into that when diving into IPA, but
> I can not recall the conclusion..
Spec-wise, yes IPA do own these 0x146A8000 - 0x146AA000 .
But, not sure what they use it for.

-Mukesh
>
> Konrad
>> arch/arm64/boot/dts/qcom/sm8450.dtsi | 15 +++++++++++++++
>> 1 file changed, 15 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
>> index 5704750..474ea1b 100644
>> --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
>> @@ -3536,6 +3536,21 @@
>> };
>> };
>>
>> + sram@146aa000 {
>> + compatible = "qcom,sm8450-imem", "syscon", "simple-mfd";
>> + reg = <0 0x146aa000 0 0x1000>;
>> +
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> +
>> + ranges = <0 0 0x146aa000 0x1000>;
>> +
>> + pil-reloc@94c {
>> + compatible = "qcom,pil-reloc-info";
>> + reg = <0x94c 0xc8>;
>> + };
>> + };
>> +
>> apps_rsc: rsc@17a00000 {
>> label = "apps_rsc";
>> compatible = "qcom,rpmh-rsc";

2023-02-06 21:03:00

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH 2/2] arm64: dts: qcom: sm8450: Add IMEM and PIL info region

On Mon, Feb 06, 2023 at 08:06:13PM +0530, Mukesh Ojha wrote:
>
>
> On 2/4/2023 3:07 AM, Konrad Dybcio wrote:
> >
> >
> > On 3.02.2023 18:04, Mukesh Ojha wrote:
> > > Add a simple-mfd representing IMEM on SM8450 and define the PIL
> > > relocation info region, so that post mortem tools will be able
> > > to locate the loaded remoteprocs.
> > >
> > > Signed-off-by: Mukesh Ojha <[email protected]>
> > > ---
> > From XBL:
> >
> > 0x14680000, 0x0002A000, "IMEM Base"
> >
> > Is there anything in that wider address range that would interest
> > us? I recall Alex once dug into that when diving into IPA, but
> > I can not recall the conclusion..
> Spec-wise, yes IPA do own these 0x146A8000 - 0x146AA000 .
> But, not sure what they use it for.
>

The DT should not reflect the organization structure. Let's see if Alex
have any input on this.

Thanks,
Bjorn

> -Mukesh
> >
> > Konrad
> > > arch/arm64/boot/dts/qcom/sm8450.dtsi | 15 +++++++++++++++
> > > 1 file changed, 15 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> > > index 5704750..474ea1b 100644
> > > --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> > > @@ -3536,6 +3536,21 @@
> > > };
> > > };
> > > + sram@146aa000 {
> > > + compatible = "qcom,sm8450-imem", "syscon", "simple-mfd";
> > > + reg = <0 0x146aa000 0 0x1000>;
> > > +
> > > + #address-cells = <1>;
> > > + #size-cells = <1>;
> > > +
> > > + ranges = <0 0 0x146aa000 0x1000>;
> > > +
> > > + pil-reloc@94c {
> > > + compatible = "qcom,pil-reloc-info";
> > > + reg = <0x94c 0xc8>;
> > > + };
> > > + };
> > > +
> > > apps_rsc: rsc@17a00000 {
> > > label = "apps_rsc";
> > > compatible = "qcom,rpmh-rsc";

2023-02-14 12:35:03

by Mukesh Ojha

[permalink] [raw]
Subject: Re: [PATCH 2/2] arm64: dts: qcom: sm8450: Add IMEM and PIL info region

+@alex,

Please comment.

-Mukesh

On 2/7/2023 2:34 AM, Bjorn Andersson wrote:
> On Mon, Feb 06, 2023 at 08:06:13PM +0530, Mukesh Ojha wrote:
>>
>>
>> On 2/4/2023 3:07 AM, Konrad Dybcio wrote:
>>>
>>>
>>> On 3.02.2023 18:04, Mukesh Ojha wrote:
>>>> Add a simple-mfd representing IMEM on SM8450 and define the PIL
>>>> relocation info region, so that post mortem tools will be able
>>>> to locate the loaded remoteprocs.
>>>>
>>>> Signed-off-by: Mukesh Ojha <[email protected]>
>>>> ---
>>> From XBL:
>>>
>>> 0x14680000, 0x0002A000, "IMEM Base"
>>>
>>> Is there anything in that wider address range that would interest
>>> us? I recall Alex once dug into that when diving into IPA, but
>>> I can not recall the conclusion..
>> Spec-wise, yes IPA do own these 0x146A8000 - 0x146AA000 .
>> But, not sure what they use it for.
>>
>
> The DT should not reflect the organization structure. Let's see if Alex
> have any input on this.
>
> Thanks,
> Bjorn
>
>> -Mukesh
>>>
>>> Konrad
>>>> arch/arm64/boot/dts/qcom/sm8450.dtsi | 15 +++++++++++++++
>>>> 1 file changed, 15 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
>>>> index 5704750..474ea1b 100644
>>>> --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
>>>> +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
>>>> @@ -3536,6 +3536,21 @@
>>>> };
>>>> };
>>>> + sram@146aa000 {
>>>> + compatible = "qcom,sm8450-imem", "syscon", "simple-mfd";
>>>> + reg = <0 0x146aa000 0 0x1000>;
>>>> +
>>>> + #address-cells = <1>;
>>>> + #size-cells = <1>;
>>>> +
>>>> + ranges = <0 0 0x146aa000 0x1000>;
>>>> +
>>>> + pil-reloc@94c {
>>>> + compatible = "qcom,pil-reloc-info";
>>>> + reg = <0x94c 0xc8>;
>>>> + };
>>>> + };
>>>> +
>>>> apps_rsc: rsc@17a00000 {
>>>> label = "apps_rsc";
>>>> compatible = "qcom,rpmh-rsc";

2023-02-14 14:28:53

by Alex Elder

[permalink] [raw]
Subject: Re: [PATCH 2/2] arm64: dts: qcom: sm8450: Add IMEM and PIL info region

On 2/14/23 6:34 AM, Mukesh Ojha wrote:
> +@alex,
>
> Please comment.
>
> -Mukesh
>
> On 2/7/2023 2:34 AM, Bjorn Andersson wrote:
>> On Mon, Feb 06, 2023 at 08:06:13PM +0530, Mukesh Ojha wrote:
>>>
>>>
>>> On 2/4/2023 3:07 AM, Konrad Dybcio wrote:
>>>>
>>>>
>>>> On 3.02.2023 18:04, Mukesh Ojha wrote:
>>>>> Add a simple-mfd representing IMEM on SM8450 and define the PIL
>>>>> relocation info region, so that post mortem tools will be able
>>>>> to locate the loaded remoteprocs.
>>>>>
>>>>> Signed-off-by: Mukesh Ojha <[email protected]>
>>>>> ---
>>>>   From XBL:
>>>>
>>>> 0x14680000, 0x0002A000, "IMEM Base"
>>>>
>>>> Is there anything in that wider address range that would interest
>>>> us? I recall Alex once dug into that when diving into IPA, but
>>>> I can not recall the conclusion..

So you're saying a boot loader defines "IMEM Base" as
a range of memory, size 0x2a000.

>>> Spec-wise, yes IPA do own these 0x146A8000 - 0x146AA000 .
>>> But, not sure what they use it for.

And a subset of that range (at offset 0x28000, size 0x2000) is
set aside for use by IPA. IPA *does* use this memory area as
"fast" memory to hold certain frequently-accessed tables.
Details of that are under the modem's control, and I don't
have knowledge of that.

The IPA driver currently maps it's portion of this memory,
but does *not* reflect that use in DT. (This is something
that will be corrected at some point, possibly soon.)


>> The DT should not reflect the organization structure. Let's see if Alex
>> have any input on this.

As I understand it there's an "imem" node in DT defined, but
it just defines the range of memory, and is something referred
to by other drivers (or other nodes in a DTS file). This seems
reasonable, and it kind of matches the view I've seen in
documents. IPA does *not* follow that pattern, and should.

I don't really have any strong input either way. I can help
define the "problem" but I don't claim to know the correct
way to represent this.

It's a contiguous block of special memory. By convention
(design) its range divided up, with portions intended to be
used for different purposes, by different hardware. IPA is
one of the users.

-Alex


>>
>> Thanks,
>> Bjorn
>>
>>> -Mukesh
>>>>
>>>> Konrad
>>>>>    arch/arm64/boot/dts/qcom/sm8450.dtsi | 15 +++++++++++++++
>>>>>    1 file changed, 15 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi
>>>>> b/arch/arm64/boot/dts/qcom/sm8450.dtsi
>>>>> index 5704750..474ea1b 100644
>>>>> --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
>>>>> +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
>>>>> @@ -3536,6 +3536,21 @@
>>>>>                };
>>>>>            };
>>>>> +        sram@146aa000 {
>>>>> +            compatible = "qcom,sm8450-imem", "syscon", "simple-mfd";
>>>>> +            reg = <0 0x146aa000 0 0x1000>;
>>>>> +
>>>>> +            #address-cells = <1>;
>>>>> +            #size-cells = <1>;
>>>>> +
>>>>> +            ranges = <0 0 0x146aa000 0x1000>;
>>>>> +
>>>>> +            pil-reloc@94c {
>>>>> +                compatible = "qcom,pil-reloc-info";
>>>>> +                reg = <0x94c 0xc8>;
>>>>> +            };
>>>>> +        };
>>>>> +
>>>>>            apps_rsc: rsc@17a00000 {
>>>>>                label = "apps_rsc";
>>>>>                compatible = "qcom,rpmh-rsc";