LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Anton Vorontsov <avorontsov@ru.mvista.com>
To: Grant Likely <grant.likely@secretlab.ca>
Cc: linux-kernel@vger.kernel.org, linuxppc-dev@ozlabs.org,
	David Brownell <dbrownell@users.sourceforge.net>,
	Pierre Ossman <drzeus-mmc@drzeus.cx>
Subject: Re: [PATCH 1/3] powerpc: Add mmc-spi-slot bindings
Date: Fri, 31 Oct 2008 02:02:53 +0300	[thread overview]
Message-ID: <20081030230253.GA11765@oksana.dev.rtsoft.ru> (raw)
In-Reply-To: <fa686aa40810301337p21ac8785gb74b12904bbd08e7@mail.gmail.com>

On Thu, Oct 30, 2008 at 02:37:31PM -0600, Grant Likely wrote:
[...]
> > +- gpios : (optional) may specify GPIOs in this order: Card-Detect GPIO,
> > +  Write-Protect GPIO.
> 
> I wonder if we're following the example of irq mappings too closely
> for the gpios property.  I like the layout of the property
> (<controller> <specifier>), but I think the 'gpios' name is getting
> too overloaded.  In this case a single property 'gpios' is being used
> to encode 2 unrelated bits of information; the write protect pin and
> the card detect pins.
> 
> In this particular case I think it is better to use 2 properties in
> this case; something like 'spi-writeprotect-gpio' and
> 'spi-carddetect-gpio' using the same specifier format.  Doing so adds
> a bit more clarity to the purpose of the properties.
> 
> I my mind I differentiate this from other examples (for instance a
> series of CS pins) based on how closely related the pin functions are.
>  So I would say for the following examples...
> 1) GPIO data bus (SPI, MDIO and I2C are great examples); all pins must
> be present - single gpio property
> 2) This MMC case (pins are optional and unrelated); separate gpio properties
> 3) LCD with backlight and contrast control pins; one gpio property for
> backlight pins, one for constrast pins.
> 
> Thoughts?

It's pretty trivial to implement (of_get_named_gpio() -- could be just
factored out of of_get_gpio()).

Though,

1. The idea is quite extreme. It needs discussion, and furthermore,
   we need to define when do we use gpios = <> and when something-gpio =
   <>; We need to be consistent, and to be consistent, the rules should
   be clear and written.

2. We should think about it very very carefully. Do we want to lose the
   track of gpios? For example, there are quite defined rules when (and
   in what properties) you may encounter memory addresses, when and
   where you can encounter interrupt specifiers. We do the same for
   gpios, and so far it works great. We need to think about any possible
   drawbacks of the scheme you purpose (we would never know where to
   expect gpios - it isn't a problem per se, but maybe it could lead
   to some problem in future? I don't know.)

Quite honestly I don't like the idea... maybe I just used to
interrupts = <>, reg = <>, ranges = <>, interrupt-map = <> and so
forth, and now my subconsciousness tells me "it's wrong to do
something-interrupt = <> stuff." ;-)

Anyway, your proposal is forward and backward compatible with the
existing scheme, and can even coexist. Thus I'd prefer to stay with
the today's gpios = <>. We can always start use the new scheme when
it will be thought out enough.

Thanks,

p.s. I'd prefer a new thread for this discussion, somewhere
in devicetree-discuss@ozlabs.org, so that it won't relate to this
particular patch.

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

  reply	other threads:[~2008-10-30 23:03 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-10-30 19:55 [PATCH 0/3 RFC] MMC SPI support for OpenFirmware platforms Anton Vorontsov
2008-10-30 19:56 ` [PATCH 1/3] powerpc: Add mmc-spi-slot bindings Anton Vorontsov
2008-10-30 20:37   ` Grant Likely
2008-10-30 23:02     ` Anton Vorontsov [this message]
2008-10-30 23:24       ` David Gibson
2008-10-30 23:28       ` Anton Vorontsov
2008-10-30 19:56 ` [PATCH 2/3] mmc: Add mmc_vddrange_to_ocrmask() helper function Anton Vorontsov
2008-11-08 20:55   ` Pierre Ossman
2008-11-26 19:54     ` [PATCH v2] " Anton Vorontsov
2008-11-30 20:06       ` Pierre Ossman
2008-12-01 11:53         ` Anton Vorontsov
2008-12-14 17:28           ` Pierre Ossman
2008-10-30 19:56 ` [PATCH 3/3] mmc_spi: Add support for OpenFirmware bindings Anton Vorontsov
2008-11-08 20:50 ` [PATCH 0/3 RFC] MMC SPI support for OpenFirmware platforms Pierre Ossman

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=20081030230253.GA11765@oksana.dev.rtsoft.ru \
    --to=avorontsov@ru.mvista.com \
    --cc=dbrownell@users.sourceforge.net \
    --cc=drzeus-mmc@drzeus.cx \
    --cc=grant.likely@secretlab.ca \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@ozlabs.org \
    --subject='Re: [PATCH 1/3] powerpc: Add mmc-spi-slot bindings' \
    /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

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).