diff mbox

Re: rtc-pxa: why are irq's only requested in the open call()?

Message ID Prayer.1.3.2.1003032327500.4583@hermes-2.csi.cam.ac.uk
State Not Applicable
Headers show

Commit Message

Jonathan Cameron March 3, 2010, 11:27 p.m. UTC 
On Mar 3 2010, Alessandro Zummo wrote:

>On Wed, 03 Mar 2010 14:02:52 +0000
>Jonathan Cameron <jic23@cam.ac.uk> wrote:
>
>> to ensure that opening inside the kernel has the same effect as
>> opening the file or just call the open op directly in rtc_class_open.
>> The only driver doing anything other than interrupt requesting in here
>> is rtc-m41t80 and I am far from sure what that is doing?
>
> we might amend rtc_class_open to handle this issue.
> patches will be appreciated ;)

Hi Alessandro.

I am a little confused as to why the code that runs on rtc_class_open is
rtc_task *task)
 	if (task == NULL || task->func == NULL)
 		return -EINVAL;
 
-	/* Cannot register while the char dev is in use */
-	if (test_and_set_bit_lock(RTC_DEV_BUSY, &rtc->flags))
-		return -EBUSY;
-
 	spin_lock_irq(&rtc->irq_task_lock);
 	if (rtc->irq_task == NULL) {
                 rtc->irq_task = task; @@ -447,8 +464,6 @@ int 
rtc_irq_register(struct rtc_device *rtc, struct rtc_task *task)
 	}
 	spin_unlock_irq(&rtc->irq_task_lock);
 
-	clear_bit_unlock(RTC_DEV_BUSY, &rtc->flags);
-
 	return retval;
 }
 EXPORT_SYMBOL_GPL(rtc_irq_register);
diff mbox

Patch

different to that that runs on device open.

For in kernel calls, the device is only exclusively taken when adding 
changing the interrupt. Thus it would I think be possible for multiple user 
run this function, whilst they all think they have exclusive control. Do we 
not want to ensure single access to the device whether in kernel or via the 
userspace interface?

To do this I would propose moving 

	if (test_and_set_bit_lock(RTC_DEV_BUSY, &rtc->flags))
		return -EBUSY;
etc into the rtc_class_open and also to add a call to
ops->open here.

Clearly we would also need the corresponding clear_bit_lock and ops->release
calls in rtc_class_close.

Assuming there is no reason that I'm missing behind the inherent 
differences in kernel and out of kernel, the only complexity I know of is 
that a lot of drivers define an ops->release. The following certainly seem 
to work fine with rtc-pxa. Not sure what my webmail will do with the 
patch...

Thanks,

Jonathan


[PATCH] rtc: Make in kernel interface behave similarly to character device.

Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
---
 drivers/rtc/interface.c |   29 ++++++++++++++++++++++-------
 1 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
index a0c8162..553ca4a 100644
--- a/drivers/rtc/interface.c
+++ b/drivers/rtc/interface.c
@@ -406,6 +406,7 @@  struct rtc_device *rtc_class_open(char *name)
 {
 	struct device *dev;
 	struct rtc_device *rtc = NULL;
+	int err;
 
 	dev = class_find_device(rtc_class, NULL, name, __rtc_match);
 	if (dev)
@@ -415,15 +416,35 @@  struct rtc_device *rtc_class_open(char *name)
 		if (!try_module_get(rtc->owner)) {
 			put_device(dev);
 			rtc = NULL;
+			goto exit;
+		}
+		/* Cannot use while the char dev is in use */
+		if (test_and_set_bit_lock(RTC_DEV_BUSY, &rtc->flags)) {
+			put_device(dev);
+			rtc = NULL;
+			goto exit;
+		}
+		err = rtc->ops->open ? rtc->ops->open(rtc->dev.parent) : 0;
+		if (err != 0) {
+			clear_bit_unlock(RTC_DEV_BUSY, &rtc->flags);
+			put_device(dev);
+			rtc = NULL;
+			goto exit;
 		}
+		spin_lock_irq(&rtc->irq_lock);
+		rtc->irq_data = 0;
+		spin_unlock_irq(&rtc->irq_lock);
 	}
-
+exit:
 	return rtc;
 }
 EXPORT_SYMBOL_GPL(rtc_class_open);
 
 void rtc_class_close(struct rtc_device *rtc)
 {
+	if (rtc->ops->release)
+		rtc->ops->release(rtc->dev.parent);
+	clear_bit_unlock(RTC_DEV_BUSY, &rtc->flags);
 	module_put(rtc->owner);
 	put_device(&rtc->dev);
 } @@ -436,10 +457,6 @@ int rtc_irq_register(struct rtc_device *rtc, struct