rk3328 pinctrl drvier and sdmmc power fix

PATCH New Kever Yang 3 participants 19 messages May 17, 2017 – Jun 9, 2017

Subsystems (3)

Arm (32-bit) Arm Rockchip Pin control

lore

Share & tools

lore

Kever Yang

author

2017-05-17 03:44 UTC #0 lore raw

cover rk3328 pinctrl drvier and sdmmc power fix

Move pinctrl registers definition in GRF header file, clean code with prefix like other Rockchip SoCs. Use Fixed regulator(gpio) for sdmmc-pwren instead of signal controled by mmc controller.

Kever Yang (6):
rockchip: pinctrl: move rk3328 grf reg definition in header file
rockchip: pinctrl: rk3328: do not set io routing
rockchip: evb-rk3328: set uart2 and sdmmc io routing
rockchip: pinctrl: rk3328: use gpio instead of sdmmc-pwren
rockchip: evb-rk3328: enable boot on regulator
rockchip: dts: rk3328-evb: add sdmmc-pwren regulator

 arch/arm/dts/rk3328-evb.dts                     |   8 +
 arch/arm/include/asm/arch-rockchip/grf_rk3328.h | 114 ++++++++++++
 board/rockchip/evb_rk3328/evb-rk3328.c          |  20 ++-
 drivers/pinctrl/rockchip/pinctrl_rk3328.c       | 226 +++++++-----------------
 4 files changed, 205 insertions(+), 163 deletions(-)

Signature · 1 line

1.9.1

Kever Yang

author

2017-05-17 03:44 UTC #1 lore raw

PATCH 1/6 rockchip: pinctrl: move rk3328 grf reg definition in header file

Move GRF register bit definition into GRF header file, remove 'GRF_' prefix and add 'GPIOmXn_' as prefix for bit meaning.

Signed-off-by Kever Yang <[email protected]>

---

 arch/arm/include/asm/arch-rockchip/grf_rk3328.h | 114 +++++++++++++
 drivers/pinctrl/rockchip/pinctrl_rk3328.c       | 205 ++++++------------------
 2 files changed, 164 insertions(+), 155 deletions(-)

arch/arm/include/asm/arch-rockchip/grf_rk3328.h +114 -0

index 2776cef..40a4de0 100644
@@ -131,4 +131,118 @@ struct rk3328_sgrf_regs {
 };
 check_member(rk3328_sgrf_regs, hdcp_key_access_mask, 0x2a0);
 
+enum {
+	/* GPIO0A_IOMUX */
+	GPIO0A5_SEL_SHIFT	= 10,
+	GPIO0A5_SEL_MASK	= 3 << GPIO0A5_SEL_SHIFT,
+	GPIO0A5_I2C3_SCL	= 2,
+
+	GPIO0A6_SEL_SHIFT	= 12,
+	GPIO0A6_SEL_MASK	= 3 << GPIO0A6_SEL_SHIFT,
+	GPIO0A6_I2C3_SDA	= 2,
+
+	GPIO0A7_SEL_SHIFT	= 14,
+	GPIO0A7_SEL_MASK	= 3 << GPIO0A7_SEL_SHIFT,
+	GPIO0A7_EMMC_DATA0	= 2,
+
+	/* GPIO0D_IOMUX*/
+	GPIO0D6_SEL_SHIFT	= 12,
+	GPIO0D6_SEL_MASK	= 3 << GPIO0D6_SEL_SHIFT,
+	GPIO0D6_GPIO		= 0,
+	GPIO0D6_SDMMC0_PWRENM1	= 3,
+
+	/* GPIO1A_IOMUX */
+	GPIO1A0_SEL_SHIFT	= 0,
+	GPIO1A0_SEL_MASK	= 0x3fff << GPIO1A0_SEL_SHIFT,
+	GPIO1A0_CARD_DATA_CLK_CMD_DETN	= 0x1555,
+
+	/* GPIO2A_IOMUX */
+	GPIO2A0_SEL_SHIFT	= 0,
+	GPIO2A0_SEL_MASK	= 3 << GPIO2A0_SEL_SHIFT,
+	GPIO2A0_UART2_TX_M1	= 1,
+
+	GPIO2A1_SEL_SHIFT	= 2,
+	GPIO2A1_SEL_MASK	= 3 << GPIO2A1_SEL_SHIFT,
+	GPIO2A1_UART2_RX_M1	= 1,
+
+	GPIO2A2_SEL_SHIFT	= 4,
+	GPIO2A2_SEL_MASK	= 3 << GPIO2A2_SEL_SHIFT,
+	GPIO2A2_PWM_IR		= 1,
+
+	GPIO2A4_SEL_SHIFT	= 8,
+	GPIO2A4_SEL_MASK	= 3 << GPIO2A4_SEL_SHIFT,
+	GPIO2A4_PWM_0		= 1,
+	GPIO2A4_I2C1_SDA,
+
+	GPIO2A5_SEL_SHIFT	= 10,
+	GPIO2A5_SEL_MASK	= 3 << GPIO2A5_SEL_SHIFT,
+	GPIO2A5_PWM_1		= 1,
+	GPIO2A5_I2C1_SCL,
+
+	GPIO2A6_SEL_SHIFT	= 12,
+	GPIO2A6_SEL_MASK	= 3 << GPIO2A6_SEL_SHIFT,
+	GPIO2A6_PWM_2		= 1,
+
+	GPIO2A7_SEL_SHIFT	= 14,
+	GPIO2A7_SEL_MASK	= 3 << GPIO2A7_SEL_SHIFT,
+	GPIO2A7_GPIO		= 0,
+	GPIO2A7_SDMMC0_PWRENM0,
+
+	/* GPIO2BL_IOMUX */
+	GPIO2BL0_SEL_SHIFT	= 0,
+	GPIO2BL0_SEL_MASK	= 0x3f << GPIO2BL0_SEL_SHIFT,
+	GPIO2BL0_SPI_CLK_TX_RX_M0	= 0x15,
+
+	GPIO2BL3_SEL_SHIFT	= 6,
+	GPIO2BL3_SEL_MASK	= 3 << GPIO2BL3_SEL_SHIFT,
+	GPIO2BL3_SPI_CSN0_M0	= 1,
+
+	GPIO2BL4_SEL_SHIFT	= 8,
+	GPIO2BL4_SEL_MASK	= 3 << GPIO2BL4_SEL_SHIFT,
+	GPIO2BL4_SPI_CSN1_M0	= 1,
+
+	GPIO2BL5_SEL_SHIFT	= 10,
+	GPIO2BL5_SEL_MASK	= 3 << GPIO2BL5_SEL_SHIFT,
+	GPIO2BL5_I2C2_SDA	= 1,
+
+	GPIO2BL6_SEL_SHIFT	= 12,
+	GPIO2BL6_SEL_MASK	= 3 << GPIO2BL6_SEL_SHIFT,
+	GPIO2BL6_I2C2_SCL	= 1,
+
+	/* GPIO2D_IOMUX */
+	GPIO2D0_SEL_SHIFT	= 0,
+	GPIO2D0_SEL_MASK	= 3 << GPIO2D0_SEL_SHIFT,
+	GPIO2D0_I2C0_SCL	= 1,
+
+	GPIO2D1_SEL_SHIFT	= 2,
+	GPIO2D1_SEL_MASK	= 3 << GPIO2D1_SEL_SHIFT,
+	GPIO2D1_I2C0_SDA	= 1,
+
+	GPIO2D4_SEL_SHIFT	= 8,
+	GPIO2D4_SEL_MASK	= 0xff << GPIO2D4_SEL_SHIFT,
+	GPIO2D4_EMMC_DATA1234	= 0xaa,
+
+	/* GPIO3C_IOMUX */
+	GPIO3C0_SEL_SHIFT	= 0,
+	GPIO3C0_SEL_MASK	= 0x3fff << GPIO3C0_SEL_SHIFT,
+	GPIO3C0_EMMC_DATA567_PWR_CLK_RSTN_CMD	= 0x2aaa,
+
+	/* COM_IOMUX */
+	UART2_IOMUX_SEL_SHIFT	= 0,
+	UART2_IOMUX_SEL_MASK	= 3 << UART2_IOMUX_SEL_SHIFT,
+	UART2_IOMUX_SEL_M0		= 0,
+	UART2_IOMUX_SEL_M1,
+
+	SPI_IOMUX_SEL_SHIFT = 4,
+	SPI_IOMUX_SEL_MASK	= 3 << SPI_IOMUX_SEL_SHIFT,
+	SPI_IOMUX_SEL_M0	= 0,
+	SPI_IOMUX_SEL_M1,
+	SPI_IOMUX_SEL_M2,
+
+	CARD_IOMUX_SEL_SHIFT	= 7,
+	CARD_IOMUX_SEL_MASK		= 1 << CARD_IOMUX_SEL_SHIFT,
+	CARD_IOMUX_SEL_M0		= 0,
+	CARD_IOMUX_SEL_M1,
+};
+
 #endif	/* __SOC_ROCKCHIP_RK3328_GRF_H__ */

drivers/pinctrl/rockchip/pinctrl_rk3328.c +50 ~~-155~~

281 lines not on this page · raw message

Signature · 1 line

1.9.1

Simon Glass

2017-05-20 02:29 UTC #2 lore raw

PATCH 1/6 rockchip: pinctrl: move rk3328 grf reg definition in header file

Quoted 10 lines · Kever Yang

Move GRF register bit definition into GRF header file, remove 'GRF_' prefix and add 'GPIOmXn_' as prefix for bit meaning.
Signed-off-by Kever Yang <[email protected]>
---
 arch/arm/include/asm/arch-rockchip/grf_rk3328.h | 114 +++++++++++++
 drivers/pinctrl/rockchip/pinctrl_rk3328.c       | 205 ++++++------------------
 2 files changed, 164 insertions(+), 155 deletions(-)

Acked-by Simon Glass <[email protected]>

Kever Yang

author

2017-05-17 03:44 UTC #3 lore raw

PATCH 2/6 rockchip: pinctrl: rk3328: do not set io routing

In reply to Kever Yang

In rk3328, some function pin may have more than one choice, and muxed with more than one IO, for example, the UART2 controller IO, TX and RX, have 3 choice(setting in com_iomux):
- M0 which mux with GPIO1A0/GPIO1A1
- M1 which mux with GPIO2A0/GPIO2A1
- usb2phy which mux with USB2.0 DP/DM pin.

We should not decide which group to use in pinctrl driver, for it may be different in different board, it should goes to board file, and the pinctrl file should setting correct iomux depends on the com_iomux value.

Signed-off-by Kever Yang <[email protected]>

---

 arch/arm/include/asm/arch-rockchip/grf_rk3328.h | 30 ++++++++--------
 drivers/pinctrl/rockchip/pinctrl_rk3328.c       | 47 +++++++++++++++----------
 2 files changed, 43 insertions(+), 34 deletions(-)

arch/arm/include/asm/arch-rockchip/grf_rk3328.h +15 ~~-15~~

index 40a4de0..f0a0781 100644
@@ -228,21 +228,21 @@ enum {
 	GPIO3C0_EMMC_DATA567_PWR_CLK_RSTN_CMD	= 0x2aaa,
 
 	/* COM_IOMUX */
-	UART2_IOMUX_SEL_SHIFT	= 0,
-	UART2_IOMUX_SEL_MASK	= 3 << UART2_IOMUX_SEL_SHIFT,
-	UART2_IOMUX_SEL_M0		= 0,
-	UART2_IOMUX_SEL_M1,
-
-	SPI_IOMUX_SEL_SHIFT = 4,
-	SPI_IOMUX_SEL_MASK	= 3 << SPI_IOMUX_SEL_SHIFT,
-	SPI_IOMUX_SEL_M0	= 0,
-	SPI_IOMUX_SEL_M1,
-	SPI_IOMUX_SEL_M2,
-
-	CARD_IOMUX_SEL_SHIFT	= 7,
-	CARD_IOMUX_SEL_MASK		= 1 << CARD_IOMUX_SEL_SHIFT,
-	CARD_IOMUX_SEL_M0		= 0,
-	CARD_IOMUX_SEL_M1,
+	IOMUX_SEL_UART2_SHIFT	= 0,
+	IOMUX_SEL_UART2_MASK	= 3 << IOMUX_SEL_UART2_SHIFT,
+	IOMUX_SEL_UART2_M0	= 0,
+	IOMUX_SEL_UART2_M1,
+
+	IOMUX_SEL_SPI_SHIFT	= 4,
+	IOMUX_SEL_SPI_MASK	= 3 << IOMUX_SEL_SPI_SHIFT,
+	IOMUX_SEL_SPI_M0	= 0,
+	IOMUX_SEL_SPI_M1,
+	IOMUX_SEL_SPI_M2,
+
+	IOMUX_SEL_SDMMC_SHIFT	= 7,
+	IOMUX_SEL_SDMMC_MASK	= 1 << IOMUX_SEL_SDMMC_SHIFT,
+	IOMUX_SEL_SDMMC_M0	= 0,
+	IOMUX_SEL_SDMMC_M1,
 };
 
 #endif	/* __SOC_ROCKCHIP_RK3328_GRF_H__ */

drivers/pinctrl/rockchip/pinctrl_rk3328.c +28 ~~-19~~

74 lines not on this page · raw message

Signature · 1 line

1.9.1

Simon Glass

2017-05-20 02:29 UTC #4 lore raw

PATCH 2/6 rockchip: pinctrl: rk3328: do not set io routing

Quoted 12 lines · Kever Yang

In rk3328, some function pin may have more than one choice, and muxed with more than one IO, for example, the UART2 controller IO, TX and RX, have 3 choice(setting in com_iomux):
- M0 which mux with GPIO1A0/GPIO1A1
- M1 which mux with GPIO2A0/GPIO2A1
- usb2phy which mux with USB2.0 DP/DM pin.
We should not decide which group to use in pinctrl driver, for it may be different in different board, it should goes to board file, and the pinctrl file should setting correct iomux depends on the com_iomux value.

Quoted 7 lines

Signed-off-by Kever Yang <[email protected]>

---

 arch/arm/include/asm/arch-rockchip/grf_rk3328.h | 30 ++++++++--------
 drivers/pinctrl/rockchip/pinctrl_rk3328.c       | 47 +++++++++++++++----------
 2 files changed, 43 insertions(+), 34 deletions(-)

Acked-by Simon Glass <[email protected]>

Kever Yang

author

2017-05-17 03:44 UTC #5 lore raw

PATCH 3/6 rockchip: evb-rk3328: set uart2 and sdmmc io routing

In reply to Kever Yang

We should set these IO routing in board file.

Signed-off-by Kever Yang <[email protected]>

---

 board/rockchip/evb_rk3328/evb-rk3328.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

board/rockchip/evb_rk3328/evb-rk3328.c +12 -0

index a7895cb..d9dc782 100644
@@ -5,7 +5,10 @@
  */
 
 #include <common.h>
+#include <asm/arch/hardware.h>
+#include <asm/arch/grf_rk3328.h>
 #include <asm/armv8/mmu.h>
+#include <asm/io.h>
 #include <dwc3-uboot.h>
 #include <usb.h>

@@ -13,6 +16,15 @@ DECLARE_GLOBAL_DATA_PTR;
 
 int board_init(void)
 {
+#define GRF_BASE	0xff100000
+	struct rk3328_grf_regs * const grf = (void *)GRF_BASE;
+
+	/* uart2 select m1, sdcard select m1*/
+	rk_clrsetreg(&grf->com_iomux,
+		     IOMUX_SEL_UART2_MASK | IOMUX_SEL_SDMMC_MASK,
+		     IOMUX_SEL_UART2_M1 << IOMUX_SEL_UART2_SHIFT |
+		     IOMUX_SEL_SDMMC_M1 << IOMUX_SEL_SDMMC_SHIFT);
+
 	return 0;
 }

Signature · 1 line

1.9.1

Simon Glass

2017-05-20 02:29 UTC #6 lore raw

PATCH 3/6 rockchip: evb-rk3328: set uart2 and sdmmc io routing

Hi Kever,

Quoted 35 lines · Kever Yang

In rk3328, some function pin may have more than one choice, and muxed with more than one IO, for example, the UART2 controller IO, TX and RX, have 3 choice(setting in com_iomux):
- M0 which mux with GPIO1A0/GPIO1A1
- M1 which mux with GPIO2A0/GPIO2A1
- usb2phy which mux with USB2.0 DP/DM pin.
We should set these IO routing in board file.
Signed-off-by Kever Yang <[email protected]>
---
 board/rockchip/evb_rk3328/evb-rk3328.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)
board/rockchip/evb_rk3328/evb-rk3328.c +4 -0
index a7895cb..d9dc782 100644
@@ -5,7 +5,10 @@
  */
 
 #include <common.h>
+#include <asm/arch/hardware.h>
+#include <asm/arch/grf_rk3328.h>
 #include <asm/armv8/mmu.h>
+#include <asm/io.h>
 #include <dwc3-uboot.h>
 #include <usb.h>
 
@@ -13,6 +16,15 @@ DECLARE_GLOBAL_DATA_PTR;
 
 int board_init(void)
 {
+#define GRF_BASE       0xff100000

board/rockchip/evb_rk3328/evb-rk3328.c DECLARE_GLOBAL_DATA_PTR; · line 29

Quoted 10 lines

board/rockchip/evb_rk3328/evb-rk3328.c +8 -0

+       struct rk3328_grf_regs * const grf = (void *)GRF_BASE;
+
+       /* uart2 select m1, sdcard select m1*/
+       rk_clrsetreg(&grf->com_iomux,
+                    IOMUX_SEL_UART2_MASK | IOMUX_SEL_SDMMC_MASK,
+                    IOMUX_SEL_UART2_M1 << IOMUX_SEL_UART2_SHIFT |
+                    IOMUX_SEL_SDMMC_M1 << IOMUX_SEL_SDMMC_SHIFT);
+
        return 0;
 }

This needs to be done via a call to some sort of driver. The above hack is OK in SPL but not in U-Boot proper.

See my comments elsewhere about using a misc driver with an IOCTL interface to do this sort of thing. Although here I wonder why you cannot use pinctrl?

Regards,
Simon

Kever Yang

author

2017-05-24 02:35 UTC #7 lore raw

PATCH 3/6 rockchip: evb-rk3328: set uart2 and sdmmc io routing

Hi Simon,

Quoted 35 lines · Simon Glass

Hi Kever,
Quoted 32 lines · Kever Yang
In rk3328, some function pin may have more than one choice, and muxed with more than one IO, for example, the UART2 controller IO, TX and RX, have 3 choice(setting in com_iomux):
- M0 which mux with GPIO1A0/GPIO1A1
- M1 which mux with GPIO2A0/GPIO2A1
- usb2phy which mux with USB2.0 DP/DM pin.
We should set these IO routing in board file.
Signed-off-by Kever Yang <[email protected]>
---
  board/rockchip/evb_rk3328/evb-rk3328.c | 12 ++++++++++++
  1 file changed, 12 insertions(+)
board/rockchip/evb_rk3328/evb-rk3328.c +3 -0
index a7895cb..d9dc782 100644
@@ -5,7 +5,10 @@
   */
 
  #include <common.h>
+#include <asm/arch/hardware.h>
+#include <asm/arch/grf_rk3328.h>
  #include <asm/armv8/mmu.h>
+#include <asm/io.h>
  #include <dwc3-uboot.h>
  #include <usb.h>
 
@@ -13,6 +16,15 @@ DECLARE_GLOBAL_DATA_PTR;
 

Quoted 15 lines

board/rockchip/evb_rk3328/evb-rk3328.c DECLARE_GLOBAL_DATA_PTR; · line 29

Quoted 13 lines

board/rockchip/evb_rk3328/evb-rk3328.c +9 -0

  int board_init(void)
  {
+#define GRF_BASE       0xff100000
+       struct rk3328_grf_regs * const grf = (void *)GRF_BASE;
+
+       /* uart2 select m1, sdcard select m1*/
+       rk_clrsetreg(&grf->com_iomux,
+                    IOMUX_SEL_UART2_MASK | IOMUX_SEL_SDMMC_MASK,
+                    IOMUX_SEL_UART2_M1 << IOMUX_SEL_UART2_SHIFT |
+                    IOMUX_SEL_SDMMC_M1 << IOMUX_SEL_SDMMC_SHIFT);
+
         return 0;
  }

This needs to be done via a call to some sort of driver. The above hack is OK in SPL but not in U-Boot proper.

Yes, SPL also needs this. I thinks here should be the right place before there is a SPL for rk3328.

Quoted 4 lines

See my comments elsewhere about using a misc driver with an IOCTL interface to do this sort of thing. Although here I wonder why you cannot use pinctrl?

This is different from traditional pinctrl, kernel also still not have final solution on this, see [0], and some people think it should be done in boot loader.

Thanks,
- Kever
[0]
http://lists.infradead.org/pipermail/linux-rockchip/2016-August/011209.html

Quoted 4 lines

Regards,
Simon

Heiko Stuebner

maintainer

2017-05-26 22:28 UTC #8 lore raw

PATCH 3/6 rockchip: evb-rk3328: set uart2 and sdmmc io routing

Hi Kever,

Am Mittwoch, 24. Mai 2017, 10:35:04 CEST schrieb Kever Yang:

Quoted 53 lines · Simon Glass

Quoted 33 lines · Kever Yang
Quoted 32 lines
In rk3328, some function pin may have more than one choice, and muxed with more than one IO, for example, the UART2 controller IO, TX and RX, have 3 choice(setting in com_iomux):
- M0 which mux with GPIO1A0/GPIO1A1
- M1 which mux with GPIO2A0/GPIO2A1
- usb2phy which mux with USB2.0 DP/DM pin.
We should set these IO routing in board file.
Signed-off-by Kever Yang <[email protected]>
---
  board/rockchip/evb_rk3328/evb-rk3328.c | 12 ++++++++++++
  1 file changed, 12 insertions(+)
board/rockchip/evb_rk3328/evb-rk3328.c +3 -0
index a7895cb..d9dc782 100644
@@ -5,7 +5,10 @@
   */
 
  #include <common.h>
+#include <asm/arch/hardware.h>
+#include <asm/arch/grf_rk3328.h>
  #include <asm/armv8/mmu.h>
+#include <asm/io.h>
  #include <dwc3-uboot.h>
  #include <usb.h>
 
@@ -13,6 +16,15 @@ DECLARE_GLOBAL_DATA_PTR;
 
Quoted 15 lines
board/rockchip/evb_rk3328/evb-rk3328.c DECLARE_GLOBAL_DATA_PTR; · line 29
Quoted 13 lines
board/rockchip/evb_rk3328/evb-rk3328.c +9 -0
  int board_init(void)
  {
+#define GRF_BASE       0xff100000
+       struct rk3328_grf_regs * const grf = (void *)GRF_BASE;
+
+       /* uart2 select m1, sdcard select m1*/
+       rk_clrsetreg(&grf->com_iomux,
+                    IOMUX_SEL_UART2_MASK | IOMUX_SEL_SDMMC_MASK,
+                    IOMUX_SEL_UART2_M1 << IOMUX_SEL_UART2_SHIFT |
+                    IOMUX_SEL_SDMMC_M1 << IOMUX_SEL_SDMMC_SHIFT);
+
         return 0;
  }
This needs to be done via a call to some sort of driver. The above hack is OK in SPL but not in U-Boot proper.
Yes, SPL also needs this. I thinks here should be the right place before there is a SPL for rk3328.
Quoted 1 line

Quoted 7 lines

Quoted 3 lines
See my comments elsewhere about using a misc driver with an IOCTL interface to do this sort of thing. Although here I wonder why you cannot use pinctrl?
This is different from traditional pinctrl, kernel also still not have final solution on this, see [0], and some people think it should be done in boot loader.

Just to point out that thanks to David Wu we now have a solution [1] on the kernel side I'm pretty happy with - as part of the pinctrl driver.

Heiko

Quoted 2 lines

[0]
http://lists.infradead.org/pipermail/linux-rockchip/2016-August/011209.html

[1] https://www.spinics.net/lists/kernel/msg2517794.html

Simon Glass

2017-06-01 03:10 UTC #9 lore raw

PATCH 3/6 rockchip: evb-rk3328: set uart2 and sdmmc io routing

In reply to Kever Yang

Hi Kever,

Quoted 61 lines · Kever Yang

Hi Simon,
Quoted 38 lines · Simon Glass
Hi Kever,
Quoted 34 lines · Kever Yang
In rk3328, some function pin may have more than one choice, and muxed with more than one IO, for example, the UART2 controller IO, TX and RX, have 3 choice(setting in com_iomux):
- M0 which mux with GPIO1A0/GPIO1A1
- M1 which mux with GPIO2A0/GPIO2A1
- usb2phy which mux with USB2.0 DP/DM pin.
We should set these IO routing in board file.
Signed-off-by Kever Yang <[email protected]>
---
  board/rockchip/evb_rk3328/evb-rk3328.c | 12 ++++++++++++
  1 file changed, 12 insertions(+)
board/rockchip/evb_rk3328/evb-rk3328.c +3 -0
index a7895cb..d9dc782 100644
@@ -5,7 +5,10 @@
   */
 
  #include <common.h>
+#include <asm/arch/hardware.h>
+#include <asm/arch/grf_rk3328.h>
  #include <asm/armv8/mmu.h>
+#include <asm/io.h>
  #include <dwc3-uboot.h>
  #include <usb.h>
 
@@ -13,6 +16,15 @@ DECLARE_GLOBAL_DATA_PTR;
 
Quoted 16 lines
board/rockchip/evb_rk3328/evb-rk3328.c DECLARE_GLOBAL_DATA_PTR; · line 29
Quoted 13 lines
board/rockchip/evb_rk3328/evb-rk3328.c +9 -0
  int board_init(void)
  {
+#define GRF_BASE       0xff100000
+       struct rk3328_grf_regs * const grf = (void *)GRF_BASE;
+
+       /* uart2 select m1, sdcard select m1*/
+       rk_clrsetreg(&grf->com_iomux,
+                    IOMUX_SEL_UART2_MASK | IOMUX_SEL_SDMMC_MASK,
+                    IOMUX_SEL_UART2_M1 << IOMUX_SEL_UART2_SHIFT |
+                    IOMUX_SEL_SDMMC_M1 << IOMUX_SEL_SDMMC_SHIFT);
+
         return 0;
  }
This needs to be done via a call to some sort of driver. The above hack is OK in SPL but not in U-Boot proper.

Quoted 2 lines

Yes, SPL also needs this. I thinks here should be the right place before there is a SPL for rk3328.

But if you are booting from an SD card, how can you need a mux to select it? Surely the boot ROM must set it up or you would not be able to boot from MMC?

When will there be SPL for rk3328?

Quoted 10 lines

Quoted 5 lines
See my comments elsewhere about using a misc driver with an IOCTL interface to do this sort of thing. Although here I wonder why you cannot use pinctrl?
This is different from traditional pinctrl, kernel also still not have final solution on this, see [0], and some people think it should be done in boot loader.

How about putting it in grf syscon driver?

Quoted 13 lines

Thanks,
- Kever
[0]
http://lists.infradead.org/pipermail/linux-rockchip/2016-August/011209.html
Quoted 5 lines
Regards,
Simon

Kever Yang

author

2017-06-07 03:28 UTC #10 lore raw

PATCH 3/6 rockchip: evb-rk3328: set uart2 and sdmmc io routing

Simon,

Quoted 60 lines · Simon Glass

Hi Kever,
Quoted 57 lines · Kever Yang
Hi Simon,
Quoted 51 lines · Simon Glass
Hi Kever,
Quoted 36 lines · Kever Yang
In rk3328, some function pin may have more than one choice, and muxed with more than one IO, for example, the UART2 controller IO, TX and RX, have 3 choice(setting in com_iomux):
- M0 which mux with GPIO1A0/GPIO1A1
- M1 which mux with GPIO2A0/GPIO2A1
- usb2phy which mux with USB2.0 DP/DM pin.
We should set these IO routing in board file.
Signed-off-by Kever Yang <[email protected]>
---
   board/rockchip/evb_rk3328/evb-rk3328.c | 12 ++++++++++++
   1 file changed, 12 insertions(+)
board/rockchip/evb_rk3328/evb-rk3328.c +4 -0
index a7895cb..d9dc782 100644
@@ -5,7 +5,10 @@
    */
 
   #include <common.h>
+#include <asm/arch/hardware.h>
+#include <asm/arch/grf_rk3328.h>
   #include <asm/armv8/mmu.h>
+#include <asm/io.h>
   #include <dwc3-uboot.h>
   #include <usb.h>
 
@@ -13,6 +16,15 @@ DECLARE_GLOBAL_DATA_PTR;
 
   int board_init(void)
   {
+#define GRF_BASE       0xff100000
board/rockchip/evb_rk3328/evb-rk3328.c DECLARE_GLOBAL_DATA_PTR; · line 29
Quoted 10 lines
board/rockchip/evb_rk3328/evb-rk3328.c +8 -0
+       struct rk3328_grf_regs * const grf = (void *)GRF_BASE;
+
+       /* uart2 select m1, sdcard select m1*/
+       rk_clrsetreg(&grf->com_iomux,
+                    IOMUX_SEL_UART2_MASK | IOMUX_SEL_SDMMC_MASK,
+                    IOMUX_SEL_UART2_M1 << IOMUX_SEL_UART2_SHIFT |
+                    IOMUX_SEL_SDMMC_M1 << IOMUX_SEL_SDMMC_SHIFT);
+
          return 0;
   }
This needs to be done via a call to some sort of driver. The above hack is OK in SPL but not in U-Boot proper.

Quoted 5 lines

Quoted 2 lines
Yes, SPL also needs this. I thinks here should be the right place before there is a SPL for rk3328.
But if you are booting from an SD card, how can you need a mux to select it? Surely the boot ROM must set it up or you would not be able to boot from MMC?

If we need to boot from SD card, then we need to follow the boot ROM setting,
and hardware also need to follow it.
There is another case, the default SD card pin is used for other function in hardware,
then we use the alternative pin mux, we can't boot from SD card in bootrom, but we still can use SD card in later stage like U-Boot and kernel.

Thanks,
- Kever

Quoted 23 lines

When will there be SPL for rk3328?
Quoted 8 lines
Quoted 4 lines
See my comments elsewhere about using a misc driver with an IOCTL interface to do this sort of thing. Although here I wonder why you cannot use pinctrl?
This is different from traditional pinctrl, kernel also still not have final solution on this, see [0], and some people think it should be done in boot loader.
How about putting it in grf syscon driver?
Quoted 10 lines
Thanks,
- Kever
[0]
http://lists.infradead.org/pipermail/linux-rockchip/2016-August/011209.html
Quoted 4 lines
Regards,
Simon

Simon Glass

2017-06-09 12:27 UTC #11 lore raw

PATCH 3/6 rockchip: evb-rk3328: set uart2 and sdmmc io routing

Hi Kever,

Quoted 79 lines · Kever Yang

Simon,
Quoted 66 lines · Simon Glass
Hi Kever,
Quoted 62 lines · Kever Yang
Hi Simon,
Quoted 54 lines · Simon Glass
Hi Kever,
Quoted 37 lines · Kever Yang
In rk3328, some function pin may have more than one choice, and muxed with more than one IO, for example, the UART2 controller IO, TX and RX, have 3 choice(setting in com_iomux):
- M0 which mux with GPIO1A0/GPIO1A1
- M1 which mux with GPIO2A0/GPIO2A1
- usb2phy which mux with USB2.0 DP/DM pin.
We should set these IO routing in board file.
Signed-off-by Kever Yang <[email protected]>
---
   board/rockchip/evb_rk3328/evb-rk3328.c | 12 ++++++++++++
   1 file changed, 12 insertions(+)
board/rockchip/evb_rk3328/evb-rk3328.c +4 -0
index a7895cb..d9dc782 100644
@@ -5,7 +5,10 @@
    */
 
   #include <common.h>
+#include <asm/arch/hardware.h>
+#include <asm/arch/grf_rk3328.h>
   #include <asm/armv8/mmu.h>
+#include <asm/io.h>
   #include <dwc3-uboot.h>
   #include <usb.h>
 
@@ -13,6 +16,15 @@ DECLARE_GLOBAL_DATA_PTR;
 
   int board_init(void)
   {
+#define GRF_BASE       0xff100000
board/rockchip/evb_rk3328/evb-rk3328.c DECLARE_GLOBAL_DATA_PTR; · line 29
Quoted 10 lines
board/rockchip/evb_rk3328/evb-rk3328.c +8 -0
+       struct rk3328_grf_regs * const grf = (void *)GRF_BASE;
+
+       /* uart2 select m1, sdcard select m1*/
+       rk_clrsetreg(&grf->com_iomux,
+                    IOMUX_SEL_UART2_MASK | IOMUX_SEL_SDMMC_MASK,
+                    IOMUX_SEL_UART2_M1 << IOMUX_SEL_UART2_SHIFT |
+                    IOMUX_SEL_SDMMC_M1 << IOMUX_SEL_SDMMC_SHIFT);
+
          return 0;
   }
This needs to be done via a call to some sort of driver. The above hack is OK in SPL but not in U-Boot proper.
Quoted 6 lines
Quoted 2 lines
Yes, SPL also needs this. I thinks here should be the right place before there is a SPL for rk3328.
But if you are booting from an SD card, how can you need a mux to select it? Surely the boot ROM must set it up or you would not be able to boot from MMC?

Quoted 8 lines

If we need to boot from SD card, then we need to follow the boot ROM setting,
and hardware also need to follow it.
There is another case, the default SD card pin is used for other function in hardware,
then we use the alternative pin mux, we can't boot from SD card in bootrom, but we still can use SD card in later stage like U-Boot and kernel.

OK, understood, thanks. Can you put it in the pinctrl driver?

- Simon

Quoted 32 lines

Thanks,
- Kever
Quoted 27 lines
When will there be SPL for rk3328?
Quoted 9 lines
Quoted 4 lines
See my comments elsewhere about using a misc driver with an IOCTL interface to do this sort of thing. Although here I wonder why you cannot use pinctrl?
This is different from traditional pinctrl, kernel also still not have final solution on this, see [0], and some people think it should be done in boot loader.
How about putting it in grf syscon driver?
Quoted 12 lines
Thanks,
- Kever
[0]
http://lists.infradead.org/pipermail/linux-rockchip/2016-August/011209.html
Quoted 5 lines
Regards,
Simon

Kever Yang

author

2017-05-17 03:44 UTC #12 lore raw

PATCH 4/6 rockchip: pinctrl: rk3328: use gpio instead of sdmmc-pwren

In reply to Kever Yang

SDMMC-PWREN is a pin to control voltage for SDMMC IO, it may be high active or low active, the dwmmc driver always assume the sdmmc-pwren as high active.

Kernel treat this pin as fixed regulator instead of a pin from controller, and then it can set in dts file upon board schematic, that's a good solution, we can also do this in u-boot.

Signed-off-by Kever Yang <[email protected]>

---

 drivers/pinctrl/rockchip/pinctrl_rk3328.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

drivers/pinctrl/rockchip/pinctrl_rk3328.c +2 -4

index 716d02a..0995d9f 100644
@@ -184,13 +184,11 @@ static void pinctrl_rk3328_sdmmc_config(struct rk3328_grf_regs *grf,
 		if (com_iomux & IOMUX_SEL_SDMMC_MASK)
 			rk_clrsetreg(&grf->gpio0d_iomux,
 				     GPIO0D6_SEL_MASK,
-				     GPIO0D6_SDMMC0_PWRENM1
-				     << GPIO0D6_SEL_SHIFT);
+				     GPIO0D6_GPIO << GPIO0D6_SEL_SHIFT);
 		else
 			rk_clrsetreg(&grf->gpio2a_iomux,
 				     GPIO2A7_SEL_MASK,
-				     GPIO2A7_SDMMC0_PWRENM0
-				     << GPIO2A7_SEL_SHIFT);
+				     GPIO2A7_GPIO << GPIO2A7_SEL_SHIFT);
 		rk_clrsetreg(&grf->gpio1a_iomux,
 			     GPIO1A0_SEL_MASK,
 			     GPIO1A0_CARD_DATA_CLK_CMD_DETN

Signature · 1 line

1.9.1

Simon Glass

2017-05-20 02:29 UTC #13 lore raw

PATCH 4/6 rockchip: pinctrl: rk3328: use gpio instead of sdmmc-pwren

Quoted 8 lines · Kever Yang

SDMMC-PWREN is a pin to control voltage for SDMMC IO, it may be high active or low active, the dwmmc driver always assume the sdmmc-pwren as high active.
Kernel treat this pin as fixed regulator instead of a pin from controller, and then it can set in dts file upon board schematic, that's a good solution, we can also do this in u-boot.

Quoted 5 lines

Signed-off-by Kever Yang <[email protected]>
---
 drivers/pinctrl/rockchip/pinctrl_rk3328.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

Acked-by Simon Glass <[email protected]>

Simon Glass

2017-06-05 21:33 UTC #14 lore raw

PATCH 4/6 rockchip: pinctrl: rk3328: use gpio instead of sdmmc-pwren

Hi Kever,

Quoted 9 lines · Simon Glass

Quoted 8 lines · Kever Yang
SDMMC-PWREN is a pin to control voltage for SDMMC IO, it may be high active or low active, the dwmmc driver always assume the sdmmc-pwren as high active.
Kernel treat this pin as fixed regulator instead of a pin from controller, and then it can set in dts file upon board schematic, that's a good solution, we can also do this in u-boot.

Quoted 7 lines

Quoted 5 lines
Signed-off-by Kever Yang <[email protected]>
---
 drivers/pinctrl/rockchip/pinctrl_rk3328.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
Acked-by Simon Glass <[email protected]>

Can you please rebase this patch and the following ones? I have marked them as 'changes requested' in patchwork.

Regards,
Simon

Kever Yang

author

2017-05-17 03:44 UTC #15 lore raw

PATCH 5/6 rockchip: evb-rk3328: enable boot on regulator

In reply to Kever Yang

Enable all the boot-on regulator in default.

Signed-off-by Kever Yang <[email protected]>

---

 board/rockchip/evb_rk3328/evb-rk3328.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

board/rockchip/evb_rk3328/evb-rk3328.c +7 -1

index d9dc782..eb15cee 100644
@@ -10,12 +10,14 @@
 #include <asm/armv8/mmu.h>
 #include <asm/io.h>
 #include <dwc3-uboot.h>
+#include <power/regulator.h>
 #include <usb.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
 int board_init(void)
 {
+	int ret;
 #define GRF_BASE	0xff100000
 	struct rk3328_grf_regs * const grf = (void *)GRF_BASE;

@@ -25,7 +27,11 @@ int board_init(void)
 		     IOMUX_SEL_UART2_M1 << IOMUX_SEL_UART2_SHIFT |
 		     IOMUX_SEL_SDMMC_M1 << IOMUX_SEL_SDMMC_SHIFT);
 
-	return 0;
+	ret = regulators_enable_boot_on(false);
+	if (ret)
+		debug("%s: Cannot enable boot on regulator\n", __func__);
+
+	return ret;
 }
 
 int dram_init(void)

Signature · 1 line

1.9.1

Simon Glass

2017-05-20 02:29 UTC #16 lore raw

PATCH 5/6 rockchip: evb-rk3328: enable boot on regulator

Quoted 7 lines · Kever Yang

Enable all the boot-on regulator in default.
Signed-off-by Kever Yang <[email protected]>
---
 board/rockchip/evb_rk3328/evb-rk3328.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Acked-by Simon Glass <[email protected]>

Kever Yang

author

2017-05-17 03:44 UTC #17 lore raw

PATCH 6/6 rockchip: dts: rk3328-evb: add sdmmc-pwren regulator

In reply to Kever Yang

Use fixed regulator for sdmmc-pwren for sdmmc power.

Signed-off-by Kever Yang <[email protected]>

---

 arch/arm/dts/rk3328-evb.dts | 8 ++++++++
 1 file changed, 8 insertions(+)

arch/arm/dts/rk3328-evb.dts +8 -0

index 01794ed..7b14982 100644
@@ -14,6 +14,14 @@
 	chosen {
 		stdout-path = &uart2;
 	};
+
+	vcc3v3_sdmmc: sdmmc-pwren {
+		compatible = "regulator-fixed";
+		regulator-name = "vcc3v3";
+		gpio = <&gpio0 30 GPIO_ACTIVE_LOW>;
+		regulator-always-on;
+		regulator-boot-on;
+	};
 };
 
 &uart2 {

Signature · 1 line

1.9.1

Simon Glass

2017-05-20 02:29 UTC #18 lore raw

PATCH 6/6 rockchip: dts: rk3328-evb: add sdmmc-pwren regulator

Quoted 7 lines · Kever Yang

Use fixed regulator for sdmmc-pwren for sdmmc power.
Signed-off-by Kever Yang <[email protected]>
---
 arch/arm/dts/rk3328-evb.dts | 8 ++++++++
 1 file changed, 8 insertions(+)

Acked-by Simon Glass <[email protected]>