[U-Boot,30/52] rockchip: rk3368: grf: use shifted-constants and prefix with RK3368_

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 rename, we do this now
before having more code use this.

As some of the downstream drivers (e.g. the Designware GMAC wrapper)
may need to include the grf-header of multiple devices, we rename the
various defines for the RK3368 by prefixing them with the device name.
This avoids future trouble during driver integration.

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

 arch/arm/include/asm/arch-rockchip/grf_rk3368.h | 617 +++++++++++++-----------
 drivers/pinctrl/rockchip/pinctrl_rk3368.c       |  17 +-
 2 files changed, 334 insertions(+), 300 deletions(-)

Hi Philipp:


On 2017年07月19日 04:36, Philipp Tomsich 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 rename, we do this now
> before having more code use this.
>
> As some of the downstream drivers (e.g. the Designware GMAC wrapper)
> may need to include the grf-header of multiple devices, we rename the
> various defines for the RK3368 by prefixing them with the device name.
> This avoids future trouble during driver integration.

     Is that really necessary to add such a prefix for all the register 
definition, just to fit  the GMAC dirver?
Maybe the gmac driver can define the platform specific macro in its own 
code like dwmac-rk in the kernel. Then we can keep the register header 
file a little tidy.

> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
>
>   arch/arm/include/asm/arch-rockchip/grf_rk3368.h | 617 +++++++++++++-----------
>   drivers/pinctrl/rockchip/pinctrl_rk3368.c       |  17 +-
>   2 files changed, 334 insertions(+), 300 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..a97dc1e 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,
> -	GPIO0C6_GPIO            = 0,
> -	GPIO0C6_LCDC_D18,
> -	GPIO0C6_TRACE_D8,
> -	GPIO0C6_UART1_CTSN,
> -
> -	GPIO0C5_SHIFT           = 10,
> -	GPIO0C5_MASK            = 3 << GPIO0C5_SHIFT,
> -	GPIO0C5_GPIO            = 0,
> -	GPIO0C5_LCDC_D17,
> -	GPIO0C5_TRACE_D7,
> -	GPIO0C5_UART1_SOUT,
> -
> -	GPIO0C4_SHIFT           = 8,
> -	GPIO0C4_MASK            = 3 << GPIO0C4_SHIFT,
> -	GPIO0C4_GPIO            = 0,
> -	GPIO0C4_LCDC_D16,
> -	GPIO0C4_TRACE_D6,
> -	GPIO0C4_UART1_SIN,
> -
> -	GPIO0C3_SHIFT           = 6,
> -	GPIO0C3_MASK            = 3 << GPIO0C3_SHIFT,
> -	GPIO0C3_GPIO            = 0,
> -	GPIO0C3_LCDC_D15,
> -	GPIO0C3_TRACE_D5,
> -	GPIO0C3_MCU_JTAG_TDO,
> -
> -	GPIO0C2_SHIFT           = 4,
> -	GPIO0C2_MASK            = 3 << GPIO0C2_SHIFT,
> -	GPIO0C2_GPIO            = 0,
> -	GPIO0C2_LCDC_D14,
> -	GPIO0C2_TRACE_D4,
> -	GPIO0C2_MCU_JTAG_TDI,
> -
> -	GPIO0C1_SHIFT           = 2,
> -	GPIO0C1_MASK            = 3 << GPIO0C1_SHIFT,
> -	GPIO0C1_GPIO            = 0,
> -	GPIO0C1_LCDC_D13,
> -	GPIO0C1_TRACE_D3,
> -	GPIO0C1_MCU_JTAG_TRTSN,
> -
> -	GPIO0C0_SHIFT           = 0,
> -	GPIO0C0_MASK            = 3 << GPIO0C0_SHIFT,
> -	GPIO0C0_GPIO            = 0,
> -	GPIO0C0_LCDC_D12,
> -	GPIO0C0_TRACE_D2,
> -	GPIO0C0_MCU_JTAG_TDO,
> +	RK3368_GPIO0C7_MASK	       = GENMASK(15, 14),
> +	RK3368_GPIO0C7_GPIO	       = 0,
> +	RK3368_GPIO0C7_LCDC_D19        = (1 << 14),
> +	RK3368_GPIO0C7_TRACE_D9        = (2 << 14),
> +	RK3368_GPIO0C7_UART1_RTSN      = (3 << 14),
> +
> +	RK3368_GPIO0C6_MASK            = GENMASK(13, 12),
> +	RK3368_GPIO0C6_GPIO            = 0,
> +	RK3368_GPIO0C6_LCDC_D18        = (1 << 12),
> +	RK3368_GPIO0C6_TRACE_D8        = (2 << 12),
> +	RK3368_GPIO0C6_UART1_CTSN      = (3 << 12),
> +
> +	RK3368_GPIO0C5_MASK            = GENMASK(11, 10),
> +	RK3368_GPIO0C5_GPIO            = 0,
> +	RK3368_GPIO0C5_LCDC_D17        = (1 << 10),
> +	RK3368_GPIO0C5_TRACE_D7        = (2 << 10),
> +	RK3368_GPIO0C5_UART1_SOUT      = (3 << 10),
> +
> +	RK3368_GPIO0C4_MASK            = GENMASK(9, 8),
> +	RK3368_GPIO0C4_GPIO            = 0,
> +	RK3368_GPIO0C4_LCDC_D16        = (1 << 8),
> +	RK3368_GPIO0C4_TRACE_D6        = (2 << 8),
> +	RK3368_GPIO0C4_UART1_SIN       = (3 << 8),
> +
> +	RK3368_GPIO0C3_MASK            = GENMASK(7, 6),
> +	RK3368_GPIO0C3_GPIO            = 0,
> +	RK3368_GPIO0C3_LCDC_D15        = (1 << 6),
> +	RK3368_GPIO0C3_TRACE_D5        = (2 << 6),
> +	RK3368_GPIO0C3_MCU_JTAG_TDO    = (3 << 6),
> +
> +	RK3368_GPIO0C2_MASK            = GENMASK(5, 4),
> +	RK3368_GPIO0C2_GPIO            = 0,
> +	RK3368_GPIO0C2_LCDC_D14        = (1 << 4),
> +	RK3368_GPIO0C2_TRACE_D4        = (2 << 4),
> +	RK3368_GPIO0C2_MCU_JTAG_TDI    = (3 << 4),
> +
> +	RK3368_GPIO0C1_MASK            = GENMASK(3, 2),
> +	RK3368_GPIO0C1_GPIO            = 0,
> +	RK3368_GPIO0C1_LCDC_D13        = (1 << 2),
> +	RK3368_GPIO0C1_TRACE_D3        = (2 << 2),
> +	RK3368_GPIO0C1_MCU_JTAG_TRTSN  = (3 << 2),
> +
> +	RK3368_GPIO0C0_MASK            = GENMASK(1, 0),
> +	RK3368_GPIO0C0_GPIO            = 0,
> +	RK3368_GPIO0C0_LCDC_D12        = (1 << 0),
> +	RK3368_GPIO0C0_TRACE_D2        = (2 << 0),
> +	RK3368_GPIO0C0_MCU_JTAG_TDO    = (3 << 0),
>   };
>   
>   /*GRF_GPIO0D_IOMUX*/
>   enum {
> -	GPIO0D7_SHIFT           = 14,
> -	GPIO0D7_MASK            = 3 << GPIO0D7_SHIFT,
> -	GPIO0D7_GPIO            = 0,
> -	GPIO0D7_LCDC_DCLK,
> -	GPIO0D7_TRACE_CTL,
> -	GPIO0D7_PMU_DEBUG5,
> -
> -	GPIO0D6_SHIFT           = 12,
> -	GPIO0D6_MASK            = 3 << GPIO0D6_SHIFT,
> -	GPIO0D6_GPIO            = 0,
> -	GPIO0D6_LCDC_DEN,
> -	GPIO0D6_TRACE_CLK,
> -	GPIO0D6_PMU_DEBUG4,
> -
> -	GPIO0D5_SHIFT           = 10,
> -	GPIO0D5_MASK            = 3 << GPIO0D5_SHIFT,
> -	GPIO0D5_GPIO            = 0,
> -	GPIO0D5_LCDC_VSYNC,
> -	GPIO0D5_TRACE_D15,
> -	GPIO0D5_PMU_DEBUG3,
> -
> -	GPIO0D4_SHIFT           = 8,
> -	GPIO0D4_MASK            = 3 << GPIO0D4_SHIFT,
> -	GPIO0D4_GPIO            = 0,
> -	GPIO0D4_LCDC_HSYNC,
> -	GPIO0D4_TRACE_D14,
> -	GPIO0D4_PMU_DEBUG2,
> -
> -	GPIO0D3_SHIFT           = 6,
> -	GPIO0D3_MASK            = 3 << GPIO0D3_SHIFT,
> -	GPIO0D3_GPIO            = 0,
> -	GPIO0D3_LCDC_D23,
> -	GPIO0D3_TRACE_D13,
> -	GPIO0D3_UART4_SIN,
> -
> -	GPIO0D2_SHIFT           = 4,
> -	GPIO0D2_MASK            = 3 << GPIO0D2_SHIFT,
> -	GPIO0D2_GPIO            = 0,
> -	GPIO0D2_LCDC_D22,
> -	GPIO0D2_TRACE_D12,
> -	GPIO0D2_UART4_SOUT,
> -
> -	GPIO0D1_SHIFT           = 2,
> -	GPIO0D1_MASK            = 3 << GPIO0D1_SHIFT,
> -	GPIO0D1_GPIO            = 0,
> -	GPIO0D1_LCDC_D21,
> -	GPIO0D1_TRACE_D11,
> -	GPIO0D1_UART4_RTSN,
> -
> -	GPIO0D0_SHIFT           = 0,
> -	GPIO0D0_MASK            = 3 << GPIO0D0_SHIFT,
> -	GPIO0D0_GPIO            = 0,
> -	GPIO0D0_LCDC_D20,
> -	GPIO0D0_TRACE_D10,
> -	GPIO0D0_UART4_CTSN,
> +	RK3368_GPIO0D7_MASK            = GENMASK(15, 14),
> +	RK3368_GPIO0D7_GPIO            = 0,
> +	RK3368_GPIO0D7_LCDC_DCLK       = (1 << 14),
> +	RK3368_GPIO0D7_TRACE_CTL       = (2 << 14),
> +	RK3368_GPIO0D7_PMU_DEBUG5      = (3 << 14),
> +
> +	RK3368_GPIO0D6_MASK            = GENMASK(13, 12),
> +	RK3368_GPIO0D6_GPIO            = 0,
> +	RK3368_GPIO0D6_LCDC_DEN        = (1 << 12),
> +	RK3368_GPIO0D6_TRACE_CLK       = (2 << 12),
> +	RK3368_GPIO0D6_PMU_DEBUG4      = (3 << 12),
> +
> +	RK3368_GPIO0D5_MASK            = GENMASK(11, 10),
> +	RK3368_GPIO0D5_GPIO            = 0,
> +	GPIO0D5_LCDC_VSYNC             = (1 << 10),
> +	RK3368_GPIO0D5_TRACE_D15       = (2 << 10),
> +	RK3368_GPIO0D5_PMU_DEBUG3      = (3 << 10),
> +
> +	RK3368_GPIO0D4_MASK            = GENMASK(9, 8),
> +	RK3368_GPIO0D4_GPIO            = 0,
> +	RK3368_GPIO0D4_LCDC_HSYNC      = (1 << 8),
> +	RK3368_GPIO0D4_TRACE_D14       = (2 << 8),
> +	RK3368_GPIO0D4_PMU_DEBUG2      = (3 << 8),
> +
> +	RK3368_GPIO0D3_MASK            = GENMASK(7, 6),
> +	RK3368_GPIO0D3_GPIO            = 0,
> +	RK3368_GPIO0D3_LCDC_D23        = (1 << 6),
> +	RK3368_GPIO0D3_TRACE_D13       = (2 << 6),
> +	RK3368_GPIO0D3_UART4_SIN       = (3 << 6),
> +
> +	RK3368_GPIO0D2_MASK            = GENMASK(5, 4),
> +	RK3368_GPIO0D2_GPIO            = 0,
> +	RK3368_GPIO0D2_LCDC_D22        = (1 << 4),
> +	RK3368_GPIO0D2_TRACE_D12       = (2 << 4),
> +	RK3368_GPIO0D2_UART4_SOUT      = (3 << 4),
> +
> +	RK3368_GPIO0D1_MASK            = GENMASK(3, 2),
> +	RK3368_GPIO0D1_GPIO            = 0,
> +	RK3368_GPIO0D1_LCDC_D21        = (1 << 2),
> +	RK3368_GPIO0D1_TRACE_D11       = (2 << 2),
> +	RK3368_GPIO0D1_UART4_RTSN      = (3 << 2),
> +
> +	RK3368_GPIO0D0_MASK            = GENMASK(1, 0),
> +	RK3368_GPIO0D0_GPIO            = 0,
> +	RK3368_GPIO0D0_LCDC_D20        = (1 << 0),
> +	RK3368_GPIO0D0_TRACE_D10       = (2 << 0),
> +	RK3368_GPIO0D0_UART4_CTSN      = (3 << 0),
>   };
>   
>   /*GRF_GPIO2A_IOMUX*/
>   enum {
> -	GPIO2A7_SHIFT           = 14,
> -	GPIO2A7_MASK            = 3 << GPIO2A7_SHIFT,
> -	GPIO2A7_GPIO            = 0,
> -	GPIO2A7_SDMMC0_D2,
> -	GPIO2A7_JTAG_TCK,
> -
> -	GPIO2A6_SHIFT           = 12,
> -	GPIO2A6_MASK            = 3 << GPIO2A6_SHIFT,
> -	GPIO2A6_GPIO            = 0,
> -	GPIO2A6_SDMMC0_D1,
> -	GPIO2A6_UART2_SIN,
> -
> -	GPIO2A5_SHIFT           = 10,
> -	GPIO2A5_MASK            = 3 << GPIO2A5_SHIFT,
> -	GPIO2A5_GPIO            = 0,
> -	GPIO2A5_SDMMC0_D0,
> -	GPIO2A5_UART2_SOUT,
> -
> -	GPIO2A4_SHIFT           = 8,
> -	GPIO2A4_MASK            = 3 << GPIO2A4_SHIFT,
> -	GPIO2A4_GPIO            = 0,
> -	GPIO2A4_FLASH_DQS,
> -	GPIO2A4_EMMC_CLKO,
> -
> -	GPIO2A3_SHIFT           = 6,
> -	GPIO2A3_MASK            = 3 << GPIO2A3_SHIFT,
> -	GPIO2A3_GPIO            = 0,
> -	GPIO2A3_FLASH_CSN3,
> -	GPIO2A3_EMMC_RSTNO,
> -
> -	GPIO2A2_SHIFT           = 4,
> -	GPIO2A2_MASK            = 3 << GPIO2A2_SHIFT,
> -	GPIO2A2_GPIO           = 0,
> -	GPIO2A2_FLASH_CSN2,
> -
> -	GPIO2A1_SHIFT           = 2,
> -	GPIO2A1_MASK            = 3 << GPIO2A1_SHIFT,
> -	GPIO2A1_GPIO            = 0,
> -	GPIO2A1_FLASH_CSN1,
> -
> -	GPIO2A0_SHIFT           = 0,
> -	GPIO2A0_MASK            = 3 << GPIO2A0_SHIFT,
> -	GPIO2A0_GPIO            = 0,
> -	GPIO2A0_FLASH_CSN0,
> +	RK3368_GPIO2A7_MASK            = GENMASK(15, 14),
> +	RK3368_GPIO2A7_GPIO            = 0,
> +	RK3368_GPIO2A7_SDMMC0_D2       = (1 << 14),
> +	RK3368_GPIO2A7_JTAG_TCK        = (2 << 14),
> +
> +	RK3368_GPIO2A6_MASK            = GENMASK(13, 12),
> +	RK3368_GPIO2A6_GPIO            = 0,
> +	RK3368_GPIO2A6_SDMMC0_D1       = (1 << 12),
> +	RK3368_GPIO2A6_UART2_SIN       = (2 << 12),
> +
> +	RK3368_GPIO2A5_MASK            = GENMASK(11, 10),
> +	RK3368_GPIO2A5_GPIO            = 0,
> +	RK3368_GPIO2A5_SDMMC0_D0       = (1 << 10),
> +	RK3368_GPIO2A5_UART2_SOUT      = (2 << 10),
> +
> +	RK3368_GPIO2A4_MASK            = GENMASK(9, 8),
> +	RK3368_GPIO2A4_RK3368_GPIO     = 0,
> +	RK3368_GPIO2A4_FLASH_DQS       = (1 << 8),
> +	RK3368_GPIO2A4_EMMC_CLKOUT     = (2 << 8),
> +
> +	RK3368_GPIO2A3_MASK            = GENMASK(7, 6),
> +	RK3368_GPIO2A3_GPIO            = 0,
> +	RK3368_GPIO2A3_FLASH_CSN3      = (1 << 6),
> +	RK3368_GPIO2A3_EMMC_RSTNOUT    = (2 << 6),
> +
> +	RK3368_GPIO2A2_MASK            = GENMASK(5, 4),
> +	RK3368_GPIO2A2_GPIO            = 0,
> +	RK3368_GPIO2A2_FLASH_CSN2      = (1 << 4),
> +
> +	RK3368_GPIO2A1_MASK            = GENMASK(3, 2),
> +	RK3368_GPIO2A1_GPIO            = 0,
> +	RK3368_GPIO2A1_FLASH_CSN1      = (1 << 2),
> +
> +	RK3368_GPIO2A0_MASK            = GENMASK(1, 0),
> +	RK3368_GPIO2A0_GPIO            = 0,
> +	RK3368_GPIO2A0_FLASH_CSN0      = (1 << 0),
>   };
>   
> -/*GRF_GPIO2D_IOMUX*/
> +/*GRF_RK3368_GPIO2D_IOMUX*/
>   enum {
> -	GPIO2D7_SHIFT           = 14,
> -	GPIO2D7_MASK            = 3 << GPIO2D7_SHIFT,
> -	GPIO2D7_GPIO            = 0,
> -	GPIO2D7_SDIO0_D3,
> -
> -	GPIO2D6_SHIFT           = 12,
> -	GPIO2D6_MASK            = 3 << GPIO2D6_SHIFT,
> -	GPIO2D6_GPIO            = 0,
> -	GPIO2D6_SDIO0_D2,
> -
> -	GPIO2D5_SHIFT           = 10,
> -	GPIO2D5_MASK            = 3 << GPIO2D5_SHIFT,
> -	GPIO2D5_GPIO            = 0,
> -	GPIO2D5_SDIO0_D1,
> -
> -	GPIO2D4_SHIFT           = 8,
> -	GPIO2D4_MASK            = 3 << GPIO2D4_SHIFT,
> -	GPIO2D4_GPIO            = 0,
> -	GPIO2D4_SDIO0_D0,
> -
> -	GPIO2D3_SHIFT           = 6,
> -	GPIO2D3_MASK            = 3 << GPIO2D3_SHIFT,
> -	GPIO2D3_GPIO            = 0,
> -	GPIO2D3_UART0_RTS0,
> -
> -	GPIO2D2_SHIFT           = 4,
> -	GPIO2D2_MASK            = 3 << GPIO2D2_SHIFT,
> -	GPIO2D2_GPIO            = 0,
> -	GPIO2D2_UART0_CTS0,
> -
> -	GPIO2D1_SHIFT           = 2,
> -	GPIO2D1_MASK            = 3 << GPIO2D1_SHIFT,
> -	GPIO2D1_GPIO            = 0,
> -	GPIO2D1_UART0_SOUT,
> -
> -	GPIO2D0_SHIFT           = 0,
> -	GPIO2D0_MASK            = 3 << GPIO2D0_SHIFT,
> -	GPIO2D0_GPIO            = 0,
> -	GPIO2D0_UART0_SIN,
> +	RK3368_GPIO2D7_MASK            = GENMASK(15, 14),
> +	RK3368_GPIO2D7_GPIO            = 0,
> +	RK3368_GPIO2D7_SDIO0_D3        = (1 << 14),
> +
> +	RK3368_GPIO2D6_MASK            = GENMASK(13, 12),
> +	RK3368_GPIO2D6_GPIO            = 0,
> +	RK3368_GPIO2D6_SDIO0_D2        = (1 << 12),
> +
> +	RK3368_GPIO2D5_MASK            = GENMASK(11, 10),
> +	RK3368_GPIO2D5_GPIO            = 0,
> +	RK3368_GPIO2D5_SDIO0_D1        = (1 << 10),
> +
> +	RK3368_GPIO2D4_MASK            = GENMASK(9, 8),
> +	RK3368_GPIO2D4_GPIO            = 0,
> +	RK3368_GPIO2D4_SDIO0_D0        = (1 << 8),
> +
> +	RK3368_GPIO2D3_MASK            = GENMASK(7, 6),
> +	RK3368_GPIO2D3_GPIO            = 0,
> +	RK3368_GPIO2D3_UART0_RTS0      = (1 << 6),
> +
> +	RK3368_GPIO2D2_MASK            = GENMASK(5, 4),
> +	RK3368_GPIO2D2_GPIO            = 0,
> +	RK3368_GPIO2D2_UART0_CTS0      = (1 << 4),
> +
> +	RK3368_GPIO2D1_MASK            = GENMASK(3, 2),
> +	RK3368_GPIO2D1_GPIO            = 0,
> +	RK3368_GPIO2D1_UART0_SOUT      = (1 << 2),
> +
> +	RK3368_GPIO2D0_MASK            = GENMASK(1, 0),
> +	RK3368_GPIO2D0_GPIO            = 0,
> +	RK3368_GPIO2D0_UART0_SIN       = (1 << 0),
> +};
> +
> +/* GRF_GPIO1C_IOMUX */
> +enum {
> +	RK3368_GPIO1C7_MASK            = GENMASK(15, 14),
> +	RK3368_GPIO1C7_GPIO            = 0,
> +	RK3368_GPIO1C7_EMMC_DATA5      = (2 << 14),
> +
> +	RK3368_GPIO1C6_MASK            = GENMASK(13, 12),
> +	RK3368_GPIO1C6_GPIO            = 0,
> +	RK3368_GPIO1C6_EMMC_DATA4      = (2 << 12),
> +
> +	RK3368_GPIO1C5_MASK            = GENMASK(11, 10),
> +	RK3368_GPIO1C5_GPIO            = 0,
> +	RK3368_GPIO1C5_EMMC_DATA3      = (2 << 10),
> +
> +	RK3368_GPIO1C4_MASK            = GENMASK(9, 8),
> +	RK3368_GPIO1C4_GPIO            = 0,
> +	RK3368_GPIO1C4_EMMC_DATA2      = (2 << 8),
> +
> +	RK3368_GPIO1C3_MASK            = GENMASK(7, 6),
> +	RK3368_GPIO1C3_GPIO            = 0,
> +	RK3368_GPIO1C3_EMMC_DATA1      = (2 << 6),
> +
> +	RK3368_GPIO1C2_MASK            = GENMASK(5, 4),
> +	RK3368_GPIO1C2_GPIO            = 0,
> +	RK3368_GPIO1C2_EMMC_DATA0      = (2 << 4),
> +};
> +
> +/* GRF_GPIO1D_IOMUX*/
> +enum {
> +	RK3368_GPIO1D3_MASK            = GENMASK(7, 6),
> +	RK3368_GPIO1D3_GPIO            = 0,
> +	RK3368_GPIO1D3_EMMC_PWREN      = (2 << 6),
> +
> +	RK3368_GPIO1D2_MASK            = GENMASK(5, 4),
> +	RK3368_GPIO1D2_GPIO            = 0,
> +	RK3368_GPIO1D2_EMMC_CMD        = (2 << 4),
> +
> +	RK3368_GPIO1D1_MASK            = GENMASK(3, 2),
> +	RK3368_GPIO1D1_GPIO            = 0,
> +	RK3368_GPIO1D1_EMMC_DATA7      = (2 << 2),
> +
> +	RK3368_GPIO1D0_MASK            = GENMASK(1, 0),
> +	RK3368_GPIO1D0_GPIO            = 0,
> +	RK3368_GPIO1D0_EMMC_DATA6      = (2 << 0),
> +};
> +
> +
> +/*GRF_GPIO3B_IOMUX*/
> +enum {
> +	RK3368_GPIO3B7_MASK            = GENMASK(15, 14),
> +	RK3368_GPIO3B7_GPIO            = 0,
> +	RK3368_GPIO3B7_MAC_RXD0        = (1 << 14),
> +
> +	RK3368_GPIO3B6_MASK            = GENMASK(13, 12),
> +	RK3368_GPIO3B6_GPIO            = 0,
> +	RK3368_GPIO3B6_MAC_TXD3        = (1 << 12),
> +
> +	RK3368_GPIO3B5_MASK            = GENMASK(11, 10),
> +	RK3368_GPIO3B5_GPIO            = 0,
> +	RK3368_GPIO3B5_MAC_TXEN        = (1 << 10),
> +
> +	RK3368_GPIO3B4_MASK            = GENMASK(9, 8),
> +	RK3368_GPIO3B4_GPIO            = 0,
> +	RK3368_GPIO3B4_MAC_COL         = (1 << 8),
> +
> +	RK3368_GPIO3B3_MASK            = GENMASK(7, 6),
> +	RK3368_GPIO3B3_GPIO            = 0,
> +	RK3368_GPIO3B3_MAC_CRS         = (1 << 6),
> +
> +	RK3368_GPIO3B2_MASK            = GENMASK(5, 4),
> +	RK3368_GPIO3B2_GPIO            = 0,
> +	RK3368_GPIO3B2_MAC_TXD2        = (1 << 4),
> +
> +	RK3368_GPIO3B1_MASK            = GENMASK(3, 2),
> +	RK3368_GPIO3B1_GPIO            = 0,
> +	RK3368_GPIO3B1_MAC_TXD1        = (1 << 2),
> +
> +	RK3368_GPIO3B0_MASK            = GENMASK(1, 0),
> +	RK3368_GPIO3B0_GPIO            = 0,
> +	RK3368_GPIO3B0_MAC_TXD0        = (1 << 0),
> +	RK3368_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_GPIO            = 0,
> -	GPIO3C6_MAC_CLK,
> -	GPIO3C6_ISP_SHUTTERTRIG,
> -
> -	GPIO3C5_SHIFT           = 10,
> -	GPIO3C5_MASK            = 3 << GPIO3C5_SHIFT,
> -	GPIO3C5_GPIO            = 0,
> -	GPIO3C5_MAC_RXER,
> -	GPIO3C5_ISP_PRELIGHTTRIG,
> -
> -	GPIO3C4_SHIFT           = 8,
> -	GPIO3C4_MASK            = 3 << GPIO3C4_SHIFT,
> -	GPIO3C4_GPIO            = 0,
> -	GPIO3C4_MAC_RXDV,
> -	GPIO3C4_ISP_FLASHTRIGOUT,
> -
> -	GPIO3C3_SHIFT           = 6,
> -	GPIO3C3_MASK            = 3 << GPIO3C3_SHIFT,
> -	GPIO3C3_GPIO            = 0,
> -	GPIO3C3_MAC_RXDV,
> -	GPIO3C3_EMMC_RSTNO,
> -
> -	GPIO3C2_SHIFT           = 4,
> -	GPIO3C2_MASK            = 3 << GPIO3C2_SHIFT,
> -	GPIO3C2_MAC_MDC            = 0,
> -	GPIO3C2_ISP_SHUTTEREN,
> -
> -	GPIO3C1_SHIFT           = 2,
> -	GPIO3C1_MASK            = 3 << GPIO3C1_SHIFT,
> -	GPIO3C1_GPIO            = 0,
> -	GPIO3C1_MAC_RXD2,
> -	GPIO3C1_UART3_RTSN,
> -
> -	GPIO3C0_SHIFT           = 0,
> -	GPIO3C0_MASK            = 3 << GPIO3C0_SHIFT,
> -	GPIO3C0_GPIO            = 0,
> -	GPIO3C0_MAC_RXD1,
> -	GPIO3C0_UART3_CTSN,
> -	GPIO3C0_GPS_RFCLK,
> +	RK3368_GPIO3C6_MASK            = GENMASK(13, 12),
> +	RK3368_GPIO3C6_GPIO            = 0,
> +	RK3368_GPIO3C6_MAC_CLK         = (1 << 12),
> +
> +	RK3368_GPIO3C5_MASK            = GENMASK(11, 10),
> +	RK3368_GPIO3C5_GPIO            = 0,
> +	RK3368_GPIO3C5_MAC_RXEN        = (1 << 10),
> +
> +	RK3368_GPIO3C4_MASK            = GENMASK(9, 8),
> +	RK3368_GPIO3C4_GPIO            = 0,
> +	RK3368_GPIO3C4_MAC_RXDV        = (1 << 8),
> +
> +	RK3368_GPIO3C3_MASK            = GENMASK(7, 6),
> +	RK3368_GPIO3C3_GPIO            = 0,
> +	RK3368_GPIO3C3_MAC_MDC         = (1 << 6),
> +
> +	RK3368_GPIO3C2_MASK            = GENMASK(5, 4),
> +	RK3368_GPIO3C2_GPIO            = 0,
> +	RK3368_GPIO3C2_MAC_RXD3        = (1 << 4),
> +
> +	RK3368_GPIO3C1_MASK            = GENMASK(3, 2),
> +	RK3368_GPIO3C1_GPIO            = 0,
> +	RK3368_GPIO3C1_MAC_RXD2        = (1 << 2),
> +
> +	RK3368_GPIO3C0_MASK            = GENMASK(1, 0),
> +	RK3368_GPIO3C0_GPIO            = 0,
> +	RK3368_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_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,
> -	GPIO3D1_GPIO            = 0,
> -	GPIO3D1_MAC_RXCLKIN,
> -	GPIO3D1_I2C4_SCL,
> -
> -	GPIO3D0_SHIFT           = 0,
> -	GPIO3D0_MASK            = 3 << GPIO3D0_SHIFT,
> -	GPIO3D0_GPIO            = 0,
> -	GPIO3D0_MAC_MDIO,
> -	GPIO3D0_I2C4_SDA,
> +	RK3368_GPIO3D4_MASK            = GENMASK(9, 8),
> +	RK3368_GPIO3D4_GPIO            = 0,
> +	RK3368_GPIO3D4_MAC_TXCLK       = (1 << 8),
> +
> +	RK3368_GPIO3D1_MASK            = GENMASK(3, 2),
> +	RK3368_GPIO3D1_GPIO            = 0,
> +	RK3368_GPIO3D1_MAC_RXCLK       = (1 << 2),
> +
> +	RK3368_GPIO3D0_MASK            = GENMASK(1, 0),
> +	RK3368_GPIO3D0_GPIO            = 0,
> +	RK3368_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*/
> @@ -440,4 +445,34 @@ enum {
>   	MCU_CODE_BASE_BIT31_BIT28_SHIFT		= 0,
>   	MCU_CODE_BASE_BIT31_BIT28_MASK		= GENMASK(3, 0),
>   };
> +
> +/*GRF_SOC_CON15*/
> +enum {
> +	RK3368_GMAC_PHY_INTF_SEL_RGMII = 1 << 9,
> +	RK3368_GMAC_PHY_INTF_SEL_MASK = GENMASK(11, 9),
> +	RK3368_RMII_MODE_MASK  = BIT(6),
> +	RK3368_RMII_MODE       = BIT(6),
> +	RK3368_GMAC_CLK_SEL_2_5M = 2 << 4,
> +	RK3368_GMAC_CLK_SEL_25M = 3 << 4,
> +	RK3368_GMAC_CLK_SEL_125M = 0 << 4,
> +	RK3368_GMAC_CLK_SEL_MASK = GENMASK(5, 4),
> +};
> +
> +/* GRF_SOC_CON16 */
> +enum {
> +	RK3368_RXCLK_DLY_ENA_GMAC_MASK = BIT(15),
> +	RK3368_RXCLK_DLY_ENA_GMAC_DISABLE = 0,
> +	RK3368_RXCLK_DLY_ENA_GMAC_ENABLE = BIT(15),
> +
> +	RK3368_TXCLK_DLY_ENA_GMAC_MASK = BIT(7),
> +	RK3368_TXCLK_DLY_ENA_GMAC_DISABLE = 0,
> +	RK3368_TXCLK_DLY_ENA_GMAC_ENABLE = BIT(7),
> +
> +	RK3368_CLK_RX_DL_CFG_GMAC_SHIFT = 8,
> +	RK3368_CLK_RX_DL_CFG_GMAC_MASK = GENMASK(14, 8),
> +
> +	RK3368_CLK_TX_DL_CFG_GMAC_SHIFT = 0,
> +	RK3368_CLK_TX_DL_CFG_GMAC_MASK = GENMASK(6, 0),
> +};
> +
>   #endif
> diff --git a/drivers/pinctrl/rockchip/pinctrl_rk3368.c b/drivers/pinctrl/rockchip/pinctrl_rk3368.c
> index bdf0758..b77c5ab 100644
> --- a/drivers/pinctrl/rockchip/pinctrl_rk3368.c
> +++ b/drivers/pinctrl/rockchip/pinctrl_rk3368.c
> @@ -30,9 +30,9 @@ static void pinctrl_rk3368_uart_config(struct rk3368_pinctrl_priv *priv,
>   	switch (uart_id) {
>   	case PERIPH_ID_UART2:
>   		rk_clrsetreg(&grf->gpio2a_iomux,
> -			     GPIO2A6_MASK | GPIO2A5_MASK,
> -			     GPIO2A6_UART2_SIN << GPIO2A6_SHIFT |
> -			     GPIO2A5_UART2_SOUT << GPIO2A5_SHIFT);
> +			     RK3368_GPIO2A6_MASK | RK3368_GPIO2A5_MASK,
> +			     RK3368_GPIO2A6_UART2_SIN |
> +			     RK3368_GPIO2A5_UART2_SOUT);
>   		break;
>   	case PERIPH_ID_UART0:
>   		break;
> @@ -42,12 +42,11 @@ static void pinctrl_rk3368_uart_config(struct rk3368_pinctrl_priv *priv,
>   		break;
>   	case PERIPH_ID_UART4:
>   		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);
> +			     RK3368_GPIO0D0_MASK | RK3368_GPIO0D1_MASK |
> +			     RK3368_GPIO0D2_MASK | RK3368_GPIO0D3_MASK,
> +			     RK3368_GPIO0D0_GPIO | RK3368_GPIO0D1_GPIO |
> +			     RK3368_GPIO0D2_UART4_SOUT |
> +			     RK3368_GPIO0D3_UART4_SIN);
>   		break;
>   	default:
>   		debug("uart id = %d iomux error!\n", uart_id);

> On 21 Jul 2017, at 05:50, Andy Yan <andy.yan@rock-chips.com> wrote:
> 
> Hi Philipp:
> 
> 
> On 2017年07月19日 04:36, Philipp Tomsich 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 rename, we do this now
>> before having more code use this.
>> 
>> As some of the downstream drivers (e.g. the Designware GMAC wrapper)
>> may need to include the grf-header of multiple devices, we rename the
>> various defines for the RK3368 by prefixing them with the device name.
>> This avoids future trouble during driver integration.
> 
>    Is that really necessary to add such a prefix for all the register definition, just to fit  the GMAC dirver?
> Maybe the gmac driver can define the platform specific macro in its own code like dwmac-rk in the kernel. Then we can keep the register header file a little tidy.

The problem is not the GMAC driver (although it was the motivation for doing
this change), but rather how easy it is to pick up the wrong grf-file and refer
to the wrong definitions/values… the conflicts are not through the GMAC-specifc
defines but rather through things like the IOMUX masks or (before I moved
this to the DDR-driver) things like the defines to enable DRAM in GRF.

Until we find a way to consistently structure things in such a way that
	(a) common constructs can be shared (e.g. the masks in the IOMUX)
	(b) specific definitions (e.g. the pin-out for a specific device) is not visible outside of its driver
I prefer the names to explicitly refer to each device.
  
That said, the longer-term plan (especially with a larger number of devices
being supported) needs to include a consistent rework of how (and where)
we manage all these definitions from the grf-header files. 

> 
>> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
>> ---
>> 
>>  arch/arm/include/asm/arch-rockchip/grf_rk3368.h | 617 +++++++++++++-----------
>>  drivers/pinctrl/rockchip/pinctrl_rk3368.c       |  17 +-
>>  2 files changed, 334 insertions(+), 300 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..a97dc1e 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,
>> -	GPIO0C6_GPIO            = 0,
>> -	GPIO0C6_LCDC_D18,
>> -	GPIO0C6_TRACE_D8,
>> -	GPIO0C6_UART1_CTSN,
>> -
>> -	GPIO0C5_SHIFT           = 10,
>> -	GPIO0C5_MASK            = 3 << GPIO0C5_SHIFT,
>> -	GPIO0C5_GPIO            = 0,
>> -	GPIO0C5_LCDC_D17,
>> -	GPIO0C5_TRACE_D7,
>> -	GPIO0C5_UART1_SOUT,
>> -
>> -	GPIO0C4_SHIFT           = 8,
>> -	GPIO0C4_MASK            = 3 << GPIO0C4_SHIFT,
>> -	GPIO0C4_GPIO            = 0,
>> -	GPIO0C4_LCDC_D16,
>> -	GPIO0C4_TRACE_D6,
>> -	GPIO0C4_UART1_SIN,
>> -
>> -	GPIO0C3_SHIFT           = 6,
>> -	GPIO0C3_MASK            = 3 << GPIO0C3_SHIFT,
>> -	GPIO0C3_GPIO            = 0,
>> -	GPIO0C3_LCDC_D15,
>> -	GPIO0C3_TRACE_D5,
>> -	GPIO0C3_MCU_JTAG_TDO,
>> -
>> -	GPIO0C2_SHIFT           = 4,
>> -	GPIO0C2_MASK            = 3 << GPIO0C2_SHIFT,
>> -	GPIO0C2_GPIO            = 0,
>> -	GPIO0C2_LCDC_D14,
>> -	GPIO0C2_TRACE_D4,
>> -	GPIO0C2_MCU_JTAG_TDI,
>> -
>> -	GPIO0C1_SHIFT           = 2,
>> -	GPIO0C1_MASK            = 3 << GPIO0C1_SHIFT,
>> -	GPIO0C1_GPIO            = 0,
>> -	GPIO0C1_LCDC_D13,
>> -	GPIO0C1_TRACE_D3,
>> -	GPIO0C1_MCU_JTAG_TRTSN,
>> -
>> -	GPIO0C0_SHIFT           = 0,
>> -	GPIO0C0_MASK            = 3 << GPIO0C0_SHIFT,
>> -	GPIO0C0_GPIO            = 0,
>> -	GPIO0C0_LCDC_D12,
>> -	GPIO0C0_TRACE_D2,
>> -	GPIO0C0_MCU_JTAG_TDO,
>> +	RK3368_GPIO0C7_MASK	       = GENMASK(15, 14),
>> +	RK3368_GPIO0C7_GPIO	       = 0,
>> +	RK3368_GPIO0C7_LCDC_D19        = (1 << 14),
>> +	RK3368_GPIO0C7_TRACE_D9        = (2 << 14),
>> +	RK3368_GPIO0C7_UART1_RTSN      = (3 << 14),
>> +
>> +	RK3368_GPIO0C6_MASK            = GENMASK(13, 12),
>> +	RK3368_GPIO0C6_GPIO            = 0,
>> +	RK3368_GPIO0C6_LCDC_D18        = (1 << 12),
>> +	RK3368_GPIO0C6_TRACE_D8        = (2 << 12),
>> +	RK3368_GPIO0C6_UART1_CTSN      = (3 << 12),
>> +
>> +	RK3368_GPIO0C5_MASK            = GENMASK(11, 10),
>> +	RK3368_GPIO0C5_GPIO            = 0,
>> +	RK3368_GPIO0C5_LCDC_D17        = (1 << 10),
>> +	RK3368_GPIO0C5_TRACE_D7        = (2 << 10),
>> +	RK3368_GPIO0C5_UART1_SOUT      = (3 << 10),
>> +
>> +	RK3368_GPIO0C4_MASK            = GENMASK(9, 8),
>> +	RK3368_GPIO0C4_GPIO            = 0,
>> +	RK3368_GPIO0C4_LCDC_D16        = (1 << 8),
>> +	RK3368_GPIO0C4_TRACE_D6        = (2 << 8),
>> +	RK3368_GPIO0C4_UART1_SIN       = (3 << 8),
>> +
>> +	RK3368_GPIO0C3_MASK            = GENMASK(7, 6),
>> +	RK3368_GPIO0C3_GPIO            = 0,
>> +	RK3368_GPIO0C3_LCDC_D15        = (1 << 6),
>> +	RK3368_GPIO0C3_TRACE_D5        = (2 << 6),
>> +	RK3368_GPIO0C3_MCU_JTAG_TDO    = (3 << 6),
>> +
>> +	RK3368_GPIO0C2_MASK            = GENMASK(5, 4),
>> +	RK3368_GPIO0C2_GPIO            = 0,
>> +	RK3368_GPIO0C2_LCDC_D14        = (1 << 4),
>> +	RK3368_GPIO0C2_TRACE_D4        = (2 << 4),
>> +	RK3368_GPIO0C2_MCU_JTAG_TDI    = (3 << 4),
>> +
>> +	RK3368_GPIO0C1_MASK            = GENMASK(3, 2),
>> +	RK3368_GPIO0C1_GPIO            = 0,
>> +	RK3368_GPIO0C1_LCDC_D13        = (1 << 2),
>> +	RK3368_GPIO0C1_TRACE_D3        = (2 << 2),
>> +	RK3368_GPIO0C1_MCU_JTAG_TRTSN  = (3 << 2),
>> +
>> +	RK3368_GPIO0C0_MASK            = GENMASK(1, 0),
>> +	RK3368_GPIO0C0_GPIO            = 0,
>> +	RK3368_GPIO0C0_LCDC_D12        = (1 << 0),
>> +	RK3368_GPIO0C0_TRACE_D2        = (2 << 0),
>> +	RK3368_GPIO0C0_MCU_JTAG_TDO    = (3 << 0),
>>  };
>>    /*GRF_GPIO0D_IOMUX*/
>>  enum {
>> -	GPIO0D7_SHIFT           = 14,
>> -	GPIO0D7_MASK            = 3 << GPIO0D7_SHIFT,
>> -	GPIO0D7_GPIO            = 0,
>> -	GPIO0D7_LCDC_DCLK,
>> -	GPIO0D7_TRACE_CTL,
>> -	GPIO0D7_PMU_DEBUG5,
>> -
>> -	GPIO0D6_SHIFT           = 12,
>> -	GPIO0D6_MASK            = 3 << GPIO0D6_SHIFT,
>> -	GPIO0D6_GPIO            = 0,
>> -	GPIO0D6_LCDC_DEN,
>> -	GPIO0D6_TRACE_CLK,
>> -	GPIO0D6_PMU_DEBUG4,
>> -
>> -	GPIO0D5_SHIFT           = 10,
>> -	GPIO0D5_MASK            = 3 << GPIO0D5_SHIFT,
>> -	GPIO0D5_GPIO            = 0,
>> -	GPIO0D5_LCDC_VSYNC,
>> -	GPIO0D5_TRACE_D15,
>> -	GPIO0D5_PMU_DEBUG3,
>> -
>> -	GPIO0D4_SHIFT           = 8,
>> -	GPIO0D4_MASK            = 3 << GPIO0D4_SHIFT,
>> -	GPIO0D4_GPIO            = 0,
>> -	GPIO0D4_LCDC_HSYNC,
>> -	GPIO0D4_TRACE_D14,
>> -	GPIO0D4_PMU_DEBUG2,
>> -
>> -	GPIO0D3_SHIFT           = 6,
>> -	GPIO0D3_MASK            = 3 << GPIO0D3_SHIFT,
>> -	GPIO0D3_GPIO            = 0,
>> -	GPIO0D3_LCDC_D23,
>> -	GPIO0D3_TRACE_D13,
>> -	GPIO0D3_UART4_SIN,
>> -
>> -	GPIO0D2_SHIFT           = 4,
>> -	GPIO0D2_MASK            = 3 << GPIO0D2_SHIFT,
>> -	GPIO0D2_GPIO            = 0,
>> -	GPIO0D2_LCDC_D22,
>> -	GPIO0D2_TRACE_D12,
>> -	GPIO0D2_UART4_SOUT,
>> -
>> -	GPIO0D1_SHIFT           = 2,
>> -	GPIO0D1_MASK            = 3 << GPIO0D1_SHIFT,
>> -	GPIO0D1_GPIO            = 0,
>> -	GPIO0D1_LCDC_D21,
>> -	GPIO0D1_TRACE_D11,
>> -	GPIO0D1_UART4_RTSN,
>> -
>> -	GPIO0D0_SHIFT           = 0,
>> -	GPIO0D0_MASK            = 3 << GPIO0D0_SHIFT,
>> -	GPIO0D0_GPIO            = 0,
>> -	GPIO0D0_LCDC_D20,
>> -	GPIO0D0_TRACE_D10,
>> -	GPIO0D0_UART4_CTSN,
>> +	RK3368_GPIO0D7_MASK            = GENMASK(15, 14),
>> +	RK3368_GPIO0D7_GPIO            = 0,
>> +	RK3368_GPIO0D7_LCDC_DCLK       = (1 << 14),
>> +	RK3368_GPIO0D7_TRACE_CTL       = (2 << 14),
>> +	RK3368_GPIO0D7_PMU_DEBUG5      = (3 << 14),
>> +
>> +	RK3368_GPIO0D6_MASK            = GENMASK(13, 12),
>> +	RK3368_GPIO0D6_GPIO            = 0,
>> +	RK3368_GPIO0D6_LCDC_DEN        = (1 << 12),
>> +	RK3368_GPIO0D6_TRACE_CLK       = (2 << 12),
>> +	RK3368_GPIO0D6_PMU_DEBUG4      = (3 << 12),
>> +
>> +	RK3368_GPIO0D5_MASK            = GENMASK(11, 10),
>> +	RK3368_GPIO0D5_GPIO            = 0,
>> +	GPIO0D5_LCDC_VSYNC             = (1 << 10),
>> +	RK3368_GPIO0D5_TRACE_D15       = (2 << 10),
>> +	RK3368_GPIO0D5_PMU_DEBUG3      = (3 << 10),
>> +
>> +	RK3368_GPIO0D4_MASK            = GENMASK(9, 8),
>> +	RK3368_GPIO0D4_GPIO            = 0,
>> +	RK3368_GPIO0D4_LCDC_HSYNC      = (1 << 8),
>> +	RK3368_GPIO0D4_TRACE_D14       = (2 << 8),
>> +	RK3368_GPIO0D4_PMU_DEBUG2      = (3 << 8),
>> +
>> +	RK3368_GPIO0D3_MASK            = GENMASK(7, 6),
>> +	RK3368_GPIO0D3_GPIO            = 0,
>> +	RK3368_GPIO0D3_LCDC_D23        = (1 << 6),
>> +	RK3368_GPIO0D3_TRACE_D13       = (2 << 6),
>> +	RK3368_GPIO0D3_UART4_SIN       = (3 << 6),
>> +
>> +	RK3368_GPIO0D2_MASK            = GENMASK(5, 4),
>> +	RK3368_GPIO0D2_GPIO            = 0,
>> +	RK3368_GPIO0D2_LCDC_D22        = (1 << 4),
>> +	RK3368_GPIO0D2_TRACE_D12       = (2 << 4),
>> +	RK3368_GPIO0D2_UART4_SOUT      = (3 << 4),
>> +
>> +	RK3368_GPIO0D1_MASK            = GENMASK(3, 2),
>> +	RK3368_GPIO0D1_GPIO            = 0,
>> +	RK3368_GPIO0D1_LCDC_D21        = (1 << 2),
>> +	RK3368_GPIO0D1_TRACE_D11       = (2 << 2),
>> +	RK3368_GPIO0D1_UART4_RTSN      = (3 << 2),
>> +
>> +	RK3368_GPIO0D0_MASK            = GENMASK(1, 0),
>> +	RK3368_GPIO0D0_GPIO            = 0,
>> +	RK3368_GPIO0D0_LCDC_D20        = (1 << 0),
>> +	RK3368_GPIO0D0_TRACE_D10       = (2 << 0),
>> +	RK3368_GPIO0D0_UART4_CTSN      = (3 << 0),
>>  };
>>    /*GRF_GPIO2A_IOMUX*/
>>  enum {
>> -	GPIO2A7_SHIFT           = 14,
>> -	GPIO2A7_MASK            = 3 << GPIO2A7_SHIFT,
>> -	GPIO2A7_GPIO            = 0,
>> -	GPIO2A7_SDMMC0_D2,
>> -	GPIO2A7_JTAG_TCK,
>> -
>> -	GPIO2A6_SHIFT           = 12,
>> -	GPIO2A6_MASK            = 3 << GPIO2A6_SHIFT,
>> -	GPIO2A6_GPIO            = 0,
>> -	GPIO2A6_SDMMC0_D1,
>> -	GPIO2A6_UART2_SIN,
>> -
>> -	GPIO2A5_SHIFT           = 10,
>> -	GPIO2A5_MASK            = 3 << GPIO2A5_SHIFT,
>> -	GPIO2A5_GPIO            = 0,
>> -	GPIO2A5_SDMMC0_D0,
>> -	GPIO2A5_UART2_SOUT,
>> -
>> -	GPIO2A4_SHIFT           = 8,
>> -	GPIO2A4_MASK            = 3 << GPIO2A4_SHIFT,
>> -	GPIO2A4_GPIO            = 0,
>> -	GPIO2A4_FLASH_DQS,
>> -	GPIO2A4_EMMC_CLKO,
>> -
>> -	GPIO2A3_SHIFT           = 6,
>> -	GPIO2A3_MASK            = 3 << GPIO2A3_SHIFT,
>> -	GPIO2A3_GPIO            = 0,
>> -	GPIO2A3_FLASH_CSN3,
>> -	GPIO2A3_EMMC_RSTNO,
>> -
>> -	GPIO2A2_SHIFT           = 4,
>> -	GPIO2A2_MASK            = 3 << GPIO2A2_SHIFT,
>> -	GPIO2A2_GPIO           = 0,
>> -	GPIO2A2_FLASH_CSN2,
>> -
>> -	GPIO2A1_SHIFT           = 2,
>> -	GPIO2A1_MASK            = 3 << GPIO2A1_SHIFT,
>> -	GPIO2A1_GPIO            = 0,
>> -	GPIO2A1_FLASH_CSN1,
>> -
>> -	GPIO2A0_SHIFT           = 0,
>> -	GPIO2A0_MASK            = 3 << GPIO2A0_SHIFT,
>> -	GPIO2A0_GPIO            = 0,
>> -	GPIO2A0_FLASH_CSN0,
>> +	RK3368_GPIO2A7_MASK            = GENMASK(15, 14),
>> +	RK3368_GPIO2A7_GPIO            = 0,
>> +	RK3368_GPIO2A7_SDMMC0_D2       = (1 << 14),
>> +	RK3368_GPIO2A7_JTAG_TCK        = (2 << 14),
>> +
>> +	RK3368_GPIO2A6_MASK            = GENMASK(13, 12),
>> +	RK3368_GPIO2A6_GPIO            = 0,
>> +	RK3368_GPIO2A6_SDMMC0_D1       = (1 << 12),
>> +	RK3368_GPIO2A6_UART2_SIN       = (2 << 12),
>> +
>> +	RK3368_GPIO2A5_MASK            = GENMASK(11, 10),
>> +	RK3368_GPIO2A5_GPIO            = 0,
>> +	RK3368_GPIO2A5_SDMMC0_D0       = (1 << 10),
>> +	RK3368_GPIO2A5_UART2_SOUT      = (2 << 10),
>> +
>> +	RK3368_GPIO2A4_MASK            = GENMASK(9, 8),
>> +	RK3368_GPIO2A4_RK3368_GPIO     = 0,
>> +	RK3368_GPIO2A4_FLASH_DQS       = (1 << 8),
>> +	RK3368_GPIO2A4_EMMC_CLKOUT     = (2 << 8),
>> +
>> +	RK3368_GPIO2A3_MASK            = GENMASK(7, 6),
>> +	RK3368_GPIO2A3_GPIO            = 0,
>> +	RK3368_GPIO2A3_FLASH_CSN3      = (1 << 6),
>> +	RK3368_GPIO2A3_EMMC_RSTNOUT    = (2 << 6),
>> +
>> +	RK3368_GPIO2A2_MASK            = GENMASK(5, 4),
>> +	RK3368_GPIO2A2_GPIO            = 0,
>> +	RK3368_GPIO2A2_FLASH_CSN2      = (1 << 4),
>> +
>> +	RK3368_GPIO2A1_MASK            = GENMASK(3, 2),
>> +	RK3368_GPIO2A1_GPIO            = 0,
>> +	RK3368_GPIO2A1_FLASH_CSN1      = (1 << 2),
>> +
>> +	RK3368_GPIO2A0_MASK            = GENMASK(1, 0),
>> +	RK3368_GPIO2A0_GPIO            = 0,
>> +	RK3368_GPIO2A0_FLASH_CSN0      = (1 << 0),
>>  };
>>  -/*GRF_GPIO2D_IOMUX*/
>> +/*GRF_RK3368_GPIO2D_IOMUX*/
>>  enum {
>> -	GPIO2D7_SHIFT           = 14,
>> -	GPIO2D7_MASK            = 3 << GPIO2D7_SHIFT,
>> -	GPIO2D7_GPIO            = 0,
>> -	GPIO2D7_SDIO0_D3,
>> -
>> -	GPIO2D6_SHIFT           = 12,
>> -	GPIO2D6_MASK            = 3 << GPIO2D6_SHIFT,
>> -	GPIO2D6_GPIO            = 0,
>> -	GPIO2D6_SDIO0_D2,
>> -
>> -	GPIO2D5_SHIFT           = 10,
>> -	GPIO2D5_MASK            = 3 << GPIO2D5_SHIFT,
>> -	GPIO2D5_GPIO            = 0,
>> -	GPIO2D5_SDIO0_D1,
>> -
>> -	GPIO2D4_SHIFT           = 8,
>> -	GPIO2D4_MASK            = 3 << GPIO2D4_SHIFT,
>> -	GPIO2D4_GPIO            = 0,
>> -	GPIO2D4_SDIO0_D0,
>> -
>> -	GPIO2D3_SHIFT           = 6,
>> -	GPIO2D3_MASK            = 3 << GPIO2D3_SHIFT,
>> -	GPIO2D3_GPIO            = 0,
>> -	GPIO2D3_UART0_RTS0,
>> -
>> -	GPIO2D2_SHIFT           = 4,
>> -	GPIO2D2_MASK            = 3 << GPIO2D2_SHIFT,
>> -	GPIO2D2_GPIO            = 0,
>> -	GPIO2D2_UART0_CTS0,
>> -
>> -	GPIO2D1_SHIFT           = 2,
>> -	GPIO2D1_MASK            = 3 << GPIO2D1_SHIFT,
>> -	GPIO2D1_GPIO            = 0,
>> -	GPIO2D1_UART0_SOUT,
>> -
>> -	GPIO2D0_SHIFT           = 0,
>> -	GPIO2D0_MASK            = 3 << GPIO2D0_SHIFT,
>> -	GPIO2D0_GPIO            = 0,
>> -	GPIO2D0_UART0_SIN,
>> +	RK3368_GPIO2D7_MASK            = GENMASK(15, 14),
>> +	RK3368_GPIO2D7_GPIO            = 0,
>> +	RK3368_GPIO2D7_SDIO0_D3        = (1 << 14),
>> +
>> +	RK3368_GPIO2D6_MASK            = GENMASK(13, 12),
>> +	RK3368_GPIO2D6_GPIO            = 0,
>> +	RK3368_GPIO2D6_SDIO0_D2        = (1 << 12),
>> +
>> +	RK3368_GPIO2D5_MASK            = GENMASK(11, 10),
>> +	RK3368_GPIO2D5_GPIO            = 0,
>> +	RK3368_GPIO2D5_SDIO0_D1        = (1 << 10),
>> +
>> +	RK3368_GPIO2D4_MASK            = GENMASK(9, 8),
>> +	RK3368_GPIO2D4_GPIO            = 0,
>> +	RK3368_GPIO2D4_SDIO0_D0        = (1 << 8),
>> +
>> +	RK3368_GPIO2D3_MASK            = GENMASK(7, 6),
>> +	RK3368_GPIO2D3_GPIO            = 0,
>> +	RK3368_GPIO2D3_UART0_RTS0      = (1 << 6),
>> +
>> +	RK3368_GPIO2D2_MASK            = GENMASK(5, 4),
>> +	RK3368_GPIO2D2_GPIO            = 0,
>> +	RK3368_GPIO2D2_UART0_CTS0      = (1 << 4),
>> +
>> +	RK3368_GPIO2D1_MASK            = GENMASK(3, 2),
>> +	RK3368_GPIO2D1_GPIO            = 0,
>> +	RK3368_GPIO2D1_UART0_SOUT      = (1 << 2),
>> +
>> +	RK3368_GPIO2D0_MASK            = GENMASK(1, 0),
>> +	RK3368_GPIO2D0_GPIO            = 0,
>> +	RK3368_GPIO2D0_UART0_SIN       = (1 << 0),
>> +};
>> +
>> +/* GRF_GPIO1C_IOMUX */
>> +enum {
>> +	RK3368_GPIO1C7_MASK            = GENMASK(15, 14),
>> +	RK3368_GPIO1C7_GPIO            = 0,
>> +	RK3368_GPIO1C7_EMMC_DATA5      = (2 << 14),
>> +
>> +	RK3368_GPIO1C6_MASK            = GENMASK(13, 12),
>> +	RK3368_GPIO1C6_GPIO            = 0,
>> +	RK3368_GPIO1C6_EMMC_DATA4      = (2 << 12),
>> +
>> +	RK3368_GPIO1C5_MASK            = GENMASK(11, 10),
>> +	RK3368_GPIO1C5_GPIO            = 0,
>> +	RK3368_GPIO1C5_EMMC_DATA3      = (2 << 10),
>> +
>> +	RK3368_GPIO1C4_MASK            = GENMASK(9, 8),
>> +	RK3368_GPIO1C4_GPIO            = 0,
>> +	RK3368_GPIO1C4_EMMC_DATA2      = (2 << 8),
>> +
>> +	RK3368_GPIO1C3_MASK            = GENMASK(7, 6),
>> +	RK3368_GPIO1C3_GPIO            = 0,
>> +	RK3368_GPIO1C3_EMMC_DATA1      = (2 << 6),
>> +
>> +	RK3368_GPIO1C2_MASK            = GENMASK(5, 4),
>> +	RK3368_GPIO1C2_GPIO            = 0,
>> +	RK3368_GPIO1C2_EMMC_DATA0      = (2 << 4),
>> +};
>> +
>> +/* GRF_GPIO1D_IOMUX*/
>> +enum {
>> +	RK3368_GPIO1D3_MASK            = GENMASK(7, 6),
>> +	RK3368_GPIO1D3_GPIO            = 0,
>> +	RK3368_GPIO1D3_EMMC_PWREN      = (2 << 6),
>> +
>> +	RK3368_GPIO1D2_MASK            = GENMASK(5, 4),
>> +	RK3368_GPIO1D2_GPIO            = 0,
>> +	RK3368_GPIO1D2_EMMC_CMD        = (2 << 4),
>> +
>> +	RK3368_GPIO1D1_MASK            = GENMASK(3, 2),
>> +	RK3368_GPIO1D1_GPIO            = 0,
>> +	RK3368_GPIO1D1_EMMC_DATA7      = (2 << 2),
>> +
>> +	RK3368_GPIO1D0_MASK            = GENMASK(1, 0),
>> +	RK3368_GPIO1D0_GPIO            = 0,
>> +	RK3368_GPIO1D0_EMMC_DATA6      = (2 << 0),
>> +};
>> +
>> +
>> +/*GRF_GPIO3B_IOMUX*/
>> +enum {
>> +	RK3368_GPIO3B7_MASK            = GENMASK(15, 14),
>> +	RK3368_GPIO3B7_GPIO            = 0,
>> +	RK3368_GPIO3B7_MAC_RXD0        = (1 << 14),
>> +
>> +	RK3368_GPIO3B6_MASK            = GENMASK(13, 12),
>> +	RK3368_GPIO3B6_GPIO            = 0,
>> +	RK3368_GPIO3B6_MAC_TXD3        = (1 << 12),
>> +
>> +	RK3368_GPIO3B5_MASK            = GENMASK(11, 10),
>> +	RK3368_GPIO3B5_GPIO            = 0,
>> +	RK3368_GPIO3B5_MAC_TXEN        = (1 << 10),
>> +
>> +	RK3368_GPIO3B4_MASK            = GENMASK(9, 8),
>> +	RK3368_GPIO3B4_GPIO            = 0,
>> +	RK3368_GPIO3B4_MAC_COL         = (1 << 8),
>> +
>> +	RK3368_GPIO3B3_MASK            = GENMASK(7, 6),
>> +	RK3368_GPIO3B3_GPIO            = 0,
>> +	RK3368_GPIO3B3_MAC_CRS         = (1 << 6),
>> +
>> +	RK3368_GPIO3B2_MASK            = GENMASK(5, 4),
>> +	RK3368_GPIO3B2_GPIO            = 0,
>> +	RK3368_GPIO3B2_MAC_TXD2        = (1 << 4),
>> +
>> +	RK3368_GPIO3B1_MASK            = GENMASK(3, 2),
>> +	RK3368_GPIO3B1_GPIO            = 0,
>> +	RK3368_GPIO3B1_MAC_TXD1        = (1 << 2),
>> +
>> +	RK3368_GPIO3B0_MASK            = GENMASK(1, 0),
>> +	RK3368_GPIO3B0_GPIO            = 0,
>> +	RK3368_GPIO3B0_MAC_TXD0        = (1 << 0),
>> +	RK3368_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_GPIO            = 0,
>> -	GPIO3C6_MAC_CLK,
>> -	GPIO3C6_ISP_SHUTTERTRIG,
>> -
>> -	GPIO3C5_SHIFT           = 10,
>> -	GPIO3C5_MASK            = 3 << GPIO3C5_SHIFT,
>> -	GPIO3C5_GPIO            = 0,
>> -	GPIO3C5_MAC_RXER,
>> -	GPIO3C5_ISP_PRELIGHTTRIG,
>> -
>> -	GPIO3C4_SHIFT           = 8,
>> -	GPIO3C4_MASK            = 3 << GPIO3C4_SHIFT,
>> -	GPIO3C4_GPIO            = 0,
>> -	GPIO3C4_MAC_RXDV,
>> -	GPIO3C4_ISP_FLASHTRIGOUT,
>> -
>> -	GPIO3C3_SHIFT           = 6,
>> -	GPIO3C3_MASK            = 3 << GPIO3C3_SHIFT,
>> -	GPIO3C3_GPIO            = 0,
>> -	GPIO3C3_MAC_RXDV,
>> -	GPIO3C3_EMMC_RSTNO,
>> -
>> -	GPIO3C2_SHIFT           = 4,
>> -	GPIO3C2_MASK            = 3 << GPIO3C2_SHIFT,
>> -	GPIO3C2_MAC_MDC            = 0,
>> -	GPIO3C2_ISP_SHUTTEREN,
>> -
>> -	GPIO3C1_SHIFT           = 2,
>> -	GPIO3C1_MASK            = 3 << GPIO3C1_SHIFT,
>> -	GPIO3C1_GPIO            = 0,
>> -	GPIO3C1_MAC_RXD2,
>> -	GPIO3C1_UART3_RTSN,
>> -
>> -	GPIO3C0_SHIFT           = 0,
>> -	GPIO3C0_MASK            = 3 << GPIO3C0_SHIFT,
>> -	GPIO3C0_GPIO            = 0,
>> -	GPIO3C0_MAC_RXD1,
>> -	GPIO3C0_UART3_CTSN,
>> -	GPIO3C0_GPS_RFCLK,
>> +	RK3368_GPIO3C6_MASK            = GENMASK(13, 12),
>> +	RK3368_GPIO3C6_GPIO            = 0,
>> +	RK3368_GPIO3C6_MAC_CLK         = (1 << 12),
>> +
>> +	RK3368_GPIO3C5_MASK            = GENMASK(11, 10),
>> +	RK3368_GPIO3C5_GPIO            = 0,
>> +	RK3368_GPIO3C5_MAC_RXEN        = (1 << 10),
>> +
>> +	RK3368_GPIO3C4_MASK            = GENMASK(9, 8),
>> +	RK3368_GPIO3C4_GPIO            = 0,
>> +	RK3368_GPIO3C4_MAC_RXDV        = (1 << 8),
>> +
>> +	RK3368_GPIO3C3_MASK            = GENMASK(7, 6),
>> +	RK3368_GPIO3C3_GPIO            = 0,
>> +	RK3368_GPIO3C3_MAC_MDC         = (1 << 6),
>> +
>> +	RK3368_GPIO3C2_MASK            = GENMASK(5, 4),
>> +	RK3368_GPIO3C2_GPIO            = 0,
>> +	RK3368_GPIO3C2_MAC_RXD3        = (1 << 4),
>> +
>> +	RK3368_GPIO3C1_MASK            = GENMASK(3, 2),
>> +	RK3368_GPIO3C1_GPIO            = 0,
>> +	RK3368_GPIO3C1_MAC_RXD2        = (1 << 2),
>> +
>> +	RK3368_GPIO3C0_MASK            = GENMASK(1, 0),
>> +	RK3368_GPIO3C0_GPIO            = 0,
>> +	RK3368_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_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,
>> -	GPIO3D1_GPIO            = 0,
>> -	GPIO3D1_MAC_RXCLKIN,
>> -	GPIO3D1_I2C4_SCL,
>> -
>> -	GPIO3D0_SHIFT           = 0,
>> -	GPIO3D0_MASK            = 3 << GPIO3D0_SHIFT,
>> -	GPIO3D0_GPIO            = 0,
>> -	GPIO3D0_MAC_MDIO,
>> -	GPIO3D0_I2C4_SDA,
>> +	RK3368_GPIO3D4_MASK            = GENMASK(9, 8),
>> +	RK3368_GPIO3D4_GPIO            = 0,
>> +	RK3368_GPIO3D4_MAC_TXCLK       = (1 << 8),
>> +
>> +	RK3368_GPIO3D1_MASK            = GENMASK(3, 2),
>> +	RK3368_GPIO3D1_GPIO            = 0,
>> +	RK3368_GPIO3D1_MAC_RXCLK       = (1 << 2),
>> +
>> +	RK3368_GPIO3D0_MASK            = GENMASK(1, 0),
>> +	RK3368_GPIO3D0_GPIO            = 0,
>> +	RK3368_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*/
>> @@ -440,4 +445,34 @@ enum {
>>  	MCU_CODE_BASE_BIT31_BIT28_SHIFT		= 0,
>>  	MCU_CODE_BASE_BIT31_BIT28_MASK		= GENMASK(3, 0),
>>  };
>> +
>> +/*GRF_SOC_CON15*/
>> +enum {
>> +	RK3368_GMAC_PHY_INTF_SEL_RGMII = 1 << 9,
>> +	RK3368_GMAC_PHY_INTF_SEL_MASK = GENMASK(11, 9),
>> +	RK3368_RMII_MODE_MASK  = BIT(6),
>> +	RK3368_RMII_MODE       = BIT(6),
>> +	RK3368_GMAC_CLK_SEL_2_5M = 2 << 4,
>> +	RK3368_GMAC_CLK_SEL_25M = 3 << 4,
>> +	RK3368_GMAC_CLK_SEL_125M = 0 << 4,
>> +	RK3368_GMAC_CLK_SEL_MASK = GENMASK(5, 4),
>> +};
>> +
>> +/* GRF_SOC_CON16 */
>> +enum {
>> +	RK3368_RXCLK_DLY_ENA_GMAC_MASK = BIT(15),
>> +	RK3368_RXCLK_DLY_ENA_GMAC_DISABLE = 0,
>> +	RK3368_RXCLK_DLY_ENA_GMAC_ENABLE = BIT(15),
>> +
>> +	RK3368_TXCLK_DLY_ENA_GMAC_MASK = BIT(7),
>> +	RK3368_TXCLK_DLY_ENA_GMAC_DISABLE = 0,
>> +	RK3368_TXCLK_DLY_ENA_GMAC_ENABLE = BIT(7),
>> +
>> +	RK3368_CLK_RX_DL_CFG_GMAC_SHIFT = 8,
>> +	RK3368_CLK_RX_DL_CFG_GMAC_MASK = GENMASK(14, 8),
>> +
>> +	RK3368_CLK_TX_DL_CFG_GMAC_SHIFT = 0,
>> +	RK3368_CLK_TX_DL_CFG_GMAC_MASK = GENMASK(6, 0),
>> +};
>> +
>>  #endif
>> diff --git a/drivers/pinctrl/rockchip/pinctrl_rk3368.c b/drivers/pinctrl/rockchip/pinctrl_rk3368.c
>> index bdf0758..b77c5ab 100644
>> --- a/drivers/pinctrl/rockchip/pinctrl_rk3368.c
>> +++ b/drivers/pinctrl/rockchip/pinctrl_rk3368.c
>> @@ -30,9 +30,9 @@ static void pinctrl_rk3368_uart_config(struct rk3368_pinctrl_priv *priv,
>>  	switch (uart_id) {
>>  	case PERIPH_ID_UART2:
>>  		rk_clrsetreg(&grf->gpio2a_iomux,
>> -			     GPIO2A6_MASK | GPIO2A5_MASK,
>> -			     GPIO2A6_UART2_SIN << GPIO2A6_SHIFT |
>> -			     GPIO2A5_UART2_SOUT << GPIO2A5_SHIFT);
>> +			     RK3368_GPIO2A6_MASK | RK3368_GPIO2A5_MASK,
>> +			     RK3368_GPIO2A6_UART2_SIN |
>> +			     RK3368_GPIO2A5_UART2_SOUT);
>>  		break;
>>  	case PERIPH_ID_UART0:
>>  		break;
>> @@ -42,12 +42,11 @@ static void pinctrl_rk3368_uart_config(struct rk3368_pinctrl_priv *priv,
>>  		break;
>>  	case PERIPH_ID_UART4:
>>  		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);
>> +			     RK3368_GPIO0D0_MASK | RK3368_GPIO0D1_MASK |
>> +			     RK3368_GPIO0D2_MASK | RK3368_GPIO0D3_MASK,
>> +			     RK3368_GPIO0D0_GPIO | RK3368_GPIO0D1_GPIO |
>> +			     RK3368_GPIO0D2_UART4_SOUT |
>> +			     RK3368_GPIO0D3_UART4_SIN);
>>  		break;
>>  	default:
>>  		debug("uart id = %d iomux error!\n", uart_id);
> 
>

Hi Philipp:


On 2017年07月21日 16:34, Dr. Philipp Tomsich wrote:
>> On 21 Jul 2017, at 05:50, Andy Yan <andy.yan@rock-chips.com> wrote:
>>
>> Hi Philipp:
>>
>>
>> On 2017年07月19日 04:36, Philipp Tomsich 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 rename, we do this now
>>> before having more code use this.
>>>
>>> As some of the downstream drivers (e.g. the Designware GMAC wrapper)
>>> may need to include the grf-header of multiple devices, we rename the
>>> various defines for the RK3368 by prefixing them with the device name.
>>> This avoids future trouble during driver integration.
>>     Is that really necessary to add such a prefix for all the register definition, just to fit  the GMAC dirver?
>> Maybe the gmac driver can define the platform specific macro in its own code like dwmac-rk in the kernel. Then we can keep the register header file a little tidy.
> The problem is not the GMAC driver (although it was the motivation for doing
> this change), but rather how easy it is to pick up the wrong grf-file and refer
> to the wrong definitions/values… the conflicts are not through the GMAC-specifc
> defines but rather through things like the IOMUX masks or (before I moved
> this to the DDR-driver) things like the defines to enable DRAM in GRF.
>
> Until we find a way to consistently structure things in such a way that
> 	(a) common constructs can be shared (e.g. the masks in the IOMUX)
> 	(b) specific definitions (e.g. the pin-out for a specific device) is not visible outside of its driver
> I prefer the names to explicitly refer to each device.
>    
> That said, the longer-term plan (especially with a larger number of devices
> being supported) needs to include a consistent rework of how (and where)
> we manage all these definitions from the grf-header files.
It's true that many devices use GRF, we also face this condition in the 
kernel land.And all the drivers in kernel handle this device specific 
definition in the driver itself(by of_device_id->data or MACRO). And 
there is also no need to use the grf constructs, the common syscon api 
will provide  you the grf base address. I think this keeps things 
simple, we don't need to rely on too much of the grf or cru header, 
maybe some day we can remove the header file, just as clean as the kernel.
On the other hand, add such a prefix of rk3368 breaks the consistency 
with other rockchip devices.

>
>>> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
>>> ---
>>>
>>>   arch/arm/include/asm/arch-rockchip/grf_rk3368.h | 617 +++++++++++++-----------
>>>   drivers/pinctrl/rockchip/pinctrl_rk3368.c       |  17 +-
>>>   2 files changed, 334 insertions(+), 300 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..a97dc1e 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,
>>> -	GPIO0C6_GPIO            = 0,
>>> -	GPIO0C6_LCDC_D18,
>>> -	GPIO0C6_TRACE_D8,
>>> -	GPIO0C6_UART1_CTSN,
>>> -
>>> -	GPIO0C5_SHIFT           = 10,
>>> -	GPIO0C5_MASK            = 3 << GPIO0C5_SHIFT,
>>> -	GPIO0C5_GPIO            = 0,
>>> -	GPIO0C5_LCDC_D17,
>>> -	GPIO0C5_TRACE_D7,
>>> -	GPIO0C5_UART1_SOUT,
>>> -
>>> -	GPIO0C4_SHIFT           = 8,
>>> -	GPIO0C4_MASK            = 3 << GPIO0C4_SHIFT,
>>> -	GPIO0C4_GPIO            = 0,
>>> -	GPIO0C4_LCDC_D16,
>>> -	GPIO0C4_TRACE_D6,
>>> -	GPIO0C4_UART1_SIN,
>>> -
>>> -	GPIO0C3_SHIFT           = 6,
>>> -	GPIO0C3_MASK            = 3 << GPIO0C3_SHIFT,
>>> -	GPIO0C3_GPIO            = 0,
>>> -	GPIO0C3_LCDC_D15,
>>> -	GPIO0C3_TRACE_D5,
>>> -	GPIO0C3_MCU_JTAG_TDO,
>>> -
>>> -	GPIO0C2_SHIFT           = 4,
>>> -	GPIO0C2_MASK            = 3 << GPIO0C2_SHIFT,
>>> -	GPIO0C2_GPIO            = 0,
>>> -	GPIO0C2_LCDC_D14,
>>> -	GPIO0C2_TRACE_D4,
>>> -	GPIO0C2_MCU_JTAG_TDI,
>>> -
>>> -	GPIO0C1_SHIFT           = 2,
>>> -	GPIO0C1_MASK            = 3 << GPIO0C1_SHIFT,
>>> -	GPIO0C1_GPIO            = 0,
>>> -	GPIO0C1_LCDC_D13,
>>> -	GPIO0C1_TRACE_D3,
>>> -	GPIO0C1_MCU_JTAG_TRTSN,
>>> -
>>> -	GPIO0C0_SHIFT           = 0,
>>> -	GPIO0C0_MASK            = 3 << GPIO0C0_SHIFT,
>>> -	GPIO0C0_GPIO            = 0,
>>> -	GPIO0C0_LCDC_D12,
>>> -	GPIO0C0_TRACE_D2,
>>> -	GPIO0C0_MCU_JTAG_TDO,
>>> +	RK3368_GPIO0C7_MASK	       = GENMASK(15, 14),
>>> +	RK3368_GPIO0C7_GPIO	       = 0,
>>> +	RK3368_GPIO0C7_LCDC_D19        = (1 << 14),
>>> +	RK3368_GPIO0C7_TRACE_D9        = (2 << 14),
>>> +	RK3368_GPIO0C7_UART1_RTSN      = (3 << 14),
>>> +
>>> +	RK3368_GPIO0C6_MASK            = GENMASK(13, 12),
>>> +	RK3368_GPIO0C6_GPIO            = 0,
>>> +	RK3368_GPIO0C6_LCDC_D18        = (1 << 12),
>>> +	RK3368_GPIO0C6_TRACE_D8        = (2 << 12),
>>> +	RK3368_GPIO0C6_UART1_CTSN      = (3 << 12),
>>> +
>>> +	RK3368_GPIO0C5_MASK            = GENMASK(11, 10),
>>> +	RK3368_GPIO0C5_GPIO            = 0,
>>> +	RK3368_GPIO0C5_LCDC_D17        = (1 << 10),
>>> +	RK3368_GPIO0C5_TRACE_D7        = (2 << 10),
>>> +	RK3368_GPIO0C5_UART1_SOUT      = (3 << 10),
>>> +
>>> +	RK3368_GPIO0C4_MASK            = GENMASK(9, 8),
>>> +	RK3368_GPIO0C4_GPIO            = 0,
>>> +	RK3368_GPIO0C4_LCDC_D16        = (1 << 8),
>>> +	RK3368_GPIO0C4_TRACE_D6        = (2 << 8),
>>> +	RK3368_GPIO0C4_UART1_SIN       = (3 << 8),
>>> +
>>> +	RK3368_GPIO0C3_MASK            = GENMASK(7, 6),
>>> +	RK3368_GPIO0C3_GPIO            = 0,
>>> +	RK3368_GPIO0C3_LCDC_D15        = (1 << 6),
>>> +	RK3368_GPIO0C3_TRACE_D5        = (2 << 6),
>>> +	RK3368_GPIO0C3_MCU_JTAG_TDO    = (3 << 6),
>>> +
>>> +	RK3368_GPIO0C2_MASK            = GENMASK(5, 4),
>>> +	RK3368_GPIO0C2_GPIO            = 0,
>>> +	RK3368_GPIO0C2_LCDC_D14        = (1 << 4),
>>> +	RK3368_GPIO0C2_TRACE_D4        = (2 << 4),
>>> +	RK3368_GPIO0C2_MCU_JTAG_TDI    = (3 << 4),
>>> +
>>> +	RK3368_GPIO0C1_MASK            = GENMASK(3, 2),
>>> +	RK3368_GPIO0C1_GPIO            = 0,
>>> +	RK3368_GPIO0C1_LCDC_D13        = (1 << 2),
>>> +	RK3368_GPIO0C1_TRACE_D3        = (2 << 2),
>>> +	RK3368_GPIO0C1_MCU_JTAG_TRTSN  = (3 << 2),
>>> +
>>> +	RK3368_GPIO0C0_MASK            = GENMASK(1, 0),
>>> +	RK3368_GPIO0C0_GPIO            = 0,
>>> +	RK3368_GPIO0C0_LCDC_D12        = (1 << 0),
>>> +	RK3368_GPIO0C0_TRACE_D2        = (2 << 0),
>>> +	RK3368_GPIO0C0_MCU_JTAG_TDO    = (3 << 0),
>>>   };
>>>     /*GRF_GPIO0D_IOMUX*/
>>>   enum {
>>> -	GPIO0D7_SHIFT           = 14,
>>> -	GPIO0D7_MASK            = 3 << GPIO0D7_SHIFT,
>>> -	GPIO0D7_GPIO            = 0,
>>> -	GPIO0D7_LCDC_DCLK,
>>> -	GPIO0D7_TRACE_CTL,
>>> -	GPIO0D7_PMU_DEBUG5,
>>> -
>>> -	GPIO0D6_SHIFT           = 12,
>>> -	GPIO0D6_MASK            = 3 << GPIO0D6_SHIFT,
>>> -	GPIO0D6_GPIO            = 0,
>>> -	GPIO0D6_LCDC_DEN,
>>> -	GPIO0D6_TRACE_CLK,
>>> -	GPIO0D6_PMU_DEBUG4,
>>> -
>>> -	GPIO0D5_SHIFT           = 10,
>>> -	GPIO0D5_MASK            = 3 << GPIO0D5_SHIFT,
>>> -	GPIO0D5_GPIO            = 0,
>>> -	GPIO0D5_LCDC_VSYNC,
>>> -	GPIO0D5_TRACE_D15,
>>> -	GPIO0D5_PMU_DEBUG3,
>>> -
>>> -	GPIO0D4_SHIFT           = 8,
>>> -	GPIO0D4_MASK            = 3 << GPIO0D4_SHIFT,
>>> -	GPIO0D4_GPIO            = 0,
>>> -	GPIO0D4_LCDC_HSYNC,
>>> -	GPIO0D4_TRACE_D14,
>>> -	GPIO0D4_PMU_DEBUG2,
>>> -
>>> -	GPIO0D3_SHIFT           = 6,
>>> -	GPIO0D3_MASK            = 3 << GPIO0D3_SHIFT,
>>> -	GPIO0D3_GPIO            = 0,
>>> -	GPIO0D3_LCDC_D23,
>>> -	GPIO0D3_TRACE_D13,
>>> -	GPIO0D3_UART4_SIN,
>>> -
>>> -	GPIO0D2_SHIFT           = 4,
>>> -	GPIO0D2_MASK            = 3 << GPIO0D2_SHIFT,
>>> -	GPIO0D2_GPIO            = 0,
>>> -	GPIO0D2_LCDC_D22,
>>> -	GPIO0D2_TRACE_D12,
>>> -	GPIO0D2_UART4_SOUT,
>>> -
>>> -	GPIO0D1_SHIFT           = 2,
>>> -	GPIO0D1_MASK            = 3 << GPIO0D1_SHIFT,
>>> -	GPIO0D1_GPIO            = 0,
>>> -	GPIO0D1_LCDC_D21,
>>> -	GPIO0D1_TRACE_D11,
>>> -	GPIO0D1_UART4_RTSN,
>>> -
>>> -	GPIO0D0_SHIFT           = 0,
>>> -	GPIO0D0_MASK            = 3 << GPIO0D0_SHIFT,
>>> -	GPIO0D0_GPIO            = 0,
>>> -	GPIO0D0_LCDC_D20,
>>> -	GPIO0D0_TRACE_D10,
>>> -	GPIO0D0_UART4_CTSN,
>>> +	RK3368_GPIO0D7_MASK            = GENMASK(15, 14),
>>> +	RK3368_GPIO0D7_GPIO            = 0,
>>> +	RK3368_GPIO0D7_LCDC_DCLK       = (1 << 14),
>>> +	RK3368_GPIO0D7_TRACE_CTL       = (2 << 14),
>>> +	RK3368_GPIO0D7_PMU_DEBUG5      = (3 << 14),
>>> +
>>> +	RK3368_GPIO0D6_MASK            = GENMASK(13, 12),
>>> +	RK3368_GPIO0D6_GPIO            = 0,
>>> +	RK3368_GPIO0D6_LCDC_DEN        = (1 << 12),
>>> +	RK3368_GPIO0D6_TRACE_CLK       = (2 << 12),
>>> +	RK3368_GPIO0D6_PMU_DEBUG4      = (3 << 12),
>>> +
>>> +	RK3368_GPIO0D5_MASK            = GENMASK(11, 10),
>>> +	RK3368_GPIO0D5_GPIO            = 0,
>>> +	GPIO0D5_LCDC_VSYNC             = (1 << 10),
>>> +	RK3368_GPIO0D5_TRACE_D15       = (2 << 10),
>>> +	RK3368_GPIO0D5_PMU_DEBUG3      = (3 << 10),
>>> +
>>> +	RK3368_GPIO0D4_MASK            = GENMASK(9, 8),
>>> +	RK3368_GPIO0D4_GPIO            = 0,
>>> +	RK3368_GPIO0D4_LCDC_HSYNC      = (1 << 8),
>>> +	RK3368_GPIO0D4_TRACE_D14       = (2 << 8),
>>> +	RK3368_GPIO0D4_PMU_DEBUG2      = (3 << 8),
>>> +
>>> +	RK3368_GPIO0D3_MASK            = GENMASK(7, 6),
>>> +	RK3368_GPIO0D3_GPIO            = 0,
>>> +	RK3368_GPIO0D3_LCDC_D23        = (1 << 6),
>>> +	RK3368_GPIO0D3_TRACE_D13       = (2 << 6),
>>> +	RK3368_GPIO0D3_UART4_SIN       = (3 << 6),
>>> +
>>> +	RK3368_GPIO0D2_MASK            = GENMASK(5, 4),
>>> +	RK3368_GPIO0D2_GPIO            = 0,
>>> +	RK3368_GPIO0D2_LCDC_D22        = (1 << 4),
>>> +	RK3368_GPIO0D2_TRACE_D12       = (2 << 4),
>>> +	RK3368_GPIO0D2_UART4_SOUT      = (3 << 4),
>>> +
>>> +	RK3368_GPIO0D1_MASK            = GENMASK(3, 2),
>>> +	RK3368_GPIO0D1_GPIO            = 0,
>>> +	RK3368_GPIO0D1_LCDC_D21        = (1 << 2),
>>> +	RK3368_GPIO0D1_TRACE_D11       = (2 << 2),
>>> +	RK3368_GPIO0D1_UART4_RTSN      = (3 << 2),
>>> +
>>> +	RK3368_GPIO0D0_MASK            = GENMASK(1, 0),
>>> +	RK3368_GPIO0D0_GPIO            = 0,
>>> +	RK3368_GPIO0D0_LCDC_D20        = (1 << 0),
>>> +	RK3368_GPIO0D0_TRACE_D10       = (2 << 0),
>>> +	RK3368_GPIO0D0_UART4_CTSN      = (3 << 0),
>>>   };
>>>     /*GRF_GPIO2A_IOMUX*/
>>>   enum {
>>> -	GPIO2A7_SHIFT           = 14,
>>> -	GPIO2A7_MASK            = 3 << GPIO2A7_SHIFT,
>>> -	GPIO2A7_GPIO            = 0,
>>> -	GPIO2A7_SDMMC0_D2,
>>> -	GPIO2A7_JTAG_TCK,
>>> -
>>> -	GPIO2A6_SHIFT           = 12,
>>> -	GPIO2A6_MASK            = 3 << GPIO2A6_SHIFT,
>>> -	GPIO2A6_GPIO            = 0,
>>> -	GPIO2A6_SDMMC0_D1,
>>> -	GPIO2A6_UART2_SIN,
>>> -
>>> -	GPIO2A5_SHIFT           = 10,
>>> -	GPIO2A5_MASK            = 3 << GPIO2A5_SHIFT,
>>> -	GPIO2A5_GPIO            = 0,
>>> -	GPIO2A5_SDMMC0_D0,
>>> -	GPIO2A5_UART2_SOUT,
>>> -
>>> -	GPIO2A4_SHIFT           = 8,
>>> -	GPIO2A4_MASK            = 3 << GPIO2A4_SHIFT,
>>> -	GPIO2A4_GPIO            = 0,
>>> -	GPIO2A4_FLASH_DQS,
>>> -	GPIO2A4_EMMC_CLKO,
>>> -
>>> -	GPIO2A3_SHIFT           = 6,
>>> -	GPIO2A3_MASK            = 3 << GPIO2A3_SHIFT,
>>> -	GPIO2A3_GPIO            = 0,
>>> -	GPIO2A3_FLASH_CSN3,
>>> -	GPIO2A3_EMMC_RSTNO,
>>> -
>>> -	GPIO2A2_SHIFT           = 4,
>>> -	GPIO2A2_MASK            = 3 << GPIO2A2_SHIFT,
>>> -	GPIO2A2_GPIO           = 0,
>>> -	GPIO2A2_FLASH_CSN2,
>>> -
>>> -	GPIO2A1_SHIFT           = 2,
>>> -	GPIO2A1_MASK            = 3 << GPIO2A1_SHIFT,
>>> -	GPIO2A1_GPIO            = 0,
>>> -	GPIO2A1_FLASH_CSN1,
>>> -
>>> -	GPIO2A0_SHIFT           = 0,
>>> -	GPIO2A0_MASK            = 3 << GPIO2A0_SHIFT,
>>> -	GPIO2A0_GPIO            = 0,
>>> -	GPIO2A0_FLASH_CSN0,
>>> +	RK3368_GPIO2A7_MASK            = GENMASK(15, 14),
>>> +	RK3368_GPIO2A7_GPIO            = 0,
>>> +	RK3368_GPIO2A7_SDMMC0_D2       = (1 << 14),
>>> +	RK3368_GPIO2A7_JTAG_TCK        = (2 << 14),
>>> +
>>> +	RK3368_GPIO2A6_MASK            = GENMASK(13, 12),
>>> +	RK3368_GPIO2A6_GPIO            = 0,
>>> +	RK3368_GPIO2A6_SDMMC0_D1       = (1 << 12),
>>> +	RK3368_GPIO2A6_UART2_SIN       = (2 << 12),
>>> +
>>> +	RK3368_GPIO2A5_MASK            = GENMASK(11, 10),
>>> +	RK3368_GPIO2A5_GPIO            = 0,
>>> +	RK3368_GPIO2A5_SDMMC0_D0       = (1 << 10),
>>> +	RK3368_GPIO2A5_UART2_SOUT      = (2 << 10),
>>> +
>>> +	RK3368_GPIO2A4_MASK            = GENMASK(9, 8),
>>> +	RK3368_GPIO2A4_RK3368_GPIO     = 0,
>>> +	RK3368_GPIO2A4_FLASH_DQS       = (1 << 8),
>>> +	RK3368_GPIO2A4_EMMC_CLKOUT     = (2 << 8),
>>> +
>>> +	RK3368_GPIO2A3_MASK            = GENMASK(7, 6),
>>> +	RK3368_GPIO2A3_GPIO            = 0,
>>> +	RK3368_GPIO2A3_FLASH_CSN3      = (1 << 6),
>>> +	RK3368_GPIO2A3_EMMC_RSTNOUT    = (2 << 6),
>>> +
>>> +	RK3368_GPIO2A2_MASK            = GENMASK(5, 4),
>>> +	RK3368_GPIO2A2_GPIO            = 0,
>>> +	RK3368_GPIO2A2_FLASH_CSN2      = (1 << 4),
>>> +
>>> +	RK3368_GPIO2A1_MASK            = GENMASK(3, 2),
>>> +	RK3368_GPIO2A1_GPIO            = 0,
>>> +	RK3368_GPIO2A1_FLASH_CSN1      = (1 << 2),
>>> +
>>> +	RK3368_GPIO2A0_MASK            = GENMASK(1, 0),
>>> +	RK3368_GPIO2A0_GPIO            = 0,
>>> +	RK3368_GPIO2A0_FLASH_CSN0      = (1 << 0),
>>>   };
>>>   -/*GRF_GPIO2D_IOMUX*/
>>> +/*GRF_RK3368_GPIO2D_IOMUX*/
>>>   enum {
>>> -	GPIO2D7_SHIFT           = 14,
>>> -	GPIO2D7_MASK            = 3 << GPIO2D7_SHIFT,
>>> -	GPIO2D7_GPIO            = 0,
>>> -	GPIO2D7_SDIO0_D3,
>>> -
>>> -	GPIO2D6_SHIFT           = 12,
>>> -	GPIO2D6_MASK            = 3 << GPIO2D6_SHIFT,
>>> -	GPIO2D6_GPIO            = 0,
>>> -	GPIO2D6_SDIO0_D2,
>>> -
>>> -	GPIO2D5_SHIFT           = 10,
>>> -	GPIO2D5_MASK            = 3 << GPIO2D5_SHIFT,
>>> -	GPIO2D5_GPIO            = 0,
>>> -	GPIO2D5_SDIO0_D1,
>>> -
>>> -	GPIO2D4_SHIFT           = 8,
>>> -	GPIO2D4_MASK            = 3 << GPIO2D4_SHIFT,
>>> -	GPIO2D4_GPIO            = 0,
>>> -	GPIO2D4_SDIO0_D0,
>>> -
>>> -	GPIO2D3_SHIFT           = 6,
>>> -	GPIO2D3_MASK            = 3 << GPIO2D3_SHIFT,
>>> -	GPIO2D3_GPIO            = 0,
>>> -	GPIO2D3_UART0_RTS0,
>>> -
>>> -	GPIO2D2_SHIFT           = 4,
>>> -	GPIO2D2_MASK            = 3 << GPIO2D2_SHIFT,
>>> -	GPIO2D2_GPIO            = 0,
>>> -	GPIO2D2_UART0_CTS0,
>>> -
>>> -	GPIO2D1_SHIFT           = 2,
>>> -	GPIO2D1_MASK            = 3 << GPIO2D1_SHIFT,
>>> -	GPIO2D1_GPIO            = 0,
>>> -	GPIO2D1_UART0_SOUT,
>>> -
>>> -	GPIO2D0_SHIFT           = 0,
>>> -	GPIO2D0_MASK            = 3 << GPIO2D0_SHIFT,
>>> -	GPIO2D0_GPIO            = 0,
>>> -	GPIO2D0_UART0_SIN,
>>> +	RK3368_GPIO2D7_MASK            = GENMASK(15, 14),
>>> +	RK3368_GPIO2D7_GPIO            = 0,
>>> +	RK3368_GPIO2D7_SDIO0_D3        = (1 << 14),
>>> +
>>> +	RK3368_GPIO2D6_MASK            = GENMASK(13, 12),
>>> +	RK3368_GPIO2D6_GPIO            = 0,
>>> +	RK3368_GPIO2D6_SDIO0_D2        = (1 << 12),
>>> +
>>> +	RK3368_GPIO2D5_MASK            = GENMASK(11, 10),
>>> +	RK3368_GPIO2D5_GPIO            = 0,
>>> +	RK3368_GPIO2D5_SDIO0_D1        = (1 << 10),
>>> +
>>> +	RK3368_GPIO2D4_MASK            = GENMASK(9, 8),
>>> +	RK3368_GPIO2D4_GPIO            = 0,
>>> +	RK3368_GPIO2D4_SDIO0_D0        = (1 << 8),
>>> +
>>> +	RK3368_GPIO2D3_MASK            = GENMASK(7, 6),
>>> +	RK3368_GPIO2D3_GPIO            = 0,
>>> +	RK3368_GPIO2D3_UART0_RTS0      = (1 << 6),
>>> +
>>> +	RK3368_GPIO2D2_MASK            = GENMASK(5, 4),
>>> +	RK3368_GPIO2D2_GPIO            = 0,
>>> +	RK3368_GPIO2D2_UART0_CTS0      = (1 << 4),
>>> +
>>> +	RK3368_GPIO2D1_MASK            = GENMASK(3, 2),
>>> +	RK3368_GPIO2D1_GPIO            = 0,
>>> +	RK3368_GPIO2D1_UART0_SOUT      = (1 << 2),
>>> +
>>> +	RK3368_GPIO2D0_MASK            = GENMASK(1, 0),
>>> +	RK3368_GPIO2D0_GPIO            = 0,
>>> +	RK3368_GPIO2D0_UART0_SIN       = (1 << 0),
>>> +};
>>> +
>>> +/* GRF_GPIO1C_IOMUX */
>>> +enum {
>>> +	RK3368_GPIO1C7_MASK            = GENMASK(15, 14),
>>> +	RK3368_GPIO1C7_GPIO            = 0,
>>> +	RK3368_GPIO1C7_EMMC_DATA5      = (2 << 14),
>>> +
>>> +	RK3368_GPIO1C6_MASK            = GENMASK(13, 12),
>>> +	RK3368_GPIO1C6_GPIO            = 0,
>>> +	RK3368_GPIO1C6_EMMC_DATA4      = (2 << 12),
>>> +
>>> +	RK3368_GPIO1C5_MASK            = GENMASK(11, 10),
>>> +	RK3368_GPIO1C5_GPIO            = 0,
>>> +	RK3368_GPIO1C5_EMMC_DATA3      = (2 << 10),
>>> +
>>> +	RK3368_GPIO1C4_MASK            = GENMASK(9, 8),
>>> +	RK3368_GPIO1C4_GPIO            = 0,
>>> +	RK3368_GPIO1C4_EMMC_DATA2      = (2 << 8),
>>> +
>>> +	RK3368_GPIO1C3_MASK            = GENMASK(7, 6),
>>> +	RK3368_GPIO1C3_GPIO            = 0,
>>> +	RK3368_GPIO1C3_EMMC_DATA1      = (2 << 6),
>>> +
>>> +	RK3368_GPIO1C2_MASK            = GENMASK(5, 4),
>>> +	RK3368_GPIO1C2_GPIO            = 0,
>>> +	RK3368_GPIO1C2_EMMC_DATA0      = (2 << 4),
>>> +};
>>> +
>>> +/* GRF_GPIO1D_IOMUX*/
>>> +enum {
>>> +	RK3368_GPIO1D3_MASK            = GENMASK(7, 6),
>>> +	RK3368_GPIO1D3_GPIO            = 0,
>>> +	RK3368_GPIO1D3_EMMC_PWREN      = (2 << 6),
>>> +
>>> +	RK3368_GPIO1D2_MASK            = GENMASK(5, 4),
>>> +	RK3368_GPIO1D2_GPIO            = 0,
>>> +	RK3368_GPIO1D2_EMMC_CMD        = (2 << 4),
>>> +
>>> +	RK3368_GPIO1D1_MASK            = GENMASK(3, 2),
>>> +	RK3368_GPIO1D1_GPIO            = 0,
>>> +	RK3368_GPIO1D1_EMMC_DATA7      = (2 << 2),
>>> +
>>> +	RK3368_GPIO1D0_MASK            = GENMASK(1, 0),
>>> +	RK3368_GPIO1D0_GPIO            = 0,
>>> +	RK3368_GPIO1D0_EMMC_DATA6      = (2 << 0),
>>> +};
>>> +
>>> +
>>> +/*GRF_GPIO3B_IOMUX*/
>>> +enum {
>>> +	RK3368_GPIO3B7_MASK            = GENMASK(15, 14),
>>> +	RK3368_GPIO3B7_GPIO            = 0,
>>> +	RK3368_GPIO3B7_MAC_RXD0        = (1 << 14),
>>> +
>>> +	RK3368_GPIO3B6_MASK            = GENMASK(13, 12),
>>> +	RK3368_GPIO3B6_GPIO            = 0,
>>> +	RK3368_GPIO3B6_MAC_TXD3        = (1 << 12),
>>> +
>>> +	RK3368_GPIO3B5_MASK            = GENMASK(11, 10),
>>> +	RK3368_GPIO3B5_GPIO            = 0,
>>> +	RK3368_GPIO3B5_MAC_TXEN        = (1 << 10),
>>> +
>>> +	RK3368_GPIO3B4_MASK            = GENMASK(9, 8),
>>> +	RK3368_GPIO3B4_GPIO            = 0,
>>> +	RK3368_GPIO3B4_MAC_COL         = (1 << 8),
>>> +
>>> +	RK3368_GPIO3B3_MASK            = GENMASK(7, 6),
>>> +	RK3368_GPIO3B3_GPIO            = 0,
>>> +	RK3368_GPIO3B3_MAC_CRS         = (1 << 6),
>>> +
>>> +	RK3368_GPIO3B2_MASK            = GENMASK(5, 4),
>>> +	RK3368_GPIO3B2_GPIO            = 0,
>>> +	RK3368_GPIO3B2_MAC_TXD2        = (1 << 4),
>>> +
>>> +	RK3368_GPIO3B1_MASK            = GENMASK(3, 2),
>>> +	RK3368_GPIO3B1_GPIO            = 0,
>>> +	RK3368_GPIO3B1_MAC_TXD1        = (1 << 2),
>>> +
>>> +	RK3368_GPIO3B0_MASK            = GENMASK(1, 0),
>>> +	RK3368_GPIO3B0_GPIO            = 0,
>>> +	RK3368_GPIO3B0_MAC_TXD0        = (1 << 0),
>>> +	RK3368_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_GPIO            = 0,
>>> -	GPIO3C6_MAC_CLK,
>>> -	GPIO3C6_ISP_SHUTTERTRIG,
>>> -
>>> -	GPIO3C5_SHIFT           = 10,
>>> -	GPIO3C5_MASK            = 3 << GPIO3C5_SHIFT,
>>> -	GPIO3C5_GPIO            = 0,
>>> -	GPIO3C5_MAC_RXER,
>>> -	GPIO3C5_ISP_PRELIGHTTRIG,
>>> -
>>> -	GPIO3C4_SHIFT           = 8,
>>> -	GPIO3C4_MASK            = 3 << GPIO3C4_SHIFT,
>>> -	GPIO3C4_GPIO            = 0,
>>> -	GPIO3C4_MAC_RXDV,
>>> -	GPIO3C4_ISP_FLASHTRIGOUT,
>>> -
>>> -	GPIO3C3_SHIFT           = 6,
>>> -	GPIO3C3_MASK            = 3 << GPIO3C3_SHIFT,
>>> -	GPIO3C3_GPIO            = 0,
>>> -	GPIO3C3_MAC_RXDV,
>>> -	GPIO3C3_EMMC_RSTNO,
>>> -
>>> -	GPIO3C2_SHIFT           = 4,
>>> -	GPIO3C2_MASK            = 3 << GPIO3C2_SHIFT,
>>> -	GPIO3C2_MAC_MDC            = 0,
>>> -	GPIO3C2_ISP_SHUTTEREN,
>>> -
>>> -	GPIO3C1_SHIFT           = 2,
>>> -	GPIO3C1_MASK            = 3 << GPIO3C1_SHIFT,
>>> -	GPIO3C1_GPIO            = 0,
>>> -	GPIO3C1_MAC_RXD2,
>>> -	GPIO3C1_UART3_RTSN,
>>> -
>>> -	GPIO3C0_SHIFT           = 0,
>>> -	GPIO3C0_MASK            = 3 << GPIO3C0_SHIFT,
>>> -	GPIO3C0_GPIO            = 0,
>>> -	GPIO3C0_MAC_RXD1,
>>> -	GPIO3C0_UART3_CTSN,
>>> -	GPIO3C0_GPS_RFCLK,
>>> +	RK3368_GPIO3C6_MASK            = GENMASK(13, 12),
>>> +	RK3368_GPIO3C6_GPIO            = 0,
>>> +	RK3368_GPIO3C6_MAC_CLK         = (1 << 12),
>>> +
>>> +	RK3368_GPIO3C5_MASK            = GENMASK(11, 10),
>>> +	RK3368_GPIO3C5_GPIO            = 0,
>>> +	RK3368_GPIO3C5_MAC_RXEN        = (1 << 10),
>>> +
>>> +	RK3368_GPIO3C4_MASK            = GENMASK(9, 8),
>>> +	RK3368_GPIO3C4_GPIO            = 0,
>>> +	RK3368_GPIO3C4_MAC_RXDV        = (1 << 8),
>>> +
>>> +	RK3368_GPIO3C3_MASK            = GENMASK(7, 6),
>>> +	RK3368_GPIO3C3_GPIO            = 0,
>>> +	RK3368_GPIO3C3_MAC_MDC         = (1 << 6),
>>> +
>>> +	RK3368_GPIO3C2_MASK            = GENMASK(5, 4),
>>> +	RK3368_GPIO3C2_GPIO            = 0,
>>> +	RK3368_GPIO3C2_MAC_RXD3        = (1 << 4),
>>> +
>>> +	RK3368_GPIO3C1_MASK            = GENMASK(3, 2),
>>> +	RK3368_GPIO3C1_GPIO            = 0,
>>> +	RK3368_GPIO3C1_MAC_RXD2        = (1 << 2),
>>> +
>>> +	RK3368_GPIO3C0_MASK            = GENMASK(1, 0),
>>> +	RK3368_GPIO3C0_GPIO            = 0,
>>> +	RK3368_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_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,
>>> -	GPIO3D1_GPIO            = 0,
>>> -	GPIO3D1_MAC_RXCLKIN,
>>> -	GPIO3D1_I2C4_SCL,
>>> -
>>> -	GPIO3D0_SHIFT           = 0,
>>> -	GPIO3D0_MASK            = 3 << GPIO3D0_SHIFT,
>>> -	GPIO3D0_GPIO            = 0,
>>> -	GPIO3D0_MAC_MDIO,
>>> -	GPIO3D0_I2C4_SDA,
>>> +	RK3368_GPIO3D4_MASK            = GENMASK(9, 8),
>>> +	RK3368_GPIO3D4_GPIO            = 0,
>>> +	RK3368_GPIO3D4_MAC_TXCLK       = (1 << 8),
>>> +
>>> +	RK3368_GPIO3D1_MASK            = GENMASK(3, 2),
>>> +	RK3368_GPIO3D1_GPIO            = 0,
>>> +	RK3368_GPIO3D1_MAC_RXCLK       = (1 << 2),
>>> +
>>> +	RK3368_GPIO3D0_MASK            = GENMASK(1, 0),
>>> +	RK3368_GPIO3D0_GPIO            = 0,
>>> +	RK3368_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*/
>>> @@ -440,4 +445,34 @@ enum {
>>>   	MCU_CODE_BASE_BIT31_BIT28_SHIFT		= 0,
>>>   	MCU_CODE_BASE_BIT31_BIT28_MASK		= GENMASK(3, 0),
>>>   };
>>> +
>>> +/*GRF_SOC_CON15*/
>>> +enum {
>>> +	RK3368_GMAC_PHY_INTF_SEL_RGMII = 1 << 9,
>>> +	RK3368_GMAC_PHY_INTF_SEL_MASK = GENMASK(11, 9),
>>> +	RK3368_RMII_MODE_MASK  = BIT(6),
>>> +	RK3368_RMII_MODE       = BIT(6),
>>> +	RK3368_GMAC_CLK_SEL_2_5M = 2 << 4,
>>> +	RK3368_GMAC_CLK_SEL_25M = 3 << 4,
>>> +	RK3368_GMAC_CLK_SEL_125M = 0 << 4,
>>> +	RK3368_GMAC_CLK_SEL_MASK = GENMASK(5, 4),
>>> +};
>>> +
>>> +/* GRF_SOC_CON16 */
>>> +enum {
>>> +	RK3368_RXCLK_DLY_ENA_GMAC_MASK = BIT(15),
>>> +	RK3368_RXCLK_DLY_ENA_GMAC_DISABLE = 0,
>>> +	RK3368_RXCLK_DLY_ENA_GMAC_ENABLE = BIT(15),
>>> +
>>> +	RK3368_TXCLK_DLY_ENA_GMAC_MASK = BIT(7),
>>> +	RK3368_TXCLK_DLY_ENA_GMAC_DISABLE = 0,
>>> +	RK3368_TXCLK_DLY_ENA_GMAC_ENABLE = BIT(7),
>>> +
>>> +	RK3368_CLK_RX_DL_CFG_GMAC_SHIFT = 8,
>>> +	RK3368_CLK_RX_DL_CFG_GMAC_MASK = GENMASK(14, 8),
>>> +
>>> +	RK3368_CLK_TX_DL_CFG_GMAC_SHIFT = 0,
>>> +	RK3368_CLK_TX_DL_CFG_GMAC_MASK = GENMASK(6, 0),
>>> +};
>>> +
>>>   #endif
>>> diff --git a/drivers/pinctrl/rockchip/pinctrl_rk3368.c b/drivers/pinctrl/rockchip/pinctrl_rk3368.c
>>> index bdf0758..b77c5ab 100644
>>> --- a/drivers/pinctrl/rockchip/pinctrl_rk3368.c
>>> +++ b/drivers/pinctrl/rockchip/pinctrl_rk3368.c
>>> @@ -30,9 +30,9 @@ static void pinctrl_rk3368_uart_config(struct rk3368_pinctrl_priv *priv,
>>>   	switch (uart_id) {
>>>   	case PERIPH_ID_UART2:
>>>   		rk_clrsetreg(&grf->gpio2a_iomux,
>>> -			     GPIO2A6_MASK | GPIO2A5_MASK,
>>> -			     GPIO2A6_UART2_SIN << GPIO2A6_SHIFT |
>>> -			     GPIO2A5_UART2_SOUT << GPIO2A5_SHIFT);
>>> +			     RK3368_GPIO2A6_MASK | RK3368_GPIO2A5_MASK,
>>> +			     RK3368_GPIO2A6_UART2_SIN |
>>> +			     RK3368_GPIO2A5_UART2_SOUT);
>>>   		break;
>>>   	case PERIPH_ID_UART0:
>>>   		break;
>>> @@ -42,12 +42,11 @@ static void pinctrl_rk3368_uart_config(struct rk3368_pinctrl_priv *priv,
>>>   		break;
>>>   	case PERIPH_ID_UART4:
>>>   		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);
>>> +			     RK3368_GPIO0D0_MASK | RK3368_GPIO0D1_MASK |
>>> +			     RK3368_GPIO0D2_MASK | RK3368_GPIO0D3_MASK,
>>> +			     RK3368_GPIO0D0_GPIO | RK3368_GPIO0D1_GPIO |
>>> +			     RK3368_GPIO0D2_UART4_SOUT |
>>> +			     RK3368_GPIO0D3_UART4_SIN);
>>>   		break;
>>>   	default:
>>>   		debug("uart id = %d iomux error!\n", uart_id);
>>
>
>
>

> On 24 Jul 2017, at 10:34, Andy Yan <andy.yan@rock-chips.com> wrote:
> 
> Hi Philipp:
> 
> 
> On 2017年07月21日 16:34, Dr. Philipp Tomsich wrote:
>>> On 21 Jul 2017, at 05:50, Andy Yan <andy.yan@rock-chips.com> wrote:
>>> 
>>> Hi Philipp:
>>> 
>>> 
>>> On 2017年07月19日 04:36, Philipp Tomsich 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 rename, we do this now
>>>> before having more code use this.
>>>> 
>>>> As some of the downstream drivers (e.g. the Designware GMAC wrapper)
>>>> may need to include the grf-header of multiple devices, we rename the
>>>> various defines for the RK3368 by prefixing them with the device name.
>>>> This avoids future trouble during driver integration.
>>>    Is that really necessary to add such a prefix for all the register definition, just to fit  the GMAC dirver?
>>> Maybe the gmac driver can define the platform specific macro in its own code like dwmac-rk in the kernel. Then we can keep the register header file a little tidy.
>> The problem is not the GMAC driver (although it was the motivation for doing
>> this change), but rather how easy it is to pick up the wrong grf-file and refer
>> to the wrong definitions/values… the conflicts are not through the GMAC-specifc
>> defines but rather through things like the IOMUX masks or (before I moved
>> this to the DDR-driver) things like the defines to enable DRAM in GRF.
>> 
>> Until we find a way to consistently structure things in such a way that
>> 	(a) common constructs can be shared (e.g. the masks in the IOMUX)
>> 	(b) specific definitions (e.g. the pin-out for a specific device) is not visible outside of its driver
>> I prefer the names to explicitly refer to each device.
>>   That said, the longer-term plan (especially with a larger number of devices
>> being supported) needs to include a consistent rework of how (and where)
>> we manage all these definitions from the grf-header files.
> It's true that many devices use GRF, we also face this condition in the kernel land.And all the drivers in kernel handle this device specific definition in the driver itself(by of_device_id->data or MACRO). And there is also no need to use the grf constructs, the common syscon api will provide  you the grf base address. I think this keeps things simple, we don't need to rely on too much of the grf or cru header, maybe some day we can remove the header file, just as clean as the kernel.

The problem is not the base-address of GRF, but rather the need to pull in the
register structure (so we can refer to individual registers by name instead of by
hard-coded offset): as we pull this in today, the definitions for the various IOMUX
entries will also be pulled in, which will cause duplicate definitions.

> On the other hand, add such a prefix of rk3368 breaks the consistency with other rockchip devices.

I do agree that we need to clean this up across all the boards and drivers
supported today.  As of today, I can not resolve this on a RK3368-only basis,
as I’d need to clean up the RK3288 at the same time (and don’t want to do this
in a RK3368-focused series that already has way too much going on).

Let’s try to clean this up for the next (i.e. the one after this) merge window.
Once we have bit-definitions out of the grf-header, I’ll remove the RK3368_
prefixes...

> 
>> 
>>>> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
>>>> ---
>>>> 
>>>>  arch/arm/include/asm/arch-rockchip/grf_rk3368.h | 617 +++++++++++++-----------
>>>>  drivers/pinctrl/rockchip/pinctrl_rk3368.c       |  17 +-
>>>>  2 files changed, 334 insertions(+), 300 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..a97dc1e 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,
>>>> -	GPIO0C6_GPIO            = 0,
>>>> -	GPIO0C6_LCDC_D18,
>>>> -	GPIO0C6_TRACE_D8,
>>>> -	GPIO0C6_UART1_CTSN,
>>>> -
>>>> -	GPIO0C5_SHIFT           = 10,
>>>> -	GPIO0C5_MASK            = 3 << GPIO0C5_SHIFT,
>>>> -	GPIO0C5_GPIO            = 0,
>>>> -	GPIO0C5_LCDC_D17,
>>>> -	GPIO0C5_TRACE_D7,
>>>> -	GPIO0C5_UART1_SOUT,
>>>> -
>>>> -	GPIO0C4_SHIFT           = 8,
>>>> -	GPIO0C4_MASK            = 3 << GPIO0C4_SHIFT,
>>>> -	GPIO0C4_GPIO            = 0,
>>>> -	GPIO0C4_LCDC_D16,
>>>> -	GPIO0C4_TRACE_D6,
>>>> -	GPIO0C4_UART1_SIN,
>>>> -
>>>> -	GPIO0C3_SHIFT           = 6,
>>>> -	GPIO0C3_MASK            = 3 << GPIO0C3_SHIFT,
>>>> -	GPIO0C3_GPIO            = 0,
>>>> -	GPIO0C3_LCDC_D15,
>>>> -	GPIO0C3_TRACE_D5,
>>>> -	GPIO0C3_MCU_JTAG_TDO,
>>>> -
>>>> -	GPIO0C2_SHIFT           = 4,
>>>> -	GPIO0C2_MASK            = 3 << GPIO0C2_SHIFT,
>>>> -	GPIO0C2_GPIO            = 0,
>>>> -	GPIO0C2_LCDC_D14,
>>>> -	GPIO0C2_TRACE_D4,
>>>> -	GPIO0C2_MCU_JTAG_TDI,
>>>> -
>>>> -	GPIO0C1_SHIFT           = 2,
>>>> -	GPIO0C1_MASK            = 3 << GPIO0C1_SHIFT,
>>>> -	GPIO0C1_GPIO            = 0,
>>>> -	GPIO0C1_LCDC_D13,
>>>> -	GPIO0C1_TRACE_D3,
>>>> -	GPIO0C1_MCU_JTAG_TRTSN,
>>>> -
>>>> -	GPIO0C0_SHIFT           = 0,
>>>> -	GPIO0C0_MASK            = 3 << GPIO0C0_SHIFT,
>>>> -	GPIO0C0_GPIO            = 0,
>>>> -	GPIO0C0_LCDC_D12,
>>>> -	GPIO0C0_TRACE_D2,
>>>> -	GPIO0C0_MCU_JTAG_TDO,
>>>> +	RK3368_GPIO0C7_MASK	       = GENMASK(15, 14),
>>>> +	RK3368_GPIO0C7_GPIO	       = 0,
>>>> +	RK3368_GPIO0C7_LCDC_D19        = (1 << 14),
>>>> +	RK3368_GPIO0C7_TRACE_D9        = (2 << 14),
>>>> +	RK3368_GPIO0C7_UART1_RTSN      = (3 << 14),
>>>> +
>>>> +	RK3368_GPIO0C6_MASK            = GENMASK(13, 12),
>>>> +	RK3368_GPIO0C6_GPIO            = 0,
>>>> +	RK3368_GPIO0C6_LCDC_D18        = (1 << 12),
>>>> +	RK3368_GPIO0C6_TRACE_D8        = (2 << 12),
>>>> +	RK3368_GPIO0C6_UART1_CTSN      = (3 << 12),
>>>> +
>>>> +	RK3368_GPIO0C5_MASK            = GENMASK(11, 10),
>>>> +	RK3368_GPIO0C5_GPIO            = 0,
>>>> +	RK3368_GPIO0C5_LCDC_D17        = (1 << 10),
>>>> +	RK3368_GPIO0C5_TRACE_D7        = (2 << 10),
>>>> +	RK3368_GPIO0C5_UART1_SOUT      = (3 << 10),
>>>> +
>>>> +	RK3368_GPIO0C4_MASK            = GENMASK(9, 8),
>>>> +	RK3368_GPIO0C4_GPIO            = 0,
>>>> +	RK3368_GPIO0C4_LCDC_D16        = (1 << 8),
>>>> +	RK3368_GPIO0C4_TRACE_D6        = (2 << 8),
>>>> +	RK3368_GPIO0C4_UART1_SIN       = (3 << 8),
>>>> +
>>>> +	RK3368_GPIO0C3_MASK            = GENMASK(7, 6),
>>>> +	RK3368_GPIO0C3_GPIO            = 0,
>>>> +	RK3368_GPIO0C3_LCDC_D15        = (1 << 6),
>>>> +	RK3368_GPIO0C3_TRACE_D5        = (2 << 6),
>>>> +	RK3368_GPIO0C3_MCU_JTAG_TDO    = (3 << 6),
>>>> +
>>>> +	RK3368_GPIO0C2_MASK            = GENMASK(5, 4),
>>>> +	RK3368_GPIO0C2_GPIO            = 0,
>>>> +	RK3368_GPIO0C2_LCDC_D14        = (1 << 4),
>>>> +	RK3368_GPIO0C2_TRACE_D4        = (2 << 4),
>>>> +	RK3368_GPIO0C2_MCU_JTAG_TDI    = (3 << 4),
>>>> +
>>>> +	RK3368_GPIO0C1_MASK            = GENMASK(3, 2),
>>>> +	RK3368_GPIO0C1_GPIO            = 0,
>>>> +	RK3368_GPIO0C1_LCDC_D13        = (1 << 2),
>>>> +	RK3368_GPIO0C1_TRACE_D3        = (2 << 2),
>>>> +	RK3368_GPIO0C1_MCU_JTAG_TRTSN  = (3 << 2),
>>>> +
>>>> +	RK3368_GPIO0C0_MASK            = GENMASK(1, 0),
>>>> +	RK3368_GPIO0C0_GPIO            = 0,
>>>> +	RK3368_GPIO0C0_LCDC_D12        = (1 << 0),
>>>> +	RK3368_GPIO0C0_TRACE_D2        = (2 << 0),
>>>> +	RK3368_GPIO0C0_MCU_JTAG_TDO    = (3 << 0),
>>>>  };
>>>>    /*GRF_GPIO0D_IOMUX*/
>>>>  enum {
>>>> -	GPIO0D7_SHIFT           = 14,
>>>> -	GPIO0D7_MASK            = 3 << GPIO0D7_SHIFT,
>>>> -	GPIO0D7_GPIO            = 0,
>>>> -	GPIO0D7_LCDC_DCLK,
>>>> -	GPIO0D7_TRACE_CTL,
>>>> -	GPIO0D7_PMU_DEBUG5,
>>>> -
>>>> -	GPIO0D6_SHIFT           = 12,
>>>> -	GPIO0D6_MASK            = 3 << GPIO0D6_SHIFT,
>>>> -	GPIO0D6_GPIO            = 0,
>>>> -	GPIO0D6_LCDC_DEN,
>>>> -	GPIO0D6_TRACE_CLK,
>>>> -	GPIO0D6_PMU_DEBUG4,
>>>> -
>>>> -	GPIO0D5_SHIFT           = 10,
>>>> -	GPIO0D5_MASK            = 3 << GPIO0D5_SHIFT,
>>>> -	GPIO0D5_GPIO            = 0,
>>>> -	GPIO0D5_LCDC_VSYNC,
>>>> -	GPIO0D5_TRACE_D15,
>>>> -	GPIO0D5_PMU_DEBUG3,
>>>> -
>>>> -	GPIO0D4_SHIFT           = 8,
>>>> -	GPIO0D4_MASK            = 3 << GPIO0D4_SHIFT,
>>>> -	GPIO0D4_GPIO            = 0,
>>>> -	GPIO0D4_LCDC_HSYNC,
>>>> -	GPIO0D4_TRACE_D14,
>>>> -	GPIO0D4_PMU_DEBUG2,
>>>> -
>>>> -	GPIO0D3_SHIFT           = 6,
>>>> -	GPIO0D3_MASK            = 3 << GPIO0D3_SHIFT,
>>>> -	GPIO0D3_GPIO            = 0,
>>>> -	GPIO0D3_LCDC_D23,
>>>> -	GPIO0D3_TRACE_D13,
>>>> -	GPIO0D3_UART4_SIN,
>>>> -
>>>> -	GPIO0D2_SHIFT           = 4,
>>>> -	GPIO0D2_MASK            = 3 << GPIO0D2_SHIFT,
>>>> -	GPIO0D2_GPIO            = 0,
>>>> -	GPIO0D2_LCDC_D22,
>>>> -	GPIO0D2_TRACE_D12,
>>>> -	GPIO0D2_UART4_SOUT,
>>>> -
>>>> -	GPIO0D1_SHIFT           = 2,
>>>> -	GPIO0D1_MASK            = 3 << GPIO0D1_SHIFT,
>>>> -	GPIO0D1_GPIO            = 0,
>>>> -	GPIO0D1_LCDC_D21,
>>>> -	GPIO0D1_TRACE_D11,
>>>> -	GPIO0D1_UART4_RTSN,
>>>> -
>>>> -	GPIO0D0_SHIFT           = 0,
>>>> -	GPIO0D0_MASK            = 3 << GPIO0D0_SHIFT,
>>>> -	GPIO0D0_GPIO            = 0,
>>>> -	GPIO0D0_LCDC_D20,
>>>> -	GPIO0D0_TRACE_D10,
>>>> -	GPIO0D0_UART4_CTSN,
>>>> +	RK3368_GPIO0D7_MASK            = GENMASK(15, 14),
>>>> +	RK3368_GPIO0D7_GPIO            = 0,
>>>> +	RK3368_GPIO0D7_LCDC_DCLK       = (1 << 14),
>>>> +	RK3368_GPIO0D7_TRACE_CTL       = (2 << 14),
>>>> +	RK3368_GPIO0D7_PMU_DEBUG5      = (3 << 14),
>>>> +
>>>> +	RK3368_GPIO0D6_MASK            = GENMASK(13, 12),
>>>> +	RK3368_GPIO0D6_GPIO            = 0,
>>>> +	RK3368_GPIO0D6_LCDC_DEN        = (1 << 12),
>>>> +	RK3368_GPIO0D6_TRACE_CLK       = (2 << 12),
>>>> +	RK3368_GPIO0D6_PMU_DEBUG4      = (3 << 12),
>>>> +
>>>> +	RK3368_GPIO0D5_MASK            = GENMASK(11, 10),
>>>> +	RK3368_GPIO0D5_GPIO            = 0,
>>>> +	GPIO0D5_LCDC_VSYNC             = (1 << 10),
>>>> +	RK3368_GPIO0D5_TRACE_D15       = (2 << 10),
>>>> +	RK3368_GPIO0D5_PMU_DEBUG3      = (3 << 10),
>>>> +
>>>> +	RK3368_GPIO0D4_MASK            = GENMASK(9, 8),
>>>> +	RK3368_GPIO0D4_GPIO            = 0,
>>>> +	RK3368_GPIO0D4_LCDC_HSYNC      = (1 << 8),
>>>> +	RK3368_GPIO0D4_TRACE_D14       = (2 << 8),
>>>> +	RK3368_GPIO0D4_PMU_DEBUG2      = (3 << 8),
>>>> +
>>>> +	RK3368_GPIO0D3_MASK            = GENMASK(7, 6),
>>>> +	RK3368_GPIO0D3_GPIO            = 0,
>>>> +	RK3368_GPIO0D3_LCDC_D23        = (1 << 6),
>>>> +	RK3368_GPIO0D3_TRACE_D13       = (2 << 6),
>>>> +	RK3368_GPIO0D3_UART4_SIN       = (3 << 6),
>>>> +
>>>> +	RK3368_GPIO0D2_MASK            = GENMASK(5, 4),
>>>> +	RK3368_GPIO0D2_GPIO            = 0,
>>>> +	RK3368_GPIO0D2_LCDC_D22        = (1 << 4),
>>>> +	RK3368_GPIO0D2_TRACE_D12       = (2 << 4),
>>>> +	RK3368_GPIO0D2_UART4_SOUT      = (3 << 4),
>>>> +
>>>> +	RK3368_GPIO0D1_MASK            = GENMASK(3, 2),
>>>> +	RK3368_GPIO0D1_GPIO            = 0,
>>>> +	RK3368_GPIO0D1_LCDC_D21        = (1 << 2),
>>>> +	RK3368_GPIO0D1_TRACE_D11       = (2 << 2),
>>>> +	RK3368_GPIO0D1_UART4_RTSN      = (3 << 2),
>>>> +
>>>> +	RK3368_GPIO0D0_MASK            = GENMASK(1, 0),
>>>> +	RK3368_GPIO0D0_GPIO            = 0,
>>>> +	RK3368_GPIO0D0_LCDC_D20        = (1 << 0),
>>>> +	RK3368_GPIO0D0_TRACE_D10       = (2 << 0),
>>>> +	RK3368_GPIO0D0_UART4_CTSN      = (3 << 0),
>>>>  };
>>>>    /*GRF_GPIO2A_IOMUX*/
>>>>  enum {
>>>> -	GPIO2A7_SHIFT           = 14,
>>>> -	GPIO2A7_MASK            = 3 << GPIO2A7_SHIFT,
>>>> -	GPIO2A7_GPIO            = 0,
>>>> -	GPIO2A7_SDMMC0_D2,
>>>> -	GPIO2A7_JTAG_TCK,
>>>> -
>>>> -	GPIO2A6_SHIFT           = 12,
>>>> -	GPIO2A6_MASK            = 3 << GPIO2A6_SHIFT,
>>>> -	GPIO2A6_GPIO            = 0,
>>>> -	GPIO2A6_SDMMC0_D1,
>>>> -	GPIO2A6_UART2_SIN,
>>>> -
>>>> -	GPIO2A5_SHIFT           = 10,
>>>> -	GPIO2A5_MASK            = 3 << GPIO2A5_SHIFT,
>>>> -	GPIO2A5_GPIO            = 0,
>>>> -	GPIO2A5_SDMMC0_D0,
>>>> -	GPIO2A5_UART2_SOUT,
>>>> -
>>>> -	GPIO2A4_SHIFT           = 8,
>>>> -	GPIO2A4_MASK            = 3 << GPIO2A4_SHIFT,
>>>> -	GPIO2A4_GPIO            = 0,
>>>> -	GPIO2A4_FLASH_DQS,
>>>> -	GPIO2A4_EMMC_CLKO,
>>>> -
>>>> -	GPIO2A3_SHIFT           = 6,
>>>> -	GPIO2A3_MASK            = 3 << GPIO2A3_SHIFT,
>>>> -	GPIO2A3_GPIO            = 0,
>>>> -	GPIO2A3_FLASH_CSN3,
>>>> -	GPIO2A3_EMMC_RSTNO,
>>>> -
>>>> -	GPIO2A2_SHIFT           = 4,
>>>> -	GPIO2A2_MASK            = 3 << GPIO2A2_SHIFT,
>>>> -	GPIO2A2_GPIO           = 0,
>>>> -	GPIO2A2_FLASH_CSN2,
>>>> -
>>>> -	GPIO2A1_SHIFT           = 2,
>>>> -	GPIO2A1_MASK            = 3 << GPIO2A1_SHIFT,
>>>> -	GPIO2A1_GPIO            = 0,
>>>> -	GPIO2A1_FLASH_CSN1,
>>>> -
>>>> -	GPIO2A0_SHIFT           = 0,
>>>> -	GPIO2A0_MASK            = 3 << GPIO2A0_SHIFT,
>>>> -	GPIO2A0_GPIO            = 0,
>>>> -	GPIO2A0_FLASH_CSN0,
>>>> +	RK3368_GPIO2A7_MASK            = GENMASK(15, 14),
>>>> +	RK3368_GPIO2A7_GPIO            = 0,
>>>> +	RK3368_GPIO2A7_SDMMC0_D2       = (1 << 14),
>>>> +	RK3368_GPIO2A7_JTAG_TCK        = (2 << 14),
>>>> +
>>>> +	RK3368_GPIO2A6_MASK            = GENMASK(13, 12),
>>>> +	RK3368_GPIO2A6_GPIO            = 0,
>>>> +	RK3368_GPIO2A6_SDMMC0_D1       = (1 << 12),
>>>> +	RK3368_GPIO2A6_UART2_SIN       = (2 << 12),
>>>> +
>>>> +	RK3368_GPIO2A5_MASK            = GENMASK(11, 10),
>>>> +	RK3368_GPIO2A5_GPIO            = 0,
>>>> +	RK3368_GPIO2A5_SDMMC0_D0       = (1 << 10),
>>>> +	RK3368_GPIO2A5_UART2_SOUT      = (2 << 10),
>>>> +
>>>> +	RK3368_GPIO2A4_MASK            = GENMASK(9, 8),
>>>> +	RK3368_GPIO2A4_RK3368_GPIO     = 0,
>>>> +	RK3368_GPIO2A4_FLASH_DQS       = (1 << 8),
>>>> +	RK3368_GPIO2A4_EMMC_CLKOUT     = (2 << 8),
>>>> +
>>>> +	RK3368_GPIO2A3_MASK            = GENMASK(7, 6),
>>>> +	RK3368_GPIO2A3_GPIO            = 0,
>>>> +	RK3368_GPIO2A3_FLASH_CSN3      = (1 << 6),
>>>> +	RK3368_GPIO2A3_EMMC_RSTNOUT    = (2 << 6),
>>>> +
>>>> +	RK3368_GPIO2A2_MASK            = GENMASK(5, 4),
>>>> +	RK3368_GPIO2A2_GPIO            = 0,
>>>> +	RK3368_GPIO2A2_FLASH_CSN2      = (1 << 4),
>>>> +
>>>> +	RK3368_GPIO2A1_MASK            = GENMASK(3, 2),
>>>> +	RK3368_GPIO2A1_GPIO            = 0,
>>>> +	RK3368_GPIO2A1_FLASH_CSN1      = (1 << 2),
>>>> +
>>>> +	RK3368_GPIO2A0_MASK            = GENMASK(1, 0),
>>>> +	RK3368_GPIO2A0_GPIO            = 0,
>>>> +	RK3368_GPIO2A0_FLASH_CSN0      = (1 << 0),
>>>>  };
>>>>  -/*GRF_GPIO2D_IOMUX*/
>>>> +/*GRF_RK3368_GPIO2D_IOMUX*/
>>>>  enum {
>>>> -	GPIO2D7_SHIFT           = 14,
>>>> -	GPIO2D7_MASK            = 3 << GPIO2D7_SHIFT,
>>>> -	GPIO2D7_GPIO            = 0,
>>>> -	GPIO2D7_SDIO0_D3,
>>>> -
>>>> -	GPIO2D6_SHIFT           = 12,
>>>> -	GPIO2D6_MASK            = 3 << GPIO2D6_SHIFT,
>>>> -	GPIO2D6_GPIO            = 0,
>>>> -	GPIO2D6_SDIO0_D2,
>>>> -
>>>> -	GPIO2D5_SHIFT           = 10,
>>>> -	GPIO2D5_MASK            = 3 << GPIO2D5_SHIFT,
>>>> -	GPIO2D5_GPIO            = 0,
>>>> -	GPIO2D5_SDIO0_D1,
>>>> -
>>>> -	GPIO2D4_SHIFT           = 8,
>>>> -	GPIO2D4_MASK            = 3 << GPIO2D4_SHIFT,
>>>> -	GPIO2D4_GPIO            = 0,
>>>> -	GPIO2D4_SDIO0_D0,
>>>> -
>>>> -	GPIO2D3_SHIFT           = 6,
>>>> -	GPIO2D3_MASK            = 3 << GPIO2D3_SHIFT,
>>>> -	GPIO2D3_GPIO            = 0,
>>>> -	GPIO2D3_UART0_RTS0,
>>>> -
>>>> -	GPIO2D2_SHIFT           = 4,
>>>> -	GPIO2D2_MASK            = 3 << GPIO2D2_SHIFT,
>>>> -	GPIO2D2_GPIO            = 0,
>>>> -	GPIO2D2_UART0_CTS0,
>>>> -
>>>> -	GPIO2D1_SHIFT           = 2,
>>>> -	GPIO2D1_MASK            = 3 << GPIO2D1_SHIFT,
>>>> -	GPIO2D1_GPIO            = 0,
>>>> -	GPIO2D1_UART0_SOUT,
>>>> -
>>>> -	GPIO2D0_SHIFT           = 0,
>>>> -	GPIO2D0_MASK            = 3 << GPIO2D0_SHIFT,
>>>> -	GPIO2D0_GPIO            = 0,
>>>> -	GPIO2D0_UART0_SIN,
>>>> +	RK3368_GPIO2D7_MASK            = GENMASK(15, 14),
>>>> +	RK3368_GPIO2D7_GPIO            = 0,
>>>> +	RK3368_GPIO2D7_SDIO0_D3        = (1 << 14),
>>>> +
>>>> +	RK3368_GPIO2D6_MASK            = GENMASK(13, 12),
>>>> +	RK3368_GPIO2D6_GPIO            = 0,
>>>> +	RK3368_GPIO2D6_SDIO0_D2        = (1 << 12),
>>>> +
>>>> +	RK3368_GPIO2D5_MASK            = GENMASK(11, 10),
>>>> +	RK3368_GPIO2D5_GPIO            = 0,
>>>> +	RK3368_GPIO2D5_SDIO0_D1        = (1 << 10),
>>>> +
>>>> +	RK3368_GPIO2D4_MASK            = GENMASK(9, 8),
>>>> +	RK3368_GPIO2D4_GPIO            = 0,
>>>> +	RK3368_GPIO2D4_SDIO0_D0        = (1 << 8),
>>>> +
>>>> +	RK3368_GPIO2D3_MASK            = GENMASK(7, 6),
>>>> +	RK3368_GPIO2D3_GPIO            = 0,
>>>> +	RK3368_GPIO2D3_UART0_RTS0      = (1 << 6),
>>>> +
>>>> +	RK3368_GPIO2D2_MASK            = GENMASK(5, 4),
>>>> +	RK3368_GPIO2D2_GPIO            = 0,
>>>> +	RK3368_GPIO2D2_UART0_CTS0      = (1 << 4),
>>>> +
>>>> +	RK3368_GPIO2D1_MASK            = GENMASK(3, 2),
>>>> +	RK3368_GPIO2D1_GPIO            = 0,
>>>> +	RK3368_GPIO2D1_UART0_SOUT      = (1 << 2),
>>>> +
>>>> +	RK3368_GPIO2D0_MASK            = GENMASK(1, 0),
>>>> +	RK3368_GPIO2D0_GPIO            = 0,
>>>> +	RK3368_GPIO2D0_UART0_SIN       = (1 << 0),
>>>> +};
>>>> +
>>>> +/* GRF_GPIO1C_IOMUX */
>>>> +enum {
>>>> +	RK3368_GPIO1C7_MASK            = GENMASK(15, 14),
>>>> +	RK3368_GPIO1C7_GPIO            = 0,
>>>> +	RK3368_GPIO1C7_EMMC_DATA5      = (2 << 14),
>>>> +
>>>> +	RK3368_GPIO1C6_MASK            = GENMASK(13, 12),
>>>> +	RK3368_GPIO1C6_GPIO            = 0,
>>>> +	RK3368_GPIO1C6_EMMC_DATA4      = (2 << 12),
>>>> +
>>>> +	RK3368_GPIO1C5_MASK            = GENMASK(11, 10),
>>>> +	RK3368_GPIO1C5_GPIO            = 0,
>>>> +	RK3368_GPIO1C5_EMMC_DATA3      = (2 << 10),
>>>> +
>>>> +	RK3368_GPIO1C4_MASK            = GENMASK(9, 8),
>>>> +	RK3368_GPIO1C4_GPIO            = 0,
>>>> +	RK3368_GPIO1C4_EMMC_DATA2      = (2 << 8),
>>>> +
>>>> +	RK3368_GPIO1C3_MASK            = GENMASK(7, 6),
>>>> +	RK3368_GPIO1C3_GPIO            = 0,
>>>> +	RK3368_GPIO1C3_EMMC_DATA1      = (2 << 6),
>>>> +
>>>> +	RK3368_GPIO1C2_MASK            = GENMASK(5, 4),
>>>> +	RK3368_GPIO1C2_GPIO            = 0,
>>>> +	RK3368_GPIO1C2_EMMC_DATA0      = (2 << 4),
>>>> +};
>>>> +
>>>> +/* GRF_GPIO1D_IOMUX*/
>>>> +enum {
>>>> +	RK3368_GPIO1D3_MASK            = GENMASK(7, 6),
>>>> +	RK3368_GPIO1D3_GPIO            = 0,
>>>> +	RK3368_GPIO1D3_EMMC_PWREN      = (2 << 6),
>>>> +
>>>> +	RK3368_GPIO1D2_MASK            = GENMASK(5, 4),
>>>> +	RK3368_GPIO1D2_GPIO            = 0,
>>>> +	RK3368_GPIO1D2_EMMC_CMD        = (2 << 4),
>>>> +
>>>> +	RK3368_GPIO1D1_MASK            = GENMASK(3, 2),
>>>> +	RK3368_GPIO1D1_GPIO            = 0,
>>>> +	RK3368_GPIO1D1_EMMC_DATA7      = (2 << 2),
>>>> +
>>>> +	RK3368_GPIO1D0_MASK            = GENMASK(1, 0),
>>>> +	RK3368_GPIO1D0_GPIO            = 0,
>>>> +	RK3368_GPIO1D0_EMMC_DATA6      = (2 << 0),
>>>> +};
>>>> +
>>>> +
>>>> +/*GRF_GPIO3B_IOMUX*/
>>>> +enum {
>>>> +	RK3368_GPIO3B7_MASK            = GENMASK(15, 14),
>>>> +	RK3368_GPIO3B7_GPIO            = 0,
>>>> +	RK3368_GPIO3B7_MAC_RXD0        = (1 << 14),
>>>> +
>>>> +	RK3368_GPIO3B6_MASK            = GENMASK(13, 12),
>>>> +	RK3368_GPIO3B6_GPIO            = 0,
>>>> +	RK3368_GPIO3B6_MAC_TXD3        = (1 << 12),
>>>> +
>>>> +	RK3368_GPIO3B5_MASK            = GENMASK(11, 10),
>>>> +	RK3368_GPIO3B5_GPIO            = 0,
>>>> +	RK3368_GPIO3B5_MAC_TXEN        = (1 << 10),
>>>> +
>>>> +	RK3368_GPIO3B4_MASK            = GENMASK(9, 8),
>>>> +	RK3368_GPIO3B4_GPIO            = 0,
>>>> +	RK3368_GPIO3B4_MAC_COL         = (1 << 8),
>>>> +
>>>> +	RK3368_GPIO3B3_MASK            = GENMASK(7, 6),
>>>> +	RK3368_GPIO3B3_GPIO            = 0,
>>>> +	RK3368_GPIO3B3_MAC_CRS         = (1 << 6),
>>>> +
>>>> +	RK3368_GPIO3B2_MASK            = GENMASK(5, 4),
>>>> +	RK3368_GPIO3B2_GPIO            = 0,
>>>> +	RK3368_GPIO3B2_MAC_TXD2        = (1 << 4),
>>>> +
>>>> +	RK3368_GPIO3B1_MASK            = GENMASK(3, 2),
>>>> +	RK3368_GPIO3B1_GPIO            = 0,
>>>> +	RK3368_GPIO3B1_MAC_TXD1        = (1 << 2),
>>>> +
>>>> +	RK3368_GPIO3B0_MASK            = GENMASK(1, 0),
>>>> +	RK3368_GPIO3B0_GPIO            = 0,
>>>> +	RK3368_GPIO3B0_MAC_TXD0        = (1 << 0),
>>>> +	RK3368_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_GPIO            = 0,
>>>> -	GPIO3C6_MAC_CLK,
>>>> -	GPIO3C6_ISP_SHUTTERTRIG,
>>>> -
>>>> -	GPIO3C5_SHIFT           = 10,
>>>> -	GPIO3C5_MASK            = 3 << GPIO3C5_SHIFT,
>>>> -	GPIO3C5_GPIO            = 0,
>>>> -	GPIO3C5_MAC_RXER,
>>>> -	GPIO3C5_ISP_PRELIGHTTRIG,
>>>> -
>>>> -	GPIO3C4_SHIFT           = 8,
>>>> -	GPIO3C4_MASK            = 3 << GPIO3C4_SHIFT,
>>>> -	GPIO3C4_GPIO            = 0,
>>>> -	GPIO3C4_MAC_RXDV,
>>>> -	GPIO3C4_ISP_FLASHTRIGOUT,
>>>> -
>>>> -	GPIO3C3_SHIFT           = 6,
>>>> -	GPIO3C3_MASK            = 3 << GPIO3C3_SHIFT,
>>>> -	GPIO3C3_GPIO            = 0,
>>>> -	GPIO3C3_MAC_RXDV,
>>>> -	GPIO3C3_EMMC_RSTNO,
>>>> -
>>>> -	GPIO3C2_SHIFT           = 4,
>>>> -	GPIO3C2_MASK            = 3 << GPIO3C2_SHIFT,
>>>> -	GPIO3C2_MAC_MDC            = 0,
>>>> -	GPIO3C2_ISP_SHUTTEREN,
>>>> -
>>>> -	GPIO3C1_SHIFT           = 2,
>>>> -	GPIO3C1_MASK            = 3 << GPIO3C1_SHIFT,
>>>> -	GPIO3C1_GPIO            = 0,
>>>> -	GPIO3C1_MAC_RXD2,
>>>> -	GPIO3C1_UART3_RTSN,
>>>> -
>>>> -	GPIO3C0_SHIFT           = 0,
>>>> -	GPIO3C0_MASK            = 3 << GPIO3C0_SHIFT,
>>>> -	GPIO3C0_GPIO            = 0,
>>>> -	GPIO3C0_MAC_RXD1,
>>>> -	GPIO3C0_UART3_CTSN,
>>>> -	GPIO3C0_GPS_RFCLK,
>>>> +	RK3368_GPIO3C6_MASK            = GENMASK(13, 12),
>>>> +	RK3368_GPIO3C6_GPIO            = 0,
>>>> +	RK3368_GPIO3C6_MAC_CLK         = (1 << 12),
>>>> +
>>>> +	RK3368_GPIO3C5_MASK            = GENMASK(11, 10),
>>>> +	RK3368_GPIO3C5_GPIO            = 0,
>>>> +	RK3368_GPIO3C5_MAC_RXEN        = (1 << 10),
>>>> +
>>>> +	RK3368_GPIO3C4_MASK            = GENMASK(9, 8),
>>>> +	RK3368_GPIO3C4_GPIO            = 0,
>>>> +	RK3368_GPIO3C4_MAC_RXDV        = (1 << 8),
>>>> +
>>>> +	RK3368_GPIO3C3_MASK            = GENMASK(7, 6),
>>>> +	RK3368_GPIO3C3_GPIO            = 0,
>>>> +	RK3368_GPIO3C3_MAC_MDC         = (1 << 6),
>>>> +
>>>> +	RK3368_GPIO3C2_MASK            = GENMASK(5, 4),
>>>> +	RK3368_GPIO3C2_GPIO            = 0,
>>>> +	RK3368_GPIO3C2_MAC_RXD3        = (1 << 4),
>>>> +
>>>> +	RK3368_GPIO3C1_MASK            = GENMASK(3, 2),
>>>> +	RK3368_GPIO3C1_GPIO            = 0,
>>>> +	RK3368_GPIO3C1_MAC_RXD2        = (1 << 2),
>>>> +
>>>> +	RK3368_GPIO3C0_MASK            = GENMASK(1, 0),
>>>> +	RK3368_GPIO3C0_GPIO            = 0,
>>>> +	RK3368_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_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,
>>>> -	GPIO3D1_GPIO            = 0,
>>>> -	GPIO3D1_MAC_RXCLKIN,
>>>> -	GPIO3D1_I2C4_SCL,
>>>> -
>>>> -	GPIO3D0_SHIFT           = 0,
>>>> -	GPIO3D0_MASK            = 3 << GPIO3D0_SHIFT,
>>>> -	GPIO3D0_GPIO            = 0,
>>>> -	GPIO3D0_MAC_MDIO,
>>>> -	GPIO3D0_I2C4_SDA,
>>>> +	RK3368_GPIO3D4_MASK            = GENMASK(9, 8),
>>>> +	RK3368_GPIO3D4_GPIO            = 0,
>>>> +	RK3368_GPIO3D4_MAC_TXCLK       = (1 << 8),
>>>> +
>>>> +	RK3368_GPIO3D1_MASK            = GENMASK(3, 2),
>>>> +	RK3368_GPIO3D1_GPIO            = 0,
>>>> +	RK3368_GPIO3D1_MAC_RXCLK       = (1 << 2),
>>>> +
>>>> +	RK3368_GPIO3D0_MASK            = GENMASK(1, 0),
>>>> +	RK3368_GPIO3D0_GPIO            = 0,
>>>> +	RK3368_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*/
>>>> @@ -440,4 +445,34 @@ enum {
>>>>  	MCU_CODE_BASE_BIT31_BIT28_SHIFT		= 0,
>>>>  	MCU_CODE_BASE_BIT31_BIT28_MASK		= GENMASK(3, 0),
>>>>  };
>>>> +
>>>> +/*GRF_SOC_CON15*/
>>>> +enum {
>>>> +	RK3368_GMAC_PHY_INTF_SEL_RGMII = 1 << 9,
>>>> +	RK3368_GMAC_PHY_INTF_SEL_MASK = GENMASK(11, 9),
>>>> +	RK3368_RMII_MODE_MASK  = BIT(6),
>>>> +	RK3368_RMII_MODE       = BIT(6),
>>>> +	RK3368_GMAC_CLK_SEL_2_5M = 2 << 4,
>>>> +	RK3368_GMAC_CLK_SEL_25M = 3 << 4,
>>>> +	RK3368_GMAC_CLK_SEL_125M = 0 << 4,
>>>> +	RK3368_GMAC_CLK_SEL_MASK = GENMASK(5, 4),
>>>> +};
>>>> +
>>>> +/* GRF_SOC_CON16 */
>>>> +enum {
>>>> +	RK3368_RXCLK_DLY_ENA_GMAC_MASK = BIT(15),
>>>> +	RK3368_RXCLK_DLY_ENA_GMAC_DISABLE = 0,
>>>> +	RK3368_RXCLK_DLY_ENA_GMAC_ENABLE = BIT(15),
>>>> +
>>>> +	RK3368_TXCLK_DLY_ENA_GMAC_MASK = BIT(7),
>>>> +	RK3368_TXCLK_DLY_ENA_GMAC_DISABLE = 0,
>>>> +	RK3368_TXCLK_DLY_ENA_GMAC_ENABLE = BIT(7),
>>>> +
>>>> +	RK3368_CLK_RX_DL_CFG_GMAC_SHIFT = 8,
>>>> +	RK3368_CLK_RX_DL_CFG_GMAC_MASK = GENMASK(14, 8),
>>>> +
>>>> +	RK3368_CLK_TX_DL_CFG_GMAC_SHIFT = 0,
>>>> +	RK3368_CLK_TX_DL_CFG_GMAC_MASK = GENMASK(6, 0),
>>>> +};
>>>> +
>>>>  #endif
>>>> diff --git a/drivers/pinctrl/rockchip/pinctrl_rk3368.c b/drivers/pinctrl/rockchip/pinctrl_rk3368.c
>>>> index bdf0758..b77c5ab 100644
>>>> --- a/drivers/pinctrl/rockchip/pinctrl_rk3368.c
>>>> +++ b/drivers/pinctrl/rockchip/pinctrl_rk3368.c
>>>> @@ -30,9 +30,9 @@ static void pinctrl_rk3368_uart_config(struct rk3368_pinctrl_priv *priv,
>>>>  	switch (uart_id) {
>>>>  	case PERIPH_ID_UART2:
>>>>  		rk_clrsetreg(&grf->gpio2a_iomux,
>>>> -			     GPIO2A6_MASK | GPIO2A5_MASK,
>>>> -			     GPIO2A6_UART2_SIN << GPIO2A6_SHIFT |
>>>> -			     GPIO2A5_UART2_SOUT << GPIO2A5_SHIFT);
>>>> +			     RK3368_GPIO2A6_MASK | RK3368_GPIO2A5_MASK,
>>>> +			     RK3368_GPIO2A6_UART2_SIN |
>>>> +			     RK3368_GPIO2A5_UART2_SOUT);
>>>>  		break;
>>>>  	case PERIPH_ID_UART0:
>>>>  		break;
>>>> @@ -42,12 +42,11 @@ static void pinctrl_rk3368_uart_config(struct rk3368_pinctrl_priv *priv,
>>>>  		break;
>>>>  	case PERIPH_ID_UART4:
>>>>  		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);
>>>> +			     RK3368_GPIO0D0_MASK | RK3368_GPIO0D1_MASK |
>>>> +			     RK3368_GPIO0D2_MASK | RK3368_GPIO0D3_MASK,
>>>> +			     RK3368_GPIO0D0_GPIO | RK3368_GPIO0D1_GPIO |
>>>> +			     RK3368_GPIO0D2_UART4_SOUT |
>>>> +			     RK3368_GPIO0D3_UART4_SIN);
>>>>  		break;
>>>>  	default:
>>>>  		debug("uart id = %d iomux error!\n", uart_id);

Hi Philipp,


On 07/24/2017 05:11 PM, Dr. Philipp Tomsich wrote:
>> On 24 Jul 2017, at 10:34, Andy Yan <andy.yan@rock-chips.com> wrote:
>>
>> Hi Philipp:
>>
>>
>> On 2017年07月21日 16:34, Dr. Philipp Tomsich wrote:
>>>> On 21 Jul 2017, at 05:50, Andy Yan <andy.yan@rock-chips.com> wrote:
>>>>
>>>> Hi Philipp:
>>>>
>>>>
>>>> On 2017年07月19日 04:36, Philipp Tomsich 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 rename, we do this now
>>>>> before having more code use this.
>>>>>
>>>>> As some of the downstream drivers (e.g. the Designware GMAC wrapper)
>>>>> may need to include the grf-header of multiple devices, we rename the
>>>>> various defines for the RK3368 by prefixing them with the device name.
>>>>> This avoids future trouble during driver integration.
>>>>     Is that really necessary to add such a prefix for all the register definition, just to fit  the GMAC dirver?
>>>> Maybe the gmac driver can define the platform specific macro in its own code like dwmac-rk in the kernel. Then we can keep the register header file a little tidy.
>>> The problem is not the GMAC driver (although it was the motivation for doing
>>> this change), but rather how easy it is to pick up the wrong grf-file and refer
>>> to the wrong definitions/values… the conflicts are not through the GMAC-specifc
>>> defines but rather through things like the IOMUX masks or (before I moved
>>> this to the DDR-driver) things like the defines to enable DRAM in GRF.
>>>
>>> Until we find a way to consistently structure things in such a way that
>>> 	(a) common constructs can be shared (e.g. the masks in the IOMUX)
>>> 	(b) specific definitions (e.g. the pin-out for a specific device) is not visible outside of its driver
>>> I prefer the names to explicitly refer to each device.
>>>    That said, the longer-term plan (especially with a larger number of devices
>>> being supported) needs to include a consistent rework of how (and where)
>>> we manage all these definitions from the grf-header files.
>> It's true that many devices use GRF, we also face this condition in the kernel land.And all the drivers in kernel handle this device specific definition in the driver itself(by of_device_id->data or MACRO). And there is also no need to use the grf constructs, the common syscon api will provide  you the grf base address. I think this keeps things simple, we don't need to rely on too much of the grf or cru header, maybe some day we can remove the header file, just as clean as the kernel.
> The problem is not the base-address of GRF, but rather the need to pull in the
> register structure (so we can refer to individual registers by name instead of by
> hard-coded offset): as we pull this in today, the definitions for the various IOMUX
> entries will also be pulled in, which will cause duplicate definitions.

I really don't like the idea of add the prefix, which end with longer 
MACRO but no other improve.
I think the GRF is used mostly for the IOMUX, in this case the better 
solution is
port the kernel rockchip-pinctrl driver in uboot, which not rely on the 
dtsi instead of header file.

If we do this, we can remove most of the GRF header definition for 
IOMUX, I can ask David Wu
to do this, but maybe few weeks later.

Thanks,
- Kever
>
>> On the other hand, add such a prefix of rk3368 breaks the consistency with other rockchip devices.
> I do agree that we need to clean this up across all the boards and drivers
> supported today.  As of today, I can not resolve this on a RK3368-only basis,
> as I’d need to clean up the RK3288 at the same time (and don’t want to do this
> in a RK3368-focused series that already has way too much going on).
>
> Let’s try to clean this up for the next (i.e. the one after this) merge window.
> Once we have bit-definitions out of the grf-header, I’ll remove the RK3368_
> prefixes...
>
>>>>> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
>>>>> ---
>>>>>
>>>>>   arch/arm/include/asm/arch-rockchip/grf_rk3368.h | 617 +++++++++++++-----------
>>>>>   drivers/pinctrl/rockchip/pinctrl_rk3368.c       |  17 +-
>>>>>   2 files changed, 334 insertions(+), 300 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..a97dc1e 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,
>>>>> -	GPIO0C6_GPIO            = 0,
>>>>> -	GPIO0C6_LCDC_D18,
>>>>> -	GPIO0C6_TRACE_D8,
>>>>> -	GPIO0C6_UART1_CTSN,
>>>>> -
>>>>> -	GPIO0C5_SHIFT           = 10,
>>>>> -	GPIO0C5_MASK            = 3 << GPIO0C5_SHIFT,
>>>>> -	GPIO0C5_GPIO            = 0,
>>>>> -	GPIO0C5_LCDC_D17,
>>>>> -	GPIO0C5_TRACE_D7,
>>>>> -	GPIO0C5_UART1_SOUT,
>>>>> -
>>>>> -	GPIO0C4_SHIFT           = 8,
>>>>> -	GPIO0C4_MASK            = 3 << GPIO0C4_SHIFT,
>>>>> -	GPIO0C4_GPIO            = 0,
>>>>> -	GPIO0C4_LCDC_D16,
>>>>> -	GPIO0C4_TRACE_D6,
>>>>> -	GPIO0C4_UART1_SIN,
>>>>> -
>>>>> -	GPIO0C3_SHIFT           = 6,
>>>>> -	GPIO0C3_MASK            = 3 << GPIO0C3_SHIFT,
>>>>> -	GPIO0C3_GPIO            = 0,
>>>>> -	GPIO0C3_LCDC_D15,
>>>>> -	GPIO0C3_TRACE_D5,
>>>>> -	GPIO0C3_MCU_JTAG_TDO,
>>>>> -
>>>>> -	GPIO0C2_SHIFT           = 4,
>>>>> -	GPIO0C2_MASK            = 3 << GPIO0C2_SHIFT,
>>>>> -	GPIO0C2_GPIO            = 0,
>>>>> -	GPIO0C2_LCDC_D14,
>>>>> -	GPIO0C2_TRACE_D4,
>>>>> -	GPIO0C2_MCU_JTAG_TDI,
>>>>> -
>>>>> -	GPIO0C1_SHIFT           = 2,
>>>>> -	GPIO0C1_MASK            = 3 << GPIO0C1_SHIFT,
>>>>> -	GPIO0C1_GPIO            = 0,
>>>>> -	GPIO0C1_LCDC_D13,
>>>>> -	GPIO0C1_TRACE_D3,
>>>>> -	GPIO0C1_MCU_JTAG_TRTSN,
>>>>> -
>>>>> -	GPIO0C0_SHIFT           = 0,
>>>>> -	GPIO0C0_MASK            = 3 << GPIO0C0_SHIFT,
>>>>> -	GPIO0C0_GPIO            = 0,
>>>>> -	GPIO0C0_LCDC_D12,
>>>>> -	GPIO0C0_TRACE_D2,
>>>>> -	GPIO0C0_MCU_JTAG_TDO,
>>>>> +	RK3368_GPIO0C7_MASK	       = GENMASK(15, 14),
>>>>> +	RK3368_GPIO0C7_GPIO	       = 0,
>>>>> +	RK3368_GPIO0C7_LCDC_D19        = (1 << 14),
>>>>> +	RK3368_GPIO0C7_TRACE_D9        = (2 << 14),
>>>>> +	RK3368_GPIO0C7_UART1_RTSN      = (3 << 14),
>>>>> +
>>>>> +	RK3368_GPIO0C6_MASK            = GENMASK(13, 12),
>>>>> +	RK3368_GPIO0C6_GPIO            = 0,
>>>>> +	RK3368_GPIO0C6_LCDC_D18        = (1 << 12),
>>>>> +	RK3368_GPIO0C6_TRACE_D8        = (2 << 12),
>>>>> +	RK3368_GPIO0C6_UART1_CTSN      = (3 << 12),
>>>>> +
>>>>> +	RK3368_GPIO0C5_MASK            = GENMASK(11, 10),
>>>>> +	RK3368_GPIO0C5_GPIO            = 0,
>>>>> +	RK3368_GPIO0C5_LCDC_D17        = (1 << 10),
>>>>> +	RK3368_GPIO0C5_TRACE_D7        = (2 << 10),
>>>>> +	RK3368_GPIO0C5_UART1_SOUT      = (3 << 10),
>>>>> +
>>>>> +	RK3368_GPIO0C4_MASK            = GENMASK(9, 8),
>>>>> +	RK3368_GPIO0C4_GPIO            = 0,
>>>>> +	RK3368_GPIO0C4_LCDC_D16        = (1 << 8),
>>>>> +	RK3368_GPIO0C4_TRACE_D6        = (2 << 8),
>>>>> +	RK3368_GPIO0C4_UART1_SIN       = (3 << 8),
>>>>> +
>>>>> +	RK3368_GPIO0C3_MASK            = GENMASK(7, 6),
>>>>> +	RK3368_GPIO0C3_GPIO            = 0,
>>>>> +	RK3368_GPIO0C3_LCDC_D15        = (1 << 6),
>>>>> +	RK3368_GPIO0C3_TRACE_D5        = (2 << 6),
>>>>> +	RK3368_GPIO0C3_MCU_JTAG_TDO    = (3 << 6),
>>>>> +
>>>>> +	RK3368_GPIO0C2_MASK            = GENMASK(5, 4),
>>>>> +	RK3368_GPIO0C2_GPIO            = 0,
>>>>> +	RK3368_GPIO0C2_LCDC_D14        = (1 << 4),
>>>>> +	RK3368_GPIO0C2_TRACE_D4        = (2 << 4),
>>>>> +	RK3368_GPIO0C2_MCU_JTAG_TDI    = (3 << 4),
>>>>> +
>>>>> +	RK3368_GPIO0C1_MASK            = GENMASK(3, 2),
>>>>> +	RK3368_GPIO0C1_GPIO            = 0,
>>>>> +	RK3368_GPIO0C1_LCDC_D13        = (1 << 2),
>>>>> +	RK3368_GPIO0C1_TRACE_D3        = (2 << 2),
>>>>> +	RK3368_GPIO0C1_MCU_JTAG_TRTSN  = (3 << 2),
>>>>> +
>>>>> +	RK3368_GPIO0C0_MASK            = GENMASK(1, 0),
>>>>> +	RK3368_GPIO0C0_GPIO            = 0,
>>>>> +	RK3368_GPIO0C0_LCDC_D12        = (1 << 0),
>>>>> +	RK3368_GPIO0C0_TRACE_D2        = (2 << 0),
>>>>> +	RK3368_GPIO0C0_MCU_JTAG_TDO    = (3 << 0),
>>>>>   };
>>>>>     /*GRF_GPIO0D_IOMUX*/
>>>>>   enum {
>>>>> -	GPIO0D7_SHIFT           = 14,
>>>>> -	GPIO0D7_MASK            = 3 << GPIO0D7_SHIFT,
>>>>> -	GPIO0D7_GPIO            = 0,
>>>>> -	GPIO0D7_LCDC_DCLK,
>>>>> -	GPIO0D7_TRACE_CTL,
>>>>> -	GPIO0D7_PMU_DEBUG5,
>>>>> -
>>>>> -	GPIO0D6_SHIFT           = 12,
>>>>> -	GPIO0D6_MASK            = 3 << GPIO0D6_SHIFT,
>>>>> -	GPIO0D6_GPIO            = 0,
>>>>> -	GPIO0D6_LCDC_DEN,
>>>>> -	GPIO0D6_TRACE_CLK,
>>>>> -	GPIO0D6_PMU_DEBUG4,
>>>>> -
>>>>> -	GPIO0D5_SHIFT           = 10,
>>>>> -	GPIO0D5_MASK            = 3 << GPIO0D5_SHIFT,
>>>>> -	GPIO0D5_GPIO            = 0,
>>>>> -	GPIO0D5_LCDC_VSYNC,
>>>>> -	GPIO0D5_TRACE_D15,
>>>>> -	GPIO0D5_PMU_DEBUG3,
>>>>> -
>>>>> -	GPIO0D4_SHIFT           = 8,
>>>>> -	GPIO0D4_MASK            = 3 << GPIO0D4_SHIFT,
>>>>> -	GPIO0D4_GPIO            = 0,
>>>>> -	GPIO0D4_LCDC_HSYNC,
>>>>> -	GPIO0D4_TRACE_D14,
>>>>> -	GPIO0D4_PMU_DEBUG2,
>>>>> -
>>>>> -	GPIO0D3_SHIFT           = 6,
>>>>> -	GPIO0D3_MASK            = 3 << GPIO0D3_SHIFT,
>>>>> -	GPIO0D3_GPIO            = 0,
>>>>> -	GPIO0D3_LCDC_D23,
>>>>> -	GPIO0D3_TRACE_D13,
>>>>> -	GPIO0D3_UART4_SIN,
>>>>> -
>>>>> -	GPIO0D2_SHIFT           = 4,
>>>>> -	GPIO0D2_MASK            = 3 << GPIO0D2_SHIFT,
>>>>> -	GPIO0D2_GPIO            = 0,
>>>>> -	GPIO0D2_LCDC_D22,
>>>>> -	GPIO0D2_TRACE_D12,
>>>>> -	GPIO0D2_UART4_SOUT,
>>>>> -
>>>>> -	GPIO0D1_SHIFT           = 2,
>>>>> -	GPIO0D1_MASK            = 3 << GPIO0D1_SHIFT,
>>>>> -	GPIO0D1_GPIO            = 0,
>>>>> -	GPIO0D1_LCDC_D21,
>>>>> -	GPIO0D1_TRACE_D11,
>>>>> -	GPIO0D1_UART4_RTSN,
>>>>> -
>>>>> -	GPIO0D0_SHIFT           = 0,
>>>>> -	GPIO0D0_MASK            = 3 << GPIO0D0_SHIFT,
>>>>> -	GPIO0D0_GPIO            = 0,
>>>>> -	GPIO0D0_LCDC_D20,
>>>>> -	GPIO0D0_TRACE_D10,
>>>>> -	GPIO0D0_UART4_CTSN,
>>>>> +	RK3368_GPIO0D7_MASK            = GENMASK(15, 14),
>>>>> +	RK3368_GPIO0D7_GPIO            = 0,
>>>>> +	RK3368_GPIO0D7_LCDC_DCLK       = (1 << 14),
>>>>> +	RK3368_GPIO0D7_TRACE_CTL       = (2 << 14),
>>>>> +	RK3368_GPIO0D7_PMU_DEBUG5      = (3 << 14),
>>>>> +
>>>>> +	RK3368_GPIO0D6_MASK            = GENMASK(13, 12),
>>>>> +	RK3368_GPIO0D6_GPIO            = 0,
>>>>> +	RK3368_GPIO0D6_LCDC_DEN        = (1 << 12),
>>>>> +	RK3368_GPIO0D6_TRACE_CLK       = (2 << 12),
>>>>> +	RK3368_GPIO0D6_PMU_DEBUG4      = (3 << 12),
>>>>> +
>>>>> +	RK3368_GPIO0D5_MASK            = GENMASK(11, 10),
>>>>> +	RK3368_GPIO0D5_GPIO            = 0,
>>>>> +	GPIO0D5_LCDC_VSYNC             = (1 << 10),
>>>>> +	RK3368_GPIO0D5_TRACE_D15       = (2 << 10),
>>>>> +	RK3368_GPIO0D5_PMU_DEBUG3      = (3 << 10),
>>>>> +
>>>>> +	RK3368_GPIO0D4_MASK            = GENMASK(9, 8),
>>>>> +	RK3368_GPIO0D4_GPIO            = 0,
>>>>> +	RK3368_GPIO0D4_LCDC_HSYNC      = (1 << 8),
>>>>> +	RK3368_GPIO0D4_TRACE_D14       = (2 << 8),
>>>>> +	RK3368_GPIO0D4_PMU_DEBUG2      = (3 << 8),
>>>>> +
>>>>> +	RK3368_GPIO0D3_MASK            = GENMASK(7, 6),
>>>>> +	RK3368_GPIO0D3_GPIO            = 0,
>>>>> +	RK3368_GPIO0D3_LCDC_D23        = (1 << 6),
>>>>> +	RK3368_GPIO0D3_TRACE_D13       = (2 << 6),
>>>>> +	RK3368_GPIO0D3_UART4_SIN       = (3 << 6),
>>>>> +
>>>>> +	RK3368_GPIO0D2_MASK            = GENMASK(5, 4),
>>>>> +	RK3368_GPIO0D2_GPIO            = 0,
>>>>> +	RK3368_GPIO0D2_LCDC_D22        = (1 << 4),
>>>>> +	RK3368_GPIO0D2_TRACE_D12       = (2 << 4),
>>>>> +	RK3368_GPIO0D2_UART4_SOUT      = (3 << 4),
>>>>> +
>>>>> +	RK3368_GPIO0D1_MASK            = GENMASK(3, 2),
>>>>> +	RK3368_GPIO0D1_GPIO            = 0,
>>>>> +	RK3368_GPIO0D1_LCDC_D21        = (1 << 2),
>>>>> +	RK3368_GPIO0D1_TRACE_D11       = (2 << 2),
>>>>> +	RK3368_GPIO0D1_UART4_RTSN      = (3 << 2),
>>>>> +
>>>>> +	RK3368_GPIO0D0_MASK            = GENMASK(1, 0),
>>>>> +	RK3368_GPIO0D0_GPIO            = 0,
>>>>> +	RK3368_GPIO0D0_LCDC_D20        = (1 << 0),
>>>>> +	RK3368_GPIO0D0_TRACE_D10       = (2 << 0),
>>>>> +	RK3368_GPIO0D0_UART4_CTSN      = (3 << 0),
>>>>>   };
>>>>>     /*GRF_GPIO2A_IOMUX*/
>>>>>   enum {
>>>>> -	GPIO2A7_SHIFT           = 14,
>>>>> -	GPIO2A7_MASK            = 3 << GPIO2A7_SHIFT,
>>>>> -	GPIO2A7_GPIO            = 0,
>>>>> -	GPIO2A7_SDMMC0_D2,
>>>>> -	GPIO2A7_JTAG_TCK,
>>>>> -
>>>>> -	GPIO2A6_SHIFT           = 12,
>>>>> -	GPIO2A6_MASK            = 3 << GPIO2A6_SHIFT,
>>>>> -	GPIO2A6_GPIO            = 0,
>>>>> -	GPIO2A6_SDMMC0_D1,
>>>>> -	GPIO2A6_UART2_SIN,
>>>>> -
>>>>> -	GPIO2A5_SHIFT           = 10,
>>>>> -	GPIO2A5_MASK            = 3 << GPIO2A5_SHIFT,
>>>>> -	GPIO2A5_GPIO            = 0,
>>>>> -	GPIO2A5_SDMMC0_D0,
>>>>> -	GPIO2A5_UART2_SOUT,
>>>>> -
>>>>> -	GPIO2A4_SHIFT           = 8,
>>>>> -	GPIO2A4_MASK            = 3 << GPIO2A4_SHIFT,
>>>>> -	GPIO2A4_GPIO            = 0,
>>>>> -	GPIO2A4_FLASH_DQS,
>>>>> -	GPIO2A4_EMMC_CLKO,
>>>>> -
>>>>> -	GPIO2A3_SHIFT           = 6,
>>>>> -	GPIO2A3_MASK            = 3 << GPIO2A3_SHIFT,
>>>>> -	GPIO2A3_GPIO            = 0,
>>>>> -	GPIO2A3_FLASH_CSN3,
>>>>> -	GPIO2A3_EMMC_RSTNO,
>>>>> -
>>>>> -	GPIO2A2_SHIFT           = 4,
>>>>> -	GPIO2A2_MASK            = 3 << GPIO2A2_SHIFT,
>>>>> -	GPIO2A2_GPIO           = 0,
>>>>> -	GPIO2A2_FLASH_CSN2,
>>>>> -
>>>>> -	GPIO2A1_SHIFT           = 2,
>>>>> -	GPIO2A1_MASK            = 3 << GPIO2A1_SHIFT,
>>>>> -	GPIO2A1_GPIO            = 0,
>>>>> -	GPIO2A1_FLASH_CSN1,
>>>>> -
>>>>> -	GPIO2A0_SHIFT           = 0,
>>>>> -	GPIO2A0_MASK            = 3 << GPIO2A0_SHIFT,
>>>>> -	GPIO2A0_GPIO            = 0,
>>>>> -	GPIO2A0_FLASH_CSN0,
>>>>> +	RK3368_GPIO2A7_MASK            = GENMASK(15, 14),
>>>>> +	RK3368_GPIO2A7_GPIO            = 0,
>>>>> +	RK3368_GPIO2A7_SDMMC0_D2       = (1 << 14),
>>>>> +	RK3368_GPIO2A7_JTAG_TCK        = (2 << 14),
>>>>> +
>>>>> +	RK3368_GPIO2A6_MASK            = GENMASK(13, 12),
>>>>> +	RK3368_GPIO2A6_GPIO            = 0,
>>>>> +	RK3368_GPIO2A6_SDMMC0_D1       = (1 << 12),
>>>>> +	RK3368_GPIO2A6_UART2_SIN       = (2 << 12),
>>>>> +
>>>>> +	RK3368_GPIO2A5_MASK            = GENMASK(11, 10),
>>>>> +	RK3368_GPIO2A5_GPIO            = 0,
>>>>> +	RK3368_GPIO2A5_SDMMC0_D0       = (1 << 10),
>>>>> +	RK3368_GPIO2A5_UART2_SOUT      = (2 << 10),
>>>>> +
>>>>> +	RK3368_GPIO2A4_MASK            = GENMASK(9, 8),
>>>>> +	RK3368_GPIO2A4_RK3368_GPIO     = 0,
>>>>> +	RK3368_GPIO2A4_FLASH_DQS       = (1 << 8),
>>>>> +	RK3368_GPIO2A4_EMMC_CLKOUT     = (2 << 8),
>>>>> +
>>>>> +	RK3368_GPIO2A3_MASK            = GENMASK(7, 6),
>>>>> +	RK3368_GPIO2A3_GPIO            = 0,
>>>>> +	RK3368_GPIO2A3_FLASH_CSN3      = (1 << 6),
>>>>> +	RK3368_GPIO2A3_EMMC_RSTNOUT    = (2 << 6),
>>>>> +
>>>>> +	RK3368_GPIO2A2_MASK            = GENMASK(5, 4),
>>>>> +	RK3368_GPIO2A2_GPIO            = 0,
>>>>> +	RK3368_GPIO2A2_FLASH_CSN2      = (1 << 4),
>>>>> +
>>>>> +	RK3368_GPIO2A1_MASK            = GENMASK(3, 2),
>>>>> +	RK3368_GPIO2A1_GPIO            = 0,
>>>>> +	RK3368_GPIO2A1_FLASH_CSN1      = (1 << 2),
>>>>> +
>>>>> +	RK3368_GPIO2A0_MASK            = GENMASK(1, 0),
>>>>> +	RK3368_GPIO2A0_GPIO            = 0,
>>>>> +	RK3368_GPIO2A0_FLASH_CSN0      = (1 << 0),
>>>>>   };
>>>>>   -/*GRF_GPIO2D_IOMUX*/
>>>>> +/*GRF_RK3368_GPIO2D_IOMUX*/
>>>>>   enum {
>>>>> -	GPIO2D7_SHIFT           = 14,
>>>>> -	GPIO2D7_MASK            = 3 << GPIO2D7_SHIFT,
>>>>> -	GPIO2D7_GPIO            = 0,
>>>>> -	GPIO2D7_SDIO0_D3,
>>>>> -
>>>>> -	GPIO2D6_SHIFT           = 12,
>>>>> -	GPIO2D6_MASK            = 3 << GPIO2D6_SHIFT,
>>>>> -	GPIO2D6_GPIO            = 0,
>>>>> -	GPIO2D6_SDIO0_D2,
>>>>> -
>>>>> -	GPIO2D5_SHIFT           = 10,
>>>>> -	GPIO2D5_MASK            = 3 << GPIO2D5_SHIFT,
>>>>> -	GPIO2D5_GPIO            = 0,
>>>>> -	GPIO2D5_SDIO0_D1,
>>>>> -
>>>>> -	GPIO2D4_SHIFT           = 8,
>>>>> -	GPIO2D4_MASK            = 3 << GPIO2D4_SHIFT,
>>>>> -	GPIO2D4_GPIO            = 0,
>>>>> -	GPIO2D4_SDIO0_D0,
>>>>> -
>>>>> -	GPIO2D3_SHIFT           = 6,
>>>>> -	GPIO2D3_MASK            = 3 << GPIO2D3_SHIFT,
>>>>> -	GPIO2D3_GPIO            = 0,
>>>>> -	GPIO2D3_UART0_RTS0,
>>>>> -
>>>>> -	GPIO2D2_SHIFT           = 4,
>>>>> -	GPIO2D2_MASK            = 3 << GPIO2D2_SHIFT,
>>>>> -	GPIO2D2_GPIO            = 0,
>>>>> -	GPIO2D2_UART0_CTS0,
>>>>> -
>>>>> -	GPIO2D1_SHIFT           = 2,
>>>>> -	GPIO2D1_MASK            = 3 << GPIO2D1_SHIFT,
>>>>> -	GPIO2D1_GPIO            = 0,
>>>>> -	GPIO2D1_UART0_SOUT,
>>>>> -
>>>>> -	GPIO2D0_SHIFT           = 0,
>>>>> -	GPIO2D0_MASK            = 3 << GPIO2D0_SHIFT,
>>>>> -	GPIO2D0_GPIO            = 0,
>>>>> -	GPIO2D0_UART0_SIN,
>>>>> +	RK3368_GPIO2D7_MASK            = GENMASK(15, 14),
>>>>> +	RK3368_GPIO2D7_GPIO            = 0,
>>>>> +	RK3368_GPIO2D7_SDIO0_D3        = (1 << 14),
>>>>> +
>>>>> +	RK3368_GPIO2D6_MASK            = GENMASK(13, 12),
>>>>> +	RK3368_GPIO2D6_GPIO            = 0,
>>>>> +	RK3368_GPIO2D6_SDIO0_D2        = (1 << 12),
>>>>> +
>>>>> +	RK3368_GPIO2D5_MASK            = GENMASK(11, 10),
>>>>> +	RK3368_GPIO2D5_GPIO            = 0,
>>>>> +	RK3368_GPIO2D5_SDIO0_D1        = (1 << 10),
>>>>> +
>>>>> +	RK3368_GPIO2D4_MASK            = GENMASK(9, 8),
>>>>> +	RK3368_GPIO2D4_GPIO            = 0,
>>>>> +	RK3368_GPIO2D4_SDIO0_D0        = (1 << 8),
>>>>> +
>>>>> +	RK3368_GPIO2D3_MASK            = GENMASK(7, 6),
>>>>> +	RK3368_GPIO2D3_GPIO            = 0,
>>>>> +	RK3368_GPIO2D3_UART0_RTS0      = (1 << 6),
>>>>> +
>>>>> +	RK3368_GPIO2D2_MASK            = GENMASK(5, 4),
>>>>> +	RK3368_GPIO2D2_GPIO            = 0,
>>>>> +	RK3368_GPIO2D2_UART0_CTS0      = (1 << 4),
>>>>> +
>>>>> +	RK3368_GPIO2D1_MASK            = GENMASK(3, 2),
>>>>> +	RK3368_GPIO2D1_GPIO            = 0,
>>>>> +	RK3368_GPIO2D1_UART0_SOUT      = (1 << 2),
>>>>> +
>>>>> +	RK3368_GPIO2D0_MASK            = GENMASK(1, 0),
>>>>> +	RK3368_GPIO2D0_GPIO            = 0,
>>>>> +	RK3368_GPIO2D0_UART0_SIN       = (1 << 0),
>>>>> +};
>>>>> +
>>>>> +/* GRF_GPIO1C_IOMUX */
>>>>> +enum {
>>>>> +	RK3368_GPIO1C7_MASK            = GENMASK(15, 14),
>>>>> +	RK3368_GPIO1C7_GPIO            = 0,
>>>>> +	RK3368_GPIO1C7_EMMC_DATA5      = (2 << 14),
>>>>> +
>>>>> +	RK3368_GPIO1C6_MASK            = GENMASK(13, 12),
>>>>> +	RK3368_GPIO1C6_GPIO            = 0,
>>>>> +	RK3368_GPIO1C6_EMMC_DATA4      = (2 << 12),
>>>>> +
>>>>> +	RK3368_GPIO1C5_MASK            = GENMASK(11, 10),
>>>>> +	RK3368_GPIO1C5_GPIO            = 0,
>>>>> +	RK3368_GPIO1C5_EMMC_DATA3      = (2 << 10),
>>>>> +
>>>>> +	RK3368_GPIO1C4_MASK            = GENMASK(9, 8),
>>>>> +	RK3368_GPIO1C4_GPIO            = 0,
>>>>> +	RK3368_GPIO1C4_EMMC_DATA2      = (2 << 8),
>>>>> +
>>>>> +	RK3368_GPIO1C3_MASK            = GENMASK(7, 6),
>>>>> +	RK3368_GPIO1C3_GPIO            = 0,
>>>>> +	RK3368_GPIO1C3_EMMC_DATA1      = (2 << 6),
>>>>> +
>>>>> +	RK3368_GPIO1C2_MASK            = GENMASK(5, 4),
>>>>> +	RK3368_GPIO1C2_GPIO            = 0,
>>>>> +	RK3368_GPIO1C2_EMMC_DATA0      = (2 << 4),
>>>>> +};
>>>>> +
>>>>> +/* GRF_GPIO1D_IOMUX*/
>>>>> +enum {
>>>>> +	RK3368_GPIO1D3_MASK            = GENMASK(7, 6),
>>>>> +	RK3368_GPIO1D3_GPIO            = 0,
>>>>> +	RK3368_GPIO1D3_EMMC_PWREN      = (2 << 6),
>>>>> +
>>>>> +	RK3368_GPIO1D2_MASK            = GENMASK(5, 4),
>>>>> +	RK3368_GPIO1D2_GPIO            = 0,
>>>>> +	RK3368_GPIO1D2_EMMC_CMD        = (2 << 4),
>>>>> +
>>>>> +	RK3368_GPIO1D1_MASK            = GENMASK(3, 2),
>>>>> +	RK3368_GPIO1D1_GPIO            = 0,
>>>>> +	RK3368_GPIO1D1_EMMC_DATA7      = (2 << 2),
>>>>> +
>>>>> +	RK3368_GPIO1D0_MASK            = GENMASK(1, 0),
>>>>> +	RK3368_GPIO1D0_GPIO            = 0,
>>>>> +	RK3368_GPIO1D0_EMMC_DATA6      = (2 << 0),
>>>>> +};
>>>>> +
>>>>> +
>>>>> +/*GRF_GPIO3B_IOMUX*/
>>>>> +enum {
>>>>> +	RK3368_GPIO3B7_MASK            = GENMASK(15, 14),
>>>>> +	RK3368_GPIO3B7_GPIO            = 0,
>>>>> +	RK3368_GPIO3B7_MAC_RXD0        = (1 << 14),
>>>>> +
>>>>> +	RK3368_GPIO3B6_MASK            = GENMASK(13, 12),
>>>>> +	RK3368_GPIO3B6_GPIO            = 0,
>>>>> +	RK3368_GPIO3B6_MAC_TXD3        = (1 << 12),
>>>>> +
>>>>> +	RK3368_GPIO3B5_MASK            = GENMASK(11, 10),
>>>>> +	RK3368_GPIO3B5_GPIO            = 0,
>>>>> +	RK3368_GPIO3B5_MAC_TXEN        = (1 << 10),
>>>>> +
>>>>> +	RK3368_GPIO3B4_MASK            = GENMASK(9, 8),
>>>>> +	RK3368_GPIO3B4_GPIO            = 0,
>>>>> +	RK3368_GPIO3B4_MAC_COL         = (1 << 8),
>>>>> +
>>>>> +	RK3368_GPIO3B3_MASK            = GENMASK(7, 6),
>>>>> +	RK3368_GPIO3B3_GPIO            = 0,
>>>>> +	RK3368_GPIO3B3_MAC_CRS         = (1 << 6),
>>>>> +
>>>>> +	RK3368_GPIO3B2_MASK            = GENMASK(5, 4),
>>>>> +	RK3368_GPIO3B2_GPIO            = 0,
>>>>> +	RK3368_GPIO3B2_MAC_TXD2        = (1 << 4),
>>>>> +
>>>>> +	RK3368_GPIO3B1_MASK            = GENMASK(3, 2),
>>>>> +	RK3368_GPIO3B1_GPIO            = 0,
>>>>> +	RK3368_GPIO3B1_MAC_TXD1        = (1 << 2),
>>>>> +
>>>>> +	RK3368_GPIO3B0_MASK            = GENMASK(1, 0),
>>>>> +	RK3368_GPIO3B0_GPIO            = 0,
>>>>> +	RK3368_GPIO3B0_MAC_TXD0        = (1 << 0),
>>>>> +	RK3368_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_GPIO            = 0,
>>>>> -	GPIO3C6_MAC_CLK,
>>>>> -	GPIO3C6_ISP_SHUTTERTRIG,
>>>>> -
>>>>> -	GPIO3C5_SHIFT           = 10,
>>>>> -	GPIO3C5_MASK            = 3 << GPIO3C5_SHIFT,
>>>>> -	GPIO3C5_GPIO            = 0,
>>>>> -	GPIO3C5_MAC_RXER,
>>>>> -	GPIO3C5_ISP_PRELIGHTTRIG,
>>>>> -
>>>>> -	GPIO3C4_SHIFT           = 8,
>>>>> -	GPIO3C4_MASK            = 3 << GPIO3C4_SHIFT,
>>>>> -	GPIO3C4_GPIO            = 0,
>>>>> -	GPIO3C4_MAC_RXDV,
>>>>> -	GPIO3C4_ISP_FLASHTRIGOUT,
>>>>> -
>>>>> -	GPIO3C3_SHIFT           = 6,
>>>>> -	GPIO3C3_MASK            = 3 << GPIO3C3_SHIFT,
>>>>> -	GPIO3C3_GPIO            = 0,
>>>>> -	GPIO3C3_MAC_RXDV,
>>>>> -	GPIO3C3_EMMC_RSTNO,
>>>>> -
>>>>> -	GPIO3C2_SHIFT           = 4,
>>>>> -	GPIO3C2_MASK            = 3 << GPIO3C2_SHIFT,
>>>>> -	GPIO3C2_MAC_MDC            = 0,
>>>>> -	GPIO3C2_ISP_SHUTTEREN,
>>>>> -
>>>>> -	GPIO3C1_SHIFT           = 2,
>>>>> -	GPIO3C1_MASK            = 3 << GPIO3C1_SHIFT,
>>>>> -	GPIO3C1_GPIO            = 0,
>>>>> -	GPIO3C1_MAC_RXD2,
>>>>> -	GPIO3C1_UART3_RTSN,
>>>>> -
>>>>> -	GPIO3C0_SHIFT           = 0,
>>>>> -	GPIO3C0_MASK            = 3 << GPIO3C0_SHIFT,
>>>>> -	GPIO3C0_GPIO            = 0,
>>>>> -	GPIO3C0_MAC_RXD1,
>>>>> -	GPIO3C0_UART3_CTSN,
>>>>> -	GPIO3C0_GPS_RFCLK,
>>>>> +	RK3368_GPIO3C6_MASK            = GENMASK(13, 12),
>>>>> +	RK3368_GPIO3C6_GPIO            = 0,
>>>>> +	RK3368_GPIO3C6_MAC_CLK         = (1 << 12),
>>>>> +
>>>>> +	RK3368_GPIO3C5_MASK            = GENMASK(11, 10),
>>>>> +	RK3368_GPIO3C5_GPIO            = 0,
>>>>> +	RK3368_GPIO3C5_MAC_RXEN        = (1 << 10),
>>>>> +
>>>>> +	RK3368_GPIO3C4_MASK            = GENMASK(9, 8),
>>>>> +	RK3368_GPIO3C4_GPIO            = 0,
>>>>> +	RK3368_GPIO3C4_MAC_RXDV        = (1 << 8),
>>>>> +
>>>>> +	RK3368_GPIO3C3_MASK            = GENMASK(7, 6),
>>>>> +	RK3368_GPIO3C3_GPIO            = 0,
>>>>> +	RK3368_GPIO3C3_MAC_MDC         = (1 << 6),
>>>>> +
>>>>> +	RK3368_GPIO3C2_MASK            = GENMASK(5, 4),
>>>>> +	RK3368_GPIO3C2_GPIO            = 0,
>>>>> +	RK3368_GPIO3C2_MAC_RXD3        = (1 << 4),
>>>>> +
>>>>> +	RK3368_GPIO3C1_MASK            = GENMASK(3, 2),
>>>>> +	RK3368_GPIO3C1_GPIO            = 0,
>>>>> +	RK3368_GPIO3C1_MAC_RXD2        = (1 << 2),
>>>>> +
>>>>> +	RK3368_GPIO3C0_MASK            = GENMASK(1, 0),
>>>>> +	RK3368_GPIO3C0_GPIO            = 0,
>>>>> +	RK3368_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_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,
>>>>> -	GPIO3D1_GPIO            = 0,
>>>>> -	GPIO3D1_MAC_RXCLKIN,
>>>>> -	GPIO3D1_I2C4_SCL,
>>>>> -
>>>>> -	GPIO3D0_SHIFT           = 0,
>>>>> -	GPIO3D0_MASK            = 3 << GPIO3D0_SHIFT,
>>>>> -	GPIO3D0_GPIO            = 0,
>>>>> -	GPIO3D0_MAC_MDIO,
>>>>> -	GPIO3D0_I2C4_SDA,
>>>>> +	RK3368_GPIO3D4_MASK            = GENMASK(9, 8),
>>>>> +	RK3368_GPIO3D4_GPIO            = 0,
>>>>> +	RK3368_GPIO3D4_MAC_TXCLK       = (1 << 8),
>>>>> +
>>>>> +	RK3368_GPIO3D1_MASK            = GENMASK(3, 2),
>>>>> +	RK3368_GPIO3D1_GPIO            = 0,
>>>>> +	RK3368_GPIO3D1_MAC_RXCLK       = (1 << 2),
>>>>> +
>>>>> +	RK3368_GPIO3D0_MASK            = GENMASK(1, 0),
>>>>> +	RK3368_GPIO3D0_GPIO            = 0,
>>>>> +	RK3368_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*/
>>>>> @@ -440,4 +445,34 @@ enum {
>>>>>   	MCU_CODE_BASE_BIT31_BIT28_SHIFT		= 0,
>>>>>   	MCU_CODE_BASE_BIT31_BIT28_MASK		= GENMASK(3, 0),
>>>>>   };
>>>>> +
>>>>> +/*GRF_SOC_CON15*/
>>>>> +enum {
>>>>> +	RK3368_GMAC_PHY_INTF_SEL_RGMII = 1 << 9,
>>>>> +	RK3368_GMAC_PHY_INTF_SEL_MASK = GENMASK(11, 9),
>>>>> +	RK3368_RMII_MODE_MASK  = BIT(6),
>>>>> +	RK3368_RMII_MODE       = BIT(6),
>>>>> +	RK3368_GMAC_CLK_SEL_2_5M = 2 << 4,
>>>>> +	RK3368_GMAC_CLK_SEL_25M = 3 << 4,
>>>>> +	RK3368_GMAC_CLK_SEL_125M = 0 << 4,
>>>>> +	RK3368_GMAC_CLK_SEL_MASK = GENMASK(5, 4),
>>>>> +};
>>>>> +
>>>>> +/* GRF_SOC_CON16 */
>>>>> +enum {
>>>>> +	RK3368_RXCLK_DLY_ENA_GMAC_MASK = BIT(15),
>>>>> +	RK3368_RXCLK_DLY_ENA_GMAC_DISABLE = 0,
>>>>> +	RK3368_RXCLK_DLY_ENA_GMAC_ENABLE = BIT(15),
>>>>> +
>>>>> +	RK3368_TXCLK_DLY_ENA_GMAC_MASK = BIT(7),
>>>>> +	RK3368_TXCLK_DLY_ENA_GMAC_DISABLE = 0,
>>>>> +	RK3368_TXCLK_DLY_ENA_GMAC_ENABLE = BIT(7),
>>>>> +
>>>>> +	RK3368_CLK_RX_DL_CFG_GMAC_SHIFT = 8,
>>>>> +	RK3368_CLK_RX_DL_CFG_GMAC_MASK = GENMASK(14, 8),
>>>>> +
>>>>> +	RK3368_CLK_TX_DL_CFG_GMAC_SHIFT = 0,
>>>>> +	RK3368_CLK_TX_DL_CFG_GMAC_MASK = GENMASK(6, 0),
>>>>> +};
>>>>> +
>>>>>   #endif
>>>>> diff --git a/drivers/pinctrl/rockchip/pinctrl_rk3368.c b/drivers/pinctrl/rockchip/pinctrl_rk3368.c
>>>>> index bdf0758..b77c5ab 100644
>>>>> --- a/drivers/pinctrl/rockchip/pinctrl_rk3368.c
>>>>> +++ b/drivers/pinctrl/rockchip/pinctrl_rk3368.c
>>>>> @@ -30,9 +30,9 @@ static void pinctrl_rk3368_uart_config(struct rk3368_pinctrl_priv *priv,
>>>>>   	switch (uart_id) {
>>>>>   	case PERIPH_ID_UART2:
>>>>>   		rk_clrsetreg(&grf->gpio2a_iomux,
>>>>> -			     GPIO2A6_MASK | GPIO2A5_MASK,
>>>>> -			     GPIO2A6_UART2_SIN << GPIO2A6_SHIFT |
>>>>> -			     GPIO2A5_UART2_SOUT << GPIO2A5_SHIFT);
>>>>> +			     RK3368_GPIO2A6_MASK | RK3368_GPIO2A5_MASK,
>>>>> +			     RK3368_GPIO2A6_UART2_SIN |
>>>>> +			     RK3368_GPIO2A5_UART2_SOUT);
>>>>>   		break;
>>>>>   	case PERIPH_ID_UART0:
>>>>>   		break;
>>>>> @@ -42,12 +42,11 @@ static void pinctrl_rk3368_uart_config(struct rk3368_pinctrl_priv *priv,
>>>>>   		break;
>>>>>   	case PERIPH_ID_UART4:
>>>>>   		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);
>>>>> +			     RK3368_GPIO0D0_MASK | RK3368_GPIO0D1_MASK |
>>>>> +			     RK3368_GPIO0D2_MASK | RK3368_GPIO0D3_MASK,
>>>>> +			     RK3368_GPIO0D0_GPIO | RK3368_GPIO0D1_GPIO |
>>>>> +			     RK3368_GPIO0D2_UART4_SOUT |
>>>>> +			     RK3368_GPIO0D3_UART4_SIN);
>>>>>   		break;
>>>>>   	default:
>>>>>   		debug("uart id = %d iomux error!\n", uart_id);
>

> On 24 Jul 2017, at 14:45, Kever Yang <kever.yang@rock-chips.com> wrote:
> 
> Hi Philipp,
> 
> 
> On 07/24/2017 05:11 PM, Dr. Philipp Tomsich wrote:
>>> On 24 Jul 2017, at 10:34, Andy Yan <andy.yan@rock-chips.com> wrote:
>>> 
>>> Hi Philipp:
>>> 
>>> 
>>> On 2017年07月21日 16:34, Dr. Philipp Tomsich wrote:
>>>>> On 21 Jul 2017, at 05:50, Andy Yan <andy.yan@rock-chips.com> wrote:
>>>>> 
>>>>> Hi Philipp:
>>>>> 
>>>>> 
>>>>> On 2017年07月19日 04:36, Philipp Tomsich 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 rename, we do this now
>>>>>> before having more code use this.
>>>>>> 
>>>>>> As some of the downstream drivers (e.g. the Designware GMAC wrapper)
>>>>>> may need to include the grf-header of multiple devices, we rename the
>>>>>> various defines for the RK3368 by prefixing them with the device name.
>>>>>> This avoids future trouble during driver integration.
>>>>>    Is that really necessary to add such a prefix for all the register definition, just to fit  the GMAC dirver?
>>>>> Maybe the gmac driver can define the platform specific macro in its own code like dwmac-rk in the kernel. Then we can keep the register header file a little tidy.
>>>> The problem is not the GMAC driver (although it was the motivation for doing
>>>> this change), but rather how easy it is to pick up the wrong grf-file and refer
>>>> to the wrong definitions/values… the conflicts are not through the GMAC-specifc
>>>> defines but rather through things like the IOMUX masks or (before I moved
>>>> this to the DDR-driver) things like the defines to enable DRAM in GRF.
>>>> 
>>>> Until we find a way to consistently structure things in such a way that
>>>> 	(a) common constructs can be shared (e.g. the masks in the IOMUX)
>>>> 	(b) specific definitions (e.g. the pin-out for a specific device) is not visible outside of its driver
>>>> I prefer the names to explicitly refer to each device.
>>>>   That said, the longer-term plan (especially with a larger number of devices
>>>> being supported) needs to include a consistent rework of how (and where)
>>>> we manage all these definitions from the grf-header files.
>>> It's true that many devices use GRF, we also face this condition in the kernel land.And all the drivers in kernel handle this device specific definition in the driver itself(by of_device_id->data or MACRO). And there is also no need to use the grf constructs, the common syscon api will provide  you the grf base address. I think this keeps things simple, we don't need to rely on too much of the grf or cru header, maybe some day we can remove the header file, just as clean as the kernel.
>> The problem is not the base-address of GRF, but rather the need to pull in the
>> register structure (so we can refer to individual registers by name instead of by
>> hard-coded offset): as we pull this in today, the definitions for the various IOMUX
>> entries will also be pulled in, which will cause duplicate definitions.
> 
> I really don't like the idea of add the prefix, which end with longer MACRO but no other improve.
> I think the GRF is used mostly for the IOMUX, in this case the better solution is
> port the kernel rockchip-pinctrl driver in uboot, which not rely on the dtsi instead of header file.
> 
> If we do this, we can remove most of the GRF header definition for IOMUX, I can ask David Wu
> to do this, but maybe few weeks later.

As indicated earlier: this is a temporary solution (although we have a local saying that
"nothing is more permanent than a temporary solution”) and should remain limited to
the RK3368 until we can clean this up across all boards.

Pulling this into the pinctrl drivers is an excellent choice.
Please watch out for board_debug_uart_init(…), which also directly accesses the IOMUX
constants and will need to be restructured together with pinctrl drivers.

Thanks!
—Philipp.

> Thanks,
> - Kever
>> 
>>> On the other hand, add such a prefix of rk3368 breaks the consistency with other rockchip devices.
>> I do agree that we need to clean this up across all the boards and drivers
>> supported today.  As of today, I can not resolve this on a RK3368-only basis,
>> as I’d need to clean up the RK3288 at the same time (and don’t want to do this
>> in a RK3368-focused series that already has way too much going on).
>> 
>> Let’s try to clean this up for the next (i.e. the one after this) merge window.
>> Once we have bit-definitions out of the grf-header, I’ll remove the RK3368_
>> prefixes...
>> 
>>>>>> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
>>>>>> ---
>>>>>> 
>>>>>>  arch/arm/include/asm/arch-rockchip/grf_rk3368.h | 617 +++++++++++++-----------
>>>>>>  drivers/pinctrl/rockchip/pinctrl_rk3368.c       |  17 +-
>>>>>>  2 files changed, 334 insertions(+), 300 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..a97dc1e 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,
>>>>>> -	GPIO0C6_GPIO            = 0,
>>>>>> -	GPIO0C6_LCDC_D18,
>>>>>> -	GPIO0C6_TRACE_D8,
>>>>>> -	GPIO0C6_UART1_CTSN,
>>>>>> -
>>>>>> -	GPIO0C5_SHIFT           = 10,
>>>>>> -	GPIO0C5_MASK            = 3 << GPIO0C5_SHIFT,
>>>>>> -	GPIO0C5_GPIO            = 0,
>>>>>> -	GPIO0C5_LCDC_D17,
>>>>>> -	GPIO0C5_TRACE_D7,
>>>>>> -	GPIO0C5_UART1_SOUT,
>>>>>> -
>>>>>> -	GPIO0C4_SHIFT           = 8,
>>>>>> -	GPIO0C4_MASK            = 3 << GPIO0C4_SHIFT,
>>>>>> -	GPIO0C4_GPIO            = 0,
>>>>>> -	GPIO0C4_LCDC_D16,
>>>>>> -	GPIO0C4_TRACE_D6,
>>>>>> -	GPIO0C4_UART1_SIN,
>>>>>> -
>>>>>> -	GPIO0C3_SHIFT           = 6,
>>>>>> -	GPIO0C3_MASK            = 3 << GPIO0C3_SHIFT,
>>>>>> -	GPIO0C3_GPIO            = 0,
>>>>>> -	GPIO0C3_LCDC_D15,
>>>>>> -	GPIO0C3_TRACE_D5,
>>>>>> -	GPIO0C3_MCU_JTAG_TDO,
>>>>>> -
>>>>>> -	GPIO0C2_SHIFT           = 4,
>>>>>> -	GPIO0C2_MASK            = 3 << GPIO0C2_SHIFT,
>>>>>> -	GPIO0C2_GPIO            = 0,
>>>>>> -	GPIO0C2_LCDC_D14,
>>>>>> -	GPIO0C2_TRACE_D4,
>>>>>> -	GPIO0C2_MCU_JTAG_TDI,
>>>>>> -
>>>>>> -	GPIO0C1_SHIFT           = 2,
>>>>>> -	GPIO0C1_MASK            = 3 << GPIO0C1_SHIFT,
>>>>>> -	GPIO0C1_GPIO            = 0,
>>>>>> -	GPIO0C1_LCDC_D13,
>>>>>> -	GPIO0C1_TRACE_D3,
>>>>>> -	GPIO0C1_MCU_JTAG_TRTSN,
>>>>>> -
>>>>>> -	GPIO0C0_SHIFT           = 0,
>>>>>> -	GPIO0C0_MASK            = 3 << GPIO0C0_SHIFT,
>>>>>> -	GPIO0C0_GPIO            = 0,
>>>>>> -	GPIO0C0_LCDC_D12,
>>>>>> -	GPIO0C0_TRACE_D2,
>>>>>> -	GPIO0C0_MCU_JTAG_TDO,
>>>>>> +	RK3368_GPIO0C7_MASK	       = GENMASK(15, 14),
>>>>>> +	RK3368_GPIO0C7_GPIO	       = 0,
>>>>>> +	RK3368_GPIO0C7_LCDC_D19        = (1 << 14),
>>>>>> +	RK3368_GPIO0C7_TRACE_D9        = (2 << 14),
>>>>>> +	RK3368_GPIO0C7_UART1_RTSN      = (3 << 14),
>>>>>> +
>>>>>> +	RK3368_GPIO0C6_MASK            = GENMASK(13, 12),
>>>>>> +	RK3368_GPIO0C6_GPIO            = 0,
>>>>>> +	RK3368_GPIO0C6_LCDC_D18        = (1 << 12),
>>>>>> +	RK3368_GPIO0C6_TRACE_D8        = (2 << 12),
>>>>>> +	RK3368_GPIO0C6_UART1_CTSN      = (3 << 12),
>>>>>> +
>>>>>> +	RK3368_GPIO0C5_MASK            = GENMASK(11, 10),
>>>>>> +	RK3368_GPIO0C5_GPIO            = 0,
>>>>>> +	RK3368_GPIO0C5_LCDC_D17        = (1 << 10),
>>>>>> +	RK3368_GPIO0C5_TRACE_D7        = (2 << 10),
>>>>>> +	RK3368_GPIO0C5_UART1_SOUT      = (3 << 10),
>>>>>> +
>>>>>> +	RK3368_GPIO0C4_MASK            = GENMASK(9, 8),
>>>>>> +	RK3368_GPIO0C4_GPIO            = 0,
>>>>>> +	RK3368_GPIO0C4_LCDC_D16        = (1 << 8),
>>>>>> +	RK3368_GPIO0C4_TRACE_D6        = (2 << 8),
>>>>>> +	RK3368_GPIO0C4_UART1_SIN       = (3 << 8),
>>>>>> +
>>>>>> +	RK3368_GPIO0C3_MASK            = GENMASK(7, 6),
>>>>>> +	RK3368_GPIO0C3_GPIO            = 0,
>>>>>> +	RK3368_GPIO0C3_LCDC_D15        = (1 << 6),
>>>>>> +	RK3368_GPIO0C3_TRACE_D5        = (2 << 6),
>>>>>> +	RK3368_GPIO0C3_MCU_JTAG_TDO    = (3 << 6),
>>>>>> +
>>>>>> +	RK3368_GPIO0C2_MASK            = GENMASK(5, 4),
>>>>>> +	RK3368_GPIO0C2_GPIO            = 0,
>>>>>> +	RK3368_GPIO0C2_LCDC_D14        = (1 << 4),
>>>>>> +	RK3368_GPIO0C2_TRACE_D4        = (2 << 4),
>>>>>> +	RK3368_GPIO0C2_MCU_JTAG_TDI    = (3 << 4),
>>>>>> +
>>>>>> +	RK3368_GPIO0C1_MASK            = GENMASK(3, 2),
>>>>>> +	RK3368_GPIO0C1_GPIO            = 0,
>>>>>> +	RK3368_GPIO0C1_LCDC_D13        = (1 << 2),
>>>>>> +	RK3368_GPIO0C1_TRACE_D3        = (2 << 2),
>>>>>> +	RK3368_GPIO0C1_MCU_JTAG_TRTSN  = (3 << 2),
>>>>>> +
>>>>>> +	RK3368_GPIO0C0_MASK            = GENMASK(1, 0),
>>>>>> +	RK3368_GPIO0C0_GPIO            = 0,
>>>>>> +	RK3368_GPIO0C0_LCDC_D12        = (1 << 0),
>>>>>> +	RK3368_GPIO0C0_TRACE_D2        = (2 << 0),
>>>>>> +	RK3368_GPIO0C0_MCU_JTAG_TDO    = (3 << 0),
>>>>>>  };
>>>>>>    /*GRF_GPIO0D_IOMUX*/
>>>>>>  enum {
>>>>>> -	GPIO0D7_SHIFT           = 14,
>>>>>> -	GPIO0D7_MASK            = 3 << GPIO0D7_SHIFT,
>>>>>> -	GPIO0D7_GPIO            = 0,
>>>>>> -	GPIO0D7_LCDC_DCLK,
>>>>>> -	GPIO0D7_TRACE_CTL,
>>>>>> -	GPIO0D7_PMU_DEBUG5,
>>>>>> -
>>>>>> -	GPIO0D6_SHIFT           = 12,
>>>>>> -	GPIO0D6_MASK            = 3 << GPIO0D6_SHIFT,
>>>>>> -	GPIO0D6_GPIO            = 0,
>>>>>> -	GPIO0D6_LCDC_DEN,
>>>>>> -	GPIO0D6_TRACE_CLK,
>>>>>> -	GPIO0D6_PMU_DEBUG4,
>>>>>> -
>>>>>> -	GPIO0D5_SHIFT           = 10,
>>>>>> -	GPIO0D5_MASK            = 3 << GPIO0D5_SHIFT,
>>>>>> -	GPIO0D5_GPIO            = 0,
>>>>>> -	GPIO0D5_LCDC_VSYNC,
>>>>>> -	GPIO0D5_TRACE_D15,
>>>>>> -	GPIO0D5_PMU_DEBUG3,
>>>>>> -
>>>>>> -	GPIO0D4_SHIFT           = 8,
>>>>>> -	GPIO0D4_MASK            = 3 << GPIO0D4_SHIFT,
>>>>>> -	GPIO0D4_GPIO            = 0,
>>>>>> -	GPIO0D4_LCDC_HSYNC,
>>>>>> -	GPIO0D4_TRACE_D14,
>>>>>> -	GPIO0D4_PMU_DEBUG2,
>>>>>> -
>>>>>> -	GPIO0D3_SHIFT           = 6,
>>>>>> -	GPIO0D3_MASK            = 3 << GPIO0D3_SHIFT,
>>>>>> -	GPIO0D3_GPIO            = 0,
>>>>>> -	GPIO0D3_LCDC_D23,
>>>>>> -	GPIO0D3_TRACE_D13,
>>>>>> -	GPIO0D3_UART4_SIN,
>>>>>> -
>>>>>> -	GPIO0D2_SHIFT           = 4,
>>>>>> -	GPIO0D2_MASK            = 3 << GPIO0D2_SHIFT,
>>>>>> -	GPIO0D2_GPIO            = 0,
>>>>>> -	GPIO0D2_LCDC_D22,
>>>>>> -	GPIO0D2_TRACE_D12,
>>>>>> -	GPIO0D2_UART4_SOUT,
>>>>>> -
>>>>>> -	GPIO0D1_SHIFT           = 2,
>>>>>> -	GPIO0D1_MASK            = 3 << GPIO0D1_SHIFT,
>>>>>> -	GPIO0D1_GPIO            = 0,
>>>>>> -	GPIO0D1_LCDC_D21,
>>>>>> -	GPIO0D1_TRACE_D11,
>>>>>> -	GPIO0D1_UART4_RTSN,
>>>>>> -
>>>>>> -	GPIO0D0_SHIFT           = 0,
>>>>>> -	GPIO0D0_MASK            = 3 << GPIO0D0_SHIFT,
>>>>>> -	GPIO0D0_GPIO            = 0,
>>>>>> -	GPIO0D0_LCDC_D20,
>>>>>> -	GPIO0D0_TRACE_D10,
>>>>>> -	GPIO0D0_UART4_CTSN,
>>>>>> +	RK3368_GPIO0D7_MASK            = GENMASK(15, 14),
>>>>>> +	RK3368_GPIO0D7_GPIO            = 0,
>>>>>> +	RK3368_GPIO0D7_LCDC_DCLK       = (1 << 14),
>>>>>> +	RK3368_GPIO0D7_TRACE_CTL       = (2 << 14),
>>>>>> +	RK3368_GPIO0D7_PMU_DEBUG5      = (3 << 14),
>>>>>> +
>>>>>> +	RK3368_GPIO0D6_MASK            = GENMASK(13, 12),
>>>>>> +	RK3368_GPIO0D6_GPIO            = 0,
>>>>>> +	RK3368_GPIO0D6_LCDC_DEN        = (1 << 12),
>>>>>> +	RK3368_GPIO0D6_TRACE_CLK       = (2 << 12),
>>>>>> +	RK3368_GPIO0D6_PMU_DEBUG4      = (3 << 12),
>>>>>> +
>>>>>> +	RK3368_GPIO0D5_MASK            = GENMASK(11, 10),
>>>>>> +	RK3368_GPIO0D5_GPIO            = 0,
>>>>>> +	GPIO0D5_LCDC_VSYNC             = (1 << 10),
>>>>>> +	RK3368_GPIO0D5_TRACE_D15       = (2 << 10),
>>>>>> +	RK3368_GPIO0D5_PMU_DEBUG3      = (3 << 10),
>>>>>> +
>>>>>> +	RK3368_GPIO0D4_MASK            = GENMASK(9, 8),
>>>>>> +	RK3368_GPIO0D4_GPIO            = 0,
>>>>>> +	RK3368_GPIO0D4_LCDC_HSYNC      = (1 << 8),
>>>>>> +	RK3368_GPIO0D4_TRACE_D14       = (2 << 8),
>>>>>> +	RK3368_GPIO0D4_PMU_DEBUG2      = (3 << 8),
>>>>>> +
>>>>>> +	RK3368_GPIO0D3_MASK            = GENMASK(7, 6),
>>>>>> +	RK3368_GPIO0D3_GPIO            = 0,
>>>>>> +	RK3368_GPIO0D3_LCDC_D23        = (1 << 6),
>>>>>> +	RK3368_GPIO0D3_TRACE_D13       = (2 << 6),
>>>>>> +	RK3368_GPIO0D3_UART4_SIN       = (3 << 6),
>>>>>> +
>>>>>> +	RK3368_GPIO0D2_MASK            = GENMASK(5, 4),
>>>>>> +	RK3368_GPIO0D2_GPIO            = 0,
>>>>>> +	RK3368_GPIO0D2_LCDC_D22        = (1 << 4),
>>>>>> +	RK3368_GPIO0D2_TRACE_D12       = (2 << 4),
>>>>>> +	RK3368_GPIO0D2_UART4_SOUT      = (3 << 4),
>>>>>> +
>>>>>> +	RK3368_GPIO0D1_MASK            = GENMASK(3, 2),
>>>>>> +	RK3368_GPIO0D1_GPIO            = 0,
>>>>>> +	RK3368_GPIO0D1_LCDC_D21        = (1 << 2),
>>>>>> +	RK3368_GPIO0D1_TRACE_D11       = (2 << 2),
>>>>>> +	RK3368_GPIO0D1_UART4_RTSN      = (3 << 2),
>>>>>> +
>>>>>> +	RK3368_GPIO0D0_MASK            = GENMASK(1, 0),
>>>>>> +	RK3368_GPIO0D0_GPIO            = 0,
>>>>>> +	RK3368_GPIO0D0_LCDC_D20        = (1 << 0),
>>>>>> +	RK3368_GPIO0D0_TRACE_D10       = (2 << 0),
>>>>>> +	RK3368_GPIO0D0_UART4_CTSN      = (3 << 0),
>>>>>>  };
>>>>>>    /*GRF_GPIO2A_IOMUX*/
>>>>>>  enum {
>>>>>> -	GPIO2A7_SHIFT           = 14,
>>>>>> -	GPIO2A7_MASK            = 3 << GPIO2A7_SHIFT,
>>>>>> -	GPIO2A7_GPIO            = 0,
>>>>>> -	GPIO2A7_SDMMC0_D2,
>>>>>> -	GPIO2A7_JTAG_TCK,
>>>>>> -
>>>>>> -	GPIO2A6_SHIFT           = 12,
>>>>>> -	GPIO2A6_MASK            = 3 << GPIO2A6_SHIFT,
>>>>>> -	GPIO2A6_GPIO            = 0,
>>>>>> -	GPIO2A6_SDMMC0_D1,
>>>>>> -	GPIO2A6_UART2_SIN,
>>>>>> -
>>>>>> -	GPIO2A5_SHIFT           = 10,
>>>>>> -	GPIO2A5_MASK            = 3 << GPIO2A5_SHIFT,
>>>>>> -	GPIO2A5_GPIO            = 0,
>>>>>> -	GPIO2A5_SDMMC0_D0,
>>>>>> -	GPIO2A5_UART2_SOUT,
>>>>>> -
>>>>>> -	GPIO2A4_SHIFT           = 8,
>>>>>> -	GPIO2A4_MASK            = 3 << GPIO2A4_SHIFT,
>>>>>> -	GPIO2A4_GPIO            = 0,
>>>>>> -	GPIO2A4_FLASH_DQS,
>>>>>> -	GPIO2A4_EMMC_CLKO,
>>>>>> -
>>>>>> -	GPIO2A3_SHIFT           = 6,
>>>>>> -	GPIO2A3_MASK            = 3 << GPIO2A3_SHIFT,
>>>>>> -	GPIO2A3_GPIO            = 0,
>>>>>> -	GPIO2A3_FLASH_CSN3,
>>>>>> -	GPIO2A3_EMMC_RSTNO,
>>>>>> -
>>>>>> -	GPIO2A2_SHIFT           = 4,
>>>>>> -	GPIO2A2_MASK            = 3 << GPIO2A2_SHIFT,
>>>>>> -	GPIO2A2_GPIO           = 0,
>>>>>> -	GPIO2A2_FLASH_CSN2,
>>>>>> -
>>>>>> -	GPIO2A1_SHIFT           = 2,
>>>>>> -	GPIO2A1_MASK            = 3 << GPIO2A1_SHIFT,
>>>>>> -	GPIO2A1_GPIO            = 0,
>>>>>> -	GPIO2A1_FLASH_CSN1,
>>>>>> -
>>>>>> -	GPIO2A0_SHIFT           = 0,
>>>>>> -	GPIO2A0_MASK            = 3 << GPIO2A0_SHIFT,
>>>>>> -	GPIO2A0_GPIO            = 0,
>>>>>> -	GPIO2A0_FLASH_CSN0,
>>>>>> +	RK3368_GPIO2A7_MASK            = GENMASK(15, 14),
>>>>>> +	RK3368_GPIO2A7_GPIO            = 0,
>>>>>> +	RK3368_GPIO2A7_SDMMC0_D2       = (1 << 14),
>>>>>> +	RK3368_GPIO2A7_JTAG_TCK        = (2 << 14),
>>>>>> +
>>>>>> +	RK3368_GPIO2A6_MASK            = GENMASK(13, 12),
>>>>>> +	RK3368_GPIO2A6_GPIO            = 0,
>>>>>> +	RK3368_GPIO2A6_SDMMC0_D1       = (1 << 12),
>>>>>> +	RK3368_GPIO2A6_UART2_SIN       = (2 << 12),
>>>>>> +
>>>>>> +	RK3368_GPIO2A5_MASK            = GENMASK(11, 10),
>>>>>> +	RK3368_GPIO2A5_GPIO            = 0,
>>>>>> +	RK3368_GPIO2A5_SDMMC0_D0       = (1 << 10),
>>>>>> +	RK3368_GPIO2A5_UART2_SOUT      = (2 << 10),
>>>>>> +
>>>>>> +	RK3368_GPIO2A4_MASK            = GENMASK(9, 8),
>>>>>> +	RK3368_GPIO2A4_RK3368_GPIO     = 0,
>>>>>> +	RK3368_GPIO2A4_FLASH_DQS       = (1 << 8),
>>>>>> +	RK3368_GPIO2A4_EMMC_CLKOUT     = (2 << 8),
>>>>>> +
>>>>>> +	RK3368_GPIO2A3_MASK            = GENMASK(7, 6),
>>>>>> +	RK3368_GPIO2A3_GPIO            = 0,
>>>>>> +	RK3368_GPIO2A3_FLASH_CSN3      = (1 << 6),
>>>>>> +	RK3368_GPIO2A3_EMMC_RSTNOUT    = (2 << 6),
>>>>>> +
>>>>>> +	RK3368_GPIO2A2_MASK            = GENMASK(5, 4),
>>>>>> +	RK3368_GPIO2A2_GPIO            = 0,
>>>>>> +	RK3368_GPIO2A2_FLASH_CSN2      = (1 << 4),
>>>>>> +
>>>>>> +	RK3368_GPIO2A1_MASK            = GENMASK(3, 2),
>>>>>> +	RK3368_GPIO2A1_GPIO            = 0,
>>>>>> +	RK3368_GPIO2A1_FLASH_CSN1      = (1 << 2),
>>>>>> +
>>>>>> +	RK3368_GPIO2A0_MASK            = GENMASK(1, 0),
>>>>>> +	RK3368_GPIO2A0_GPIO            = 0,
>>>>>> +	RK3368_GPIO2A0_FLASH_CSN0      = (1 << 0),
>>>>>>  };
>>>>>>  -/*GRF_GPIO2D_IOMUX*/
>>>>>> +/*GRF_RK3368_GPIO2D_IOMUX*/
>>>>>>  enum {
>>>>>> -	GPIO2D7_SHIFT           = 14,
>>>>>> -	GPIO2D7_MASK            = 3 << GPIO2D7_SHIFT,
>>>>>> -	GPIO2D7_GPIO            = 0,
>>>>>> -	GPIO2D7_SDIO0_D3,
>>>>>> -
>>>>>> -	GPIO2D6_SHIFT           = 12,
>>>>>> -	GPIO2D6_MASK            = 3 << GPIO2D6_SHIFT,
>>>>>> -	GPIO2D6_GPIO            = 0,
>>>>>> -	GPIO2D6_SDIO0_D2,
>>>>>> -
>>>>>> -	GPIO2D5_SHIFT           = 10,
>>>>>> -	GPIO2D5_MASK            = 3 << GPIO2D5_SHIFT,
>>>>>> -	GPIO2D5_GPIO            = 0,
>>>>>> -	GPIO2D5_SDIO0_D1,
>>>>>> -
>>>>>> -	GPIO2D4_SHIFT           = 8,
>>>>>> -	GPIO2D4_MASK            = 3 << GPIO2D4_SHIFT,
>>>>>> -	GPIO2D4_GPIO            = 0,
>>>>>> -	GPIO2D4_SDIO0_D0,
>>>>>> -
>>>>>> -	GPIO2D3_SHIFT           = 6,
>>>>>> -	GPIO2D3_MASK            = 3 << GPIO2D3_SHIFT,
>>>>>> -	GPIO2D3_GPIO            = 0,
>>>>>> -	GPIO2D3_UART0_RTS0,
>>>>>> -
>>>>>> -	GPIO2D2_SHIFT           = 4,
>>>>>> -	GPIO2D2_MASK            = 3 << GPIO2D2_SHIFT,
>>>>>> -	GPIO2D2_GPIO            = 0,
>>>>>> -	GPIO2D2_UART0_CTS0,
>>>>>> -
>>>>>> -	GPIO2D1_SHIFT           = 2,
>>>>>> -	GPIO2D1_MASK            = 3 << GPIO2D1_SHIFT,
>>>>>> -	GPIO2D1_GPIO            = 0,
>>>>>> -	GPIO2D1_UART0_SOUT,
>>>>>> -
>>>>>> -	GPIO2D0_SHIFT           = 0,
>>>>>> -	GPIO2D0_MASK            = 3 << GPIO2D0_SHIFT,
>>>>>> -	GPIO2D0_GPIO            = 0,
>>>>>> -	GPIO2D0_UART0_SIN,
>>>>>> +	RK3368_GPIO2D7_MASK            = GENMASK(15, 14),
>>>>>> +	RK3368_GPIO2D7_GPIO            = 0,
>>>>>> +	RK3368_GPIO2D7_SDIO0_D3        = (1 << 14),
>>>>>> +
>>>>>> +	RK3368_GPIO2D6_MASK            = GENMASK(13, 12),
>>>>>> +	RK3368_GPIO2D6_GPIO            = 0,
>>>>>> +	RK3368_GPIO2D6_SDIO0_D2        = (1 << 12),
>>>>>> +
>>>>>> +	RK3368_GPIO2D5_MASK            = GENMASK(11, 10),
>>>>>> +	RK3368_GPIO2D5_GPIO            = 0,
>>>>>> +	RK3368_GPIO2D5_SDIO0_D1        = (1 << 10),
>>>>>> +
>>>>>> +	RK3368_GPIO2D4_MASK            = GENMASK(9, 8),
>>>>>> +	RK3368_GPIO2D4_GPIO            = 0,
>>>>>> +	RK3368_GPIO2D4_SDIO0_D0        = (1 << 8),
>>>>>> +
>>>>>> +	RK3368_GPIO2D3_MASK            = GENMASK(7, 6),
>>>>>> +	RK3368_GPIO2D3_GPIO            = 0,
>>>>>> +	RK3368_GPIO2D3_UART0_RTS0      = (1 << 6),
>>>>>> +
>>>>>> +	RK3368_GPIO2D2_MASK            = GENMASK(5, 4),
>>>>>> +	RK3368_GPIO2D2_GPIO            = 0,
>>>>>> +	RK3368_GPIO2D2_UART0_CTS0      = (1 << 4),
>>>>>> +
>>>>>> +	RK3368_GPIO2D1_MASK            = GENMASK(3, 2),
>>>>>> +	RK3368_GPIO2D1_GPIO            = 0,
>>>>>> +	RK3368_GPIO2D1_UART0_SOUT      = (1 << 2),
>>>>>> +
>>>>>> +	RK3368_GPIO2D0_MASK            = GENMASK(1, 0),
>>>>>> +	RK3368_GPIO2D0_GPIO            = 0,
>>>>>> +	RK3368_GPIO2D0_UART0_SIN       = (1 << 0),
>>>>>> +};
>>>>>> +
>>>>>> +/* GRF_GPIO1C_IOMUX */
>>>>>> +enum {
>>>>>> +	RK3368_GPIO1C7_MASK            = GENMASK(15, 14),
>>>>>> +	RK3368_GPIO1C7_GPIO            = 0,
>>>>>> +	RK3368_GPIO1C7_EMMC_DATA5      = (2 << 14),
>>>>>> +
>>>>>> +	RK3368_GPIO1C6_MASK            = GENMASK(13, 12),
>>>>>> +	RK3368_GPIO1C6_GPIO            = 0,
>>>>>> +	RK3368_GPIO1C6_EMMC_DATA4      = (2 << 12),
>>>>>> +
>>>>>> +	RK3368_GPIO1C5_MASK            = GENMASK(11, 10),
>>>>>> +	RK3368_GPIO1C5_GPIO            = 0,
>>>>>> +	RK3368_GPIO1C5_EMMC_DATA3      = (2 << 10),
>>>>>> +
>>>>>> +	RK3368_GPIO1C4_MASK            = GENMASK(9, 8),
>>>>>> +	RK3368_GPIO1C4_GPIO            = 0,
>>>>>> +	RK3368_GPIO1C4_EMMC_DATA2      = (2 << 8),
>>>>>> +
>>>>>> +	RK3368_GPIO1C3_MASK            = GENMASK(7, 6),
>>>>>> +	RK3368_GPIO1C3_GPIO            = 0,
>>>>>> +	RK3368_GPIO1C3_EMMC_DATA1      = (2 << 6),
>>>>>> +
>>>>>> +	RK3368_GPIO1C2_MASK            = GENMASK(5, 4),
>>>>>> +	RK3368_GPIO1C2_GPIO            = 0,
>>>>>> +	RK3368_GPIO1C2_EMMC_DATA0      = (2 << 4),
>>>>>> +};
>>>>>> +
>>>>>> +/* GRF_GPIO1D_IOMUX*/
>>>>>> +enum {
>>>>>> +	RK3368_GPIO1D3_MASK            = GENMASK(7, 6),
>>>>>> +	RK3368_GPIO1D3_GPIO            = 0,
>>>>>> +	RK3368_GPIO1D3_EMMC_PWREN      = (2 << 6),
>>>>>> +
>>>>>> +	RK3368_GPIO1D2_MASK            = GENMASK(5, 4),
>>>>>> +	RK3368_GPIO1D2_GPIO            = 0,
>>>>>> +	RK3368_GPIO1D2_EMMC_CMD        = (2 << 4),
>>>>>> +
>>>>>> +	RK3368_GPIO1D1_MASK            = GENMASK(3, 2),
>>>>>> +	RK3368_GPIO1D1_GPIO            = 0,
>>>>>> +	RK3368_GPIO1D1_EMMC_DATA7      = (2 << 2),
>>>>>> +
>>>>>> +	RK3368_GPIO1D0_MASK            = GENMASK(1, 0),
>>>>>> +	RK3368_GPIO1D0_GPIO            = 0,
>>>>>> +	RK3368_GPIO1D0_EMMC_DATA6      = (2 << 0),
>>>>>> +};
>>>>>> +
>>>>>> +
>>>>>> +/*GRF_GPIO3B_IOMUX*/
>>>>>> +enum {
>>>>>> +	RK3368_GPIO3B7_MASK            = GENMASK(15, 14),
>>>>>> +	RK3368_GPIO3B7_GPIO            = 0,
>>>>>> +	RK3368_GPIO3B7_MAC_RXD0        = (1 << 14),
>>>>>> +
>>>>>> +	RK3368_GPIO3B6_MASK            = GENMASK(13, 12),
>>>>>> +	RK3368_GPIO3B6_GPIO            = 0,
>>>>>> +	RK3368_GPIO3B6_MAC_TXD3        = (1 << 12),
>>>>>> +
>>>>>> +	RK3368_GPIO3B5_MASK            = GENMASK(11, 10),
>>>>>> +	RK3368_GPIO3B5_GPIO            = 0,
>>>>>> +	RK3368_GPIO3B5_MAC_TXEN        = (1 << 10),
>>>>>> +
>>>>>> +	RK3368_GPIO3B4_MASK            = GENMASK(9, 8),
>>>>>> +	RK3368_GPIO3B4_GPIO            = 0,
>>>>>> +	RK3368_GPIO3B4_MAC_COL         = (1 << 8),
>>>>>> +
>>>>>> +	RK3368_GPIO3B3_MASK            = GENMASK(7, 6),
>>>>>> +	RK3368_GPIO3B3_GPIO            = 0,
>>>>>> +	RK3368_GPIO3B3_MAC_CRS         = (1 << 6),
>>>>>> +
>>>>>> +	RK3368_GPIO3B2_MASK            = GENMASK(5, 4),
>>>>>> +	RK3368_GPIO3B2_GPIO            = 0,
>>>>>> +	RK3368_GPIO3B2_MAC_TXD2        = (1 << 4),
>>>>>> +
>>>>>> +	RK3368_GPIO3B1_MASK            = GENMASK(3, 2),
>>>>>> +	RK3368_GPIO3B1_GPIO            = 0,
>>>>>> +	RK3368_GPIO3B1_MAC_TXD1        = (1 << 2),
>>>>>> +
>>>>>> +	RK3368_GPIO3B0_MASK            = GENMASK(1, 0),
>>>>>> +	RK3368_GPIO3B0_GPIO            = 0,
>>>>>> +	RK3368_GPIO3B0_MAC_TXD0        = (1 << 0),
>>>>>> +	RK3368_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_GPIO            = 0,
>>>>>> -	GPIO3C6_MAC_CLK,
>>>>>> -	GPIO3C6_ISP_SHUTTERTRIG,
>>>>>> -
>>>>>> -	GPIO3C5_SHIFT           = 10,
>>>>>> -	GPIO3C5_MASK            = 3 << GPIO3C5_SHIFT,
>>>>>> -	GPIO3C5_GPIO            = 0,
>>>>>> -	GPIO3C5_MAC_RXER,
>>>>>> -	GPIO3C5_ISP_PRELIGHTTRIG,
>>>>>> -
>>>>>> -	GPIO3C4_SHIFT           = 8,
>>>>>> -	GPIO3C4_MASK            = 3 << GPIO3C4_SHIFT,
>>>>>> -	GPIO3C4_GPIO            = 0,
>>>>>> -	GPIO3C4_MAC_RXDV,
>>>>>> -	GPIO3C4_ISP_FLASHTRIGOUT,
>>>>>> -
>>>>>> -	GPIO3C3_SHIFT           = 6,
>>>>>> -	GPIO3C3_MASK            = 3 << GPIO3C3_SHIFT,
>>>>>> -	GPIO3C3_GPIO            = 0,
>>>>>> -	GPIO3C3_MAC_RXDV,
>>>>>> -	GPIO3C3_EMMC_RSTNO,
>>>>>> -
>>>>>> -	GPIO3C2_SHIFT           = 4,
>>>>>> -	GPIO3C2_MASK            = 3 << GPIO3C2_SHIFT,
>>>>>> -	GPIO3C2_MAC_MDC            = 0,
>>>>>> -	GPIO3C2_ISP_SHUTTEREN,
>>>>>> -
>>>>>> -	GPIO3C1_SHIFT           = 2,
>>>>>> -	GPIO3C1_MASK            = 3 << GPIO3C1_SHIFT,
>>>>>> -	GPIO3C1_GPIO            = 0,
>>>>>> -	GPIO3C1_MAC_RXD2,
>>>>>> -	GPIO3C1_UART3_RTSN,
>>>>>> -
>>>>>> -	GPIO3C0_SHIFT           = 0,
>>>>>> -	GPIO3C0_MASK            = 3 << GPIO3C0_SHIFT,
>>>>>> -	GPIO3C0_GPIO            = 0,
>>>>>> -	GPIO3C0_MAC_RXD1,
>>>>>> -	GPIO3C0_UART3_CTSN,
>>>>>> -	GPIO3C0_GPS_RFCLK,
>>>>>> +	RK3368_GPIO3C6_MASK            = GENMASK(13, 12),
>>>>>> +	RK3368_GPIO3C6_GPIO            = 0,
>>>>>> +	RK3368_GPIO3C6_MAC_CLK         = (1 << 12),
>>>>>> +
>>>>>> +	RK3368_GPIO3C5_MASK            = GENMASK(11, 10),
>>>>>> +	RK3368_GPIO3C5_GPIO            = 0,
>>>>>> +	RK3368_GPIO3C5_MAC_RXEN        = (1 << 10),
>>>>>> +
>>>>>> +	RK3368_GPIO3C4_MASK            = GENMASK(9, 8),
>>>>>> +	RK3368_GPIO3C4_GPIO            = 0,
>>>>>> +	RK3368_GPIO3C4_MAC_RXDV        = (1 << 8),
>>>>>> +
>>>>>> +	RK3368_GPIO3C3_MASK            = GENMASK(7, 6),
>>>>>> +	RK3368_GPIO3C3_GPIO            = 0,
>>>>>> +	RK3368_GPIO3C3_MAC_MDC         = (1 << 6),
>>>>>> +
>>>>>> +	RK3368_GPIO3C2_MASK            = GENMASK(5, 4),
>>>>>> +	RK3368_GPIO3C2_GPIO            = 0,
>>>>>> +	RK3368_GPIO3C2_MAC_RXD3        = (1 << 4),
>>>>>> +
>>>>>> +	RK3368_GPIO3C1_MASK            = GENMASK(3, 2),
>>>>>> +	RK3368_GPIO3C1_GPIO            = 0,
>>>>>> +	RK3368_GPIO3C1_MAC_RXD2        = (1 << 2),
>>>>>> +
>>>>>> +	RK3368_GPIO3C0_MASK            = GENMASK(1, 0),
>>>>>> +	RK3368_GPIO3C0_GPIO            = 0,
>>>>>> +	RK3368_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_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,
>>>>>> -	GPIO3D1_GPIO            = 0,
>>>>>> -	GPIO3D1_MAC_RXCLKIN,
>>>>>> -	GPIO3D1_I2C4_SCL,
>>>>>> -
>>>>>> -	GPIO3D0_SHIFT           = 0,
>>>>>> -	GPIO3D0_MASK            = 3 << GPIO3D0_SHIFT,
>>>>>> -	GPIO3D0_GPIO            = 0,
>>>>>> -	GPIO3D0_MAC_MDIO,
>>>>>> -	GPIO3D0_I2C4_SDA,
>>>>>> +	RK3368_GPIO3D4_MASK            = GENMASK(9, 8),
>>>>>> +	RK3368_GPIO3D4_GPIO            = 0,
>>>>>> +	RK3368_GPIO3D4_MAC_TXCLK       = (1 << 8),
>>>>>> +
>>>>>> +	RK3368_GPIO3D1_MASK            = GENMASK(3, 2),
>>>>>> +	RK3368_GPIO3D1_GPIO            = 0,
>>>>>> +	RK3368_GPIO3D1_MAC_RXCLK       = (1 << 2),
>>>>>> +
>>>>>> +	RK3368_GPIO3D0_MASK            = GENMASK(1, 0),
>>>>>> +	RK3368_GPIO3D0_GPIO            = 0,
>>>>>> +	RK3368_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*/
>>>>>> @@ -440,4 +445,34 @@ enum {
>>>>>>  	MCU_CODE_BASE_BIT31_BIT28_SHIFT		= 0,
>>>>>>  	MCU_CODE_BASE_BIT31_BIT28_MASK		= GENMASK(3, 0),
>>>>>>  };
>>>>>> +
>>>>>> +/*GRF_SOC_CON15*/
>>>>>> +enum {
>>>>>> +	RK3368_GMAC_PHY_INTF_SEL_RGMII = 1 << 9,
>>>>>> +	RK3368_GMAC_PHY_INTF_SEL_MASK = GENMASK(11, 9),
>>>>>> +	RK3368_RMII_MODE_MASK  = BIT(6),
>>>>>> +	RK3368_RMII_MODE       = BIT(6),
>>>>>> +	RK3368_GMAC_CLK_SEL_2_5M = 2 << 4,
>>>>>> +	RK3368_GMAC_CLK_SEL_25M = 3 << 4,
>>>>>> +	RK3368_GMAC_CLK_SEL_125M = 0 << 4,
>>>>>> +	RK3368_GMAC_CLK_SEL_MASK = GENMASK(5, 4),
>>>>>> +};
>>>>>> +
>>>>>> +/* GRF_SOC_CON16 */
>>>>>> +enum {
>>>>>> +	RK3368_RXCLK_DLY_ENA_GMAC_MASK = BIT(15),
>>>>>> +	RK3368_RXCLK_DLY_ENA_GMAC_DISABLE = 0,
>>>>>> +	RK3368_RXCLK_DLY_ENA_GMAC_ENABLE = BIT(15),
>>>>>> +
>>>>>> +	RK3368_TXCLK_DLY_ENA_GMAC_MASK = BIT(7),
>>>>>> +	RK3368_TXCLK_DLY_ENA_GMAC_DISABLE = 0,
>>>>>> +	RK3368_TXCLK_DLY_ENA_GMAC_ENABLE = BIT(7),
>>>>>> +
>>>>>> +	RK3368_CLK_RX_DL_CFG_GMAC_SHIFT = 8,
>>>>>> +	RK3368_CLK_RX_DL_CFG_GMAC_MASK = GENMASK(14, 8),
>>>>>> +
>>>>>> +	RK3368_CLK_TX_DL_CFG_GMAC_SHIFT = 0,
>>>>>> +	RK3368_CLK_TX_DL_CFG_GMAC_MASK = GENMASK(6, 0),
>>>>>> +};
>>>>>> +
>>>>>>  #endif
>>>>>> diff --git a/drivers/pinctrl/rockchip/pinctrl_rk3368.c b/drivers/pinctrl/rockchip/pinctrl_rk3368.c
>>>>>> index bdf0758..b77c5ab 100644
>>>>>> --- a/drivers/pinctrl/rockchip/pinctrl_rk3368.c
>>>>>> +++ b/drivers/pinctrl/rockchip/pinctrl_rk3368.c
>>>>>> @@ -30,9 +30,9 @@ static void pinctrl_rk3368_uart_config(struct rk3368_pinctrl_priv *priv,
>>>>>>  	switch (uart_id) {
>>>>>>  	case PERIPH_ID_UART2:
>>>>>>  		rk_clrsetreg(&grf->gpio2a_iomux,
>>>>>> -			     GPIO2A6_MASK | GPIO2A5_MASK,
>>>>>> -			     GPIO2A6_UART2_SIN << GPIO2A6_SHIFT |
>>>>>> -			     GPIO2A5_UART2_SOUT << GPIO2A5_SHIFT);
>>>>>> +			     RK3368_GPIO2A6_MASK | RK3368_GPIO2A5_MASK,
>>>>>> +			     RK3368_GPIO2A6_UART2_SIN |
>>>>>> +			     RK3368_GPIO2A5_UART2_SOUT);
>>>>>>  		break;
>>>>>>  	case PERIPH_ID_UART0:
>>>>>>  		break;
>>>>>> @@ -42,12 +42,11 @@ static void pinctrl_rk3368_uart_config(struct rk3368_pinctrl_priv *priv,
>>>>>>  		break;
>>>>>>  	case PERIPH_ID_UART4:
>>>>>>  		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);
>>>>>> +			     RK3368_GPIO0D0_MASK | RK3368_GPIO0D1_MASK |
>>>>>> +			     RK3368_GPIO0D2_MASK | RK3368_GPIO0D3_MASK,
>>>>>> +			     RK3368_GPIO0D0_GPIO | RK3368_GPIO0D1_GPIO |
>>>>>> +			     RK3368_GPIO0D2_UART4_SOUT |
>>>>>> +			     RK3368_GPIO0D3_UART4_SIN);
>>>>>>  		break;
>>>>>>  	default:
>>>>>>  		debug("uart id = %d iomux error!\n", uart_id);

Hi Philipp,

On 24 July 2017 at 07:10, Dr. Philipp Tomsich
<philipp.tomsich@theobroma-systems.com> wrote:
>
>
> > On 24 Jul 2017, at 14:45, Kever Yang <kever.yang@rock-chips.com> wrote:
> >
> > Hi Philipp,
> >
> >
> > On 07/24/2017 05:11 PM, Dr. Philipp Tomsich wrote:
> >>> On 24 Jul 2017, at 10:34, Andy Yan <andy.yan@rock-chips.com> wrote:
> >>>
> >>> Hi Philipp:
> >>>
> >>>
> >>> On 2017年07月21日 16:34, Dr. Philipp Tomsich wrote:
> >>>>> On 21 Jul 2017, at 05:50, Andy Yan <andy.yan@rock-chips.com> wrote:
> >>>>>
> >>>>> Hi Philipp:
> >>>>>
> >>>>>
> >>>>> On 2017年07月19日 04:36, Philipp Tomsich 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 rename, we do this now
> >>>>>> before having more code use this.
> >>>>>>
> >>>>>> As some of the downstream drivers (e.g. the Designware GMAC wrapper)
> >>>>>> may need to include the grf-header of multiple devices, we rename the
> >>>>>> various defines for the RK3368 by prefixing them with the device name.
> >>>>>> This avoids future trouble during driver integration.
> >>>>>    Is that really necessary to add such a prefix for all the register definition, just to fit  the GMAC dirver?
> >>>>> Maybe the gmac driver can define the platform specific macro in its own code like dwmac-rk in the kernel. Then we can keep the register header file a little tidy.
> >>>> The problem is not the GMAC driver (although it was the motivation for doing
> >>>> this change), but rather how easy it is to pick up the wrong grf-file and refer
> >>>> to the wrong definitions/values… the conflicts are not through the GMAC-specifc
> >>>> defines but rather through things like the IOMUX masks or (before I moved
> >>>> this to the DDR-driver) things like the defines to enable DRAM in GRF.
> >>>>
> >>>> Until we find a way to consistently structure things in such a way that
> >>>>    (a) common constructs can be shared (e.g. the masks in the IOMUX)
> >>>>    (b) specific definitions (e.g. the pin-out for a specific device) is not visible outside of its driver
> >>>> I prefer the names to explicitly refer to each device.
> >>>>   That said, the longer-term plan (especially with a larger number of devices
> >>>> being supported) needs to include a consistent rework of how (and where)
> >>>> we manage all these definitions from the grf-header files.
> >>> It's true that many devices use GRF, we also face this condition in the kernel land.And all the drivers in kernel handle this device specific definition in the driver itself(by of_device_id->data or MACRO). And there is also no need to use the grf constructs, the common syscon api will provide  you the grf base address. I think this keeps things simple, we don't need to rely on too much of the grf or cru header, maybe some day we can remove the header file, just as clean as the kernel.
> >> The problem is not the base-address of GRF, but rather the need to pull in the
> >> register structure (so we can refer to individual registers by name instead of by
> >> hard-coded offset): as we pull this in today, the definitions for the various IOMUX
> >> entries will also be pulled in, which will cause duplicate definitions.
> >
> > I really don't like the idea of add the prefix, which end with longer MACRO but no other improve.
> > I think the GRF is used mostly for the IOMUX, in this case the better solution is
> > port the kernel rockchip-pinctrl driver in uboot, which not rely on the dtsi instead of header file.
> >
> > If we do this, we can remove most of the GRF header definition for IOMUX, I can ask David Wu
> > to do this, but maybe few weeks later.
>
> As indicated earlier: this is a temporary solution (although we have a local saying that
> "nothing is more permanent than a temporary solution”) and should remain limited to
> the RK3368 until we can clean this up across all boards.

Can we drop the prefix for now? I do worry about the precedent this
sets. If this is needed anywhere then it suggests to me that we have
not split up the drivers correctly. We should not be including two
different GRF headers into the same file. E.g. there should be
different pinctrl drivers for each SoC.

>
> Pulling this into the pinctrl drivers is an excellent choice.
> Please watch out for board_debug_uart_init(…), which also directly accesses the IOMUX
> constants and will need to be restructured together with pinctrl drivers.
>
> Thanks!
> —Philipp.
>
> > Thanks,
> > - Kever
> >>
> >>> On the other hand, add such a prefix of rk3368 breaks the consistency with other rockchip devices.
> >> I do agree that we need to clean this up across all the boards and drivers
> >> supported today.  As of today, I can not resolve this on a RK3368-only basis,
> >> as I’d need to clean up the RK3288 at the same time (and don’t want to do this
> >> in a RK3368-focused series that already has way too much going on).
> >>
> >> Let’s try to clean this up for the next (i.e. the one after this) merge window.
> >> Once we have bit-definitions out of the grf-header, I’ll remove the RK3368_
> >> prefixes...
> >>
> >>>>>> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> >>>>>> ---
> >>>>>>

Regards,
Simon

> On 24 Jul 2017, at 18:43, Simon Glass <sjg@chromium.org> wrote:
> 
> Hi Philipp,
> 
> On 24 July 2017 at 07:10, Dr. Philipp Tomsich
> <philipp.tomsich@theobroma-systems.com> wrote:
>> 
>> 
>>> On 24 Jul 2017, at 14:45, Kever Yang <kever.yang@rock-chips.com> wrote:
>>> 
>>> Hi Philipp,
>>> 
>>> 
>>> On 07/24/2017 05:11 PM, Dr. Philipp Tomsich wrote:
>>>>> On 24 Jul 2017, at 10:34, Andy Yan <andy.yan@rock-chips.com> wrote:
>>>>> 
>>>>> Hi Philipp:
>>>>> 
>>>>> 
>>>>> On 2017年07月21日 16:34, Dr. Philipp Tomsich wrote:
>>>>>>> On 21 Jul 2017, at 05:50, Andy Yan <andy.yan@rock-chips.com> wrote:
>>>>>>> 
>>>>>>> Hi Philipp:
>>>>>>> 
>>>>>>> 
>>>>>>> On 2017年07月19日 04:36, Philipp Tomsich 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 rename, we do this now
>>>>>>>> before having more code use this.
>>>>>>>> 
>>>>>>>> As some of the downstream drivers (e.g. the Designware GMAC wrapper)
>>>>>>>> may need to include the grf-header of multiple devices, we rename the
>>>>>>>> various defines for the RK3368 by prefixing them with the device name.
>>>>>>>> This avoids future trouble during driver integration.
>>>>>>>   Is that really necessary to add such a prefix for all the register definition, just to fit  the GMAC dirver?
>>>>>>> Maybe the gmac driver can define the platform specific macro in its own code like dwmac-rk in the kernel. Then we can keep the register header file a little tidy.
>>>>>> The problem is not the GMAC driver (although it was the motivation for doing
>>>>>> this change), but rather how easy it is to pick up the wrong grf-file and refer
>>>>>> to the wrong definitions/values… the conflicts are not through the GMAC-specifc
>>>>>> defines but rather through things like the IOMUX masks or (before I moved
>>>>>> this to the DDR-driver) things like the defines to enable DRAM in GRF.
>>>>>> 
>>>>>> Until we find a way to consistently structure things in such a way that
>>>>>>   (a) common constructs can be shared (e.g. the masks in the IOMUX)
>>>>>>   (b) specific definitions (e.g. the pin-out for a specific device) is not visible outside of its driver
>>>>>> I prefer the names to explicitly refer to each device.
>>>>>>  That said, the longer-term plan (especially with a larger number of devices
>>>>>> being supported) needs to include a consistent rework of how (and where)
>>>>>> we manage all these definitions from the grf-header files.
>>>>> It's true that many devices use GRF, we also face this condition in the kernel land.And all the drivers in kernel handle this device specific definition in the driver itself(by of_device_id->data or MACRO). And there is also no need to use the grf constructs, the common syscon api will provide  you the grf base address. I think this keeps things simple, we don't need to rely on too much of the grf or cru header, maybe some day we can remove the header file, just as clean as the kernel.
>>>> The problem is not the base-address of GRF, but rather the need to pull in the
>>>> register structure (so we can refer to individual registers by name instead of by
>>>> hard-coded offset): as we pull this in today, the definitions for the various IOMUX
>>>> entries will also be pulled in, which will cause duplicate definitions.
>>> 
>>> I really don't like the idea of add the prefix, which end with longer MACRO but no other improve.
>>> I think the GRF is used mostly for the IOMUX, in this case the better solution is
>>> port the kernel rockchip-pinctrl driver in uboot, which not rely on the dtsi instead of header file.
>>> 
>>> If we do this, we can remove most of the GRF header definition for IOMUX, I can ask David Wu
>>> to do this, but maybe few weeks later.
>> 
>> As indicated earlier: this is a temporary solution (although we have a local saying that
>> "nothing is more permanent than a temporary solution”) and should remain limited to
>> the RK3368 until we can clean this up across all boards.
> 
> Can we drop the prefix for now? I do worry about the precedent this
> sets. If this is needed anywhere then it suggests to me that we have
> not split up the drivers correctly. We should not be including two
> different GRF headers into the same file. E.g. there should be
> different pinctrl drivers for each SoC.

If we drop it, we can’t get RK3368-support merged into the GMAC driver for now.
Splitting the Rockchip-wrapper of the GMAC-driver doesn’t appear like a workable
solution either, as this would create a new file for each chip-variant in drivers/net.

I’ll just pull the RK3368 GMAC (driver) support for now and split it off this series.

>> Pulling this into the pinctrl drivers is an excellent choice.
>> Please watch out for board_debug_uart_init(…), which also directly accesses the IOMUX
>> constants and will need to be restructured together with pinctrl drivers.
>> 
>> Thanks!
>> —Philipp.
>> 
>>> Thanks,
>>> - Kever
>>>> 
>>>>> On the other hand, add such a prefix of rk3368 breaks the consistency with other rockchip devices.
>>>> I do agree that we need to clean this up across all the boards and drivers
>>>> supported today.  As of today, I can not resolve this on a RK3368-only basis,
>>>> as I’d need to clean up the RK3288 at the same time (and don’t want to do this
>>>> in a RK3368-focused series that already has way too much going on).
>>>> 
>>>> Let’s try to clean this up for the next (i.e. the one after this) merge window.
>>>> Once we have bit-definitions out of the grf-header, I’ll remove the RK3368_
>>>> prefixes...
>>>> 
>>>>>>>> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
>>>>>>>> ---
>>>>>>>> 
> 
> Regards,
> Simon

Hi Philipp,

On 24 July 2017 at 10:49, Dr. Philipp Tomsich
<philipp.tomsich@theobroma-systems.com> wrote:
>
>> On 24 Jul 2017, at 18:43, Simon Glass <sjg@chromium.org> wrote:
>>
>> Hi Philipp,
>>
>> On 24 July 2017 at 07:10, Dr. Philipp Tomsich
>> <philipp.tomsich@theobroma-systems.com> wrote:
>>>
>>>
>>>> On 24 Jul 2017, at 14:45, Kever Yang <kever.yang@rock-chips.com> wrote:
>>>>
>>>> Hi Philipp,
>>>>
>>>>
>>>> On 07/24/2017 05:11 PM, Dr. Philipp Tomsich wrote:
>>>>>> On 24 Jul 2017, at 10:34, Andy Yan <andy.yan@rock-chips.com> wrote:
>>>>>>
>>>>>> Hi Philipp:
>>>>>>
>>>>>>
>>>>>> On 2017年07月21日 16:34, Dr. Philipp Tomsich wrote:
>>>>>>>> On 21 Jul 2017, at 05:50, Andy Yan <andy.yan@rock-chips.com> wrote:
>>>>>>>>
>>>>>>>> Hi Philipp:
>>>>>>>>
>>>>>>>>
>>>>>>>> On 2017年07月19日 04:36, Philipp Tomsich 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 rename, we do this now
>>>>>>>>> before having more code use this.
>>>>>>>>>
>>>>>>>>> As some of the downstream drivers (e.g. the Designware GMAC wrapper)
>>>>>>>>> may need to include the grf-header of multiple devices, we rename the
>>>>>>>>> various defines for the RK3368 by prefixing them with the device name.
>>>>>>>>> This avoids future trouble during driver integration.
>>>>>>>>   Is that really necessary to add such a prefix for all the register definition, just to fit  the GMAC dirver?
>>>>>>>> Maybe the gmac driver can define the platform specific macro in its own code like dwmac-rk in the kernel. Then we can keep the register header file a little tidy.
>>>>>>> The problem is not the GMAC driver (although it was the motivation for doing
>>>>>>> this change), but rather how easy it is to pick up the wrong grf-file and refer
>>>>>>> to the wrong definitions/values… the conflicts are not through the GMAC-specifc
>>>>>>> defines but rather through things like the IOMUX masks or (before I moved
>>>>>>> this to the DDR-driver) things like the defines to enable DRAM in GRF.
>>>>>>>
>>>>>>> Until we find a way to consistently structure things in such a way that
>>>>>>>   (a) common constructs can be shared (e.g. the masks in the IOMUX)
>>>>>>>   (b) specific definitions (e.g. the pin-out for a specific device) is not visible outside of its driver
>>>>>>> I prefer the names to explicitly refer to each device.
>>>>>>>  That said, the longer-term plan (especially with a larger number of devices
>>>>>>> being supported) needs to include a consistent rework of how (and where)
>>>>>>> we manage all these definitions from the grf-header files.
>>>>>> It's true that many devices use GRF, we also face this condition in the kernel land.And all the drivers in kernel handle this device specific definition in the driver itself(by of_device_id->data or MACRO). And there is also no need to use the grf constructs, the common syscon api will provide  you the grf base address. I think this keeps things simple, we don't need to rely on too much of the grf or cru header, maybe some day we can remove the header file, just as clean as the kernel.
>>>>> The problem is not the base-address of GRF, but rather the need to pull in the
>>>>> register structure (so we can refer to individual registers by name instead of by
>>>>> hard-coded offset): as we pull this in today, the definitions for the various IOMUX
>>>>> entries will also be pulled in, which will cause duplicate definitions.
>>>>
>>>> I really don't like the idea of add the prefix, which end with longer MACRO but no other improve.
>>>> I think the GRF is used mostly for the IOMUX, in this case the better solution is
>>>> port the kernel rockchip-pinctrl driver in uboot, which not rely on the dtsi instead of header file.
>>>>
>>>> If we do this, we can remove most of the GRF header definition for IOMUX, I can ask David Wu
>>>> to do this, but maybe few weeks later.
>>>
>>> As indicated earlier: this is a temporary solution (although we have a local saying that
>>> "nothing is more permanent than a temporary solution”) and should remain limited to
>>> the RK3368 until we can clean this up across all boards.
>>
>> Can we drop the prefix for now? I do worry about the precedent this
>> sets. If this is needed anywhere then it suggests to me that we have
>> not split up the drivers correctly. We should not be including two
>> different GRF headers into the same file. E.g. there should be
>> different pinctrl drivers for each SoC.
>
> If we drop it, we can’t get RK3368-support merged into the GMAC driver for now.
> Splitting the Rockchip-wrapper of the GMAC-driver doesn’t appear like a workable
> solution either, as this would create a new file for each chip-variant in drivers/net.
>
> I’ll just pull the RK3368 GMAC (driver) support for now and split it off this series.

My view of GRF is that we should try to have a driver for the various
operations that each thing needs (e.g. using the ioctl model). Imagine
a world where we have a U-Boot that supports running on all available
Rockchip SoCs and can boot on any board. In that case we probably want
a GRF driver with operations that can be implemented by each SoC.

I have not sketched this out or tried it though. It needs more thought
/ prototyping.

I am concerned about #ifdefs in the code and including private
SoC-specific RK header files in generic Rockchip drivers. To me that
is the wrong approach.

>
>>> Pulling this into the pinctrl drivers is an excellent choice.
>>> Please watch out for board_debug_uart_init(…), which also directly accesses the IOMUX
>>> constants and will need to be restructured together with pinctrl drivers.
>>>
>>> Thanks!
>>> —Philipp.
>>>
>>>> Thanks,
>>>> - Kever
>>>>>
>>>>>> On the other hand, add such a prefix of rk3368 breaks the consistency with other rockchip devices.
>>>>> I do agree that we need to clean this up across all the boards and drivers
>>>>> supported today.  As of today, I can not resolve this on a RK3368-only basis,
>>>>> as I’d need to clean up the RK3288 at the same time (and don’t want to do this
>>>>> in a RK3368-focused series that already has way too much going on).
>>>>>
>>>>> Let’s try to clean this up for the next (i.e. the one after this) merge window.
>>>>> Once we have bit-definitions out of the grf-header, I’ll remove the RK3368_
>>>>> prefixes...
>>>>>
>>>>>>>>> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
>>>>>>>>> ---
>>>>>>>>>


Regards,
Simon

> On 24 Jul 2017, at 19:17, Simon Glass <sjg@chromium.org> wrote:
> 
> Hi Philipp,
> 
> On 24 July 2017 at 10:49, Dr. Philipp Tomsich
> <philipp.tomsich@theobroma-systems.com> wrote:
>> 
>>> On 24 Jul 2017, at 18:43, Simon Glass <sjg@chromium.org> wrote:
>>> 
>>> Hi Philipp,
>>> 
>>> On 24 July 2017 at 07:10, Dr. Philipp Tomsich
>>> <philipp.tomsich@theobroma-systems.com> wrote:
>>>> 
>>>> 
>>>>> On 24 Jul 2017, at 14:45, Kever Yang <kever.yang@rock-chips.com> wrote:
>>>>> 
>>>>> Hi Philipp,
>>>>> 
>>>>> 
>>>>> On 07/24/2017 05:11 PM, Dr. Philipp Tomsich wrote:
>>>>>>> On 24 Jul 2017, at 10:34, Andy Yan <andy.yan@rock-chips.com> wrote:
>>>>>>> 
>>>>>>> Hi Philipp:
>>>>>>> 
>>>>>>> 
>>>>>>> On 2017年07月21日 16:34, Dr. Philipp Tomsich wrote:
>>>>>>>>> On 21 Jul 2017, at 05:50, Andy Yan <andy.yan@rock-chips.com> wrote:
>>>>>>>>> 
>>>>>>>>> Hi Philipp:
>>>>>>>>> 
>>>>>>>>> 
>>>>>>>>> On 2017年07月19日 04:36, Philipp Tomsich 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 rename, we do this now
>>>>>>>>>> before having more code use this.
>>>>>>>>>> 
>>>>>>>>>> As some of the downstream drivers (e.g. the Designware GMAC wrapper)
>>>>>>>>>> may need to include the grf-header of multiple devices, we rename the
>>>>>>>>>> various defines for the RK3368 by prefixing them with the device name.
>>>>>>>>>> This avoids future trouble during driver integration.
>>>>>>>>>  Is that really necessary to add such a prefix for all the register definition, just to fit  the GMAC dirver?
>>>>>>>>> Maybe the gmac driver can define the platform specific macro in its own code like dwmac-rk in the kernel. Then we can keep the register header file a little tidy.
>>>>>>>> The problem is not the GMAC driver (although it was the motivation for doing
>>>>>>>> this change), but rather how easy it is to pick up the wrong grf-file and refer
>>>>>>>> to the wrong definitions/values… the conflicts are not through the GMAC-specifc
>>>>>>>> defines but rather through things like the IOMUX masks or (before I moved
>>>>>>>> this to the DDR-driver) things like the defines to enable DRAM in GRF.
>>>>>>>> 
>>>>>>>> Until we find a way to consistently structure things in such a way that
>>>>>>>>  (a) common constructs can be shared (e.g. the masks in the IOMUX)
>>>>>>>>  (b) specific definitions (e.g. the pin-out for a specific device) is not visible outside of its driver
>>>>>>>> I prefer the names to explicitly refer to each device.
>>>>>>>> That said, the longer-term plan (especially with a larger number of devices
>>>>>>>> being supported) needs to include a consistent rework of how (and where)
>>>>>>>> we manage all these definitions from the grf-header files.
>>>>>>> It's true that many devices use GRF, we also face this condition in the kernel land.And all the drivers in kernel handle this device specific definition in the driver itself(by of_device_id->data or MACRO). And there is also no need to use the grf constructs, the common syscon api will provide  you the grf base address. I think this keeps things simple, we don't need to rely on too much of the grf or cru header, maybe some day we can remove the header file, just as clean as the kernel.
>>>>>> The problem is not the base-address of GRF, but rather the need to pull in the
>>>>>> register structure (so we can refer to individual registers by name instead of by
>>>>>> hard-coded offset): as we pull this in today, the definitions for the various IOMUX
>>>>>> entries will also be pulled in, which will cause duplicate definitions.
>>>>> 
>>>>> I really don't like the idea of add the prefix, which end with longer MACRO but no other improve.
>>>>> I think the GRF is used mostly for the IOMUX, in this case the better solution is
>>>>> port the kernel rockchip-pinctrl driver in uboot, which not rely on the dtsi instead of header file.
>>>>> 
>>>>> If we do this, we can remove most of the GRF header definition for IOMUX, I can ask David Wu
>>>>> to do this, but maybe few weeks later.
>>>> 
>>>> As indicated earlier: this is a temporary solution (although we have a local saying that
>>>> "nothing is more permanent than a temporary solution”) and should remain limited to
>>>> the RK3368 until we can clean this up across all boards.
>>> 
>>> Can we drop the prefix for now? I do worry about the precedent this
>>> sets. If this is needed anywhere then it suggests to me that we have
>>> not split up the drivers correctly. We should not be including two
>>> different GRF headers into the same file. E.g. there should be
>>> different pinctrl drivers for each SoC.
>> 
>> If we drop it, we can’t get RK3368-support merged into the GMAC driver for now.
>> Splitting the Rockchip-wrapper of the GMAC-driver doesn’t appear like a workable
>> solution either, as this would create a new file for each chip-variant in drivers/net.
>> 
>> I’ll just pull the RK3368 GMAC (driver) support for now and split it off this series.
> 
> My view of GRF is that we should try to have a driver for the various
> operations that each thing needs (e.g. using the ioctl model). Imagine
> a world where we have a U-Boot that supports running on all available
> Rockchip SoCs and can boot on any board. In that case we probably want
> a GRF driver with operations that can be implemented by each SoC.
> 
> I have not sketched this out or tried it though. It needs more thought
> / prototyping.
> 
> I am concerned about #ifdefs in the code and including private
> SoC-specific RK header files in generic Rockchip drivers. To me that
> is the wrong approach.
> 

This echoes some of my ideas, especially the litmus-test of considering
an universal U-Boot for all Rockchip devices.

I currently don’t see us winning much by moving the inclusion to new
drivers (as these would then need to include the RK header files).  The
direction that I am taking this avoids any chip-specific #ifdef as well, as
the device-model offers plenty better solutions.

In fact, I had sketched out a misc-device that would provide the required
functionality for the GMAC driver last week and this would have resolved
my issue at hand.  Yet, this seems a bit overkill, as it would add a number
of misc-devices w/o gaining any simplification (i.e. each misc-device would
then include the grf-header for one of the chips).

Either way, there’s one detail that I haven’t been able to fully flesh out yet:
the TPL/SPL stages use the GRF directly to set up the debug uart pin-mux.
I haven’t had time to see if there’s space to pull pinctrl in to resolve this
specific case—either way, it shows that there may be cases where we
need to access the GRF w/o the device-model support…

My hope is that the few remaining cases of needing the GRF bits before
the device-model is ready are sufficiently few to accept duplicating the
bit-definitions (e.g. I’d be willing to live with this, if it’s only the debug-uart).

—Philipp.