On 05/01/2013 06:35:38 PM, Anthony Foiani wrote:
Scott --
Thanks again for the quick reply.
On 05/01/2013 12:05 PM, Scott Wood wrote:
On 04/30/2013 09:06:56 PM, Anthony Foiani wrote:
Instead of a new property name, I would instead check for my
specific board type (let's call it a foo-8315) in the top-level
compatible list? So I'd change my devtree to have this top-level
compatible:
/ {
compatible = "example,foo-8315", "fsl,mpc8315erdb";
It should really only have compatible = "example,foo-8315", since
it's not 100% compatible with fsl,mpc8315erdb (at least due to this
bug, but probably there are other differences as well).
Then I guess I don't understand the proper use of "compatible" (or is
the root node special?)
It's only special in that 100% compatibility is much less likely to be
true of an entire system than of a particular component.
E.g., the DTS for the "parent" board (MPC8315ERDB) has multiple
entries for the crypto "compatible" value:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/powerpc/boot/dts/mpc8315erdb.dts?id=refs/tags/v3.9#n286
(or: *http://preview.tinyurl.com/btlqxgo* )
| crypto@30000 {
compatible = "fsl,sec3.3", "fsl,sec3.1",
"fsl,sec3.0",
"fsl,sec2.4", "fsl,sec2.2",
"fsl,sec2.1",
"fsl,sec2.0";
reg = <0x30000 0x10000>;|
I read this as meaning: "if you have to ask if a certain feature is
compatible with some 'foo', then this board provides that
compatibility". Not as "if a value is in the compatibility flag,
then it is 100% compatible with that value". (Although maybe that's
true in the case of the SEC, so perhaps that a bad example.)
AFAIK there is backwards compatibility with these SEC versions. If
not, they shouldn't be listed.
For what it's worth, the upstream vendor did have a separate
root-node "compatible" value -- which called a board-specific
function in a board-specific C file, both of which were direct cut &
paste copies from the MPC8315ERDB function / file. My gut instinct
is that this degree of duplication is unhealthy and incorrect, but if
my solution is considered abuse of the device tree, then I can try to
do it a different way next time.
It's quite possible to use the same C file for multiple similar boards
with different compatibles. This is done often, including
mpc831x_erdb.c.
Given those diffs, it didn't seem much of a stretch to use compatible
= "fsl,mpc8315erdb"
The criteria for claiming compatibility should be based in the hardware
itself, not whether a particular file in Linux needs any changes.
Or do you mean that you would not set this on any board's device
tree by default, and instead have users set it if they encounter
problems?
No, I would expect to set it on all the boards, so using the
compatibility hack above would work.
You mean all the boards that have the bug, which doesn't include any
upstream device tree, right?
As mentioned above, my primary concern is the use of these cards in
the project/product I'm working on. My answer has been to apply this
fix (and the matching change to the device tree I supply as a part of
the boot image). I feel that I'm trying to do the right thing by
getting some of these changes publicly visible, but I fear that I'll
also have to go down the route of "not enough time or money to
properly upstream it".
"doesn't include upstream device tree" ... no, the device tree was
supplied with the original set of patches from the vendor.
I'm not saying that the device tree not being upstream is a problem --
actually the opposite, that it means we don't have compatibility to
maintain with an already-accepted device tree.
-Scott
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
