On Mon, Jan 16, 2017 at 07:23:45PM +0200, Andy Shevchenko wrote:
> 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
>
> 82a51c38f199 ("x86/platform/intel-mid: Enable RTC on Intel Merrifield")
Referring to commits on linux-next gitsums is not proper as they change
every day, so you might as well leave them out.
> does it in a wrong place,
There are other things wrong with it.. It had:
diff --git a/arch/x86/platform/intel-mid/mrfld.c
b/arch/x86/platform/intel-mid/mrfld.c
index e0607c77a1bd..ae7bdeb0e507 100644
--- a/arch/x86/platform/intel-mid/mrfld.c
+++ b/arch/x86/platform/intel-mid/mrfld.c
@@ -91,6 +91,7 @@ static unsigned long __init tangier_calibrate_tsc(void)
static void __init tangier_arch_setup(void)
{
x86_platform.calibrate_tsc = tangier_calibrate_tsc;
+ x86_platform.legacy.rtc = 1;
}
/* tangier arch ops */
You want to set quirks by setting the x86_platform.set_legacy_features given
then on x86_early_init_platform_quirks() we have:
if (x86_platform.set_legacy_features)
x86_platform.set_legacy_features();
For example see xen_dom0_set_legacy_features().
> 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.
Doing it the above way would also make it a quirk only for the needed
devices.
>
> Move interrupt allocation to arch/x86/kernel/rtc.c module and allocate
> it for Intel MID platform devices only.
>
> Fixes: 82a51c38f199 ("x86/platform/intel-mid: Enable RTC on Intel Merrifield")
> Cc: Luis R. Rodriguez <[email protected]>
> Signed-off-by: Andy Shevchenko <[email protected]>
> ---
> - add headers and #ifdef to make kbuild bot happy
> - re-test on PNP platform with acpi=off, thus, update patch accordingly
>
> arch/x86/kernel/rtc.c | 21 +++++++++++++++++++++
> arch/x86/platform/intel-mid/sfi.c | 14 --------------
> 2 files changed, 21 insertions(+), 14 deletions(-)
>
> diff --git a/arch/x86/kernel/rtc.c b/arch/x86/kernel/rtc.c
> index 5b21cb7d84d6..bf4cb88b9186 100644
> --- a/arch/x86/kernel/rtc.c
> +++ b/arch/x86/kernel/rtc.c
> @@ -12,7 +12,9 @@
> #include <asm/vsyscall.h>
> #include <asm/x86_init.h>
> #include <asm/time.h>
> +#include <asm/hw_irq.h>
> #include <asm/intel-mid.h>
> +#include <asm/io_apic.h>
> #include <asm/setup.h>
>
> #ifdef CONFIG_X86_32
> @@ -155,6 +157,20 @@ void read_persistent_clock(struct timespec *ts)
> x86_platform.get_wallclock(ts);
> }
>
> +#ifdef CONFIG_X86_IO_APIC
> +static __init int allocate_rtc_cmos_irq(void)
> +{
> + struct irq_alloc_info info;
> +
> + if (!intel_mid_identify_cpu())
> + return 0;
> +
> + ioapic_set_alloc_attr(&info, NUMA_NO_NODE, 1, 0);
> + return mp_map_gsi_to_irq(RTC_IRQ, IOAPIC_MAP_ALLOC, &info);
> +}
> +#else
> +static inline int allocate_rtc_cmos_irq(void) { return 0; }
> +#endif
>
> static struct resource rtc_resources[] = {
> [0] = {
> @@ -178,6 +194,7 @@ static struct platform_device rtc_device = {
>
> static __init int add_rtc_cmos(void)
> {
> + int ret;
> #ifdef CONFIG_PNP
> static const char * const ids[] __initconst =
> { "PNP0b00", "PNP0b01", "PNP0b02", };
> @@ -197,6 +214,10 @@ static __init int add_rtc_cmos(void)
> if (!x86_platform.legacy.rtc)
> return -ENODEV;
>
> + ret = allocate_rtc_cmos_irq();
> + if (ret < 0)
> + return ret;
> +
Ugh this is seriously ugly, can't we avoid this sort of thing with the
callback and then let the internal MID code do what it needs?
Luis
