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

[Stephen Warren]

On 05/05/2014 05:00 PM, Simon Glass wrote:
> Hi Stephen,
> 
> On 5 May 2014 16:07, Stephen Warren <swarren at wwwdotorg.org
> <mailto:swarren at wwwdotorg.org>> wrote:
> 
>     On 05/05/2014 03:30 PM, Simon Glass wrote:
>     ...
>     > 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.
> 
>     Sigh. This is getting silly. All the APIs in SW need to just take a GPIO
>     ID in the flat range 0..N (N==223 for Tegra20) and deal with it.
>     Anything that expose banks anywhere, either as a parameter to public
>     functions exported from the GPIO controller driver, or as the existence
>     of separate drivers for separate banks, or as a command-line argument
>     that the user sees, or ..., whether it be the U-Boot GPIO core or the
>     Tegra GPIO driver itself that causes this, is just pointless.
> 
> 
> For exynos, the banks are not contiguous and there is quite a bit of
> fiddling to make this all work. You may have seen the series going in at
> the moment to number the GPIOs properly.
> 
> I understand that Tegra is very straightforward though.

Sure, HW that truly is designed as separate banks should be represented
as such. However, the existence of such HW shouldn't force *other* HW to
artificially expose banks of GPIOs when there's no need/desire to.

>     > 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.
> 
>     The GPIO names are meaningless. I say this because all the Tegra
>     schematics (and documentation that drives them) use the pin/pad name,
>     which is almost always entirely different from the GPIO name. You have
>     to map the pin name back to either the GPIO name or ID using a lookup
>     table (such as the kernel's drivers/pinctrl/pinctrl-tegra20.c). Given
>     the need for a lookup table, we should just use the simpler GPIO ID and
>     not worry about GPIO names. There's no point screwing around with text
>     names when we can just use simple numbers.
> 
>     In DT, GPIOs are specified by integer ID too. Admittedly we have macros
>     that calculate those IDs from the bank/port/offset, but that was
>     probably a mistake, since the bank/port/offset names aren't meaningful.
> 
> 
> U-Boot provides a friendly named interface for GPIOs. I see that as a
> requirement for driver model too. As someone who has spent a lot of time
> at the command line fiddling with hardware, I don't want to go backwards
> in the driver model conversion. Similarly, using numbers in the DT is
> very unfriendly and painful IMO.
> 
> If we can agree on the friendly names, then let's talk about how to
> simplify things.

To be honest, I disagree that meaningless names are friendly. Almost
everything else deals with numbers. The values in DT are numbers. The
debugfs files in Linux are numbers. The sysfs ABI in Linux is numbers.
The current GPIO interface in U-Boot is numbers. The correlation with
pinctrl pins is numbers.

If the following happens, then I could live with a (part-time)
name-based API:

* The U-Boot commands accept either a name or a number. That would allow
people to use what they want.

* The API implemented by U-Boot GPIO drivers uses numbers exclusively.

Note that when I say numbers above, all the numbering should be relative
to a particular controller. So, I don't mean something like:

gpio set 1056

... where 1056 is 1000 (Tegra GPIO controller base) + 56 (Tegra GPIO
ID). Instead, I would expect the command-line interface to be:

gpio set tegra 56
gpio set tegra PH0

... where tegra is the name of the GPIO controller instance, and 56/PH0
is the GPIO ID within that one GPIO controller.

The controller name could be pca9555-0 (0th instance of a pca9555
device) or i2c-0-56 (I2C bus 0 device address 0x56) or whatever naming
style you want.

To support this, I would expect the GPIO driver API to contain just two
APIs that know about the GPIO names:

int name_to_gpio_id(const char *gpio_name);
const char *gpio_id_to_name(int gpio_id);

All the other GPIO driver APIs would take "int gpio_id"
(controller-relative integer ID).