Re: [PATCH v3 4/8] pinctrl: sunxi: support moved power configuration registers

["Jernej Škrabec" <jernej.skrabec@gmail.com>]

Dne petek, 28. februar 2025 ob 00:14:43 Srednjeevropski standardni čas je Andre Przywara napisal(a):
> The Allwinner pincontroller IP features some registers to control the
> withstand voltage of each pin group. So far those registers were always
> located at the same offset, but the A523 SoC has moved them (probably to
> accommodate all eleven pin banks).
> 
> Add a flag to note this feature, and use that to program the registers
> either at offset 0x340 or 0x380. So far no pincontroller driver uses
> this flag, but we need it for the upcoming A523 support.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  drivers/pinctrl/sunxi/pinctrl-sunxi.c | 15 +++++++++++----
>  drivers/pinctrl/sunxi/pinctrl-sunxi.h |  7 +++++--
>  2 files changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> index 83a031ceb29f2..fc12e6f807e4d 100644
> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> @@ -736,9 +736,9 @@ static int sunxi_pinctrl_set_io_bias_cfg(struct sunxi_pinctrl *pctl,
>  		val = uV > 1800000 && uV <= 2500000 ? BIT(bank) : 0;
>  
>  		raw_spin_lock_irqsave(&pctl->lock, flags);
> -		reg = readl(pctl->membase + PIO_POW_MOD_CTL_REG);
> +		reg = readl(pctl->membase + pctl->pow_mod_sel_offset);
>  		reg &= ~BIT(bank);
> -		writel(reg | val, pctl->membase + PIO_POW_MOD_CTL_REG);
> +		writel(reg | val, pctl->membase + pctl->pow_mod_sel_offset);

These two are missing "+ PIO_POW_MOD_CTL_OFS" right?

>  		raw_spin_unlock_irqrestore(&pctl->lock, flags);
>  
>  		fallthrough;
> @@ -746,9 +746,12 @@ static int sunxi_pinctrl_set_io_bias_cfg(struct sunxi_pinctrl *pctl,
>  		val = uV <= 1800000 ? 1 : 0;
>  
>  		raw_spin_lock_irqsave(&pctl->lock, flags);
> -		reg = readl(pctl->membase + PIO_POW_MOD_SEL_REG);
> +		reg = readl(pctl->membase + pctl->pow_mod_sel_offset +
> +			    PIO_POW_MOD_CTL_OFS);
>  		reg &= ~(1 << bank);
> -		writel(reg | val << bank, pctl->membase + PIO_POW_MOD_SEL_REG);
> +		writel(reg | val << bank,
> +		       pctl->membase + pctl->pow_mod_sel_offset +
> +		       PIO_POW_MOD_CTL_OFS);

And these two have "+ PIO_POW_MOD_CTL_OFS" too much, right?

Best regards,
Jernej

>  		raw_spin_unlock_irqrestore(&pctl->lock, flags);
>  		return 0;
>  	default:
> @@ -1520,6 +1523,10 @@ int sunxi_pinctrl_init_with_flags(struct platform_device *pdev,
>  		pctl->pull_regs_offset = PULL_REGS_OFFSET;
>  		pctl->dlevel_field_width = DLEVEL_FIELD_WIDTH;
>  	}
> +	if (flags & SUNXI_PINCTRL_ELEVEN_BANKS)
> +		pctl->pow_mod_sel_offset = PIO_11B_POW_MOD_SEL_REG;
> +	else
> +		pctl->pow_mod_sel_offset = PIO_POW_MOD_SEL_REG;
>  
>  	pctl->irq_array = devm_kcalloc(&pdev->dev,
>  				       IRQ_PER_BANK * pctl->desc->irq_banks,
> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.h b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
> index 6cf721876d89d..742fc795c7664 100644
> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.h
> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
> @@ -87,9 +87,11 @@
>  #define SUNXI_PINCTRL_VARIANT_MASK	GENMASK(7, 0)
>  #define SUNXI_PINCTRL_NEW_REG_LAYOUT	BIT(8)
>  #define SUNXI_PINCTRL_PORTF_SWITCH	BIT(9)
> +#define SUNXI_PINCTRL_ELEVEN_BANKS	BIT(10)
>  
> -#define PIO_POW_MOD_SEL_REG	0x340
> -#define PIO_POW_MOD_CTL_REG	0x344
> +#define PIO_POW_MOD_SEL_REG		0x340
> +#define PIO_11B_POW_MOD_SEL_REG		0x380
> +#define PIO_POW_MOD_CTL_OFS		0x004
>  
>  #define PIO_BANK_K_OFFSET		0x500
>  
> @@ -173,6 +175,7 @@ struct sunxi_pinctrl {
>  	u32				bank_mem_size;
>  	u32				pull_regs_offset;
>  	u32				dlevel_field_width;
> +	u32				pow_mod_sel_offset;
>  };
>  
>  #define SUNXI_PIN(_pin, ...)					\
>