| Message ID | 20250403-dt-cpu-schema-v1-6-076be7171a85@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Arm cpu schema clean-ups | expand |
On Thu, Apr 03, 2025 at 09:59:27PM -0500, Rob Herring (Arm) wrote: > The "qcom,acc" and "qcom,saw" properties aren't valid with "spin-table" > enable-method nor are they used on 64-bit kernels, so they can be > dropped. > The bootloader we currently use on these devices reads these properties to set up the spin-table, so removing these will break booting secondary CPU cores. The motivation for implementing it that way was that 32-bit vs 64-bit kernel shouldn't be relevant for the describing the hardware blocks in the device tree. The code in the bootloader is generic and handles different SoCs (e.g. msm8916 with 4 cores and msm8939 with 8 cores, the enable sequences are identical). Can we keep this in somehow? To be fair, I'm not sure what property we could match on to check if these properties are allowed ... Thanks, Stephan > The "spin-table" enable-method requires "cpu-release-addr" property, > so add a dummy entry. It is assumed the bootloader will fill in the > correct values. > > Signed-off-by: Rob Herring (Arm) <robh@kernel.org> > --- > arch/arm64/boot/dts/qcom/msm8939.dtsi | 24 ++++++++---------------- > 1 file changed, 8 insertions(+), 16 deletions(-) > > diff --git a/arch/arm64/boot/dts/qcom/msm8939.dtsi b/arch/arm64/boot/dts/qcom/msm8939.dtsi > index 7cd5660de1b3..36f2ba3fb81c 100644 > --- a/arch/arm64/boot/dts/qcom/msm8939.dtsi > +++ b/arch/arm64/boot/dts/qcom/msm8939.dtsi > @@ -46,10 +46,9 @@ cpu0: cpu@100 { > compatible = "arm,cortex-a53"; > device_type = "cpu"; > enable-method = "spin-table"; > + cpu-release-addr = /bits/ 64 <0>; > reg = <0x100>; > next-level-cache = <&l2_1>; > - qcom,acc = <&acc0>; > - qcom,saw = <&saw0>; > cpu-idle-states = <&cpu_sleep_0>; > clocks = <&apcs1_mbox>; > #cooling-cells = <2>; > @@ -64,10 +63,9 @@ cpu1: cpu@101 { > compatible = "arm,cortex-a53"; > device_type = "cpu"; > enable-method = "spin-table"; > + cpu-release-addr = /bits/ 64 <0>; > reg = <0x101>; > next-level-cache = <&l2_1>; > - qcom,acc = <&acc1>; > - qcom,saw = <&saw1>; > cpu-idle-states = <&cpu_sleep_0>; > clocks = <&apcs1_mbox>; > #cooling-cells = <2>; > @@ -77,10 +75,9 @@ cpu2: cpu@102 { > compatible = "arm,cortex-a53"; > device_type = "cpu"; > enable-method = "spin-table"; > + cpu-release-addr = /bits/ 64 <0>; > reg = <0x102>; > next-level-cache = <&l2_1>; > - qcom,acc = <&acc2>; > - qcom,saw = <&saw2>; > cpu-idle-states = <&cpu_sleep_0>; > clocks = <&apcs1_mbox>; > #cooling-cells = <2>; > @@ -90,10 +87,9 @@ cpu3: cpu@103 { > compatible = "arm,cortex-a53"; > device_type = "cpu"; > enable-method = "spin-table"; > + cpu-release-addr = /bits/ 64 <0>; > reg = <0x103>; > next-level-cache = <&l2_1>; > - qcom,acc = <&acc3>; > - qcom,saw = <&saw3>; > cpu-idle-states = <&cpu_sleep_0>; > clocks = <&apcs1_mbox>; > #cooling-cells = <2>; > @@ -103,9 +99,8 @@ cpu4: cpu@0 { > compatible = "arm,cortex-a53"; > device_type = "cpu"; > enable-method = "spin-table"; > + cpu-release-addr = /bits/ 64 <0>; > reg = <0x0>; > - qcom,acc = <&acc4>; > - qcom,saw = <&saw4>; > cpu-idle-states = <&cpu_sleep_0>; > clocks = <&apcs0_mbox>; > #cooling-cells = <2>; > @@ -121,10 +116,9 @@ cpu5: cpu@1 { > compatible = "arm,cortex-a53"; > device_type = "cpu"; > enable-method = "spin-table"; > + cpu-release-addr = /bits/ 64 <0>; > reg = <0x1>; > next-level-cache = <&l2_0>; > - qcom,acc = <&acc5>; > - qcom,saw = <&saw5>; > cpu-idle-states = <&cpu_sleep_0>; > clocks = <&apcs0_mbox>; > #cooling-cells = <2>; > @@ -134,10 +128,9 @@ cpu6: cpu@2 { > compatible = "arm,cortex-a53"; > device_type = "cpu"; > enable-method = "spin-table"; > + cpu-release-addr = /bits/ 64 <0>; > reg = <0x2>; > next-level-cache = <&l2_0>; > - qcom,acc = <&acc6>; > - qcom,saw = <&saw6>; > cpu-idle-states = <&cpu_sleep_0>; > clocks = <&apcs0_mbox>; > #cooling-cells = <2>; > @@ -147,10 +140,9 @@ cpu7: cpu@3 { > compatible = "arm,cortex-a53"; > device_type = "cpu"; > enable-method = "spin-table"; > + cpu-release-addr = /bits/ 64 <0>; > reg = <0x3>; > next-level-cache = <&l2_0>; > - qcom,acc = <&acc7>; > - qcom,saw = <&saw7>; > cpu-idle-states = <&cpu_sleep_0>; > clocks = <&apcs0_mbox>; > #cooling-cells = <2>; > > -- > 2.47.2 >
On Fri, Apr 4, 2025 at 7:04 AM Stephan Gerhold <stephan.gerhold@linaro.org> wrote: > > On Thu, Apr 03, 2025 at 09:59:27PM -0500, Rob Herring (Arm) wrote: > > The "qcom,acc" and "qcom,saw" properties aren't valid with "spin-table" > > enable-method nor are they used on 64-bit kernels, so they can be > > dropped. > > > > The bootloader we currently use on these devices reads these properties > to set up the spin-table, so removing these will break booting secondary > CPU cores. > > The motivation for implementing it that way was that 32-bit vs 64-bit > kernel shouldn't be relevant for the describing the hardware blocks in > the device tree. The code in the bootloader is generic and handles > different SoCs (e.g. msm8916 with 4 cores and msm8939 with 8 cores, the > enable sequences are identical). > > Can we keep this in somehow? To be fair, I'm not sure what property we > could match on to check if these properties are allowed ... Yes, we can keep them. We'll have to allow them with "spin-table" and "psci" I guess. Rob
On Fri, Apr 4, 2025 at 8:18 AM Rob Herring <robh@kernel.org> wrote: > > On Fri, Apr 4, 2025 at 7:04 AM Stephan Gerhold > <stephan.gerhold@linaro.org> wrote: > > > > On Thu, Apr 03, 2025 at 09:59:27PM -0500, Rob Herring (Arm) wrote: > > > The "qcom,acc" and "qcom,saw" properties aren't valid with "spin-table" > > > enable-method nor are they used on 64-bit kernels, so they can be > > > dropped. > > > > > > > The bootloader we currently use on these devices reads these properties > > to set up the spin-table, so removing these will break booting secondary > > CPU cores. > > > > The motivation for implementing it that way was that 32-bit vs 64-bit > > kernel shouldn't be relevant for the describing the hardware blocks in > > the device tree. The code in the bootloader is generic and handles > > different SoCs (e.g. msm8916 with 4 cores and msm8939 with 8 cores, the > > enable sequences are identical). > > > > Can we keep this in somehow? To be fair, I'm not sure what property we > > could match on to check if these properties are allowed ... > > Yes, we can keep them. We'll have to allow them with "spin-table" and > "psci" I guess. I came up with something like this instead: - if: # 2 Qualcomm platforms bootloaders need qcom,acc and qcom,saw yet use # "spin-table" or "psci" enable-methods. Disallowing the properties for # all other CPUs is the best we can do as there's not any way to # distinguish these Qualcomm platforms. properties: compatible: not: const: arm,cortex-a53 then: properties: qcom,acc: false qcom,saw: false Not ideal as there are lots of A53s, but better than enabling for every 64-bit platform. Also, is adding the cpu-release-addr fine? It could trip up a bootloader tests if the property is already present and doesn't update it in that case. Rob
diff --git a/arch/arm64/boot/dts/qcom/msm8939.dtsi b/arch/arm64/boot/dts/qcom/msm8939.dtsi index 7cd5660de1b3..36f2ba3fb81c 100644 --- a/arch/arm64/boot/dts/qcom/msm8939.dtsi +++ b/arch/arm64/boot/dts/qcom/msm8939.dtsi @@ -46,10 +46,9 @@ cpu0: cpu@100 { compatible = "arm,cortex-a53"; device_type = "cpu"; enable-method = "spin-table"; + cpu-release-addr = /bits/ 64 <0>; reg = <0x100>; next-level-cache = <&l2_1>; - qcom,acc = <&acc0>; - qcom,saw = <&saw0>; cpu-idle-states = <&cpu_sleep_0>; clocks = <&apcs1_mbox>; #cooling-cells = <2>; @@ -64,10 +63,9 @@ cpu1: cpu@101 { compatible = "arm,cortex-a53"; device_type = "cpu"; enable-method = "spin-table"; + cpu-release-addr = /bits/ 64 <0>; reg = <0x101>; next-level-cache = <&l2_1>; - qcom,acc = <&acc1>; - qcom,saw = <&saw1>; cpu-idle-states = <&cpu_sleep_0>; clocks = <&apcs1_mbox>; #cooling-cells = <2>; @@ -77,10 +75,9 @@ cpu2: cpu@102 { compatible = "arm,cortex-a53"; device_type = "cpu"; enable-method = "spin-table"; + cpu-release-addr = /bits/ 64 <0>; reg = <0x102>; next-level-cache = <&l2_1>; - qcom,acc = <&acc2>; - qcom,saw = <&saw2>; cpu-idle-states = <&cpu_sleep_0>; clocks = <&apcs1_mbox>; #cooling-cells = <2>; @@ -90,10 +87,9 @@ cpu3: cpu@103 { compatible = "arm,cortex-a53"; device_type = "cpu"; enable-method = "spin-table"; + cpu-release-addr = /bits/ 64 <0>; reg = <0x103>; next-level-cache = <&l2_1>; - qcom,acc = <&acc3>; - qcom,saw = <&saw3>; cpu-idle-states = <&cpu_sleep_0>; clocks = <&apcs1_mbox>; #cooling-cells = <2>; @@ -103,9 +99,8 @@ cpu4: cpu@0 { compatible = "arm,cortex-a53"; device_type = "cpu"; enable-method = "spin-table"; + cpu-release-addr = /bits/ 64 <0>; reg = <0x0>; - qcom,acc = <&acc4>; - qcom,saw = <&saw4>; cpu-idle-states = <&cpu_sleep_0>; clocks = <&apcs0_mbox>; #cooling-cells = <2>; @@ -121,10 +116,9 @@ cpu5: cpu@1 { compatible = "arm,cortex-a53"; device_type = "cpu"; enable-method = "spin-table"; + cpu-release-addr = /bits/ 64 <0>; reg = <0x1>; next-level-cache = <&l2_0>; - qcom,acc = <&acc5>; - qcom,saw = <&saw5>; cpu-idle-states = <&cpu_sleep_0>; clocks = <&apcs0_mbox>; #cooling-cells = <2>; @@ -134,10 +128,9 @@ cpu6: cpu@2 { compatible = "arm,cortex-a53"; device_type = "cpu"; enable-method = "spin-table"; + cpu-release-addr = /bits/ 64 <0>; reg = <0x2>; next-level-cache = <&l2_0>; - qcom,acc = <&acc6>; - qcom,saw = <&saw6>; cpu-idle-states = <&cpu_sleep_0>; clocks = <&apcs0_mbox>; #cooling-cells = <2>; @@ -147,10 +140,9 @@ cpu7: cpu@3 { compatible = "arm,cortex-a53"; device_type = "cpu"; enable-method = "spin-table"; + cpu-release-addr = /bits/ 64 <0>; reg = <0x3>; next-level-cache = <&l2_0>; - qcom,acc = <&acc7>; - qcom,saw = <&saw7>; cpu-idle-states = <&cpu_sleep_0>; clocks = <&apcs0_mbox>; #cooling-cells = <2>;
The "qcom,acc" and "qcom,saw" properties aren't valid with "spin-table" enable-method nor are they used on 64-bit kernels, so they can be dropped. The "spin-table" enable-method requires "cpu-release-addr" property, so add a dummy entry. It is assumed the bootloader will fill in the correct values. Signed-off-by: Rob Herring (Arm) <robh@kernel.org> --- arch/arm64/boot/dts/qcom/msm8939.dtsi | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-)