This allows platform_device_add a chance to call insert_resource
on all of the resources from OF. At a minimum this fills in proc/iomem
and presumably makes resource tracking and conflict detection work
better.
Signed-off-by: Jason Gunthorpe <[email protected]>
---
drivers/of/device.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
Tested on PPC32 and ARM32 embedded kernels.
diff --git a/drivers/of/device.c b/drivers/of/device.c
index 4c74e4f..a5b67dc 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -62,7 +62,7 @@ int of_device_add(struct platform_device *ofdev)
if (!ofdev->dev.parent)
set_dev_node(&ofdev->dev, of_node_to_nid(ofdev->dev.of_node));
- return device_add(&ofdev->dev);
+ return platform_device_add(ofdev);
}
int of_device_register(struct platform_device *pdev)
--
1.7.4.1
On Wed, 21 Nov 2012 00:24:48 -0700, Jason Gunthorpe <[email protected]> wrote:
> This allows platform_device_add a chance to call insert_resource
> on all of the resources from OF. At a minimum this fills in proc/iomem
> and presumably makes resource tracking and conflict detection work
> better.
>
> Signed-off-by: Jason Gunthorpe <[email protected]>
> ---
> drivers/of/device.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> Tested on PPC32 and ARM32 embedded kernels.
>
> diff --git a/drivers/of/device.c b/drivers/of/device.c
> index 4c74e4f..a5b67dc 100644
> --- a/drivers/of/device.c
> +++ b/drivers/of/device.c
> @@ -62,7 +62,7 @@ int of_device_add(struct platform_device *ofdev)
> if (!ofdev->dev.parent)
> set_dev_node(&ofdev->dev, of_node_to_nid(ofdev->dev.of_node));
>
> - return device_add(&ofdev->dev);
> + return platform_device_add(ofdev);
> }
>
> int of_device_register(struct platform_device *pdev)
This has the side effect of moving all devices at the root of the tree
from /sys/devices/ to /sys/devices/platform. It also has the possibility
of breaking if any devices get registered with overlapping regions. I
think there are some powerpc 5200 boards that do this, and I'm not sure
about the larger Power boxen.
I've got a more nuanced version of this patch that I'm trying to get
published today for review. I'll add you to the cc list.
g.
> --
> 1.7.4.1
>
--
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.
On Wed, Nov 21, 2012 at 3:51 PM, Grant Likely <[email protected]> wrote:
> On Wed, 21 Nov 2012 00:24:48 -0700, Jason Gunthorpe <[email protected]> wrote:
>> This allows platform_device_add a chance to call insert_resource
>> on all of the resources from OF. At a minimum this fills in proc/iomem
>> and presumably makes resource tracking and conflict detection work
>> better.
>>
>> Signed-off-by: Jason Gunthorpe <[email protected]>
>> ---
>> drivers/of/device.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> Tested on PPC32 and ARM32 embedded kernels.
>>
>> diff --git a/drivers/of/device.c b/drivers/of/device.c
>> index 4c74e4f..a5b67dc 100644
>> --- a/drivers/of/device.c
>> +++ b/drivers/of/device.c
>> @@ -62,7 +62,7 @@ int of_device_add(struct platform_device *ofdev)
>> if (!ofdev->dev.parent)
>> set_dev_node(&ofdev->dev, of_node_to_nid(ofdev->dev.of_node));
>>
>> - return device_add(&ofdev->dev);
>> + return platform_device_add(ofdev);
>> }
>>
>> int of_device_register(struct platform_device *pdev)
>
> This has the side effect of moving all devices at the root of the tree
> from /sys/devices/ to /sys/devices/platform. It also has the possibility
> of breaking if any devices get registered with overlapping regions. I
> think there are some powerpc 5200 boards that do this, and I'm not sure
> about the larger Power boxen.
>
> I've got a more nuanced version of this patch that I'm trying to get
> published today for review. I'll add you to the cc list.
However, while on this topic;
Ben. Do you have any objections to registering all OF generated
platform devices under /sys/devices/platform instead of /sys/devices?
I don't much like the platform directory, but it would make the code
more consistent for DT and non-DT users.
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
On Wed, Nov 21, 2012 at 03:51:04PM +0000, Grant Likely wrote:
> On Wed, 21 Nov 2012 00:24:48 -0700, Jason Gunthorpe <[email protected]> wrote:
> > This allows platform_device_add a chance to call insert_resource
> > on all of the resources from OF. At a minimum this fills in proc/iomem
> > and presumably makes resource tracking and conflict detection work
> > better.
> >
> > Signed-off-by: Jason Gunthorpe <[email protected]>
> > drivers/of/device.c | 2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > Tested on PPC32 and ARM32 embedded kernels.
> >
> > diff --git a/drivers/of/device.c b/drivers/of/device.c
> > index 4c74e4f..a5b67dc 100644
> > +++ b/drivers/of/device.c
> > @@ -62,7 +62,7 @@ int of_device_add(struct platform_device *ofdev)
> > if (!ofdev->dev.parent)
> > set_dev_node(&ofdev->dev, of_node_to_nid(ofdev->dev.of_node));
> >
> > - return device_add(&ofdev->dev);
> > + return platform_device_add(ofdev);
> > }
> >
> > int of_device_register(struct platform_device *pdev)
>
> This has the side effect of moving all devices at the root of the tree
> from /sys/devices/ to /sys/devices/platform. It also has the possibility
> of breaking if any devices get registered with overlapping regions. I
> think there are some powerpc 5200 boards that do this, and I'm not sure
> about the larger Power boxen.
Okay, I'll try to test your patch.
I know sensible overlapping seems to work:
e0000000-e7ffffff : PCIe 0 MEM
e0000000-e000ffff : 0000:00:01.0
e0000000-e0000fff : /pex@e0000000/chip@0/chip_control@0
e0000008-e000000b : dat
e0000008-e000000b : dat
e000000c-e000000f : set
e000000c-e000000f : set
e0000010-e0000013 : dirin
e0000010-e0000013 : dirin
Which is nesting the generic gpio driver under a larger region..
And:
f1010100-f101013f : /internal@f1000000/gpio@10100
f1010100-f1010103 : /internal@f1000000/chip_cfg@0
Which is nesting a register set for one OF platform_device under
another.
It seems to be handled automatically.
My motivations are two fold:
- Having a mostly empty /proc/iomem isn't great for diagnostics
- I'll send a patch today that adds some more code to
platform_device_add
Thanks Grant,
Jason
On Wed, Nov 21, 2012 at 5:44 PM, Jason Gunthorpe
<[email protected]> wrote:
> On Wed, Nov 21, 2012 at 03:51:04PM +0000, Grant Likely wrote:
>> On Wed, 21 Nov 2012 00:24:48 -0700, Jason Gunthorpe <[email protected]> wrote:
>> > This allows platform_device_add a chance to call insert_resource
>> > on all of the resources from OF. At a minimum this fills in proc/iomem
>> > and presumably makes resource tracking and conflict detection work
>> > better.
>> >
>> > Signed-off-by: Jason Gunthorpe <[email protected]>
>> > drivers/of/device.c | 2 +-
>> > 1 files changed, 1 insertions(+), 1 deletions(-)
>> >
>> > Tested on PPC32 and ARM32 embedded kernels.
>> >
>> > diff --git a/drivers/of/device.c b/drivers/of/device.c
>> > index 4c74e4f..a5b67dc 100644
>> > +++ b/drivers/of/device.c
>> > @@ -62,7 +62,7 @@ int of_device_add(struct platform_device *ofdev)
>> > if (!ofdev->dev.parent)
>> > set_dev_node(&ofdev->dev, of_node_to_nid(ofdev->dev.of_node));
>> >
>> > - return device_add(&ofdev->dev);
>> > + return platform_device_add(ofdev);
>> > }
>> >
>> > int of_device_register(struct platform_device *pdev)
>>
>> This has the side effect of moving all devices at the root of the tree
>> from /sys/devices/ to /sys/devices/platform. It also has the possibility
>> of breaking if any devices get registered with overlapping regions. I
>> think there are some powerpc 5200 boards that do this, and I'm not sure
>> about the larger Power boxen.
>
> Okay, I'll try to test your patch.
>
> I know sensible overlapping seems to work:
>
> e0000000-e7ffffff : PCIe 0 MEM
> e0000000-e000ffff : 0000:00:01.0
> e0000000-e0000fff : /pex@e0000000/chip@0/chip_control@0
> e0000008-e000000b : dat
> e0000008-e000000b : dat
> e000000c-e000000f : set
> e000000c-e000000f : set
> e0000010-e0000013 : dirin
> e0000010-e0000013 : dirin
>
> Which is nesting the generic gpio driver under a larger region..
Try two sibling nodes with overlapping addresses. There are powerpc
device trees doing that even though it isn't legal by the ofw and
epapr specs.
g.
On Wed, Nov 21, 2012 at 06:07:46PM +0000, Grant Likely wrote:
> > Which is nesting the generic gpio driver under a larger region..
>
> Try two sibling nodes with overlapping addresses. There are powerpc
> device trees doing that even though it isn't legal by the ofw and
> epapr specs.
Both my examples were using sibling nodes in the OF tree.
pex@e0000000 {
device_type = "pci";
ranges = <0x02000000 0x00000000 0x00000000 0xe0000000 0x0 0x8000000>;
bus-range = <0x0 0xFF>;
chip@0 {
ranges = <0x02000000 0x00000000 0x00000000 0x02000000 0x00000000 0x00000000 0x0 0x8000000>;
chip_control@0 {
compatible = "orc,chip,control";
assigned-addresses = <0x02000000 0x0 0x0 0x0 4096>;
};
gpio3: chip_gpio@8 {
#gpio-cells = <2>;
compatible = "linux,basic-mmio-gpio";
gpio-controller;
reg-names = "dat", "set", "dirin";
assigned-addresses = <0x02000000 0x0 0x8 0x0 4>,
<0x02000000 0x0 0xc 0x0 4>,
<0x02000000 0x0 0x10 0x0 4>;
};
Non-conformant yes, but it is the simplest way to get linux to bind
two drivers to the same memory space.
Jason
On Thu, Nov 22, 2012 at 03:36:21PM +0000, Grant Likely wrote:
> Hmm... I've not tried it with assigned-address. I tried with two sibling
> platform devices using just the 'reg' property. That the kernel will
> complain about. For powerpc-only, the patch I posted allows the device
> to get registered anyway even though the range incorrectly overlaps.
My second example was done with the reg property..
gpio0: gpio@10100 {
compatible = "marvell,orion-gpio";
#gpio-cells = <2>;
gpio-controller;
reg = <0x10100 0x40>;
}
chip_cfg@0 {
compatible = "orc,chip_config";
// Doubles up on gpio0
reg = <0x10100 0x4>;
};
f1010100-f101013f : /internal@f1000000/gpio@10100
f1010100-f1010103 : /internal@f1000000/chip_cfg@0
What did you try? Maybe order matters?
Regards,
Jason
On Wed, 21 Nov 2012 11:14:30 -0700, Jason Gunthorpe <[email protected]> wrote:
> On Wed, Nov 21, 2012 at 06:07:46PM +0000, Grant Likely wrote:
>
> > > Which is nesting the generic gpio driver under a larger region..
> >
> > Try two sibling nodes with overlapping addresses. There are powerpc
> > device trees doing that even though it isn't legal by the ofw and
> > epapr specs.
>
> Both my examples were using sibling nodes in the OF tree.
>
> pex@e0000000 {
> device_type = "pci";
> ranges = <0x02000000 0x00000000 0x00000000 0xe0000000 0x0 0x8000000>;
> bus-range = <0x0 0xFF>;
> chip@0 {
> ranges = <0x02000000 0x00000000 0x00000000 0x02000000 0x00000000 0x00000000 0x0 0x8000000>;
> chip_control@0 {
> compatible = "orc,chip,control";
> assigned-addresses = <0x02000000 0x0 0x0 0x0 4096>;
> };
>
> gpio3: chip_gpio@8 {
> #gpio-cells = <2>;
> compatible = "linux,basic-mmio-gpio";
> gpio-controller;
> reg-names = "dat", "set", "dirin";
> assigned-addresses = <0x02000000 0x0 0x8 0x0 4>,
> <0x02000000 0x0 0xc 0x0 4>,
> <0x02000000 0x0 0x10 0x0 4>;
> };
>
> Non-conformant yes, but it is the simplest way to get linux to bind
> two drivers to the same memory space.
Hmm... I've not tried it with assigned-address. I tried with two sibling
platform devices using just the 'reg' property. That the kernel will
complain about. For powerpc-only, the patch I posted allows the device
to get registered anyway even though the range incorrectly overlaps.
g.
On Thu, 22 Nov 2012 10:30:20 -0700, Jason Gunthorpe <[email protected]> wrote:
> On Thu, Nov 22, 2012 at 03:36:21PM +0000, Grant Likely wrote:
>
> > Hmm... I've not tried it with assigned-address. I tried with two sibling
> > platform devices using just the 'reg' property. That the kernel will
> > complain about. For powerpc-only, the patch I posted allows the device
> > to get registered anyway even though the range incorrectly overlaps.
>
> My second example was done with the reg property..
>
> gpio0: gpio@10100 {
> compatible = "marvell,orion-gpio";
> #gpio-cells = <2>;
> gpio-controller;
> reg = <0x10100 0x40>;
> }
> chip_cfg@0 {
> compatible = "orc,chip_config";
> // Doubles up on gpio0
> reg = <0x10100 0x4>;
> };
>
>
> f1010100-f101013f : /internal@f1000000/gpio@10100
> f1010100-f1010103 : /internal@f1000000/chip_cfg@0
>
> What did you try? Maybe order matters?
It might. I tried on qemu with versatile. I've written a new test block
with different overlaps. Here's the block and the results:
dummy@10201000 {
compatible = "acme,test";
reg = <0x10201000 0x1000>;
};
overlap@10200800 {
compatible = "acme,test";
reg = <0x10200800 0x1000>;
};
overlap@10201800 {
compatible = "acme,test";
reg = <0x10201800 0x1000>;
};
overlap@10201400 {
compatible = "acme,test";
reg = <0x10201400 0x800>;
};
overlap@10200c00 {
compatible = "acme,test";
reg = <0x10200c00 0x1800>;
};
>From the kernel log:
10200800.overlap: failed to claim resource 0
10201800.overlap: failed to claim resource 0
# ls /sys/bus/platform/devices/
10002000.i2c 10010000.net 10201000.dummy alarmtimer
10003000.intc 10140000.intc 10201400.overlap amba.0
10008000.lcd 10200c00.overlap 34000000.flash fpga.1
So, overlaps that are completely inside or completely outside the
already registered range don't appear to be detected. That may be a bug
(unless it is designed to work that way)
g.
--
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.
On Thu, 22 Nov 2012 10:30:20 -0700, Jason Gunthorpe <[email protected]> wrote:
> On Thu, Nov 22, 2012 at 03:36:21PM +0000, Grant Likely wrote:
>
> > Hmm... I've not tried it with assigned-address. I tried with two sibling
> > platform devices using just the 'reg' property. That the kernel will
> > complain about. For powerpc-only, the patch I posted allows the device
> > to get registered anyway even though the range incorrectly overlaps.
>
> My second example was done with the reg property..
>
> gpio0: gpio@10100 {
> compatible = "marvell,orion-gpio";
> #gpio-cells = <2>;
> gpio-controller;
> reg = <0x10100 0x40>;
> }
> chip_cfg@0 {
> compatible = "orc,chip_config";
> // Doubles up on gpio0
> reg = <0x10100 0x4>;
> };
>
>
> f1010100-f101013f : /internal@f1000000/gpio@10100
> f1010100-f1010103 : /internal@f1000000/chip_cfg@0
>
> What did you try? Maybe order matters?
(Sorry for not replying to my own mail; I'm doing this offline and my
sent mail doesn't show)
Looks like it is by design. With my dummy devices I see this:
10200c00-102023ff : /amba/overlap@10200c00
10201000-10201fff : /amba/dummy@10201000
10201400-10201bff : /amba/overlap@10201400
All three of those devices are siblings in the device tree.
g.