| Message ID | 1398682281-18772-1-git-send-email-festevam@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
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
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
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
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
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 --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;