On 3/22/2013 1:23 PM, Prabhakar lad wrote: > From: Lad, Prabhakar <prabhakar.csengg@xxxxxxxxx> > > 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@xxxxxxxxx> > --- > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
