On 04/19/2013 01:38 PM, Eunchul Kim wrote: >> static void fimc_set_type_ctrl(struct fimc_context *ctx, enum fimc_wb wb) >> @@ -1628,7 +1617,9 @@ static int fimc_ippdrv_start(struct device *dev, enum >> drm_exynos_ipp_cmd cmd) >> fimc_handle_lastend(ctx, true); >> >> /* setup FIMD */ >> - fimc_set_camblk_fimd0_wb(ctx); >> + ret = fimc_set_camblk_fimd0_wb(ctx); >> + if (ret < 0) > > - please adds error log information for indicating error. OK, I'll add it. >> + return ret; >> >> set_wb.enable = 1; >> set_wb.refresh = property->refresh_rate; >> @@ -1784,26 +1775,50 @@ e_clk_free: >> return ret; >> } >> >> +static int fimc_parse_dt(struct fimc_context *ctx) >> +{ >> + struct device_node *node = ctx->dev->of_node; >> + >> + if (!of_property_read_bool(node, "samsung,lcd-wb")) > > - It also need error log ? No, but it definitely deserves some comment why it is done like this. Please see my other reply to Inki. >> + return -ENODEV; >> + >> + if (of_property_read_u32(node, "clock-frequency", >> + &ctx->clk_frequency)) >> + ctx->clk_frequency = FIMC_DEFAULT_LCLK_FREQUENCY; > > - We have many kind of H/W version of FIMC device. I think we need to use pdata > instead of static value. This is just a fallback value that will be used when "clock-frequency" property is not found in fimc node in the device tree. >> + ctx->id = of_alias_get_id(node, "fimc"); >> + if (ctx->id < 0) >> + return -EINVAL; >> + >> + return 0; >> +} >> + >> static int fimc_probe(struct platform_device *pdev) >> { >> struct device *dev = &pdev->dev; >> struct fimc_context *ctx; >> struct resource *res; >> struct exynos_drm_ippdrv *ippdrv; >> - struct exynos_drm_fimc_pdata *pdata; >> - struct fimc_driverdata *ddata; >> int ret; >> >> - pdata = pdev->dev.platform_data; >> - if (!pdata) { >> - dev_err(dev, "no platform data specified.\n"); >> - return -EINVAL; >> - } >> + if (!dev->of_node) > > - ditto.(error log) Probably won't hurt, I'll add it. >> + return -ENODEV; >> >> ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL); >> if (!ctx) >> return -ENOMEM; >> >> + ctx->dev = dev; >> + >> + ret = fimc_parse_dt(ctx); >> + if (ret < 0) >> + return ret; >> + >> + ctx->sysreg = syscon_regmap_lookup_by_phandle(dev->of_node, >> + "samsung,sysreg"); >> + if (IS_ERR(ctx->sysreg)) > > - ditto.(error log) OK. Will add here as well. _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel