Skip to contentSkip to conversation

ata: libata-core: Fix ata_pci_shutdown_one()

PATCH In review Damien Le Moal 3 participants 5 messages Nov 6, 2023 – Nov 8, 2023 libata

lore
Share & tools
lore

Damien Le Moal

author maintainer
#0 lore raw

PATCH 1/1 ata: libata-core: Fix ata_pci_shutdown_one()

Commit 5b6fba546da2 ("ata: libata-core: Detach a port devices on shutdown") modified the function ata_pci_shutdown_one() to stop (suspend) devices attached to the ports of a PCI AHCI adapter to ensure that drives are spun down before shutting down a system. However, this is done only for PCI adapters and not for other types of adapters. This limitation was addressed with commit 24eca2dce0f8 ("scsi: sd: Introduce manage_shutdown device flag"). With this, all ATA disks are spun down on system shutdown.

This reverts commit 5b6fba546da2 as the change introduced is now useless.

Signed-off-by Damien Le Moal <[email protected]>
---
 drivers/ata/libata-core.c | 16 +---------------
 1 file changed, 1 insertion(+), 15 deletions(-)
drivers/ata/libata-core.c +1 -15
index 6fb4e8dc8c3c..09ed67772fae 100644
@@ -6180,24 +6180,10 @@ EXPORT_SYMBOL_GPL(ata_pci_remove_one);
 void ata_pci_shutdown_one(struct pci_dev *pdev)
 {
 	struct ata_host *host = pci_get_drvdata(pdev);
-	struct ata_port *ap;
-	unsigned long flags;
 	int i;
 
-	/* Tell EH to disable all devices */
-	for (i = 0; i < host->n_ports; i++) {
-		ap = host->ports[i];
-		spin_lock_irqsave(ap->lock, flags);
-		ap->pflags |= ATA_PFLAG_UNLOADING;
-		ata_port_schedule_eh(ap);
-		spin_unlock_irqrestore(ap->lock, flags);
-	}
-
 	for (i = 0; i < host->n_ports; i++) {
-		ap = host->ports[i];
-
-		/* Wait for EH to complete before freezing the port */
-		ata_port_wait_eh(ap);
+		struct ata_port *ap = host->ports[i];
 
 		ap->pflags |= ATA_PFLAG_FROZEN;
Signature · 1 line
2.41.0
#1 lore raw

Hello!

Quoted 11 lines · Damien Le Moal

Commit 5b6fba546da2 ("ata: libata-core: Detach a port devices on shutdown") modified the function ata_pci_shutdown_one() to stop (suspend) devices attached to the ports of a PCI AHCI adapter to ensure that drives are spun down before shutting down a system. However, this is done only for PCI adapters and not for other types of adapters. This limitation was addressed with commit 24eca2dce0f8 ("scsi: sd: Introduce manage_shutdown device flag"). With this, all ATA disks are spun down on system shutdown.

This reverts commit 5b6fba546da2 as the change introduced is now useless.

You didn't use 'git revert'?

Quoted 1 line
Signed-off-by Damien Le Moal <[email protected]>

[...]

MBR, Sergey

author maintainer
#2 lore raw
Quoted 13 lines · Sergey Shtylyov

Hello!

Quoted 9 lines · Damien Le Moal

Commit 5b6fba546da2 ("ata: libata-core: Detach a port devices on shutdown") modified the function ata_pci_shutdown_one() to stop (suspend) devices attached to the ports of a PCI AHCI adapter to ensure that drives are spun down before shutting down a system. However, this is done only for PCI adapters and not for other types of adapters. This limitation was addressed with commit 24eca2dce0f8 ("scsi: sd: Introduce manage_shutdown device flag"). With this, all ATA disks are spun down on system shutdown.

Quoted 4 lines
Quoted 2 lines

This reverts commit 5b6fba546da2 as the change introduced is now useless.

You didn't use 'git revert'?

I did, but I wrote a proper commit message.

Quoted 5 lines
Quoted 2 lines
Signed-off-by Damien Le Moal <[email protected]>

[...]

MBR, Sergey

Signature · 2 lines
Damien Le Moal
Western Digital Research
Reviewed-by
#3 lore raw
Quoted 14 lines · Damien Le Moal
Quoted 13 lines · Sergey Shtylyov

Hello!

Quoted 9 lines · Damien Le Moal

Commit 5b6fba546da2 ("ata: libata-core: Detach a port devices on shutdown") modified the function ata_pci_shutdown_one() to stop (suspend) devices attached to the ports of a PCI AHCI adapter to ensure that drives are spun down before shutting down a system. However, this is done only for PCI adapters and not for other types of adapters. This limitation was addressed with commit 24eca2dce0f8 ("scsi: sd: Introduce manage_shutdown device flag"). With this, all ATA disks are spun down on system shutdown.

Quoted 6 lines
Quoted 4 lines
Quoted 2 lines

This reverts commit 5b6fba546da2 as the change introduced is now useless.

You didn't use 'git revert'?

I did, but I wrote a proper commit message.

I think Sergey's question was because you didn't keep the automatically generated one liner will the full SHA1 as the first line in your summary (which is then followed by the reason for the revert), see e.g.:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ac2263b588dffd3a1efd7ed0b156ea6c5aea200d

You have the line as the as final line in the summary, and shortened it to not have the full SHA1.

Personally, I think I prefer having the full SHA1 as the first line, as you directly see that it is a revert (without needing the read to the end).

But I think it is fine either way:

Reviewed-by Niklas Cassel <[email protected]>
author maintainer
#4 lore raw
Quoted 6 lines · Niklas Cassel

Personally, I think I prefer having the full SHA1 as the first line, as you directly see that it is a revert (without needing the read to the end).

But I think it is fine either way:

Reviewed-by Niklas Cassel <[email protected]>

I tweaked the commit message to keep the full commit ID, making clear it is a "git revert". Thanks.

Signature · 2 lines
Damien Le Moal
Western Digital Research