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@xxxxxxxxxxxxxxx> Intel Finland Oy -- 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
