Hi Geert,
On Tuesday, January 24, 2017, Geert Uytterhoeven wrote:
> > From what I can tell, that makes the register space readable...but the
> > IP block is not fully functional unless you delay a little.
>
> If you know the minimum delay needed, and it's not too long, it can be
> added to the enable path.
I can play around and see. I know udealy(100) works OK, but then I have
to have a delay that's as long as the slowest peripheral.
If it was just to turn a clock on once, or once in a while, that's OK. But
it seems like runtime pm wants to turn the clocks on/off as much as
possible.
> > But, I think I'd like to disable runtime pm for RZ/A1 in the drivers
> > for now Because 'functional' is better than 'lower-power-but-broken'
>
> I prefer not doing that in the individual drivers, as they're shared with
> other SoCs.
What I meant was looking at the compatible string and only doing
it for RZ/A1.
For example, in rspi_probe():
if (of_device_is_compatible(np, " renesas,rspi-r7s72100"))
master->auto_runtime_pm = false;
else
master->auto_runtime_pm = true;
I would do the same kind of thing for riic.
> I think you can handle that in drivers/clk/renesas/clk-mstp.c:
> - in cpg_mstp_attach_dev(), add a call to pm_clk_resume(dev) after the
> call to pm_clk_add_clk(),
> - in cpg_mstp_detach_dev(), add a call to pm_clk_suspend(dev) before the
> call to pm_clk_destroy().
> Yes, that means the module clocks are enabled all the time.
> Of course when running on RZ/A1H ;-)
That might be OK.
But won't the individual drivers still want to keep turning clocks on and off manually?
(unless I'm not understanding that the underlying clock routines will basically just
ignore everything). But even if that' the case...that just wasted CPU cycles (remember,
I'm only working with a 400MHz single core here running XIP_KERNEL)
I think I should at least put the dummy read in cpg_mstp_clock_endisable() first,
then maybe I can see what drivers work. Something like this:
spin_lock_irqsave(&group->lock, flags);
value = cpg_mstp_read(group, group->smstpcr);
if (enable)
value &= ~bitmask;
else
value |= bitmask;
cpg_mstp_write(group, value, group->smstpcr);
+ if (!group->mstpsr) {
+ /* dummy read to ensure write has completed */
+ clk_readl(group->smstpcr);
+ barrier_data(group->smstpcr);
+ }
spin_unlock_irqrestore(&group->lock, flags);
Chris
