HI Stephen,
On 5 May 2014 15:14, Stephen Warren swarren@wwwdotorg.org wrote:
On 05/05/2014 01:53 PM, Simon Glass wrote:
Hi Stephen,
On 5 May 2014 11:29, Stephen Warren swarren@wwwdotorg.org wrote:
On 05/05/2014 10:09 AM, Simon Glass wrote:
This is an implementation of GPIOs for Tegra that uses driver model. It has been tested on trimslice and also using the new iotrace feature.
The implementation uses a top-level GPIO device (which has no actual GPIOS). Under this all the banks are created as separate GPIO devices.
I don't think that dividing the GPIOs into banks is appropriate. While the textual naming of Tegra GPIOs is based on the bank concept, I think that division should only apply to the textual naming, and not any data structures or device registration, etc. In fact, I often refer to Tegra GPIOs using their integer ID rather than their name, and dividing the controller into separate banks probably technically disallows this, since there would be no architected guarantee that the numbering of the banks would be sequential. Contiguous numbering of all GPIOs within the controller is also useful for correlation with pinmux numbering of pins.
This is how the GPIO uclass works at present. Do you think that should change also? It would mean enhancing the uclass to support multiple GPIO bank names, with each having a number of GPIOs attached. This is the sort of complexity that adds to code size. On the other hand it's something that we may want to do anyway as more SOC's GPIO drivers are converted.
Surely this means simplifying the core to completely remove any concept of GPIO banks. I really don't see any need for a 2-level hierarchy. Just have one struct/object/... that represents a GPIO controller/module/chip/... Each of those has N GPIOs numbered 0..N. Linux has managed to get away without a 2-level controller/bank approach, so there's certainly evidence it works in practice.
I think you have it backwards...the current implementation has a single level of hierarchy. Each driver handles one bank (or 'port' in the case of Tegra). What you are talking about is having a single driver handle multiple banks, thus requiring that the driver have a second level to deal with banks, over and above the device. We might end up with that, but I would prefer to move to it when we have evidence that it is a general need.
...
Since driver model is not yet available before relocation, or in SPL, a special function is provided for seaboard's SPL code.
Perhaps we should just remove that code from Seaboard, since sharing the UART/SPI pins really isn't a useful feature.
saveenv to SPI won't work without it though.
The environment is stored in eMMC.
SPI simply doesn't work sanely on Seaboard, and we should just rip out any support for it at all. On Seaboard itself, people can just set the jumpers to ignore SPI completely. On Springbank, something similar can be done with the pull-up/down resistors. I've been using my Springbank without any of the resistors present, while disables SPI completely, for years without issue...
OK, how about you submit a patch to do that, and then I can drop my special case (or maybe not if your other patch goes in)?
+static char *gpio_port_name(int base_port) {
char *name, *s;name = malloc(3);It seems like this should be statically allocated (e.g. as part of *plat in gpio_tegra_bind() or something?). Still, if we stop splitting things into banks, we can completely get rid of this.
No, we still need the name so that 'gpio input T3' works corrrectly.
Why do we need GPIO names at all? "gpio input 155" works just as well, and to be honest is often a lot more convenient that trying to maps things back to a name.
Eh? We need to support named GPIOs in U-Boot. 155 is a meaningless number which drivers people back and forth to the datasheets, their calculators, a long table, etc. Even the Tegra device tree has moved away from numbers to GPIO names, I notice.
Regards, Simon