lkml.org 
[lkml]   [2013]   [Jan]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH 6/6 v14] gpio: Add block gpio to several gpio drivers
From
On Thu, Jan 24, 2013 at 1:19 PM, Roland Stigge <stigge@antcom.de> wrote:
> On 01/24/2013 01:02 PM, Stijn Devriendt wrote:
>>> +static void lpc32xx_gpio_set_block_p3(struct gpio_chip *chip,
>>> + unsigned long mask,
>>> + unsigned long values)
>>> +{
>>> + struct lpc32xx_gpio_chip *group = to_lpc32xx_gpio(chip);
>>> + u32 set_bits = values & mask;
>>> + u32 clr_bits = ~values & mask;
>>> +
>>> + /* States of GPIO 0-5 start at bit 25 */
>>> + set_bits <<= 25;
>>> + clr_bits <<= 25;
>>> +
>>> + /* Note: On LPC32xx, GPOs can only be set at once or cleared at once,
>>> + * but not set and cleared at once
>>> + */
>>> + if (set_bits)
>>> + __raw_writel(set_bits, group->gpio_grp->outp_set);
>>> + if (clr_bits)
>>> + __raw_writel(clr_bits, group->gpio_grp->outp_clr);
>>> +}
>>> +
>>
>> In my patch, I go out of the way of this kind of thing for a simple reason:
>> You may generate incorrect timing by doing this.
>
> You are right, certain things like synchronous on+off is not really
> possible.
>
> However, the above at least supports switching on simulaneously, and
> switching off simultaneously, which is an improvement in certain cases
> (and this certain hardware part doesn't support more). Maybe this
> certain driver behaviour can be documented even better than just in the
> driver source.
>

The question here is: do you expect a user of the block-GPIO API to
go look into the base-driver code to see what will be supported?

In my version of the patch this means:
- do not provide a single GPIO-block that crosses multiple base-drivers
- only provide gpio_block_get/set for GPIO drivers that support the complete
operation in a single go. (for example, in the above example there would be
no gpio_block_set() function)

Perhaps the best approach is to make this explicit: Allow drivers to expose
their capabilities wrt timing and allow users to request strict-timing or
loose-timing. Loose-timing allows multiple gpio-drivers to be combined and
allows drivers with separate set/clear, hi/lo registers to be used.
Of course, for a first version you may as well leave it out. Perhaps the
use-cases for cross-GPIO-driver blocks are not worth the extra complexity
as of today?

Regards,
Stijn

> If there is consensus that everything else in the patch set is settled,
> and we just don't want to support behaviour like the above - well, we
> can still drop it.
>
> Thanks for the note,
>
> Roland
>


\
 
 \ /
  Last update: 2013-01-27 13:41    [W:2.736 / U:1.116 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site