From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751257AbdAPTEq (ORCPT ); Mon, 16 Jan 2017 14:04:46 -0500 Received: from mx2.suse.de ([195.135.220.15]:37052 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750803AbdAPTEm (ORCPT ); Mon, 16 Jan 2017 14:04:42 -0500 Date: Mon, 16 Jan 2017 20:04:37 +0100 From: "Luis R. Rodriguez" To: Andy Shevchenko Cc: Ingo Molnar , Thomas Gleixner , "H . Peter Anvin" , linux-kernel@vger.kernel.org, x86@kernel.org, "Luis R . Rodriguez" Subject: Re: [PATCH v2 1/1] x86/rtc: Allocate interrupt for platform device Message-ID: <20170116190437.GB13946@wotan.suse.de> References: <20170116172345.3823-1-andriy.shevchenko@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170116172345.3823-1-andriy.shevchenko@linux.intel.com> User-Agent: Mutt/1.6.0 (2016-04-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > Signed-off-by: Andy Shevchenko > --- > - 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 > #include > #include > +#include > #include > +#include > #include > > #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