[v2] i.MX27: Fix emma-prp clocks in mx2_camera.c
Commit Message
This driver wasn't converted to the new clock changes
(clk_prepare_enable/clk_disable_unprepare). Also naming
of emma-prp related clocks for the i.MX27 was not correct.
Signed-off-by: Javier Martin <javier.martin@vista-silicon.com>
---
arch/arm/mach-imx/clk-imx27.c | 8 ++++---
drivers/media/video/mx2_camera.c | 47 +++++++++++++++++++++-----------------
2 files changed, 31 insertions(+), 24 deletions(-)
Comments
On Fri, Jul 06, 2012 at 09:13:11AM +0200, Javier Martin wrote:
> This driver wasn't converted to the new clock changes
> (clk_prepare_enable/clk_disable_unprepare). Also naming
> of emma-prp related clocks for the i.MX27 was not correct.
>
> Signed-off-by: Javier Martin <javier.martin@vista-silicon.com>
> ---
> arch/arm/mach-imx/clk-imx27.c | 8 ++++---
> drivers/media/video/mx2_camera.c | 47 +++++++++++++++++++++-----------------
> 2 files changed, 31 insertions(+), 24 deletions(-)
>
> @@ -1616,23 +1616,12 @@ static int __devinit mx27_camera_emma_init(struct mx2_camera_dev *pcdev)
> goto exit_iounmap;
> }
>
> - pcdev->clk_emma = clk_get(NULL, "emma");
> - if (IS_ERR(pcdev->clk_emma)) {
> - err = PTR_ERR(pcdev->clk_emma);
> - goto exit_free_irq;
> - }
> -
> - clk_enable(pcdev->clk_emma);
> -
> err = mx27_camera_emma_prp_reset(pcdev);
> if (err)
> - goto exit_clk_emma_put;
> + goto exit_free_irq;
>
> return err;
>
> -exit_clk_emma_put:
> - clk_disable(pcdev->clk_emma);
> - clk_put(pcdev->clk_emma);
> exit_free_irq:
> free_irq(pcdev->irq_emma, pcdev);
> exit_iounmap:
> @@ -1655,6 +1644,7 @@ static int __devinit mx2_camera_probe(struct platform_device *pdev)
>
> res_csi = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> irq_csi = platform_get_irq(pdev, 0);
> +
> if (res_csi == NULL || irq_csi < 0) {
> dev_err(&pdev->dev, "Missing platform resources data\n");
> err = -ENODEV;
> @@ -1668,12 +1658,26 @@ static int __devinit mx2_camera_probe(struct platform_device *pdev)
> goto exit;
> }
>
> - pcdev->clk_csi = clk_get(&pdev->dev, NULL);
> + pcdev->clk_csi = devm_clk_get(&pdev->dev, "ahb");
> if (IS_ERR(pcdev->clk_csi)) {
> dev_err(&pdev->dev, "Could not get csi clock\n");
> err = PTR_ERR(pcdev->clk_csi);
> goto exit_kfree;
> }
> + pcdev->clk_emma_ipg = devm_clk_get(&pdev->dev, "emma-ipg");
> + if (IS_ERR(pcdev->clk_emma_ipg)) {
> + err = PTR_ERR(pcdev->clk_emma_ipg);
> + goto exit_kfree;
> + }
> + pcdev->clk_emma_ahb = devm_clk_get(&pdev->dev, "emma-ahb");
> + if (IS_ERR(pcdev->clk_emma_ahb)) {
> + err = PTR_ERR(pcdev->clk_emma_ahb);
> + goto exit_kfree;
> + }
> +
> + clk_prepare_enable(pcdev->clk_csi);
> + clk_prepare_enable(pcdev->clk_emma_ipg);
> + clk_prepare_enable(pcdev->clk_emma_ahb);
>
> pcdev->res_csi = res_csi;
> pcdev->pdata = pdev->dev.platform_data;
> @@ -1768,8 +1772,8 @@ exit_free_emma:
> eallocctx:
> if (cpu_is_mx27()) {
> free_irq(pcdev->irq_emma, pcdev);
> - clk_disable(pcdev->clk_emma);
> - clk_put(pcdev->clk_emma);
> + clk_disable_unprepare(pcdev->clk_emma_ipg);
> + clk_disable_unprepare(pcdev->clk_emma_ahb);
The clk_disable_unprepare is inside a cpu_is_mx27() which seems correct.
Shouldn't the corresponding clk_get be in cpu_is_mx27() aswell?
Sascha
On 6 July 2012 09:34, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Fri, Jul 06, 2012 at 09:13:11AM +0200, Javier Martin wrote:
>> This driver wasn't converted to the new clock changes
>> (clk_prepare_enable/clk_disable_unprepare). Also naming
>> of emma-prp related clocks for the i.MX27 was not correct.
>>
>> Signed-off-by: Javier Martin <javier.martin@vista-silicon.com>
>> ---
>> arch/arm/mach-imx/clk-imx27.c | 8 ++++---
>> drivers/media/video/mx2_camera.c | 47 +++++++++++++++++++++-----------------
>> 2 files changed, 31 insertions(+), 24 deletions(-)
>>
>> @@ -1616,23 +1616,12 @@ static int __devinit mx27_camera_emma_init(struct mx2_camera_dev *pcdev)
>> goto exit_iounmap;
>> }
>>
>> - pcdev->clk_emma = clk_get(NULL, "emma");
>> - if (IS_ERR(pcdev->clk_emma)) {
>> - err = PTR_ERR(pcdev->clk_emma);
>> - goto exit_free_irq;
>> - }
>> -
>> - clk_enable(pcdev->clk_emma);
>> -
>> err = mx27_camera_emma_prp_reset(pcdev);
>> if (err)
>> - goto exit_clk_emma_put;
>> + goto exit_free_irq;
>>
>> return err;
>>
>> -exit_clk_emma_put:
>> - clk_disable(pcdev->clk_emma);
>> - clk_put(pcdev->clk_emma);
>> exit_free_irq:
>> free_irq(pcdev->irq_emma, pcdev);
>> exit_iounmap:
>> @@ -1655,6 +1644,7 @@ static int __devinit mx2_camera_probe(struct platform_device *pdev)
>>
>> res_csi = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> irq_csi = platform_get_irq(pdev, 0);
>> +
>> if (res_csi == NULL || irq_csi < 0) {
>> dev_err(&pdev->dev, "Missing platform resources data\n");
>> err = -ENODEV;
>> @@ -1668,12 +1658,26 @@ static int __devinit mx2_camera_probe(struct platform_device *pdev)
>> goto exit;
>> }
>>
>> - pcdev->clk_csi = clk_get(&pdev->dev, NULL);
>> + pcdev->clk_csi = devm_clk_get(&pdev->dev, "ahb");
>> if (IS_ERR(pcdev->clk_csi)) {
>> dev_err(&pdev->dev, "Could not get csi clock\n");
>> err = PTR_ERR(pcdev->clk_csi);
>> goto exit_kfree;
>> }
>> + pcdev->clk_emma_ipg = devm_clk_get(&pdev->dev, "emma-ipg");
>> + if (IS_ERR(pcdev->clk_emma_ipg)) {
>> + err = PTR_ERR(pcdev->clk_emma_ipg);
>> + goto exit_kfree;
>> + }
>> + pcdev->clk_emma_ahb = devm_clk_get(&pdev->dev, "emma-ahb");
>> + if (IS_ERR(pcdev->clk_emma_ahb)) {
>> + err = PTR_ERR(pcdev->clk_emma_ahb);
>> + goto exit_kfree;
>> + }
>> +
>> + clk_prepare_enable(pcdev->clk_csi);
>> + clk_prepare_enable(pcdev->clk_emma_ipg);
>> + clk_prepare_enable(pcdev->clk_emma_ahb);
>>
>> pcdev->res_csi = res_csi;
>> pcdev->pdata = pdev->dev.platform_data;
>> @@ -1768,8 +1772,8 @@ exit_free_emma:
>> eallocctx:
>> if (cpu_is_mx27()) {
>> free_irq(pcdev->irq_emma, pcdev);
>> - clk_disable(pcdev->clk_emma);
>> - clk_put(pcdev->clk_emma);
>> + clk_disable_unprepare(pcdev->clk_emma_ipg);
>> + clk_disable_unprepare(pcdev->clk_emma_ahb);
>
> The clk_disable_unprepare is inside a cpu_is_mx27() which seems correct.
> Shouldn't the corresponding clk_get be in cpu_is_mx27() aswell?
Yes indeed. Should I fix it in a new version of this patch or should I
send another one instead?
On Fri, Jul 06, 2012 at 09:46:46AM +0200, javier Martin wrote:
> On 6 July 2012 09:34, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > On Fri, Jul 06, 2012 at 09:13:11AM +0200, Javier Martin wrote:
> >> if (cpu_is_mx27()) {
> >> free_irq(pcdev->irq_emma, pcdev);
> >> - clk_disable(pcdev->clk_emma);
> >> - clk_put(pcdev->clk_emma);
> >> + clk_disable_unprepare(pcdev->clk_emma_ipg);
> >> + clk_disable_unprepare(pcdev->clk_emma_ahb);
> >
> > The clk_disable_unprepare is inside a cpu_is_mx27() which seems correct.
> > Shouldn't the corresponding clk_get be in cpu_is_mx27() aswell?
>
> Yes indeed. Should I fix it in a new version of this patch or should I
> send another one instead?
Another version of this patch should be fine.
Sascha
@@ -223,7 +223,7 @@ int __init mx27_clocks_init(unsigned long fref)
clk_register_clkdev(clk[per3_gate], "per", "imx-fb.0");
clk_register_clkdev(clk[lcdc_ipg_gate], "ipg", "imx-fb.0");
clk_register_clkdev(clk[lcdc_ahb_gate], "ahb", "imx-fb.0");
- clk_register_clkdev(clk[csi_ahb_gate], NULL, "mx2-camera.0");
+ clk_register_clkdev(clk[csi_ahb_gate], "ahb", "mx2-camera.0");
clk_register_clkdev(clk[usb_div], "per", "fsl-usb2-udc");
clk_register_clkdev(clk[usb_ipg_gate], "ipg", "fsl-usb2-udc");
clk_register_clkdev(clk[usb_ahb_gate], "ahb", "fsl-usb2-udc");
@@ -250,8 +250,10 @@ int __init mx27_clocks_init(unsigned long fref)
clk_register_clkdev(clk[i2c2_ipg_gate], NULL, "imx-i2c.1");
clk_register_clkdev(clk[owire_ipg_gate], NULL, "mxc_w1.0");
clk_register_clkdev(clk[kpp_ipg_gate], NULL, "imx-keypad");
- clk_register_clkdev(clk[emma_ahb_gate], "ahb", "imx-emma");
- clk_register_clkdev(clk[emma_ipg_gate], "ipg", "imx-emma");
+ clk_register_clkdev(clk[emma_ahb_gate], "emma-ahb", "mx2-camera.0");
+ clk_register_clkdev(clk[emma_ipg_gate], "emma-ipg", "mx2-camera.0");
+ clk_register_clkdev(clk[emma_ahb_gate], "ahb", "m2m-emmaprp.0");
+ clk_register_clkdev(clk[emma_ipg_gate], "ipg", "m2m-emmaprp.0");
clk_register_clkdev(clk[iim_ipg_gate], "iim", NULL);
clk_register_clkdev(clk[gpio_ipg_gate], "gpio", NULL);
clk_register_clkdev(clk[brom_ahb_gate], "brom", NULL);
@@ -270,7 +270,7 @@ struct mx2_camera_dev {
struct device *dev;
struct soc_camera_host soc_host;
struct soc_camera_device *icd;
- struct clk *clk_csi, *clk_emma;
+ struct clk *clk_csi, *clk_emma_ahb, *clk_emma_ipg;
unsigned int irq_csi, irq_emma;
void __iomem *base_csi, *base_emma;
@@ -417,7 +417,7 @@ static int mx2_camera_add_device(struct soc_camera_device *icd)
if (pcdev->icd)
return -EBUSY;
- ret = clk_enable(pcdev->clk_csi);
+ ret = clk_prepare_enable(pcdev->clk_csi);
if (ret < 0)
return ret;
@@ -1616,23 +1616,12 @@ static int __devinit mx27_camera_emma_init(struct mx2_camera_dev *pcdev)
goto exit_iounmap;
}
- pcdev->clk_emma = clk_get(NULL, "emma");
- if (IS_ERR(pcdev->clk_emma)) {
- err = PTR_ERR(pcdev->clk_emma);
- goto exit_free_irq;
- }
-
- clk_enable(pcdev->clk_emma);
-
err = mx27_camera_emma_prp_reset(pcdev);
if (err)
- goto exit_clk_emma_put;
+ goto exit_free_irq;
return err;
-exit_clk_emma_put:
- clk_disable(pcdev->clk_emma);
- clk_put(pcdev->clk_emma);
exit_free_irq:
free_irq(pcdev->irq_emma, pcdev);
exit_iounmap:
@@ -1655,6 +1644,7 @@ static int __devinit mx2_camera_probe(struct platform_device *pdev)
res_csi = platform_get_resource(pdev, IORESOURCE_MEM, 0);
irq_csi = platform_get_irq(pdev, 0);
+
if (res_csi == NULL || irq_csi < 0) {
dev_err(&pdev->dev, "Missing platform resources data\n");
err = -ENODEV;
@@ -1668,12 +1658,26 @@ static int __devinit mx2_camera_probe(struct platform_device *pdev)
goto exit;
}
- pcdev->clk_csi = clk_get(&pdev->dev, NULL);
+ pcdev->clk_csi = devm_clk_get(&pdev->dev, "ahb");
if (IS_ERR(pcdev->clk_csi)) {
dev_err(&pdev->dev, "Could not get csi clock\n");
err = PTR_ERR(pcdev->clk_csi);
goto exit_kfree;
}
+ pcdev->clk_emma_ipg = devm_clk_get(&pdev->dev, "emma-ipg");
+ if (IS_ERR(pcdev->clk_emma_ipg)) {
+ err = PTR_ERR(pcdev->clk_emma_ipg);
+ goto exit_kfree;
+ }
+ pcdev->clk_emma_ahb = devm_clk_get(&pdev->dev, "emma-ahb");
+ if (IS_ERR(pcdev->clk_emma_ahb)) {
+ err = PTR_ERR(pcdev->clk_emma_ahb);
+ goto exit_kfree;
+ }
+
+ clk_prepare_enable(pcdev->clk_csi);
+ clk_prepare_enable(pcdev->clk_emma_ipg);
+ clk_prepare_enable(pcdev->clk_emma_ahb);
pcdev->res_csi = res_csi;
pcdev->pdata = pdev->dev.platform_data;
@@ -1768,8 +1772,8 @@ exit_free_emma:
eallocctx:
if (cpu_is_mx27()) {
free_irq(pcdev->irq_emma, pcdev);
- clk_disable(pcdev->clk_emma);
- clk_put(pcdev->clk_emma);
+ clk_disable_unprepare(pcdev->clk_emma_ipg);
+ clk_disable_unprepare(pcdev->clk_emma_ahb);
iounmap(pcdev->base_emma);
release_mem_region(pcdev->res_emma->start, resource_size(pcdev->res_emma));
}
@@ -1781,7 +1785,9 @@ exit_iounmap:
exit_release:
release_mem_region(res_csi->start, resource_size(res_csi));
exit_dma_free:
- clk_put(pcdev->clk_csi);
+ clk_disable_unprepare(pcdev->clk_emma_ipg);
+ clk_disable_unprepare(pcdev->clk_emma_ahb);
+ clk_disable_unprepare(pcdev->clk_csi);
exit_kfree:
kfree(pcdev);
exit:
@@ -1795,7 +1801,6 @@ static int __devexit mx2_camera_remove(struct platform_device *pdev)
struct mx2_camera_dev, soc_host);
struct resource *res;
- clk_put(pcdev->clk_csi);
if (cpu_is_mx25())
free_irq(pcdev->irq_csi, pcdev);
if (cpu_is_mx27())
@@ -1808,8 +1813,8 @@ static int __devexit mx2_camera_remove(struct platform_device *pdev)
iounmap(pcdev->base_csi);
if (cpu_is_mx27()) {
- clk_disable(pcdev->clk_emma);
- clk_put(pcdev->clk_emma);
+ clk_disable_unprepare(pcdev->clk_emma_ipg);
+ clk_disable_unprepare(pcdev->clk_emma_ahb);
iounmap(pcdev->base_emma);
res = pcdev->res_emma;
release_mem_region(res->start, resource_size(res));