linux-kernel - Re: [PATCH v8] clk: qcom: Add lpass clock controller driver for SDM845

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: <154117671164.88331.752297297364277275@swboyd.mtv.corp.google.com>
Date:   Fri, 02 Nov 2018 09:38:31 -0700
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,
        Taniya Das <tdas@...eaurora.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>
Subject: Re: [PATCH v8] clk: qcom: Add lpass clock controller driver for SDM845

Quoting Taniya Das (2018-10-28 00:35:40)
> Add support for the lpass clock controller found on SDM845 based devices.
> This would allow lpass peripheral loader drivers to control the clocks to
> bring the subsystem out of reset.
> LPASS clocks present on the global clock controller would be registered
> with the clock framework based on the device tree flag. Also do not gate
> these clocks if they are left unused, as the lpass clocks require the
> global clock controller lpass clocks to be enabled before they are
> accessed. Mark the GCC lpass clocks as CRITICAL, for the LPASS clock
> access.

Thanks for the useful commit text!

> 
> diff --git a/drivers/clk/qcom/gcc-sdm845.c b/drivers/clk/qcom/gcc-sdm845.c
> index 08d593e..9d60474 100644
> --- a/drivers/clk/qcom/gcc-sdm845.c
> +++ b/drivers/clk/qcom/gcc-sdm845.c
> @@ -3583,6 +3613,13 @@ static int gcc_sdm845_probe(struct platform_device *pdev)
>         if (ret)
>                 return ret;
> 
> +       if (!of_property_read_bool(pdev->dev.of_node, "qcom,lpass-protected")) {

Bjorn told me over IRC that the QSPI clks are also protected on MTP and
can't be read. So we're now causing MTP to fail on linus' tree. I think
we need to add a generic property to the GCC node to indicate what clks
are protected and shouldn't be touched by the OS. The function would go
and knock out any protected clks by replacing their pointers with NULL.
The property would be:

   protected-clocks = <clock-specifier-list>;

So for this node it would look like:

   gcc: clock-controller@...0 {
        reg = <0xf00 0xbaf>;

        protected-clocks = <&gcc GCC_LPASS_Q6_AXI_CLK>,
                           <&gcc GCC_LPASS_SWAY_CLK>,
                           <&gcc GCC_QSPI_CORE_CLK_SRC>,
                           etc.
   };

Sorry for derailing this patch series so late in the process. This patch
will need to be updated to work with that new DT property instead of
having a custom qcom property. Otherwise the patch looks good and I
think we're ready to merge it when the merge window closes and I can
rewind clk-next.