Re: Re: [PATCH v1 1/2] mmc: rockchip_sdhci: add phy and clock config for rk3399

2021-06-28 Thread 赵仪峰
Hi Jaehoon,

> Does it impossible to move to phy subsystem about codes relevant to phy?
I already check the mmc code, but I didn't find that the sdhci framework
supports the PHY interface. It would be troublesome to support it. 
At present, only the EMMC of rk3399 has PHY, while rk3568 only has some
configuration registers for IO.

The other suggestions from you have been modified on V2 and have been mailed.
Please check it again. 

Thank you.

Yifeng.zhao
 


>
Hi Yifeng,
 
On 6/7/21 4:38 PM, Yifeng Zhao wrote:
> Add clock, phy and other configuration, it is convenient to support
> new controller. Here a short summary of the changes:
> - support HS200 and HS400 config by dts
> - Remove OF_PLATDATA related code
> - Reorder header inclusion
> - Add data width config for 1, 4, 8 bits
> - Add phy ops
> - add ops set_ios_post to modify the parameters of phy when the
>   clock changes
 
Does it impossible to move to phy subsystem about codes relevant to phy?
 
> 
> Signed-off-by: Yifeng Zhao 
> ---
> 
>  drivers/mmc/rockchip_sdhci.c | 334 +++
>  1 file changed, 303 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c
> index d95f8b2a15..05ed998eca 100644
> --- a/drivers/mmc/rockchip_sdhci.c
> +++ b/drivers/mmc/rockchip_sdhci.c
> @@ -6,33 +6,285 @@
>   */
>  
>  #include 
> +#include 
>  #include 
> +#include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
>  #include 
>  #include 
> -#include 
> +#include 
>  
> +#include 
> +#include 
>  /* 400KHz is max freq for card ID etc. Use that as min */
>  #define EMMC_MIN_FREQ 40
> +#define KHz (1000)
> +#define MHz (1000 * KHz)
> +
> +#define PHYCTRL_CALDONE_MASK 0x1
> +#define PHYCTRL_CALDONE_SHIFT 0x6
> +#define PHYCTRL_CALDONE_DONE 0x1
> +#define PHYCTRL_DLLRDY_MASK 0x1
> +#define PHYCTRL_DLLRDY_SHIFT 0x5
> +#define PHYCTRL_DLLRDY_DONE 0x1
> +#define PHYCTRL_FREQSEL_200M 0x0
> +#define PHYCTRL_FREQSEL_50M 0x1
> +#define PHYCTRL_FREQSEL_100M 0x2
> +#define PHYCTRL_FREQSEL_150M 0x3
>  
>  struct rockchip_sdhc_plat {
> -#if CONFIG_IS_ENABLED(OF_PLATDATA)
> - struct dtd_rockchip_rk3399_sdhci_5_1 dtplat;
> -#endif
>  struct mmc_config cfg;
>  struct mmc mmc;
>  };
>  
> +struct rockchip_emmc_phy {
> + u32 emmcphy_con[7];
> + u32 reserved;
> + u32 emmcphy_status;
> +};
> +
>  struct rockchip_sdhc {
>  struct sdhci_host host;
> + struct udevice *dev;
>  void *base;
> + struct rockchip_emmc_phy *phy;
> + struct clk emmc_clk;
> +};
> +
> +struct sdhci_data {
> + int (*emmc_set_clock)(struct sdhci_host *host, unsigned int clock);
> + int (*emmc_phy_init)(struct udevice *dev);
> + int (*get_phy)(struct udevice *dev);
> +};
> +
> +static int rk3399_emmc_phy_init(struct udevice *dev)
> +{
> + return 0;
> +}
> +
> +static void rk3399_emmc_phy_power_on(struct rockchip_emmc_phy *phy, u32 
> clock)
> +{
> + u32 caldone, dllrdy, freqsel;
> + uint start;
> +
> + writel(RK_CLRSETBITS(7 << 4, 0), &phy->emmcphy_con[6]);
> + writel(RK_CLRSETBITS(1 << 11, 1 << 11), &phy->emmcphy_con[0]);
> + writel(RK_CLRSETBITS(0xf << 7, 6 << 7), &phy->emmcphy_con[0]);
> +
> + /*
> + * According to the user manual, calpad calibration
> + * cycle takes more than 2us without the minimal recommended
> + * value, so we may need a little margin here
> + */
> + udelay(3);
> + writel(RK_CLRSETBITS(1, 1), &phy->emmcphy_con[6]);
> +
> + /*
> + * According to the user manual, it asks driver to
> + * wait 5us for calpad busy trimming. But it seems that
> + * 5us of caldone isn't enough for all cases.
> + */
> + udelay(500);
> + caldone = readl(&phy->emmcphy_status);
> + caldone = (caldone >> PHYCTRL_CALDONE_SHIFT) & PHYCTRL_CALDONE_MASK;
> + if (caldone != PHYCTRL_CALDONE_DONE) {
> + printf("%s: caldone timeout.\n", __func__);
> + return;
> + }
> +
> + /* Set the frequency of the DLL operation */
> + if (clock < 75 * MHz)
> + freqsel = PHYCTRL_FREQSEL_50M;
> + else if (clock < 125 * MHz)
> + freqsel = PHYCTRL_FREQSEL_100M;
> + else if (clock < 175 * MHz)
> + freqsel = PHYCTRL_FREQSEL_150M;
> + else
> + freqsel = PHYCTRL_FREQSEL_200M;
> +
> + /* Set the frequency of the DLL operation */
> + writel(RK_CLRSETBITS(3 << 12, freqsel << 12), &phy->emmcphy_con[0]);
> + writel(RK_CLRSETBITS(1 << 1, 1 << 1), &phy->emmcphy_con[6]);
> +
> + start = get_timer(0);
> +
> + do {
> + udelay(1);
> + dllrdy = readl(&phy->emmcphy_status);
> + dllrdy = (dllrdy >> PHYCTRL_DLLRDY_SHIFT) & PHYCTRL_DLLRDY_MASK;
> + if (dllrdy == PHYCTRL_DLLRDY_DONE)
> + break;
> + } while (get_timer(start) < 5);
> +
> + if (dllrdy != PHYCTRL_DLLRDY_DONE)
> + printf("%s: dllrdy timeout.\n", __func__);
> +}
> +
> +static void rk3399_emmc_phy_power_off(struct rockchip_emmc_phy *phy)
> +{
> + writel(RK_CLRSETBITS(1, 0), &phy->emmcphy_con[6]);
> + writel(RK_CLRSETBITS(1 << 1, 0), &phy->emmcphy_con[6]);
> +}
> +
> +static int rockchip_emmc_set_clock(struct sdhci_host *host, unsigned int 
> clock)
 
Does it impossible to reuse sdhci_set_clock()? It's

Re: [PATCH v1 1/2] mmc: rockchip_sdhci: add phy and clock config for rk3399

2021-06-07 Thread Jaehoon Chung
Hi Yifeng,

On 6/7/21 4:38 PM, Yifeng Zhao wrote:
> Add clock, phy and other configuration, it is convenient to support
> new controller. Here a short summary of the changes:
> - support HS200 and HS400 config by dts
> - Remove OF_PLATDATA related code
> - Reorder header inclusion
> - Add data width config for 1, 4, 8 bits
> - Add phy ops
> - add ops set_ios_post to modify the parameters of phy when the
>   clock changes

Does it impossible to move to phy subsystem about codes relevant to phy?

> 
> Signed-off-by: Yifeng Zhao 
> ---
> 
>  drivers/mmc/rockchip_sdhci.c | 334 +++
>  1 file changed, 303 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c
> index d95f8b2a15..05ed998eca 100644
> --- a/drivers/mmc/rockchip_sdhci.c
> +++ b/drivers/mmc/rockchip_sdhci.c
> @@ -6,33 +6,285 @@
>   */
>  
>  #include 
> +#include 
>  #include 
> +#include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
>  #include 
>  #include 
> -#include 
> +#include 
>  
> +#include 
> +#include 
>  /* 400KHz is max freq for card ID etc. Use that as min */
>  #define EMMC_MIN_FREQ40
> +#define KHz  (1000)
> +#define MHz  (1000 * KHz)
> +
> +#define PHYCTRL_CALDONE_MASK 0x1
> +#define PHYCTRL_CALDONE_SHIFT0x6
> +#define PHYCTRL_CALDONE_DONE 0x1
> +#define PHYCTRL_DLLRDY_MASK  0x1
> +#define PHYCTRL_DLLRDY_SHIFT 0x5
> +#define PHYCTRL_DLLRDY_DONE  0x1
> +#define PHYCTRL_FREQSEL_200M 0x0
> +#define PHYCTRL_FREQSEL_50M  0x1
> +#define PHYCTRL_FREQSEL_100M 0x2
> +#define PHYCTRL_FREQSEL_150M 0x3
>  
>  struct rockchip_sdhc_plat {
> -#if CONFIG_IS_ENABLED(OF_PLATDATA)
> - struct dtd_rockchip_rk3399_sdhci_5_1 dtplat;
> -#endif
>   struct mmc_config cfg;
>   struct mmc mmc;
>  };
>  
> +struct rockchip_emmc_phy {
> + u32 emmcphy_con[7];
> + u32 reserved;
> + u32 emmcphy_status;
> +};
> +
>  struct rockchip_sdhc {
>   struct sdhci_host host;
> + struct udevice *dev;
>   void *base;
> + struct rockchip_emmc_phy *phy;
> + struct clk emmc_clk;
> +};
> +
> +struct sdhci_data {
> + int (*emmc_set_clock)(struct sdhci_host *host, unsigned int clock);
> + int (*emmc_phy_init)(struct udevice *dev);
> + int (*get_phy)(struct udevice *dev);
> +};
> +
> +static int rk3399_emmc_phy_init(struct udevice *dev)
> +{
> + return 0;
> +}
> +
> +static void rk3399_emmc_phy_power_on(struct rockchip_emmc_phy *phy, u32 
> clock)
> +{
> + u32 caldone, dllrdy, freqsel;
> + uint start;
> +
> + writel(RK_CLRSETBITS(7 << 4, 0), &phy->emmcphy_con[6]);
> + writel(RK_CLRSETBITS(1 << 11, 1 << 11), &phy->emmcphy_con[0]);
> + writel(RK_CLRSETBITS(0xf << 7, 6 << 7), &phy->emmcphy_con[0]);
> +
> + /*
> +  * According to the user manual, calpad calibration
> +  * cycle takes more than 2us without the minimal recommended
> +  * value, so we may need a little margin here
> +  */
> + udelay(3);
> + writel(RK_CLRSETBITS(1, 1), &phy->emmcphy_con[6]);
> +
> + /*
> +  * According to the user manual, it asks driver to
> +  * wait 5us for calpad busy trimming. But it seems that
> +  * 5us of caldone isn't enough for all cases.
> +  */
> + udelay(500);
> + caldone = readl(&phy->emmcphy_status);
> + caldone = (caldone >> PHYCTRL_CALDONE_SHIFT) & PHYCTRL_CALDONE_MASK;
> + if (caldone != PHYCTRL_CALDONE_DONE) {
> + printf("%s: caldone timeout.\n", __func__);
> + return;
> + }
> +
> + /* Set the frequency of the DLL operation */
> + if (clock < 75 * MHz)
> + freqsel = PHYCTRL_FREQSEL_50M;
> + else if (clock < 125 * MHz)
> + freqsel = PHYCTRL_FREQSEL_100M;
> + else if (clock < 175 * MHz)
> + freqsel = PHYCTRL_FREQSEL_150M;
> + else
> + freqsel = PHYCTRL_FREQSEL_200M;
> +
> + /* Set the frequency of the DLL operation */
> + writel(RK_CLRSETBITS(3 << 12, freqsel << 12), &phy->emmcphy_con[0]);
> + writel(RK_CLRSETBITS(1 << 1, 1 << 1), &phy->emmcphy_con[6]);
> +
> + start = get_timer(0);
> +
> + do {
> + udelay(1);
> + dllrdy = readl(&phy->emmcphy_status);
> + dllrdy = (dllrdy >> PHYCTRL_DLLRDY_SHIFT) & PHYCTRL_DLLRDY_MASK;
> + if (dllrdy == PHYCTRL_DLLRDY_DONE)
> + break;
> + } while (get_timer(start) < 5);
> +
> + if (dllrdy != PHYCTRL_DLLRDY_DONE)
> + printf("%s: dllrdy timeout.\n", __func__);
> +}
> +
> +static void rk3399_emmc_phy_power_off(struct rockchip_emmc_phy *phy)
> +{
> + writel(RK_CLRSETBITS(1, 0), &phy->emmcphy_con[6]);
> + writel(RK_CLRSETBITS(1 << 1, 0), &phy->emmcphy_con[6]);
> +}
> +
> +static int rockchip_emmc_set_clock(struct sdhci_host *host, unsigned int 
> clock)

Does it impossible to reuse sdhci_set_clock()? It's alm

[PATCH v1 1/2] mmc: rockchip_sdhci: add phy and clock config for rk3399

2021-06-07 Thread Yifeng Zhao
Add clock, phy and other configuration, it is convenient to support
new controller. Here a short summary of the changes:
- support HS200 and HS400 config by dts
- Remove OF_PLATDATA related code
- Reorder header inclusion
- Add data width config for 1, 4, 8 bits
- Add phy ops
- add ops set_ios_post to modify the parameters of phy when the
  clock changes

Signed-off-by: Yifeng Zhao 
---

 drivers/mmc/rockchip_sdhci.c | 334 +++
 1 file changed, 303 insertions(+), 31 deletions(-)

diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c
index d95f8b2a15..05ed998eca 100644
--- a/drivers/mmc/rockchip_sdhci.c
+++ b/drivers/mmc/rockchip_sdhci.c
@@ -6,33 +6,285 @@
  */
 
 #include 
+#include 
 #include 
+#include 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
 #include 
-#include 
+#include 
 
+#include 
+#include 
 /* 400KHz is max freq for card ID etc. Use that as min */
 #define EMMC_MIN_FREQ  40
+#define KHz(1000)
+#define MHz(1000 * KHz)
+
+#define PHYCTRL_CALDONE_MASK   0x1
+#define PHYCTRL_CALDONE_SHIFT  0x6
+#define PHYCTRL_CALDONE_DONE   0x1
+#define PHYCTRL_DLLRDY_MASK0x1
+#define PHYCTRL_DLLRDY_SHIFT   0x5
+#define PHYCTRL_DLLRDY_DONE0x1
+#define PHYCTRL_FREQSEL_200M   0x0
+#define PHYCTRL_FREQSEL_50M0x1
+#define PHYCTRL_FREQSEL_100M   0x2
+#define PHYCTRL_FREQSEL_150M   0x3
 
 struct rockchip_sdhc_plat {
-#if CONFIG_IS_ENABLED(OF_PLATDATA)
-   struct dtd_rockchip_rk3399_sdhci_5_1 dtplat;
-#endif
struct mmc_config cfg;
struct mmc mmc;
 };
 
+struct rockchip_emmc_phy {
+   u32 emmcphy_con[7];
+   u32 reserved;
+   u32 emmcphy_status;
+};
+
 struct rockchip_sdhc {
struct sdhci_host host;
+   struct udevice *dev;
void *base;
+   struct rockchip_emmc_phy *phy;
+   struct clk emmc_clk;
+};
+
+struct sdhci_data {
+   int (*emmc_set_clock)(struct sdhci_host *host, unsigned int clock);
+   int (*emmc_phy_init)(struct udevice *dev);
+   int (*get_phy)(struct udevice *dev);
+};
+
+static int rk3399_emmc_phy_init(struct udevice *dev)
+{
+   return 0;
+}
+
+static void rk3399_emmc_phy_power_on(struct rockchip_emmc_phy *phy, u32 clock)
+{
+   u32 caldone, dllrdy, freqsel;
+   uint start;
+
+   writel(RK_CLRSETBITS(7 << 4, 0), &phy->emmcphy_con[6]);
+   writel(RK_CLRSETBITS(1 << 11, 1 << 11), &phy->emmcphy_con[0]);
+   writel(RK_CLRSETBITS(0xf << 7, 6 << 7), &phy->emmcphy_con[0]);
+
+   /*
+* According to the user manual, calpad calibration
+* cycle takes more than 2us without the minimal recommended
+* value, so we may need a little margin here
+*/
+   udelay(3);
+   writel(RK_CLRSETBITS(1, 1), &phy->emmcphy_con[6]);
+
+   /*
+* According to the user manual, it asks driver to
+* wait 5us for calpad busy trimming. But it seems that
+* 5us of caldone isn't enough for all cases.
+*/
+   udelay(500);
+   caldone = readl(&phy->emmcphy_status);
+   caldone = (caldone >> PHYCTRL_CALDONE_SHIFT) & PHYCTRL_CALDONE_MASK;
+   if (caldone != PHYCTRL_CALDONE_DONE) {
+   printf("%s: caldone timeout.\n", __func__);
+   return;
+   }
+
+   /* Set the frequency of the DLL operation */
+   if (clock < 75 * MHz)
+   freqsel = PHYCTRL_FREQSEL_50M;
+   else if (clock < 125 * MHz)
+   freqsel = PHYCTRL_FREQSEL_100M;
+   else if (clock < 175 * MHz)
+   freqsel = PHYCTRL_FREQSEL_150M;
+   else
+   freqsel = PHYCTRL_FREQSEL_200M;
+
+   /* Set the frequency of the DLL operation */
+   writel(RK_CLRSETBITS(3 << 12, freqsel << 12), &phy->emmcphy_con[0]);
+   writel(RK_CLRSETBITS(1 << 1, 1 << 1), &phy->emmcphy_con[6]);
+
+   start = get_timer(0);
+
+   do {
+   udelay(1);
+   dllrdy = readl(&phy->emmcphy_status);
+   dllrdy = (dllrdy >> PHYCTRL_DLLRDY_SHIFT) & PHYCTRL_DLLRDY_MASK;
+   if (dllrdy == PHYCTRL_DLLRDY_DONE)
+   break;
+   } while (get_timer(start) < 5);
+
+   if (dllrdy != PHYCTRL_DLLRDY_DONE)
+   printf("%s: dllrdy timeout.\n", __func__);
+}
+
+static void rk3399_emmc_phy_power_off(struct rockchip_emmc_phy *phy)
+{
+   writel(RK_CLRSETBITS(1, 0), &phy->emmcphy_con[6]);
+   writel(RK_CLRSETBITS(1 << 1, 0), &phy->emmcphy_con[6]);
+}
+
+static int rockchip_emmc_set_clock(struct sdhci_host *host, unsigned int clock)
+{
+   unsigned int div, clk = 0, timeout;
+   unsigned int input_clk;
+   struct rockchip_sdhc *priv =
+   container_of(host, struct rockchip_sdhc, host);
+
+   /* Wait max 20 ms */
+   timeout = 2000;
+   while (sdhci_readl(host, SDHCI_PRESENT_STATE) &
+  (SDHCI_CMD_INHIBIT | SDHCI_DATA