VT8251 SATA controller improvements for viaide(4)

[Andrius V <vezhlys%gmail.com@localhost>]

Hi,

I am working on improvements to the viaide(4) driver, specifically for VIA
controllers (despite the name, it also supports other vendors like AMD/NVIDIA).

Recent changes include more complete support for CX700/VX800, VT6415 IDE
controller support, basic VT8261 support (unfortunately, the motherboard died,
so further work is unlikely), VT8237S IDE identification, VX900 RAID mode
support, and general VIA RAID improvements.

My final "big" goal is to improve support for the VT8251 chipset controller in
IDE/RAID modes (AHCI mode already works fine). Current support is incomplete and
has several issues:

1) A bogus wd0 attaches if no drive is present on the first SATA port.
2) No drives are detected on SATA ports other than the first.
3) PCI ID 0x3349 is defined as a VT8237R integrated controller, but is actually
   a VT8251 integrated controller.

The VT8251 integrated controller has 2 SATA channels with 2 ports each, and 2
PATA channels. The SATA controller uses PCI ID 0x3349 in IDE/RAID modes
(at least on the motherboard I have). It may use either 0x3349 or 0x6287 in
AHCI mode (I believe this depends on the CD/CE revisions of the southbridge,
visible in chip markings).

The PATA controller appears under PCI ID 0x0571, which is reused by many
chipsets. Identification is done using the ISA bridge:
https://nxr.netbsd.org/xref/src/sys/dev/pci/viaide.c?r=1.94#567

The SATA controller currently attaches using 'via_sata_chip_map_7()`:
https://nxr.netbsd.org/xref/src/sys/dev/pci/viaide.c?r=1.94#345

My proposal is to use 'via_chip_map()` for attaching the VT8251 SATA controller,
similar to recent changes for the CX700/VX800 controller:
https://nxr.netbsd.org/diff/src/sys/dev/pci/viaide.c?r1=%2Fsrc%2Fsys%2Fdev%2Fpci%2Fviaide.c%401.92&r2=%2Fsrc%2Fsys%2Fdev%2Fpci%2Fviaide.c%401.91

That controller also has two SATA ports per channel (but only one channel).
'via_sata_chip_map_7()` works better with older one-port-per-channel controllers
(VT8237R/A/S), and limits to one disk per channel.

However, simply switching to 'via_chip_map()` is not enough. Some problems
remain:

1) Attaching a drive on port 4 (second channel, second port) may cause
   never-ending timeouts.
2) Attaching a SATA disk to any second channel port (typically ports 3/4)
   causes PATA drives to fail to attach at all.

After a fairly long investigation, I found the cause. The last two
'pci_conf_write()` calls in 'via_setup_channel()` are not suitable for SATA
controllers:
https://nxr.netbsd.org/xref/src/sys/dev/pci/viaide.c#922

These two offsets have a different meaning for SATA vs. IDE/PATA controllers and
should not be used for VT8251's SATA controller (and likely also for VX900,
VT8261, and the first SATA channel of VX800). This conclusion is based on
available datasheets for other VIA SATA controllers (datasheet for VT8251 itself
is unavailable).

The problem does not visibly manifest on the first channel, but it affects the
second channel and even the PATA controller. For example, offset 49 on the
VT8237R is described as "PATA/SATA Shared Function", and VT8251 might similar
meaning. Disabling those writes makes all disks on all ports attach properly
in any tested combination.

Now I am considering how best to implement a fix. I see a few options:

1) Add a new flag in 'via_chip_map()` indicating whether it is a SATA
controller,
   and assign the proper setup function:
     sc->sc_wdcdev.sc_atac.atac_set_modes =
         sata_channel ? sata_setup_channel : via_setup_channel;

2) Add a sata_channel flag to the ata_channel struct and skip the
   problematic writes:
     if (!chp->sata_channel) {
         pci_conf_write(...);
         pci_conf_write(...);
     }
   Alternatively, call sata_setup_channel() early inside via_setup_channel():
     if (chp->sata_channel) {
         sata_setup_channel();
         return;
     }

3) Use ide_flags instead to achieve the same goal:
https://nxr.netbsd.org/xref/src/sys/dev/pci/pciidevar.h#180

I'd appreciate any feedback on which direction is preferred or what
are other alternatives.

I can also prepare patches or give additional details if needed.

Thank you.

Regards,
Andrius V