So, like, David Gibson said:
>
> On Thu, Feb 05, 2015 at 12:20:48PM +0000, Mark Rutland wrote:
> > [Adding dtc folk]
> >
> > On Thu, Feb 05, 2015 at 12:09:20PM +0000, Will Deacon wrote:
> > > On Thu, Feb 05, 2015 at 11:59:33AM +0000, Mark Rutland wrote:
> > > > On Thu, Feb 05, 2015 at 11:54:16AM +0000, Will Deacon wrote:
> > > > > On Thu, Feb 05, 2015 at 11:46:42AM +0000, Mark Rutland wrote:
> > > > > > On Mon, Jan 26, 2015 at 05:54:15PM +0000, Will Deacon wrote:
> > > > > > > diff --git a/arch/arm64/boot/dts/arm/juno.dts b/arch/arm64/boot/dts/arm/juno.dts
> > > > > > > index cb3073e4e7a8..4ed9287aaef1 100644
> > > > > > > --- a/arch/arm64/boot/dts/arm/juno.dts
> > > > > > > +++ b/arch/arm64/boot/dts/arm/juno.dts
> > > > > > > @@ -107,11 +107,11 @@
> > > > > > > pmu {
> > > > > > > compatible = "arm,armv8-pmuv3";
> > > > > > > interrupts = <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>,
> > > > > > > + <GIC_SPI 02 IRQ_TYPE_LEVEL_HIGH>,
> > > > > > > + <GIC_SPI 06 IRQ_TYPE_LEVEL_HIGH>,
> > > > > > > <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>,
> > > > > > > <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>,
> > > > > > > - <GIC_SPI 30 IRQ_TYPE_LEVEL_HIGH>,
> > > > > > > - <GIC_SPI 02 IRQ_TYPE_LEVEL_HIGH>,
> > > > > > > - <GIC_SPI 06 IRQ_TYPE_LEVEL_HIGH>;
> > > > > > > + <GIC_SPI 30 IRQ_TYPE_LEVEL_HIGH>;
> > > > > > > };
> > > > > >
> > > > > > I am very much not keen on this. While this may get things working
> > > > > > today, it completely relies on Linux-internal details (the order of CPU
> > > > > > bringup, which in this case is different from the order of entries in
> > > > > > /cpus).
> > > > > >
> > > > > > In all other dts that I am aware of, the order of entries in /cpus
> > > > > > aligns with the order of interrupts in the PMU node, and the first entry
> > > > > > is the boot CPU.
> > > > > >
> > > > > > I think that we should ensure that the ordering of CPU nodes matches the
> > > > > > order of interrupts here. That way we can fall back to that ordering (if
> > > > > > not explicitly overridden), and even after an arbitrary logical
> > > > > > renumbering (e.g. after a kexec) the relationship should stay intact.
> > > > >
> > > > > There are a few problems with reordering the CPU nodes:
> > > > >
> > > > > (1) It breaks any existing users of taskset to pin on big/little
> > > > > clusters.
> > > >
> > > > This is unfortunate, but this is also the case if the boot CPU is
> > > > different.
> > >
> > > Right, so don't change the boot CPU. In that vain, we also shouldn't change
> > > the CPU order in the .dts -- the current .dts is working for taskset and
> > > we shouldn't break people's scripts just because they want to use the PMU.
> >
> > I think this is an orthogonal discussion. If Linux is booted on a
> > different CPU, it's not the fault of Linux that CPU0 is different.
> >
> > > > > (2) It's not generally possible if, for example, the bootloader decides
> > > > > to boot Linux on a different CPU then we have no choice but to
> > > > > change the PMU interrupt order.
> > > >
> > > > In that case _this_ patch is broken.
> > >
> > > Why? I'm not denying that changing the boot CPU causes problems, I'm saying
> > > that you *can't* fix that by changing the CPU node order. You still have
> > > to change the interrupt order in that case, so why not just localise the
> > > changes there in the first place?
> >
> > If we're going to try to maintain support for these DTs long-term (with
> > kexec and whatever logical renumbering can occur there), then we need a
> > consistent invariant that we can rely on to associate interrupts and
> > CPUs correctly.
> >
> > The Linux logical ordering is not invariant, so we know that this _will_
> > break.
> >
> > As far as I am aware, every other DT lists the boot CPU first, and the
> > order of entries in /cpus mathes the logical order. Using the order of
> > entries in /cpus will remain consistent in the face of arbitrary
> > renumbering, and is (currently) consistent with logical numbering.
> >
> > So keeping the CPU nodes and interrupt entries in the same order
> > provides us with a long-term consistent order, regardless of which CPU
> > is the boot CPU.
> >
> > This DT is currently broken. If we're going to make it work we should do
> > so in a manner that will continue to work. Anything else is a broken
> > bodge that hurts us in the long-term as we'll have to hack around it.
> >
> > > > If we associate the interrupt with a CPU by node order, the relationship
> > > > is preserved regardless of which CPU is the boot CPU (whether it was the
> > > > bootloader's choice, kexec, or whatever).
> > >
> > > Sure, and that requires code changes. If we're going to change the code,
> > > then I'd much rather we make the binding explicit, like I did in the
> > > follow-up patches to this one. As I mentioned before, this is a .dts fix
> > > to get things working with the current code. It's really too late to argue
> > > about the existing binding, even if it sucks.
> >
> > Sure, the binding sucks.
> >
> > This DT has also _never_ worked.
> >
> > If we're going to fix things, let's not introduce a middle step that's
> > broken in a different way.
> >
> > > > > (3) I didn't think that the ordering of CPU nodes was guaranteed to be
> > > > > preserved by dtc, whereas the order of the interrupts will be.
> > > >
> > > > The order of nodes is presently preserved.
> > >
> > > It's not about the present behaviour; I need a _guarantee_ that dtc/libfdt
> > > will *never* reorder CPU nodes. Today's working .dts file needs to continue
> > > to work with future tools.
> >
> > Jon, David, Grant, thoughts?
>
> As a general rule, neither dtc nor libfdt will re-order any nodes
> unless you explicitly ask them to (e.g. dtc's "-s" option). That
> said, you should try not to rely on dt order.
Hi guys,
As you explicitly solicited my opinion, I will tell you what I think.
First, I agree with David: DTC and libfdt are not gratuitously
re-ordering the nodes within the tree. But nothing should rely
on that behaviour either. It is a tree, and the children of a
particular node are unordered. There *are* manipulation primitives
that can cause a restructuring of the tree, and DTS consumers should
be prepared to accomodate that.
Second, that Linux assumes an ordering on nodes from the tree is
really unfortunate. I think we should try to remoce any such
dependency or accomodate a more relaxed DTB read.
Finally, remember that the DTS is supposed describe the hardware.
The hardware doesn't (*usually*) require an ordering on its
components. In a true SMP, it is S -- symmetric -- and shouldn't
rely on one Core being more first than another. That one is treated
as a special entity is entirely a SW description. As such, it
really shouldn't be represented as some tacit or hidden fact within
the DTS.
HTH,
jdl
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
