mbox series

[v3,00/23] arm64: dts: qcom: Fix remoteproc memory base and length

Message ID 20241213-dts-qcom-cdsp-mpss-base-address-v3-0-2e0036fccd8d@linaro.org
Headers show
Series arm64: dts: qcom: Fix remoteproc memory base and length | expand

Message

Krzysztof Kozlowski Dec. 13, 2024, 2:53 p.m. UTC
Changes in v3:
- Add Rb tags
- Add four new patches (at the end) for sdx75 and sm6115
- Link to v2: https://lore.kernel.org/r/20241209-dts-qcom-cdsp-mpss-base-address-v2-0-d85a3bd5cced@linaro.org

Changes in v2:
- arm64: dts: qcom: x1e80100: Fix ADSP...:
  Commit msg corrections, second paragraph (Johan)
- Add tags
- Link to v1: https://lore.kernel.org/r/20241206-dts-qcom-cdsp-mpss-base-address-v1-0-2f349e4d5a63@linaro.org

Konrad pointed out during SM8750 review, that numbers are odd, so I
looked at datasheets and downstream DTS for all previous platforms.

Most numbers are odd.

Older platforms like SM8150, SM8250, SC7280, SC8180X seem fine. I could
not check few like SDX75 or SM6115, due to lack of access to datasheets.

SM8350, SM8450, SM8550 tested on hardware. Others not, but I don't
expect any failures because PAS drivers do not use the address space.
Which also explains why odd numbers did not result in any failures.

Best regards,
Krzysztof

---
Krzysztof Kozlowski (23):
      arm64: dts: qcom: sm8350: Fix ADSP memory base and length
      arm64: dts: qcom: sm8350: Fix CDSP memory base and length
      arm64: dts: qcom: sm8350: Fix MPSS memory length
      arm64: dts: qcom: sm8450: Fix ADSP memory base and length
      arm64: dts: qcom: sm8450: Fix CDSP memory length
      arm64: dts: qcom: sm8450: Fix MPSS memory length
      arm64: dts: qcom: sm8550: Fix ADSP memory base and length
      arm64: dts: qcom: sm8550: Fix CDSP memory length
      arm64: dts: qcom: sm8550: Fix MPSS memory length
      arm64: dts: qcom: sm8650: Fix ADSP memory base and length
      arm64: dts: qcom: sm8650: Fix CDSP memory length
      arm64: dts: qcom: sm8650: Fix MPSS memory length
      arm64: dts: qcom: x1e80100: Fix ADSP memory base and length
      arm64: dts: qcom: x1e80100: Fix CDSP memory length
      arm64: dts: qcom: sm6350: Fix ADSP memory length
      arm64: dts: qcom: sm6350: Fix MPSS memory length
      [RFT] arm64: dts: qcom: sm6375: Fix ADSP memory length
      [RFT] arm64: dts: qcom: sm6375: Fix CDSP memory base and length
      [RFT] arm64: dts: qcom: sm6375: Fix MPSS memory base and length
      arm64: dts: qcom: sdx75: Fix MPSS memory length
      arm64: dts: qcom: sm6115: Fix MPSS memory length
      arm64: dts: qcom: sm6115: Fix CDSP memory length
      arm64: dts: qcom: sm6115: Fix ADSP memory base and length

 arch/arm64/boot/dts/qcom/sdx75.dtsi    |   2 +-
 arch/arm64/boot/dts/qcom/sm6115.dtsi   |   8 +-
 arch/arm64/boot/dts/qcom/sm6350.dtsi   |   4 +-
 arch/arm64/boot/dts/qcom/sm6375.dtsi   |  10 +-
 arch/arm64/boot/dts/qcom/sm8350.dtsi   | 492 ++++++++++++++++-----------------
 arch/arm64/boot/dts/qcom/sm8450.dtsi   | 216 +++++++--------
 arch/arm64/boot/dts/qcom/sm8550.dtsi   | 266 +++++++++---------
 arch/arm64/boot/dts/qcom/sm8650.dtsi   | 300 ++++++++++----------
 arch/arm64/boot/dts/qcom/x1e80100.dtsi | 276 +++++++++---------
 9 files changed, 787 insertions(+), 787 deletions(-)
---
base-commit: c245a7a79602ccbee780c004c1e4abcda66aec32
change-id: 20241206-dts-qcom-cdsp-mpss-base-address-ae7c406ac9ac

Best regards,

Comments

Konrad Dybcio Dec. 13, 2024, 2:57 p.m. UTC | #1
On 13.12.2024 3:54 PM, Krzysztof Kozlowski wrote:
> The address space in MPSS/Modem PAS (Peripheral Authentication Service)
> remoteproc node should point to the QDSP PUB address space
> (QDSP6...SS_PUB) which has a length of 0x10000.  Value of 0x100 was
> copied from older DTS, but it grew since then.
> 
> This should have no functional impact on Linux users, because PAS loader
> does not use this address space at all.
> 
> Cc: stable@vger.kernel.org
> Fixes: 96ce9227fdbc ("arm64: dts: qcom: sm6115: Add remoteproc nodes")
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> ---

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

Konrad
Konrad Dybcio Dec. 13, 2024, 2:58 p.m. UTC | #2
On 13.12.2024 3:54 PM, Krzysztof Kozlowski wrote:
> The address space in ADSP PAS (Peripheral Authentication Service)
> remoteproc node should point to the QDSP PUB address space
> (QDSP6...SS_PUB): 0x0a40_0000 with length of 0x4040.
> 
> 0x0ab0_0000, value used so far, is the SSC_QUPV3 block, so entierly
> unrelated.
> 
> Correct the base address and length, which should have no functional
> impact on Linux users, because PAS loader does not use this address
> space at all.
> 
> Cc: stable@vger.kernel.org
> Fixes: 96ce9227fdbc ("arm64: dts: qcom: sm6115: Add remoteproc nodes")
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> ---

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

Konrad
Krzysztof Kozlowski Dec. 13, 2024, 3:45 p.m. UTC | #3
On 13/12/2024 16:35, Johan Hovold wrote:
> On Fri, Dec 13, 2024 at 03:54:02PM +0100, Krzysztof Kozlowski wrote:
>> The address space in ADSP PAS (Peripheral Authentication Service)
>> remoteproc node should point to the QDSP PUB address space
>> (QDSP6...SS_PUB): 0x0680_0000 with length of 0x10000.
>>
>> 0x3000_0000, value used so far, is the main region of CDSP and was
>> simply copied from other/older DTS.
>>
>> Correct the base address and length, which also moves the node to
>> different place to keep things sorted by unit address.  The diff looks
>> big, but only the unit address and "reg" property were changed.  This
>> should have no functional impact on Linux users, because PAS loader does
>> not use this address space at all.
>>
>> Fixes: 5f2a9cd4b104 ("arm64: dts: qcom: x1e80100: Add ADSP/CDSP remoteproc nodes")
>> Cc: stable@vger.kernel.org
> 
> Why bother with backporting any of these when there is no functional
> impact?


Not sure, I assumed someone might be using kernel DTS from stable
branches in other projects. Kernel is the source of DTS and stable
kernel has the DTS in both stable and fixed way. If 3rd party project
keeps pulling always latest DTS from latest kernel, they will see so
many ABI breaks and so many incompatibilities (we discussed it in
Vienna) that they will probably curse their approach and say "never
again". Using stable branch DTS could be a solution.

Such 3rd party project might actually use above device nodes in their
drivers. It's just some of Linux kernel drivers which do not use them
(other like PIL seems to use addresses).

Plus DTS is used by 3rd party Linux kernels (out of tree), which while
we do not care in a way of driving our development, but we do consider
them possible users. They also might be relying on stable kernel branch
for this.

Best regards,
Krzysztof
Johan Hovold Dec. 13, 2024, 4:01 p.m. UTC | #4
On Fri, Dec 13, 2024 at 04:45:30PM +0100, Krzysztof Kozlowski wrote:
> On 13/12/2024 16:35, Johan Hovold wrote:
> > On Fri, Dec 13, 2024 at 03:54:02PM +0100, Krzysztof Kozlowski wrote:
> >> The address space in ADSP PAS (Peripheral Authentication Service)
> >> remoteproc node should point to the QDSP PUB address space
> >> (QDSP6...SS_PUB): 0x0680_0000 with length of 0x10000.
> >>
> >> 0x3000_0000, value used so far, is the main region of CDSP and was
> >> simply copied from other/older DTS.
> >>
> >> Correct the base address and length, which also moves the node to
> >> different place to keep things sorted by unit address.  The diff looks
> >> big, but only the unit address and "reg" property were changed.  This
> >> should have no functional impact on Linux users, because PAS loader does
> >> not use this address space at all.
> >>
> >> Fixes: 5f2a9cd4b104 ("arm64: dts: qcom: x1e80100: Add ADSP/CDSP remoteproc nodes")
> >> Cc: stable@vger.kernel.org
> > 
> > Why bother with backporting any of these when there is no functional
> > impact?
> 
> Not sure, I assumed someone might be using kernel DTS from stable
> branches in other projects. Kernel is the source of DTS and stable
> kernel has the DTS in both stable and fixed way. If 3rd party project
> keeps pulling always latest DTS from latest kernel, they will see so
> many ABI breaks and so many incompatibilities (we discussed it in
> Vienna) that they will probably curse their approach and say "never
> again". Using stable branch DTS could be a solution.

That makes some sense.

> Such 3rd party project might actually use above device nodes in their
> drivers. It's just some of Linux kernel drivers which do not use them
> (other like PIL seems to use addresses).

But this is more questionable given that the current addresses are
completely off in this case.

> Plus DTS is used by 3rd party Linux kernels (out of tree), which while
> we do not care in a way of driving our development, but we do consider
> them possible users. They also might be relying on stable kernel branch
> for this.

Same here.

I realise this is a bit of a grey area, but given the size of the diffs
and the no functional impact this could be an opportunity to try to
uphold the stable kernel rules:

	- It cannot be bigger than 100 lines, with context.
	- It must either fix a real bug that bothers people or just add
	  a device ID.

Johan
Konrad Dybcio Dec. 23, 2024, 10:52 a.m. UTC | #5
On 23.12.2024 10:21 AM, Aiqun(Maria) Yu wrote:
> On 12/13/2024 10:53 PM, Krzysztof Kozlowski wrote:
>> Changes in v3:
>> - Add Rb tags
>> - Add four new patches (at the end) for sdx75 and sm6115
>> - Link to v2: https://lore.kernel.org/r/20241209-dts-qcom-cdsp-mpss-base-address-v2-0-d85a3bd5cced@linaro.org
>>
>> Changes in v2:
>> - arm64: dts: qcom: x1e80100: Fix ADSP...:
>>   Commit msg corrections, second paragraph (Johan)
>> - Add tags
>> - Link to v1: https://lore.kernel.org/r/20241206-dts-qcom-cdsp-mpss-base-address-v1-0-2f349e4d5a63@linaro.org
>>
>> Konrad pointed out during SM8750 review, that numbers are odd, so I
>> looked at datasheets and downstream DTS for all previous platforms.
>>
>> Most numbers are odd.
>>
>> Older platforms like SM8150, SM8250, SC7280, SC8180X seem fine. I could
>> not check few like SDX75 or SM6115, due to lack of access to datasheets.
>>
>> SM8350, SM8450, SM8550 tested on hardware. Others not, but I don't
>> expect any failures because PAS drivers do not use the address space.
>> Which also explains why odd numbers did not result in any failures.
> 
> In my opinion, the "QCOM_Q6V5_PAS" based Peripheral Authentication
> platforms may have the register information completely removed.
> 
> There are two types of Peripheral Authentication supported:
>   "QCOM_Q6V5_MSS" (self-authenticating)
>   "QCOM_Q6V5_PAS" (trust-zone based Authentication)
> For "QCOM_Q6V5_PAS" based Peripheral Authentication platforms, use SCM
> calls instead of the register-based mechanism. So it is no need to
> expose the PUB reg addresses for those platforms.

(Unfortunately) not all boards using the same SoC have the same
firmware stack, and it's not obvious that self-authentication is not
useful. Plus having an accurate register space description in the
DT is "nice".

Konrad