Re: [PATCH v2 09/10] Bluetooth: hci_bcm: Handle errors properly

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Wed, 2018-01-03 at 19:54 +0100, Lukas Wunner wrote:
> On Wed, Jan 03, 2018 at 06:08:42PM +0200, Andy Shevchenko wrote:
> > On Tue, 2018-01-02 at 20:08 +0100, Lukas Wunner wrote:
> > > +	int err = 0;
> > 
> > Obviously redundant assignment.
> 
> No, you need to look at it in the context of patch [10/10], which
> changes
> the following portion of bcm_gpio_set_power():

Then why this assignment here?

Move it to related patch.

> 
> -	gpiod_set_value(dev->shutdown, powered);
> +	if (x86_apple_machine)
> +		err = bcm_apple_set_power(dev, powered);
> +	else
> +		gpiod_set_value(dev->shutdown, powered);
> +	if (err)
> +		goto err_clk_disable;
> 
> I need to set err = 0 in case the gpiod_set_value() code path is
> chosen.
> 
> Of course I could only declare "int err;" in this patch and change it
> to
> "int err = 0;" in the next patch, but then I would expect an objection
> along the lines of:  You're changing code you've just added in the
> prior
> patch, this is silly.

Nope. Assignment belongs to when code really needs it. It overrides
ping-ponging style in this case I suppose.

> Long term the gpio API will be changed such that gpiod_set_value()
> returns a negative errno or 0 (instead of void), as we're already
> doing for gpiod_get_value().  *Then* I can change this to declare
> "int err;" and set "err = gpiod_set_value(...)".

It's a good roadmap, though out of scope of the current approach AFAICS.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy