Re: [PATCH 05/11] [POWERPC] qe_lib: support for gpio_set_dedicated

[Timur Tabi <timur@freescale.com>]

Anton Vorontsov wrote:

> +static int qe_gpio_set_dedicated(struct gpio_chip *gc, unsigned int gpio,
> +				 int func)
> +{
> +	struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
> +	struct qe_gpio_chip *qe_gc = to_qe_gpio_chip(mm_gc);
> +	struct port_regs __iomem *regs = mm_gc->regs;
> +	struct port_regs *sregs = &qe_gc->saved_regs;
> +	unsigned long flags;
> +	u32 mask1 = 1 << (NUM_OF_PINS - (gpio + 1));
> +	u32 mask2 = 0x3 << (NUM_OF_PINS - (gpio % (NUM_OF_PINS / 2) + 1) * 2);
> +	bool second_reg = gpio > (NUM_OF_PINS / 2) - 1;
> +
> +	spin_lock_irqsave(&qe_gc->lock, flags);
> +
> +	if (second_reg)
> +		clrsetbits_be32(&regs->cpdir2, mask2, sregs->cpdir2 & mask2);
> +	else
> +		clrsetbits_be32(&regs->cpdir1, mask2, sregs->cpdir1 & mask2);
> +
> +	if (second_reg)
> +		clrsetbits_be32(&regs->cppar2, mask2, sregs->cppar2 & mask2);
> +	else
> +		clrsetbits_be32(&regs->cppar1, mask2, sregs->cppar1 & mask2);

You could combine these into one if-statement.

I took a quick look at all of your QE lib patches.  They generally look okay, 
but I haven't examined them thoroughly enough to formally ACK them.

-- 
Timur Tabi
Linux kernel developer at Freescale