Si3112 S-ATA bug preventing use of udma5.

Si3112 S-ATA bug preventing use of udma5.

[Brenden Matthews]

In 2.6.5 (not sure about other versions) there is a bug/problem in the
drivers/ide/pci/siimage.c driver.  I have only tested this with my Si3112
chipset on an ASUS A7N8X-Deluxe mobo.  I did not discover this fix, and
I'll post a url below to the original source.  No idea if this has been
fixed/looked at already, but it definitely needs to be resolved.

hdparm -t speed without fix: 16mb/s
hdparm -t speed with fix: 55mb/s

http://forums.gentoo.org/viewtopic.php?t=111300

Incase the link is down/broken, to fix the bug change line 269 of
drivers/ide/pci/siimage.c from:

        u32 speedt              = 0;

to:
        u16 speedt              = 0;



Regards,
Brenden

Re: Si3112 S-ATA bug preventing use of udma5.

[Benjamin Herrenschmidt]

> Incase the link is down/broken, to fix the bug change line 269 of
> drivers/ide/pci/siimage.c from:
> 
>         u32 speedt              = 0;
> 
> to:
>         u16 speedt              = 0;

> The crux of the problem is that the first arguent to OUTW (out WORD)
> was a doubleword. The arguments were getting all screwed up on the stack.
> The lower order 16-bit were being used in the second argument of OUTW,
> and the upper order word was being used as the whole first argument,
> which was always 0000. So basically the on-disk controller was being
> programmed with erroneous settings. This fixes it and SATA on the SiI3112
> is now good on Linux. Apply this fix to linux/drivers/ide/pci/siimage.c
> and recompile your Kernel for SATA love.

Hrm... that's strange. I'd tend to think it's a bogus definition
of outw on this architecture (x86 ?) instead. an u32 should be casted
down to u16 without problem.


Ben.

Re: Si3112 S-ATA bug preventing use of udma5.

[Roland Dreier]

    Brenden> Incase the link is down/broken, to fix the bug change line 269
    Brenden> of drivers/ide/pci/siimage.c from:

    Brenden> u32 speedt = 0;

    Brenden> to: u16 speedt = 0;

    Brenden> The crux of the problem is that the first arguent to OUTW
    Brenden> (out WORD) was a doubleword. The arguments were getting
    Brenden> all screwed up on the stack.  The lower order 16-bit were
    Brenden> being used in the second argument of OUTW, and the upper
    Brenden> order word was being used as the whole first argument,
    Brenden> which was always 0000.

    Ben> Hrm... that's strange. I'd tend to think it's a bogus
    Ben> definition of outw on this architecture (x86 ?) instead. an
    Ben> u32 should be casted down to u16 without problem.

Assuming I'm reading the siimage.c code correctly, it's calling
default_hwif_mmiops() to set up its OUTW pointer, which sets the
function pointer to call

static void ide_mm_outw (u16 value, unsigned long port)
{
        writew(value, port);
}

In asm-i386/io.h, we have

#define writew(b,addr) (*(volatile unsigned short *) __io_virt(addr) = (b))

Finally, siimage.c does

                hwif->OUTW(speedt, addr);

and speedt is a u32 -- however, as you say, the compiler should just
cast speedt down to a u16.  What am I missing?

 - R.

Re: Si3112 S-ATA bug preventing use of udma5.

[Benjamin Herrenschmidt]

On Sat, 2004-04-24 at 12:30, Roland Dreier wrote:

> Finally, siimage.c does
> 
>                 hwif->OUTW(speedt, addr);
> 
> and speedt is a u32 -- however, as you say, the compiler should just
> cast speedt down to a u16.  What am I missing?

This is just a normal function call, there should be nothing special,
so either you are missing something else or you are suffering from
incorrectly compiled code... I'm no x86 expert so I can't help
analyzing the codegen here though.

Ben.

Re: Si3112 S-ATA bug preventing use of udma5.

[Denis Vlasenko]

On Saturday 24 April 2004 07:31, Benjamin Herrenschmidt wrote:
> On Sat, 2004-04-24 at 12:30, Roland Dreier wrote:
> > Finally, siimage.c does
> >
> >                 hwif->OUTW(speedt, addr);
> >
> > and speedt is a u32 -- however, as you say, the compiler should just
> > cast speedt down to a u16.  What am I missing?
>
> This is just a normal function call, there should be nothing special,
> so either you are missing something else or you are suffering from
> incorrectly compiled code... I'm no x86 expert so I can't help
> analyzing the codegen here though.

place a pair of bogus calls:

void marker(void);
....
	marker();
	hwif->OUTW(speedt, addr);
	marker();
...

and objdump -d resulting .o file.
It will be easy to find corresponding asm fragment.
--
vda