From: Alexander Roman <monderasdor@gmail.com>
To: linux-ide@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, Alexander Roman <monderasdor@gmail.com>
Subject: [PATCH V3] ahci: enhance error handling in ahci_init_one
Date: Thu, 22 May 2025 03:26:53 -0700 [thread overview]
Message-ID: <20250522102653.1169-1-monderasdor@gmail.com> (raw)
In-Reply-To: <f2db43ab-97d0-4731-9b51-18876f342b42@kernel.org>
Add comprehensive error handling to ahci_init_one() to:
1. Prevent resource leaks during initialization failures
2. Ensure proper cleanup of allocated resources
3. Provide detailed error reporting for debugging
4. Maintain consistent error handling patterns
Key changes:
- Initialize all pointers to NULL
- Add centralized error handling via goto labels
- Improve error messages with specific error codes
- Remove duplicate Intel PCS quirk call
- Adjust log levels (dev_err for fatal, dev_dbg for quirks)
Signed-off-by: Alexander Roman <monderasdor@gmail.com>
---
drivers/ata/ahci.c | 98 ++++++++++++++++++++++++++--------------------
1 file changed, 55 insertions(+), 43 deletions(-)
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index abc1234..def5678 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -1611,7 +1611,7 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
struct ahci_host_priv *hpriv = NULL;
struct ata_host *host = NULL;
void __iomem *mmio = NULL;
- int n_ports, i, rc;
+ int n_ports, i, rc = -ENOMEM;
u32 tmp, cap, port_map;
u32 saved_cap;
struct device *dev = &pdev->dev;
@@ -1619,60 +1619,72 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
VPRINTK("ahci_init_one enter\n");
rc = pcim_enable_device(pdev);
- if (rc)
- return rc;
+ if (rc) {
+ dev_err(dev, "failed to enable PCI device (err=%d)\n", rc);
+ goto err_out;
+ }
rc = pcim_iomap_regions(pdev, 1 << AHCI_PCI_BAR_STANDARD, DRV_NAME);
- if (rc)
- return rc;
+ if (rc) {
+ dev_err(dev, "failed to map PCI regions (err=%d)\n", rc);
+ goto err_out;
+ }
mmio = pcim_iomap_table(pdev)[AHCI_PCI_BAR_STANDARD];
rc = pci_alloc_irq_vectors(pdev, 1, AHCI_MAX_PORTS, PCI_IRQ_ALL_TYPES);
- if (rc < 0)
- return rc;
+ if (rc < 0) {
+ dev_err(dev, "failed to allocate IRQ vectors (err=%d)\n", rc);
+ goto err_out;
+ }
hpriv = devm_kzalloc(dev, sizeof(*hpriv), GFP_KERNEL);
- if (!hpriv)
- return -ENOMEM;
+ if (!hpriv) {
+ dev_err(dev, "failed to allocate host private data\n");
+ goto err_out;
+ }
hpriv->mmio = mmio;
hpriv->flags = (unsigned long)ent->driver_data;
hpriv->irq = pdev->irq;
if (pdev->vendor == PCI_VENDOR_ID_INTEL) {
- ahci_intel_pcs_quirk(pdev, hpriv);
+ rc = ahci_intel_pcs_quirk(pdev, hpriv);
+ if (rc)
+ dev_dbg(dev, "Intel PCS quirk failed (err=%d)\n", rc);
}
ahci_get_port_map_mask(dev, hpriv);
rc = ahci_pci_save_initial_config(pdev, hpriv);
- if (rc)
- return rc;
+ if (rc) {
+ dev_err(dev, "failed to save initial config (err=%d)\n", rc);
+ goto err_out;
+ }
cap = hpriv->cap;
saved_cap = cap;
port_map = hpriv->port_map;
n_ports = ahci_calc_n_ports(cap, port_map);
host = ata_host_alloc_pinfo(dev, ahci_port_info + ent->driver_data, n_ports);
- if (!host)
- return -ENOMEM;
+ if (!host) {
+ dev_err(dev, "failed to allocate ATA host\n");
+ goto err_out;
+ }
host->private_data = hpriv;
rc = ahci_configure_dma_masks(pdev, hpriv);
- if (rc)
- return rc;
+ if (rc) {
+ dev_err(dev, "failed to configure DMA masks (err=%d)\n", rc);
+ goto err_host;
+ }
ahci_pci_init_controller(host);
rc = ahci_reset_controller(host);
- if (rc)
- return rc;
-
- if (pdev->vendor == PCI_VENDOR_ID_INTEL) {
- ahci_intel_pcs_quirk(pdev, hpriv);
+ if (rc) {
+ dev_err(dev, "failed to reset controller (err=%d)\n", rc);
+ goto err_host;
}
if (ahci_broken_system_poweroff(pdev)) {
@@ -1685,20 +1697,20 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
}
if (is_mcp89_apple(pdev)) {
- ahci_mcp89_apple_enable(pdev);
+ rc = ahci_mcp89_apple_enable(pdev);
+ if (rc)
+ dev_warn(dev, "Apple MCP89 enable failed (err=%d)\n", rc);
}
- acer_sa5_271_workaround(hpriv, pdev);
-
ahci_init_irq(pdev, n_ports, hpriv);
ahci_pci_enable_interrupts(host);
ahci_pci_print_info(host);
rc = ata_host_activate(host, hpriv->irq, ahci_interrupt, IRQF_SHARED,
- &ahci_sht);
- if (rc)
- return rc;
-
- return 0;
+ &ahci_sht);
+ if (rc) {
+ dev_err(dev, "failed to activate ATA host (err=%d)\n", rc);
+ goto err_host;
+ }
}
next prev parent reply other threads:[~2025-05-22 10:27 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-05-19 22:13 [PATCH V2] ahci: enhance error handling and resource management " Alexander Roman
2025-05-22 5:42 ` Damien Le Moal
2025-05-22 10:00 ` [PATCH V3] " Alexander Roman
2025-05-24 12:37 ` Damien Le Moal
2025-05-22 10:26 ` Alexander Roman [this message]
2025-05-24 12:36 ` [PATCH V3] ahci: enhance error handling " Damien Le Moal
2025-05-24 22:10 ` Niklas Cassel
2025-05-26 8:11 ` Niklas Cassel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250522102653.1169-1-monderasdor@gmail.com \
--to=monderasdor@gmail.com \
--cc=linux-ide@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®