Re: [PATCH] olpc_battery: bind to device tree

[Grant Likely]

On Fri, Feb 25, 2011 at 7:19 AM, Daniel Drake <dsd@xxxxxxxxxx> wrote:
> On 23 February 2011 20:34, Grant Likely <grant.likely@xxxxxxxxxxxx> wrote:
>> As mentioned in the other thread, matching by name is strongly
>> discouraged.  It isn't very accurate and compatible is the preferred
>> method for binding devices.  'battery' in particular is highly
>> non-specific.
>>
>> I do understand that you don't have a compatible property in the
>> current firmware, and to a certain extent we have to live with what
>> we're given by the kernel.  However, I think it would be better in the
>> OLPC case to find the battery node and add a compatible property
>> before registering a platform_device for it.  (or use a bus notifier
>> to tell you when it is registered, and add 'compatible' at that
>> point.)  That way we the uncertainty is taken care of in the board
>> support code without polluting the driver matching namespace.
>
> Thanks for the review. This and the rest of your feedback makes sense.
>
> Would you mind commenting on exactly how this should look?

Before calling of_platform_bus_probe(), search the device tree for the
battery node and use prom_add_property() to add a compatible prop.

>
> Here are the changes i'm planning to make, both to the firmware and
> with kernel code as you suggest to fixup device trees for systems with
> old firmware:
>
> /battery@0/compatible property added with value "olpc-battery" (XO-1 and XO-1.5)
> /pci/isa@f/rtc@i70/compatible property prepended with "olpc-xo1-rtc,"
> (XO-1 only)
> /pci/display@1,1/dcon device added, name=dcon compatible=olpc-dcon
> (XO-1 and XO-1.5)

Compatible properties should generally be in the form:
"<vendor>,<part>".  So these should probably be:

battery:  compatible = "olpc,xo1-battery";
rtc: compatible = "olpc,xo1-rtc";
dcon: compatible = "olpc,xo1-dcon";

I would explicitly specify the "xo1-" part in all three to protect
against the eventuality of a new xo revision that has does something
incompatible.  ie. "olpc-battery" doesn't help if, say, XO-1.75 has a
different battery.  Newer configurations can claim compatibility with
the old if they are truly compatible.

>
> In addition to the battery patch you reviewed, we plan to make the
> olpc-rtc and DCON drivers bind to device tree nodes, which is the
> reason behind the other changes.

okay.

g.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/