[PATCH 3/7] misc: ixp4-beeper: use gpiolib strictly

Linus Walleij linus.walleij at linaro.org
Fri Sep 13 04:35:26 EDT 2013


On Wed, Sep 11, 2013 at 3:58 AM, Alex Courbot <acourbot at nvidia.com> wrote:
> On 09/10/2013 09:30 PM, Linus Walleij wrote:
>>         /* flip the beeper output */
>> -       *IXP4XX_GPIO_GPOUTR ^= (1 << (unsigned int) dev_id);
>> +       gpio_set_value(pin, ~gpio_get_value(pin));
>
>
> Don't you want
>
>         gpio_set_value(pin, !gpio_get_value(pin));
>
> instead? From the deleted line I suppose you guess to invert the GPIO value,
> but using ~ will invert all the bits of the integer returned by
> gpio_get_value(), meaning that if it returns 1 you will end up calling
> gpio_set_value(-2). In practice your gpio is very unlikely to ever be set to
> 0.

You're right, fixed this.

>>         free_irq(IRQ_IXP4XX_TIMER2, (void *)dev->id);
>> +       gpio_free(dev->id);
>
>
> Just wondering, is there a reason for not using the devm_gpio functions
> here?

I didn't want to change the style of the whole driver, but I can make
a separate patch switching the whole driver to use managed
resources at some later point.

Yours,
Linus Walleij



More information about the linux-arm-kernel mailing list