From fd3a6837d8e18cb7be80dcca1283276290336a7a Mon Sep 17 00:00:00 2001 From: Damien Le Moal Date: Mon, 6 Nov 2023 13:00:49 +0900 Subject: ata: libata-core: Fix ata_pci_shutdown_one() This reverts commit 5b6fba546da246b3d0dd8465c07783e22629cc53. 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, which make the changes introduced with 5b6fba546da2 useless. Signed-off-by: Damien Le Moal Reviewed-by: Niklas Cassel --- drivers/ata/libata-core.c | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 6fb4e8dc8c3c..09ed67772fae 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -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; -- cgit v1.2.3 From 36f10a914a7b7169b5e399d1671ba2169c0801f3 Mon Sep 17 00:00:00 2001 From: Uwe Kleine-König Date: Sun, 5 Nov 2023 16:00:36 +0100 Subject: ata: pata_falcon: Stop using module_platform_driver_probe() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On today's platforms the benefit of platform_driver_probe() isn't that relevant any more. It allows to drop some code after booting (or module loading) for .probe() and discard the .remove() function completely if the driver is built-in. This typically saves a few 100k. The downside of platform_driver_probe() is that the driver cannot be bound and unbound at runtime which is ancient and so slightly complicates testing. There are also thoughts to deprecate platform_driver_probe() because it adds some complexity in the driver core for little gain. Also many drivers don't use it correctly. This driver for example misses to mark the driver struct with __ref which is needed to suppress a (W=1) modpost warning. Signed-off-by: Uwe Kleine-König Reviewed-by: Sergey Shtylyov Reviewed-by: Niklas Cassel Signed-off-by: Damien Le Moal --- drivers/ata/pata_falcon.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/ata/pata_falcon.c b/drivers/ata/pata_falcon.c index 0c2ae430f5aa..9dfc2df47cf8 100644 --- a/drivers/ata/pata_falcon.c +++ b/drivers/ata/pata_falcon.c @@ -121,7 +121,7 @@ static struct ata_port_operations pata_falcon_ops = { .set_mode = pata_falcon_set_mode, }; -static int __init pata_falcon_init_one(struct platform_device *pdev) +static int pata_falcon_init_one(struct platform_device *pdev) { struct resource *base_mem_res, *ctl_mem_res; struct resource *base_res, *ctl_res, *irq_res; @@ -216,7 +216,7 @@ static int __init pata_falcon_init_one(struct platform_device *pdev) IRQF_SHARED, &pata_falcon_sht); } -static int __exit pata_falcon_remove_one(struct platform_device *pdev) +static int pata_falcon_remove_one(struct platform_device *pdev) { struct ata_host *host = platform_get_drvdata(pdev); @@ -226,13 +226,14 @@ static int __exit pata_falcon_remove_one(struct platform_device *pdev) } static struct platform_driver pata_falcon_driver = { - .remove = __exit_p(pata_falcon_remove_one), + .probe = pata_falcon_init_one, + .remove = pata_falcon_remove_one, .driver = { .name = "atari-falcon-ide", }, }; -module_platform_driver_probe(pata_falcon_driver, pata_falcon_init_one); +module_platform_driver(pata_falcon_driver); MODULE_AUTHOR("Bartlomiej Zolnierkiewicz"); MODULE_DESCRIPTION("low-level driver for Atari Falcon PATA"); -- cgit v1.2.3 From 0b2771dd52570698c1b92e428f8bf2a224e7a2c3 Mon Sep 17 00:00:00 2001 From: Uwe Kleine-König Date: Sun, 5 Nov 2023 16:00:37 +0100 Subject: ata: pata_gayle: Stop using module_platform_driver_probe() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On today's platforms the benefit of platform_driver_probe() isn't that relevant any more. It allows to drop some code after booting (or module loading) for .probe() and discard the .remove() function completely if the driver is built-in. This typically saves a few 100k. The downside of platform_driver_probe() is that the driver cannot be bound and unbound at runtime which is ancient and so slightly complicates testing. There are also thoughts to deprecate platform_driver_probe() because it adds some complexity in the driver core for little gain. Also many drivers don't use it correctly. This driver for example misses to mark the driver struct with __ref which is needed to suppress a (W=1) modpost warning. Signed-off-by: Uwe Kleine-König Reviewed-by: Sergey Shtylyov Reviewed-by: Niklas Cassel Signed-off-by: Damien Le Moal --- drivers/ata/pata_gayle.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/ata/pata_gayle.c b/drivers/ata/pata_gayle.c index 3bdbe2b65a2b..e10411b02047 100644 --- a/drivers/ata/pata_gayle.c +++ b/drivers/ata/pata_gayle.c @@ -124,7 +124,7 @@ static struct ata_port_operations pata_gayle_a4000_ops = { .set_mode = pata_gayle_set_mode, }; -static int __init pata_gayle_init_one(struct platform_device *pdev) +static int pata_gayle_init_one(struct platform_device *pdev) { struct resource *res; struct gayle_ide_platform_data *pdata; @@ -193,7 +193,7 @@ static int __init pata_gayle_init_one(struct platform_device *pdev) return 0; } -static int __exit pata_gayle_remove_one(struct platform_device *pdev) +static int pata_gayle_remove_one(struct platform_device *pdev) { struct ata_host *host = platform_get_drvdata(pdev); @@ -203,13 +203,14 @@ static int __exit pata_gayle_remove_one(struct platform_device *pdev) } static struct platform_driver pata_gayle_driver = { - .remove = __exit_p(pata_gayle_remove_one), + .probe = pata_gayle_init_one, + .remove = pata_gayle_remove_one, .driver = { .name = "amiga-gayle-ide", }, }; -module_platform_driver_probe(pata_gayle_driver, pata_gayle_init_one); +module_platform_driver(pata_gayle_driver); MODULE_AUTHOR("Bartlomiej Zolnierkiewicz"); MODULE_DESCRIPTION("low-level driver for Amiga Gayle PATA"); -- cgit v1.2.3 From 47d4708dfa54ad0e84e47bc681c133c2146b0f56 Mon Sep 17 00:00:00 2001 From: Uwe Kleine-König Date: Sun, 5 Nov 2023 16:00:38 +0100 Subject: ata: pata_falcon: Convert to platform remove callback returning void MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The .remove() callback for a platform driver returns an int which makes many driver authors wrongly assume it's possible to do error handling by returning an error code. However the value returned is ignored (apart from emitting a warning) and this typically results in resource leaks. To improve here there is a quest to make the remove callback return void. In the first step of this quest all drivers are converted to .remove_new(), which already returns void. Eventually after all drivers are converted, .remove_new() will be renamed to .remove(). Trivially convert this driver from always returning zero in the remove callback to the void returning variant. Signed-off-by: Uwe Kleine-König Reviewed-by: Sergey Shtylyov Reviewed-by: Niklas Cassel Signed-off-by: Damien Le Moal --- drivers/ata/pata_falcon.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/ata/pata_falcon.c b/drivers/ata/pata_falcon.c index 9dfc2df47cf8..18ceefd176df 100644 --- a/drivers/ata/pata_falcon.c +++ b/drivers/ata/pata_falcon.c @@ -216,18 +216,16 @@ static int pata_falcon_init_one(struct platform_device *pdev) IRQF_SHARED, &pata_falcon_sht); } -static int pata_falcon_remove_one(struct platform_device *pdev) +static void pata_falcon_remove_one(struct platform_device *pdev) { struct ata_host *host = platform_get_drvdata(pdev); ata_host_detach(host); - - return 0; } static struct platform_driver pata_falcon_driver = { .probe = pata_falcon_init_one, - .remove = pata_falcon_remove_one, + .remove_new = pata_falcon_remove_one, .driver = { .name = "atari-falcon-ide", }, -- cgit v1.2.3 From 99bce5182d8fe90e5c57e9d99f831baaa94f90cb Mon Sep 17 00:00:00 2001 From: Uwe Kleine-König Date: Sun, 5 Nov 2023 16:00:39 +0100 Subject: ata: pata_gayle: Convert to platform remove callback returning void MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The .remove() callback for a platform driver returns an int which makes many driver authors wrongly assume it's possible to do error handling by returning an error code. However the value returned is ignored (apart from emitting a warning) and this typically results in resource leaks. To improve here there is a quest to make the remove callback return void. In the first step of this quest all drivers are converted to .remove_new(), which already returns void. Eventually after all drivers are converted, .remove_new() will be renamed to .remove(). Trivially convert this driver from always returning zero in the remove callback to the void returning variant. Signed-off-by: Uwe Kleine-König Reviewed-by: Sergey Shtylyov Reviewed-by: Niklas Cassel Signed-off-by: Damien Le Moal --- drivers/ata/pata_gayle.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/ata/pata_gayle.c b/drivers/ata/pata_gayle.c index e10411b02047..94df60ac2307 100644 --- a/drivers/ata/pata_gayle.c +++ b/drivers/ata/pata_gayle.c @@ -193,18 +193,16 @@ static int pata_gayle_init_one(struct platform_device *pdev) return 0; } -static int pata_gayle_remove_one(struct platform_device *pdev) +static void pata_gayle_remove_one(struct platform_device *pdev) { struct ata_host *host = platform_get_drvdata(pdev); ata_host_detach(host); - - return 0; } static struct platform_driver pata_gayle_driver = { .probe = pata_gayle_init_one, - .remove = pata_gayle_remove_one, + .remove_new = pata_gayle_remove_one, .driver = { .name = "amiga-gayle-ide", }, -- cgit v1.2.3