| Message ID | 20200705191944.404933-1-matheus@castello.eng.br (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v1] arm64: dts: actions: Fix smp Bringing up secondary CPUs | expand |
Hi Matheus, On 7/5/20 4:19 PM, Matheus Castello wrote: > Change the enable-method to fix the failed to boot errors: > > [ 0.040330] smp: Bringing up secondary CPUs ... > [ 0.040683] psci: failed to boot CPU1 (-22) > [ 0.040691] CPU1: failed to boot: -22 > [ 0.041062] psci: failed to boot CPU2 (-22) > [ 0.041071] CPU2: failed to boot: -22 > [ 0.041408] psci: failed to boot CPU3 (-22) > [ 0.041417] CPU3: failed to boot: -22 > [ 0.041443] smp: Brought up 1 node, 1 CPU > [ 0.041451] SMP: Total of 1 processors activated. > > Tested on Caninos Labrador v3 based on Actions Semi S700. > > Signed-off-by: Matheus Castello <matheus@castello.eng.br> I tested on the Caninos Labrador v3, and I don't get this errors anymore with this patch, and I have all 4 CPUs: [ 0.076961] smp: Brought up 1 node, 4 CPUs [ 0.574290] SMP: Total of 4 processors activated. Full boot log without your patch: http://ix.io/2qYU Full boot log with your patch: http://ix.io/2qYV Tested-by: Helen Koike <helen.koike@collabora.com> Thanks Helen > --- > arch/arm64/boot/dts/actions/s700.dtsi | 33 +++++++++++++++++++-------- > 1 file changed, 23 insertions(+), 10 deletions(-) > > -- > 2.27.0 > > diff --git a/arch/arm64/boot/dts/actions/s700.dtsi b/arch/arm64/boot/dts/actions/s700.dtsi > index 2006ad5424fa..b28dbbcad27b 100644 > --- a/arch/arm64/boot/dts/actions/s700.dtsi > +++ b/arch/arm64/boot/dts/actions/s700.dtsi > @@ -14,37 +14,50 @@ / { > #size-cells = <2>; > > cpus { > - #address-cells = <2>; > + #address-cells = <1>; > #size-cells = <0>; > - > + > cpu0: cpu@0 { > device_type = "cpu"; > compatible = "arm,cortex-a53"; > - reg = <0x0 0x0>; > - enable-method = "psci"; > + reg = <0x0>; > + enable-method = "spin-table"; > + cpu-release-addr = <0 0x1f000020>; > + next-level-cache = <&L2>; > }; > > cpu1: cpu@1 { > device_type = "cpu"; > compatible = "arm,cortex-a53"; > - reg = <0x0 0x1>; > - enable-method = "psci"; > + reg = <0x1>; > + enable-method = "spin-table"; > + cpu-release-addr = <0 0x1f000020>; > + next-level-cache = <&L2>; > }; > > cpu2: cpu@2 { > device_type = "cpu"; > compatible = "arm,cortex-a53"; > - reg = <0x0 0x2>; > - enable-method = "psci"; > + reg = <0x2>; > + enable-method = "spin-table"; > + cpu-release-addr = <0 0x1f000020>; > + next-level-cache = <&L2>; > }; > > cpu3: cpu@3 { > device_type = "cpu"; > compatible = "arm,cortex-a53"; > - reg = <0x0 0x3>; > - enable-method = "psci"; > + reg = <0x3>; > + enable-method = "spin-table"; > + cpu-release-addr = <0 0x1f000020>; > + next-level-cache = <&L2>; > }; > }; > + > + L2: l2-cache { > + compatible = "cache"; > + cache-level = <2>; > + }; > > reserved-memory { > #address-cells = <2>; >
Hi Matheus, Am 05.07.20 um 21:19 schrieb Matheus Castello: > Change the enable-method to fix the failed to boot errors: > > [ 0.040330] smp: Bringing up secondary CPUs ... > [ 0.040683] psci: failed to boot CPU1 (-22) > [ 0.040691] CPU1: failed to boot: -22 > [ 0.041062] psci: failed to boot CPU2 (-22) > [ 0.041071] CPU2: failed to boot: -22 > [ 0.041408] psci: failed to boot CPU3 (-22) > [ 0.041417] CPU3: failed to boot: -22 > [ 0.041443] smp: Brought up 1 node, 1 CPU > [ 0.041451] SMP: Total of 1 processors activated. > > Tested on Caninos Labrador v3 based on Actions Semi S700. > > Signed-off-by: Matheus Castello <matheus@castello.eng.br> > --- > arch/arm64/boot/dts/actions/s700.dtsi | 33 +++++++++++++++++++-------- > 1 file changed, 23 insertions(+), 10 deletions(-) NACK. For starters, if this were an actual fix, it should have a Fixes header. Don't do random changes in a single patch and call it a "fix". I don't see what changing the cell size has to do with smp, nor adding L2 cache. The latter could be a patch of its own, after fixes are applied (to avoid conflicts when backporting that fix to older branches). A cell size of two used to be perfectly valid, please checking the DT binding. Finally, you're changing generic S700 code here - you can't just break Cubieboard7 just because your Labrador has too old BL31 firmware. Can't you just update its TF-A firmware and use the standard PSCI interface for Linux? If not, then you should add your workaround to your module's/board's .dts(i) instead of the SoC's .dtsi. Thanks, Andreas
Hi Andreas, Em 7/5/20 7:09 PM, Andreas Färber escreveu: > Hi Matheus, > > Am 05.07.20 um 21:19 schrieb Matheus Castello: >> Change the enable-method to fix the failed to boot errors: >> >> [ 0.040330] smp: Bringing up secondary CPUs ... >> [ 0.040683] psci: failed to boot CPU1 (-22) >> [ 0.040691] CPU1: failed to boot: -22 >> [ 0.041062] psci: failed to boot CPU2 (-22) >> [ 0.041071] CPU2: failed to boot: -22 >> [ 0.041408] psci: failed to boot CPU3 (-22) >> [ 0.041417] CPU3: failed to boot: -22 >> [ 0.041443] smp: Brought up 1 node, 1 CPU >> [ 0.041451] SMP: Total of 1 processors activated. >> >> Tested on Caninos Labrador v3 based on Actions Semi S700. >> >> Signed-off-by: Matheus Castello <matheus@castello.eng.br> >> --- >> arch/arm64/boot/dts/actions/s700.dtsi | 33 +++++++++++++++++++-------- >> 1 file changed, 23 insertions(+), 10 deletions(-) > > NACK. > > For starters, if this were an actual fix, it should have a Fixes header. > > Don't do random changes in a single patch and call it a "fix". I don't > see what changing the cell size has to do with smp, nor adding L2 cache. > The latter could be a patch of its own, after fixes are applied (to > avoid conflicts when backporting that fix to older branches). A cell > size of two used to be perfectly valid, please checking the DT binding. > > Finally, you're changing generic S700 code here - you can't just break > Cubieboard7 just because your Labrador has too old BL31 firmware. Can't Sorry for that, got it. > you just update its TF-A firmware and use the standard PSCI interface > for Linux? If not, then you should add your workaround to your > module's/board's .dts(i) instead of the SoC's .dtsi. > Yes, the vendor does not seem to support this for now. I think for now the best thing to do is to leave the workaround on the module's .dtsi. Thank you for the tips. > Thanks, > Andreas > BR, Matheus Castello
diff --git a/arch/arm64/boot/dts/actions/s700.dtsi b/arch/arm64/boot/dts/actions/s700.dtsi index 2006ad5424fa..b28dbbcad27b 100644 --- a/arch/arm64/boot/dts/actions/s700.dtsi +++ b/arch/arm64/boot/dts/actions/s700.dtsi @@ -14,37 +14,50 @@ / { #size-cells = <2>; cpus { - #address-cells = <2>; + #address-cells = <1>; #size-cells = <0>; - + cpu0: cpu@0 { device_type = "cpu"; compatible = "arm,cortex-a53"; - reg = <0x0 0x0>; - enable-method = "psci"; + reg = <0x0>; + enable-method = "spin-table"; + cpu-release-addr = <0 0x1f000020>; + next-level-cache = <&L2>; }; cpu1: cpu@1 { device_type = "cpu"; compatible = "arm,cortex-a53"; - reg = <0x0 0x1>; - enable-method = "psci"; + reg = <0x1>; + enable-method = "spin-table"; + cpu-release-addr = <0 0x1f000020>; + next-level-cache = <&L2>; }; cpu2: cpu@2 { device_type = "cpu"; compatible = "arm,cortex-a53"; - reg = <0x0 0x2>; - enable-method = "psci"; + reg = <0x2>; + enable-method = "spin-table"; + cpu-release-addr = <0 0x1f000020>; + next-level-cache = <&L2>; }; cpu3: cpu@3 { device_type = "cpu"; compatible = "arm,cortex-a53"; - reg = <0x0 0x3>; - enable-method = "psci"; + reg = <0x3>; + enable-method = "spin-table"; + cpu-release-addr = <0 0x1f000020>; + next-level-cache = <&L2>; }; }; + + L2: l2-cache { + compatible = "cache"; + cache-level = <2>; + }; reserved-memory { #address-cells = <2>;
Change the enable-method to fix the failed to boot errors: [ 0.040330] smp: Bringing up secondary CPUs ... [ 0.040683] psci: failed to boot CPU1 (-22) [ 0.040691] CPU1: failed to boot: -22 [ 0.041062] psci: failed to boot CPU2 (-22) [ 0.041071] CPU2: failed to boot: -22 [ 0.041408] psci: failed to boot CPU3 (-22) [ 0.041417] CPU3: failed to boot: -22 [ 0.041443] smp: Brought up 1 node, 1 CPU [ 0.041451] SMP: Total of 1 processors activated. Tested on Caninos Labrador v3 based on Actions Semi S700. Signed-off-by: Matheus Castello <matheus@castello.eng.br> --- arch/arm64/boot/dts/actions/s700.dtsi | 33 +++++++++++++++++++-------- 1 file changed, 23 insertions(+), 10 deletions(-) -- 2.27.0