Dear Mike Turquette, On Thu, 24 Jul 2014 10:52:51 -0700, Mike Turquette wrote: > > > This is racy. You don't hold the clk_enable lock so it could be enable > > > between the conditional check and executing clk_cpu_on_set_rate. > > > > Right. > > > > > How do you ensure that secondary CPU clocks are not enabled/disabled > > > when changing rates? > > > > In practice, this currently cannot happen: we enable the secondary CPU > > clocks before starting the secondary CPUs, and we never ever disable or > > re-enable again those clocks. So with the present code, I believe there > > is no problem. Even when we do CPU hotplug, we don't turn off the CPU > > clocks, simply because they cannot be turned off: the enable/disable > > state is only used here as an indication so that the clock driver knows > > which frequency change strategy it should apply. > > > > But you're anyway right, I'll send a followup patch adding the lock. > > Would that be OK for you? > > Sounds good. Can you also fix up the changelog in patch #2? After that > I am happy with this series. I guess Jason will take it in and send it > for his PR? The fixup for the commit log in patch #2 cannot be done, because the commit has already been merged in arm-soc, if I understood correctly. Jason said: """ > The commit log of this commit was not adjusted consequently, and this > is my fault. Jason, is it still time to change this commit log? If there are no code changes, I'd prefer not to. We're rather late in the game. Even though it's not ideal, the commit in question does have a Link: tag pointing at the patch email on which this conversation is based. So a frustrated future developer won't be frustrated long. :) """ I'll do the lock patch. Thanks! Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
