linux-kernel - Re: [PATCH v1] clk: qcom: lpass: Add CLK_IGNORE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <154750471090.169631.5712250327468594228@swboyd.mtv.corp.google.com>
Date:   Mon, 14 Jan 2019 14:25:10 -0800
From:   Stephen Boyd <sboyd@...nel.org>
To:     Michael Turquette <mturquette@...libre.com>,
        Taniya Das <tdas@...eaurora.org>
Cc:     Andy Gross <andy.gross@...aro.org>,
        David Brown <david.brown@...aro.org>,
        Rajendra Nayak <rnayak@...eaurora.org>,
        linux-arm-msm@...r.kernel.org, linux-soc@...r.kernel.org,
        linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1] clk: qcom: lpass: Add CLK_IGNORE_UNUSED for lpass clocks

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