linux-kernel - Re: [PATCH] olpc_battery: bind to device tree

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <AANLkTimpWxEvQ1wgQzZ2RRA=0s_hEmKFAyz1bVr8nKai@mail.gmail.com>
Date:	Fri, 25 Feb 2011 08:33:44 -0700
From:	Grant Likely <grant.likely@...retlab.ca>
To:	Daniel Drake <dsd@...top.org>
Cc:	devicetree-discuss <devicetree-discuss@...ts.ozlabs.org>,
	linux-kernel@...r.kernel.org, dilinger@...ued.net
Subject: Re: [PATCH] olpc_battery: bind to device tree

On Fri, Feb 25, 2011 at 7:19 AM, Daniel Drake <dsd@...top.org> wrote:
> On 23 February 2011 20:34, Grant Likely <grant.likely@...retlab.ca> 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@...ompatible property added with value "olpc-battery" (XO-1 and XO-1.5)
> /pci/isa@...tc@.../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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/