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, &regulator_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, &regulator_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--;
        }

Reply via email to