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

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

[John Keeping]

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>
---

 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);
 	}
 
 	return 0;
-- 
2.9.0.465.g8850cbc.dirty

[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@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@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