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 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".
