Hi Wim,
Today's linux-next merge of the watchdog tree got a conflict in
drivers/watchdog/iTCO_wdt.c between commit 887c8ec7219f ("watchdog:
Convert iTCO_wdt driver to mfd model") from the tree and commit
c3614aa19d3e ("watchdog: iTCO_wdt.c: fix printk format warnings") from
the watchdog tree.
I fixed it up (see below) and can carry the fix as necessary.
--
Cheers,
Stephen Rothwell [email protected]
diff --cc drivers/watchdog/iTCO_wdt.c
index 741528b,2aab56f..0000000
--- a/drivers/watchdog/iTCO_wdt.c
+++ b/drivers/watchdog/iTCO_wdt.c
@@@ -572,33 -837,32 +572,33 @@@ static int __devinit iTCO_wdt_probe(str
iTCO_wdt_set_NO_REBOOT_bit();
/* The TCO logic uses the TCO_EN bit in the SMI_EN register */
- if (!request_region(SMI_EN, 4, "iTCO_wdt")) {
- pr_err("I/O address 0x%04lx already in use, device disabled\n",
+ if (!request_region(iTCO_wdt_private.smi_res->start,
+ resource_size(iTCO_wdt_private.smi_res), dev->name)) {
+ pr_err("I/O address 0x%04llx already in use, device disabled\n",
- SMI_EN);
+ (u64)SMI_EN);
- ret = -EIO;
- goto out_unmap;
+ ret = -EBUSY;
+ goto unmap_gcs;
}
if (turn_SMI_watchdog_clear_off >= iTCO_wdt_private.iTCO_version) {
- /* Bit 13: TCO_EN -> 0 = Disables TCO logic generating an SMI# */
+ /*
+ * Bit 13: TCO_EN -> 0
+ * Disables TCO logic generating an SMI#
+ */
val32 = inl(SMI_EN);
val32 &= 0xffffdfff; /* Turn off SMI clearing watchdog */
outl(val32, SMI_EN);
}
- /* The TCO I/O registers reside in a 32-byte range pointed to
- by the TCOBASE value */
- if (!request_region(TCOBASE, 0x20, "iTCO_wdt")) {
- pr_err("I/O address 0x%04lx already in use, device disabled\n",
+ if (!request_region(iTCO_wdt_private.tco_res->start,
+ resource_size(iTCO_wdt_private.tco_res), dev->name)) {
+ pr_err("I/O address 0x%04llx already in use, device disabled\n",
- TCOBASE);
+ (u64)TCOBASE);
- ret = -EIO;
- goto unreg_smi_en;
+ ret = -EBUSY;
+ goto unreg_smi;
}
- pr_info("Found a %s TCO device (Version=%d, TCOBASE=0x%04lx)\n",
- iTCO_chipset_info[ent->driver_data].name,
- iTCO_chipset_info[ent->driver_data].iTCO_version,
- (u64)TCOBASE);
+ pr_info("Found a %s TCO device (Version=%d, TCOBASE=0x%04llx)\n",
- ich_info->name, ich_info->iTCO_version, TCOBASE);
++ ich_info->name, ich_info->iTCO_version, (u64)TCOBASE);
/* Clear out the (probably old) status */
outw(0x0008, TCO1_STS); /* Clear the Time Out Status bit */
Hi Stephen,
> Today's linux-next merge of the watchdog tree got a conflict in
> drivers/watchdog/iTCO_wdt.c between commit 887c8ec7219f ("watchdog:
> Convert iTCO_wdt driver to mfd model") from the tree and commit
> c3614aa19d3e ("watchdog: iTCO_wdt.c: fix printk format warnings") from
> the watchdog tree.
>
> I fixed it up (see below) and can carry the fix as necessary.
Thanks for fixing this up.
But I'm a bit surprised: I wasn't Cc'ed about this patch to Convert
the iTCO_wdt driver to the mfd model...
Kind regards,
Wim.
At 01:32 AM 5/18/2012, Wim Van Sebroeck wrote:
>Hi Stephen,
>
> > Today's linux-next merge of the watchdog tree got a conflict in
> > drivers/watchdog/iTCO_wdt.c between commit 887c8ec7219f ("watchdog:
> > Convert iTCO_wdt driver to mfd model") from the tree and commit
> > c3614aa19d3e ("watchdog: iTCO_wdt.c: fix printk format warnings") from
> > the watchdog tree.
> >
> > I fixed it up (see below) and can carry the fix as necessary.
>
>Thanks for fixing this up.
>
>But I'm a bit surprised: I wasn't Cc'ed about this patch to Convert
>the iTCO_wdt driver to the mfd model...
Oh my. Kind of embarrassing to forget one of the maintainers in all
the discussions which tree this patch set should go through. Even
though I didn't much of the code, my sign-off is there, and I should
have made sure that you are on the Cc: list. Sorry for that, and I
owe you a beer or two if we ever meet in person.
Hope you are ok with the changes - the patch set already missed the
last commit window because of the which-tree-to-use issue, and it
would be sad to miss another one. And Jean is really waiting for it
to go in to be able to push some related patches which need the gpio driver.
Thanks,
Guenter
Hi Guenter,
> >> Today's linux-next merge of the watchdog tree got a conflict in
> >> drivers/watchdog/iTCO_wdt.c between commit 887c8ec7219f ("watchdog:
> >> Convert iTCO_wdt driver to mfd model") from the tree and commit
> >> c3614aa19d3e ("watchdog: iTCO_wdt.c: fix printk format warnings") from
> >> the watchdog tree.
> >>
> >> I fixed it up (see below) and can carry the fix as necessary.
> >
> >Thanks for fixing this up.
> >
> >But I'm a bit surprised: I wasn't Cc'ed about this patch to Convert
> >the iTCO_wdt driver to the mfd model...
> Oh my. Kind of embarrassing to forget one of the maintainers in all
> the discussions which tree this patch set should go through. Even
> though I didn't much of the code, my sign-off is there, and I should
> have made sure that you are on the Cc: list. Sorry for that, and I
> owe you a beer or two if we ever meet in person.
The maintainer and the writer of the driver: that's at least two beers indeed :-).
> Hope you are ok with the changes - the patch set already missed the
> last commit window because of the which-tree-to-use issue, and it
> would be sad to miss another one. And Jean is really waiting for it
> to go in to be able to push some related patches which need the gpio driver.
I'm OK with it (although this means I can't go ahead yet with a watchdog core
conversion of the iTCO_wdt driver), except for the comile warning that the patch
introduces (as reported by Randy).
The original driver printed addresses as %04lx, this new driver uses %04llx but
doesn't typecast the values correctly. See message from Randy below.
Wether or not %04lx or %04llx is the best to use is off-course another question.
I don't think we have a clear standard yet, but I'm fine with either one of them.
Kind regards,
Wim.
---
Date: Mon, 14 May 2012 13:15:20 -0700
From: Randy Dunlap <[email protected]>
To: Stephen Rothwell <[email protected]>
CC: [email protected], LKML <[email protected]>,
Wim Van Sebroeck <[email protected]>,
[email protected]
Subject: [PATCH -next] wdt: fix iTCO printk format warnings
From: Randy Dunlap <[email protected]>
Fix printk format warnings:
drivers/watchdog/iTCO_wdt.c:577:3: warning: format '%04llx' expects type 'long long unsigned int', but argument 2 has type 'resource_size_t'
drivers/watchdog/iTCO_wdt.c:594:3: warning: format '%04llx' expects type 'long long unsigned int', but argument 2 has type 'resource_size_t'
drivers/watchdog/iTCO_wdt.c:600:2: warning: format '%04llx' expects type 'long long unsigned int', but argument 4 has type 'resource_size_t'
Signed-off-by: Randy Dunlap <[email protected]>
Cc: Wim Van Sebroeck <[email protected]>
---
drivers/watchdog/iTCO_wdt.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
--- linux-next-20120514.orig/drivers/watchdog/iTCO_wdt.c
+++ linux-next-20120514/drivers/watchdog/iTCO_wdt.c
@@ -575,7 +575,7 @@ static int __devinit iTCO_wdt_probe(stru
if (!request_region(iTCO_wdt_private.smi_res->start,
resource_size(iTCO_wdt_private.smi_res), dev->name)) {
pr_err("I/O address 0x%04llx already in use, device disabled\n",
- SMI_EN);
+ (u64)SMI_EN);
ret = -EBUSY;
goto unmap_gcs;
}
@@ -592,13 +592,13 @@ static int __devinit iTCO_wdt_probe(stru
if (!request_region(iTCO_wdt_private.tco_res->start,
resource_size(iTCO_wdt_private.tco_res), dev->name)) {
pr_err("I/O address 0x%04llx already in use, device disabled\n",
- TCOBASE);
+ (u64)TCOBASE);
ret = -EBUSY;
goto unreg_smi;
}
pr_info("Found a %s TCO device (Version=%d, TCOBASE=0x%04llx)\n",
- ich_info->name, ich_info->iTCO_version, TCOBASE);
+ ich_info->name, ich_info->iTCO_version, (u64)TCOBASE);
/* Clear out the (probably old) status */
outw(0x0008, TCO1_STS); /* Clear the Time Out Status bit */
At 09:35 AM 5/22/2012, Wim Van Sebroeck wrote:
>Hi Guenter,
>
> > >> Today's linux-next merge of the watchdog tree got a conflict in
> > >> drivers/watchdog/iTCO_wdt.c between commit 887c8ec7219f ("watchdog:
> > >> Convert iTCO_wdt driver to mfd model") from the tree and commit
> > >> c3614aa19d3e ("watchdog: iTCO_wdt.c: fix printk format warnings") from
> > >> the watchdog tree.
> > >>
> > >> I fixed it up (see below) and can carry the fix as necessary.
> > >
> > >Thanks for fixing this up.
> > >
> > >But I'm a bit surprised: I wasn't Cc'ed about this patch to Convert
> > >the iTCO_wdt driver to the mfd model...
> > Oh my. Kind of embarrassing to forget one of the maintainers in all
> > the discussions which tree this patch set should go through. Even
> > though I didn't much of the code, my sign-off is there, and I should
> > have made sure that you are on the Cc: list. Sorry for that, and I
> > owe you a beer or two if we ever meet in person.
>
>The maintainer and the writer of the driver: that's at least two
>beers indeed :-).
Make it three. Going to be a happy night.
> > Hope you are ok with the changes - the patch set already missed the
> > last commit window because of the which-tree-to-use issue, and it
> > would be sad to miss another one. And Jean is really waiting for it
> > to go in to be able to push some related patches which need the
> gpio driver.
>
>I'm OK with it (although this means I can't go ahead yet with a watchdog core
>conversion of the iTCO_wdt driver), except for the comile warning
>that the patch
>introduces (as reported by Randy).
>
>The original driver printed addresses as %04lx, this new driver uses
>%04llx but
>doesn't typecast the values correctly. See message from Randy below.
>Wether or not %04lx or %04llx is the best to use is off-course
>another question.
>I don't think we have a clear standard yet, but I'm fine with either
>one of them.
Me too, though %04lx and typecast to long should really be sufficient.
I'd guess the change was made because resource_size_t is sometimes a long
and sometimes a long long, depending on the platform. I hit that
problem a couple of times myself.
How do we make sure that Randy's patch makes it upstream ? The iTCO
patch is currently in Sam's tree.
Thanks,
Guenter
>Kind regards,
>Wim.
>---
>Date: Mon, 14 May 2012 13:15:20 -0700
>From: Randy Dunlap <[email protected]>
>To: Stephen Rothwell <[email protected]>
>CC: [email protected], LKML <[email protected]>,
> Wim Van Sebroeck <[email protected]>,
> [email protected]
>Subject: [PATCH -next] wdt: fix iTCO printk format warnings
>
>From: Randy Dunlap <[email protected]>
>
>Fix printk format warnings:
>
>drivers/watchdog/iTCO_wdt.c:577:3: warning: format '%04llx' expects
>type 'long long unsigned int', but argument 2 has type 'resource_size_t'
>drivers/watchdog/iTCO_wdt.c:594:3: warning: format '%04llx' expects
>type 'long long unsigned int', but argument 2 has type 'resource_size_t'
>drivers/watchdog/iTCO_wdt.c:600:2: warning: format '%04llx' expects
>type 'long long unsigned int', but argument 4 has type 'resource_size_t'
>
>Signed-off-by: Randy Dunlap <[email protected]>
>Cc: Wim Van Sebroeck <[email protected]>
>---
> drivers/watchdog/iTCO_wdt.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
>--- linux-next-20120514.orig/drivers/watchdog/iTCO_wdt.c
>+++ linux-next-20120514/drivers/watchdog/iTCO_wdt.c
>@@ -575,7 +575,7 @@ static int __devinit iTCO_wdt_probe(stru
> if (!request_region(iTCO_wdt_private.smi_res->start,
> resource_size(iTCO_wdt_private.smi_res),
> dev->name)) {
> pr_err("I/O address 0x%04llx already in use, device
> disabled\n",
>- SMI_EN);
>+ (u64)SMI_EN);
> ret = -EBUSY;
> goto unmap_gcs;
> }
>@@ -592,13 +592,13 @@ static int __devinit iTCO_wdt_probe(stru
> if (!request_region(iTCO_wdt_private.tco_res->start,
> resource_size(iTCO_wdt_private.tco_res),
> dev->name)) {
> pr_err("I/O address 0x%04llx already in use, device
> disabled\n",
>- TCOBASE);
>+ (u64)TCOBASE);
> ret = -EBUSY;
> goto unreg_smi;
> }
>
> pr_info("Found a %s TCO device (Version=%d, TCOBASE=0x%04llx)\n",
>- ich_info->name, ich_info->iTCO_version, TCOBASE);
>+ ich_info->name, ich_info->iTCO_version, (u64)TCOBASE);
>
> /* Clear out the (probably old) status */
> outw(0x0008, TCO1_STS); /* Clear the Time Out Status bit */
Hi guys,
On Tue, 22 May 2012 15:37:04 -0700 Guenter Roeck <[email protected]> wrote:
>
> >I don't think we have a clear standard yet, but I'm fine with either
> >one of them.
>
> Me too, though %04lx and typecast to long should really be sufficient.
>
> I'd guess the change was made because resource_size_t is sometimes a long
> and sometimes a long long, depending on the platform. I hit that
> problem a couple of times myself.
Exactly. Since resource_size_t can be either 32 bit or 64 bit on 32 bit
platforms, you must print it as %llx and cast it to u64 always to
prevent these warnings. If you cast it to (unsigned) long you could
possibly truncate the value.
--
Cheers,
Stephen Rothwell [email protected]
Hi Sam,
Since %04llx with a typecast to u64 is the way to go (Thanks Stephen and Randy)...
> How do we make sure that Randy's patch makes it upstream ? The iTCO
> patch is currently in Sam's tree.
Can you carry Randy's patch/fix (since it fixes the change introduced by the iTCO_wdt mfd patch)?
If not then I will apply the patch later on once the iTCO_wdt mfd patch is mainstream.
Kind regards,
Wim.
Hi Wim,
On Wed, May 23, 2012 at 07:44:30AM +0200, Wim Van Sebroeck wrote:
> Hi Sam,
>
> Since %04llx with a typecast to u64 is the way to go (Thanks Stephen and Randy)...
>
> > How do we make sure that Randy's patch makes it upstream ? The iTCO
> > patch is currently in Sam's tree.
>
> Can you carry Randy's patch/fix (since it fixes the change introduced by the iTCO_wdt mfd patch)?
> If not then I will apply the patch later on once the iTCO_wdt mfd patch is mainstream.
>
I'd appreciate if you could take it. I'm trying to avoid taking more patches
at this moment.
Cheers,
Samuel.
--
Intel Open Source Technology Centre
http://oss.intel.com/
Hi Sam,
> > Since %04llx with a typecast to u64 is the way to go (Thanks Stephen and Randy)...
> >
> > > How do we make sure that Randy's patch makes it upstream ? The iTCO
> > > patch is currently in Sam's tree.
> >
> > Can you carry Randy's patch/fix (since it fixes the change introduced by the iTCO_wdt mfd patch)?
> > If not then I will apply the patch later on once the iTCO_wdt mfd patch is mainstream.
> >
> I'd appreciate if you could take it. I'm trying to avoid taking more patches
> at this moment.
No problem. I'll take it then.
Kind regards,
Wim.