Re: [PATCH v9 08/15] gpio: pl061: convert register access to regmap

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Thu, Oct 01, 2026 at 08:40:57PM +0800, Long Zhao via B4 Relay wrote:

> Replace readb()/writeb() with a MMIO regmap while keeping the existing
> gpio_chip and girq irqchip. Use a raw spinlock so irq_ack() can run
> under the irq descriptor lock.

...

>  static int pl061_get_direction(struct gpio_chip *gc, unsigned offset)
>  {
>  	struct pl061 *pl061 = gpiochip_get_data(gc);
> +	unsigned int gpiodir = 0;
>  
> -	if (readb(pl061->base + GPIODIR) & BIT(offset))
> +	regmap_read(pl061->regmap, GPIODIR, &gpiodir);

Why not returning an error?

> +	if (gpiodir & BIT(offset))
>  		return GPIO_LINE_DIRECTION_OUT;
>  
>  	return GPIO_LINE_DIRECTION_IN;

I believe this can use regmap_test_bits().

...

> static int pl061_direction_input(struct gpio_chip *gc, unsigned offset)
>  {
>  	struct pl061 *pl061 = gpiochip_get_data(gc);
>  	unsigned long flags;
> -	unsigned char gpiodir;
>  
>  	raw_spin_lock_irqsave(&pl061->lock, flags);
> -	gpiodir = readb(pl061->base + GPIODIR);
> -	gpiodir &= ~(BIT(offset));
> -	writeb(gpiodir, pl061->base + GPIODIR);
> +	regmap_update_bits(pl061->regmap, GPIODIR, BIT(offset), 0);

_clear_bits()

>  	raw_spin_unlock_irqrestore(&pl061->lock, flags);
>  
>  	return 0;

...

> static int pl061_direction_output(struct gpio_chip *gc, unsigned offset,
>  {
>  	struct pl061 *pl061 = gpiochip_get_data(gc);
>  	unsigned long flags;
> -	unsigned char gpiodir;
> +	unsigned int mask = BIT(offset);
>  
>  	raw_spin_lock_irqsave(&pl061->lock, flags);
> -	writeb(!!value << offset, pl061->base + (BIT(offset + 2)));
> -	gpiodir = readb(pl061->base + GPIODIR);
> -	gpiodir |= BIT(offset);
> -	writeb(gpiodir, pl061->base + GPIODIR);
> +	regmap_write(pl061->regmap, BIT(offset + PL061_DATA_OFFSET),
> +		     !!value << offset);

_set_bits()

> +	regmap_update_bits(pl061->regmap, GPIODIR, mask, mask);
>  
>  	/*
>  	 * gpio value is set again, because pl061 doesn't allow to set value of
>  	 * a gpio pin before configuring it in OUT mode.
>  	 */
> -	writeb(!!value << offset, pl061->base + (BIT(offset + 2)));
> +	regmap_write(pl061->regmap, BIT(offset + PL061_DATA_OFFSET),
> +		     !!value << offset);

Ditto.

>  	raw_spin_unlock_irqrestore(&pl061->lock, flags);
>  
>  	return 0;

...

> static int pl061_direction_output(struct gpio_chip *gc, unsigned offset,
>  static int pl061_get_value(struct gpio_chip *gc, unsigned offset)
>  {
>  	struct pl061 *pl061 = gpiochip_get_data(gc);
> +	unsigned int val = 0;
> +
> +	regmap_read(pl061->regmap, BIT(offset + PL061_DATA_OFFSET), &val);
>  
> -	return !!readb(pl061->base + (BIT(offset + 2)));
> +	return !!val;

_test_bits()

>  }

...

...and so on...

-- 
With Best Regards,
Andy Shevchenko