On 09/12/2013 02:07 PM, Arun Kumar K wrote:
+static int fimc_is_probe(struct platform_device *pdev)
+{
+ struct device *dev =&pdev->dev;
+ struct resource *res;
+ struct fimc_is *is;
+ void __iomem *regs;
+ struct device_node *node;
+ int irq, ret;
+ int i;
+
+ dev_dbg(dev, "FIMC-IS Probe Enter\n");
+
+ if (!pdev->dev.of_node)
nit: Could be simplified to:
if (!dev->of_node)
+ return -ENODEV;
+
+ is = devm_kzalloc(&pdev->dev, sizeof(*is), GFP_KERNEL);
+ if (!is)
+ return -ENOMEM;
+
+ is->pdev = pdev;
+
+ is->drvdata = fimc_is_get_drvdata(pdev);
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ regs = devm_ioremap_resource(dev, res);
+ if (IS_ERR(regs))
+ return PTR_ERR(regs);
+
+ /* Get the PMU base */
+ node = of_parse_phandle(dev->of_node, "samsung,pmu", 0);
+ if (!node)
+ return -ENODEV;
+ is->pmu_regs = of_iomap(node, 0);
+ if (!is->pmu_regs)
+ return -ENOMEM;
+
+ irq = irq_of_parse_and_map(dev->of_node, 0);
+ if (irq< 0) {
+ dev_err(dev, "Failed to get IRQ\n");
+ return irq;
+ }
+
+ ret = fimc_is_configure_clocks(is);
+ if (ret< 0) {
+ dev_err(dev, "clocks configuration failed\n");
+ goto err_clk;
+ }
+
+ platform_set_drvdata(pdev, is);
+ pm_runtime_enable(dev);
+
+ ret = pm_runtime_get_sync(dev);
+ if (ret< 0)
+ goto err_pm;
Is activating the device at this point really needed ? Perhaps you
could drop the pm_runtime_get/put calls ?
+ is->alloc_ctx = vb2_dma_contig_init_ctx(dev);
+ if (IS_ERR(is->alloc_ctx)) {
+ ret = PTR_ERR(is->alloc_ctx);
+ goto err_vb;
+ }
+
+ /* Get IS-sensor contexts */
+ ret = fimc_is_parse_sensor(is);
+ if (ret< 0)
+ goto err_vb;
+
+ /* Initialize FIMC Pipeline */
+ for (i = 0; i< is->drvdata->num_instances; i++) {
+ ret = fimc_is_pipeline_init(&is->pipeline[i], i, is);
+ if (ret< 0)
+ goto err_sd;
+ }
+
+ /* Initialize FIMC Interface */
+ ret = fimc_is_interface_init(&is->interface, regs, irq);
+ if (ret< 0)
+ goto err_sd;
+
+ pm_runtime_put(dev);
+
+ dev_dbg(dev, "FIMC-IS registered successfully\n");
+
+ return 0;
+
+err_sd:
+ fimc_is_pipelines_destroy(is);
+err_vb:
+ vb2_dma_contig_cleanup_ctx(is->alloc_ctx);
+err_pm:
+ pm_runtime_put(dev);
+err_clk:
+ fimc_is_put_clocks(is);
+
+ return ret;
+}
+
+int fimc_is_clk_enable(struct fimc_is *is)
+{
+ int ret;
+
+ ret = clk_enable(is->clock[IS_CLK_ISP]);
+ if (ret)
+ return ret;
+ ret = clk_enable(is->clock[IS_CLK_MCU_ISP]);
Shouldn't you disable the first enabled clock when this call fails ?
+ return ret;
+}
[...]
+static void *fimc_is_get_drvdata(struct platform_device *pdev)
+{
+ struct fimc_is_drvdata *driver_data = NULL;
+
+ if (pdev->dev.of_node) {
pdev->dev.of_node is being tested against NULL before call to this
function, you could make this code slightly simpler with that assumption.
+ const struct of_device_id *match;
+ match = of_match_node(exynos5_fimc_is_match,
+ pdev->dev.of_node);
+ if (match)
+ driver_data = (struct fimc_is_drvdata *)match->data;
+ }
+ return driver_data;
+}
--
Thanks,
Sylwester
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
