Re: [PATCH V5] clk: bcm: Add driver for BCM53573 ILP clock

[RafaÅ MiÅecki]

On 31 August 2016 at 18:16, Rob Herring <robh@xxxxxxxxxx> wrote:
> On Thu, Aug 25, 2016 at 02:42:52PM +0200, RafaÅ MiÅecki wrote:
>> On 23 August 2016 at 21:55, Rob Herring <robh@xxxxxxxxxx> wrote:
>> > On Tue, Aug 23, 2016 at 08:25:59AM +0200, RafaÅ MiÅecki wrote:
>> >> From: RafaÅ MiÅecki <rafal@xxxxxxxxxx>
>> >>
>> >> This clock is present on BCM53573 devices (including BCM47189) that use
>> >> Cortex-A7. ILP is a part of PMU (Power Management Unit) and so it should
>> >> be defined as one of its subnodes (subdevices). For more details see
>> >> Documentation entry.
>> >>
>> >> Unfortunately there isn't a set of registers related to ILP clock only.
>> >> We use registers 0x66c, 0x674 and 0x6dc and between them there are e.g.
>> >> "retention*" and "control_ext" regs. This is why this driver maps all
>> >> 0x1000 B of space.
>> >
>> > Then describe the block as a syscon which has several functions of
>> > which clocks are one.
>>
>> This isn't clear to me, sorry, could you describe it? Would you like
>> me to update commit message or documentation? Is code fine as is?
>
> Let me put it another way, when you need to use the other registers, how
> do you plan to describe them in DT? We don't really want a node per
> register, nor do I want to get binding docs one by one as you add each
> function. Instead describe the block with all the misc functions as a
> whole. What would you call that block? Still ILP or something else?
>
> Also, you if you do have multiple drivers all needing to access this
> single block, then that is when you need syscon and regmap.

I got this now, thanks for your patience.

One last question: what is preferred register reference:
1) Parent-child (of_get_parent + syscon_node_to_regmap)
or
2) DT property (syscon_regmap_lookup_by_phandle(dt, "foo"))

-- 
RafaÅ