Cyril Chemparathy <[email protected]> writes:
> This patch adds support for the tnetv107x gpio controller.
>
> Key differences between davinci and tnetv107x controllers:
> - register map - davinci's controller is organized into banks of 32 gpios,
> tnetv107x has a single space with arrays of registers for in, out,
> direction, etc.
> - davinci's controller has separate set/clear registers for output, tnetv107x
> has a single direct mapped register.
>
> This patch does not yet add gpio irq support on this controller.
>
> Signed-off-by: Cyril Chemparathy <[email protected]>
I know Sergei isn't used to me agreeing with him ;) but I tend to
agree with him on the increased bulk of gpio_set_value().
It's not simply the increased size that is an issue here. The point
of these inlines (as the comment suggests) is for low-overhead
bit-banging. With the introduction of different methods depending on
GPIO type, as well as the locking for the tnetv style, this is no
longer low-overhead in my mind.
Below is an alternate solution.
[...]
> @@ -97,14 +98,30 @@ static inline void gpio_set_value(unsigned gpio, int
> value)
> {
> if (__builtin_constant_p(value) && gpio < davinci_soc_info.gpio_num) {
Here add '&& (ctlr->set_data != ctlr->clr_data)' to the if clause
> struct davinci_gpio_controller *ctlr;
> - u32 mask;
> + u32 mask, data;
> + unsigned long flags;
>
> ctlr = __gpio_to_controller(gpio);
> mask = __gpio_mask(gpio);
> - if (value)
> - __raw_writel(mask, ctlr->set_data);
> - else
> - __raw_writel(mask, ctlr->clr_data);
and then this part can stay as is for the traditional style...
> +
> + if (ctlr->set_data != ctlr->clr_data) {
> + /* traditional set/clear registers */
> + if (value)
> + __raw_writel(mask, ctlr->set_data);
> + else
> + __raw_writel(mask, ctlr->clr_data);
> + } else {
> + /* tnetv107x style single out register */
> + spin_lock_irqsave(&ctlr->lock, flags);
> + data = __raw_readl(ctlr->set_data);
> + if (value)
> + data |= mask;
> + else
> + data &= ~mask;
> + __raw_writel(data, ctlr->set_data);
> + spin_unlock_irqrestore(&ctlr->lock, flags);
> + }
> +
> return;
> }
Then just drop all of this part and let the tnetv-style fall through
to the __gpio_set_value() which will call the GPIO chip's ->set()
method.
Kevin
_______________________________________________
Davinci-linux-open-source mailing list
[email protected]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
