[PATCH] PXA: Fix wakealarm capability for pxa-rtc

[Robert Jarzmik]

Marek Vasut <marek.vasut at gmail.com> writes:

> Dne So 31. července 2010 23:07:06 Robert Jarzmik napsal(a):
>> Marek Vasut <marek.vasut at gmail.com> writes:
>> > Dne So 31. července 2010 17:38:06 Robert Jarzmik napsal(a):
>> >> Marek Vasut <marek.vasut at 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).

Are we talking about the same /sys/devices/platform/pxa-rtc/power/wakeup ?
Aren't you looking for /sys/class/rtc/rtc0/power/wakeup ?

Because the code you're showing seems to be related to the class sysfs entries,
not the driver's unless I'm mistaken.

I'll join Alessandro to the discussion, he might have some insight for [1].

Cheers.

--
Robert


>> 
>> 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

-- 
Robert