Re: [PATCH v2 1/2] mmc: sdhci-of-arasan: add phy support for sdhci-of-arasan

From: Shawn Lin
Date: Thu Sep 17 2015 - 11:20:35 EST


On 2015/9/17 19:44, Ulf Hansson wrote:
On 14 September 2015 at 08:29, Shawn Lin <shawn.lin@xxxxxxxxxxxxxx> wrote:
This patch adds Generic PHY access for sdhci-of-arasan. Driver
can get PHY handler from dt-binding, and power-on/init the PHY.
Also we add pm ops for PHY here if CONFIG_PM_SLEEP is enabled.
Currently, it's just mandatory for arasan,sdhci-5.1.

I am trying to understand how a PHY can be used together with a
MMC/SD/SDIO controller. Normally the card connector doesn't hold any
intelligence, so I wonder if PHY is correctly used here.

Could you try to explain, HW-wise, what role the PHY has for you?


Thanks for comment, Ulf.

It's the first time we introduce phy into mmc subsystem,also it's my first time to use phy for emmc in real Soc.
so definitely we need to discuss it deliberately.

From my view, phy is an active-power, differential sampling and more
well-designed analog *IO component* for ultra-highspeed device to guarantee its signal integrity, like USB, UFS, DDR-RAM,MIPI and so on.

(1)Firstly it contains DLL to generate delayline and phase for sampling data from mmc devices. When sdhci controller executes tuning and sends out tuning sequence, and PHY can auto change its delayline and phase
in order to test if this "sample timing" is okay. If not, CRC err is generate back to the controller. Then after SDHCI tune timing for 40 times, also PHY have scanned all the "sample windows" by trying different delayline and phase combination, and finally auto-select the best sample timing for the "sample windows". And sdhci HOST CONTROL
2 Register[6:7] is controlled by PHY in my case.

(2)Then phy can programming source/sink impedance to avoid signal reflex. Given that JEDEC has come to HS533(highspeed 533MHz), this function is imperative to be implemented just like the role of ODT for DDR-RAM's PHY.

(3)Phy is integrated with diff pull-up/down resistance value and open-drain type for HW design.

(4)Phy can also have some enable-bit to decide whether all mmc signal(clk/data/cmd/strobe) can output to the devices or not.


For the card, even for host controller itself, phy can just be regarded as a need-to-do-something-before-used GPIO.
So, before we start initializing card, we need to power up PHY, enable clk for it and configure all the stuffs. Also we need to power down it for power-saving when into suspend. That's all we need to do and that's all the generic phy framework had provided by four interface: phy_init/phy_exit/phy_power_on/phy_power_off.


Sincerely welcome any comments here to make things better. :)


Kind regards
Uffe


Signed-off-by: Shawn Lin <shawn.lin@xxxxxxxxxxxxxx>

Serise-changes: 2
- Keep phy as a mandatory requirement for arasan,sdhci-5.1

---

Changes in v2: None

drivers/mmc/host/sdhci-of-arasan.c | 97 ++++++++++++++++++++++++++++++++++++++
1 file changed, 97 insertions(+)

diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
index 75379cb..2c13ef8 100644
--- a/drivers/mmc/host/sdhci-of-arasan.c
+++ b/drivers/mmc/host/sdhci-of-arasan.c
@@ -21,6 +21,7 @@

#include <linux/module.h>
#include <linux/of_device.h>
+#include <linux/phy/phy.h>
#include "sdhci-pltfm.h"

#define SDHCI_ARASAN_CLK_CTRL_OFFSET 0x2c
@@ -35,6 +36,7 @@
*/
struct sdhci_arasan_data {
struct clk *clk_ahb;
+ struct phy *phy;
};

static unsigned int sdhci_arasan_get_timeout_clock(struct sdhci_host *host)
@@ -70,6 +72,62 @@ static struct sdhci_pltfm_data sdhci_arasan_pdata = {

#ifdef CONFIG_PM_SLEEP
/**
+ * sdhci_arasan_suspend_phy - Suspend phy method for the driver
+ * @phy: Handler of phy structure
+ * Returns 0 on success and error value on error
+ *
+ * Put the phy in a deactive state.
+ */
+static int sdhci_arasan_suspend_phy(struct phy *phy)
+{
+ int ret;
+
+ ret = phy_exit(phy);
+ if (ret < 0)
+ goto err_phy_exit;
+
+ ret = phy_power_off(phy);
+ if (ret < 0)
+ goto err_phy_pwr_off;
+
+ return 0;
+
+err_phy_pwr_off:
+ phy_power_on(phy);
+err_phy_exit:
+ phy_init(phy);
+ return ret;
+}
+
+/**
+ * sdhci_arasan_resume_phy - Resume phy method for the driver
+ * @phy: Handler of phy structure
+ * Returns 0 on success and error value on error
+ *
+ * Put the phy in a active state.
+ */
+static int sdhci_arasan_resume_phy(struct phy *phy)
+{
+ int ret;
+
+ ret = phy_power_on(phy);
+ if (ret < 0)
+ goto err_phy_pwr_on;
+
+ ret = phy_init(phy);
+ if (ret < 0)
+ goto err_phy_init;
+
+ return 0;
+
+err_phy_init:
+ phy_exit(phy);
+err_phy_pwr_on:
+ phy_power_off(phy);
+ return ret;
+}
+
+/**
* sdhci_arasan_suspend - Suspend method for the driver
* @dev: Address of the device structure
* Returns 0 on success and error value on error
@@ -91,6 +149,12 @@ static int sdhci_arasan_suspend(struct device *dev)
clk_disable(pltfm_host->clk);
clk_disable(sdhci_arasan->clk_ahb);

+ if (sdhci_arasan->phy) {
+ ret = sdhci_arasan_suspend_phy(sdhci_arasan->phy);
+ if (ret < 0)
+ return ret;
+ }
+
return 0;
}

@@ -122,6 +186,12 @@ static int sdhci_arasan_resume(struct device *dev)
return ret;
}

+ if (sdhci_arasan->phy) {
+ ret = sdhci_arasan_resume_phy(sdhci_arasan->phy);
+ if (ret < 0)
+ return ret;
+ }
+
return sdhci_resume_host(host);
}
#endif /* ! CONFIG_PM_SLEEP */
@@ -166,6 +236,33 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
goto clk_dis_ahb;
}

+ sdhci_arasan->phy = NULL;
+ if (of_device_is_compatible(pdev->dev.of_node,
+ "arasan,sdhci-5.1")) {
+ sdhci_arasan->phy = devm_phy_get(&pdev->dev,
+ "phy_arasan");
+ if (IS_ERR(sdhci_arasan->phy)) {
+ ret = -ENODEV;
+ dev_err(&pdev->dev, "No phy for arasan,sdhci-5.1.\n");
+ goto clk_dis_ahb;
+ }
+
+ ret = phy_power_on(sdhci_arasan->phy);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "phy_power_on err.\n");
+ phy_power_off(sdhci_arasan->phy);
+ goto clk_dis_ahb;
+ }
+
+ ret = phy_init(sdhci_arasan->phy);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "phy_init err.\n");
+ phy_exit(sdhci_arasan->phy);
+ phy_power_off(sdhci_arasan->phy);
+ goto clk_dis_ahb;
+ }
+ }
+
host = sdhci_pltfm_init(pdev, &sdhci_arasan_pdata, 0);
if (IS_ERR(host)) {
ret = PTR_ERR(host);
--
2.3.7


--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html





--
Best Regards
Shawn Lin

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/