On Sat, Feb 25, 2012 at 6:34 PM, Russell King - ARM Linux
<linux@xxxxxxxxxxxxxxxx> wrote:
> Can someone explain to me this:
>
> #define GPIO_INDEX(bank, gpio) (gpio % bank->width)
> #define GPIO_BIT(bank, gpio) (1 << GPIO_INDEX(bank, gpio))
>
> static int _get_gpio_datain(struct gpio_bank *bank, int gpio)
> {
> void __iomem *reg = bank->base + bank->regs->datain;
>
> return (__raw_readl(reg) & GPIO_BIT(bank, gpio)) != 0;
> }
>
> static int gpio_get(struct gpio_chip *chip, unsigned offset)
> {
> struct gpio_bank *bank = container_of(chip, struct gpio_bank, chip);
> void __iomem *reg = bank->base;
> int gpio = chip->base + offset;
> u32 mask = GPIO_BIT(bank, gpio);
>
> if (gpio_is_input(bank, mask))
> return _get_gpio_datain(bank, gpio);
> else
> return _get_gpio_dataout(bank, gpio);
> }
>
> Given that bank->width on OMAP is either 32 or 16, and GPIO numbers for
> any GPIO chip are always aligned to 32 or 16, why does this code bother
> adding the chips base gpio number and then modulo the width?
As far as I understand we could very well use offset directly.
The macro is used to derive the same offset value in this case.
Therefore, _get_gpio_datain(bank, gpio) and _get_gpio_dataout(bank, gpio)
can be passed offset instead of (bank->base + offset) as suggested by you below.
And of course, offset can be used to derive mask both here and in the
called functions.
>
> Surely this means if - for argument sake - you registered a GPIO chip
> with 8 lines followed by one with 16 lines, GPIO0..7 would be chip 0
> bit 0..7, GPIO8..15 would be chip 1 bit 8..15, GPIO16..23 would be
> chip 1 bit 0..7.
>
> However, if you registered a GPIO chip with 16 lines first, it would
> mean GPIO0..15 would be chip 0 bit 0..15, and GPIO16..31 would be
> chip 1 bit 0..15.
>
> Surely this kind of behaviour is not intended?
>
> Is there a reason why the bitmask can't just be (1 << offset) where
> offset is passed into these functions as GPIO number - chip->base ?
Yes, I have made the following changes and tested on OMAP4 and OMAP1(bootup).
[PATCH] gpio/omap: remove redundant decoding of gpio offset
---
drivers/gpio/gpio-omap.c | 18 +++++++-----------
1 files changed, 7 insertions(+), 11 deletions(-)
diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 5953ece..2196747 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -133,18 +133,18 @@ static void _set_gpio_dataout_mask(struct
gpio_bank *bank, int gpio, int enable)
bank->context.dataout = l;
}
-static int _get_gpio_datain(struct gpio_bank *bank, int gpio)
+static int _get_gpio_datain(struct gpio_bank *bank, int offset)
{
void __iomem *reg = bank->base + bank->regs->datain;
- return (__raw_readl(reg) & GPIO_BIT(bank, gpio)) != 0;
+ return (__raw_readl(reg) & (1 << offset)) != 0;
}
-static int _get_gpio_dataout(struct gpio_bank *bank, int gpio)
+static int _get_gpio_dataout(struct gpio_bank *bank, int offset)
{
void __iomem *reg = bank->base + bank->regs->dataout;
- return (__raw_readl(reg) & GPIO_BIT(bank, gpio)) != 0;
+ return (__raw_readl(reg) & (1 << offset)) != 0;
}
static inline void _gpio_rmw(void __iomem *base, u32 reg, u32 mask, bool set)
@@ -849,19 +849,15 @@ static int gpio_is_input(struct gpio_bank *bank, int mask)
static int gpio_get(struct gpio_chip *chip, unsigned offset)
{
struct gpio_bank *bank;
- void __iomem *reg;
- int gpio;
u32 mask;
- gpio = chip->base + offset;
bank = container_of(chip, struct gpio_bank, chip);
- reg = bank->base;
- mask = GPIO_BIT(bank, gpio);
+ mask = (1 << offset);
if (gpio_is_input(bank, mask))
- return _get_gpio_datain(bank, gpio);
+ return _get_gpio_datain(bank, offset);
else
- return _get_gpio_dataout(bank, gpio);
+ return _get_gpio_dataout(bank, offset);
}
static int gpio_output(struct gpio_chip *chip, unsigned offset, int value)
--
1.7.0.4
--
Tarun
> --
> 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
--
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
