linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: b-cousson@ti.com (Cousson, Benoit)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] of: Add a reg-names property to name reg entries
Date: Wed, 26 Oct 2011 19:40:45 +0200	[thread overview]
Message-ID: <4EA8461D.6070600@ti.com> (raw)
In-Reply-To: <FD4B90D7-4B26-42C2-8CF9-7AA9971BCBEB@kernel.crashing.org>

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

      parent reply	other threads:[~2011-10-26 17:40 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-10-24 15:54 [PATCH] of: Add a reg-names property to name reg entries Benoit Cousson
2011-10-24 22:19 ` Grant Likely
2011-10-24 22:56   ` Cousson, Benoit
2011-10-25  4:49     ` Grant Likely
2011-10-25  8:26 ` Tony Lindgren
2011-10-25 10:29   ` Segher Boessenkool
2011-10-25 13:40     ` Cousson, Benoit
2011-10-25 14:17       ` Segher Boessenkool
2011-10-25 16:10         ` Cousson, Benoit
2011-10-26  3:57           ` Segher Boessenkool
2011-10-26 12:23             ` Tony Lindgren
2011-10-26 17:40             ` Cousson, Benoit [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4EA8461D.6070600@ti.com \
    --to=b-cousson@ti.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).