[PATCH V9 2/7] phy: Add drivers for PCIe and SATA phy on SPEAr13xx

On Thursday 10 July 2014 07:00 PM, Viresh Kumar wrote:

On 10 July 2014 18:47, Kishon Vijay Abraham I [off-list ref] wrote:

quoted

On Thursday 10 July 2014 12:56 PM, Viresh Kumar wrote:

quoted

From: Pratyush Anand <redacted>

ARM based ST Microelectronics's SPEAr1310/40 platforms uses ST's phy (known as
'miphy') for PCIe and SATA. This patch adds drivers for these miphys.

This also adds proper bindings for miphys.

Acked-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Pratyush Anand <redacted>
Tested-by: Mohit Kumar <redacted>
Cc: Kishon Vijay Abraham I <redacted>
[viresh: fixed logs/cclist/checkpatch warnings, broken into smaller patches]
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 .../devicetree/bindings/phy/st-spear1310-miphy.txt |  12 +
 .../devicetree/bindings/phy/st-spear1340-miphy.txt |  11 +
 drivers/phy/Kconfig                                |  12 +
 drivers/phy/Makefile                               |   2 +
 drivers/phy/phy-spear1310-miphy.c                  | 274 +++++++++++++++++++
 drivers/phy/phy-spear1340-miphy.c                  | 302 +++++++++++++++++++++

Please send separate patche for each driver.

These were both around SPEAr and were on the same lines.
So sending these in a single patch shouldn't be a big issue I believe.

In case another version is required, I may do it.

yes please.

-Kishon

quoted

quoted

diff --git a/Documentation/devicetree/bindings/phy/st-spear1310-miphy.txt b/Documentation/devicetree/bindings/phy/st-spear1310-miphy.txt
new file mode 100644
index 0000000..b9b281a
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/st-spear1310-miphy.txt

We generally create a single document for a SoC vendor. So just use st-phy.txt.

Probably yes.

quoted

<snip>

Please keep line containing file names while removing stuff, makes it easy
to locate code.

quoted

quoted

+
+static int spear1340_miphy_sata_init(struct spear1340_miphy_priv *priv)
+{
+     regmap_update_bits(priv->misc, SPEAR1340_PCIE_SATA_CFG,
+                        SPEAR1340_PCIE_SATA_CFG_MASK,
+                        SPEAR1340_SATA_CFG_VAL);
+     regmap_update_bits(priv->misc, SPEAR1340_PCIE_MIPHY_CFG,
+                        SPEAR1340_PCIE_MIPHY_CFG_MASK,
+                        SPEAR1340_PCIE_SATA_MIPHY_CFG_SATA_25M_CRYSTAL_CLK);
+     /* Switch on sata power domain */
+     regmap_update_bits(priv->misc, SPEAR1340_PCM_CFG,
+                        SPEAR1340_PCM_CFG_SATA_POWER_EN,
+                        SPEAR1340_PCM_CFG_SATA_POWER_EN);
+     msleep(20);
+     /* Disable PCIE SATA Controller reset */
+     regmap_update_bits(priv->misc, SPEAR1340_PERIP1_SW_RST,
+                        SPEAR1340_PERIP1_SW_RSATA, 0);
+     msleep(20);

Please add a comment for all delays added.

@Pratyush/Mohit: please let me know what to add here.

quoted

quoted

+#ifdef CONFIG_PM_SLEEP
+static int spear1340_miphy_suspend(struct device *dev)
+{
+     struct spear1340_miphy_priv *priv = dev_get_drvdata(dev);
+     int ret = 0;
+
+     if (priv->mode == SATA)
+             ret = spear1340_miphy_sata_exit(priv);

Shouldn't this be spear1340_miphy_init()?

quoted

+
+     return ret;
+}
+
+static int spear1340_miphy_resume(struct device *dev)
+{
+     struct spear1340_miphy_priv *priv = dev_get_drvdata(dev);
+     int ret = 0;
+
+     if (priv->mode == SATA)
+             ret = spear1340_miphy_sata_init(priv);

And here spear1340_miphy_exit()? Why only for sata phys?

@Mohit/Pratyush ??

Thanks Kishon for another round of review :)