something strange in libata-core.c for kernel 2.6.22-rc3

something strange in libata-core.c for kernel 2.6.22-rc3

[l.genoni]

Mybe I am wrong, but if you are detecting 40-wire cable to set them to 
DMA/33, why the check includes also 80-wire cables configuring them to 
DMA/33 too?

With this patch my nvidia4 IDE controllers detects correctly and configure 
correctly DMA/100 for my HD and DMA/33 for my DVD (the first uses a 
80-wire cable, the second a 40-wire cable).

Am I wrong somewhere?

--- libata-core.c.orig  2007-05-20 14:31:25.000000000 +0200
+++ libata-core.c       2007-05-20 14:34:01.000000000 +0200
@@ -3901,8 +3901,7 @@
                 /* UDMA/44 or higher would be available */
                 if((ap->cbl == ATA_CBL_PATA40) ||
                     (ata_drive_40wire(dev->id) &&
-                    (ap->cbl == ATA_CBL_PATA_UNK ||
-                     ap->cbl == ATA_CBL_PATA80))) {
+                    (ap->cbl == ATA_CBL_PATA_UNK))) {
                         ata_dev_printk(dev, KERN_WARNING,
                                  "limited to UDMA/33 due to 40-wire cable\n");
                         xfer_mask &= ~(0xF8 << ATA_SHIFT_UDMA);

regards

Luigi

Re: something strange in libata-core.c for kernel 2.6.22-rc3

[Tejun Heo]

l.genoni@oltrelinux.com wrote:
> 
> Mybe I am wrong, but if you are detecting 40-wire cable to set them to
> DMA/33, why the check includes also 80-wire cables configuring them to
> DMA/33 too?
> 
> With this patch my nvidia4 IDE controllers detects correctly and
> configure correctly DMA/100 for my HD and DMA/33 for my DVD (the first
> uses a 80-wire cable, the second a 40-wire cable).
> 
> Am I wrong somewhere?

That's the drive side verification of 80c cable check, so if the
condition triggers we downgrade 80c or unknown to 40c.  Cable detection
on nvidia PATA is a disaster.  You're supposed to do some ACPI dancing
and drive side detection is completely bogus.  Eeeek....

Alan, did you have a chance to test the ACPI cable detection?  It just
didn't work when I tried it.  It always returned 80c on my machine.

-- 
tejun

Re: something strange in libata-core.c for kernel 2.6.22-rc3

[Robert Hancock]

l.genoni@oltrelinux.com wrote:
> 
> Mybe I am wrong, but if you are detecting 40-wire cable to set them to 
> DMA/33, why the check includes also 80-wire cables configuring them to 
> DMA/33 too?
> 
> With this patch my nvidia4 IDE controllers detects correctly and 
> configure correctly DMA/100 for my HD and DMA/33 for my DVD (the first 
> uses a 80-wire cable, the second a 40-wire cable).
> 
> Am I wrong somewhere?
> 
> --- libata-core.c.orig  2007-05-20 14:31:25.000000000 +0200
> +++ libata-core.c       2007-05-20 14:34:01.000000000 +0200
> @@ -3901,8 +3901,7 @@
>                 /* UDMA/44 or higher would be available */
>                 if((ap->cbl == ATA_CBL_PATA40) ||
>                     (ata_drive_40wire(dev->id) &&
> -                    (ap->cbl == ATA_CBL_PATA_UNK ||
> -                     ap->cbl == ATA_CBL_PATA80))) {
> +                    (ap->cbl == ATA_CBL_PATA_UNK))) {
>                         ata_dev_printk(dev, KERN_WARNING,
>                                  "limited to UDMA/33 due to 40-wire 
> cable\n");
>                         xfer_mask &= ~(0xF8 << ATA_SHIFT_UDMA);

It only does that for ATA_CBL_PATA80 if ata_drive_40wire returns true, 
which means that the drive is detecting a 40-wire cable on its side.

-- 
Robert Hancock      Saskatoon, SK, Canada
To email, remove "nospam" from hancockr@nospamshaw.ca
Home Page: http://www.roberthancock.com/

Re: something strange in libata-core.c for kernel 2.6.22-rc3

[Robert Hancock]

Tejun Heo wrote:
> l.genoni@oltrelinux.com wrote:
>> Mybe I am wrong, but if you are detecting 40-wire cable to set them to
>> DMA/33, why the check includes also 80-wire cables configuring them to
>> DMA/33 too?
>>
>> With this patch my nvidia4 IDE controllers detects correctly and
>> configure correctly DMA/100 for my HD and DMA/33 for my DVD (the first
>> uses a 80-wire cable, the second a 40-wire cable).
>>
>> Am I wrong somewhere?
> 
> That's the drive side verification of 80c cable check, so if the
> condition triggers we downgrade 80c or unknown to 40c.  Cable detection
> on nvidia PATA is a disaster.  You're supposed to do some ACPI dancing
> and drive side detection is completely bogus.  Eeeek....
> 
> Alan, did you have a chance to test the ACPI cable detection?  It just
> didn't work when I tried it.  It always returned 80c on my machine.

Hopefully when we get that support in and working it will solve a lot of 
these issues (and others, like the laptops that have a short 40-wire 
cable that is good for high UDMA speeds which we presently have to 
hard-code detection for specific models).

-- 
Robert Hancock      Saskatoon, SK, Canada
To email, remove "nospam" from hancockr@nospamshaw.ca
Home Page: http://www.roberthancock.com/

Re: something strange in libata-core.c for kernel 2.6.22-rc3

[Robert Hancock]

Tejun Heo wrote:
> l.genoni@oltrelinux.com wrote:
>> Mybe I am wrong, but if you are detecting 40-wire cable to set them to
>> DMA/33, why the check includes also 80-wire cables configuring them to
>> DMA/33 too?
>>
>> With this patch my nvidia4 IDE controllers detects correctly and
>> configure correctly DMA/100 for my HD and DMA/33 for my DVD (the first
>> uses a 80-wire cable, the second a 40-wire cable).
>>
>> Am I wrong somewhere?
> 
> That's the drive side verification of 80c cable check, so if the
> condition triggers we downgrade 80c or unknown to 40c.  Cable detection
> on nvidia PATA is a disaster.  You're supposed to do some ACPI dancing
> and drive side detection is completely bogus.  Eeeek....
> 
> Alan, did you have a chance to test the ACPI cable detection?  It just
> didn't work when I tried it.  It always returned 80c on my machine.

On a whim I started poking around in the disassembled ACPI DSDT code for 
my Asus A8N-SLI Deluxe board, which is one of these chipsets. The 
original thought was that the STM/GTM trick on these chipsets is 
supposed to allow us to determine what modes we should use based on what 
modes it sets up appropriately. Unfortunately, unless I'm missing 
something in the AML (which is possible) it doesn't seem like there is 
any validation being done on the settings passed in. The settings appear 
to essentially just get programmed into the controller when STM is 
called and read back on GTM. The only complication is some logic on the 
_PTS method (Prepare to Sleep) which stores the current settings into 
some variables, and in STM, if a flag was set by the _PTS method, the 
previous settings for all registers are stored back first before writing 
the requested values into the correct places.

So in this case, obviously the approach used by pata_acpi, etc. won't 
work for cable detection. Whatever magic register on the chipset 
contains the cable detect value, the AML doesn't seem to be accessing 
it. The ACPI spec doesn't really give any guarantee that the "try STM on 
all possible modes" trick will work either, since there seems to be no 
mention of the AML being required to validate the mode and the STM 
function has no return value to indicate failure.

I guess this means that what we have to do is trust that the BIOS set up 
a reasonable mode and base the cable detect on that (either by reading 
back the boot-up controller registers, or by calling GTM). I imagine 
this is what the Windows default IDE driver is doing (just using the 
boot-up mode and feeding it back using GTM/STM on suspend/resume cycles).

-- 
Robert Hancock      Saskatoon, SK, Canada
To email, remove "nospam" from hancockr@nospamshaw.ca
Home Page: http://www.roberthancock.com/

Re: something strange in libata-core.c for kernel 2.6.22-rc3

[Tejun Heo]

Robert Hancock wrote:
>> Alan, did you have a chance to test the ACPI cable detection?  It just
>> didn't work when I tried it.  It always returned 80c on my machine.
> 
> On a whim I started poking around in the disassembled ACPI DSDT code for
> my Asus A8N-SLI Deluxe board, which is one of these chipsets. The
> original thought was that the STM/GTM trick on these chipsets is
> supposed to allow us to determine what modes we should use based on what
> modes it sets up appropriately. Unfortunately, unless I'm missing
> something in the AML (which is possible) it doesn't seem like there is
> any validation being done on the settings passed in. The settings appear
> to essentially just get programmed into the controller when STM is
> called and read back on GTM.

Yeah, that's consistent to what I've seen on my machine which is a
variant of A8N.  No matter what value I through at _STM, _GTM just
echoed the result thus always leading to 80c configuration.

> I guess this means that what we have to do is trust that the BIOS set up
> a reasonable mode and base the cable detect on that (either by reading
> back the boot-up controller registers, or by calling GTM). I imagine
> this is what the Windows default IDE driver is doing (just using the
> boot-up mode and feeding it back using GTM/STM on suspend/resume cycles).

Alan, what do you think?

-- 
tejun

Re: something strange in libata-core.c for kernel 2.6.22-rc3

[Alan Cox]

> Yeah, that's consistent to what I've seen on my machine which is a
> variant of A8N.  No matter what value I through at _STM, _GTM just
> echoed the result thus always leading to 80c configuration.
> 
> > I guess this means that what we have to do is trust that the BIOS set up
> > a reasonable mode and base the cable detect on that (either by reading
> > back the boot-up controller registers, or by calling GTM). I imagine
> > this is what the Windows default IDE driver is doing (just using the
> > boot-up mode and feeding it back using GTM/STM on suspend/resume cycles).
> 
> Alan, what do you think?

Interesting, sounds like it is still useful rather than just reading the
registers as the GTM/STM seem to survive resume cycles which drive config
may not (eg if the driver is loaded after a s2ram/resume.

If it just echoes back we should also be able to detect this by using
knowingly invalid values.

Alan

Re: something strange in libata-core.c for kernel 2.6.22-rc3

[Robert Hancock]

Alan Cox wrote:
>> Yeah, that's consistent to what I've seen on my machine which is a
>> variant of A8N.  No matter what value I through at _STM, _GTM just
>> echoed the result thus always leading to 80c configuration.
>>
>>> I guess this means that what we have to do is trust that the BIOS set up
>>> a reasonable mode and base the cable detect on that (either by reading
>>> back the boot-up controller registers, or by calling GTM). I imagine
>>> this is what the Windows default IDE driver is doing (just using the
>>> boot-up mode and feeding it back using GTM/STM on suspend/resume cycles).
>> Alan, what do you think?
> 
> Interesting, sounds like it is still useful rather than just reading the
> registers as the GTM/STM seem to survive resume cycles which drive config
> may not (eg if the driver is loaded after a s2ram/resume.

I don't think that case is handled in this BIOS anyway - if you call GTM 
after resume without previously calling STM, it's just going to read 
whatever random values are in the controller and give you timings based 
on that, which presumably will be junk.

It looks like the main purpose for what it's doing with saving those 
registers in the _PTS method is to save and restore a couple of 
controller registers called ID20 (PCI config space offset 0x50, 16 bits) 
and ID22 (PCI config space offset 0x5C, 32 bits) which aren't otherwise 
used in the AML. According to pata_amd, for the AMD IDE interface the 
former is some reserved bits as well as the cable detect bits, while the 
latter is the cycle time and address setup time register. Presumably 
those aren't really the cable detect bits though, since the detection 
based on those bits in pata_amd doesn't really work..

> If it just echoes back we should also be able to detect this by using
> knowingly invalid values.

Well, this implementation doesn't purely echo back the same values, it 
echoes back values derived from what the controller was actually set to, 
so I imagine if you put in something ridiculous it would come back with 
the closest possible mode that it was set to (PIO mode 0, etc.)

I suspect the implementation we would need to use (which doesn't depend 
on anything not given in the spec) would be:

-On driver load, execute _GTM to get the timing mode the BIOS had set. 
Assume this represents the fastest modes the controller supports, and 
set cable detect based on whether it includes UDMA modes > 2.

-If we decide to set a slower mode (speed down due to errors, etc.), set 
it using _STM and then read back the actual values that were set using 
_GTM (for possible use in suspend/resume).

-On resume after suspend, re-set the last mode using _STM followed by 
executing _GTF and running those commands.

This won't handle the case where the driver is loaded after the system 
was already suspended to RAM and resumed, however I don't know exactly 
how one could handle that in this situation..

-- 
Robert Hancock      Saskatoon, SK, Canada
To email, remove "nospam" from hancockr@nospamshaw.ca
Home Page: http://www.roberthancock.com/

Re: something strange in libata-core.c for kernel 2.6.22-rc3

[Jeff Garzik]

Stupid BIOS crap like that is largely the reason why I am reluctant to 
merge pata_acpi.  There is a lot of knowledge like this yet to be 
discovered.

I would feel far more comfortable disassembling the AML like you are 
doing...

	Jeff