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 -- To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
