ARM: omap: Convert to using %pOFn instead of device_node.name

PATCH In review Rob Herring 63 participants 163 messages Aug 28, 2018 – Oct 8, 2018 Page 2 of 2

Subsystems (121)

Networking Network drivers DRM drivers Arm (32-bit) +117 more subsystems

lore

Share & tools

lore

David Miller

maintainer Applied

2018-08-30 02:41 UTC #149 lore raw

Re: [PATCH v2] net: ethernet: Convert to using %pOFn instead of device_node.name

In reply to Rob Herring

From: Rob Herring <[email protected]>
Date: Tue, 28 Aug 2018 10:44:30 -0500

Quoted 7 lines

In preparation to remove the node name pointer from struct device_node, convert printf users to use the %pOFn format specifier.
Signed-off-by Rob Herring <[email protected]>
---
v2:
- fix missing brackets in netcp

Applied to net-next.

Rob Herring

author

2018-08-28 15:44 UTC #150 lore raw

PATCH v2 1/1 sparc: Convert to using %pOFn instead of device_node.name

In reply to Rob Herring

In preparation to remove the node name pointer from struct device_node, convert printf users to use the %pOFn format specifier.

Cc "David S. Miller" <[email protected]>

Cc [email protected]

Signed-off-by Rob Herring <[email protected]>

---
v2:
- snprint -> snprintf typo fix

 arch/sparc/kernel/auxio_64.c |  4 +--
 arch/sparc/kernel/power.c    |  4 +--
 arch/sparc/kernel/prom_32.c  | 26 +++++++-------
 arch/sparc/kernel/prom_64.c  | 68 ++++++++++++++++++------------------
 4 files changed, 51 insertions(+), 51 deletions(-)

arch/sparc/kernel/auxio_64.c +2 -2

index 4e8f56c3793c..cc42225c20f3 100644
@@ -115,8 +115,8 @@ static int auxio_probe(struct platform_device *dev)
 		auxio_devtype = AUXIO_TYPE_SBUS;
 		size = 1;
 	} else {
-		printk("auxio: Unknown parent bus type [%s]\n",
-		       dp->parent->name);
+		printk("auxio: Unknown parent bus type [%pOFn]\n",
+		       dp->parent);
 		return -ENODEV;
 	}
 	auxio_register = of_ioremap(&dev->resource[0], 0, size, "auxio");

arch/sparc/kernel/power.c +2 -2

index 92627abce311..d941875dd718 100644
@@ -41,8 +41,8 @@ static int power_probe(struct platform_device *op)
 
 	power_reg = of_ioremap(res, 0, 0x4, "power");
 
-	printk(KERN_INFO "%s: Control reg at %llx\n",
-	       op->dev.of_node->name, res->start);
+	printk(KERN_INFO "%pOFn: Control reg at %llx\n",
+	       op->dev.of_node, res->start);
 
 	if (has_button_interrupt(irq, op->dev.of_node)) {
 		if (request_irq(irq,

arch/sparc/kernel/prom_32.c +13 ~~-13~~

index b51cbb9e87dc..17c87d29ff20 100644
@@ -68,8 +68,8 @@ static void __init sparc32_path_component(struct device_node *dp, char *tmp_buf)
 		return;
 
 	regs = rprop->value;
-	sprintf(tmp_buf, "%s@%x,%x",
-		dp->name,
+	sprintf(tmp_buf, "%pOFn@%x,%x",
+		dp,
 		regs->which_io, regs->phys_addr);
 }

@@ -84,8 +84,8 @@ static void __init sbus_path_component(struct device_node *dp, char *tmp_buf)
 		return;
 
 	regs = prop->value;
-	sprintf(tmp_buf, "%s@%x,%x",
-		dp->name,
+	sprintf(tmp_buf, "%pOFn@%x,%x",
+		dp,
 		regs->which_io,
 		regs->phys_addr);
 }

@@ -104,13 +104,13 @@ static void __init pci_path_component(struct device_node *dp, char *tmp_buf)
 	regs = prop->value;
 	devfn = (regs->phys_hi >> 8) & 0xff;
 	if (devfn & 0x07) {
-		sprintf(tmp_buf, "%s@%x,%x",
-			dp->name,
+		sprintf(tmp_buf, "%pOFn@%x,%x",
+			dp,
 			devfn >> 3,
 			devfn & 0x07);
 	} else {
-		sprintf(tmp_buf, "%s@%x",
-			dp->name,
+		sprintf(tmp_buf, "%pOFn@%x",
+			dp,
 			devfn >> 3);
 	}
 }

@@ -127,8 +127,8 @@ static void __init ebus_path_component(struct device_node *dp, char *tmp_buf)
 
 	regs = prop->value;
 
-	sprintf(tmp_buf, "%s@%x,%x",
-		dp->name,
+	sprintf(tmp_buf, "%pOFn@%x,%x",
+		dp,
 		regs->which_io, regs->phys_addr);
 }

@@ -167,8 +167,8 @@ static void __init ambapp_path_component(struct device_node *dp, char *tmp_buf)
 		return;
 	device = prop->value;
 
-	sprintf(tmp_buf, "%s:%d:%d@%x,%x",
-		dp->name, *vendor, *device,
+	sprintf(tmp_buf, "%pOFn:%d:%d@%x,%x",
+		dp, *vendor, *device,
 		*intr, reg0);
 }

@@ -201,7 +201,7 @@ char * __init build_path_component(struct device_node *dp)
 	tmp_buf[0] = '\0';
 	__build_path_component(dp, tmp_buf);
 	if (tmp_buf[0] = '\0')
-		strcpy(tmp_buf, dp->name);
+		snprintf(tmp_buf, sizeof(tmp_buf), "%pOFn", dp);
 
 	n = prom_early_alloc(strlen(tmp_buf) + 1);
 	strcpy(n, tmp_buf);

arch/sparc/kernel/prom_64.c +34 ~~-34~~

156 lines not on this page · raw message

--
2.17.1

David Miller

maintainer Applied

2018-10-08 05:57 UTC #151 lore raw

Re: [PATCH v2] sparc: Convert to using %pOFn instead of device_node.name

From: Rob Herring <[email protected]>
Date: Tue, 28 Aug 2018 10:44:32 -0500

Quoted 9 lines

In preparation to remove the node name pointer from struct device_node, convert printf users to use the %pOFn format specifier.
Cc "David S. Miller" <[email protected]>
Cc [email protected]
Signed-off-by Rob Herring <[email protected]>
---
v2:
- snprint -> snprintf typo fix

Applied.

Takashi Iwai

2018-09-03 21:45 UTC #153 lore raw

Re: [alsa-devel] [PATCH v2] sound: Convert to using %pOFn instead of device_node.name

Quoted 10 lines · Rob Herring

In preparation to remove the node name pointer from struct device_node, convert printf users to use the %pOFn format specifier.
Cc Johannes Berg <[email protected]>
Cc Jaroslav Kysela <[email protected]>
Cc Takashi Iwai <[email protected]>
Cc [email protected]
Cc [email protected]
Signed-off-by Rob Herring <[email protected]>

Applied now, thanks.

Takashi

Rob Herring

author

2018-09-12 20:27 UTC #156 lore raw

Re: [PATCH v2] staging: Convert to using %pOFn instead of device_node.name

+Joe P

Quoted 18 lines · Mauro Carvalho Chehab

Em Tue, 28 Aug 2018 10:44:33 -0500
Rob Herring <[email protected]> escreveu:
Quoted 14 lines
In preparation to remove the node name pointer from struct device_node, convert printf users to use the %pOFn format specifier.
Cc Steve Longerbeam <[email protected]>
Cc Philipp Zabel <[email protected]>
Cc Mauro Carvalho Chehab <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
Cc [email protected]
Cc [email protected]
Signed-off-by Rob Herring <[email protected]>
---
v2:
- fix conditional use of node name vs devname for imx

Quoted 6 lines

Quoted 3 lines
 drivers/staging/media/imx/imx-media-dev.c | 15 ++++++++++-----
 drivers/staging/media/imx/imx-media-of.c  |  4 ++--
 drivers/staging/mt7621-eth/mdio.c         |  4 ++--
It would be better if you had submitted the staging/media stuff on a separate patch, as they usually go via the media tree.

Sorry, I thought Greg took all of staging.

A problem with MAINTAINERS is there is no way to tell who applies patches for a given path vs. anyone else listed. This frequently happens when the maintainer organization doesn't match the directory org. If we distinguished this, then it would be quite easy to see when you've created a patch that needs to be split to different maintainers (or an explanation why it isn't). Whatever happened with splitting up MAINTAINERS? If there was a file for each maintainer tree, then it would be easier to extract that information.

Or maybe we just need to be stricter with the 'M' vs. 'R' tag and 'M' means that is the person who applies the patch. I don't think many drivers have their own tree and maintainer except for a few big ones.

Rob

Quoted 103 lines

As I don't foresee any conflicts on that part of the code, I'm Ok if Greg pick it and submit via his tree.

So,

Acked-by Mauro Carvalho Chehab <[email protected]>

If you prefer instead that I would pick the media part, please split it into two patches.

Regards,
Mauro

Quoted 83 lines

 3 files changed, 14 insertions(+), 9 deletions(-)

drivers/staging/media/imx/imx-media-dev.c +10 -5

index b0be80f05767..3f48f5ceb6ea 100644
@@ -89,8 +89,12 @@ int imx_media_add_async_subdev(struct imx_media_dev *imxmd,
 
      /* return -EEXIST if this asd already added */
      if (find_async_subdev(imxmd, fwnode, devname)) {
-             dev_dbg(imxmd->md.dev, "%s: already added %s\n",
-                     __func__, np ? np->name : devname);
+             if (np)
+                     dev_dbg(imxmd->md.dev, "%s: already added %pOFn\n",
+                     __func__, np);
+             else
+                     dev_dbg(imxmd->md.dev, "%s: already added %s\n",
+                     __func__, devname);
              ret = -EEXIST;
              goto out;
      }

@@ -105,19 +109,20 @@ int imx_media_add_async_subdev(struct imx_media_dev *imxmd,
      if (fwnode) {
              asd->match_type = V4L2_ASYNC_MATCH_FWNODE;
              asd->match.fwnode = fwnode;
+             dev_dbg(imxmd->md.dev, "%s: added %pOFn, match type FWNODE\n",
+                     __func__, np);
      } else {
              asd->match_type = V4L2_ASYNC_MATCH_DEVNAME;
              asd->match.device_name = devname;
              imxasd->pdev = pdev;
+             dev_dbg(imxmd->md.dev, "%s: added %s, match type DEVNAME\n",
+                     __func__, devname);
      }
 
      list_add_tail(&imxasd->list, &imxmd->asd_list);
 
      imxmd->subdev_notifier.num_subdevs++;
 
-     dev_dbg(imxmd->md.dev, "%s: added %s, match type %s\n",
-             __func__, np ? np->name : devname, np ? "FWNODE" : "DEVNAME");
-
 out:
      mutex_unlock(&imxmd->mutex);
      return ret;

drivers/staging/media/imx/imx-media-of.c +2 -2

index acde372c6795..163437e421c5 100644
@@ -79,8 +79,8 @@ of_parse_subdev(struct imx_media_dev *imxmd, struct device_node *sd_np,
      int i, num_ports, ret;
 
      if (!of_device_is_available(sd_np)) {
-             dev_dbg(imxmd->md.dev, "%s: %s not enabled\n", __func__,
-                     sd_np->name);
+             dev_dbg(imxmd->md.dev, "%s: %pOFn not enabled\n", __func__,
+                     sd_np);
              /* unavailable is not an error */
              return 0;
      }

drivers/staging/mt7621-eth/mdio.c +2 -3

index 7ad0c4141205..9ffa8f771235 100644
@@ -70,7 +70,7 @@ int mtk_connect_phy_node(struct mtk_eth *eth, struct mtk_mac *mac,
      _port = of_get_property(phy_node, "reg", NULL);
 
      if (!_port || (be32_to_cpu(*_port) >= 0x20)) {
-             pr_err("%s: invalid port id\n", phy_node->name);
+             pr_err("%pOFn: invalid port id\n", phy_node);
              return -EINVAL;
      }
      port = be32_to_cpu(*_port);

@@ -249,7 +249,7 @@ int mtk_mdio_init(struct mtk_eth *eth)
      eth->mii_bus->priv = eth;
      eth->mii_bus->parent = eth->dev;
 
-     snprintf(eth->mii_bus->id, MII_BUS_ID_SIZE, "%s", mii_np->name);
+     snprintf(eth->mii_bus->id, MII_BUS_ID_SIZE, "%pOFn", mii_np);
      err = of_mdiobus_register(eth->mii_bus, mii_np);
      if (err)
              goto err_free_bus;
--

Quoted 1 line

2.17.1

Thanks,
Mauro

Mauro Carvalho Chehab

2018-09-12 21:07 UTC #157 lore raw

Re: [PATCH v2] staging: Convert to using %pOFn instead of device_node.name

Em Wed, 12 Sep 2018 15:26:48 -0500
Rob Herring <[email protected]> escreveu:

Quoted 22 lines

+Joe P
Quoted 18 lines · Mauro Carvalho Chehab
Em Tue, 28 Aug 2018 10:44:33 -0500
Rob Herring <[email protected]> escreveu:
Quoted 14 lines
In preparation to remove the node name pointer from struct device_node, convert printf users to use the %pOFn format specifier.
Cc Steve Longerbeam <[email protected]>
Cc Philipp Zabel <[email protected]>
Cc Mauro Carvalho Chehab <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
Cc [email protected]
Cc [email protected]
Signed-off-by Rob Herring <[email protected]>
---
v2:
- fix conditional use of node name vs devname for imx

Quoted 8 lines

Quoted 6 lines
Quoted 3 lines
 drivers/staging/media/imx/imx-media-dev.c | 15 ++++++++++-----
 drivers/staging/media/imx/imx-media-of.c  |  4 ++--
 drivers/staging/mt7621-eth/mdio.c         |  4 ++--  
It would be better if you had submitted the staging/media stuff on a separate patch, as they usually go via the media tree.
Sorry, I thought Greg took all of staging.

No, I usually take media patches on staging. It seems that at least the IIO subsystem does the same:

IIO SUBSYSTEM AND DRIVERS
M: Jonathan Cameron <[email protected]>
R: Hartmut Knaack <[email protected]>
R: Lars-Peter Clausen <[email protected]>
R: Peter Meerwald-Stadler <[email protected]>
L: [email protected]
T: git git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git
S: Maintained
F: Documentation/ABI/testing/configfs-iio*
F: Documentation/ABI/testing/sysfs-bus-iio*
F: Documentation/devicetree/bindings/iio/
F: drivers/iio/
F: drivers/staging/iio/
F: include/linux/iio/
F: tools/iio/

Anyway, as I said before, I don't have any issues if Greg takes this specific patch.

Quoted 8 lines

A problem with MAINTAINERS is there is no way to tell who applies patches for a given path vs. anyone else listed. This frequently happens when the maintainer organization doesn't match the directory org. If we distinguished this, then it would be quite easy to see when you've created a patch that needs to be split to different maintainers (or an explanation why it isn't). Whatever happened with splitting up MAINTAINERS? If there was a file for each maintainer tree, then it would be easier to extract that information.

Yes, but, on the other hand, get_maintainers.pl would likely take more time to process, if a patch touches multiple subsystems.

Quoted 4 lines

Or maybe we just need to be stricter with the 'M' vs. 'R' tag and 'M' means that is the person who applies the patch. I don't think many drivers have their own tree and maintainer except for a few big ones.

Hmm... just getting a random file under staging/media:

./scripts/get_maintainer.pl -f drivers/staging/media/imx/imx-ic.h
Steve Longerbeam <[email protected]> (maintainer:MEDIA DRIVERS FOR FREESCALE IMX) Philipp Zabel <[email protected]> (maintainer:MEDIA DRIVERS FOR FREESCALE IMX) Mauro Carvalho Chehab <[email protected]> (maintainer:MEDIA INPUT INFRASTRUCTURE (V4L/DVB)) Greg Kroah-Hartman <[email protected]> (supporter:STAGING SUBSYSTEM) [email protected] (open list:MEDIA DRIVERS FOR FREESCALE IMX) [email protected] (open list:STAGING SUBSYSTEM) [email protected] (open list)

It seems that the maintainers are already ordered by the tree depth (placing the most relevant results first).

So, both driver maintainers appear first, then me and finally Greg (as supporter).

Mailing lists are also ordered by relevance: media ML, then staging ML and finally LKML.

Thanks,
Mauro

Rob Herring

author

2018-09-12 22:49 UTC #158 lore raw

Re: [PATCH v2] staging: Convert to using %pOFn instead of device_node.name

Quoted 39 lines · Mauro Carvalho Chehab

Em Wed, 12 Sep 2018 15:26:48 -0500
Rob Herring <[email protected]> escreveu:
Quoted 22 lines
+Joe P
Quoted 18 lines · Mauro Carvalho Chehab
Em Tue, 28 Aug 2018 10:44:33 -0500
Rob Herring <[email protected]> escreveu:
Quoted 14 lines
In preparation to remove the node name pointer from struct device_node, convert printf users to use the %pOFn format specifier.
Cc Steve Longerbeam <[email protected]>
Cc Philipp Zabel <[email protected]>
Cc Mauro Carvalho Chehab <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
Cc [email protected]
Cc [email protected]
Signed-off-by Rob Herring <[email protected]>
---
v2:
- fix conditional use of node name vs devname for imx
Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
 drivers/staging/media/imx/imx-media-dev.c | 15 ++++++++++-----
 drivers/staging/media/imx/imx-media-of.c  |  4 ++--
 drivers/staging/mt7621-eth/mdio.c         |  4 ++--
It would be better if you had submitted the staging/media stuff on a separate patch, as they usually go via the media tree.
Sorry, I thought Greg took all of staging.
No, I usually take media patches on staging. It seems that at least the IIO subsystem does the same:
IIO SUBSYSTEM AND DRIVERS

Quoted 14 lines

M:      Jonathan Cameron <[email protected]>
R:      Hartmut Knaack <[email protected]>
R:      Lars-Peter Clausen <[email protected]>
R:      Peter Meerwald-Stadler <[email protected]>
L:      [email protected]
T:      git git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git
S:      Maintained
F:      Documentation/ABI/testing/configfs-iio*
F:      Documentation/ABI/testing/sysfs-bus-iio*
F:      Documentation/devicetree/bindings/iio/
F:      drivers/iio/
F:      drivers/staging/iio/
F:      include/linux/iio/
F:      tools/iio/

Yes, the information is there, but needs human processing.

Quoted 2 lines

Anyway, as I said before, I don't have any issues if Greg takes this specific patch.

I know. I'm just thinking about how to improve things. Ideally, what I'd like is a script that spits out a To list for who applies the patch and a Cc list of reviewers.

Quoted 28 lines

Quoted 8 lines
A problem with MAINTAINERS is there is no way to tell who applies patches for a given path vs. anyone else listed. This frequently happens when the maintainer organization doesn't match the directory org. If we distinguished this, then it would be quite easy to see when you've created a patch that needs to be split to different maintainers (or an explanation why it isn't). Whatever happened with splitting up MAINTAINERS? If there was a file for each maintainer tree, then it would be easier to extract that information.
Yes, but, on the other hand, get_maintainers.pl would likely take more time to process, if a patch touches multiple subsystems.
Quoted 4 lines
Or maybe we just need to be stricter with the 'M' vs. 'R' tag and 'M' means that is the person who applies the patch. I don't think many drivers have their own tree and maintainer except for a few big ones.
Hmm... just getting a random file under staging/media:
./scripts/get_maintainer.pl -f drivers/staging/media/imx/imx-ic.h
Steve Longerbeam <[email protected]> (maintainer:MEDIA DRIVERS FOR FREESCALE IMX) Philipp Zabel <[email protected]> (maintainer:MEDIA DRIVERS FOR FREESCALE IMX) Mauro Carvalho Chehab <[email protected]> (maintainer:MEDIA INPUT INFRASTRUCTURE (V4L/DVB)) Greg Kroah-Hartman <[email protected]> (supporter:STAGING SUBSYSTEM) [email protected] (open list:MEDIA DRIVERS FOR FREESCALE IMX) [email protected] (open list:STAGING SUBSYSTEM) [email protected] (open list)

Quoted 2 lines

It seems that the maintainers are already ordered by the tree depth (placing the most relevant results first).

Most relevant in the sense of who reviews and cares about the changes, but not to answer who applies this patch. I could say I just need to ignore supporters and look at the last maintainer, but then that breaks for the case below. I don't think 'maintainer' vs. 'supporter' is very well maintained either.

$ scripts/get_maintainer.pl -f drivers/staging/olpc_dcon/
Jens Frederich <[email protected]> (maintainer:STAGING - OLPC
SECONDARY DISPLAY CONTROLLER (DCON))
Daniel Drake <[email protected]> (maintainer:STAGING - OLPC SECONDARY
DISPLAY CONTROLLER (DCON))
Jon Nettleton <[email protected]> (maintainer:STAGING - OLPC
SECONDARY DISPLAY CONTROLLER (DCON))
Greg Kroah-Hartman <[email protected]> (supporter:STAGING SUBSYSTEM)
[email protected] (open list:STAGING SUBSYSTEM)
[email protected] (open list)

Maybe Jon has a tree and applies it. Who knows... You either have to know who are "real" maintainers (the ones applying patches) or go look at git history to see Greg is the only non-author S-o-B.

Quoted 5 lines

So, both driver maintainers appear first, then me and finally Greg (as supporter).
Mailing lists are also ordered by relevance: media ML, then staging ML and finally LKML.

Another clue, yes, but something you can script?

Rob

Joe Perches

2018-09-13 20:45 UTC #159 lore raw

Re: [PATCH v2] staging: Convert to using %pOFn instead of device_node.name

In reply to Rob Herring

Quoted 2 lines · Rob Herring

A problem with MAINTAINERS is there is no way to tell who applies patches for a given path vs. anyone else listed.

try the --scm option

Rob Herring

author

2018-09-13 21:27 UTC #160 lore raw

Re: [PATCH v2] staging: Convert to using %pOFn instead of device_node.name

Quoted 6 lines · Joe Perches

Quoted 2 lines · Rob Herring
A problem with MAINTAINERS is there is no way to tell who applies patches for a given path vs. anyone else listed.
try the --scm option

That kind of helps if the maintainer has listed a tree, but gives wrong results if not. And you still have to figure out who owns which tree. That's not hard, but it's not scriptable.

IMO, we should reserve 'M:' for maintainers with trees and use 'R:' driver maintainers. That's redefining M as "maintainer" rather than "mail patches to". You could still have both for a entry so you can know who to go bug when your patch hasn't been applied.

Rob

Joe Perches

2018-09-14 11:42 UTC #161 lore raw

Re: [PATCH v2] staging: Convert to using %pOFn instead of device_node.name

Quoted 10 lines · Rob Herring

Quoted 6 lines · Joe Perches
Quoted 2 lines · Rob Herring
A problem with MAINTAINERS is there is no way to tell who applies patches for a given path vs. anyone else listed.
try the --scm option
That kind of helps if the maintainer has listed a tree, but gives wrong results if not.

If there isn't a tree listed, it's not really maintained.

Quoted 2 lines

And you still have to figure out who owns which
tree. That's not hard, but it's not scriptable.

a get_maintainer scripted loop using
--maxdepth=<incrementing_from_1> --scm --m --nor --nol
until a result could work.

Quoted 4 lines

IMO, we should reserve 'M:' for maintainers with trees and use 'R:' driver maintainers. That's redefining M as "maintainer" rather than "mail patches to". You could still have both for a entry so you can know who to go bug when your patch hasn't been applied.

IMO, most M: entries in MAINTAINERS are either reviewers or used-once when created and longer active or just vanity.

Tony Lindgren

maintainer

2018-09-07 17:14 UTC #162 lore raw

In reply to Rob Herring

Quoted 9 lines · Rob Herring

In preparation to remove the node name pointer from struct device_node, convert printf users to use the %pOFn format specifier.
Cc "Beno?t Cousson" <[email protected]>
Cc Paul Walmsley <[email protected]>
Cc Tony Lindgren <[email protected]>
Cc linux-omap at vger.kernel.org
Cc linux-arm-kernel at lists.infradead.org
Signed-off-by Rob Herring <[email protected]>

Quoted 3 lines

---
v2:
- fix error in format specifier position

Thanks applying into omap-for-v4.20/soc.

Regards,

Tony