linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: marek.vasut@gmail.com (Marek Vasut)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] PXA: Fix wakealarm capability for pxa-rtc
Date: Sun, 1 Aug 2010 02:13:24 +0200	[thread overview]
Message-ID: <201008010213.24191.marek.vasut@gmail.com> (raw)
In-Reply-To: <87mxt7z7gl.fsf@free.fr>

Dne So 31. ?ervence 2010 23:07:06 Robert Jarzmik napsal(a):
> Marek Vasut <marek.vasut@gmail.com> writes:
> > Dne So 31. ?ervence 2010 17:38:06 Robert Jarzmik napsal(a):
> >> Marek Vasut <marek.vasut@gmail.com> writes:
> >> > The wakealarm capability was incorrectly set after the RTC was
> >> > registered, which resulted into the sysfs node not being created.
> >> 
> >> On my platform, without this patch:
> >> > cat /sys/devices/platform/pxa-rtc/power/wakeup
> >> > enabled
> >> 
> >> The sysfs node is created. What's the issue you're having ?
> > 
> > It's not created for me without this patch (and if you read the code, it
> > is impossible to be created without this patch).
> 
> I read the code, and I see:
> rtc_device_register()
>   -> device_register()
>     -> device_initialize()
>       -> device_init_wakeup(dev, 0)
> 
> So with or without your patch, device_init_wakeup() is called. The
> difference is that with your patch, we end up with a disabled wakeup
> capability. I showed the code sequence, your turn to show your "if you
> read the code".

drivers/rtc/rtc-sysfs.c

217 static inline int rtc_does_wakealarm(struct rtc_device *rtc)
218 {
219         if (!device_can_wakeup(rtc->dev.parent))
220                 return 0;
221         return rtc->ops->set_alarm != NULL;
222 }
223 
224 
225 void rtc_sysfs_add_device(struct rtc_device *rtc)
226 {
227         int err;
228 
229         /* not all RTCs support both alarms and wakeup */
230         if (!rtc_does_wakealarm(rtc))
231                 return;
232 
233         err = device_create_file(&rtc->dev, &dev_attr_wakealarm);
234         if (err)
235                 dev_err(rtc->dev.parent,
236                         "failed to create alarm attribute, %d\n", err);
237 }

For me, it leaves the function on rtc_does_wakealarm(); That's because 
device_can_wakeup on line 219 returns 0;

include/linux/pm_wakeup.h
45 static inline bool device_can_wakeup(struct device *dev)
46 {
47         return dev->power.can_wakeup;
48 }

This dev->power.can_wakeup is set by:

include/linux/pm_wakeup.h
35 static inline void device_init_wakeup(struct device *dev, bool val)
36 {
37         dev->power.can_wakeup = dev->power.should_wakeup = val;
38 }

But, the rtc_sysfs_add_device is called by:

drivers/rtc/class.c
114 struct rtc_device *rtc_device_register(const char *name, struct device *dev,
115                                         const struct rtc_class_ops *ops,
116                                         struct module *owner)
117 {

So the dev->power.can_wakeup must be set before registering the device.
> 
> Moreover, my platform without your patch prooves that the sysfs entry is
> created.

My platform (sharp akita) without this patch doesn't even show the wakealarm 
sysfs node. So something is certainly wrong in this driver, explain what it is 
please.
> 
> So unless you prove me wrong, I will reject the patch.

Ok, then I'd like to see you come up with better patch that actually fixes my 
platform.
> 
> Cheers.

Cheers

  reply	other threads:[~2010-08-01  0:13 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-07-31  4:22 [PATCH] PXA: Fix wakealarm capability for pxa-rtc Marek Vasut
2010-07-31 15:38 ` Robert Jarzmik
2010-07-31 20:15   ` Marek Vasut
2010-07-31 21:07     ` Robert Jarzmik
2010-08-01  0:13       ` Marek Vasut [this message]
2010-08-01  0:48         ` Robert Jarzmik
2010-08-01  0:53           ` Marek Vasut
2010-08-01 11:15             ` Robert Jarzmik
2010-08-01 15:38               ` Marek Vasut
2010-08-01 19:32                 ` Robert Jarzmik
2010-08-03  0:03                 ` Alessandro Zummo
2010-08-03  3:10                   ` Marek Vasut
2010-08-06 19:59                     ` Alessandro Zummo
2010-08-06 21:39                       ` Marek Vasut
2010-08-07  9:43                       ` Robert Jarzmik
2010-08-18  4:08                         ` Eric Miao
2010-08-18  5:04                           ` Marek Vasut
2010-08-18  6:15                             ` Eric Miao
2010-08-01  8:42 ` Igor Grinberg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201008010213.24191.marek.vasut@gmail.com \
    --to=marek.vasut@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).