If of_platform_populate() fails, we must shutdown the FPGA, as already done
in the remove function.

Fixes: 5b143d2a6ede ("bus: add driver for the Technologic Systems NBUS")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
Compile tested only.

This patch is speculative and based on the output of one of my scripts that
tries to spot calls in .remove function that are not also in the error
handling path of the probe. I'm not familiar with the pwm_ API.

I don't think that the locking in the remove function is needed here.

Review with care.
---
 drivers/bus/ts-nbus.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/bus/ts-nbus.c b/drivers/bus/ts-nbus.c
index baf22a82c47a..fa1bb9b78446 100644
--- a/drivers/bus/ts-nbus.c
+++ b/drivers/bus/ts-nbus.c
@@ -309,13 +309,19 @@ static int ts_nbus_probe(struct platform_device *pdev)
 	dev_set_drvdata(dev, ts_nbus);
 
 	ret = of_platform_populate(dev->of_node, NULL, NULL, dev);
-	if (ret < 0)
-		return dev_err_probe(dev, ret,
-				     "failed to populate platform devices on bus\n");
+	if (ret < 0) {
+		dev_err_probe(dev, ret,
+			      "failed to populate platform devices on bus\n");
+		goto err_disable_pwm;
+	}
 
 	dev_info(dev, "initialized\n");
 
 	return 0;
+
+err_disable_pwm:
+	pwm_disable(ts_nbus->pwm);
+	return ret;
 }
 
 static void ts_nbus_remove(struct platform_device *pdev)
-- 
2.44.0

Le 16/03/2024 à 17:31, Christophe JAILLET a écrit :
> If of_platform_populate() fails, we must shutdown the FPGA, as already done
> in the remove function.
> 
> Fixes: 5b143d2a6ede ("bus: add driver for the Technologic Systems NBUS")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> Compile tested only.
> 
> This patch is speculative and based on the output of one of my scripts that
> tries to spot calls in .remove function that are not also in the error
> handling path of the probe. I'm not familiar with the pwm_ API.
> 
> I don't think that the locking in the remove function is needed here.
> 
> Review with care.
> ---
>   drivers/bus/ts-nbus.c | 12 +++++++++---
>   1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/bus/ts-nbus.c b/drivers/bus/ts-nbus.c
> index baf22a82c47a..fa1bb9b78446 100644
> --- a/drivers/bus/ts-nbus.c
> +++ b/drivers/bus/ts-nbus.c
> @@ -309,13 +309,19 @@ static int ts_nbus_probe(struct platform_device *pdev)
>   	dev_set_drvdata(dev, ts_nbus);
>   
>   	ret = of_platform_populate(dev->of_node, NULL, NULL, dev);
> -	if (ret < 0)
> -		return dev_err_probe(dev, ret,
> -				     "failed to populate platform devices on bus\n");
> +	if (ret < 0) {
> +		dev_err_probe(dev, ret,
> +			      "failed to populate platform devices on bus\n");
> +		goto err_disable_pwm;
> +	}
>   
>   	dev_info(dev, "initialized\n");
>   
>   	return 0;
> +
> +err_disable_pwm:
> +	pwm_disable(ts_nbus->pwm);
> +	return ret;
>   }
>   
>   static void ts_nbus_remove(struct platform_device *pdev)

Hi,

Polite reminder.

I sent this patch a few months ago, but it never got any feed-back.

Adding Uwe and Sean who touched this file in the last months.

CJ

On Sun, Sep 22, 2024 at 03:32:44PM +0200, Christophe JAILLET wrote:
> Le 16/03/2024 à 17:31, Christophe JAILLET a écrit :
> > If of_platform_populate() fails, we must shutdown the FPGA, as already done
> > in the remove function.
> > 
> > Fixes: 5b143d2a6ede ("bus: add driver for the Technologic Systems NBUS")
> > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> > ---
> > Compile tested only.
> > 
> > This patch is speculative and based on the output of one of my scripts that
> > tries to spot calls in .remove function that are not also in the error
> > handling path of the probe. I'm not familiar with the pwm_ API.
> > 
> > I don't think that the locking in the remove function is needed here.
> > 
> > Review with care.
> > ---
> 
> I sent this patch a few months ago, but it never got any feed-back.

The patch looks right. I don't know how the ts_nbus works, but I'd hope
that at the moment ts_nbus_remove() is called, there are no more devices
on that bus that rely on the PWM being on. In that case the locking in
ts_nbus_remove() could be dropped, too.

Best regards
Uwe