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? > >>> 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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
