Return -EINVAL if "bridge->n_sensors == 0".  Don't return success.

Fixes: 881ca25978c6 ("media: ipu3-cio2: rename cio2 bridge to ipu bridge and move out of ipu3")
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
 drivers/media/pci/intel/ipu-bridge.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/media/pci/intel/ipu-bridge.c b/drivers/media/pci/intel/ipu-bridge.c
index 61750cc98d70..a009ee73e26f 100644
--- a/drivers/media/pci/intel/ipu-bridge.c
+++ b/drivers/media/pci/intel/ipu-bridge.c
@@ -839,8 +839,10 @@ int ipu_bridge_init(struct device *dev,
 		bridge->data_lanes[i] = i + 1;
 
 	ret = ipu_bridge_connect_sensors(bridge);
-	if (ret || bridge->n_sensors == 0)
+	if (ret || bridge->n_sensors == 0) {
+		ret = ret ?: -EINVAL;
 		goto err_unregister_ipu;
+	}
 
 	dev_info(dev, "Connected %d cameras\n", bridge->n_sensors);
 
-- 
2.43.0

On Fri, May 10, 2024 at 06:10:37PM +0300, Dan Carpenter wrote:
> Return -EINVAL if "bridge->n_sensors == 0".  Don't return success.

...

>  	ret = ipu_bridge_connect_sensors(bridge);
> -	if (ret || bridge->n_sensors == 0)
> +	if (ret || bridge->n_sensors == 0) {
> +		ret = ret ?: -EINVAL;
>  		goto err_unregister_ipu;
> +	}

I would split:

	ret = ipu_bridge_connect_sensors(bridge);
	if (ret)
		goto err_unregister_ipu;

	if (bridge->n_sensors == 0) {
		ret = -EINVAL;
		goto err_unregister_ipu;
	}

-- 
With Best Regards,
Andy Shevchenko

On Fri, May 10, 2024 at 06:18:22PM +0300, Andy Shevchenko wrote:
> On Fri, May 10, 2024 at 06:10:37PM +0300, Dan Carpenter wrote:
> > Return -EINVAL if "bridge->n_sensors == 0".  Don't return success.
> 
> ...
> 
> >  	ret = ipu_bridge_connect_sensors(bridge);
> > -	if (ret || bridge->n_sensors == 0)
> > +	if (ret || bridge->n_sensors == 0) {
> > +		ret = ret ?: -EINVAL;
> >  		goto err_unregister_ipu;
> > +	}
> 
> I would split:
> 
> 	ret = ipu_bridge_connect_sensors(bridge);
> 	if (ret)
> 		goto err_unregister_ipu;
> 
> 	if (bridge->n_sensors == 0) {
> 		ret = -EINVAL;
> 		goto err_unregister_ipu;
> 	}

It's always hard to know which way to go on these...  I wrote it that
way in my first draft.  It's my prefered way as well but not everyone
agrees.  I'll resend.

regards,
dan carpenter

On Fri, May 10, 2024 at 06:27:30PM +0300, Dan Carpenter wrote:
> On Fri, May 10, 2024 at 06:18:22PM +0300, Andy Shevchenko wrote:
> > On Fri, May 10, 2024 at 06:10:37PM +0300, Dan Carpenter wrote:
> > > Return -EINVAL if "bridge->n_sensors == 0".  Don't return success.

...

> > >  	ret = ipu_bridge_connect_sensors(bridge);
> > > -	if (ret || bridge->n_sensors == 0)
> > > +	if (ret || bridge->n_sensors == 0) {
> > > +		ret = ret ?: -EINVAL;
> > >  		goto err_unregister_ipu;
> > > +	}
> > 
> > I would split:
> > 
> > 	ret = ipu_bridge_connect_sensors(bridge);
> > 	if (ret)
> > 		goto err_unregister_ipu;
> > 
> > 	if (bridge->n_sensors == 0) {
> > 		ret = -EINVAL;
> > 		goto err_unregister_ipu;
> > 	}
> 
> It's always hard to know which way to go on these...  I wrote it that
> way in my first draft.  It's my prefered way as well but not everyone
> agrees.  I'll resend.

Is the generated assembly the same?

-- 
With Best Regards,
Andy Shevchenko

On Fri, May 10, 2024 at 06:36:36PM +0300, Andy Shevchenko wrote:
> On Fri, May 10, 2024 at 06:27:30PM +0300, Dan Carpenter wrote:
> > On Fri, May 10, 2024 at 06:18:22PM +0300, Andy Shevchenko wrote:
> > > On Fri, May 10, 2024 at 06:10:37PM +0300, Dan Carpenter wrote:
> > > > Return -EINVAL if "bridge->n_sensors == 0".  Don't return success.
> 
> ...
> 
> > > >  	ret = ipu_bridge_connect_sensors(bridge);
> > > > -	if (ret || bridge->n_sensors == 0)
> > > > +	if (ret || bridge->n_sensors == 0) {
> > > > +		ret = ret ?: -EINVAL;
> > > >  		goto err_unregister_ipu;
> > > > +	}
> > > 
> > > I would split:
> > > 
> > > 	ret = ipu_bridge_connect_sensors(bridge);
> > > 	if (ret)
> > > 		goto err_unregister_ipu;
> > > 
> > > 	if (bridge->n_sensors == 0) {
> > > 		ret = -EINVAL;
> > > 		goto err_unregister_ipu;
> > > 	}
> > 
> > It's always hard to know which way to go on these...  I wrote it that
> > way in my first draft.  It's my prefered way as well but not everyone
> > agrees.  I'll resend.
> 
> Is the generated assembly the same?

Yeah, it does.

regards,
dan carpenter