PCI/pwrctrl: Propagate dev_err_probe return value

[PATCH v1] PCI/pwrctrl: Propagate dev_err_probe return value

Posted by Anand Moon 3 months, 3 weeks ago

Ensure that the return value from dev_err_probe() is consistently assigned
back to return in all error paths within pci_pwrctrl_slot_probe()
function. This ensures the original error code are propagation for
debugging.

Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
 drivers/pci/pwrctrl/slot.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/pwrctrl/slot.c b/drivers/pci/pwrctrl/slot.c
index 3320494b62d89..36a6282fd222d 100644
--- a/drivers/pci/pwrctrl/slot.c
+++ b/drivers/pci/pwrctrl/slot.c
@@ -41,14 +41,13 @@ static int pci_pwrctrl_slot_probe(struct platform_device *pdev)
 	ret = of_regulator_bulk_get_all(dev, dev_of_node(dev),
 					&slot->supplies);
 	if (ret < 0) {
-		dev_err_probe(dev, ret, "Failed to get slot regulators\n");
-		return ret;
+		return dev_err_probe(dev, ret, "Failed to get slot regulators\n");
 	}
 
 	slot->num_supplies = ret;
 	ret = regulator_bulk_enable(slot->num_supplies, slot->supplies);
 	if (ret < 0) {
-		dev_err_probe(dev, ret, "Failed to enable slot regulators\n");
+		ret = dev_err_probe(dev, ret, "Failed to enable slot regulators\n");
 		regulator_bulk_free(slot->num_supplies, slot->supplies);
 		return ret;
 	}

base-commit: f406055cb18c6e299c4a783fc1effeb16be41803
-- 
2.50.1

Re: [PATCH v1] PCI/pwrctrl: Propagate dev_err_probe return value

Posted by Manivannan Sadhasivam 3 months, 1 week ago

On Sat, Oct 18, 2025 at 12:32:18PM +0530, Anand Moon wrote:
> Ensure that the return value from dev_err_probe() is consistently assigned
> back to return in all error paths within pci_pwrctrl_slot_probe()
> function. This ensures the original error code are propagation for
> debugging.
> 
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
>  drivers/pci/pwrctrl/slot.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/pwrctrl/slot.c b/drivers/pci/pwrctrl/slot.c
> index 3320494b62d89..36a6282fd222d 100644
> --- a/drivers/pci/pwrctrl/slot.c
> +++ b/drivers/pci/pwrctrl/slot.c
> @@ -41,14 +41,13 @@ static int pci_pwrctrl_slot_probe(struct platform_device *pdev)
>  	ret = of_regulator_bulk_get_all(dev, dev_of_node(dev),
>  					&slot->supplies);
>  	if (ret < 0) {
> -		dev_err_probe(dev, ret, "Failed to get slot regulators\n");
> -		return ret;
> +		return dev_err_probe(dev, ret, "Failed to get slot regulators\n");
>  	}
>  
>  	slot->num_supplies = ret;
>  	ret = regulator_bulk_enable(slot->num_supplies, slot->supplies);
>  	if (ret < 0) {
> -		dev_err_probe(dev, ret, "Failed to enable slot regulators\n");
> +		ret = dev_err_probe(dev, ret, "Failed to enable slot regulators\n");

Again a pointless change.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH] PCI/pwrctrl: Propagate dev_err_probe return value

Posted by Markus Elfring 3 months, 3 weeks ago

> Ensure that the return value from dev_err_probe() is consistently assigned
> back to return in all error paths within pci_pwrctrl_slot_probe()
> function. This ensures the original error code are propagation for
> debugging.

I find the change description improvable.


I propose to take another source code transformation approach into account.
https://elixir.bootlin.com/linux/v6.17.1/source/drivers/base/core.c#L5031-L5075

Example:
https://elixir.bootlin.com/linux/v6.17.1/source/drivers/pci/pwrctrl/slot.c#L30-L80

	ret = dev_err_probe(dev,
			    of_regulator_bulk_get_all(dev, dev_of_node(dev), &slot->supplies),
			    "Failed to get slot regulators\n");
	if (ret)
		return ret;


Regards,
Markus

Re: [PATCH v1] PCI/pwrctrl: Propagate dev_err_probe return value

Posted by Christophe JAILLET 3 months, 3 weeks ago

Le 18/10/2025 à 09:02, Anand Moon a écrit :
> Ensure that the return value from dev_err_probe() is consistently assigned
> back to return in all error paths within pci_pwrctrl_slot_probe()
> function. This ensures the original error code are propagation for
> debugging.
> 
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
>   drivers/pci/pwrctrl/slot.c | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/pwrctrl/slot.c b/drivers/pci/pwrctrl/slot.c
> index 3320494b62d89..36a6282fd222d 100644
> --- a/drivers/pci/pwrctrl/slot.c
> +++ b/drivers/pci/pwrctrl/slot.c
> @@ -41,14 +41,13 @@ static int pci_pwrctrl_slot_probe(struct platform_device *pdev)
>   	ret = of_regulator_bulk_get_all(dev, dev_of_node(dev),
>   					&slot->supplies);
>   	if (ret < 0) {
> -		dev_err_probe(dev, ret, "Failed to get slot regulators\n");
> -		return ret;
> +		return dev_err_probe(dev, ret, "Failed to get slot regulators\n");
>   	}

Extra {} are now unneeded.

>   
>   	slot->num_supplies = ret;
>   	ret = regulator_bulk_enable(slot->num_supplies, slot->supplies);
>   	if (ret < 0) {
> -		dev_err_probe(dev, ret, "Failed to enable slot regulators\n");
> +		ret = dev_err_probe(dev, ret, "Failed to enable slot regulators\n");
>   		regulator_bulk_free(slot->num_supplies, slot->supplies);
>   		return ret;

Doing:
    		regulator_bulk_free(slot->num_supplies, slot->supplies);
    		return dev_err_probe(dev, ret, "Failed to enable slot regulators\n");

Would be more consistent.

CJ


>   	}
> 
> base-commit: f406055cb18c6e299c4a783fc1effeb16be41803

Re: [PATCH] PCI/pwrctrl: Propagate dev_err_probe return value

Posted by Markus Elfring 3 months, 3 weeks ago

> >   	slot->num_supplies = ret;
> >   	ret = regulator_bulk_enable(slot->num_supplies, slot->supplies);
> >   	if (ret < 0) {
> > -		dev_err_probe(dev, ret, "Failed to enable slot regulators\n");
> > +		ret = dev_err_probe(dev, ret, "Failed to enable slot regulators\n");
> >   		regulator_bulk_free(slot->num_supplies, slot->supplies);
> >   		return ret;
> 
> Doing:
>     		regulator_bulk_free(slot->num_supplies, slot->supplies);
>     		return dev_err_probe(dev, ret, "Failed to enable slot regulators\n");
> 
> Would be more consistent.

How does this view fit to the commit ab81f2f79c683c94bac622aafafbe8232e547159
("PCI/pwrctrl: Fix double cleanup on devm_add_action_or_reset() failure")
from 2025-08-13?

Regards,
Markus

Re: [PATCH] PCI/pwrctrl: Propagate dev_err_probe return value

Posted by Anand Moon 3 months, 3 weeks ago

Hi Markus,

On Sat, 18 Oct 2025 at 21:36, Markus Elfring <Markus.Elfring@web.de> wrote:
>
> > >     slot->num_supplies = ret;
> > >     ret = regulator_bulk_enable(slot->num_supplies, slot->supplies);
> > >     if (ret < 0) {
> > > -           dev_err_probe(dev, ret, "Failed to enable slot regulators\n");
> > > +           ret = dev_err_probe(dev, ret, "Failed to enable slot regulators\n");
> > >             regulator_bulk_free(slot->num_supplies, slot->supplies);
> > >             return ret;
> >
> > Doing:
> >               regulator_bulk_free(slot->num_supplies, slot->supplies);
> >               return dev_err_probe(dev, ret, "Failed to enable slot regulators\n");
> >
> > Would be more consistent.
>
> How does this view fit to the commit ab81f2f79c683c94bac622aafafbe8232e547159
> ("PCI/pwrctrl: Fix double cleanup on devm_add_action_or_reset() failure")
> from 2025-08-13?
>
Thank you for your guidance. My previous understanding was incorrect.

> Regards,
> Markus
>
Thanks
-Anand

Re: PCI/pwrctrl: Propagate dev_err_probe return value

Posted by Markus Elfring 3 months, 3 weeks ago

>> How does this view fit to the commit ab81f2f79c683c94bac622aafafbe8232e547159
>> ("PCI/pwrctrl: Fix double cleanup on devm_add_action_or_reset() failure")
>> from 2025-08-13?
>>
> Thank you for your guidance. My previous understanding was incorrect.

Will an adjusted software understanding influence further collateral evolutions?

Regards,
Markus

Re: PCI/pwrctrl: Propagate dev_err_probe return value

Posted by Anand Moon 3 months, 3 weeks ago

Hi Markus,

On Sun, 19 Oct 2025 at 14:26, Markus Elfring <Markus.Elfring@web.de> wrote:
>
> >> How does this view fit to the commit ab81f2f79c683c94bac622aafafbe8232e547159
> >> ("PCI/pwrctrl: Fix double cleanup on devm_add_action_or_reset() failure")
> >> from 2025-08-13?
> >>
> > Thank you for your guidance. My previous understanding was incorrect.
>
> Will an adjusted software understanding influence further collateral evolutions?
>
I will try to be more correct and improve myself.
Sorry for the inconvenience,

> Regards,
> Markus

Thanks
-Anand