Re: [PATCH 2/2] of: use platform_device_add

[Jason Gunthorpe]

On Sun, Feb 17, 2013 at 10:49:08AM +0000, Grant Likely wrote:
> > > 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@020e0000 {
> > >                 compatible = "fsl,imx6q-iomuxc-gpr", "syscon";
> > >                 reg = <0x020e0000 0x38>;
> > >         };
> > >
> > >         iomuxc: iomuxc@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?

> >> 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
--
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/