Re: [PATCH] pata_hpt3x2n: Clean up DPLL stuff
On Mon, 01 Oct 2007 17:12:03 +0400 Sergei Shtylyov [EMAIL PROTECTED] wrote: Hello. Alan Cox wrote: Nobody commented when I asked for review earlier so it must be ok 8) It's not that I've seen this before so it must be ok 8) Not by me at least, so let me NAK it. 8-) Lets stick it in -mm to be sure Now let's unstick it. :-) Ok I shall revisit that. Andrew please drop - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] pata_hpt3x2n: Clean up DPLL stuff
Alan Cox wrote: On Mon, 01 Oct 2007 17:12:03 +0400 Sergei Shtylyov [EMAIL PROTECTED] wrote: Hello. Alan Cox wrote: Nobody commented when I asked for review earlier so it must be ok 8) It's not that I've seen this before so it must be ok 8) Not by me at least, so let me NAK it. 8-) Lets stick it in -mm to be sure Now let's unstick it. :-) Ok I shall revisit that. Andrew please drop As I noted at the time of posting, this is in libata-dev.git#for-testing (and thus propagated to -mm via that route). Consider it dropped, from my end. Jeff - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] pata_hpt3x2n: Clean up DPLL stuff
Hello.
Alan Cox wrote:
Nobody commented when I asked for review earlier so it must be ok 8)
It's not that I've seen this before
so it must be ok 8)
Not by me at least, so let me NAK it. 8-)
Lets stick it in -mm to be sure
Now let's unstick it. :-)
Signed-off-by: Alan Cox [EMAIL PROTECTED]
diff -u --new-file --exclude-from /usr/src/exclude --recursive
linux.vanilla-2.6.23rc8-mm1/drivers/ata/pata_hpt37x.c
linux-2.6.23rc8-mm1/drivers/ata/pata_hpt37x.c
--- linux.vanilla-2.6.23rc8-mm1/drivers/ata/pata_hpt37x.c 2007-09-26
16:46:48.0 +0100
+++ linux-2.6.23rc8-mm1/drivers/ata/pata_hpt37x.c 2007-09-18
16:44:32.0 +0100
Wait, I thought you're patching pata_hpt3x2n!
@@ -844,6 +844,46 @@
/* Never went stable */
return 0;
}
+
+static void *hpt_tune_function(struct pci_dev *dev, int dpll, int clock_slot)
+{
+ static const int MHz[4] = { 33, 40, 50, 66 };
+ /*
+* For non UDMA133 capable devices we should
+* use a 50MHz DPLL by choice
+*/
+ unsigned int f_low, f_high;
+ int adjust;
+
+ f_low = (MHz[clock_slot] * 48) / MHz[dpll];
+ f_high = f_low + 2;
+ if (clock_slot 1)
+ f_high += 2;
+ /* Select the DPLL clock. */
+ pci_write_config_byte(dev, 0x5b, 0x21);
+ pci_write_config_dword(dev, 0x5C, (f_high 16) | f_low | 0x100);
Could move the f_low/high writes to hpt37x_calibrate_dpll()...
+ for(adjust = 0; adjust 8; adjust++) {
+ if (hpt37x_calibrate_dpll(dev))
+ break;
+ /* See if it'll settle at a fractionally different clock */
+ if (adjust 1)
+ f_low -= adjust 1;
+ else
+ f_high += adjust 1;
+ pci_write_config_dword(dev, 0x5C, (f_high 16) | f_low |
0x100);
+ }
+ if (adjust == 8) {
+ printk(KERN_WARNING hpt37x: DPLL did not stabilize.\n);
Deserves KERN_ERR. Wait, I remember to have changed it to KERN_ERR.
+ return NULL;
+ }
+ printk(KERN_INFO hpt37x: Bus clock %dMHz, using DPLL.\n, MHz[dpll]);
That won't do -- it reintroduces the DPLL clock ISO bus clock reporting
bug (that I've fixed just recently)!
@@ -944,7 +984,7 @@
u8 mcr1;
u32 freq;
int prefer_dpll = 1;
-
+ int hpt374alt = 0;
unsigned long iobase = pci_resource_start(dev, 4);
const struct hpt_chip *chip_table;
@@ -1046,16 +1086,28 @@
if (chip_table == hpt372a)
outb(0x0e, iobase + 0x9c);
+ /*
+* PLL must be done once
+*/
+
+ if (chip_table == hpt374 PCI_FUNC(dev-devfn) 1) {
+ /* The HPT374 secondary devfn is tuned to 50MHz when we find
+ the primary */
Nah, the reason for that is completely different -- BIOS saves no clock
data for function 1 but calibrates it for 50 MHz anyway, thus making the
driver read already distorted f_CNT...
Not actually dangerous as the value yields 33 MHz PCI clock after clamping but
WTF...
+ port_info = *port;
+ port_info.private_data = hpt37x_timings_50;
+ }
No, this does not seem correct -- the function 1 should be tuned (for the
x86 case where there was no BIOS available to tune it beforehand). I don't see
where the manual tells that the DPLL circuitry is shared. Contrarywise, I'm
seeing it say on p. 3-22:
2. Each function has its own DPLL module, so you need set DPLL clock on every
function
/* Some devices do not let this value be accessed via PCI space
according to the old driver */
-
freq = inl(iobase + 0x90);
if ((freq 12) != 0xABCDE) {
int i;
u8 sr;
u32 total = 0;
-
+
printk(KERN_WARNING pata_hpt37x: BIOS has not set timing
clocks.\n);
+ if (hpt374alt == 1)
+ printk(KERN_ERR pata_hpt37x: No saved frequency on primary
function.\n);
What's so erratic about this? And where hpt374alt is set to 1?
+ private_data = hpt_tune_function(dev, dpll, clock_slot);
+ /* For the HPT374 tune both channels together from fn 0 */
+ if (chip_table == hpt374 !(PCI_FUNC(dev-devfn) 1)) {
+ struct pci_dev *pair = pci_get_slot(dev-bus,
dev-devfn + 1);
+ if (pair != NULL) {
+ hpt_tune_function(pair, dpll, clock_slot);
+ pci_dev_put(pair);
+ }
Ah, I see it now: you've decided to tune 2 functions of HPT374 at once...
}
- if (dpll == 3)
- private_data = (void *)hpt37x_timings_66;
- else
- private_data = (void *)hpt37x_timings_50;
-
-
Re: [PATCH] pata_hpt3x2n: Clean up DPLL stuff
Alan Cox wrote: Nobody commented when I asked for review earlier so it must be ok 8) Lets stick it in -mm to be sure Signed-off-by: Alan Cox [EMAIL PROTECTED] applied to libata-dev.git#for-testing - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] pata_hpt3x2n: Clean up DPLL stuff
Nobody commented when I asked for review earlier so it must be ok 8)
Lets stick it in -mm to be sure
Signed-off-by: Alan Cox [EMAIL PROTECTED]
diff -u --new-file --exclude-from /usr/src/exclude --recursive
linux.vanilla-2.6.23rc8-mm1/drivers/ata/pata_hpt37x.c
linux-2.6.23rc8-mm1/drivers/ata/pata_hpt37x.c
--- linux.vanilla-2.6.23rc8-mm1/drivers/ata/pata_hpt37x.c 2007-09-26
16:46:48.0 +0100
+++ linux-2.6.23rc8-mm1/drivers/ata/pata_hpt37x.c 2007-09-18
16:44:32.0 +0100
@@ -1,5 +1,5 @@
/*
- * Libata driver for the highpoint 37x and 30x UDMA66 ATA controllers.
+ * Libata driver for the highpoint 37x and 30x UDMA ATA controllers.
*
* This driver is heavily based upon:
*
@@ -844,6 +844,46 @@
/* Never went stable */
return 0;
}
+
+static void *hpt_tune_function(struct pci_dev *dev, int dpll, int clock_slot)
+{
+ static const int MHz[4] = { 33, 40, 50, 66 };
+ /*
+* For non UDMA133 capable devices we should
+* use a 50MHz DPLL by choice
+*/
+ unsigned int f_low, f_high;
+ int adjust;
+
+ f_low = (MHz[clock_slot] * 48) / MHz[dpll];
+ f_high = f_low + 2;
+ if (clock_slot 1)
+ f_high += 2;
+ /* Select the DPLL clock. */
+ pci_write_config_byte(dev, 0x5b, 0x21);
+ pci_write_config_dword(dev, 0x5C, (f_high 16) | f_low | 0x100);
+
+ for(adjust = 0; adjust 8; adjust++) {
+ if (hpt37x_calibrate_dpll(dev))
+ break;
+ /* See if it'll settle at a fractionally different clock */
+ if (adjust 1)
+ f_low -= adjust 1;
+ else
+ f_high += adjust 1;
+ pci_write_config_dword(dev, 0x5C, (f_high 16) | f_low |
0x100);
+ }
+ if (adjust == 8) {
+ printk(KERN_WARNING hpt37x: DPLL did not stabilize.\n);
+ return NULL;
+ }
+ printk(KERN_INFO hpt37x: Bus clock %dMHz, using DPLL.\n, MHz[dpll]);
+ if (dpll == 3)
+ return hpt37x_timings_66;
+ else
+ return hpt37x_timings_50;
+}
+
/**
* hpt37x_init_one - Initialise an HPT37X/302
* @dev: PCI device
@@ -944,7 +984,7 @@
u8 mcr1;
u32 freq;
int prefer_dpll = 1;
-
+ int hpt374alt = 0;
unsigned long iobase = pci_resource_start(dev, 4);
const struct hpt_chip *chip_table;
@@ -1046,16 +1086,28 @@
if (chip_table == hpt372a)
outb(0x0e, iobase + 0x9c);
+ /*
+* PLL must be done once
+*/
+
+ if (chip_table == hpt374 PCI_FUNC(dev-devfn) 1) {
+ /* The HPT374 secondary devfn is tuned to 50MHz when we find
+ the primary */
+ port_info = *port;
+ port_info.private_data = hpt37x_timings_50;
+ }
/* Some devices do not let this value be accessed via PCI space
according to the old driver */
-
freq = inl(iobase + 0x90);
if ((freq 12) != 0xABCDE) {
int i;
u8 sr;
u32 total = 0;
-
+
printk(KERN_WARNING pata_hpt37x: BIOS has not set timing
clocks.\n);
+ if (hpt374alt == 1)
+ printk(KERN_ERR pata_hpt37x: No saved frequency on
primary function.\n);
+
/* This is the process the HPT371 BIOS is reported to use */
for(i = 0; i 128; i++) {
@@ -1074,48 +1126,19 @@
clock_slot = hpt37x_clock_slot(freq, chip_table-base);
if (chip_table-clocks[clock_slot] == NULL || prefer_dpll) {
- /*
-* We need to try PLL mode instead
-*
-* For non UDMA133 capable devices we should
-* use a 50MHz DPLL by choice
-*/
- unsigned int f_low, f_high;
- int dpll, adjust;
-
/* Compute DPLL */
- dpll = (port-udma_mask 0xC0) ? 3 : 2;
-
- f_low = (MHz[clock_slot] * 48) / MHz[dpll];
- f_high = f_low + 2;
- if (clock_slot 1)
- f_high += 2;
-
- /* Select the DPLL clock. */
- pci_write_config_byte(dev, 0x5b, 0x21);
- pci_write_config_dword(dev, 0x5C, (f_high 16) | f_low |
0x100);
-
- for(adjust = 0; adjust 8; adjust++) {
- if (hpt37x_calibrate_dpll(dev))
- break;
- /* See if it'll settle at a fractionally different
clock */
- if (adjust 1)
- f_low -= adjust 1;
- else
- f_high += adjust 1;
- pci_write_config_dword(dev, 0x5C, (f_high 16) |
f_low | 0x100);
-
