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]>
---
Changes in v2:
Checking for memory allocation errors and returning -ENOMEM.
Changes in v3:
Using devm_kasprintf instead of open-coding it.
lib/devres.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/lib/devres.c b/lib/devres.c
index 6ef51f159c54..ca0d28727cce 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,15 @@ __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)
+ pretty_name = devm_kasprintf(dev, GFP_KERNEL, "%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
On Mon, Jun 01, 2020 at 12:58:26PM +0300, 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
As this is changing the format of a user-visable file, what tools just
broke that are used to parsing the old format?
And are you sure about this? That's not how my system looks at all, I
have fun things like:
ac000000-da0fffff : PCI Bus 0000:03
ac000000-da0fffff : PCI Bus 0000:04
ac000000-c3efffff : PCI Bus 0000:06
c3f00000-c3ffffff : PCI Bus 0000:39
c3f00000-c3f0ffff : 0000:39:00.0
c3f00000-c3f0ffff : xhci-hcd
c4000000-d9ffffff : PCI Bus 0000:3a
c4000000-d9ffffff : PCI Bus 0000:3b
c4000000-c40fffff : PCI Bus 0000:3c
c4000000-c400ffff : 0000:3c:00.0
c4000000-c400ffff : xhci-hcd
c4010000-c4010fff : 0000:3c:00.0
c4011000-c4011fff : 0000:3c:00.0
c4100000-c41fffff : PCI Bus 0000:3d
c4100000-c410ffff : 0000:3d:00.0
c4100000-c410ffff : xhci-hcd
c4110000-c4110fff : 0000:3d:00.0
c4111000-c4111fff : 0000:3d:00.0
c4200000-c42fffff : PCI Bus 0000:3e
c4200000-c4207fff : 0000:3e:00.0
c4200000-c4207fff : xhci-hcd
c4300000-c43fffff : PCI Bus 0000:3f
c4300000-c437ffff : 0000:3f:00.0
c4380000-c4383fff : 0000:3f:00.0
c4400000-d9ffffff : PCI Bus 0000:40
da000000-da0fffff : PCI Bus 0000:05
da000000-da03ffff : 0000:05:00.0
da040000-da040fff : 0000:05:00.0
which is a mix of the resources in some places, and just driver names in
others.
But, that does imply that your change will not break anything as the
parsing of this mess is probably just "anything after the ':'
character...
thanks,
greg k-h
Hi Greg,
On Mon, 1 Jun 2020 at 13:04, Greg KH <[email protected]> wrote:
>
> On Mon, Jun 01, 2020 at 12:58:26PM +0300, 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
>
> As this is changing the format of a user-visable file, what tools just
> broke that are used to parsing the old format?
>
All the same tools that broke after 8d84b18f5678 was merged. I am not
entirely sure why the 'stable ABI' argument was not brought up there
as well.
> And are you sure about this? That's not how my system looks at all, I
> have fun things like:
>
> ac000000-da0fffff : PCI Bus 0000:03
> ac000000-da0fffff : PCI Bus 0000:04
> ac000000-c3efffff : PCI Bus 0000:06
> c3f00000-c3ffffff : PCI Bus 0000:39
> c3f00000-c3f0ffff : 0000:39:00.0
> c3f00000-c3f0ffff : xhci-hcd
> c4000000-d9ffffff : PCI Bus 0000:3a
> c4000000-d9ffffff : PCI Bus 0000:3b
> c4000000-c40fffff : PCI Bus 0000:3c
> c4000000-c400ffff : 0000:3c:00.0
> c4000000-c400ffff : xhci-hcd
> c4010000-c4010fff : 0000:3c:00.0
> c4011000-c4011fff : 0000:3c:00.0
> c4100000-c41fffff : PCI Bus 0000:3d
> c4100000-c410ffff : 0000:3d:00.0
> c4100000-c410ffff : xhci-hcd
> c4110000-c4110fff : 0000:3d:00.0
> c4111000-c4111fff : 0000:3d:00.0
> c4200000-c42fffff : PCI Bus 0000:3e
> c4200000-c4207fff : 0000:3e:00.0
> c4200000-c4207fff : xhci-hcd
> c4300000-c43fffff : PCI Bus 0000:3f
> c4300000-c437ffff : 0000:3f:00.0
> c4380000-c4383fff : 0000:3f:00.0
> c4400000-d9ffffff : PCI Bus 0000:40
> da000000-da0fffff : PCI Bus 0000:05
> da000000-da03ffff : 0000:05:00.0
> da040000-da040fff : 0000:05:00.0
>
>
> which is a mix of the resources in some places, and just driver names in
> others.
>
> But, that does imply that your change will not break anything as the
> parsing of this mess is probably just "anything after the ':'
> character...
>
> thanks,
>
> greg k-h
With this patch you'll just have more (potentially redundant)
information. I'm not really sure how to satisfy everyone here. I was
completely fine with pre-8d84b18f5678 behavior.
Thanks,
-Vladimir
On Mon, Jun 01, 2020 at 01:13:16PM +0300, Vladimir Oltean wrote:
> Hi Greg,
>
> On Mon, 1 Jun 2020 at 13:04, Greg KH <[email protected]> wrote:
> >
> > On Mon, Jun 01, 2020 at 12:58:26PM +0300, 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
> >
> > As this is changing the format of a user-visable file, what tools just
> > broke that are used to parsing the old format?
> >
>
> All the same tools that broke after 8d84b18f5678 was merged. I am not
> entirely sure why the 'stable ABI' argument was not brought up there
> as well.
>
> > And are you sure about this? That's not how my system looks at all, I
> > have fun things like:
> >
> > ac000000-da0fffff : PCI Bus 0000:03
> > ac000000-da0fffff : PCI Bus 0000:04
> > ac000000-c3efffff : PCI Bus 0000:06
> > c3f00000-c3ffffff : PCI Bus 0000:39
> > c3f00000-c3f0ffff : 0000:39:00.0
> > c3f00000-c3f0ffff : xhci-hcd
> > c4000000-d9ffffff : PCI Bus 0000:3a
> > c4000000-d9ffffff : PCI Bus 0000:3b
> > c4000000-c40fffff : PCI Bus 0000:3c
> > c4000000-c400ffff : 0000:3c:00.0
> > c4000000-c400ffff : xhci-hcd
> > c4010000-c4010fff : 0000:3c:00.0
> > c4011000-c4011fff : 0000:3c:00.0
> > c4100000-c41fffff : PCI Bus 0000:3d
> > c4100000-c410ffff : 0000:3d:00.0
> > c4100000-c410ffff : xhci-hcd
> > c4110000-c4110fff : 0000:3d:00.0
> > c4111000-c4111fff : 0000:3d:00.0
> > c4200000-c42fffff : PCI Bus 0000:3e
> > c4200000-c4207fff : 0000:3e:00.0
> > c4200000-c4207fff : xhci-hcd
> > c4300000-c43fffff : PCI Bus 0000:3f
> > c4300000-c437ffff : 0000:3f:00.0
> > c4380000-c4383fff : 0000:3f:00.0
> > c4400000-d9ffffff : PCI Bus 0000:40
> > da000000-da0fffff : PCI Bus 0000:05
> > da000000-da03ffff : 0000:05:00.0
> > da040000-da040fff : 0000:05:00.0
> >
> >
> > which is a mix of the resources in some places, and just driver names in
> > others.
> >
> > But, that does imply that your change will not break anything as the
> > parsing of this mess is probably just "anything after the ':'
> > character...
> >
> > thanks,
> >
> > greg k-h
>
> With this patch you'll just have more (potentially redundant)
> information. I'm not really sure how to satisfy everyone here. I was
> completely fine with pre-8d84b18f5678 behavior.
Fair enough, I'll try it out after 5.8-rc1 is out, as I can't do
anything about this until that happens.
thanks,
greg k-h
On 06/01/2020 12:58 PM, 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]>
> ---
> Changes in v2:
> Checking for memory allocation errors and returning -ENOMEM.
>
> Changes in v3:
> Using devm_kasprintf instead of open-coding it.
>
> lib/devres.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/lib/devres.c b/lib/devres.c
> index 6ef51f159c54..ca0d28727cce 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,15 @@ __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)
> + pretty_name = devm_kasprintf(dev, GFP_KERNEL, "%s %s",
What about "%s:%s"? I suspect it'd be better on the ABI side of things?
[...]
MBR, Sergei
Hi Sergei,
On Mon, 1 Jun 2020 at 21:48, Sergei Shtylyov
<[email protected]> wrote:
>
> On 06/01/2020 12:58 PM, 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]>
> > ---
> > Changes in v2:
> > Checking for memory allocation errors and returning -ENOMEM.
> >
> > Changes in v3:
> > Using devm_kasprintf instead of open-coding it.
> >
> > lib/devres.c | 11 ++++++++++-
> > 1 file changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/devres.c b/lib/devres.c
> > index 6ef51f159c54..ca0d28727cce 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,15 @@ __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)
> > + pretty_name = devm_kasprintf(dev, GFP_KERNEL, "%s %s",
>
> What about "%s:%s"? I suspect it'd be better on the ABI side of things?
>
> [...]
>
> MBR, Sergei
I don't have a particular preference, but out of curiosity, why would
it be better?
Thanks,
-Vladimir
On 06/01/2020 11:03 PM, Vladimir Oltean wrote:
> Hi Sergei,
>
> On Mon, 1 Jun 2020 at 21:48, Sergei Shtylyov
> <[email protected]> wrote:
>>
>> On 06/01/2020 12:58 PM, 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]>
>>> ---
>>> Changes in v2:
>>> Checking for memory allocation errors and returning -ENOMEM.
>>>
>>> Changes in v3:
>>> Using devm_kasprintf instead of open-coding it.
>>>
>>> lib/devres.c | 11 ++++++++++-
>>> 1 file changed, 10 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/lib/devres.c b/lib/devres.c
>>> index 6ef51f159c54..ca0d28727cce 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,15 @@ __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)
>>> + pretty_name = devm_kasprintf(dev, GFP_KERNEL, "%s %s",
>>
>> What about "%s:%s"? I suspect it'd be better on the ABI side of things?
>>
>> [...]
>>
>> MBR, Sergei
>
> I don't have a particular preference, but out of curiosity, why would
> it be better?
No space amidst the name.
> Thanks,
> -Vladimir
MBR, Sergei