Re: [RFC PATCH 3/6] x86/apic: Extract APIC timer related code from apic_bsp_setup()

[Dou Liyang <douly.fnst@cn.fujitsu.com>]

Hi Thomas,

At 04/05/2017 07:56 PM, Thomas Gleixner wrote:
> On Wed, 29 Mar 2017, Dou Liyang wrote:
>> +/* Setup local APIC timer and get the Id*/
>> +static int __init apic_bsp_timer_setup(void)
>
> This does not make sense. The id and the timers have nothing to do with
> each other.

Yes, Indeed. Here is more like the rest work for APIC setup, which
can't be executed earlier, The name is not suitable. I will refactor
it.

>> +{
>> +	int id;
>> +
>> +	if (x2apic_mode)
>> +		id = apic_read(APIC_LDR);
>> +	else
>> +		id = GET_APIC_LOGICAL_ID(apic_read(APIC_LDR));
>> +
>> +	if (!skip_ioapic_setup && nr_ioapics && nr_legacy_irqs())
>> +		check_timer();
>
> Why are you moving this to the APIC? check_timer() has absolutely nothing

Because, the jiffies is used in check_timer() for checking timer irq 
works(timer_irq_works()) and can't work at the beginning.

So, keep check_timer() in IOAPIC setup function (setup_IO_APIC()) which
has been moved to init_IRQ() will make the kernel not boot up.

> to do with the apic timer. It's a IOAPIC only issue and required to test
> that the IRQ0 interrupt delivery works through the IOAPIC.
>

Yes, I see, split check_timer() is not a good way, and move the APIC
initialization to the end of init_IRQ() is also not well.

Thanks,
	Liyang

> Thanks,
>
> 	tglx
>
>
>