2003-07-31 14:56:53

by Marcelo Penna Guerra

[permalink] [raw]
Subject: [PATCH] Merge the changes from siimage 2.4.22-pre9 to 2.6.0-test2

Hi,

I don't know if anyone is working on this, but I merged the changes from
siimage 1.06 present in 2.4.x to 2.6.0-test2. I'm really not very familiar
with the IDE code, so it's probably a good idea if someone could take a look
at it. All I can tell is it's working with the SiI3112A present in my
motherboard and it's a lot more stable than before.

I also added the suggestion from Andre to make the driver not care about cable
detection and found a possible bug in the 2.4.x code.

In 2.4.22-pre9:

if (!is_sata(hwif))
? ?? ?hwif->atapi_dma = 1;

and DMA is not enabled by default.

I think it should be:

if (is_sata(hwif))
? ?? ?hwif->atapi_dma = 1;

With this DMA is enabled by default on my board and works great.

Marcelo Penna Guerra


Attachments:
(No filename) (754.00 B)
siimage-1.06.diff (31.25 kB)
Download all attachments
Subject: Re: [PATCH] Merge the changes from siimage 2.4.22-pre9 to 2.6.0-test2


On Thu, 31 Jul 2003, Marcelo Penna Guerra wrote:

> Hi,

Hi,

> I don't know if anyone is working on this, but I merged the changes from
> siimage 1.06 present in 2.4.x to 2.6.0-test2. I'm really not very familiar
> with the IDE code, so it's probably a good idea if someone could take a look
> at it. All I can tell is it's working with the SiI3112A present in my
> motherboard and it's a lot more stable than before.

Thanks, I'll take a look.

What do you mean by "a lot more stable than before"?

> I also added the suggestion from Andre to make the driver not care about cable
> detection and found a possible bug in the 2.4.x code.

Can you separate your changes from forward-port?

> In 2.4.22-pre9:
>
> if (!is_sata(hwif))
> ? ?? ?hwif->atapi_dma = 1;
>
> and DMA is not enabled by default.

This means that if this is a SATA controller DMA shouldn't be
enabled on ATAPI devices. There are probably some reasons to do this.

> I think it should be:
>
> if (is_sata(hwif))
> ? ?? ?hwif->atapi_dma = 1;
>
> With this DMA is enabled by default on my board and works great.

Are you aware of side effect?
[ Disabling DMA on ATAPI devices on SiI680. ]

Please consult this change with Alan :-).

> Marcelo Penna Guerra

Regards,
--
Bartlomiej


2003-07-31 16:02:59

by Alan

[permalink] [raw]
Subject: Re: [PATCH] Merge the changes from siimage 2.4.22-pre9 to 2.6.0-test2

On Iau, 2003-07-31 at 18:49, Marcelo Penna Guerra wrote:
> if (!is_sata(hwif))
> hwif->atapi_dma = 1;
>
> and DMA is not enabled by default.
>
> I think it should be:
>
> if (is_sata(hwif))
> hwif->atapi_dma = 1;
>
> With this DMA is enabled by default on my board and works great.

You've accidentally worked around a different bug

2003-07-31 17:08:02

by Marcelo Penna Guerra

[permalink] [raw]
Subject: Re: [PATCH] Merge the changes from siimage 2.4.22-pre9 to 2.6.0-test2

On Thursday 31 July 2003 12:25, Bartlomiej Zolnierkiewicz wrote:
> What do you mean by "a lot more stable than before"?

I mean it doesn't crash the system during intensive read/write operations. It
has something to do with limiting the max bk per request to 7.5 (hwif-
>rqsize=15).

> Can you separate your changes from forward-port?

They are really small changes. It's the one I mentioned on the first e-mail
and this one, that was suggested by Andre Hedrick on another post:

if(is_sata(hwif))
{
+ drive->id->hw_config |= 0x6000;
if(strstr(drive->id->model, "Maxtor"))
return 3;
return 4;
}

> Are you aware of side effect?
> [ Disabling DMA on ATAPI devices on SiI680. ]
>
> Please consult this change with Alan :-).

No, I'm not. Sorry. I'm look for more information. But dma is working with my
SiI3112A, everything is stable, so maybe it should be enabled by default for
this chipset.

Also, there's a change missing in this patch. The PCI id for SiI1210SA is
missing in 2.6.x.

#define PCI_DEVICE_ID_SII_680? ?? ?0x0680
+#define PCI_DEVICE_ID_SII_1210SA? ?0x0240
#define PCI_DEVICE_ID_SII_3112? ?? ?0x3112

Anyway, thank you for taking a look at this.

Marcelo Penna Guerra

2003-07-31 17:24:08

by Alan

[permalink] [raw]
Subject: Re: [PATCH] Merge the changes from siimage 2.4.22-pre9 to 2.6.0-test2

On Iau, 2003-07-31 at 20:32, Marcelo Penna Guerra wrote:
> I mean it doesn't crash the system during intensive read/write operations. It
> has something to do with limiting the max bk per request to 7.5 (hwif-
> >rqsize=15).

Already handled in 2.4.22pre-ac

> if(is_sata(hwif))
> {
> + drive->id->hw_config |= 0x6000;
> if(strstr(drive->id->model, "Maxtor"))
> return 3;
> return 4;
> }

We never check the cable bits for SATA so this is a no-op


Subject: Re: [PATCH] Merge the changes from siimage 2.4.22-pre9 to 2.6.0-test2


On Thu, 31 Jul 2003, Marcelo Penna Guerra wrote:

> > Are you aware of side effect?
> > [ Disabling DMA on ATAPI devices on SiI680. ]
> >
> > Please consult this change with Alan :-).
>
> No, I'm not. Sorry. I'm look for more information. But dma is working with
> my SiI3112A, everything is stable, so maybe it should be enabled by default
> for this chipset.

Real bug is in siimage_config_drive_for_dma():

if (!(hwif->atapi_dma))
goto fast_ata_pio;

Remove these two lines and DMA should be auto-enabled.

BTW unfortunately "it works for me" approach doesn't work in IDE ;-).
--
Bartlomiej

2003-08-02 08:39:33

by Andre Hedrick

[permalink] [raw]
Subject: Re: [PATCH] Merge the changes from siimage 2.4.22-pre9 to 2.6.0-test2


There is a future proposal to deal with SATAPI devices.
It has to do with the features register having a bit set to define
direction of IO for the PHY.

Expect a proposal to be public in 2-6 months, my best guess.

Obviously I am talking about things that do not exist today again, sorry.

-a


On Thu, 31 Jul 2003, Bartlomiej Zolnierkiewicz wrote:

>
> On Thu, 31 Jul 2003, Marcelo Penna Guerra wrote:
>
> > Hi,
>
> Hi,
>
> > I don't know if anyone is working on this, but I merged the changes from
> > siimage 1.06 present in 2.4.x to 2.6.0-test2. I'm really not very familiar
> > with the IDE code, so it's probably a good idea if someone could take a look
> > at it. All I can tell is it's working with the SiI3112A present in my
> > motherboard and it's a lot more stable than before.
>
> Thanks, I'll take a look.
>
> What do you mean by "a lot more stable than before"?
>
> > I also added the suggestion from Andre to make the driver not care about cable
> > detection and found a possible bug in the 2.4.x code.
>
> Can you separate your changes from forward-port?
>
> > In 2.4.22-pre9:
> >
> > if (!is_sata(hwif))
> > ? ?? ?hwif->atapi_dma = 1;
> >
> > and DMA is not enabled by default.
>
> This means that if this is a SATA controller DMA shouldn't be
> enabled on ATAPI devices. There are probably some reasons to do this.
>
> > I think it should be:
> >
> > if (is_sata(hwif))
> > ? ?? ?hwif->atapi_dma = 1;
> >
> > With this DMA is enabled by default on my board and works great.
>
> Are you aware of side effect?
> [ Disabling DMA on ATAPI devices on SiI680. ]
>
> Please consult this change with Alan :-).
>
> > Marcelo Penna Guerra
>
> Regards,
> --
> Bartlomiej
>
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>

2003-08-07 09:49:39

by CASINO_E

[permalink] [raw]
Subject: Re: [PATCH] Merge the changes from siimage 2.4.22-pre9 to 2.6.0-test2

Alan Cox <[email protected]> wrote...
> > if(is_sata(hwif))
> > {
> > + drive->id->hw_config |= 0x6000;
> > if(strstr(drive->id->model, "Maxtor"))
> > return 3;
> > return 4;
> > }
>
> We never check the cable bits for SATA so this is a no-op

Alan,

Without this, in 2.4.22-pre10-ac1, ide_ata66_check() in ide-iops.c
returns 1. This causes, for example, that hdparm -i shows only udma
modes 0 to 2 (although the drive has been set to udma6) and refuses to
set any value above udma2 with an ugly "Speed warnings UDMA 3/4/5 is
not functional".

I do not understand the ide subsystem enough (I should say "at all"),
but maybe there are other implications...

Eduardo.



2003-08-07 12:34:34

by Alan

[permalink] [raw]
Subject: Re: [PATCH] Merge the changes from siimage 2.4.22-pre9 to 2.6.0-test2

On Iau, 2003-08-07 at 10:49, CASINO_E wrote:
> > We never check the cable bits for SATA so this is a no-op
>
> Alan,
>
> Without this, in 2.4.22-pre10-ac1, ide_ata66_check() in ide-iops.c
> returns 1. This causes, for example, that hdparm -i shows only udma
> modes 0 to 2 (although the drive has been set to udma6) and refuses to
> set any value above udma2 with an ugly "Speed warnings UDMA 3/4/5 is
> not functional".

Then the bug is that ide_ata66_check is getting called at all in the
SATA controller case

Subject: Re: [PATCH] Merge the changes from siimage 2.4.22-pre9 to 2.6.0-test2


On 7 Aug 2003, Alan Cox wrote:

> On Iau, 2003-08-07 at 10:49, CASINO_E wrote:
> > > We never check the cable bits for SATA so this is a no-op
> >
> > Alan,
> >
> > Without this, in 2.4.22-pre10-ac1, ide_ata66_check() in ide-iops.c
> > returns 1. This causes, for example, that hdparm -i shows only udma
> > modes 0 to 2 (although the drive has been set to udma6) and refuses to
> > set any value above udma2 with an ugly "Speed warnings UDMA 3/4/5 is
> > not functional".
>
> Then the bug is that ide_ata66_check is getting called at all in the
> SATA controller case

drive->sata ?

2003-08-07 16:01:38

by Alan

[permalink] [raw]
Subject: Re: [PATCH] Merge the changes from siimage 2.4.22-pre9 to 2.6.0-test2

On Iau, 2003-08-07 at 14:35, Bartlomiej Zolnierkiewicz wrote:
> > Then the bug is that ide_ata66_check is getting called at all in the
> > SATA controller case
>
> drive->sata ?

drive->sata or hwif->sata. Something like that appears to be needed, or
making ata66_check a HWIF() function so it can be overriden just like
the cable detect half is ?

--
Alan Cox <[email protected]>