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

[Simon Glass]

Hi John,

On 21 July 2016 at 04:46, John Keeping <john at metanate.com> 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 at metanate.com>
> ---
>
>  drivers/pinctrl/rockchip/pinctrl_rk3288.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pinctrl/rockchip/pinctrl_rk3288.c b/drivers/pinctrl/rockchip/pinctrl_rk3288.c
> index 8cb3b82..ead4a67 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,10 @@ 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);
> +
> +       value = readl(addr);
> +       value |= (mask << (shift + 16)) | (muxval << shift);
> +       writel(value, addr);
>
>         /* Handle pullup/pulldown */
>         if (flags) {
> @@ -615,7 +619,10 @@ 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);
> +
> +               value = readl(addr);
> +               value |= (3 << (shift + 16)) | (val << shift);
> +               writel(value, addr);

This looks fine and thanks for the fix. But can you please add a short
comment at each site explaining why we cannot use rk_clrsetreg()?

>         }
>
>         return 0;
> --
> 2.9.0.465.g8850cbc.dirty
>

Regards,
Simon