[01/10] rockchip: rk3288: Add pinctrl support for the gmac ethernet interface

Message ID 1443692893-19905-2-git-send-email-sjoerd.simons@collabora.co.uk
State New
Headers show

Commit Message

Sjoerd Simons Oct. 1, 2015, 9:48 a.m.
Add support for the gmac ethernet interface to pinctrl. This hardcodes
the setup to match that of the firefly and Radxa Rock2 boards, using the
RGMII phy mode for gmac interface and GPIO4B0 as the phy reset GPIO.

Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
---

 arch/arm/include/asm/arch-rockchip/grf_rk3288.h | 228 ++++++++++++++++++++++++
 arch/arm/include/asm/arch-rockchip/periph.h     |   1 +
 drivers/pinctrl/rockchip/pinctrl_rk3288.c       | 102 +++++++++++
 3 files changed, 331 insertions(+)

Comments

Simon Glass Oct. 3, 2015, 2:29 p.m. | #1
On 1 October 2015 at 10:48, Sjoerd Simons <sjoerd.simons@collabora.co.uk> wrote:
> Add support for the gmac ethernet interface to pinctrl. This hardcodes
> the setup to match that of the firefly and Radxa Rock2 boards, using the
> RGMII phy mode for gmac interface and GPIO4B0 as the phy reset GPIO.
>
> Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
> ---
>
>  arch/arm/include/asm/arch-rockchip/grf_rk3288.h | 228 ++++++++++++++++++++++++
>  arch/arm/include/asm/arch-rockchip/periph.h     |   1 +
>  drivers/pinctrl/rockchip/pinctrl_rk3288.c       | 102 +++++++++++
>  3 files changed, 331 insertions(+)
>
> diff --git a/arch/arm/include/asm/arch-rockchip/grf_rk3288.h b/arch/arm/include/asm/arch-rockchip/grf_rk3288.h
> index 0117a17..b7dda47 100644
> --- a/arch/arm/include/asm/arch-rockchip/grf_rk3288.h
> +++ b/arch/arm/include/asm/arch-rockchip/grf_rk3288.h
> @@ -283,6 +283,163 @@ enum {
>         GPIO3C0_EMMC_CMD,
>  };
>
> +/* GRF_GPIO3DL_IOMUX */
> +enum {
> +       GPIO3D3_SHIFT           = 12,
> +       GPIO3D3_MASK            = 7,
> +       GPIO3D3_GPIO            = 0,
> +       GPIO3D3_FLASH1_DATA3,
> +       GPIO3D3_HOST_DOUT3,
> +       GPIO3D3_MAC_RXD3,
> +       GPIO3D3_SDIO1_DATA3,
> +
> +       GPIO3D2_SHIFT           = 8,
> +       GPIO3D2_MASK            = 7,
> +       GPIO3D2_GPIO            = 0,
> +       GPIO3D2_FLASH1_DATA2,
> +       GPIO3D2_HOST_DOUT2,
> +       GPIO3D2_MAC_RXD2,
> +       GPIO3D2_SDIO1_DATA2,
> +
> +       GPIO3D1_SHIFT           = 4,
> +       GPIO3D1_MASK            = 7,
> +       GPIO3D1_GPIO            = 0,
> +       GPIO3DL1_FLASH1_DATA1,
> +       GPIO3D1_HOST_DOUT1,
> +       GPIO3D1_MAC_TXD3,
> +       GPIO3D1_SDIO1_DATA1,
> +
> +       GPIO3D0_SHIFT           = 0,
> +       GPIO3D0_MASK            = 7,
> +       GPIO3D0_GPIO            = 0,
> +       GPIO3D0_FLASH1_DATA0,
> +       GPIO3D0_HOST_DOUT0,
> +       GPIO3D0_MAC_TXD2,
> +       GPIO3D0_SDIO1_DATA0,
> +};
> +
> +/* GRF_GPIO3HL_IOMUX */
> +enum {
> +       GPIO3D7_SHIFT           = 12,
> +       GPIO3D7_MASK            = 7,
> +       GPIO3D7_GPIO            = 0,
> +       GPIO3D7_FLASH1_DATA7,
> +       GPIO3D7_HOST_DOUT7,
> +       GPIO3D7_MAC_RXD1,
> +       GPIO3D7_SDIO1_INTN,
> +
> +       GPIO3D6_SHIFT           = 8,
> +       GPIO3D6_MASK            = 7,
> +       GPIO3D6_GPIO            = 0,
> +       GPIO3D6_FLASH1_DATA6,
> +       GPIO3D6_HOST_DOUT6,
> +       GPIO3D6_MAC_RXD0,
> +       GPIO3D6_SDIO1_BKPWR,
> +
> +       GPIO3D5_SHIFT           = 4,
> +       GPIO3D5_MASK            = 7,
> +       GPIO3D5_GPIO            = 0,
> +       GPIO3D5_FLASH1_DATA5,
> +       GPIO3D5_HOST_DOUT5,
> +       GPIO3D5_MAC_TXD1,
> +       GPIO3D5_SDIO1_WRPRT,
> +
> +       GPIO3D4_SHIFT           = 0,
> +       GPIO3D4_MASK            = 7,
> +       GPIO3D4_GPIO            = 0,
> +       GPIO3D4_FLASH1_DATA4,
> +       GPIO3D4_HOST_DOUT4,
> +       GPIO3D4_MAC_TXD0,
> +       GPIO3D4_SDIO1_DETECTN,
> +};
> +
> +/* GRF_GPIO4AL_IOMUX */
> +enum {
> +       GPIO4A3_SHIFT           = 12,
> +       GPIO4A3_MASK            = 7,
> +       GPIO4A3_GPIO            = 0,
> +       GPIO4A3_FLASH1_ALE,
> +       GPIO4A3_HOST_DOUT9,
> +       GPIO4A3_MAC_CLK,
> +       GPIO4A3_FLASH0_CSN6,
> +
> +       GPIO4A2_SHIFT           = 8,
> +       GPIO4A2_MASK            = 7,
> +       GPIO4A2_GPIO            = 0,
> +       GPIO4A2_FLASH1_RDN,
> +       GPIO4A2_HOST_DOUT8,
> +       GPIO4A2_MAC_RXER,
> +       GPIO4A2_FLASH0_CSN5,
> +
> +       GPIO4A1_SHIFT           = 4,
> +       GPIO4A1_MASK            = 7,
> +       GPIO4A1_GPIO            = 0,
> +       GPIO4A1_FLASH1_WP,
> +       GPIO4A1_HOST_CKOUTN,
> +       GPIO4A1_MAC_TXDV,
> +       GPIO4A1_FLASH0_CSN4,
> +
> +       GPIO4A0_SHIFT           = 0,
> +       GPIO4A0_MASK            = 3,
> +       GPIO4A0_GPIO            = 0,
> +       GPIO4A0_FLASH1_RDY,
> +       GPIO4A0_HOST_CKOUTP,
> +       GPIO4A0_MAC_MDC,
> +};
> +
> +/* GRF_GPIO4AH_IOMUX */
> +enum {
> +       GPIO4A7_SHIFT           = 12,
> +       GPIO4A7_MASK            = 7,
> +       GPIO4A7_GPIO            = 0,
> +       GPIO4A7_FLASH1_CSN1,
> +       GPIO4A7_HOST_DOUT13,
> +       GPIO4A7_MAC_CSR,
> +       GPIO4A7_SDIO1_CLKOUT,
> +
> +       GPIO4A6_SHIFT           = 8,
> +       GPIO4A6_MASK            = 7,
> +       GPIO4A6_GPIO            = 0,
> +       GPIO4A6_FLASH1_CSN0,
> +       GPIO4A6_HOST_DOUT12,
> +       GPIO4A6_MAC_RXCLK,
> +       GPIO4A6_SDIO1_CMD,
> +
> +       GPIO4A5_SHIFT           = 4,
> +       GPIO4A5_MASK            = 3,
> +       GPIO4A5_GPIO            = 0,
> +       GPIO4A5_FLASH1_WRN,
> +       GPIO4A5_HOST_DOUT11,
> +       GPIO4A5_MAC_MDIO,
> +
> +       GPIO4A4_SHIFT           = 0,
> +       GPIO4A4_MASK            = 7,
> +       GPIO4A4_GPIO            = 0,
> +       GPIO4A4_FLASH1_CLE,
> +       GPIO4A4_HOST_DOUT10,
> +       GPIO4A4_MAC_TXEN,
> +       GPIO4A4_FLASH0_CSN7,
> +};
> +
> +/* GRF_GPIO4BL_IOMUX */
> +enum {
> +       GPIO4B1_SHIFT           = 4,
> +       GPIO4B1_MASK            = 7,
> +       GPIO4B1_GPIO            = 0,
> +       GPIO4B1_FLASH1_CSN2,
> +       GPIO4B1_HOST_DOUT15,
> +       GPIO4B1_MAC_TXCLK,
> +       GPIO4B1_SDIO1_PWREN,
> +
> +       GPIO4B0_SHIFT           = 0,
> +       GPIO4B0_MASK            = 7,
> +       GPIO4B0_GPIO            = 0,
> +       GPIO4B0_FLASH1_DQS,
> +       GPIO4B0_HOST_DOUT14,
> +       GPIO4B0_MAC_COL,
> +       GPIO4B0_FLASH1_CSN3,
> +};
> +
>  /* GRF_GPIO4C_IOMUX */
>  enum {
>         GPIO4C7_SHIFT           = 14,
> @@ -718,6 +875,40 @@ enum {
>         MSCH0_MAINPARTIALPOP_MASK = 1,
>  };
>
> +/* GRF_SOC_CON1 */
> +enum {
> +       RMII_MODE_SHIFT = 0xe,
> +       RMII_MODE_MASK = 1,
> +       RMII_MODE = 1,
> +
> +       GMAC_CLK_SEL_SHIFT      = 0xc,
> +       GMAC_CLK_SEL_MASK       = 3,
> +       GMAC_CLK_SEL_125M       = 0,
> +       GMAC_CLK_SEL_25M,
> +       GMAC_CLK_SEL_2_5M,
> +
> +       RMII_CLK_SEL_SHIFT      = 0xb,
> +       RMII_CLK_SEL_MASK       = 1,
> +       RMII_CLK_SEL_2_5M       = 0,
> +       RMII_CLK_SEL_25M,
> +
> +       GMAC_SPEED_SHIFT        = 0xa,
> +       GMAC_SPEED_MASK         = 1,
> +       GMAC_SPEED_10M          = 0,
> +       GMAC_SPEED_100M,
> +
> +       GMAC_FLOWCTRL_SHIFT     = 0x9,
> +       GMAC_FLOWCTRL_MASK      = 1,
> +
> +       GMAC_PHY_INTF_SEL_SHIFT = 0x6,
> +       GMAC_PHY_INTF_SEL_MASK  = 0x7,
> +       GMAC_PHY_INTF_SEL_RGMII = 0x1,
> +       GMAC_PHY_INTF_SEL_RMII  = 0x4,
> +
> +       HOST_REMAP_SHIFT        = 0x5,
> +       HOST_REMAP_MASK         = 1
> +};
> +
>  /* GRF_SOC_CON2 */
>  enum {
>         UPCTL1_LPDDR3_ODT_EN_SHIFT = 0xd,
> @@ -765,4 +956,41 @@ enum {
>         PWM_PWM                 = 0,
>  };
>
> +/* GRF_SOC_CON3 */
> +enum {
> +       RXCLK_DLY_ENA_GMAC_SHIFT = 0xf,
> +       RXCLK_DLY_ENA_GMAC_MASK = 1,
> +       RXCLK_DLY_ENA_GMAC_DISABLE = 0,
> +       RXCLK_DLY_ENA_GMAC_ENABLE,
> +
> +       TXCLK_DLY_ENA_GMAC_SHIFT = 0xe,
> +       TXCLK_DLY_ENA_GMAC_MASK = 1,
> +       TXCLK_DLY_ENA_GMAC_DISABLE = 0,
> +       TXCLK_DLY_ENA_GMAC_ENABLE,
> +
> +       CLK_RX_DL_CFG_GMAC_SHIFT = 0x7,
> +       CLK_RX_DL_CFG_GMAC_MASK = 0x7f,
> +
> +       CLK_TX_DL_CFG_GMAC_SHIFT = 0x0,
> +       CLK_TX_DL_CFG_GMAC_MASK = 0x7f,
> +};
> +
> +/* GPIO Bias settings */
> +#define GPIO_BIAS_BITS 2
> +#define GPIO_BIAS_MASK(x) (0x3 << (GPIO_BIAS_BITS * x))
> +
> +#define GPIO_BIAS_2mA(x) (0x0 << (GPIO_BIAS_BITS * x))
> +#define GPIO_BIAS_4mA(x) (0x1 << (GPIO_BIAS_BITS * x))
> +#define GPIO_BIAS_8mA(x) (0x2 << (GPIO_BIAS_BITS * x))
> +#define GPIO_BIAS_12mA(x) (0x3 << (GPIO_BIAS_BITS * x))

I'd really like to avoid these sorts of things. Instead:

enum {
    GPIO_BIAS_2MA = 0,
    GPIO_BIAS_4MA,
   ...

#define GPIO_BIAS_SHIFT(x)  ((x) * 2)

Then:

 rk_clrsetreg(&grf->gpio1_e[2][3],
+                            GPIO_BIAS_16MA << GPIO_BIAS_SHIFT(1))

or similar. In other words follow the code that is there.


> +
> +/* GPIO PU/PD settings */
> +#define GPIO_PULL_BITS 2
> +#define GPIO_PULL_MASK(x) (0x3 << (GPIO_PULL_BITS * x))
> +
> +#define GPIO_PULL_NORMAL(x) (0x0 << (GPIO_PULL_BITS * x))
> +#define GPIO_PULL_UP(x) (0x1 << (GPIO_PULL_BITS * x))
> +#define GPIO_PULL_DOWN(x) (0x2 << (GPIO_PULL_BITS * x))
> +#define GPIO_PULL_REPEAT(x) (0x3 << (GPIO_PULL_BITS * x))
> +
>  #endif
> diff --git a/arch/arm/include/asm/arch-rockchip/periph.h b/arch/arm/include/asm/arch-rockchip/periph.h
> index fa6069b..239a274 100644
> --- a/arch/arm/include/asm/arch-rockchip/periph.h
> +++ b/arch/arm/include/asm/arch-rockchip/periph.h
> @@ -38,6 +38,7 @@ enum periph_id {
>         PERIPH_ID_SDMMC1,
>         PERIPH_ID_SDMMC2,
>         PERIPH_ID_HDMI,
> +       PERIPH_ID_GMAC,
>
>         PERIPH_ID_COUNT,
>
> diff --git a/drivers/pinctrl/rockchip/pinctrl_rk3288.c b/drivers/pinctrl/rockchip/pinctrl_rk3288.c
> index c432a00..e59f771 100644
> --- a/drivers/pinctrl/rockchip/pinctrl_rk3288.c
> +++ b/drivers/pinctrl/rockchip/pinctrl_rk3288.c
> @@ -298,6 +298,103 @@ static void pinctrl_rk3288_sdmmc_config(struct rk3288_grf *grf, int mmc_id)
>         }
>  }
>
> +static void pinctrl_rk3288_gmac_config(struct rk3288_grf *grf, int gmac_id)
> +{
> +       switch (gmac_id) {
> +       case PERIPH_ID_GMAC:
> +               rk_clrsetreg(&grf->gpio3dl_iomux,
> +                            GPIO3D3_MASK << GPIO3D3_SHIFT |
> +                            GPIO3D2_MASK << GPIO3D2_SHIFT |
> +                            GPIO3D2_MASK << GPIO3D1_SHIFT |
> +                            GPIO3D0_MASK << GPIO3D0_SHIFT,
> +                            GPIO3D3_MAC_RXD3 << GPIO3D3_SHIFT |
> +                            GPIO3D2_MAC_RXD2 << GPIO3D2_SHIFT |
> +                            GPIO3D1_MAC_TXD3 << GPIO3D1_SHIFT |
> +                            GPIO3D0_MAC_TXD2 << GPIO3D0_SHIFT);
> +
> +               rk_clrsetreg(&grf->gpio3dh_iomux,
> +                            GPIO3D7_MASK << GPIO3D7_SHIFT |
> +                            GPIO3D6_MASK << GPIO3D6_SHIFT |
> +                            GPIO3D5_MASK << GPIO3D5_SHIFT |
> +                            GPIO3D4_MASK << GPIO3D4_SHIFT,
> +                            GPIO3D7_MAC_RXD1 << GPIO3D7_SHIFT |
> +                            GPIO3D6_MAC_RXD0 << GPIO3D6_SHIFT |
> +                            GPIO3D5_MAC_TXD1 << GPIO3D5_SHIFT |
> +                            GPIO3D4_MAC_TXD0 << GPIO3D4_SHIFT);
> +
> +               /* switch the Tx pins to 12ma drive-strength */
> +               rk_clrsetreg(&grf->gpio1_e[2][3],
> +                            GPIO_BIAS_MASK(0) | GPIO_BIAS_MASK(1) |
> +                            GPIO_BIAS_MASK(4) | GPIO_BIAS_MASK(5),
> +                            GPIO_BIAS_12mA(0) | GPIO_BIAS_12mA(1) |
> +                            GPIO_BIAS_12mA(4) | GPIO_BIAS_12mA(5));
> +
> +               /* Set normal pull for all GPIO3D pins */
> +               rk_clrsetreg(&grf->gpio1_p[2][3],
> +                            GPIO_PULL_MASK(0) | GPIO_PULL_MASK(1) |
> +                            GPIO_PULL_MASK(2) | GPIO_PULL_MASK(3) |
> +                            GPIO_PULL_MASK(4) | GPIO_PULL_MASK(5) |
> +                            GPIO_PULL_MASK(6) | GPIO_PULL_MASK(7),
> +                            GPIO_PULL_NORMAL(0) | GPIO_PULL_NORMAL(1) |
> +                            GPIO_PULL_NORMAL(2) | GPIO_PULL_NORMAL(3) |
> +                            GPIO_PULL_NORMAL(4) | GPIO_PULL_NORMAL(5) |
> +                            GPIO_PULL_NORMAL(6) | GPIO_PULL_NORMAL(7));
> +
> +               rk_clrsetreg(&grf->gpio4al_iomux,
> +                            GPIO4A3_MASK << GPIO4A3_SHIFT |
> +                            GPIO4A1_MASK << GPIO4A1_SHIFT |
> +                            GPIO4A0_MASK << GPIO4A0_SHIFT,
> +                            GPIO4A3_MAC_CLK << GPIO4A3_SHIFT |
> +                            GPIO4A1_MAC_TXDV << GPIO4A1_SHIFT |
> +                            GPIO4A0_MAC_MDC << GPIO4A0_SHIFT);
> +
> +               rk_clrsetreg(&grf->gpio4ah_iomux,
> +                            GPIO4A6_MASK << GPIO4A6_SHIFT |
> +                            GPIO4A5_MASK << GPIO4A5_SHIFT |
> +                            GPIO4A4_MASK << GPIO4A4_SHIFT,
> +                            GPIO4A6_MAC_RXCLK << GPIO4A6_SHIFT |
> +                            GPIO4A5_MAC_MDIO << GPIO4A5_SHIFT |
> +                            GPIO4A4_MAC_TXEN << GPIO4A4_SHIFT);
> +
> +               /* switch GPIO4A4 to 12ma drive-strength */
> +               rk_clrsetreg(&grf->gpio1_e[3][0],
> +                            GPIO_BIAS_MASK(4),
> +                            GPIO_BIAS_12mA(4));
> +               /* Set normal pull for all GPIO4A pins */
> +               rk_clrsetreg(&grf->gpio1_p[3][0],
> +                            GPIO_PULL_MASK(0) | GPIO_PULL_MASK(1) |
> +                            GPIO_PULL_MASK(2) | GPIO_PULL_MASK(3) |
> +                            GPIO_PULL_MASK(4) | GPIO_PULL_MASK(5) |
> +                            GPIO_PULL_MASK(6) | GPIO_PULL_MASK(7),
> +                            GPIO_PULL_NORMAL(0) | GPIO_PULL_NORMAL(1) |
> +                            GPIO_PULL_NORMAL(2) | GPIO_PULL_NORMAL(3) |
> +                            GPIO_PULL_NORMAL(4) | GPIO_PULL_NORMAL(5) |
> +                            GPIO_PULL_NORMAL(6) | GPIO_PULL_NORMAL(7));
> +
> +               /* Assuming GPIO4B0_GPIO is phy-reset*/

Space before */

> +               rk_clrsetreg(&grf->gpio4bl_iomux,
> +                            GPIO4B1_MASK << GPIO4B1_SHIFT |
> +                            GPIO4B0_MASK << GPIO4B0_SHIFT,
> +                            GPIO4B1_MAC_TXCLK << GPIO4B1_SHIFT|
> +                            GPIO4B0_GPIO << GPIO4B0_SHIFT);

Would this normally be handled by a GPIO? Does it belong in pinmux?

> +
> +               /* switch GPIO4B1 to 12ma drive-strength */
> +               rk_clrsetreg(&grf->gpio1_e[3][1],
> +                            GPIO_BIAS_MASK(1),
> +                            GPIO_BIAS_12mA(1));
> +
> +               /* Set pull normal for GPIO4B1, pull up for GPIO4B0 */
> +               rk_clrsetreg(&grf->gpio1_p[3][1],
> +                            GPIO_PULL_MASK(0) | GPIO_PULL_MASK(1),
> +                            GPIO_PULL_UP(0) | GPIO_PULL_NORMAL(1));
> +
> +               break;
> +       default:
> +               debug("gmac id = %d iomux error!\n", gmac_id);
> +               break;
> +       }
> +}
> +
>  static void pinctrl_rk3288_hdmi_config(struct rk3288_grf *grf, int hdmi_id)
>  {
>         switch (hdmi_id) {
> @@ -354,6 +451,9 @@ static int rk3288_pinctrl_request(struct udevice *dev, int func, int flags)
>         case PERIPH_ID_SDMMC1:
>                 pinctrl_rk3288_sdmmc_config(priv->grf, func);
>                 break;
> +       case PERIPH_ID_GMAC:
> +               pinctrl_rk3288_gmac_config(priv->grf, func);
> +               break;
>         case PERIPH_ID_HDMI:
>                 pinctrl_rk3288_hdmi_config(priv->grf, func);
>                 break;
> @@ -376,6 +476,8 @@ static int rk3288_pinctrl_get_periph_id(struct udevice *dev,
>                 return -EINVAL;
>
>         switch (cell[1]) {
> +       case 27:
> +               return PERIPH_ID_GMAC;
>         case 44:
>                 return PERIPH_ID_SPI0;
>         case 45:
> --
> 2.5.3
>

Regards,
Simon
Sjoerd Simons Oct. 5, 2015, 9:04 a.m. | #2
On Sat, 2015-10-03 at 15:29 +0100, Simon Glass wrote:
> > On 1 October 2015 at 10:48, Sjoerd Simons <
> > sjoerd.simons@collabora.co.uk> wrote:Add support for the gmac
> > ethernet interface to pinctrl. This hardcodes

> > the setup to match that of the firefly and Radxa Rock2 boards,
> > using the
> > RGMII phy mode for gmac interface and GPIO4B0 as the phy reset
> > GPIO.
> > 
> > Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
> > ---
> > 
> >  arch/arm/include/asm/arch-rockchip/grf_rk3288.h | 228
> > ++++++++++++++++++++++++
> >  arch/arm/include/asm/arch-rockchip/periph.h     |   1 +
> >  drivers/pinctrl/rockchip/pinctrl_rk3288.c       | 102 +++++++++++
> >  3 files changed, 331 insertions(+)
> > 
> > diff --git a/arch/arm/include/asm/arch-rockchip/grf_rk3288.h
> > b/arch/arm/include/asm/arch-rockchip/grf_rk3288.h
> > index 0117a17..b7dda47 100644
> > --- a/arch/arm/include/asm/arch-rockchip/grf_rk3288.h
> > +++ b/arch/arm/include/asm/arch-rockchip/grf_rk3288.h
> > +               /* Assuming GPIO4B0_GPIO is phy-reset*/
> 
> Space before */
> +                            GPIO4B1_MASK << GPIO4B1_SHIFT |
> 
> Would this normally be handled by a GPIO? Does it belong in pinmux?

Hrm, i'm changing the wrong pin there aren't I, woops (well either that
or the comment is wrong)? Anyway, yes the PHY reset is typically a
GPIO, this should just set up the respective pin in GPIO mode which is
the job of pinmuxing right ? :) 

Or maybe i'm misunderstanding your comment here?
Simon Glass Oct. 9, 2015, 9:36 a.m. | #3
Hi Sjoerd,

On 5 October 2015 at 10:04, Sjoerd Simons <sjoerd.simons@collabora.co.uk> wrote:
> On Sat, 2015-10-03 at 15:29 +0100, Simon Glass wrote:
>> > On 1 October 2015 at 10:48, Sjoerd Simons <
>> > sjoerd.simons@collabora.co.uk> wrote:Add support for the gmac
>> > ethernet interface to pinctrl. This hardcodes
>
>> > the setup to match that of the firefly and Radxa Rock2 boards,
>> > using the
>> > RGMII phy mode for gmac interface and GPIO4B0 as the phy reset
>> > GPIO.
>> >
>> > Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
>> > ---
>> >
>> >  arch/arm/include/asm/arch-rockchip/grf_rk3288.h | 228
>> > ++++++++++++++++++++++++
>> >  arch/arm/include/asm/arch-rockchip/periph.h     |   1 +
>> >  drivers/pinctrl/rockchip/pinctrl_rk3288.c       | 102 +++++++++++
>> >  3 files changed, 331 insertions(+)
>> >
>> > diff --git a/arch/arm/include/asm/arch-rockchip/grf_rk3288.h
>> > b/arch/arm/include/asm/arch-rockchip/grf_rk3288.h
>> > index 0117a17..b7dda47 100644
>> > --- a/arch/arm/include/asm/arch-rockchip/grf_rk3288.h
>> > +++ b/arch/arm/include/asm/arch-rockchip/grf_rk3288.h
>> > +               /* Assuming GPIO4B0_GPIO is phy-reset*/
>>
>> Space before */
>> +                            GPIO4B1_MASK << GPIO4B1_SHIFT |
>>
>> Would this normally be handled by a GPIO? Does it belong in pinmux?
>
> Hrm, i'm changing the wrong pin there aren't I, woops (well either that
> or the comment is wrong)? Anyway, yes the PHY reset is typically a
> GPIO, this should just set up the respective pin in GPIO mode which is
> the job of pinmuxing right ? :)
>
> Or maybe i'm misunderstanding your comment here?

I would hope that this would use gpio_request_by_name() instead, with
the GPIO specified in the device tree. See for example:

vcc_sd: sdmmc-regulator {
   compatible = "regulator-fixed";
   gpio = <&gpio7 11 GPIO_ACTIVE_LOW>;
   ...
};

Regards,
Simon
Sjoerd Simons Oct. 9, 2015, 9:51 a.m. | #4
On Fri, 2015-10-09 at 10:36 +0100, Simon Glass wrote:
> Hi Sjoerd,
> 
> On 5 October 2015 at 10:04, Sjoerd Simons <
> sjoerd.simons@collabora.co.uk> wrote:
> > On Sat, 2015-10-03 at 15:29 +0100, Simon Glass wrote:
> > > > On 1 October 2015 at 10:48, Sjoerd Simons <
> > > > sjoerd.simons@collabora.co.uk> wrote:Add support for the gmac
> > > > ethernet interface to pinctrl. This hardcodes
> > 
> > > > the setup to match that of the firefly and Radxa Rock2 boards,
> > > > using the
> > > > RGMII phy mode for gmac interface and GPIO4B0 as the phy reset
> > > > GPIO.
> > > > 
> > > > Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
> > > > ---
> > > > 
> > > >  arch/arm/include/asm/arch-rockchip/grf_rk3288.h | 228
> > > > ++++++++++++++++++++++++
> > > >  arch/arm/include/asm/arch-rockchip/periph.h     |   1 +
> > > >  drivers/pinctrl/rockchip/pinctrl_rk3288.c       | 102
> > > > +++++++++++
> > > >  3 files changed, 331 insertions(+)
> > > > 
> > > > diff --git a/arch/arm/include/asm/arch-rockchip/grf_rk3288.h
> > > > b/arch/arm/include/asm/arch-rockchip/grf_rk3288.h
> > > > index 0117a17..b7dda47 100644
> > > > --- a/arch/arm/include/asm/arch-rockchip/grf_rk3288.h
> > > > +++ b/arch/arm/include/asm/arch-rockchip/grf_rk3288.h
> > > > +               /* Assuming GPIO4B0_GPIO is phy-reset*/
> > > 
> > > Space before */
> > > +                            GPIO4B1_MASK << GPIO4B1_SHIFT |
> > > 
> > > Would this normally be handled by a GPIO? Does it belong in
> > > pinmux?
> > 
> > Hrm, i'm changing the wrong pin there aren't I, woops (well either
> > that
> > or the comment is wrong)? Anyway, yes the PHY reset is typically a
> > GPIO, this should just set up the respective pin in GPIO mode which
> > is
> > the job of pinmuxing right ? :)
> > 
> > Or maybe i'm misunderstanding your comment here?
> 
> I would hope that this would use gpio_request_by_name() instead, with
> the GPIO specified in the device tree. See for example:


> vcc_sd: sdmmc-regulator {
>    compatible = "regulator-fixed";
>    gpio = <&gpio7 11 GPIO_ACTIVE_LOW>;
>    ...
> };

Sure and _that_ part is done via:
  snps,reset-gpio = <&gpio4 8 GPIO_ACTIVE_LOW>;

Which gets handled by the core designware driver in my patchset.

What this code is doing the equivalent of (in your example):

vcc_sd: sdmmc-regulator {
  ...
  pinctrl-names = "default";
  pinctrl-0 = <&sdmmc_pwr>;
  ...
};

sdmmc_pwr: sdmmc-pwr {
     rockchip,pins = <7 11 RK_FUNC_GPIO &pcfg_pull_none>;
};

In other words, it sets the function of the respective pin to be used
for GPIO. However it doesn't do any control of it.
Simon Glass Oct. 9, 2015, 1:01 p.m. | #5
Hi Sjoerd,

On 9 October 2015 at 10:51, Sjoerd Simons <sjoerd.simons@collabora.co.uk> wrote:
> On Fri, 2015-10-09 at 10:36 +0100, Simon Glass wrote:
>> Hi Sjoerd,
>>
>> On 5 October 2015 at 10:04, Sjoerd Simons <
>> sjoerd.simons@collabora.co.uk> wrote:
>> > On Sat, 2015-10-03 at 15:29 +0100, Simon Glass wrote:
>> > > > On 1 October 2015 at 10:48, Sjoerd Simons <
>> > > > sjoerd.simons@collabora.co.uk> wrote:Add support for the gmac
>> > > > ethernet interface to pinctrl. This hardcodes
>> >
>> > > > the setup to match that of the firefly and Radxa Rock2 boards,
>> > > > using the
>> > > > RGMII phy mode for gmac interface and GPIO4B0 as the phy reset
>> > > > GPIO.
>> > > >
>> > > > Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
>> > > > ---
>> > > >
>> > > >  arch/arm/include/asm/arch-rockchip/grf_rk3288.h | 228
>> > > > ++++++++++++++++++++++++
>> > > >  arch/arm/include/asm/arch-rockchip/periph.h     |   1 +
>> > > >  drivers/pinctrl/rockchip/pinctrl_rk3288.c       | 102
>> > > > +++++++++++
>> > > >  3 files changed, 331 insertions(+)
>> > > >
>> > > > diff --git a/arch/arm/include/asm/arch-rockchip/grf_rk3288.h
>> > > > b/arch/arm/include/asm/arch-rockchip/grf_rk3288.h
>> > > > index 0117a17..b7dda47 100644
>> > > > --- a/arch/arm/include/asm/arch-rockchip/grf_rk3288.h
>> > > > +++ b/arch/arm/include/asm/arch-rockchip/grf_rk3288.h
>> > > > +               /* Assuming GPIO4B0_GPIO is phy-reset*/
>> > >
>> > > Space before */
>> > > +                            GPIO4B1_MASK << GPIO4B1_SHIFT |
>> > >
>> > > Would this normally be handled by a GPIO? Does it belong in
>> > > pinmux?
>> >
>> > Hrm, i'm changing the wrong pin there aren't I, woops (well either
>> > that
>> > or the comment is wrong)? Anyway, yes the PHY reset is typically a
>> > GPIO, this should just set up the respective pin in GPIO mode which
>> > is
>> > the job of pinmuxing right ? :)
>> >
>> > Or maybe i'm misunderstanding your comment here?
>>
>> I would hope that this would use gpio_request_by_name() instead, with
>> the GPIO specified in the device tree. See for example:
>
>
>> vcc_sd: sdmmc-regulator {
>>    compatible = "regulator-fixed";
>>    gpio = <&gpio7 11 GPIO_ACTIVE_LOW>;
>>    ...
>> };
>
> Sure and _that_ part is done via:
>   snps,reset-gpio = <&gpio4 8 GPIO_ACTIVE_LOW>;
>
> Which gets handled by the core designware driver in my patchset.
>
> What this code is doing the equivalent of (in your example):
>
> vcc_sd: sdmmc-regulator {
>   ...
>   pinctrl-names = "default";
>   pinctrl-0 = <&sdmmc_pwr>;
>   ...
> };
>
> sdmmc_pwr: sdmmc-pwr {
>      rockchip,pins = <7 11 RK_FUNC_GPIO &pcfg_pull_none>;
> };
>
> In other words, it sets the function of the respective pin to be used
> for GPIO. However it doesn't do any control of it.

I see. In that case I suspect we should create a full pinctrl driver
for Rockchip. The framework is there - Masahiro has sent patches for
an implementation for Uniphier.

Regards,
Simon

Patch

diff --git a/arch/arm/include/asm/arch-rockchip/grf_rk3288.h b/arch/arm/include/asm/arch-rockchip/grf_rk3288.h
index 0117a17..b7dda47 100644
--- a/arch/arm/include/asm/arch-rockchip/grf_rk3288.h
+++ b/arch/arm/include/asm/arch-rockchip/grf_rk3288.h
@@ -283,6 +283,163 @@  enum {
 	GPIO3C0_EMMC_CMD,
 };
 
+/* GRF_GPIO3DL_IOMUX */
+enum {
+	GPIO3D3_SHIFT		= 12,
+	GPIO3D3_MASK		= 7,
+	GPIO3D3_GPIO		= 0,
+	GPIO3D3_FLASH1_DATA3,
+	GPIO3D3_HOST_DOUT3,
+	GPIO3D3_MAC_RXD3,
+	GPIO3D3_SDIO1_DATA3,
+
+	GPIO3D2_SHIFT		= 8,
+	GPIO3D2_MASK		= 7,
+	GPIO3D2_GPIO		= 0,
+	GPIO3D2_FLASH1_DATA2,
+	GPIO3D2_HOST_DOUT2,
+	GPIO3D2_MAC_RXD2,
+	GPIO3D2_SDIO1_DATA2,
+
+	GPIO3D1_SHIFT		= 4,
+	GPIO3D1_MASK		= 7,
+	GPIO3D1_GPIO		= 0,
+	GPIO3DL1_FLASH1_DATA1,
+	GPIO3D1_HOST_DOUT1,
+	GPIO3D1_MAC_TXD3,
+	GPIO3D1_SDIO1_DATA1,
+
+	GPIO3D0_SHIFT		= 0,
+	GPIO3D0_MASK		= 7,
+	GPIO3D0_GPIO		= 0,
+	GPIO3D0_FLASH1_DATA0,
+	GPIO3D0_HOST_DOUT0,
+	GPIO3D0_MAC_TXD2,
+	GPIO3D0_SDIO1_DATA0,
+};
+
+/* GRF_GPIO3HL_IOMUX */
+enum {
+	GPIO3D7_SHIFT		= 12,
+	GPIO3D7_MASK		= 7,
+	GPIO3D7_GPIO		= 0,
+	GPIO3D7_FLASH1_DATA7,
+	GPIO3D7_HOST_DOUT7,
+	GPIO3D7_MAC_RXD1,
+	GPIO3D7_SDIO1_INTN,
+
+	GPIO3D6_SHIFT		= 8,
+	GPIO3D6_MASK		= 7,
+	GPIO3D6_GPIO		= 0,
+	GPIO3D6_FLASH1_DATA6,
+	GPIO3D6_HOST_DOUT6,
+	GPIO3D6_MAC_RXD0,
+	GPIO3D6_SDIO1_BKPWR,
+
+	GPIO3D5_SHIFT		= 4,
+	GPIO3D5_MASK		= 7,
+	GPIO3D5_GPIO		= 0,
+	GPIO3D5_FLASH1_DATA5,
+	GPIO3D5_HOST_DOUT5,
+	GPIO3D5_MAC_TXD1,
+	GPIO3D5_SDIO1_WRPRT,
+
+	GPIO3D4_SHIFT		= 0,
+	GPIO3D4_MASK		= 7,
+	GPIO3D4_GPIO		= 0,
+	GPIO3D4_FLASH1_DATA4,
+	GPIO3D4_HOST_DOUT4,
+	GPIO3D4_MAC_TXD0,
+	GPIO3D4_SDIO1_DETECTN,
+};
+
+/* GRF_GPIO4AL_IOMUX */
+enum {
+	GPIO4A3_SHIFT		= 12,
+	GPIO4A3_MASK		= 7,
+	GPIO4A3_GPIO		= 0,
+	GPIO4A3_FLASH1_ALE,
+	GPIO4A3_HOST_DOUT9,
+	GPIO4A3_MAC_CLK,
+	GPIO4A3_FLASH0_CSN6,
+
+	GPIO4A2_SHIFT		= 8,
+	GPIO4A2_MASK		= 7,
+	GPIO4A2_GPIO		= 0,
+	GPIO4A2_FLASH1_RDN,
+	GPIO4A2_HOST_DOUT8,
+	GPIO4A2_MAC_RXER,
+	GPIO4A2_FLASH0_CSN5,
+
+	GPIO4A1_SHIFT		= 4,
+	GPIO4A1_MASK		= 7,
+	GPIO4A1_GPIO		= 0,
+	GPIO4A1_FLASH1_WP,
+	GPIO4A1_HOST_CKOUTN,
+	GPIO4A1_MAC_TXDV,
+	GPIO4A1_FLASH0_CSN4,
+
+	GPIO4A0_SHIFT		= 0,
+	GPIO4A0_MASK		= 3,
+	GPIO4A0_GPIO		= 0,
+	GPIO4A0_FLASH1_RDY,
+	GPIO4A0_HOST_CKOUTP,
+	GPIO4A0_MAC_MDC,
+};
+
+/* GRF_GPIO4AH_IOMUX */
+enum {
+	GPIO4A7_SHIFT		= 12,
+	GPIO4A7_MASK		= 7,
+	GPIO4A7_GPIO		= 0,
+	GPIO4A7_FLASH1_CSN1,
+	GPIO4A7_HOST_DOUT13,
+	GPIO4A7_MAC_CSR,
+	GPIO4A7_SDIO1_CLKOUT,
+
+	GPIO4A6_SHIFT		= 8,
+	GPIO4A6_MASK		= 7,
+	GPIO4A6_GPIO		= 0,
+	GPIO4A6_FLASH1_CSN0,
+	GPIO4A6_HOST_DOUT12,
+	GPIO4A6_MAC_RXCLK,
+	GPIO4A6_SDIO1_CMD,
+
+	GPIO4A5_SHIFT		= 4,
+	GPIO4A5_MASK		= 3,
+	GPIO4A5_GPIO		= 0,
+	GPIO4A5_FLASH1_WRN,
+	GPIO4A5_HOST_DOUT11,
+	GPIO4A5_MAC_MDIO,
+
+	GPIO4A4_SHIFT		= 0,
+	GPIO4A4_MASK		= 7,
+	GPIO4A4_GPIO		= 0,
+	GPIO4A4_FLASH1_CLE,
+	GPIO4A4_HOST_DOUT10,
+	GPIO4A4_MAC_TXEN,
+	GPIO4A4_FLASH0_CSN7,
+};
+
+/* GRF_GPIO4BL_IOMUX */
+enum {
+	GPIO4B1_SHIFT		= 4,
+	GPIO4B1_MASK		= 7,
+	GPIO4B1_GPIO		= 0,
+	GPIO4B1_FLASH1_CSN2,
+	GPIO4B1_HOST_DOUT15,
+	GPIO4B1_MAC_TXCLK,
+	GPIO4B1_SDIO1_PWREN,
+
+	GPIO4B0_SHIFT		= 0,
+	GPIO4B0_MASK		= 7,
+	GPIO4B0_GPIO		= 0,
+	GPIO4B0_FLASH1_DQS,
+	GPIO4B0_HOST_DOUT14,
+	GPIO4B0_MAC_COL,
+	GPIO4B0_FLASH1_CSN3,
+};
+
 /* GRF_GPIO4C_IOMUX */
 enum {
 	GPIO4C7_SHIFT		= 14,
@@ -718,6 +875,40 @@  enum {
 	MSCH0_MAINPARTIALPOP_MASK = 1,
 };
 
+/* GRF_SOC_CON1 */
+enum {
+	RMII_MODE_SHIFT = 0xe,
+	RMII_MODE_MASK = 1,
+	RMII_MODE = 1,
+
+	GMAC_CLK_SEL_SHIFT	= 0xc,
+	GMAC_CLK_SEL_MASK	= 3,
+	GMAC_CLK_SEL_125M	= 0,
+	GMAC_CLK_SEL_25M,
+	GMAC_CLK_SEL_2_5M,
+
+	RMII_CLK_SEL_SHIFT	= 0xb,
+	RMII_CLK_SEL_MASK	= 1,
+	RMII_CLK_SEL_2_5M	= 0,
+	RMII_CLK_SEL_25M,
+
+	GMAC_SPEED_SHIFT	= 0xa,
+	GMAC_SPEED_MASK		= 1,
+	GMAC_SPEED_10M		= 0,
+	GMAC_SPEED_100M,
+
+	GMAC_FLOWCTRL_SHIFT	= 0x9,
+	GMAC_FLOWCTRL_MASK	= 1,
+
+	GMAC_PHY_INTF_SEL_SHIFT	= 0x6,
+	GMAC_PHY_INTF_SEL_MASK	= 0x7,
+	GMAC_PHY_INTF_SEL_RGMII	= 0x1,
+	GMAC_PHY_INTF_SEL_RMII	= 0x4,
+
+	HOST_REMAP_SHIFT	= 0x5,
+	HOST_REMAP_MASK		= 1
+};
+
 /* GRF_SOC_CON2 */
 enum {
 	UPCTL1_LPDDR3_ODT_EN_SHIFT = 0xd,
@@ -765,4 +956,41 @@  enum {
 	PWM_PWM			= 0,
 };
 
+/* GRF_SOC_CON3 */
+enum {
+	RXCLK_DLY_ENA_GMAC_SHIFT = 0xf,
+	RXCLK_DLY_ENA_GMAC_MASK = 1,
+	RXCLK_DLY_ENA_GMAC_DISABLE = 0,
+	RXCLK_DLY_ENA_GMAC_ENABLE,
+
+	TXCLK_DLY_ENA_GMAC_SHIFT = 0xe,
+	TXCLK_DLY_ENA_GMAC_MASK = 1,
+	TXCLK_DLY_ENA_GMAC_DISABLE = 0,
+	TXCLK_DLY_ENA_GMAC_ENABLE,
+
+	CLK_RX_DL_CFG_GMAC_SHIFT = 0x7,
+	CLK_RX_DL_CFG_GMAC_MASK = 0x7f,
+
+	CLK_TX_DL_CFG_GMAC_SHIFT = 0x0,
+	CLK_TX_DL_CFG_GMAC_MASK = 0x7f,
+};
+
+/* GPIO Bias settings */
+#define GPIO_BIAS_BITS 2
+#define GPIO_BIAS_MASK(x) (0x3 << (GPIO_BIAS_BITS * x))
+
+#define GPIO_BIAS_2mA(x) (0x0 << (GPIO_BIAS_BITS * x))
+#define GPIO_BIAS_4mA(x) (0x1 << (GPIO_BIAS_BITS * x))
+#define GPIO_BIAS_8mA(x) (0x2 << (GPIO_BIAS_BITS * x))
+#define GPIO_BIAS_12mA(x) (0x3 << (GPIO_BIAS_BITS * x))
+
+/* GPIO PU/PD settings */
+#define GPIO_PULL_BITS 2
+#define GPIO_PULL_MASK(x) (0x3 << (GPIO_PULL_BITS * x))
+
+#define GPIO_PULL_NORMAL(x) (0x0 << (GPIO_PULL_BITS * x))
+#define GPIO_PULL_UP(x) (0x1 << (GPIO_PULL_BITS * x))
+#define GPIO_PULL_DOWN(x) (0x2 << (GPIO_PULL_BITS * x))
+#define GPIO_PULL_REPEAT(x) (0x3 << (GPIO_PULL_BITS * x))
+
 #endif
diff --git a/arch/arm/include/asm/arch-rockchip/periph.h b/arch/arm/include/asm/arch-rockchip/periph.h
index fa6069b..239a274 100644
--- a/arch/arm/include/asm/arch-rockchip/periph.h
+++ b/arch/arm/include/asm/arch-rockchip/periph.h
@@ -38,6 +38,7 @@  enum periph_id {
 	PERIPH_ID_SDMMC1,
 	PERIPH_ID_SDMMC2,
 	PERIPH_ID_HDMI,
+	PERIPH_ID_GMAC,
 
 	PERIPH_ID_COUNT,
 
diff --git a/drivers/pinctrl/rockchip/pinctrl_rk3288.c b/drivers/pinctrl/rockchip/pinctrl_rk3288.c
index c432a00..e59f771 100644
--- a/drivers/pinctrl/rockchip/pinctrl_rk3288.c
+++ b/drivers/pinctrl/rockchip/pinctrl_rk3288.c
@@ -298,6 +298,103 @@  static void pinctrl_rk3288_sdmmc_config(struct rk3288_grf *grf, int mmc_id)
 	}
 }
 
+static void pinctrl_rk3288_gmac_config(struct rk3288_grf *grf, int gmac_id)
+{
+	switch (gmac_id) {
+	case PERIPH_ID_GMAC:
+		rk_clrsetreg(&grf->gpio3dl_iomux,
+			     GPIO3D3_MASK << GPIO3D3_SHIFT |
+			     GPIO3D2_MASK << GPIO3D2_SHIFT |
+			     GPIO3D2_MASK << GPIO3D1_SHIFT |
+			     GPIO3D0_MASK << GPIO3D0_SHIFT,
+			     GPIO3D3_MAC_RXD3 << GPIO3D3_SHIFT |
+			     GPIO3D2_MAC_RXD2 << GPIO3D2_SHIFT |
+			     GPIO3D1_MAC_TXD3 << GPIO3D1_SHIFT |
+			     GPIO3D0_MAC_TXD2 << GPIO3D0_SHIFT);
+
+		rk_clrsetreg(&grf->gpio3dh_iomux,
+			     GPIO3D7_MASK << GPIO3D7_SHIFT |
+			     GPIO3D6_MASK << GPIO3D6_SHIFT |
+			     GPIO3D5_MASK << GPIO3D5_SHIFT |
+			     GPIO3D4_MASK << GPIO3D4_SHIFT,
+			     GPIO3D7_MAC_RXD1 << GPIO3D7_SHIFT |
+			     GPIO3D6_MAC_RXD0 << GPIO3D6_SHIFT |
+			     GPIO3D5_MAC_TXD1 << GPIO3D5_SHIFT |
+			     GPIO3D4_MAC_TXD0 << GPIO3D4_SHIFT);
+
+		/* switch the Tx pins to 12ma drive-strength */
+		rk_clrsetreg(&grf->gpio1_e[2][3],
+			     GPIO_BIAS_MASK(0) | GPIO_BIAS_MASK(1) |
+			     GPIO_BIAS_MASK(4) | GPIO_BIAS_MASK(5),
+			     GPIO_BIAS_12mA(0) | GPIO_BIAS_12mA(1) |
+			     GPIO_BIAS_12mA(4) | GPIO_BIAS_12mA(5));
+
+		/* Set normal pull for all GPIO3D pins */
+		rk_clrsetreg(&grf->gpio1_p[2][3],
+			     GPIO_PULL_MASK(0) | GPIO_PULL_MASK(1) |
+			     GPIO_PULL_MASK(2) | GPIO_PULL_MASK(3) |
+			     GPIO_PULL_MASK(4) | GPIO_PULL_MASK(5) |
+			     GPIO_PULL_MASK(6) | GPIO_PULL_MASK(7),
+			     GPIO_PULL_NORMAL(0) | GPIO_PULL_NORMAL(1) |
+			     GPIO_PULL_NORMAL(2) | GPIO_PULL_NORMAL(3) |
+			     GPIO_PULL_NORMAL(4) | GPIO_PULL_NORMAL(5) |
+			     GPIO_PULL_NORMAL(6) | GPIO_PULL_NORMAL(7));
+
+		rk_clrsetreg(&grf->gpio4al_iomux,
+			     GPIO4A3_MASK << GPIO4A3_SHIFT |
+			     GPIO4A1_MASK << GPIO4A1_SHIFT |
+			     GPIO4A0_MASK << GPIO4A0_SHIFT,
+			     GPIO4A3_MAC_CLK << GPIO4A3_SHIFT |
+			     GPIO4A1_MAC_TXDV << GPIO4A1_SHIFT |
+			     GPIO4A0_MAC_MDC << GPIO4A0_SHIFT);
+
+		rk_clrsetreg(&grf->gpio4ah_iomux,
+			     GPIO4A6_MASK << GPIO4A6_SHIFT |
+			     GPIO4A5_MASK << GPIO4A5_SHIFT |
+			     GPIO4A4_MASK << GPIO4A4_SHIFT,
+			     GPIO4A6_MAC_RXCLK << GPIO4A6_SHIFT |
+			     GPIO4A5_MAC_MDIO << GPIO4A5_SHIFT |
+			     GPIO4A4_MAC_TXEN << GPIO4A4_SHIFT);
+
+		/* switch GPIO4A4 to 12ma drive-strength */
+		rk_clrsetreg(&grf->gpio1_e[3][0],
+			     GPIO_BIAS_MASK(4),
+			     GPIO_BIAS_12mA(4));
+		/* Set normal pull for all GPIO4A pins */
+		rk_clrsetreg(&grf->gpio1_p[3][0],
+			     GPIO_PULL_MASK(0) | GPIO_PULL_MASK(1) |
+			     GPIO_PULL_MASK(2) | GPIO_PULL_MASK(3) |
+			     GPIO_PULL_MASK(4) | GPIO_PULL_MASK(5) |
+			     GPIO_PULL_MASK(6) | GPIO_PULL_MASK(7),
+			     GPIO_PULL_NORMAL(0) | GPIO_PULL_NORMAL(1) |
+			     GPIO_PULL_NORMAL(2) | GPIO_PULL_NORMAL(3) |
+			     GPIO_PULL_NORMAL(4) | GPIO_PULL_NORMAL(5) |
+			     GPIO_PULL_NORMAL(6) | GPIO_PULL_NORMAL(7));
+
+		/* Assuming GPIO4B0_GPIO is phy-reset*/
+		rk_clrsetreg(&grf->gpio4bl_iomux,
+			     GPIO4B1_MASK << GPIO4B1_SHIFT |
+			     GPIO4B0_MASK << GPIO4B0_SHIFT,
+			     GPIO4B1_MAC_TXCLK << GPIO4B1_SHIFT|
+			     GPIO4B0_GPIO << GPIO4B0_SHIFT);
+
+		/* switch GPIO4B1 to 12ma drive-strength */
+		rk_clrsetreg(&grf->gpio1_e[3][1],
+			     GPIO_BIAS_MASK(1),
+			     GPIO_BIAS_12mA(1));
+
+		/* Set pull normal for GPIO4B1, pull up for GPIO4B0 */
+		rk_clrsetreg(&grf->gpio1_p[3][1],
+			     GPIO_PULL_MASK(0) | GPIO_PULL_MASK(1),
+			     GPIO_PULL_UP(0) | GPIO_PULL_NORMAL(1));
+
+		break;
+	default:
+		debug("gmac id = %d iomux error!\n", gmac_id);
+		break;
+	}
+}
+
 static void pinctrl_rk3288_hdmi_config(struct rk3288_grf *grf, int hdmi_id)
 {
 	switch (hdmi_id) {
@@ -354,6 +451,9 @@  static int rk3288_pinctrl_request(struct udevice *dev, int func, int flags)
 	case PERIPH_ID_SDMMC1:
 		pinctrl_rk3288_sdmmc_config(priv->grf, func);
 		break;
+	case PERIPH_ID_GMAC:
+		pinctrl_rk3288_gmac_config(priv->grf, func);
+		break;
 	case PERIPH_ID_HDMI:
 		pinctrl_rk3288_hdmi_config(priv->grf, func);
 		break;
@@ -376,6 +476,8 @@  static int rk3288_pinctrl_get_periph_id(struct udevice *dev,
 		return -EINVAL;
 
 	switch (cell[1]) {
+	case 27:
+		return PERIPH_ID_GMAC;
 	case 44:
 		return PERIPH_ID_SPI0;
 	case 45: