Quoting Tero Kristo (2016-01-04 11:15:36)
> On 01/04/2016 06:37 PM, Tony Lindgren wrote:
> > * Russell King - ARM Linux <linux@xxxxxxxxxxxxxxxx> [160104 06:43]:
> >> On Mon, Jan 04, 2016 at 03:27:57PM +0200, Tero Kristo wrote:
> >>> On 01/04/2016 12:21 PM, Geert Uytterhoeven wrote:
> >>>> FWIW, there are small loops with just a cpu_relax() in various clock drivers
> >>>> under drivers/clk/shmobile/.
> >>>
> >>> Just did a quick profiling round, and the clk_enable/disable delay loops
> >>> take anything from 0...1500ns, most typically consuming some 400-600ns. So,
> >>> based on this, dropping the udelay and adding cpu_relax instead looks like a
> >>> good change. I just verified that changing the udelay to cpu_relax works
> >>> fine also, I just need to change the bail-out period to be something sane.
> >>
> >> Was that profiling done with lockdep/lock debugging enabled or disabled?
>
> omap2plus_defconfig, so lockdep was enabled. The profiling was done
> around the while {} block though, which should not have any locks within
> it (except for the SCM clocks, which may explain some of the higher
> latency numbers seen.)
>
> > And also the thing to check from the hw folks is what all do these clkctrl
> > bits really control. If they group together the OCP clock and an extra
> > functional clock for some devices the delays could be larger.
>
> Does it matter really? The latencies are only imposed to the device in
> question, and lets face it, the same latencies are there already with
> the hwmod implementation. This series moves the implementation under
> clock driver with as less modifications as possible to avoid any problems.
So long as we can all convince ourselves that the flaw is not a flaw
then I'm OK with it. No bugs were ever introduced that way ;-)
But in fairness, we've had these delays in the .enable callbacks for a
while, so this patch does not introduce the regression. Furthermore it
does clean up some code that needs more work, and I don't want to delay
that.
I won't NACK the patch due to the delays, but it would be nice to
revisit it some day.
Regards,
Mike
>
> > In general, I think we need to get rid of pm_runtime_irq_safe usage to
> > allow clocks to sleep properly. The other option is to allow toggling
> > pm_runtime_irq_safe but that probably gets super messy.
>
> That is something not to be done with this set though.
>
> -Tero
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
