Re: [PATCH] of: Add a reg-names property to name reg entries

["Cousson, Benoit" <b-cousson@ti.com>]

On 10/26/2011 5:57 AM, Segher Boessenkool wrote:
>>>>> What problem does any of this solve?  The device binding for the
>>>>> "mcasp" device will have to describe the possible "reg-names", and
>>>>> what those mean; but the binding already has to describe its "reg"
>>>>> property anyway.
>>>>
>>>> What this solve is the ability to use the
>>>> platform_get_resource_byname directly to retrieve the proper
>>>> register base address.
>>>
>>> You do not have to put it in the device tree for that, the device
>>> driver can implement this itself if it cares.
>>
>> ???
>>
>> The driver is the user of that name, so it has to be populated
>> before into the resource during device creation.
>
> It can be as simple as
>
> #define FOO_REG_INDEX 0
> #define BAR_REG_INDEX 1
>
> but you can use C strings if you want to.

But that will add an extra level of indirection...

The idea is to avoid that and use directly:

platform_get_resource_byname(pdev, IORESOURCE_MEM, "smem");
platform_get_resource_byname(pdev, IORESOURCE_MEM, "cmem");
....

>>>> The binding is just a text description that the driver will not be
>>>> able to use directly. It will have to get the resource using an
>>>> abstract index.
>>>
>>> Your reg-names are abstract identifiers just as well.
>>
>> This is the name used in the HW documentation.
>
> So?  The device binding will still have to list them, for exact spelling
> and register block size etc.

The binding is *just* a text... so instead of having to read a text file 
and then getting an index, we can just use the text that represent the 
memory entry.

>> Ordering them to get a number is purely arbitrary.
>
> Ordering them is required, "reg" is an array.  It also matters which
> entry is the first entry (for the unit address).
>
>> That's why using the name will allow the driver to get the resource
>> the way it is represented in the documentation and thus avoid some
>> intermediate number.
>
> You use an intermediate string instead, and add code and binding to
> translate that to that same number.

Nope. We use a string to get the resource directly. There is no 
intermediate index.

>>>> It thus removes a level of indirection that is error prone and
>>>> useless most of the time.
>>>
>>> It *adds* a level of indirection.  I doubt it helps prevent errors
>>> either, but who knows.
>>
>> Well, if that does not bring anything to you, you can just not use it.
>
> I could, if you did this for your device binding only, but it seems
> you are adding this as a generic thing.  I am very much against that.
> The device tree should describe the hardware; it shouldn't describe
> the description of the hardware, that's what bindings are for.

Not really, it is still a representation of the HW using a more readable 
way. I kept the original "reg" for legacy only.

This idea is to extend that to any kind of resources attached to a 
device: irq, dma_req, clock, regulator...
FWIW, using a name is already the natural way to get a clock and a 
regulator.
The point here is to extend legacy binding and thus improve the 
consistency in the device resource representation.

Benoit