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.