devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tony Lindgren <tony@atomide.com>
To: Stephen Warren <swarren@wwwdotorg.org>
Cc: Linus Walleij <linus.walleij@linaro.org>,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, linux-omap@vger.kernel.org,
	devicetree-discuss@lists.ozlabs.org,
	Stephen Warren <swarren@nvidia.com>,
	Arnd Bergmann <arnd@arndb.de>,
	Grant Likely <grant.likely@secretlab.ca>,
	Rob Herring <rob.herring@calxeda.com>
Subject: Re: [PATCH] pinctrl: Add one-register-per-pin type device tree based pinctrl driver
Date: Sun, 17 Jun 2012 22:50:36 -0700	[thread overview]
Message-ID: <20120618055036.GU12766@atomide.com> (raw)
In-Reply-To: <4FDB6000.5080703@wwwdotorg.org>

* Stephen Warren <swarren@wwwdotorg.org> [120615 09:21]:
> On 06/15/2012 03:49 AM, Tony Lindgren wrote:
> 
> (Arnd, Grant, Rob, CC'ing you mainly re: the very last set of comments
> in this email; can you take a look at Tony's patch and comment on the
> binding)

Yes please take a look.
 
> > Care to clarify a bit what you had in mind there?
> 
> Basically I meant:
> 
> 1) Remove anything TI-/OMAP-specific from the generic binding document
> 
> 2) For any TI-/OMAP-specific additions, create a binding document that
> describes those separately to the generic binding documentation.
> 
> However, given that there are no TI-/OMAP-specific additions; OMAP2 just
> uses the base generic bindings exactly as defined, I don't think you
> actually need to create an TI-/OMAP-specific document, since there's
> nothing for it to document.

OK so basically you just want the pinctrl-single binding document to be
generic. Yes I'll do that.
 
> > It's best to describe the hardware in case we need to set up some hardware
> > specific things in the driver.
> 
> There's a difference between the (set of) compatible value(s) that
> appears in the .dts file, and the compatible value(s) that the driver
> binds to.
> 
> Since this driver is purely implementing the pinctrl-single binding, I
> believe only that should be listed in the driver's of_match table.
> 
> However, since the .dts file is describing HW that is both a specific
> OMAP instance, and compatible with pinctrl-single, the .dts file can
> certainly list both. If in the future, the driver ever needs to
> specifically know the difference between the specific OMAP versions, or
> OMAP-vs-something-else, the compatible value will be there already in
> the .dts file, so this will all work. However, I'd argue that if the
> driver has to ever know that, it's probably not appropriate to say it's
> compatible with pinctrl-single any more...

Yes OK I see what you mean now. Sounds good to me, and if/when we need
some hardware specific things, we'll add specific parsing for the other
compatible values.
 
> >> One final comment: A little while before Linaro Connect in San
> >> Francisco, we were all having difficulty coming up with any kind of DT
> >> binding for pinctrl. I had suggested the possibility of creating a
> >> binding which just said "write value X to register Y, write value P to
> >> register Q, ...". This was rejected at connect, because a raw list of
> >> register writes didn't document anything or describe semantics - it was
> >> too much of a binary blob. This driver seems to be doing almost the
> >> exact same thing. In order to avoid that assertion, it'd need to truly
> >> have individual representations of which pins exist, which pingroups
> >> exist, which functions exist, and which functions can be selected onto
> >> which pins/pingroups. That would be a radically different binding. I
> >> wonder what's changed such that this kind of driver wasn't acceptable
> >> then, but is now?
> > 
> > Well that's why I had two bindings earlier: The current binding for the
> > bulk pins that's faster to parse, and a more verbose binding for drivers that
> > allows naming the functions.
> > 
> > In your previous comments at [1] you seemed to prefer this current binding
> > based on the "Why not only allow (1)?" comment. Maybe you had something else
> > in mind, care to clarify that a bit too?
> 
> Well, first I thought about semantics vs. just banging a register list a
> tiny bit more, and remembered the previous discussion.
> 
> But my comment at [1] wasn't so much about "why not just have a raw
> register list", but more regarding why support two different
> representations within the same binding; it wasn't really clear to me
> from reading the original email what the difference between the two
> representations; why have two representations, why/when-to use one vs.
> the other, that the difference was about providing a "semantic" list of
> pins/pingroups/functions vs. providing a "raw" register list.

I too would prefer more verbose .dts files. Unfortunately the parsing of
several hundred board specific pins adds quite a bit of overhead.
 
> IIRC, the pushback on a raw "bang these registers" representation came
> from Grant and/or Arnd. It'd be a good idea if they (and indeed Rob)
> could comment on this binding proposal. If they're OK with this kind of
> pretty raw representation, I'm not going to object, but I have a feeling
> they might not like it?

Yes let's see if anybody has better ideas.

Regards,

Tony

 
> > [1] http://article.gmane.org/gmane.linux.kernel/1291838

  reply	other threads:[~2012-06-18  5:50 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-06-11 13:58 [PATCH] pinctrl: Add one-register-per-pin type device tree based pinctrl driver Tony Lindgren
     [not found] ` <20120611135826.GB12766-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
2012-06-14 23:12   ` Stephen Warren
2012-06-15  9:49     ` Tony Lindgren
2012-06-15 16:17       ` Stephen Warren
2012-06-18  5:50         ` Tony Lindgren [this message]
2012-06-19 13:56           ` Tony Lindgren
2012-06-21  8:09             ` Linus Walleij
     [not found]             ` <20120619135600.GX12766-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
2012-06-21 22:13               ` Stephen Warren
     [not found]                 ` <4FE39C86.5070901-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2012-06-22  8:39                   ` Tony Lindgren
2012-06-22 17:32                     ` Stephen Warren
2012-06-26 13:43                       ` Tony Lindgren
2012-06-26 17:05                         ` Stephen Warren
2012-06-27 10:28                           ` Tony Lindgren
2012-07-10  9:11                             ` Tony Lindgren
     [not found]                               ` <20120710091131.GQ1122-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
2012-07-14 20:16                                 ` Linus Walleij
2012-07-16  7:10                                   ` Tony Lindgren

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=20120618055036.GU12766@atomide.com \
    --to=tony@atomide.com \
    --cc=arnd@arndb.de \
    --cc=devicetree-discuss@lists.ozlabs.org \
    --cc=grant.likely@secretlab.ca \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=rob.herring@calxeda.com \
    --cc=swarren@nvidia.com \
    --cc=swarren@wwwdotorg.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).