[U-Boot] Driver Model and DTS Parsing

Stephen Warren swarren at wwwdotorg.org
Wed Jul 30 21:57:37 CEST 2014


On 07/30/2014 10:09 AM, Simon Glass wrote:
> 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?

Sure, but that's going to be needed irrespective of this discussion, 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.

So just define the Tegra GPIO controller as having a single bank. The 
fact that U-Boot and Tegra both chose the word "bank" to mean different 
things doesn't mean that the U-Boot term has to be forced to apply to 
Tegra where the documentation talks about a "bank".

I don't think "bank" is a good description or level of abstraction 
anyway; U-Boot should use the term "controller", "chip", or "module" 
(which would apply to an entire HW module or GPIO expander chip), since 
"bank" is usually an internal implementation detail rather than 
something the user cares about. Put another way: all banks in a 
controller/chip/module/... would typically use the same 
operation/op/callback functions, just with different GPIO IDs or 
per-GPIO data, whereas different controllers/chips/modules/... would use 
different operation/op/callback functions. It therefore makes much more 
sense to abstract at the level of controller/chip/module/... rather than 
"bank" level, which is an internal implementation detail.

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

This may have to do with the HW module having been designed for an 8-bit 
bus, and then ported to HW with a larger register bus?

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

This can easily be avoided by using utility functions. Put the name->ID 
conversion function pointer into the uclass struct. For HW that follows 
a common conversion algorithm, have the driver fill in that pointer with 
a common function provided by the core rather than custom code. That 
common function could use some data in the uclass struct to perform the 
conversion (e.g. base GPIO ID, name prefix string, etc.)

> 2. It allows U-Boot to ensure that there are no conflicts when two
> devices 'claim' the same name

That would imply U-Boot making some run-time decisions about the naming, 
which I don't think would work.

After all, most GPIO controllers will simply number their GPIOs 0..n. 
There's guaranteed to be a conflict in this case any time you have more 
than 1 GPIO controller in the system. The way to solve this is to refer 
to GPIOs by the tuple (controller name, GPIO name/ID) rather than trying 
to map all GPIO names/IDs into a single namespace. Mapping everything 
into a single namespace means somehow modifying the GPIO names so 
they're unique.

(Now the string representation of that tuple could well be to 
concatenate the controller name plus some seperator plus the GPIO name, 
and the GPIO DM core could do the parsing to split those two parts apart 
before calling the per-GPIO-controller name->GPIOID conversion function, 
but that's semantically the same thing)

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

If you want to avoid changing the GPIO command, then the only choice is 
to map each GPIO controller's per-device integer GPIO ID space into some 
global GPIO ID space as is done today. In that case, there are no GPIO 
names, and GPIO bank/controller/... names aren't even relevant since 
they aren't user-visible. As such, I even more don't see the objection 
to modelling the Tegra GPIO controller as a single 
bank/controller/module/chip/... like it is.


More information about the U-Boot mailing list