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

[Linus Walleij <linus.walleij@xxxxxxxxxx>]

On Fri, Oct 31, 2014 at 10:39 AM, Dmitry Eremin-Solenikov
<dbaryshkov@xxxxxxxxx> wrote:
> 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?

Yes that's the point: if you use regmap mmio you get the RMW-locking
for free, with the regmap implementation.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html