Re: [PATCH 21/33 v2] clk: ux500: Add Device Tree support for the PRCC Kernelclock

From: Mike Turquette
Date: Tue Jun 18 2013 - 17:17:45 EST


Quoting Arnd Bergmann (2013-06-12 07:46:30)
> On Tuesday 11 June 2013, Lee Jones wrote:
> > This patch enables clocks to be specified from Device Tree via phandles
> > to the "prcc-kernel-clock" node.
> >
> > Cc: Mike Turquette <mturquette@xxxxxxxxxx>
> > Cc: Ulf Hansson <ulf.hansson@xxxxxxxxxx>
> > Signed-off-by: Lee Jones <lee.jones@xxxxxxxxxx>
> >
>
> I don't understand the design of the common clock subsystem here, but is it really
> necessary to hardcode all the clocks using clk_reg_prcc_kclk() here *and* register
> a clkdev *and* store the pointer in an array, when you can get all that information
> from the device tree?
>
> Mike?

I'm a bit confused by what is going on here too. There are several
different ways to handle this.

1) Since you have your own clock driver (e.g. not the basic clock types)
then you could expand the argument list of clk_reg_prcc_kclk to include
the clkdev dev_id string and toss the call to clk_register_clkdev inside
of clk_reg_prcc_kclk.

2) Move your clock data into DT and teach the driver to use of_clk_get
to fetch the clk phandle directly instead of using the string-matching
clkdev mechanisms. Of course both your clock and device bits need to be
converted to DT first.

Can you explain what prcc_kclk[] and friends are doing? Is that a legacy
clock framework thing that is still hanging around? I'm curious to know
why your clock driver needs a list of the clocks it registers.

Regards,
Mike

>
> Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/