Re: [PATCH v2 05/11] ARM: dt: tegra30: Add device node for APB MISC

[Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>]

On 01/03/2013 06:48 PM, Prashant Gaikwad wrote:
> On Thursday 03 January 2013 09:41 PM, Stephen Warren wrote:
>> On 01/02/2013 11:11 PM, Prashant Gaikwad wrote:
>>> On Thursday 03 January 2013 03:30 AM, Stephen Warren wrote:
>>>> On 12/27/2012 07:47 AM, Prashant Gaikwad wrote:
>>>>> APB misc contains multiple registers required by different modules
>>>>> such as CAR.
>>>> I don't see a DT binding document that describes what
>>>> nvidia,tegra30-apbmisc means. Also, the register range for this new
>>>> node
>>>> overlaps that for the pinmux node, so they can't both "request" their
>>>> register region. You may need multiple entries in the apbmisc reg
>>>> property to avoid this.
>>> apbmisc reg for Tegra30 can be divided into following entries:
>>>
>>> strap registers
>>> jtag configuration registers
>>> pull_up/pull_down control registers
>>> vclk control registers
>>> tvdac registers
>>> chip id revision registers
>>> pad control registers
>>>
>>> This list is not same for Tegra20 and Tegra30.
>> OK. It sounds like we need a true APB MISC driver then, to abstract the
>> differences; the clock driver really shouldn't be touching the APB MISC
>> registers in all likelihood, unless a subset of the sections you mention
>> above are truly dedicated to clock functionality.
> 
> I don't think it is a good idea to create a driver for APB MISC, all
> registers are used by different drivers.

Well, it's even worse to have a bunch of other drivers randomly trample
on a set of registers they don't own.

> Only chip id revision registers are used in clock driver.

There are already global variables exposed by the Tegra fuse driver; can
you just read those?

>>> OR
>>>
>>> another way is to add chip id revision register region to CAR node as
>>> done for pinmux node and remove apb misc node.
>>
>> The pinmux controller doesn't have a reg entry for the chip ID register.
>> I don't understand what you mean here.
> 
> I mean as we have separate entry for PAD control registers region in
> pinmux node we can have also have separate entry for chid id revision
> register region in CAR node.

The pad control registers are part of the pinmux HW, so it makes perfect
sense for the pinmux driver to control them. The APB misc registers
aren't part of the clock register set, so it doesn't make sense to the
clock driver to touch them.