Thread (6 messages) 6 messages, 5 authors, 2013-02-19

[PATCH 2/2] of: use platform_device_add

From: Jason Gunthorpe <hidden>
Date: 2013-02-19 19:03:54
Also in: linuxppc-dev, lkml

On Sun, Feb 17, 2013 at 10:49:08AM +0000, Grant Likely wrote:
quoted
quoted
The patch introduce a regression on imx6q boot.  The IOMUXC block on
imx6q is special.  It acts not only a pin controller but also a system
controller with a bunch of system level registers in there.  That's why
we currently have the following two nodes in imx6q device tree with the
same start "reg" address, which work with drivers/mfd/syscon.c and
drivers/pinctrl/pinctrl-imx6q.c respectively.

        gpr: iomuxc-gpr at 020e0000 {
                compatible = "fsl,imx6q-iomuxc-gpr", "syscon";
                reg = <0x020e0000 0x38>;
        };

        iomuxc: iomuxc at 020e0000 {
                compatible = "fsl,imx6q-iomuxc";
                reg = <0x020e0000 0x4000>;
        };

With the patch in place, pinctrl-imx6q fails to register like below.

syscon 20e0000.iomuxc: syscon regmap start 0x20e0000 end 0x20e3fff registered
imx6q-pinctrl 20e0000.iomuxc: can't request region for resource [mem 0x020e0000-0x020e3fff]
imx6q-pinctrl: probe of 20e0000.iomuxc failed with error -16
Strictly you're not supposed to do that with the device tree. There
shouldn't be two nodes using the same overlapping memory region unless
they are parent/child. That rule has been around for a long time, but
the core never checked for it. What /should/ happen is the two drivers
should be cooperating for the register region and only one device
driver probe sets up both behaviours.
This case was something we both discussed when this patch was first
brough up, and both our tests seemed like it was OK.. What is going on
here that these non-staggered regions are failing? It looks like the
platform devices registered but the devm_request_and_iormap failed?
quoted
quoted
It also breaks all of_amba_device users.

of_amba_device_create() --> amba_device_add() --> request_resource()
and fails.
Presumably that's because we no longer know what the parent resource
is supposed to be?
Hmmm, it looks that way, yes. Currently the OF code is using
iomem_resource as the parent for all amba_device_add() calls
(driver/of/platform.c), but if the parent node gets registered as a
platform device and it has the resources then the parenthood chain
doesn't match up. It isn't immediately obvious to me how to fix this.
I'm going to drop the patch from my tree. I could use some help
figuring out what the correct behaviour really should be here.
I looked for a bit and it wasn't obvious to me where the colliding
request_resource was coming from. The DTs for amba busses seem to all
be placed under the root node, or within a simple bus, so there is not
parent platform device and the use of iomem_resource should still be OK?

Jason
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help