Re: HPET : Adding Legacy Route Replacement option (2nd try)

["Randy.Dunlap" <randy.dunlap@oracle.com>]

Om Narasimhan wrote:
> Hi,
> My last mail with patch seems to be messed up. So resending the patch after
> incorporating randy's comments.
> Applies cleanly to 2.6.19-rc2

Yes.  Mostly  :)
Warning: trailing whitespace in line 685 of arch/i386/kernel/acpi/boot.c
Warning: trailing whitespace in lines 304,313,324,332,333 of Documentation/hpet.txt

Thanks for adding the externs to the hpet.h. file.

I didn't see this updated patch on lkml.  Was it dropped by the vger mail server?

You should probably cc: some of the HPET maintainers (search for "HPET"
in the MAINTAINERS file).

The kernel-parameters.txt still needs a little fixup.  See comments below.


> Signed-Off-by : Om Narasimhan <om.turyx@gmail.com>
> 
>  Documentation/hpet.txt              |   56 +++++++++++++++++++++++++++++++++++
>  Documentation/kernel-parameters.txt |    2 +
>  arch/i386/kernel/acpi/boot.c        |   18 +++++++++++
>  arch/i386/kernel/time_hpet.c        |    3 +-
>  arch/x86_64/kernel/time.c           |   16 +++++++---
>  include/asm-x86_64/hpet.h           |    1 +
>  include/linux/acpi.h                |    1 +
>  include/linux/hpet.h                |    7 ++++
>  8 files changed, 98 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/hpet.txt b/Documentation/hpet.txt
> index b7a3dc3..b9daf0d 100644
> --- a/Documentation/hpet.txt
> +++ b/Documentation/hpet.txt
> @@ -298,3 +298,59 @@ members of the hpet_task structure befor
>  hpet_control simply vectors to the hpet_ioctl routine and has the same
>  commands and respective arguments as the user API.  hpet_unregister
>  is used to terminate usage of the HPET timer reserved by hpet_register.
> +
> +		HPET Legacy Replacement Route option (hpet_lrr)
> +
> +HPET is capable of replacing the IRQ0 (connected INT0 PIN) routing for 
> +timer interrupt. The capability register (at offset 0 of HPET
> +base address) has a bit specifying if HPET chip is capbale of doing
> +this. OS can read the bit either from HW or ACPI table. (HPET ACPI
> +description table -> Event Timer block -> bit 15, page 30 of HPET
> +spec).  Ideally (I think so!) BIOS should set the ACPI table than letting
> +the OS read H/W, which gives the BIOS a way to configure either legacy
> +or Legacy replacement modes.
> +
> +Typically the motherboard has BIOS configured / hardwired IRQ0 to INT0 
> +(pin of APIC) connection. Linux assumes IRQ0 connected to INT0 unless it is
> +supplied using an override parameter in the MPTable. Some NVidia chipsets /
> +BIOS initialization code had configured to override IRQ0 -> INT0 connection
> +and later a parameter was introduce (acpi_skip_timer_override) to get IRQ0 ->
> +INT0 connection right.
> +
> +But a number of bioses (both phoenix and AMI) are not working as
> +expected. (I have an AMI BIOS which sets ACPI table bit 15 to 0 and then
> +connect IRQ0 -> INT2 internally, Another bios I have sets the ACPI table bit
> +15 to 0, but does not connect IRQ0 -> INT2. Both would result in a hang in
> +calibrate_delay() since there would not be any timer interrupts So I have 
> +provided a command line parameter which overrides
> +the BIOS ACPI entry. So, irrespctive of the BIOS' HPET ACPI Descriptor
> +table settings, if the parameter hpet_lrr=[0,1] is specified, it takes
> +precedence.
> +
> +* When to use this parameter?
> +
> +Some latest versions CK-804 (e.g),(Actually the code initializes the 
> +CK804 in the BIOS), would correctly set the HPET such that there would not 
> +be any interrupts on INT0. Linux does not handle this situation very well
> +because in linux, if HW is LRR capable, it is enabled from the OS. Still the
> +timer interrupt handler is setup for IRQ0. Under this situation, you can force
> +the parameter hpet_lrr=1, so that IRQ2 is timer interrupts.
> +
> +[root@mophia ~]# cat /proc/interrupts | grep 2:
> + 2:        163          0          0          0          1          7
> +207     955341    IO-APIC-edge  timer
> +
> +[root@mophia ~]# uptime
> + 22:52:38 up 15 min,  2 users,  load average: 0.00, 0.01, 0.02
> +
> +[root@mophia ~]# dmesg | grep -i MP-BIOS
> +
> +For 15 mts (900 sec), around 95k interrupts on timer looks kinda fine.
> +
> +* Known Bugs:
> +I have tested it only with Nvidia CK-804. There seem to be some kind of timing
> +issue between enabling the HPET with LRR set and start of tinerrupts. As a
> +result of which, calibrate_delay() hangs because there are no interrupts. If
> +you run into such a case, pass lpj=<bogomips * 500> as a work around. i.e, if
> +your bogomips is 5000, pass lpj=2500000
> +
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index ff571f9..3066b74 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -365,6 +365,8 @@ and is between 256 and 4096 characters. 
>  
>  	hpet=		[IA-32,HPET] option to disable HPET and use PIT.
>  			Format: disable
> +	hpet_lrr=	[IA32,X86_64,HPET] Option to enable/disable the HPET Legacy
> +			replacement route.

a.  Don't go past column 80 (as in "Legacy").
b.  Blank line before hpet_lrr= line.
c.  Add another line:
		Format: { 1 | 0 }

>  	cm206=		[HW,CD]
>  			Format: { auto | [<io>,][<irq>] }
> diff --git a/arch/i386/kernel/acpi/boot.c b/arch/i386/kernel/acpi/boot.c
> index 92f79cd..2c8a950 100644
> --- a/arch/i386/kernel/acpi/boot.c
> +++ b/arch/i386/kernel/acpi/boot.c
> @@ -82,6 +82,17 @@ EXPORT_SYMBOL(acpi_strict);
>  acpi_interrupt_flags acpi_sci_flags __initdata;
>  int acpi_sci_override_gsi __initdata;
>  int acpi_skip_timer_override __initdata;
> +/* HPET Legacy routing replacement option passed through ACPI Table */
> +int acpi_hpet_lrr;
> +/* cmdline opt. for faulty bioses not setting ACPI HPET entry right */
> +int hpet_lrr_force;
> +
> +static int hpet_lrr_setup (char *str)
> +{
> +	get_option(&str, &hpet_lrr_force);
> +	return 1;
> +}
> +__setup ("hpet_lrr=", hpet_lrr_setup);
>  
>  #ifdef CONFIG_X86_LOCAL_APIC
>  static u64 acpi_lapic_addr __initdata = APIC_DEFAULT_PHYS_BASE;
> @@ -669,6 +680,13 @@ #define HPET_RESOURCE_NAME_SIZE 9
>  			 "HPET %u", hpet_tbl->number);
>  		hpet_res->end = (1 * 1024) - 1;
>  	}
> +	acpi_hpet_lrr = (hpet_tbl->id & ACPI_HPET_LRR_CAP) ? 1 : 0;
> +	/* Print a message about the bios HPET ACPI Desc Table passed.
> +	 * LRR bit should not be set in the table unless IRQ0->INT2 is 
> +	 * connected. But BIOS may be faulty ...
> +	 */
> +	printk(KERN_INFO PREFIX "HPET id: %#x. ACPI LRR bit %s SET\n",
> +			hpet_tbl->id, acpi_hpet_lrr ? "": "NOT");
>  
>  #ifdef	CONFIG_X86_64
>  	vxtime.hpet_address = hpet_tbl->addr.addrl |
> diff --git a/arch/i386/kernel/time_hpet.c b/arch/i386/kernel/time_hpet.c
> index 1a2a979..01b2f67 100644
> --- a/arch/i386/kernel/time_hpet.c
> +++ b/arch/i386/kernel/time_hpet.c
> @@ -94,7 +94,8 @@ static int hpet_timer_stop_set_go(unsign
>   	 * Go!
>   	 */
>  	cfg = hpet_readl(HPET_CFG);
> -	if (hpet_use_timer)
> +	/* Ideally the following should be &&(acpi_hpet_lrr || hpet_lrr_force) */
> +	if (hpet_use_timer && hpet_lrr_force)
>  		cfg |= HPET_CFG_LEGACY;
>  	cfg |= HPET_CFG_ENABLE;
>  	hpet_writel(cfg, HPET_CFG);
> diff --git a/arch/x86_64/kernel/time.c b/arch/x86_64/kernel/time.c
> index 1ba5a44..0f5d990 100644
> --- a/arch/x86_64/kernel/time.c
> +++ b/arch/x86_64/kernel/time.c
> @@ -46,9 +46,6 @@ #include <asm/apic.h>
>  #ifdef CONFIG_CPU_FREQ
>  static void cpufreq_delayed_get(void);
>  #endif
> -extern void i8254_timer_resume(void);
> -extern int using_apic_timer;
> -
>  static char *timename = NULL;
>  
>  DEFINE_SPINLOCK(rtc_lock);
> @@ -783,7 +780,10 @@ static int hpet_timer_stop_set_go(unsign
>  		    HPET_TN_32BIT, HPET_T0_CFG);
>  		hpet_writel(hpet_tick, HPET_T0_CMP); /* next interrupt */
>  		hpet_writel(hpet_tick, HPET_T0_CMP); /* period */
> -		cfg |= HPET_CFG_LEGACY;
> +		/* Ideal value (acpi_hpet_lrr || hpet_lrr_force) */
> +		if (hpet_lrr_force)
> +			cfg |= HPET_CFG_LEGACY;
> +
>  	}
>  /*
>   * Go!
> @@ -887,6 +887,7 @@ time_cpu_notifier(struct notifier_block 
>  
>  void __init time_init(void)
>  {
> +	int timer_irq = 0;
>  	if (nohpet)
>  		vxtime.hpet_address = 0;
>  
> @@ -906,6 +907,10 @@ void __init time_init(void)
>  	  	tick_nsec = TICK_NSEC_HPET;
>  		cpu_khz = hpet_calibrate_tsc();
>  		timename = "HPET";
> +		/* Ideal value is (acpi_hpet_lrr || hpet_lrr_force) */
> +		if (hpet_lrr_force)
> +			timer_irq = HPET_TIMER_LRR_IRQ;
> +
>  #ifdef CONFIG_X86_PM_TIMER
>  	} else if (pmtmr_ioport && !vxtime.hpet_address) {
>  		vxtime_hz = PM_TIMER_FREQUENCY;
> @@ -924,7 +929,8 @@ #endif
>  	vxtime.tsc_quot = (USEC_PER_MSEC << US_SCALE) / cpu_khz;
>  	vxtime.last_tsc = get_cycles_sync();
>  	set_cyc2ns_scale(cpu_khz);
> -	setup_irq(0, &irq0);
> +	printk(KERN_WARNING PREFIX "Registering Timer IRQ = %d\n", timer_irq);
> +	setup_irq(timer_irq, &irq0);
>  	hotcpu_notifier(time_cpu_notifier, 0);
>  	time_cpu_notifier(NULL, CPU_ONLINE, (void *)(long)smp_processor_id());
>  
> diff --git a/include/asm-x86_64/hpet.h b/include/asm-x86_64/hpet.h
> index b390984..2d18b39 100644
> --- a/include/asm-x86_64/hpet.h
> +++ b/include/asm-x86_64/hpet.h
> @@ -37,6 +37,7 @@ #define HPET_CFG_ENABLE	0x001
>  #define HPET_CFG_LEGACY	0x002
>  #define	HPET_LEGACY_8254	2
>  #define	HPET_LEGACY_RTC		8
> +#define	HPET_TIMER_LRR_IRQ	2
>  
>  #define HPET_TN_LEVEL		0x0002
>  #define HPET_TN_ENABLE		0x0004
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 88b5dfd..aac4a89 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -220,6 +220,7 @@ enum acpi_interrupt_id {
>  };
>  
>  #define	ACPI_SPACE_MEM		0
> +#define	ACPI_HPET_LRR_CAP	0x8000
>  
>  struct acpi_gen_regaddr {
>  	u8  space_id;
> diff --git a/include/linux/hpet.h b/include/linux/hpet.h
> index 707f7cb..6c31473 100644
> --- a/include/linux/hpet.h
> +++ b/include/linux/hpet.h
> @@ -119,6 +119,13 @@ int hpet_register(struct hpet_task *, in
>  int hpet_unregister(struct hpet_task *);
>  int hpet_control(struct hpet_task *, unsigned int, unsigned long);
>  
> +
> +/* these are used by time.c */
> +extern void i8254_timer_resume(void);
> +extern int using_apic_timer;
> +extern int acpi_hpet_lrr;
> +extern int hpet_lrr_force;
> +
>  #endif /* __KERNEL__ */
>  
>  struct hpet_info {


-- 
~Randy