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

[Simon Glass]

HI Stephen,

On 6 May 2014 11:34, Stephen Warren <swarren at wwwdotorg.org> wrote:

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

OK, well that's mostly an implementation issue, will take a look.


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

See gpio_lookup_name() which is where this lives.

The GPIO uclass does sequentially number GPIOs, but be aware that on
platforms with multiple GPIO controllers (e.g. an I2C GPIO extender) you
might hit a problem where the tegra GPIOs are not first, so might start at
8 or 16, for example. However I think that probably can be resolved when we
come to it.

It was always my intention to support numbered GPIOs, but the current
function doesn't do that except for anonymous banks - I'll update that as a
separate patch.

OK?

Regards,
Simon