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