[U-Boot] Driver Model and DTS Parsing

[Simon Glass]

Hi Stephen,

On 30 July 2014 16:47, Stephen Warren <swarren at wwwdotorg.org> wrote:
> 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
>

This will require enhancing the gpio command further, right?

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

But that would change the meaning - at present a GPIO device in U-Boot
is a GPIO bank.

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

As an aside, using this logic, it is odd that there are only 8 GPIOs
per bank, instead of 32?

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

OK here we are talking about enhancing the uclass interface to support
conversion of names into numbers. I would prefer to have that logic be
common, and sit a level higher than the driver, because:

1. It avoids duplicating the same kind of lookup in each driver -
instead the code is in one place
2. It allows U-Boot to ensure that there are no conflicts when two
devices 'claim' the same name

I can see a future where we enhance the gpio command to be able to
specify the relevant GPIO device (in fact this has already been
discussed in another context and is a natural evolution of driver
model for many commands in U-Boot, such as 'load'). I can then see
that we might push the logic down into the driver and resolve
conflicts by requiring that the device is always specified (might this
be a pain though? An extra argument that is almost always
superfluous).

Still, I would prefer to take things a step at a time, and avoid
changing the gpio command, etc. The driver model conversion is not
supposed to be a big bang, I will be most happy if we can move over
without people having to adjust their scripts and understanding of
U-Boot. The driver model change should be 'behind the scenes' and
transparent to U-Boot users.

Regards,
Simon