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

[Lukas Wunner <lukas@wunner.de>]

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():

-	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.

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(...)".


> > +#ifdef CONFIG_PM
> > +	bcm->dev->hu = NULL;
> > +#endif
> 
> Hmm... There is no field in !PM case?

Yes, there isn't.

Thanks for the review!

Lukas