Hi Sylwester. On 7 January 2012 20:14, Sylwester Nawrocki <snjw23@xxxxxxxxx> wrote: [...] >> diff --git a/arch/arm/mach-exynos/pm_domains.c b/arch/arm/mach-exynos/pm_domains.c >> new file mode 100644 >> index 0000000..95a7c55 >> --- /dev/null >> +++ b/arch/arm/mach-exynos/pm_domains.c >> @@ -0,0 +1,183 @@ >> +/* >> + * Exynos4 Generic power domain support. >> + * >> + * Copyright (c) 2011 Samsung Electronics Co., Ltd. > > 2012 ? Ok. I will change this. > >> + * http://www.samsung.com [...] >> +static int exynos4_pd_power(struct generic_pm_domain *domain, bool power_on) >> +{ >> + struct exynos4_pm_domain *pd; >> + void __iomem *base; >> + u32 timeout, pwr; >> + char *op; >> + >> + pd = container_of(domain, struct exynos4_pm_domain, pd); >> + base = pd->base; >> + >> + pwr = (power_on) ? S5P_INT_LOCAL_PWR_EN : 0; > > Is there any value in parentheses around 'power_on' ? No. I will remove the parentheses around 'power_on'. > >> + __raw_writel(pwr, base); >> + >> + /* Wait max 1ms */ >> + timeout = 10; >> + >> + while ((__raw_readl(base + 0x4)& S5P_INT_LOCAL_PWR_EN) != pwr) { >> + if (!timeout) { >> + op = (power_on) ? "enable" : "disable"; >> + pr_err("Power domain %s %s failed\n", op, domain->name); > > How about just: > pr_err("%s power domain state change (%d) failed\n", > domain->name, power_on); > ? >From the message it will not be clear which state change failed. The state (enable/disable) would be helpful. >> + return -ETIMEDOUT; >> + } >> + timeout--; >> + cpu_relax(); > > Does cpu_relax() make any difference here ? I need to check on this. > >> + usleep_range(80, 100); >> + } >> + return 0; >> +} >> + [...] >> + pd = kzalloc(sizeof(*pd), GFP_KERNEL); >> + if (!pd) { >> + pr_err("exynos4_pm_init_power_domain: failed to " >> + "allocate memory for domain\n"); > > nit: what about: > pr_err("%s: failed to allocate memory for domain\n", > __func__); > ? Yes. This is better. I will change this. >> + return -ENOMEM; >> + } >> + [...] >> +#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 > > Could you add CSIS1 as well ? Some boards will be using both MIPI-CSI receivers. Ok. I will add CSIS1 here. Thanks for your review. Regards, Thomas. [...] -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
 |