* [PATCH] tc: fix warning and coding style
@ 2014-09-30 12:16 Thibaut Robert
2014-09-30 12:28 ` Geert Uytterhoeven
2014-09-30 16:28 ` Maciej W. Rozycki
0 siblings, 2 replies; 6+ messages in thread
From: Thibaut Robert @ 2014-09-30 12:16 UTC (permalink / raw)
To: Maciej W. Rozycki, Ralf Baechle; +Cc: linux-mips, linux-kernel, Thibaut Robert
Fix checkpatch warnings:
WARNING: Prefer [subsystem eg: netdev]_err([subsystem]dev, ... then dev_err(dev, ... then pr_err(... to printk(KERN_ERR ...
WARNING: Possible unnecessary 'out of memory' message
WARNING: quoted string split across lines
WARNING: Use #include <linux/io.h> instead of <asm/io.h>
Fix gcc warning:
warning: format ‘%d’ expects argument of type ‘int’, but argument 4 has type ‘resource_size_t’ [-Wformat=]
As resource_size_t can be 32 or 64 bits (depending on CONFIG_RESOURCES_64BIT), this patch uses "%lld" format along with a cast to u64 for printing resource_size_t values
Signed-off-by: Thibaut Robert <thibaut.robert@gmail.com>
---
drivers/tc/tc.c | 28 ++++++++++++----------------
1 file changed, 12 insertions(+), 16 deletions(-)
diff --git a/drivers/tc/tc.c b/drivers/tc/tc.c
index 9465623..9b1f831 100644
--- a/drivers/tc/tc.c
+++ b/drivers/tc/tc.c
@@ -12,6 +12,7 @@
#include <linux/compiler.h>
#include <linux/errno.h>
#include <linux/init.h>
+#include <linux/io.h>
#include <linux/ioport.h>
#include <linux/kernel.h>
#include <linux/list.h>
@@ -21,8 +22,6 @@
#include <linux/tc.h>
#include <linux/types.h>
-#include <asm/io.h>
-
static struct tc_bus tc_bus = {
.name = "TURBOchannel",
};
@@ -82,11 +81,9 @@ static void __init tc_bus_add_devices(struct tc_bus *tbus)
/* Found a board, allocate it an entry in the list */
tdev = kzalloc(sizeof(*tdev), GFP_KERNEL);
- if (!tdev) {
- printk(KERN_ERR "tc%x: unable to allocate tc_dev\n",
- slot);
+ if (!tdev)
goto out_err;
- }
+
dev_set_name(&tdev->dev, "tc%x", slot);
tdev->bus = tbus;
tdev->dev.parent = &tbus->dev;
@@ -105,7 +102,7 @@ static void __init tc_bus_add_devices(struct tc_bus *tbus)
tdev->vendor[8] = 0;
tdev->name[8] = 0;
- pr_info("%s: %s %s %s\n", dev_name(&tdev->dev), tdev->vendor,
+ dev_info(&tdev->dev, "%s %s %s\n", tdev->vendor,
tdev->name, tdev->firmware);
devsize = readb(module + offset + TC_SLOT_SIZE);
@@ -117,10 +114,10 @@ static void __init tc_bus_add_devices(struct tc_bus *tbus)
tdev->resource.start = extslotaddr;
tdev->resource.end = extslotaddr + devsize - 1;
} else {
- printk(KERN_ERR "%s: Cannot provide slot space "
- "(%dMiB required, up to %dMiB supported)\n",
- dev_name(&tdev->dev), devsize >> 20,
- max(slotsize, extslotsize) >> 20);
+ dev_err(&tdev->dev,
+ "Cannot provide slot space (%lluMiB required, up to %lluMiB supported)\n",
+ (u64) devsize >> 20,
+ (u64) max(slotsize, extslotsize) >> 20);
kfree(tdev);
goto out_err;
}
@@ -159,8 +156,8 @@ static int __init tc_init(void)
if (tc_bus.info.slot_size) {
unsigned int tc_clock = tc_get_speed(&tc_bus) / 100000;
- pr_info("tc: TURBOchannel rev. %d at %d.%d MHz "
- "(with%s parity)\n", tc_bus.info.revision,
+ pr_info("tc: TURBOchannel rev. %d at %d.%d MHz (with%s parity)\n",
+ tc_bus.info.revision,
tc_clock / 10, tc_clock % 10,
tc_bus.info.parity ? "" : "out");
@@ -172,7 +169,7 @@ static int __init tc_init(void)
tc_bus.resource[0].flags = IORESOURCE_MEM;
if (request_resource(&iomem_resource,
&tc_bus.resource[0]) < 0) {
- printk(KERN_ERR "tc: Cannot reserve resource\n");
+ pr_err("tc: Cannot reserve resource\n");
return 0;
}
if (tc_bus.ext_slot_size) {
@@ -184,8 +181,7 @@ static int __init tc_init(void)
tc_bus.resource[1].flags = IORESOURCE_MEM;
if (request_resource(&iomem_resource,
&tc_bus.resource[1]) < 0) {
- printk(KERN_ERR
- "tc: Cannot reserve resource\n");
+ pr_err("tc: Cannot reserve resource\n");
release_resource(&tc_bus.resource[0]);
return 0;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] tc: fix warning and coding style
2014-09-30 12:16 [PATCH] tc: fix warning and coding style Thibaut Robert
@ 2014-09-30 12:28 ` Geert Uytterhoeven
2014-09-30 13:51 ` Thibaut Robert
2014-09-30 16:28 ` Maciej W. Rozycki
1 sibling, 1 reply; 6+ messages in thread
From: Geert Uytterhoeven @ 2014-09-30 12:28 UTC (permalink / raw)
To: Thibaut Robert
Cc: Maciej W. Rozycki, Ralf Baechle, Linux MIPS Mailing List,
linux-kernel@vger.kernel.org
Hi Thibaut,
On Tue, Sep 30, 2014 at 2:16 PM, Thibaut Robert
<thibaut.robert@gmail.com> wrote:
> Fix gcc warning:
> warning: format ‘%d’ expects argument of type ‘int’, but argument 4 has type ‘resource_size_t’ [-Wformat=]
>
> As resource_size_t can be 32 or 64 bits (depending on CONFIG_RESOURCES_64BIT), this patch uses "%lld" format along with a cast to u64 for printing resource_size_t values
Please use %pR instead (cfr. Documentation/printk-formats.txt).
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tc: fix warning and coding style
2014-09-30 12:28 ` Geert Uytterhoeven
@ 2014-09-30 13:51 ` Thibaut Robert
2014-09-30 14:08 ` Geert Uytterhoeven
0 siblings, 1 reply; 6+ messages in thread
From: Thibaut Robert @ 2014-09-30 13:51 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Maciej W. Rozycki, Ralf Baechle, Linux MIPS Mailing List,
linux-kernel@vger.kernel.org
Hi Geert,
Le mardi 30 septembre 2014 à 14:28 +0200, Geert Uytterhoeven a écrit :
> Hi Thibaut,
>
> On Tue, Sep 30, 2014 at 2:16 PM, Thibaut Robert
> <thibaut.robert@gmail.com> wrote:
> > Fix gcc warning:
> > warning: format ‘%d’ expects argument of type ‘int’, but argument 4 has type ‘resource_size_t’ [-Wformat=]
> >
> > As resource_size_t can be 32 or 64 bits (depending on CONFIG_RESOURCES_64BIT), this patch uses "%lld" format along with a cast to u64 for printing resource_size_t values
>
> Please use %pR instead (cfr. Documentation/printk-formats.txt).
This code use 'resource_size_t' but I think %pR is for 'struct resource'. I got inspired by this patch: https://lkml.org/lkml/2008/8/29/187
However, I've thought again, and '%u' is probably enough for displaying a size in MiB. So I propose the following :
(Parens around the cast were also missing in my first patch):
@@ -117,10 +114,10 @@ static void __init tc_bus_add_devices(struct tc_bus *tbus)
tdev->resource.start = extslotaddr;
tdev->resource.end = extslotaddr + devsize - 1;
} else {
- printk(KERN_ERR "%s: Cannot provide slot space "
- "(%dMiB required, up to %dMiB supported)\n",
- dev_name(&tdev->dev), devsize >> 20,
- max(slotsize, extslotsize) >> 20);
+ dev_err(&tdev->dev,
+ "Cannot provide slot space (%uMiB required, up to %uMiB supported)\n",
+ (unsigned int) (devsize >> 20),
+ (unsigned int) (max(slotsize, extslotsize) >> 20));
kfree(tdev);
goto out_err;
}
WDYT ?
Thibaut
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tc: fix warning and coding style
2014-09-30 13:51 ` Thibaut Robert
@ 2014-09-30 14:08 ` Geert Uytterhoeven
2014-09-30 14:31 ` Thibaut Robert
0 siblings, 1 reply; 6+ messages in thread
From: Geert Uytterhoeven @ 2014-09-30 14:08 UTC (permalink / raw)
To: Thibaut Robert
Cc: Maciej W. Rozycki, Ralf Baechle, Linux MIPS Mailing List,
linux-kernel@vger.kernel.org
Hi Thibaut,
On Tue, Sep 30, 2014 at 3:51 PM, Thibaut Robert
<thibaut.robert@gmail.com> wrote:
> Le mardi 30 septembre 2014 à 14:28 +0200, Geert Uytterhoeven a écrit :
>> On Tue, Sep 30, 2014 at 2:16 PM, Thibaut Robert
>> <thibaut.robert@gmail.com> wrote:
>> > Fix gcc warning:
>> > warning: format ‘%d’ expects argument of type ‘int’, but argument 4 has type ‘resource_size_t’ [-Wformat=]
>> >
>> > As resource_size_t can be 32 or 64 bits (depending on CONFIG_RESOURCES_64BIT), this patch uses "%lld" format along with a cast to u64 for printing resource_size_t values
>>
>> Please use %pR instead (cfr. Documentation/printk-formats.txt).
>
> This code use 'resource_size_t' but I think %pR is for 'struct resource'. I got inspired by this patch: https://lkml.org/lkml/2008/8/29/187
Oops, sorry for (mis)behaving like a bad script.
> However, I've thought again, and '%u' is probably enough for displaying a size in MiB. So I propose the following :
> (Parens around the cast were also missing in my first patch):
>
> @@ -117,10 +114,10 @@ static void __init tc_bus_add_devices(struct tc_bus *tbus)
> tdev->resource.start = extslotaddr;
> tdev->resource.end = extslotaddr + devsize - 1;
> } else {
> - printk(KERN_ERR "%s: Cannot provide slot space "
> - "(%dMiB required, up to %dMiB supported)\n",
> - dev_name(&tdev->dev), devsize >> 20,
> - max(slotsize, extslotsize) >> 20);
> + dev_err(&tdev->dev,
> + "Cannot provide slot space (%uMiB required, up to %uMiB supported)\n",
> + (unsigned int) (devsize >> 20),
> + (unsigned int) (max(slotsize, extslotsize) >> 20));
> kfree(tdev);
> goto out_err;
> }
>
>
> WDYT ?
I guess that's acceptable, as tc is probably limited to 32-bit anyway. Maciej?
Note that there's also %pa, for phys_addr_t/resource_size_t.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tc: fix warning and coding style
2014-09-30 14:08 ` Geert Uytterhoeven
@ 2014-09-30 14:31 ` Thibaut Robert
0 siblings, 0 replies; 6+ messages in thread
From: Thibaut Robert @ 2014-09-30 14:31 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Maciej W. Rozycki, Ralf Baechle, Linux MIPS Mailing List,
linux-kernel@vger.kernel.org
Le mardi 30 septembre 2014 à 16:08 +0200, Geert Uytterhoeven a écrit :
> I guess that's acceptable, as tc is probably limited to 32-bit anyway. Maciej?
>
> Note that there's also %pa, for phys_addr_t/resource_size_t.
Yes, but unfortunately that needs a reference so we can't use it.
Thibaut
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tc: fix warning and coding style
2014-09-30 12:16 [PATCH] tc: fix warning and coding style Thibaut Robert
2014-09-30 12:28 ` Geert Uytterhoeven
@ 2014-09-30 16:28 ` Maciej W. Rozycki
1 sibling, 0 replies; 6+ messages in thread
From: Maciej W. Rozycki @ 2014-09-30 16:28 UTC (permalink / raw)
To: Thibaut Robert, Ralf Baechle; +Cc: linux-mips, linux-kernel
On Tue, 30 Sep 2014, Thibaut Robert wrote:
> Fix checkpatch warnings:
> WARNING: Prefer [subsystem eg: netdev]_err([subsystem]dev, ... then dev_err(dev, ... then pr_err(... to printk(KERN_ERR ...
> WARNING: Possible unnecessary 'out of memory' message
> WARNING: quoted string split across lines
> WARNING: Use #include <linux/io.h> instead of <asm/io.h>
>
> Fix gcc warning:
> warning: format ‘%d’ expects argument of type ‘int’, but argument 4 has type ‘resource_size_t’ [-Wformat=]
>
> As resource_size_t can be 32 or 64 bits (depending on CONFIG_RESOURCES_64BIT), this patch uses "%lld" format along with a cast to u64 for printing resource_size_t values
>
> Signed-off-by: Thibaut Robert <thibaut.robert@gmail.com>
> ---
NAK. These issues have already been taken care of via the LMO tree; the
original change has been archived here:
http://www.linux-mips.org/cgi-bin/mesg.cgi?a=linux-mips&i=alpine.LFD.2.11.1404062030280.15266%40eddie.linux-mips.org
and is on the way to Linus's tree (IIUC; Ralf, please acknowledge).
If you think there's anything wrong still left afterwards, except from
the message wrapping (as I'm not going to approve any modification to go
beyond 79 columns; this is nonsense), then please send an incremental
change on top of that.
Thanks for your contribution anyway.
Maciej
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-09-30 16:28 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-30 12:16 [PATCH] tc: fix warning and coding style Thibaut Robert
2014-09-30 12:28 ` Geert Uytterhoeven
2014-09-30 13:51 ` Thibaut Robert
2014-09-30 14:08 ` Geert Uytterhoeven
2014-09-30 14:31 ` Thibaut Robert
2014-09-30 16:28 ` Maciej W. Rozycki
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.