pci: pci-mvebu: Use '%pa' for printing 'phys_addr_t' type

PATCH Superseded Fabio Estevam 3 participants 6 messages Apr 28, 2014 – Apr 29, 2014 PCI

lore

Share & tools

lore

Fabio Estevam

author

2014-04-28 10:51 UTC #0 lore raw

PATCH 1/1 pci: pci-mvebu: Use '%pa' for printing 'phys_addr_t' type

From: Fabio Estevam <[email protected]>

Fix the following build warning that happens when building multi_v7_defconfig with CONFIG_ARM_LPAE=y:

drivers/pci/host/pci-mvebu.c:334:5: warning: format '%x' expects argument of type 'unsigned int', but argument 3 has type 'phys_addr_t' [-Wformat=]

While at it, also use '%zx' for printing 'size_t'.

Reported-by Olof's autobuilder <[email protected]>

Signed-off-by Fabio Estevam <[email protected]>

---
 drivers/pci/host/pci-mvebu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/pci/host/pci-mvebu.c +2 -2

index e384e25..572a7b5 100644
@@ -330,8 +330,8 @@ static void mvebu_pcie_add_windows(struct mvebu_pcie_port *port,
 							sz, remap);
 		if (ret) {
 			dev_err(&port->pcie->pdev->dev,
-				"Could not create MBus window at 0x%x, size 0x%x: %d\n",
-				base, sz, ret);
+				"Could not create MBus window at %pa, size 0x%zx: %d\n",
+				&base, sz, ret);
 			mvebu_pcie_del_windows(port, base - size_mapped,
 					       size_mapped);
 			return;

Signature · 1 line

1.8.3.2

Bjorn Helgaas

maintainer

2014-04-29 00:17 UTC #2 lore raw

In reply to Fabio Estevam

Quoted 20 lines · Fabio Estevam

From: Fabio Estevam <[email protected]>
Fix the following build warning that happens when building multi_v7_defconfig with CONFIG_ARM_LPAE=y:
drivers/pci/host/pci-mvebu.c:334:5: warning: format '%x' expects argument of type 'unsigned int', but argument 3 has type 'phys_addr_t' [-Wformat=]
While at it, also use '%zx' for printing 'size_t'.
Reported-by Olof's autobuilder <[email protected]>
Signed-off-by Fabio Estevam <[email protected]>
---
 drivers/pci/host/pci-mvebu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/pci/host/pci-mvebu.c +0 -0
index e384e25..572a7b5 100644

drivers/pci/host/pci-mvebu.c static void mvebu_pcie_add_windows(struct mvebu_pcie_port *port, · line 334

Quoted 7 lines

drivers/pci/host/pci-mvebu.c +2 -2

@@ -330,8 +330,8 @@ static void mvebu_pcie_add_windows(struct mvebu_pcie_port *port,
                                                        sz, remap);
                if (ret) {
                        dev_err(&port->pcie->pdev->dev,
-                               "Could not create MBus window at 0x%x, size 0x%x: %d\n",
-                               base, sz, ret);
+                               "Could not create MBus window at %pa, size 0x%zx: %d\n",
+                               &base, sz, ret);

This is OK, but can we use the same "[mem 0x...-0x...]" format that %pR prints a struct resource with? That would make it look more like other similar information we print elsewhere. When you're debugging a problem, sometimes you have to compare address information from different places, and it's convenient if they all look similar.

Quoted 6 lines

drivers/pci/host/pci-mvebu.c +0 -1

                        mvebu_pcie_del_windows(port, base - size_mapped,
                                               size_mapped);
                        return;
--

1.8.3.2

Fabio Estevam

author

2014-04-29 00:52 UTC #3 lore raw

Quoted 26 lines · Bjorn Helgaas

Quoted 25 lines · Fabio Estevam
From: Fabio Estevam <[email protected]>
Fix the following build warning that happens when building multi_v7_defconfig with CONFIG_ARM_LPAE=y:
drivers/pci/host/pci-mvebu.c:334:5: warning: format '%x' expects argument of type 'unsigned int', but argument 3 has type 'phys_addr_t' [-Wformat=]
While at it, also use '%zx' for printing 'size_t'.
Reported-by Olof's autobuilder <[email protected]>
Signed-off-by Fabio Estevam <[email protected]>
---
 drivers/pci/host/pci-mvebu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/pci/host/pci-mvebu.c +0 -2
index e384e25..572a7b5 100644
@@ -330,8 +330,8 @@ static void mvebu_pcie_add_windows(struct mvebu_pcie_port *port,
                                                        sz, remap);
                if (ret) {
                        dev_err(&port->pcie->pdev->dev,
-                               "Could not create MBus window at 0x%x, size 0x%x: %d\n",
-                               base, sz, ret);

Quoted 8 lines

drivers/pci/host/pci-mvebu.c static void mvebu_pcie_add_windows(struct mvebu_pcie_port *port, · line 334
Quoted 2 lines
drivers/pci/host/pci-mvebu.c +2 -0
+                               "Could not create MBus window at %pa, size 0x%zx: %d\n",
+                               &base, sz, ret);
This is OK, but can we use the same "[mem 0x...-0x...]" format that %pR prints a struct resource with? That would make it look more like other similar information we print elsewhere. When you're debugging a problem, sometimes you have to compare address information from different places, and it's convenient if they all look similar.

Understood your point, but in order to use %pR we need to use a 'struct resource', which is something that the current code is not using. 'sz' is calculated inside the function, so it is helpful to print it as is.

IMHO if %pR should be used to improve readability, then it should be handled via a separate patch.

This one does not introduce any functional change.

Bjorn Helgaas

maintainer

2014-04-29 03:07 UTC #4 lore raw

Quoted 36 lines · Fabio Estevam

Quoted 26 lines · Bjorn Helgaas
Quoted 25 lines · Fabio Estevam
From: Fabio Estevam <[email protected]>
Fix the following build warning that happens when building multi_v7_defconfig with CONFIG_ARM_LPAE=y:
drivers/pci/host/pci-mvebu.c:334:5: warning: format '%x' expects argument of type 'unsigned int', but argument 3 has type 'phys_addr_t' [-Wformat=]
While at it, also use '%zx' for printing 'size_t'.
Reported-by Olof's autobuilder <[email protected]>
Signed-off-by Fabio Estevam <[email protected]>
---
 drivers/pci/host/pci-mvebu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/pci/host/pci-mvebu.c +0 -2
index e384e25..572a7b5 100644
@@ -330,8 +330,8 @@ static void mvebu_pcie_add_windows(struct mvebu_pcie_port *port,
                                                        sz, remap);
                if (ret) {
                        dev_err(&port->pcie->pdev->dev,
-                               "Could not create MBus window at 0x%x, size 0x%x: %d\n",
-                               base, sz, ret);
Quoted 8 lines
drivers/pci/host/pci-mvebu.c static void mvebu_pcie_add_windows(struct mvebu_pcie_port *port, · line 334
Quoted 2 lines
drivers/pci/host/pci-mvebu.c +2 -0
+                               "Could not create MBus window at %pa, size 0x%zx: %d\n",
+                               &base, sz, ret);
This is OK, but can we use the same "[mem 0x...-0x...]" format that %pR prints a struct resource with? That would make it look more like other similar information we print elsewhere. When you're debugging a problem, sometimes you have to compare address information from different places, and it's convenient if they all look similar.

Quoted 4 lines

Understood your point, but in order to use %pR we need to use a 'struct resource', which is something that the current code is not using. 'sz' is calculated inside the function, so it is helpful to print it as is.

I didn't suggest changing the code to use an actual "struct resource".
I suggested using a similar printk format, e.g., "[mem
%#010llx-%#010llx]".

Fabio Estevam

author

2014-04-29 03:29 UTC #5 lore raw

Quoted 3 lines · Bjorn Helgaas

I didn't suggest changing the code to use an actual "struct resource".
I suggested using a similar printk format, e.g., "[mem
%#010llx-%#010llx]".

Thanks for the clarification. Just sent v2 as suggested.