Re: [PATCH v17 3/4] soc: rockchip: power-domain: Add power domain driver

From: Caesar Wang
Date: Sat Sep 05 2015 - 23:05:45 EST


Kevin,

Thanks for having a look into it.


å 2015å09æ03æ 02:28, Kevin Hilman åé:
Caesar Wang <wxt@xxxxxxxxxxxxxx> writes:

This driver is found on RK3288 SoCs.

In order to meet high performance and low power requirements, a power
management unit is designed or saving power when RK3288 in low power
mode.
The RK3288 PMU is dedicated for managing the power of the whole chip.

PMU can work in the Low Power Mode by setting bit[0] of PMU_PWRMODE_CON
register. After setting the register, PMU would enter the Low Power mode.
In the low power mode, pmu will auto power on/off the specified power
domain, send idle req to specified power domain, shut down/up pll and
so on. All of above are configurable by setting corresponding registers.

Signed-off-by: jinkun.hong <jinkun.hong@xxxxxxxxxxxxxx>
Signed-off-by: Caesar Wang <wxt@xxxxxxxxxxxxxx>
[...]

+static void rockchip_pm_remove_one_domain(struct rockchip_pm_domain *pd)
+{
+ int i;
+
+ for (i = 0; i < pd->num_clks; i++) {
+ clk_unprepare(pd->clks[i]);
+ clk_put(pd->clks[i]);
+ }

You don't set pd->num_clks = 0 here, which means other places that
iterate over the clocks might race with this and try to use clocks that
have been unprepared/put.

Agree, we should set the "pd->num_cloks=0' in here.
This might be over-paranoid, but in particular, this could race with
rockchip_pd_power().

Also not setting the pd->num_clks to zero would be a problem for a
power-controller that is configured as a module which could be unloaded
and reloaded (I know that doesn't really work now, but it will
eventually, I hope.)

Yep.

Maybe use the mutex here? It should at least protect the zeroing of
pm->num_clks.

Sound resonable.
Done.

---
Thanks,
Caesar

Kevin

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/linux-rockchip

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/