linux-kernel - Re: [PATCH 21/33] clk: ux500: Add Device Tree support for the PRCC Kernel clock

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:	Thu, 22 Aug 2013 10:21:30 +0100
From:	Lee Jones <lee.jones@...aro.org>
To:	Linus Walleij <linus.walleij@...aro.org>
Cc:	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Arnd Bergmann <arnd@...db.de>,
	Linus WALLEIJ <linus.walleij@...ricsson.com>,
	Srinidhi KASAGAR <srinidhi.kasagar@...ricsson.com>,
	Mike Turquette <mturquette@...aro.org>,
	Ulf Hansson <ulf.hansson@...aro.org>
Subject: Re: [PATCH 21/33] clk: ux500: Add Device Tree support for the PRCC
 Kernel clock

On Thu, 22 Aug 2013, Linus Walleij wrote:

> On Wed, Aug 21, 2013 at 12:14 PM, Lee Jones <lee.jones@...aro.org> wrote:
> > On Wed, 21 Aug 2013, Linus Walleij wrote:
> 
> >> Isn't it possible to fork a function u8500_clk_init_dt() to add all the
> >> clocks in the DT probe path and keep this function
> >> u8500_clk_init() as it is?
> >
> > Yes, we probably could do that, but as we're ripping out ATAG booting
> > support from the ux500 platform, I'll just remove the
> > clk_register_clkdev()s during the process.
> 
> I really do not like the approach of uglifying something and then
> beautifying it later... I prefer each step in isolation to be good
> looking, or you will be confused when traversing the history.

So then we have a few options, some more realistic than others.

1. Duplicate each of the; clk_reg_prcmu_*(), clk_reg_prcc_pclk(),
clk_reg_prcc_kclk() calls into your proposed u8500_clk_init_dt(),
which, while keeping everything separate would be unrealistic.

2. Move both clk_register_clkdev() and the struct clock arrays into
'drivers/clk/ux500/clk-prcmu.c' and 'drivers/clk/ux500/clk-prcc.c' and
make the correct decisions in clk_reg_prcmu() and clk_reg_prcc(). This
would be more viable, but would entail splitting the defines and the
struct clock arrays (stores), probably creating a little more
disparity. It would also mean adding 3 parameters (con_id, dev_fmt and
array_index) to each of; clk_reg_prcmu_gate(),
clk_reg_prcmu_scalable(), clk_reg_prcmu_opp_gate(),
clk_reg_prcmu_scalable_rate(), clk_reg_prcmu_rate(),
clk_reg_prcmu_opp_volt_scalable, clk_reg_prcmu() and 4 parameters
(con_id, dev_fmt, array_index_x, array_index_y) to each of
clk_reg_prcc_pclk(), clk_reg_prcc_kclk() and clk_reg_prcc().

Or, the most viable option:

3. Leave it as it is. All we're doing here is populating an array of
pointers. It costs practically nothing, continues legacy ATAG support
as well as adding Device Tree support. It's far neater than the other
two options, by a long shot. And then, when we're ready to take out
ATAG booting support (which I'm working on right now), we just remove
the clk_register_clkdev() calls and we're left with the nice and neat
macros.

Unless there's a viable 4th option which hasn't popped into my head
just yet. I'm all hears. :)

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/