* [U-Boot] [PATCH v2] rockchip: rk3288: Fix pinctrl for GPIO bank 0
@ 2016-07-25 9:02 John Keeping
2016-07-25 9:16 ` Kever Yang
0 siblings, 1 reply; 3+ messages in thread
From: John Keeping @ 2016-07-25 9:02 UTC (permalink / raw)
To: u-boot
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;
--
2.9.0.465.g8850cbc.dirty
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [U-Boot] [PATCH v2] rockchip: rk3288: Fix pinctrl for GPIO bank 0
2016-07-25 9:02 [U-Boot] [PATCH v2] rockchip: rk3288: Fix pinctrl for GPIO bank 0 John Keeping
@ 2016-07-25 9:16 ` Kever Yang
2016-07-28 3:33 ` Simon Glass
0 siblings, 1 reply; 3+ messages in thread
From: Kever Yang @ 2016-07-25 9:16 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* [U-Boot] [PATCH v2] rockchip: rk3288: Fix pinctrl for GPIO bank 0
2016-07-25 9:16 ` Kever Yang
@ 2016-07-28 3:33 ` Simon Glass
0 siblings, 0 replies; 3+ messages in thread
From: Simon Glass @ 2016-07-28 3:33 UTC (permalink / raw)
To: u-boot
On 25 July 2016 at 03:16, Kever Yang <kever.yang@rock-chips.com> wrote:
> 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
Applied to u-boot-rockchip, thanks!
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-07-28 3:33 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-25 9:02 [U-Boot] [PATCH v2] rockchip: rk3288: Fix pinctrl for GPIO bank 0 John Keeping
2016-07-25 9:16 ` Kever Yang
2016-07-28 3:33 ` Simon Glass
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.