On Wednesday 07 March 2012 12:15 PM, Tarun Kanti DebBarma wrote:
> In gpio_get(), _get_gpio_datain() and _get_gpio_dataout() get rid of
> un-necessary operation to compute gpio mask. The gpio offset passed
> to gpio_get() is sufficient to do that.
>
> Here is Russell's original comment:
> 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?
>
> 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 ?
>
> Reported-by: Russell King - ARM Linux <linux@xxxxxxxxxxxxxxxx>
> Signed-off-by: Tarun Kanti DebBarma <tarun.kanti@xxxxxx>
Looks good.
Reviewed-by: Santosh Shilimkar <santosh.shilimkar@xxxxxx>
Regards
Santosh
--
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
