Re: [PATCH 2/2] mpc85xx_edac: change to use new definitions for PCI EDAC regspace

[Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>]

On 7/24/10, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Fri, Jul 23, 2010 at 6:20 PM, Dmitry Eremin-Solenikov
> <dbaryshkov@gmail.com> wrote:
>> Hello,
>>
>> On 7/22/10, Grant Likely <grant.likely@secretlab.ca> wrote:
>>> On Thu, Jul 22, 2010 at 10:48 AM, Dmitry Eremin-Solenikov
>>> <dbaryshkov@gmail.com> wrote:
>>>> Hello,
>>>>
>>>> On Thu, Jul 22, 2010 at 7:38 PM, Kumar Gala <galak@kernel.crashing.org>
>>>> wrote:
>>>>>
>>>>> On Jul 21, 2010, at 7:03 PM, Dmitry Eremin-Solenikov wrote:
>>>>>
>>>>>> Currently (as mpc8540-pci) devices are not created on of_platform bus,
>>>>>> mpc85xx_edac can't probe to them. Follow the change to dts trees to
>>>>>> bind
>>>>>> not to the main mpc8540-pci node but to special mpc85xx-pci-error
>>>>>> nodes,
>>>>>> present on soc bus.
>>>>>>
>>>>>> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
>>>>>> ---
>>>>>> drivers/edac/mpc85xx_edac.c |   18 +++++++++---------
>>>>>> 1 files changed, 9 insertions(+), 9 deletions(-)
>>>>>
>>>>> Nak.
>>>>>
>>>>> We already have a node in the dts for the PCI controller.  Lets update
>>>>> the platform code to add the pci controller to the
>>>>> of_platform_bus_probe
>>>>> list.
>>>>
>>>> I've had that idea. However it's really look strange to me to call
>>>> of_platform_bus_probe() on the bus node, for which we (IMO) explicitly
>>>> won't like for
>>>> child devices (PCI devices) to be added to of_platform bus. Would it
>>>> be suitable to just call of_platform_device_create for it (Or do i
>>>> miss someth<ing)?
>>>
>>> Try the attached patch (lightly tested).  If it works for you then
>>> I'll post it for wider review.
>>
>> Yes, this patch worked for me. However it looks a bit like a hack for me.
>
> I'll probably refine it a bit before merging, but I don't think it is
> a hack.  It reflects the behaviour that makes sense when registering
> devices hanging off the root node.  If a device node is a child of the
> root, then we know it isn't hanging off an i2c or pci bus, or anything
> else.  It is essentially a system device.
>
> The troublesome bit is that the root node also has memory, cpus,
> chosen and aliases nodes which are not devices.  In the vast majority
> of cases, we want all the device nodes that are children of the root
> to be registered, but we don't want to register the special nodes.
> Checking for the presence of a compatible property is a pretty good
> test for determining whether or not a node actually represents a
> device, especially because all users of of_platform_bus_probe() seem
> to be FDT users where we've been very strict about enforcing that
> drivers must use the compatible property for matching to device nodes.


Now it's clear to me, thanks for the explanation.

BTW: On 2.6.35-rc6 I had to make 'compat' and 'match' variables const.

-- 
With best wishes
Dmitry

------------------------------------------------------------------------------
This SF.net email is sponsored by Sprint
What will you do first with EVO, the first 4G phone?
Visit sprint.com/first -- http://p.sf.net/sfu/sprint-com-first