[PATCH 0/5] ata: pata_arasan and ahci_platform updates

[PATCH 0/5] ata: pata_arasan and ahci_platform updates

[Viresh Kumar]

Hi Jeff,

This patchset contains updates for pata_arasan and ahci_platform drivers.
- First patch was earlier sent alone, just included here.
- Second one was sent earlier to you and following was the discussion there:
  http://patchwork.ozlabs.org/patch/99677/

  It is used by SPEAr13xx for which i would be upstreaming support shortly. Will
  cc you there.

- Others add DT support for them.

Viresh Kumar (5):
  ata/pata_arasan_cf: Move arasan_cf_pm_ops out of #ifdef, #endif
    macros
  ata: pata_arasan: Add clk_{un}prepare() support
  ata: pata_arasan: add Device Tree probing capability
  ata: ahci_platform: Add synopsys ahci controller in DT's compatible
    list
  ata/ahci_platform: Add clock framework support

 .../ata/{calxeda-sata.txt => ahci-platform.txt}    |    5 +-
 .../devicetree/bindings/ata/pata-arasan.txt        |   17 +++++
 drivers/ata/ahci.h                                 |    4 +
 drivers/ata/ahci_platform.c                        |   64 +++++++++++++++++--
 drivers/ata/pata_arasan_cf.c                       |   16 ++++-
 5 files changed, 92 insertions(+), 14 deletions(-)
 rename Documentation/devicetree/bindings/ata/{calxeda-sata.txt => ahci-platform.txt} (90%)
 create mode 100644 Documentation/devicetree/bindings/ata/pata-arasan.txt

-- 
1.7.9

[PATCH 1/5] ata/pata_arasan_cf: Move arasan_cf_pm_ops out of #ifdef, #endif macros

[Viresh Kumar]

#ifdef, #endif is not required in definition/usage of arasan_cf_pm_ops. So, move
this definition and its usage outside of them.

Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
---
 drivers/ata/pata_arasan_cf.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/drivers/ata/pata_arasan_cf.c b/drivers/ata/pata_arasan_cf.c
index fc2db2a..3239517 100644
--- a/drivers/ata/pata_arasan_cf.c
+++ b/drivers/ata/pata_arasan_cf.c
@@ -943,9 +943,9 @@ static int arasan_cf_resume(struct device *dev)
 
 	return 0;
 }
+#endif
 
 static SIMPLE_DEV_PM_OPS(arasan_cf_pm_ops, arasan_cf_suspend, arasan_cf_resume);
-#endif
 
 static struct platform_driver arasan_cf_driver = {
 	.probe		= arasan_cf_probe,
@@ -953,9 +953,7 @@ static struct platform_driver arasan_cf_driver = {
 	.driver		= {
 		.name	= DRIVER_NAME,
 		.owner	= THIS_MODULE,
-#ifdef CONFIG_PM
 		.pm	= &arasan_cf_pm_ops,
-#endif
 	},
 };
 
-- 
1.7.9

[PATCH 2/5] ata: pata_arasan: Add clk_{un}prepare() support

[Viresh Kumar]

clk_{un}prepare is mandatory for platforms using common clock framework. Since
this driver is used by SPEAr platform, which supports common clock framework,
add clk_{un}prepare() support for it.

Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
---
 drivers/ata/pata_arasan_cf.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/pata_arasan_cf.c b/drivers/ata/pata_arasan_cf.c
index 3239517..492db9c 100644
--- a/drivers/ata/pata_arasan_cf.c
+++ b/drivers/ata/pata_arasan_cf.c
@@ -313,7 +313,7 @@ static int cf_init(struct arasan_cf_dev *acdev)
 	int ret = 0;
 
 #ifdef CONFIG_HAVE_CLK
-	ret = clk_enable(acdev->clk);
+	ret = clk_prepare_enable(acdev->clk);
 	if (ret) {
 		dev_dbg(acdev->host->dev, "clock enable failed");
 		return ret;
@@ -345,7 +345,7 @@ static void cf_exit(struct arasan_cf_dev *acdev)
 			acdev->vbase + OP_MODE);
 	spin_unlock_irqrestore(&acdev->host->lock, flags);
 #ifdef CONFIG_HAVE_CLK
-	clk_disable(acdev->clk);
+	clk_disable_unprepare(acdev->clk);
 #endif
 }
 
-- 
1.7.9

[PATCH 3/5] ata: pata_arasan: add Device Tree probing capability

[Viresh Kumar]

SPEAr platforms now support DT and so must convert all drivers to support DT.
This patch adds DT probing support for Arasan Compact Flash controller and
updates its documentation too.

Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
---
 .../devicetree/bindings/ata/pata-arasan.txt        |   17 +++++++++++++++++
 drivers/ata/pata_arasan_cf.c                       |   10 ++++++++++
 2 files changed, 27 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/ata/pata-arasan.txt

diff --git a/Documentation/devicetree/bindings/ata/pata-arasan.txt b/Documentation/devicetree/bindings/ata/pata-arasan.txt
new file mode 100644
index 0000000..95ec7f8
--- /dev/null
+++ b/Documentation/devicetree/bindings/ata/pata-arasan.txt
@@ -0,0 +1,17 @@
+* ARASAN PATA COMPACT FLASH CONTROLLER
+
+Required properties:
+- compatible: "arasan,cf-spear1340"
+- reg: Address range of the CF registers
+- interrupt-parent: Should be the phandle for the interrupt controller
+  that services interrupts for this device
+- interrupt: Should contain the CF interrupt number
+
+Example:
+
+	cf@fc000000 {
+		compatible = "arasan,cf-spear1340";
+		reg = <0xfc000000 0x1000>;
+		interrupt-parent = <&vic1>;
+		interrupts = <12>;
+	};
diff --git a/drivers/ata/pata_arasan_cf.c b/drivers/ata/pata_arasan_cf.c
index 492db9c..f583124 100644
--- a/drivers/ata/pata_arasan_cf.c
+++ b/drivers/ata/pata_arasan_cf.c
@@ -31,6 +31,7 @@
 #include <linux/kernel.h>
 #include <linux/libata.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/pata_arasan_cf_data.h>
 #include <linux/platform_device.h>
 #include <linux/pm.h>
@@ -947,6 +948,14 @@ static int arasan_cf_resume(struct device *dev)
 
 static SIMPLE_DEV_PM_OPS(arasan_cf_pm_ops, arasan_cf_suspend, arasan_cf_resume);
 
+#ifdef CONFIG_OF
+static const struct of_device_id arasan_cf_id_table[] = {
+	{ .compatible = "arasan,cf-spear1340" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, arasan_cf_id_table);
+#endif
+
 static struct platform_driver arasan_cf_driver = {
 	.probe		= arasan_cf_probe,
 	.remove		= __devexit_p(arasan_cf_remove),
@@ -954,6 +963,7 @@ static struct platform_driver arasan_cf_driver = {
 		.name	= DRIVER_NAME,
 		.owner	= THIS_MODULE,
 		.pm	= &arasan_cf_pm_ops,
+		.of_match_table = of_match_ptr(arasan_cf_id_table),
 	},
 };
 
-- 
1.7.9

[PATCH 4/5] ata: ahci_platform: Add synopsys ahci controller in DT's compatible list

[Viresh Kumar]

SPEAr13xx series of SoCs contain Synopsys AHCI SATA Controller which shares
ahci_platform driver with other controller versions.

This patch updates DT compatible list for ahci_platform. It also updates and
renames binding documentation to more generic name.

Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
Cc: Rob Herring <rob.herring@calxeda.com>
---
 .../ata/{calxeda-sata.txt => ahci-platform.txt}    |    5 ++---
 drivers/ata/ahci_platform.c                        |    1 +
 2 files changed, 3 insertions(+), 3 deletions(-)
 rename Documentation/devicetree/bindings/ata/{calxeda-sata.txt => ahci-platform.txt} (90%)

diff --git a/Documentation/devicetree/bindings/ata/calxeda-sata.txt b/Documentation/devicetree/bindings/ata/ahci-platform.txt
similarity index 90%
rename from Documentation/devicetree/bindings/ata/calxeda-sata.txt
rename to Documentation/devicetree/bindings/ata/ahci-platform.txt
index 79caa56..8bb8a76 100644
--- a/Documentation/devicetree/bindings/ata/calxeda-sata.txt
+++ b/Documentation/devicetree/bindings/ata/ahci-platform.txt
@@ -1,10 +1,10 @@
-* Calxeda SATA Controller
+* AHCI SATA Controller
 
 SATA nodes are defined to describe on-chip Serial ATA controllers.
 Each SATA controller should have its own node.
 
 Required properties:
-- compatible        : compatible list, contains "calxeda,hb-ahci"
+- compatible        : compatible list, contains "calxeda,hb-ahci" or "snps,spear-ahci"
 - interrupts        : <interrupt mapping for SATA IRQ>
 - reg               : <registers mapping>
 
@@ -14,4 +14,3 @@ Example:
                 reg = <0xffe08000 0x1000>;
                 interrupts = <115>;
         };
-
diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
index 0c86c77..9e419e1 100644
--- a/drivers/ata/ahci_platform.c
+++ b/drivers/ata/ahci_platform.c
@@ -280,6 +280,7 @@ static struct dev_pm_ops ahci_pm_ops = {
 
 static const struct of_device_id ahci_of_match[] = {
 	{ .compatible = "calxeda,hb-ahci", },
+	{ .compatible = "snps,spear-ahci", },
 	{},
 };
 MODULE_DEVICE_TABLE(of, ahci_of_match);
-- 
1.7.9

[PATCH 5/5] ata/ahci_platform: Add clock framework support

[Viresh Kumar]

On many architectures, drivers are supposed to prepare/unprepare &
enable/disable functional clock of device. This patch adds clock support for
ahci_platform.

Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
---
 drivers/ata/ahci.h          |    4 +++
 drivers/ata/ahci_platform.c |   63 ++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 60 insertions(+), 7 deletions(-)

diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
index c2594dd..89172c8 100644
--- a/drivers/ata/ahci.h
+++ b/drivers/ata/ahci.h
@@ -35,6 +35,7 @@
 #ifndef _AHCI_H
 #define _AHCI_H
 
+#include <linux/clk.h>
 #include <linux/libata.h>
 
 /* Enclosure Management Control */
@@ -302,6 +303,9 @@ struct ahci_host_priv {
 	u32 			em_loc; /* enclosure management location */
 	u32			em_buf_sz;	/* EM buffer size in byte */
 	u32			em_msg_type;	/* EM message type */
+#ifdef CONFIG_HAVE_CLK
+	struct clk		*clk;	/* only if HAVE_CLK is defined */
+#endif
 };
 
 extern int ahci_ignore_sss;
diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
index 9e419e1..730e214 100644
--- a/drivers/ata/ahci_platform.c
+++ b/drivers/ata/ahci_platform.c
@@ -12,6 +12,7 @@
  * any later version.
  */
 
+#include <linux/clk.h>
 #include <linux/kernel.h>
 #include <linux/gfp.h>
 #include <linux/module.h>
@@ -117,6 +118,20 @@ static int __init ahci_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
+#ifdef CONFIG_HAVE_CLK
+	hpriv->clk = clk_get(dev, NULL);
+	if (IS_ERR(hpriv->clk)) {
+		dev_err(dev, "Clock not found\n");
+		return PTR_ERR(hpriv->clk);
+	}
+
+	rc = clk_prepare_enable(hpriv->clk);
+	if (rc) {
+		dev_err(dev, "clock prepare enable failed");
+		goto free_clk;
+	}
+#endif
+
 	/*
 	 * Some platforms might need to prepare for mmio region access,
 	 * which could be done in the following init call. So, the mmio
@@ -126,7 +141,7 @@ static int __init ahci_probe(struct platform_device *pdev)
 	if (pdata && pdata->init) {
 		rc = pdata->init(dev, hpriv->mmio);
 		if (rc)
-			return rc;
+			goto disable_unprepare_clk;
 	}
 
 	ahci_save_initial_config(dev, hpriv,
@@ -152,7 +167,7 @@ static int __init ahci_probe(struct platform_device *pdev)
 	host = ata_host_alloc_pinfo(dev, ppi, n_ports);
 	if (!host) {
 		rc = -ENOMEM;
-		goto err0;
+		goto pdata_exit;
 	}
 
 	host->private_data = hpriv;
@@ -182,7 +197,7 @@ static int __init ahci_probe(struct platform_device *pdev)
 
 	rc = ahci_reset_controller(host);
 	if (rc)
-		goto err0;
+		goto pdata_exit;
 
 	ahci_init_controller(host);
 	ahci_print_info(host, "platform");
@@ -190,12 +205,18 @@ static int __init ahci_probe(struct platform_device *pdev)
 	rc = ata_host_activate(host, irq, ahci_interrupt, IRQF_SHARED,
 			       &ahci_platform_sht);
 	if (rc)
-		goto err0;
+		goto pdata_exit;
 
 	return 0;
-err0:
+pdata_exit:
 	if (pdata && pdata->exit)
 		pdata->exit(dev);
+disable_unprepare_clk:
+#ifdef CONFIG_HAVE_CLK
+	clk_disable_unprepare(hpriv->clk);
+free_clk:
+	clk_put(hpriv->clk);
+#endif
 	return rc;
 }
 
@@ -204,12 +225,19 @@ static int __devexit ahci_remove(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct ahci_platform_data *pdata = dev_get_platdata(dev);
 	struct ata_host *host = dev_get_drvdata(dev);
+#ifdef CONFIG_HAVE_CLK
+	struct ahci_host_priv *hpriv = host->private_data;
+#endif
 
 	ata_host_detach(host);
 
 	if (pdata && pdata->exit)
 		pdata->exit(dev);
 
+#ifdef CONFIG_HAVE_CLK
+	clk_disable_unprepare(hpriv->clk);
+	clk_put(hpriv->clk);
+#endif
 	return 0;
 }
 
@@ -244,6 +272,11 @@ static int ahci_suspend(struct device *dev)
 
 	if (pdata && pdata->suspend)
 		return pdata->suspend(dev);
+
+#ifdef CONFIG_HAVE_CLK
+	clk_disable_unprepare(hpriv->clk);
+#endif
+
 	return 0;
 }
 
@@ -253,16 +286,26 @@ static int ahci_resume(struct device *dev)
 	struct ata_host *host = dev_get_drvdata(dev);
 	int rc;
 
+#ifdef CONFIG_HAVE_CLK
+	struct ahci_host_priv *hpriv = host->private_data;
+
+	rc = clk_prepare_enable(hpriv->clk);
+	if (rc) {
+		dev_err(dev, "clock prepare enable failed");
+		return rc;
+	}
+#endif
+
 	if (pdata && pdata->resume) {
 		rc = pdata->resume(dev);
 		if (rc)
-			return rc;
+			goto disable_unprepare_clk;
 	}
 
 	if (dev->power.power_state.event == PM_EVENT_SUSPEND) {
 		rc = ahci_reset_controller(host);
 		if (rc)
-			return rc;
+			goto disable_unprepare_clk;
 
 		ahci_init_controller(host);
 	}
@@ -270,6 +313,12 @@ static int ahci_resume(struct device *dev)
 	ata_host_resume(host);
 
 	return 0;
+
+disable_unprepare_clk:
+#ifdef CONFIG_HAVE_CLK
+	clk_disable_unprepare(hpriv->clk);
+#endif
+	return rc;
 }
 
 static struct dev_pm_ops ahci_pm_ops = {
-- 
1.7.9

Re: [PATCH 5/5] ata/ahci_platform: Add clock framework support

[Sergei Shtylyov]

Hello.

On 04/20/2012 06:38 PM, Viresh Kumar wrote:

> On many architectures, drivers are supposed to prepare/unprepare&
> enable/disable functional clock of device. This patch adds clock support for
> ahci_platform.

> Signed-off-by: Viresh Kumar<viresh.kumar@st.com>
[...]

> diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
> index 9e419e1..730e214 100644
> --- a/drivers/ata/ahci_platform.c
> +++ b/drivers/ata/ahci_platform.c
> @@ -12,6 +12,7 @@
>    * any later version.
>    */
>
> +#include<linux/clk.h>
>   #include<linux/kernel.h>
>   #include<linux/gfp.h>
>   #include<linux/module.h>
> @@ -117,6 +118,20 @@ static int __init ahci_probe(struct platform_device *pdev)
>   		return -ENOMEM;
>   	}
>
> +#ifdef CONFIG_HAVE_CLK
> +	hpriv->clk = clk_get(dev, NULL);
> +	if (IS_ERR(hpriv->clk)) {
> +		dev_err(dev, "Clock not found\n");
> +		return PTR_ERR(hpriv->clk);
> +	}
> +
> +	rc = clk_prepare_enable(hpriv->clk);
> +	if (rc) {
> +		dev_err(dev, "clock prepare enable failed");
> +		goto free_clk;
> +	}
> +#endif
> +
>   	/*
>   	 * Some platforms might need to prepare for mmio region access,
>   	 * which could be done in the following init call. So, the mmio
[...]
> @@ -190,12 +205,18 @@ static int __init ahci_probe(struct platform_device *pdev)
>   	rc = ata_host_activate(host, irq, ahci_interrupt, IRQF_SHARED,
>   			&ahci_platform_sht);
>   	if (rc)
> -		goto err0;
> +		goto pdata_exit;
>
>   	return 0;
> -err0:
> +pdata_exit:
>   	if (pdata&&  pdata->exit)
>   		pdata->exit(dev);
> +disable_unprepare_clk:
> +#ifdef CONFIG_HAVE_CLK
> +	clk_disable_unprepare(hpriv->clk);
> +free_clk:
> +	clk_put(hpriv->clk);
> +#endif
>   	return rc;
>   }
>
> @@ -204,12 +225,19 @@ static int __devexit ahci_remove(struct platform_device *pdev)
>   	struct device *dev =&pdev->dev;
>   	struct ahci_platform_data *pdata = dev_get_platdata(dev);
>   	struct ata_host *host = dev_get_drvdata(dev);
> +#ifdef CONFIG_HAVE_CLK
> +	struct ahci_host_priv *hpriv = host->private_data;
> +#endif
>
>   	ata_host_detach(host);
>
>   	if (pdata&&  pdata->exit)
>   		pdata->exit(dev);
>
> +#ifdef CONFIG_HAVE_CLK
> +	clk_disable_unprepare(hpriv->clk);
> +	clk_put(hpriv->clk);
> +#endif
>   	return 0;
>   }
>
> @@ -244,6 +272,11 @@ static int ahci_suspend(struct device *dev)
>
>   	if (pdata&&  pdata->suspend)
>   		return pdata->suspend(dev);
> +
> +#ifdef CONFIG_HAVE_CLK
> +	clk_disable_unprepare(hpriv->clk);
> +#endif
> +
>   	return 0;
>   }
>
> @@ -253,16 +286,26 @@ static int ahci_resume(struct device *dev)
>   	struct ata_host *host = dev_get_drvdata(dev);
>   	int rc;
>
> +#ifdef CONFIG_HAVE_CLK
> +	struct ahci_host_priv *hpriv = host->private_data;
> +
> +	rc = clk_prepare_enable(hpriv->clk);
> +	if (rc) {
> +		dev_err(dev, "clock prepare enable failed");
> +		return rc;
> +	}
> +#endif
> +
[...]
> @@ -270,6 +313,12 @@ static int ahci_resume(struct device *dev)
>   	ata_host_resume(host);
>
>   	return 0;
> +
> +disable_unprepare_clk:
> +#ifdef CONFIG_HAVE_CLK
> +	clk_disable_unprepare(hpriv->clk);
> +#endif

    Can't we have empty inlines for !defined(CONFIG_HAVE_CLK) case in 
<linux/clk.h>. Otherwise there's too much uglye #ifdef'ery.

MBR, Sergei

Re: [PATCH 5/5] ata/ahci_platform: Add clock framework support

[viresh kumar]

On 4/20/12, Sergei Shtylyov <sshtylyov@mvista.com> wrote:
> On 04/20/2012 06:38 PM, Viresh Kumar wrote:
>> diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
>> @@ -117,6 +118,20 @@ static int __init ahci_probe(struct platform_device
>> *pdev)
>>   		return -ENOMEM;
>>   	}
>>
>> +#ifdef CONFIG_HAVE_CLK
>> +	hpriv->clk = clk_get(dev, NULL);
>> +	if (IS_ERR(hpriv->clk)) {
>> +		dev_err(dev, "Clock not found\n");
>> +		return PTR_ERR(hpriv->clk);
>> +	}
>> +
>> +	rc = clk_prepare_enable(hpriv->clk);
>> +	if (rc) {
>> +		dev_err(dev, "clock prepare enable failed");
>> +		goto free_clk;
>> +	}
>> +#endif
>> +
>>   	/*
>>   	 * Some platforms might need to prepare for mmio region access,
>>   	 * which could be done in the following init call. So, the mmio
> [...]
>> @@ -190,12 +205,18 @@ static int __init ahci_probe(struct platform_device
>> *pdev)
>>   	rc = ata_host_activate(host, irq, ahci_interrupt, IRQF_SHARED,
>>   			&ahci_platform_sht);
>>   	if (rc)
>> -		goto err0;
>> +		goto pdata_exit;
>>
>>   	return 0;
>> -err0:
>> +pdata_exit:
>>   	if (pdata&&  pdata->exit)
>>   		pdata->exit(dev);
>> +disable_unprepare_clk:
>> +#ifdef CONFIG_HAVE_CLK
>> +	clk_disable_unprepare(hpriv->clk);
>> +free_clk:
>> +	clk_put(hpriv->clk);
>> +#endif
>>   	return rc;
>>   }
>>
>> @@ -204,12 +225,19 @@ static int __devexit ahci_remove(struct
>> platform_device *pdev)
>>   	struct device *dev =&pdev->dev;
>>   	struct ahci_platform_data *pdata = dev_get_platdata(dev);
>>   	struct ata_host *host = dev_get_drvdata(dev);
>> +#ifdef CONFIG_HAVE_CLK
>> +	struct ahci_host_priv *hpriv = host->private_data;
>> +#endif
>>
>>   	ata_host_detach(host);
>>
>>   	if (pdata&&  pdata->exit)
>>   		pdata->exit(dev);
>>
>> +#ifdef CONFIG_HAVE_CLK
>> +	clk_disable_unprepare(hpriv->clk);
>> +	clk_put(hpriv->clk);
>> +#endif
>>   	return 0;
>>   }
>>
>> @@ -244,6 +272,11 @@ static int ahci_suspend(struct device *dev)
>>
>>   	if (pdata&&  pdata->suspend)
>>   		return pdata->suspend(dev);
>> +
>> +#ifdef CONFIG_HAVE_CLK
>> +	clk_disable_unprepare(hpriv->clk);
>> +#endif
>> +
>>   	return 0;
>>   }
>>
>> @@ -253,16 +286,26 @@ static int ahci_resume(struct device *dev)
>>   	struct ata_host *host = dev_get_drvdata(dev);
>>   	int rc;
>>
>> +#ifdef CONFIG_HAVE_CLK
>> +	struct ahci_host_priv *hpriv = host->private_data;
>> +
>> +	rc = clk_prepare_enable(hpriv->clk);
>> +	if (rc) {
>> +		dev_err(dev, "clock prepare enable failed");
>> +		return rc;
>> +	}
>> +#endif
>> +
> [...]
>> @@ -270,6 +313,12 @@ static int ahci_resume(struct device *dev)
>>   	ata_host_resume(host);
>>
>>   	return 0;
>> +
>> +disable_unprepare_clk:
>> +#ifdef CONFIG_HAVE_CLK
>> +	clk_disable_unprepare(hpriv->clk);
>> +#endif
>
>     Can't we have empty inlines for !defined(CONFIG_HAVE_CLK) case in
> <linux/clk.h>. Otherwise there's too much uglye #ifdef'ery.

Isn't that simple. Lot of platforms defining these routines clk_get(),
clk_put(), etc.
So, if we create dummy routines, they will get errors for multiple declarations.

Even this CONFIG_HAVE_CLK is not the right macro i think.

@Mike: Can you please suggest how to tackle this problem.

--
Viresh

[PATCH 5/5] ata/ahci_platform: Add clock framework support

[viresh kumar]

On 4/20/12, Sergei Shtylyov <sshtylyov@mvista.com> wrote:
> On 04/20/2012 06:38 PM, Viresh Kumar wrote:
>> diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
>> @@ -117,6 +118,20 @@ static int __init ahci_probe(struct platform_device
>> *pdev)
>>   		return -ENOMEM;
>>   	}
>>
>> +#ifdef CONFIG_HAVE_CLK
>> +	hpriv->clk = clk_get(dev, NULL);
>> +	if (IS_ERR(hpriv->clk)) {
>> +		dev_err(dev, "Clock not found\n");
>> +		return PTR_ERR(hpriv->clk);
>> +	}
>> +
>> +	rc = clk_prepare_enable(hpriv->clk);
>> +	if (rc) {
>> +		dev_err(dev, "clock prepare enable failed");
>> +		goto free_clk;
>> +	}
>> +#endif
>> +
>>   	/*
>>   	 * Some platforms might need to prepare for mmio region access,
>>   	 * which could be done in the following init call. So, the mmio
> [...]
>> @@ -190,12 +205,18 @@ static int __init ahci_probe(struct platform_device
>> *pdev)
>>   	rc = ata_host_activate(host, irq, ahci_interrupt, IRQF_SHARED,
>>   			&ahci_platform_sht);
>>   	if (rc)
>> -		goto err0;
>> +		goto pdata_exit;
>>
>>   	return 0;
>> -err0:
>> +pdata_exit:
>>   	if (pdata&&  pdata->exit)
>>   		pdata->exit(dev);
>> +disable_unprepare_clk:
>> +#ifdef CONFIG_HAVE_CLK
>> +	clk_disable_unprepare(hpriv->clk);
>> +free_clk:
>> +	clk_put(hpriv->clk);
>> +#endif
>>   	return rc;
>>   }
>>
>> @@ -204,12 +225,19 @@ static int __devexit ahci_remove(struct
>> platform_device *pdev)
>>   	struct device *dev =&pdev->dev;
>>   	struct ahci_platform_data *pdata = dev_get_platdata(dev);
>>   	struct ata_host *host = dev_get_drvdata(dev);
>> +#ifdef CONFIG_HAVE_CLK
>> +	struct ahci_host_priv *hpriv = host->private_data;
>> +#endif
>>
>>   	ata_host_detach(host);
>>
>>   	if (pdata&&  pdata->exit)
>>   		pdata->exit(dev);
>>
>> +#ifdef CONFIG_HAVE_CLK
>> +	clk_disable_unprepare(hpriv->clk);
>> +	clk_put(hpriv->clk);
>> +#endif
>>   	return 0;
>>   }
>>
>> @@ -244,6 +272,11 @@ static int ahci_suspend(struct device *dev)
>>
>>   	if (pdata&&  pdata->suspend)
>>   		return pdata->suspend(dev);
>> +
>> +#ifdef CONFIG_HAVE_CLK
>> +	clk_disable_unprepare(hpriv->clk);
>> +#endif
>> +
>>   	return 0;
>>   }
>>
>> @@ -253,16 +286,26 @@ static int ahci_resume(struct device *dev)
>>   	struct ata_host *host = dev_get_drvdata(dev);
>>   	int rc;
>>
>> +#ifdef CONFIG_HAVE_CLK
>> +	struct ahci_host_priv *hpriv = host->private_data;
>> +
>> +	rc = clk_prepare_enable(hpriv->clk);
>> +	if (rc) {
>> +		dev_err(dev, "clock prepare enable failed");
>> +		return rc;
>> +	}
>> +#endif
>> +
> [...]
>> @@ -270,6 +313,12 @@ static int ahci_resume(struct device *dev)
>>   	ata_host_resume(host);
>>
>>   	return 0;
>> +
>> +disable_unprepare_clk:
>> +#ifdef CONFIG_HAVE_CLK
>> +	clk_disable_unprepare(hpriv->clk);
>> +#endif
>
>     Can't we have empty inlines for !defined(CONFIG_HAVE_CLK) case in
> <linux/clk.h>. Otherwise there's too much uglye #ifdef'ery.

Isn't that simple. Lot of platforms defining these routines clk_get(),
clk_put(), etc.
So, if we create dummy routines, they will get errors for multiple declarations.

Even this CONFIG_HAVE_CLK is not the right macro i think.

@Mike: Can you please suggest how to tackle this problem.

--
Viresh

Re: [PATCH 5/5] ata/ahci_platform: Add clock framework support

[Sergei Shtylyov]

Hello.

On 04/20/2012 09:41 PM, viresh kumar wrote:

>>> diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
>>> @@ -117,6 +118,20 @@ static int __init ahci_probe(struct platform_device
>>> *pdev)
>>>    		return -ENOMEM;
>>>    	}
>>>
>>> +#ifdef CONFIG_HAVE_CLK
>>> +	hpriv->clk = clk_get(dev, NULL);
>>> +	if (IS_ERR(hpriv->clk)) {
>>> +		dev_err(dev, "Clock not found\n");
>>> +		return PTR_ERR(hpriv->clk);
>>> +	}
>>> +
>>> +	rc = clk_prepare_enable(hpriv->clk);
>>> +	if (rc) {
>>> +		dev_err(dev, "clock prepare enable failed");
>>> +		goto free_clk;
>>> +	}
>>> +#endif
>>> +
>>>    	/*
>>>    	 * Some platforms might need to prepare for mmio region access,
>>>    	 * which could be done in the following init call. So, the mmio
>> [...]
>>> @@ -190,12 +205,18 @@ static int __init ahci_probe(struct platform_device
>>> *pdev)
>>>    	rc = ata_host_activate(host, irq, ahci_interrupt, IRQF_SHARED,
>>>    			&ahci_platform_sht);
>>>    	if (rc)
>>> -		goto err0;
>>> +		goto pdata_exit;
>>>
>>>    	return 0;
>>> -err0:
>>> +pdata_exit:
>>>    	if (pdata&&   pdata->exit)
>>>    		pdata->exit(dev);
>>> +disable_unprepare_clk:
>>> +#ifdef CONFIG_HAVE_CLK
>>> +	clk_disable_unprepare(hpriv->clk);
>>> +free_clk:
>>> +	clk_put(hpriv->clk);
>>> +#endif
>>>    	return rc;
>>>    }
>>>
>>> @@ -204,12 +225,19 @@ static int __devexit ahci_remove(struct
>>> platform_device *pdev)
>>>    	struct device *dev =&pdev->dev;
>>>    	struct ahci_platform_data *pdata = dev_get_platdata(dev);
>>>    	struct ata_host *host = dev_get_drvdata(dev);
>>> +#ifdef CONFIG_HAVE_CLK
>>> +	struct ahci_host_priv *hpriv = host->private_data;
>>> +#endif
>>>
>>>    	ata_host_detach(host);
>>>
>>>    	if (pdata&&   pdata->exit)
>>>    		pdata->exit(dev);
>>>
>>> +#ifdef CONFIG_HAVE_CLK
>>> +	clk_disable_unprepare(hpriv->clk);
>>> +	clk_put(hpriv->clk);
>>> +#endif
>>>    	return 0;
>>>    }
>>>
>>> @@ -244,6 +272,11 @@ static int ahci_suspend(struct device *dev)
>>>
>>>    	if (pdata&&   pdata->suspend)
>>>    		return pdata->suspend(dev);
>>> +
>>> +#ifdef CONFIG_HAVE_CLK
>>> +	clk_disable_unprepare(hpriv->clk);
>>> +#endif
>>> +
>>>    	return 0;
>>>    }
>>>
>>> @@ -253,16 +286,26 @@ static int ahci_resume(struct device *dev)
>>>    	struct ata_host *host = dev_get_drvdata(dev);
>>>    	int rc;
>>>
>>> +#ifdef CONFIG_HAVE_CLK
>>> +	struct ahci_host_priv *hpriv = host->private_data;
>>> +
>>> +	rc = clk_prepare_enable(hpriv->clk);
>>> +	if (rc) {
>>> +		dev_err(dev, "clock prepare enable failed");
>>> +		return rc;
>>> +	}
>>> +#endif
>>> +
>> [...]
>>> @@ -270,6 +313,12 @@ static int ahci_resume(struct device *dev)
>>>    	ata_host_resume(host);
>>>
>>>    	return 0;
>>> +
>>> +disable_unprepare_clk:
>>> +#ifdef CONFIG_HAVE_CLK
>>> +	clk_disable_unprepare(hpriv->clk);
>>> +#endif

>>      Can't we have empty inlines for !defined(CONFIG_HAVE_CLK) case in
>> <linux/clk.h>. Otherwise there's too much uglye #ifdef'ery.

> Isn't that simple. Lot of platforms defining these routines clk_get(),
> clk_put(), etc.
> So, if we create dummy routines, they will get errors for multiple declarations.

    But those platforms should have CONFIG_HAVE_CLK defined...

WBR, Sergei

[PATCH 5/5] ata/ahci_platform: Add clock framework support

[Sergei Shtylyov]

Hello.

On 04/20/2012 09:41 PM, viresh kumar wrote:

>>> diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
>>> @@ -117,6 +118,20 @@ static int __init ahci_probe(struct platform_device
>>> *pdev)
>>>    		return -ENOMEM;
>>>    	}
>>>
>>> +#ifdef CONFIG_HAVE_CLK
>>> +	hpriv->clk = clk_get(dev, NULL);
>>> +	if (IS_ERR(hpriv->clk)) {
>>> +		dev_err(dev, "Clock not found\n");
>>> +		return PTR_ERR(hpriv->clk);
>>> +	}
>>> +
>>> +	rc = clk_prepare_enable(hpriv->clk);
>>> +	if (rc) {
>>> +		dev_err(dev, "clock prepare enable failed");
>>> +		goto free_clk;
>>> +	}
>>> +#endif
>>> +
>>>    	/*
>>>    	 * Some platforms might need to prepare for mmio region access,
>>>    	 * which could be done in the following init call. So, the mmio
>> [...]
>>> @@ -190,12 +205,18 @@ static int __init ahci_probe(struct platform_device
>>> *pdev)
>>>    	rc = ata_host_activate(host, irq, ahci_interrupt, IRQF_SHARED,
>>>    			&ahci_platform_sht);
>>>    	if (rc)
>>> -		goto err0;
>>> +		goto pdata_exit;
>>>
>>>    	return 0;
>>> -err0:
>>> +pdata_exit:
>>>    	if (pdata&&   pdata->exit)
>>>    		pdata->exit(dev);
>>> +disable_unprepare_clk:
>>> +#ifdef CONFIG_HAVE_CLK
>>> +	clk_disable_unprepare(hpriv->clk);
>>> +free_clk:
>>> +	clk_put(hpriv->clk);
>>> +#endif
>>>    	return rc;
>>>    }
>>>
>>> @@ -204,12 +225,19 @@ static int __devexit ahci_remove(struct
>>> platform_device *pdev)
>>>    	struct device *dev =&pdev->dev;
>>>    	struct ahci_platform_data *pdata = dev_get_platdata(dev);
>>>    	struct ata_host *host = dev_get_drvdata(dev);
>>> +#ifdef CONFIG_HAVE_CLK
>>> +	struct ahci_host_priv *hpriv = host->private_data;
>>> +#endif
>>>
>>>    	ata_host_detach(host);
>>>
>>>    	if (pdata&&   pdata->exit)
>>>    		pdata->exit(dev);
>>>
>>> +#ifdef CONFIG_HAVE_CLK
>>> +	clk_disable_unprepare(hpriv->clk);
>>> +	clk_put(hpriv->clk);
>>> +#endif
>>>    	return 0;
>>>    }
>>>
>>> @@ -244,6 +272,11 @@ static int ahci_suspend(struct device *dev)
>>>
>>>    	if (pdata&&   pdata->suspend)
>>>    		return pdata->suspend(dev);
>>> +
>>> +#ifdef CONFIG_HAVE_CLK
>>> +	clk_disable_unprepare(hpriv->clk);
>>> +#endif
>>> +
>>>    	return 0;
>>>    }
>>>
>>> @@ -253,16 +286,26 @@ static int ahci_resume(struct device *dev)
>>>    	struct ata_host *host = dev_get_drvdata(dev);
>>>    	int rc;
>>>
>>> +#ifdef CONFIG_HAVE_CLK
>>> +	struct ahci_host_priv *hpriv = host->private_data;
>>> +
>>> +	rc = clk_prepare_enable(hpriv->clk);
>>> +	if (rc) {
>>> +		dev_err(dev, "clock prepare enable failed");
>>> +		return rc;
>>> +	}
>>> +#endif
>>> +
>> [...]
>>> @@ -270,6 +313,12 @@ static int ahci_resume(struct device *dev)
>>>    	ata_host_resume(host);
>>>
>>>    	return 0;
>>> +
>>> +disable_unprepare_clk:
>>> +#ifdef CONFIG_HAVE_CLK
>>> +	clk_disable_unprepare(hpriv->clk);
>>> +#endif

>>      Can't we have empty inlines for !defined(CONFIG_HAVE_CLK) case in
>> <linux/clk.h>. Otherwise there's too much uglye #ifdef'ery.

> Isn't that simple. Lot of platforms defining these routines clk_get(),
> clk_put(), etc.
> So, if we create dummy routines, they will get errors for multiple declarations.

    But those platforms should have CONFIG_HAVE_CLK defined...

WBR, Sergei