Messages in this thread | | | From | Joel Stanley <> | Date | Thu, 23 Feb 2017 00:19:05 +1030 | Subject | Re: [PATCH] gpio: aspeed: Add debounce support |
| |
On Wed, Feb 22, 2017 at 2:46 PM, Joel Stanley <joel@jms.id.au> wrote: >> + /* >> + * Check if a timer is already configured for the requested >> + * debounce period. If so, just add @offset as a user of this >> + * timer >> + */ >> + for (i = 0; i < ARRAY_SIZE(debounce_timers); i++) { >> + u32 cycles; >> + >> + addr = gpio->base + debounce_timers[i]; >> + cycles = ioread32(addr); >> + >> + if (requested_cycles == cycles) >> + break; >> + } >> + >> + /* Otherwise, allocate a timer */ > > Is "otherwise" in reference to the check above? Is the break above > supposed to be a goto?
I realised later that the if i == ARRAY_SIZE test below will always be false when the break was hit, so in effect we are doing a goto "Register @offset as a user of timer i".
I think adding a goto would make it easier to read.
If you're not a fan then perhaps tweak the comment?
Cheers,
Joel
> >> + if (i == ARRAY_SIZE(debounce_timers)) { >> + for (i = 0; i < ARRAY_SIZE(gpio->timer_users); i++) { >> + if (gpio->timer_users[i] == 0) >> + break; >> + } >> + >> + /* We have run out of timers */ >> + if (i == ARRAY_SIZE(gpio->timer_users)) { >> + dev_warn(chip->parent, >> + "Debounce timers exhausted, cannot debounce for period %luus\n", >> + usecs); >> + rc = -EPERM; >> + goto err; >> + } >> + >> + /* Timer update */ >> + addr = gpio->base + debounce_timers[i]; >> + iowrite32(requested_cycles, addr); >> + } >> + >> + /* Register @offset as a user of timer i */ >> + newdt->offset = offset; >> + newdt->timer = i; >> + hash_add(gpio->offset_timer, &newdt->node, offset);
| |