* [PATCH v3] devres: keep both device name and resource name in pretty name
@ 2020-06-01 9:58 Vladimir Oltean
2020-06-01 10:04 ` Greg KH
2020-06-01 18:48 ` Sergei Shtylyov
0 siblings, 2 replies; 7+ messages in thread
From: Vladimir Oltean @ 2020-06-01 9:58 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>
---
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v3] devres: keep both device name and resource name in pretty name
2020-06-01 9:58 [PATCH v3] devres: keep both device name and resource name in pretty name Vladimir Oltean
@ 2020-06-01 10:04 ` Greg KH
2020-06-01 10:13 ` Vladimir Oltean
2020-06-01 18:48 ` Sergei Shtylyov
1 sibling, 1 reply; 7+ messages in thread
From: Greg KH @ 2020-06-01 10:04 UTC (permalink / raw)
To: Vladimir Oltean
Cc: arnd, akpm, sergei.shtylyov, bgolaszewski, mika.westerberg,
efremov, ztuowen, linux-kernel, netdev
On Mon, Jun 01, 2020 at 12:58:26PM +0300, 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
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] devres: keep both device name and resource name in pretty name
2020-06-01 10:04 ` Greg KH
@ 2020-06-01 10:13 ` Vladimir Oltean
2020-06-01 10:21 ` Greg KH
0 siblings, 1 reply; 7+ messages in thread
From: Vladimir Oltean @ 2020-06-01 10:13 UTC (permalink / raw)
To: Greg KH
Cc: Arnd Bergmann, Andrew Morton, sergei.shtylyov, bgolaszewski,
mika.westerberg, efremov, ztuowen, lkml, netdev
Hi Greg,
On Mon, 1 Jun 2020 at 13:04, Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Mon, Jun 01, 2020 at 12:58:26PM +0300, 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
>
> 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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] devres: keep both device name and resource name in pretty name
2020-06-01 10:13 ` Vladimir Oltean
@ 2020-06-01 10:21 ` Greg KH
0 siblings, 0 replies; 7+ messages in thread
From: Greg KH @ 2020-06-01 10:21 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Arnd Bergmann, Andrew Morton, sergei.shtylyov, bgolaszewski,
mika.westerberg, efremov, ztuowen, lkml, netdev
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 <gregkh@linuxfoundation.org> wrote:
> >
> > On Mon, Jun 01, 2020 at 12:58:26PM +0300, 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
> >
> > 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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] devres: keep both device name and resource name in pretty name
2020-06-01 9:58 [PATCH v3] devres: keep both device name and resource name in pretty name Vladimir Oltean
2020-06-01 10:04 ` Greg KH
@ 2020-06-01 18:48 ` Sergei Shtylyov
2020-06-01 20:03 ` Vladimir Oltean
1 sibling, 1 reply; 7+ messages in thread
From: Sergei Shtylyov @ 2020-06-01 18:48 UTC (permalink / raw)
To: Vladimir Oltean, gregkh, arnd, akpm
Cc: bgolaszewski, mika.westerberg, efremov, ztuowen, linux-kernel, netdev
On 06/01/2020 12:58 PM, 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>
> ---
> 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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] devres: keep both device name and resource name in pretty name
2020-06-01 18:48 ` Sergei Shtylyov
@ 2020-06-01 20:03 ` Vladimir Oltean
2020-06-01 20:11 ` Sergei Shtylyov
0 siblings, 1 reply; 7+ messages in thread
From: Vladimir Oltean @ 2020-06-01 20:03 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 21:48, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
>
> On 06/01/2020 12:58 PM, 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>
> > ---
> > 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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] devres: keep both device name and resource name in pretty name
2020-06-01 20:03 ` Vladimir Oltean
@ 2020-06-01 20:11 ` Sergei Shtylyov
0 siblings, 0 replies; 7+ messages in thread
From: Sergei Shtylyov @ 2020-06-01 20:11 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Greg Kroah-Hartman, Arnd Bergmann, Andrew Morton, bgolaszewski,
mika.westerberg, efremov, ztuowen, lkml, netdev
On 06/01/2020 11:03 PM, Vladimir Oltean wrote:
> Hi Sergei,
>
> On Mon, 1 Jun 2020 at 21:48, Sergei Shtylyov
> <sergei.shtylyov@cogentembedded.com> wrote:
>>
>> On 06/01/2020 12:58 PM, 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>
>>> ---
>>> 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
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-06-01 20:11 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-01 9:58 [PATCH v3] devres: keep both device name and resource name in pretty name Vladimir Oltean
2020-06-01 10:04 ` Greg KH
2020-06-01 10:13 ` Vladimir Oltean
2020-06-01 10:21 ` Greg KH
2020-06-01 18:48 ` Sergei Shtylyov
2020-06-01 20:03 ` Vladimir Oltean
2020-06-01 20:11 ` Sergei Shtylyov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.