Re: [PATCH v3 2/3] mmc: rockchip_sdhci: Add support for RK3568

2021-08-11 Thread Philipp Tomsich
On Tue, 29 Jun 2021 at 10:24, Yifeng Zhao  wrote:
>
> This patch adds support for the RK3568 platform to this driver.
>
> Signed-off-by: Yifeng Zhao 

I thought I had raised an objection to this patch previously, but did
not see a discussion...
So here we go again.

In 2017, we decided to split the HDMI drivers such that there is a
common core driver
and a "mini-driver" wrapping it, so we don't pull in the extra code
for every chip. See
the original comment from Simon at
   
https://patchwork.ozlabs.org/project/uboot/patch/1493394792-20743-4-git-send-email-philipp.toms...@theobroma-systems.com/#1648072

Given that we established that pattern already—and that there are
practical benefits
(e.g., in code-size), we should follow the same pattern for the SDHCI driver.

Thank you,
Philipp.


Re: [PATCH v3 2/3] mmc: rockchip_sdhci: Add support for RK3568

2021-08-11 Thread Kever Yang
Yifeng Zhao  于2021年6月29日周二 下午7:40写道:
>
> This patch adds support for the RK3568 platform to this driver.
>
> Signed-off-by: Yifeng Zhao 

Reviewed-by: Kever Yang 

Thanks,
- Kever
> ---
>
> Changes in v3:
> - Config the interface clock by clk_set_rate directly
>
> Changes in v2:
> - Used sdhci_set_clock api to set clock.
> - Used read_poll_timeout api to check dll status.
>
>  drivers/mmc/rockchip_sdhci.c | 109 +++
>  1 file changed, 109 insertions(+)
>
> diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c
> index eff134c8f5..1ac00587d4 100644
> --- a/drivers/mmc/rockchip_sdhci.c
> +++ b/drivers/mmc/rockchip_sdhci.c
> @@ -42,6 +42,34 @@
> x) >> PHYCTRL_DLLRDY_SHIFT) & PHYCTRL_DLLRDY_MASK) ==\
> PHYCTRL_DLLRDY_DONE)
>
> +/* Rockchip specific Registers */
> +#define DWCMSHC_EMMC_DLL_CTRL  0x800
> +#define DWCMSHC_EMMC_DLL_CTRL_RESETBIT(1)
> +#define DWCMSHC_EMMC_DLL_RXCLK 0x804
> +#define DWCMSHC_EMMC_DLL_TXCLK 0x808
> +#define DWCMSHC_EMMC_DLL_STRBIN0x80c
> +#define DWCMSHC_EMMC_DLL_STATUS0   0x840
> +#define DWCMSHC_EMMC_DLL_STATUS1   0x844
> +#define DWCMSHC_EMMC_DLL_START BIT(0)
> +#define DWCMSHC_EMMC_DLL_RXCLK_SRCSEL  29
> +#define DWCMSHC_EMMC_DLL_START_POINT   16
> +#define DWCMSHC_EMMC_DLL_START_DEFAULT 5
> +#define DWCMSHC_EMMC_DLL_INC_VALUE 2
> +#define DWCMSHC_EMMC_DLL_INC   8
> +#define DWCMSHC_EMMC_DLL_DLYENABIT(27)
> +#define DLL_TXCLK_TAPNUM_DEFAULT   0x10
> +#define DLL_STRBIN_TAPNUM_DEFAULT  0x3
> +#define DLL_TXCLK_TAPNUM_FROM_SW   BIT(24)
> +#define DWCMSHC_EMMC_DLL_LOCKEDBIT(8)
> +#define DWCMSHC_EMMC_DLL_TIMEOUT   BIT(9)
> +#define DLL_RXCLK_NO_INVERTER  1
> +#define DLL_RXCLK_INVERTER 0
> +#define DWCMSHC_ENHANCED_STROBEBIT(8)
> +#define DLL_LOCK_WO_TMOUT(x) \
> +   x) & DWCMSHC_EMMC_DLL_LOCKED) == DWCMSHC_EMMC_DLL_LOCKED) && \
> +   (((x) & DWCMSHC_EMMC_DLL_TIMEOUT) == 0))
> +#define ROCKCHIP_MAX_CLKS  3
> +
>  struct rockchip_sdhc_plat {
> struct mmc_config cfg;
> struct mmc mmc;
> @@ -167,6 +195,77 @@ static int rk3399_sdhci_emmc_set_clock(struct sdhci_host 
> *host, unsigned int clo
> return 0;
>  }
>
> +static int rk3568_emmc_phy_init(struct udevice *dev)
> +{
> +   struct rockchip_sdhc *prv = dev_get_priv(dev);
> +   struct sdhci_host *host = >host;
> +   u32 extra;
> +
> +   extra = DLL_RXCLK_NO_INVERTER << DWCMSHC_EMMC_DLL_RXCLK_SRCSEL;
> +   sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_RXCLK);
> +
> +   return 0;
> +}
> +
> +static int rk3568_sdhci_emmc_set_clock(struct sdhci_host *host, unsigned int 
> clock)
> +{
> +   struct rockchip_sdhc *priv = container_of(host, struct rockchip_sdhc, 
> host);
> +   int val, ret;
> +   u32 extra;
> +
> +   if (clock > host->max_clk)
> +   clock = host->max_clk;
> +   if (clock)
> +   clk_set_rate(>emmc_clk, clock);
> +
> +   sdhci_set_clock(host->mmc, clock);
> +
> +   if (clock >= 100 * MHz) {
> +   /* reset DLL */
> +   sdhci_writel(host, DWCMSHC_EMMC_DLL_CTRL_RESET, 
> DWCMSHC_EMMC_DLL_CTRL);
> +   udelay(1);
> +   sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_CTRL);
> +
> +   /* Init DLL settings */
> +   extra = DWCMSHC_EMMC_DLL_START_DEFAULT << 
> DWCMSHC_EMMC_DLL_START_POINT |
> +   DWCMSHC_EMMC_DLL_INC_VALUE << DWCMSHC_EMMC_DLL_INC |
> +   DWCMSHC_EMMC_DLL_START;
> +   sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_CTRL);
> +
> +   ret = read_poll_timeout(readl, host->ioaddr + 
> DWCMSHC_EMMC_DLL_STATUS0,
> +   val, DLL_LOCK_WO_TMOUT(val), 1, 500);
> +   if (ret)
> +   return ret;
> +
> +   extra = DWCMSHC_EMMC_DLL_DLYENA |
> +   DLL_RXCLK_NO_INVERTER << 
> DWCMSHC_EMMC_DLL_RXCLK_SRCSEL;
> +   sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_RXCLK);
> +
> +   extra = DWCMSHC_EMMC_DLL_DLYENA |
> +   DLL_TXCLK_TAPNUM_DEFAULT |
> +   DLL_TXCLK_TAPNUM_FROM_SW;
> +   sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_TXCLK);
> +
> +   extra = DWCMSHC_EMMC_DLL_DLYENA |
> +   DLL_STRBIN_TAPNUM_DEFAULT;
> +   sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_STRBIN);
> +   } else {
> +   /* reset the clock phase when the frequency is lower than 
> 100MHz */
> +   sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_CTRL);
> +   extra = DLL_RXCLK_NO_INVERTER << 
> DWCMSHC_EMMC_DLL_RXCLK_SRCSEL;
> +   sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_RXCLK);
> +   sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_TXCLK);
> +   

Re: [PATCH v3 2/3] mmc: rockchip_sdhci: Add support for RK3568

2021-06-29 Thread Jaehoon Chung
On 6/29/21 5:24 PM, Yifeng Zhao wrote:
> This patch adds support for the RK3568 platform to this driver.
> 
> Signed-off-by: Yifeng Zhao 


Reviewed-by: Jaehoon Chung 

Best Regards,
Jaehoon Chung

> ---
> 
> Changes in v3:
> - Config the interface clock by clk_set_rate directly
> 
> Changes in v2:
> - Used sdhci_set_clock api to set clock.
> - Used read_poll_timeout api to check dll status.
> 
>  drivers/mmc/rockchip_sdhci.c | 109 +++
>  1 file changed, 109 insertions(+)
> 
> diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c
> index eff134c8f5..1ac00587d4 100644
> --- a/drivers/mmc/rockchip_sdhci.c
> +++ b/drivers/mmc/rockchip_sdhci.c
> @@ -42,6 +42,34 @@
>   x) >> PHYCTRL_DLLRDY_SHIFT) & PHYCTRL_DLLRDY_MASK) ==\
>   PHYCTRL_DLLRDY_DONE)
>  
> +/* Rockchip specific Registers */
> +#define DWCMSHC_EMMC_DLL_CTRL0x800
> +#define DWCMSHC_EMMC_DLL_CTRL_RESET  BIT(1)
> +#define DWCMSHC_EMMC_DLL_RXCLK   0x804
> +#define DWCMSHC_EMMC_DLL_TXCLK   0x808
> +#define DWCMSHC_EMMC_DLL_STRBIN  0x80c
> +#define DWCMSHC_EMMC_DLL_STATUS0 0x840
> +#define DWCMSHC_EMMC_DLL_STATUS1 0x844
> +#define DWCMSHC_EMMC_DLL_START   BIT(0)
> +#define DWCMSHC_EMMC_DLL_RXCLK_SRCSEL29
> +#define DWCMSHC_EMMC_DLL_START_POINT 16
> +#define DWCMSHC_EMMC_DLL_START_DEFAULT   5
> +#define DWCMSHC_EMMC_DLL_INC_VALUE   2
> +#define DWCMSHC_EMMC_DLL_INC 8
> +#define DWCMSHC_EMMC_DLL_DLYENA  BIT(27)
> +#define DLL_TXCLK_TAPNUM_DEFAULT 0x10
> +#define DLL_STRBIN_TAPNUM_DEFAULT0x3
> +#define DLL_TXCLK_TAPNUM_FROM_SW BIT(24)
> +#define DWCMSHC_EMMC_DLL_LOCKED  BIT(8)
> +#define DWCMSHC_EMMC_DLL_TIMEOUT BIT(9)
> +#define DLL_RXCLK_NO_INVERTER1
> +#define DLL_RXCLK_INVERTER   0
> +#define DWCMSHC_ENHANCED_STROBE  BIT(8)
> +#define DLL_LOCK_WO_TMOUT(x) \
> + x) & DWCMSHC_EMMC_DLL_LOCKED) == DWCMSHC_EMMC_DLL_LOCKED) && \
> + (((x) & DWCMSHC_EMMC_DLL_TIMEOUT) == 0))
> +#define ROCKCHIP_MAX_CLKS3
> +
>  struct rockchip_sdhc_plat {
>   struct mmc_config cfg;
>   struct mmc mmc;
> @@ -167,6 +195,77 @@ static int rk3399_sdhci_emmc_set_clock(struct sdhci_host 
> *host, unsigned int clo
>   return 0;
>  }
>  
> +static int rk3568_emmc_phy_init(struct udevice *dev)
> +{
> + struct rockchip_sdhc *prv = dev_get_priv(dev);
> + struct sdhci_host *host = >host;
> + u32 extra;
> +
> + extra = DLL_RXCLK_NO_INVERTER << DWCMSHC_EMMC_DLL_RXCLK_SRCSEL;
> + sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_RXCLK);
> +
> + return 0;
> +}
> +
> +static int rk3568_sdhci_emmc_set_clock(struct sdhci_host *host, unsigned int 
> clock)
> +{
> + struct rockchip_sdhc *priv = container_of(host, struct rockchip_sdhc, 
> host);
> + int val, ret;
> + u32 extra;
> +
> + if (clock > host->max_clk)
> + clock = host->max_clk;
> + if (clock)
> + clk_set_rate(>emmc_clk, clock);
> +
> + sdhci_set_clock(host->mmc, clock);
> +
> + if (clock >= 100 * MHz) {
> + /* reset DLL */
> + sdhci_writel(host, DWCMSHC_EMMC_DLL_CTRL_RESET, 
> DWCMSHC_EMMC_DLL_CTRL);
> + udelay(1);
> + sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_CTRL);
> +
> + /* Init DLL settings */
> + extra = DWCMSHC_EMMC_DLL_START_DEFAULT << 
> DWCMSHC_EMMC_DLL_START_POINT |
> + DWCMSHC_EMMC_DLL_INC_VALUE << DWCMSHC_EMMC_DLL_INC |
> + DWCMSHC_EMMC_DLL_START;
> + sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_CTRL);
> +
> + ret = read_poll_timeout(readl, host->ioaddr + 
> DWCMSHC_EMMC_DLL_STATUS0,
> + val, DLL_LOCK_WO_TMOUT(val), 1, 500);
> + if (ret)
> + return ret;
> +
> + extra = DWCMSHC_EMMC_DLL_DLYENA |
> + DLL_RXCLK_NO_INVERTER << DWCMSHC_EMMC_DLL_RXCLK_SRCSEL;
> + sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_RXCLK);
> +
> + extra = DWCMSHC_EMMC_DLL_DLYENA |
> + DLL_TXCLK_TAPNUM_DEFAULT |
> + DLL_TXCLK_TAPNUM_FROM_SW;
> + sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_TXCLK);
> +
> + extra = DWCMSHC_EMMC_DLL_DLYENA |
> + DLL_STRBIN_TAPNUM_DEFAULT;
> + sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_STRBIN);
> + } else {
> + /* reset the clock phase when the frequency is lower than 
> 100MHz */
> + sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_CTRL);
> + extra = DLL_RXCLK_NO_INVERTER << DWCMSHC_EMMC_DLL_RXCLK_SRCSEL;
> + sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_RXCLK);
> + sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_TXCLK);
> + sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_STRBIN);
> + }
> +
> + return 0;

[PATCH v3 2/3] mmc: rockchip_sdhci: Add support for RK3568

2021-06-29 Thread Yifeng Zhao
This patch adds support for the RK3568 platform to this driver.

Signed-off-by: Yifeng Zhao 
---

Changes in v3:
- Config the interface clock by clk_set_rate directly

Changes in v2:
- Used sdhci_set_clock api to set clock.
- Used read_poll_timeout api to check dll status.

 drivers/mmc/rockchip_sdhci.c | 109 +++
 1 file changed, 109 insertions(+)

diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c
index eff134c8f5..1ac00587d4 100644
--- a/drivers/mmc/rockchip_sdhci.c
+++ b/drivers/mmc/rockchip_sdhci.c
@@ -42,6 +42,34 @@
x) >> PHYCTRL_DLLRDY_SHIFT) & PHYCTRL_DLLRDY_MASK) ==\
PHYCTRL_DLLRDY_DONE)
 
+/* Rockchip specific Registers */
+#define DWCMSHC_EMMC_DLL_CTRL  0x800
+#define DWCMSHC_EMMC_DLL_CTRL_RESETBIT(1)
+#define DWCMSHC_EMMC_DLL_RXCLK 0x804
+#define DWCMSHC_EMMC_DLL_TXCLK 0x808
+#define DWCMSHC_EMMC_DLL_STRBIN0x80c
+#define DWCMSHC_EMMC_DLL_STATUS0   0x840
+#define DWCMSHC_EMMC_DLL_STATUS1   0x844
+#define DWCMSHC_EMMC_DLL_START BIT(0)
+#define DWCMSHC_EMMC_DLL_RXCLK_SRCSEL  29
+#define DWCMSHC_EMMC_DLL_START_POINT   16
+#define DWCMSHC_EMMC_DLL_START_DEFAULT 5
+#define DWCMSHC_EMMC_DLL_INC_VALUE 2
+#define DWCMSHC_EMMC_DLL_INC   8
+#define DWCMSHC_EMMC_DLL_DLYENABIT(27)
+#define DLL_TXCLK_TAPNUM_DEFAULT   0x10
+#define DLL_STRBIN_TAPNUM_DEFAULT  0x3
+#define DLL_TXCLK_TAPNUM_FROM_SW   BIT(24)
+#define DWCMSHC_EMMC_DLL_LOCKEDBIT(8)
+#define DWCMSHC_EMMC_DLL_TIMEOUT   BIT(9)
+#define DLL_RXCLK_NO_INVERTER  1
+#define DLL_RXCLK_INVERTER 0
+#define DWCMSHC_ENHANCED_STROBEBIT(8)
+#define DLL_LOCK_WO_TMOUT(x) \
+   x) & DWCMSHC_EMMC_DLL_LOCKED) == DWCMSHC_EMMC_DLL_LOCKED) && \
+   (((x) & DWCMSHC_EMMC_DLL_TIMEOUT) == 0))
+#define ROCKCHIP_MAX_CLKS  3
+
 struct rockchip_sdhc_plat {
struct mmc_config cfg;
struct mmc mmc;
@@ -167,6 +195,77 @@ static int rk3399_sdhci_emmc_set_clock(struct sdhci_host 
*host, unsigned int clo
return 0;
 }
 
+static int rk3568_emmc_phy_init(struct udevice *dev)
+{
+   struct rockchip_sdhc *prv = dev_get_priv(dev);
+   struct sdhci_host *host = >host;
+   u32 extra;
+
+   extra = DLL_RXCLK_NO_INVERTER << DWCMSHC_EMMC_DLL_RXCLK_SRCSEL;
+   sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_RXCLK);
+
+   return 0;
+}
+
+static int rk3568_sdhci_emmc_set_clock(struct sdhci_host *host, unsigned int 
clock)
+{
+   struct rockchip_sdhc *priv = container_of(host, struct rockchip_sdhc, 
host);
+   int val, ret;
+   u32 extra;
+
+   if (clock > host->max_clk)
+   clock = host->max_clk;
+   if (clock)
+   clk_set_rate(>emmc_clk, clock);
+
+   sdhci_set_clock(host->mmc, clock);
+
+   if (clock >= 100 * MHz) {
+   /* reset DLL */
+   sdhci_writel(host, DWCMSHC_EMMC_DLL_CTRL_RESET, 
DWCMSHC_EMMC_DLL_CTRL);
+   udelay(1);
+   sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_CTRL);
+
+   /* Init DLL settings */
+   extra = DWCMSHC_EMMC_DLL_START_DEFAULT << 
DWCMSHC_EMMC_DLL_START_POINT |
+   DWCMSHC_EMMC_DLL_INC_VALUE << DWCMSHC_EMMC_DLL_INC |
+   DWCMSHC_EMMC_DLL_START;
+   sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_CTRL);
+
+   ret = read_poll_timeout(readl, host->ioaddr + 
DWCMSHC_EMMC_DLL_STATUS0,
+   val, DLL_LOCK_WO_TMOUT(val), 1, 500);
+   if (ret)
+   return ret;
+
+   extra = DWCMSHC_EMMC_DLL_DLYENA |
+   DLL_RXCLK_NO_INVERTER << DWCMSHC_EMMC_DLL_RXCLK_SRCSEL;
+   sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_RXCLK);
+
+   extra = DWCMSHC_EMMC_DLL_DLYENA |
+   DLL_TXCLK_TAPNUM_DEFAULT |
+   DLL_TXCLK_TAPNUM_FROM_SW;
+   sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_TXCLK);
+
+   extra = DWCMSHC_EMMC_DLL_DLYENA |
+   DLL_STRBIN_TAPNUM_DEFAULT;
+   sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_STRBIN);
+   } else {
+   /* reset the clock phase when the frequency is lower than 
100MHz */
+   sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_CTRL);
+   extra = DLL_RXCLK_NO_INVERTER << DWCMSHC_EMMC_DLL_RXCLK_SRCSEL;
+   sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_RXCLK);
+   sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_TXCLK);
+   sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_STRBIN);
+   }
+
+   return 0;
+}
+
+static int rk3568_emmc_get_phy(struct udevice *dev)
+{
+   return 0;
+}
+
 static int rockchip_sdhci_set_ios_post(struct sdhci_host *host)
 {
struct rockchip_sdhc *priv = container_of(host, struct rockchip_sdhc, 
host);
@@ -339,11 +438,21 @@