Hello Stephen,
On 1/19/2019 12:31 AM, Stephen Boyd wrote:
Quoting Taniya Das (2019-01-17 03:19:22)
On 1/15/2019 3:55 AM, Stephen Boyd wrote:
Quoting Taniya Das (2019-01-13 22:12:39)
On 1/8/2019 2:34 AM, Stephen Boyd wrote:
As far as I know, I'm not suggesting the use of CLK_IS_CRITICAL here.
But removing CLK_IS_CRITICAL and relying on some random bootloader
behavior also looks wrong. Can you clarify what's going on?
To enable LPASS clocks the requirement is to enable the GCC_LPASS_SWAY
clock.
1) If the LPASS drivers are enabled/probed before the clock late init
the client would take care to maintain the dependency to enable the
GCC_LPASS_SWAY clock before enabling the LPASS clocks.
2) There could be a condition where the LPASS drivers would probe/init
later the clock late_init. When the clock_late_init would try to access
the LPASS clocks, since we cannot maintain the dependency this access
would fail. To avoid this the earlier patch has made the GCC_LPASS_SWAY
clock as CRITICAL.
3) Marking the GCC_LPASS_SWAY clock as CRITICAL has a issue, in the case
where the LPASS subsystem would be restarted due to some critical
failure on LPASS. Toggling the restart register of LPASS would clear the
hardware state of this clock and thus the next access of the LPASS
clocks would result in failure of the system.
4) To avoid issues happening in (2) and (3) all the LPASS clocks chould
be safely marked as CLK_IGNORE_UNUSED. And lpass drivers would take care
of the dependency to enable the required clocks.
Ok, so why can't we enable/disable the lpass sway clk in the
prepare/unprepare phase of the lpass clk driver paths? Or why can't we
forcibly enable this lpass sway clk after the reset is deasserted? Which
clk controller is the reset part of? GCC or LPASS?
It is part of Always On Subsystem.
Ok. Is this merged upstream?
It still sounds like the LPASS clk driver isn't handling dependencies it
has on accessing registers, but maybe we can get away with not handling
the dependency still if we make the reset "do the right thing" and turn
the clk back on so it stays "critical".
This is a reset from hardware and it does not bring back the clock to
the previous state and so we can not mark it "critical". I would submit
the next series with comments updated. Please let me know in case you
have any comments.
Can we have the always on subsystem reset code go hit this clk enable
bit back on?
There is no code, it is a reset from hardware.
And also have the LPASS clk driver get this GCC sway clk
and enable it during probe? Maybe we need to get some sort of API in the
clk provider layer that can tell us that the clk state has changed now
and it needs to be restored. I haven't thought about it deeply but that
may be the best solution here.
Would it be possible to go about with the current patch and then have it
updated with the possible solutions.
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation.
--
