Re: [PATCH v2 5/5] arm64: dts: Add Unisoc's SC9863A SoC support

[Chunyan Zhang]

On Tue, 3 Dec 2019 at 00:33, Rob Herring <robh@xxxxxxxxxx> wrote:
>
> On Mon, Nov 25, 2019 at 2:34 AM Chunyan Zhang <zhang.lyra@xxxxxxxxx> wrote:
> >
> > On Fri, 15 Nov 2019 at 22:43, Rob Herring <robh@xxxxxxxxxx> wrote:
> > >
> > > On Fri, Nov 15, 2019 at 2:59 AM Chunyan Zhang <zhang.lyra@xxxxxxxxx> wrote:
> > > >
> > > > On Fri, 15 Nov 2019 at 05:05, Rob Herring <robh@xxxxxxxxxx> wrote:
> > > > >
> > > > > On Mon, Nov 11, 2019 at 05:02:30PM +0800, Chunyan Zhang wrote:
> > > > > >
> > > > > > Add basic DT to support Unisoc's SC9863A, with this patch,
> > > > > > the board sp9863a-1h10 can run into console.
> > > > > >
> > > > > > Signed-off-by: Chunyan Zhang <chunyan.zhang@xxxxxxxxxx>
> > > > > > ---
> > > > > >  arch/arm64/boot/dts/sprd/Makefile         |   3 +-
> > > > > >  arch/arm64/boot/dts/sprd/sc9863a.dtsi     | 536 ++++++++++++++++++++++
> > > > > >  arch/arm64/boot/dts/sprd/sharkl3.dtsi     | 188 ++++++++
> > > > > >  arch/arm64/boot/dts/sprd/sp9863a-1h10.dts |  40 ++
> > > > > >  4 files changed, 766 insertions(+), 1 deletion(-)
> > > > > >  create mode 100644 arch/arm64/boot/dts/sprd/sc9863a.dtsi
> > > > > >  create mode 100644 arch/arm64/boot/dts/sprd/sharkl3.dtsi
> > > > > >  create mode 100644 arch/arm64/boot/dts/sprd/sp9863a-1h10.dts
> > > > > >
> > > > > > diff --git a/arch/arm64/boot/dts/sprd/Makefile b/arch/arm64/boot/dts/sprd/Makefile
> > > > > > index 2bdc23804f40..f4f1f5148cc2 100644
> > > > > > --- a/arch/arm64/boot/dts/sprd/Makefile
> > > > > > +++ b/arch/arm64/boot/dts/sprd/Makefile
> > > > > > @@ -1,3 +1,4 @@
> > > > > >  # SPDX-License-Identifier: GPL-2.0
> > > > > >  dtb-$(CONFIG_ARCH_SPRD) += sc9836-openphone.dtb \
> > > > > > -                     sp9860g-1h10.dtb
> > > > > > +                     sp9860g-1h10.dtb        \
> > > > > > +                     sp9863a-1h10.dtb
> > > > > > diff --git a/arch/arm64/boot/dts/sprd/sc9863a.dtsi b/arch/arm64/boot/dts/sprd/sc9863a.dtsi
> > > > > > new file mode 100644
> > > > > > index 000000000000..578d71a932d9
> > > > > > --- /dev/null
> > > > > > +++ b/arch/arm64/boot/dts/sprd/sc9863a.dtsi
> > > > > > @@ -0,0 +1,536 @@
> > > > > > +// SPDX-License-Identifier: GPL-2.0-only
> > > > > > +/*
> > > > > > + * Unisoc SC9863A SoC DTS file
> > > > > > + *
> > > > > > + * Copyright (C) 2019, Unisoc Inc.
> > > > > > + */
> > > > > > +
> > > > > > +#include <dt-bindings/interrupt-controller/arm-gic.h>
> > > > > > +#include "sharkl3.dtsi"
> > > > > > +
> > > > > > +/ {
> > > > > > +     cpus {
> > > > > > +             #address-cells = <2>;
> > > > > > +             #size-cells = <0>;
> > > > > > +
> > > > > > +             cpu-map {
> > > > > > +                     cluster0 {
> > > > > > +                             core0 {
> > > > > > +                                     cpu = <&CPU0>;
> > > > > > +                             };
> > > > > > +                             core1 {
> > > > > > +                                     cpu = <&CPU1>;
> > > > > > +                             };
> > > > > > +                             core2 {
> > > > > > +                                     cpu = <&CPU2>;
> > > > > > +                             };
> > > > > > +                             core3 {
> > > > > > +                                     cpu = <&CPU3>;
> > > > > > +                             };
> > > > > > +                     };
> > > > > > +
> > > > > > +                     cluster1 {
> > > > > > +                             core0 {
> > > > > > +                                     cpu = <&CPU4>;
> > > > > > +                             };
> > > > > > +                             core1 {
> > > > > > +                                     cpu = <&CPU5>;
> > > > > > +                             };
> > > > > > +                             core2 {
> > > > > > +                                     cpu = <&CPU6>;
> > > > > > +                             };
> > > > > > +                             core3 {
> > > > > > +                                     cpu = <&CPU7>;
> > > > > > +                             };
> > > > > > +                     };
> > > > > > +             };
> > > > > > +
> > > > > > +             CPU0: cpu@0 {
> > > > > > +                     device_type = "cpu";
> > > > > > +                     compatible = "arm,cortex-a55";
> > > > > > +                     reg = <0x0 0x0>;
> > > > > > +                     enable-method = "psci";
> > > > > > +                     cpu-idle-states = <&CORE_PD>;
> > > > > > +             };
> > > > > > +
> > > > > > +             CPU1: cpu@100 {
> > > > >
> > > > > Your numbering seems odd. This follows the MPIDR reg? Normally a cluster
> > > > > would share the same number in one of the bytes.
> > > >
> > > > We're using A55, and the spec says that bit[15:8] identifies
> > > > individual cores within the local DynamIQâ cluster
> > >
> > > Okay.
> > >
> > > > Also, we only support one cluster.
> > >
> > > cpu-map shows 2 clusters.
> >
> > From the scheduler view, we have two clusters, but there's actually
> > one physical cluster only.
>
> What's the scheduler? ;)

It refers to EAS actually, which has a out-of-tree concept of Phantom
Domains which are not congruent to the real cluster physical domains
for DynamIQ.
But now I understand the problem (saw the previous similar discussions
on other DynamIQ SoC), dividing the cores into two phantom clusters is
a workaround and cannot be merged into mainline.  :)

>
> DT describes the physical system.

Got it, I will fix that to move all cores into a single cluster.

Thanks for the review.
Chunyan

>
> Rob