[PATCH] sis5513 fix for SiS962 chipset

[PATCH] sis5513 fix for SiS962 chipset

[Thomas Gleixner]

Hi,

1. If the fake 5513 id bit is not set by the BIOS we must have the 5518
id in the device table.

2. If the register remapping is not set by the BIOS then the enable bit
check in ide_pci_setup_ports will fail. It's safe to switch to the
remapping mode here. Keeping the not remapped mode would need quite big
changes AFAICS.

Works with 2.4.27 and 2.6.8. Please apply.

tglx
________________________________________________________________________
SCO: Linux does not exist
Linux: SCO's claim does not exist
Wallstreet: SCO does not exist for long
________________________________________________________________________

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

diff -urN linux-2.6.8.1.org/drivers/ide/pci/sis5513.c
linux-2.6.8.1/drivers/ide/pci/sis5513.c
--- linux-2.6.8.1.org/drivers/ide/pci/sis5513.c	2004-08-14
12:55:32.000000000 +0200
+++ linux-2.6.8.1/drivers/ide/pci/sis5513.c	2004-09-10
15:50:50.000000000 +0200
@@ -788,6 +788,15 @@
 			if (trueid == 0x5518) {
 				printk(KERN_INFO "SIS5513: SiS 962/963 MuTIOL IDE UDMA133
controller\n");
 				chipset_family = ATA_133;
+				
+				/* Check for 5513 compability mapping 
+				 * We must use this, else the port enabled code will fail,
+				 * as it expects the enablebits at 0x4a. 
+				 */
+				if (!(idemisc & 0x40000000)) {
+					pci_write_config_dword(dev, 0x54, idemisc | 0x40000000);
+					printk (KERN_INFO "SIS5513: Switching to 5513 register
mapping\n");
+				}
 			}
 	}
 
@@ -963,6 +972,7 @@
 
 static struct pci_device_id sis5513_pci_tbl[] = {
 	{ PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_5513, PCI_ANY_ID, PCI_ANY_ID, 0,
0, 0},
+	{ PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_5518, PCI_ANY_ID, PCI_ANY_ID, 0,
0, 0},
 	{ 0, },
 };
 MODULE_DEVICE_TABLE(pci, sis5513_pci_tbl);

Re: [PATCH] sis5513 fix for SiS962 chipset

[Lionel Bouton]

Thomas Gleixner wrote the following on 09/10/2004 04:29 PM :

>Hi,
>
>1. If the fake 5513 id bit is not set by the BIOS we must have the 5518
>id in the device table.
>
>2. If the register remapping is not set by the BIOS then the enable bit
>check in ide_pci_setup_ports will fail. It's safe to switch to the
>remapping mode here. Keeping the not remapped mode would need quite big
>changes AFAICS.
>  
>

I was worried about these when 5518 was introduced but couldn't test on 
the hardware I had at hand and nobody reported hitting this so it went 
under the carpet. What hardware did you use to test ?

-- 
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
 

Re: [PATCH] sis5513 fix for SiS962 chipset

[Thomas Gleixner]

On Fri, 2004-09-10 at 16:53, Lionel Bouton wrote:
> Thomas Gleixner wrote the following on 09/10/2004 04:29 PM :
> 
> >Hi,
> >
> >1. If the fake 5513 id bit is not set by the BIOS we must have the 5518
> >id in the device table.
> >
> >2. If the register remapping is not set by the BIOS then the enable bit
> >check in ide_pci_setup_ports will fail. It's safe to switch to the
> >remapping mode here. Keeping the not remapped mode would need quite big
> >changes AFAICS.
>
> I was worried about these when 5518 was introduced but couldn't test on 
> the hardware I had at hand and nobody reported hitting this so it went 
> under the carpet. What hardware did you use to test ?

Compact PCI ICP-P4 from Inova Computers.

They have not set the 5513 fake id bit and the remapping bit isn't set
either. I did thorough testing both on 2.4 and 2.6 and encountered no
problems yet. I suspect there are more boards around with similar
settings.

tglx

Re: [PATCH] sis5513 fix for SiS962 chipset

[Lionel Bouton]

Thomas Gleixner wrote the following on 09/10/2004 05:06 PM :

>On Fri, 2004-09-10 at 16:53, Lionel Bouton wrote:
>  
>
>> [...]
>>
>> What hardware did you use to test ?
>>    
>>
>
>Compact PCI ICP-P4 from Inova Computers.
>
>  
>

I see it's not really a cutting-edge design (2002). Apparently nobody 
seemed to care about Linux IDE performance before :-|

Re: [PATCH] sis5513 fix for SiS962 chipset

[Thomas Gleixner]

On Fri, 2004-09-10 at 17:31, Lionel Bouton wrote:

> I see it's not really a cutting-edge design (2002). Apparently nobody 
> seemed to care about Linux IDE performance before :-|

The chipset was often used in Notebooks. I just checked one of them and
it has both the 5513 fake bits set, so I never noticed. :) 
I think most developers were following the recommended settings in the
manual, but some obviously did not.

tglx

Re: [PATCH] sis5513 fix for SiS962 chipset

[Bartlomiej Zolnierkiewicz]

On Friday 10 September 2004 17:31, Lionel Bouton wrote:
> Thomas Gleixner wrote the following on 09/10/2004 05:06 PM :
> 
> >On Fri, 2004-09-10 at 16:53, Lionel Bouton wrote:
> >  
> >
> >> [...]
> >>
> >> What hardware did you use to test ?
> >>    
> >>
> >
> >Compact PCI ICP-P4 from Inova Computers.
> >
> >  
> >
> 
> I see it's not really a cutting-edge design (2002). Apparently nobody 
> seemed to care about Linux IDE performance before :-|

Yep. :/  Lionel, can I push this fix upstream?

Could somebody enlighten me what exactly 'remapping mode' does?

Bartlomiej

Re: [PATCH] sis5513 fix for SiS962 chipset

[Thomas Gleixner]

On Fri, 2004-09-10 at 23:21, Bartlomiej Zolnierkiewicz wrote:
> Yep. :/  Lionel, can I push this fix upstream?
> 
> Could somebody enlighten me what exactly 'remapping mode' does?
> 
> Bartlomiej

The 5518 version of the IDE controller has a config bit which (re)maps
the main config registers to 5513 compatible offsets. So the 5513 code
can be used with some minor tweaks. If you don't use the remapping you
have to modify / rewrite quite a portion of the code in order to get it
working with the 5518 chip version.

tglx

Re: [PATCH] sis5513 fix for SiS962 chipset

[Lionel Bouton]

Bartlomiej Zolnierkiewicz wrote the following on 09/10/2004 11:21 PM :

>>I see it's not really a cutting-edge design (2002). Apparently nobody 
>>seemed to care about Linux IDE performance before :-|
>>    
>>
>
>Yep. :/  Lionel, can I push this fix upstream?
>  
>

Please do.

In fact I think I'm only a speed bumper here. There's so little work to 
do on the sis5513 driver now that I don't follow IDE work closely 
anymore and must refresh my memories even for such simple patches. I 
believe there are people around more fit than me for sis5513 maintenance.
With SATA, sis5513.c will soon be considered legacy code. I don't think 
a dedicated maintainer is usefull anymore, seems to me it would be 
better handled by someone with a broader view of IDE chipsets than me.

Any volunteeer ?

Lionel.

Re: [PATCH] sis5513 fix for SiS962 chipset

[Bartlomiej Zolnierkiewicz]

On Saturday 11 September 2004 12:30, Lionel Bouton wrote:
> Bartlomiej Zolnierkiewicz wrote the following on 09/10/2004 11:21 PM :
> 
> >>I see it's not really a cutting-edge design (2002). Apparently nobody 
> >>seemed to care about Linux IDE performance before :-|
> >>    
> >>
> >
> >Yep. :/  Lionel, can I push this fix upstream?
> >  
> >
> 
> Please do.

OK, will do.

> In fact I think I'm only a speed bumper here. There's so little work to 
> do on the sis5513 driver now that I don't follow IDE work closely 
> anymore and must refresh my memories even for such simple patches. I 
> believe there are people around more fit than me for sis5513 maintenance.

I'm not aware of any such person...

> With SATA, sis5513.c will soon be considered legacy code. I don't think 
> a dedicated maintainer is usefull anymore, seems to me it would be 
> better handled by someone with a broader view of IDE chipsets than me.

Dedicated maintainers are very useful even for legacy drivers.
This way the subsystem maintainer doesn't have to worry about
every fscking driver (less bugs, faster development). :-)

If you really want to give up maintainership of sis5513 please
send a patch to Linus+me removing you from MAINTAINERS file.

Anyway, thanks for all your hard work on sis5513 driver.

Re: [PATCH] sis5513 fix for SiS962 chipset

[Lionel Bouton]

Bartlomiej Zolnierkiewicz wrote the following on 09/11/2004 06:02 PM :

>>In fact I think I'm only a speed bumper here. There's so little work to 
>>do on the sis5513 driver now that I don't follow IDE work closely 
>>anymore and must refresh my memories even for such simple patches. I 
>>believe there are people around more fit than me for sis5513 maintenance.
>>    
>>
>
>I'm not aware of any such person...
>
>  
>

Vojtech seemed quite proficient at sis5513.c hacking :-)

>
>Dedicated maintainers are very useful even for legacy drivers.
>This way the subsystem maintainer doesn't have to worry about
>every fscking driver (less bugs, faster development). :-)
>
>  
>

Ok, if you think a dedicated maintainer is the way to go, I'll continue 
to review occasional patches. In fact I enjoy doing this from time to 
time, I was merely worried about not being the best fit for the job anymore.

Regards,

Lionel.