Skip to contentSkip to conversation

x86/platform/intel-mid: Fix RTC handling

PATCH Superseded Andy Shevchenko 2 participants 6 messages Jan 18, 2017 – Jan 19, 2017

Subsystems (2)
x86 Intel MID (Mobile Internet Device) platform

v3 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Andy Shevchenko

author
#0 lore raw

v3 cover x86/platform/intel-mid: Fix RTC handling

Current RTC approach breaks some platforms. Rewrite it completely and make avaiable only for intended platforms.

Tested on Intel Merrifield, ASuS T100TA (normal and acpi=off cases).

Since v2:
- add mp_map_gsi_to_irq() fix to the series as patch 1
- add patch 3
- rewrite RTC approach as another platform code for Intel MID

Andy Shevchenko (3):
x86/ioapic: Return suitable error code in mp_map_gsi_to_irq() x86/platform/intel-mid: Allocate RTC interrupt for Merrifield x86/platform/intel-mid: Don't shadow error code of mp_map_gsi_to_irq()

 arch/x86/kernel/apic/io_apic.c                     |  4 +-
 arch/x86/platform/intel-mid/device_libs/Makefile   |  1 +
 .../intel-mid/device_libs/platform_mrfld_rtc.c     | 48 ++++++++++++++++++++++
 .../intel-mid/device_libs/platform_mrfld_wdt.c     | 10 ++---
 arch/x86/platform/intel-mid/sfi.c                  | 14 -------
 5 files changed, 56 insertions(+), 21 deletions(-)
 create mode 100644 arch/x86/platform/intel-mid/device_libs/platform_mrfld_rtc.c
Signature · 1 line
2.11.0
author
#1 lore raw

PATCH v3 1/3 x86/ioapic: Return suitable error code in mp_map_gsi_to_irq()

mp_map_gsi_to_irq() in some cases might return legacy -1, which would be wrongly interpreted as -EPERM.

Correct those cases to return proper error code.

Signed-off-by Andy Shevchenko <[email protected]>
---
 arch/x86/kernel/apic/io_apic.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/x86/kernel/apic/io_apic.c +2 -2
index 945e512a112a..f62c38d325da 100644
@@ -1107,12 +1107,12 @@ int mp_map_gsi_to_irq(u32 gsi, unsigned int flags, struct irq_alloc_info *info)
 
 	ioapic = mp_find_ioapic(gsi);
 	if (ioapic < 0)
-		return -1;
+		return -ENODEV;
 
 	pin = mp_find_ioapic_pin(ioapic, gsi);
 	idx = find_irq_entry(ioapic, pin, mp_INT);
 	if ((flags & IOAPIC_MAP_CHECK) && idx < 0)
-		return -1;
+		return -ENODEV;
 
 	return mp_map_pin_to_irq(gsi, idx, ioapic, pin, flags, info);
 }
Signature · 1 line
2.11.0
author
#2 lore raw

PATCH v3 2/3 x86/platform/intel-mid: Allocate RTC interrupt for Merrifield

In reply to Andy Shevchenko

Legacy RTC requires interrupt line 8 to be dedicated for it. On Intel MID platforms the legacy PIC is absent and in order to make RTC work we need to allocate interrupt separately.

Current solution brought by the commit:

de1c2540aa4f ("x86/platform/intel-mid: Enable RTC on Intel Merrifield")

does it in a wrong place, and since it's done unconditionally for all x86 devices, some of them, e.g. PNP based, might get it wrong -- at the beginning x86_platform.legacy.rtc flag is set for all x86 devices.

Move intel_mid_legacy_rtc_init() to its own module and call it before x86 RTC CMOS initialization.

Cc Luis R. Rodriguez <[email protected]>
Signed-off-by Andy Shevchenko <[email protected]>
---
 arch/x86/platform/intel-mid/device_libs/Makefile   |  1 +
 .../intel-mid/device_libs/platform_mrfld_rtc.c     | 48 ++++++++++++++++++++++
 arch/x86/platform/intel-mid/sfi.c                  | 14 -------
 3 files changed, 49 insertions(+), 14 deletions(-)
 create mode 100644 arch/x86/platform/intel-mid/device_libs/platform_mrfld_rtc.c
arch/x86/platform/intel-mid/device_libs/Makefile +1 -0
index 8be58eefb281..5901036311c4 100644
@@ -27,6 +27,7 @@ obj-$(subst m,y,$(CONFIG_GPIO_PCA953X)) += platform_pcal9555a.o
 obj-$(subst m,y,$(CONFIG_GPIO_PCA953X)) += platform_tca6416.o
 # MISC Devices
 obj-$(subst m,y,$(CONFIG_KEYBOARD_GPIO)) += platform_gpio_keys.o
+obj-$(subst m,y,$(CONFIG_RTC_DRV_CMOS)) += platform_mrfld_rtc.o
 obj-$(subst m,y,$(CONFIG_INTEL_MID_WATCHDOG)) += platform_mrfld_wdt.o
 # PWM
 obj-$(subst m,y,$(CONFIG_PINCTRL_MERRIFIELD)) += platform_mrfld_pwm.o
arch/x86/platform/intel-mid/device_libs/platform_mrfld_rtc.c new file +48 -0

48 lines not on this page · raw message

arch/x86/platform/intel-mid/sfi.c +0 -14
index e4d4cabbb370..19b43e3a9f0f 100644
@@ -41,7 +41,6 @@
 #include <asm/intel_scu_ipc.h>
 #include <asm/apb_timer.h>
 #include <asm/reboot.h>
-#include <asm/time.h>
 
 #define	SFI_SIG_OEM0	"OEM0"
 #define MAX_IPCDEVS	24

@@ -540,21 +539,8 @@ static int __init sfi_parse_devs(struct sfi_table_header *table)
 	return 0;
 }
 
-static int __init intel_mid_legacy_rtc_init(void)
-{
-	struct irq_alloc_info info;
-
-	if (!x86_platform.legacy.rtc)
-		return -ENODEV;
-
-	ioapic_set_alloc_attr(&info, NUMA_NO_NODE, 1, 0);
-	return mp_map_gsi_to_irq(RTC_IRQ, IOAPIC_MAP_ALLOC, &info);
-}
-
 static int __init intel_mid_platform_init(void)
 {
-	intel_mid_legacy_rtc_init();
-
 	sfi_table_parse(SFI_SIG_GPIO, NULL, NULL, sfi_parse_gpio);
 	sfi_table_parse(SFI_SIG_DEVS, NULL, NULL, sfi_parse_devs);
 	return 0;
Signature · 1 line
2.11.0
#3 lore raw

Re: [PATCH v3 2/3] x86/platform/intel-mid: Allocate RTC interrupt for Merrifield

Quoted 25 lines · Andy Shevchenko
arch/x86/platform/intel-mid/device_libs/platform_mrfld_rtc.c new file +25 -0
+#include <linux/init.h>
+
+#include <asm/hw_irq.h>
+#include <asm/intel-mid.h>
+#include <asm/io_apic.h>
+#include <asm/time.h>
+#include <asm/x86_init.h>
+
+static int __init mrfld_legacy_rtc_alloc_irq(void)
+{
+	struct irq_alloc_info info;
+	int ret;
+
+	if (!x86_platform.legacy.rtc)
+		return -ENODEV;
+
+	ioapic_set_alloc_attr(&info, NUMA_NO_NODE, 1, 0);
+	ret = mp_map_gsi_to_irq(RTC_IRQ, IOAPIC_MAP_ALLOC, &info);
+	if (ret < 0) {
+		pr_info("Failed to allocate RTC interrupt. Disabling RTC\n");
+		x86_platform.legacy.rtc = 0;
+		return ret;
+	}
+
+	return 0;

arch/x86/platform/intel-mid/device_libs/platform_mrfld_rtc.c line 48

Quoted 10 lines
arch/x86/platform/intel-mid/device_libs/platform_mrfld_rtc.c new file +10 -0
+}
+
+static int __init mrfld_legacy_rtc_init(void)
+{
+	if (intel_mid_identify_cpu() != INTEL_MID_CPU_CHIP_TANGIER)
+		return -ENODEV;
+
+	return mrfld_legacy_rtc_alloc_irq();
+}
+rootfs_initcall(mrfld_legacy_rtc_init);

rootfs_initcall???? That does not make any sense at all. I know you need it before the device initcalls, but just using a random initcall level before device_initcall is wrong.

arch_initcall is much more suitable.

Thanks,

tglx

author
#4 lore raw

Re: [PATCH v3 2/3] x86/platform/intel-mid: Allocate RTC interrupt for Merrifield

Quoted 35 lines · Thomas Gleixner
Quoted 34 lines · Andy Shevchenko
arch/x86/platform/intel-mid/device_libs/platform_mrfld_rtc.c new file +20 -0
+#include <linux/init.h>
+
+#include <asm/hw_irq.h>
+#include <asm/intel-mid.h>
+#include <asm/io_apic.h>
+#include <asm/time.h>
+#include <asm/x86_init.h>
+
+static int __init mrfld_legacy_rtc_alloc_irq(void)
+{
+	struct irq_alloc_info info;
+	int ret;
+
+	if (!x86_platform.legacy.rtc)
+		return -ENODEV;
+
+	ioapic_set_alloc_attr(&info, NUMA_NO_NODE, 1, 0);
+	ret = mp_map_gsi_to_irq(RTC_IRQ, IOAPIC_MAP_ALLOC, &info);
+	if (ret < 0) {
+		pr_info("Failed to allocate RTC interrupt.

Disabling RTC\n");

arch/x86/platform/intel-mid/device_libs/platform_mrfld_rtc.c new file +13 -0
+		x86_platform.legacy.rtc = 0;
+		return ret;
+	}
+
+	return 0;
+}
+
+static int __init mrfld_legacy_rtc_init(void)
+{
+	if (intel_mid_identify_cpu() != INTEL_MID_CPU_CHIP_TANGIER)
+		return -ENODEV;
+
+	return mrfld_legacy_rtc_alloc_irq();
Quoted 8 lines

Review

Quoted 2 lines
(quoted diff) +2 -0
+}
+rootfs_initcall(mrfld_legacy_rtc_init);

rootfs_initcall???? That does not make any sense at all. I know you need it
before the device initcalls, but just using a random initcall level before
device_initcall is wrong.

I copied watchdog approach, would you like me to move watchdog there as well?

Quoted 2 lines

arch_initcall is much more suitable.

So, I think I need to send v4, but I would wait for answer for the above.

Thanks for review!

Signature · 2 lines
Andy Shevchenko <[email protected]>
Intel Finland Oy
author
#5 lore raw

PATCH v3 3/3 x86/platform/intel-mid: Don't shadow error code of mp_map_gsi_to_irq()

In reply to Andy Shevchenko

When call mp_map_gsi_to_irq() and return its error code do not shadow it. Note that 0 is not an error.

Signed-off-by Andy Shevchenko <[email protected]>
---
 arch/x86/platform/intel-mid/device_libs/platform_mrfld_wdt.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
arch/x86/platform/intel-mid/device_libs/platform_mrfld_wdt.c +5 -5
index 3f1f1c77d090..8a10a56f2840 100644
@@ -28,9 +28,9 @@ static struct platform_device wdt_dev = {
 
 static int tangier_probe(struct platform_device *pdev)
 {
-	int gsi;
 	struct irq_alloc_info info;
 	struct intel_mid_wdt_pdata *pdata = pdev->dev.platform_data;
+	int gsi, irq;
 
 	if (!pdata)
 		return -EINVAL;

@@ -38,10 +38,10 @@ static int tangier_probe(struct platform_device *pdev)
 	/* IOAPIC builds identity mapping between GSI and IRQ on MID */
 	gsi = pdata->irq;
 	ioapic_set_alloc_attr(&info, cpu_to_node(0), 1, 0);
-	if (mp_map_gsi_to_irq(gsi, IOAPIC_MAP_ALLOC, &info) <= 0) {
-		dev_warn(&pdev->dev, "cannot find interrupt %d in ioapic\n",
-			 gsi);
-		return -EINVAL;
+	irq = mp_map_gsi_to_irq(gsi, IOAPIC_MAP_ALLOC, &info);
+	if (irq < 0) {
+		dev_warn(&pdev->dev, "cannot find interrupt %d in ioapic\n", gsi);
+		return irq;
 	}
 
 	return 0;
Signature · 1 line
2.11.0