Re: [PATCH V3] ahci: enhance error handling in ahci_init_one

[Niklas Cassel <cassel@kernel.org>]

On 24 May 2025 14:36:24 CEST, Damien Le Moal <dlemoal@kernel.org> wrote:
>On 5/22/25 12:26, Alexander Roman wrote:
>> 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>
>
>I received 2 x v3 patches with different commit messages and titles, but these 2
>patches touch the same code.. Very confusing...
>Which one is the "correct" patch you want us to consider ?
>
>And please send patches to *all* maintainers of the subsystem.
>You can check that with "scripts/get_maintainer.pl driver/ata"
>(you are missing Niklas).
>
>Note: it is too late to apply this patch anyway. If accepted, it will go in
>during 6.16-rc1. So no rush to clean this up. Take your time and make a proper
>patch please.
>
>
>> ---
>>  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");

There is no VPRINTK() here since a long time ago.

So this must be based on some ancient kernel version.

Please base your patches on:
https://git.kernel.org/pub/scm/linux/kernel/git/libata/linux.git/log/?h=for-next


Kind regards,
Niklas
Hello Alexander,