[PATCH] net: airoha: Fix an error handling path in airoha_probe()

[PATCH] net: airoha: Fix an error handling path in airoha_probe()

[Christophe JAILLET]

If an error occurs after a successful airoha_hw_init() call,
airoha_ppe_deinit() needs to be called as already done in the remove
function.

Fixes: 00a7678310fe ("net: airoha: Introduce flowtable offload support")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
Compile tested-only
---
 drivers/net/ethernet/airoha/airoha_eth.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
index 69e523dd4186..252b32ceb064 100644
--- a/drivers/net/ethernet/airoha/airoha_eth.c
+++ b/drivers/net/ethernet/airoha/airoha_eth.c
@@ -2631,6 +2631,8 @@ static int airoha_probe(struct platform_device *pdev)
 		}
 	}
 	free_netdev(eth->napi_dev);
+
+	airoha_ppe_deinit(eth);
 	platform_set_drvdata(pdev, NULL);
 
 	return err;
-- 
2.49.0

Re: [PATCH] net: airoha: Fix an error handling path in airoha_probe()

[Lorenzo Bianconi]

[-- Attachment #1: Type: text/plain, Size: 1745 bytes --]

> If an error occurs after a successful airoha_hw_init() call,
> airoha_ppe_deinit() needs to be called as already done in the remove
> function.
> 
> Fixes: 00a7678310fe ("net: airoha: Introduce flowtable offload support")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> Compile tested-only
> ---
>  drivers/net/ethernet/airoha/airoha_eth.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
> index 69e523dd4186..252b32ceb064 100644
> --- a/drivers/net/ethernet/airoha/airoha_eth.c
> +++ b/drivers/net/ethernet/airoha/airoha_eth.c
> @@ -2631,6 +2631,8 @@ static int airoha_probe(struct platform_device *pdev)
>  		}
>  	}
>  	free_netdev(eth->napi_dev);
> +
> +	airoha_ppe_deinit(eth);
>  	platform_set_drvdata(pdev, NULL);
>  
>  	return err;
> -- 
> 2.49.0
> 

Hi Christophe,

I agree we are missing a airoha_ppe_deinit() call in the probe error path,
but we should move it above after stopping the NAPI since if airoha_hw_init()
fails we will undo the work done by airoha_ppe_init(). Something like:

diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
index 16c7896f931f..37d9678798d1 100644
--- a/drivers/net/ethernet/airoha/airoha_eth.c
+++ b/drivers/net/ethernet/airoha/airoha_eth.c
@@ -2959,6 +2959,7 @@ static int airoha_probe(struct platform_device *pdev)
 error_napi_stop:
 	for (i = 0; i < ARRAY_SIZE(eth->qdma); i++)
 		airoha_qdma_stop_napi(&eth->qdma[i]);
+	airoha_ppe_init(eth);
 error_hw_cleanup:
 	for (i = 0; i < ARRAY_SIZE(eth->qdma); i++)
 		airoha_hw_cleanup(&eth->qdma[i]);


Agree?

Regards,
Lorenzo

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] net: airoha: Fix an error handling path in airoha_probe()

[Lorenzo Bianconi]

[-- Attachment #1: Type: text/plain, Size: 1928 bytes --]

> > If an error occurs after a successful airoha_hw_init() call,
> > airoha_ppe_deinit() needs to be called as already done in the remove
> > function.
> > 
> > Fixes: 00a7678310fe ("net: airoha: Introduce flowtable offload support")
> > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> > ---
> > Compile tested-only
> > ---
> >  drivers/net/ethernet/airoha/airoha_eth.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
> > index 69e523dd4186..252b32ceb064 100644
> > --- a/drivers/net/ethernet/airoha/airoha_eth.c
> > +++ b/drivers/net/ethernet/airoha/airoha_eth.c
> > @@ -2631,6 +2631,8 @@ static int airoha_probe(struct platform_device *pdev)
> >  		}
> >  	}
> >  	free_netdev(eth->napi_dev);
> > +
> > +	airoha_ppe_deinit(eth);
> >  	platform_set_drvdata(pdev, NULL);
> >  
> >  	return err;
> > -- 
> > 2.49.0
> > 
> 
> Hi Christophe,
> 
> I agree we are missing a airoha_ppe_deinit() call in the probe error path,
> but we should move it above after stopping the NAPI since if airoha_hw_init()
> fails we will undo the work done by airoha_ppe_init(). Something like:
> 
> diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
> index 16c7896f931f..37d9678798d1 100644
> --- a/drivers/net/ethernet/airoha/airoha_eth.c
> +++ b/drivers/net/ethernet/airoha/airoha_eth.c
> @@ -2959,6 +2959,7 @@ static int airoha_probe(struct platform_device *pdev)
>  error_napi_stop:
>  	for (i = 0; i < ARRAY_SIZE(eth->qdma); i++)
>  		airoha_qdma_stop_napi(&eth->qdma[i]);
> +	airoha_ppe_init(eth);
>  error_hw_cleanup:
>  	for (i = 0; i < ARRAY_SIZE(eth->qdma); i++)
>  		airoha_hw_cleanup(&eth->qdma[i]);
> 

Hi Christophe,

any plan to repost this fix?

Regards,
Lorenzo

> 
> Agree?
> 
> Regards,
> Lorenzo



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] net: airoha: Fix an error handling path in airoha_probe()

[Christophe JAILLET]

Le 29/04/2025 à 16:22, Lorenzo Bianconi a écrit :
>>> If an error occurs after a successful airoha_hw_init() call,
>>> airoha_ppe_deinit() needs to be called as already done in the remove
>>> function.
>>>
>>> Fixes: 00a7678310fe ("net: airoha: Introduce flowtable offload support")
>>> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
>>> ---
>>> Compile tested-only
>>> ---
>>>   drivers/net/ethernet/airoha/airoha_eth.c | 2 ++
>>>   1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
>>> index 69e523dd4186..252b32ceb064 100644
>>> --- a/drivers/net/ethernet/airoha/airoha_eth.c
>>> +++ b/drivers/net/ethernet/airoha/airoha_eth.c
>>> @@ -2631,6 +2631,8 @@ static int airoha_probe(struct platform_device *pdev)
>>>   		}
>>>   	}
>>>   	free_netdev(eth->napi_dev);
>>> +
>>> +	airoha_ppe_deinit(eth);
>>>   	platform_set_drvdata(pdev, NULL);
>>>   
>>>   	return err;
>>> -- 
>>> 2.49.0
>>>
>>
>> Hi Christophe,
>>
>> I agree we are missing a airoha_ppe_deinit() call in the probe error path,
>> but we should move it above after stopping the NAPI since if airoha_hw_init()
>> fails we will undo the work done by airoha_ppe_init(). Something like:
>>
>> diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
>> index 16c7896f931f..37d9678798d1 100644
>> --- a/drivers/net/ethernet/airoha/airoha_eth.c
>> +++ b/drivers/net/ethernet/airoha/airoha_eth.c
>> @@ -2959,6 +2959,7 @@ static int airoha_probe(struct platform_device *pdev)
>>   error_napi_stop:
>>   	for (i = 0; i < ARRAY_SIZE(eth->qdma); i++)
>>   		airoha_qdma_stop_napi(&eth->qdma[i]);
>> +	airoha_ppe_init(eth);
>>   error_hw_cleanup:
>>   	for (i = 0; i < ARRAY_SIZE(eth->qdma); i++)
>>   		airoha_hw_cleanup(&eth->qdma[i]);
>>
> 
> Hi Christophe,
> 
> any plan to repost this fix?

Hi,

I'll send a v2, but I currently don't have time to look at it.
Will need a few more days.

CJ

> 
> Regards,
> Lorenzo
> 
>>
>> Agree?
>>
>> Regards,
>> Lorenzo
> 
> 

Re: [PATCH] net: airoha: Fix an error handling path in airoha_probe()

[Christophe JAILLET]

Le 18/04/2025 à 14:28, Lorenzo Bianconi a écrit :
>> If an error occurs after a successful airoha_hw_init() call,
>> airoha_ppe_deinit() needs to be called as already done in the remove
>> function.
>>
>> Fixes: 00a7678310fe ("net: airoha: Introduce flowtable offload support")
>> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
>> ---
>> Compile tested-only
>> ---
>>   drivers/net/ethernet/airoha/airoha_eth.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
>> index 69e523dd4186..252b32ceb064 100644
>> --- a/drivers/net/ethernet/airoha/airoha_eth.c
>> +++ b/drivers/net/ethernet/airoha/airoha_eth.c
>> @@ -2631,6 +2631,8 @@ static int airoha_probe(struct platform_device *pdev)
>>   		}
>>   	}
>>   	free_netdev(eth->napi_dev);
>> +
>> +	airoha_ppe_deinit(eth);
>>   	platform_set_drvdata(pdev, NULL);
>>   
>>   	return err;
>> -- 
>> 2.49.0
>>
> 
> Hi Christophe,
> 
> I agree we are missing a airoha_ppe_deinit() call in the probe error path,
> but we should move it above after stopping the NAPI since if airoha_hw_init()
> fails we will undo the work done by airoha_ppe_init(). Something like:

Agreed.

I'll send a v2 with as a small series, because of another leak I found 
while looking at it.

And while at it, I'll propose a few clean-ups.

CJ


> diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
> index 16c7896f931f..37d9678798d1 100644
> --- a/drivers/net/ethernet/airoha/airoha_eth.c
> +++ b/drivers/net/ethernet/airoha/airoha_eth.c
> @@ -2959,6 +2959,7 @@ static int airoha_probe(struct platform_device *pdev)
>   error_napi_stop:
>   	for (i = 0; i < ARRAY_SIZE(eth->qdma); i++)
>   		airoha_qdma_stop_napi(&eth->qdma[i]);
> +	airoha_ppe_init(eth);
>   error_hw_cleanup:
>   	for (i = 0; i < ARRAY_SIZE(eth->qdma); i++)
>   		airoha_hw_cleanup(&eth->qdma[i]);
> 
> 
> Agree?
> 
> Regards,
> Lorenzo