[PATCH] rtc: cmos: Do not export alarm rtc_ops when we do not support alarms

[PATCH] rtc: cmos: Do not export alarm rtc_ops when we do not support alarms

[Hans de Goede]

When there is no IRQ configured for the RTC, the rtc-cmos code does not
support alarms, all alarm rtc_ops fail with -EIO / -EINVAL.

The rtc-core expects a rtc driver which does not support rtc alarms to
not have alarm ops at all. Otherwise the wakealarm sysfs attr will read
as empty rather then returning an error, making it impossible for
userspace to find out beforehand if alarms are supported.

A system without an IRQ for the RTC before this patch:
[root@localhost ~]# cat /sys/class/rtc/rtc0/wakealarm
[root@localhost ~]#

After this patch:
[root@localhost ~]# cat /sys/class/rtc/rtc0/wakealarm
cat: /sys/class/rtc/rtc0/wakealarm: No such file or directory
[root@localhost ~]#

This fixes gnome-session + systemd trying to use suspend-then-hibernate,
which causes systemd to abort the suspend when writing the RTC alarm fails.

BugLink: https://github.com/systemd/systemd/issues/9988
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/rtc/rtc-cmos.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c
index cd3a2411bc2f..490cdbca4430 100644
--- a/drivers/rtc/rtc-cmos.c
+++ b/drivers/rtc/rtc-cmos.c
@@ -244,6 +244,7 @@ static int cmos_read_alarm(struct device *dev, struct rtc_wkalrm *t)
 	struct cmos_rtc	*cmos = dev_get_drvdata(dev);
 	unsigned char	rtc_control;
 
+	/* This not only a rtc_op, but also called directly */
 	if (!is_valid_irq(cmos->irq))
 		return -EIO;
 
@@ -439,6 +440,7 @@ static int cmos_set_alarm(struct device *dev, struct rtc_wkalrm *t)
 	unsigned char mon, mday, hrs, min, sec, rtc_control;
 	int ret;
 
+	/* This not only a rtc_op, but also called directly */
 	if (!is_valid_irq(cmos->irq))
 		return -EIO;
 
@@ -503,9 +505,6 @@ static int cmos_alarm_irq_enable(struct device *dev, unsigned int enabled)
 	struct cmos_rtc	*cmos = dev_get_drvdata(dev);
 	unsigned long	flags;
 
-	if (!is_valid_irq(cmos->irq))
-		return -EINVAL;
-
 	spin_lock_irqsave(&rtc_lock, flags);
 
 	if (enabled)
@@ -566,6 +565,12 @@ static const struct rtc_class_ops cmos_rtc_ops = {
 	.alarm_irq_enable	= cmos_alarm_irq_enable,
 };
 
+static const struct rtc_class_ops cmos_rtc_ops_no_alarm = {
+	.read_time		= cmos_read_time,
+	.set_time		= cmos_set_time,
+	.proc			= cmos_procfs,
+};
+
 /*----------------------------------------------------------------*/
 
 /*
@@ -842,9 +847,12 @@ cmos_do_probe(struct device *dev, struct resource *ports, int rtc_irq)
 			dev_dbg(dev, "IRQ %d is already in use\n", rtc_irq);
 			goto cleanup1;
 		}
+
+		cmos_rtc.rtc->ops = &cmos_rtc_ops;
+	} else {
+		cmos_rtc.rtc->ops = &cmos_rtc_ops_no_alarm;
 	}
 
-	cmos_rtc.rtc->ops = &cmos_rtc_ops;
 	cmos_rtc.rtc->nvram_old_abi = true;
 	retval = rtc_register_device(cmos_rtc.rtc);
 	if (retval)
-- 
2.19.0.rc0

Re: [PATCH] rtc: cmos: Do not export alarm rtc_ops when we do not support alarms

[Alexandre Belloni]

On 04/09/2018 16:51:29+0200, Hans de Goede wrote:
> When there is no IRQ configured for the RTC, the rtc-cmos code does not
> support alarms, all alarm rtc_ops fail with -EIO / -EINVAL.
> 
> The rtc-core expects a rtc driver which does not support rtc alarms to
> not have alarm ops at all. Otherwise the wakealarm sysfs attr will read
> as empty rather then returning an error, making it impossible for
> userspace to find out beforehand if alarms are supported.
> 
> A system without an IRQ for the RTC before this patch:
> [root@localhost ~]# cat /sys/class/rtc/rtc0/wakealarm
> [root@localhost ~]#
> 
> After this patch:
> [root@localhost ~]# cat /sys/class/rtc/rtc0/wakealarm
> cat: /sys/class/rtc/rtc0/wakealarm: No such file or directory
> [root@localhost ~]#
> 
> This fixes gnome-session + systemd trying to use suspend-then-hibernate,
> which causes systemd to abort the suspend when writing the RTC alarm fails.
> 
> BugLink: https://github.com/systemd/systemd/issues/9988
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/rtc/rtc-cmos.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
Applied, thanks.

However, you must be using an ancient kernel as my email address changed
a while ago ;)

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH] rtc: cmos: Do not export alarm rtc_ops when we do not support alarms

[Hans de Goede]

Hi,

On 08-09-18 16:37, Alexandre Belloni wrote:
> On 04/09/2018 16:51:29+0200, Hans de Goede wrote:
>> When there is no IRQ configured for the RTC, the rtc-cmos code does not
>> support alarms, all alarm rtc_ops fail with -EIO / -EINVAL.
>>
>> The rtc-core expects a rtc driver which does not support rtc alarms to
>> not have alarm ops at all. Otherwise the wakealarm sysfs attr will read
>> as empty rather then returning an error, making it impossible for
>> userspace to find out beforehand if alarms are supported.
>>
>> A system without an IRQ for the RTC before this patch:
>> [root@localhost ~]# cat /sys/class/rtc/rtc0/wakealarm
>> [root@localhost ~]#
>>
>> After this patch:
>> [root@localhost ~]# cat /sys/class/rtc/rtc0/wakealarm
>> cat: /sys/class/rtc/rtc0/wakealarm: No such file or directory
>> [root@localhost ~]#
>>
>> This fixes gnome-session + systemd trying to use suspend-then-hibernate,
>> which causes systemd to abort the suspend when writing the RTC alarm fails.
>>
>> BugLink: https://github.com/systemd/systemd/issues/9988
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   drivers/rtc/rtc-cmos.c | 16 ++++++++++++----
>>   1 file changed, 12 insertions(+), 4 deletions(-)
>>
> Applied, thanks.

I'm not seeing this patch in 4.20-rc1 and it is not in

https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git/log/?h=rtc-next

So it seems that this one has fallen through the cracks?

Regards,

Hans

Re: [PATCH] rtc: cmos: Do not export alarm rtc_ops when we do not support alarms

[Alexandre Belloni]

On 07/11/2018 13:49:06+0100, Hans de Goede wrote:
> Hi,
> 
> On 08-09-18 16:37, Alexandre Belloni wrote:
> > On 04/09/2018 16:51:29+0200, Hans de Goede wrote:
> > > When there is no IRQ configured for the RTC, the rtc-cmos code does not
> > > support alarms, all alarm rtc_ops fail with -EIO / -EINVAL.
> > > 
> > > The rtc-core expects a rtc driver which does not support rtc alarms to
> > > not have alarm ops at all. Otherwise the wakealarm sysfs attr will read
> > > as empty rather then returning an error, making it impossible for
> > > userspace to find out beforehand if alarms are supported.
> > > 
> > > A system without an IRQ for the RTC before this patch:
> > > [root@localhost ~]# cat /sys/class/rtc/rtc0/wakealarm
> > > [root@localhost ~]#
> > > 
> > > After this patch:
> > > [root@localhost ~]# cat /sys/class/rtc/rtc0/wakealarm
> > > cat: /sys/class/rtc/rtc0/wakealarm: No such file or directory
> > > [root@localhost ~]#
> > > 
> > > This fixes gnome-session + systemd trying to use suspend-then-hibernate,
> > > which causes systemd to abort the suspend when writing the RTC alarm fails.
> > > 
> > > BugLink: https://github.com/systemd/systemd/issues/9988
> > > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > > ---
> > >   drivers/rtc/rtc-cmos.c | 16 ++++++++++++----
> > >   1 file changed, 12 insertions(+), 4 deletions(-)
> > > 
> > Applied, thanks.
> 
> I'm not seeing this patch in 4.20-rc1 and it is not in
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git/log/?h=rtc-next
> 
> So it seems that this one has fallen through the cracks?
> 

Indeed, I don't what happened, I'll queue it as a fix for 4.20. I
checked and this is the only one that was missing.

Thanks for the warning.

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com