* Grygorii.Strashko@xxxxxxxxxx <grygorii.strashko@xxxxxxxxxx> [150423 04:13]:
> On 04/21/2015 07:08 PM, Tony Lindgren wrote:
> > @@ -438,11 +447,30 @@ static void omap_enable_gpio_module(struct gpio_bank *bank, unsigned offset)
> > writel_relaxed(ctrl, reg);
> > bank->context.ctrl = ctrl;
> > }
> > +
> > + if (is_irq) {
> > + omap_set_gpio_direction(bank, offset, 1);
> > + bank->irq_usage |= BIT(offset);
> > + } else {
> > + omap_set_gpio_triggering(bank, offset, IRQ_TYPE_NONE);
> > + bank->mod_usage |= BIT(offset);
> > + }
>
> The OMAP GPIO driver implements two Core interfaces IRQ-chip and GPIO-chip which, in general,
> more or less independent.
>
> So, I don't think, that it's good to mix GPIO-IRQ-chip specific code with GPIO-chip code.
> And this even don't really correspond the purpose of omap_enable_gpio_module() :( and might
> introduce misunderstanding of code. The worst thing is that future fixes in IRQ-chip may
> affect on on GPIO-chip and vise versa :(
Hmm I'm thinking omap_enable/disable_gpio_module() eventually becomes
our runtime_resume/suspend(). Currently the enabling and disabling is
buggy for GPIO for some corner cases.. AFAIK the only difference between
enabling GPIO vs GPIO-IRQ is the calling of omap_set_gpio_direction
vs omap_set_gpio_triggering. Or at least that's the way it should be
unless I'm missing something?
> Could we keep omap_xxx_gpio_module() functions responsible only for GPIO bank PM and
> enabling/disabling?
If you're thinking about just thinking about having separate wrappers around
it like this::
static void omap_enable_gpio_module(struct gpio_bank *bank, unsigned offset,
bool is_irq)
{
...
}
static void omap_enable_gpio((struct gpio_bank *bank, unsigned offset)
{
omap_enable_gpio_module(bpio_bank, offset, 0);
}
static void omap_enable_gpio_irq((struct gpio_bank *bank, unsigned offset)
{
omap_enable_gpio_module(bpio_bank, offset, 1);
}
Then yes makes sense to me. Or do you have something else in mind?
Regards,
Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
