Re: [PATCH v6 1/1] input: add driver for Bosch Sensortec's BMA150 accelerometer

On 22:15 Sun 07 Aug     , Dmitry Torokhov wrote:

On Sun, Aug 07, 2011 at 06:23:25PM +0200, Eric Andersson wrote:

quoted

quoted

On Sun, Jul 31, 2011 at 12:49:17AM +0200, Dmitry Torokhov wrote:

quoted

quoted

quoted

+static int bma150_open(struct bma150_data *bma150)
+{
+ int ret;
+
+ ret = pm_runtime_set_active(&bma150->client->dev);
+ if (ret < 0)
+         return ret;
+
+ pm_runtime_enable(&bma150->client->dev);

I am pretty sure you want to call pm_runtime_enable() in bma150_probe()
so that parent controller can be suspended until somebody calls
bma150_open() and we mark the device as active (which, in turn, should
wake up its parent).

If I call pm_runtime_enable() I cannot use pm_runtime_set_active() later
according to the comment in __pm_runtime_set_status (runtime.c):
"If runtime PM of the device is disabled or its power.runtime_error
field is different from zero, the status may be changed either to
RPM_ACTIVE, or to RPM_SUSPENDED..."

If the PM of the device is enabled it will return -EAGAIN. Of course, we
could enable() in probe, then disable(); set_active(); enable(); in
open, but that seems a bit confused, right?

Hmm, indeed. I do not like the idea about disable/set_active/enable so I
guess we'll have to keep track of the current mode themselves and call
bma150_set_mode() ourselves if, after calling pm_runtime_get/put_sync()
we find that our mode is different from what we expect it to be.

I also noticed that you did not properly free IRQ on device removal and
also polled devices need to be freed always (unlike regular input
devices that should be only unregistered). The default_cfg can't be
__initdata because it is used by __devinit functions. And my version of
GCC can't figure out that ipoll_dev never used uninitialized and gives
false warning.

I tried correcting these issues in the patch below, along with renaming
'ret' to 'error' which I prefer when we dealing with error handling.
Could you please git it a try and if everything still works I'll fold it
and commit.

Thanks Dmitry! I tried your patch and it worked like a charm!

Excellent, thanks for testing.

quoted

Do you take it from here or would you like me to send an updated
version that includes your patch?

No, there is no need for that. I'll fold them all together and queue for
2.6.32.

Great thanks! Don't forget to add Alan as reviewer. I think he forgot
reply-all when he mailed me:
"I'm happy with this one.
Reviewed-by: Alan Cox <redacted>"

I take the other patch that makes polled devices do poll upon opening
working well for you too.. Should I queue for .2 as well?

Yes, why not!

-- 
Best regards,
 Eric

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