Re: [PATCH 1/1] libata: pata_pdc2027x PLL input clock fix

2 messages, 2 authors, 2007-07-11 · open the first message on its own page

Re: [PATCH 1/1] libata: pata_pdc2027x PLL input clock fix

From: Mikael Pettersson <hidden>
Date: 2007-07-10 23:15:12

On Tue, 10 Jul 2007 11:52:59 +0800, Albert Lee wrote:
quoted hunk
quoted
quoted
Recently the PLL input clock of pata_pdc2027x is sometimes detected
higer than expected (e.g. 20.027 MHz compared to 16.714 MHz).
It seems sometimes the mdelay() function is not as precise as it
used to be. Per Alan's advice, HT or power management might affect
the precision of mdelay().

This patch calls gettimeofday() to mesure the time elapsed and
calculate the PLL input clock accordingly.

Unfortunately this breaks pata_pdc2027x on my PowerMac G3:
<snip>
quoted
In fairness, this is a slightly non-standard PowerMac G3, in that it
has a G4 upgrade processor. The firmware doesn't recognise the CPU
and gives some CPU core frequency-related properties too low values.
However, the bus frequency property _is_ correct, which is what
most or all timing stuff should be based on.

Looks like a platform bug.
According to the document, do_gettimeofday() has microsecond
resolution. Since the driver calls mdelay(100) (100000 microseconds), 
it won't affect the PLL input clock calculation much if somehow
do_gettimeofday() drifts several (say 100) microseconds.

Could you please apply the attached debug patch and collect more info
on the PowerMac G3. Hopefully we can have more clue. Thanks.
--
albert

(BTW, maybe opening a bug in bugzilla.kernel.org would help the
debugging.)
--- 00_libata-dev/drivers/ata/pata_pdc2027x.c	2007-07-07 09:58:55.000000000 +0800
+++ 01_debug/drivers/ata/pata_pdc2027x.c	2007-07-10 11:18:38.000000000 +0800
@@ -722,6 +722,15 @@ static long pdc_detect_pll_input_clock(s
 	pll_clock = (start_count - end_count) / 100 *
 		(100000000 / usec_elapsed);
 
+	do_gettimeofday(&start_time);
+	mdelay(37);
+	do_gettimeofday(&end_time);
+	usec_elapsed = (end_time.tv_sec - start_time.tv_sec) * 1000000 +
+		(end_time.tv_usec - start_time.tv_usec);
+	printk(KERN_ERR "usec_elapsed for mdelay(37) [%ld]\n", usec_elapsed);
+	printk(KERN_ERR "start time: [%ld]s [%ld]us \n", start_time.tv_sec, start_time.tv_usec);
+	printk(KERN_ERR "end   time: [%ld]s [%ld]us \n", end_time.tv_sec, end_time.tv_usec);
+
 	PDPRINTK("start[%ld] end[%ld] \n", start_count, end_count);
 	PDPRINTK("PLL input clock[%ld]Hz\n", pll_clock);
2.6.22 + this prints the following on my G3:

pata_pdc2027x 0000:00:0e.0: version 0.9
usec_elapsed for mdelay(37) [35431]
start time: [1184112028]s [775333]us 
end   time: [1184112028]s [810764]us 
pata_pdc2027x 0000:00:0e.0: PLL input clock 1691741 kHz
pata_pdc2027x: Invalid PLL input clock 1691741kHz, give up!

/Mikael

Re: [PATCH 1/1] libata: pata_pdc2027x PLL input clock fix

From: Albert Lee <hidden>
Date: 2007-07-11 02:45:49

Mikael Pettersson wrote:

2.6.22 + this prints the following on my G3:

pata_pdc2027x 0000:00:0e.0: version 0.9
usec_elapsed for mdelay(37) [35431]
start time: [1184112028]s [775333]us 
end   time: [1184112028]s [810764]us 
pata_pdc2027x 0000:00:0e.0: PLL input clock 1691741 kHz
pata_pdc2027x: Invalid PLL input clock 1691741kHz, give up!
My x86 box got this:
Jul 11 10:02:17 p4ht-s kernel: [  260.746980] ACPI: PCI Interrupt 0000:02:05.0[A] -> Link [LNK1] -> GSI 10 (level, low) -> IRQ 10
Jul 11 10:02:17 p4ht-s kernel: [  260.882905] usec_elapsed for mdelay(37) [36734]
Jul 11 10:02:17 p4ht-s kernel: [  260.882911] start time: [1184119336]s [999802]us 
Jul 11 10:02:17 p4ht-s kernel: [  260.882914] end   time: [1184119337]s [36536]us 
Jul 11 10:02:17 p4ht-s kernel: [  260.882917] pata_pdc2027x 0000:02:05.0: PLL input clock 16829 kHz

So, it seems both mdelay(37) and do_gettimeofday() are working properly on PowerMac G3.
Maybe the calculated wrong PLL input is due to wrong reading of the counter register?
Could you please try the attached debug patch for more clue, thanks.

--
albert

diff -Nrup 00_libata-dev/drivers/ata/pata_pdc2027x.c 01_debug/drivers/ata/pata_pdc2027x.c
--- 00_libata-dev/drivers/ata/pata_pdc2027x.c	2007-07-07 09:58:55.000000000 +0800
+++ 01_debug/drivers/ata/pata_pdc2027x.c	2007-07-11 10:41:41.000000000 +0800
@@ -574,8 +574,8 @@ retry:
 
 	counter = (bccrh << 15) | bccrl;
 
-	PDPRINTK("bccrh [%X] bccrl [%X]\n", bccrh,  bccrl);
-	PDPRINTK("bccrhv[%X] bccrlv[%X]\n", bccrhv, bccrlv);
+	printk(KERN_ERR "bccrh [%X] bccrl [%X]\n", bccrh,  bccrl);
+	printk(KERN_ERR "bccrhv[%X] bccrlv[%X]\n", bccrhv, bccrlv);
 
 	/*
 	 * The 30-bit decreasing counter are read by 2 pieces.
@@ -584,7 +584,7 @@ retry:
 	 */
 	if (retry && !(bccrh == bccrhv && bccrl >= bccrlv)) {
 		retry--;
-		PDPRINTK("rereading counter\n");
+		printk(KERN_ERR "rereading counter\n");
 		goto retry;
 	}
 
@@ -722,8 +722,21 @@ static long pdc_detect_pll_input_clock(s
 	pll_clock = (start_count - end_count) / 100 *
 		(100000000 / usec_elapsed);
 
-	PDPRINTK("start[%ld] end[%ld] \n", start_count, end_count);
-	PDPRINTK("PLL input clock[%ld]Hz\n", pll_clock);
+	printk(KERN_ERR "start[%ld] end[%ld] \n", start_count, end_count);
+	printk(KERN_ERR "usec_elapsed for mdelay(100) [%ld]\n", usec_elapsed);
+	printk(KERN_ERR "start time: [%ld]s [%ld]us \n", start_time.tv_sec, start_time.tv_usec);
+	printk(KERN_ERR "end   time: [%ld]s [%ld]us \n", end_time.tv_sec, end_time.tv_usec);
+	printk(KERN_ERR "PLL input clock[%ld]Hz\n", pll_clock);
+
+	/* mdelay(37) for comparison */
+	do_gettimeofday(&start_time);
+	mdelay(37);
+	do_gettimeofday(&end_time);
+	usec_elapsed = (end_time.tv_sec - start_time.tv_sec) * 1000000 +
+		(end_time.tv_usec - start_time.tv_usec);
+	printk(KERN_ERR "usec_elapsed for mdelay(37) [%ld]\n", usec_elapsed);
+	printk(KERN_ERR "start time: [%ld]s [%ld]us \n", start_time.tv_sec, start_time.tv_usec);
+	printk(KERN_ERR "end   time: [%ld]s [%ld]us \n", end_time.tv_sec, end_time.tv_usec);
 
 	return pll_clock;
 }
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help