Dear Mr. Park.

On 9 January 2012 05:57, Kyungmin Park [off-list ref] wrote:
[...]

quoted

+ * Exynos4 specific wrapper around the generic power domain
+ */
+struct exynos4_pm_domain {
+     void __iomem *base;
+     char const *name;
+     bool is_off;
+     struct generic_pm_domain pd;
+};

Even though you tested it at exynos4, we already know exynos5 will be used soon.
so start to use the exynos prefix if possible.

Ok. I will change exynos4 to exynos.

[...]

quoted

+
+static __init int exynos4_pm_init_power_domain(void)
+{
+     int idx;
+     struct device_node *np;

The np is only used at CONFIG_ON.

Ok. I will rework this.

quoted

+
+#ifdef CONFIG_OF
+     if (!of_have_populated_dt())
+             goto no_dt;
+
+     for_each_compatible_node(np, NULL, "samsung,exynos4210-pd") {
+             struct exynos4_pm_domain *pd;
+
+             pd = kzalloc(sizeof(*pd), GFP_KERNEL);
+             if (!pd) {
+                     pr_err("exynos4_pm_init_power_domain: failed to "
+                                     "allocate memory for domain\n");
+                     return -ENOMEM;
+             }
+
+             if (of_get_property(np, "samsung,exynos4210-pd-off", NULL))
+                     pd->is_off = true;
+             pd->name = np->name;
+             pd->base = of_iomap(np, 0);
+             pd->pd.power_off = exynos4_pd_power_off;
+             pd->pd.power_on = exynos4_pd_power_on;
+             pd->pd.of_node = np;
+             pm_genpd_init(&pd->pd, NULL, false);
+     }
+     return 0;
+#endif /* CONFIG_OF */
+
+ no_dt:

You should add the "#endif /* CONFIG_OF */ since no_dt is used at
CONFIG_OF case.

Ok.

quoted

+     for (idx = 0; idx < ARRAY_SIZE(exynos4_pm_domains); idx++)
+             pm_genpd_init(&exynos4_pm_domains[idx]->pd, NULL,
+                             exynos4_pm_domains[idx]->is_off);
+

[...]

quoted

+#ifdef CONFIG_S5P_DEV_CSIS0
+     if (pm_genpd_add_device(&exynos4_pd_cam.pd, &s5p_device_mipi_csis0.dev))
+             pr_info("error in adding csis0 to cam power domain\n");
+#endif

Also need to add CSIS1 as Sylwester mentioned.

Ok. I will add CSIS1.

Thanks for your review.

Regards,
Thomas.