Andreas Fenkart <andreas.fenkart@xxxxxxxxxxxxxxxxxxx> writes: > By also making it return the modified value, we save the readl > needed to update the context. > > Signed-off-by: Andreas Fenkart <andreas.fenkart@xxxxxxxxxxxxxxxxxxx> Great cleanups, Thanks! Some minor comments below... Also, it would be nice to see a cover letter for this series describing how it was tested, on what platforms, did it include PM testing (off-mode, etc.). [...] > @@ -94,20 +107,10 @@ static int irq_to_gpio(struct gpio_bank *bank, unsigned int gpio_irq) > > static void _set_gpio_direction(struct gpio_bank *bank, int gpio, int is_input) > { > - void __iomem *reg = bank->base; > - u32 l; > - > - reg += bank->regs->direction; > - l = __raw_readl(reg); > - if (is_input) > - l |= 1 << gpio; > - else > - l &= ~(1 << gpio); > - __raw_writel(l, reg); > - bank->context.oe = l; > + bank->context.oe = _gpio_rmw(bank->base, bank->regs->direction, 1 << > + gpio, is_input); wrapping is funny here, IMO the "1 <<" should be on the next line along with "gpio". [...] > @@ -450,19 +428,16 @@ static int gpio_irq_type(struct irq_data *d, unsigned type) > > static void _clear_gpio_irqbank(struct gpio_bank *bank, int gpio_mask) > { > - void __iomem *reg = bank->base; > + void __iomem *base = bank->base; > > - reg += bank->regs->irqstatus; > - __raw_writel(gpio_mask, reg); > + __raw_writel(gpio_mask, base + bank->regs->irqstatus); > > /* Workaround for clearing DSP GPIO interrupts to allow retention */ > - if (bank->regs->irqstatus2) { > - reg = bank->base + bank->regs->irqstatus2; > - __raw_writel(gpio_mask, reg); > - } > + if (bank->regs->irqstatus2) > + __raw_writel(gpio_mask, base + bank->regs->irqstatus2); > > /* Flush posted write for the irq status to avoid spurious interrupts */ > - __raw_readl(reg); > + __raw_readl(base + bank->regs->irqstatus); All of the changes in this function are not related to the change described in the changelog. Either make a separate patch, or add a description of this cleanup to the changelog too. Thanks, Kevin -- 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