[U-Boot] Driver Model and DTS Parsing

[Stephen Warren]

On 07/30/2014 09:26 AM, Simon Glass wrote:
> Hi Stephen,
>
> On 12 June 2014 23:31, Stephen Warren <swarren at wwwdotorg.org> wrote:
>> On 06/11/2014 10:55 PM, Simon Glass wrote:
>> ...
>>> Tegra doesn't have much in the device tree for GPIOs - it seems to be
>>> all hard-coded in the software. So I ended up with the code you saw
>>> which just iterates over a known number of banks, creating a device
>>> for each.
>>
>> That still sounds wrong. Tegra HW has a single GPIO controller that
>> exposes a bunch of GPIOs. It isn't logically divided into banks or any
>> other construct that is multi-level  Although the naming of the
>> individual GPIOs does call something a bank, that's just a name of a
>> register, not separate HW blocks. It's just going to be confusing to
>> users if the U-Boot representation doesn't match what the HW actually has.
>
> I'm getting back to this as I just re-issued the series.
>
> I don't see the mismatch you are referring to here. U-Boot people are
> used to seeing GPIOs as named banks, and the Tegra TRM uses bank names
> also.

The mismaatch is that in HW, there is a single GPIO controller that has 
a large number of GPIOs, not a number of GPIO controllers that each has 
a smaller number of GPIOs.

U-Boot's commands/APIs/... should model this directly; a single 
controller object that has a large number of GPIOs within it.

As such, an example user-visible GPIO command needs to be roughly 
something like:

# using integer IDs
gpio set tegra 128
          ^^    ^^ (controller instance name) (GPIO ID)
or:

# using names within the controller
gpio set tegra PQ0
          ^^    ^^ (controller instance name) (GPIO name)

(note that there must be separate controller ID and GPIO ID parameters 
in the commands in order to e.g. differentiate between 2 instances of 
the same I2C GPIO expander chip; something like pca9555 at a0@i2c0, 
pca9555 at i2c1@a4)

not:

gpio set tegraP 10
          ^^     ^^ (hypothetical bank name) (GPIO ID within bank)

or:

gpio set P10
          ^^ GPIO name without any controller ID

>> There's zero extra indirection caused by SW correctly describing the HW
>> as a single bank. I have absolutely no idea what you mean my extra
>> indirection here; any time there is a driver for a GPIO, you call a
>> function to set a GPIO. That doesn't change based on whether there are
>> 32 or 1 GPIO controller drivers.  The only difference is how many
>> drivers you have to search through to find the right one. For Tegra at
>> least, I'm arguing for 1 driver to match the 1 HW module.
>
> OK let me explain a bit more.
>
> At the moment, the GPIO uclass supports a single GPIO bank, defined as
> something that has a name, like A, or B.

The GPIO bank name should just be "Tegra". The Tegra TRM specifies a 
single GPIO controller, in the address map etc., and there should be a 
single U-Boot object that represents it.  Really the phrase "bank" in 
U-Boot needs to be replaced with "controller".

 > Within that bank there can be
> several individual GPIO lines. This is what the Tegra TRM refers to as
> A0, A1, B0, B1, etc.

While the TRM does use the phrase "bank", I believe this is just a 
collision with the term you happened to choose. It's not used for the 
same semantic purposes. There's no intent to divide the Tegra GPIO 
controller into a bunch of logically separate HW blocks. "bank" in the 
TRM is just a convenient word to refer the fact that more than 32 GPIOs 
are supported, so they don't all fit into a single 32-bit register.

So, the semantics of the HW are:

A single GPIO controller named "tegra".

Within that, there are a bunch of GPIOs. Each has a number e.g. 0..250 
(for Tegra124) and a name (PA0, PA1, ... PA7, PB0, PB1, ..., PFF2). 
Users should be able to refer to those GPIOs either by integer ID, or by 
name.

To support this, the GPIO uclass would need:

* A string name for the controller

* A set of functions/ops to manipulate the GPIOs (e.g. set input, set 
output, set output value) that accept integer IDs as the parameter for 
the GPIO ID.

* If GPIOs have names as well as numbers, an optional function to 
convert a string name to an integer GPIO ID.

> Should we wish to support banks A-Z, AA, BB, etc. all as a single GPIO
> device, we would need to redo the uclass to support this.

No you wouldn't. Just put all the GPIOs into a single uclass instance. 
For naming, you can have the optional string->int conversion function in 
the uclass, or perhaps just ignore names (the kernel operates on 
integers for GPIOs...).