Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200826120254.8902-1-matthias.schiffer@ew.tq-group.com>
 <4dd06b79-1402-d7cf-9676-1f9a9526da12@gmail.com> <9eb72c6561333661599411e49072928385629999.camel@ew.tq-group.com>
 <ac64852a-7f2a-6005-f914-268670cd4f95@gmail.com> <CAL_Jsq+1LsTBdVaODVfmB0eme2jMpNL4VgKk-OM7rQWyyF0Jbw@mail.gmail.com>
 <d7dc1017818e935acf9ba838080bcc3c11b30888.camel@ew.tq-group.com> <735c00caa90746d20bfaafa42c4d911c61729228.camel@ew.tq-group.com>
In-Reply-To: <735c00caa90746d20bfaafa42c4d911c61729228.camel@ew.tq-group.com>
From:   Rob Herring <robh+dt@kernel.org>
Date:   Fri, 11 Sep 2020 08:43:29 -0600
Message-ID: <CAL_Jsq+SuJ5F=SxGfZp0g9V8TPejSxEu8Ya45qfDxhZ0rL8ETg@mail.gmail.com>
Subject: Re: [PATCH] of: skip disabled CPU nodes
To:     Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
Cc:     devicetree@vger.kernel.org,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Frank Rowand <frowand.list@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Sep 9, 2020 at 2:58 AM Matthias Schiffer
<matthias.schiffer@ew.tq-group.com> wrote:
>
> On Thu, 2020-08-27 at 09:10 +0200, Matthias Schiffer wrote:
> > On Wed, 2020-08-26 at 13:26 -0600, Rob Herring wrote:
> > > On Wed, Aug 26, 2020 at 8:47 AM Frank Rowand <
> > > frowand.list@gmail.com>
> > > wrote:
> > > >
> > > > Hi Rob,
> > > >
> > > > On 2020-08-26 08:54, Matthias Schiffer wrote:
> [snip]
> > > > >
> > > > > Hmm, I see. This difference in behaviour is quite unfortunate,
> > > > > as
> > > > > I'm
> > > > > currently looking for a way to *really* disable a CPU core.
> > > > >
> > > > > In arch/arm64/boot/dts/freescale/imx8mn.dtsi (and other
> > > > > variants
> > > > > of the
> > > > > i.MX8M), there are 4 CPU nodes for the full-featured quad-core
> > > > > version.
> > > > > The reduced single- and dual-core versions are currently
> > > > > handled
> > > > > in
> > > > > NXP's U-Boot fork by deleting the additional nodes.
> > > > >
> > > > > Not doing so causes the kernel to hang for a while when trying
> > > > > to
> > > > > online the non-existent cores during boot (at least in linux-
> > > > > imx
> > > > > 5.4 -
> > > > > I have not checked a more recent mainline kernel yet), but the
> > > > > deletion
> > > > > is non-trivial to do without leaving dangling phandle
> > > > > references.
> > > >
> > > > Any thoughts on implementing another universal property that
> > > > means
> > > > something like "the hardware described by this node does not
> > > > exist
> > > > or is so broken that you better not use it".
> > >
> > > There's a couple of options:
> > >
> > > The DT spec defines 'fail' value for status. We could use that
> > > instead
> > > of 'disabled'.
> > >
> > > The spec behavior with cpu 'disabled' is only on PPC AFAIK. On
> > > arm/arm64 (probably riscv now too) we've never followed it where we
> > > online 'disabled' CPUs. So we could just make the check conditional
> > > on
> > > !IS_ENABLED(CONFIG_PPC). This would need some spec update.
> >
> > On ARM(64), the "disabled" status on CPUs doesn't have any effect. I
> > assume this changed with the mentioned commit c961cb3be906 "of: Fix
> > cpu
> > node iterator to not ignore disabled cpu nodes", as reverting it
> > gives
> > me the desired behaviour of considering the disabled CPUs non-
> > existent.
> >
> > So it seems that we already changed the interpretation in a non-
> > compatible way once (back in v4.20), and somehow PPC has yet another
> > different behaviour?
> >
> > How do we get out of this mess? Is going back to the v4.19 logic for
> > non-PPC platforms an acceptable regression fix, or would this be
> > considered another breaking change?

Yes, this is my 2nd option above. Need to double check MIPS and Sparc though.

> Any new insights on this? I'll gladly provide patches or proposals for
> a spec update if we can decide where we want to go with this.

I gave 2 options. I'm fine with either one (or both). Using 'fail' is
the simplest.

Rob