pata_hpt366: fix clock detection
PATCH 1/3 pata_hpt366: fix clock detection
pata_hpt366 had its clock detection wrong and detected 25Mhz as 40Mhz and vice-versa. Fix it.
--- drivers/ata/pata_hpt366.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/ata/pata_hpt366.c +2 -2
@@ -382,10 +382,10 @@ static int hpt36x_init_one(struct pci_de/* PCI clocking determines the ATA timing values to use */ /* info_hpt366 is safe against re-entry so we can scribble on it */ switch ((reg1 &0x700 ) >>8 ) { -case 5 : +case 9 : hpriv = &hpt366_40;break ; -case 9 : +case 5 : hpriv = &hpt366_25;break ;default :
PATCH 2/3 pata_hpt366: fix cable detection
hpt366 is strange in that its cable detection register uses the higher bit for master, so the testing should be 2 >> port_no instead of 1 << port_no. Fix it.
Info provided by Alan Cox.
--- drivers/ata/pata_hpt366.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/ata/pata_hpt366.c +1 -1
@@ -215,7 +215,7 @@ static int hpt36x_cable_detect(struct atstruct pci_dev *pdev =to_pci_dev (ap->host->dev);pci_read_config_byte (pdev,0x5A , &ata66); -if (ata66 & (1 << ap->port_no)) +if (ata66 & (2 >> ap->port_no))return ATA_CBL_PATA40 ;return ATA_CBL_PATA80 ; }
PATCH 3/3 pata_hpt366: update mode programming
Update mode programming logic of pata_hpt366 such that it's identical to that of IDE hpt366 driver. The differences were...
* pata_hpt366 used 0xCFFF8FFFF to mask pio modes and 0x3FFFFFFF dma modes. IDE hpt366 uses 0xC1F8FFFF for PIO, 0x303800FF for MWDMA and 0x30070000 for UDMA.
* pata_hpt366 doesn't set 0x08000000 for PIO unless it's already set and always turns it on for MWDMA/UDMA. IDE hpt366 doesn't bother with the bit. It always uses what was there.
* IDE hpt366 always clears 0xC0000000. pata_hpt366 doesn't.
---
Alan, can you please ack this if you think it's okay. Jeff, please don't commit till Alan acks. Thanks.
drivers/ata/pata_hpt366.c | 50 ++++++++++++++++++++++++++-------------------- 1 file changed, 29 insertions(+), 21 deletions(-)
drivers/ata/pata_hpt366.c +29 -21
@@ -188,25 +188,41 @@ static unsigned long hpt366_filter(struc }/** -* hpt36x_find_mode - reset the hpt36x bus +* hpt36x_calc_timing - calculate timing value for transfer mode * @ap: ATA port -* @speed: transfer mode +* @cur_timing: current timing value +* @speed: target transfer mode * * Return the 32bit register programming information for this channel * that matches the speed provided. */ -static u32 hpt36x_find_mode (struct ata_port *ap,int speed) +static u32 hpt36x_calc_timing (struct ata_port *ap,u32 cur_timing,int speed) {struct hpt_clock *clocks = ap->host->private_data; +u32 mask; -while (clocks->xfer_speed) { +if (speed <XFER_MW_DMA_0 ) + mask =0xc1f8ffff ; +else if (speed <XFER_UDMA_0 ) + mask =0x303800ff ; +else + mask =0x30070000 ; + +while (clocks->xfer_speed) {if (clocks->xfer_speed == speed) -return clocks->timing; +break ; clocks++; } -BUG (); -return 0xffffffffU ;/* silence compiler warning */ +if (!clocks->xfer_speed) +BUG (); + +/* +* Combine new mode bits with old config bits and disable +* on-chip PIO FIFO/buffer (and PIO MST mode as well) to avoid +* problems handling I/O errors later. +*/ +return ((cur_timing & ~mask) | (clocks->timing & mask)) & ~0xc0000000 ; }static int hpt36x_cable_detect (struct ata_port *ap)
@@ -232,8 +248,7 @@ static void hpt366_set_piomode(struct at {struct pci_dev *pdev =to_pci_dev (ap->host->dev);u32 addr1, addr2; -u32 reg; -u32 mode; +u32 reg, timing;u8 fast; addr1 =0x40 +4 * (adev->devno +2 * ap->port_no);
@@ -247,11 +262,8 @@ static void hpt366_set_piomode(struct at }pci_read_config_dword (pdev, addr1, ®); - mode =hpt36x_find_mode (ap, adev->pio_mode); - mode &= ~0x8000000 ;/* No FIFO in PIO */ - mode &= ~0x30070000 ;/* Leave config bits alone */ - reg &=0x30070000 ;/* Strip timing bits */ -pci_write_config_dword (pdev, addr1, reg | mode); + timing =hpt36x_calc_timing (ap, reg, adev->pio_mode); +pci_write_config_dword (pdev, addr1, timing); }/**
@@ -267,8 +279,7 @@ static void hpt366_set_dmamode(struct at {struct pci_dev *pdev =to_pci_dev (ap->host->dev);u32 addr1, addr2; -u32 reg; -u32 mode; +u32 reg, timing;u8 fast; addr1 =0x40 +4 * (adev->devno +2 * ap->port_no);
@@ -282,11 +293,8 @@ static void hpt366_set_dmamode(struct at }pci_read_config_dword (pdev, addr1, ®); - mode =hpt36x_find_mode (ap, adev->dma_mode); - mode |=0x8000000 ;/* FIFO in MWDMA or UDMA */ - mode &= ~0xC0000000 ;/* Leave config bits alone */ - reg &=0xC0000000 ;/* Strip timing bits */ -pci_write_config_dword (pdev, addr1, reg | mode); + timing =hpt36x_calc_timing (ap, reg, adev->dma_mode); +pci_write_config_dword (pdev, addr1, timing); }static struct scsi_host_template hpt36x_sht = {
Re: [PATCH #upstream-fixes 2/3] pata_hpt366: fix cable detection
In reply to Tejun HeoHello.
Quoted 3 lines · Tejun Heo
hpt366 is strange in that its cable detection register uses the higher bit for master, so the testing should be 2 >>
port_noinstead of 1 <<
For primary -- the cable bits are per-channel, not per-device.
Quoted 6 lines
port_no. Fix it.Info provided by Alan Cox.
Signed-off-by Tejun Heo <[email protected]>
drivers/ata/pata_hpt366.c static int hpt36x_cable_detect(struct at · line 219
Quoted 11 lines
Index:
work/drivers/ata/pata_hpt366.c
===================================================================drivers/ata/pata_hpt366.c +1
-1@@ -215,7 +215,7 @@ static int hpt36x_cable_detect(struct atstruct pci_dev *pdev =to_pci_dev (ap->host->dev);pci_read_config_byte (pdev,0x5A , &ata66); -if (ata66 & (1 << ap->port_no)) +if (ata66 & (2 >> ap->port_no))
HPT36x are single channel per function, so the shift can be removed.
MBR, Sergei
Re: [PATCH #upstream-fixes 2/3] pata_hpt366: fix cable detection
Quoted 6 lines
drivers/ata/pata_hpt366.c static int hpt36x_cable_detect(struct at · line 218
Quoted 4 lines
drivers/ata/pata_hpt366.c +1
-1pci_read_config_byte (pdev,0x5A , &ata66); -if (ata66 & (1 << ap->port_no)) +if (ata66 & (2 >> ap->port_no))HPT36x are single channel per function, so the shift can be removed.
In the case of two channels are you sure the bits appear as bit 2 in both cases ?
Alan
Re: [PATCH #upstream-fixes 2/3] pata_hpt366: fix cable detection
Hello.
Quoted 8 lines · Alan Cox
Quoted 5 lines
drivers/ata/pata_hpt366.c static int hpt36x_cable_detect(struct at · line 218
Quoted 3 lines
pci_read_config_byte(pdev, 0x5A, &ata66);drivers/ata/pata_hpt366.c +1
-1-if (ata66 & (1 << ap->port_no)) +if (ata66 & (2 >> ap->port_no))HPT36x are single channel per function, so the shift can be removed.
In the case of two channels are you sure the bits appear as bit 2 in both cases ?
Oops, they don't. The cable select register (0x5a) seems to be shared by
both functions (with function 0 representing "primary" and the function 1 "secondary" channel). But in this case, the logic remains broken -- I don't think that function #1 has its ap->port_no set to 1. The IDE driver appears broken as well then. Damn Highpoint for their brain damaged design...
Quoted 1 line
Alan
MBR, Sergei
Re: [PATCH #upstream-fixes 2/3] pata_hpt366: fix cable detection
In reply to Sergei ShtylyovQuoted 5 lines · Hello, I wrote:
Quoted 2 lines
hpt366 is strange in that its cable detection register uses the higher bit for master, so the testing should be 2 >>
port_noinstead of 1 <<For primary -- the cable bits are per-channel, not per-device.
Quoted 7 lines
Quoted 5 lines
port_no. Fix it.Info provided by Alan Cox.
Signed-off-by Tejun Heo <[email protected]>Acked-by Sergei Shtylyov <[email protected]>
I have to NAK this. HPT366 design is just too brain damaged for both IDE
and libata drivers to get it right so far.
Quoted 7 lines
Quoted 7 lines
Index:
work/drivers/ata/pata_hpt366.c
===================================================================drivers/ata/pata_hpt366.c +0
-0@@ -215,7 +215,7 @@ static int hpt36x_cable_detect(struct atstruct pci_dev *pdev =to_pci_dev (ap->host->dev);
Quoted 6 lines
drivers/ata/pata_hpt366.c static int hpt36x_cable_detect(struct at
Quoted 4 lines
drivers/ata/pata_hpt366.c +1
-1pci_read_config_byte (pdev,0x5A , &ata66); -if (ata66 & (1 << ap->port_no)) +if (ata66 & (2 >> ap->port_no))HPT36x are single channel per function, so the shift can be removed.
It should be replaced by more sophisticated logic for function 1 in fact...
MBR, Sergei
Re: [PATCH #upstream-fixes 2/3] pata_hpt366: fix cable detection
Quoted 32 lines · Sergei Shtylyov
Quoted 5 lines · Hello, I wrote:
Quoted 2 lines
hpt366 is strange in that its cable detection register uses the higher bit for master, so the testing should be 2 >>
port_noinstead of 1 <<For primary -- the cable bits are per-channel, not per-device.
Quoted 7 lines
Quoted 5 lines
port_no. Fix it.Info provided by Alan Cox.
Signed-off-by Tejun Heo <[email protected]>Acked-by Sergei Shtylyov <[email protected]>I have to NAK this. HPT366 design is just too brain damaged for both
IDE and libata drivers to get it right so far.Quoted 7 lines
Quoted 7 lines
Index:
work/drivers/ata/pata_hpt366.c
===================================================================drivers/ata/pata_hpt366.c +0
-0@@ -215,7 +215,7 @@ static int hpt36x_cable_detect(struct atstruct pci_dev *pdev =to_pci_dev (ap->host->dev);Quoted 6 lines
drivers/ata/pata_hpt366.c static int hpt36x_cable_detect(struct at
Quoted 4 lines
drivers/ata/pata_hpt366.c +1
-1pci_read_config_byte (pdev,0x5A , &ata66); -if (ata66 & (1 << ap->port_no)) +if (ata66 & (2 >> ap->port_no))HPT36x are single channel per function, so the shift can be removed.
Quoted 2 lines
It should be replaced by more sophisticated logic for function 1 in
fact...
Ah... crap. I don't know anything about this controller. Care to post the correct patch? I'll be happy to test.
Thanks.
Signature · 1 line
tejun
Quoted 13 lines · Tejun Heo
pata_hpt366had its clock detection wrong and detected 25Mhz as 40Mhz and vice-versa. Fix it.Signed-off-by Tejun Heo <[email protected]>--- drivers/ata/pata_hpt366.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)Index:
work/drivers/ata/pata_hpt366.c
===================================================================
drivers/ata/pata_hpt366.c static int hpt36x_init_one(struct pci_de · line 389
Quoted 10 lines
drivers/ata/pata_hpt366.c +2
-2@@ -382,10 +382,10 @@ static int hpt36x_init_one(struct pci_de/* PCI clocking determines the ATA timing values to use */ /* info_hpt366 is safe against re-entry so we can scribble on it */ switch ((reg1 &0x700 ) >>8 ) { -case 5 : +case 9 : hpriv = &hpt366_40;break ; -case 9 : +case 5 : hpriv = &hpt366_25;
applied #upstream-fixes
Avoiding patch #2 in this series b/c the thread seemed to indicate it needed more work
Avoiding patch #3 in this series awaiting Alan's ack (did I miss it?)