[i2c] [PATCH 3/5] powerpc: Document device nodes for I2C devices.

[Segher Boessenkool]

>>> I might be a little less noisy about it if there was
>>> some kind of edict for devices never to wander outside
>>> of their own node in the device tree, but there isn't.
>>
>> I'm not sure what you mean here.  It is best practice
>> for device nodes to be reasonably self-contained though.
>> Of course not completely; every node always has to refer
>> to its parent bus, etc.  Device drivers will sometimes
>> have to refer to board model for board-specific workarounds.
>
> Right. So, my question is, what is the difference between
> device A and B on the same chipset or using the same IP core,
> which cannot be described by board model or board-specific
> handling in other places in the device tree?

I still don't understand the question.

> I see a lot of clutter on my particular tree here; the USB
> controller has 'ohci-bigendian, ohci-be, mpc5200-ohci,
> mpc5200-usb' in it's compatible nodes. The model is mpc5200-ohci.
>
> I would love to know why ohci-bigendian isn't just used. It
> is perfectly obvious from the location of the node (it's
> parent is /builtin or /soc, which has model 'mpc5200b').

"fsl,mpc5200b-usb" is the exact name.  This one can be matched
against if the driver needs to do things specific to this
chip.

"fsl,mpc5200-usb" is one step below; it can be used by a driver
if that driver can drive every USB in the 5200 family.

"usb-ohci-bigendian" can be matched against by the USB OHCI
driver if there is no need for any 5200-specific quirks.

(And yes I know I changed some names here).

> The various names and compatibles I think are just information
> for information's sake.

Lots of people go overboard with them.  "compatible" is a very
important property though: it is the main thing that is used
to match OS device drivers with devices.

> Prepending mpc5200 to it is useless
> information.

Not at all.  If the 5200 needs any special treatment, this
is where the device driver (or some early quirk step, or
whatever) will see it has to deal with that.

> The ONLY differentiation I see relevant here is
> that the device is big-endian. Isn't that a standard property
> though in most device trees, and not part of the device name?

It isn't a standard property.  It would have been a better
way to go about this though, yes -- not put the "big-endian"
in the name, but in a separate property.

> I don't think you could put the information in there any other
> way, but then again, if you are running an MPC5200B, how many
> other ways can you configure the USB device? You can practically
> assume big-endianess.

But then you need to put chipset-specific information in the
kernel, where it isn't necessary at all.  There are quite
a few USB OHCs that are big-endian.

> The same goes for the Ethernet (mpc5200b-fec, mpc5200-fec)
> which is a ridiculous differentiation. It's parent node
> as defined in the same device-tree bindings document
> is ALWAYS going to be the 'soc' node, which has a model
> which perfectly describes the SoC. Even THAT node has
> compatible properties (mpc5200-soc, mpc5200b-soc) in
> the documentation.

You shouldn't have to look at any other nodes if all you're
trying to figure out is what one device is exactly.

> Again it is information for information's sake.

Not at all.  I'm sorry you don't see that.  It seems
you'd rather want one single node that says "5200" but
that's just not how the device tree is supposed to be
used.

> A driver for
> platform USB as implemented in Linux only needs one of them,
> and none of the detailing on how much of it pertains to the
> chip revision is required in-node.

Maybe the 5200b USB is actually *fully* compatible to the
5200 USB.  So you got lucky, and the device driver can
simply match on plain 5200 for both, since the 5200b device
trees state it is compatible to that, too.  Now OTOH if some
sneaky bug would show up that requires a workaround on 5200b
devices, you can match on that instead.

> The device tree has become a dumping ground for decisions
> made by Linux developers ("what shall I call this node?"
> and changing their mind each time), and is no better for
> driver support for it if you only track the mainline.

There are many bad device trees around yes.

> I2S is another one on this particular chip example;
>
> ~~
> PSC in i2s mode:  The mpc5200 and mpc5200b PSCs are not compatible 
> when in
> i2s mode.  An 'mpc5200b-psc-i2s' node cannot include 'mpc5200-psc-i2s' 
> in the
> compatible field.
> ~~
>
> All you need to know is that it's an i2s controller as defined
> by the board designer. Secondary to that, you need to know
> that it is a PSC I2S controller (the same distinction is true
> of MPC52xx SPI). This is what your model attribute is for,
> right?

No, the "model" property is for defining the exact
"part number" (or similar) for the device.  I'll quote:

	Standard property name to define a manufacturer’s model
	number.

	prop-encoded-array: Text string, encoded with encode-string.
	A manufacturer-dependent string that generally specifies the
	model name and number (including revision level) for this
	device. The format of the text string is arbitrary, although
	in conventional usage the string begins with the name of the
	device’s manufacturer as with the “name” property. Although
	there is no standard interpretation for the value of the
	“model” property, a specific device driver might use it to
	learn, for instance, the revision level of its particular
	device.

> That you are running on an MPC5200 or MPC5200B is
> defined by the SoC node already.

Irrelevant, as I've said many times now.

> If you want to use any of
> the devices on the chip, you NEED to ioremap the register
> space, which entails accessing the parent SoC node.

But the probing routine doesn't have to; there are
helper functions for that.

> So the advantage is lost; driver code isn't succinct,
> and certainly having lists of devices which 'fit'
> certain names is a wasteful concept. You trade off
> 20 lines of code to differentiate the chip with 20
> lines of compatibility tables.
>
> In fact, this kind of device naming and using compatibility
> properties is practically discouraged by Ben's device
> tree document (Section III, note 3)

I think you just read it wrong.  Either way, you should
read the standard OF documentation as well, it would
certainly help you understand some of the concepts here.

>>> It is quite another
>>> matter to make it a kind of Linux-programmers errata
>>> replacement framework and artificially recreate already
>>> easily-accessible information.
>>
>> No one is proposing that I hope.  This information indeed
>> is already easily available in most cases -- namely, in
>> the device tree.
>
> And if it isn't, it is always accessible by the chip in
> question anyway. Want the SVR of the SoC? Well, accessing
> the SVR on the SoC is something any driver can do, the same
> for any general purpose register, and even if the device
> node is not in the tree.. it can still access it because
> the registers are there.

While you could do that for some SoC devices perhaps,
_iff_ you know for sure that no matter what those SVR
or whatever registers live at certain addresses when
all you really know is that you have one specific SoC
device, you cannot for most other devices.  Why would
you special case some SoC devices?  It is an encapsulation
violation no matter what.

> WhileI think it would be cute to put it in the device tree,
> it is far from an essential value.

Nothing is "essential", certainly you *can* boot a Linux
kernel without any device tree.  Duh.

> All in all I think the flat device tree is being abused,

Yes it is, but the things that you complain about aren't
the problem -- things that require implicit knowledge
(about Linux for example) are.

> Like you said, that's just my opinion on it, but I have
> seen too many people nag at Genesi/bplan for the quirks
> in our device tree, and a lot of effort went into making
> our device trees fit the expectations of Linux where it
> was absolutely forced upon us by Linux driver writers.

You could have avoided most of those problems by working
with us from the start.  Developing in a hidden corner
for months and then dropping all your code on us seldom
gives the results you hope for.  Anyway, let's not rehash
*that* discussion.

> What I saw is a lot of effort being expended into
> looking at device drivers past and present and trying
> to interpret a complete lack of coherent documentation

There is *plenty* very good documentation.  And you
can always ask.

> and going back to the original goal of the
> device tree - it describes the system, but it mostly
> exists to abstract devices for -drivers in the
> firmware- and not in Linux.

Rubbish.

> /soc/usb/storage/disk at 0,0
> was a block device handle far, far before it was
> simply an advertisement of an attached disk..

It *always* is the second, and it *can* be the first.


Segher