ke 14.5.2025 klo 20.57 Niklas Cassel (cassel@xxxxxxxxxx) kirjoitti: > > Hello Mikko, > > On Mon, May 12, 2025 at 05:16:14PM +0300, Mikko Juhani Korhonen wrote: > > > > > Yes, but for some reason now I get different results. I wonder what's > > different now. The earlier results were on configuration > > sata ports 5,6 -> WDC WD20EFAX-68FB5N0 > > so LPM had to be disabled (it's impossible to run anything) but can't > > remember what kernel version and which variation I used to disable it. > > > > Now with vanilla 6.14.5 and configuration: > > sata ports 5,6 -> WDC WD5000AAKX-001CA0 > > sata ports 3,4 -> WDC WD20EFAX-68FB5N0 > > I get: > > Well we still see: > > > /dev/sda: > > > > ATA device, with non-removable media > > Model Number: WDC WD20EFAX-68FB5N0 > > Firmware Revision: 82.00A82 > > Commands/features: > > Enabled Supported: > > * Host-initiated interface power management > > * Device-initiated interface power management > Hello folks!! > This drive supports both HIPM and DIPM, > and has both enabled. (Which makes sense since lpm-pol 3 includes DIPM.) Yes. > > /dev/sdb: > > > > ATA device, with non-removable media > > Model Number: WDC WD20EFAX-68FB5N0 > > Firmware Revision: 82.00A82 > > Commands/features: > > Enabled Supported: > > * Host-initiated interface power management > > Device-initiated interface power management > > This drive supports HIPM and DIPM, > but only HIPM has been enabled. (Which does not make sense since > lpm-pol 3 includes DIPM...) > > I have no idea what is going on here... It seems I can no longer reproduce this, DIPM is always enabled on this configuration. > > I would add some debug prints around: > https://github.com/torvalds/linux/blob/v6.15-rc6/drivers/ata/libata-eh.c#L3512-L3520 > > To > 1) Make sure that we actually send down the SET FEATURES command for > to the drive. I confirmed this, at least ata_dev_set_feature(dev, SETFEATURES_SATA_ENABLE, SATA_DIPM); gets called. > > 2) Check the return code (err_mask). Error code is zero. However now DIPM is enabled so I guess this is expected. > Did you do something with this drive? > Was this perhaps the drive that got timeout? and for some reason > the timeouts caused DIPM to get disabled? Just reminding that the answer is no :) > Did you get any timeouts on the other drive of the same model (/dev/sda) ? No, as this is configuration where they are on ports 3-4.. > > > /dev/sdc: > > > > ATA device, with non-removable media > > Model Number: WDC WD5000AAKX-001CA0 > > Firmware Revision: 15.01H15 > > Commands/features: > > Enabled Supported: > > * Host-initiated interface power management > > This drive appears to only support HIPM, it does not support DIPM. > It has HIPM enabled, because that is the only feature supported by > the drive. Yes > Looking at what you told before: > > WD5000AAKX-001CA0 works with LPM enabled (lpm-pol 3), on port 5-6. > WD20EFAX-68FB5N0 gets timeouts with LPM enabled (lpm-pol 3), on port 5-6. > > Would again suggest that your controller has issues with DIPM. > The reason why you don't need the "no lpm" quirk on the > "WDC WD5000AAKX-001CA0" drive is most likely because it doesn't support DIPM. Yes most likely. > > Remind me again, without any quirks, do you get timeouts for the > "WDC WD20EFAX-68FB5N0" drive on all drives, or just on port 5-6? Only on ports 5-6, never on 3-4. > > I would really like to know why DIPM is not enabled on your device, > even though it claims support for it, and you are using lpm-pol 3. Yes, but now I can't reproduce it. Any ideas how could we debug this further? I don't have. Or shall we just disable dipm on ports 5-6 along lines of: diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c index 163ac909bd06..65dadac93461 100644 --- a/drivers/ata/ahci.c +++ b/drivers/ata/ahci.c @@ -52,6 +52,7 @@ enum board_ids { board_ahci_ign_iferr, board_ahci_no_debounce_delay, board_ahci_no_msi, + board_ahci_no_dipm_ports45, /* * board_ahci_pcs_quirk is for legacy Intel platforms. * Modern Intel platforms should use board_ahci instead. @@ -152,6 +153,12 @@ static const struct ata_port_info ahci_port_info[] = { .udma_mask = ATA_UDMA6, .port_ops = &ahci_ops, }, + [board_ahci_no_dipm_ports45] = { + .flags = AHCI_FLAG_COMMON | ATA_FLAG_NO_DIPM_PORTS45, + .pio_mask = ATA_PIO4, + .udma_mask = ATA_UDMA6, + .port_ops = &ahci_ops, + }, [board_ahci_no_msi] = { AHCI_HFLAGS (AHCI_HFLAG_NO_MSI), .flags = AHCI_FLAG_COMMON, @@ -466,6 +473,7 @@ static const struct pci_device_id ahci_pci_tbl[] = { { PCI_VDEVICE(AMD, 0x7801), board_ahci_no_debounce_delay }, /* AMD Hudson-2 (AHCI mode) */ { PCI_VDEVICE(AMD, 0x7900), board_ahci }, /* AMD CZ */ { PCI_VDEVICE(AMD, 0x7901), board_ahci }, /* AMD Green Sardine */ + { PCI_VDEVICE(AMD, 0x43EB), board_ahci_no_dipm_ports45 }, /* 500 Series Chipset */ /* AMD is using RAID class only for ahci controllers */ { PCI_VENDOR_ID_AMD, PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_STORAGE_RAID << 8, 0xffffff, board_ahci }, diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c index b990c1ee0b12..de4e058401a5 100644 --- a/drivers/ata/libata-eh.c +++ b/drivers/ata/libata-eh.c @@ -3432,7 +3432,10 @@ static int ata_eh_set_lpm(struct ata_link *link, enum ata_lpm_policy policy, struct ata_eh_context *ehc = &link->eh_context; struct ata_device *dev, *link_dev = NULL, *lpm_dev = NULL; enum ata_lpm_policy old_policy = link->lpm_policy; - bool no_dipm = link->ap->flags & ATA_FLAG_NO_DIPM; + bool no_dipm = link->ap->flags & ATA_FLAG_NO_DIPM || + ( link->ap->flags & ATA_FLAG_NO_DIPM_PORTS45 && + ( link->ap->port_no == 4 || + link->ap->port_no == 5 )); unsigned int hints = ATA_LPM_EMPTY | ATA_LPM_HIPM; unsigned int err_mask; int rc; diff --git a/include/linux/libata.h b/include/linux/libata.h index e5695998acb0..467a91362d2d 100644 --- a/include/linux/libata.h +++ b/include/linux/libata.h @@ -218,6 +218,7 @@ enum { * led */ ATA_FLAG_NO_DIPM = (1 << 23), /* host not happy with DIPM */ ATA_FLAG_SAS_HOST = (1 << 24), /* SAS host */ + ATA_FLAG_NO_DIPM_PORTS45= (1 << 25), /* host ports 45 not happy with DIPM */ /* bits 24:31 of ap->flags are reserved for LLD specific flags */ or for all ports (Damien's patch earlier in this thread) ? I don't know. best regards, Mikko
