Re: [PATCH] soc/tegra: pmc: Fix "scheduling while atomic"

[Dmitry Osipenko]

On 27.05.2016 15:46, Jon Hunter wrote:
> On 26/05/16 18:01, Dmitry Osipenko wrote:
> > On 26.05.2016 18:27, Jon Hunter wrote:
> > > On 26/05/16 15:57, Dmitry Osipenko wrote:
>
> ...
>
> > > > That's how I see it:
> > > >
> > > > +----------------------------------------------+
> > > > |                    CPU 0                     |
> > > > +-------------------+--------------------------+
> > > > |    Idle thread    | Interactive gov. thread  |
> > > > +----------------------------------------------+
> > > > |     inactive      |                          |
> > > > |                   |                          |
> > > > |                   |   CPU freq. change       |
> > > > |                   |                          |
> > > > |                   |   clk_set_rate()         |
> > > > |                   |                          |
> > > > |       ...         |   clk_prepare_lock()     |
> > > > |                   |                          |
> > > > |                   |   PRE rate notifier call |
> > > > |                   |                          |
> > > > |                   |   schedule               |
> > >
> > > What is this notifier doing? Is there some sort of hardware activity
> > > that it is waiting for to complete?
> >
> > It changes regulator voltage if required. So at least I2C would cause
> > scheduling on wait_for_completion_timeout().
>
> Yes, of course that would make sense. What is interesting/odd in this
> case is that the frequency is increasing (voltage scaled pre frequency
> change) but yet you are entering LP2. May be that is possible? I guess
> this problem may also occur on reducing frequency as well?

Sorry, possible what? Surely it might happen on the POST notify, I just used PRE 
for example.

There are no active tasks while CPU changing the frequency, so kernel enters 
idle mode while waiting for the HW completion - the interrupt. I don't see 
anything wrong here.

> What are you using in the v3.18 kernel for exit_latency and
> target_residency? The current mainline has 5000us and 10000us,
> respectively.

Default stock latencies, not sure why you are asking. It's essentially a 
mainline kernel with some added device drivers and android patches.

> It does seem that this could be triggered in the right circumstances and
> I have to say I don't like the fact that this could be fragile as it is
> today. Have you thought about adding a post clock notifier for pclk in
> the PMC driver as an alternative to the change you are suggesting?

No, I haven't. Sounds like a good idea, thanks for the suggestion! I'll try it 
and send V2 if it will be okay, otherwise will report the problem.

--
Dmitry