diff mbox

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

Message ID 1398682281-18772-1-git-send-email-festevam@gmail.com (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Fabio Estevam April 28, 2014, 10:51 a.m. UTC
From: Fabio Estevam <fabio.estevam@freescale.com>

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 <build@lixom.net>
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
 drivers/pci/host/pci-mvebu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Jingoo Han April 28, 2014, 11:08 a.m. UTC | #1
On Monday, April 28, 2014 7:51 PM, Fabio Estevam wrote:
> 
> From: Fabio Estevam <fabio.estevam@freescale.com>
> 
> 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 <build@lixom.net>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>

Reviewed-by: Jingoo Han <jg1.han@samsung.com>

Best regards,
Jingoo Han

> ---
>  drivers/pci/host/pci-mvebu.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c
> index e384e25..572a7b5 100644
> --- a/drivers/pci/host/pci-mvebu.c
> +++ b/drivers/pci/host/pci-mvebu.c
> @@ -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;
> --
> 1.8.3.2

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bjorn Helgaas April 29, 2014, 12:16 a.m. UTC | #2
On Mon, Apr 28, 2014 at 4:51 AM, Fabio Estevam <festevam@gmail.com> wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
>
> 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 <build@lixom.net>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
>  drivers/pci/host/pci-mvebu.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c
> index e384e25..572a7b5 100644
> --- a/drivers/pci/host/pci-mvebu.c
> +++ b/drivers/pci/host/pci-mvebu.c
> @@ -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.

>                         mvebu_pcie_del_windows(port, base - size_mapped,
>                                                size_mapped);
>                         return;
> --
> 1.8.3.2
>
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Fabio Estevam April 29, 2014, 12:52 a.m. UTC | #3
On Mon, Apr 28, 2014 at 9:16 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Mon, Apr 28, 2014 at 4:51 AM, Fabio Estevam <festevam@gmail.com> wrote:
>> From: Fabio Estevam <fabio.estevam@freescale.com>
>>
>> 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 <build@lixom.net>
>> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
>> ---
>>  drivers/pci/host/pci-mvebu.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c
>> index e384e25..572a7b5 100644
>> --- a/drivers/pci/host/pci-mvebu.c
>> +++ b/drivers/pci/host/pci-mvebu.c
>> @@ -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.

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.
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bjorn Helgaas April 29, 2014, 3:06 a.m. UTC | #4
On Mon, Apr 28, 2014 at 6:52 PM, Fabio Estevam <festevam@gmail.com> wrote:
> On Mon, Apr 28, 2014 at 9:16 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> On Mon, Apr 28, 2014 at 4:51 AM, Fabio Estevam <festevam@gmail.com> wrote:
>>> From: Fabio Estevam <fabio.estevam@freescale.com>
>>>
>>> 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 <build@lixom.net>
>>> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
>>> ---
>>>  drivers/pci/host/pci-mvebu.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c
>>> index e384e25..572a7b5 100644
>>> --- a/drivers/pci/host/pci-mvebu.c
>>> +++ b/drivers/pci/host/pci-mvebu.c
>>> @@ -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.
>
> 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]".
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Fabio Estevam April 29, 2014, 3:29 a.m. UTC | #5
On Tue, Apr 29, 2014 at 12:06 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:

> 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c
index e384e25..572a7b5 100644
--- a/drivers/pci/host/pci-mvebu.c
+++ b/drivers/pci/host/pci-mvebu.c
@@ -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;