[PATCHv5 11/35] ARM: OMAP2+: clock: move clock provider infrastructure to clock driver

Tero Kristo t-kristo at ti.com
Thu Mar 26 03:55:27 PDT 2015


On 03/26/2015 09:24 AM, Tero Kristo wrote:
> On 03/26/2015 01:17 AM, Tony Lindgren wrote:
>> * Tero Kristo <t-kristo at ti.com> [150325 08:12]:
>>>
>>> Splits the clock provider init out of the PRM driver and moves it to
>>> clock driver. This is needed so that once the PRCM drivers are
>>> separated,
>>> they can logically just access the clock driver not needing to go
>>> through
>>> common PRM code. This would be wrong in the case of control module for
>>> example.
>> ...
>>
>>> --- a/arch/arm/mach-omap2/clock.c
>>> +++ b/arch/arm/mach-omap2/clock.c
>> ...
>>> -u32 omap2_clk_readl(struct clk_hw_omap *clk, void __iomem *reg)
>>> +u32 omap2_clk_memmap_readl(void __iomem *reg)
>>>   {
>>> -    u32 val;
>>> +    struct clk_omap_reg *r = (struct clk_omap_reg *)®
>>>
>>> -    if (clk->flags & MEMMAP_ADDRESSING) {
>>> -        struct clk_omap_reg *r = (struct clk_omap_reg *)®
>>> -        val = readl_relaxed(clk_memmaps[r->index] + r->offset);
>>> -    } else {
>>> -        val = readl_relaxed(reg);
>>> -    }
>>> +    return readl_relaxed(clk_memmaps[r->index] + r->offset);
>>> +}
>>
>> The cast from void __iomem *reg to struct clk_omap_reg *r looks still
>> nasty.. Why don't you add the IO address into struct clk_omap_reg:
>>
>> struct clk_omap_reg {
>>     u16 offset;
>>     u16 index;
>>     struct regmap *regmap;
>>     void __iomem *addr;
>> };
>> ...
>>
>> Then populate it during init and then have the clock code use it
>> directly if available? Then it seems you would not need the
>> static struct clk_iomap *clk_memmaps[CLK_MAX_MEMMAPS] at all?
>
> Doing a change like this should probably be planned, but it is a larger
> modification. Currently none of the low-level clock APIs support this,
> but instead expect a direct iomem pointer against which they can do
> arithmetic operations. The major problem is the companion clocks, which
> just XOR some bits in the registers to get ICLK / IDLEST register offset
> from FCLK.
>
> So, for now, clock code just uses the void __iomem pointer as a storage
> class for struct clk_omap_reg, on which arithmetic operations can be done.

I did this change as a trial, and this is the diff required to get it 
working:

  arch/arm/mach-omap2/clkt_iclk.c |   20 ++++++++---------
  arch/arm/mach-omap2/clock.c     |   47 
+++++++++++++++++----------------------
  arch/arm/mach-omap2/clock.h     |    8 +++----
  arch/arm/mach-omap2/clock2430.c |    5 +++--
  arch/arm/mach-omap2/clock34xx.c |   36 ++++++++++++++----------------
  arch/arm/mach-omap2/clock3517.c |   20 ++++++++---------
  arch/arm/mach-omap2/cm.h        |    4 +++-
  arch/arm/mach-omap2/cm2xxx.c    |    9 +++-----
  arch/arm/mach-omap2/cm3xxx.c    |   10 +++------
  drivers/clk/ti/clk.c            |   10 +++++----
  drivers/clk/ti/divider.c        |   24 ++++++++++++++------
  drivers/clk/ti/dpll.c           |   11 ++++-----
  drivers/clk/ti/gate.c           |   21 +++++++++++------
  drivers/clk/ti/interface.c      |    9 ++++----
  drivers/clk/ti/mux.c            |   22 ++++++++++++------
  include/linux/clk/ti.h          |    5 +++--
  16 files changed, 137 insertions(+), 124 deletions(-)

I think we should probably keep this out of this set now and do this 
while moving the OMAP core clock support code under clock driver... just 
to keep it more easily manageable.

-Tero



More information about the linux-arm-kernel mailing list