2003-05-12 13:48:15

by Lionel Bouton

[permalink] [raw]
Subject: [SiS IDE patch] various fixes

diff -urN -X dontdiff --exclude=tmp_include_depends linux-2.4.21-rc2-ac1/drivers/ide/pci/sis5513.c linux-2.4.21-rc2-ac1-sis5513/drivers/ide/pci/sis5513.c
--- linux-2.4.21-rc2-ac1/drivers/ide/pci/sis5513.c 2003-05-12 00:10:19.000000000 +0200
+++ linux-2.4.21-rc2-ac1-sis5513/drivers/ide/pci/sis5513.c 2003-05-12 15:49:54.000000000 +0200
@@ -161,9 +161,10 @@
{ "SiS748", PCI_DEVICE_ID_SI_748, ATA_133, 0 },
{ "SiS746", PCI_DEVICE_ID_SI_746, ATA_133, 0 },
{ "SiS745", PCI_DEVICE_ID_SI_745, ATA_133, 0 },
- { "SiS740", PCI_DEVICE_ID_SI_740, ATA_100, 0 },
+ { "SiS740", PCI_DEVICE_ID_SI_740, ATA_133, 0 },
{ "SiS735", PCI_DEVICE_ID_SI_735, ATA_100, SIS5513_LATENCY },
{ "SiS730", PCI_DEVICE_ID_SI_730, ATA_100a, SIS5513_LATENCY },
+ { "SiS655", PCI_DEVICE_ID_SI_655, ATA_133, 0 },
{ "SiS652", PCI_DEVICE_ID_SI_652, ATA_133, 0 },
{ "SiS651", PCI_DEVICE_ID_SI_651, ATA_133, 0 },
{ "SiS650", PCI_DEVICE_ID_SI_650, ATA_133, 0 },
@@ -257,8 +258,8 @@
static char* chipset_capability[] = {
"ATA", "ATA 16",
"ATA 33", "ATA 66",
- "ATA 100", "ATA 100",
- "ATA 133", "ATA 133"
+ "ATA 100 (1st gen)", "ATA 100 (2nd gen)",
+ "ATA 133 (1st gen)", "ATA 133 (2nd gen)"
};

#if defined(DISPLAY_SIS_TIMINGS) && defined(CONFIG_PROC_FS)
@@ -331,8 +332,8 @@
// Configuration space remapped to 0x70
drive_pci = 0x70;
}
- pci_read_config_dword(bmide_dev, (unsigned long)drive_pci+8*pos, &regdw0);
- pci_read_config_dword(bmide_dev, (unsigned long)drive_pci+8*pos+4, &regdw1);
+ pci_read_config_dword(bmide_dev, (unsigned long)drive_pci+4*pos, &regdw0);
+ pci_read_config_dword(bmide_dev, (unsigned long)drive_pci+4*pos+8, &regdw1);
p += sprintf(p, "Drive %d:\n", pos);
}

@@ -357,8 +358,7 @@
case ATA_100a: p += sprintf(p, cycle_time[(reg01 & 0x70) >> 4]); break;
case ATA_100:
case ATA_133a: p += sprintf(p, cycle_time[reg01 & 0x0F]); break;
- case ATA_133:
- default: p += sprintf(p, "133+ ?"); break;
+ default: p += sprintf(p, "?"); break;
}
p += sprintf(p, " \t UDMA Cycle Time ");
switch(chipset_family) {
@@ -367,42 +367,39 @@
case ATA_100a: p += sprintf(p, cycle_time[(reg11 & 0x70) >> 4]); break;
case ATA_100:
case ATA_133a: p += sprintf(p, cycle_time[reg11 & 0x0F]); break;
- case ATA_133:
- default: p += sprintf(p, "133+ ?"); break;
+ default: p += sprintf(p, "?"); break;
}
p += sprintf(p, "\n");
}

+ if (chipset_family < ATA_133) { /* else case TODO */
/* Data Active */
- p += sprintf(p, " Data Active Time ");
- switch(chipset_family) {
- case ATA_00:
- case ATA_16: /* confirmed */
- case ATA_33:
- case ATA_66:
- case ATA_100a: p += sprintf(p, active_time[reg01 & 0x07]); break;
- case ATA_100:
- case ATA_133a: p += sprintf(p, active_time[(reg00 & 0x70) >> 4]); break;
- case ATA_133:
- default: p += sprintf(p, "133+ ?"); break;
- }
- p += sprintf(p, " \t Data Active Time ");
- switch(chipset_family) {
- case ATA_00:
- case ATA_16:
- case ATA_33:
- case ATA_66:
- case ATA_100a: p += sprintf(p, active_time[reg11 & 0x07]); break;
- case ATA_100:
- case ATA_133a: p += sprintf(p, active_time[(reg10 & 0x70) >> 4]); break;
- case ATA_133:
- default: p += sprintf(p, "133+ ?"); break;
- }
- p += sprintf(p, "\n");
+ p += sprintf(p, " Data Active Time ");
+ switch(chipset_family) {
+ case ATA_00:
+ case ATA_16: /* confirmed */
+ case ATA_33:
+ case ATA_66:
+ case ATA_100a: p += sprintf(p, active_time[reg01 & 0x07]); break;
+ case ATA_100:
+ case ATA_133a: p += sprintf(p, active_time[(reg00 & 0x70) >> 4]); break;
+ default: p += sprintf(p, "?"); break;
+ }
+ p += sprintf(p, " \t Data Active Time ");
+ switch(chipset_family) {
+ case ATA_00:
+ case ATA_16:
+ case ATA_33:
+ case ATA_66:
+ case ATA_100a: p += sprintf(p, active_time[reg11 & 0x07]); break;
+ case ATA_100:
+ case ATA_133a: p += sprintf(p, active_time[(reg10 & 0x70) >> 4]); break;
+ default: p += sprintf(p, "?"); break;
+ }
+ p += sprintf(p, "\n");

/* Data Recovery */
/* warning: may need (reg&0x07) for pre ATA66 chips */
- if (chipset_family < ATA_133) {
p += sprintf(p, " Data Recovery Time %s \t Data Recovery Time %s\n",
recovery_time[reg00 & 0x0f], recovery_time[reg10 & 0x0f]);
}
@@ -430,7 +427,6 @@

p += sprintf(p, "\nSiS 5513 ");
switch(chipset_family) {
- case ATA_00: p += sprintf(p, "Unknown???"); break;
case ATA_16: p += sprintf(p, "DMA 16"); break;
case ATA_33: p += sprintf(p, "Ultra 33"); break;
case ATA_66: p += sprintf(p, "Ultra 66"); break;
@@ -867,6 +863,19 @@
return sis5513_config_drive_xfer_rate(drive);
}

+/* Helper function used at init time
+ * returns a PCI device revision ID
+ * (used to detect different IDE controller versions)
+ */
+static u8 __init devfn_rev(int device, int function)
+{
+ u8 revision;
+ /* Find device */
+ struct pci_dev* dev = pci_find_slot(0,PCI_DEVFN(device,function));
+ pci_read_config_byte(dev, PCI_REVISION_ID, &revision);
+ return revision;
+}
+
/* Chip detection and general config */
static unsigned int __init init_chipset_sis5513 (struct pci_dev *dev, const char *name)
{
@@ -887,26 +896,24 @@
/* check 100/133 chipset family */
if (chipset_family == ATA_133) {
u32 reg54h;
- u16 reg02h;
+ u16 devid;
pci_read_config_dword(dev, 0x54, &reg54h);
+ /* SiS962 and above report 0x5518 dev id if high bit is cleared */
pci_write_config_dword(dev, 0x54, (reg54h & 0x7fffffff));
- pci_read_config_word(dev, 0x02, &reg02h);
+ pci_read_config_word(dev, 0x02, &devid);
+ /* restore register 0x54 */
pci_write_config_dword(dev, 0x54, reg54h);
+
/* devid 5518 here means SiS962 or later
- which supports ATA133 */
- if (reg02h != 0x5518) {
+ which supports ATA133.
+ These are refered by chipset_family = ATA133
+ */
+ if (devid != 0x5518) {
u8 reg49h;
- unsigned long sbrev;
/* SiS961 family */
-
- /*
- * FIXME !!! GAK!!!!!!!!!! PCI DIRECT POKING
- */
- outl(0x80001008, 0x0cf8);
- sbrev = inl(0x0cfc);
-
pci_read_config_byte(dev, 0x49, &reg49h);
- if (((sbrev & 0xff) == 0x10) && (reg49h & 0x80))
+ /* check isa bridge device rev id */
+ if (((devfn_rev(2,0) & 0xff) == 0x10) && (reg49h & 0x80))
chipset_family = ATA_133a;
else
chipset_family = ATA_100;
@@ -924,6 +931,14 @@
u8 latency = (chipset_family == ATA_100)? 0x80 : 0x10; /* Lacking specs */
pci_write_config_byte(dev, PCI_LATENCY_TIMER, latency);
}
+
+ /* Special case for SiS630 : 630S/ET is ATA_100a */
+ if (SiSHostChipInfo[i].host_id == PCI_DEVICE_ID_SI_630) {
+ /* check host device rev id */
+ if (devfn_rev(0,0) >= 0x30) {
+ chipset_family = ATA_100a;
+ }
+ }
}

/* Make general config ops here
diff -urN -X dontdiff --exclude=tmp_include_depends linux-2.4.21-rc2-ac1/include/linux/pci_ids.h linux-2.4.21-rc2-ac1-sis5513/include/linux/pci_ids.h
--- linux-2.4.21-rc2-ac1/include/linux/pci_ids.h 2003-05-12 00:12:32.000000000 +0200
+++ linux-2.4.21-rc2-ac1-sis5513/include/linux/pci_ids.h 2003-05-12 00:35:40.000000000 +0200
@@ -501,6 +501,7 @@
#define PCI_DEVICE_ID_SI_650 0x0650
#define PCI_DEVICE_ID_SI_651 0x0651
#define PCI_DEVICE_ID_SI_652 0x0652
+#define PCI_DEVICE_ID_SI_655 0x0655
#define PCI_DEVICE_ID_SI_730 0x0730
#define PCI_DEVICE_ID_SI_630_VGA 0x6300
#define PCI_DEVICE_ID_SI_730_VGA 0x7300


Attachments:
sis.patch.20030512_1ac (7.23 kB)

2003-05-15 11:59:28

by Marcin Wiacek

[permalink] [raw]
Subject: RE: [SiS IDE patch] various fixes

Hi,

> here's a patch against 2.4.21-rc2-ac1 (applies cleanly on 2.4.21-rc2
> too) which brings the following to the
> SiS IDE driver :
> - support for SiS655,
> - support for SiS630S/ET UDMA5 mode,
> - corrected /proc/ide/sis output for ATA133 chipsets (drives'
> positions
> were swapped),
> - use of pci_read_config instead of direct pci poking for
> SiS962+ detection,
> I don't have the hardware for testing any of these changes, so SiS
> owners, please test and report if you :
> - use an ATA133 chipset,
> - use a SiS630 (original, or SiS630S/ET).

I wanted to check if very carefully (Sis 655). DMA seems to be enabled
for HDD. But still there are some problems OR with 2.4.21 at all (I
quess something wrong generally in IDE support) OR still with Sis
driver. Why ? It seems, that DMA is NOT enabled for DVD and CDRW. I hear
it - they work very slow. Also hdparm shows errors, when get info about
them. In attachments examples.

Also there are some problems with Silicon Image driver (see
"DriveReadySeekCompleteError" in syslog). Because of it rather there is
problem with 2.4.21.

What can be done with it ? (both with enabling DMA for non HDD devices
and errors). If any of developers need something, please tell me.

Pozdrowienia/Best Regards
--
Marcin Wiacek (http://www.mwiacek.com [email protected])


Attachments:
LOGS.ZIP (2.10 kB)

2003-05-15 12:39:30

by Lionel Bouton

[permalink] [raw]
Subject: Re: [SiS IDE patch] various fixes

Marcin Wiacek wrote:

>I wanted to check if very carefully (Sis 655). DMA seems to be enabled
>for HDD. But still there are some problems OR with 2.4.21 at all (I
>quess something wrong generally in IDE support) OR still with Sis
>driver. Why ? It seems, that DMA is NOT enabled for DVD and CDRW. I hear
>it - they work very slow. Also hdparm shows errors, when get info about
>them. In attachments examples.
>

Please, send us your exact kernel version and the .config file used to
make your kernel.

--
Lionel Bouton - inet6
---------------------------------------------------------------------
o Siege social: 51, rue de Verdun - 92158 Suresnes
/ _ __ _ Acces Bureaux: 33 rue Benoit Malon - 92150 Suresnes
/ /\ /_ / /_ France
\/ \/_ / /_/ Tel. +33 (0) 1 41 44 85 36
Inetsys S.A. Fax +33 (0) 1 46 97 20 10



2003-05-15 15:07:21

by Marcin Wiacek

[permalink] [raw]
Subject: RE: [SiS IDE patch] various fixes


Hi,

> >I wanted to check if very carefully (Sis 655). DMA seems to
> be enabled
> >for HDD. But still there are some problems OR with 2.4.21 at all (I
> >quess something wrong generally in IDE support) OR still with Sis
> >driver. Why ? It seems, that DMA is NOT enabled for DVD and
> CDRW. I hear
> >it - they work very slow. Also hdparm shows errors, when get
> info about
> >them. In attachments examples.
> Please, send us your exact kernel version and the .config
> file used to
> make your kernel.

Config in attachment. Kernel is from Mandrake 9.1 with Your patch added
(version in Makefile in attachment). I had similiar problems with CDRW,
DVD and kernel 2.4.20 patched to 2.4.21 pre 7 or similiar (on my old
PC).

Pozdrowienia/Best Regards
--
Marcin Wiacek (http://www.mwiacek.com [email protected])


Attachments:
CONFIG.ZIP (12.14 kB)

2003-05-15 15:40:43

by Lionel Bouton

[permalink] [raw]
Subject: Re: [SiS IDE patch] various fixes

Marcin Wiacek wrote:

>I wanted to check if very carefully (Sis 655). DMA seems to be enabled
>for HDD. But still there are some problems OR with 2.4.21 at all (I
>quess something wrong generally in IDE support) OR still with Sis
>driver. Why ? It seems, that DMA is NOT enabled for DVD and CDRW. I hear
>it - they work very slow. Also hdparm shows errors, when get info about
>them. In attachments examples.
>

These errors are normal (the requested info is meaningless on CD/DVD
drives).

From the .config you sent, " CONFIG_IDEDMA_ONLYDISK is not set ", so
dma being disabled for your CD/DVD drives may be related to them having
bugs in DMA mode, the kernel IDE subsystem disables DMA or higher dma
modes with peripherals it knows to be buggy. Please send the output of
"hdparm -i /dev/<yourdrive>"


>
>Also there are some problems with Silicon Image driver (see
>"DriveReadySeekCompleteError" in syslog). Because of it rather there is
>problem with 2.4.21.
>
>

IIRC this message is in fact harmless, do you have any other problem
with the harddrive ?

Regards,

--
Lionel Bouton - inet6
---------------------------------------------------------------------
o Siege social: 51, rue de Verdun - 92158 Suresnes
/ _ __ _ Acces Bureaux: 33 rue Benoit Malon - 92150 Suresnes
/ /\ /_ / /_ France
\/ \/_ / /_/ Tel. +33 (0) 1 41 44 85 36
Inetsys S.A. Fax +33 (0) 1 46 97 20 10



2003-05-15 21:33:57

by Marcin Wiacek

[permalink] [raw]
Subject: RE: [SiS IDE patch] various fixes


Hi,

> These errors are normal (the requested info is meaningless on CD/DVD
> drives).
[...]
> From the .config you sent, " CONFIG_IDEDMA_ONLYDISK is not set ", so
> dma being disabled for your CD/DVD drives may be related to
> them having
> bugs in DMA mode, the kernel IDE subsystem disables DMA or higher dma
> modes with peripherals it knows to be buggy. Please send the
> output of
> "hdparm -i /dev/<yourdrive>"
[....]
> IIRC this message is in fact harmless, do you have any other problem
> with the harddrive ?

With all respect. I'm NOT newbie (also in Linux). 2.4.21 is first kernel
version, where I have problems with IDE subsystem (both on old and new
PC).

When I enabled both standard IDE and SiS IDE drivers, errors
("DriveReadySeekCompleteError" and similiar) don't allow system to
start.

When I use SIS IDE only, DMA is not enabled by default (for UDMA 33
devices - earlier in 2.4.20 and earlier IT WAS DONE).

Today I bough new IDE card with Silicon Image and Silicon Image IDE
driver also shows errors with HDD (which is 100% OK).

Please treat it seriously. I know, how to configure Linux kernel - I
make it from few years. In 2.4.21 there is REALLY something wrong.
Please do not say, that errors are normal - earlier they didn't exist
and weren't shown anywhere (also in hdparm). I'm here and can give all
required info for tracking bug. Please treat it seriously. When kernel
shows errors in some parameters configurations, doesn't allow to use
some combination of config parameters or when doesn't enable DMA, there
is something wrong.

In attachment config info from hdparm.

Pozdrowienia/Best Regards
--
Marcin Wiacek (http://www.mwiacek.com [email protected])


Attachments:
ID.ZIP (1.78 kB)

2003-05-16 10:48:10

by Vojtech Pavlik

[permalink] [raw]
Subject: Re: [SiS IDE patch] various fixes

On Mon, May 12, 2003 at 04:00:47PM +0200, Lionel Bouton wrote:

> here's a patch against 2.4.21-rc2-ac1 (applies cleanly on 2.4.21-rc2
> too) which brings the following to the
> SiS IDE driver :
>
> - support for SiS655,
> - support for SiS630S/ET UDMA5 mode,
> - corrected /proc/ide/sis output for ATA133 chipsets (drives' positions
> were swapped),
> - use of pci_read_config instead of direct pci poking for SiS962+ detection,
>
> I don't have the hardware for testing any of these changes, so SiS
> owners, please test and report if you :
>
> - use an ATA133 chipset,
> - use a SiS630 (original, or SiS630S/ET).

Hi!

I also have updated the sis5513 driver, to work correctly with
SiS961/961B/962/963 IDE controllers. Patch attached, against current
Marcelo tree. It does a little of cleanup (removing debug code).

Would you merge the changes or should I?

--
Vojtech Pavlik
SuSE Labs, SuSE CR


Attachments:
(No filename) (906.00 B)
sis-marcelo.diff (22.76 kB)
Download all attachments