linux-kernel - Re: [PATCH] clk-si5341: Support NVM programming through sysfs

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] [day] [month] [year] [list]

Message-ID: <160525459234.60232.7901604955432682238@swboyd.mtv.corp.google.com>
Date:   Fri, 13 Nov 2020 00:03:12 -0800
From:   Stephen Boyd <sboyd@...nel.org>
To:     Mike Looijmans <mike.looijmans@...ic.nl>, linux-clk@...r.kernel.org
Cc:     mturquette@...libre.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] clk-si5341: Support NVM programming through sysfs

Quoting Mike Looijmans (2020-11-05 01:06:54)
> On 05-11-2020 02:48, Stephen Boyd wrote:
> > Quoting Mike Looijmans (2020-11-03 06:17:41)
> >> Export an attribute program_nvm_bank that when read reports the current
> >> bank value. To program the chip's current state into NVM, write the
> >> magic value 0xC7 into this attribute.
> >>
> >> Signed-off-by: Mike Looijmans <mike.looijmans@...ic.nl>
> >> ---
> > 
> > Any chance this can be done through the nvmem framework?
> 
> This part doesn't fit. The purpose is to store the current state of the clock 
> chip into its non-volatile storage so it boots up with that configuration the 
> next POR. Main use case is that some vendors initialize PLLs only in a 
> bootloader and thus need the clock running at boot. Or it might just be to 
> save on that 300ms initialization time.

Can these details be put in the commit text? It would go a long way
towards helping reviewers understand the motivation if it is to speed up
boot and stash away clk configuration across resets. Why is userspace
involved? Why can't the driver write to the non-volatile storage with
shutdown or reboot driver hooks instead of implementing a bespoke sysfs
ABI?

> 
> Having said that, the clock chip does have some "scratch" areas that'd be 
> useful as NVMEM storage. That'd be for a separate patch.
> 
> For this device to be NVMEM compatible, nvmem would need to have a sort of 
> transaction model, where you write several values and then "commit" them all 
> to NVM in one call. The nvmem framework wasn't intended for that I think.

I suppose patches could be written to support that if desired.