lkml.org 
[lkml]   [2013]   [Aug]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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@linaro.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@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2013-08-22 11:41    [W:0.097 / U:0.124 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site