linux-kernel - Re: [RESEND PATCH v4] clk/axs10x: Add I2S PLL clock driver

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:	Mon, 18 Apr 2016 17:19:14 +0530
From:	Vineet Gupta <Vineet.Gupta1@...opsys.com>
To:	Jose Abreu <Jose.Abreu@...opsys.com>,
	Stephen Boyd <sboyd@...eaurora.org>
CC:	<linux-kernel@...r.kernel.org>, <linux-clk@...r.kernel.org>,
	<linux-snps-arc@...ts.infradead.org>,
	<CARLOS.PALMINHA@...opsys.com>, <Alexey.Brodkin@...opsys.com>,
	<mturquette@...libre.com>
Subject: Re: [RESEND PATCH v4] clk/axs10x: Add I2S PLL clock driver

On Monday 18 April 2016 04:00 PM, Jose Abreu wrote:
>>> +	if (readl((void *)FPGA_VER_INFO) <= FPGA_VER_27M) {
>> > Please don't readl directly from addresses. I think I mentioned
>> > that before and didn't get back to you when you replied asking
>> > for other solutions. I still think a proper DT is in order
>> > instead of doing this check for ref_clk.
> I think that the DT approach would be better but I also think that using two DT
> files with only one change between them is not viable. I can see some alternatives:
>     1) Pass the region of FPGA version in reg field of DT so that writel is not
> directly used;
>     2) Create a dummy parent clock driver that reads from FPGA version register
> and returns the rate;
>     3) Last resort: Use two DT files for each FPGA version.
> 
> @Vineet, @Alexey: Can you give some suggestions?
> 
> Some background:
> We are expecting a new firmware release for the AXS board that will change the
> reference clock value of the I2S PLL from 27MHz to 28.224MHz. Due to this change
> the dividers of this PLL will change. Right now I am directly reading from the
> FPGA version register but Stephen suggested to use a DT approach so that this
> rate is declared as parent clock. This would be a good solution but would
> require the usage of two different DT files (one for the current firmware and
> another for the new firmware), which I think is not ideal. What is your opinion?
> Some other solutions are listed above.

Consider this my ignorance of clk drivers, what exactly is the problem with that
readl() for FPGA ver. Having 2 versions of DT is annoyance for sure, but the
bigger headache is that it still won't help cases of users mixing and matching
boards and DT. IMO this runtime check is pretty nice and will support both types
of boards with exact same code/DT !

FWIW, both solutions #1 and #3 seem to imply a different DT - no ?

And I really don't see how #2 makes things more elegant/abstracted w.r.t clk
framework ?

So I prefer what you had before.
-Vineet