linux-kernel - Re: [PATCH 2/5] clk: lpc32xx: read-only divider can propagate rate change

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]

Date:   Sat, 6 Jan 2018 16:04:24 +0200
From:   Vladimir Zapolskiy <vz@...ia.com>
To:     Jerome Brunet <jbrunet@...libre.com>,
        Stephen Boyd <sboyd@...eaurora.org>,
        Michael Turquette <mturquette@...libre.com>,
        linux-clk@...r.kernel.org
Cc:     linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-arm-msm@...r.kernel.org,
        Sylvain Lemieux <slemieux.tyco@...il.com>,
        Andy Gross <andy.gross@...aro.org>,
        David Brown <david.brown@...aro.org>
Subject: Re: [PATCH 2/5] clk: lpc32xx: read-only divider can propagate rate
 change

Hi Jerome,

On 01/05/2018 09:40 PM, Jerome Brunet wrote:
> On Fri, 2018-01-05 at 20:12 +0200, Vladimir Zapolskiy wrote:
>> Hi Jerome,
>>
>> On 01/05/2018 07:09 PM, Jerome Brunet wrote:
>>> When a divider clock has CLK_DIVIDER_READ_ONLY set, it means that the
>>> register shall be left un-touched, but it does not mean the clock
>>> should stop rate propagation if CLK_SET_RATE_PARENT is set
>>>
>>
>> okay, the statement sounds correct, but there is no such clocks on LPC32xx,
>> thus I hardly can confirm that adding dead/inapplicable code is a fix.
>>
>>> This properly handled in qcom clk-regmap-divider but it was not in the
>>> lpc32xx divider
>>>
>>> Fixes: f7c82a60ba26 ("clk: lpc32xx: add common clock framework driver")
>>> Signed-off-by: Jerome Brunet <jbrunet@...libre.com>
>>
>> I would suggest to drop two LPC32xx clock driver changes from the series.
> 
> Hi Vladimir,
> 
> This is fine by me. Whether LPC32xx supports CLK_DIVIDER_READ_ONLY is up to you,
> but you should be consistent about it.
> 
> I added the fix to LPC32xx because it looks like the generic divider (a lot) and

right, the relevant divider operations were copied, however the difference
is important, unfortunately there is no simple option to get rid of regmap,
because System Control Block registers are shared with a number of other
device drivers.

> appears to support CLK_DIVIDER_READ_ONLY. If it does not, could you please kill
> the related code ?

The driver supports CLK_DIVIDER_READ_ONLY clocks, and it should not be
changed, but all such clocks don't have children with CLK_SET_RATE_PARENT
property, which invalidates your fix for LPC32xx. Please let me know,
if I missed something.

--
With best wishes,
Vladimir