ARM machine specific DT probing

ARM machine specific DT probing

[Nicolas Pitre]

While reviewing the ARM DT patches I've come to a misunderstanding of 
how the specific machine support is meant to be matched with DT.

What Jeremy did is to add a probe_dt method in the mdesc structure, and 
then the core is calling them in sequence until one of them returns 
success.

now, the "compatible" property is explained here:

http://devicetree.org/Device_Tree_Usage#Understanding_the_compatible_Property

>From my understanding, this could allow for a kernel that doesn't yet 
support the specifics of a particular board to still be able to work 
using basic common support.  But for this to work, wouldn't it be 
necessary for the core code to try to find the best match itself rather 
than letting each machine's  probe_dt decide on their own?


Nicolas

Re: ARM machine specific DT probing

[Jeremy Kerr]

Hi Nicolas,

> What Jeremy did is to add a probe_dt method in the mdesc structure, and 
> then the core is calling them in sequence until one of them returns 
> success.
> 
> now, the "compatible" property is explained here:
> 
> http://devicetree.org/Device_Tree_Usage#Understanding_the_compatible_Property
> 
> From my understanding, this could allow for a kernel that doesn't yet 
> support the specifics of a particular board to still be able to work 
> using basic common support.  But for this to work, wouldn't it be 
> necessary for the core code to try to find the best match itself rather 
> than letting each machine's  probe_dt decide on their own?

At present, the probe_dt function is a binary match/no-match indicator,
so the core code just selects the first match it finds. This means that
we'll need one mdesc per machine; I'm intending to keep it simple to
start with.

My intention in the longer-term is to allow probe_dt to indicate
less-specific matches (eg, match on the SoC family), and change the core
code to not break out of the loop on the first match, but continue to
look for a better match. This way, we can have one mdesc to support a
SoC family, but still allow that to be overridden if there's a more
specific (eg, single machine) mdesc compiled in.

The reason I do this in the machine-specific code (rather than the core
code checking the compatible property) is to allow the probe_dt function
to check arbitrary data in the device tree. Perhaps we have two
variations of a machine - both with the same compatible property, but
distinguishable in some other way, and we need a separate mdesc for
whatever reason.

Cheers,


Jeremy

Re: ARM machine specific DT probing

[David Gibson]

On Thu, Sep 09, 2010 at 09:07:59AM +0800, Jeremy Kerr wrote:
> Hi Nicolas,
> 
> > What Jeremy did is to add a probe_dt method in the mdesc structure, and 
> > then the core is calling them in sequence until one of them returns 
> > success.
> > 
> > now, the "compatible" property is explained here:
> > 
> > http://devicetree.org/Device_Tree_Usage#Understanding_the_compatible_Property
> > 
> > From my understanding, this could allow for a kernel that doesn't yet 
> > support the specifics of a particular board to still be able to work 
> > using basic common support.  But for this to work, wouldn't it be 
> > necessary for the core code to try to find the best match itself rather 
> > than letting each machine's  probe_dt decide on their own?
> 
> At present, the probe_dt function is a binary match/no-match indicator,
> so the core code just selects the first match it finds. This means that
> we'll need one mdesc per machine; I'm intending to keep it simple to
> start with.
> 
> My intention in the longer-term is to allow probe_dt to indicate
> less-specific matches (eg, match on the SoC family), and change the core
> code to not break out of the loop on the first match, but continue to
> look for a better match. This way, we can have one mdesc to support a
> SoC family, but still allow that to be overridden if there's a more
> specific (eg, single machine) mdesc compiled in.

Hrm.  The trouble with this idea is that it needs some measure of
"specificness of match", and I'm not sure you can come up with an
encoding of that which is better than just order in the match
table. i.e. if you construct your match table such that more specific
matches precede less specific ones, you don't need to keep scanning
the table and then work out which is the "best" match.

> The reason I do this in the machine-specific code (rather than the core
> code checking the compatible property) is to allow the probe_dt function
> to check arbitrary data in the device tree. Perhaps we have two
> variations of a machine - both with the same compatible property, but
> distinguishable in some other way, and we need a separate mdesc for
> whatever reason.

Exactly: you do need to cover this unfortunate, but inevitable case.
The trouble is I don't see how you can have a specificness measure
without starting to make assumptions about what the probe function is
checking.

Getting the probe table ordered by specificness will be fiddly in some
cases, but not, I suspect more so than the hard cases would be anyway
with a specificness measure.  And I think it's as versatile an
encoding of specificness as you're likely to get.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: ARM machine specific DT probing

[Jeremy Kerr]

Hi David,

> Hrm.  The trouble with this idea is that it needs some measure of
> "specificness of match",

I was originally thinking an enum, something to indicate that the match
is for a machine or SoC or SoC-family, but that may not be flexible
enough.

Essentially, all we really need to indicate is that "this match is more
specific than that other match", which the match-table-ordering would
work fine for.

With the present infrastructure, we'd need to enforce this by
controlling the link order. However, I don't have any good ideas about
how we could do this neatly.

Maybe we could get make to work out the dependencies (this mdesc needs
to go before that one) for us :D

Cheers,


Jeremy

Re: ARM machine specific DT probing

[David Gibson]

On Mon, Sep 13, 2010 at 01:30:02PM +0800, Jeremy Kerr wrote:
> Hi David,
> 
> > Hrm.  The trouble with this idea is that it needs some measure of
> > "specificness of match",
> 
> I was originally thinking an enum, something to indicate that the match
> is for a machine or SoC or SoC-family, but that may not be flexible
> enough.
> 
> Essentially, all we really need to indicate is that "this match is more
> specific than that other match", which the match-table-ordering would
> work fine for.
> 
> With the present infrastructure, we'd need to enforce this by
> controlling the link order. However, I don't have any good ideas about
> how we could do this neatly.

Right, hence the fiddly.  As a first cut, you could just split the
table into multiple pieces - traverse the "per-soc" table, then the
"per-machine" table.  That should be pretty easy to implement and if
you build your macros and whatnot correctly it should be possible to
then generalize that without requiring changes on the individual
machine description declaration side.

> Maybe we could get make to work out the dependencies (this mdesc needs
> to go before that one) for us :D

Heh, maybe.  Interesting idea, though potentially hairy.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: ARM machine specific DT probing

[Grant Likely]

On Mon, Sep 13, 2010 at 04:03:25PM +1000, David Gibson wrote:
> On Mon, Sep 13, 2010 at 01:30:02PM +0800, Jeremy Kerr wrote:
> > Hi David,
> > 
> > > Hrm.  The trouble with this idea is that it needs some measure of
> > > "specificness of match",
> > 
> > I was originally thinking an enum, something to indicate that the match
> > is for a machine or SoC or SoC-family, but that may not be flexible
> > enough.
> > 
> > Essentially, all we really need to indicate is that "this match is more
> > specific than that other match", which the match-table-ordering would
> > work fine for.
> > 
> > With the present infrastructure, we'd need to enforce this by
> > controlling the link order. However, I don't have any good ideas about
> > how we could do this neatly.
> 
> Right, hence the fiddly.  As a first cut, you could just split the
> table into multiple pieces - traverse the "per-soc" table, then the
> "per-machine" table.  That should be pretty easy to implement and if
> you build your macros and whatnot correctly it should be possible to
> then generalize that without requiring changes on the individual
> machine description declaration side.
> 
> > Maybe we could get make to work out the dependencies (this mdesc needs
> > to go before that one) for us :D
> 
> Heh, maybe.  Interesting idea, though potentially hairy.

Ugh... before implementing any or all of these ideas, it is worth
going back to Nicolas' original question.

Nicolas rightly pointed out that the compatible property already
provides the mechanism for prioritized matching so that the operating
system can select the correct support code for a device.  Why then
does each board provide its own probe_dt() hook instead of being
implemented in the core code?  The answer lies partially in the
semantics of the compatible property and also a bit in the mists of
powerpc history (and is probably due for a rethink on both counts).

Concerning the compatible property...

For a device node, compatible is straight forward.  It specifies the
exact part number, followed by an option list of devices to which it
is 100% backwards, both at the register and the device tree binding
levels.  If it isn't 100% compatible, then it cannot claim
compatibility.  Since most devices are well contained, it is
relatively easy to make the call about whether or not compatibility
can be claimed.  ie. It is well established that an ns16550 is
compatible with the ns8250.

However, what does compatible mean at the board level?  Can a
BeagleBoard-xM claim compatibility with the original BeagleBoard?  Or
even can a -b1 BeagleBoard claim compatibility with the original -a1
revision?  The -b1 has more SDRAM than the original, and the -xM has
removed the on board NAND flash.  Neither change can be considered to
be "100% backwards compatible", so is it valid to claim compatibility
with the older board?  Clearly the difference between the boards is
still described in the body of the tree so claiming compatibility
appears to be the Right Thing To Do, but a strict reading would say
no.

To date we've dealt with the ambiguity by simply stating that it
doesn't make sense to claim compatibility at the top level.  Each
board uses a unique identifier and the kernel is modified (by adding
the new string) for each board.  I don't like having to modify the
kernel for this, but the decision to do it this way has a certain amount of sanity.  This decision was made early in the transition to device tree on powerpc.  We were being conservative and didn't want to use ambiguous descriptions that would be hard to back off from later... at least not until we had more experience with how to use the tree (more on that later).

Concerning powerpc history...

PowerPC history is paved with OpenFirmware passing oddball device
trees to the kernel.  Things like multiple machines from IBM that only
contain "chrp" in the top level compatible list.  Before the flattened
form of the device tree was even conceived, the problem existed that
'compatible' was not sufficient to uniquely identify the machine.
Some machines don't even have a compatible value.  Each machine port
had to look at different properties in the tree, and therefore each
platform supplied an .probe() hook to determine what kind of machine
it was.  The design reflects the fact that sometimes custom code is
needed to parse an oddball tree.

benh has also been strongly against powerpc embedded board support
that can match against a family of boards or SoCs.  Instead, he has
required that each platform code have a list of specific boards that
it works with.  Example: arch/powerpc/platforms/5200/mpc5200_simple.c.
He doesn't want to get back into the situation where each machine type
gets a whole bunch of complex board-specific workarounds.  My opinion
is this was a good starting point, but it is probably overly
conservative.  I think it would be reasonable to have board support
that matches on, for example, the SoC instead of the board provided
that there remains a method for doing board-specific overrides.  The
moment a machine needs to  do something 'special' the solution is to
register a new machine type that only matches against the special
board.

Jeremy's patches simply carry over to ARM the established pattern.  At
the time I agreed with the patch, as I wanted to preserve the ability
to do custom matching.  However, there are some differences in the ARM
use case:

- We don't have any relevant incumbent OpenFirmware machines, and
  OpenFirmware on arm will remain a minority.
- the device tree blob is maintained as a separate data file
  independent of boot fimrware.  (firmware doesn't generate it)
- Even if firmware provide a non-replaceable 'bad' dt blob, we've have
  the ability to replace it at boot time.
- The above three points mean that the ARM dt support is not a slave
  to the boot firmware shipped on the board.
- 'compatible' semantics are better established now that when PowerPC
  openfirmware support was added.  It can be coded for the desired
  behaviour.

ARM device tree usage is new, so we don't need to support legacy
device trees with custom probe code.  Trying to anticipation how
people will do it wrong is just borrowing trouble.

Okay, after some mulling, here is my proposal:

- implement compatible matching in the core code
  - This will enforce device tree authors to put valid data in the top
    level compatible value.  Otherwise Linux will not boot.
  - Each OF-enabled board can supply an of_device_id table for
    matching purposes.
- remove the .probe_dt hook
  - The of_device_id table will cover all the known use-cases for ARM
  - If we ever do end up in a situation where custom probe code is
    needed, then it can be added back; but only as a secondary match
    mechanism.  Most likely we will never need it though.
- Revisit the meaning of top-level compatible.
  - I still don't think it makes sense for one board to claim
    compatibility with another, but it may be reasonable for the
    board level to claim compatibility with the SoC used.
  - However, it needs to be strictly defined as to what it means when
    the top level claims compatibility with the SoC.  What specific
    SoC is used?  What assumptions are made?  etc.  It certainly won't
    be valid to make such a claim before the new compatible value is
    documented.
  - Regardless, I want to be careful here so that a decision isn't
    made that is both wrong and hard to recover from.

Thoughts?
g.

Re: ARM machine specific DT probing

[Scott Wood]

On Thu, 16 Sep 2010 11:40:20 -0600
Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:

> However, what does compatible mean at the board level?  Can a
> BeagleBoard-xM claim compatibility with the original BeagleBoard?  Or
> even can a -b1 BeagleBoard claim compatibility with the original -a1
> revision?  The -b1 has more SDRAM than the original, and the -xM has
> removed the on board NAND flash.  Neither change can be considered to
> be "100% backwards compatible", so is it valid to claim compatibility
> with the older board?  Clearly the difference between the boards is
> still described in the body of the tree so claiming compatibility
> appears to be the Right Thing To Do, but a strict reading would say
> no.

It gets even uglier with virtualization, where you can have arbitrary
subsets of a particular board's devices available -- and things like
the interrupt controller that are currently normally hardcoded in the
platform file may be swapped out for a paravirt interface.

> benh has also been strongly against powerpc embedded board support
> that can match against a family of boards or SoCs.  Instead, he has
> required that each platform code have a list of specific boards that
> it works with.  Example: arch/powerpc/platforms/5200/mpc5200_simple.c.
> He doesn't want to get back into the situation where each machine type
> gets a whole bunch of complex board-specific workarounds. 

Even as things are now, such workarounds can still be pretty awkward.

They may actually be SoC (family) workarounds (and thus you end up with
a bunch of identical machine_initcalls or similar), or it may need to
go in the middle of code that doesn't normally go in the platform file,
etc.

And it excuses ordinary, non-workaround code (PCI and interrupt setup)
sitting around duplicated in a bunch of platform files instead of being
device-tree-based.

> - Revisit the meaning of top-level compatible.
>   - I still don't think it makes sense for one board to claim
>     compatibility with another,

Most of the time it doesn't, but there may be circumstances where things
are only added, or where a new revision just fixes bugs but software
that works around the bugs will still work.

If we don't allow claiming compatibility in those cases, it may
encourage people to lie and claim to just be that old board with no
more specific entry in the list (or just not put the board rev in the
name at all -- which might be reasonable if the rev info is presented
in a separate property, allowing things like greater-than/less-than
comparisons).

>     but it may be reasonable for the
>     board level to claim compatibility with the SoC used.

Or a family of SoCs, or a family of boards, etc... The criteria simply
ought to be that it is well documented what it means to be compatible
with that string, and that there be something very specific that code
can match on instead if^H^Hwhen things go wrong.

-Scott

Re: ARM machine specific DT probing

[David Gibson]

On Thu, Sep 16, 2010 at 05:56:17PM -0500, Scott Wood wrote:
> On Thu, 16 Sep 2010 11:40:20 -0600
> Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
[snip]
> > - Revisit the meaning of top-level compatible.
> >   - I still don't think it makes sense for one board to claim
> >     compatibility with another,
> 
> Most of the time it doesn't, but there may be circumstances where things
> are only added, or where a new revision just fixes bugs but software
> that works around the bugs will still work.
> 
> If we don't allow claiming compatibility in those cases, it may
> encourage people to lie and claim to just be that old board with no
> more specific entry in the list (or just not put the board rev in the
> name at all -- which might be reasonable if the rev info is presented
> in a separate property, allowing things like greater-than/less-than
> comparisons).

I concur on the whole.  Claiming compatibility at the top-level has
its problems, but I don't think outright banning it is a nett win.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson