* [PATCH 0/2] davinci: vpss: clock cleanup
@ 2013-03-22 7:53 Prabhakar lad
2013-03-22 7:53 ` [PATCH 1/2] media: davinci: vpss: enable vpss clocks Prabhakar lad
2013-03-22 7:53 ` [PATCH 2/2] media: davinci: vpbe: venc: move the enabling of vpss clocks to driver Prabhakar lad
0 siblings, 2 replies; 10+ messages in thread
From: Prabhakar lad @ 2013-03-22 7:53 UTC (permalink / raw)
To: linux-arm-kernel
From: Lad, Prabhakar <prabhakar.csengg@gmail.com>
This patch series cleanup's the VPSS clock enabling.
The first patch removes vpss clock enabling from the capture
drivers and moves it to the VPSS driver itself.
The second patch moves the venc_enable_vpss_clock() to the driver
which was being done in platform code.
Lad, Prabhakar (2):
media: davinci: vpss: enable vpss clocks
media: davinci: vpbe: venc: move the enabling of vpss clocks to
driver
arch/arm/mach-davinci/dm355.c | 3 -
arch/arm/mach-davinci/dm365.c | 9 +++-
arch/arm/mach-davinci/dm644x.c | 5 --
drivers/media/platform/davinci/dm355_ccdc.c | 39 +----------------
drivers/media/platform/davinci/dm644x_ccdc.c | 44 -------------------
drivers/media/platform/davinci/isif.c | 28 ++----------
drivers/media/platform/davinci/vpbe_venc.c | 26 +++++++++++
drivers/media/platform/davinci/vpss.c | 60 ++++++++++++++++++++++++++
8 files changed, 98 insertions(+), 116 deletions(-)
--
1.7.4.1
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 1/2] media: davinci: vpss: enable vpss clocks 2013-03-22 7:53 [PATCH 0/2] davinci: vpss: clock cleanup Prabhakar lad @ 2013-03-22 7:53 ` Prabhakar lad 2013-03-25 5:32 ` Sekhar Nori 2013-03-22 7:53 ` [PATCH 2/2] media: davinci: vpbe: venc: move the enabling of vpss clocks to driver Prabhakar lad 1 sibling, 1 reply; 10+ messages in thread From: Prabhakar lad @ 2013-03-22 7:53 UTC (permalink / raw) To: linux-arm-kernel From: Lad, Prabhakar <prabhakar.csengg@gmail.com> By default the VPSS clocks are only enabled in capture driver for davinci family which creates duplicates. This patch adds support to enable the VPSS clocks in VPSS driver. This avoids duplication of code and also adding clock aliases. This patch cleanups the VPSS clock enabling in the capture driver, and also removes the clock alias in machine file. Along side adds a vpss slave clock for DM365 as mentioned by Sekhar (https://patchwork.kernel.org/patch/1221261/). Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com> --- arch/arm/mach-davinci/dm355.c | 3 - arch/arm/mach-davinci/dm365.c | 9 +++- arch/arm/mach-davinci/dm644x.c | 5 -- drivers/media/platform/davinci/dm355_ccdc.c | 39 +---------------- drivers/media/platform/davinci/dm644x_ccdc.c | 44 ------------------- drivers/media/platform/davinci/isif.c | 28 ++---------- drivers/media/platform/davinci/vpss.c | 60 ++++++++++++++++++++++++++ 7 files changed, 72 insertions(+), 116 deletions(-) diff --git a/arch/arm/mach-davinci/dm355.c b/arch/arm/mach-davinci/dm355.c index b49c3b7..a917983 100644 --- a/arch/arm/mach-davinci/dm355.c +++ b/arch/arm/mach-davinci/dm355.c @@ -873,9 +873,6 @@ static int __init dm355_init_devices(void) if (!cpu_is_davinci_dm355()) return 0; - /* Add ccdc clock aliases */ - clk_add_alias("master", dm355_ccdc_dev.name, "vpss_master", NULL); - clk_add_alias("slave", dm355_ccdc_dev.name, "vpss_master", NULL); davinci_cfg_reg(DM355_INT_EDMA_CC); platform_device_register(&dm355_edma_device); platform_device_register(&dm355_vpss_device); diff --git a/arch/arm/mach-davinci/dm365.c b/arch/arm/mach-davinci/dm365.c index 6c39805..f4c19f7 100644 --- a/arch/arm/mach-davinci/dm365.c +++ b/arch/arm/mach-davinci/dm365.c @@ -257,6 +257,12 @@ static struct clk vpss_master_clk = { .flags = CLK_PSC, }; +static struct clk vpss_slave_clk = { + .name = "vpss_slave", + .parent = &pll1_sysclk5, + .lpsc = DAVINCI_LPSC_VPSSSLV, +}; + static struct clk arm_clk = { .name = "arm_clk", .parent = &pll2_sysclk2, @@ -450,6 +456,7 @@ static struct clk_lookup dm365_clks[] = { CLK(NULL, "pll2_sysclk9", &pll2_sysclk9), CLK(NULL, "vpss_dac", &vpss_dac_clk), CLK(NULL, "vpss_master", &vpss_master_clk), + CLK(NULL, "vpss_slave", &vpss_slave_clk), CLK(NULL, "arm", &arm_clk), CLK(NULL, "uart0", &uart0_clk), CLK(NULL, "uart1", &uart1_clk), @@ -1239,8 +1246,6 @@ static int __init dm365_init_devices(void) clk_add_alias(NULL, dev_name(&dm365_mdio_device.dev), NULL, &dm365_emac_device.dev); - /* Add isif clock alias */ - clk_add_alias("master", dm365_isif_dev.name, "vpss_master", NULL); platform_device_register(&dm365_vpss_device); platform_device_register(&dm365_isif_dev); platform_device_register(&vpfe_capture_dev); diff --git a/arch/arm/mach-davinci/dm644x.c b/arch/arm/mach-davinci/dm644x.c index ee0e994..026e7e3 100644 --- a/arch/arm/mach-davinci/dm644x.c +++ b/arch/arm/mach-davinci/dm644x.c @@ -901,11 +901,6 @@ int __init dm644x_init_video(struct vpfe_config *vpfe_cfg, dm644x_vpfe_dev.dev.platform_data = vpfe_cfg; platform_device_register(&dm644x_ccdc_dev); platform_device_register(&dm644x_vpfe_dev); - /* Add ccdc clock aliases */ - clk_add_alias("master", dm644x_ccdc_dev.name, - "vpss_master", NULL); - clk_add_alias("slave", dm644x_ccdc_dev.name, - "vpss_slave", NULL); } if (vpbe_cfg) { diff --git a/drivers/media/platform/davinci/dm355_ccdc.c b/drivers/media/platform/davinci/dm355_ccdc.c index 2364dba..05f8fb7 100644 --- a/drivers/media/platform/davinci/dm355_ccdc.c +++ b/drivers/media/platform/davinci/dm355_ccdc.c @@ -37,7 +37,6 @@ #include <linux/platform_device.h> #include <linux/uaccess.h> #include <linux/videodev2.h> -#include <linux/clk.h> #include <linux/err.h> #include <linux/module.h> @@ -59,10 +58,6 @@ static struct ccdc_oper_config { struct ccdc_params_raw bayer; /* YCbCr configuration */ struct ccdc_params_ycbcr ycbcr; - /* Master clock */ - struct clk *mclk; - /* slave clock */ - struct clk *sclk; /* ccdc base address */ void __iomem *base_addr; } ccdc_cfg = { @@ -997,32 +992,10 @@ static int dm355_ccdc_probe(struct platform_device *pdev) goto fail_nomem; } - /* Get and enable Master clock */ - ccdc_cfg.mclk = clk_get(&pdev->dev, "master"); - if (IS_ERR(ccdc_cfg.mclk)) { - status = PTR_ERR(ccdc_cfg.mclk); - goto fail_nomap; - } - if (clk_prepare_enable(ccdc_cfg.mclk)) { - status = -ENODEV; - goto fail_mclk; - } - - /* Get and enable Slave clock */ - ccdc_cfg.sclk = clk_get(&pdev->dev, "slave"); - if (IS_ERR(ccdc_cfg.sclk)) { - status = PTR_ERR(ccdc_cfg.sclk); - goto fail_mclk; - } - if (clk_prepare_enable(ccdc_cfg.sclk)) { - status = -ENODEV; - goto fail_sclk; - } - /* Platform data holds setup_pinmux function ptr */ if (NULL == pdev->dev.platform_data) { status = -ENODEV; - goto fail_sclk; + goto fail_nomap; } setup_pinmux = pdev->dev.platform_data; /* @@ -1033,12 +1006,6 @@ static int dm355_ccdc_probe(struct platform_device *pdev) ccdc_cfg.dev = &pdev->dev; printk(KERN_NOTICE "%s is registered with vpfe.\n", ccdc_hw_dev.name); return 0; -fail_sclk: - clk_disable_unprepare(ccdc_cfg.sclk); - clk_put(ccdc_cfg.sclk); -fail_mclk: - clk_disable_unprepare(ccdc_cfg.mclk); - clk_put(ccdc_cfg.mclk); fail_nomap: iounmap(ccdc_cfg.base_addr); fail_nomem: @@ -1052,10 +1019,6 @@ static int dm355_ccdc_remove(struct platform_device *pdev) { struct resource *res; - clk_disable_unprepare(ccdc_cfg.sclk); - clk_disable_unprepare(ccdc_cfg.mclk); - clk_put(ccdc_cfg.mclk); - clk_put(ccdc_cfg.sclk); iounmap(ccdc_cfg.base_addr); res = platform_get_resource(pdev, IORESOURCE_MEM, 0); if (res) diff --git a/drivers/media/platform/davinci/dm644x_ccdc.c b/drivers/media/platform/davinci/dm644x_ccdc.c index 971d639..30fa084 100644 --- a/drivers/media/platform/davinci/dm644x_ccdc.c +++ b/drivers/media/platform/davinci/dm644x_ccdc.c @@ -38,7 +38,6 @@ #include <linux/uaccess.h> #include <linux/videodev2.h> #include <linux/gfp.h> -#include <linux/clk.h> #include <linux/err.h> #include <linux/module.h> @@ -60,10 +59,6 @@ static struct ccdc_oper_config { struct ccdc_params_raw bayer; /* YCbCr configuration */ struct ccdc_params_ycbcr ycbcr; - /* Master clock */ - struct clk *mclk; - /* slave clock */ - struct clk *sclk; /* ccdc base address */ void __iomem *base_addr; } ccdc_cfg = { @@ -991,38 +986,9 @@ static int dm644x_ccdc_probe(struct platform_device *pdev) goto fail_nomem; } - /* Get and enable Master clock */ - ccdc_cfg.mclk = clk_get(&pdev->dev, "master"); - if (IS_ERR(ccdc_cfg.mclk)) { - status = PTR_ERR(ccdc_cfg.mclk); - goto fail_nomap; - } - if (clk_prepare_enable(ccdc_cfg.mclk)) { - status = -ENODEV; - goto fail_mclk; - } - - /* Get and enable Slave clock */ - ccdc_cfg.sclk = clk_get(&pdev->dev, "slave"); - if (IS_ERR(ccdc_cfg.sclk)) { - status = PTR_ERR(ccdc_cfg.sclk); - goto fail_mclk; - } - if (clk_prepare_enable(ccdc_cfg.sclk)) { - status = -ENODEV; - goto fail_sclk; - } ccdc_cfg.dev = &pdev->dev; printk(KERN_NOTICE "%s is registered with vpfe.\n", ccdc_hw_dev.name); return 0; -fail_sclk: - clk_disable_unprepare(ccdc_cfg.sclk); - clk_put(ccdc_cfg.sclk); -fail_mclk: - clk_disable_unprepare(ccdc_cfg.mclk); - clk_put(ccdc_cfg.mclk); -fail_nomap: - iounmap(ccdc_cfg.base_addr); fail_nomem: release_mem_region(res->start, resource_size(res)); fail_nores: @@ -1034,10 +1000,6 @@ static int dm644x_ccdc_remove(struct platform_device *pdev) { struct resource *res; - clk_disable_unprepare(ccdc_cfg.mclk); - clk_disable_unprepare(ccdc_cfg.sclk); - clk_put(ccdc_cfg.mclk); - clk_put(ccdc_cfg.sclk); iounmap(ccdc_cfg.base_addr); res = platform_get_resource(pdev, IORESOURCE_MEM, 0); if (res) @@ -1052,18 +1014,12 @@ static int dm644x_ccdc_suspend(struct device *dev) ccdc_save_context(); /* Disable CCDC */ ccdc_enable(0); - /* Disable both master and slave clock */ - clk_disable_unprepare(ccdc_cfg.mclk); - clk_disable_unprepare(ccdc_cfg.sclk); return 0; } static int dm644x_ccdc_resume(struct device *dev) { - /* Enable both master and slave clock */ - clk_prepare_enable(ccdc_cfg.mclk); - clk_prepare_enable(ccdc_cfg.sclk); /* Restore CCDC context */ ccdc_restore_context(); diff --git a/drivers/media/platform/davinci/isif.c b/drivers/media/platform/davinci/isif.c index abc3ae3..3332cca 100644 --- a/drivers/media/platform/davinci/isif.c +++ b/drivers/media/platform/davinci/isif.c @@ -32,7 +32,6 @@ #include <linux/uaccess.h> #include <linux/io.h> #include <linux/videodev2.h> -#include <linux/clk.h> #include <linux/err.h> #include <linux/module.h> @@ -88,8 +87,6 @@ static struct isif_oper_config { struct isif_ycbcr_config ycbcr; struct isif_params_raw bayer; enum isif_data_pack data_pack; - /* Master clock */ - struct clk *mclk; /* ISIF base address */ void __iomem *base_addr; /* ISIF Linear Table 0 */ @@ -1039,6 +1036,10 @@ static int isif_probe(struct platform_device *pdev) void *__iomem addr; int status = 0, i; + /* Platform data holds setup_pinmux function ptr */ + if (!pdev->dev.platform_data) + return -ENODEV; + /* * first try to register with vpfe. If not correct platform, then we * don't have to iomap @@ -1047,22 +1048,6 @@ static int isif_probe(struct platform_device *pdev) if (status < 0) return status; - /* Get and enable Master clock */ - isif_cfg.mclk = clk_get(&pdev->dev, "master"); - if (IS_ERR(isif_cfg.mclk)) { - status = PTR_ERR(isif_cfg.mclk); - goto fail_mclk; - } - if (clk_prepare_enable(isif_cfg.mclk)) { - status = -ENODEV; - goto fail_mclk; - } - - /* Platform data holds setup_pinmux function ptr */ - if (NULL == pdev->dev.platform_data) { - status = -ENODEV; - goto fail_mclk; - } setup_pinmux = pdev->dev.platform_data; /* * setup Mux configuration for ccdc which may be different for @@ -1124,9 +1109,6 @@ fail_nobase_res: release_mem_region(res->start, resource_size(res)); i--; } -fail_mclk: - clk_disable_unprepare(isif_cfg.mclk); - clk_put(isif_cfg.mclk); vpfe_unregister_ccdc_device(&isif_hw_dev); return status; } @@ -1146,8 +1128,6 @@ static int isif_remove(struct platform_device *pdev) i++; } vpfe_unregister_ccdc_device(&isif_hw_dev); - clk_disable_unprepare(isif_cfg.mclk); - clk_put(isif_cfg.mclk); return 0; } diff --git a/drivers/media/platform/davinci/vpss.c b/drivers/media/platform/davinci/vpss.c index a19c552..db69317 100644 --- a/drivers/media/platform/davinci/vpss.c +++ b/drivers/media/platform/davinci/vpss.c @@ -17,6 +17,7 @@ * * common vpss system module platform driver for all video drivers. */ +#include <linux/clk.h> #include <linux/kernel.h> #include <linux/sched.h> #include <linux/init.h> @@ -126,6 +127,10 @@ struct vpss_oper_config { enum vpss_platform_type platform; spinlock_t vpss_lock; struct vpss_hw_ops hw_ops; + /* Master clock */ + struct clk *mclk; + /* slave clock */ + struct clk *sclk; }; static struct vpss_oper_config oper_cfg; @@ -429,6 +434,26 @@ static int vpss_probe(struct platform_device *pdev) return -ENODEV; } + /* Get and enable Master clock */ + oper_cfg.mclk = clk_get(&pdev->dev, "vpss_master"); + if (IS_ERR(oper_cfg.mclk)) { + status = PTR_ERR(oper_cfg.mclk); + goto fail_getclk; + } + status = clk_prepare_enable(oper_cfg.mclk); + if (status) + goto fail_mclk; + + /* Get and enable Slave clock */ + oper_cfg.sclk = clk_get(&pdev->dev, "vpss_slave"); + if (IS_ERR(oper_cfg.sclk)) { + status = PTR_ERR(oper_cfg.sclk); + goto fail_mclk; + } + status = clk_prepare_enable(oper_cfg.sclk); + if (status) + goto fail_sclk; + dev_info(&pdev->dev, "%s vpss probed\n", platform_name); r1 = platform_get_resource(pdev, IORESOURCE_MEM, 0); if (!r1) @@ -500,6 +525,13 @@ fail2: iounmap(oper_cfg.vpss_regs_base0); fail1: release_mem_region(r1->start, resource_size(r1)); +fail_sclk: + clk_disable_unprepare(oper_cfg.sclk); + clk_put(oper_cfg.sclk); +fail_mclk: + clk_disable_unprepare(oper_cfg.mclk); + clk_put(oper_cfg.mclk); +fail_getclk: return status; } @@ -510,6 +542,10 @@ static int vpss_remove(struct platform_device *pdev) iounmap(oper_cfg.vpss_regs_base0); res = platform_get_resource(pdev, IORESOURCE_MEM, 0); release_mem_region(res->start, resource_size(res)); + clk_disable_unprepare(oper_cfg.mclk); + clk_disable_unprepare(oper_cfg.sclk); + clk_put(oper_cfg.mclk); + clk_put(oper_cfg.sclk); if (oper_cfg.platform == DM355 || oper_cfg.platform == DM365) { iounmap(oper_cfg.vpss_regs_base1); res = platform_get_resource(pdev, IORESOURCE_MEM, 1); @@ -518,10 +554,34 @@ static int vpss_remove(struct platform_device *pdev) return 0; } +static int vpss_suspend(struct device *dev) +{ + /* Disable both master and slave clock */ + clk_disable_unprepare(oper_cfg.mclk); + clk_disable_unprepare(oper_cfg.sclk); + + return 0; +} + +static int vpss_resume(struct device *dev) +{ + /* Enable both master and slave clock */ + clk_prepare_enable(oper_cfg.mclk); + clk_prepare_enable(oper_cfg.sclk); + + return 0; +} + +static const struct dev_pm_ops vpss_pm_ops = { + .suspend = vpss_suspend, + .resume = vpss_resume, +}; + static struct platform_driver vpss_driver = { .driver = { .name = "vpss", .owner = THIS_MODULE, + .pm = &vpss_pm_ops, }, .remove = vpss_remove, .probe = vpss_probe, -- 1.7.4.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 1/2] media: davinci: vpss: enable vpss clocks 2013-03-22 7:53 ` [PATCH 1/2] media: davinci: vpss: enable vpss clocks Prabhakar lad @ 2013-03-25 5:32 ` Sekhar Nori 2013-03-25 10:14 ` Prabhakar Lad 0 siblings, 1 reply; 10+ messages in thread From: Sekhar Nori @ 2013-03-25 5:32 UTC (permalink / raw) To: linux-arm-kernel On 3/22/2013 1:23 PM, Prabhakar lad wrote: > From: Lad, Prabhakar <prabhakar.csengg@gmail.com> > > By default the VPSS clocks are only enabled in capture driver > for davinci family which creates duplicates. This > patch adds support to enable the VPSS clocks in VPSS driver. > This avoids duplication of code and also adding clock aliases. > This patch cleanups the VPSS clock enabling in the capture driver, > and also removes the clock alias in machine file. Along side adds > a vpss slave clock for DM365 as mentioned by Sekhar > (https://patchwork.kernel.org/patch/1221261/). > > Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com> > --- > arch/arm/mach-davinci/dm355.c | 3 - > arch/arm/mach-davinci/dm365.c | 9 +++- > arch/arm/mach-davinci/dm644x.c | 5 -- > drivers/media/platform/davinci/dm355_ccdc.c | 39 +---------------- > drivers/media/platform/davinci/dm644x_ccdc.c | 44 ------------------- > drivers/media/platform/davinci/isif.c | 28 ++---------- > drivers/media/platform/davinci/vpss.c | 60 ++++++++++++++++++++++++++ > 7 files changed, 72 insertions(+), 116 deletions(-) > > static struct clk arm_clk = { > .name = "arm_clk", > .parent = &pll2_sysclk2, > @@ -450,6 +456,7 @@ static struct clk_lookup dm365_clks[] = { > CLK(NULL, "pll2_sysclk9", &pll2_sysclk9), > CLK(NULL, "vpss_dac", &vpss_dac_clk), > CLK(NULL, "vpss_master", &vpss_master_clk), > + CLK(NULL, "vpss_slave", &vpss_slave_clk), These should use device name for look-up instead of relying just on con_id. So the entry should look like: CLK("vpss", "slave", &vpss_slave_clk), > CLK(NULL, "arm", &arm_clk), > CLK(NULL, "uart0", &uart0_clk), > CLK(NULL, "uart1", &uart1_clk), > @@ -1239,8 +1246,6 @@ static int __init dm365_init_devices(void) > clk_add_alias(NULL, dev_name(&dm365_mdio_device.dev), > NULL, &dm365_emac_device.dev); > > - /* Add isif clock alias */ > - clk_add_alias("master", dm365_isif_dev.name, "vpss_master", NULL); > platform_device_register(&dm365_vpss_device); > platform_device_register(&dm365_isif_dev); > platform_device_register(&vpfe_capture_dev); > diff --git a/arch/arm/mach-davinci/dm644x.c b/arch/arm/mach-davinci/dm644x.c > index ee0e994..026e7e3 100644 > --- a/arch/arm/mach-davinci/dm644x.c > +++ b/arch/arm/mach-davinci/dm644x.c > @@ -901,11 +901,6 @@ int __init dm644x_init_video(struct vpfe_config *vpfe_cfg, > dm644x_vpfe_dev.dev.platform_data = vpfe_cfg; > platform_device_register(&dm644x_ccdc_dev); > platform_device_register(&dm644x_vpfe_dev); > - /* Add ccdc clock aliases */ > - clk_add_alias("master", dm644x_ccdc_dev.name, > - "vpss_master", NULL); > - clk_add_alias("slave", dm644x_ccdc_dev.name, > - "vpss_slave", NULL); > } > > if (vpbe_cfg) { > diff --git a/drivers/media/platform/davinci/dm355_ccdc.c b/drivers/media/platform/davinci/dm355_ccdc.c > index 2364dba..05f8fb7 100644 > --- a/drivers/media/platform/davinci/dm355_ccdc.c > +++ b/drivers/media/platform/davinci/dm355_ccdc.c > @@ -37,7 +37,6 @@ > #include <linux/platform_device.h> > #include <linux/uaccess.h> > #include <linux/videodev2.h> > -#include <linux/clk.h> > #include <linux/err.h> > #include <linux/module.h> > > @@ -59,10 +58,6 @@ static struct ccdc_oper_config { > struct ccdc_params_raw bayer; > /* YCbCr configuration */ > struct ccdc_params_ycbcr ycbcr; > - /* Master clock */ > - struct clk *mclk; > - /* slave clock */ > - struct clk *sclk; > /* ccdc base address */ > void __iomem *base_addr; > } ccdc_cfg = { > @@ -997,32 +992,10 @@ static int dm355_ccdc_probe(struct platform_device *pdev) > goto fail_nomem; > } > > - /* Get and enable Master clock */ > - ccdc_cfg.mclk = clk_get(&pdev->dev, "master"); > - if (IS_ERR(ccdc_cfg.mclk)) { > - status = PTR_ERR(ccdc_cfg.mclk); > - goto fail_nomap; > - } > - if (clk_prepare_enable(ccdc_cfg.mclk)) { > - status = -ENODEV; > - goto fail_mclk; > - } > - > - /* Get and enable Slave clock */ > - ccdc_cfg.sclk = clk_get(&pdev->dev, "slave"); > - if (IS_ERR(ccdc_cfg.sclk)) { > - status = PTR_ERR(ccdc_cfg.sclk); > - goto fail_mclk; > - } > - if (clk_prepare_enable(ccdc_cfg.sclk)) { > - status = -ENODEV; > - goto fail_sclk; > - } > - > /* Platform data holds setup_pinmux function ptr */ > if (NULL == pdev->dev.platform_data) { > status = -ENODEV; > - goto fail_sclk; > + goto fail_nomap; > } > setup_pinmux = pdev->dev.platform_data; > /* > @@ -1033,12 +1006,6 @@ static int dm355_ccdc_probe(struct platform_device *pdev) > ccdc_cfg.dev = &pdev->dev; > printk(KERN_NOTICE "%s is registered with vpfe.\n", ccdc_hw_dev.name); > return 0; > -fail_sclk: > - clk_disable_unprepare(ccdc_cfg.sclk); > - clk_put(ccdc_cfg.sclk); > -fail_mclk: > - clk_disable_unprepare(ccdc_cfg.mclk); > - clk_put(ccdc_cfg.mclk); > fail_nomap: > iounmap(ccdc_cfg.base_addr); > fail_nomem: > @@ -1052,10 +1019,6 @@ static int dm355_ccdc_remove(struct platform_device *pdev) > { > struct resource *res; > > - clk_disable_unprepare(ccdc_cfg.sclk); > - clk_disable_unprepare(ccdc_cfg.mclk); > - clk_put(ccdc_cfg.mclk); > - clk_put(ccdc_cfg.sclk); > iounmap(ccdc_cfg.base_addr); > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > if (res) > diff --git a/drivers/media/platform/davinci/dm644x_ccdc.c b/drivers/media/platform/davinci/dm644x_ccdc.c > index 971d639..30fa084 100644 > --- a/drivers/media/platform/davinci/dm644x_ccdc.c > +++ b/drivers/media/platform/davinci/dm644x_ccdc.c > @@ -38,7 +38,6 @@ > #include <linux/uaccess.h> > #include <linux/videodev2.h> > #include <linux/gfp.h> > -#include <linux/clk.h> > #include <linux/err.h> > #include <linux/module.h> > > @@ -60,10 +59,6 @@ static struct ccdc_oper_config { > struct ccdc_params_raw bayer; > /* YCbCr configuration */ > struct ccdc_params_ycbcr ycbcr; > - /* Master clock */ > - struct clk *mclk; > - /* slave clock */ > - struct clk *sclk; > /* ccdc base address */ > void __iomem *base_addr; > } ccdc_cfg = { > @@ -991,38 +986,9 @@ static int dm644x_ccdc_probe(struct platform_device *pdev) > goto fail_nomem; > } > > - /* Get and enable Master clock */ > - ccdc_cfg.mclk = clk_get(&pdev->dev, "master"); > - if (IS_ERR(ccdc_cfg.mclk)) { > - status = PTR_ERR(ccdc_cfg.mclk); > - goto fail_nomap; > - } > - if (clk_prepare_enable(ccdc_cfg.mclk)) { > - status = -ENODEV; > - goto fail_mclk; > - } > - > - /* Get and enable Slave clock */ > - ccdc_cfg.sclk = clk_get(&pdev->dev, "slave"); > - if (IS_ERR(ccdc_cfg.sclk)) { > - status = PTR_ERR(ccdc_cfg.sclk); > - goto fail_mclk; > - } > - if (clk_prepare_enable(ccdc_cfg.sclk)) { > - status = -ENODEV; > - goto fail_sclk; > - } > ccdc_cfg.dev = &pdev->dev; > printk(KERN_NOTICE "%s is registered with vpfe.\n", ccdc_hw_dev.name); > return 0; > -fail_sclk: > - clk_disable_unprepare(ccdc_cfg.sclk); > - clk_put(ccdc_cfg.sclk); > -fail_mclk: > - clk_disable_unprepare(ccdc_cfg.mclk); > - clk_put(ccdc_cfg.mclk); > -fail_nomap: > - iounmap(ccdc_cfg.base_addr); > fail_nomem: > release_mem_region(res->start, resource_size(res)); > fail_nores: > @@ -1034,10 +1000,6 @@ static int dm644x_ccdc_remove(struct platform_device *pdev) > { > struct resource *res; > > - clk_disable_unprepare(ccdc_cfg.mclk); > - clk_disable_unprepare(ccdc_cfg.sclk); > - clk_put(ccdc_cfg.mclk); > - clk_put(ccdc_cfg.sclk); > iounmap(ccdc_cfg.base_addr); > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > if (res) > @@ -1052,18 +1014,12 @@ static int dm644x_ccdc_suspend(struct device *dev) > ccdc_save_context(); > /* Disable CCDC */ > ccdc_enable(0); > - /* Disable both master and slave clock */ > - clk_disable_unprepare(ccdc_cfg.mclk); > - clk_disable_unprepare(ccdc_cfg.sclk); > > return 0; > } > > static int dm644x_ccdc_resume(struct device *dev) > { > - /* Enable both master and slave clock */ > - clk_prepare_enable(ccdc_cfg.mclk); > - clk_prepare_enable(ccdc_cfg.sclk); > /* Restore CCDC context */ > ccdc_restore_context(); > > diff --git a/drivers/media/platform/davinci/isif.c b/drivers/media/platform/davinci/isif.c > index abc3ae3..3332cca 100644 > --- a/drivers/media/platform/davinci/isif.c > +++ b/drivers/media/platform/davinci/isif.c > @@ -32,7 +32,6 @@ > #include <linux/uaccess.h> > #include <linux/io.h> > #include <linux/videodev2.h> > -#include <linux/clk.h> > #include <linux/err.h> > #include <linux/module.h> > > @@ -88,8 +87,6 @@ static struct isif_oper_config { > struct isif_ycbcr_config ycbcr; > struct isif_params_raw bayer; > enum isif_data_pack data_pack; > - /* Master clock */ > - struct clk *mclk; > /* ISIF base address */ > void __iomem *base_addr; > /* ISIF Linear Table 0 */ > @@ -1039,6 +1036,10 @@ static int isif_probe(struct platform_device *pdev) > void *__iomem addr; > int status = 0, i; > > + /* Platform data holds setup_pinmux function ptr */ > + if (!pdev->dev.platform_data) > + return -ENODEV; > + This change seems unrelated. I suggest moving it to a different patch or atleast note it in the description. > /* > * first try to register with vpfe. If not correct platform, then we > * don't have to iomap > @@ -1047,22 +1048,6 @@ static int isif_probe(struct platform_device *pdev) > if (status < 0) > return status; > > - /* Get and enable Master clock */ > - isif_cfg.mclk = clk_get(&pdev->dev, "master"); > - if (IS_ERR(isif_cfg.mclk)) { > - status = PTR_ERR(isif_cfg.mclk); > - goto fail_mclk; > - } > - if (clk_prepare_enable(isif_cfg.mclk)) { > - status = -ENODEV; > - goto fail_mclk; > - } > - > - /* Platform data holds setup_pinmux function ptr */ > - if (NULL == pdev->dev.platform_data) { > - status = -ENODEV; > - goto fail_mclk; > - } > setup_pinmux = pdev->dev.platform_data; > /* > * setup Mux configuration for ccdc which may be different for > @@ -1124,9 +1109,6 @@ fail_nobase_res: > release_mem_region(res->start, resource_size(res)); > i--; > } > -fail_mclk: > - clk_disable_unprepare(isif_cfg.mclk); > - clk_put(isif_cfg.mclk); > vpfe_unregister_ccdc_device(&isif_hw_dev); > return status; > } > @@ -1146,8 +1128,6 @@ static int isif_remove(struct platform_device *pdev) > i++; > } > vpfe_unregister_ccdc_device(&isif_hw_dev); > - clk_disable_unprepare(isif_cfg.mclk); > - clk_put(isif_cfg.mclk); > return 0; > } > > diff --git a/drivers/media/platform/davinci/vpss.c b/drivers/media/platform/davinci/vpss.c > index a19c552..db69317 100644 > --- a/drivers/media/platform/davinci/vpss.c > +++ b/drivers/media/platform/davinci/vpss.c > @@ -17,6 +17,7 @@ > * > * common vpss system module platform driver for all video drivers. > */ > +#include <linux/clk.h> > #include <linux/kernel.h> > #include <linux/sched.h> > #include <linux/init.h> > @@ -126,6 +127,10 @@ struct vpss_oper_config { > enum vpss_platform_type platform; > spinlock_t vpss_lock; > struct vpss_hw_ops hw_ops; > + /* Master clock */ > + struct clk *mclk; > + /* slave clock */ > + struct clk *sclk; > }; > > static struct vpss_oper_config oper_cfg; > @@ -429,6 +434,26 @@ static int vpss_probe(struct platform_device *pdev) > return -ENODEV; > } > > + /* Get and enable Master clock */ > + oper_cfg.mclk = clk_get(&pdev->dev, "vpss_master"); use devm_clk_get() here to simplify the error handling. > + if (IS_ERR(oper_cfg.mclk)) { > + status = PTR_ERR(oper_cfg.mclk); > + goto fail_getclk; > + } > + status = clk_prepare_enable(oper_cfg.mclk); > + if (status) > + goto fail_mclk; > + > + /* Get and enable Slave clock */ > + oper_cfg.sclk = clk_get(&pdev->dev, "vpss_slave"); > + if (IS_ERR(oper_cfg.sclk)) { > + status = PTR_ERR(oper_cfg.sclk); > + goto fail_mclk; > + } > + status = clk_prepare_enable(oper_cfg.sclk); > + if (status) > + goto fail_sclk; > + > dev_info(&pdev->dev, "%s vpss probed\n", platform_name); > r1 = platform_get_resource(pdev, IORESOURCE_MEM, 0); > if (!r1) > @@ -500,6 +525,13 @@ fail2: > iounmap(oper_cfg.vpss_regs_base0); > fail1: > release_mem_region(r1->start, resource_size(r1)); > +fail_sclk: > + clk_disable_unprepare(oper_cfg.sclk); > + clk_put(oper_cfg.sclk); > +fail_mclk: > + clk_disable_unprepare(oper_cfg.mclk); > + clk_put(oper_cfg.mclk); > +fail_getclk: > return status; > } > > @@ -510,6 +542,10 @@ static int vpss_remove(struct platform_device *pdev) > iounmap(oper_cfg.vpss_regs_base0); > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > release_mem_region(res->start, resource_size(res)); > + clk_disable_unprepare(oper_cfg.mclk); > + clk_disable_unprepare(oper_cfg.sclk); > + clk_put(oper_cfg.mclk); > + clk_put(oper_cfg.sclk); > if (oper_cfg.platform == DM355 || oper_cfg.platform == DM365) { > iounmap(oper_cfg.vpss_regs_base1); > res = platform_get_resource(pdev, IORESOURCE_MEM, 1); > @@ -518,10 +554,34 @@ static int vpss_remove(struct platform_device *pdev) > return 0; > } > > +static int vpss_suspend(struct device *dev) > +{ > + /* Disable both master and slave clock */ > + clk_disable_unprepare(oper_cfg.mclk); > + clk_disable_unprepare(oper_cfg.sclk); > + > + return 0; > +} > + > +static int vpss_resume(struct device *dev) > +{ > + /* Enable both master and slave clock */ > + clk_prepare_enable(oper_cfg.mclk); > + clk_prepare_enable(oper_cfg.sclk); > + > + return 0; > +} > + > +static const struct dev_pm_ops vpss_pm_ops = { > + .suspend = vpss_suspend, > + .resume = vpss_resume, > +}; Addition of suspend support seems unrelated to this patch. May be make a seperate patch for it and while at it, please use PM runtime instead of direct clock enable/disable. Have a look at the davinci_emac driver which was converted to use PM runtime recently. Let me know how you want to handle this patch. I suppose you intend this should go through my tree because of other dependent platform changes? Thanks, Sekhar ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] media: davinci: vpss: enable vpss clocks 2013-03-25 5:32 ` Sekhar Nori @ 2013-03-25 10:14 ` Prabhakar Lad 2013-03-25 10:24 ` Sekhar Nori 0 siblings, 1 reply; 10+ messages in thread From: Prabhakar Lad @ 2013-03-25 10:14 UTC (permalink / raw) To: linux-arm-kernel Hi Sekhar, Thanks for the review! On Mon, Mar 25, 2013 at 11:02 AM, Sekhar Nori <nsekhar@ti.com> wrote: > On 3/22/2013 1:23 PM, Prabhakar lad wrote: >> From: Lad, Prabhakar <prabhakar.csengg@gmail.com> >> >> By default the VPSS clocks are only enabled in capture driver >> for davinci family which creates duplicates. This >> patch adds support to enable the VPSS clocks in VPSS driver. >> This avoids duplication of code and also adding clock aliases. >> This patch cleanups the VPSS clock enabling in the capture driver, >> and also removes the clock alias in machine file. Along side adds >> a vpss slave clock for DM365 as mentioned by Sekhar >> (https://patchwork.kernel.org/patch/1221261/). >> >> Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com> >> --- >> arch/arm/mach-davinci/dm355.c | 3 - >> arch/arm/mach-davinci/dm365.c | 9 +++- >> arch/arm/mach-davinci/dm644x.c | 5 -- >> drivers/media/platform/davinci/dm355_ccdc.c | 39 +---------------- >> drivers/media/platform/davinci/dm644x_ccdc.c | 44 ------------------- >> drivers/media/platform/davinci/isif.c | 28 ++---------- >> drivers/media/platform/davinci/vpss.c | 60 ++++++++++++++++++++++++++ >> 7 files changed, 72 insertions(+), 116 deletions(-) >> >> static struct clk arm_clk = { >> .name = "arm_clk", >> .parent = &pll2_sysclk2, >> @@ -450,6 +456,7 @@ static struct clk_lookup dm365_clks[] = { >> CLK(NULL, "pll2_sysclk9", &pll2_sysclk9), >> CLK(NULL, "vpss_dac", &vpss_dac_clk), >> CLK(NULL, "vpss_master", &vpss_master_clk), >> + CLK(NULL, "vpss_slave", &vpss_slave_clk), > > These should use device name for look-up instead of relying just on > con_id. So the entry should look like: > > CLK("vpss", "slave", &vpss_slave_clk), > OK >> CLK(NULL, "arm", &arm_clk), >> CLK(NULL, "uart0", &uart0_clk), >> CLK(NULL, "uart1", &uart1_clk), >> @@ -1239,8 +1246,6 @@ static int __init dm365_init_devices(void) >> clk_add_alias(NULL, dev_name(&dm365_mdio_device.dev), >> NULL, &dm365_emac_device.dev); >> >> - /* Add isif clock alias */ >> - clk_add_alias("master", dm365_isif_dev.name, "vpss_master", NULL); >> platform_device_register(&dm365_vpss_device); >> platform_device_register(&dm365_isif_dev); >> platform_device_register(&vpfe_capture_dev); >> diff --git a/arch/arm/mach-davinci/dm644x.c b/arch/arm/mach-davinci/dm644x.c >> index ee0e994..026e7e3 100644 >> --- a/arch/arm/mach-davinci/dm644x.c >> +++ b/arch/arm/mach-davinci/dm644x.c >> @@ -901,11 +901,6 @@ int __init dm644x_init_video(struct vpfe_config *vpfe_cfg, >> dm644x_vpfe_dev.dev.platform_data = vpfe_cfg; >> platform_device_register(&dm644x_ccdc_dev); >> platform_device_register(&dm644x_vpfe_dev); >> - /* Add ccdc clock aliases */ >> - clk_add_alias("master", dm644x_ccdc_dev.name, >> - "vpss_master", NULL); >> - clk_add_alias("slave", dm644x_ccdc_dev.name, >> - "vpss_slave", NULL); >> } >> >> if (vpbe_cfg) { >> diff --git a/drivers/media/platform/davinci/dm355_ccdc.c b/drivers/media/platform/davinci/dm355_ccdc.c >> index 2364dba..05f8fb7 100644 >> --- a/drivers/media/platform/davinci/dm355_ccdc.c >> +++ b/drivers/media/platform/davinci/dm355_ccdc.c >> @@ -37,7 +37,6 @@ >> #include <linux/platform_device.h> >> #include <linux/uaccess.h> >> #include <linux/videodev2.h> >> -#include <linux/clk.h> >> #include <linux/err.h> >> #include <linux/module.h> >> >> @@ -59,10 +58,6 @@ static struct ccdc_oper_config { >> struct ccdc_params_raw bayer; >> /* YCbCr configuration */ >> struct ccdc_params_ycbcr ycbcr; >> - /* Master clock */ >> - struct clk *mclk; >> - /* slave clock */ >> - struct clk *sclk; >> /* ccdc base address */ >> void __iomem *base_addr; >> } ccdc_cfg = { >> @@ -997,32 +992,10 @@ static int dm355_ccdc_probe(struct platform_device *pdev) >> goto fail_nomem; >> } >> >> - /* Get and enable Master clock */ >> - ccdc_cfg.mclk = clk_get(&pdev->dev, "master"); >> - if (IS_ERR(ccdc_cfg.mclk)) { >> - status = PTR_ERR(ccdc_cfg.mclk); >> - goto fail_nomap; >> - } >> - if (clk_prepare_enable(ccdc_cfg.mclk)) { >> - status = -ENODEV; >> - goto fail_mclk; >> - } >> - >> - /* Get and enable Slave clock */ >> - ccdc_cfg.sclk = clk_get(&pdev->dev, "slave"); >> - if (IS_ERR(ccdc_cfg.sclk)) { >> - status = PTR_ERR(ccdc_cfg.sclk); >> - goto fail_mclk; >> - } >> - if (clk_prepare_enable(ccdc_cfg.sclk)) { >> - status = -ENODEV; >> - goto fail_sclk; >> - } >> - >> /* Platform data holds setup_pinmux function ptr */ >> if (NULL == pdev->dev.platform_data) { >> status = -ENODEV; >> - goto fail_sclk; >> + goto fail_nomap; >> } >> setup_pinmux = pdev->dev.platform_data; >> /* >> @@ -1033,12 +1006,6 @@ static int dm355_ccdc_probe(struct platform_device *pdev) >> ccdc_cfg.dev = &pdev->dev; >> printk(KERN_NOTICE "%s is registered with vpfe.\n", ccdc_hw_dev.name); >> return 0; >> -fail_sclk: >> - clk_disable_unprepare(ccdc_cfg.sclk); >> - clk_put(ccdc_cfg.sclk); >> -fail_mclk: >> - clk_disable_unprepare(ccdc_cfg.mclk); >> - clk_put(ccdc_cfg.mclk); >> fail_nomap: >> iounmap(ccdc_cfg.base_addr); >> fail_nomem: >> @@ -1052,10 +1019,6 @@ static int dm355_ccdc_remove(struct platform_device *pdev) >> { >> struct resource *res; >> >> - clk_disable_unprepare(ccdc_cfg.sclk); >> - clk_disable_unprepare(ccdc_cfg.mclk); >> - clk_put(ccdc_cfg.mclk); >> - clk_put(ccdc_cfg.sclk); >> iounmap(ccdc_cfg.base_addr); >> res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> if (res) >> diff --git a/drivers/media/platform/davinci/dm644x_ccdc.c b/drivers/media/platform/davinci/dm644x_ccdc.c >> index 971d639..30fa084 100644 >> --- a/drivers/media/platform/davinci/dm644x_ccdc.c >> +++ b/drivers/media/platform/davinci/dm644x_ccdc.c >> @@ -38,7 +38,6 @@ >> #include <linux/uaccess.h> >> #include <linux/videodev2.h> >> #include <linux/gfp.h> >> -#include <linux/clk.h> >> #include <linux/err.h> >> #include <linux/module.h> >> >> @@ -60,10 +59,6 @@ static struct ccdc_oper_config { >> struct ccdc_params_raw bayer; >> /* YCbCr configuration */ >> struct ccdc_params_ycbcr ycbcr; >> - /* Master clock */ >> - struct clk *mclk; >> - /* slave clock */ >> - struct clk *sclk; >> /* ccdc base address */ >> void __iomem *base_addr; >> } ccdc_cfg = { >> @@ -991,38 +986,9 @@ static int dm644x_ccdc_probe(struct platform_device *pdev) >> goto fail_nomem; >> } >> >> - /* Get and enable Master clock */ >> - ccdc_cfg.mclk = clk_get(&pdev->dev, "master"); >> - if (IS_ERR(ccdc_cfg.mclk)) { >> - status = PTR_ERR(ccdc_cfg.mclk); >> - goto fail_nomap; >> - } >> - if (clk_prepare_enable(ccdc_cfg.mclk)) { >> - status = -ENODEV; >> - goto fail_mclk; >> - } >> - >> - /* Get and enable Slave clock */ >> - ccdc_cfg.sclk = clk_get(&pdev->dev, "slave"); >> - if (IS_ERR(ccdc_cfg.sclk)) { >> - status = PTR_ERR(ccdc_cfg.sclk); >> - goto fail_mclk; >> - } >> - if (clk_prepare_enable(ccdc_cfg.sclk)) { >> - status = -ENODEV; >> - goto fail_sclk; >> - } >> ccdc_cfg.dev = &pdev->dev; >> printk(KERN_NOTICE "%s is registered with vpfe.\n", ccdc_hw_dev.name); >> return 0; >> -fail_sclk: >> - clk_disable_unprepare(ccdc_cfg.sclk); >> - clk_put(ccdc_cfg.sclk); >> -fail_mclk: >> - clk_disable_unprepare(ccdc_cfg.mclk); >> - clk_put(ccdc_cfg.mclk); >> -fail_nomap: >> - iounmap(ccdc_cfg.base_addr); >> fail_nomem: >> release_mem_region(res->start, resource_size(res)); >> fail_nores: >> @@ -1034,10 +1000,6 @@ static int dm644x_ccdc_remove(struct platform_device *pdev) >> { >> struct resource *res; >> >> - clk_disable_unprepare(ccdc_cfg.mclk); >> - clk_disable_unprepare(ccdc_cfg.sclk); >> - clk_put(ccdc_cfg.mclk); >> - clk_put(ccdc_cfg.sclk); >> iounmap(ccdc_cfg.base_addr); >> res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> if (res) >> @@ -1052,18 +1014,12 @@ static int dm644x_ccdc_suspend(struct device *dev) >> ccdc_save_context(); >> /* Disable CCDC */ >> ccdc_enable(0); >> - /* Disable both master and slave clock */ >> - clk_disable_unprepare(ccdc_cfg.mclk); >> - clk_disable_unprepare(ccdc_cfg.sclk); >> >> return 0; >> } >> >> static int dm644x_ccdc_resume(struct device *dev) >> { >> - /* Enable both master and slave clock */ >> - clk_prepare_enable(ccdc_cfg.mclk); >> - clk_prepare_enable(ccdc_cfg.sclk); >> /* Restore CCDC context */ >> ccdc_restore_context(); >> >> diff --git a/drivers/media/platform/davinci/isif.c b/drivers/media/platform/davinci/isif.c >> index abc3ae3..3332cca 100644 >> --- a/drivers/media/platform/davinci/isif.c >> +++ b/drivers/media/platform/davinci/isif.c >> @@ -32,7 +32,6 @@ >> #include <linux/uaccess.h> >> #include <linux/io.h> >> #include <linux/videodev2.h> >> -#include <linux/clk.h> >> #include <linux/err.h> >> #include <linux/module.h> >> >> @@ -88,8 +87,6 @@ static struct isif_oper_config { >> struct isif_ycbcr_config ycbcr; >> struct isif_params_raw bayer; >> enum isif_data_pack data_pack; >> - /* Master clock */ >> - struct clk *mclk; >> /* ISIF base address */ >> void __iomem *base_addr; >> /* ISIF Linear Table 0 */ >> @@ -1039,6 +1036,10 @@ static int isif_probe(struct platform_device *pdev) >> void *__iomem addr; >> int status = 0, i; >> >> + /* Platform data holds setup_pinmux function ptr */ >> + if (!pdev->dev.platform_data) >> + return -ENODEV; >> + > > This change seems unrelated. I suggest moving it to a different patch or > atleast note it in the description. > Its just a movement, while fixing the cleanups. I'll add some description about it. >> /* >> * first try to register with vpfe. If not correct platform, then we >> * don't have to iomap >> @@ -1047,22 +1048,6 @@ static int isif_probe(struct platform_device *pdev) >> if (status < 0) >> return status; >> >> - /* Get and enable Master clock */ >> - isif_cfg.mclk = clk_get(&pdev->dev, "master"); >> - if (IS_ERR(isif_cfg.mclk)) { >> - status = PTR_ERR(isif_cfg.mclk); >> - goto fail_mclk; >> - } >> - if (clk_prepare_enable(isif_cfg.mclk)) { >> - status = -ENODEV; >> - goto fail_mclk; >> - } >> - >> - /* Platform data holds setup_pinmux function ptr */ >> - if (NULL == pdev->dev.platform_data) { >> - status = -ENODEV; >> - goto fail_mclk; >> - } >> setup_pinmux = pdev->dev.platform_data; >> /* >> * setup Mux configuration for ccdc which may be different for >> @@ -1124,9 +1109,6 @@ fail_nobase_res: >> release_mem_region(res->start, resource_size(res)); >> i--; >> } >> -fail_mclk: >> - clk_disable_unprepare(isif_cfg.mclk); >> - clk_put(isif_cfg.mclk); >> vpfe_unregister_ccdc_device(&isif_hw_dev); >> return status; >> } >> @@ -1146,8 +1128,6 @@ static int isif_remove(struct platform_device *pdev) >> i++; >> } >> vpfe_unregister_ccdc_device(&isif_hw_dev); >> - clk_disable_unprepare(isif_cfg.mclk); >> - clk_put(isif_cfg.mclk); >> return 0; >> } >> >> diff --git a/drivers/media/platform/davinci/vpss.c b/drivers/media/platform/davinci/vpss.c >> index a19c552..db69317 100644 >> --- a/drivers/media/platform/davinci/vpss.c >> +++ b/drivers/media/platform/davinci/vpss.c >> @@ -17,6 +17,7 @@ >> * >> * common vpss system module platform driver for all video drivers. >> */ >> +#include <linux/clk.h> >> #include <linux/kernel.h> >> #include <linux/sched.h> >> #include <linux/init.h> >> @@ -126,6 +127,10 @@ struct vpss_oper_config { >> enum vpss_platform_type platform; >> spinlock_t vpss_lock; >> struct vpss_hw_ops hw_ops; >> + /* Master clock */ >> + struct clk *mclk; >> + /* slave clock */ >> + struct clk *sclk; >> }; >> >> static struct vpss_oper_config oper_cfg; >> @@ -429,6 +434,26 @@ static int vpss_probe(struct platform_device *pdev) >> return -ENODEV; >> } >> >> + /* Get and enable Master clock */ >> + oper_cfg.mclk = clk_get(&pdev->dev, "vpss_master"); > > use devm_clk_get() here to simplify the error handling. > OK >> + if (IS_ERR(oper_cfg.mclk)) { >> + status = PTR_ERR(oper_cfg.mclk); >> + goto fail_getclk; >> + } >> + status = clk_prepare_enable(oper_cfg.mclk); >> + if (status) >> + goto fail_mclk; >> + >> + /* Get and enable Slave clock */ >> + oper_cfg.sclk = clk_get(&pdev->dev, "vpss_slave"); >> + if (IS_ERR(oper_cfg.sclk)) { >> + status = PTR_ERR(oper_cfg.sclk); >> + goto fail_mclk; >> + } >> + status = clk_prepare_enable(oper_cfg.sclk); >> + if (status) >> + goto fail_sclk; >> + >> dev_info(&pdev->dev, "%s vpss probed\n", platform_name); >> r1 = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> if (!r1) >> @@ -500,6 +525,13 @@ fail2: >> iounmap(oper_cfg.vpss_regs_base0); >> fail1: >> release_mem_region(r1->start, resource_size(r1)); >> +fail_sclk: >> + clk_disable_unprepare(oper_cfg.sclk); >> + clk_put(oper_cfg.sclk); >> +fail_mclk: >> + clk_disable_unprepare(oper_cfg.mclk); >> + clk_put(oper_cfg.mclk); >> +fail_getclk: >> return status; >> } >> >> @@ -510,6 +542,10 @@ static int vpss_remove(struct platform_device *pdev) >> iounmap(oper_cfg.vpss_regs_base0); >> res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> release_mem_region(res->start, resource_size(res)); >> + clk_disable_unprepare(oper_cfg.mclk); >> + clk_disable_unprepare(oper_cfg.sclk); >> + clk_put(oper_cfg.mclk); >> + clk_put(oper_cfg.sclk); >> if (oper_cfg.platform == DM355 || oper_cfg.platform == DM365) { >> iounmap(oper_cfg.vpss_regs_base1); >> res = platform_get_resource(pdev, IORESOURCE_MEM, 1); >> @@ -518,10 +554,34 @@ static int vpss_remove(struct platform_device *pdev) >> return 0; >> } >> > >> +static int vpss_suspend(struct device *dev) >> +{ >> + /* Disable both master and slave clock */ >> + clk_disable_unprepare(oper_cfg.mclk); >> + clk_disable_unprepare(oper_cfg.sclk); >> + >> + return 0; >> +} >> + >> +static int vpss_resume(struct device *dev) >> +{ >> + /* Enable both master and slave clock */ >> + clk_prepare_enable(oper_cfg.mclk); >> + clk_prepare_enable(oper_cfg.sclk); >> + >> + return 0; >> +} >> + >> +static const struct dev_pm_ops vpss_pm_ops = { >> + .suspend = vpss_suspend, >> + .resume = vpss_resume, >> +}; > > Addition of suspend support seems unrelated to this patch. May be make a > seperate patch for it and while at it, please use PM runtime instead of > direct clock enable/disable. Have a look at the davinci_emac driver > which was converted to use PM runtime recently. > I felt having in same patch would be a good idea, since the clock enabling/disabling where removed from dm644x_ccdc.c for suspend/resume and add it in vpss. If you still feel it needs to be separate patch let me know. > Let me know how you want to handle this patch. I suppose you intend this > should go through my tree because of other dependent platform changes? > I want this series to go through the media tree because of few dependencies (dependency on ths7353 video amplifier driver which recently got merged into media tree) Regards, --Prabhakar > Thanks, > Sekhar ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] media: davinci: vpss: enable vpss clocks 2013-03-25 10:14 ` Prabhakar Lad @ 2013-03-25 10:24 ` Sekhar Nori 2013-03-25 10:33 ` Prabhakar Lad 0 siblings, 1 reply; 10+ messages in thread From: Sekhar Nori @ 2013-03-25 10:24 UTC (permalink / raw) To: linux-arm-kernel On 3/25/2013 3:44 PM, Prabhakar Lad wrote: > Hi Sekhar, > > Thanks for the review! > > On Mon, Mar 25, 2013 at 11:02 AM, Sekhar Nori <nsekhar@ti.com> wrote: >> On 3/22/2013 1:23 PM, Prabhakar lad wrote: >>> From: Lad, Prabhakar <prabhakar.csengg@gmail.com> >>> >>> By default the VPSS clocks are only enabled in capture driver >>> for davinci family which creates duplicates. This >>> patch adds support to enable the VPSS clocks in VPSS driver. >>> This avoids duplication of code and also adding clock aliases. >>> This patch cleanups the VPSS clock enabling in the capture driver, >>> and also removes the clock alias in machine file. Along side adds >>> a vpss slave clock for DM365 as mentioned by Sekhar >>> (https://patchwork.kernel.org/patch/1221261/). >>> >>> Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com> >>> --- >>> arch/arm/mach-davinci/dm355.c | 3 - >>> arch/arm/mach-davinci/dm365.c | 9 +++- >>> arch/arm/mach-davinci/dm644x.c | 5 -- >>> drivers/media/platform/davinci/dm355_ccdc.c | 39 +---------------- >>> drivers/media/platform/davinci/dm644x_ccdc.c | 44 ------------------- >>> drivers/media/platform/davinci/isif.c | 28 ++---------- >>> drivers/media/platform/davinci/vpss.c | 60 ++++++++++++++++++++++++++ >>> 7 files changed, 72 insertions(+), 116 deletions(-) >>> >>> static struct clk arm_clk = { >>> .name = "arm_clk", >>> .parent = &pll2_sysclk2, >>> @@ -450,6 +456,7 @@ static struct clk_lookup dm365_clks[] = { >>> CLK(NULL, "pll2_sysclk9", &pll2_sysclk9), >>> CLK(NULL, "vpss_dac", &vpss_dac_clk), >>> CLK(NULL, "vpss_master", &vpss_master_clk), >>> + CLK(NULL, "vpss_slave", &vpss_slave_clk), >> >> These should use device name for look-up instead of relying just on >> con_id. So the entry should look like: >> >> CLK("vpss", "slave", &vpss_slave_clk), >> > OK > >>> CLK(NULL, "arm", &arm_clk), >>> CLK(NULL, "uart0", &uart0_clk), >>> CLK(NULL, "uart1", &uart1_clk), >>> @@ -1239,8 +1246,6 @@ static int __init dm365_init_devices(void) >>> clk_add_alias(NULL, dev_name(&dm365_mdio_device.dev), >>> NULL, &dm365_emac_device.dev); >>> >>> - /* Add isif clock alias */ >>> - clk_add_alias("master", dm365_isif_dev.name, "vpss_master", NULL); >>> platform_device_register(&dm365_vpss_device); >>> platform_device_register(&dm365_isif_dev); >>> platform_device_register(&vpfe_capture_dev); >>> diff --git a/arch/arm/mach-davinci/dm644x.c b/arch/arm/mach-davinci/dm644x.c >>> index ee0e994..026e7e3 100644 >>> --- a/arch/arm/mach-davinci/dm644x.c >>> +++ b/arch/arm/mach-davinci/dm644x.c >>> @@ -901,11 +901,6 @@ int __init dm644x_init_video(struct vpfe_config *vpfe_cfg, >>> dm644x_vpfe_dev.dev.platform_data = vpfe_cfg; >>> platform_device_register(&dm644x_ccdc_dev); >>> platform_device_register(&dm644x_vpfe_dev); >>> - /* Add ccdc clock aliases */ >>> - clk_add_alias("master", dm644x_ccdc_dev.name, >>> - "vpss_master", NULL); >>> - clk_add_alias("slave", dm644x_ccdc_dev.name, >>> - "vpss_slave", NULL); >>> } >>> >>> if (vpbe_cfg) { >>> diff --git a/drivers/media/platform/davinci/dm355_ccdc.c b/drivers/media/platform/davinci/dm355_ccdc.c >>> index 2364dba..05f8fb7 100644 >>> --- a/drivers/media/platform/davinci/dm355_ccdc.c >>> +++ b/drivers/media/platform/davinci/dm355_ccdc.c >>> @@ -37,7 +37,6 @@ >>> #include <linux/platform_device.h> >>> #include <linux/uaccess.h> >>> #include <linux/videodev2.h> >>> -#include <linux/clk.h> >>> #include <linux/err.h> >>> #include <linux/module.h> >>> >>> @@ -59,10 +58,6 @@ static struct ccdc_oper_config { >>> struct ccdc_params_raw bayer; >>> /* YCbCr configuration */ >>> struct ccdc_params_ycbcr ycbcr; >>> - /* Master clock */ >>> - struct clk *mclk; >>> - /* slave clock */ >>> - struct clk *sclk; >>> /* ccdc base address */ >>> void __iomem *base_addr; >>> } ccdc_cfg = { >>> @@ -997,32 +992,10 @@ static int dm355_ccdc_probe(struct platform_device *pdev) >>> goto fail_nomem; >>> } >>> >>> - /* Get and enable Master clock */ >>> - ccdc_cfg.mclk = clk_get(&pdev->dev, "master"); >>> - if (IS_ERR(ccdc_cfg.mclk)) { >>> - status = PTR_ERR(ccdc_cfg.mclk); >>> - goto fail_nomap; >>> - } >>> - if (clk_prepare_enable(ccdc_cfg.mclk)) { >>> - status = -ENODEV; >>> - goto fail_mclk; >>> - } >>> - >>> - /* Get and enable Slave clock */ >>> - ccdc_cfg.sclk = clk_get(&pdev->dev, "slave"); >>> - if (IS_ERR(ccdc_cfg.sclk)) { >>> - status = PTR_ERR(ccdc_cfg.sclk); >>> - goto fail_mclk; >>> - } >>> - if (clk_prepare_enable(ccdc_cfg.sclk)) { >>> - status = -ENODEV; >>> - goto fail_sclk; >>> - } >>> - >>> /* Platform data holds setup_pinmux function ptr */ >>> if (NULL == pdev->dev.platform_data) { >>> status = -ENODEV; >>> - goto fail_sclk; >>> + goto fail_nomap; >>> } >>> setup_pinmux = pdev->dev.platform_data; >>> /* >>> @@ -1033,12 +1006,6 @@ static int dm355_ccdc_probe(struct platform_device *pdev) >>> ccdc_cfg.dev = &pdev->dev; >>> printk(KERN_NOTICE "%s is registered with vpfe.\n", ccdc_hw_dev.name); >>> return 0; >>> -fail_sclk: >>> - clk_disable_unprepare(ccdc_cfg.sclk); >>> - clk_put(ccdc_cfg.sclk); >>> -fail_mclk: >>> - clk_disable_unprepare(ccdc_cfg.mclk); >>> - clk_put(ccdc_cfg.mclk); >>> fail_nomap: >>> iounmap(ccdc_cfg.base_addr); >>> fail_nomem: >>> @@ -1052,10 +1019,6 @@ static int dm355_ccdc_remove(struct platform_device *pdev) >>> { >>> struct resource *res; >>> >>> - clk_disable_unprepare(ccdc_cfg.sclk); >>> - clk_disable_unprepare(ccdc_cfg.mclk); >>> - clk_put(ccdc_cfg.mclk); >>> - clk_put(ccdc_cfg.sclk); >>> iounmap(ccdc_cfg.base_addr); >>> res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >>> if (res) >>> diff --git a/drivers/media/platform/davinci/dm644x_ccdc.c b/drivers/media/platform/davinci/dm644x_ccdc.c >>> index 971d639..30fa084 100644 >>> --- a/drivers/media/platform/davinci/dm644x_ccdc.c >>> +++ b/drivers/media/platform/davinci/dm644x_ccdc.c >>> @@ -38,7 +38,6 @@ >>> #include <linux/uaccess.h> >>> #include <linux/videodev2.h> >>> #include <linux/gfp.h> >>> -#include <linux/clk.h> >>> #include <linux/err.h> >>> #include <linux/module.h> >>> >>> @@ -60,10 +59,6 @@ static struct ccdc_oper_config { >>> struct ccdc_params_raw bayer; >>> /* YCbCr configuration */ >>> struct ccdc_params_ycbcr ycbcr; >>> - /* Master clock */ >>> - struct clk *mclk; >>> - /* slave clock */ >>> - struct clk *sclk; >>> /* ccdc base address */ >>> void __iomem *base_addr; >>> } ccdc_cfg = { >>> @@ -991,38 +986,9 @@ static int dm644x_ccdc_probe(struct platform_device *pdev) >>> goto fail_nomem; >>> } >>> >>> - /* Get and enable Master clock */ >>> - ccdc_cfg.mclk = clk_get(&pdev->dev, "master"); >>> - if (IS_ERR(ccdc_cfg.mclk)) { >>> - status = PTR_ERR(ccdc_cfg.mclk); >>> - goto fail_nomap; >>> - } >>> - if (clk_prepare_enable(ccdc_cfg.mclk)) { >>> - status = -ENODEV; >>> - goto fail_mclk; >>> - } >>> - >>> - /* Get and enable Slave clock */ >>> - ccdc_cfg.sclk = clk_get(&pdev->dev, "slave"); >>> - if (IS_ERR(ccdc_cfg.sclk)) { >>> - status = PTR_ERR(ccdc_cfg.sclk); >>> - goto fail_mclk; >>> - } >>> - if (clk_prepare_enable(ccdc_cfg.sclk)) { >>> - status = -ENODEV; >>> - goto fail_sclk; >>> - } >>> ccdc_cfg.dev = &pdev->dev; >>> printk(KERN_NOTICE "%s is registered with vpfe.\n", ccdc_hw_dev.name); >>> return 0; >>> -fail_sclk: >>> - clk_disable_unprepare(ccdc_cfg.sclk); >>> - clk_put(ccdc_cfg.sclk); >>> -fail_mclk: >>> - clk_disable_unprepare(ccdc_cfg.mclk); >>> - clk_put(ccdc_cfg.mclk); >>> -fail_nomap: >>> - iounmap(ccdc_cfg.base_addr); >>> fail_nomem: >>> release_mem_region(res->start, resource_size(res)); >>> fail_nores: >>> @@ -1034,10 +1000,6 @@ static int dm644x_ccdc_remove(struct platform_device *pdev) >>> { >>> struct resource *res; >>> >>> - clk_disable_unprepare(ccdc_cfg.mclk); >>> - clk_disable_unprepare(ccdc_cfg.sclk); >>> - clk_put(ccdc_cfg.mclk); >>> - clk_put(ccdc_cfg.sclk); >>> iounmap(ccdc_cfg.base_addr); >>> res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >>> if (res) >>> @@ -1052,18 +1014,12 @@ static int dm644x_ccdc_suspend(struct device *dev) >>> ccdc_save_context(); >>> /* Disable CCDC */ >>> ccdc_enable(0); >>> - /* Disable both master and slave clock */ >>> - clk_disable_unprepare(ccdc_cfg.mclk); >>> - clk_disable_unprepare(ccdc_cfg.sclk); >>> >>> return 0; >>> } >>> >>> static int dm644x_ccdc_resume(struct device *dev) >>> { >>> - /* Enable both master and slave clock */ >>> - clk_prepare_enable(ccdc_cfg.mclk); >>> - clk_prepare_enable(ccdc_cfg.sclk); >>> /* Restore CCDC context */ >>> ccdc_restore_context(); >>> >>> diff --git a/drivers/media/platform/davinci/isif.c b/drivers/media/platform/davinci/isif.c >>> index abc3ae3..3332cca 100644 >>> --- a/drivers/media/platform/davinci/isif.c >>> +++ b/drivers/media/platform/davinci/isif.c >>> @@ -32,7 +32,6 @@ >>> #include <linux/uaccess.h> >>> #include <linux/io.h> >>> #include <linux/videodev2.h> >>> -#include <linux/clk.h> >>> #include <linux/err.h> >>> #include <linux/module.h> >>> >>> @@ -88,8 +87,6 @@ static struct isif_oper_config { >>> struct isif_ycbcr_config ycbcr; >>> struct isif_params_raw bayer; >>> enum isif_data_pack data_pack; >>> - /* Master clock */ >>> - struct clk *mclk; >>> /* ISIF base address */ >>> void __iomem *base_addr; >>> /* ISIF Linear Table 0 */ >>> @@ -1039,6 +1036,10 @@ static int isif_probe(struct platform_device *pdev) >>> void *__iomem addr; >>> int status = 0, i; >>> >>> + /* Platform data holds setup_pinmux function ptr */ >>> + if (!pdev->dev.platform_data) >>> + return -ENODEV; >>> + >> >> This change seems unrelated. I suggest moving it to a different patch or >> atleast note it in the description. >> > Its just a movement, while fixing the cleanups. I'll add some > description about it. This is fine as is. I failed to notice the movement. > >>> /* >>> * first try to register with vpfe. If not correct platform, then we >>> * don't have to iomap >>> @@ -1047,22 +1048,6 @@ static int isif_probe(struct platform_device *pdev) >>> if (status < 0) >>> return status; >>> >>> - /* Get and enable Master clock */ >>> - isif_cfg.mclk = clk_get(&pdev->dev, "master"); >>> - if (IS_ERR(isif_cfg.mclk)) { >>> - status = PTR_ERR(isif_cfg.mclk); >>> - goto fail_mclk; >>> - } >>> - if (clk_prepare_enable(isif_cfg.mclk)) { >>> - status = -ENODEV; >>> - goto fail_mclk; >>> - } >>> - >>> - /* Platform data holds setup_pinmux function ptr */ >>> - if (NULL == pdev->dev.platform_data) { >>> - status = -ENODEV; >>> - goto fail_mclk; >>> - } >>> setup_pinmux = pdev->dev.platform_data; >>> /* >>> * setup Mux configuration for ccdc which may be different for >>> @@ -1124,9 +1109,6 @@ fail_nobase_res: >>> release_mem_region(res->start, resource_size(res)); >>> i--; >>> } >>> -fail_mclk: >>> - clk_disable_unprepare(isif_cfg.mclk); >>> - clk_put(isif_cfg.mclk); >>> vpfe_unregister_ccdc_device(&isif_hw_dev); >>> return status; >>> } >>> @@ -1146,8 +1128,6 @@ static int isif_remove(struct platform_device *pdev) >>> i++; >>> } >>> vpfe_unregister_ccdc_device(&isif_hw_dev); >>> - clk_disable_unprepare(isif_cfg.mclk); >>> - clk_put(isif_cfg.mclk); >>> return 0; >>> } >>> >>> diff --git a/drivers/media/platform/davinci/vpss.c b/drivers/media/platform/davinci/vpss.c >>> index a19c552..db69317 100644 >>> --- a/drivers/media/platform/davinci/vpss.c >>> +++ b/drivers/media/platform/davinci/vpss.c >>> @@ -17,6 +17,7 @@ >>> * >>> * common vpss system module platform driver for all video drivers. >>> */ >>> +#include <linux/clk.h> >>> #include <linux/kernel.h> >>> #include <linux/sched.h> >>> #include <linux/init.h> >>> @@ -126,6 +127,10 @@ struct vpss_oper_config { >>> enum vpss_platform_type platform; >>> spinlock_t vpss_lock; >>> struct vpss_hw_ops hw_ops; >>> + /* Master clock */ >>> + struct clk *mclk; >>> + /* slave clock */ >>> + struct clk *sclk; >>> }; >>> >>> static struct vpss_oper_config oper_cfg; >>> @@ -429,6 +434,26 @@ static int vpss_probe(struct platform_device *pdev) >>> return -ENODEV; >>> } >>> >>> + /* Get and enable Master clock */ >>> + oper_cfg.mclk = clk_get(&pdev->dev, "vpss_master"); >> >> use devm_clk_get() here to simplify the error handling. >> > OK > >>> + if (IS_ERR(oper_cfg.mclk)) { >>> + status = PTR_ERR(oper_cfg.mclk); >>> + goto fail_getclk; >>> + } >>> + status = clk_prepare_enable(oper_cfg.mclk); >>> + if (status) >>> + goto fail_mclk; >>> + >>> + /* Get and enable Slave clock */ >>> + oper_cfg.sclk = clk_get(&pdev->dev, "vpss_slave"); >>> + if (IS_ERR(oper_cfg.sclk)) { >>> + status = PTR_ERR(oper_cfg.sclk); >>> + goto fail_mclk; >>> + } >>> + status = clk_prepare_enable(oper_cfg.sclk); >>> + if (status) >>> + goto fail_sclk; >>> + >>> dev_info(&pdev->dev, "%s vpss probed\n", platform_name); >>> r1 = platform_get_resource(pdev, IORESOURCE_MEM, 0); >>> if (!r1) >>> @@ -500,6 +525,13 @@ fail2: >>> iounmap(oper_cfg.vpss_regs_base0); >>> fail1: >>> release_mem_region(r1->start, resource_size(r1)); >>> +fail_sclk: >>> + clk_disable_unprepare(oper_cfg.sclk); >>> + clk_put(oper_cfg.sclk); >>> +fail_mclk: >>> + clk_disable_unprepare(oper_cfg.mclk); >>> + clk_put(oper_cfg.mclk); >>> +fail_getclk: >>> return status; >>> } >>> >>> @@ -510,6 +542,10 @@ static int vpss_remove(struct platform_device *pdev) >>> iounmap(oper_cfg.vpss_regs_base0); >>> res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >>> release_mem_region(res->start, resource_size(res)); >>> + clk_disable_unprepare(oper_cfg.mclk); >>> + clk_disable_unprepare(oper_cfg.sclk); >>> + clk_put(oper_cfg.mclk); >>> + clk_put(oper_cfg.sclk); >>> if (oper_cfg.platform == DM355 || oper_cfg.platform == DM365) { >>> iounmap(oper_cfg.vpss_regs_base1); >>> res = platform_get_resource(pdev, IORESOURCE_MEM, 1); >>> @@ -518,10 +554,34 @@ static int vpss_remove(struct platform_device *pdev) >>> return 0; >>> } >>> >> >>> +static int vpss_suspend(struct device *dev) >>> +{ >>> + /* Disable both master and slave clock */ >>> + clk_disable_unprepare(oper_cfg.mclk); >>> + clk_disable_unprepare(oper_cfg.sclk); >>> + >>> + return 0; >>> +} >>> + >>> +static int vpss_resume(struct device *dev) >>> +{ >>> + /* Enable both master and slave clock */ >>> + clk_prepare_enable(oper_cfg.mclk); >>> + clk_prepare_enable(oper_cfg.sclk); >>> + >>> + return 0; >>> +} >>> + >>> +static const struct dev_pm_ops vpss_pm_ops = { >>> + .suspend = vpss_suspend, >>> + .resume = vpss_resume, >>> +}; >> >> Addition of suspend support seems unrelated to this patch. May be make a >> seperate patch for it and while at it, please use PM runtime instead of >> direct clock enable/disable. Have a look at the davinci_emac driver >> which was converted to use PM runtime recently. >> > I felt having in same patch would be a good idea, since the clock > enabling/disabling > where removed from dm644x_ccdc.c for suspend/resume and add it in > vpss. If you still > feel it needs to be separate patch let me know. Okay, please add some description for this. Its not terribly obvious. > >> Let me know how you want to handle this patch. I suppose you intend this >> should go through my tree because of other dependent platform changes? >> > I want this series to go through the media tree because of few dependencies > (dependency on ths7353 video amplifier driver which recently got > merged into media tree) Hmm, this patch does not touch ths7353 so are you sure there is a merge dependency? There are a lot of platform changes you are doing in some of the recent patches you submitted based on this patch and ideally those go through my tree to ease merging for upstream. That said, if there is a good reason for this and the other platform changes to go through media tree, I am okay with that too. Thanks, Sekhar ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] media: davinci: vpss: enable vpss clocks 2013-03-25 10:24 ` Sekhar Nori @ 2013-03-25 10:33 ` Prabhakar Lad 0 siblings, 0 replies; 10+ messages in thread From: Prabhakar Lad @ 2013-03-25 10:33 UTC (permalink / raw) To: linux-arm-kernel Sekhar, On Mon, Mar 25, 2013 at 3:54 PM, Sekhar Nori <nsekhar@ti.com> wrote: > On 3/25/2013 3:44 PM, Prabhakar Lad wrote: >> Hi Sekhar, >> >> Thanks for the review! >> >> On Mon, Mar 25, 2013 at 11:02 AM, Sekhar Nori <nsekhar@ti.com> wrote: >>> On 3/22/2013 1:23 PM, Prabhakar lad wrote: >>>> From: Lad, Prabhakar <prabhakar.csengg@gmail.com> >>>> >>>> By default the VPSS clocks are only enabled in capture driver >>>> for davinci family which creates duplicates. This >>>> patch adds support to enable the VPSS clocks in VPSS driver. >>>> This avoids duplication of code and also adding clock aliases. >>>> This patch cleanups the VPSS clock enabling in the capture driver, >>>> and also removes the clock alias in machine file. Along side adds >>>> a vpss slave clock for DM365 as mentioned by Sekhar >>>> (https://patchwork.kernel.org/patch/1221261/). >>>> >>>> Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com> >>>> --- >>>> arch/arm/mach-davinci/dm355.c | 3 - >>>> arch/arm/mach-davinci/dm365.c | 9 +++- >>>> arch/arm/mach-davinci/dm644x.c | 5 -- >>>> drivers/media/platform/davinci/dm355_ccdc.c | 39 +---------------- >>>> drivers/media/platform/davinci/dm644x_ccdc.c | 44 ------------------- >>>> drivers/media/platform/davinci/isif.c | 28 ++---------- >>>> drivers/media/platform/davinci/vpss.c | 60 ++++++++++++++++++++++++++ >>>> 7 files changed, 72 insertions(+), 116 deletions(-) >>>> >>>> static struct clk arm_clk = { >>>> .name = "arm_clk", >>>> .parent = &pll2_sysclk2, >>>> @@ -450,6 +456,7 @@ static struct clk_lookup dm365_clks[] = { >>>> CLK(NULL, "pll2_sysclk9", &pll2_sysclk9), >>>> CLK(NULL, "vpss_dac", &vpss_dac_clk), >>>> CLK(NULL, "vpss_master", &vpss_master_clk), >>>> + CLK(NULL, "vpss_slave", &vpss_slave_clk), >>> >>> These should use device name for look-up instead of relying just on >>> con_id. So the entry should look like: >>> >>> CLK("vpss", "slave", &vpss_slave_clk), >>> >> OK >> >>>> CLK(NULL, "arm", &arm_clk), >>>> CLK(NULL, "uart0", &uart0_clk), >>>> CLK(NULL, "uart1", &uart1_clk), >>>> @@ -1239,8 +1246,6 @@ static int __init dm365_init_devices(void) >>>> clk_add_alias(NULL, dev_name(&dm365_mdio_device.dev), >>>> NULL, &dm365_emac_device.dev); >>>> >>>> - /* Add isif clock alias */ >>>> - clk_add_alias("master", dm365_isif_dev.name, "vpss_master", NULL); >>>> platform_device_register(&dm365_vpss_device); >>>> platform_device_register(&dm365_isif_dev); >>>> platform_device_register(&vpfe_capture_dev); >>>> diff --git a/arch/arm/mach-davinci/dm644x.c b/arch/arm/mach-davinci/dm644x.c >>>> index ee0e994..026e7e3 100644 >>>> --- a/arch/arm/mach-davinci/dm644x.c >>>> +++ b/arch/arm/mach-davinci/dm644x.c >>>> @@ -901,11 +901,6 @@ int __init dm644x_init_video(struct vpfe_config *vpfe_cfg, >>>> dm644x_vpfe_dev.dev.platform_data = vpfe_cfg; >>>> platform_device_register(&dm644x_ccdc_dev); >>>> platform_device_register(&dm644x_vpfe_dev); >>>> - /* Add ccdc clock aliases */ >>>> - clk_add_alias("master", dm644x_ccdc_dev.name, >>>> - "vpss_master", NULL); >>>> - clk_add_alias("slave", dm644x_ccdc_dev.name, >>>> - "vpss_slave", NULL); >>>> } >>>> >>>> if (vpbe_cfg) { >>>> diff --git a/drivers/media/platform/davinci/dm355_ccdc.c b/drivers/media/platform/davinci/dm355_ccdc.c >>>> index 2364dba..05f8fb7 100644 >>>> --- a/drivers/media/platform/davinci/dm355_ccdc.c >>>> +++ b/drivers/media/platform/davinci/dm355_ccdc.c >>>> @@ -37,7 +37,6 @@ >>>> #include <linux/platform_device.h> >>>> #include <linux/uaccess.h> >>>> #include <linux/videodev2.h> >>>> -#include <linux/clk.h> >>>> #include <linux/err.h> >>>> #include <linux/module.h> >>>> >>>> @@ -59,10 +58,6 @@ static struct ccdc_oper_config { >>>> struct ccdc_params_raw bayer; >>>> /* YCbCr configuration */ >>>> struct ccdc_params_ycbcr ycbcr; >>>> - /* Master clock */ >>>> - struct clk *mclk; >>>> - /* slave clock */ >>>> - struct clk *sclk; >>>> /* ccdc base address */ >>>> void __iomem *base_addr; >>>> } ccdc_cfg = { >>>> @@ -997,32 +992,10 @@ static int dm355_ccdc_probe(struct platform_device *pdev) >>>> goto fail_nomem; >>>> } >>>> >>>> - /* Get and enable Master clock */ >>>> - ccdc_cfg.mclk = clk_get(&pdev->dev, "master"); >>>> - if (IS_ERR(ccdc_cfg.mclk)) { >>>> - status = PTR_ERR(ccdc_cfg.mclk); >>>> - goto fail_nomap; >>>> - } >>>> - if (clk_prepare_enable(ccdc_cfg.mclk)) { >>>> - status = -ENODEV; >>>> - goto fail_mclk; >>>> - } >>>> - >>>> - /* Get and enable Slave clock */ >>>> - ccdc_cfg.sclk = clk_get(&pdev->dev, "slave"); >>>> - if (IS_ERR(ccdc_cfg.sclk)) { >>>> - status = PTR_ERR(ccdc_cfg.sclk); >>>> - goto fail_mclk; >>>> - } >>>> - if (clk_prepare_enable(ccdc_cfg.sclk)) { >>>> - status = -ENODEV; >>>> - goto fail_sclk; >>>> - } >>>> - >>>> /* Platform data holds setup_pinmux function ptr */ >>>> if (NULL == pdev->dev.platform_data) { >>>> status = -ENODEV; >>>> - goto fail_sclk; >>>> + goto fail_nomap; >>>> } >>>> setup_pinmux = pdev->dev.platform_data; >>>> /* >>>> @@ -1033,12 +1006,6 @@ static int dm355_ccdc_probe(struct platform_device *pdev) >>>> ccdc_cfg.dev = &pdev->dev; >>>> printk(KERN_NOTICE "%s is registered with vpfe.\n", ccdc_hw_dev.name); >>>> return 0; >>>> -fail_sclk: >>>> - clk_disable_unprepare(ccdc_cfg.sclk); >>>> - clk_put(ccdc_cfg.sclk); >>>> -fail_mclk: >>>> - clk_disable_unprepare(ccdc_cfg.mclk); >>>> - clk_put(ccdc_cfg.mclk); >>>> fail_nomap: >>>> iounmap(ccdc_cfg.base_addr); >>>> fail_nomem: >>>> @@ -1052,10 +1019,6 @@ static int dm355_ccdc_remove(struct platform_device *pdev) >>>> { >>>> struct resource *res; >>>> >>>> - clk_disable_unprepare(ccdc_cfg.sclk); >>>> - clk_disable_unprepare(ccdc_cfg.mclk); >>>> - clk_put(ccdc_cfg.mclk); >>>> - clk_put(ccdc_cfg.sclk); >>>> iounmap(ccdc_cfg.base_addr); >>>> res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >>>> if (res) >>>> diff --git a/drivers/media/platform/davinci/dm644x_ccdc.c b/drivers/media/platform/davinci/dm644x_ccdc.c >>>> index 971d639..30fa084 100644 >>>> --- a/drivers/media/platform/davinci/dm644x_ccdc.c >>>> +++ b/drivers/media/platform/davinci/dm644x_ccdc.c >>>> @@ -38,7 +38,6 @@ >>>> #include <linux/uaccess.h> >>>> #include <linux/videodev2.h> >>>> #include <linux/gfp.h> >>>> -#include <linux/clk.h> >>>> #include <linux/err.h> >>>> #include <linux/module.h> >>>> >>>> @@ -60,10 +59,6 @@ static struct ccdc_oper_config { >>>> struct ccdc_params_raw bayer; >>>> /* YCbCr configuration */ >>>> struct ccdc_params_ycbcr ycbcr; >>>> - /* Master clock */ >>>> - struct clk *mclk; >>>> - /* slave clock */ >>>> - struct clk *sclk; >>>> /* ccdc base address */ >>>> void __iomem *base_addr; >>>> } ccdc_cfg = { >>>> @@ -991,38 +986,9 @@ static int dm644x_ccdc_probe(struct platform_device *pdev) >>>> goto fail_nomem; >>>> } >>>> >>>> - /* Get and enable Master clock */ >>>> - ccdc_cfg.mclk = clk_get(&pdev->dev, "master"); >>>> - if (IS_ERR(ccdc_cfg.mclk)) { >>>> - status = PTR_ERR(ccdc_cfg.mclk); >>>> - goto fail_nomap; >>>> - } >>>> - if (clk_prepare_enable(ccdc_cfg.mclk)) { >>>> - status = -ENODEV; >>>> - goto fail_mclk; >>>> - } >>>> - >>>> - /* Get and enable Slave clock */ >>>> - ccdc_cfg.sclk = clk_get(&pdev->dev, "slave"); >>>> - if (IS_ERR(ccdc_cfg.sclk)) { >>>> - status = PTR_ERR(ccdc_cfg.sclk); >>>> - goto fail_mclk; >>>> - } >>>> - if (clk_prepare_enable(ccdc_cfg.sclk)) { >>>> - status = -ENODEV; >>>> - goto fail_sclk; >>>> - } >>>> ccdc_cfg.dev = &pdev->dev; >>>> printk(KERN_NOTICE "%s is registered with vpfe.\n", ccdc_hw_dev.name); >>>> return 0; >>>> -fail_sclk: >>>> - clk_disable_unprepare(ccdc_cfg.sclk); >>>> - clk_put(ccdc_cfg.sclk); >>>> -fail_mclk: >>>> - clk_disable_unprepare(ccdc_cfg.mclk); >>>> - clk_put(ccdc_cfg.mclk); >>>> -fail_nomap: >>>> - iounmap(ccdc_cfg.base_addr); >>>> fail_nomem: >>>> release_mem_region(res->start, resource_size(res)); >>>> fail_nores: >>>> @@ -1034,10 +1000,6 @@ static int dm644x_ccdc_remove(struct platform_device *pdev) >>>> { >>>> struct resource *res; >>>> >>>> - clk_disable_unprepare(ccdc_cfg.mclk); >>>> - clk_disable_unprepare(ccdc_cfg.sclk); >>>> - clk_put(ccdc_cfg.mclk); >>>> - clk_put(ccdc_cfg.sclk); >>>> iounmap(ccdc_cfg.base_addr); >>>> res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >>>> if (res) >>>> @@ -1052,18 +1014,12 @@ static int dm644x_ccdc_suspend(struct device *dev) >>>> ccdc_save_context(); >>>> /* Disable CCDC */ >>>> ccdc_enable(0); >>>> - /* Disable both master and slave clock */ >>>> - clk_disable_unprepare(ccdc_cfg.mclk); >>>> - clk_disable_unprepare(ccdc_cfg.sclk); >>>> >>>> return 0; >>>> } >>>> >>>> static int dm644x_ccdc_resume(struct device *dev) >>>> { >>>> - /* Enable both master and slave clock */ >>>> - clk_prepare_enable(ccdc_cfg.mclk); >>>> - clk_prepare_enable(ccdc_cfg.sclk); >>>> /* Restore CCDC context */ >>>> ccdc_restore_context(); >>>> >>>> diff --git a/drivers/media/platform/davinci/isif.c b/drivers/media/platform/davinci/isif.c >>>> index abc3ae3..3332cca 100644 >>>> --- a/drivers/media/platform/davinci/isif.c >>>> +++ b/drivers/media/platform/davinci/isif.c >>>> @@ -32,7 +32,6 @@ >>>> #include <linux/uaccess.h> >>>> #include <linux/io.h> >>>> #include <linux/videodev2.h> >>>> -#include <linux/clk.h> >>>> #include <linux/err.h> >>>> #include <linux/module.h> >>>> >>>> @@ -88,8 +87,6 @@ static struct isif_oper_config { >>>> struct isif_ycbcr_config ycbcr; >>>> struct isif_params_raw bayer; >>>> enum isif_data_pack data_pack; >>>> - /* Master clock */ >>>> - struct clk *mclk; >>>> /* ISIF base address */ >>>> void __iomem *base_addr; >>>> /* ISIF Linear Table 0 */ >>>> @@ -1039,6 +1036,10 @@ static int isif_probe(struct platform_device *pdev) >>>> void *__iomem addr; >>>> int status = 0, i; >>>> >>>> + /* Platform data holds setup_pinmux function ptr */ >>>> + if (!pdev->dev.platform_data) >>>> + return -ENODEV; >>>> + >>> >>> This change seems unrelated. I suggest moving it to a different patch or >>> atleast note it in the description. >>> >> Its just a movement, while fixing the cleanups. I'll add some >> description about it. > > This is fine as is. I failed to notice the movement. > >> >>>> /* >>>> * first try to register with vpfe. If not correct platform, then we >>>> * don't have to iomap >>>> @@ -1047,22 +1048,6 @@ static int isif_probe(struct platform_device *pdev) >>>> if (status < 0) >>>> return status; >>>> >>>> - /* Get and enable Master clock */ >>>> - isif_cfg.mclk = clk_get(&pdev->dev, "master"); >>>> - if (IS_ERR(isif_cfg.mclk)) { >>>> - status = PTR_ERR(isif_cfg.mclk); >>>> - goto fail_mclk; >>>> - } >>>> - if (clk_prepare_enable(isif_cfg.mclk)) { >>>> - status = -ENODEV; >>>> - goto fail_mclk; >>>> - } >>>> - >>>> - /* Platform data holds setup_pinmux function ptr */ >>>> - if (NULL == pdev->dev.platform_data) { >>>> - status = -ENODEV; >>>> - goto fail_mclk; >>>> - } >>>> setup_pinmux = pdev->dev.platform_data; >>>> /* >>>> * setup Mux configuration for ccdc which may be different for >>>> @@ -1124,9 +1109,6 @@ fail_nobase_res: >>>> release_mem_region(res->start, resource_size(res)); >>>> i--; >>>> } >>>> -fail_mclk: >>>> - clk_disable_unprepare(isif_cfg.mclk); >>>> - clk_put(isif_cfg.mclk); >>>> vpfe_unregister_ccdc_device(&isif_hw_dev); >>>> return status; >>>> } >>>> @@ -1146,8 +1128,6 @@ static int isif_remove(struct platform_device *pdev) >>>> i++; >>>> } >>>> vpfe_unregister_ccdc_device(&isif_hw_dev); >>>> - clk_disable_unprepare(isif_cfg.mclk); >>>> - clk_put(isif_cfg.mclk); >>>> return 0; >>>> } >>>> >>>> diff --git a/drivers/media/platform/davinci/vpss.c b/drivers/media/platform/davinci/vpss.c >>>> index a19c552..db69317 100644 >>>> --- a/drivers/media/platform/davinci/vpss.c >>>> +++ b/drivers/media/platform/davinci/vpss.c >>>> @@ -17,6 +17,7 @@ >>>> * >>>> * common vpss system module platform driver for all video drivers. >>>> */ >>>> +#include <linux/clk.h> >>>> #include <linux/kernel.h> >>>> #include <linux/sched.h> >>>> #include <linux/init.h> >>>> @@ -126,6 +127,10 @@ struct vpss_oper_config { >>>> enum vpss_platform_type platform; >>>> spinlock_t vpss_lock; >>>> struct vpss_hw_ops hw_ops; >>>> + /* Master clock */ >>>> + struct clk *mclk; >>>> + /* slave clock */ >>>> + struct clk *sclk; >>>> }; >>>> >>>> static struct vpss_oper_config oper_cfg; >>>> @@ -429,6 +434,26 @@ static int vpss_probe(struct platform_device *pdev) >>>> return -ENODEV; >>>> } >>>> >>>> + /* Get and enable Master clock */ >>>> + oper_cfg.mclk = clk_get(&pdev->dev, "vpss_master"); >>> >>> use devm_clk_get() here to simplify the error handling. >>> >> OK >> >>>> + if (IS_ERR(oper_cfg.mclk)) { >>>> + status = PTR_ERR(oper_cfg.mclk); >>>> + goto fail_getclk; >>>> + } >>>> + status = clk_prepare_enable(oper_cfg.mclk); >>>> + if (status) >>>> + goto fail_mclk; >>>> + >>>> + /* Get and enable Slave clock */ >>>> + oper_cfg.sclk = clk_get(&pdev->dev, "vpss_slave"); >>>> + if (IS_ERR(oper_cfg.sclk)) { >>>> + status = PTR_ERR(oper_cfg.sclk); >>>> + goto fail_mclk; >>>> + } >>>> + status = clk_prepare_enable(oper_cfg.sclk); >>>> + if (status) >>>> + goto fail_sclk; >>>> + >>>> dev_info(&pdev->dev, "%s vpss probed\n", platform_name); >>>> r1 = platform_get_resource(pdev, IORESOURCE_MEM, 0); >>>> if (!r1) >>>> @@ -500,6 +525,13 @@ fail2: >>>> iounmap(oper_cfg.vpss_regs_base0); >>>> fail1: >>>> release_mem_region(r1->start, resource_size(r1)); >>>> +fail_sclk: >>>> + clk_disable_unprepare(oper_cfg.sclk); >>>> + clk_put(oper_cfg.sclk); >>>> +fail_mclk: >>>> + clk_disable_unprepare(oper_cfg.mclk); >>>> + clk_put(oper_cfg.mclk); >>>> +fail_getclk: >>>> return status; >>>> } >>>> >>>> @@ -510,6 +542,10 @@ static int vpss_remove(struct platform_device *pdev) >>>> iounmap(oper_cfg.vpss_regs_base0); >>>> res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >>>> release_mem_region(res->start, resource_size(res)); >>>> + clk_disable_unprepare(oper_cfg.mclk); >>>> + clk_disable_unprepare(oper_cfg.sclk); >>>> + clk_put(oper_cfg.mclk); >>>> + clk_put(oper_cfg.sclk); >>>> if (oper_cfg.platform == DM355 || oper_cfg.platform == DM365) { >>>> iounmap(oper_cfg.vpss_regs_base1); >>>> res = platform_get_resource(pdev, IORESOURCE_MEM, 1); >>>> @@ -518,10 +554,34 @@ static int vpss_remove(struct platform_device *pdev) >>>> return 0; >>>> } >>>> >>> >>>> +static int vpss_suspend(struct device *dev) >>>> +{ >>>> + /* Disable both master and slave clock */ >>>> + clk_disable_unprepare(oper_cfg.mclk); >>>> + clk_disable_unprepare(oper_cfg.sclk); >>>> + >>>> + return 0; >>>> +} >>>> + >>>> +static int vpss_resume(struct device *dev) >>>> +{ >>>> + /* Enable both master and slave clock */ >>>> + clk_prepare_enable(oper_cfg.mclk); >>>> + clk_prepare_enable(oper_cfg.sclk); >>>> + >>>> + return 0; >>>> +} >>>> + >>>> +static const struct dev_pm_ops vpss_pm_ops = { >>>> + .suspend = vpss_suspend, >>>> + .resume = vpss_resume, >>>> +}; >>> >>> Addition of suspend support seems unrelated to this patch. May be make a >>> seperate patch for it and while at it, please use PM runtime instead of >>> direct clock enable/disable. Have a look at the davinci_emac driver >>> which was converted to use PM runtime recently. >>> >> I felt having in same patch would be a good idea, since the clock >> enabling/disabling >> where removed from dm644x_ccdc.c for suspend/resume and add it in >> vpss. If you still >> feel it needs to be separate patch let me know. > > Okay, please add some description for this. Its not terribly obvious. > >> >>> Let me know how you want to handle this patch. I suppose you intend this >>> should go through my tree because of other dependent platform changes? >>> >> I want this series to go through the media tree because of few dependencies >> (dependency on ths7353 video amplifier driver which recently got >> merged into media tree) > > Hmm, this patch does not touch ths7353 so are you sure there is a merge > dependency? There are a lot of platform changes you are doing in some of > the recent patches you submitted based on this patch and ideally those > go through my tree to ease merging for upstream. That said, if there is > a good reason for this and the other platform changes to go through > media tree, I am okay with that too. > Yes this patch doesnt touch ths7353, but the machine patches for DM365 which I posted earlier have a dependency on it. And also I have planned to take the machine patches for DM365 and DM355 through media tree itself. [1] this is one is also of dependent patch which is already merged into media tree. So to avoid build/dependency I fell this series and the machine patches for DM365/DM355 should go through media tree. [1] http://git.linuxtv.org/media_tree.git/commitdiff/ef2d41b19b8100ce63eabba9ee87953aa685921a Regards, --Prabhakar > Thanks, > Sekhar ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] media: davinci: vpbe: venc: move the enabling of vpss clocks to driver 2013-03-22 7:53 [PATCH 0/2] davinci: vpss: clock cleanup Prabhakar lad 2013-03-22 7:53 ` [PATCH 1/2] media: davinci: vpss: enable vpss clocks Prabhakar lad @ 2013-03-22 7:53 ` Prabhakar lad 2013-03-25 5:39 ` Sekhar Nori 1 sibling, 1 reply; 10+ messages in thread From: Prabhakar lad @ 2013-03-22 7:53 UTC (permalink / raw) To: linux-arm-kernel From: Lad, Prabhakar <prabhakar.csengg@gmail.com> The vpss clocks were enabled by calling a exported function from a driver in a machine code. calling driver code from platform code is incorrect way. This patch fixes this issue and calls the function from driver code itself. Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com> --- Note: This patch is based on the comment from Sekhar (https://patchwork-mail1.kernel.org/patch/2278441/). Shekar I haven't completely removed the callback, I just added the function calls after the callback. As you mentioned just to pass the VPSS_CLK_CTRL as a resource to venc but the VPSS_CLK_CTRL is already being used by VPSS driver. I'll take this cleanup task later point of time. drivers/media/platform/davinci/vpbe_venc.c | 26 ++++++++++++++++++++++++++ 1 files changed, 26 insertions(+), 0 deletions(-) diff --git a/drivers/media/platform/davinci/vpbe_venc.c b/drivers/media/platform/davinci/vpbe_venc.c index f15f211..26bdf9b 100644 --- a/drivers/media/platform/davinci/vpbe_venc.c +++ b/drivers/media/platform/davinci/vpbe_venc.c @@ -202,6 +202,26 @@ static void venc_enabledigitaloutput(struct v4l2_subdev *sd, int benable) } } +static void +venc_enable_vpss_clock(int venc_type, + enum vpbe_enc_timings_type type, + unsigned int pclock) +{ + if (venc_type == VPBE_VERSION_1) + return; + + if (venc_type == VPBE_VERSION_2 && (type == VPBE_ENC_STD || + (type == VPBE_ENC_DV_TIMINGS && pclock <= 27000000))) { + vpss_enable_clock(VPSS_VENC_CLOCK_SEL, 1); + vpss_enable_clock(VPSS_VPBE_CLOCK, 1); + return; + } + + if (venc_type == VPBE_VERSION_3 && type == VPBE_ENC_STD) + vpss_enable_clock(VPSS_VENC_CLOCK_SEL, 0); + +} + #define VDAC_CONFIG_SD_V3 0x0E21A6B6 #define VDAC_CONFIG_SD_V2 0x081141CF /* @@ -220,6 +240,7 @@ static int venc_set_ntsc(struct v4l2_subdev *sd) if (pdata->setup_clock(VPBE_ENC_STD, V4L2_STD_525_60) < 0) return -EINVAL; + venc_enable_vpss_clock(venc->venc_type, VPBE_ENC_STD, V4L2_STD_525_60); venc_enabledigitaloutput(sd, 0); if (venc->venc_type == VPBE_VERSION_3) { @@ -265,6 +286,7 @@ static int venc_set_pal(struct v4l2_subdev *sd) if (venc->pdata->setup_clock(VPBE_ENC_STD, V4L2_STD_625_50) < 0) return -EINVAL; + venc_enable_vpss_clock(venc->venc_type, VPBE_ENC_STD, V4L2_STD_625_50); venc_enabledigitaloutput(sd, 0); if (venc->venc_type == VPBE_VERSION_3) { @@ -319,6 +341,7 @@ static int venc_set_480p59_94(struct v4l2_subdev *sd) if (pdata->setup_clock(VPBE_ENC_DV_TIMINGS, 27000000) < 0) return -EINVAL; + venc_enable_vpss_clock(venc->venc_type, VPBE_ENC_DV_TIMINGS, 27000000); venc_enabledigitaloutput(sd, 0); if (venc->venc_type == VPBE_VERSION_2) @@ -366,6 +389,7 @@ static int venc_set_576p50(struct v4l2_subdev *sd) if (pdata->setup_clock(VPBE_ENC_DV_TIMINGS, 27000000) < 0) return -EINVAL; + venc_enable_vpss_clock(venc->venc_type, VPBE_ENC_DV_TIMINGS, 27000000); venc_enabledigitaloutput(sd, 0); if (venc->venc_type == VPBE_VERSION_2) @@ -406,6 +430,7 @@ static int venc_set_720p60_internal(struct v4l2_subdev *sd) if (pdata->setup_clock(VPBE_ENC_DV_TIMINGS, 74250000) < 0) return -EINVAL; + venc_enable_vpss_clock(venc->venc_type, VPBE_ENC_DV_TIMINGS, 74250000); venc_enabledigitaloutput(sd, 0); venc_write(sd, VENC_OSDCLK0, 0); @@ -434,6 +459,7 @@ static int venc_set_1080i30_internal(struct v4l2_subdev *sd) if (pdata->setup_clock(VPBE_ENC_DV_TIMINGS, 74250000) < 0) return -EINVAL; + venc_enable_vpss_clock(venc->venc_type, VPBE_ENC_DV_TIMINGS, 74250000); venc_enabledigitaloutput(sd, 0); venc_write(sd, VENC_OSDCLK0, 0); -- 1.7.4.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] media: davinci: vpbe: venc: move the enabling of vpss clocks to driver 2013-03-22 7:53 ` [PATCH 2/2] media: davinci: vpbe: venc: move the enabling of vpss clocks to driver Prabhakar lad @ 2013-03-25 5:39 ` Sekhar Nori 2013-03-25 10:15 ` Prabhakar Lad 0 siblings, 1 reply; 10+ messages in thread From: Sekhar Nori @ 2013-03-25 5:39 UTC (permalink / raw) To: linux-arm-kernel On 3/22/2013 1:23 PM, Prabhakar lad wrote: > From: Lad, Prabhakar <prabhakar.csengg@gmail.com> > > The vpss clocks were enabled by calling a exported function from a driver > in a machine code. calling driver code from platform code is incorrect way. > > This patch fixes this issue and calls the function from driver code itself. > > Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com> > --- > Note: This patch is based on the comment from Sekhar > (https://patchwork-mail1.kernel.org/patch/2278441/). > Shekar I haven't completely removed the callback, I just added > the function calls after the callback. As you mentioned just to > pass the VPSS_CLK_CTRL as a resource to venc but the VPSS_CLK_CTRL > is already being used by VPSS driver. I'll take this cleanup task later > point of time. Fine by me. Thanks, Sekhar ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] media: davinci: vpbe: venc: move the enabling of vpss clocks to driver 2013-03-25 5:39 ` Sekhar Nori @ 2013-03-25 10:15 ` Prabhakar Lad 2013-03-25 11:18 ` Sekhar Nori 0 siblings, 1 reply; 10+ messages in thread From: Prabhakar Lad @ 2013-03-25 10:15 UTC (permalink / raw) To: linux-arm-kernel Hi Sekhar, On Mon, Mar 25, 2013 at 11:09 AM, Sekhar Nori <nsekhar@ti.com> wrote: > On 3/22/2013 1:23 PM, Prabhakar lad wrote: >> From: Lad, Prabhakar <prabhakar.csengg@gmail.com> >> >> The vpss clocks were enabled by calling a exported function from a driver >> in a machine code. calling driver code from platform code is incorrect way. >> >> This patch fixes this issue and calls the function from driver code itself. >> >> Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com> >> --- >> Note: This patch is based on the comment from Sekhar >> (https://patchwork-mail1.kernel.org/patch/2278441/). >> Shekar I haven't completely removed the callback, I just added >> the function calls after the callback. As you mentioned just to >> pass the VPSS_CLK_CTRL as a resource to venc but the VPSS_CLK_CTRL >> is already being used by VPSS driver. I'll take this cleanup task later >> point of time. > > Fine by me. > Can I have your ACK on this patch ? Regards, --Prabhakar > Thanks, > Sekhar ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] media: davinci: vpbe: venc: move the enabling of vpss clocks to driver 2013-03-25 10:15 ` Prabhakar Lad @ 2013-03-25 11:18 ` Sekhar Nori 0 siblings, 0 replies; 10+ messages in thread From: Sekhar Nori @ 2013-03-25 11:18 UTC (permalink / raw) To: linux-arm-kernel On 3/25/2013 3:45 PM, Prabhakar Lad wrote: > Hi Sekhar, > > On Mon, Mar 25, 2013 at 11:09 AM, Sekhar Nori <nsekhar@ti.com> wrote: >> On 3/22/2013 1:23 PM, Prabhakar lad wrote: >>> From: Lad, Prabhakar <prabhakar.csengg@gmail.com> >>> >>> The vpss clocks were enabled by calling a exported function from a driver >>> in a machine code. calling driver code from platform code is incorrect way. >>> >>> This patch fixes this issue and calls the function from driver code itself. >>> >>> Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com> >>> --- >>> Note: This patch is based on the comment from Sekhar >>> (https://patchwork-mail1.kernel.org/patch/2278441/). >>> Shekar I haven't completely removed the callback, I just added >>> the function calls after the callback. As you mentioned just to >>> pass the VPSS_CLK_CTRL as a resource to venc but the VPSS_CLK_CTRL >>> is already being used by VPSS driver. I'll take this cleanup task later >>> point of time. >> >> Fine by me. >> > Can I have your ACK on this patch ? The 'fine' from me was for the approach, not not patch ;). Seriously though, since this patch is only touching media/ I haven't really done a detailed enough review of it. In any case, it should be OK to merge this without my ack. Thanks, Sekhar ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2013-03-25 11:18 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-03-22 7:53 [PATCH 0/2] davinci: vpss: clock cleanup Prabhakar lad 2013-03-22 7:53 ` [PATCH 1/2] media: davinci: vpss: enable vpss clocks Prabhakar lad 2013-03-25 5:32 ` Sekhar Nori 2013-03-25 10:14 ` Prabhakar Lad 2013-03-25 10:24 ` Sekhar Nori 2013-03-25 10:33 ` Prabhakar Lad 2013-03-22 7:53 ` [PATCH 2/2] media: davinci: vpbe: venc: move the enabling of vpss clocks to driver Prabhakar lad 2013-03-25 5:39 ` Sekhar Nori 2013-03-25 10:15 ` Prabhakar Lad 2013-03-25 11:18 ` Sekhar Nori
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).