diff mbox

[U-Boot,v2,29/56] rockchip: rk3368: grf: use shifted-constants

Message ID 1501065662-52029-30-git-send-email-philipp.tomsich@theobroma-systems.com
State Superseded
Delegated to: Philipp Tomsich
Headers show

Commit Message

Philipp Tomsich July 26, 2017, 10:40 a.m. UTC
The RK3368 GRF header was still defines with a shifted-mask but with
non-shifted function selectors for the IOMUX defines.  As the RK3368
support is still fresh enough to allow a quick change, we do this now
before having more code use this.

Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>

---

Changes in v2:
- dropped the RK3368_ prefix for the GRF constants

 arch/arm/include/asm/arch-rockchip/grf_rk3368.h | 441 ++++++++++++------------
 drivers/pinctrl/rockchip/pinctrl_rk3368.c       |   9 +-
 2 files changed, 226 insertions(+), 224 deletions(-)

Comments

Simon Glass July 28, 2017, 3:38 a.m. UTC | #1
Hi Philipp,

On 26 July 2017 at 04:40, Philipp Tomsich
<philipp.tomsich@theobroma-systems.com> wrote:
> The RK3368 GRF header was still defines with a shifted-mask but with
> non-shifted function selectors for the IOMUX defines.  As the RK3368
> support is still fresh enough to allow a quick change, we do this now
> before having more code use this.
>
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
>
> ---
>
> Changes in v2:
> - dropped the RK3368_ prefix for the GRF constants
>
>  arch/arm/include/asm/arch-rockchip/grf_rk3368.h | 441 ++++++++++++------------
>  drivers/pinctrl/rockchip/pinctrl_rk3368.c       |   9 +-
>  2 files changed, 226 insertions(+), 224 deletions(-)
>
> diff --git a/arch/arm/include/asm/arch-rockchip/grf_rk3368.h b/arch/arm/include/asm/arch-rockchip/grf_rk3368.h
> index a438f5d..1966960 100644
> --- a/arch/arm/include/asm/arch-rockchip/grf_rk3368.h
> +++ b/arch/arm/include/asm/arch-rockchip/grf_rk3368.h
> @@ -1,4 +1,6 @@
> -/* (C) Copyright 2016 Rockchip Electronics Co., Ltd
> +/*
> + * (C) Copyright 2016 Rockchip Electronics Co., Ltd
> + * (C) Copyright 2017 Theobroma Systems Design und Consulting GmbH
>   *
>   * SPDX-License-Identifier:     GPL-2.0+
>   */
> @@ -100,315 +102,318 @@ check_member(rk3368_pmu_grf, os_reg[0], 0x200);
>
>  /*GRF_GPIO0C_IOMUX*/
>  enum {
> -       GPIO0C7_SHIFT           = 14,
> -       GPIO0C7_MASK            = 3 << GPIO0C7_SHIFT,
> -       GPIO0C7_GPIO            = 0,
> -       GPIO0C7_LCDC_D19,
> -       GPIO0C7_TRACE_D9,
> -       GPIO0C7_UART1_RTSN,
> -
> -       GPIO0C6_SHIFT           = 12,
> -       GPIO0C6_MASK            = 3 << GPIO0C6_SHIFT,
> +       GPIO0C7_MASK           = GENMASK(15, 14),
> +       GPIO0C7_GPIO           = 0,
> +       GPIO0C7_LCDC_D19        = (1 << 14),
> +       GPIO0C7_TRACE_D9        = (2 << 14),
> +       GPIO0C7_UART1_RTSN      = (3 << 14),

Can we keep the SHIFT macros so we(e.g.) don't have to open-code the
'14' each time?
Philipp Tomsich July 28, 2017, 8:53 a.m. UTC | #2
> On 28 Jul 2017, at 05:38, Simon Glass <sjg@chromium.org> wrote:
> 
> Hi Philipp,
> 
> On 26 July 2017 at 04:40, Philipp Tomsich
> <philipp.tomsich@theobroma-systems.com> wrote:
>> The RK3368 GRF header was still defines with a shifted-mask but with
>> non-shifted function selectors for the IOMUX defines.  As the RK3368
>> support is still fresh enough to allow a quick change, we do this now
>> before having more code use this.
>> 
>> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
>> 
>> ---
>> 
>> Changes in v2:
>> - dropped the RK3368_ prefix for the GRF constants
>> 
>> arch/arm/include/asm/arch-rockchip/grf_rk3368.h | 441 ++++++++++++------------
>> drivers/pinctrl/rockchip/pinctrl_rk3368.c       |   9 +-
>> 2 files changed, 226 insertions(+), 224 deletions(-)
>> 
>> diff --git a/arch/arm/include/asm/arch-rockchip/grf_rk3368.h b/arch/arm/include/asm/arch-rockchip/grf_rk3368.h
>> index a438f5d..1966960 100644
>> --- a/arch/arm/include/asm/arch-rockchip/grf_rk3368.h
>> +++ b/arch/arm/include/asm/arch-rockchip/grf_rk3368.h
>> @@ -1,4 +1,6 @@
>> -/* (C) Copyright 2016 Rockchip Electronics Co., Ltd
>> +/*
>> + * (C) Copyright 2016 Rockchip Electronics Co., Ltd
>> + * (C) Copyright 2017 Theobroma Systems Design und Consulting GmbH
>>  *
>>  * SPDX-License-Identifier:     GPL-2.0+
>>  */
>> @@ -100,315 +102,318 @@ check_member(rk3368_pmu_grf, os_reg[0], 0x200);
>> 
>> /*GRF_GPIO0C_IOMUX*/
>> enum {
>> -       GPIO0C7_SHIFT           = 14,
>> -       GPIO0C7_MASK            = 3 << GPIO0C7_SHIFT,
>> -       GPIO0C7_GPIO            = 0,
>> -       GPIO0C7_LCDC_D19,
>> -       GPIO0C7_TRACE_D9,
>> -       GPIO0C7_UART1_RTSN,
>> -
>> -       GPIO0C6_SHIFT           = 12,
>> -       GPIO0C6_MASK            = 3 << GPIO0C6_SHIFT,
>> +       GPIO0C7_MASK           = GENMASK(15, 14),
>> +       GPIO0C7_GPIO           = 0,
>> +       GPIO0C7_LCDC_D19        = (1 << 14),
>> +       GPIO0C7_TRACE_D9        = (2 << 14),
>> +       GPIO0C7_UART1_RTSN      = (3 << 14),
> 
> Can we keep the SHIFT macros so we(e.g.) don't have to open-code the
> '14' each time?

In fact I wanted this to stick out, so we are motivated to move towards
a model of set_pin_function(BANK_C, PIN_7, UART1) where the
shift-value can then calculated from the pin# within the bank (and the
function id is looked up from a config-table for this pin).
Simon Glass Aug. 1, 2017, 9:49 a.m. UTC | #3
On 28 July 2017 at 02:53, Dr. Philipp Tomsich
<philipp.tomsich@theobroma-systems.com> wrote:
>
>> On 28 Jul 2017, at 05:38, Simon Glass <sjg@chromium.org> wrote:
>>
>> Hi Philipp,
>>
>> On 26 July 2017 at 04:40, Philipp Tomsich
>> <philipp.tomsich@theobroma-systems.com> wrote:
>>> The RK3368 GRF header was still defines with a shifted-mask but with
>>> non-shifted function selectors for the IOMUX defines.  As the RK3368
>>> support is still fresh enough to allow a quick change, we do this now
>>> before having more code use this.
>>>
>>> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
>>>
>>> ---
>>>
>>> Changes in v2:
>>> - dropped the RK3368_ prefix for the GRF constants
>>>
>>> arch/arm/include/asm/arch-rockchip/grf_rk3368.h | 441 ++++++++++++------------
>>> drivers/pinctrl/rockchip/pinctrl_rk3368.c       |   9 +-
>>> 2 files changed, 226 insertions(+), 224 deletions(-)
>>>
>>> diff --git a/arch/arm/include/asm/arch-rockchip/grf_rk3368.h b/arch/arm/include/asm/arch-rockchip/grf_rk3368.h
>>> index a438f5d..1966960 100644
>>> --- a/arch/arm/include/asm/arch-rockchip/grf_rk3368.h
>>> +++ b/arch/arm/include/asm/arch-rockchip/grf_rk3368.h
>>> @@ -1,4 +1,6 @@
>>> -/* (C) Copyright 2016 Rockchip Electronics Co., Ltd
>>> +/*
>>> + * (C) Copyright 2016 Rockchip Electronics Co., Ltd
>>> + * (C) Copyright 2017 Theobroma Systems Design und Consulting GmbH
>>>  *
>>>  * SPDX-License-Identifier:     GPL-2.0+
>>>  */
>>> @@ -100,315 +102,318 @@ check_member(rk3368_pmu_grf, os_reg[0], 0x200);
>>>
>>> /*GRF_GPIO0C_IOMUX*/
>>> enum {
>>> -       GPIO0C7_SHIFT           = 14,
>>> -       GPIO0C7_MASK            = 3 << GPIO0C7_SHIFT,
>>> -       GPIO0C7_GPIO            = 0,
>>> -       GPIO0C7_LCDC_D19,
>>> -       GPIO0C7_TRACE_D9,
>>> -       GPIO0C7_UART1_RTSN,
>>> -
>>> -       GPIO0C6_SHIFT           = 12,
>>> -       GPIO0C6_MASK            = 3 << GPIO0C6_SHIFT,
>>> +       GPIO0C7_MASK           = GENMASK(15, 14),
>>> +       GPIO0C7_GPIO           = 0,
>>> +       GPIO0C7_LCDC_D19        = (1 << 14),
>>> +       GPIO0C7_TRACE_D9        = (2 << 14),
>>> +       GPIO0C7_UART1_RTSN      = (3 << 14),
>>
>> Can we keep the SHIFT macros so we(e.g.) don't have to open-code the
>> '14' each time?
>
> In fact I wanted this to stick out, so we are motivated to move towards
> a model of set_pin_function(BANK_C, PIN_7, UART1) where the
> shift-value can then calculated from the pin# within the bank (and the
> function id is looked up from a config-table for this pin).

That sounds like a separate issue to me. The problem here is that it
it isn't clear where the '14' comes from and what it is common with.
Well, not clear enough to permit using an editor's search/replace
function.

Regards,
Simon
Philipp Tomsich Aug. 1, 2017, 9:53 a.m. UTC | #4
> On 01 Aug 2017, at 11:49, Simon Glass <sjg@chromium.org> wrote:
> 
> On 28 July 2017 at 02:53, Dr. Philipp Tomsich
> <philipp.tomsich@theobroma-systems.com> wrote:
>> 
>>> On 28 Jul 2017, at 05:38, Simon Glass <sjg@chromium.org> wrote:
>>> 
>>> Hi Philipp,
>>> 
>>> On 26 July 2017 at 04:40, Philipp Tomsich
>>> <philipp.tomsich@theobroma-systems.com> wrote:
>>>> The RK3368 GRF header was still defines with a shifted-mask but with
>>>> non-shifted function selectors for the IOMUX defines.  As the RK3368
>>>> support is still fresh enough to allow a quick change, we do this now
>>>> before having more code use this.
>>>> 
>>>> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
>>>> 
>>>> ---
>>>> 
>>>> Changes in v2:
>>>> - dropped the RK3368_ prefix for the GRF constants
>>>> 
>>>> arch/arm/include/asm/arch-rockchip/grf_rk3368.h | 441 ++++++++++++------------
>>>> drivers/pinctrl/rockchip/pinctrl_rk3368.c       |   9 +-
>>>> 2 files changed, 226 insertions(+), 224 deletions(-)
>>>> 
>>>> diff --git a/arch/arm/include/asm/arch-rockchip/grf_rk3368.h b/arch/arm/include/asm/arch-rockchip/grf_rk3368.h
>>>> index a438f5d..1966960 100644
>>>> --- a/arch/arm/include/asm/arch-rockchip/grf_rk3368.h
>>>> +++ b/arch/arm/include/asm/arch-rockchip/grf_rk3368.h
>>>> @@ -1,4 +1,6 @@
>>>> -/* (C) Copyright 2016 Rockchip Electronics Co., Ltd
>>>> +/*
>>>> + * (C) Copyright 2016 Rockchip Electronics Co., Ltd
>>>> + * (C) Copyright 2017 Theobroma Systems Design und Consulting GmbH
>>>> *
>>>> * SPDX-License-Identifier:     GPL-2.0+
>>>> */
>>>> @@ -100,315 +102,318 @@ check_member(rk3368_pmu_grf, os_reg[0], 0x200);
>>>> 
>>>> /*GRF_GPIO0C_IOMUX*/
>>>> enum {
>>>> -       GPIO0C7_SHIFT           = 14,
>>>> -       GPIO0C7_MASK            = 3 << GPIO0C7_SHIFT,
>>>> -       GPIO0C7_GPIO            = 0,
>>>> -       GPIO0C7_LCDC_D19,
>>>> -       GPIO0C7_TRACE_D9,
>>>> -       GPIO0C7_UART1_RTSN,
>>>> -
>>>> -       GPIO0C6_SHIFT           = 12,
>>>> -       GPIO0C6_MASK            = 3 << GPIO0C6_SHIFT,
>>>> +       GPIO0C7_MASK           = GENMASK(15, 14),
>>>> +       GPIO0C7_GPIO           = 0,
>>>> +       GPIO0C7_LCDC_D19        = (1 << 14),
>>>> +       GPIO0C7_TRACE_D9        = (2 << 14),
>>>> +       GPIO0C7_UART1_RTSN      = (3 << 14),
>>> 
>>> Can we keep the SHIFT macros so we(e.g.) don't have to open-code the
>>> '14' each time?
>> 
>> In fact I wanted this to stick out, so we are motivated to move towards
>> a model of set_pin_function(BANK_C, PIN_7, UART1) where the
>> shift-value can then calculated from the pin# within the bank (and the
>> function id is looked up from a config-table for this pin).
> 
> That sounds like a separate issue to me. The problem here is that it
> it isn't clear where the '14' comes from and what it is common with.
> Well, not clear enough to permit using an editor's search/replace
> function.

Concern understood. I’ll revise.

Regards,
Philipp.
diff mbox

Patch

diff --git a/arch/arm/include/asm/arch-rockchip/grf_rk3368.h b/arch/arm/include/asm/arch-rockchip/grf_rk3368.h
index a438f5d..1966960 100644
--- a/arch/arm/include/asm/arch-rockchip/grf_rk3368.h
+++ b/arch/arm/include/asm/arch-rockchip/grf_rk3368.h
@@ -1,4 +1,6 @@ 
-/* (C) Copyright 2016 Rockchip Electronics Co., Ltd
+/*
+ * (C) Copyright 2016 Rockchip Electronics Co., Ltd
+ * (C) Copyright 2017 Theobroma Systems Design und Consulting GmbH
  *
  * SPDX-License-Identifier:     GPL-2.0+
  */
@@ -100,315 +102,318 @@  check_member(rk3368_pmu_grf, os_reg[0], 0x200);
 
 /*GRF_GPIO0C_IOMUX*/
 enum {
-	GPIO0C7_SHIFT		= 14,
-	GPIO0C7_MASK		= 3 << GPIO0C7_SHIFT,
-	GPIO0C7_GPIO		= 0,
-	GPIO0C7_LCDC_D19,
-	GPIO0C7_TRACE_D9,
-	GPIO0C7_UART1_RTSN,
-
-	GPIO0C6_SHIFT           = 12,
-	GPIO0C6_MASK            = 3 << GPIO0C6_SHIFT,
+	GPIO0C7_MASK	       = GENMASK(15, 14),
+	GPIO0C7_GPIO	       = 0,
+	GPIO0C7_LCDC_D19        = (1 << 14),
+	GPIO0C7_TRACE_D9        = (2 << 14),
+	GPIO0C7_UART1_RTSN      = (3 << 14),
+
+	GPIO0C6_MASK            = GENMASK(13, 12),
 	GPIO0C6_GPIO            = 0,
-	GPIO0C6_LCDC_D18,
-	GPIO0C6_TRACE_D8,
-	GPIO0C6_UART1_CTSN,
+	GPIO0C6_LCDC_D18        = (1 << 12),
+	GPIO0C6_TRACE_D8        = (2 << 12),
+	GPIO0C6_UART1_CTSN      = (3 << 12),
 
-	GPIO0C5_SHIFT           = 10,
-	GPIO0C5_MASK            = 3 << GPIO0C5_SHIFT,
+	GPIO0C5_MASK            = GENMASK(11, 10),
 	GPIO0C5_GPIO            = 0,
-	GPIO0C5_LCDC_D17,
-	GPIO0C5_TRACE_D7,
-	GPIO0C5_UART1_SOUT,
+	GPIO0C5_LCDC_D17        = (1 << 10),
+	GPIO0C5_TRACE_D7        = (2 << 10),
+	GPIO0C5_UART1_SOUT      = (3 << 10),
 
-	GPIO0C4_SHIFT           = 8,
-	GPIO0C4_MASK            = 3 << GPIO0C4_SHIFT,
+	GPIO0C4_MASK            = GENMASK(9, 8),
 	GPIO0C4_GPIO            = 0,
-	GPIO0C4_LCDC_D16,
-	GPIO0C4_TRACE_D6,
-	GPIO0C4_UART1_SIN,
+	GPIO0C4_LCDC_D16        = (1 << 8),
+	GPIO0C4_TRACE_D6        = (2 << 8),
+	GPIO0C4_UART1_SIN       = (3 << 8),
 
-	GPIO0C3_SHIFT           = 6,
-	GPIO0C3_MASK            = 3 << GPIO0C3_SHIFT,
+	GPIO0C3_MASK            = GENMASK(7, 6),
 	GPIO0C3_GPIO            = 0,
-	GPIO0C3_LCDC_D15,
-	GPIO0C3_TRACE_D5,
-	GPIO0C3_MCU_JTAG_TDO,
+	GPIO0C3_LCDC_D15        = (1 << 6),
+	GPIO0C3_TRACE_D5        = (2 << 6),
+	GPIO0C3_MCU_JTAG_TDO    = (3 << 6),
 
-	GPIO0C2_SHIFT           = 4,
-	GPIO0C2_MASK            = 3 << GPIO0C2_SHIFT,
+	GPIO0C2_MASK            = GENMASK(5, 4),
 	GPIO0C2_GPIO            = 0,
-	GPIO0C2_LCDC_D14,
-	GPIO0C2_TRACE_D4,
-	GPIO0C2_MCU_JTAG_TDI,
+	GPIO0C2_LCDC_D14        = (1 << 4),
+	GPIO0C2_TRACE_D4        = (2 << 4),
+	GPIO0C2_MCU_JTAG_TDI    = (3 << 4),
 
-	GPIO0C1_SHIFT           = 2,
-	GPIO0C1_MASK            = 3 << GPIO0C1_SHIFT,
+	GPIO0C1_MASK            = GENMASK(3, 2),
 	GPIO0C1_GPIO            = 0,
-	GPIO0C1_LCDC_D13,
-	GPIO0C1_TRACE_D3,
-	GPIO0C1_MCU_JTAG_TRTSN,
+	GPIO0C1_LCDC_D13        = (1 << 2),
+	GPIO0C1_TRACE_D3        = (2 << 2),
+	GPIO0C1_MCU_JTAG_TRTSN  = (3 << 2),
 
-	GPIO0C0_SHIFT           = 0,
-	GPIO0C0_MASK            = 3 << GPIO0C0_SHIFT,
+	GPIO0C0_MASK            = GENMASK(1, 0),
 	GPIO0C0_GPIO            = 0,
-	GPIO0C0_LCDC_D12,
-	GPIO0C0_TRACE_D2,
-	GPIO0C0_MCU_JTAG_TDO,
+	GPIO0C0_LCDC_D12        = (1 << 0),
+	GPIO0C0_TRACE_D2        = (2 << 0),
+	GPIO0C0_MCU_JTAG_TDO    = (3 << 0),
 };
 
 /*GRF_GPIO0D_IOMUX*/
 enum {
-	GPIO0D7_SHIFT           = 14,
-	GPIO0D7_MASK            = 3 << GPIO0D7_SHIFT,
+	GPIO0D7_MASK            = GENMASK(15, 14),
 	GPIO0D7_GPIO            = 0,
-	GPIO0D7_LCDC_DCLK,
-	GPIO0D7_TRACE_CTL,
-	GPIO0D7_PMU_DEBUG5,
+	GPIO0D7_LCDC_DCLK       = (1 << 14),
+	GPIO0D7_TRACE_CTL       = (2 << 14),
+	GPIO0D7_PMU_DEBUG5      = (3 << 14),
 
-	GPIO0D6_SHIFT           = 12,
-	GPIO0D6_MASK            = 3 << GPIO0D6_SHIFT,
+	GPIO0D6_MASK            = GENMASK(13, 12),
 	GPIO0D6_GPIO            = 0,
-	GPIO0D6_LCDC_DEN,
-	GPIO0D6_TRACE_CLK,
-	GPIO0D6_PMU_DEBUG4,
+	GPIO0D6_LCDC_DEN        = (1 << 12),
+	GPIO0D6_TRACE_CLK       = (2 << 12),
+	GPIO0D6_PMU_DEBUG4      = (3 << 12),
 
-	GPIO0D5_SHIFT           = 10,
-	GPIO0D5_MASK            = 3 << GPIO0D5_SHIFT,
+	GPIO0D5_MASK            = GENMASK(11, 10),
 	GPIO0D5_GPIO            = 0,
-	GPIO0D5_LCDC_VSYNC,
-	GPIO0D5_TRACE_D15,
-	GPIO0D5_PMU_DEBUG3,
+	GPIO0D5_LCDC_VSYNC             = (1 << 10),
+	GPIO0D5_TRACE_D15       = (2 << 10),
+	GPIO0D5_PMU_DEBUG3      = (3 << 10),
 
-	GPIO0D4_SHIFT           = 8,
-	GPIO0D4_MASK            = 3 << GPIO0D4_SHIFT,
+	GPIO0D4_MASK            = GENMASK(9, 8),
 	GPIO0D4_GPIO            = 0,
-	GPIO0D4_LCDC_HSYNC,
-	GPIO0D4_TRACE_D14,
-	GPIO0D4_PMU_DEBUG2,
+	GPIO0D4_LCDC_HSYNC      = (1 << 8),
+	GPIO0D4_TRACE_D14       = (2 << 8),
+	GPIO0D4_PMU_DEBUG2      = (3 << 8),
 
-	GPIO0D3_SHIFT           = 6,
-	GPIO0D3_MASK            = 3 << GPIO0D3_SHIFT,
+	GPIO0D3_MASK            = GENMASK(7, 6),
 	GPIO0D3_GPIO            = 0,
-	GPIO0D3_LCDC_D23,
-	GPIO0D3_TRACE_D13,
-	GPIO0D3_UART4_SIN,
+	GPIO0D3_LCDC_D23        = (1 << 6),
+	GPIO0D3_TRACE_D13       = (2 << 6),
+	GPIO0D3_UART4_SIN       = (3 << 6),
 
-	GPIO0D2_SHIFT           = 4,
-	GPIO0D2_MASK            = 3 << GPIO0D2_SHIFT,
+	GPIO0D2_MASK            = GENMASK(5, 4),
 	GPIO0D2_GPIO            = 0,
-	GPIO0D2_LCDC_D22,
-	GPIO0D2_TRACE_D12,
-	GPIO0D2_UART4_SOUT,
+	GPIO0D2_LCDC_D22        = (1 << 4),
+	GPIO0D2_TRACE_D12       = (2 << 4),
+	GPIO0D2_UART4_SOUT      = (3 << 4),
 
-	GPIO0D1_SHIFT           = 2,
-	GPIO0D1_MASK            = 3 << GPIO0D1_SHIFT,
+	GPIO0D1_MASK            = GENMASK(3, 2),
 	GPIO0D1_GPIO            = 0,
-	GPIO0D1_LCDC_D21,
-	GPIO0D1_TRACE_D11,
-	GPIO0D1_UART4_RTSN,
+	GPIO0D1_LCDC_D21        = (1 << 2),
+	GPIO0D1_TRACE_D11       = (2 << 2),
+	GPIO0D1_UART4_RTSN      = (3 << 2),
 
-	GPIO0D0_SHIFT           = 0,
-	GPIO0D0_MASK            = 3 << GPIO0D0_SHIFT,
+	GPIO0D0_MASK            = GENMASK(1, 0),
 	GPIO0D0_GPIO            = 0,
-	GPIO0D0_LCDC_D20,
-	GPIO0D0_TRACE_D10,
-	GPIO0D0_UART4_CTSN,
+	GPIO0D0_LCDC_D20        = (1 << 0),
+	GPIO0D0_TRACE_D10       = (2 << 0),
+	GPIO0D0_UART4_CTSN      = (3 << 0),
 };
 
 /*GRF_GPIO2A_IOMUX*/
 enum {
-	GPIO2A7_SHIFT           = 14,
-	GPIO2A7_MASK            = 3 << GPIO2A7_SHIFT,
+	GPIO2A7_MASK            = GENMASK(15, 14),
 	GPIO2A7_GPIO            = 0,
-	GPIO2A7_SDMMC0_D2,
-	GPIO2A7_JTAG_TCK,
+	GPIO2A7_SDMMC0_D2       = (1 << 14),
+	GPIO2A7_JTAG_TCK        = (2 << 14),
 
-	GPIO2A6_SHIFT           = 12,
-	GPIO2A6_MASK            = 3 << GPIO2A6_SHIFT,
+	GPIO2A6_MASK            = GENMASK(13, 12),
 	GPIO2A6_GPIO            = 0,
-	GPIO2A6_SDMMC0_D1,
-	GPIO2A6_UART2_SIN,
+	GPIO2A6_SDMMC0_D1       = (1 << 12),
+	GPIO2A6_UART2_SIN       = (2 << 12),
 
-	GPIO2A5_SHIFT           = 10,
-	GPIO2A5_MASK            = 3 << GPIO2A5_SHIFT,
+	GPIO2A5_MASK            = GENMASK(11, 10),
 	GPIO2A5_GPIO            = 0,
-	GPIO2A5_SDMMC0_D0,
-	GPIO2A5_UART2_SOUT,
+	GPIO2A5_SDMMC0_D0       = (1 << 10),
+	GPIO2A5_UART2_SOUT      = (2 << 10),
 
-	GPIO2A4_SHIFT           = 8,
-	GPIO2A4_MASK            = 3 << GPIO2A4_SHIFT,
-	GPIO2A4_GPIO            = 0,
-	GPIO2A4_FLASH_DQS,
-	GPIO2A4_EMMC_CLKO,
+	GPIO2A4_MASK            = GENMASK(9, 8),
+	GPIO2A4_GPIO     = 0,
+	GPIO2A4_FLASH_DQS       = (1 << 8),
+	GPIO2A4_EMMC_CLKOUT     = (2 << 8),
 
-	GPIO2A3_SHIFT           = 6,
-	GPIO2A3_MASK            = 3 << GPIO2A3_SHIFT,
+	GPIO2A3_MASK            = GENMASK(7, 6),
 	GPIO2A3_GPIO            = 0,
-	GPIO2A3_FLASH_CSN3,
-	GPIO2A3_EMMC_RSTNO,
+	GPIO2A3_FLASH_CSN3      = (1 << 6),
+	GPIO2A3_EMMC_RSTNOUT    = (2 << 6),
 
-	GPIO2A2_SHIFT           = 4,
-	GPIO2A2_MASK            = 3 << GPIO2A2_SHIFT,
-	GPIO2A2_GPIO           = 0,
-	GPIO2A2_FLASH_CSN2,
+	GPIO2A2_MASK            = GENMASK(5, 4),
+	GPIO2A2_GPIO            = 0,
+	GPIO2A2_FLASH_CSN2      = (1 << 4),
 
-	GPIO2A1_SHIFT           = 2,
-	GPIO2A1_MASK            = 3 << GPIO2A1_SHIFT,
+	GPIO2A1_MASK            = GENMASK(3, 2),
 	GPIO2A1_GPIO            = 0,
-	GPIO2A1_FLASH_CSN1,
+	GPIO2A1_FLASH_CSN1      = (1 << 2),
 
-	GPIO2A0_SHIFT           = 0,
-	GPIO2A0_MASK            = 3 << GPIO2A0_SHIFT,
+	GPIO2A0_MASK            = GENMASK(1, 0),
 	GPIO2A0_GPIO            = 0,
-	GPIO2A0_FLASH_CSN0,
+	GPIO2A0_FLASH_CSN0      = (1 << 0),
 };
 
 /*GRF_GPIO2D_IOMUX*/
 enum {
-	GPIO2D7_SHIFT           = 14,
-	GPIO2D7_MASK            = 3 << GPIO2D7_SHIFT,
+	GPIO2D7_MASK            = GENMASK(15, 14),
 	GPIO2D7_GPIO            = 0,
-	GPIO2D7_SDIO0_D3,
+	GPIO2D7_SDIO0_D3        = (1 << 14),
 
-	GPIO2D6_SHIFT           = 12,
-	GPIO2D6_MASK            = 3 << GPIO2D6_SHIFT,
+	GPIO2D6_MASK            = GENMASK(13, 12),
 	GPIO2D6_GPIO            = 0,
-	GPIO2D6_SDIO0_D2,
+	GPIO2D6_SDIO0_D2        = (1 << 12),
 
-	GPIO2D5_SHIFT           = 10,
-	GPIO2D5_MASK            = 3 << GPIO2D5_SHIFT,
+	GPIO2D5_MASK            = GENMASK(11, 10),
 	GPIO2D5_GPIO            = 0,
-	GPIO2D5_SDIO0_D1,
+	GPIO2D5_SDIO0_D1        = (1 << 10),
 
-	GPIO2D4_SHIFT           = 8,
-	GPIO2D4_MASK            = 3 << GPIO2D4_SHIFT,
+	GPIO2D4_MASK            = GENMASK(9, 8),
 	GPIO2D4_GPIO            = 0,
-	GPIO2D4_SDIO0_D0,
+	GPIO2D4_SDIO0_D0        = (1 << 8),
 
-	GPIO2D3_SHIFT           = 6,
-	GPIO2D3_MASK            = 3 << GPIO2D3_SHIFT,
+	GPIO2D3_MASK            = GENMASK(7, 6),
 	GPIO2D3_GPIO            = 0,
-	GPIO2D3_UART0_RTS0,
+	GPIO2D3_UART0_RTS0      = (1 << 6),
 
-	GPIO2D2_SHIFT           = 4,
-	GPIO2D2_MASK            = 3 << GPIO2D2_SHIFT,
+	GPIO2D2_MASK            = GENMASK(5, 4),
 	GPIO2D2_GPIO            = 0,
-	GPIO2D2_UART0_CTS0,
+	GPIO2D2_UART0_CTS0      = (1 << 4),
 
-	GPIO2D1_SHIFT           = 2,
-	GPIO2D1_MASK            = 3 << GPIO2D1_SHIFT,
+	GPIO2D1_MASK            = GENMASK(3, 2),
 	GPIO2D1_GPIO            = 0,
-	GPIO2D1_UART0_SOUT,
+	GPIO2D1_UART0_SOUT      = (1 << 2),
 
-	GPIO2D0_SHIFT           = 0,
-	GPIO2D0_MASK            = 3 << GPIO2D0_SHIFT,
+	GPIO2D0_MASK            = GENMASK(1, 0),
 	GPIO2D0_GPIO            = 0,
-	GPIO2D0_UART0_SIN,
+	GPIO2D0_UART0_SIN       = (1 << 0),
+};
+
+/* GRF_GPIO1C_IOMUX */
+enum {
+	GPIO1C7_MASK            = GENMASK(15, 14),
+	GPIO1C7_GPIO            = 0,
+	GPIO1C7_EMMC_DATA5      = (2 << 14),
+
+	GPIO1C6_MASK            = GENMASK(13, 12),
+	GPIO1C6_GPIO            = 0,
+	GPIO1C6_EMMC_DATA4      = (2 << 12),
+
+	GPIO1C5_MASK            = GENMASK(11, 10),
+	GPIO1C5_GPIO            = 0,
+	GPIO1C5_EMMC_DATA3      = (2 << 10),
+
+	GPIO1C4_MASK            = GENMASK(9, 8),
+	GPIO1C4_GPIO            = 0,
+	GPIO1C4_EMMC_DATA2      = (2 << 8),
+
+	GPIO1C3_MASK            = GENMASK(7, 6),
+	GPIO1C3_GPIO            = 0,
+	GPIO1C3_EMMC_DATA1      = (2 << 6),
+
+	GPIO1C2_MASK            = GENMASK(5, 4),
+	GPIO1C2_GPIO            = 0,
+	GPIO1C2_EMMC_DATA0      = (2 << 4),
+};
+
+/* GRF_GPIO1D_IOMUX*/
+enum {
+	GPIO1D3_MASK            = GENMASK(7, 6),
+	GPIO1D3_GPIO            = 0,
+	GPIO1D3_EMMC_PWREN      = (2 << 6),
+
+	GPIO1D2_MASK            = GENMASK(5, 4),
+	GPIO1D2_GPIO            = 0,
+	GPIO1D2_EMMC_CMD        = (2 << 4),
+
+	GPIO1D1_MASK            = GENMASK(3, 2),
+	GPIO1D1_GPIO            = 0,
+	GPIO1D1_EMMC_DATA7      = (2 << 2),
+
+	GPIO1D0_MASK            = GENMASK(1, 0),
+	GPIO1D0_GPIO            = 0,
+	GPIO1D0_EMMC_DATA6      = (2 << 0),
+};
+
+
+/*GRF_GPIO3B_IOMUX*/
+enum {
+	GPIO3B7_MASK            = GENMASK(15, 14),
+	GPIO3B7_GPIO            = 0,
+	GPIO3B7_MAC_RXD0        = (1 << 14),
+
+	GPIO3B6_MASK            = GENMASK(13, 12),
+	GPIO3B6_GPIO            = 0,
+	GPIO3B6_MAC_TXD3        = (1 << 12),
+
+	GPIO3B5_MASK            = GENMASK(11, 10),
+	GPIO3B5_GPIO            = 0,
+	GPIO3B5_MAC_TXEN        = (1 << 10),
+
+	GPIO3B4_MASK            = GENMASK(9, 8),
+	GPIO3B4_GPIO            = 0,
+	GPIO3B4_MAC_COL         = (1 << 8),
+
+	GPIO3B3_MASK            = GENMASK(7, 6),
+	GPIO3B3_GPIO            = 0,
+	GPIO3B3_MAC_CRS         = (1 << 6),
+
+	GPIO3B2_MASK            = GENMASK(5, 4),
+	GPIO3B2_GPIO            = 0,
+	GPIO3B2_MAC_TXD2        = (1 << 4),
+
+	GPIO3B1_MASK            = GENMASK(3, 2),
+	GPIO3B1_GPIO            = 0,
+	GPIO3B1_MAC_TXD1        = (1 << 2),
+
+	GPIO3B0_MASK            = GENMASK(1, 0),
+	GPIO3B0_GPIO            = 0,
+	GPIO3B0_MAC_TXD0        = (1 << 0),
+	GPIO3B0_PWM0            = (2 << 0),
 };
 
 /*GRF_GPIO3C_IOMUX*/
 enum {
-	GPIO3C7_SHIFT           = 14,
-	GPIO3C7_MASK            = 3 << GPIO3C7_SHIFT,
-	GPIO3C7_GPIO            = 0,
-	GPIO3C7_EDPHDMI_CECINOUT,
-	GPIO3C7_ISP_FLASHTRIGIN,
-
-	GPIO3C6_SHIFT           = 12,
-	GPIO3C6_MASK            = 3 << GPIO3C6_SHIFT,
+	GPIO3C6_MASK            = GENMASK(13, 12),
 	GPIO3C6_GPIO            = 0,
-	GPIO3C6_MAC_CLK,
-	GPIO3C6_ISP_SHUTTERTRIG,
+	GPIO3C6_MAC_CLK         = (1 << 12),
 
-	GPIO3C5_SHIFT           = 10,
-	GPIO3C5_MASK            = 3 << GPIO3C5_SHIFT,
+	GPIO3C5_MASK            = GENMASK(11, 10),
 	GPIO3C5_GPIO            = 0,
-	GPIO3C5_MAC_RXER,
-	GPIO3C5_ISP_PRELIGHTTRIG,
+	GPIO3C5_MAC_RXEN        = (1 << 10),
 
-	GPIO3C4_SHIFT           = 8,
-	GPIO3C4_MASK            = 3 << GPIO3C4_SHIFT,
+	GPIO3C4_MASK            = GENMASK(9, 8),
 	GPIO3C4_GPIO            = 0,
-	GPIO3C4_MAC_RXDV,
-	GPIO3C4_ISP_FLASHTRIGOUT,
+	GPIO3C4_MAC_RXDV        = (1 << 8),
 
-	GPIO3C3_SHIFT           = 6,
-	GPIO3C3_MASK            = 3 << GPIO3C3_SHIFT,
+	GPIO3C3_MASK            = GENMASK(7, 6),
 	GPIO3C3_GPIO            = 0,
-	GPIO3C3_MAC_RXDV,
-	GPIO3C3_EMMC_RSTNO,
+	GPIO3C3_MAC_MDC         = (1 << 6),
 
-	GPIO3C2_SHIFT           = 4,
-	GPIO3C2_MASK            = 3 << GPIO3C2_SHIFT,
-	GPIO3C2_MAC_MDC            = 0,
-	GPIO3C2_ISP_SHUTTEREN,
+	GPIO3C2_MASK            = GENMASK(5, 4),
+	GPIO3C2_GPIO            = 0,
+	GPIO3C2_MAC_RXD3        = (1 << 4),
 
-	GPIO3C1_SHIFT           = 2,
-	GPIO3C1_MASK            = 3 << GPIO3C1_SHIFT,
+	GPIO3C1_MASK            = GENMASK(3, 2),
 	GPIO3C1_GPIO            = 0,
-	GPIO3C1_MAC_RXD2,
-	GPIO3C1_UART3_RTSN,
+	GPIO3C1_MAC_RXD2        = (1 << 2),
 
-	GPIO3C0_SHIFT           = 0,
-	GPIO3C0_MASK            = 3 << GPIO3C0_SHIFT,
+	GPIO3C0_MASK            = GENMASK(1, 0),
 	GPIO3C0_GPIO            = 0,
-	GPIO3C0_MAC_RXD1,
-	GPIO3C0_UART3_CTSN,
-	GPIO3C0_GPS_RFCLK,
+	GPIO3C0_MAC_RXD1        = (1 << 0),
 };
 
 /*GRF_GPIO3D_IOMUX*/
 enum {
-	GPIO3D7_SHIFT           = 14,
-	GPIO3D7_MASK            = 3 << GPIO3D7_SHIFT,
-	GPIO3D7_GPIO            = 0,
-	GPIO3D7_SC_VCC18V,
-	GPIO3D7_I2C2_SDA,
-	GPIO3D7_GPUJTAG_TCK,
-
-	GPIO3D6_SHIFT           = 12,
-	GPIO3D6_MASK            = 3 << GPIO3D6_SHIFT,
-	GPIO3D6_GPIO            = 0,
-	GPIO3D6_IR_TX,
-	GPIO3D6_UART3_SOUT,
-	GPIO3D6_PWM3,
-
-	GPIO3D5_SHIFT           = 10,
-	GPIO3D5_MASK            = 3 << GPIO3D5_SHIFT,
-	GPIO3D5_GPIO            = 0,
-	GPIO3D5_IR_RX,
-	GPIO3D5_UART3_SIN,
-
-	GPIO3D4_SHIFT           = 8,
-	GPIO3D4_MASK            = 3 << GPIO3D4_SHIFT,
+	GPIO3D4_MASK            = GENMASK(9, 8),
 	GPIO3D4_GPIO            = 0,
-	GPIO3D4_MAC_TXCLKOUT,
-	GPIO3D4_SPI1_CSN1,
-
-	GPIO3D3_SHIFT           = 6,
-	GPIO3D3_MASK            = 3 << GPIO3D3_SHIFT,
-	GPIO3D3_GPIO            = 0,
-	GPIO3D3_HDMII2C_SCL,
-	GPIO3D3_I2C5_SCL,
-
-	GPIO3D2_SHIFT           = 4,
-	GPIO3D2_MASK            = 3 << GPIO3D2_SHIFT,
-	GPIO3D2_GPIO            = 0,
-	GPIO3D2_HDMII2C_SDA,
-	GPIO3D2_I2C5_SDA,
-
-	GPIO3D1_SHIFT           = 2,
-	GPIO3D1_MASK            = 3 << GPIO3D1_SHIFT,
+	GPIO3D4_MAC_TXCLK       = (1 << 8),
+
+	GPIO3D1_MASK            = GENMASK(3, 2),
 	GPIO3D1_GPIO            = 0,
-	GPIO3D1_MAC_RXCLKIN,
-	GPIO3D1_I2C4_SCL,
+	GPIO3D1_MAC_RXCLK       = (1 << 2),
 
-	GPIO3D0_SHIFT           = 0,
-	GPIO3D0_MASK            = 3 << GPIO3D0_SHIFT,
+	GPIO3D0_MASK            = GENMASK(1, 0),
 	GPIO3D0_GPIO            = 0,
-	GPIO3D0_MAC_MDIO,
-	GPIO3D0_I2C4_SDA,
+	GPIO3D0_MAC_MDIO        = (1 << 0),
+};
+
+/* GRF_SOC_CON0 */
+enum {
+	NOC_RSP_ERR_STALL = BIT(9),
+	MOBILE_DDR_SEL = BIT(4),
+	DDR0_16BIT_EN = BIT(3),
+	MSCH0_MAINDDR3_DDR3 = BIT(2),
+	MSCH0_MAINPARTIALPOP = BIT(1),
+	UPCTL_C_ACTIVE = BIT(0),
 };
 
 /*GRF_SOC_CON11/12/13*/
diff --git a/drivers/pinctrl/rockchip/pinctrl_rk3368.c b/drivers/pinctrl/rockchip/pinctrl_rk3368.c
index bdf0758..c96459f 100644
--- a/drivers/pinctrl/rockchip/pinctrl_rk3368.c
+++ b/drivers/pinctrl/rockchip/pinctrl_rk3368.c
@@ -31,8 +31,7 @@  static void pinctrl_rk3368_uart_config(struct rk3368_pinctrl_priv *priv,
 	case PERIPH_ID_UART2:
 		rk_clrsetreg(&grf->gpio2a_iomux,
 			     GPIO2A6_MASK | GPIO2A5_MASK,
-			     GPIO2A6_UART2_SIN << GPIO2A6_SHIFT |
-			     GPIO2A5_UART2_SOUT << GPIO2A5_SHIFT);
+			     GPIO2A6_UART2_SIN | GPIO2A5_UART2_SOUT);
 		break;
 	case PERIPH_ID_UART0:
 		break;
@@ -44,10 +43,8 @@  static void pinctrl_rk3368_uart_config(struct rk3368_pinctrl_priv *priv,
 		rk_clrsetreg(&pmugrf->gpio0d_iomux,
 			     GPIO0D0_MASK | GPIO0D1_MASK |
 			     GPIO0D2_MASK | GPIO0D3_MASK,
-			     GPIO0D0_GPIO << GPIO0D0_SHIFT |
-			     GPIO0D1_GPIO << GPIO0D1_SHIFT |
-			     GPIO0D2_UART4_SOUT << GPIO0D2_SHIFT |
-			     GPIO0D3_UART4_SIN << GPIO0D3_SHIFT);
+			     GPIO0D0_GPIO | GPIO0D1_GPIO |
+			     GPIO0D2_UART4_SOUT | GPIO0D3_UART4_SIN);
 		break;
 	default:
 		debug("uart id = %d iomux error!\n", uart_id);