* [PATCH v2] devres: keep both device name and resource name in pretty name
@ 2020-05-31 18:07 Vladimir Oltean
[not found] ` <CAHp75Vc5NrDUZwv7uW+P=Ly+tz3a9XgEukX6ZgSccj_1sMYQaw@mail.gmail.com>
2020-06-01 7:51 ` Sergei Shtylyov
0 siblings, 2 replies; 7+ messages in thread
From: Vladimir Oltean @ 2020-05-31 18:07 UTC (permalink / raw)
To: gregkh, arnd, akpm
Cc: sergei.shtylyov, bgolaszewski, mika.westerberg, efremov, ztuowen,
linux-kernel, netdev
From: Vladimir Oltean <vladimir.oltean@nxp.com>
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 <vladimir.oltean@nxp.com>
---
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2] devres: keep both device name and resource name in pretty name
[not found] ` <CAHp75Vc5NrDUZwv7uW+P=Ly+tz3a9XgEukX6ZgSccj_1sMYQaw@mail.gmail.com>
@ 2020-05-31 21:13 ` Vladimir Oltean
2020-06-01 10:39 ` Andy Shevchenko
0 siblings, 1 reply; 7+ messages in thread
From: Vladimir Oltean @ 2020-05-31 21:13 UTC (permalink / raw)
To: Andy Shevchenko
Cc: gregkh, arnd, akpm, sergei.shtylyov, bgolaszewski,
mika.westerberg, efremov, ztuowen, linux-kernel, netdev
On Mon, 1 Jun 2020 at 00:05, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>
>
>
> On Sunday, May 31, 2020, Vladimir Oltean <olteanv@gmail.com> wrote:
>>
>> From: Vladimir Oltean <vladimir.oltean@nxp.com>
>>
>> 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 <vladimir.oltean@nxp.com>
>> ---
>> 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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] devres: keep both device name and resource name in pretty name
2020-05-31 18:07 [PATCH v2] devres: keep both device name and resource name in pretty name Vladimir Oltean
[not found] ` <CAHp75Vc5NrDUZwv7uW+P=Ly+tz3a9XgEukX6ZgSccj_1sMYQaw@mail.gmail.com>
@ 2020-06-01 7:51 ` Sergei Shtylyov
2020-06-01 9:36 ` Vladimir Oltean
1 sibling, 1 reply; 7+ messages in thread
From: Sergei Shtylyov @ 2020-06-01 7:51 UTC (permalink / raw)
To: Vladimir Oltean, gregkh, arnd, akpm
Cc: bgolaszewski, mika.westerberg, efremov, ztuowen, linux-kernel, netdev
Hello!
On 31.05.2020 21:07, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
>
> 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 <vladimir.oltean@nxp.com>
[...]
You didn't write the version log -- what changed since v1?
MBR, Sergei
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] devres: keep both device name and resource name in pretty name
2020-06-01 7:51 ` Sergei Shtylyov
@ 2020-06-01 9:36 ` Vladimir Oltean
2020-06-01 9:48 ` Greg Kroah-Hartman
0 siblings, 1 reply; 7+ messages in thread
From: Vladimir Oltean @ 2020-06-01 9:36 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Greg Kroah-Hartman, Arnd Bergmann, Andrew Morton, bgolaszewski,
mika.westerberg, efremov, ztuowen, lkml, netdev
Hi Sergei,
On Mon, 1 Jun 2020 at 10:51, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
>
> Hello!
>
> On 31.05.2020 21:07, Vladimir Oltean wrote:
>
> > From: Vladimir Oltean <vladimir.oltean@nxp.com>
> >
> > 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 <vladimir.oltean@nxp.com>
> [...]
>
> 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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] devres: keep both device name and resource name in pretty name
2020-06-01 9:36 ` Vladimir Oltean
@ 2020-06-01 9:48 ` Greg Kroah-Hartman
0 siblings, 0 replies; 7+ messages in thread
From: Greg Kroah-Hartman @ 2020-06-01 9:48 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Sergei Shtylyov, Arnd Bergmann, Andrew Morton, bgolaszewski,
mika.westerberg, efremov, ztuowen, lkml, netdev
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
> <sergei.shtylyov@cogentembedded.com> wrote:
> >
> > Hello!
> >
> > On 31.05.2020 21:07, Vladimir Oltean wrote:
> >
> > > From: Vladimir Oltean <vladimir.oltean@nxp.com>
> > >
> > > 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 <vladimir.oltean@nxp.com>
> > [...]
> >
> > 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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] devres: keep both device name and resource name in pretty name
2020-05-31 21:13 ` Vladimir Oltean
@ 2020-06-01 10:39 ` Andy Shevchenko
2020-06-01 10:43 ` Vladimir Oltean
0 siblings, 1 reply; 7+ messages in thread
From: Andy Shevchenko @ 2020-06-01 10:39 UTC (permalink / raw)
To: Vladimir Oltean
Cc: gregkh, arnd, akpm, sergei.shtylyov, bgolaszewski,
mika.westerberg, efremov, ztuowen, linux-kernel, netdev
On Mon, Jun 1, 2020 at 12:13 AM Vladimir Oltean <olteanv@gmail.com> wrote:
> On Mon, 1 Jun 2020 at 00:05, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > On Sunday, May 31, 2020, Vladimir Oltean <olteanv@gmail.com> 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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] devres: keep both device name and resource name in pretty name
2020-06-01 10:39 ` Andy Shevchenko
@ 2020-06-01 10:43 ` Vladimir Oltean
0 siblings, 0 replies; 7+ messages in thread
From: Vladimir Oltean @ 2020-06-01 10:43 UTC (permalink / raw)
To: Andy Shevchenko
Cc: gregkh, arnd, akpm, sergei.shtylyov, bgolaszewski,
mika.westerberg, efremov, ztuowen, linux-kernel, netdev
On Mon, 1 Jun 2020 at 13:39, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>
> On Mon, Jun 1, 2020 at 12:13 AM Vladimir Oltean <olteanv@gmail.com> wrote:
> > On Mon, 1 Jun 2020 at 00:05, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > > On Sunday, May 31, 2020, Vladimir Oltean <olteanv@gmail.com> 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
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-06-01 10:44 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-31 18:07 [PATCH v2] devres: keep both device name and resource name in pretty name Vladimir Oltean
[not found] ` <CAHp75Vc5NrDUZwv7uW+P=Ly+tz3a9XgEukX6ZgSccj_1sMYQaw@mail.gmail.com>
2020-05-31 21:13 ` Vladimir Oltean
2020-06-01 10:39 ` Andy Shevchenko
2020-06-01 10:43 ` Vladimir Oltean
2020-06-01 7:51 ` Sergei Shtylyov
2020-06-01 9:36 ` Vladimir Oltean
2020-06-01 9:48 ` Greg Kroah-Hartman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).