2020-05-31 18:10:48

by Vladimir Oltean

[permalink] [raw]
Subject: [PATCH v2] devres: keep both device name and resource name in pretty name

From: Vladimir Oltean <[email protected]>

Sometimes debugging a device is easiest using devmem on its register
map, and that can be seen with /proc/iomem. But some device drivers have
many memory regions. Take for example a networking switch. Its memory
map used to look like this in /proc/iomem:

1fc000000-1fc3fffff : pcie@1f0000000
1fc000000-1fc3fffff : 0000:00:00.5
1fc010000-1fc01ffff : sys
1fc030000-1fc03ffff : rew
1fc060000-1fc0603ff : s2
1fc070000-1fc0701ff : devcpu_gcb
1fc080000-1fc0800ff : qs
1fc090000-1fc0900cb : ptp
1fc100000-1fc10ffff : port0
1fc110000-1fc11ffff : port1
1fc120000-1fc12ffff : port2
1fc130000-1fc13ffff : port3
1fc140000-1fc14ffff : port4
1fc150000-1fc15ffff : port5
1fc200000-1fc21ffff : qsys
1fc280000-1fc28ffff : ana

But after the patch in Fixes: was applied, the information is now
presented in a much more opaque way:

1fc000000-1fc3fffff : pcie@1f0000000
1fc000000-1fc3fffff : 0000:00:00.5
1fc010000-1fc01ffff : 0000:00:00.5
1fc030000-1fc03ffff : 0000:00:00.5
1fc060000-1fc0603ff : 0000:00:00.5
1fc070000-1fc0701ff : 0000:00:00.5
1fc080000-1fc0800ff : 0000:00:00.5
1fc090000-1fc0900cb : 0000:00:00.5
1fc100000-1fc10ffff : 0000:00:00.5
1fc110000-1fc11ffff : 0000:00:00.5
1fc120000-1fc12ffff : 0000:00:00.5
1fc130000-1fc13ffff : 0000:00:00.5
1fc140000-1fc14ffff : 0000:00:00.5
1fc150000-1fc15ffff : 0000:00:00.5
1fc200000-1fc21ffff : 0000:00:00.5
1fc280000-1fc28ffff : 0000:00:00.5

That patch made a fair comment that /proc/iomem might be confusing when
it shows resources without an associated device, but we can do better
than just hide the resource name altogether. Namely, we can print the
device name _and_ the resource name. Like this:

1fc000000-1fc3fffff : pcie@1f0000000
1fc000000-1fc3fffff : 0000:00:00.5
1fc010000-1fc01ffff : 0000:00:00.5 sys
1fc030000-1fc03ffff : 0000:00:00.5 rew
1fc060000-1fc0603ff : 0000:00:00.5 s2
1fc070000-1fc0701ff : 0000:00:00.5 devcpu_gcb
1fc080000-1fc0800ff : 0000:00:00.5 qs
1fc090000-1fc0900cb : 0000:00:00.5 ptp
1fc100000-1fc10ffff : 0000:00:00.5 port0
1fc110000-1fc11ffff : 0000:00:00.5 port1
1fc120000-1fc12ffff : 0000:00:00.5 port2
1fc130000-1fc13ffff : 0000:00:00.5 port3
1fc140000-1fc14ffff : 0000:00:00.5 port4
1fc150000-1fc15ffff : 0000:00:00.5 port5
1fc200000-1fc21ffff : 0000:00:00.5 qsys
1fc280000-1fc28ffff : 0000:00:00.5 ana

Fixes: 8d84b18f5678 ("devres: always use dev_name() in devm_ioremap_resource()")
Signed-off-by: Vladimir Oltean <[email protected]>
---
lib/devres.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/lib/devres.c b/lib/devres.c
index 6ef51f159c54..3d67588c15a7 100644
--- a/lib/devres.c
+++ b/lib/devres.c
@@ -119,6 +119,7 @@ __devm_ioremap_resource(struct device *dev, const struct resource *res,
{
resource_size_t size;
void __iomem *dest_ptr;
+ char *pretty_name;

BUG_ON(!dev);

@@ -129,7 +130,21 @@ __devm_ioremap_resource(struct device *dev, const struct resource *res,

size = resource_size(res);

- if (!devm_request_mem_region(dev, res->start, size, dev_name(dev))) {
+ if (res->name) {
+ int len = strlen(dev_name(dev)) + strlen(res->name) + 2;
+
+ pretty_name = devm_kzalloc(dev, len, GFP_KERNEL);
+ if (!pretty_name)
+ return IOMEM_ERR_PTR(-ENOMEM);
+
+ sprintf(pretty_name, "%s %s", dev_name(dev), res->name);
+ } else {
+ pretty_name = devm_kstrdup(dev, dev_name(dev), GFP_KERNEL);
+ if (!pretty_name)
+ return IOMEM_ERR_PTR(-ENOMEM);
+ }
+
+ if (!devm_request_mem_region(dev, res->start, size, pretty_name)) {
dev_err(dev, "can't request region for resource %pR\n", res);
return IOMEM_ERR_PTR(-EBUSY);
}
--
2.25.1


2020-05-31 21:15:52

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH v2] devres: keep both device name and resource name in pretty name

On Mon, 1 Jun 2020 at 00:05, Andy Shevchenko <[email protected]> wrote:
>
>
>
> On Sunday, May 31, 2020, Vladimir Oltean <[email protected]> wrote:
>>
>> From: Vladimir Oltean <[email protected]>
>>
>> Sometimes debugging a device is easiest using devmem on its register
>> map, and that can be seen with /proc/iomem. But some device drivers have
>> many memory regions. Take for example a networking switch. Its memory
>> map used to look like this in /proc/iomem:
>>
>> 1fc000000-1fc3fffff : pcie@1f0000000
>> 1fc000000-1fc3fffff : 0000:00:00.5
>> 1fc010000-1fc01ffff : sys
>> 1fc030000-1fc03ffff : rew
>> 1fc060000-1fc0603ff : s2
>> 1fc070000-1fc0701ff : devcpu_gcb
>> 1fc080000-1fc0800ff : qs
>> 1fc090000-1fc0900cb : ptp
>> 1fc100000-1fc10ffff : port0
>> 1fc110000-1fc11ffff : port1
>> 1fc120000-1fc12ffff : port2
>> 1fc130000-1fc13ffff : port3
>> 1fc140000-1fc14ffff : port4
>> 1fc150000-1fc15ffff : port5
>> 1fc200000-1fc21ffff : qsys
>> 1fc280000-1fc28ffff : ana
>>
>> But after the patch in Fixes: was applied, the information is now
>> presented in a much more opaque way:
>>
>> 1fc000000-1fc3fffff : pcie@1f0000000
>> 1fc000000-1fc3fffff : 0000:00:00.5
>> 1fc010000-1fc01ffff : 0000:00:00.5
>> 1fc030000-1fc03ffff : 0000:00:00.5
>> 1fc060000-1fc0603ff : 0000:00:00.5
>> 1fc070000-1fc0701ff : 0000:00:00.5
>> 1fc080000-1fc0800ff : 0000:00:00.5
>> 1fc090000-1fc0900cb : 0000:00:00.5
>> 1fc100000-1fc10ffff : 0000:00:00.5
>> 1fc110000-1fc11ffff : 0000:00:00.5
>> 1fc120000-1fc12ffff : 0000:00:00.5
>> 1fc130000-1fc13ffff : 0000:00:00.5
>> 1fc140000-1fc14ffff : 0000:00:00.5
>> 1fc150000-1fc15ffff : 0000:00:00.5
>> 1fc200000-1fc21ffff : 0000:00:00.5
>> 1fc280000-1fc28ffff : 0000:00:00.5
>>
>> That patch made a fair comment that /proc/iomem might be confusing when
>> it shows resources without an associated device, but we can do better
>> than just hide the resource name altogether. Namely, we can print the
>> device name _and_ the resource name. Like this:
>>
>> 1fc000000-1fc3fffff : pcie@1f0000000
>> 1fc000000-1fc3fffff : 0000:00:00.5
>> 1fc010000-1fc01ffff : 0000:00:00.5 sys
>> 1fc030000-1fc03ffff : 0000:00:00.5 rew
>> 1fc060000-1fc0603ff : 0000:00:00.5 s2
>> 1fc070000-1fc0701ff : 0000:00:00.5 devcpu_gcb
>> 1fc080000-1fc0800ff : 0000:00:00.5 qs
>> 1fc090000-1fc0900cb : 0000:00:00.5 ptp
>> 1fc100000-1fc10ffff : 0000:00:00.5 port0
>> 1fc110000-1fc11ffff : 0000:00:00.5 port1
>> 1fc120000-1fc12ffff : 0000:00:00.5 port2
>> 1fc130000-1fc13ffff : 0000:00:00.5 port3
>> 1fc140000-1fc14ffff : 0000:00:00.5 port4
>> 1fc150000-1fc15ffff : 0000:00:00.5 port5
>> 1fc200000-1fc21ffff : 0000:00:00.5 qsys
>> 1fc280000-1fc28ffff : 0000:00:00.5 ana
>>
>
> All of this seems an ABI change.
> But also see below.
>

Yes, indeed. What should I understand from your comment though?

>>
>> Fixes: 8d84b18f5678 ("devres: always use dev_name() in devm_ioremap_resource()")
>> Signed-off-by: Vladimir Oltean <[email protected]>
>> ---
>> lib/devres.c | 17 ++++++++++++++++-
>> 1 file changed, 16 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/devres.c b/lib/devres.c
>> index 6ef51f159c54..3d67588c15a7 100644
>> --- a/lib/devres.c
>> +++ b/lib/devres.c
>> @@ -119,6 +119,7 @@ __devm_ioremap_resource(struct device *dev, const struct resource *res,
>> {
>> resource_size_t size;
>> void __iomem *dest_ptr;
>> + char *pretty_name;
>>
>> BUG_ON(!dev);
>>
>> @@ -129,7 +130,21 @@ __devm_ioremap_resource(struct device *dev, const struct resource *res,
>>
>> size = resource_size(res);
>>
>> - if (!devm_request_mem_region(dev, res->start, size, dev_name(dev))) {
>> + if (res->name) {
>> + int len = strlen(dev_name(dev)) + strlen(res->name) + 2;
>> +
>> + pretty_name = devm_kzalloc(dev, len, GFP_KERNEL);
>> + if (!pretty_name)
>> + return IOMEM_ERR_PTR(-ENOMEM);
>> +
>> + sprintf(pretty_name, "%s %s", dev_name(dev), res->name);
>
>
> Reimplementing devm_kasprintf(), why?
>

Mostly because I didn't remember how it was called, thanks for pointing it out.

>>
>> + } else {
>> + pretty_name = devm_kstrdup(dev, dev_name(dev), GFP_KERNEL);
>> + if (!pretty_name)
>> + return IOMEM_ERR_PTR(-ENOMEM);
>> + }
>> +
>> + if (!devm_request_mem_region(dev, res->start, size, pretty_name)) {
>> dev_err(dev, "can't request region for resource %pR\n", res);
>> return IOMEM_ERR_PTR(-EBUSY);
>> }
>> --
>> 2.25.1
>>
>
>
> --
> With Best Regards,
> Andy Shevchenko
>
>

Thanks,
-Vladimir

2020-06-01 07:53:31

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH v2] devres: keep both device name and resource name in pretty name

Hello!

On 31.05.2020 21:07, Vladimir Oltean wrote:

> From: Vladimir Oltean <[email protected]>
>
> Sometimes debugging a device is easiest using devmem on its register
> map, and that can be seen with /proc/iomem. But some device drivers have
> many memory regions. Take for example a networking switch. Its memory
> map used to look like this in /proc/iomem:
>
> 1fc000000-1fc3fffff : pcie@1f0000000
> 1fc000000-1fc3fffff : 0000:00:00.5
> 1fc010000-1fc01ffff : sys
> 1fc030000-1fc03ffff : rew
> 1fc060000-1fc0603ff : s2
> 1fc070000-1fc0701ff : devcpu_gcb
> 1fc080000-1fc0800ff : qs
> 1fc090000-1fc0900cb : ptp
> 1fc100000-1fc10ffff : port0
> 1fc110000-1fc11ffff : port1
> 1fc120000-1fc12ffff : port2
> 1fc130000-1fc13ffff : port3
> 1fc140000-1fc14ffff : port4
> 1fc150000-1fc15ffff : port5
> 1fc200000-1fc21ffff : qsys
> 1fc280000-1fc28ffff : ana
>
> But after the patch in Fixes: was applied, the information is now
> presented in a much more opaque way:
>
> 1fc000000-1fc3fffff : pcie@1f0000000
> 1fc000000-1fc3fffff : 0000:00:00.5
> 1fc010000-1fc01ffff : 0000:00:00.5
> 1fc030000-1fc03ffff : 0000:00:00.5
> 1fc060000-1fc0603ff : 0000:00:00.5
> 1fc070000-1fc0701ff : 0000:00:00.5
> 1fc080000-1fc0800ff : 0000:00:00.5
> 1fc090000-1fc0900cb : 0000:00:00.5
> 1fc100000-1fc10ffff : 0000:00:00.5
> 1fc110000-1fc11ffff : 0000:00:00.5
> 1fc120000-1fc12ffff : 0000:00:00.5
> 1fc130000-1fc13ffff : 0000:00:00.5
> 1fc140000-1fc14ffff : 0000:00:00.5
> 1fc150000-1fc15ffff : 0000:00:00.5
> 1fc200000-1fc21ffff : 0000:00:00.5
> 1fc280000-1fc28ffff : 0000:00:00.5
>
> That patch made a fair comment that /proc/iomem might be confusing when
> it shows resources without an associated device, but we can do better
> than just hide the resource name altogether. Namely, we can print the
> device name _and_ the resource name. Like this:
>
> 1fc000000-1fc3fffff : pcie@1f0000000
> 1fc000000-1fc3fffff : 0000:00:00.5
> 1fc010000-1fc01ffff : 0000:00:00.5 sys
> 1fc030000-1fc03ffff : 0000:00:00.5 rew
> 1fc060000-1fc0603ff : 0000:00:00.5 s2
> 1fc070000-1fc0701ff : 0000:00:00.5 devcpu_gcb
> 1fc080000-1fc0800ff : 0000:00:00.5 qs
> 1fc090000-1fc0900cb : 0000:00:00.5 ptp
> 1fc100000-1fc10ffff : 0000:00:00.5 port0
> 1fc110000-1fc11ffff : 0000:00:00.5 port1
> 1fc120000-1fc12ffff : 0000:00:00.5 port2
> 1fc130000-1fc13ffff : 0000:00:00.5 port3
> 1fc140000-1fc14ffff : 0000:00:00.5 port4
> 1fc150000-1fc15ffff : 0000:00:00.5 port5
> 1fc200000-1fc21ffff : 0000:00:00.5 qsys
> 1fc280000-1fc28ffff : 0000:00:00.5 ana
>
> Fixes: 8d84b18f5678 ("devres: always use dev_name() in devm_ioremap_resource()")
> Signed-off-by: Vladimir Oltean <[email protected]>
[...]

You didn't write the version log -- what changed since v1?

MBR, Sergei

2020-06-01 09:38:32

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH v2] devres: keep both device name and resource name in pretty name

Hi Sergei,

On Mon, 1 Jun 2020 at 10:51, Sergei Shtylyov
<[email protected]> wrote:
>
> Hello!
>
> On 31.05.2020 21:07, Vladimir Oltean wrote:
>
> > From: Vladimir Oltean <[email protected]>
> >
> > Sometimes debugging a device is easiest using devmem on its register
> > map, and that can be seen with /proc/iomem. But some device drivers have
> > many memory regions. Take for example a networking switch. Its memory
> > map used to look like this in /proc/iomem:
> >
> > 1fc000000-1fc3fffff : pcie@1f0000000
> > 1fc000000-1fc3fffff : 0000:00:00.5
> > 1fc010000-1fc01ffff : sys
> > 1fc030000-1fc03ffff : rew
> > 1fc060000-1fc0603ff : s2
> > 1fc070000-1fc0701ff : devcpu_gcb
> > 1fc080000-1fc0800ff : qs
> > 1fc090000-1fc0900cb : ptp
> > 1fc100000-1fc10ffff : port0
> > 1fc110000-1fc11ffff : port1
> > 1fc120000-1fc12ffff : port2
> > 1fc130000-1fc13ffff : port3
> > 1fc140000-1fc14ffff : port4
> > 1fc150000-1fc15ffff : port5
> > 1fc200000-1fc21ffff : qsys
> > 1fc280000-1fc28ffff : ana
> >
> > But after the patch in Fixes: was applied, the information is now
> > presented in a much more opaque way:
> >
> > 1fc000000-1fc3fffff : pcie@1f0000000
> > 1fc000000-1fc3fffff : 0000:00:00.5
> > 1fc010000-1fc01ffff : 0000:00:00.5
> > 1fc030000-1fc03ffff : 0000:00:00.5
> > 1fc060000-1fc0603ff : 0000:00:00.5
> > 1fc070000-1fc0701ff : 0000:00:00.5
> > 1fc080000-1fc0800ff : 0000:00:00.5
> > 1fc090000-1fc0900cb : 0000:00:00.5
> > 1fc100000-1fc10ffff : 0000:00:00.5
> > 1fc110000-1fc11ffff : 0000:00:00.5
> > 1fc120000-1fc12ffff : 0000:00:00.5
> > 1fc130000-1fc13ffff : 0000:00:00.5
> > 1fc140000-1fc14ffff : 0000:00:00.5
> > 1fc150000-1fc15ffff : 0000:00:00.5
> > 1fc200000-1fc21ffff : 0000:00:00.5
> > 1fc280000-1fc28ffff : 0000:00:00.5
> >
> > That patch made a fair comment that /proc/iomem might be confusing when
> > it shows resources without an associated device, but we can do better
> > than just hide the resource name altogether. Namely, we can print the
> > device name _and_ the resource name. Like this:
> >
> > 1fc000000-1fc3fffff : pcie@1f0000000
> > 1fc000000-1fc3fffff : 0000:00:00.5
> > 1fc010000-1fc01ffff : 0000:00:00.5 sys
> > 1fc030000-1fc03ffff : 0000:00:00.5 rew
> > 1fc060000-1fc0603ff : 0000:00:00.5 s2
> > 1fc070000-1fc0701ff : 0000:00:00.5 devcpu_gcb
> > 1fc080000-1fc0800ff : 0000:00:00.5 qs
> > 1fc090000-1fc0900cb : 0000:00:00.5 ptp
> > 1fc100000-1fc10ffff : 0000:00:00.5 port0
> > 1fc110000-1fc11ffff : 0000:00:00.5 port1
> > 1fc120000-1fc12ffff : 0000:00:00.5 port2
> > 1fc130000-1fc13ffff : 0000:00:00.5 port3
> > 1fc140000-1fc14ffff : 0000:00:00.5 port4
> > 1fc150000-1fc15ffff : 0000:00:00.5 port5
> > 1fc200000-1fc21ffff : 0000:00:00.5 qsys
> > 1fc280000-1fc28ffff : 0000:00:00.5 ana
> >
> > Fixes: 8d84b18f5678 ("devres: always use dev_name() in devm_ioremap_resource()")
> > Signed-off-by: Vladimir Oltean <[email protected]>
> [...]
>
> You didn't write the version log -- what changed since v1?
>
> MBR, Sergei

The changes in v2 are that I'm checking for memory allocation errors.

Thanks,
-Vladimir

2020-06-01 09:52:15

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v2] devres: keep both device name and resource name in pretty name

On Mon, Jun 01, 2020 at 12:36:08PM +0300, Vladimir Oltean wrote:
> Hi Sergei,
>
> On Mon, 1 Jun 2020 at 10:51, Sergei Shtylyov
> <[email protected]> wrote:
> >
> > Hello!
> >
> > On 31.05.2020 21:07, Vladimir Oltean wrote:
> >
> > > From: Vladimir Oltean <[email protected]>
> > >
> > > Sometimes debugging a device is easiest using devmem on its register
> > > map, and that can be seen with /proc/iomem. But some device drivers have
> > > many memory regions. Take for example a networking switch. Its memory
> > > map used to look like this in /proc/iomem:
> > >
> > > 1fc000000-1fc3fffff : pcie@1f0000000
> > > 1fc000000-1fc3fffff : 0000:00:00.5
> > > 1fc010000-1fc01ffff : sys
> > > 1fc030000-1fc03ffff : rew
> > > 1fc060000-1fc0603ff : s2
> > > 1fc070000-1fc0701ff : devcpu_gcb
> > > 1fc080000-1fc0800ff : qs
> > > 1fc090000-1fc0900cb : ptp
> > > 1fc100000-1fc10ffff : port0
> > > 1fc110000-1fc11ffff : port1
> > > 1fc120000-1fc12ffff : port2
> > > 1fc130000-1fc13ffff : port3
> > > 1fc140000-1fc14ffff : port4
> > > 1fc150000-1fc15ffff : port5
> > > 1fc200000-1fc21ffff : qsys
> > > 1fc280000-1fc28ffff : ana
> > >
> > > But after the patch in Fixes: was applied, the information is now
> > > presented in a much more opaque way:
> > >
> > > 1fc000000-1fc3fffff : pcie@1f0000000
> > > 1fc000000-1fc3fffff : 0000:00:00.5
> > > 1fc010000-1fc01ffff : 0000:00:00.5
> > > 1fc030000-1fc03ffff : 0000:00:00.5
> > > 1fc060000-1fc0603ff : 0000:00:00.5
> > > 1fc070000-1fc0701ff : 0000:00:00.5
> > > 1fc080000-1fc0800ff : 0000:00:00.5
> > > 1fc090000-1fc0900cb : 0000:00:00.5
> > > 1fc100000-1fc10ffff : 0000:00:00.5
> > > 1fc110000-1fc11ffff : 0000:00:00.5
> > > 1fc120000-1fc12ffff : 0000:00:00.5
> > > 1fc130000-1fc13ffff : 0000:00:00.5
> > > 1fc140000-1fc14ffff : 0000:00:00.5
> > > 1fc150000-1fc15ffff : 0000:00:00.5
> > > 1fc200000-1fc21ffff : 0000:00:00.5
> > > 1fc280000-1fc28ffff : 0000:00:00.5
> > >
> > > That patch made a fair comment that /proc/iomem might be confusing when
> > > it shows resources without an associated device, but we can do better
> > > than just hide the resource name altogether. Namely, we can print the
> > > device name _and_ the resource name. Like this:
> > >
> > > 1fc000000-1fc3fffff : pcie@1f0000000
> > > 1fc000000-1fc3fffff : 0000:00:00.5
> > > 1fc010000-1fc01ffff : 0000:00:00.5 sys
> > > 1fc030000-1fc03ffff : 0000:00:00.5 rew
> > > 1fc060000-1fc0603ff : 0000:00:00.5 s2
> > > 1fc070000-1fc0701ff : 0000:00:00.5 devcpu_gcb
> > > 1fc080000-1fc0800ff : 0000:00:00.5 qs
> > > 1fc090000-1fc0900cb : 0000:00:00.5 ptp
> > > 1fc100000-1fc10ffff : 0000:00:00.5 port0
> > > 1fc110000-1fc11ffff : 0000:00:00.5 port1
> > > 1fc120000-1fc12ffff : 0000:00:00.5 port2
> > > 1fc130000-1fc13ffff : 0000:00:00.5 port3
> > > 1fc140000-1fc14ffff : 0000:00:00.5 port4
> > > 1fc150000-1fc15ffff : 0000:00:00.5 port5
> > > 1fc200000-1fc21ffff : 0000:00:00.5 qsys
> > > 1fc280000-1fc28ffff : 0000:00:00.5 ana
> > >
> > > Fixes: 8d84b18f5678 ("devres: always use dev_name() in devm_ioremap_resource()")
> > > Signed-off-by: Vladimir Oltean <[email protected]>
> > [...]
> >
> > You didn't write the version log -- what changed since v1?
> >
> > MBR, Sergei
>
> The changes in v2 are that I'm checking for memory allocation errors.

You always need to mention that below the --- line, as the
documentation says to.

Please send a v3 with that fixed up.

thanks,

greg k-h

2020-06-01 10:44:24

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v2] devres: keep both device name and resource name in pretty name

On Mon, Jun 1, 2020 at 12:13 AM Vladimir Oltean <[email protected]> wrote:
> On Mon, 1 Jun 2020 at 00:05, Andy Shevchenko <[email protected]> wrote:
> > On Sunday, May 31, 2020, Vladimir Oltean <[email protected]> wrote:

> >> Sometimes debugging a device is easiest using devmem on its register
> >> map, and that can be seen with /proc/iomem. But some device drivers have
> >> many memory regions. Take for example a networking switch. Its memory
> >> map used to look like this in /proc/iomem:
> >>
> >> 1fc000000-1fc3fffff : pcie@1f0000000
> >> 1fc000000-1fc3fffff : 0000:00:00.5
> >> 1fc010000-1fc01ffff : sys
> >> 1fc030000-1fc03ffff : rew
> >> 1fc060000-1fc0603ff : s2
> >> 1fc070000-1fc0701ff : devcpu_gcb
> >> 1fc080000-1fc0800ff : qs
> >> 1fc090000-1fc0900cb : ptp
> >> 1fc100000-1fc10ffff : port0
> >> 1fc110000-1fc11ffff : port1
> >> 1fc120000-1fc12ffff : port2
> >> 1fc130000-1fc13ffff : port3
> >> 1fc140000-1fc14ffff : port4
> >> 1fc150000-1fc15ffff : port5
> >> 1fc200000-1fc21ffff : qsys
> >> 1fc280000-1fc28ffff : ana
> >>
> >> But after the patch in Fixes: was applied, the information is now
> >> presented in a much more opaque way:
> >>
> >> 1fc000000-1fc3fffff : pcie@1f0000000
> >> 1fc000000-1fc3fffff : 0000:00:00.5
> >> 1fc010000-1fc01ffff : 0000:00:00.5
> >> 1fc030000-1fc03ffff : 0000:00:00.5
> >> 1fc060000-1fc0603ff : 0000:00:00.5
> >> 1fc070000-1fc0701ff : 0000:00:00.5
> >> 1fc080000-1fc0800ff : 0000:00:00.5
> >> 1fc090000-1fc0900cb : 0000:00:00.5
> >> 1fc100000-1fc10ffff : 0000:00:00.5
> >> 1fc110000-1fc11ffff : 0000:00:00.5
> >> 1fc120000-1fc12ffff : 0000:00:00.5
> >> 1fc130000-1fc13ffff : 0000:00:00.5
> >> 1fc140000-1fc14ffff : 0000:00:00.5
> >> 1fc150000-1fc15ffff : 0000:00:00.5
> >> 1fc200000-1fc21ffff : 0000:00:00.5
> >> 1fc280000-1fc28ffff : 0000:00:00.5
> >>
> >> That patch made a fair comment that /proc/iomem might be confusing when
> >> it shows resources without an associated device, but we can do better
> >> than just hide the resource name altogether. Namely, we can print the
> >> device name _and_ the resource name. Like this:
> >>
> >> 1fc000000-1fc3fffff : pcie@1f0000000
> >> 1fc000000-1fc3fffff : 0000:00:00.5
> >> 1fc010000-1fc01ffff : 0000:00:00.5 sys
> >> 1fc030000-1fc03ffff : 0000:00:00.5 rew
> >> 1fc060000-1fc0603ff : 0000:00:00.5 s2
> >> 1fc070000-1fc0701ff : 0000:00:00.5 devcpu_gcb
> >> 1fc080000-1fc0800ff : 0000:00:00.5 qs
> >> 1fc090000-1fc0900cb : 0000:00:00.5 ptp
> >> 1fc100000-1fc10ffff : 0000:00:00.5 port0
> >> 1fc110000-1fc11ffff : 0000:00:00.5 port1
> >> 1fc120000-1fc12ffff : 0000:00:00.5 port2
> >> 1fc130000-1fc13ffff : 0000:00:00.5 port3
> >> 1fc140000-1fc14ffff : 0000:00:00.5 port4
> >> 1fc150000-1fc15ffff : 0000:00:00.5 port5
> >> 1fc200000-1fc21ffff : 0000:00:00.5 qsys
> >> 1fc280000-1fc28ffff : 0000:00:00.5 ana

> > All of this seems an ABI change.

> Yes, indeed. What should I understand from your comment though?

You effectively break an ABI.

--
With Best Regards,
Andy Shevchenko

2020-06-01 10:46:24

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH v2] devres: keep both device name and resource name in pretty name

On Mon, 1 Jun 2020 at 13:39, Andy Shevchenko <[email protected]> wrote:
>
> On Mon, Jun 1, 2020 at 12:13 AM Vladimir Oltean <[email protected]> wrote:
> > On Mon, 1 Jun 2020 at 00:05, Andy Shevchenko <[email protected]> wrote:
> > > On Sunday, May 31, 2020, Vladimir Oltean <[email protected]> wrote:
>
> > >> Sometimes debugging a device is easiest using devmem on its register
> > >> map, and that can be seen with /proc/iomem. But some device drivers have
> > >> many memory regions. Take for example a networking switch. Its memory
> > >> map used to look like this in /proc/iomem:
> > >>
> > >> 1fc000000-1fc3fffff : pcie@1f0000000
> > >> 1fc000000-1fc3fffff : 0000:00:00.5
> > >> 1fc010000-1fc01ffff : sys
> > >> 1fc030000-1fc03ffff : rew
> > >> 1fc060000-1fc0603ff : s2
> > >> 1fc070000-1fc0701ff : devcpu_gcb
> > >> 1fc080000-1fc0800ff : qs
> > >> 1fc090000-1fc0900cb : ptp
> > >> 1fc100000-1fc10ffff : port0
> > >> 1fc110000-1fc11ffff : port1
> > >> 1fc120000-1fc12ffff : port2
> > >> 1fc130000-1fc13ffff : port3
> > >> 1fc140000-1fc14ffff : port4
> > >> 1fc150000-1fc15ffff : port5
> > >> 1fc200000-1fc21ffff : qsys
> > >> 1fc280000-1fc28ffff : ana
> > >>
> > >> But after the patch in Fixes: was applied, the information is now
> > >> presented in a much more opaque way:
> > >>
> > >> 1fc000000-1fc3fffff : pcie@1f0000000
> > >> 1fc000000-1fc3fffff : 0000:00:00.5
> > >> 1fc010000-1fc01ffff : 0000:00:00.5
> > >> 1fc030000-1fc03ffff : 0000:00:00.5
> > >> 1fc060000-1fc0603ff : 0000:00:00.5
> > >> 1fc070000-1fc0701ff : 0000:00:00.5
> > >> 1fc080000-1fc0800ff : 0000:00:00.5
> > >> 1fc090000-1fc0900cb : 0000:00:00.5
> > >> 1fc100000-1fc10ffff : 0000:00:00.5
> > >> 1fc110000-1fc11ffff : 0000:00:00.5
> > >> 1fc120000-1fc12ffff : 0000:00:00.5
> > >> 1fc130000-1fc13ffff : 0000:00:00.5
> > >> 1fc140000-1fc14ffff : 0000:00:00.5
> > >> 1fc150000-1fc15ffff : 0000:00:00.5
> > >> 1fc200000-1fc21ffff : 0000:00:00.5
> > >> 1fc280000-1fc28ffff : 0000:00:00.5
> > >>
> > >> That patch made a fair comment that /proc/iomem might be confusing when
> > >> it shows resources without an associated device, but we can do better
> > >> than just hide the resource name altogether. Namely, we can print the
> > >> device name _and_ the resource name. Like this:
> > >>
> > >> 1fc000000-1fc3fffff : pcie@1f0000000
> > >> 1fc000000-1fc3fffff : 0000:00:00.5
> > >> 1fc010000-1fc01ffff : 0000:00:00.5 sys
> > >> 1fc030000-1fc03ffff : 0000:00:00.5 rew
> > >> 1fc060000-1fc0603ff : 0000:00:00.5 s2
> > >> 1fc070000-1fc0701ff : 0000:00:00.5 devcpu_gcb
> > >> 1fc080000-1fc0800ff : 0000:00:00.5 qs
> > >> 1fc090000-1fc0900cb : 0000:00:00.5 ptp
> > >> 1fc100000-1fc10ffff : 0000:00:00.5 port0
> > >> 1fc110000-1fc11ffff : 0000:00:00.5 port1
> > >> 1fc120000-1fc12ffff : 0000:00:00.5 port2
> > >> 1fc130000-1fc13ffff : 0000:00:00.5 port3
> > >> 1fc140000-1fc14ffff : 0000:00:00.5 port4
> > >> 1fc150000-1fc15ffff : 0000:00:00.5 port5
> > >> 1fc200000-1fc21ffff : 0000:00:00.5 qsys
> > >> 1fc280000-1fc28ffff : 0000:00:00.5 ana
>
> > > All of this seems an ABI change.
>
> > Yes, indeed. What should I understand from your comment though?
>
> You effectively break an ABI.
>
> --
> With Best Regards,
> Andy Shevchenko

I've replied to Greg about this in the v3 patch.

Regards,
-Vladimir