On Mon, Apr 14, 2025 at 12:18 AM Andrius V <vezh...@gmail.com> wrote:
>
> 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).
>
Forgot to mention that there's also 0x5287 PCI ID, which is used in
IDE mode in CE revision.
CE revision uses 0x3349 only on RAID mode.
> The PATA controller appears under PCI ID 0x0571, which is reused by many
> [...]
>
> 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.
Even with the fix below, I still get timeouts sometimes on older
revision chipset, but only with one SSD model out of few drives tested
(and only if nothing attach to port 3).
> 2) Attaching a SATA disk to any second channel port (typically ports 3/4)
> causes PATA drives to fail to attach at all.
>
> [...]
>
> 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.
I also came up with one more option to set atac_set_modes to
sata_setup_channel together with udma option and use null check to set
via_setup_channel otherwise.
@@ -501,12 +515,18 @@ via_chip_map(struct pciide_softc *sc, const
struct pci_attach_args *pa)
no_ideconf = 1;
/* FALLTHROUGH */
case PCI_PRODUCT_VIATECH_CX700_IDE:
+ sc->sc_wdcdev.sc_atac.atac_udma_cap = 6;
+ break;
+ case PCI_PRODUCT_VIATECH_VT8251_SATA:
+ /* FALLTHROUGH */
+ case PCI_PRODUCT_VIATECH_VT8251_SATA_2:
/* FALLTHROUGH */
case PCI_PRODUCT_VIATECH_VT8261_SATA:
/* FALLTHROUGH */
case PCI_PRODUCT_VIATECH_VX900_IDE:
/* FALLTHROUGH */
case PCI_PRODUCT_VIATECH_VX900_RAID:
+ sc->sc_wdcdev.sc_atac.atac_set_modes = sata_setup_channel;
sc->sc_wdcdev.sc_atac.atac_udma_cap = 6;
break;
default:
@@ -662,7 +682,8 @@ via_chip_map(struct pciide_softc *sc, const struct
pci_attach_args *pa)
}
sc->sc_wdcdev.sc_atac.atac_pio_cap = 4;
sc->sc_wdcdev.sc_atac.atac_dma_cap = 2;
- sc->sc_wdcdev.sc_atac.atac_set_modes = via_setup_channel;
+ if (sc->sc_wdcdev.sc_atac.atac_set_modes == NULL)
+ sc->sc_wdcdev.sc_atac.atac_set_modes = via_setup_channel;
sc->sc_wdcdev.sc_atac.atac_channels = sc->wdc_chanarray;
if (single_channel)
Prepared the full patch for review and comments
https://netbsd.org/~andvar/viaide-vt8251.diff
>
> Thank you.
>
> Regards,
> Andrius V
