Re: [git pull?] clocksource: ACPI pmtmr bugfixes [Was: Re: ACPI PM-Timer on K6-3 SiS5591: Houston...]

[Andrew Morton <akpm@linux-foundation.org>]

On Mon, 18 Aug 2008 21:03:25 +0200
Dominik Brodowski <linux@dominikbrodowski.net> wrote:

> Hi,
> 
> anyone willing to take these two patches? Possibly 2.6.27 material; hasn't
> been in -next or -mm.
> 
> Best,
> 	Dominik
> 
> 
> The following changes since commit 796aadeb1b2db9b5d463946766c5bbfd7717158c:
>   Linus Torvalds (1):
>         Merge branch 'fixes' of git://git.kernel.org/.../davej/cpufreq
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/brodo/pcmcia-2.6.git clocksource
> 
> Dominik Brodowski (2):
>       acpi_pm.c: use proper read function also in errata mode.
>       acpi_pm.c: check for monotonicity
> 
>  drivers/clocksource/acpi_pm.c |   50 +++++++++++++++++++++++-----------------
>  1 files changed, 29 insertions(+), 21 deletions(-)

A bare git URL is somewhat user-unfriendly.

<fiddle, fiddle>

: commit b985f0517e31c1204b5aafb94f86202948f00e16
: Author: Dominik Brodowski <linux@dominikbrodowski.net>
: Date:   Sun Aug 10 21:24:21 2008 +0200
: 
:     acpi_pm.c: use proper read function also in errata mode.
:     
:     When acpi_pm is used in errata mode (three reads instead of one), also the
:     acpi_pm init functions need to use three reads instead of just one.

hm, why?  Was there some observeable problem which this change improved?

:     Thanks to Andreas Mohr for spotting this issue.
:     
:     CC: John Stultz <johnstul@us.ibm.com>
:     CC: Thomas Gleixner <tglx@linutronix.de>
:     CC: Ingo Molnar <mingo@elte.hu>
:     Signed-off-by: Dominik Brodowski <linux@dominikbrodowski.net>
: 
: diff --git a/drivers/clocksource/acpi_pm.c b/drivers/clocksource/acpi_pm.c
: index 5ca1d80..2c00edd 100644
: --- a/drivers/clocksource/acpi_pm.c
: +++ b/drivers/clocksource/acpi_pm.c
: @@ -151,13 +151,13 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_SERVERWORKS, PCI_DEVICE_ID_SERVERWORKS_LE,
:   */
:  static int verify_pmtmr_rate(void)
:  {
: -	u32 value1, value2;
: +	cycle_t value1, value2;
:  	unsigned long count, delta;
:  
:  	mach_prepare_counter();
: -	value1 = read_pmtmr();
: +	value1 = clocksource_acpi_pm.read()
:  	mach_countup(&count);
: -	value2 = read_pmtmr();
: +	value2 = clocksource_acpi_pm.read()
:  	delta = (value2 - value1) & ACPI_PM_MASK;
:  
:  	/* Check that the PMTMR delta is within 5% of what we expect */
: @@ -177,7 +177,7 @@ static int verify_pmtmr_rate(void)
:  
:  static int __init init_acpi_pm_clocksource(void)
:  {
: -	u32 value1, value2;
: +	cycle_t value1, value2;
:  	unsigned int i;
:  
:  	if (!pmtmr_ioport)
: @@ -187,9 +187,9 @@ static int __init init_acpi_pm_clocksource(void)
:  						clocksource_acpi_pm.shift);
:  
:  	/* "verify" this timing source: */
: -	value1 = read_pmtmr();
: +	value1 = clocksource_acpi_pm.read();
:  	for (i = 0; i < 10000; i++) {
: -		value2 = read_pmtmr();
: +		value2 = clocksource_acpi_pm.read();
:  		if (value2 == value1)
:  			continue;
:  		if (value2 > value1)
: @@ -197,11 +197,11 @@ static int __init init_acpi_pm_clocksource(void)
:  		if ((value2 < value1) && ((value2) < 0xFFF))
:  			goto pm_good;
:  		printk(KERN_INFO "PM-Timer had inconsistent results:"
: -			" 0x%#x, 0x%#x - aborting.\n", value1, value2);
: +			" 0x%#llx, 0x%#llx - aborting.\n", value1, value2);
:  		return -EINVAL;
:  	}
:  	printk(KERN_INFO "PM-Timer had no reasonable result:"
: -			" 0x%#x - aborting.\n", value1);
: +			" 0x%#llx - aborting.\n", value1);
:  	return -ENODEV;
:  
:  pm_good:
: 


and

: commit a44299593315e055f28fe96b4767a85c46b2955f
: Author: Dominik Brodowski <linux@dominikbrodowski.net>
: Date:   Sun Aug 10 21:34:54 2008 +0200
: 
:     acpi_pm.c: check for monotonicity
:     
:     Expand the check for monotonicity by doing ten tests instead of one.

Why?

:     Applies on top of "acpi_pm.c: use proper read function also in errata mode."
:     
:     CC: John Stultz <johnstul@us.ibm.com>
:     CC: Thomas Gleixner <tglx@linutronix.de>
:     CC: Ingo Molnar <mingo@elte.hu>
:     Signed-off-by: Dominik Brodowski <linux@dominikbrodowski.net>
: 
: diff --git a/drivers/clocksource/acpi_pm.c b/drivers/clocksource/acpi_pm.c
: index 2c00edd..f05c4fb 100644
: --- a/drivers/clocksource/acpi_pm.c
: +++ b/drivers/clocksource/acpi_pm.c
: @@ -178,7 +178,7 @@ static int verify_pmtmr_rate(void)
:  static int __init init_acpi_pm_clocksource(void)
:  {
:  	cycle_t value1, value2;
: -	unsigned int i;
: +	unsigned int i, j, good = 0;
:  
:  	if (!pmtmr_ioport)
:  		return -ENODEV;
: @@ -187,24 +187,32 @@ static int __init init_acpi_pm_clocksource(void)
:  						clocksource_acpi_pm.shift);
:  
:  	/* "verify" this timing source: */
: -	value1 = clocksource_acpi_pm.read();
: -	for (i = 0; i < 10000; i++) {
: -		value2 = clocksource_acpi_pm.read();
: -		if (value2 == value1)
: -			continue;
: -		if (value2 > value1)
: -			goto pm_good;
: -		if ((value2 < value1) && ((value2) < 0xFFF))
: -			goto pm_good;
: -		printk(KERN_INFO "PM-Timer had inconsistent results:"
: -			" 0x%#llx, 0x%#llx - aborting.\n", value1, value2);
: -		return -EINVAL;
: +	for (j = 0; j < 10; j++) {
: +		value1 = clocksource_acpi_pm.read();
: +		for (i = 0; i < 10000; i++) {
: +			value2 = clocksource_acpi_pm.read();
: +			if (value2 == value1)
: +				continue;
: +			if (value2 > value1)
: +				good++;
: +				break;
: +			if ((value2 < value1) && ((value2) < 0xFFF))
: +				good++;
: +				break;
: +			printk(KERN_INFO "PM-Timer had inconsistent results:"
: +			       " 0x%#llx, 0x%#llx - aborting.\n",
: +			       value1, value2);
: +			return -EINVAL;
: +		}
: +		udelay(300 * i);
: +	}
: +
: +	if (good != 10) {
: +		printk(KERN_INFO "PM-Timer had no reasonable result:"
: +		       " 0x%#llx - aborting.\n", value1);
: +		return -ENODEV;
:  	}
: -	printk(KERN_INFO "PM-Timer had no reasonable result:"
: -			" 0x%#llx - aborting.\n", value1);
: -	return -ENODEV;
:  
: -pm_good:
:  	if (verify_pmtmr_rate() != 0)
:  		return -ENODEV;
:  


I guess this file falls under Thomas's git-hrt tree.  I can queue the
patches up and spam Thomas with them, but I'm at a bit of a loss
regarding their priority due to the above questions.