[U-Boot] [RFC PATCH v2 13/13] tegra: Convert tegra GPIO driver to use driver model

[Stephen Warren]

On 05/05/2014 01:53 PM, Simon Glass wrote:
> Hi Stephen,
> 
> On 5 May 2014 11:29, Stephen Warren <swarren at 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.

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

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