On 07/30/2014 09:26 AM, Simon Glass wrote:
Hi Stephen,
On 12 June 2014 23:31, Stephen Warren swarren@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
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...).