All of lore.kernel.org
 help / color / mirror / Atom feed
From: Anton Vorontsov <avorontsov@ru.mvista.com>
To: Matt Sealey <matt@genesi-usa.com>
Cc: Mitch Bradley <wmb@firmworks.com>,
	linuxppc-dev list <linuxppc-dev@ozlabs.org>,
	devicetree-discuss list <devicetree-discuss@ozlabs.org>
Subject: Re: GPIO - marking individual pins (not) available in device tree
Date: Tue, 28 Oct 2008 05:37:22 +0300	[thread overview]
Message-ID: <20081028023722.GA30057@oksana.dev.rtsoft.ru> (raw)
In-Reply-To: <490666B0.2010500@genesi-usa.com>

On Mon, Oct 27, 2008 at 08:11:12PM -0500, Matt Sealey wrote:
> Anton Vorontsov wrote:
>> On Mon, Oct 27, 2008 at 04:56:57PM -0500, Matt Sealey wrote:
>>>
>>> A bunch of things spring to mind:
>>>
>>> *   How do you define a GPIO bank in a device tree, not a controller
>>
>> Not a controller? What do you mean by "bank"? There is no such
>> thing. There are GPIO controllers, and _maybe_ _their_ banks of
>> GPIOs.
>
> I don't know what you want to call it, I don't know what the official
> Linux term might be for a grouped bunch of GPIO used for a peripheral.

You don't know this term because there isn't any. There is no term
for "bunch of GPIO used for a peripheral".

But there are:

- gpio controllers;
- devices that use some gpios;
- gpios = <>; property that is used to denote which gpios the device
  is using.

What's so hard about that?

>>>     but a grouping of pins which fit that pattern, of which there
>>>     may be multiple groupings for multiple peripheral functions
>>
>> Why do you need this, _exactly_? What problem this would solve? But
>> see below, this is still possible to implement.
>
> I made a pretty good example with the lcd controller, I thought..

Yes, thanks for the example. And I don't see any problems with
describing the lcd controller.

>> But I still wonder what problem exactly you're trying to solve
>> here? Prevent requesting reserved gpios? I don't see any point
>> in this, really.
>
> There's plenty of reason for it, it's totally wrong to be able
> to request a GPIO which has been multiplexed to another device
> in the SoC, and probably completely undefined behaviour if you
> start toggling a pin that's been assigned to an internal
> controller.

Then just don't specify the wrong GPIO in the gpios = <>! It is
that simple.

You don't specify the SOC's peripheral memory in the /memory node,
do you? I bet you don't. Can you? Yes. Bad things will happen? Sure.

You may bring the gpio sysfs example. But I can answer back with the
/dev/mem example. There are plenty of things you can do bad things
with, even being in the userspace.

> Since you have no way of knowing except intimate board design
> knowledge in the driver.. well, that defeats the purpose of the
> device tree in general and is a step back into hardcoded information
> in board support from arch/ppc days etc..
> 
> This information SHOULD be in the device tree specifically because
> there is one very popular chip here which has GPIO multiplexed with
> other very-often-used peripherals, and I can think of at least 5
> other chips which also have device trees and GPIO potential (4 of
> which we have designed boards around) which fit the same model and
> would have the same requirement.
>
> There are two ways you can do it; implicit and explicit. If a
> pin is not defined for a peripheral in the device tree, it is
> not requestable from GPIOLIB. Or, you make sure that you specify

This is doable and _maybe_ a good idea. Though I don't see much
value in this.

>> No problem. Define bindings for "8 GPIOs data group".
>>
>> /* 8bit Parallel I/O port using arbitrary gpios */
>> byte_pio: byte-pio {
>> 	compatible = "byte-pio";
>> 	gpios = <&controller1 0 1   /* data line 0 */
>> 		 &controller2 12 0  /* data line 1 */
>> 		 ...>;
>> };
>>
>> lcd-controller {
>> 	/* the lcd controller can simply use the pio_out_8(),
>> 	 * pio_in_8() API. The API is easily implemented. */
>> 	lcd-data-port = <&byte_pio>;
>> }
>>
>>> and 1 for data/control selection, it would be nice for the
>>> software to know which pin is which and, slightly unrelated,
>>> which way around the data pins went - MSB first or LSB first
>>> - from the device tree, as this is a BOARD LEVEL DESIGN DECISION
>>> which is what the device tree is meant to abstract - in the same
>>> way we define i2c controllers and clients?)
>
> And this? What about the other two control lines? Do they just get
> set in the gpios property of the lcd-controller? How do you determine
> which is which?

This depends on how will you write the bindings.

> Comments don't get compiled..

If you _really_ want to complicate things, you can write gpios for
devices like this:

device {
	data0-gpio = <&controller1 1 1>;
	...
	rw-gpio = <&controller1 2 1>;
}

This _will_ get compiled in. This is insane, but this is more friendly
to a device tree reader, if you like.

> and a driver will have
> to be written FROM the comments, hardcoding the pins into it, again.

What do you mean by "hardcoding"? Let's see: interrupts = <33 32>;
and then extracting them via
of_irq_to_resource(node, 0, &tx_irq),
of_irq_to_resource(node, 1, &rx_irq).

Does this mean "hard-coding"?

[...]
>>> I don't feel the current spec actually takes this into account
>>> and the patch submission the other day from Wolfgang made me
>>> think that if a "base" register is the obvious solution to some
>>> problem, then it either can't be that clear to others (i.e. it
>>> is not just me being stupid) or it is simply not possible under
>>> the current binding.
>>
>> Wolfgang's patch proved to be unnecessary, you haven't seen
>> the solution?
>
> Wolfgang's patch would never have been considered by Wolfgang
> if the solution wasn't more obvious... this is why I started
> the thread :D

Oh, device tree/correct bindings/proper ways to solve things
aren't always obvious? It's news to me. ;-)

[...]
>> I don't know how did you come to these conclusions. Pretty much
>> people were involved into the gpio bindings discussion.
>
> I was watching the threads and I expected a little more than 20
> lines of documentation and a couple LED drivers would come out
> of it.
>
> GPIOLIB excited me when I talked to the original author about
> it, now that there has to be a device tree behind it that is
> an absolutely undefined, gpio-controller-specific implementation
> for every chip

You seem to disagree with the whole device tree idea and the OF
in general. Interrupts are so controller-specific stuff that
we should reconsider using it, right?

> or even every board with significant information
> hardcoded into drivers, I am considerably less enthused.

What's so bad about board-specific drivers for board-specific
devices? If a gpio-header is board-specific, then we have to
write a board-specific driver for it. To make things better
we can write the driver in a such way that the driver could be
easily adopted/extended for similar boards/setups.

OR we can write bindings that could fully describe the gpio
header, and then just use the universal driver for it.

Both solutions are doable.

>>> Yes, your idea, Mitch's discussion was great, I just don't think
>>> anything will get done about it (emotional moment) as usual.
>>
>> Hm. If idea looks ok, then it is matter of implementation. And
>> the gpio-header case is quite easy to implement, really.
>
> Okay. I think I am understanding this enough that I could write
> a bit more comprehensive documentation for it that would
> encompass the result of this discussion and some other things
> going on right now...
>
> As for implementation, since the only hardware I have here to
> test is a) broken b) only has 3 GPIO pins or b) working but
> such a horrid design I wouldn't use it as a doorstop, I don't
> think I am really qualified to put it into action..

Yeah, there are always excuses when it comes to send patches... ;-)

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

  reply	other threads:[~2008-10-28  2:37 UTC|newest]

Thread overview: 76+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-10-23 21:32 GPIO - marking individual pins (not) available in device tree Matt Sealey
2008-10-23 21:32 ` Matt Sealey
2008-10-23 22:22 ` Mitch Bradley
2008-10-23 23:05   ` Matt Sealey
2008-10-24  0:52     ` Mitch Bradley
2008-10-24  3:29       ` David Gibson
2008-10-24  3:29         ` David Gibson
2008-10-24  4:17         ` Mitch Bradley
2008-10-24  4:17           ` Mitch Bradley
2008-10-24  4:45           ` David Gibson
2008-10-24  4:45             ` David Gibson
2008-10-24 22:14             ` Matt Sealey
2008-10-26 23:47               ` David Gibson
2008-10-27 15:40                 ` Matt Sealey
2008-10-27 18:34                   ` Anton Vorontsov
2008-10-27 18:56                     ` Matt Sealey
2008-10-27 20:10                       ` Anton Vorontsov
2008-10-27 20:10                         ` Anton Vorontsov
2008-10-27 21:56                         ` Matt Sealey
2008-10-27 21:56                           ` Matt Sealey
2008-10-27 23:12                           ` Anton Vorontsov
2008-10-27 23:12                             ` Anton Vorontsov
2008-10-27 23:40                             ` Anton Vorontsov
2008-10-28  0:47                               ` Matt Sealey
2008-10-28  1:11                             ` Matt Sealey
2008-10-28  1:11                               ` Matt Sealey
2008-10-28  2:37                               ` Anton Vorontsov [this message]
2008-10-28 16:53                                 ` Matt Sealey
2008-10-28 16:53                                   ` Matt Sealey
2008-10-28 17:39                                   ` Grant Likely
2008-10-28 19:46                                     ` Matt Sealey
2008-10-28 19:46                                       ` Matt Sealey
2008-10-28  0:15                   ` David Gibson
2008-10-28  0:15                     ` David Gibson
2008-10-28  0:51                     ` Matt Sealey
2008-10-28  1:50                       ` David Gibson
2008-10-28  5:20                       ` Grant Likely
2008-10-28  5:20                         ` Grant Likely
2008-10-24 22:03       ` Matt Sealey
2008-10-24 22:20         ` Stephen Neuendorffer
2008-10-24 22:20           ` Stephen Neuendorffer
2008-10-26 21:39           ` Matt Sealey
2008-10-24 23:44         ` Mitch Bradley
2008-10-26 21:13           ` Matt Sealey
2008-10-26 21:13             ` Matt Sealey
2008-10-26 23:53             ` David Gibson
2008-10-27 16:12               ` Matt Sealey
2008-10-27 16:12                 ` Matt Sealey
2008-10-27 16:35                 ` Scott Wood
2008-10-27 16:35                   ` Scott Wood
2008-10-27 17:05                   ` Matt Sealey
2008-10-27 17:25                     ` Scott Wood
2008-10-27 17:49                       ` Matt Sealey
2008-10-27 17:54                         ` Scott Wood
2008-10-28  0:38                           ` David Gibson
2008-10-28  0:38                             ` David Gibson
2008-10-28  0:34                 ` David Gibson
2008-10-28  0:34                   ` David Gibson
2008-10-24  4:58     ` David Gibson
2008-10-24  3:27   ` David Gibson
2008-10-24  3:27     ` David Gibson
2008-10-24 16:41 ` Anton Vorontsov
2008-10-24 17:01   ` Anton Vorontsov
2008-10-24 22:17     ` Matt Sealey
2008-10-24 22:17       ` Matt Sealey
2008-10-24 22:37       ` Anton Vorontsov
  -- strict thread matches above, loose matches on Subject: below --
2008-10-28 13:31 Konstantinos Margaritis
2008-10-28 14:11 ` Anton Vorontsov
2008-10-28 14:11   ` Anton Vorontsov
2008-10-28 14:15 ` Grant Likely
2008-10-28 14:15   ` Grant Likely
2008-10-28 17:06   ` Matt Sealey
2008-10-28 17:06     ` Matt Sealey
2008-10-28 17:32     ` Grant Likely
2008-10-28 23:37 ` David Gibson
2008-10-28 23:37   ` David Gibson

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=20081028023722.GA30057@oksana.dev.rtsoft.ru \
    --to=avorontsov@ru.mvista.com \
    --cc=devicetree-discuss@ozlabs.org \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=matt@genesi-usa.com \
    --cc=wmb@firmworks.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.