2009-07-01 17:32:30

by Hartley Sweeten

[permalink] [raw]
Subject: Problem with /proc/iomem on ARM

Hello Kay,

I just noticed a problem with /proc/iomem on my ARM system
that I think was introduced by your patch:

commit 1d559e29138834bbcdf34ac072232bf543bfc4e0
Author: Kay Sievers <[email protected]>
Date: Tue Jan 6 10:44:43 2009 -0800

arm: struct device - replace bus_id with dev_name(), dev_set_name()

Cc: Russell King <[email protected]>
Signed-off-by: Kay Sievers <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>

On my ep93xx ARM system I have three pl010 uarts: "apb:uart1",
"apb:uart2", and "apb:uart3". For /proc/iomem I used to see:

/ # cat proc/iomem
...
808c0000-808c0fff : apb:uart1
808c0000-808c003f : uart-pl010
808d0000-808d0fff : apb:uart2
808d0000-808d003f : uart-pl010
808e0000-808e0fff : apb:uart3
808e0000-808e003f : uart-pl010
...

Now it shows up as:

/ # cat proc/iomem
...
808c0000-808c0fff : <BAD>
808c0000-808c003f : uart-pl010
808d0000-808d0fff : <BAD>
808d0000-808d003f : uart-pl010
808e0000-808e0fff : <BAD>
808e0000-808e003f : uart-pl010
...

Is this a regression or is there something else going on?

This was seen with kernel 2.6.30. I havn't looked at /proc/iomem for
a while so I'm not sure when this first started.

Regards,
Hartley


2009-07-01 17:55:26

by Kay Sievers

[permalink] [raw]
Subject: Re: Problem with /proc/iomem on ARM

On Wed, Jul 1, 2009 at 19:32, H Hartley
Sweeten<[email protected]> wrote:
> I just noticed a problem with /proc/iomem on my ARM system
> that I think was introduced by your patch:
>
> commit 1d559e29138834bbcdf34ac072232bf543bfc4e0
> Author: Kay Sievers <[email protected]>
> Date:   Tue Jan 6 10:44:43 2009 -0800
>
>    arm: struct device - replace bus_id with dev_name(), dev_set_name()
>
>    Cc: Russell King <[email protected]>
>    Signed-off-by: Kay Sievers <[email protected]>
>    Signed-off-by: Greg Kroah-Hartman <[email protected]>

> This was seen with kernel 2.6.30.  I havn't looked at /proc/iomem for
> a while so I'm not sure when this first started.

Are you sure, 8a577ffc75d9194fe8cdb7479236f2081c26ca1f isn't in your tree?

After all, it seems like this should be fixed in arm somewhere, not to
copy and store internal driver core pointers, but use the device
itself to retrieve the values.

Thanks,
Kay

2009-07-01 18:01:23

by Hartley Sweeten

[permalink] [raw]
Subject: RE: Problem with /proc/iomem on ARM

On Wednesday, July 01, 2009 10:55 AM, Kay Sievers wrote:
> On Wed, Jul 1, 2009 at 19:32, H Hartley wrote:
>> I just noticed a problem with /proc/iomem on my ARM system
>> that I think was introduced by your patch:
>>
>> commit 1d559e29138834bbcdf34ac072232bf543bfc4e0
>> Author: Kay Sievers <[email protected]>
>> Date:   Tue Jan 6 10:44:43 2009 -0800
>>
>>    arm: struct device - replace bus_id with dev_name(), dev_set_name()
>>
>>    Cc: Russell King <[email protected]>
>>    Signed-off-by: Kay Sievers <[email protected]>
>>    Signed-off-by: Greg Kroah-Hartman <[email protected]>
>
>> This was seen with kernel 2.6.30.  I havn't looked at /proc/iomem for
>> a while so I'm not sure when this first started.
>
> Are you sure, 8a577ffc75d9194fe8cdb7479236f2081c26ca1f isn't in your tree?

That commit is in my tree. It appears that commit only fixes pci devices.
The serial ports on my system are amba bus devices.

> After all, it seems like this should be fixed in arm somewhere, not to
> copy and store internal driver core pointers, but use the device
> itself to retrieve the values.

Maybe the problem is in drivers/amba/bus.c? amba_device_register() does:

dev->res.name = dev_name(&dev->dev);

But there is nothing in the code about setting the kobj.

Regards,
Hartley
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2009-07-01 18:13:05

by Kay Sievers

[permalink] [raw]
Subject: Re: Problem with /proc/iomem on ARM

On Wed, Jul 1, 2009 at 20:01, H Hartley
Sweeten<[email protected]> wrote:

> The serial ports on my system are amba bus devices.
>
>> After all, it seems like this should be fixed in arm somewhere, not to
>> copy and store internal driver core pointers, but use the device
>> itself to retrieve the values.
>
> Maybe the problem is in drivers/amba/bus.c? amba_device_register() does:
>
>        dev->res.name = dev_name(&dev->dev);

Looks like. If you get the name directly from dev->init_name. Does that work?

Thanks,
Kay

2009-07-01 18:25:06

by Hartley Sweeten

[permalink] [raw]
Subject: RE: Problem with /proc/iomem on ARM

On Wednesday, July 01, 2009 11:13 AM, Kay Sievers wrote:
>> The serial ports on my system are amba bus devices.
>>
>>> After all, it seems like this should be fixed in arm somewhere, not to
>>> copy and store internal driver core pointers, but use the device
>>> itself to retrieve the values.
>>
>> Maybe the problem is in drivers/amba/bus.c? amba_device_register() does:
>>
>>        dev->res.name = dev_name(&dev->dev);
>
> Looks like. If you get the name directly from dev->init_name. Does that work?
>

The following patch does fix /proc/iomem for the amba uarts on my system.
Is this the correct approach to fixing the issue?

Regards,
Hartley

---

amba: set resource init_name

Noticed on ep93xx ARM system, /proc/iomem is missing the names for amba bus
devices.

I appears that drivers/amba/bus.c does not set the kobject to allow dev_name()
to return the device name. Just use the init_name directly.

Signed-off-by: H Hartley Sweeten <[email protected]>

---

diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
index 3d763fd..9bfb9f7 100644
--- a/drivers/amba/bus.c
+++ b/drivers/amba/bus.c
@@ -210,7 +210,7 @@ int amba_device_register(struct amba_device *dev, struct resource *parent)
dev->dev.release = amba_device_release;
dev->dev.bus = &amba_bustype;
dev->dev.dma_mask = &dev->dma_mask;
- dev->res.name = dev_name(&dev->dev);
+ dev->res.name = dev->dev.init_name;

if (!dev->dev.coherent_dma_mask && dev->dma_mask)
dev_warn(&dev->dev, "coherent dma mask is unset\n");
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2009-07-01 18:47:18

by Kay Sievers

[permalink] [raw]
Subject: Re: Problem with /proc/iomem on ARM

On Wed, Jul 1, 2009 at 20:24, H Hartley
Sweeten<[email protected]> wrote:
> On Wednesday, July 01, 2009 11:13 AM, Kay Sievers wrote:
>>> The serial ports on my system are amba bus devices.
>>>
>>>> After all, it seems like this should be fixed in arm somewhere, not to
>>>> copy and store internal driver core pointers, but use the device
>>>> itself to retrieve the values.
>>>
>>> Maybe the problem is in drivers/amba/bus.c? amba_device_register() does:
>>>
>>>        dev->res.name = dev_name(&dev->dev);
>>
>> Looks like. If you get the name directly from dev->init_name. Does that work?
>>
>
> The following patch does fix /proc/iomem for the amba uarts on my system.
> Is this the correct approach to fixing the issue?

Looks fine to work around the issue that statically allocated struct
devices can cause. The proper fix would be to convert them to dynamic
objects and get rid of .init_name entirely.

Thanks,
Kay

2009-07-08 22:10:08

by Hartley Sweeten

[permalink] [raw]
Subject: RE: Problem with /proc/iomem on ARM

On Wednesday, July 01, 2009 11:47 AM, Kay Sievers wrote:
> On Wed, Jul 1, 2009 at 20:24, H Hartley Sweeten wrote:
>> On Wednesday, July 01, 2009 11:13 AM, Kay Sievers wrote:
>>>> The serial ports on my system are amba bus devices.
>>>>
>>>>> After all, it seems like this should be fixed in arm somewhere,
>>>>> not to copy and store internal driver core pointers, but use the
>>>>> device itself to retrieve the values.
>>>>
>>>> Maybe the problem is in drivers/amba/bus.c? amba_device_register()
>>>> does:
>>>>
>>>>        dev->res.name = dev_name(&dev->dev);
>>>
>>> Looks like. If you get the name directly from dev->init_name. Does
>>> that work?
>>>
>>
>> The following patch does fix /proc/iomem for the amba uarts on my
>> system. Is this the correct approach to fixing the issue?
>
> Looks fine to work around the issue that statically allocated struct
> devices can cause. The proper fix would be to convert them to dynamic
> objects and get rid of .init_name entirely.

Disregard.

Russell King added a patch that fixed the issue. I just showed up.

commit 557dca5f48a45df88a73e69ee0700cfd4e2358c9
Author: Russell King <[email protected]>
Date: Sun Jul 5 22:39:08 2009 +0100

[ARM] amba: fix amba device resources

AMBA device resources were being reported as:

10004000-10004fff : <BAD>

This is because dev_name() was returning NULL prior to device_register.
Ensure that the struct device is properly initialized, and the name is
set before adding it to the device tree.

Signed-off-by: Russell King <[email protected]>

Thanks,
Hartley
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?