[PATCH 0/3] Exynos5250 SATA Support

[PATCH 0/3] Exynos5250 SATA Support

[Yuvaraj Kumar C D]

This series adds the SATA support on exynos5250 SOC.Also it 
uses generic phy framework patch[1] posted by Kishon Vijay 
Abraham I. 
	[1].drivers: phy: add generic PHY framework.

Yuvaraj Kumar C D (3):
  ahci: exynos: add ahci sata support on Exynos platform
  Phy: Exynos: Add Exynos5250 sata phy driver
  ARM: dts: Enable ahci sata and sata phy

 arch/arm/boot/dts/exynos5250-arndale.dts  |    9 +-
 arch/arm/boot/dts/exynos5250-smdk5250.dts |    6 +-
 arch/arm/boot/dts/exynos5250.dtsi         |   21 ++-
 drivers/ata/Kconfig                       |    9 ++
 drivers/ata/Makefile                      |    1 +
 drivers/ata/ahci_exynos.c                 |  226 ++++++++++++++++++++++++++
 drivers/phy/Kconfig                       |    6 +
 drivers/phy/Makefile                      |    1 +
 drivers/phy/exynos/Kconfig                |    5 +
 drivers/phy/exynos/Makefile               |    5 +
 drivers/phy/exynos/exynos5250_phy_i2c.c   |   53 ++++++
 drivers/phy/exynos/sata_phy_exynos5250.c  |  248 +++++++++++++++++++++++++++++
 drivers/phy/exynos/sata_phy_exynos5250.h  |   33 ++++
 13 files changed, 613 insertions(+), 10 deletions(-)
 create mode 100644 drivers/ata/ahci_exynos.c
 create mode 100644 drivers/phy/exynos/Kconfig
 create mode 100644 drivers/phy/exynos/Makefile
 create mode 100644 drivers/phy/exynos/exynos5250_phy_i2c.c
 create mode 100644 drivers/phy/exynos/sata_phy_exynos5250.c
 create mode 100644 drivers/phy/exynos/sata_phy_exynos5250.h

-- 
1.7.9.5

[PATCH 1/3] ahci: exynos: add ahci sata support on Exynos platform

[Yuvaraj Kumar C D]

Exynos5250 contains one Synopsys AHCI SATA controller.The avalaible
ahci_platform driver is not sufficient to handle the AHCI PHY and PHY
clock related initialization.

This patch adds exynos specific ahci sata driver,contained the exynos
specific initialized codes, re-use the generic ahci_platform driver, and
keep the generic ahci_platform driver clean as much as possible.

This patch depends on the below patch
	[1].drivers: phy: add generic PHY framework
		by Kishon Vijay Abraham I<kishon@ti.com>

Signed-off-by: Yuvaraj Kumar C D <yuvaraj.cd@samsung.com>
---
 drivers/ata/Kconfig       |    9 ++
 drivers/ata/Makefile      |    1 +
 drivers/ata/ahci_exynos.c |  226 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 236 insertions(+)
 create mode 100644 drivers/ata/ahci_exynos.c

diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index 4e73772..99b2392 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -106,6 +106,15 @@ config AHCI_IMX
 
 	  If unsure, say N.
 
+config AHCI_EXYNOS
+	tristate "Samsung Exynos AHCI SATA support"
+	depends on SATA_AHCI_PLATFORM
+	help
+	  This option enables support for the Samsung's Exynos SoC's
+	  onboard AHCI SATA.
+
+	  If unsure, say N.
+
 config SATA_FSL
 	tristate "Freescale 3.0Gbps SATA support"
 	depends on FSL_SOC
diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
index 46518c6..0e1f420f 100644
--- a/drivers/ata/Makefile
+++ b/drivers/ata/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_SATA_SIL24)	+= sata_sil24.o
 obj-$(CONFIG_SATA_DWC)		+= sata_dwc_460ex.o
 obj-$(CONFIG_SATA_HIGHBANK)	+= sata_highbank.o libahci.o
 obj-$(CONFIG_AHCI_IMX)		+= ahci_imx.o
+obj-$(CONFIG_AHCI_EXYNOS)	+= ahci_exynos.o
 
 # SFF w/ custom DMA
 obj-$(CONFIG_PDC_ADMA)		+= pdc_adma.o
diff --git a/drivers/ata/ahci_exynos.c b/drivers/ata/ahci_exynos.c
new file mode 100644
index 0000000..7f0af00
--- /dev/null
+++ b/drivers/ata/ahci_exynos.c
@@ -0,0 +1,226 @@
+/*
+ * Samsung AHCI SATA platform driver
+ * Copyright 2013 Samsung Electronics Co., Ltd.
+ *
+ * based on the AHCI SATA platform driver by Jeff Garzik and Anton Vorontsov
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/i2c.h>
+#include <linux/io.h>
+#include <linux/ahci_platform.h>
+#include <linux/of_device.h>
+#include <linux/phy/phy.h>
+#include "ahci.h"
+
+#define MHZ             (1000 * 1000)
+
+struct exynos_ahci_priv {
+	struct platform_device *ahci_pdev;
+	struct clk *sclk;
+	unsigned int freq;
+	struct phy *phy;
+};
+
+static int exynos_sata_init(struct device *dev, void __iomem *mmio)
+{
+	struct exynos_ahci_priv *priv = dev_get_drvdata(dev->parent);
+	int ret;
+
+	priv->phy = devm_phy_get(dev , "sata-phy");
+	if (IS_ERR(priv->phy))
+		return PTR_ERR(priv->phy);
+
+	ret = phy_init(priv->phy);
+	if (ret < 0) {
+			dev_err(dev, "failed to init SATA PHY\n");
+			return ret;
+	}
+
+	ret = clk_prepare_enable(priv->sclk);
+	if (ret < 0) {
+		dev_err(dev, "failed to enable source clk\n");
+		return ret;
+	}
+
+	ret = clk_set_rate(priv->sclk, priv->freq * MHZ);
+	if (ret < 0) {
+		dev_err(dev, "failed to set clk frequency\n");
+		clk_disable_unprepare(priv->sclk);
+		return ret;
+	}
+
+	return 0;
+}
+
+static void exynos_sata_exit(struct device *dev)
+{
+	struct exynos_ahci_priv *priv = dev_get_drvdata(dev->parent);
+	if (!IS_ERR(priv->sclk))
+		clk_disable_unprepare(priv->sclk);
+}
+
+static int exynos_sata_suspend(struct device *dev)
+{
+	struct exynos_ahci_priv *priv = dev_get_drvdata(dev->parent);
+
+	if (!IS_ERR(priv->sclk))
+		clk_disable_unprepare(priv->sclk);
+	phy_power_off(priv->phy);
+	return 0;
+}
+
+static int exynos_sata_resume(struct device *dev)
+{
+	struct exynos_ahci_priv *priv = dev_get_drvdata(dev->parent);
+	phy_power_on(priv->phy);
+	exynos_sata_init(dev, NULL);
+	return 0;
+}
+
+static struct ahci_platform_data exynos_sata_pdata = {
+	.init = exynos_sata_init,
+	.exit = exynos_sata_exit,
+	.suspend = exynos_sata_suspend,
+	.resume = exynos_sata_resume,
+};
+
+static const struct of_device_id exynos_ahci_of_match[] = {
+	{ .compatible = "samsung,exynos5250-sata-ahci",
+		.data = &exynos_sata_pdata},
+	{},
+};
+MODULE_DEVICE_TABLE(of, exynos_ahci_of_match);
+
+static int exynos_sata_parse_dt(struct device_node *np,
+				struct exynos_ahci_priv *priv)
+{
+	if (!np)
+		return -EINVAL;
+	return of_property_read_u32(np, "samsung,sata-freq",
+						&priv->freq);
+}
+
+static int exynos_ahci_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct resource *mem, *irq, res[2];
+	const struct of_device_id *of_id;
+	const struct ahci_platform_data *pdata = NULL;
+	struct exynos_ahci_priv *priv;
+	struct device *ahci_dev;
+	struct platform_device *ahci_pdev;
+	int ret;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv) {
+		dev_err(dev, "can't alloc ahci_host_priv\n");
+		return -ENOMEM;
+	}
+
+	ahci_pdev = platform_device_alloc("ahci", -1);
+	if (!ahci_pdev)
+		return -ENODEV;
+
+	ahci_dev = &ahci_pdev->dev;
+	ahci_dev->parent = dev;
+
+	ret = exynos_sata_parse_dt(dev->of_node, priv);
+	if (ret < 0) {
+			dev_err(dev, "failed to parse device tree\n");
+			goto err_out;
+	}
+
+	priv->sclk = devm_clk_get(dev, "sclk_sata");
+	if (IS_ERR(priv->sclk)) {
+		dev_err(dev, "failed to get sclk_sata\n");
+		ret = PTR_ERR(priv->sclk);
+		goto err_out;
+	}
+	priv->ahci_pdev = ahci_pdev;
+	platform_set_drvdata(pdev, priv);
+
+	of_id = of_match_device(exynos_ahci_of_match, dev);
+	if (of_id) {
+		pdata = of_id->data;
+	} else {
+		ret = -EINVAL;
+		goto err_out;
+	}
+
+	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+	if (!mem || !irq) {
+		dev_err(dev, "no mmio/irq resource\n");
+		ret = -ENOMEM;
+		goto err_out;
+	}
+
+	res[0] = *mem;
+	res[1] = *irq;
+
+	ahci_dev->coherent_dma_mask = DMA_BIT_MASK(32);
+	ahci_dev->dma_mask = &ahci_dev->coherent_dma_mask;
+	ahci_dev->of_node = dev->of_node;
+
+	ret = platform_device_add_resources(ahci_pdev, res, 2);
+	if (ret)
+		goto err_out;
+
+	ret = platform_device_add_data(ahci_pdev, pdata, sizeof(*pdata));
+	if (ret)
+		goto err_out;
+
+	ret = platform_device_add(ahci_pdev);
+	if (ret) {
+
+
+err_out:
+		platform_set_drvdata(pdev, NULL);
+		platform_device_put(ahci_pdev);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int exynos_ahci_remove(struct platform_device *pdev)
+{
+	struct exynos_ahci_priv *priv = platform_get_drvdata(pdev);
+	struct platform_device *ahci_pdev = priv->ahci_pdev;
+
+	platform_device_unregister(ahci_pdev);
+	platform_set_drvdata(pdev, NULL);
+
+	return 0;
+}
+
+static struct platform_driver exynos_ahci_driver = {
+	.probe = exynos_ahci_probe,
+	.remove = exynos_ahci_remove,
+	.driver = {
+		.name = "sata-exynos",
+		.owner = THIS_MODULE,
+		.of_match_table = exynos_ahci_of_match,
+	},
+};
+module_platform_driver(exynos_ahci_driver);
+
+MODULE_DESCRIPTION("Samsung Exynos AHCI SATA platform driver");
+MODULE_AUTHOR("Yuvaraj C D <yuvaraj.cd@samsung.com>");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("ahci:exynos");
-- 
1.7.9.5

[PATCH 1/3] ahci: exynos: add ahci sata support on Exynos platform

[Sachin Kamat]

Hi Yuvaraj,

On 1 October 2013 12:03, Yuvaraj Kumar C D <yuvaraj.cd@gmail.com> wrote:
> Exynos5250 contains one Synopsys AHCI SATA controller.The avalaible
> ahci_platform driver is not sufficient to handle the AHCI PHY and PHY
> clock related initialization.

> +err_out:
> +               platform_set_drvdata(pdev, NULL);

This is not necessary. Driver core clears it upon detach or failure.

> +               platform_device_put(ahci_pdev);
> +               return ret;
> +       }
> +
> +       return 0;
> +}
> +
> +static int exynos_ahci_remove(struct platform_device *pdev)
> +{
> +       struct exynos_ahci_priv *priv = platform_get_drvdata(pdev);
> +       struct platform_device *ahci_pdev = priv->ahci_pdev;
> +
> +       platform_device_unregister(ahci_pdev);
> +       platform_set_drvdata(pdev, NULL);

ditto

-- 
With warm regards,
Sachin

[PATCH 1/3] ahci: exynos: add ahci sata support on Exynos platform

[Kishon Vijay Abraham I]

On Tuesday 01 October 2013 12:03 PM, Yuvaraj Kumar C D wrote:
> Exynos5250 contains one Synopsys AHCI SATA controller.The avalaible
> ahci_platform driver is not sufficient to handle the AHCI PHY and PHY
> clock related initialization.
> 
> This patch adds exynos specific ahci sata driver,contained the exynos
> specific initialized codes, re-use the generic ahci_platform driver, and
> keep the generic ahci_platform driver clean as much as possible.
> 
> This patch depends on the below patch
> 	[1].drivers: phy: add generic PHY framework
> 		by Kishon Vijay Abraham I<kishon@ti.com>
> 
> Signed-off-by: Yuvaraj Kumar C D <yuvaraj.cd@samsung.com>
> ---
>  drivers/ata/Kconfig       |    9 ++
>  drivers/ata/Makefile      |    1 +
>  drivers/ata/ahci_exynos.c |  226 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 236 insertions(+)
>  create mode 100644 drivers/ata/ahci_exynos.c
> 
> diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
> index 4e73772..99b2392 100644
> --- a/drivers/ata/Kconfig
> +++ b/drivers/ata/Kconfig
> @@ -106,6 +106,15 @@ config AHCI_IMX
>  
>  	  If unsure, say N.
>  
> +config AHCI_EXYNOS
> +	tristate "Samsung Exynos AHCI SATA support"
> +	depends on SATA_AHCI_PLATFORM

Can we select GENERIC_PHY here?
> +	help
> +	  This option enables support for the Samsung's Exynos SoC's
> +	  onboard AHCI SATA.
> +
> +	  If unsure, say N.
> +
>  config SATA_FSL
>  	tristate "Freescale 3.0Gbps SATA support"
>  	depends on FSL_SOC
> diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
> index 46518c6..0e1f420f 100644
> --- a/drivers/ata/Makefile
> +++ b/drivers/ata/Makefile
> @@ -11,6 +11,7 @@ obj-$(CONFIG_SATA_SIL24)	+= sata_sil24.o
>  obj-$(CONFIG_SATA_DWC)		+= sata_dwc_460ex.o
>  obj-$(CONFIG_SATA_HIGHBANK)	+= sata_highbank.o libahci.o
>  obj-$(CONFIG_AHCI_IMX)		+= ahci_imx.o
> +obj-$(CONFIG_AHCI_EXYNOS)	+= ahci_exynos.o
>  
>  # SFF w/ custom DMA
>  obj-$(CONFIG_PDC_ADMA)		+= pdc_adma.o
> diff --git a/drivers/ata/ahci_exynos.c b/drivers/ata/ahci_exynos.c
> new file mode 100644
> index 0000000..7f0af00
> --- /dev/null
> +++ b/drivers/ata/ahci_exynos.c
> @@ -0,0 +1,226 @@
> +/*
> + * Samsung AHCI SATA platform driver
> + * Copyright 2013 Samsung Electronics Co., Ltd.
> + *
> + * based on the AHCI SATA platform driver by Jeff Garzik and Anton Vorontsov
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/i2c.h>
> +#include <linux/io.h>
> +#include <linux/ahci_platform.h>
> +#include <linux/of_device.h>
> +#include <linux/phy/phy.h>
> +#include "ahci.h"
> +
> +#define MHZ             (1000 * 1000)
> +
> +struct exynos_ahci_priv {
> +	struct platform_device *ahci_pdev;
> +	struct clk *sclk;
> +	unsigned int freq;
> +	struct phy *phy;
> +};
> +
> +static int exynos_sata_init(struct device *dev, void __iomem *mmio)
> +{
> +	struct exynos_ahci_priv *priv = dev_get_drvdata(dev->parent);
> +	int ret;
> +
> +	priv->phy = devm_phy_get(dev , "sata-phy");
                                    ^^
                             spurious space here

phy_get should be called from probe.
> +	if (IS_ERR(priv->phy))
> +		return PTR_ERR(priv->phy);
> +
> +	ret = phy_init(priv->phy);
> +	if (ret < 0) {
> +			dev_err(dev, "failed to init SATA PHY\n");
single tab is sufficient here and below.
> +			return ret;
> +	}
> +
> +	ret = clk_prepare_enable(priv->sclk);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to enable source clk\n");
> +		return ret;
> +	}
> +
> +	ret = clk_set_rate(priv->sclk, priv->freq * MHZ);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to set clk frequency\n");
> +		clk_disable_unprepare(priv->sclk);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static void exynos_sata_exit(struct device *dev)
> +{
> +	struct exynos_ahci_priv *priv = dev_get_drvdata(dev->parent);
> +	if (!IS_ERR(priv->sclk))

This error check is not needed. You fail probe if you dont get clock anyways.
> +		clk_disable_unprepare(priv->sclk);
> +}
> +
> +static int exynos_sata_suspend(struct device *dev)
> +{
> +	struct exynos_ahci_priv *priv = dev_get_drvdata(dev->parent);
> +
> +	if (!IS_ERR(priv->sclk))
ditto..
> +		clk_disable_unprepare(priv->sclk);
> +	phy_power_off(priv->phy);
> +	return 0;
> +}
> +
> +static int exynos_sata_resume(struct device *dev)
> +{
> +	struct exynos_ahci_priv *priv = dev_get_drvdata(dev->parent);
> +	phy_power_on(priv->phy);
> +	exynos_sata_init(dev, NULL);
> +	return 0;
> +}
> +
> +static struct ahci_platform_data exynos_sata_pdata = {
> +	.init = exynos_sata_init,

Why is exynos_sata_init given as callback and also in exynos_sata_resume?
> +	.exit = exynos_sata_exit,
> +	.suspend = exynos_sata_suspend,
> +	.resume = exynos_sata_resume,

Can't we use this in runtime_suspend/runtime_resume or suspend/resume pm ops?
> +};
> +
> +static const struct of_device_id exynos_ahci_of_match[] = {
> +	{ .compatible = "samsung,exynos5250-sata-ahci",
> +		.data = &exynos_sata_pdata},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, exynos_ahci_of_match);
> +
> +static int exynos_sata_parse_dt(struct device_node *np,
> +				struct exynos_ahci_priv *priv)
> +{
> +	if (!np)
> +		return -EINVAL;
> +	return of_property_read_u32(np, "samsung,sata-freq",
> +						&priv->freq);
> +}
> +
> +static int exynos_ahci_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct resource *mem, *irq, res[2];
> +	const struct of_device_id *of_id;
> +	const struct ahci_platform_data *pdata = NULL;
> +	struct exynos_ahci_priv *priv;
> +	struct device *ahci_dev;
> +	struct platform_device *ahci_pdev;
> +	int ret;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv) {
> +		dev_err(dev, "can't alloc ahci_host_priv\n");

error message not needed. kzalloc will give a dump if it fails.
> +		return -ENOMEM;
> +	}
> +
> +	ahci_pdev = platform_device_alloc("ahci", -1);

Should be using PLATFORM_DEVID_AUTO unless it is absolutely necessary.
> +	if (!ahci_pdev)
> +		return -ENODEV;
> +
> +	ahci_dev = &ahci_pdev->dev;
> +	ahci_dev->parent = dev;
> +
> +	ret = exynos_sata_parse_dt(dev->of_node, priv);
> +	if (ret < 0) {
> +			dev_err(dev, "failed to parse device tree\n");
> +			goto err_out;
> +	}
> +
> +	priv->sclk = devm_clk_get(dev, "sclk_sata");
> +	if (IS_ERR(priv->sclk)) {
> +		dev_err(dev, "failed to get sclk_sata\n");
> +		ret = PTR_ERR(priv->sclk);
> +		goto err_out;
> +	}
> +	priv->ahci_pdev = ahci_pdev;
> +	platform_set_drvdata(pdev, priv);
> +
> +	of_id = of_match_device(exynos_ahci_of_match, dev);
> +	if (of_id) {
> +		pdata = of_id->data;
> +	} else {
> +		ret = -EINVAL;
> +		goto err_out;
> +	}
> +
> +	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> +	if (!mem || !irq) {
> +		dev_err(dev, "no mmio/irq resource\n");
> +		ret = -ENOMEM;
> +		goto err_out;
> +	}
> +
> +	res[0] = *mem;
> +	res[1] = *irq;
> +
> +	ahci_dev->coherent_dma_mask = DMA_BIT_MASK(32);
> +	ahci_dev->dma_mask = &ahci_dev->coherent_dma_mask;
> +	ahci_dev->of_node = dev->of_node;
> +
> +	ret = platform_device_add_resources(ahci_pdev, res, 2);
> +	if (ret)
> +		goto err_out;
> +
> +	ret = platform_device_add_data(ahci_pdev, pdata, sizeof(*pdata));
> +	if (ret)
> +		goto err_out;
> +
> +	ret = platform_device_add(ahci_pdev);
> +	if (ret) {
> +
> +
> +err_out:
> +		platform_set_drvdata(pdev, NULL);
> +		platform_device_put(ahci_pdev);
> +		return ret;
Move this to the end of this function and then goto err_out for better readability.

> +	}
> +
> +	return 0;
> +}
> +
> +static int exynos_ahci_remove(struct platform_device *pdev)
> +{
> +	struct exynos_ahci_priv *priv = platform_get_drvdata(pdev);
> +	struct platform_device *ahci_pdev = priv->ahci_pdev;
> +
> +	platform_device_unregister(ahci_pdev);
> +	platform_set_drvdata(pdev, NULL);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver exynos_ahci_driver = {
> +	.probe = exynos_ahci_probe,
> +	.remove = exynos_ahci_remove,
> +	.driver = {
> +		.name = "sata-exynos",
> +		.owner = THIS_MODULE,
> +		.of_match_table = exynos_ahci_of_match,
> +	},
> +};
> +module_platform_driver(exynos_ahci_driver);
> +
> +MODULE_DESCRIPTION("Samsung Exynos AHCI SATA platform driver");
> +MODULE_AUTHOR("Yuvaraj C D <yuvaraj.cd@samsung.com>");
> +MODULE_LICENSE("GPL");

GPL v2?

Thanks
Kishon

[PATCH 1/3] ahci: exynos: add ahci sata support on Exynos platform

[Bartlomiej Zolnierkiewicz]

Hi,

On Tuesday, October 01, 2013 12:03:01 PM Yuvaraj Kumar C D wrote:
> Exynos5250 contains one Synopsys AHCI SATA controller.The avalaible
> ahci_platform driver is not sufficient to handle the AHCI PHY and PHY
> clock related initialization.
> 
> This patch adds exynos specific ahci sata driver,contained the exynos
> specific initialized codes, re-use the generic ahci_platform driver, and
> keep the generic ahci_platform driver clean as much as possible.
> 
> This patch depends on the below patch
> 	[1].drivers: phy: add generic PHY framework
> 		by Kishon Vijay Abraham I<kishon@ti.com>
> 
> Signed-off-by: Yuvaraj Kumar C D <yuvaraj.cd@samsung.com>
> ---
>  drivers/ata/Kconfig       |    9 ++
>  drivers/ata/Makefile      |    1 +
>  drivers/ata/ahci_exynos.c |  226 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 236 insertions(+)
>  create mode 100644 drivers/ata/ahci_exynos.c
> 
> diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
> index 4e73772..99b2392 100644
> --- a/drivers/ata/Kconfig
> +++ b/drivers/ata/Kconfig
> @@ -106,6 +106,15 @@ config AHCI_IMX
>  
>  	  If unsure, say N.
>  
> +config AHCI_EXYNOS
> +	tristate "Samsung Exynos AHCI SATA support"
> +	depends on SATA_AHCI_PLATFORM

This should also depend on SOC_EXYNOS5250.

> +	help
> +	  This option enables support for the Samsung's Exynos SoC's
> +	  onboard AHCI SATA.
> +
> +	  If unsure, say N.
> +
>  config SATA_FSL
>  	tristate "Freescale 3.0Gbps SATA support"
>  	depends on FSL_SOC
> diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
> index 46518c6..0e1f420f 100644
> --- a/drivers/ata/Makefile
> +++ b/drivers/ata/Makefile
> @@ -11,6 +11,7 @@ obj-$(CONFIG_SATA_SIL24)	+= sata_sil24.o
>  obj-$(CONFIG_SATA_DWC)		+= sata_dwc_460ex.o
>  obj-$(CONFIG_SATA_HIGHBANK)	+= sata_highbank.o libahci.o
>  obj-$(CONFIG_AHCI_IMX)		+= ahci_imx.o
> +obj-$(CONFIG_AHCI_EXYNOS)	+= ahci_exynos.o
>  
>  # SFF w/ custom DMA
>  obj-$(CONFIG_PDC_ADMA)		+= pdc_adma.o
> diff --git a/drivers/ata/ahci_exynos.c b/drivers/ata/ahci_exynos.c
> new file mode 100644
> index 0000000..7f0af00
> --- /dev/null
> +++ b/drivers/ata/ahci_exynos.c
> @@ -0,0 +1,226 @@
> +/*
> + * Samsung AHCI SATA platform driver
> + * Copyright 2013 Samsung Electronics Co., Ltd.
> + *
> + * based on the AHCI SATA platform driver by Jeff Garzik and Anton Vorontsov
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/i2c.h>

This include doesn't seem to be needed.

> +#include <linux/io.h>
> +#include <linux/ahci_platform.h>
> +#include <linux/of_device.h>
> +#include <linux/phy/phy.h>
> +#include "ahci.h"
> +
> +#define MHZ             (1000 * 1000)
> +
> +struct exynos_ahci_priv {
> +	struct platform_device *ahci_pdev;
> +	struct clk *sclk;
> +	unsigned int freq;
> +	struct phy *phy;
> +};
> +
> +static int exynos_sata_init(struct device *dev, void __iomem *mmio)
> +{
> +	struct exynos_ahci_priv *priv = dev_get_drvdata(dev->parent);
> +	int ret;
> +
> +	priv->phy = devm_phy_get(dev , "sata-phy");
> +	if (IS_ERR(priv->phy))
> +		return PTR_ERR(priv->phy);

This should happen in ->probe (exynos_sata_init() is also called in
exynos_sata_resume() so the above code will be executed on every
resume operation which is incorrect).

Also please take a look at the following patch:

https://lkml.org/lkml/2013/9/19/173

it adds PHY support to ahci_platform driver, maybe it can be used
for your needs as well.

> +	ret = phy_init(priv->phy);
> +	if (ret < 0) {
> +			dev_err(dev, "failed to init SATA PHY\n");
> +			return ret;
> +	}
> +
> +	ret = clk_prepare_enable(priv->sclk);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to enable source clk\n");
> +		return ret;
> +	}
> +
> +	ret = clk_set_rate(priv->sclk, priv->freq * MHZ);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to set clk frequency\n");
> +		clk_disable_unprepare(priv->sclk);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static void exynos_sata_exit(struct device *dev)
> +{
> +	struct exynos_ahci_priv *priv = dev_get_drvdata(dev->parent);
> +	if (!IS_ERR(priv->sclk))
> +		clk_disable_unprepare(priv->sclk);
> +}
> +
> +static int exynos_sata_suspend(struct device *dev)
> +{
> +	struct exynos_ahci_priv *priv = dev_get_drvdata(dev->parent);
> +
> +	if (!IS_ERR(priv->sclk))
> +		clk_disable_unprepare(priv->sclk);
> +	phy_power_off(priv->phy);
> +	return 0;
> +}
> +
> +static int exynos_sata_resume(struct device *dev)
> +{
> +	struct exynos_ahci_priv *priv = dev_get_drvdata(dev->parent);
> +	phy_power_on(priv->phy);
> +	exynos_sata_init(dev, NULL);
> +	return 0;

It should be:

	return exynos_sata_init(dev, NULL);

Also please fix exynos_sata_suspend() to call exynos_sata_exit().

[...]

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

[PATCH 1/3] ahci: exynos: add ahci sata support on Exynos platform

[Jingoo Han]

On Thursday, October 03, 2013 8:32 PM, Bartlomiej Zolnierkiewicz wrote:
> On Tuesday, October 01, 2013 12:03:01 PM Yuvaraj Kumar C D wrote:
> > Exynos5250 contains one Synopsys AHCI SATA controller.The avalaible
> > ahci_platform driver is not sufficient to handle the AHCI PHY and PHY
> > clock related initialization.
> >
> > This patch adds exynos specific ahci sata driver,contained the exynos
> > specific initialized codes, re-use the generic ahci_platform driver, and
> > keep the generic ahci_platform driver clean as much as possible.
> >
> > This patch depends on the below patch
> > 	[1].drivers: phy: add generic PHY framework
> > 		by Kishon Vijay Abraham I<kishon@ti.com>
> >
> > Signed-off-by: Yuvaraj Kumar C D <yuvaraj.cd@samsung.com>
> > ---
> >  drivers/ata/Kconfig       |    9 ++
> >  drivers/ata/Makefile      |    1 +
> >  drivers/ata/ahci_exynos.c |  226 +++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 236 insertions(+)
> >  create mode 100644 drivers/ata/ahci_exynos.c
> >


[.....]

> > +	priv->phy = devm_phy_get(dev , "sata-phy");
> > +	if (IS_ERR(priv->phy))
> > +		return PTR_ERR(priv->phy);

[.....]

> Also please take a look at the following patch:
> 
> https://lkml.org/lkml/2013/9/19/173
> 
> it adds PHY support to ahci_platform driver, maybe it can be used
> for your needs as well.

I also agree with Bartlomiej Zolnierkiewicz's opinion.
'ahci_exynos.c' just calls PHY API, without any additional control
for Exynos AHCI IP.

Best regards,
Jingoo Han

> 
> > +	ret = phy_init(priv->phy);
> > +	if (ret < 0) {
> > +			dev_err(dev, "failed to init SATA PHY\n");
> > +			return ret;
> > +	}
> > +

[PATCH 1/3] ahci: exynos: add ahci sata support on Exynos platform

[Yuvaraj Kumar]

On Fri, Oct 4, 2013 at 6:03 AM, Jingoo Han <jg1.han@samsung.com> wrote:
> On Thursday, October 03, 2013 8:32 PM, Bartlomiej Zolnierkiewicz wrote:
>> On Tuesday, October 01, 2013 12:03:01 PM Yuvaraj Kumar C D wrote:
>> > Exynos5250 contains one Synopsys AHCI SATA controller.The avalaible
>> > ahci_platform driver is not sufficient to handle the AHCI PHY and PHY
>> > clock related initialization.
>> >
>> > This patch adds exynos specific ahci sata driver,contained the exynos
>> > specific initialized codes, re-use the generic ahci_platform driver, and
>> > keep the generic ahci_platform driver clean as much as possible.
>> >
>> > This patch depends on the below patch
>> >     [1].drivers: phy: add generic PHY framework
>> >             by Kishon Vijay Abraham I<kishon@ti.com>
>> >
>> > Signed-off-by: Yuvaraj Kumar C D <yuvaraj.cd@samsung.com>
>> > ---
>> >  drivers/ata/Kconfig       |    9 ++
>> >  drivers/ata/Makefile      |    1 +
>> >  drivers/ata/ahci_exynos.c |  226 +++++++++++++++++++++++++++++++++++++++++++++
>> >  3 files changed, 236 insertions(+)
>> >  create mode 100644 drivers/ata/ahci_exynos.c
>> >
>
>
> [.....]
>
>> > +   priv->phy = devm_phy_get(dev , "sata-phy");
>> > +   if (IS_ERR(priv->phy))
>> > +           return PTR_ERR(priv->phy);
>
> [.....]
>
>> Also please take a look at the following patch:
>>
>> https://lkml.org/lkml/2013/9/19/173
>>
>> it adds PHY support to ahci_platform driver, maybe it can be used
>> for your needs as well.
>
> I also agree with Bartlomiej Zolnierkiewicz's opinion.
> 'ahci_exynos.c' just calls PHY API, without any additional control
> for Exynos AHCI IP.
In addition to PHY handling,it also deals with the special clock
sclk_sata which is not dealt in ahci_platform.c(certainly exynos
specific).

Morever there is a wrapper driver to handle the platform specific
things for the sata.Please refer the  patch[1]
[1]ata: ti_sata: Add Texas Instruments SATA Wrapper driver
https://lkml.org/lkml/2013/9/19/166

[2]ahci_imx: add ahci sata support on imx platforms

I think, if we have platform specific driver like ahci_xxx.c , it
would be better to handle the sata PHY in ahci_xxx.c so that we can
retain and re-use the ahci_platform.c as it is.

Further comments will be much appreciated.

>
> Best regards,
> Jingoo Han
>
>>
>> > +   ret = phy_init(priv->phy);
>> > +   if (ret < 0) {
>> > +                   dev_err(dev, "failed to init SATA PHY\n");
>> > +                   return ret;
>> > +   }
>> > +
>

[PATCH 1/3] ahci: exynos: add ahci sata support on Exynos platform

[Roger Quadros]

Hi,

On 10/08/2013 02:44 PM, Yuvaraj Kumar wrote:
> On Fri, Oct 4, 2013 at 6:03 AM, Jingoo Han <jg1.han@samsung.com> wrote:
>> On Thursday, October 03, 2013 8:32 PM, Bartlomiej Zolnierkiewicz wrote:
>>> On Tuesday, October 01, 2013 12:03:01 PM Yuvaraj Kumar C D wrote:
>>>> Exynos5250 contains one Synopsys AHCI SATA controller.The avalaible
>>>> ahci_platform driver is not sufficient to handle the AHCI PHY and PHY
>>>> clock related initialization.
>>>>
>>>> This patch adds exynos specific ahci sata driver,contained the exynos
>>>> specific initialized codes, re-use the generic ahci_platform driver, and
>>>> keep the generic ahci_platform driver clean as much as possible.
>>>>
>>>> This patch depends on the below patch
>>>>     [1].drivers: phy: add generic PHY framework
>>>>             by Kishon Vijay Abraham I<kishon@ti.com>
>>>>
>>>> Signed-off-by: Yuvaraj Kumar C D <yuvaraj.cd@samsung.com>
>>>> ---
>>>>  drivers/ata/Kconfig       |    9 ++
>>>>  drivers/ata/Makefile      |    1 +
>>>>  drivers/ata/ahci_exynos.c |  226 +++++++++++++++++++++++++++++++++++++++++++++
>>>>  3 files changed, 236 insertions(+)
>>>>  create mode 100644 drivers/ata/ahci_exynos.c
>>>>
>>
>>
>> [.....]
>>
>>>> +   priv->phy = devm_phy_get(dev , "sata-phy");
>>>> +   if (IS_ERR(priv->phy))
>>>> +           return PTR_ERR(priv->phy);
>>
>> [.....]
>>
>>> Also please take a look at the following patch:
>>>
>>> https://lkml.org/lkml/2013/9/19/173
>>>
>>> it adds PHY support to ahci_platform driver, maybe it can be used
>>> for your needs as well.
>>
>> I also agree with Bartlomiej Zolnierkiewicz's opinion.
>> 'ahci_exynos.c' just calls PHY API, without any additional control
>> for Exynos AHCI IP.
> In addition to PHY handling,it also deals with the special clock
> sclk_sata which is not dealt in ahci_platform.c(certainly exynos
> specific).
> 
> Morever there is a wrapper driver to handle the platform specific
> things for the sata.Please refer the  patch[1]
> [1]ata: ti_sata: Add Texas Instruments SATA Wrapper driver
> https://lkml.org/lkml/2013/9/19/166

This driver doesn't do anything at the moment since the OMAP hwmod layer
takes care of enabling the relevant clocks. So I was planning to drop it
for now.

> 
> [2]ahci_imx: add ahci sata support on imx platforms
> 
> I think, if we have platform specific driver like ahci_xxx.c , it
> would be better to handle the sata PHY in ahci_xxx.c so that we can
> retain and re-use the ahci_platform.c as it is.
> 

The Generic PHY framework [3] has been merged into Greg's usb-next branch.
The operations there are pretty generic and IMO the PHY can be handled in
the ahci_platform driver.

[3] Generic PHY framework
    https://lkml.org/lkml/2013/9/27/581

cheers,
-roger

> Further comments will be much appreciated.
> 
>>
>> Best regards,
>> Jingoo Han
>>
>>>
>>>> +   ret = phy_init(priv->phy);
>>>> +   if (ret < 0) {
>>>> +                   dev_err(dev, "failed to init SATA PHY\n");
>>>> +                   return ret;
>>>> +   }
>>>> +
>>

[PATCH 1/3] ahci: exynos: add ahci sata support on Exynos platform

[Jingoo Han]

On Tuesday, October 08, 2013 8:45 PM, Yuvaraj Kumar wrote:
> On Fri, Oct 4, 2013 at 6:03 AM, Jingoo Han <jg1.han@samsung.com> wrote:
> > On Thursday, October 03, 2013 8:32 PM, Bartlomiej Zolnierkiewicz wrote:
> >> On Tuesday, October 01, 2013 12:03:01 PM Yuvaraj Kumar C D wrote:
> >> > Exynos5250 contains one Synopsys AHCI SATA controller.The avalaible
> >> > ahci_platform driver is not sufficient to handle the AHCI PHY and PHY
> >> > clock related initialization.
> >> >
> >> > This patch adds exynos specific ahci sata driver,contained the exynos
> >> > specific initialized codes, re-use the generic ahci_platform driver, and
> >> > keep the generic ahci_platform driver clean as much as possible.
> >> >
> >> > This patch depends on the below patch
> >> >     [1].drivers: phy: add generic PHY framework
> >> >             by Kishon Vijay Abraham I<kishon@ti.com>
> >> >
> >> > Signed-off-by: Yuvaraj Kumar C D <yuvaraj.cd@samsung.com>
> >> > ---
> >> >  drivers/ata/Kconfig       |    9 ++
> >> >  drivers/ata/Makefile      |    1 +
> >> >  drivers/ata/ahci_exynos.c |  226 +++++++++++++++++++++++++++++++++++++++++++++
> >> >  3 files changed, 236 insertions(+)
> >> >  create mode 100644 drivers/ata/ahci_exynos.c
> >> >
> >
> >
> > [.....]
> >
> >> > +   priv->phy = devm_phy_get(dev , "sata-phy");
> >> > +   if (IS_ERR(priv->phy))
> >> > +           return PTR_ERR(priv->phy);
> >
> > [.....]
> >
> >> Also please take a look at the following patch:
> >>
> >> https://lkml.org/lkml/2013/9/19/173
> >>
> >> it adds PHY support to ahci_platform driver, maybe it can be used
> >> for your needs as well.
> >
> > I also agree with Bartlomiej Zolnierkiewicz's opinion.
> > 'ahci_exynos.c' just calls PHY API, without any additional control
> > for Exynos AHCI IP.
> In addition to PHY handling,it also deals with the special clock
> sclk_sata which is not dealt in ahci_platform.c(certainly exynos
> specific).

Handling the special clock 'sclk_sata' is another issue.
Please, look at that other 'sclk_*'s are handled.
Also, if possible, please add 'the code handling the special clock'
to 'ahci_platform.c'.

> 
> Morever there is a wrapper driver to handle the platform specific
> things for the sata.Please refer the  patch[1]
> [1]ata: ti_sata: Add Texas Instruments SATA Wrapper driver
> https://lkml.org/lkml/2013/9/19/166

As Roger Quadros mentioned, 'ti_sata' driver will be dropped.

> [2]ahci_imx: add ahci sata support on imx platforms
> 

'ahci_imx' is necessary, because 'ahci_imx' controls some
specific registers.

However, 'ahci_exynos.c' does not control any registers.

> I think, if we have platform specific driver like ahci_xxx.c , it
> would be better to handle the sata PHY in ahci_xxx.c so that we can
> retain and re-use the ahci_platform.c as it is.

I think that the platform specific driver like ahci_xxx.c is not
necessary, because 'ahci_exynos.c' does not control any special
registers.

Best regards,
Jingoo Han

> 
> Further comments will be much appreciated.
> 

[PATCH 1/3] ahci: exynos: add ahci sata support on Exynos platform

[Tomasz Figa]

Hi,

On Friday 11 of October 2013 15:49:04 Jingoo Han wrote:
> On Tuesday, October 08, 2013 8:45 PM, Yuvaraj Kumar wrote:
> > On Fri, Oct 4, 2013 at 6:03 AM, Jingoo Han <jg1.han@samsung.com> wrote:
> > > On Thursday, October 03, 2013 8:32 PM, Bartlomiej Zolnierkiewicz wrote:
> > >> On Tuesday, October 01, 2013 12:03:01 PM Yuvaraj Kumar C D wrote:
> > >> > Exynos5250 contains one Synopsys AHCI SATA controller.The avalaible
> > >> > ahci_platform driver is not sufficient to handle the AHCI PHY and PHY
> > >> > clock related initialization.
> > >> >
> > >> > This patch adds exynos specific ahci sata driver,contained the exynos
> > >> > specific initialized codes, re-use the generic ahci_platform driver, and
> > >> > keep the generic ahci_platform driver clean as much as possible.
> > >> >
> > >> > This patch depends on the below patch
> > >> >     [1].drivers: phy: add generic PHY framework
> > >> >             by Kishon Vijay Abraham I<kishon@ti.com>
> > >> >
> > >> > Signed-off-by: Yuvaraj Kumar C D <yuvaraj.cd@samsung.com>
> > >> > ---
> > >> >  drivers/ata/Kconfig       |    9 ++
> > >> >  drivers/ata/Makefile      |    1 +
> > >> >  drivers/ata/ahci_exynos.c |  226 +++++++++++++++++++++++++++++++++++++++++++++
> > >> >  3 files changed, 236 insertions(+)
> > >> >  create mode 100644 drivers/ata/ahci_exynos.c
> > >> >
> > >
> > >
> > > [.....]
> > >
> > >> > +   priv->phy = devm_phy_get(dev , "sata-phy");
> > >> > +   if (IS_ERR(priv->phy))
> > >> > +           return PTR_ERR(priv->phy);
> > >
> > > [.....]
> > >
> > >> Also please take a look at the following patch:
> > >>
> > >> https://lkml.org/lkml/2013/9/19/173
> > >>
> > >> it adds PHY support to ahci_platform driver, maybe it can be used
> > >> for your needs as well.
> > >
> > > I also agree with Bartlomiej Zolnierkiewicz's opinion.
> > > 'ahci_exynos.c' just calls PHY API, without any additional control
> > > for Exynos AHCI IP.
> > In addition to PHY handling,it also deals with the special clock
> > sclk_sata which is not dealt in ahci_platform.c(certainly exynos
> > specific).
> 
> Handling the special clock 'sclk_sata' is another issue.
> Please, look at that other 'sclk_*'s are handled.
> Also, if possible, please add 'the code handling the special clock'
> to 'ahci_platform.c'.
> 
> > 
> > Morever there is a wrapper driver to handle the platform specific
> > things for the sata.Please refer the  patch[1]
> > [1]ata: ti_sata: Add Texas Instruments SATA Wrapper driver
> > https://lkml.org/lkml/2013/9/19/166
> 
> As Roger Quadros mentioned, 'ti_sata' driver will be dropped.
> 
> > [2]ahci_imx: add ahci sata support on imx platforms
> > 
> 
> 'ahci_imx' is necessary, because 'ahci_imx' controls some
> specific registers.
> 
> However, 'ahci_exynos.c' does not control any registers.
> 
> > I think, if we have platform specific driver like ahci_xxx.c , it
> > would be better to handle the sata PHY in ahci_xxx.c so that we can
> > retain and re-use the ahci_platform.c as it is.
> 
> I think that the platform specific driver like ahci_xxx.c is not
> necessary, because 'ahci_exynos.c' does not control any special
> registers.

My big +1 to all the JIngoo's points above.

Best regards,
Tomasz

[PATCH 2/3] Phy: Exynos: Add Exynos5250 sata phy driver

[Yuvaraj Kumar C D]

This patch adds the sata phy driver for Exynos5250.Exynos5250 sata
phy comprises of CMU and TRSV blocks which are of I2C register Map.
So this patch also adds a i2c client driver, which is used configure
the CMU and TRSV block of exynos5250 SATA PHY.

This patch incorporates the generic phy framework to deal with sata
phy.

This patch depends on the below patch
	[1].drivers: phy: add generic PHY framework
		by Kishon Vijay Abraham I<kishon@ti.com>

Signed-off-by: Yuvaraj Kumar C D <yuvaraj.cd@samsung.com>
Signed-off-by: Girish K S <ks.giri@samsung.com>
Signed-off-by: Vasanth Ananthan <vasanth.a@samsung.com>
---
 drivers/phy/Kconfig                      |    6 +
 drivers/phy/Makefile                     |    1 +
 drivers/phy/exynos/Kconfig               |    5 +
 drivers/phy/exynos/Makefile              |    5 +
 drivers/phy/exynos/exynos5250_phy_i2c.c  |   53 +++++++
 drivers/phy/exynos/sata_phy_exynos5250.c |  248 ++++++++++++++++++++++++++++++
 drivers/phy/exynos/sata_phy_exynos5250.h |   33 ++++
 7 files changed, 351 insertions(+)
 create mode 100644 drivers/phy/exynos/Kconfig
 create mode 100644 drivers/phy/exynos/Makefile
 create mode 100644 drivers/phy/exynos/exynos5250_phy_i2c.c
 create mode 100644 drivers/phy/exynos/sata_phy_exynos5250.c
 create mode 100644 drivers/phy/exynos/sata_phy_exynos5250.h

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 5f85909..ab3d1c6 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -11,3 +11,9 @@ menuconfig GENERIC_PHY
 	  devices present in the kernel. This layer will have the generic
 	  API by which phy drivers can create PHY using the phy framework and
 	  phy users can obtain reference to the PHY.
+
+if GENERIC_PHY
+
+source "drivers/phy/exynos/Kconfig"
+
+endif
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 9e9560f..e0223d7 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -3,3 +3,4 @@
 #
 
 obj-$(CONFIG_GENERIC_PHY)	+= phy-core.o
+obj-$(CONFIG_PHY_SAMSUNG_SATA)	+= exynos/
diff --git a/drivers/phy/exynos/Kconfig b/drivers/phy/exynos/Kconfig
new file mode 100644
index 0000000..fa125fb
--- /dev/null
+++ b/drivers/phy/exynos/Kconfig
@@ -0,0 +1,5 @@
+config PHY_SAMSUNG_SATA
+	tristate "Samsung Sata SerDes/PHY driver"
+	help
+	  Support for Samsung sata SerDes/Phy found on Samsung
+	  SoCs.
diff --git a/drivers/phy/exynos/Makefile b/drivers/phy/exynos/Makefile
new file mode 100644
index 0000000..50dc7eb
--- /dev/null
+++ b/drivers/phy/exynos/Makefile
@@ -0,0 +1,5 @@
+#
+# Makefile for the exynos phy drivers.
+#
+ccflags-y := -Idrivers/phy/exynos
+obj-$(CONFIG_PHY_SAMSUNG_SATA)	+= sata_phy_exynos5250.o exynos5250_phy_i2c.o
diff --git a/drivers/phy/exynos/exynos5250_phy_i2c.c b/drivers/phy/exynos/exynos5250_phy_i2c.c
new file mode 100644
index 0000000..9c75d3b
--- /dev/null
+++ b/drivers/phy/exynos/exynos5250_phy_i2c.c
@@ -0,0 +1,53 @@
+/*
+ * Copyright (C) 2013 Samsung Electronics Co.Ltd
+ * Author:
+ *	Yuvaraj C D <yuvaraj.cd@samsung.com>
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include "sata_phy_exynos5250.h"
+
+static int exynos_sata_i2c_probe(struct i2c_client *client,
+		const struct i2c_device_id *i2c_id)
+{
+	sataphy_attach_i2c_client(client);
+
+	dev_info(&client->adapter->dev,
+		"attached %s into i2c adapter successfully\n",
+		client->name);
+
+	return 0;
+}
+
+static int exynos_sata_i2c_remove(struct i2c_client *client)
+{
+	dev_info(&client->adapter->dev,
+		"detached %s from i2c adapter successfully\n",
+		client->name);
+
+	return 0;
+}
+
+static const struct i2c_device_id phy_i2c_device_match[] = {
+	{ "sata-phy-i2c", 0 },
+};
+MODULE_DEVICE_TABLE(of, phy_i2c_device_match);
+
+struct i2c_driver sataphy_i2c_driver = {
+	.probe    = exynos_sata_i2c_probe,
+	.id_table = phy_i2c_device_match,
+	.remove		= exynos_sata_i2c_remove,
+	.driver   = {
+		.name = "sata-phy-i2c",
+		.owner = THIS_MODULE,
+		.of_match_table = (void *)phy_i2c_device_match,
+		},
+};
diff --git a/drivers/phy/exynos/sata_phy_exynos5250.c b/drivers/phy/exynos/sata_phy_exynos5250.c
new file mode 100644
index 0000000..726c10e
--- /dev/null
+++ b/drivers/phy/exynos/sata_phy_exynos5250.c
@@ -0,0 +1,248 @@
+/*
+ * Samsung SATA SerDes(PHY) driver
+ *
+ * Copyright (C) 2013 Samsung Electronics Co., Ltd.
+ * Authors: Girish K S <ks.giri@samsung.com>
+ *         Yuvaraj Kumar C D <yuvaraj.cd@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/phy/phy.h>
+#include <linux/i2c.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+#include <linux/clk.h>
+#include "sata_phy_exynos5250.h"
+
+static struct i2c_client *phy_i2c_client;
+
+struct exynos_sata_phy {
+	struct phy *phy;
+	struct clk *phyclk;
+	void __iomem *regs;
+	void __iomem *pmureg;
+};
+
+static bool wait_for_reg_status(void __iomem *base, u32 reg, u32 checkbit,
+				u32 status)
+{
+	unsigned long timeout = jiffies + usecs_to_jiffies(1000);
+	while (time_before(jiffies, timeout)) {
+		if ((readl(base + reg) & checkbit) == status)
+			return true;
+	}
+	return false;
+}
+
+void sataphy_attach_i2c_client(struct i2c_client *sata_phy)
+{
+	if (sata_phy)
+		phy_i2c_client = sata_phy;
+}
+
+static int __set_phy_state(struct exynos_sata_phy *state, unsigned int on)
+{
+	u32 reg;
+
+	reg = readl(state->pmureg);
+	if (on)
+		reg |= EXYNOS_SATA_PHY_EN;
+	else
+		reg &= ~EXYNOS_SATA_PHY_EN;
+	writel(reg, state->pmureg);
+
+	return 0;
+}
+
+static int exynos_sata_phy_power_on(struct phy *phy)
+{
+	struct exynos_sata_phy *state = phy_get_drvdata(phy);
+
+	return __set_phy_state(state, 1);
+}
+
+static int exynos_sata_phy_power_off(struct phy *phy)
+{
+	struct exynos_sata_phy *state = phy_get_drvdata(phy);
+
+	return __set_phy_state(state, 0);
+}
+
+static int exynos_sataphy_parse_dt(struct device *dev,
+				struct exynos_sata_phy *sata)
+{
+	struct device_node *np = dev->of_node;
+	struct device_node *sataphy_pmu;
+
+	sataphy_pmu = of_get_child_by_name(np, "sataphy-pmu");
+	if (!sataphy_pmu) {
+		dev_err(dev, "No PMU interface for sata-phy\n");
+		return -ENODEV;
+	}
+
+	sata->pmureg = of_iomap(sataphy_pmu, 0);
+	if (!sata->pmureg) {
+		dev_err(dev, "Can't get sata-phy pmu control register\n");
+		of_node_put(sataphy_pmu);
+		return -ENXIO;
+	}
+
+	of_node_put(sataphy_pmu);
+	return 0;
+}
+
+static int exynos_sata_phy_init(struct phy *phy)
+{
+	u32 val;
+	int ret = 0;
+	u8 buf[] = { 0x3A, 0x0B };
+	struct exynos_sata_phy *sata_phy = phy_get_drvdata(phy);
+
+	if (!phy_i2c_client)
+		return -EPROBE_DEFER;
+
+	writel(EXYNOS_SATA_PHY_EN, sata_phy->pmureg);
+
+	val = 0;
+	writel(val, sata_phy->regs + EXYNOS5_SATA_RESET);
+
+	val = readl(sata_phy->regs + EXYNOS5_SATA_RESET);
+	val |= 0xFF;
+	writel(val, sata_phy->regs + EXYNOS5_SATA_RESET);
+
+	val = readl(sata_phy->regs + EXYNOS5_SATA_RESET);
+	val |= LINK_RESET;
+	writel(val, sata_phy->regs + EXYNOS5_SATA_RESET);
+
+	val = readl(sata_phy->regs + EXYNOS5_SATA_RESET);
+	val |= RESET_CMN_RST_N;
+	writel(val, sata_phy->regs + EXYNOS5_SATA_RESET);
+	val = readl(sata_phy->regs + EXYNOS5_SATA_PHSATA_CTRLM);
+	val &= ~PHCTRLM_REF_RATE;
+	writel(val, sata_phy->regs + EXYNOS5_SATA_PHSATA_CTRLM);
+
+	/* High speed enable for Gen3 */
+	val = readl(sata_phy->regs + EXYNOS5_SATA_PHSATA_CTRLM);
+	val |= PHCTRLM_HIGH_SPEED;
+	writel(val, sata_phy->regs + EXYNOS5_SATA_PHSATA_CTRLM);
+
+	val |= CTRL0_P0_PHY_CALIBRATED_SEL | CTRL0_P0_PHY_CALIBRATED;
+	writel(val, sata_phy->regs + EXYNOS5_SATA_CTRL0);
+
+	writel(0x2, sata_phy->regs + EXYNOS5_SATA_MODE0);
+
+	ret = i2c_master_send(phy_i2c_client, buf, sizeof(buf));
+	if (ret < 0)
+		return -ENXIO;
+
+	/* release cmu reset */
+	val = readl(sata_phy->regs + EXYNOS5_SATA_RESET);
+	val &= ~RESET_CMN_RST_N;
+	writel(val, sata_phy->regs + EXYNOS5_SATA_RESET);
+
+	val = readl(sata_phy->regs + EXYNOS5_SATA_RESET);
+	val |= RESET_CMN_RST_N;
+	writel(val, sata_phy->regs + EXYNOS5_SATA_RESET);
+
+	return (wait_for_reg_status(sata_phy->regs, EXYNOS5_SATA_PHSATA_STATM,
+		PHSTATM_PLL_LOCKED, 1)) ? 0 : -EINVAL;
+
+}
+
+static struct phy_ops exynos_sata_phy_ops = {
+	.init		= exynos_sata_phy_init,
+	.power_on	= exynos_sata_phy_power_on,
+	.power_off	= exynos_sata_phy_power_off,
+	.owner		= THIS_MODULE,
+};
+
+static int exynos_sata_phy_probe(struct platform_device *pdev)
+{
+	struct exynos_sata_phy *sata;
+	struct device *dev = &pdev->dev;
+	struct resource *res;
+	struct phy_provider *phy_provider;
+	char label[9];
+	int ret = 0;
+
+	sata = devm_kzalloc(dev, sizeof(*sata), GFP_KERNEL);
+	if (!sata)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+	sata->regs = devm_ioremap_resource(dev, res);
+	if (IS_ERR(sata->regs))
+		return PTR_ERR(sata->regs);
+
+	dev_set_drvdata(dev, sata);
+
+	if (i2c_add_driver(&sataphy_i2c_driver)) {
+		dev_err(dev, "failed to register sataphy i2c driver\n");
+		return -ENOENT;
+	}
+
+	sata->phyclk = devm_clk_get(dev, "sata_phyctrl");
+	if (IS_ERR(sata->phyclk)) {
+		dev_err(dev, "failed to get clk for PHY\n");
+		return PTR_ERR(sata->phyclk);
+	}
+
+	ret = clk_prepare_enable(sata->phyclk);
+	if (ret < 0) {
+		dev_err(dev, "failed to enable source clk\n");
+		return ret;
+	}
+
+	if (dev->of_node) {
+		ret = exynos_sataphy_parse_dt(dev, sata);
+		if (ret)
+			return ret;
+	}
+
+	phy_provider = devm_of_phy_provider_register(dev,
+					of_phy_simple_xlate);
+	if (IS_ERR(phy_provider))
+		return PTR_ERR(phy_provider);
+
+	snprintf(label, sizeof(label), "%s.%d", "sata-phy", pdev->id);
+
+	sata->phy = devm_phy_create(dev, pdev->id, &exynos_sata_phy_ops, label);
+	if (IS_ERR(sata->phy)) {
+		dev_err(dev, "failed to create PHY %s\n", label);
+		return PTR_ERR(sata->phy);
+	}
+	phy_set_drvdata(sata->phy, sata);
+
+	return 0;
+}
+
+static const struct of_device_id exynos_sata_phy_of_match[] = {
+	{ .compatible = "samsung,exynos5250-sata-phy" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, exynos_sata_phy_of_match);
+
+static struct platform_driver exynos_sata_phy_driver = {
+	.probe	= exynos_sata_phy_probe,
+	.driver = {
+		.of_match_table	= exynos_sata_phy_of_match,
+		.name  = "samsung,sata-phy",
+		.owner = THIS_MODULE,
+	}
+};
+module_platform_driver(exynos_sata_phy_driver);
+
+MODULE_DESCRIPTION("Samsung SerDes PHY driver");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("ks.giri <ks.giri@samsung.com>");
+MODULE_AUTHOR("Yuvaraj C D <yuvaraj.cd@samsung.com>");
diff --git a/drivers/phy/exynos/sata_phy_exynos5250.h b/drivers/phy/exynos/sata_phy_exynos5250.h
new file mode 100644
index 0000000..64e38a1
--- /dev/null
+++ b/drivers/phy/exynos/sata_phy_exynos5250.h
@@ -0,0 +1,33 @@
+/*
+ *
+ * Copyright (c) 2013 Samsung Electronics Co., Ltd.
+ * Author:
+ *	Yuvaraj Kumar C D<yuvaraj.cd@samsung.com>
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ */
+
+#define EXYNOS5_SATA_RESET		0x4
+#define EXYNOS5_SATA_MODE0              0x10
+#define EXYNOS5_SATA_CTRL0              0x14
+#define EXYNOS5_SATA_STAT0		0x18
+#define EXYNOS5_SATA_PHSATA_CTRLM       0xE0
+#define EXYNOS5_SATA_PHSATA_CTRL0       0xE4
+#define EXYNOS5_SATA_PHSATA_STATM       0xF0
+#define EXYNOS5_SATA_PHSTAT0            0xF4
+
+#define RESET_CMN_RST_N			(1 << 1)
+#define LINK_RESET			0xF0000
+#define CTRL0_P0_PHY_CALIBRATED_SEL	(1 << 9)
+#define CTRL0_P0_PHY_CALIBRATED		(1 << 8)
+#define PHCTRLM_REF_RATE		(1 << 1)
+#define PHCTRLM_HIGH_SPEED		(1 << 0)
+#define PHSTATM_PLL_LOCKED		(1 << 0)
+#define SATA_PHY_CON_RESET              (LINK_RESET | 3F)
+#define EXYNOS_SATA_PHY_EN		(1 << 0)
+
+void sataphy_attach_i2c_client(struct i2c_client *sata_phy);
+extern struct i2c_driver sataphy_i2c_driver;
-- 
1.7.9.5

[PATCH 2/3] Phy: Exynos: Add Exynos5250 sata phy driver

[Sachin Kamat]

Hi Yuvaraj,

On 1 October 2013 12:03, Yuvaraj Kumar C D <yuvaraj.cd@gmail.com> wrote:

> +static int exynos_sata_i2c_remove(struct i2c_client *client)
> +{
> +       dev_info(&client->adapter->dev,
> +               "detached %s from i2c adapter successfully\n",
> +               client->name);
> +
> +       return 0;
> +}
> +

Since this function does not do anything, this could be removed.

> +static const struct i2c_device_id phy_i2c_device_match[] = {
> +       { "sata-phy-i2c", 0 },
> +};
> +MODULE_DEVICE_TABLE(of, phy_i2c_device_match);
> +
> +struct i2c_driver sataphy_i2c_driver = {
> +       .probe    = exynos_sata_i2c_probe,
> +       .id_table = phy_i2c_device_match,
> +       .remove         = exynos_sata_i2c_remove,
> +       .driver   = {
> +               .name = "sata-phy-i2c",
> +               .owner = THIS_MODULE,
> +               .of_match_table = (void *)phy_i2c_device_match,

type casting is not required.

-- 
With warm regards,
Sachin

[PATCH 2/3] Phy: Exynos: Add Exynos5250 sata phy driver

[Kishon Vijay Abraham I]

On Tuesday 01 October 2013 12:03 PM, Yuvaraj Kumar C D wrote:
> This patch adds the sata phy driver for Exynos5250.Exynos5250 sata
> phy comprises of CMU and TRSV blocks which are of I2C register Map.
> So this patch also adds a i2c client driver, which is used configure
> the CMU and TRSV block of exynos5250 SATA PHY.

Why not make the Exynos5250 sata phy as a i2c client driver instead?
> 
> This patch incorporates the generic phy framework to deal with sata
> phy.
> 
> This patch depends on the below patch
> 	[1].drivers: phy: add generic PHY framework
> 		by Kishon Vijay Abraham I<kishon@ti.com>
> 
> Signed-off-by: Yuvaraj Kumar C D <yuvaraj.cd@samsung.com>
> Signed-off-by: Girish K S <ks.giri@samsung.com>
> Signed-off-by: Vasanth Ananthan <vasanth.a@samsung.com>
> ---
>  drivers/phy/Kconfig                      |    6 +
>  drivers/phy/Makefile                     |    1 +
>  drivers/phy/exynos/Kconfig               |    5 +
>  drivers/phy/exynos/Makefile              |    5 +
>  drivers/phy/exynos/exynos5250_phy_i2c.c  |   53 +++++++
>  drivers/phy/exynos/sata_phy_exynos5250.c |  248 ++++++++++++++++++++++++++++++
>  drivers/phy/exynos/sata_phy_exynos5250.h |   33 ++++
>  7 files changed, 351 insertions(+)
>  create mode 100644 drivers/phy/exynos/Kconfig
>  create mode 100644 drivers/phy/exynos/Makefile
>  create mode 100644 drivers/phy/exynos/exynos5250_phy_i2c.c
>  create mode 100644 drivers/phy/exynos/sata_phy_exynos5250.c
>  create mode 100644 drivers/phy/exynos/sata_phy_exynos5250.h
> 
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 5f85909..ab3d1c6 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -11,3 +11,9 @@ menuconfig GENERIC_PHY
>  	  devices present in the kernel. This layer will have the generic
>  	  API by which phy drivers can create PHY using the phy framework and
>  	  phy users can obtain reference to the PHY.
> +
> +if GENERIC_PHY

NAK. Just select GENERIC_PHY from your driver Kconfig.
> +
> +source "drivers/phy/exynos/Kconfig"
> +
> +endif
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index 9e9560f..e0223d7 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -3,3 +3,4 @@
>  #
>  
>  obj-$(CONFIG_GENERIC_PHY)	+= phy-core.o
> +obj-$(CONFIG_PHY_SAMSUNG_SATA)	+= exynos/

simply have phy-exynos5250 in drivers/phy.
> diff --git a/drivers/phy/exynos/Kconfig b/drivers/phy/exynos/Kconfig
> new file mode 100644
> index 0000000..fa125fb
> --- /dev/null
> +++ b/drivers/phy/exynos/Kconfig
> @@ -0,0 +1,5 @@
> +config PHY_SAMSUNG_SATA
> +	tristate "Samsung Sata SerDes/PHY driver"
> +	help
> +	  Support for Samsung sata SerDes/Phy found on Samsung
> +	  SoCs.
> diff --git a/drivers/phy/exynos/Makefile b/drivers/phy/exynos/Makefile
> new file mode 100644
> index 0000000..50dc7eb
> --- /dev/null
> +++ b/drivers/phy/exynos/Makefile
> @@ -0,0 +1,5 @@
> +#
> +# Makefile for the exynos phy drivers.
> +#
> +ccflags-y := -Idrivers/phy/exynos
> +obj-$(CONFIG_PHY_SAMSUNG_SATA)	+= sata_phy_exynos5250.o exynos5250_phy_i2c.o
> diff --git a/drivers/phy/exynos/exynos5250_phy_i2c.c b/drivers/phy/exynos/exynos5250_phy_i2c.c
> new file mode 100644
> index 0000000..9c75d3b
> --- /dev/null
> +++ b/drivers/phy/exynos/exynos5250_phy_i2c.c
> @@ -0,0 +1,53 @@
> +/*
> + * Copyright (C) 2013 Samsung Electronics Co.Ltd
> + * Author:
> + *	Yuvaraj C D <yuvaraj.cd@samsung.com>
> + *
> + * This program is free software; you can redistribute  it and/or modify it
> + * under  the terms of  the GNU General  Public License as published by the
> + * Free Software Foundation;  either version 2 of the  License, or (at your
> + * option) any later version.
> + *
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include "sata_phy_exynos5250.h"
> +
> +static int exynos_sata_i2c_probe(struct i2c_client *client,
> +		const struct i2c_device_id *i2c_id)
> +{
> +	sataphy_attach_i2c_client(client);
> +
> +	dev_info(&client->adapter->dev,
> +		"attached %s into i2c adapter successfully\n",
> +		client->name);
> +
> +	return 0;
> +}
> +
> +static int exynos_sata_i2c_remove(struct i2c_client *client)
> +{
> +	dev_info(&client->adapter->dev,
> +		"detached %s from i2c adapter successfully\n",
> +		client->name);
> +
> +	return 0;
> +}
> +
> +static const struct i2c_device_id phy_i2c_device_match[] = {
> +	{ "sata-phy-i2c", 0 },
> +};
> +MODULE_DEVICE_TABLE(of, phy_i2c_device_match);
> +
> +struct i2c_driver sataphy_i2c_driver = {
> +	.probe    = exynos_sata_i2c_probe,
> +	.id_table = phy_i2c_device_match,
> +	.remove		= exynos_sata_i2c_remove,
> +	.driver   = {
> +		.name = "sata-phy-i2c",
> +		.owner = THIS_MODULE,
> +		.of_match_table = (void *)phy_i2c_device_match,
> +		},
> +};

As I just mentioned above, we can merge this driver with the below one.
> diff --git a/drivers/phy/exynos/sata_phy_exynos5250.c b/drivers/phy/exynos/sata_phy_exynos5250.c
> new file mode 100644
> index 0000000..726c10e
> --- /dev/null
> +++ b/drivers/phy/exynos/sata_phy_exynos5250.c
> @@ -0,0 +1,248 @@
> +/*
> + * Samsung SATA SerDes(PHY) driver
> + *
> + * Copyright (C) 2013 Samsung Electronics Co., Ltd.
> + * Authors: Girish K S <ks.giri@samsung.com>
> + *         Yuvaraj Kumar C D <yuvaraj.cd@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/phy/phy.h>
> +#include <linux/i2c.h>
> +#include <linux/platform_device.h>
> +#include <linux/spinlock.h>
> +#include <linux/clk.h>
> +#include "sata_phy_exynos5250.h"
> +
> +static struct i2c_client *phy_i2c_client;
> +
> +struct exynos_sata_phy {
> +	struct phy *phy;
> +	struct clk *phyclk;
> +	void __iomem *regs;
> +	void __iomem *pmureg;
> +};
> +
> +static bool wait_for_reg_status(void __iomem *base, u32 reg, u32 checkbit,
> +				u32 status)
> +{
> +	unsigned long timeout = jiffies + usecs_to_jiffies(1000);
> +	while (time_before(jiffies, timeout)) {
> +		if ((readl(base + reg) & checkbit) == status)
> +			return true;
> +	}
> +	return false;
> +}
> +
> +void sataphy_attach_i2c_client(struct i2c_client *sata_phy)
> +{
> +	if (sata_phy)
> +		phy_i2c_client = sata_phy;
> +}
> +
> +static int __set_phy_state(struct exynos_sata_phy *state, unsigned int on)
> +{
> +	u32 reg;
> +
> +	reg = readl(state->pmureg);
> +	if (on)
> +		reg |= EXYNOS_SATA_PHY_EN;
> +	else
> +		reg &= ~EXYNOS_SATA_PHY_EN;
> +	writel(reg, state->pmureg);
> +
> +	return 0;
> +}
> +
> +static int exynos_sata_phy_power_on(struct phy *phy)
> +{
> +	struct exynos_sata_phy *state = phy_get_drvdata(phy);
> +
> +	return __set_phy_state(state, 1);
> +}
> +
> +static int exynos_sata_phy_power_off(struct phy *phy)
> +{
> +	struct exynos_sata_phy *state = phy_get_drvdata(phy);
> +
> +	return __set_phy_state(state, 0);
> +}
> +
> +static int exynos_sataphy_parse_dt(struct device *dev,
> +				struct exynos_sata_phy *sata)
> +{
> +	struct device_node *np = dev->of_node;
> +	struct device_node *sataphy_pmu;
> +
> +	sataphy_pmu = of_get_child_by_name(np, "sataphy-pmu");
> +	if (!sataphy_pmu) {
> +		dev_err(dev, "No PMU interface for sata-phy\n");
> +		return -ENODEV;
> +	}
> +
> +	sata->pmureg = of_iomap(sataphy_pmu, 0);
> +	if (!sata->pmureg) {
> +		dev_err(dev, "Can't get sata-phy pmu control register\n");
> +		of_node_put(sataphy_pmu);
> +		return -ENXIO;
> +	}
> +
> +	of_node_put(sataphy_pmu);
> +	return 0;
> +}
> +
> +static int exynos_sata_phy_init(struct phy *phy)
> +{
> +	u32 val;
> +	int ret = 0;
> +	u8 buf[] = { 0x3A, 0x0B };
> +	struct exynos_sata_phy *sata_phy = phy_get_drvdata(phy);
> +
> +	if (!phy_i2c_client)
> +		return -EPROBE_DEFER;
> +
> +	writel(EXYNOS_SATA_PHY_EN, sata_phy->pmureg);
> +
> +	val = 0;
> +	writel(val, sata_phy->regs + EXYNOS5_SATA_RESET);
> +
> +	val = readl(sata_phy->regs + EXYNOS5_SATA_RESET);
> +	val |= 0xFF;
> +	writel(val, sata_phy->regs + EXYNOS5_SATA_RESET);
> +
> +	val = readl(sata_phy->regs + EXYNOS5_SATA_RESET);
> +	val |= LINK_RESET;
> +	writel(val, sata_phy->regs + EXYNOS5_SATA_RESET);
> +
> +	val = readl(sata_phy->regs + EXYNOS5_SATA_RESET);
> +	val |= RESET_CMN_RST_N;
> +	writel(val, sata_phy->regs + EXYNOS5_SATA_RESET);
> +	val = readl(sata_phy->regs + EXYNOS5_SATA_PHSATA_CTRLM);
> +	val &= ~PHCTRLM_REF_RATE;
> +	writel(val, sata_phy->regs + EXYNOS5_SATA_PHSATA_CTRLM);
> +
> +	/* High speed enable for Gen3 */
> +	val = readl(sata_phy->regs + EXYNOS5_SATA_PHSATA_CTRLM);
> +	val |= PHCTRLM_HIGH_SPEED;
> +	writel(val, sata_phy->regs + EXYNOS5_SATA_PHSATA_CTRLM);
> +
> +	val |= CTRL0_P0_PHY_CALIBRATED_SEL | CTRL0_P0_PHY_CALIBRATED;
> +	writel(val, sata_phy->regs + EXYNOS5_SATA_CTRL0);
> +
> +	writel(0x2, sata_phy->regs + EXYNOS5_SATA_MODE0);
> +
> +	ret = i2c_master_send(phy_i2c_client, buf, sizeof(buf));
> +	if (ret < 0)
> +		return -ENXIO;
> +
> +	/* release cmu reset */
> +	val = readl(sata_phy->regs + EXYNOS5_SATA_RESET);
> +	val &= ~RESET_CMN_RST_N;
> +	writel(val, sata_phy->regs + EXYNOS5_SATA_RESET);
> +
> +	val = readl(sata_phy->regs + EXYNOS5_SATA_RESET);
> +	val |= RESET_CMN_RST_N;
> +	writel(val, sata_phy->regs + EXYNOS5_SATA_RESET);
> +
> +	return (wait_for_reg_status(sata_phy->regs, EXYNOS5_SATA_PHSATA_STATM,
> +		PHSTATM_PLL_LOCKED, 1)) ? 0 : -EINVAL;
> +
> +}
> +
> +static struct phy_ops exynos_sata_phy_ops = {
> +	.init		= exynos_sata_phy_init,
> +	.power_on	= exynos_sata_phy_power_on,
> +	.power_off	= exynos_sata_phy_power_off,
> +	.owner		= THIS_MODULE,
> +};
> +
> +static int exynos_sata_phy_probe(struct platform_device *pdev)
> +{
> +	struct exynos_sata_phy *sata;
> +	struct device *dev = &pdev->dev;
> +	struct resource *res;
> +	struct phy_provider *phy_provider;
> +	char label[9];
> +	int ret = 0;
> +
> +	sata = devm_kzalloc(dev, sizeof(*sata), GFP_KERNEL);
> +	if (!sata)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +
> +	sata->regs = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(sata->regs))
> +		return PTR_ERR(sata->regs);
> +
> +	dev_set_drvdata(dev, sata);
> +
> +	if (i2c_add_driver(&sataphy_i2c_driver)) {
> +		dev_err(dev, "failed to register sataphy i2c driver\n");
> +		return -ENOENT;
> +	}
> +
> +	sata->phyclk = devm_clk_get(dev, "sata_phyctrl");
> +	if (IS_ERR(sata->phyclk)) {
> +		dev_err(dev, "failed to get clk for PHY\n");
> +		return PTR_ERR(sata->phyclk);
> +	}
> +
> +	ret = clk_prepare_enable(sata->phyclk);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to enable source clk\n");
> +		return ret;
> +	}
> +
> +	if (dev->of_node) {
> +		ret = exynos_sataphy_parse_dt(dev, sata);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	phy_provider = devm_of_phy_provider_register(dev,
> +					of_phy_simple_xlate);
> +	if (IS_ERR(phy_provider))
> +		return PTR_ERR(phy_provider);
> +
> +	snprintf(label, sizeof(label), "%s.%d", "sata-phy", pdev->id);
> +
> +	sata->phy = devm_phy_create(dev, pdev->id, &exynos_sata_phy_ops, label);

Alright. You are using a older version of PHY framework. I'll wait for you to
adapt to the latest version of PHY framework [1] before reviewing any further.

[1] git://git.kernel.org/pub/scm/linux/kernel/git/kishon/linux-phy.git next

Thanks
Kishon

[PATCH 2/3] Phy: Exynos: Add Exynos5250 sata phy driver

[Yuvaraj Cd]

On Tue, Oct 1, 2013 at 6:21 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote:
> On Tuesday 01 October 2013 12:03 PM, Yuvaraj Kumar C D wrote:
>> This patch adds the sata phy driver for Exynos5250.Exynos5250 sata
>> phy comprises of CMU and TRSV blocks which are of I2C register Map.
>> So this patch also adds a i2c client driver, which is used configure
>> the CMU and TRSV block of exynos5250 SATA PHY.
>
> Why not make the Exynos5250 sata phy as a i2c client driver instead?
>>
>> This patch incorporates the generic phy framework to deal with sata
>> phy.
>>
>> This patch depends on the below patch
>>       [1].drivers: phy: add generic PHY framework
>>               by Kishon Vijay Abraham I<kishon@ti.com>
>>
>> Signed-off-by: Yuvaraj Kumar C D <yuvaraj.cd@samsung.com>
>> Signed-off-by: Girish K S <ks.giri@samsung.com>
>> Signed-off-by: Vasanth Ananthan <vasanth.a@samsung.com>
>> ---
>>  drivers/phy/Kconfig                      |    6 +
>>  drivers/phy/Makefile                     |    1 +
>>  drivers/phy/exynos/Kconfig               |    5 +
>>  drivers/phy/exynos/Makefile              |    5 +
>>  drivers/phy/exynos/exynos5250_phy_i2c.c  |   53 +++++++
>>  drivers/phy/exynos/sata_phy_exynos5250.c |  248 ++++++++++++++++++++++++++++++
>>  drivers/phy/exynos/sata_phy_exynos5250.h |   33 ++++
>>  7 files changed, 351 insertions(+)
>>  create mode 100644 drivers/phy/exynos/Kconfig
>>  create mode 100644 drivers/phy/exynos/Makefile
>>  create mode 100644 drivers/phy/exynos/exynos5250_phy_i2c.c
>>  create mode 100644 drivers/phy/exynos/sata_phy_exynos5250.c
>>  create mode 100644 drivers/phy/exynos/sata_phy_exynos5250.h
>>
>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>> index 5f85909..ab3d1c6 100644
>> --- a/drivers/phy/Kconfig
>> +++ b/drivers/phy/Kconfig
>> @@ -11,3 +11,9 @@ menuconfig GENERIC_PHY
>>         devices present in the kernel. This layer will have the generic
>>         API by which phy drivers can create PHY using the phy framework and
>>         phy users can obtain reference to the PHY.
>> +
>> +if GENERIC_PHY
>
> NAK. Just select GENERIC_PHY from your driver Kconfig.
>> +
>> +source "drivers/phy/exynos/Kconfig"
>> +
>> +endif
>> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
>> index 9e9560f..e0223d7 100644
>> --- a/drivers/phy/Makefile
>> +++ b/drivers/phy/Makefile
>> @@ -3,3 +3,4 @@
>>  #
>>
>>  obj-$(CONFIG_GENERIC_PHY)    += phy-core.o
>> +obj-$(CONFIG_PHY_SAMSUNG_SATA)       += exynos/
>
> simply have phy-exynos5250 in drivers/phy.
ok.
>> diff --git a/drivers/phy/exynos/Kconfig b/drivers/phy/exynos/Kconfig
>> new file mode 100644
>> index 0000000..fa125fb
>> --- /dev/null
>> +++ b/drivers/phy/exynos/Kconfig
>> @@ -0,0 +1,5 @@
>> +config PHY_SAMSUNG_SATA
>> +     tristate "Samsung Sata SerDes/PHY driver"
>> +     help
>> +       Support for Samsung sata SerDes/Phy found on Samsung
>> +       SoCs.
>> diff --git a/drivers/phy/exynos/Makefile b/drivers/phy/exynos/Makefile
>> new file mode 100644
>> index 0000000..50dc7eb
>> --- /dev/null
>> +++ b/drivers/phy/exynos/Makefile
>> @@ -0,0 +1,5 @@
>> +#
>> +# Makefile for the exynos phy drivers.
>> +#
>> +ccflags-y := -Idrivers/phy/exynos
>> +obj-$(CONFIG_PHY_SAMSUNG_SATA)       += sata_phy_exynos5250.o exynos5250_phy_i2c.o
>> diff --git a/drivers/phy/exynos/exynos5250_phy_i2c.c b/drivers/phy/exynos/exynos5250_phy_i2c.c
>> new file mode 100644
>> index 0000000..9c75d3b
>> --- /dev/null
>> +++ b/drivers/phy/exynos/exynos5250_phy_i2c.c
>> @@ -0,0 +1,53 @@
>> +/*
>> + * Copyright (C) 2013 Samsung Electronics Co.Ltd
>> + * Author:
>> + *   Yuvaraj C D <yuvaraj.cd@samsung.com>
>> + *
>> + * This program is free software; you can redistribute  it and/or modify it
>> + * under  the terms of  the GNU General  Public License as published by the
>> + * Free Software Foundation;  either version 2 of the  License, or (at your
>> + * option) any later version.
>> + *
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/i2c.h>
>> +#include <linux/module.h>
>> +#include "sata_phy_exynos5250.h"
>> +
>> +static int exynos_sata_i2c_probe(struct i2c_client *client,
>> +             const struct i2c_device_id *i2c_id)
>> +{
>> +     sataphy_attach_i2c_client(client);
>> +
>> +     dev_info(&client->adapter->dev,
>> +             "attached %s into i2c adapter successfully\n",
>> +             client->name);
>> +
>> +     return 0;
>> +}
>> +
>> +static int exynos_sata_i2c_remove(struct i2c_client *client)
>> +{
>> +     dev_info(&client->adapter->dev,
>> +             "detached %s from i2c adapter successfully\n",
>> +             client->name);
>> +
>> +     return 0;
>> +}
>> +
>> +static const struct i2c_device_id phy_i2c_device_match[] = {
>> +     { "sata-phy-i2c", 0 },
>> +};
>> +MODULE_DEVICE_TABLE(of, phy_i2c_device_match);
>> +
>> +struct i2c_driver sataphy_i2c_driver = {
>> +     .probe    = exynos_sata_i2c_probe,
>> +     .id_table = phy_i2c_device_match,
>> +     .remove         = exynos_sata_i2c_remove,
>> +     .driver   = {
>> +             .name = "sata-phy-i2c",
>> +             .owner = THIS_MODULE,
>> +             .of_match_table = (void *)phy_i2c_device_match,
>> +             },
>> +};
>
> As I just mentioned above, we can merge this driver with the below one.
True, Initially it was merged.But already existing drivers of which
are of similar to this kind were done in this way.
Please refer  /drivers/gpu/drm//exynos/exynos_hdmiphy.c

>> diff --git a/drivers/phy/exynos/sata_phy_exynos5250.c b/drivers/phy/exynos/sata_phy_exynos5250.c
>> new file mode 100644
>> index 0000000..726c10e
>> --- /dev/null
>> +++ b/drivers/phy/exynos/sata_phy_exynos5250.c
>> @@ -0,0 +1,248 @@
>> +/*
>> + * Samsung SATA SerDes(PHY) driver
>> + *
>> + * Copyright (C) 2013 Samsung Electronics Co., Ltd.
>> + * Authors: Girish K S <ks.giri@samsung.com>
>> + *         Yuvaraj Kumar C D <yuvaraj.cd@samsung.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include <linux/delay.h>
>> +#include <linux/io.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_address.h>
>> +#include <linux/phy/phy.h>
>> +#include <linux/i2c.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/spinlock.h>
>> +#include <linux/clk.h>
>> +#include "sata_phy_exynos5250.h"
>> +
>> +static struct i2c_client *phy_i2c_client;
>> +
>> +struct exynos_sata_phy {
>> +     struct phy *phy;
>> +     struct clk *phyclk;
>> +     void __iomem *regs;
>> +     void __iomem *pmureg;
>> +};
>> +
>> +static bool wait_for_reg_status(void __iomem *base, u32 reg, u32 checkbit,
>> +                             u32 status)
>> +{
>> +     unsigned long timeout = jiffies + usecs_to_jiffies(1000);
>> +     while (time_before(jiffies, timeout)) {
>> +             if ((readl(base + reg) & checkbit) == status)
>> +                     return true;
>> +     }
>> +     return false;
>> +}
>> +
>> +void sataphy_attach_i2c_client(struct i2c_client *sata_phy)
>> +{
>> +     if (sata_phy)
>> +             phy_i2c_client = sata_phy;
>> +}
>> +
>> +static int __set_phy_state(struct exynos_sata_phy *state, unsigned int on)
>> +{
>> +     u32 reg;
>> +
>> +     reg = readl(state->pmureg);
>> +     if (on)
>> +             reg |= EXYNOS_SATA_PHY_EN;
>> +     else
>> +             reg &= ~EXYNOS_SATA_PHY_EN;
>> +     writel(reg, state->pmureg);
>> +
>> +     return 0;
>> +}
>> +
>> +static int exynos_sata_phy_power_on(struct phy *phy)
>> +{
>> +     struct exynos_sata_phy *state = phy_get_drvdata(phy);
>> +
>> +     return __set_phy_state(state, 1);
>> +}
>> +
>> +static int exynos_sata_phy_power_off(struct phy *phy)
>> +{
>> +     struct exynos_sata_phy *state = phy_get_drvdata(phy);
>> +
>> +     return __set_phy_state(state, 0);
>> +}
>> +
>> +static int exynos_sataphy_parse_dt(struct device *dev,
>> +                             struct exynos_sata_phy *sata)
>> +{
>> +     struct device_node *np = dev->of_node;
>> +     struct device_node *sataphy_pmu;
>> +
>> +     sataphy_pmu = of_get_child_by_name(np, "sataphy-pmu");
>> +     if (!sataphy_pmu) {
>> +             dev_err(dev, "No PMU interface for sata-phy\n");
>> +             return -ENODEV;
>> +     }
>> +
>> +     sata->pmureg = of_iomap(sataphy_pmu, 0);
>> +     if (!sata->pmureg) {
>> +             dev_err(dev, "Can't get sata-phy pmu control register\n");
>> +             of_node_put(sataphy_pmu);
>> +             return -ENXIO;
>> +     }
>> +
>> +     of_node_put(sataphy_pmu);
>> +     return 0;
>> +}
>> +
>> +static int exynos_sata_phy_init(struct phy *phy)
>> +{
>> +     u32 val;
>> +     int ret = 0;
>> +     u8 buf[] = { 0x3A, 0x0B };
>> +     struct exynos_sata_phy *sata_phy = phy_get_drvdata(phy);
>> +
>> +     if (!phy_i2c_client)
>> +             return -EPROBE_DEFER;
>> +
>> +     writel(EXYNOS_SATA_PHY_EN, sata_phy->pmureg);
>> +
>> +     val = 0;
>> +     writel(val, sata_phy->regs + EXYNOS5_SATA_RESET);
>> +
>> +     val = readl(sata_phy->regs + EXYNOS5_SATA_RESET);
>> +     val |= 0xFF;
>> +     writel(val, sata_phy->regs + EXYNOS5_SATA_RESET);
>> +
>> +     val = readl(sata_phy->regs + EXYNOS5_SATA_RESET);
>> +     val |= LINK_RESET;
>> +     writel(val, sata_phy->regs + EXYNOS5_SATA_RESET);
>> +
>> +     val = readl(sata_phy->regs + EXYNOS5_SATA_RESET);
>> +     val |= RESET_CMN_RST_N;
>> +     writel(val, sata_phy->regs + EXYNOS5_SATA_RESET);
>> +     val = readl(sata_phy->regs + EXYNOS5_SATA_PHSATA_CTRLM);
>> +     val &= ~PHCTRLM_REF_RATE;
>> +     writel(val, sata_phy->regs + EXYNOS5_SATA_PHSATA_CTRLM);
>> +
>> +     /* High speed enable for Gen3 */
>> +     val = readl(sata_phy->regs + EXYNOS5_SATA_PHSATA_CTRLM);
>> +     val |= PHCTRLM_HIGH_SPEED;
>> +     writel(val, sata_phy->regs + EXYNOS5_SATA_PHSATA_CTRLM);
>> +
>> +     val |= CTRL0_P0_PHY_CALIBRATED_SEL | CTRL0_P0_PHY_CALIBRATED;
>> +     writel(val, sata_phy->regs + EXYNOS5_SATA_CTRL0);
>> +
>> +     writel(0x2, sata_phy->regs + EXYNOS5_SATA_MODE0);
>> +
>> +     ret = i2c_master_send(phy_i2c_client, buf, sizeof(buf));
>> +     if (ret < 0)
>> +             return -ENXIO;
>> +
>> +     /* release cmu reset */
>> +     val = readl(sata_phy->regs + EXYNOS5_SATA_RESET);
>> +     val &= ~RESET_CMN_RST_N;
>> +     writel(val, sata_phy->regs + EXYNOS5_SATA_RESET);
>> +
>> +     val = readl(sata_phy->regs + EXYNOS5_SATA_RESET);
>> +     val |= RESET_CMN_RST_N;
>> +     writel(val, sata_phy->regs + EXYNOS5_SATA_RESET);
>> +
>> +     return (wait_for_reg_status(sata_phy->regs, EXYNOS5_SATA_PHSATA_STATM,
>> +             PHSTATM_PLL_LOCKED, 1)) ? 0 : -EINVAL;
>> +
>> +}
>> +
>> +static struct phy_ops exynos_sata_phy_ops = {
>> +     .init           = exynos_sata_phy_init,
>> +     .power_on       = exynos_sata_phy_power_on,
>> +     .power_off      = exynos_sata_phy_power_off,
>> +     .owner          = THIS_MODULE,
>> +};
>> +
>> +static int exynos_sata_phy_probe(struct platform_device *pdev)
>> +{
>> +     struct exynos_sata_phy *sata;
>> +     struct device *dev = &pdev->dev;
>> +     struct resource *res;
>> +     struct phy_provider *phy_provider;
>> +     char label[9];
>> +     int ret = 0;
>> +
>> +     sata = devm_kzalloc(dev, sizeof(*sata), GFP_KERNEL);
>> +     if (!sata)
>> +             return -ENOMEM;
>> +
>> +     res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +
>> +     sata->regs = devm_ioremap_resource(dev, res);
>> +     if (IS_ERR(sata->regs))
>> +             return PTR_ERR(sata->regs);
>> +
>> +     dev_set_drvdata(dev, sata);
>> +
>> +     if (i2c_add_driver(&sataphy_i2c_driver)) {
>> +             dev_err(dev, "failed to register sataphy i2c driver\n");
>> +             return -ENOENT;
>> +     }
>> +
>> +     sata->phyclk = devm_clk_get(dev, "sata_phyctrl");
>> +     if (IS_ERR(sata->phyclk)) {
>> +             dev_err(dev, "failed to get clk for PHY\n");
>> +             return PTR_ERR(sata->phyclk);
>> +     }
>> +
>> +     ret = clk_prepare_enable(sata->phyclk);
>> +     if (ret < 0) {
>> +             dev_err(dev, "failed to enable source clk\n");
>> +             return ret;
>> +     }
>> +
>> +     if (dev->of_node) {
>> +             ret = exynos_sataphy_parse_dt(dev, sata);
>> +             if (ret)
>> +                     return ret;
>> +     }
>> +
>> +     phy_provider = devm_of_phy_provider_register(dev,
>> +                                     of_phy_simple_xlate);
>> +     if (IS_ERR(phy_provider))
>> +             return PTR_ERR(phy_provider);
>> +
>> +     snprintf(label, sizeof(label), "%s.%d", "sata-phy", pdev->id);
>> +
>> +     sata->phy = devm_phy_create(dev, pdev->id, &exynos_sata_phy_ops, label);
>
> Alright. You are using a older version of PHY framework. I'll wait for you to
> adapt to the latest version of PHY framework [1] before reviewing any further.
Sure,I will adapt to the latest version of PHY framework and post soon.
>
> [1] git://git.kernel.org/pub/scm/linux/kernel/git/kishon/linux-phy.git next
>
> Thanks
> Kishon
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/3] Phy: Exynos: Add Exynos5250 sata phy driver

[Kishon Vijay Abraham I]

Hi,

On Monday 07 October 2013 07:35 PM, Yuvaraj Cd wrote:
> On Tue, Oct 1, 2013 at 6:21 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote:
>> On Tuesday 01 October 2013 12:03 PM, Yuvaraj Kumar C D wrote:
>>> This patch adds the sata phy driver for Exynos5250.Exynos5250 sata
>>> phy comprises of CMU and TRSV blocks which are of I2C register Map.
>>> So this patch also adds a i2c client driver, which is used configure
>>> the CMU and TRSV block of exynos5250 SATA PHY.
>>
>> Why not make the Exynos5250 sata phy as a i2c client driver instead?
>>>
>>> This patch incorporates the generic phy framework to deal with sata
>>> phy.
>>>
>>> This patch depends on the below patch
>>>       [1].drivers: phy: add generic PHY framework
>>>               by Kishon Vijay Abraham I<kishon@ti.com>
>>>
>>> Signed-off-by: Yuvaraj Kumar C D <yuvaraj.cd@samsung.com>
>>> Signed-off-by: Girish K S <ks.giri@samsung.com>
>>> Signed-off-by: Vasanth Ananthan <vasanth.a@samsung.com>
>>> ---
>>>  drivers/phy/Kconfig                      |    6 +
>>>  drivers/phy/Makefile                     |    1 +
>>>  drivers/phy/exynos/Kconfig               |    5 +
>>>  drivers/phy/exynos/Makefile              |    5 +
>>>  drivers/phy/exynos/exynos5250_phy_i2c.c  |   53 +++++++
>>>  drivers/phy/exynos/sata_phy_exynos5250.c |  248 ++++++++++++++++++++++++++++++
>>>  drivers/phy/exynos/sata_phy_exynos5250.h |   33 ++++
>>>  7 files changed, 351 insertions(+)
>>>  create mode 100644 drivers/phy/exynos/Kconfig
>>>  create mode 100644 drivers/phy/exynos/Makefile
>>>  create mode 100644 drivers/phy/exynos/exynos5250_phy_i2c.c
>>>  create mode 100644 drivers/phy/exynos/sata_phy_exynos5250.c
>>>  create mode 100644 drivers/phy/exynos/sata_phy_exynos5250.h
>>>
>>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>>> index 5f85909..ab3d1c6 100644
>>> --- a/drivers/phy/Kconfig
>>> +++ b/drivers/phy/Kconfig
>>> @@ -11,3 +11,9 @@ menuconfig GENERIC_PHY
>>>         devices present in the kernel. This layer will have the generic
>>>         API by which phy drivers can create PHY using the phy framework and
>>>         phy users can obtain reference to the PHY.
>>> +
>>> +if GENERIC_PHY
>>
>> NAK. Just select GENERIC_PHY from your driver Kconfig.
>>> +
>>> +source "drivers/phy/exynos/Kconfig"
>>> +
>>> +endif
>>> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
>>> index 9e9560f..e0223d7 100644
>>> --- a/drivers/phy/Makefile
>>> +++ b/drivers/phy/Makefile
>>> @@ -3,3 +3,4 @@
>>>  #
>>>
>>>  obj-$(CONFIG_GENERIC_PHY)    += phy-core.o
>>> +obj-$(CONFIG_PHY_SAMSUNG_SATA)       += exynos/
>>
>> simply have phy-exynos5250 in drivers/phy.
> ok.
>>> diff --git a/drivers/phy/exynos/Kconfig b/drivers/phy/exynos/Kconfig
>>> new file mode 100644
>>> index 0000000..fa125fb
>>> --- /dev/null
>>> +++ b/drivers/phy/exynos/Kconfig
>>> @@ -0,0 +1,5 @@
>>> +config PHY_SAMSUNG_SATA
>>> +     tristate "Samsung Sata SerDes/PHY driver"
>>> +     help
>>> +       Support for Samsung sata SerDes/Phy found on Samsung
>>> +       SoCs.
>>> diff --git a/drivers/phy/exynos/Makefile b/drivers/phy/exynos/Makefile
>>> new file mode 100644
>>> index 0000000..50dc7eb
>>> --- /dev/null
>>> +++ b/drivers/phy/exynos/Makefile
>>> @@ -0,0 +1,5 @@
>>> +#
>>> +# Makefile for the exynos phy drivers.
>>> +#
>>> +ccflags-y := -Idrivers/phy/exynos
>>> +obj-$(CONFIG_PHY_SAMSUNG_SATA)       += sata_phy_exynos5250.o exynos5250_phy_i2c.o
>>> diff --git a/drivers/phy/exynos/exynos5250_phy_i2c.c b/drivers/phy/exynos/exynos5250_phy_i2c.c
>>> new file mode 100644
>>> index 0000000..9c75d3b
>>> --- /dev/null
>>> +++ b/drivers/phy/exynos/exynos5250_phy_i2c.c
>>> @@ -0,0 +1,53 @@
>>> +/*
>>> + * Copyright (C) 2013 Samsung Electronics Co.Ltd
>>> + * Author:
>>> + *   Yuvaraj C D <yuvaraj.cd@samsung.com>
>>> + *
>>> + * This program is free software; you can redistribute  it and/or modify it
>>> + * under  the terms of  the GNU General  Public License as published by the
>>> + * Free Software Foundation;  either version 2 of the  License, or (at your
>>> + * option) any later version.
>>> + *
>>> + */
>>> +
>>> +#include <linux/kernel.h>
>>> +#include <linux/i2c.h>
>>> +#include <linux/module.h>
>>> +#include "sata_phy_exynos5250.h"
>>> +
>>> +static int exynos_sata_i2c_probe(struct i2c_client *client,
>>> +             const struct i2c_device_id *i2c_id)
>>> +{
>>> +     sataphy_attach_i2c_client(client);
>>> +
>>> +     dev_info(&client->adapter->dev,
>>> +             "attached %s into i2c adapter successfully\n",
>>> +             client->name);
>>> +
>>> +     return 0;
>>> +}
>>> +
>>> +static int exynos_sata_i2c_remove(struct i2c_client *client)
>>> +{
>>> +     dev_info(&client->adapter->dev,
>>> +             "detached %s from i2c adapter successfully\n",
>>> +             client->name);
>>> +
>>> +     return 0;
>>> +}
>>> +
>>> +static const struct i2c_device_id phy_i2c_device_match[] = {
>>> +     { "sata-phy-i2c", 0 },
>>> +};
>>> +MODULE_DEVICE_TABLE(of, phy_i2c_device_match);
>>> +
>>> +struct i2c_driver sataphy_i2c_driver = {
>>> +     .probe    = exynos_sata_i2c_probe,
>>> +     .id_table = phy_i2c_device_match,
>>> +     .remove         = exynos_sata_i2c_remove,
>>> +     .driver   = {
>>> +             .name = "sata-phy-i2c",
>>> +             .owner = THIS_MODULE,
>>> +             .of_match_table = (void *)phy_i2c_device_match,
>>> +             },
>>> +};
>>
>> As I just mentioned above, we can merge this driver with the below one.
> True, Initially it was merged.But already existing drivers of which
> are of similar to this kind were done in this way.
> Please refer  /drivers/gpu/drm//exynos/exynos_hdmiphy.c

Can you point to any discussions where it was decided to go with this approach?

Thanks
Kishon

[PATCH 2/3] Phy: Exynos: Add Exynos5250 sata phy driver

[Yuvaraj Kumar]

On Thu, Nov 14, 2013 at 11:18 AM, Kishon Vijay Abraham I <kishon@ti.com> wrote:
> Hi,
>
> On Monday 07 October 2013 07:35 PM, Yuvaraj Cd wrote:
>> On Tue, Oct 1, 2013 at 6:21 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote:
>>> On Tuesday 01 October 2013 12:03 PM, Yuvaraj Kumar C D wrote:
>>>> This patch adds the sata phy driver for Exynos5250.Exynos5250 sata
>>>> phy comprises of CMU and TRSV blocks which are of I2C register Map.
>>>> So this patch also adds a i2c client driver, which is used configure
>>>> the CMU and TRSV block of exynos5250 SATA PHY.
>>>
>>> Why not make the Exynos5250 sata phy as a i2c client driver instead?
>>>>
>>>> This patch incorporates the generic phy framework to deal with sata
>>>> phy.
>>>>
>>>> This patch depends on the below patch
>>>>       [1].drivers: phy: add generic PHY framework
>>>>               by Kishon Vijay Abraham I<kishon@ti.com>
>>>>
>>>> Signed-off-by: Yuvaraj Kumar C D <yuvaraj.cd@samsung.com>
>>>> Signed-off-by: Girish K S <ks.giri@samsung.com>
>>>> Signed-off-by: Vasanth Ananthan <vasanth.a@samsung.com>
>>>> ---
>>>>  drivers/phy/Kconfig                      |    6 +
>>>>  drivers/phy/Makefile                     |    1 +
>>>>  drivers/phy/exynos/Kconfig               |    5 +
>>>>  drivers/phy/exynos/Makefile              |    5 +
>>>>  drivers/phy/exynos/exynos5250_phy_i2c.c  |   53 +++++++
>>>>  drivers/phy/exynos/sata_phy_exynos5250.c |  248 ++++++++++++++++++++++++++++++
>>>>  drivers/phy/exynos/sata_phy_exynos5250.h |   33 ++++
>>>>  7 files changed, 351 insertions(+)
>>>>  create mode 100644 drivers/phy/exynos/Kconfig
>>>>  create mode 100644 drivers/phy/exynos/Makefile
>>>>  create mode 100644 drivers/phy/exynos/exynos5250_phy_i2c.c
>>>>  create mode 100644 drivers/phy/exynos/sata_phy_exynos5250.c
>>>>  create mode 100644 drivers/phy/exynos/sata_phy_exynos5250.h
>>>>
>>>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>>>> index 5f85909..ab3d1c6 100644
>>>> --- a/drivers/phy/Kconfig
>>>> +++ b/drivers/phy/Kconfig
>>>> @@ -11,3 +11,9 @@ menuconfig GENERIC_PHY
>>>>         devices present in the kernel. This layer will have the generic
>>>>         API by which phy drivers can create PHY using the phy framework and
>>>>         phy users can obtain reference to the PHY.
>>>> +
>>>> +if GENERIC_PHY
>>>
>>> NAK. Just select GENERIC_PHY from your driver Kconfig.
>>>> +
>>>> +source "drivers/phy/exynos/Kconfig"
>>>> +
>>>> +endif
>>>> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
>>>> index 9e9560f..e0223d7 100644
>>>> --- a/drivers/phy/Makefile
>>>> +++ b/drivers/phy/Makefile
>>>> @@ -3,3 +3,4 @@
>>>>  #
>>>>
>>>>  obj-$(CONFIG_GENERIC_PHY)    += phy-core.o
>>>> +obj-$(CONFIG_PHY_SAMSUNG_SATA)       += exynos/
>>>
>>> simply have phy-exynos5250 in drivers/phy.
>> ok.
>>>> diff --git a/drivers/phy/exynos/Kconfig b/drivers/phy/exynos/Kconfig
>>>> new file mode 100644
>>>> index 0000000..fa125fb
>>>> --- /dev/null
>>>> +++ b/drivers/phy/exynos/Kconfig
>>>> @@ -0,0 +1,5 @@
>>>> +config PHY_SAMSUNG_SATA
>>>> +     tristate "Samsung Sata SerDes/PHY driver"
>>>> +     help
>>>> +       Support for Samsung sata SerDes/Phy found on Samsung
>>>> +       SoCs.
>>>> diff --git a/drivers/phy/exynos/Makefile b/drivers/phy/exynos/Makefile
>>>> new file mode 100644
>>>> index 0000000..50dc7eb
>>>> --- /dev/null
>>>> +++ b/drivers/phy/exynos/Makefile
>>>> @@ -0,0 +1,5 @@
>>>> +#
>>>> +# Makefile for the exynos phy drivers.
>>>> +#
>>>> +ccflags-y := -Idrivers/phy/exynos
>>>> +obj-$(CONFIG_PHY_SAMSUNG_SATA)       += sata_phy_exynos5250.o exynos5250_phy_i2c.o
>>>> diff --git a/drivers/phy/exynos/exynos5250_phy_i2c.c b/drivers/phy/exynos/exynos5250_phy_i2c.c
>>>> new file mode 100644
>>>> index 0000000..9c75d3b
>>>> --- /dev/null
>>>> +++ b/drivers/phy/exynos/exynos5250_phy_i2c.c
>>>> @@ -0,0 +1,53 @@
>>>> +/*
>>>> + * Copyright (C) 2013 Samsung Electronics Co.Ltd
>>>> + * Author:
>>>> + *   Yuvaraj C D <yuvaraj.cd@samsung.com>
>>>> + *
>>>> + * This program is free software; you can redistribute  it and/or modify it
>>>> + * under  the terms of  the GNU General  Public License as published by the
>>>> + * Free Software Foundation;  either version 2 of the  License, or (at your
>>>> + * option) any later version.
>>>> + *
>>>> + */
>>>> +
>>>> +#include <linux/kernel.h>
>>>> +#include <linux/i2c.h>
>>>> +#include <linux/module.h>
>>>> +#include "sata_phy_exynos5250.h"
>>>> +
>>>> +static int exynos_sata_i2c_probe(struct i2c_client *client,
>>>> +             const struct i2c_device_id *i2c_id)
>>>> +{
>>>> +     sataphy_attach_i2c_client(client);
>>>> +
>>>> +     dev_info(&client->adapter->dev,
>>>> +             "attached %s into i2c adapter successfully\n",
>>>> +             client->name);
>>>> +
>>>> +     return 0;
>>>> +}
>>>> +
>>>> +static int exynos_sata_i2c_remove(struct i2c_client *client)
>>>> +{
>>>> +     dev_info(&client->adapter->dev,
>>>> +             "detached %s from i2c adapter successfully\n",
>>>> +             client->name);
>>>> +
>>>> +     return 0;
>>>> +}
>>>> +
>>>> +static const struct i2c_device_id phy_i2c_device_match[] = {
>>>> +     { "sata-phy-i2c", 0 },
>>>> +};
>>>> +MODULE_DEVICE_TABLE(of, phy_i2c_device_match);
>>>> +
>>>> +struct i2c_driver sataphy_i2c_driver = {
>>>> +     .probe    = exynos_sata_i2c_probe,
>>>> +     .id_table = phy_i2c_device_match,
>>>> +     .remove         = exynos_sata_i2c_remove,
>>>> +     .driver   = {
>>>> +             .name = "sata-phy-i2c",
>>>> +             .owner = THIS_MODULE,
>>>> +             .of_match_table = (void *)phy_i2c_device_match,
>>>> +             },
>>>> +};
>>>
>>> As I just mentioned above, we can merge this driver with the below one.
>> True, Initially it was merged.But already existing drivers of which
>> are of similar to this kind were done in this way.
>> Please refer  /drivers/gpu/drm//exynos/exynos_hdmiphy.c
>
> Can you point to any discussions where it was decided to go with this approach?
Sorry,what I meant is exynos_hdmiphy.c is already exist in the
mainline with this approach.

Initially it was merged on my local patches(Not upstreamed).But after
referring to the
exynos_hdmiphy.c driver and with lot of internal discussions ,it was
decided to go with
the already existing approach.

>
> Thanks
> Kishon

[PATCH 2/3] Phy: Exynos: Add Exynos5250 sata phy driver

[Kishon Vijay Abraham I]

On Friday 15 November 2013 11:17 AM, Yuvaraj Kumar wrote:
> On Thu, Nov 14, 2013 at 11:18 AM, Kishon Vijay Abraham I <kishon@ti.com> wrote:
>> Hi,
>>
>> On Monday 07 October 2013 07:35 PM, Yuvaraj Cd wrote:
>>> On Tue, Oct 1, 2013 at 6:21 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote:
>>>> On Tuesday 01 October 2013 12:03 PM, Yuvaraj Kumar C D wrote:
>>>>> This patch adds the sata phy driver for Exynos5250.Exynos5250 sata
>>>>> phy comprises of CMU and TRSV blocks which are of I2C register Map.
>>>>> So this patch also adds a i2c client driver, which is used configure
>>>>> the CMU and TRSV block of exynos5250 SATA PHY.
>>>>
>>>> Why not make the Exynos5250 sata phy as a i2c client driver instead?
>>>>>
>>>>> This patch incorporates the generic phy framework to deal with sata
>>>>> phy.
>>>>>
>>>>> This patch depends on the below patch
>>>>>       [1].drivers: phy: add generic PHY framework
>>>>>               by Kishon Vijay Abraham I<kishon@ti.com>
>>>>>
>>>>> Signed-off-by: Yuvaraj Kumar C D <yuvaraj.cd@samsung.com>
>>>>> Signed-off-by: Girish K S <ks.giri@samsung.com>
>>>>> Signed-off-by: Vasanth Ananthan <vasanth.a@samsung.com>
>>>>> ---
>>>>>  drivers/phy/Kconfig                      |    6 +
>>>>>  drivers/phy/Makefile                     |    1 +
>>>>>  drivers/phy/exynos/Kconfig               |    5 +
>>>>>  drivers/phy/exynos/Makefile              |    5 +
>>>>>  drivers/phy/exynos/exynos5250_phy_i2c.c  |   53 +++++++
>>>>>  drivers/phy/exynos/sata_phy_exynos5250.c |  248 ++++++++++++++++++++++++++++++
>>>>>  drivers/phy/exynos/sata_phy_exynos5250.h |   33 ++++
>>>>>  7 files changed, 351 insertions(+)
>>>>>  create mode 100644 drivers/phy/exynos/Kconfig
>>>>>  create mode 100644 drivers/phy/exynos/Makefile
>>>>>  create mode 100644 drivers/phy/exynos/exynos5250_phy_i2c.c
>>>>>  create mode 100644 drivers/phy/exynos/sata_phy_exynos5250.c
>>>>>  create mode 100644 drivers/phy/exynos/sata_phy_exynos5250.h
>>>>>
>>>>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>>>>> index 5f85909..ab3d1c6 100644
>>>>> --- a/drivers/phy/Kconfig
>>>>> +++ b/drivers/phy/Kconfig
>>>>> @@ -11,3 +11,9 @@ menuconfig GENERIC_PHY
>>>>>         devices present in the kernel. This layer will have the generic
>>>>>         API by which phy drivers can create PHY using the phy framework and
>>>>>         phy users can obtain reference to the PHY.
>>>>> +
>>>>> +if GENERIC_PHY
>>>>
>>>> NAK. Just select GENERIC_PHY from your driver Kconfig.
>>>>> +
>>>>> +source "drivers/phy/exynos/Kconfig"
>>>>> +
>>>>> +endif
>>>>> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
>>>>> index 9e9560f..e0223d7 100644
>>>>> --- a/drivers/phy/Makefile
>>>>> +++ b/drivers/phy/Makefile
>>>>> @@ -3,3 +3,4 @@
>>>>>  #
>>>>>
>>>>>  obj-$(CONFIG_GENERIC_PHY)    += phy-core.o
>>>>> +obj-$(CONFIG_PHY_SAMSUNG_SATA)       += exynos/
>>>>
>>>> simply have phy-exynos5250 in drivers/phy.
>>> ok.
>>>>> diff --git a/drivers/phy/exynos/Kconfig b/drivers/phy/exynos/Kconfig
>>>>> new file mode 100644
>>>>> index 0000000..fa125fb
>>>>> --- /dev/null
>>>>> +++ b/drivers/phy/exynos/Kconfig
>>>>> @@ -0,0 +1,5 @@
>>>>> +config PHY_SAMSUNG_SATA
>>>>> +     tristate "Samsung Sata SerDes/PHY driver"
>>>>> +     help
>>>>> +       Support for Samsung sata SerDes/Phy found on Samsung
>>>>> +       SoCs.
>>>>> diff --git a/drivers/phy/exynos/Makefile b/drivers/phy/exynos/Makefile
>>>>> new file mode 100644
>>>>> index 0000000..50dc7eb
>>>>> --- /dev/null
>>>>> +++ b/drivers/phy/exynos/Makefile
>>>>> @@ -0,0 +1,5 @@
>>>>> +#
>>>>> +# Makefile for the exynos phy drivers.
>>>>> +#
>>>>> +ccflags-y := -Idrivers/phy/exynos
>>>>> +obj-$(CONFIG_PHY_SAMSUNG_SATA)       += sata_phy_exynos5250.o exynos5250_phy_i2c.o
>>>>> diff --git a/drivers/phy/exynos/exynos5250_phy_i2c.c b/drivers/phy/exynos/exynos5250_phy_i2c.c
>>>>> new file mode 100644
>>>>> index 0000000..9c75d3b
>>>>> --- /dev/null
>>>>> +++ b/drivers/phy/exynos/exynos5250_phy_i2c.c
>>>>> @@ -0,0 +1,53 @@
>>>>> +/*
>>>>> + * Copyright (C) 2013 Samsung Electronics Co.Ltd
>>>>> + * Author:
>>>>> + *   Yuvaraj C D <yuvaraj.cd@samsung.com>
>>>>> + *
>>>>> + * This program is free software; you can redistribute  it and/or modify it
>>>>> + * under  the terms of  the GNU General  Public License as published by the
>>>>> + * Free Software Foundation;  either version 2 of the  License, or (at your
>>>>> + * option) any later version.
>>>>> + *
>>>>> + */
>>>>> +
>>>>> +#include <linux/kernel.h>
>>>>> +#include <linux/i2c.h>
>>>>> +#include <linux/module.h>
>>>>> +#include "sata_phy_exynos5250.h"
>>>>> +
>>>>> +static int exynos_sata_i2c_probe(struct i2c_client *client,
>>>>> +             const struct i2c_device_id *i2c_id)
>>>>> +{
>>>>> +     sataphy_attach_i2c_client(client);
>>>>> +
>>>>> +     dev_info(&client->adapter->dev,
>>>>> +             "attached %s into i2c adapter successfully\n",
>>>>> +             client->name);
>>>>> +
>>>>> +     return 0;
>>>>> +}
>>>>> +
>>>>> +static int exynos_sata_i2c_remove(struct i2c_client *client)
>>>>> +{
>>>>> +     dev_info(&client->adapter->dev,
>>>>> +             "detached %s from i2c adapter successfully\n",
>>>>> +             client->name);
>>>>> +
>>>>> +     return 0;
>>>>> +}
>>>>> +
>>>>> +static const struct i2c_device_id phy_i2c_device_match[] = {
>>>>> +     { "sata-phy-i2c", 0 },
>>>>> +};
>>>>> +MODULE_DEVICE_TABLE(of, phy_i2c_device_match);
>>>>> +
>>>>> +struct i2c_driver sataphy_i2c_driver = {
>>>>> +     .probe    = exynos_sata_i2c_probe,
>>>>> +     .id_table = phy_i2c_device_match,
>>>>> +     .remove         = exynos_sata_i2c_remove,
>>>>> +     .driver   = {
>>>>> +             .name = "sata-phy-i2c",
>>>>> +             .owner = THIS_MODULE,
>>>>> +             .of_match_table = (void *)phy_i2c_device_match,
>>>>> +             },
>>>>> +};
>>>>
>>>> As I just mentioned above, we can merge this driver with the below one.
>>> True, Initially it was merged.But already existing drivers of which
>>> are of similar to this kind were done in this way.
>>> Please refer  /drivers/gpu/drm//exynos/exynos_hdmiphy.c
>>
>> Can you point to any discussions where it was decided to go with this approach?
> Sorry,what I meant is exynos_hdmiphy.c is already exist in the
> mainline with this approach.
> 
> Initially it was merged on my local patches(Not upstreamed).But after
> referring to the
> exynos_hdmiphy.c driver and with lot of internal discussions ,it was
> decided to go with

I don't have any idea about the internal discussions whatever you had. IMO
Exynos5250 sata phy driver should be modelled as an i2c client driver if you
don't have a good reason to do otherwise.

Thanks
Kishon

> the already existing approach.
> 
>>
>> Thanks
>> Kishon

[PATCH 2/3] Phy: Exynos: Add Exynos5250 sata phy driver

[Yuvaraj Kumar]

On Tue, Nov 19, 2013 at 3:22 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote:
> On Friday 15 November 2013 11:17 AM, Yuvaraj Kumar wrote:
>> On Thu, Nov 14, 2013 at 11:18 AM, Kishon Vijay Abraham I <kishon@ti.com> wrote:
>>> Hi,
>>>
>>> On Monday 07 October 2013 07:35 PM, Yuvaraj Cd wrote:
>>>> On Tue, Oct 1, 2013 at 6:21 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote:
>>>>> On Tuesday 01 October 2013 12:03 PM, Yuvaraj Kumar C D wrote:
>>>>>> This patch adds the sata phy driver for Exynos5250.Exynos5250 sata
>>>>>> phy comprises of CMU and TRSV blocks which are of I2C register Map.
>>>>>> So this patch also adds a i2c client driver, which is used configure
>>>>>> the CMU and TRSV block of exynos5250 SATA PHY.
>>>>>
>>>>> Why not make the Exynos5250 sata phy as a i2c client driver instead?
>>>>>>
>>>>>> This patch incorporates the generic phy framework to deal with sata
>>>>>> phy.
>>>>>>
>>>>>> This patch depends on the below patch
>>>>>>       [1].drivers: phy: add generic PHY framework
>>>>>>               by Kishon Vijay Abraham I<kishon@ti.com>
>>>>>>
>>>>>> Signed-off-by: Yuvaraj Kumar C D <yuvaraj.cd@samsung.com>
>>>>>> Signed-off-by: Girish K S <ks.giri@samsung.com>
>>>>>> Signed-off-by: Vasanth Ananthan <vasanth.a@samsung.com>
>>>>>> ---
>>>>>>  drivers/phy/Kconfig                      |    6 +
>>>>>>  drivers/phy/Makefile                     |    1 +
>>>>>>  drivers/phy/exynos/Kconfig               |    5 +
>>>>>>  drivers/phy/exynos/Makefile              |    5 +
>>>>>>  drivers/phy/exynos/exynos5250_phy_i2c.c  |   53 +++++++
>>>>>>  drivers/phy/exynos/sata_phy_exynos5250.c |  248 ++++++++++++++++++++++++++++++
>>>>>>  drivers/phy/exynos/sata_phy_exynos5250.h |   33 ++++
>>>>>>  7 files changed, 351 insertions(+)
>>>>>>  create mode 100644 drivers/phy/exynos/Kconfig
>>>>>>  create mode 100644 drivers/phy/exynos/Makefile
>>>>>>  create mode 100644 drivers/phy/exynos/exynos5250_phy_i2c.c
>>>>>>  create mode 100644 drivers/phy/exynos/sata_phy_exynos5250.c
>>>>>>  create mode 100644 drivers/phy/exynos/sata_phy_exynos5250.h
>>>>>>
>>>>>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>>>>>> index 5f85909..ab3d1c6 100644
>>>>>> --- a/drivers/phy/Kconfig
>>>>>> +++ b/drivers/phy/Kconfig
>>>>>> @@ -11,3 +11,9 @@ menuconfig GENERIC_PHY
>>>>>>         devices present in the kernel. This layer will have the generic
>>>>>>         API by which phy drivers can create PHY using the phy framework and
>>>>>>         phy users can obtain reference to the PHY.
>>>>>> +
>>>>>> +if GENERIC_PHY
>>>>>
>>>>> NAK. Just select GENERIC_PHY from your driver Kconfig.
>>>>>> +
>>>>>> +source "drivers/phy/exynos/Kconfig"
>>>>>> +
>>>>>> +endif
>>>>>> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
>>>>>> index 9e9560f..e0223d7 100644
>>>>>> --- a/drivers/phy/Makefile
>>>>>> +++ b/drivers/phy/Makefile
>>>>>> @@ -3,3 +3,4 @@
>>>>>>  #
>>>>>>
>>>>>>  obj-$(CONFIG_GENERIC_PHY)    += phy-core.o
>>>>>> +obj-$(CONFIG_PHY_SAMSUNG_SATA)       += exynos/
>>>>>
>>>>> simply have phy-exynos5250 in drivers/phy.
>>>> ok.
>>>>>> diff --git a/drivers/phy/exynos/Kconfig b/drivers/phy/exynos/Kconfig
>>>>>> new file mode 100644
>>>>>> index 0000000..fa125fb
>>>>>> --- /dev/null
>>>>>> +++ b/drivers/phy/exynos/Kconfig
>>>>>> @@ -0,0 +1,5 @@
>>>>>> +config PHY_SAMSUNG_SATA
>>>>>> +     tristate "Samsung Sata SerDes/PHY driver"
>>>>>> +     help
>>>>>> +       Support for Samsung sata SerDes/Phy found on Samsung
>>>>>> +       SoCs.
>>>>>> diff --git a/drivers/phy/exynos/Makefile b/drivers/phy/exynos/Makefile
>>>>>> new file mode 100644
>>>>>> index 0000000..50dc7eb
>>>>>> --- /dev/null
>>>>>> +++ b/drivers/phy/exynos/Makefile
>>>>>> @@ -0,0 +1,5 @@
>>>>>> +#
>>>>>> +# Makefile for the exynos phy drivers.
>>>>>> +#
>>>>>> +ccflags-y := -Idrivers/phy/exynos
>>>>>> +obj-$(CONFIG_PHY_SAMSUNG_SATA)       += sata_phy_exynos5250.o exynos5250_phy_i2c.o
>>>>>> diff --git a/drivers/phy/exynos/exynos5250_phy_i2c.c b/drivers/phy/exynos/exynos5250_phy_i2c.c
>>>>>> new file mode 100644
>>>>>> index 0000000..9c75d3b
>>>>>> --- /dev/null
>>>>>> +++ b/drivers/phy/exynos/exynos5250_phy_i2c.c
>>>>>> @@ -0,0 +1,53 @@
>>>>>> +/*
>>>>>> + * Copyright (C) 2013 Samsung Electronics Co.Ltd
>>>>>> + * Author:
>>>>>> + *   Yuvaraj C D <yuvaraj.cd@samsung.com>
>>>>>> + *
>>>>>> + * This program is free software; you can redistribute  it and/or modify it
>>>>>> + * under  the terms of  the GNU General  Public License as published by the
>>>>>> + * Free Software Foundation;  either version 2 of the  License, or (at your
>>>>>> + * option) any later version.
>>>>>> + *
>>>>>> + */
>>>>>> +
>>>>>> +#include <linux/kernel.h>
>>>>>> +#include <linux/i2c.h>
>>>>>> +#include <linux/module.h>
>>>>>> +#include "sata_phy_exynos5250.h"
>>>>>> +
>>>>>> +static int exynos_sata_i2c_probe(struct i2c_client *client,
>>>>>> +             const struct i2c_device_id *i2c_id)
>>>>>> +{
>>>>>> +     sataphy_attach_i2c_client(client);
>>>>>> +
>>>>>> +     dev_info(&client->adapter->dev,
>>>>>> +             "attached %s into i2c adapter successfully\n",
>>>>>> +             client->name);
>>>>>> +
>>>>>> +     return 0;
>>>>>> +}
>>>>>> +
>>>>>> +static int exynos_sata_i2c_remove(struct i2c_client *client)
>>>>>> +{
>>>>>> +     dev_info(&client->adapter->dev,
>>>>>> +             "detached %s from i2c adapter successfully\n",
>>>>>> +             client->name);
>>>>>> +
>>>>>> +     return 0;
>>>>>> +}
>>>>>> +
>>>>>> +static const struct i2c_device_id phy_i2c_device_match[] = {
>>>>>> +     { "sata-phy-i2c", 0 },
>>>>>> +};
>>>>>> +MODULE_DEVICE_TABLE(of, phy_i2c_device_match);
>>>>>> +
>>>>>> +struct i2c_driver sataphy_i2c_driver = {
>>>>>> +     .probe    = exynos_sata_i2c_probe,
>>>>>> +     .id_table = phy_i2c_device_match,
>>>>>> +     .remove         = exynos_sata_i2c_remove,
>>>>>> +     .driver   = {
>>>>>> +             .name = "sata-phy-i2c",
>>>>>> +             .owner = THIS_MODULE,
>>>>>> +             .of_match_table = (void *)phy_i2c_device_match,
>>>>>> +             },
>>>>>> +};
>>>>>
>>>>> As I just mentioned above, we can merge this driver with the below one.
>>>> True, Initially it was merged.But already existing drivers of which
>>>> are of similar to this kind were done in this way.
>>>> Please refer  /drivers/gpu/drm//exynos/exynos_hdmiphy.c
>>>
>>> Can you point to any discussions where it was decided to go with this approach?
>> Sorry,what I meant is exynos_hdmiphy.c is already exist in the
>> mainline with this approach.
>>
>> Initially it was merged on my local patches(Not upstreamed).But after
>> referring to the
>> exynos_hdmiphy.c driver and with lot of internal discussions ,it was
>> decided to go with
>
> I don't have any idea about the internal discussions whatever you had. IMO
> Exynos5250 sata phy driver should be modelled as an i2c client driver if you
> don't have a good reason to do otherwise.
In Exynos5250,some of the registers of SATA PHY controller are I/O
mapped and some are accessible
only through I2C controller.
As the whole registers of SATA PHY controller cannot be accessible
through I2C bus, it cannot be made as an i2c client driver.
>
> Thanks
> Kishon
>
>> the already existing approach.
>>
>>>
>>> Thanks
>>> Kishon
>

[PATCH 2/3] Phy: Exynos: Add Exynos5250 sata phy driver

[Kishon Vijay Abraham I]

On Tuesday 19 November 2013 03:42 PM, Yuvaraj Kumar wrote:
> On Tue, Nov 19, 2013 at 3:22 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote:
>> On Friday 15 November 2013 11:17 AM, Yuvaraj Kumar wrote:
>>> On Thu, Nov 14, 2013 at 11:18 AM, Kishon Vijay Abraham I <kishon@ti.com> wrote:
>>>> Hi,
>>>>
>>>> On Monday 07 October 2013 07:35 PM, Yuvaraj Cd wrote:
>>>>> On Tue, Oct 1, 2013 at 6:21 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote:
>>>>>> On Tuesday 01 October 2013 12:03 PM, Yuvaraj Kumar C D wrote:
>>>>>>> This patch adds the sata phy driver for Exynos5250.Exynos5250 sata
>>>>>>> phy comprises of CMU and TRSV blocks which are of I2C register Map.
>>>>>>> So this patch also adds a i2c client driver, which is used configure
>>>>>>> the CMU and TRSV block of exynos5250 SATA PHY.
>>>>>>
>>>>>> Why not make the Exynos5250 sata phy as a i2c client driver instead?
>>>>>>>
>>>>>>> This patch incorporates the generic phy framework to deal with sata
>>>>>>> phy.
>>>>>>>
>>>>>>> This patch depends on the below patch
>>>>>>>       [1].drivers: phy: add generic PHY framework
>>>>>>>               by Kishon Vijay Abraham I<kishon@ti.com>
>>>>>>>
>>>>>>> Signed-off-by: Yuvaraj Kumar C D <yuvaraj.cd@samsung.com>
>>>>>>> Signed-off-by: Girish K S <ks.giri@samsung.com>
>>>>>>> Signed-off-by: Vasanth Ananthan <vasanth.a@samsung.com>
>>>>>>> ---
>>>>>>>  drivers/phy/Kconfig                      |    6 +
>>>>>>>  drivers/phy/Makefile                     |    1 +
>>>>>>>  drivers/phy/exynos/Kconfig               |    5 +
>>>>>>>  drivers/phy/exynos/Makefile              |    5 +
>>>>>>>  drivers/phy/exynos/exynos5250_phy_i2c.c  |   53 +++++++
>>>>>>>  drivers/phy/exynos/sata_phy_exynos5250.c |  248 ++++++++++++++++++++++++++++++
>>>>>>>  drivers/phy/exynos/sata_phy_exynos5250.h |   33 ++++
>>>>>>>  7 files changed, 351 insertions(+)
>>>>>>>  create mode 100644 drivers/phy/exynos/Kconfig
>>>>>>>  create mode 100644 drivers/phy/exynos/Makefile
>>>>>>>  create mode 100644 drivers/phy/exynos/exynos5250_phy_i2c.c
>>>>>>>  create mode 100644 drivers/phy/exynos/sata_phy_exynos5250.c
>>>>>>>  create mode 100644 drivers/phy/exynos/sata_phy_exynos5250.h
>>>>>>>
>>>>>>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>>>>>>> index 5f85909..ab3d1c6 100644
>>>>>>> --- a/drivers/phy/Kconfig
>>>>>>> +++ b/drivers/phy/Kconfig
>>>>>>> @@ -11,3 +11,9 @@ menuconfig GENERIC_PHY
>>>>>>>         devices present in the kernel. This layer will have the generic
>>>>>>>         API by which phy drivers can create PHY using the phy framework and
>>>>>>>         phy users can obtain reference to the PHY.
>>>>>>> +
>>>>>>> +if GENERIC_PHY
>>>>>>
>>>>>> NAK. Just select GENERIC_PHY from your driver Kconfig.
>>>>>>> +
>>>>>>> +source "drivers/phy/exynos/Kconfig"
>>>>>>> +
>>>>>>> +endif
>>>>>>> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
>>>>>>> index 9e9560f..e0223d7 100644
>>>>>>> --- a/drivers/phy/Makefile
>>>>>>> +++ b/drivers/phy/Makefile
>>>>>>> @@ -3,3 +3,4 @@
>>>>>>>  #
>>>>>>>
>>>>>>>  obj-$(CONFIG_GENERIC_PHY)    += phy-core.o
>>>>>>> +obj-$(CONFIG_PHY_SAMSUNG_SATA)       += exynos/
>>>>>>
>>>>>> simply have phy-exynos5250 in drivers/phy.
>>>>> ok.
>>>>>>> diff --git a/drivers/phy/exynos/Kconfig b/drivers/phy/exynos/Kconfig
>>>>>>> new file mode 100644
>>>>>>> index 0000000..fa125fb
>>>>>>> --- /dev/null
>>>>>>> +++ b/drivers/phy/exynos/Kconfig
>>>>>>> @@ -0,0 +1,5 @@
>>>>>>> +config PHY_SAMSUNG_SATA
>>>>>>> +     tristate "Samsung Sata SerDes/PHY driver"
>>>>>>> +     help
>>>>>>> +       Support for Samsung sata SerDes/Phy found on Samsung
>>>>>>> +       SoCs.
>>>>>>> diff --git a/drivers/phy/exynos/Makefile b/drivers/phy/exynos/Makefile
>>>>>>> new file mode 100644
>>>>>>> index 0000000..50dc7eb
>>>>>>> --- /dev/null
>>>>>>> +++ b/drivers/phy/exynos/Makefile
>>>>>>> @@ -0,0 +1,5 @@
>>>>>>> +#
>>>>>>> +# Makefile for the exynos phy drivers.
>>>>>>> +#
>>>>>>> +ccflags-y := -Idrivers/phy/exynos
>>>>>>> +obj-$(CONFIG_PHY_SAMSUNG_SATA)       += sata_phy_exynos5250.o exynos5250_phy_i2c.o
>>>>>>> diff --git a/drivers/phy/exynos/exynos5250_phy_i2c.c b/drivers/phy/exynos/exynos5250_phy_i2c.c
>>>>>>> new file mode 100644
>>>>>>> index 0000000..9c75d3b
>>>>>>> --- /dev/null
>>>>>>> +++ b/drivers/phy/exynos/exynos5250_phy_i2c.c
>>>>>>> @@ -0,0 +1,53 @@
>>>>>>> +/*
>>>>>>> + * Copyright (C) 2013 Samsung Electronics Co.Ltd
>>>>>>> + * Author:
>>>>>>> + *   Yuvaraj C D <yuvaraj.cd@samsung.com>
>>>>>>> + *
>>>>>>> + * This program is free software; you can redistribute  it and/or modify it
>>>>>>> + * under  the terms of  the GNU General  Public License as published by the
>>>>>>> + * Free Software Foundation;  either version 2 of the  License, or (at your
>>>>>>> + * option) any later version.
>>>>>>> + *
>>>>>>> + */
>>>>>>> +
>>>>>>> +#include <linux/kernel.h>
>>>>>>> +#include <linux/i2c.h>
>>>>>>> +#include <linux/module.h>
>>>>>>> +#include "sata_phy_exynos5250.h"
>>>>>>> +
>>>>>>> +static int exynos_sata_i2c_probe(struct i2c_client *client,
>>>>>>> +             const struct i2c_device_id *i2c_id)
>>>>>>> +{
>>>>>>> +     sataphy_attach_i2c_client(client);
>>>>>>> +
>>>>>>> +     dev_info(&client->adapter->dev,
>>>>>>> +             "attached %s into i2c adapter successfully\n",
>>>>>>> +             client->name);
>>>>>>> +
>>>>>>> +     return 0;
>>>>>>> +}
>>>>>>> +
>>>>>>> +static int exynos_sata_i2c_remove(struct i2c_client *client)
>>>>>>> +{
>>>>>>> +     dev_info(&client->adapter->dev,
>>>>>>> +             "detached %s from i2c adapter successfully\n",
>>>>>>> +             client->name);
>>>>>>> +
>>>>>>> +     return 0;
>>>>>>> +}
>>>>>>> +
>>>>>>> +static const struct i2c_device_id phy_i2c_device_match[] = {
>>>>>>> +     { "sata-phy-i2c", 0 },
>>>>>>> +};
>>>>>>> +MODULE_DEVICE_TABLE(of, phy_i2c_device_match);
>>>>>>> +
>>>>>>> +struct i2c_driver sataphy_i2c_driver = {
>>>>>>> +     .probe    = exynos_sata_i2c_probe,
>>>>>>> +     .id_table = phy_i2c_device_match,
>>>>>>> +     .remove         = exynos_sata_i2c_remove,
>>>>>>> +     .driver   = {
>>>>>>> +             .name = "sata-phy-i2c",
>>>>>>> +             .owner = THIS_MODULE,
>>>>>>> +             .of_match_table = (void *)phy_i2c_device_match,
>>>>>>> +             },
>>>>>>> +};
>>>>>>
>>>>>> As I just mentioned above, we can merge this driver with the below one.
>>>>> True, Initially it was merged.But already existing drivers of which
>>>>> are of similar to this kind were done in this way.
>>>>> Please refer  /drivers/gpu/drm//exynos/exynos_hdmiphy.c
>>>>
>>>> Can you point to any discussions where it was decided to go with this approach?
>>> Sorry,what I meant is exynos_hdmiphy.c is already exist in the
>>> mainline with this approach.
>>>
>>> Initially it was merged on my local patches(Not upstreamed).But after
>>> referring to the
>>> exynos_hdmiphy.c driver and with lot of internal discussions ,it was
>>> decided to go with
>>
>> I don't have any idea about the internal discussions whatever you had. IMO
>> Exynos5250 sata phy driver should be modelled as an i2c client driver if you
>> don't have a good reason to do otherwise.
> In Exynos5250,some of the registers of SATA PHY controller are I/O
> mapped and some are accessible
> only through I2C controller.
> As the whole registers of SATA PHY controller cannot be accessible
> through I2C bus, it cannot be made as an i2c client driver.

Ah ok.. Makes sense. Thanks for clarifying.

Cheers
Kishon

>>
>> Thanks
>> Kishon
>>
>>> the already existing approach.
>>>
>>>>
>>>> Thanks
>>>> Kishon
>>

[PATCH 3/3] ARM: dts: Enable ahci sata and sata phy

[Yuvaraj Kumar C D]

This patch adds dt entry for ahci sata controller and its
corresponding phy controller.phy node has been added w.r.t
new generic phy framework.

Signed-off-by: Yuvaraj Kumar C D <yuvaraj.cd@samsung.com>
---
 arch/arm/boot/dts/exynos5250-arndale.dts  |    9 ++++++++-
 arch/arm/boot/dts/exynos5250-smdk5250.dts |    6 +-----
 arch/arm/boot/dts/exynos5250.dtsi         |   21 +++++++++++++++++----
 3 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/arch/arm/boot/dts/exynos5250-arndale.dts b/arch/arm/boot/dts/exynos5250-arndale.dts
index cee55fa..50db341 100644
--- a/arch/arm/boot/dts/exynos5250-arndale.dts
+++ b/arch/arm/boot/dts/exynos5250-arndale.dts
@@ -363,7 +363,14 @@
 	};
 
 	i2c at 121D0000 {
-		status = "disabled";
+		samsung,i2c-sda-delay = <100>;
+		samsung,i2c-max-bus-freq = <40000>;
+		samsung,i2c-slave-addr = <0x38>;
+
+		sata-phy {
+			compatible = "sata-phy-i2c";
+			reg = <0x38>;
+		};
 	};
 
 	dwmmc_0: dwmmc0 at 12200000 {
diff --git a/arch/arm/boot/dts/exynos5250-smdk5250.dts b/arch/arm/boot/dts/exynos5250-smdk5250.dts
index 2538b32..21b4b25a 100644
--- a/arch/arm/boot/dts/exynos5250-smdk5250.dts
+++ b/arch/arm/boot/dts/exynos5250-smdk5250.dts
@@ -91,15 +91,11 @@
 		samsung,i2c-slave-addr = <0x38>;
 
 		sata-phy {
-			compatible = "samsung,sata-phy";
+			compatible = "sata-phy-i2c";
 			reg = <0x38>;
 		};
 	};
 
-	sata at 122F0000 {
-		samsung,sata-freq = <66>;
-	};
-
 	i2c at 12C80000 {
 		samsung,i2c-sda-delay = <100>;
 		samsung,i2c-max-bus-freq = <66000>;
diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
index 668ce5d..46a0e3b 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -46,6 +46,7 @@
 		i2c6 = &i2c_6;
 		i2c7 = &i2c_7;
 		i2c8 = &i2c_8;
+		i2c9 = &i2c_9;
 		pinctrl0 = &pinctrl_0;
 		pinctrl1 = &pinctrl_1;
 		pinctrl2 = &pinctrl_2;
@@ -216,16 +217,28 @@
 	};
 
 	sata at 122F0000 {
-		compatible = "samsung,exynos5-sata-ahci";
+		compatible = "samsung,exynos5250-sata-ahci";
+		samsung,sata-freq = <66>;
 		reg = <0x122F0000 0x1ff>;
 		interrupts = <0 115 0>;
 		clocks = <&clock 277>, <&clock 143>;
 		clock-names = "sata", "sclk_sata";
+		phys = <&sata_phy>;
+		phy-names = "sata-phy";
 	};
 
-	sata-phy at 12170000 {
-		compatible = "samsung,exynos5-sata-phy";
+	sata_phy:sata-phy at 12170000 {
+		compatible = "samsung,exynos5250-sata-phy";
 		reg = <0x12170000 0x1ff>;
+		clocks = <&clock 287>;
+		clock-names = "sata_phyctrl";
+		#phy-cells = <0>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+		sataphy-pmu {
+			reg = <0x10040724 0x4>;
+			};
 	};
 
 	i2c_0: i2c at 12C60000 {
@@ -334,7 +347,7 @@
 		clock-names = "i2c";
 	};
 
-	i2c at 121D0000 {
+	i2c_9: i2c at 121D0000 {
                 compatible = "samsung,exynos5-sata-phy-i2c";
                 reg = <0x121D0000 0x100>;
                 #address-cells = <1>;
-- 
1.7.9.5

[PATCH 3/3] ARM: dts: Enable ahci sata and sata phy

[Sachin Kamat]

Hi Yuvaraj,

On 1 October 2013 12:03, Yuvaraj Kumar C D <yuvaraj.cd@gmail.com> wrote:
> This patch adds dt entry for ahci sata controller and its
> corresponding phy controller.phy node has been added w.r.t
> new generic phy framework.

[snip]
> +
> +               sata-phy {

Shouldn't this be sata-phy at 38?

> +                       compatible = "sata-phy-i2c";
> +                       reg = <0x38>;
> +               };

[snip]
> -       sata-phy at 12170000 {
> -               compatible = "samsung,exynos5-sata-phy";
> +       sata_phy:sata-phy at 12170000 {

nit: A space after sata_phy: would be good.


-- 
With warm regards,
Sachin