Re: [PATCH 02/15] GPIO: port LoCoMo gpio support from old driver

[Dmitry Eremin-Solenikov <dbaryshkov@xxxxxxxxx>]

2014-10-31 10:48 GMT+03:00 Linus Walleij <linus.walleij@xxxxxxxxxx>:
> On Tue, Oct 28, 2014 at 1:01 AM, Dmitry Eremin-Solenikov
> <dbaryshkov@xxxxxxxxx> wrote:
>
>> Add gpiolib driver for gpio pins placed on the LoCoMo GA.
>>
>> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@xxxxxxxxx>
>

[skipped]

> (etc, everywhere this pattern occurs).
>> +static void locomo_gpio_set(struct gpio_chip *chip,
>> +               unsigned offset, int value)
>> +{
>> +       struct locomo_gpio *lg = container_of(chip, struct locomo_gpio, gpio);
>> +       unsigned long flags;
>> +
>> +       spin_lock_irqsave(&lg->lock, flags);
>> +
>> +       __locomo_gpio_set(chip, offset, value);
>> +
>> +       spin_unlock_irqrestore(&lg->lock, flags);
>
> If you actually always have to be getting and releasing a spin lock around
> the register writes, contemplate using regmap-mmio because that
> is part of what it does.
>
> But is this locking really necessary?

I have a custom of doing such locking and never having to think about
somebody breaking into RMW cycles.

Also isn't regmap an overkill here? Wouldn't regmap also do a lock/unlock
around each register read/write/RMW?

>> +static int locomo_gpio_remove(struct platform_device *pdev)
>> +{
>> +       struct locomo_gpio *lg = platform_get_drvdata(pdev);
>> +       int ret;
>> +
>> +       ret = gpiochip_remove(&lg->gpio);
>> +       if (ret) {
>> +               dev_err(&pdev->dev, "Can't remove gpio chip: %d\n", ret);
>> +               return ret;
>> +       }
>
> The return value from gpiochip_remove() has been removed in v3.18-rc1
> so this will not compile.

Yes, the fix will be in the next iteration. This patchset was based on 3.17

-- 
With best wishes
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html