2018-10-02 01:08:59

by Maciej W. Rozycki

[permalink] [raw]
Subject: [PATCH 0/2] rtc: cmos: Fix non-ACPI (non-x86) platform support

Hi,

This mini patch series fixes issues introduced recently for non-ACPI
platforms using `rtc-cmos'. Please see individual change descriptions for
details.

These have been run-time verified with the DECstation (arch/mips/dec).
Please apply.

Maciej


2018-10-02 01:09:09

by Maciej W. Rozycki

[permalink] [raw]
Subject: [PATCH 1/2] rtc: cmos: Fix non-ACPI undefined reference to `hpet_rtc_interrupt'

Fix a commit 311ee9c151ad ("rtc: cmos: allow using ACPI for RTC alarm
instead of HPET") `rtc-cmos' regression causing a link error:

drivers/rtc/rtc-cmos.o: In function `cmos_platform_probe':
rtc-cmos.c:(.init.text+0x33c): undefined reference to `hpet_rtc_interrupt'
rtc-cmos.c:(.init.text+0x3f4): undefined reference to `hpet_rtc_interrupt'

with non-ACPI platforms using this driver. The cause is the change of
the condition guarding the use of `hpet_rtc_interrupt'.

Previously it was a call to `is_hpet_enabled'. That function is static
inline and has a hardcoded 0 result for non-ACPI platforms, which imply
!HPET_EMULATE_RTC. Consequently the compiler optimized the whole block
away including the reference to `hpet_rtc_interrupt', which never made
it to the link stage.

Now the guarding condition is a call to `use_hpet_alarm', which is not
static inline and therefore the compiler may not be able to prove that
it actually always returns 0 for non-ACPI platforms. Consequently the
build breaks with an unsatisfied reference, because `hpet_rtc_interrupt'
is nowhere defined at link time.

Fix the problem by marking `use_hpet_alarm' inline. As the `inline'
keyword serves as an optimization hint rather than a requirement the
compiler is still free to choose whether inlining will be beneficial or
not for ACPI platforms.

Signed-off-by: Maciej W. Rozycki <[email protected]>
Fixes: 311ee9c151ad ("rtc: cmos: allow using ACPI for RTC alarm instead of HPET")
Cc: [email protected] # 4.18+
---
drivers/rtc/rtc-cmos.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

linux-rtc-cmos-use-hpet-alarm-inline.diff
Index: linux-20180812-4maxp64/drivers/rtc/rtc-cmos.c
===================================================================
--- linux-20180812-4maxp64.orig/drivers/rtc/rtc-cmos.c
+++ linux-20180812-4maxp64/drivers/rtc/rtc-cmos.c
@@ -167,7 +167,7 @@ static inline int hpet_unregister_irq_ha
#endif

/* Don't use HPET for RTC Alarm event if ACPI Fixed event is used */
-static int use_hpet_alarm(void)
+static inline int use_hpet_alarm(void)
{
return is_hpet_enabled() && !use_acpi_alarm;
}

2018-10-02 01:10:48

by Maciej W. Rozycki

[permalink] [raw]
Subject: [PATCH 2/2] rtc: cmos: Remove the `use_acpi_alarm' module parameter for !ACPI

Fix a problem with commit 311ee9c151ad ("rtc: cmos: allow using ACPI for
RTC alarm instead of HPET") defining `use_acpi_alarm' module parameter
even for non-ACPI platforms, which ignore it. Wrap the definition into
#ifdef CONFIG_ACPI and use a static inline wrapper function, hardcoded
to return 0 and consequently optimized away for !ACPI, following the
existing pattern with HPET handling functions.

Signed-off-by: Maciej W. Rozycki <[email protected]>
Fixes: 311ee9c151ad ("rtc: cmos: allow using ACPI for RTC alarm instead of HPET")
Cc: [email protected] # 4.18+
---
drivers/rtc/rtc-cmos.c | 27 ++++++++++++++++++++-------
1 file changed, 20 insertions(+), 7 deletions(-)

linux-rtc-cmos-use-acpi-alarm.diff
Index: linux-20180812-4maxp64/drivers/rtc/rtc-cmos.c
===================================================================
--- linux-20180812-4maxp64.orig/drivers/rtc/rtc-cmos.c
+++ linux-20180812-4maxp64/drivers/rtc/rtc-cmos.c
@@ -50,6 +50,7 @@
/* this is for "generic access to PC-style RTC" using CMOS_READ/CMOS_WRITE */
#include <linux/mc146818rtc.h>

+#ifdef CONFIG_ACPI
/*
* Use ACPI SCI to replace HPET interrupt for RTC Alarm event
*
@@ -61,6 +62,18 @@
static bool use_acpi_alarm;
module_param(use_acpi_alarm, bool, 0444);

+static inline int cmos_use_acpi_alarm(void)
+{
+ return use_acpi_alarm;
+}
+#else /* !CONFIG_ACPI */
+
+static inline int cmos_use_acpi_alarm(void)
+{
+ return 0;
+}
+#endif
+
struct cmos_rtc {
struct rtc_device *rtc;
struct device *dev;
@@ -169,7 +182,7 @@ static inline int hpet_unregister_irq_ha
/* Don't use HPET for RTC Alarm event if ACPI Fixed event is used */
static inline int use_hpet_alarm(void)
{
- return is_hpet_enabled() && !use_acpi_alarm;
+ return is_hpet_enabled() && !cmos_use_acpi_alarm();
}

/*----------------------------------------------------------------*/
@@ -340,7 +353,7 @@ static void cmos_irq_enable(struct cmos_
if (use_hpet_alarm())
hpet_set_rtc_irq_bit(mask);

- if ((mask & RTC_AIE) && use_acpi_alarm) {
+ if ((mask & RTC_AIE) && cmos_use_acpi_alarm()) {
if (cmos->wake_on)
cmos->wake_on(cmos->dev);
}
@@ -358,7 +371,7 @@ static void cmos_irq_disable(struct cmos
if (use_hpet_alarm())
hpet_mask_rtc_irq_bit(mask);

- if ((mask & RTC_AIE) && use_acpi_alarm) {
+ if ((mask & RTC_AIE) && cmos_use_acpi_alarm()) {
if (cmos->wake_off)
cmos->wake_off(cmos->dev);
}
@@ -980,7 +993,7 @@ static int cmos_suspend(struct device *d
}
spin_unlock_irq(&rtc_lock);

- if ((tmp & RTC_AIE) && !use_acpi_alarm) {
+ if ((tmp & RTC_AIE) && !cmos_use_acpi_alarm()) {
cmos->enabled_wake = 1;
if (cmos->wake_on)
cmos->wake_on(dev);
@@ -1031,7 +1044,7 @@ static void cmos_check_wkalrm(struct dev
* ACPI RTC wake event is cleared after resume from STR,
* ACK the rtc irq here
*/
- if (t_now >= cmos->alarm_expires && use_acpi_alarm) {
+ if (t_now >= cmos->alarm_expires && cmos_use_acpi_alarm()) {
cmos_interrupt(0, (void *)cmos->rtc);
return;
}
@@ -1053,7 +1066,7 @@ static int __maybe_unused cmos_resume(st
struct cmos_rtc *cmos = dev_get_drvdata(dev);
unsigned char tmp;

- if (cmos->enabled_wake && !use_acpi_alarm) {
+ if (cmos->enabled_wake && !cmos_use_acpi_alarm()) {
if (cmos->wake_off)
cmos->wake_off(dev);
else
@@ -1132,7 +1145,7 @@ static u32 rtc_handler(void *context)
* Or else, ACPI SCI is enabled during suspend/resume only,
* update rtc irq in that case.
*/
- if (use_acpi_alarm)
+ if (cmos_use_acpi_alarm())
cmos_interrupt(0, (void *)cmos->rtc);
else {
/* Fix me: can we use cmos_interrupt() here as well? */

2018-10-04 10:30:48

by Alexandre Belloni

[permalink] [raw]
Subject: Re: [PATCH 0/2] rtc: cmos: Fix non-ACPI (non-x86) platform support

On 02/10/2018 02:08:37+0100, Maciej W. Rozycki wrote:
> Hi,
>
> This mini patch series fixes issues introduced recently for non-ACPI
> platforms using `rtc-cmos'. Please see individual change descriptions for
> details.
>
> These have been run-time verified with the DECstation (arch/mips/dec).
> Please apply.
>

Both applied, thanks!

> Maciej

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