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

login
register
mail settings
Submitter viresh kumar
Date April 20, 2012, 2:38 p.m.
Message ID <8308fd88100d4af942d920d51bf24184fcb33b72.1334932338.git.viresh.kumar@st.com>
Download mbox | patch
Permalink /patch/154052/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

viresh kumar - April 20, 2012, 2:38 p.m.
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(-)
Sergei Shtylyov - April 20, 2012, 2:58 p.m.
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
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
viresh kumar - April 20, 2012, 5:41 p.m.
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
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sergei Shtylyov - April 20, 2012, 6:09 p.m.
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

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

Patch

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 = {