[PATCH 0/2] twl4030-madc fixes

[PATCH 0/2] twl4030-madc fixes

[Aaro Koskinen]

Hello,

Two fixes for twl4030-madc:

- The first one converts the ioctl to unlocked one.

- The second one should return the conversion status to user quicker, 
both in OK and timeout cases. There's also fix for the timeout case 
(currently the request remains active) and some cleanup.

A.

[PATCH 1/2] twl4030-madc: Convert ioctl to unlocked

[Aaro Koskinen]

Use unlocked ioctl instead of taking the BKL for an operation that can
take some milliseconds. The driver mutex is now taken before the active
status check, so that the concurrent users of this ioctl won't start
seeing EBUSY (previously guaranteed by the BKL).

Signed-off-by: Aaro Koskinen <Aaro.Koskinen@nokia.com>
---
 drivers/i2c/chips/twl4030-madc.c |   16 +++++++++-------
 1 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/chips/twl4030-madc.c b/drivers/i2c/chips/twl4030-madc.c
index 19dacf5..8997381 100644
--- a/drivers/i2c/chips/twl4030-madc.c
+++ b/drivers/i2c/chips/twl4030-madc.c
@@ -269,17 +269,19 @@ int twl4030_madc_conversion(struct twl4030_madc_request *req)
 	if (unlikely(!req))
 		return -EINVAL;
 
+	mutex_lock(&the_madc->lock);
+
 	/* Do we have a conversion request ongoing */
-	if (the_madc->requests[req->method].active)
-		return -EBUSY;
+	if (the_madc->requests[req->method].active) {
+		ret = -EBUSY;
+		goto out;
+	}
 
 	ch_msb = (req->channels >> 8) & 0xff;
 	ch_lsb = req->channels & 0xff;
 
 	method = &twl4030_conversion_methods[req->method];
 
-	mutex_lock(&the_madc->lock);
-
 	/* Select channels to be converted */
 	twl4030_madc_write(the_madc, method->sel + 1, ch_msb);
 	twl4030_madc_write(the_madc, method->sel, ch_lsb);
@@ -366,8 +368,8 @@ static int twl4030_madc_set_power(struct twl4030_madc_data *madc, int on)
 	return 0;
 }
 
-static int twl4030_madc_ioctl(struct inode *inode, struct file *filp,
-			      unsigned int cmd, unsigned long arg)
+static long twl4030_madc_ioctl(struct file *filp, unsigned int cmd,
+			       unsigned long arg)
 {
 	struct twl4030_madc_user_parms par;
 	int val, ret;
@@ -413,7 +415,7 @@ static int twl4030_madc_ioctl(struct inode *inode, struct file *filp,
 
 static struct file_operations twl4030_madc_fileops = {
 	.owner = THIS_MODULE,
-	.ioctl = twl4030_madc_ioctl
+	.unlocked_ioctl = twl4030_madc_ioctl
 };
 
 static struct miscdevice twl4030_madc_device = {
-- 
1.5.4.3

[PATCH 2/2] twl4030-madc: Let the operation complete faster

[Aaro Koskinen]

No need to wait before the first read, as the operation is likely
completed already. For timeout 50 milliseconds is an overkill, poll the
register for the duration of 5 milliseconds at maximum.

Also, clear the active flag in case of timeout.

Signed-off-by: Aaro Koskinen <Aaro.Koskinen@nokia.com>
---
 drivers/i2c/chips/twl4030-madc.c |   28 +++++++++++++++-------------
 1 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/drivers/i2c/chips/twl4030-madc.c b/drivers/i2c/chips/twl4030-madc.c
index 8997381..d3e0a7f 100644
--- a/drivers/i2c/chips/twl4030-madc.c
+++ b/drivers/i2c/chips/twl4030-madc.c
@@ -246,24 +246,28 @@ static inline void twl4030_madc_start_conversion(struct twl4030_madc_data *madc,
 	}
 }
 
-static void twl4030_madc_wait_conversion_ready_ms(
+static int twl4030_madc_wait_conversion_ready(
 		struct twl4030_madc_data *madc,
-		u8 *time, u8 status_reg)
+		unsigned int timeout_ms, u8 status_reg)
 {
-	u8 reg = 0;
+	unsigned long timeout;
 
+	timeout = jiffies + msecs_to_jiffies(timeout_ms);
 	do {
-		msleep(1);
-		(*time)--;
+		u8 reg;
+
 		reg = twl4030_madc_read(madc, status_reg);
-	} while (((reg & TWL4030_MADC_BUSY) && !(reg & TWL4030_MADC_EOC_SW)) &&
-		  (*time != 0));
+		if (!(reg & TWL4030_MADC_BUSY) && (reg & TWL4030_MADC_EOC_SW))
+			return 0;
+	} while (!time_after(jiffies, timeout));
+
+	return -EAGAIN;
 }
 
 int twl4030_madc_conversion(struct twl4030_madc_request *req)
 {
 	const struct twl4030_madc_conversion_method *method;
-	u8 wait_time, ch_msb, ch_lsb;
+	u8 ch_msb, ch_lsb;
 	int ret;
 
 	if (unlikely(!req))
@@ -310,12 +314,10 @@ int twl4030_madc_conversion(struct twl4030_madc_request *req)
 	the_madc->requests[req->method].active = 1;
 
 	/* Wait until conversion is ready (ctrl register returns EOC) */
-	wait_time = 50;
-	twl4030_madc_wait_conversion_ready_ms(the_madc,
-			&wait_time, method->ctrl);
-	if (wait_time == 0) {
+	ret = twl4030_madc_wait_conversion_ready(the_madc, 5, method->ctrl);
+	if (ret) {
 		dev_dbg(the_madc->dev, "conversion timeout!\n");
-		ret = -EAGAIN;
+		the_madc->requests[req->method].active = 0;
 		goto out;
 	}
 
-- 
1.5.4.3

Re: [PATCH 0/2] twl4030-madc fixes

[David Brownell]

On Friday 20 February 2009, Aaro Koskinen wrote:
> Hello,
> 
> Two fixes for twl4030-madc:
> 
> - The first one converts the ioctl to unlocked one.
> 
> - The second one should return the conversion status to user quicker, 
> both in OK and timeout cases. There's also fix for the timeout case 
> (currently the request remains active) and some cleanup.

They both look OK to me.

Re: [PATCH 0/2] twl4030-madc fixes

[Tony Lindgren]

* David Brownell <david-b@pacbell.net> [090222 13:59]:
> On Friday 20 February 2009, Aaro Koskinen wrote:
> > Hello,
> > 
> > Two fixes for twl4030-madc:
> > 
> > - The first one converts the ioctl to unlocked one.
> > 
> > - The second one should return the conversion status to user quicker, 
> > both in OK and timeout cases. There's also fix for the timeout case 
> > (currently the request remains active) and some cleanup.
> 
> They both look OK to me.

Aaro, please post these to the accociated driver lists to get these
integrated. Also Cc l-o so people can easily track what's happening.

Regards,

Tony

Re: [PATCH 0/2] twl4030-madc fixes

[Felipe Balbi]

On Fri, Feb 27, 2009 at 09:03:31AM -0800, Tony Lindgren wrote:
> * David Brownell <david-b@pacbell.net> [090222 13:59]:
> > On Friday 20 February 2009, Aaro Koskinen wrote:
> > > Hello,
> > > 
> > > Two fixes for twl4030-madc:
> > > 
> > > - The first one converts the ioctl to unlocked one.
> > > 
> > > - The second one should return the conversion status to user quicker, 
> > > both in OK and timeout cases. There's also fix for the timeout case 
> > > (currently the request remains active) and some cleanup.
> > 
> > They both look OK to me.
> 
> Aaro, please post these to the accociated driver lists to get these
> integrated. Also Cc l-o so people can easily track what's happening.

twl4030 madc is still in l-o only. I guess we need to figure out a way
to get that moved to hwmon and push it via the proper mailing list.

Until then, bummer, it's gonna stay here :-(

-- 
balbi

Re: [PATCH 0/2] twl4030-madc fixes

[Tony Lindgren]

* Felipe Balbi <me@felipebalbi.com> [090227 09:46]:
> On Fri, Feb 27, 2009 at 09:03:31AM -0800, Tony Lindgren wrote:
> > * David Brownell <david-b@pacbell.net> [090222 13:59]:
> > > On Friday 20 February 2009, Aaro Koskinen wrote:
> > > > Hello,
> > > > 
> > > > Two fixes for twl4030-madc:
> > > > 
> > > > - The first one converts the ioctl to unlocked one.
> > > > 
> > > > - The second one should return the conversion status to user quicker, 
> > > > both in OK and timeout cases. There's also fix for the timeout case 
> > > > (currently the request remains active) and some cleanup.
> > > 
> > > They both look OK to me.
> > 
> > Aaro, please post these to the accociated driver lists to get these
> > integrated. Also Cc l-o so people can easily track what's happening.
> 
> twl4030 madc is still in l-o only. I guess we need to figure out a way
> to get that moved to hwmon and push it via the proper mailing list.
> 
> Until then, bummer, it's gonna stay here :-(

I think after 2.6.29 gets tagged it's time to reset all the twl code to
the mainline.

So everybody with twl patches, please try to get them integrated to
the mainline this coming merge window!

Tony

Re: [PATCH 0/2] twl4030-madc fixes

[Tony Lindgren]

* Tony Lindgren <tony@atomide.com> [090227 09:56]:
> * Felipe Balbi <me@felipebalbi.com> [090227 09:46]:
> > On Fri, Feb 27, 2009 at 09:03:31AM -0800, Tony Lindgren wrote:
> > > * David Brownell <david-b@pacbell.net> [090222 13:59]:
> > > > On Friday 20 February 2009, Aaro Koskinen wrote:
> > > > > Hello,
> > > > > 
> > > > > Two fixes for twl4030-madc:
> > > > > 
> > > > > - The first one converts the ioctl to unlocked one.
> > > > > 
> > > > > - The second one should return the conversion status to user quicker, 
> > > > > both in OK and timeout cases. There's also fix for the timeout case 
> > > > > (currently the request remains active) and some cleanup.
> > > > 
> > > > They both look OK to me.
> > > 
> > > Aaro, please post these to the accociated driver lists to get these
> > > integrated. Also Cc l-o so people can easily track what's happening.
> > 
> > twl4030 madc is still in l-o only. I guess we need to figure out a way
> > to get that moved to hwmon and push it via the proper mailing list.
> > 
> > Until then, bummer, it's gonna stay here :-(
> 
> I think after 2.6.29 gets tagged it's time to reset all the twl code to
> the mainline.
> 
> So everybody with twl patches, please try to get them integrated to
> the mainline this coming merge window!

I've applied Aaro's fixes to l-o for so it's easier to find them. But
will remove any non-mainline twl code after v2.6.29-omap1 gets tagged.

Regards,

Tony