On Tue, 2017-04-18 at 16:49 +0100, Mark Brown wrote: > On Tue, Apr 18, 2017 at 11:39:34PM +0800, hubiaoyong wrote: > > in the function regulator_ena_gpio_free, the if branch contains > > the return statement, so remove the else statement. > > Why is it a benefit to make this change?
In general, reducing source code indentation is a good thing. The logic today is: /* Free the GPIO only in case of no use */ list_for_each_entry_safe(pin, n, ®ulator_ena_gpio_list, list) { if (pin->gpiod == rdev->ena_pin->gpiod) { if (pin->request_count <= 1) { pin->request_count = 0; gpiod_put(pin->gpiod); list_del(&pin->list); kfree(pin); rdev->ena_pin = NULL; return; } else { pin->request_count--; } } } Perhaps it's better written as: /* Free the GPIO only in case of no use */ list_for_each_entry_safe(pin, n, ®ulator_ena_gpio_list, list) { if (pin->gpiod != rdev->ena_pin->gpiod) continue; if (pin->request_count <= 1) { pin->request_count = 0; gpiod_put(pin->gpiod); list_del(&pin->list); kfree(pin); rdev->ena_pin = NULL; return; } pin->request_count--; }