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 |
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
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
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
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
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