[PATCH] watchdog: another ioremap() fix

[PATCH] watchdog: another ioremap() fix

[Felipe Balbi]

convert to use ioremap() and __raw_{read/write} friends.

Signed-off-by: Felipe Balbi <felipe.balbi@nokia.com>
---
 drivers/watchdog/omap_wdt.c |   52 +++++++++++++++++++++++++-----------------
 1 files changed, 31 insertions(+), 21 deletions(-)

diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
index b111dec..e395743 100644
--- a/drivers/watchdog/omap_wdt.c
+++ b/drivers/watchdog/omap_wdt.c
@@ -71,12 +71,12 @@ static void omap_wdt_ping(struct omap_wdt_dev *wdev)
 {
 	void __iomem    *base = wdev->base;
 	/* wait for posted write to complete */
-	while ((omap_readl(base + OMAP_WATCHDOG_WPS)) & 0x08)
+	while ((__raw_readl(base + OMAP_WATCHDOG_WPS)) & 0x08)
 		cpu_relax();
 	wdt_trgr_pattern = ~wdt_trgr_pattern;
-	omap_writel(wdt_trgr_pattern, (base + OMAP_WATCHDOG_TGR));
+	__raw_writel(wdt_trgr_pattern, (base + OMAP_WATCHDOG_TGR));
 	/* wait for posted write to complete */
-	while ((omap_readl(base + OMAP_WATCHDOG_WPS)) & 0x08)
+	while ((__raw_readl(base + OMAP_WATCHDOG_WPS)) & 0x08)
 		cpu_relax();
 	/* reloaded WCRR from WLDR */
 }
@@ -86,11 +86,11 @@ static void omap_wdt_enable(struct omap_wdt_dev *wdev)
 	void __iomem *base;
 	base = wdev->base;
 	/* Sequence to enable the watchdog */
-	omap_writel(0xBBBB, base + OMAP_WATCHDOG_SPR);
-	while ((omap_readl(base + OMAP_WATCHDOG_WPS)) & 0x10)
+	__raw_writel(0xBBBB, base + OMAP_WATCHDOG_SPR);
+	while ((__raw_readl(base + OMAP_WATCHDOG_WPS)) & 0x10)
 		cpu_relax();
-	omap_writel(0x4444, base + OMAP_WATCHDOG_SPR);
-	while ((omap_readl(base + OMAP_WATCHDOG_WPS)) & 0x10)
+	__raw_writel(0x4444, base + OMAP_WATCHDOG_SPR);
+	while ((__raw_readl(base + OMAP_WATCHDOG_WPS)) & 0x10)
 		cpu_relax();
 }
 
@@ -99,11 +99,11 @@ static void omap_wdt_disable(struct omap_wdt_dev *wdev)
 	void __iomem *base;
 	base = wdev->base;
 	/* sequence required to disable watchdog */
-	omap_writel(0xAAAA, base + OMAP_WATCHDOG_SPR);	/* TIMER_MODE */
-	while (omap_readl(base + OMAP_WATCHDOG_WPS) & 0x10)
+	__raw_writel(0xAAAA, base + OMAP_WATCHDOG_SPR);	/* TIMER_MODE */
+	while (__raw_readl(base + OMAP_WATCHDOG_WPS) & 0x10)
 		cpu_relax();
-	omap_writel(0x5555, base + OMAP_WATCHDOG_SPR);	/* TIMER_MODE */
-	while (omap_readl(base + OMAP_WATCHDOG_WPS) & 0x10)
+	__raw_writel(0x5555, base + OMAP_WATCHDOG_SPR);	/* TIMER_MODE */
+	while (__raw_readl(base + OMAP_WATCHDOG_WPS) & 0x10)
 		cpu_relax();
 }
 
@@ -123,10 +123,10 @@ static void omap_wdt_set_timeout(struct omap_wdt_dev *wdev)
 	base = wdev->base;
 
 	/* just count up at 32 KHz */
-	while (omap_readl(base + OMAP_WATCHDOG_WPS) & 0x04)
+	while (__raw_readl(base + OMAP_WATCHDOG_WPS) & 0x04)
 		cpu_relax();
-	omap_writel(pre_margin, base + OMAP_WATCHDOG_LDR);
-	while (omap_readl(base + OMAP_WATCHDOG_WPS) & 0x04)
+	__raw_writel(pre_margin, base + OMAP_WATCHDOG_LDR);
+	while (__raw_readl(base + OMAP_WATCHDOG_WPS) & 0x04)
 		cpu_relax();
 }
 
@@ -152,10 +152,10 @@ static int omap_wdt_open(struct inode *inode, struct file *file)
 	}
 
 	/* initialize prescaler */
-	while (omap_readl(base + OMAP_WATCHDOG_WPS) & 0x01)
+	while (__raw_readl(base + OMAP_WATCHDOG_WPS) & 0x01)
 		cpu_relax();
-	omap_writel((1 << 5) | (PTV << 2), base + OMAP_WATCHDOG_CNTRL);
-	while (omap_readl(base + OMAP_WATCHDOG_WPS) & 0x01)
+	__raw_writel((1 << 5) | (PTV << 2), base + OMAP_WATCHDOG_CNTRL);
+	while (__raw_readl(base + OMAP_WATCHDOG_WPS) & 0x01)
 		cpu_relax();
 
 	file->private_data = (void *) wdev;
@@ -225,7 +225,7 @@ omap_wdt_ioctl(struct inode *inode, struct file *file,
 		return put_user(0, (int __user *)arg);
 	case WDIOC_GETBOOTSTATUS:
 		if (cpu_is_omap16xx())
-			return put_user(omap_readw(ARM_SYSST),
+			return put_user(__raw_readw(ARM_SYSST),
 					(int __user *)arg);
 		if (cpu_is_omap24xx())
 			return put_user(omap_prcm_get_reset_sources(),
@@ -324,7 +324,12 @@ static int __init omap_wdt_probe(struct platform_device *pdev)
 			goto fail;
 		}
 	}
-	wdev->base = (void __iomem *) (mem->start);
+	wdev->base = ioremap(res->start, res->end - res->start + 1);
+	if (!wdev->base) {
+		ret = -ENOMEM;
+		goto fail;
+	}
+
 	platform_set_drvdata(pdev, wdev);
 
 	omap_wdt_disable(wdev);
@@ -340,11 +345,11 @@ static int __init omap_wdt_probe(struct platform_device *pdev)
 		goto fail;
 
 	pr_info("OMAP Watchdog Timer Rev 0x%02x: initial timeout %d sec\n",
-		omap_readl(wdev->base + OMAP_WATCHDOG_REV) & 0xFF,
+		__raw_readl(wdev->base + OMAP_WATCHDOG_REV) & 0xFF,
 		timer_margin);
 
 	/* autogate OCP interface clock */
-	omap_writel(0x01, wdev->base + OMAP_WATCHDOG_SYS_CONFIG);
+	__raw_writel(0x01, wdev->base + OMAP_WATCHDOG_SYS_CONFIG);
 
 	omap_wdt_dev = pdev;
 
@@ -359,6 +364,8 @@ fail:
 			clk_put(wdev->mpu_wdt_ick);
 		if (wdev->mpu_wdt_fck)
 			clk_put(wdev->mpu_wdt_fck);
+		if (wdev->base)
+			iounmap(wdev->base);
 		kfree(wdev);
 	}
 	if (mem) {
@@ -396,6 +403,9 @@ static int omap_wdt_remove(struct platform_device *pdev)
 		clk_put(wdev->mpu_wdt_fck);
 		wdev->mpu_wdt_fck = NULL;
 	}
+	if (wdev->base)
+		iounmap(wdev->base);
+
 	kfree(wdev);
 	omap_wdt_dev = NULL;
 	return 0;
-- 
1.6.0.1.308.gede4c

Re: [PATCH] watchdog: another ioremap() fix

[David Brownell]

On Thursday 18 September 2008, Felipe Balbi wrote:
> convert to use ioremap() and __raw_{read/write} friends.

Why is this going to the OMAP list, not to LKML and cc'd
the watchdog maintainer?  (I added Wim to the cc list...)

If it's worth having, it's worth having in mainline...



> Signed-off-by: Felipe Balbi <felipe.balbi@nokia.com>
> ---
>  drivers/watchdog/omap_wdt.c |   52 +++++++++++++++++++++++++-----------------
>  1 files changed, 31 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
> index b111dec..e395743 100644
> --- a/drivers/watchdog/omap_wdt.c
> +++ b/drivers/watchdog/omap_wdt.c
> @@ -71,12 +71,12 @@ static void omap_wdt_ping(struct omap_wdt_dev *wdev)
>  {
>  	void __iomem    *base = wdev->base;
>  	/* wait for posted write to complete */
> -	while ((omap_readl(base + OMAP_WATCHDOG_WPS)) & 0x08)
> +	while ((__raw_readl(base + OMAP_WATCHDOG_WPS)) & 0x08)
>  		cpu_relax();
>  	wdt_trgr_pattern = ~wdt_trgr_pattern;
> -	omap_writel(wdt_trgr_pattern, (base + OMAP_WATCHDOG_TGR));
> +	__raw_writel(wdt_trgr_pattern, (base + OMAP_WATCHDOG_TGR));
>  	/* wait for posted write to complete */
> -	while ((omap_readl(base + OMAP_WATCHDOG_WPS)) & 0x08)
> +	while ((__raw_readl(base + OMAP_WATCHDOG_WPS)) & 0x08)
>  		cpu_relax();
>  	/* reloaded WCRR from WLDR */
>  }
> @@ -86,11 +86,11 @@ static void omap_wdt_enable(struct omap_wdt_dev *wdev)
>  	void __iomem *base;
>  	base = wdev->base;
>  	/* Sequence to enable the watchdog */
> -	omap_writel(0xBBBB, base + OMAP_WATCHDOG_SPR);
> -	while ((omap_readl(base + OMAP_WATCHDOG_WPS)) & 0x10)
> +	__raw_writel(0xBBBB, base + OMAP_WATCHDOG_SPR);
> +	while ((__raw_readl(base + OMAP_WATCHDOG_WPS)) & 0x10)
>  		cpu_relax();
> -	omap_writel(0x4444, base + OMAP_WATCHDOG_SPR);
> -	while ((omap_readl(base + OMAP_WATCHDOG_WPS)) & 0x10)
> +	__raw_writel(0x4444, base + OMAP_WATCHDOG_SPR);
> +	while ((__raw_readl(base + OMAP_WATCHDOG_WPS)) & 0x10)
>  		cpu_relax();
>  }
>  
> @@ -99,11 +99,11 @@ static void omap_wdt_disable(struct omap_wdt_dev *wdev)
>  	void __iomem *base;
>  	base = wdev->base;
>  	/* sequence required to disable watchdog */
> -	omap_writel(0xAAAA, base + OMAP_WATCHDOG_SPR);	/* TIMER_MODE */
> -	while (omap_readl(base + OMAP_WATCHDOG_WPS) & 0x10)
> +	__raw_writel(0xAAAA, base + OMAP_WATCHDOG_SPR);	/* TIMER_MODE */
> +	while (__raw_readl(base + OMAP_WATCHDOG_WPS) & 0x10)
>  		cpu_relax();
> -	omap_writel(0x5555, base + OMAP_WATCHDOG_SPR);	/* TIMER_MODE */
> -	while (omap_readl(base + OMAP_WATCHDOG_WPS) & 0x10)
> +	__raw_writel(0x5555, base + OMAP_WATCHDOG_SPR);	/* TIMER_MODE */
> +	while (__raw_readl(base + OMAP_WATCHDOG_WPS) & 0x10)
>  		cpu_relax();
>  }
>  
> @@ -123,10 +123,10 @@ static void omap_wdt_set_timeout(struct omap_wdt_dev *wdev)
>  	base = wdev->base;
>  
>  	/* just count up at 32 KHz */
> -	while (omap_readl(base + OMAP_WATCHDOG_WPS) & 0x04)
> +	while (__raw_readl(base + OMAP_WATCHDOG_WPS) & 0x04)
>  		cpu_relax();
> -	omap_writel(pre_margin, base + OMAP_WATCHDOG_LDR);
> -	while (omap_readl(base + OMAP_WATCHDOG_WPS) & 0x04)
> +	__raw_writel(pre_margin, base + OMAP_WATCHDOG_LDR);
> +	while (__raw_readl(base + OMAP_WATCHDOG_WPS) & 0x04)
>  		cpu_relax();
>  }
>  
> @@ -152,10 +152,10 @@ static int omap_wdt_open(struct inode *inode, struct file *file)
>  	}
>  
>  	/* initialize prescaler */
> -	while (omap_readl(base + OMAP_WATCHDOG_WPS) & 0x01)
> +	while (__raw_readl(base + OMAP_WATCHDOG_WPS) & 0x01)
>  		cpu_relax();
> -	omap_writel((1 << 5) | (PTV << 2), base + OMAP_WATCHDOG_CNTRL);
> -	while (omap_readl(base + OMAP_WATCHDOG_WPS) & 0x01)
> +	__raw_writel((1 << 5) | (PTV << 2), base + OMAP_WATCHDOG_CNTRL);
> +	while (__raw_readl(base + OMAP_WATCHDOG_WPS) & 0x01)
>  		cpu_relax();
>  
>  	file->private_data = (void *) wdev;
> @@ -225,7 +225,7 @@ omap_wdt_ioctl(struct inode *inode, struct file *file,
>  		return put_user(0, (int __user *)arg);
>  	case WDIOC_GETBOOTSTATUS:
>  		if (cpu_is_omap16xx())
> -			return put_user(omap_readw(ARM_SYSST),
> +			return put_user(__raw_readw(ARM_SYSST),
>  					(int __user *)arg);
>  		if (cpu_is_omap24xx())
>  			return put_user(omap_prcm_get_reset_sources(),
> @@ -324,7 +324,12 @@ static int __init omap_wdt_probe(struct platform_device *pdev)
>  			goto fail;
>  		}
>  	}
> -	wdev->base = (void __iomem *) (mem->start);
> +	wdev->base = ioremap(res->start, res->end - res->start + 1);
> +	if (!wdev->base) {
> +		ret = -ENOMEM;
> +		goto fail;
> +	}
> +
>  	platform_set_drvdata(pdev, wdev);
>  
>  	omap_wdt_disable(wdev);
> @@ -340,11 +345,11 @@ static int __init omap_wdt_probe(struct platform_device *pdev)
>  		goto fail;
>  
>  	pr_info("OMAP Watchdog Timer Rev 0x%02x: initial timeout %d sec\n",
> -		omap_readl(wdev->base + OMAP_WATCHDOG_REV) & 0xFF,
> +		__raw_readl(wdev->base + OMAP_WATCHDOG_REV) & 0xFF,
>  		timer_margin);
>  
>  	/* autogate OCP interface clock */
> -	omap_writel(0x01, wdev->base + OMAP_WATCHDOG_SYS_CONFIG);
> +	__raw_writel(0x01, wdev->base + OMAP_WATCHDOG_SYS_CONFIG);
>  
>  	omap_wdt_dev = pdev;
>  
> @@ -359,6 +364,8 @@ fail:
>  			clk_put(wdev->mpu_wdt_ick);
>  		if (wdev->mpu_wdt_fck)
>  			clk_put(wdev->mpu_wdt_fck);
> +		if (wdev->base)
> +			iounmap(wdev->base);
>  		kfree(wdev);
>  	}
>  	if (mem) {
> @@ -396,6 +403,9 @@ static int omap_wdt_remove(struct platform_device *pdev)
>  		clk_put(wdev->mpu_wdt_fck);
>  		wdev->mpu_wdt_fck = NULL;
>  	}
> +	if (wdev->base)
> +		iounmap(wdev->base);
> +
>  	kfree(wdev);
>  	omap_wdt_dev = NULL;
>  	return 0;
> -- 
> 1.6.0.1.308.gede4c
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

Re: [PATCH] watchdog: another ioremap() fix

[Felipe Balbi]

On Thu, Sep 18, 2008 at 10:03:16AM -0700, David Brownell wrote:
> On Thursday 18 September 2008, Felipe Balbi wrote:
> > convert to use ioremap() and __raw_{read/write} friends.
> 
> Why is this going to the OMAP list, not to LKML and cc'd
> the watchdog maintainer?  (I added Wim to the cc list...)
> 
> If it's worth having, it's worth having in mainline...

My bad, forgot to Cc lkml. I'll resend when Wim gives his ok on the
patch.

-- 
balbi

Re: [PATCH] watchdog: another ioremap() fix

[Wim Van Sebroeck]

Hi Balbi,

> > > convert to use ioremap() and __raw_{read/write} friends.
> > 
> > Why is this going to the OMAP list, not to LKML and cc'd
> > the watchdog maintainer?  (I added Wim to the cc list...)
> > 
> > If it's worth having, it's worth having in mainline...
> 
> My bad, forgot to Cc lkml. I'll resend when Wim gives his ok on the
> patch.

This is signed-off-by me.

Greetings,
Wim.

Re: [PATCH] watchdog: another ioremap() fix

[Russell King - ARM Linux]

On Thu, Sep 18, 2008 at 10:03:16AM -0700, David Brownell wrote:
> On Thursday 18 September 2008, Felipe Balbi wrote:
> > convert to use ioremap() and __raw_{read/write} friends.
> 
> Why is this going to the OMAP list, not to LKML and cc'd
> the watchdog maintainer?  (I added Wim to the cc list...)
> 
> If it's worth having, it's worth having in mainline...

Indeed, this is definitely something that can go into mainline now and
trickle back down into the OMAP tree.

In other words, don't merge this into any OMAP tree, but get it into
mainline and wait for it to come back.  Same goes for any other fixes.
That means less work needs to be done by a very small number of
individuals who don't have enough time to push everything from the OMAP
tree upstream.


However, the omap ioremap changes aren't in mainline yet.  Even so, that
doesn't stop this change going in - the ioremap() changes only make
ioremap() more efficient for those regions that are statically mapped.
The end guarantees as far as the drivers are concerned (== give me a
virtual mapping for the bus address/size I'm requesting) remain true
no matter what.


The only real down side is that there's already a rather large delta
between the Tony's version and the mainline version:

 drivers/watchdog/omap_wdt.c |  314 +++++++++++++++++---------------------------
 1 file changed, 122 insertions(+), 192 deletions(-)

which needs resolving first.  It's no good sending Wim a patch which can
only be applied to Tony's version of the driver.  Again, this is probably
the result of not pushing people hard enough to send their fixes upstream.


Now, I could leave this email at that, but I won't, because it'll be
ineffectual.  Yes yes yes is what everyone's saying.  We want OMAP to
be more merged with mainline, but, please Tony apply this patch so we
can have it working while we wait.

NO.  That's completely the wrong attitude.  As soon as it's in Tony's
tree, everyone goes on their merry way and forgets about it, leaving
it entirely up to Tony to sort out the resulting divergence with
mainline.  That's does not scale, and the fact that OMAP struggles
to merge with mainline proves it.

What I say to Tony: do you want OMAP to be merged into mainline, or
don't you.  If you do, do _not_ accept this patch but get the changes
already in your tree for omap_wdt.c up to Wim, and then get this patch
there.  Provide the carrot by getting the driver up to date in mainline,
and the stick by only accepting patches for this driver once they've
been acked by Wim.  Explicitly ask Wim to ack them for you.

Then you have one less driver to worry about constantly being out of
date with mainline.

Yes, it's probably radical for this community, but it _needs_ to happen.

Re: [PATCH] watchdog: another ioremap() fix

[Felipe Balbi]

Hi,

On Thu, Sep 18, 2008 at 08:50:07PM +0100, Russell King - ARM Linux wrote:
> On Thu, Sep 18, 2008 at 10:03:16AM -0700, David Brownell wrote:
> > On Thursday 18 September 2008, Felipe Balbi wrote:
> > > convert to use ioremap() and __raw_{read/write} friends.
> > 
> > Why is this going to the OMAP list, not to LKML and cc'd
> > the watchdog maintainer?  (I added Wim to the cc list...)
> > 
> > If it's worth having, it's worth having in mainline...
> 
> Indeed, this is definitely something that can go into mainline now and
> trickle back down into the OMAP tree.
> 
> In other words, don't merge this into any OMAP tree, but get it into
> mainline and wait for it to come back.  Same goes for any other fixes.
> That means less work needs to be done by a very small number of
> individuals who don't have enough time to push everything from the OMAP
> tree upstream.

Unfortunately I'll have to disagree here, a diff on omap_wdt.c against
mainline shows quite a few changes since omap3 was introduced. Several
patches never went to mainline. I'll prepare a sync up patch from omap to
mainline and send both, but the omap3 stuff, won't be able to go in.
Maybe i'll try to get rid of all the if (cpu_is_XXX()) stuff in
omap_wdt.c and then send my patch on top of that.

> The only real down side is that there's already a rather large delta
> between the Tony's version and the mainline version:
> 
>  drivers/watchdog/omap_wdt.c |  314 +++++++++++++++++---------------------------
>  1 file changed, 122 insertions(+), 192 deletions(-)

Exactly.

> which needs resolving first.  It's no good sending Wim a patch which can
> only be applied to Tony's version of the driver.  Again, this is probably
> the result of not pushing people hard enough to send their fixes upstream.

Sure it is.

> Now, I could leave this email at that, but I won't, because it'll be
> ineffectual.  Yes yes yes is what everyone's saying.  We want OMAP to
> be more merged with mainline, but, please Tony apply this patch so we
> can have it working while we wait.
> 
> NO.  That's completely the wrong attitude.  As soon as it's in Tony's
> tree, everyone goes on their merry way and forgets about it, leaving
> it entirely up to Tony to sort out the resulting divergence with
> mainline.  That's does not scale, and the fact that OMAP struggles
> to merge with mainline proves it.

totally agree with you here.

> What I say to Tony: do you want OMAP to be merged into mainline, or
> don't you.  If you do, do _not_ accept this patch but get the changes
> already in your tree for omap_wdt.c up to Wim, and then get this patch
> there.  Provide the carrot by getting the driver up to date in mainline,
> and the stick by only accepting patches for this driver once they've
> been acked by Wim.  Explicitly ask Wim to ack them for you.
> 
> Then you have one less driver to worry about constantly being out of
> date with mainline.
> 
> Yes, it's probably radical for this community, but it _needs_ to happen.

Well, what can I say. I'll try to get rid of those cpu conditional code
in the driver and sync omap_wdt.c with mainline. Send all the patches
via Wim, so Tony can get them later.

-- 
balbi

Re: [PATCH] watchdog: another ioremap() fix

[Russell King - ARM Linux]

On Thu, Sep 18, 2008 at 11:02:58PM +0300, Felipe Balbi wrote:
> Hi,
> 
> On Thu, Sep 18, 2008 at 08:50:07PM +0100, Russell King - ARM Linux wrote:
> > On Thu, Sep 18, 2008 at 10:03:16AM -0700, David Brownell wrote:
> > > On Thursday 18 September 2008, Felipe Balbi wrote:
> > > > convert to use ioremap() and __raw_{read/write} friends.
> > > 
> > > Why is this going to the OMAP list, not to LKML and cc'd
> > > the watchdog maintainer?  (I added Wim to the cc list...)
> > > 
> > > If it's worth having, it's worth having in mainline...
> > 
> > Indeed, this is definitely something that can go into mainline now and
> > trickle back down into the OMAP tree.
> > 
> > In other words, don't merge this into any OMAP tree, but get it into
> > mainline and wait for it to come back.  Same goes for any other fixes.
> > That means less work needs to be done by a very small number of
> > individuals who don't have enough time to push everything from the OMAP
> > tree upstream.
> 
> Unfortunately I'll have to disagree here, a diff on omap_wdt.c against
> mainline shows quite a few changes since omap3 was introduced. Several
> patches never went to mainline. I'll prepare a sync up patch from omap to
> mainline and send both, but the omap3 stuff, won't be able to go in.
> Maybe i'll try to get rid of all the if (cpu_is_XXX()) stuff in
> omap_wdt.c and then send my patch on top of that.

Actually, you're wrong on that.  You can take the entire driver and
build it as it currently stands in mainline.  I just did exactly
that by doing the following:

$ diff -u linux-2.6-rmk/drivers/watchdog/omap_wdt.c \
  linux-2.6-omap/drivers/watchdog/omap_wdt.c > wdt.diff
$ cd linux-2.6-rmk
$ patch -p1 -i ../wdt.diff
patching file drivers/watchdog/omap_wdt.c
$ emake O=../build/omap2/ drivers/watchdog/
...
  CC      drivers/watchdog/omap_wdt.o
  LD      drivers/watchdog/built-in.o

So there should be no problems with merging up the OMAP3 changes in
this driver.

Re: [PATCH] watchdog: another ioremap() fix

[Russell King - ARM Linux]

On Thu, Sep 18, 2008 at 11:02:58PM +0300, Felipe Balbi wrote:
> Well, what can I say. I'll try to get rid of those cpu conditional code
> in the driver and sync omap_wdt.c with mainline. Send all the patches
> via Wim, so Tony can get them later.

BTW, I'd also suggest copying Andrew Morton as well - I think Wim doesn't
have very much time to spend doing kernel stuff, so you may not receive
a reply for a while.  If Andrew takes the patches, then you have to
worry less about Wim forgetting about them (or if Andrew times out
pinging Wim, akpm may decide to send them up to Linus himself, or akpm
might re-route them via someone else like me.)

Re: [PATCH] watchdog: another ioremap() fix

[Felipe Balbi]

On Thu, Sep 18, 2008 at 09:34:01PM +0100, Russell King - ARM Linux wrote:
> On Thu, Sep 18, 2008 at 11:02:58PM +0300, Felipe Balbi wrote:
> > Well, what can I say. I'll try to get rid of those cpu conditional code
> > in the driver and sync omap_wdt.c with mainline. Send all the patches
> > via Wim, so Tony can get them later.
> 
> BTW, I'd also suggest copying Andrew Morton as well - I think Wim doesn't
> have very much time to spend doing kernel stuff, so you may not receive
> a reply for a while.  If Andrew takes the patches, then you have to
> worry less about Wim forgetting about them (or if Andrew times out
> pinging Wim, akpm may decide to send them up to Linus himself, or akpm
> might re-route them via someone else like me.)

Thanks for the hint :-)

-- 
balbi

Re: [PATCH] watchdog: another ioremap() fix

[Wim Van Sebroeck]

Hi All,

> On Thu, Sep 18, 2008 at 11:02:58PM +0300, Felipe Balbi wrote:
> > Well, what can I say. I'll try to get rid of those cpu conditional code
> > in the driver and sync omap_wdt.c with mainline. Send all the patches
> > via Wim, so Tony can get them later.
> 
> BTW, I'd also suggest copying Andrew Morton as well - I think Wim doesn't
> have very much time to spend doing kernel stuff, so you may not receive
> a reply for a while.  If Andrew takes the patches, then you have to
> worry less about Wim forgetting about them (or if Andrew times out
> pinging Wim, akpm may decide to send them up to Linus himself, or akpm
> might re-route them via someone else like me.)

Please indeed Cc: Andrew Morton as well, for the reasons that Russell
explained above.

(The not have very much time means that I have been occupied with our
new house. We moved 15th of august and we still have some small details
to fix (like electricity, the basics for the garden so that the kids can
play safely enough, some plumbing work, ...). But the good news is that
we allready moved and that we are getting there.
On the other hand I have now an extra constraint due to the fact that I
was fired at work 3 weeks ago and that I thus have to find a new job...)

Greetings,
Wim.

Re: [PATCH] watchdog: another ioremap() fix

[David Brownell]

On Thursday 18 September 2008, Russell King - ARM Linux wrote:
> What I say to Tony: do you want OMAP to be merged into mainline, or
> don't you.  If you do, do _not_ accept this patch but get the changes
> already in your tree for omap_wdt.c up to Wim, and then get this patch
> there.  Provide the carrot by getting the driver up to date in mainline,
> and the stick by only accepting patches for this driver once they've
> been acked by Wim.  Explicitly ask Wim to ack them for you.
> 
> Then you have one less driver to worry about constantly being out of
> date with mainline.

That sounds like a plan.  One could go further and work
something like the "stable" tree ... not merging into the
OMAP tree until the patch is merged, or at least queued,
for upstream.  Minimal exceptions, if any.  (And not for
this watchdog!)

That should be workable for most drivers; sometimes with
a few arch/arm/plat-omap/include/mach/*.h updates you may
need to ack.  Things like CBUS support (no framework for
that is upstream) are cases where it'd fail.

I'm not exactly in this particular loop except as a gadfly
with relevant insight/background/experience, but I suspect
that a basic goal *MUST* be to avoid having Tony and/or
Russell in the loop unless it's unavoidable.  

- Dave
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] watchdog: another ioremap() fix

[Tony Lindgren]

* David Brownell <david-b@pacbell.net> [080918 13:11]:
> On Thursday 18 September 2008, Russell King - ARM Linux wrote:
> > What I say to Tony: do you want OMAP to be merged into mainline, or
> > don't you.  If you do, do _not_ accept this patch but get the changes
> > already in your tree for omap_wdt.c up to Wim, and then get this patch
> > there.  Provide the carrot by getting the driver up to date in mainline,
> > and the stick by only accepting patches for this driver once they've
> > been acked by Wim.  Explicitly ask Wim to ack them for you.
> > 
> > Then you have one less driver to worry about constantly being out of
> > date with mainline.
> 
> That sounds like a plan.  One could go further and work
> something like the "stable" tree ... not merging into the
> OMAP tree until the patch is merged, or at least queued,
> for upstream.  Minimal exceptions, if any.  (And not for
> this watchdog!)
> 
> That should be workable for most drivers; sometimes with
> a few arch/arm/plat-omap/include/mach/*.h updates you may
> need to ack.  Things like CBUS support (no framework for
> that is upstream) are cases where it'd fail.
> 
> I'm not exactly in this particular loop except as a gadfly
> with relevant insight/background/experience, but I suspect
> that a basic goal *MUST* be to avoid having Tony and/or
> Russell in the loop unless it's unavoidable.  

Been offline few days, glad to see the watchdog updates moving forward :)

Yes I agree we should deal with drivers directly on the appropriate
mailing lists. And then have them fall downstream to l-o tree.

We can use l-o tree for testing and integration, but let's add
a new rule like Russell is suggesting: Let's get at least an ack from
the appropriate maintainer before we apply the patches into l-o tree.

And of course we've already moved quite a bit of the driver development
to the right mailing lists like USB (except ehci-hcd, grr) and audio.

Cheers,

Tony





--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] watchdog: another ioremap() fix

[George G. Davis]

On Thu, Sep 18, 2008 at 04:55:46PM +0300, Felipe Balbi wrote:
> convert to use ioremap() and __raw_{read/write} friends.
> 
> Signed-off-by: Felipe Balbi <felipe.balbi@nokia.com>

Signed-off-by: George G. Davis <gdavis@mvista.com>

> ---
>  drivers/watchdog/omap_wdt.c |   52 +++++++++++++++++++++++++-----------------
>  1 files changed, 31 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
> index b111dec..e395743 100644
> --- a/drivers/watchdog/omap_wdt.c
> +++ b/drivers/watchdog/omap_wdt.c
> @@ -71,12 +71,12 @@ static void omap_wdt_ping(struct omap_wdt_dev *wdev)
>  {
>  	void __iomem    *base = wdev->base;
>  	/* wait for posted write to complete */
> -	while ((omap_readl(base + OMAP_WATCHDOG_WPS)) & 0x08)
> +	while ((__raw_readl(base + OMAP_WATCHDOG_WPS)) & 0x08)
>  		cpu_relax();
>  	wdt_trgr_pattern = ~wdt_trgr_pattern;
> -	omap_writel(wdt_trgr_pattern, (base + OMAP_WATCHDOG_TGR));
> +	__raw_writel(wdt_trgr_pattern, (base + OMAP_WATCHDOG_TGR));
>  	/* wait for posted write to complete */
> -	while ((omap_readl(base + OMAP_WATCHDOG_WPS)) & 0x08)
> +	while ((__raw_readl(base + OMAP_WATCHDOG_WPS)) & 0x08)
>  		cpu_relax();
>  	/* reloaded WCRR from WLDR */
>  }
> @@ -86,11 +86,11 @@ static void omap_wdt_enable(struct omap_wdt_dev *wdev)
>  	void __iomem *base;
>  	base = wdev->base;
>  	/* Sequence to enable the watchdog */
> -	omap_writel(0xBBBB, base + OMAP_WATCHDOG_SPR);
> -	while ((omap_readl(base + OMAP_WATCHDOG_WPS)) & 0x10)
> +	__raw_writel(0xBBBB, base + OMAP_WATCHDOG_SPR);
> +	while ((__raw_readl(base + OMAP_WATCHDOG_WPS)) & 0x10)
>  		cpu_relax();
> -	omap_writel(0x4444, base + OMAP_WATCHDOG_SPR);
> -	while ((omap_readl(base + OMAP_WATCHDOG_WPS)) & 0x10)
> +	__raw_writel(0x4444, base + OMAP_WATCHDOG_SPR);
> +	while ((__raw_readl(base + OMAP_WATCHDOG_WPS)) & 0x10)
>  		cpu_relax();
>  }
>  
> @@ -99,11 +99,11 @@ static void omap_wdt_disable(struct omap_wdt_dev *wdev)
>  	void __iomem *base;
>  	base = wdev->base;
>  	/* sequence required to disable watchdog */
> -	omap_writel(0xAAAA, base + OMAP_WATCHDOG_SPR);	/* TIMER_MODE */
> -	while (omap_readl(base + OMAP_WATCHDOG_WPS) & 0x10)
> +	__raw_writel(0xAAAA, base + OMAP_WATCHDOG_SPR);	/* TIMER_MODE */
> +	while (__raw_readl(base + OMAP_WATCHDOG_WPS) & 0x10)
>  		cpu_relax();
> -	omap_writel(0x5555, base + OMAP_WATCHDOG_SPR);	/* TIMER_MODE */
> -	while (omap_readl(base + OMAP_WATCHDOG_WPS) & 0x10)
> +	__raw_writel(0x5555, base + OMAP_WATCHDOG_SPR);	/* TIMER_MODE */
> +	while (__raw_readl(base + OMAP_WATCHDOG_WPS) & 0x10)
>  		cpu_relax();
>  }
>  
> @@ -123,10 +123,10 @@ static void omap_wdt_set_timeout(struct omap_wdt_dev *wdev)
>  	base = wdev->base;
>  
>  	/* just count up at 32 KHz */
> -	while (omap_readl(base + OMAP_WATCHDOG_WPS) & 0x04)
> +	while (__raw_readl(base + OMAP_WATCHDOG_WPS) & 0x04)
>  		cpu_relax();
> -	omap_writel(pre_margin, base + OMAP_WATCHDOG_LDR);
> -	while (omap_readl(base + OMAP_WATCHDOG_WPS) & 0x04)
> +	__raw_writel(pre_margin, base + OMAP_WATCHDOG_LDR);
> +	while (__raw_readl(base + OMAP_WATCHDOG_WPS) & 0x04)
>  		cpu_relax();
>  }
>  
> @@ -152,10 +152,10 @@ static int omap_wdt_open(struct inode *inode, struct file *file)
>  	}
>  
>  	/* initialize prescaler */
> -	while (omap_readl(base + OMAP_WATCHDOG_WPS) & 0x01)
> +	while (__raw_readl(base + OMAP_WATCHDOG_WPS) & 0x01)
>  		cpu_relax();
> -	omap_writel((1 << 5) | (PTV << 2), base + OMAP_WATCHDOG_CNTRL);
> -	while (omap_readl(base + OMAP_WATCHDOG_WPS) & 0x01)
> +	__raw_writel((1 << 5) | (PTV << 2), base + OMAP_WATCHDOG_CNTRL);
> +	while (__raw_readl(base + OMAP_WATCHDOG_WPS) & 0x01)
>  		cpu_relax();
>  
>  	file->private_data = (void *) wdev;
> @@ -225,7 +225,7 @@ omap_wdt_ioctl(struct inode *inode, struct file *file,
>  		return put_user(0, (int __user *)arg);
>  	case WDIOC_GETBOOTSTATUS:
>  		if (cpu_is_omap16xx())
> -			return put_user(omap_readw(ARM_SYSST),
> +			return put_user(__raw_readw(ARM_SYSST),
>  					(int __user *)arg);
>  		if (cpu_is_omap24xx())
>  			return put_user(omap_prcm_get_reset_sources(),
> @@ -324,7 +324,12 @@ static int __init omap_wdt_probe(struct platform_device *pdev)
>  			goto fail;
>  		}
>  	}
> -	wdev->base = (void __iomem *) (mem->start);
> +	wdev->base = ioremap(res->start, res->end - res->start + 1);
> +	if (!wdev->base) {
> +		ret = -ENOMEM;
> +		goto fail;
> +	}
> +
>  	platform_set_drvdata(pdev, wdev);
>  
>  	omap_wdt_disable(wdev);
> @@ -340,11 +345,11 @@ static int __init omap_wdt_probe(struct platform_device *pdev)
>  		goto fail;
>  
>  	pr_info("OMAP Watchdog Timer Rev 0x%02x: initial timeout %d sec\n",
> -		omap_readl(wdev->base + OMAP_WATCHDOG_REV) & 0xFF,
> +		__raw_readl(wdev->base + OMAP_WATCHDOG_REV) & 0xFF,
>  		timer_margin);
>  
>  	/* autogate OCP interface clock */
> -	omap_writel(0x01, wdev->base + OMAP_WATCHDOG_SYS_CONFIG);
> +	__raw_writel(0x01, wdev->base + OMAP_WATCHDOG_SYS_CONFIG);
>  
>  	omap_wdt_dev = pdev;
>  
> @@ -359,6 +364,8 @@ fail:
>  			clk_put(wdev->mpu_wdt_ick);
>  		if (wdev->mpu_wdt_fck)
>  			clk_put(wdev->mpu_wdt_fck);
> +		if (wdev->base)
> +			iounmap(wdev->base);
>  		kfree(wdev);
>  	}
>  	if (mem) {
> @@ -396,6 +403,9 @@ static int omap_wdt_remove(struct platform_device *pdev)
>  		clk_put(wdev->mpu_wdt_fck);
>  		wdev->mpu_wdt_fck = NULL;
>  	}
> +	if (wdev->base)
> +		iounmap(wdev->base);
> +

arch/arm/plat-omap/devices.c

>  	kfree(wdev);
>  	omap_wdt_dev = NULL;
>  	return 0;
> -- 
> 1.6.0.1.308.gede4c

Re: [PATCH] watchdog: another ioremap() fix

[Felipe Balbi]

On Thu, Sep 18, 2008 at 02:29:19PM -0400, George G. Davis wrote:
> arch/arm/plat-omap/devices.c

What did you want to mean with this comment ??

-- 
balbi

Re: [PATCH] watchdog: another ioremap() fix

[George G. Davis]

On Thu, Sep 18, 2008 at 09:40:23PM +0300, Felipe Balbi wrote:
> On Thu, Sep 18, 2008 at 02:29:19PM -0400, George G. Davis wrote:
> > arch/arm/plat-omap/devices.c
> 
> What did you want to mean with this comment ??

oops, just left over from looking over the patch and taking notes.
Disregard it please.  : )

--
Regards,
George
> 
> -- 
> balbi
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] watchdog: another ioremap() fix

[Felipe Balbi]

On Thu, Sep 18, 2008 at 02:45:31PM -0400, George G. Davis wrote:
> On Thu, Sep 18, 2008 at 09:40:23PM +0300, Felipe Balbi wrote:
> > On Thu, Sep 18, 2008 at 02:29:19PM -0400, George G. Davis wrote:
> > > arch/arm/plat-omap/devices.c
> > 
> > What did you want to mean with this comment ??
> 
> oops, just left over from looking over the patch and taking notes.
> Disregard it please.  : )

ok, thanks :-)

-- 
balbi