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