On 07/30/2014 10:09 AM, Simon Glass wrote:
Hi Stephen,
On 30 July 2014 16:47, Stephen Warren <swar...@wwwdotorg.org> wrote:
On 07/30/2014 09:26 AM, Simon Glass wrote:
Hi Stephen,
On 12 June 2014 23:31, Stephen Warren <swar...@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@a0@i2c0, pca9555@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.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
