Problem with /proc/iomem on ARM

Problem with /proc/iomem on ARM

[H Hartley Sweeten]

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 <kay.sievers@vrfy.org>
Date:   Tue Jan 6 10:44:43 2009 -0800

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

    Cc: Russell King <rmk@arm.linux.org.uk>
    Signed-off-by: Kay Sievers <kay.sievers@vrfy.org>
    Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

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

Re: Problem with /proc/iomem on ARM

[Kay Sievers]

On Wed, Jul 1, 2009 at 19:32, H Hartley
Sweeten<hartleys@visionengravers.com> 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 <kay.sievers@vrfy.org>
> Date:   Tue Jan 6 10:44:43 2009 -0800
>
>    arm: struct device - replace bus_id with dev_name(), dev_set_name()
>
>    Cc: Russell King <rmk@arm.linux.org.uk>
>    Signed-off-by: Kay Sievers <kay.sievers@vrfy.org>
>    Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

> 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

RE: Problem with /proc/iomem on ARM

[H Hartley Sweeten]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1461 bytes --]

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 <kay.sievers@vrfy.org>
>> Date:   Tue Jan 6 10:44:43 2009 -0800
>>
>>    arm: struct device - replace bus_id with dev_name(), dev_set_name()
>>
>>    Cc: Russell King <rmk@arm.linux.org.uk>
>>    Signed-off-by: Kay Sievers <kay.sievers@vrfy.org>
>>    Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
>
>> 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Ç+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: Problem with /proc/iomem on ARM

[Kay Sievers]

On Wed, Jul 1, 2009 at 20:01, H Hartley
Sweeten<hartleys@visionengravers.com> 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

RE: Problem with /proc/iomem on ARM

[H Hartley Sweeten]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1675 bytes --]

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 <hsweeten@visionengravers.com>

---

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Ç+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: Problem with /proc/iomem on ARM

[Kay Sievers]

On Wed, Jul 1, 2009 at 20:24, H Hartley
Sweeten<hartleys@visionengravers.com> 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

RE: Problem with /proc/iomem on ARM

[H Hartley Sweeten]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1767 bytes --]

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 <rmk@dyn-67.arm.linux.org.uk>
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 <rmk+kernel@arm.linux.org.uk>

Thanks,
Hartley
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥