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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux