[U-Boot] [PATCH v2] rockchip: rk3288: Fix pinctrl for GPIO bank 0

[Kever Yang <kever.yang@rock-chips.com>]

Hi John,

On 07/25/2016 05:02 PM, John Keeping wrote:
> Bank 0 is the "PMU GPIO" bank which is controlled by the PMU registers
> rather than the GRF registers.  In the GRF the top half of the register
> is used as a mask so that some bits can be updated without affecting the
> others, but in the PMU this feature is not provided and the top half of
> the register is reserved.
>
> Take the same approach as the Linux driver to update the value via
> read-modify-write but setting the mask for only the bits that have
> changed.  The PMU registers ignore the top 16 bits so this works for
> both GRF and PMU iomux registers.
>
> Signed-off-by: John Keeping <john@metanate.com>
>
> ---
>
> Changes in v2:
> - Clear mask before setting muxval
> - Add comments explaining why rk_clrsetreg() can't be used
>
>   drivers/pinctrl/rockchip/pinctrl_rk3288.c | 21 +++++++++++++++++++--
>   1 file changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pinctrl/rockchip/pinctrl_rk3288.c b/drivers/pinctrl/rockchip/pinctrl_rk3288.c
> index 8cb3b82..0cdaac0 100644
> --- a/drivers/pinctrl/rockchip/pinctrl_rk3288.c
> +++ b/drivers/pinctrl/rockchip/pinctrl_rk3288.c
> @@ -589,6 +589,7 @@ static int rk3288_pinctrl_set_pins(struct udevice *dev, int banknum, int index,
>   	struct rk3288_pinctrl_priv *priv = dev_get_priv(dev);
>   	uint shift, ind = index;
>   	uint mask;
> +	uint value;
>   	u32 *addr;
>   	int ret;
>   
> @@ -597,7 +598,18 @@ static int rk3288_pinctrl_set_pins(struct udevice *dev, int banknum, int index,
>   					  &mask);
>   	if (ret)
>   		return ret;
> -	rk_clrsetreg(addr, mask << shift, muxval << shift);
> +
> +	/*
> +	 * PMU_GPIO0 registers cannot be selectively written so we cannot use
> +	 * rk_clrsetreg() here.  However, the upper 16 bits are reserved and
> +	 * are ignored when written, so we can use the same code as for the
> +	 * other GPIO banks providing that we preserve the value of the other
> +	 * bits.
> +	 */
> +	value = readl(addr);
> +	value &= ~(mask << shift);
> +	value |= (mask << (shift + 16)) | (muxval << shift);
> +	writel(value, addr);
>   
>   	/* Handle pullup/pulldown */
>   	if (flags) {
> @@ -615,7 +627,12 @@ static int rk3288_pinctrl_set_pins(struct udevice *dev, int banknum, int index,
>   			addr = &priv->grf->gpio1_p[banknum - 1][ind];
>   		debug("%s: addr=%p, val=%x, shift=%x\n", __func__, addr, val,
>   		      shift);
> -		rk_clrsetreg(addr, 3 << shift, val << shift);
> +
> +		/* As above, rk_clrsetreg() cannot be used here. */
> +		value = readl(addr);
> +		value &= ~(mask << shift);
> +		value |= (3 << (shift + 16)) | (val << shift);
> +		writel(value, addr);
>   	}
>   
>   	return 0;
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>

Thanks,
- Kever