Message ID | 1307984291-9774-1-git-send-email-linus.walleij@stericsson.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Mon, 2011-06-13 at 18:58 +0200, Linus Walleij wrote: > This creates a subsystem for handling of pinmux devices. These are > devices that enable and disable groups of pins on primarily PGA and > BGA type of chip packages and common in embedded systems. Trivia only: > diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c [] > +int pin_is_valid(int pin) > +{ > + return pin >= 0 && pin < num_pins; > +} > +EXPORT_SYMBOL_GPL(pin_is_valid); bool pin_is_valid? > +/* Deletes a range of pin descriptors */ > +static void pinctrl_free_pindescs(struct pinctrl_pin_desc const *pins, > + unsigned num_pins) const struct pinctrl_pin_desc *pins > +{ > + int i; > + > + for (i = 0; i < num_pins; i++) { > + struct pin_desc *pindesc; > + > + spin_lock(&pin_desc_tree_lock); > + pindesc = radix_tree_lookup(&pin_desc_tree, pins[i].number); > + if (pindesc != NULL) { > + radix_tree_delete(&pin_desc_tree, pins[i].number); > + num_pins --; No space please > + } > + spin_unlock(&pin_desc_tree_lock); > + kfree(pindesc); > + } > +} Is it really worthwhile to have spin_lock/unlock in the loop? > +static int pinctrl_register_one_pin(unsigned number, const char *name) > +{ > + /* Copy optional basic pin info */ > + if (name) { > + strncpy(pindesc->name, name, 16); strlcpy > + pindesc->name[15] = '\0'; > + } > + > + spin_lock(&pin_desc_tree_lock); > + radix_tree_insert(&pin_desc_tree, number, pindesc); > + num_pins ++; No space please > + spin_unlock(&pin_desc_tree_lock); > + return 0; > +} > + > +/* Passing in 0 num_pins means "sparse" */ > +static int pinctrl_register_pins(struct pinctrl_pin_desc const *pins, > + unsigned num_descs, unsigned num_pins) [] > + * If we are registerering dense pinlists, fill in all holes with registering > + * anonymous pins. > + */ > + for (i = 0; i < num_pins; i++) { > + char pinname[16]; > + struct pin_desc *pindesc; > + > + spin_lock(&pin_desc_tree_lock); > + pindesc = radix_tree_lookup(&pin_desc_tree, i); > + spin_unlock(&pin_desc_tree_lock); > + /* Already registered this one, take next */ > + if (pindesc) > + continue; > + > + snprintf(pinname, 15, "anonymous %u", i); > + pinname[15] = '\0'; strlcpy > +int pinctrl_register_pins_dense(struct pinctrl_pin_desc const *pins, > + unsigned num_descs, unsigned num_pins) > +{ > + int ret; > + unsigned i; > + > + ret = pinctrl_register_pins(pins, num_descs, num_pins); > + if (ret) { > + for (i = 0; i < num_pins; i++) { > + struct pin_desc *pindesc; > + > + spin_lock(&pin_desc_tree_lock); > + pindesc = radix_tree_lookup(&pin_desc_tree, i); > + if (pindesc != NULL) { > + radix_tree_delete(&pin_desc_tree, i); > + num_pins --; > + } > + spin_unlock(&pin_desc_tree_lock); > + kfree(pindesc); > + } Second use of this pattern. Maybe use pinctrl_free_pindescs?
On Mon, Jun 13, 2011 at 10:58 AM, Linus Walleij <linus.walleij@stericsson.com> wrote: > From: Linus Walleij <linus.walleij@linaro.org> > > This creates a subsystem for handling of pinmux devices. These are > devices that enable and disable groups of pins on primarily PGA and > BGA type of chip packages and common in embedded systems. > > This is being done to depopulate the arch/arm/* directory of such > custom drivers and try to abstract the infrastructure they all > need. See the Documentation/pinmux.txt file that is part of this > patch for more details. Hi Linus, Overall (but without a deep dive into the implementation details) I think I generally like the approach. To sum up, it looks like the conceptual model is thus: - A pinmux driver enumerates and registers all the pins that it has - Setup code and/or driver code requests blocks of pins (functions) when it needs them - If all the pins are available, it gets them, otherwise it the allocation fails - pins cannot be claimed by more than one device - it is up to the pinmux driver to actually program the device and determine whether or not the requested pin mode is actually configurable. Even if pins are available, it may be that other constraints prevent it from actually being programmed Ultimately, it still is up to the board designer and board port engineer to ensure that the system can actually provide the requested pin configuration. My understanding is that in the majority of cases pinmux will probably want/need to be setup and machine_init time, and device drivers won't really know or care about pinmux; it will already be set up for them when the driver is probed. Any power management issues will also be handled by platform/soc code when the dependent devices are in PM states. How does that line up with your conceptual model of pinmux? Other comments below. > > Cc: Grant Likely <grant.likely@secretlab.ca> > Cc: Stephen Warren <swarren@nvidia.com> > Cc: Joe Perches <joe@perches.com> > Cc: Russell King <linux@arm.linux.org.uk> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > ChangeLog v2->v3: > - Renamed subsystem folder to "pinctrl" since we will likely > want to keep other pin control such as biasing in this > subsystem too, so let us keep to something generic even though > we're mainly doing pinmux now. > - As a consequence, register pins as an abstract entity separate > from the pinmux. The muxing functions will claim pins out of the > pin pool and make sure they do not collide. Pins can now be > named by the pinctrl core. > - Converted the pin lookup from a static array into a radix tree, > I agreed with Grant Likely to try to avoid any static allocation > (which is crap for device tree stuff) so I just rewrote this > to be dynamic, just like irq number descriptors. The > platform-wide definition of number of pins goes away - this is > now just the sum total of the pins registered to the subsystem. You should consider still using a bitmap for tracking which pins are actually available, similar to how irqs are tracked. A bool in each pinmux structure is a little wasteful, and requires a lock to be held for a long time while checking all the structures. > - Make sure mappings with only a function name and no device > works properly. > +Pinmux drivers > +============== > + > +The driver will for all calls be provided an offset pin number into its own > +pin range. If you have 2 chips with 8x8 pins, the first chips pins will have > +numbers 0 thru 63 and the second one pins 64 thru 127, but the driver for the > +second chip will be passed numbers in the range 0 thru 63 anyway, base offset > +subtracted. Wait, do you really want a global numberspace here? I'd rather see callers have a direct reference to the pinmux controller instance, and use local pin numbers. Given the choice, I would not go with global numbers for GPIOs again, and I'm not so big a fan of them for irqs either. > + > +Pinmux drivers are required to supply a few callback functions, some are > +optional. Usually the enable() and disable() functions are implemented, > +writing values into some certain registers to activate a certain mux setting > +for a certain pin. > + > +A simple driver for the above example will work by setting bits 0, 1 or 2 > +into some register mamed MUX, so it enumerates its available settings and > +their pin assignments, and expose them like this: > + > +#include <linux/pinctrl/pinmux.h> > + > +struct foo_pmx_func { > + char *name; > + const unsigned int *pins; > + const unsigned num_pins; > +}; > + > +static unsigned int spi0_0_pins[] = { 0, 8, 16, 24 }; > +static unsigned int i2c0_pins[] = { 24, 25 }; > +static unsigned int spi0_1_pins[] = { 38, 46, 54, 62 }; > + > +static struct foo_pmx_func myfuncs[] = { > + { > + .name = "spi0-0", > + .pins = spi0_0_pins, > + .num_pins = ARRAY_SIZE(spi0_1_pins), > + }, > + { > + .name = "i2c0", > + .pins = i2c0_pins, > + .num_pins = ARRAY_SIZE(i2c0_pins), > + }, > + { > + .name = "spi0-1", > + .pins = spi0_1_pins, > + .num_pins = ARRAY_SIZE(spi0_1_pins), > + }, > +}; > + > +int foo_list(struct pinmux_dev *pmxdev, unsigned selector) > +{ > + if (selector >= ARRAY_SIZE(myfuncs)) > + return -EINVAL; > + return 0; > +} > + > +const char *foo_get_fname(struct pinmux_dev *pmxdev, unsigned selector) > +{ > + if (selector >= ARRAY_SIZE(myfuncs)) > + return NULL; > + return myfuncs[selector].name; > +} Is there a method to lookup the function id from the name? Going from name to number seems more useful to me than going the other way around. > + > +static int foo_get_pins(struct pinmux_dev *pmxdev, unsigned selector, > + unsigned ** const pins, unsigned * const num_pins) > +{ > + if (selector >= ARRAY_SIZE(myfuncs)) > + return -EINVAL; > + *pins = myfuncs[selector].pins; > + *num_pins = myfuncs[selector].num_pins; > + return 0; > +} > + > + > +int foo_enable(struct pinmux_dev *pmxdev, unsigned selector) > +{ > + if (selector < ARRAY_SIZE(myfuncs)) > + write((read(MUX)|(1<<selector)), MUX) > + return 0; > + } > + return -EINVAL; > +} > + > +int foo_disable(struct pinmux_dev *pmxdev, unsigned selector) > +{ > + if (selector < ARRAY_SIZE(myfuncs)) > + write((read(MUX) & ~(1<<selector)), MUX) > + return 0; > + } > + return -EINVAL; > +} > + > +struct pinmux_ops ops = { > + .list_functions = foo_list, > + .get_function_name = foo_get_fname, > + .get_function_pins = foo_get_pins, > + .enable = foo_enable, > + .disable = foo_disable, Mixing callbacks with data here. Not bad, but maybe a little odd. > +}; > + > +Now the able reader will say: "wait - the driver needs to make sure it > +can set this and that bit at the same time, because else it will collide > +and wreak havoc in my electronics, and make sure noone else is using the > +other setting that it's incompatible with". > + > +In the example activating muxing 0 and 1 at the same time setting bits > +0 and 1, uses one pin in common so they would collide. > + > +The beauty of the pinmux subsystem is that since it keeps track of all > +pins and who is using them, it will already have denied an impossible > +request like that, so the driver does not need to worry about such > +things - when it gets a selector passed in, the pinmux subsystem makes > +sure no other device or GPIO assignment is already using the selected > +pins. Sometimes that isn't enough. Some functions may not actually collide on the pins they select, but the modes will be mutually exclusive anyway. There needs to be runtime checking that the mode can actually be programmed when it is enabled (of course, it may just be that for *this* example it doesn't need to worry about it, in which case my comment is moot). > + > +The above functions are mandatory to implement for a pinmux driver. > + > +The function list could become long, especially if you can convert every > +individual pin into a GPIO pin independent of any other pins, then your > +function array can become 64 entries for each GPIO setting and then the > +device functions. For this reason there is an additional function you > +can implement to enable only GPIO on an individual pin: gpio_enable(). > + > + > +Pinmux board/machine configuration > +================================== > + > +Boards and machines define how a certain complete running system is put > +together, including how GPIOs and devices are muxed, how regulators are > +constrained and how the clock tree looks. Of course pinmux settings are also > +part of this. > + > +A pinmux config for a machine looks pretty much like a simple regulator > +configuration, so for the example array above we want to enable i2c and > +spi on the second function mapping: > + > +#include <linux/pinctrl/machine.h> > + > +static struct pinmux_map pmx_mapping[] = { > + { > + .function = "spi0-1", > + .dev_name = "foo-spi.0", > + }, > + { > + .function = "i2c0", > + .dev_name = "foo-i2c.0", > + }, > +}; I'm wary about this approach, even though I know it is already used by regulator and clk mappings. The reason I'm wary is that matching devices by name becomes tricky for anything that isn't statically created by the kernel, such as when enumerating from the device tree, because it assumes that the device name is definitively known. Specifically, Linux's preferred name for a device is not guaranteed to be available from the device tree. We very purposefully do not encode Linux kernel implementation details into the kernel so that implementation detail changes don't force dt changes. /me goes and thinks about the problem some more... Okay, I think I've got a new approach for the DT domain so that Linux gets the device names it wants for matching to clocks, regulators and this stuff. I'm going to go and code it up now. I still don't personally like matching devices by name, but by no measure is it a show stopper for me. > + > +Since the above construct is pretty common there is a helper macro to make > +it even more compact: > + > +static struct pinmux_map pmx_mapping[] = { > + PINMUX_MAP("spi0-1", "foo-spi.0"), > + PINMUX_MAP("i2c0", "foo-i2c.0"), > +}; > + > +The dev_name here matches to the unique device name that can be used to look > +up the device struct (just like with clockdev or regulators). The function name > +must match a function provided by the pinmux driver handling this pin range. > +You register this pinmux mapping to the pinmux subsystem by simply: > + > + ret = pinmux_register_mappings(&pmx_mapping, ARRAY_SIZE(pmx_mapping)); > + > + > +Pinmux requests from drivers > +============================ > + > +A driver may request a certain mux to be activated, usually just the default > +mux like this: > + > +#include <linux/pinctrl/pinmux.h> > + > +foo_probe() > +{ > + /* Allocate a state holder named "state" etc */ > + struct pinmux pmx; > + > + pmx = pinmux_get(&device, NULL); > + if IS_ERR(pmx) > + return PTR_ERR(pmx); > + pinmux_enable(pmx); > + > + state->pmx = pmx; > +} > + > +foo_remove() > +{ > + pinmux_disable(state->pmx); > + pinmux_put(state->pmx); > +} > + > +If you want a specific mux setting and not just the first one found for this > +device you can specify a specific mux setting, for example in the above example > +the second i2c0 setting: pinmux_get(&device, "spi0-2"); > + > +This get/enable/disable/put sequence can just as well be handled by bus drivers > +if you don't want each and every driver to handle it and you know the > +arrangement on your bus. I would *strongly* recommend against individual device drivers accessing the pinmux api. This is system level configuration code, and should be handled at the system level. > + > +The pins are allocated for your device when you issue the pinmux_get() call, > +after this you should be able to see this in the debugfs listing of all pins. > diff --git a/MAINTAINERS b/MAINTAINERS > index 29801f7..5caea5a 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -4933,6 +4933,11 @@ L: linux-mtd@lists.infradead.org > S: Maintained > F: drivers/mtd/devices/phram.c > > +PINMUX SUBSYSTEM > +M: Linus Walleij <linus.walleij@linaro.org> > +S: Maintained > +F: drivers/pinmux/ > + > PKTCDVD DRIVER > M: Peter Osterlund <petero2@telia.com> > S: Maintained > diff --git a/drivers/Kconfig b/drivers/Kconfig > index 3bb154d..6998d78 100644 > --- a/drivers/Kconfig > +++ b/drivers/Kconfig > @@ -56,6 +56,10 @@ source "drivers/pps/Kconfig" > > source "drivers/ptp/Kconfig" > > +# pinctrl before gpio - gpio drivers may need it GPIO controllers are just other devices, I don't think there is anything special here when compared with SPI or I2C. I don't think gpio drivers should be accessing the pinmux api directly. In my mind, the gpio layer is only about abstracting the gpio control interface to drivers. Whether or not or how the pin is routed outside the chip package is irrelevant to the driver or the gpio api. Besides, this is kconfig. The order of this file has zero bearing on the resultant kernel. It does matter for the Makefile though. > + > +source "drivers/pinctrl/Kconfig" > + > source "drivers/gpio/Kconfig" > > source "drivers/w1/Kconfig" > diff --git a/drivers/Makefile b/drivers/Makefile > index 09f3232..a590a01 100644 > --- a/drivers/Makefile > +++ b/drivers/Makefile > @@ -5,6 +5,8 @@ > # Rewritten to use lists instead of if-statements. > # > > +# GPIO must come after pinctrl as gpios may need to mux pins etc > +obj-y += pinctrl/ > obj-y += gpio/ > obj-$(CONFIG_PCI) += pci/ > obj-$(CONFIG_PARISC) += parisc/ > diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig > new file mode 100644 > index 0000000..8050fdf > --- /dev/null > +++ b/drivers/pinctrl/Kconfig > @@ -0,0 +1,29 @@ > +# > +# PINCTRL infrastructure and drivers > +# > + > +menuconfig PINCTRL > + bool "PINCTRL Support" > + depends on SYSFS && EXPERIMENTAL Hold off on the sysfs stuff. Lay down the basic API without sysfs, and add the sysfs bits as a separate patch. This becomes a kernel->userspace abi issue, and you don't want to mess that up. Is a pinmux sysfs abi really needed? What is the use-case? > + help > + This enables the PINCTRL subsystem for controlling pins > + on chip packages, for example multiplexing pins on primarily > + PGA and BGA packages for systems on chip. > + > + If unsure, say N. > + > +if PINCTRL > + > +config DEBUG_PINCTRL > + bool "Debug PINCTRL calls" > + depends on DEBUG_KERNEL > + help > + Say Y here to add some extra checks and diagnostics to PINCTRL calls. > + > +config PINMUX_U300 > + bool "U300 pinmux driver" > + depends on ARCH_U300 > + help > + Say Y here to enable the U300 pinmux driver > + PINMUX_U300 should not be here in the infrastructure patch. > +endif > diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile > new file mode 100644 > index 0000000..44d8933 > --- /dev/null > +++ b/drivers/pinctrl/Makefile > @@ -0,0 +1,6 @@ > +# generic pinmux support > + > +ccflags-$(CONFIG_DEBUG_PINMUX) += -DDEBUG > + > +obj-$(CONFIG_PINCTRL) += core.o Consider calling this pinmux.o; particularly if there is ever a chance of it becoming a module. It is conceivable that pinmux will be used for peripheral chips in a way that can/should be loaded at runtime. > +obj-$(CONFIG_PINMUX_U300) += pinmux-u300.o > diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c > new file mode 100644 > index 0000000..8fd1437 > --- /dev/null > +++ b/drivers/pinctrl/core.c > @@ -0,0 +1,1028 @@ > +/* > + * Core driver for the pinmux subsystem > + * > + * Copyright (C) 2011 ST-Ericsson SA > + * Written on behalf of Linaro for ST-Ericsson > + * Based on bits of regulator core, gpio core and clk core > + * > + * Author: Linus Walleij <linus.walleij@linaro.org> > + * > + * License terms: GNU General Public License (GPL) version 2 > + */ > +#define pr_fmt(fmt) "pinctrl core: " fmt > + > +#include <linux/kernel.h> > +#include <linux/init.h> > +#include <linux/device.h> > +#include <linux/slab.h> > +#include <linux/radix-tree.h> > +#include <linux/err.h> > +#include <linux/list.h> > +#include <linux/mutex.h> > +#include <linux/spinlock.h> > +#include <linux/sysfs.h> > +#include <linux/debugfs.h> > +#include <linux/seq_file.h> > +#include <linux/pinctrl/machine.h> > +#include <linux/pinctrl/pinmux.h> > + > +/* Global list of pinmuxes */ > +static DEFINE_MUTEX(pinmux_list_mutex); > +static LIST_HEAD(pinmux_list); > + > +/* Global list of pinmux devices */ > +static DEFINE_MUTEX(pinmuxdev_list_mutex); > +static LIST_HEAD(pinmuxdev_list); > + > +/** > + * struct pin_desc - pin descriptor for each physical pin in the arch > + * @pmxdev: corresponding pinmux device > + * @requested: whether the pin is already requested by pinmux or not > + * @name: a name for the pin, e.g. the name of the pin/pad/finger on a > + * datasheet or such > + * @function: a named muxing function for the pin that will be passed to > + * subdrivers and shown in debugfs etc > + */ > +struct pin_desc { > + struct pinmux_dev *pmxdev; > + bool requested; > + char name[16]; > + char function[16]; > +}; > +/* Global lookup of per-pin descriptors, one for each physical pin */ > +static DEFINE_SPINLOCK(pin_desc_tree_lock); > +static RADIX_TREE(pin_desc_tree, GFP_KERNEL); The radix tree should probably be per-pinmux controller local. Of course, if you make all the pinmux numbering local to the controller, then the need for a radix tree could very well go away entirely, and it would simplify everything. > +static unsigned int num_pins = 0; > + > +/** > + * struct pinmux - per-device pinmux state holder > + * @node: global list node - only for internal use > + * @dev: the device using this pinmux > + * @pmxdev: the pinmux device controlling this pinmux > + * @map: corresponding pinmux map active for this pinmux setting > + * @usecount: the number of active users of this mux setting, used to keep > + * track of nested use cases > + * @pins: an array of discrete physical pins used in this mapping, taken > + * from the global pin enumeration space (copied from pinmux map) > + * @num_pins: the number of pins in this mapping array, i.e. the number of > + * elements in .pins so we can iterate over that array (copied from > + * pinmux map) > + * @pmxdev: pinmux device handling this pinmux > + * @pmxdev_selector: the selector for the pinmux device handling this pinmux > + * @mutex: a lock for the pinmux state holder > + */ > +struct pinmux { > + struct list_head node; > + struct device *dev; > + struct pinmux_map const *map; > + unsigned usecount; > + struct pinmux_dev *pmxdev; > + unsigned pmxdev_selector; > + struct mutex mutex; > +}; > + > +int pin_is_valid(int pin) > +{ > + return pin >= 0 && pin < num_pins; > +} > +EXPORT_SYMBOL_GPL(pin_is_valid); A "pin_" prefix is very generic sounding. Though it doesn't read as well, the pinmux_ prefix should probably be used consistently. > + > +static ssize_t pinmux_name_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct pinmux_dev *pmxdev = dev_get_drvdata(dev); > + > + return sprintf(buf, "%s\n", pmxdev_get_name(pmxdev)); > +} > + > +static struct device_attribute pinmux_dev_attrs[] = { > + __ATTR(name, 0444, pinmux_name_show, NULL), > + __ATTR_NULL, > +}; > + > +static void pinmux_dev_release(struct device *dev) > +{ > + struct pinmux_dev *pmxdev = dev_get_drvdata(dev); > + kfree(pmxdev); > +} > + > +static struct class pinmux_class = { > + .name = "pinmux", > + .dev_release = pinmux_dev_release, > + .dev_attrs = pinmux_dev_attrs, > +}; > + > +/* Deletes a range of pin descriptors */ > +static void pinctrl_free_pindescs(struct pinctrl_pin_desc const *pins, > + unsigned num_pins) > +{ > + int i; > + > + for (i = 0; i < num_pins; i++) { > + struct pin_desc *pindesc; > + > + spin_lock(&pin_desc_tree_lock); > + pindesc = radix_tree_lookup(&pin_desc_tree, pins[i].number); > + if (pindesc != NULL) { > + radix_tree_delete(&pin_desc_tree, pins[i].number); > + num_pins --; > + } > + spin_unlock(&pin_desc_tree_lock); > + kfree(pindesc); > + } > +} > + > +static int pinctrl_register_one_pin(unsigned number, const char *name) > +{ > + struct pin_desc *pindesc; > + > + spin_lock(&pin_desc_tree_lock); > + pindesc = radix_tree_lookup(&pin_desc_tree, number); > + spin_unlock(&pin_desc_tree_lock); > + > + if (pindesc != NULL) { > + pr_err("pin %d already registered\n", number); > + return -EINVAL; > + } > + > + pindesc = kzalloc(sizeof(*pindesc), GFP_KERNEL); > + if (pindesc == NULL) > + return -ENOMEM; > + > + /* Copy optional basic pin info */ > + if (name) { > + strncpy(pindesc->name, name, 16); > + pindesc->name[15] = '\0'; > + } > + > + spin_lock(&pin_desc_tree_lock); > + radix_tree_insert(&pin_desc_tree, number, pindesc); > + num_pins ++; > + spin_unlock(&pin_desc_tree_lock); > + return 0; > +} > + > +/* Passing in 0 num_pins means "sparse" */ > +static int pinctrl_register_pins(struct pinctrl_pin_desc const *pins, > + unsigned num_descs, unsigned num_pins) > +{ > + unsigned i; > + int ret = 0; > + > + for (i = 0; i < num_descs; i++) { > + ret = pinctrl_register_one_pin(pins[i].number, pins[i].name); > + if (ret) > + return ret; > + } > + > + if (num_pins == 0) > + return 0; > + > + /* > + * If we are registerering dense pinlists, fill in all holes with > + * anonymous pins. > + */ > + for (i = 0; i < num_pins; i++) { > + char pinname[16]; > + struct pin_desc *pindesc; > + > + spin_lock(&pin_desc_tree_lock); > + pindesc = radix_tree_lookup(&pin_desc_tree, i); > + spin_unlock(&pin_desc_tree_lock); > + /* Already registered this one, take next */ > + if (pindesc) > + continue; > + > + snprintf(pinname, 15, "anonymous %u", i); > + pinname[15] = '\0'; > + > + ret = pinctrl_register_one_pin(i, pinname); > + if (ret) > + return ret; > + } > + > + return 0; > +} > + > +/** > + * pinctrl_register_pins_sparse() - register a range of pins for a > + * board/machine with potential holes in the pin map. The pins in > + * the holes will not be usable. > + * @pins: a range of pins to register > + * @num_descs: the number of pins descriptors passed in through the previous > + * pointer > + */ > +int pinctrl_register_pins_sparse(struct pinctrl_pin_desc const *pins, > + unsigned num_descs) > +{ > + int ret; > + > + ret = pinctrl_register_pins(pins, num_descs, 0); > + if (ret) > + pinctrl_free_pindescs(pins, num_descs); > + return ret; > + > +} > +EXPORT_SYMBOL_GPL(pinctrl_register_pins); > + > +/** > + * pinctrl_register_pins_dense() - register a range of pins for a > + * board/machine, if there are holes in the pin map, they will be > + * allocated by anonymous pins. > + * @pins: a range of pins to register > + * @num_descs: the number of pins descriptors passed in through the previous > + * pointer > + * @num_pins: the total number of pins including holes in the pin map and > + * any "air" at the end of the map, all pins from 0 to this number > + * will be allocated, the ones that does not have descriptors passed > + * in will be marked "anonymous" > + */ > +int pinctrl_register_pins_dense(struct pinctrl_pin_desc const *pins, > + unsigned num_descs, unsigned num_pins) > +{ > + int ret; > + unsigned i; > + > + ret = pinctrl_register_pins(pins, num_descs, num_pins); > + if (ret) { > + for (i = 0; i < num_pins; i++) { > + struct pin_desc *pindesc; > + > + spin_lock(&pin_desc_tree_lock); > + pindesc = radix_tree_lookup(&pin_desc_tree, i); > + if (pindesc != NULL) { > + radix_tree_delete(&pin_desc_tree, i); > + num_pins --; > + } > + spin_unlock(&pin_desc_tree_lock); > + kfree(pindesc); > + } > + } > + return ret; > +} > +EXPORT_SYMBOL_GPL(pinctrl_register_pins_sparse); Why two different methods for registering pins? Also the previous two export symbol statements give the wrong functions. Okay, enough comments for now. I think that covers the big stuff, and I've got to get some other work done. g.
Linus Walleij wrote at Monday, June 13, 2011 10:58 AM: > This creates a subsystem for handling of pinmux devices. These are > devices that enable and disable groups of pins on primarily PGA and > BGA type of chip packages and common in embedded systems. I'm a little confused by this version. In particular: * I don't see some changes that I thought we'd agreed upon during earlier review rounds, such as: ** Removing pinmux_ops members list_functions, get_function_name, get_function_pins; it seems odd not to push this into the pinmux core as data, but instead have the core pull it out of the driver in a "polling" function. ** Similarly, I'd asked for at least some documentation about how to handle the "multi-width bus" problem, but I didn't see that. * I'm confused by the new data model even more than before: ** How can the board/machine name pins? That should be a function of the chip (i.e. pinmux driver), since that's where the pins are located. A chip's pins don't have different names on different boards; the names of the signals on the PCB connected to those pins are defined by the board, but not the names of the actual pins themselves. ** struct pinmux_map requires that each function name be globally unique, since one can only specify "function" not "function on a specific chip". This can't be a requirement; what if there are two instances of the same chip on the board (think some expansion chip attached to a memory-mapped bus rather than the primary SoC itself). ** Perhaps primarily due to the lack of consideration in the documentation, I'm not convinced we have a clearly defined path to solve the "multi-width bus" issue. This needs to be a feature of the core pinmux API, rather than something that's deferred to the board/machine files setting up different function mappings, since it's not possible to solve purely in function mappins as they're defined today. Some minor mainly typo, patch-separation, etc. feedback below. ... > +Pinmux, also known as padmux, ballmux, alternate functions or mission modes > +is a way for chip vendors producing some kind of electrical packages to use > +a certain physical bin (ball, pad, finger, etc) for multiple mutually exclusive s/bin/pin/ ... > +A simple driver for the above example will work by setting bits 0, 1 or 2 > +into some register mamed MUX, so it enumerates its available settings and s/mamed/named > +++ b/drivers/pinctrl/Kconfig ... > +config PINMUX_U300 > + bool "U300 pinmux driver" > + depends on ARCH_U300 > + help > + Say Y here to enable the U300 pinmux driver > + > +endif Shouldn't that portion be part of the second patch? > +++ b/drivers/pinctrl/Makefile ... > +obj-$(CONFIG_PINMUX_U300) += pinmux-u300.o Same here. > +++ b/include/linux/pinctrl/machine.h > +/** > + * struct pinmux_map - boards/machines shall provide this map for devices > + * @function: a functional name for this mapping so it can be passed down > + * to the driver to invoke that function and be referenced by this ID > + * in e.g. pinmux_get() > + * @dev: the device using this specific mapping, may be NULL if you provide > + * .dev_name instead (this is more common) > + * @dev_name: the name of the device using this specific mapping, the name > + * must be the same that will return your struct device* s/that will return/as in/ ? > +++ b/include/linux/pinctrl/pinmux.h > +struct pinmux; > + > +#ifdef CONFIG_PINCTRL > + > +struct pinmux_dev; I think "struct pinmux_map" is needed outside that ifdef, or an include of <pinctrl/machine.h>. > +/** > + * struct pinmux_ops - pinmux operations, to be implemented by drivers > + * @request: called by the core to see if a certain pin can be muxed in > + * and made available in a certain mux setting The driver is allowed > + * to answer "no" by returning a negative error code That says "and made available in a certain mux setting", but no mux setting is passed in. s/a certain/the current/? Documentation for @free is missing here > +/** > + * struct pinmux_desc - pinmux descriptor, register this to pinmux subsystem > + * @name: name for the pinmux > + * @ops: pinmux operation table > + * @owner: module providing the pinmux, used for refcounting > + * @base: the number of the first pin handled by this pinmux, in the global > + * pin space, subtracted from a given pin to get the offset into the range > + * of a certain pinmux > + * @npins: the number of pins handled by this pinmux - note that > + * this is the number of possible pin settings, if your driver handles > + * 8 pins that each can be muxed in 3 different ways, you reserve 24 > + * pins in the global pin space and set this to 24 That's not correct, right; if you have 8 pins, you just say 8 here? The multiplier for N functions per pin is through list_functions, get_function_name, get_function_pins as I understand it. > +static inline int pinmux_register_mappings(struct pinmux_map const *map, > + unsigned num_maps) > +{ > + return 0; > +} I think you moved that to <pinmux/machine.h>? -- nvpublic
Hi Joe, thanks for the review, excellent as always. I fixed all except the below pattern, also searched the source to make sure there were no other cases of the same errors. On Mon, Jun 13, 2011 at 8:11 PM, Joe Perches <joe@perches.com> wrote: >> +int pinctrl_register_pins_dense(struct pinctrl_pin_desc const *pins, >> + unsigned num_descs, unsigned num_pins) >> +{ >> + int ret; >> + unsigned i; >> + >> + ret = pinctrl_register_pins(pins, num_descs, num_pins); >> + if (ret) { >> + for (i = 0; i < num_pins; i++) { >> + struct pin_desc *pindesc; >> + >> + spin_lock(&pin_desc_tree_lock); >> + pindesc = radix_tree_lookup(&pin_desc_tree, i); >> + if (pindesc != NULL) { >> + radix_tree_delete(&pin_desc_tree, i); >> + num_pins --; >> + } >> + spin_unlock(&pin_desc_tree_lock); >> + kfree(pindesc); >> + } > > Second use of this pattern. Maybe use pinctrl_free_pindescs? It is quite different actually - in the second case here. we loop over a list with holes, and we pick each one pin. We cannot loop over the entire pin range because in this case we don't know the size of the range. Thanks, Linus Walleij
On Mon, Jun 13, 2011 at 9:57 PM, Grant Likely <grant.likely@secretlab.ca> wrote: > (...) > To sum up, it looks like the conceptual model is thus: > > - A pinmux driver enumerates and registers all the pins that it has > - Setup code and/or driver code requests blocks of pins (functions) > when it needs them > - If all the pins are available, it gets them, otherwise it the allocation fails > - pins cannot be claimed by more than one device > - it is up to the pinmux driver to actually program the device and > determine whether or not the requested pin mode is actually > configurable. Even if pins are available, it may be that other > constraints prevent it from actually being programmed > > Ultimately, it still is up to the board designer and board port > engineer to ensure that the system can actually provide the requested > pin configuration. > > My understanding is that in the majority of cases pinmux will probably > want/need to be setup and machine_init time, and device drivers won't > really know or care about pinmux; it will already be set up for them > when the driver is probed. Any power management issues will also be > handled by platform/soc code when the dependent devices are in PM > states. > > How does that line up with your conceptual model of pinmux? 100% I'd say, so far. Devil is in the details, below. >> - Converted the pin lookup from a static array into a radix tree, >> I agreed with Grant Likely to try to avoid any static allocation >> (which is crap for device tree stuff) so I just rewrote this >> to be dynamic, just like irq number descriptors. The >> platform-wide definition of number of pins goes away - this is >> now just the sum total of the pins registered to the subsystem. > > You should consider still using a bitmap for tracking which pins are > actually available, similar to how irqs are tracked. Well that is not so simple, because that bitmap needs to have a size. And that means the stuff we're tracking is not really dynamic, but either static or a hybrid (roofed bitmap). Let's recap: - Either you have a defined number of IRQs (NR_IRQS) defined system-wide and then allocate a fixed array of descriptors like that: struct irq_desc irq_desc[NR_IRQS] (or for GPIOs: static struct gpio_desc gpio_desc[ARCH_NR_GPIOS];) - Or you dynamically allocate descriptors such as is done with sparse IRQs. The first case is the same way that gpiolib depends on the global ARCH_NR_GPIOS is used. It inevitably involves roofing the number of IRQs/GPIOs on systems where they come and go. That is static allocation, and that is what we wanted to get away from. Sparse IRQs also use a bitmap though. But it is not dynamic at all, it's either static or an assumption-based hybrid. To use a bitmap you need to know how many IRQs you have, so you know how large bitmap you need to allocate. c.f. kernel/irq/irqdesc.c: static DECLARE_BITMAP(allocated_irqs, IRQ_BITMAP_BITS); And IRQ_BITMAP_BITS comes from: kernel/irq/internals.h; #ifdef CONFIG_SPARSE_IRQ # define IRQ_BITMAP_BITS (NR_IRQS + 8196) #else # define IRQ_BITMAP_BITS NR_IRQS #endif (Needless to say this assumes you never add more than 8196 irqs on top of the fixed number of IRQs) So to use this I need to go back to a model where the system knows how many pins there are anyway or just assume something like "never more than 8192" and hardcode it like for sparse IRQs above, then that consumes 8192/32 = 256 words of memory. > A bool in each > pinmux structure is a little wasteful, If I instead have it all-dynamic, and say the boolean field even consumes 32bits in the desc (which we need anyway) you need to use more than 256 pins in your system before this hits you, 32 bits per pin. (Beware I have no clue how booleans actually are stored in structs.) It's not *that* bad, especially not compared to starting to compile in every other driver to get a single booting binary for several ARM systems, then this is peanuts ... (OK maybe a crap argument I dunno. It feels to me like footprint issues are out of fashion on recent ARM systems.) > and requires a lock to be held > for a long time while checking all the structures. I don't get this part. The lock is held when looking up the desc in the radix tree, just like in the IRQ subsystem and the radix lookup is fast enough to be in the fastpath. The only code that actually traverse the entire tree is in debugfs code or error path, surely that must be an acceptable traversal? >> +The driver will for all calls be provided an offset pin number into its own >> +pin range. If you have 2 chips with 8x8 pins, the first chips pins will have >> +numbers 0 thru 63 and the second one pins 64 thru 127, but the driver for the >> +second chip will be passed numbers in the range 0 thru 63 anyway, base offset >> +subtracted. > > Wait, do you really want a global numberspace here? I'd rather see > callers have a direct reference to the pinmux controller instance, and > use local pin numbers. The pinmux functions that are requested by machine, bus or device indeed work that way. > Given the choice, I would not go with global > numbers for GPIOs again, and I'm not so big a fan of them for irqs > either. Two reasons for a global numberspace: 1. We want to handle other stuff that relates to pincontrol in the pinctrl API, such as biasing, driver levels, load capacitance and whatever the funny pin engineers come up with. Not all pins are simultaneously muxable, many are both controlled in this way AND muxable, at the same time. Biasing etc needs to happen at pin level rather than group/function level. muxing and biasing may interact in driver level, BTW. 2. The second reason it is even there is to coexist with the GPIO subsystem, since many, many systems need to mux in GPIO on single pins today (ux500, OMAP, i.MX31 OTOMH). Some assorted blather follows: That is why requesting a singe pin for GPIO with int pinmux_request_gpio(int pin, unsigned gpio); that reserved the pin and goes all the way to an (optional) callback to the driver in int (*gpio_request_enable) (struct pinmux_dev *pmxdev, unsigned offset); that asks the hardware to mux in that singe pin as GPIO. The mentioned hardware indeed has per-pin muxing granularity BTW, so this will be useful and quick for these usecases. The alternative is to first define a function for every singular GPIO pin, and then mux in that function for each pin in the GPIO driver. This does not solve the biasing etc problems though. If we were discussing footprint issues before defining a function for each pin in the system dwarfs that :-) If instead all GPIOs are requested from the GPIO driver in ranges, say "these 32 as GPIO please" the problem goes away, and you don't need to implement your stuff using these function at all. However I feel it is a bit thick to require that, and in practical cases I've seen you actually do want to get at the single pins. (IIRC Sascha confirmed this assumption for i.MX) >> +const char *foo_get_fname(struct pinmux_dev *pmxdev, unsigned selector) >> +{ >> + if (selector >= ARRAY_SIZE(myfuncs)) >> + return NULL; >> + return myfuncs[selector].name; >> +} > Is there a method to lookup the function id from the name? Going from > name to number seems more useful to me than going the other way > around. I don't quite get it, this is based on how the regulator framework asks it's drivers to enumerate voltages, the name is entirely optional, more to get visibility of names. Enumerators is what the framework is using and requiring from the drivers. >> +struct pinmux_ops ops = { >> + .list_functions = foo_list, >> + .get_function_name = foo_get_fname, >> + .get_function_pins = foo_get_pins, >> + .enable = foo_enable, >> + .disable = foo_disable, > > Mixing callbacks with data here. Not bad, but maybe a little odd. ? The above are all functions ? >> +The beauty of the pinmux subsystem is that since it keeps track of all >> +pins and who is using them, it will already have denied an impossible >> +request like that, so the driver does not need to worry about such >> +things - when it gets a selector passed in, the pinmux subsystem makes >> +sure no other device or GPIO assignment is already using the selected >> +pins. > > Sometimes that isn't enough. Some functions may not actually collide > on the pins they select, but the modes will be mutually exclusive > anyway. There needs to be runtime checking that the mode can actually > be programmed when it is enabled (of course, it may just be that for > *this* example it doesn't need to worry about it, in which case my > comment is moot). Yeah the driver will have to take care of that and return some -EBUSY or so. >> +#include <linux/pinctrl/machine.h> >> + >> +static struct pinmux_map pmx_mapping[] = { >> + { >> + .function = "spi0-1", >> + .dev_name = "foo-spi.0", >> + }, >> + { >> + .function = "i2c0", >> + .dev_name = "foo-i2c.0", >> + }, >> +}; > > I'm wary about this approach, even though I know it is already used by > regulator and clk mappings. The reason I'm wary is that matching > devices by name becomes tricky for anything that isn't statically > created by the kernel, such as when enumerating from the device tree, > because it assumes that the device name is definitively known. > Specifically, Linux's preferred name for a device is not guaranteed to > be available from the device tree. We very purposefully do not encode > Linux kernel implementation details into the kernel so that > implementation detail changes don't force dt changes. > > /me goes and thinks about the problem some more... > > Okay, I think I've got a new approach for the DT domain so that Linux > gets the device names it wants for matching to clocks, regulators and > this stuff. I'm going to go and code it up now. I still don't > personally like matching devices by name, but by no measure is it a > show stopper for me. We can enforce it for DT-based pinmuxing and live with this name matcing for existing boardfiles I assume? >> +This get/enable/disable/put sequence can just as well be handled by bus drivers >> +if you don't want each and every driver to handle it and you know the >> +arrangement on your bus. > > I would *strongly* recommend against individual device drivers > accessing the pinmux api. This is system level configuration code, > and should be handled at the system level. As explained by Russell in earlier mails (and in my own experience too) there exist systems that need to alter muxing at runtime, say mux this thing out, mux that thing in. So it was a design requirement. Some devices will need to demux in response to runtime_pm hooks though. And the other functions of pincontrol, like biasing, definately need to happen in rutime on request from a single device. But I can try to write the doc so that it emphasize doing this in machine/system/board code or wherever you first have an opportunity to ask for something with your struct device * as an argument, OK? It could also be in the system-specific callbacks to board code, of course. Another argument for system files to handled this is that you often mux things out/in on suspend/resume, and that may be suitable for an entity that knows about all devices. Most systems I've seen don't have such centralized control over device creation, and it seems this leads to the implicit requirement that anyone wanting to so that from the board need to move away from any statically allocated devices first? Or am I getting it backwards here? >> --- a/drivers/Kconfig >> +++ b/drivers/Kconfig >> @@ -56,6 +56,10 @@ source "drivers/pps/Kconfig" >> >> source "drivers/ptp/Kconfig" >> >> +# pinctrl before gpio - gpio drivers may need it > > GPIO controllers are just other devices, I don't think there is > anything special here when compared with SPI or I2C. I don't think > gpio drivers should be accessing the pinmux api directly. > > In my mind, the gpio layer is only about abstracting the gpio control > interface to drivers. Whether or not or how the pin is routed outside > the chip package is irrelevant to the driver or the gpio api. > > Besides, this is kconfig. The order of this file has zero bearing on > the resultant kernel. It does matter for the Makefile though. See earlier discussion, I think. If you look in the current drivers/gpio/gpio-nomadik.c you see that all muxing is actually done in that GPIO driver, since it is using the same register range. (So it was natural not to break that part out, as there was no other natural place to put it anyway.) The only reason why others don't do that is (I guess) that they have a special register range for muxing. In arch/arm/mach-pxa/mfp-pxa.c you find the same kind of strong coupling between GPIO and pinmux. Actually some of my generalization work for GPIO is exactly related to this, which is in practice a custom call to the GPIO driver. Main problem - since the GPIO driver can make any pin a GPIO, it needs to interact with GPIO code to make sure it is not used for some GPIO - since we don't have a pinmux subsystem that can take care of that. So if I had that ... a bit chicken and egg problem here. (There are other custom GPIO stuff for sure though, not just this.) And doing a generic pin control subsystem is also for this reason - get biasing, drive modes and load capacitances out of the gpio subsystem and into something apropriate. I think doing this from GPIO drivers for individual pins need to be some intermediate step to consolidate the functionality to drivers/pinctrl/*, else too many requirements on how things shall be done start spreading across the board, but I'll surely try to make it as autonomous as possible. >> +menuconfig PINCTRL >> + bool "PINCTRL Support" >> + depends on SYSFS && EXPERIMENTAL > > Hold off on the sysfs stuff. Lay down the basic API without sysfs, > and add the sysfs bits as a separate patch. This becomes a > kernel->userspace abi issue, and you don't want to mess that up. It is just exposing a name right now, say "you have this pinmux with this name" > Is a pinmux sysfs abi really needed? What is the use-case? Same as for userspace GPIO control I guess. If you want to do GPIO control on a certain pin from userspace, then surely you want to be able to mux it in, and I just heard that the GPIO driver should not be able to do that itself... heh. And configuring it statically muxed-in does not make sense since you don't know whether userspace will actually use it and default mux-enabling it will consumer more power in some cases so doing it statically is not OK etc etc. >> +config PINMUX_U300 >> + bool "U300 pinmux driver" >> + depends on ARCH_U300 >> + help >> + Say Y here to enable the U300 pinmux driver >> + > > PINMUX_U300 should not be here in the infrastructure patch. Mea culpa. I fix. >> +obj-$(CONFIG_PINMUX_U300) += pinmux-u300.o This too! >> +# generic pinmux support >> + >> +ccflags-$(CONFIG_DEBUG_PINMUX) += -DDEBUG >> + >> +obj-$(CONFIG_PINCTRL) += core.o > > Consider calling this pinmux.o; particularly if there is ever a chance > of it becoming a module. It is conceivable that pinmux will be used > for peripheral chips in a way that can/should be loaded at runtime. Right now I haven't broken the pin registration API that will be reused by generic pincontrol and actual pinmux framework into different .c files, maybe I should? The idea is that the pin infrastructure is static and pinmux could actually be a module. >> +/* Global lookup of per-pin descriptors, one for each physical pin */ >> +static DEFINE_SPINLOCK(pin_desc_tree_lock); >> +static RADIX_TREE(pin_desc_tree, GFP_KERNEL); > > The radix tree should probably be per-pinmux controller local. Of > course, if you make all the pinmux numbering local to the controller, > then the need for a radix tree could very well go away entirely, and > it would simplify everything. See earlier discussion about a global numberspace I guess. If we were only doing pinmuxing with no global numberspace to avoid also correlating needs with biasing etc, this would be true, but pinmux is too glued into other stuff IMHO. >> +int pin_is_valid(int pin) >> +{ >> + return pin >= 0 && pin < num_pins; >> +} >> +EXPORT_SYMBOL_GPL(pin_is_valid); > > A "pin_" prefix is very generic sounding. Though it doesn't read as > well, the pinmux_ prefix should probably be used consistently. This is changed. pin_* prefix is for the generic top-level pinctrl API, pinmux_* is specifically for the pinmux stuff. >> pinctrl_register_pins_sparse/dense > > Why two different methods for registering pins? Explained in the kerneldoc I guess, basically some systems can control all pins from 0 ... NR_PINS_ON_THIS_SYSTEM whereas others actually have holes in this map, or may want to fill them in with several pinctrl_register_pins_sparse() calls, such as for platforms registering some pins that are common across a few boards and then a few board-specific pins. > Also the previous two export symbol statements give the wrong functions. Fixed that, saw it on the previous review mail from Joe P. > Okay, enough comments for now. I think that covers the big stuff, and > I've got to get some other work done. Thanks! I bet that other work is deciding on whether to expose gpio_to_chip() or not, hehe :-) Yours, Linus Walleij
On Tue, Jun 14, 2011 at 1:28 AM, Stephen Warren <swarren@nvidia.com> wrote: > I'm a little confused by this version. Sorry, I'll try to clarify. > In particular: > > * I don't see some changes that I thought we'd agreed upon during earlier > review rounds, such as: > > ** Removing pinmux_ops members list_functions, get_function_name, > get_function_pins; it seems odd not to push this into the pinmux core > as data, but instead have the core pull it out of the driver in a > "polling" function. I don't find any particular discussion on that, did I miss something? It might be that I simply do not agree... Anyway, this is modeled exactly on how the regulator subsystem interact with its drivers to enumerate voltages. It seems intuitive to me, it's an established kernel design pattern that drivers know the hardware particulars, and so I don't get the problem here. Would you argue that the regulator subsystem has bad design too or is this case different? Can't you just send some patch or example .h file for the API you would like to see so I understand how you think about this? I might be thinking inside the box of my current design here so help me get out of it, I just have a hard time seeing how to do it. > ** Similarly, I'd asked for at least some documentation about how to > handle the "multi-width bus" problem, but I didn't see that. SORRY! Missed it. So sure that can be added, so something like this? A B C D E F G H +---+ 8 | o | o o o o o o o | | 7 | o | o o o o o o o | | 6 | o | o o o o o o o +---+---+ 5 | o | o | o o o o o o +---+---+ +---+ 4 o o o o o o | o | o | | 3 o o o o o o | o | o | | 2 o o o o o o | o | o +-------+-------+-------+---+---+ 1 | o o | o o | o o | o | o | +-------+-------+-------+---+---+ (...) On the botton row at { A1, B1, C1, D1, E1, F1, G1, H1 } we have something special - it's an external MMC bus that can be 2, 4 or 8 bits wide, and it will consume 2, 4 or 8 pins respectively, so either { A1, B1 } are taken or { A1, B1, C1, D1 } or all of them. If we use all 8 bits, we cannot use the SPI port on pins { G4, G3, G2, G1 } of course. (...) A simple driver for the above example will work by setting bits 0, 1, 2, 3, 4 or 5 into some register named MUX, so it enumerates its available settings and their pin assignments, and expose them like this: #include <linux/pinctrl/pinmux.h> struct foo_pmx_func { char *name; const unsigned int *pins; const unsigned num_pins; }; static unsigned int spi0_0_pins[] = { 0, 8, 16, 24 }; static unsigned int i2c0_pins[] = { 24, 25 }; static unsigned int spi0_1_pins[] = { 38, 46, 54, 62 }; static unsigned int mmc0_1_pins[] = { 56, 57 }; static unsigned int mmc0_2_pins[] = { 56, 57, 58, 59 }; static unsigned int mmc0_3_pins[] = { 56, 57, 58, 59, 60, 61, 62, 63 }; static struct foo_pmx_func myfuncs[] = { { .name = "spi0-0", .pins = spi0_0_pins, .num_pins = ARRAY_SIZE(spi0_1_pins), }, { .name = "i2c0", .pins = i2c0_pins, .num_pins = ARRAY_SIZE(i2c0_pins), }, { .name = "spi0-1", .pins = spi0_1_pins, .num_pins = ARRAY_SIZE(spi0_1_pins), }, { .name = "mmc0-2bit", .pins = mmc0_1_pins, .num_pins = ARRAY_SIZE(mmc0_1_pins), }, { .name = "mmc0-4bit", .pins = mmc0_2_pins, .num_pins = ARRAY_SIZE(mmc0_2_pins), }, { .name = "mmc0-8bit", .pins = mmc0_3_pins, .num_pins = ARRAY_SIZE(mmc0_3_pins), }, }; Looks OK? > ** How can the board/machine name pins? That should be a function of the > chip (i.e. pinmux driver), since that's where the pins are located. A > chip's pins don't have different names on different boards; the names of > the signals on the PCB connected to those pins are defined by the board, > but not the names of the actual pins themselves. Actually I don't really like the use of the concept of board and machine for chip packages either. But if I say that without devicetrees it could be something like arch/arm/plat-nomadik/package-db8500.c defining the pins. Then it is still coming from the outside, for a particular platform, for a particular chip package. Then the init function in that file gets called on relevant systems. As you can see in the example implementation for U300 I actually do this in the driver itself by including the machine.h file to that one. So this driver first registers pins, then functions. I think there is broad consensus that this should come in from the device tree in the future. And if it comes from the device tree, it's still the say arch/arm/plat-nomadik/package-db8500.c file that looks up the pins from the device tree and registers them, just it gets the data from the outside. Possibly the core could be made to do this. I know too little about device tree I think :-/ > ** struct pinmux_map requires that each function name be globally unique, > since one can only specify "function" not "function on a specific chip". > This can't be a requirement; what if there are two instances of the same > chip on the board (think some expansion chip attached to a memory-mapped > bus rather than the primary SoC itself). Namespace clashes are a problem but the way I see it basically a problem with how you design the namespace. It's just a string and the document is just an example. If namespacing is a big issue, we may need naming schemes, but with only one driver as I have now I think that is overkill. Platforms already need to take care of their namespaceing, so if I create a kernel that have mmc ports on two blocks I would just prefix them, say: { .name = "chip0-mmc0", .pins = chip0_mmc0_pins, .num_pins = ARRAY_SIZE(chip0_mmc0_pins), }, { .name = "chip1-mmc0", .pins = chip1_mmc0_pins, .num_pins = ARRAY_SIZE(chip1_mmc0_pins), }, etc for the functions (the device names are namespaced by the device driver core I believe). > ** Perhaps primarily due to the lack of consideration in the documentation, > I'm not convinced we have a clearly defined path to solve the "multi-width > bus" issue. This needs to be a feature of the core pinmux API, rather than > something that's deferred to the board/machine files setting up different > function mappings, since it's not possible to solve purely in function > mappins as they're defined today. Can this be considered fixed with the above example? (Which will be in v4) > Some minor mainly typo, patch-separation, etc. feedback below. Fixed most of them... >> +++ b/include/linux/pinctrl/pinmux.h >> +struct pinmux; >> + >> +#ifdef CONFIG_PINCTRL >> + >> +struct pinmux_dev; > > I think "struct pinmux_map" is needed outside that ifdef, or an include of > <pinctrl/machine.h>. Yeah and it should only be used in machine.h as you point out later, plus I forgot to stub 2 functions for sparse/dense pin registration. Thanks! >> +/** >> + * struct pinmux_ops - pinmux operations, to be implemented by drivers >> + * @request: called by the core to see if a certain pin can be muxed in >> + * and made available in a certain mux setting The driver is allowed >> + * to answer "no" by returning a negative error code > > That says "and made available in a certain mux setting", but no mux setting > is passed in. s/a certain/the current/? Sorry the missing words are "for later being selected for a certain mux setting". So the idea is that this request control over a specific pin and then the enable() call can select it. So a first opportunity for the driver to say "no" really. I'm editing the text... > Documentation for @free is missing here Fixed it! >> +/** >> + * struct pinmux_desc - pinmux descriptor, register this to pinmux subsystem >> + * @name: name for the pinmux >> + * @ops: pinmux operation table >> + * @owner: module providing the pinmux, used for refcounting >> + * @base: the number of the first pin handled by this pinmux, in the global >> + * pin space, subtracted from a given pin to get the offset into the range >> + * of a certain pinmux >> + * @npins: the number of pins handled by this pinmux - note that >> + * this is the number of possible pin settings, if your driver handles >> + * 8 pins that each can be muxed in 3 different ways, you reserve 24 >> + * pins in the global pin space and set this to 24 > > That's not correct, right; if you have 8 pins, you just say 8 here? You are right this is a leftover a previous designed where I used a virtual pin range covering all possible mux settings. This design was insane, so thanks for helping me smoking out the last remnants. > The multiplier for N functions per pin is through list_functions, > get_function_name, get_function_pins as I understand it. Right. >> +static inline int pinmux_register_mappings(struct pinmux_map const *map, >> + unsigned num_maps) >> +{ >> + return 0; >> +} > > I think you moved that to <pinmux/machine.h>? Yep, fixed and added two missing stubs in machine.h too. Thanks a lot! Yours, Linus Walleij
Linus Walleij wrote at Tuesday, June 14, 2011 8:26 AM: > On Tue, Jun 14, 2011 at 1:28 AM, Stephen Warren <swarren@nvidia.com> wrote: > > > I'm a little confused by this version. > > Sorry, I'll try to clarify. > > > In particular: > > > > * I don't see some changes that I thought we'd agreed upon during earlier > > review rounds, such as: > > > > ** Removing pinmux_ops members list_functions, get_function_name, > > get_function_pins; it seems odd not to push this into the pinmux core > > as data, but instead have the core pull it out of the driver in a > > "polling" function. > > I don't find any particular discussion on that, did I miss something? > It might be that I simply do not agree... That was: https://lkml.org/lkml/2011/5/19/365 https://lkml.org/lkml/2011/5/19/379 > Anyway, this is modeled exactly on how the regulator subsystem > interact with its drivers to enumerate voltages. It seems intuitive > to me, it's an established kernel design pattern that drivers know > the hardware particulars, and so I don't get the problem here. > Would you argue that the regulator subsystem has bad design > too or is this case different? Well, I guess it does do something like this on a much smaller scale. I'm surprised this style exists; I'd expect the following: driver: pinmux_register_driver( ops list of pins list of functions); to be much simpler than: driver: pinmux_register_driver(ops) core: for loop: ops->list_functions ops->get_function_name ops->get_function_pins Still, this is something that doesn't affect the publically visible client interface, and is easy to change in the future without a lot of work, so probably not a huge deal. > Can't you just send some patch or example .h file for the API > you would like to see so I understand how you think about > this? Are your patches in git somewhere? It's much easier for me to pull at present than grab patches out of email; something I certainly need to fix... > I might be thinking inside the box of my current design here so > help me get out of it, I just have a hard time seeing how to > do it. > > > ** Similarly, I'd asked for at least some documentation about how to > > handle the "multi-width bus" problem, but I didn't see that. > > SORRY! Missed it. > > So sure that can be added, so something like this? > > A B C D E F G H > +---+ > 8 | o | o o o o o o o > | | > 7 | o | o o o o o o o > | | > 6 | o | o o o o o o o > +---+---+ > 5 | o | o | o o o o o o > +---+---+ +---+ > 4 o o o o o o | o | o > | | > 3 o o o o o o | o | o > | | > 2 o o o o o o | o | o > +-------+-------+-------+---+---+ > 1 | o o | o o | o o | o | o | > +-------+-------+-------+---+---+ > > (...) > > On the botton row at { A1, B1, C1, D1, E1, F1, G1, H1 } we have something > special - it's an external MMC bus that can be 2, 4 or 8 bits wide, and it will > consume 2, 4 or 8 pins respectively, so either { A1, B1 } are taken or > { A1, B1, C1, D1 } or all of them. If we use all 8 bits, we cannot use the SPI > port on pins { G4, G3, G2, G1 } of course. > > (...) > > A simple driver for the above example will work by setting bits 0, 1, 2, 3, > 4 or 5 into some register named MUX, so it enumerates its available > settings > and their pin assignments, and expose them like this: > > #include <linux/pinctrl/pinmux.h> > > struct foo_pmx_func { > char *name; > const unsigned int *pins; > const unsigned num_pins; > }; > > static unsigned int spi0_0_pins[] = { 0, 8, 16, 24 }; > static unsigned int i2c0_pins[] = { 24, 25 }; > static unsigned int spi0_1_pins[] = { 38, 46, 54, 62 }; > static unsigned int mmc0_1_pins[] = { 56, 57 }; > static unsigned int mmc0_2_pins[] = { 56, 57, 58, 59 }; > static unsigned int mmc0_3_pins[] = { 56, 57, 58, 59, 60, 61, 62, 63 }; > > static struct foo_pmx_func myfuncs[] = { > { > .name = "spi0-0", > .pins = spi0_0_pins, > .num_pins = ARRAY_SIZE(spi0_1_pins), > }, > { > .name = "i2c0", > .pins = i2c0_pins, > .num_pins = ARRAY_SIZE(i2c0_pins), > }, > { > .name = "spi0-1", > .pins = spi0_1_pins, > .num_pins = ARRAY_SIZE(spi0_1_pins), > }, > { > .name = "mmc0-2bit", > .pins = mmc0_1_pins, > .num_pins = ARRAY_SIZE(mmc0_1_pins), > }, > { > .name = "mmc0-4bit", > .pins = mmc0_2_pins, > .num_pins = ARRAY_SIZE(mmc0_2_pins), > }, > { > .name = "mmc0-8bit", > .pins = mmc0_3_pins, > .num_pins = ARRAY_SIZE(mmc0_3_pins), > }, > }; > > Looks OK? I think that's exactly the setup I was looking to avoid. See the portion of the thread starting from: https://lkml.org/lkml/2011/5/13/424 In particular, in: https://lkml.org/lkml/2011/5/18/33 I explained how I understood you intended to solve this (summarized just below) and you appeared to agree with that. The functions a pinmux driver exposes are purely driven by the smallest set of pins the HW can control at once (pin-by-pin for many HW I imagine, but groups of 1-10 pins on Tegra). To provide the "bus" abstraction, struct pinmux_map will be expanded to include N pinmux driver functions to activate per map entry. (I agreed that the actual implementation could be deferred to a later set of patches, but wanted the documentation to describe the final model so that we didn't end up people implementing the example you wrote above). So, in the example above: static unsigned int mmc0_1_pins[] = { 56, 57 }; static unsigned int mmc0_2_pins[] = { 58, 59 }; static unsigned int mmc0_3_pins[] = { 60, 61, 62, 63 }; { .name = "mmc0-1:0", .pins = mmc0_1_pins, .num_pins = ARRAY_SIZE(mmc0_1_pins), }, { .name = "mmc0-3:2", .pins = mmc0_2_pins, .num_pins = ARRAY_SIZE(mmc0_2_pins), }, { .name = "mmc0-7:4", .pins = mmc0_3_pins, .num_pins = ARRAY_SIZE(mmc0_3_pins), }, So a board that happens to use a 2-bit bus would define: static struct pinmux_map pmx_mapping[] = { { .functions = {"mmc0-1:0"}, .dev_name = "tegra-sdhci.0", }, }; But a board that happens to use an 8-bit bus would define: static struct pinmux_map pmx_mapping[] = { { .functions = {"mmc0-1:0", "mmc0-3:2", "mmc0-7:4"}, .dev_name = "tegra-sdhci.0", }, }; ... although struct pinmux_map would probably need to grow a separate name field to match against pinmux_get's 2nd parameter, rather than assuming the driver's function names and the mapping's function names were the same. Such a new name field would also introduce a second namespace that'd satisfy my next concern. > > ** How can the board/machine name pins? That should be a function of the > > chip (i.e. pinmux driver), since that's where the pins are located. A > > chip's pins don't have different names on different boards; the names of > > the signals on the PCB connected to those pins are defined by the board, > > but not the names of the actual pins themselves. > > Actually I don't really like the use of the concept of board and machine > for chip packages either. But if I say that without devicetrees it > could be something like arch/arm/plat-nomadik/package-db8500.c > defining the pins. Then it is still coming from the outside, for > a particular platform, for a particular chip package. > > Then the init function in that file gets called on relevant systems. > > As you can see in the example implementation for U300 I actually > do this in the driver itself by including the machine.h file to that one. > So this driver first registers pins, then functions. > > I think there is broad consensus that this should come in from > the device tree in the future. And if it comes from the device tree, it's > still the say arch/arm/plat-nomadik/package-db8500.c file that looks > up the pins from the device tree and registers them, just it gets the > data from the outside. I think discussion of DeviceTree here is a bit of a distraction; if the pinmux core APIs imply the correct data model, it doesn't seem especially relevant. For the pinmux drivers themselves, I expect the SoC's pinmux driver to define the set of pins available, and the set of functions available. A couple examples: There will be a single static set of pins/functions for any Tegra 2 device. Tegra 3 will have a different set of pins/functions since the HW changed quite a bit in this area. As such, DeviceTree doesn't really help much, since there will always be a custom driver per SoC, and the data a given driver uses will not vary between boards, so DeviceTree helps little. On the other hand, if you've got a very generic pinmux controller, and it gets included in N (versions of) SoCs with the same register layout, but the actual pin names related to each register bit are different, then I can see it making sense for the pinmux driver itself to parameterize itself with data from DeviceTree. Either way though, this is purely an implementation detail within the pinmux driver, and not something the core would even be aware of. However, I do think the board-specific list of struct pinmux_map will come from DeviceTree. This will vary from board to board with potentially little commonality. DeviceTree seems a great solution to keep all that varying data out of the kernel. The core pinmux code could well include the common code to parse the DeviceTree for this data. > Possibly the core could be made to do this. I know too little about > device tree I think :-/ > > > ** struct pinmux_map requires that each function name be globally unique, > > since one can only specify "function" not "function on a specific chip". > > This can't be a requirement; what if there are two instances of the same > > chip on the board (think some expansion chip attached to a memory-mapped > > bus rather than the primary SoC itself). > > Namespace clashes are a problem but the way I see it basically a > problem with how you design the namespace. It's just a string and > the document is just an example. The issue here is that the pinmux driver defines a single namespace for functions, and struct pinmux_map's definition passes that single namespace through, since the value of .function comes from that same namespace, and is also the search key for the list of driver-visible functions. As I alluded to above, if struct pinmux_map had a separate field for the search key (for pinmux_get's use) v.s. the set of driver functions that key maps to, then there can be separate namespaces, and a place to resolve conflicts: struct pinmux_map_entry { struct pinmux_dev *pmxdev const char *function; // pmxdev's specific function name-space }; struct pinmux_map { const char *name; // global driver-visible function name-space struct device *dev; const char *dev_name; int n_ functions; const struct pinmux_map_entry * functions; }; Thinking about this at a high level, this seems equivalent to the GPIO driver: There are two name-spaces there: 1) One used by clients: A global name-space, equivalent to pinmux_map.name. 2) One used by GPIO drivers: A per-driver name-space, equivalent to pinmux_map_entry.function. ... and the GPIO core maps from (1) to (2) above internally. In the pinmux case, since everything is named by a string rather than a linear number range, the mapping process needs the pinmux_map_entry.function field, rather than just substracting a GPIO driver's base GPIO number.
On Wed, Jun 15, 2011 at 12:11 AM, Stephen Warren <swarren@nvidia.com> wrote: > [Me] >> Can't you just send some patch or example .h file for the API >> you would like to see so I understand how you think about >> this? > > Are your patches in git somewhere? It's much easier for me to pull > at present than grab patches out of email; something I certainly need > to fix... Yep: git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-stericsson.git pinmux-subsystem It's based on v3.0-rc3 as of now. I tend to rebase it though. >> static unsigned int mmc0_1_pins[] = { 56, 57 }; >> static unsigned int mmc0_2_pins[] = { 56, 57, 58, 59 }; >> static unsigned int mmc0_3_pins[] = { 56, 57, 58, 59, 60, 61, 62, 63 }; >> >> static struct foo_pmx_func myfuncs[] = { >> { >> .name = "mmc0-2bit", >> .pins = mmc0_1_pins, >> .num_pins = ARRAY_SIZE(mmc0_1_pins), >> }, >> { >> .name = "mmc0-4bit", >> .pins = mmc0_2_pins, >> .num_pins = ARRAY_SIZE(mmc0_2_pins), >> }, >> { >> .name = "mmc0-8bit", >> .pins = mmc0_3_pins, >> .num_pins = ARRAY_SIZE(mmc0_3_pins), >> }, >> }; >> >> Looks OK? > > I think that's exactly the setup I was looking to avoid. See the portion > of the thread starting from: Gah. Baudelaire once said something like the reason people think they agree is that they misunderstand each other all the time. Sorry for my lameness :-/ > static unsigned int mmc0_1_pins[] = { 56, 57 }; > static unsigned int mmc0_2_pins[] = { 58, 59 }; > static unsigned int mmc0_3_pins[] = { 60, 61, 62, 63 }; > > { > .name = "mmc0-1:0", > .pins = mmc0_1_pins, > .num_pins = ARRAY_SIZE(mmc0_1_pins), > }, > { > .name = "mmc0-3:2", > .pins = mmc0_2_pins, > .num_pins = ARRAY_SIZE(mmc0_2_pins), > }, > { > .name = "mmc0-7:4", > .pins = mmc0_3_pins, > .num_pins = ARRAY_SIZE(mmc0_3_pins), > }, > > So a board that happens to use a 2-bit bus would define: > > static struct pinmux_map pmx_mapping[] = { > { > .functions = {"mmc0-1:0"}, > .dev_name = "tegra-sdhci.0", > }, > }; > > But a board that happens to use an 8-bit bus would define: > > static struct pinmux_map pmx_mapping[] = { > { > .functions = {"mmc0-1:0", "mmc0-3:2", "mmc0-7:4"}, > .dev_name = "tegra-sdhci.0", > }, > }; NOW I *think* I get it. So we're basically talking about the function mapping API here. So the idea is that one mapping can reference several functions. So with this scheme actually both ways of doing this would be possible IIUC. Note that this will be *quite* tricky to implement, since you have an array of undetermined size with elements of undetermined size in .functions = {"mmc0-1:0", "mmc0-3:2", "mmc0-7:4"}. You will need atleast to move the functions out of the mapping something like this: static char *mmc0_10[] = { "mmc0-1:0", "mmc0-3:2", "mmc0-7:4" }; static struct pinmux_map pmx_mapping[] = { { .functions = &mmc0_10, .functions_len = ARRAY_SIZE(mmc0_10); .dev_name = "tegra-sdhci.0", }, }; Which sort of kludges up the syntax for all the simple cases that will also have to add ARRAY_SIZE() and a separate string array for each of its single-function maps. So it has the downside of complicating the simple maps. What I think we need to understand at this point is what will be the most common case: that a mapping selects a single or multiple functions. If the case is rare I'd opt for my scheme (i.e. defining one function per bus width) to keep it simple, whereas if a vast majority of the function tables and mappings for say Tegra will be way simpler this way, it'll be worth the increase in complexity of the function mapping API. > ... although struct pinmux_map would probably need to grow a separate > name field I don't get this, the member "function" is a name of the function: struct pinmux_map { const char *function; (...) }; > to match against pinmux_get's 2nd parameter, rather than > assuming the driver's function names and the mapping's function names > were the same. > > Such a new name field would also introduce a second namespace that'd > satisfy my next concern. > >> > ** How can the board/machine name pins? That should be a function of the >> > chip (i.e. pinmux driver), since that's where the pins are located. A >> > chip's pins don't have different names on different boards; the names of >> > the signals on the PCB connected to those pins are defined by the board, >> > but not the names of the actual pins themselves. >> >> Actually I don't really like the use of the concept of board and machine >> for chip packages either. But if I say that without devicetrees it >> could be something like arch/arm/plat-nomadik/package-db8500.c >> defining the pins. Then it is still coming from the outside, for >> a particular platform, for a particular chip package. >> >> Then the init function in that file gets called on relevant systems. >> >> As you can see in the example implementation for U300 I actually >> do this in the driver itself by including the machine.h file to that one. >> So this driver first registers pins, then functions. >> >> I think there is broad consensus that this should come in from >> the device tree in the future. And if it comes from the device tree, it's >> still the say arch/arm/plat-nomadik/package-db8500.c file that looks >> up the pins from the device tree and registers them, just it gets the >> data from the outside. > > I think discussion of DeviceTree here is a bit of a distraction; if the > pinmux core APIs imply the correct data model, it doesn't seem especially > relevant. OK... maybe too much distraction here. All the DT noise makes me dizzy too :-P > For the pinmux drivers themselves, I expect the SoC's pinmux driver to > define the set of pins available, and the set of functions available. Yes, for those that map 1-to-1 for a certain platform. I'm a little bit sceptical due to the risk of getting into a situation where every implementation claims to be special and ending up having to consolidate a lot of stuff that really wasn't that special a few years down the road, like the ARM tree right now. But maybe that's our destiny so no big deal :-) > There will be a single static set of pins/functions for any Tegra 2 > device. Tegra 3 will have a different set of pins/functions since the > HW changed quite a bit in this area. As such, DeviceTree doesn't really > help much, since there will always be a custom driver per SoC, and the > data a given driver uses will not vary between boards, so DeviceTree > helps little. Yep I get it. s/device tree/board file/g and the same reasoning holds for anything coming from the platform/package/machine data. > On the other hand, if you've got a very generic pinmux controller, and > it gets included in N (versions of) SoCs with the same register layout, > but the actual pin names related to each register bit > are different, then I can see it making sense for the pinmux driver > itself to parameterize itself with data from DeviceTree. We have that situation already in Ux500. c.f.: arch/arm/mach-ux500/pins-db5500.h arch/arm/mach-ux500/pins-db8500.h Two ASICs, same set of hardware registers, but vastly different pin assignments. > Either way > though, this is purely an implementation detail within the pinmux driver, > and not something the core would even be aware of. OK with me for now atleast, I don't know about other people's ambitions. > However, I do think the board-specific list of struct pinmux_map will > come from DeviceTree. This will vary from board to board with potentially > little commonality. DeviceTree seems a great solution to keep all that > varying data out of the kernel. The core pinmux code could well include > the common code to parse the DeviceTree for this data. Agreed. >> > ** struct pinmux_map requires that each function name be globally unique, >> > since one can only specify "function" not "function on a specific chip". >> > This can't be a requirement; what if there are two instances of the same >> > chip on the board (think some expansion chip attached to a memory-mapped >> > bus rather than the primary SoC itself). >> >> Namespace clashes are a problem but the way I see it basically a >> problem with how you design the namespace. It's just a string and >> the document is just an example. > > The issue here is that the pinmux driver defines a single namespace for > functions, and struct pinmux_map's definition passes that single namespace > through, since the value of .function comes from that same namespace, and > is also the search key for the list of driver-visible functions. So this would be the case if you say had two identical chips on I2C, both of which could mix some their pins. That (in the sense of the present design) would be an argument to mandate to always pass the function names from platform data to drivers that handle more than once instance, so the function names are always unique. (This is more about understanding, really, tell me if I got it wrong.) > As I alluded to above, if struct pinmux_map had a separate field for the > search key (for pinmux_get's use) v.s. the set of driver functions that > key maps to, then there can be separate namespaces, and a place to resolve > conflicts: > > struct pinmux_map_entry { > struct pinmux_dev *pmxdev > const char *function; // pmxdev's specific function name-space > }; This looks scary to me: we're already trying to avoid putting struct device * into the map in favor of using dev_name to look it up. How do you intend to get struct pinmux_dev * into the map entry? Remember that the map is a static thing that is defined at compile-time, whereas struct pinmux_dev * is something that appears at runtime. It seems smarter to me to use some naming convention, but it definately need to keep track of instances, so we need something like the device_name() from the pinmux_dev:s struct device, that will be unique for each instance, then match on that. But maybe if you make a patch I can understand this better, I might just not see how this could work. > struct pinmux_map { > const char *name; // global driver-visible function name-space > struct device *dev; This struct device *dev is curretly unused btw. I even made my map a __initdata in the U300 case, and I think that should be the norm, this would only be useful in runtime, maybe with device trees. > const char *dev_name; > int n_ functions; > const struct pinmux_map_entry * functions; > }; > > Thinking about this at a high level, this seems equivalent to the GPIO > driver: Sorry, not following. Which GPIO driver? Are you referring to gpiolib? > There are two name-spaces there: > > 1) One used by clients: A global name-space, equivalent to > pinmux_map.name. > > 2) One used by GPIO drivers: A per-driver name-space, equivalent to > pinmux_map_entry.function. > > ... and the GPIO core maps from (1) to (2) above internally. > > In the pinmux case, since everything is named by a string rather than a > linear number range, the mapping process needs the pinmux_map_entry.function > field, rather than just substracting a GPIO driver's base GPIO number. I see you refer to the way that the global GPIO ID is mapped to an offset into the respective GPIO driver by subtracting the offset of each GPIO chip. (Correct me if wrong.) I think I understand what you mean, but I would claim that that an unsigned integer number space has quite different semantics from a namespace for them to be compared in that way. For example it is very simple and quick to map one into the other by a simple subtraction of an offset. In this case, with the design above you need to know two namespaces instead of one when writing your platform data, and that makes things more complex in my view. So instead of using a simple 1D string "chip0::mmc0" to identify some function in a specific driver, you use the tuple ("chip0", "mmc0") and I think it's getting complex and hard to express as struct members, but that may be because I don't quite see how clever this is. To me it seems plausible that the easiest way to do this is that if a driver wants a private namespace it just defines a separator such as "::" in above and we provide some simple string split function to get chipname and function name from that. Non-surprsingly, that is exactly what the drivers/leds subsystem commonly does to identify LEDs on different chips: drivers/leds$ grep '::' * dell-led.c: .name = "dell::lid", leds-ams-delta.c: .name = "ams-delta::camera", leds-ams-delta.c: .name = "ams-delta::advert", leds-ams-delta.c: .name = "ams-delta::email", leds-ams-delta.c: .name = "ams-delta::handsfree", leds-ams-delta.c: .name = "ams-delta::voicemail", leds-ams-delta.c: .name = "ams-delta::voice", leds-clevo-mail.c: .name = "clevo::mail", leds-cobalt-qube.c: .name = "qube::front", leds-cobalt-raq.c: .name = "raq::web", leds-cobalt-raq.c: .name = "raq::power-off", leds-net48xx.c: .name = "net48xx::error", leds-wrap.c: .name = "wrap::power", leds-wrap.c: .name = "wrap::error", leds-wrap.c: .name = "wrap::extra", So can't we use such a convention and some helpers like drivers/leds does instead of trying to actually match split the namespace to C struct members? It's still a namespace, it's just not expressed in C, but in string structure. If the first part (before the "::") shall be unique per chip instance, that can be generated in runtime from the device name or some specific .init_name to make sure it does not vary too much. If you're uncertain what to think about this I can implement this namespacing scheme for U300 so we have some example? So to summarize there are two related areas of discussion here: 1. Whether a pinmux map shall map one or 1..N functions 2. How to handle per-driver instance namespacing of functions In both cases I'm currently using simple strings and claiming that by namespacing these strings cleverly we can avoid complexity. So my answer to these are: 1. Use several functions with ovelapping maps, just name them differently 2. Use a string convention and namespace by using platform/machine/package data and string conventions such as a "::" separator While I *think* (and DO correct me!) that you would argue: 1. Make it possible to map several functions to a single device map 2. Namespace device instances by different map field members referring to specific instances Is this correctly understood, even if we may not agree? Thanks, Linus Walleij
Linus Walleij wrote at Thursday, June 16, 2011 6:47 AM: > On Wed, Jun 15, 2011 at 12:11 AM, Stephen Warren <swarren@nvidia.com> wrote: > > [Me] > >> Can't you just send some patch or example .h file for the API > >> you would like to see so I understand how you think about > >> this? > > > > Are your patches in git somewhere? It's much easier for me to pull > > at present than grab patches out of email; something I certainly need > > to fix... > > Yep: > git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-stericsson.git > pinmux-subsystem > It's based on v3.0-rc3 as of now. I tend to rebase it though. Great. Thanks. > >> static unsigned int mmc0_1_pins[] = { 56, 57 }; > >> static unsigned int mmc0_2_pins[] = { 56, 57, 58, 59 }; > >> static unsigned int mmc0_3_pins[] = { 56, 57, 58, 59, 60, 61, 62, 63 }; > >> > >> static struct foo_pmx_func myfuncs[] = { > >> { > >> .name = "mmc0-2bit", > >> .pins = mmc0_1_pins, > >> .num_pins = ARRAY_SIZE(mmc0_1_pins), > >> }, > >> { > >> .name = "mmc0-4bit", > >> .pins = mmc0_2_pins, > >> .num_pins = ARRAY_SIZE(mmc0_2_pins), > >> }, > >> { > >> .name = "mmc0-8bit", > >> .pins = mmc0_3_pins, > >> .num_pins = ARRAY_SIZE(mmc0_3_pins), > >> }, > >> }; > >> > >> Looks OK? > > > > I think that's exactly the setup I was looking to avoid. See the portion > > of the thread starting from: > > Gah. Baudelaire once said something like the reason people think > they agree is that they misunderstand each other all the time. > Sorry for my lameness :-/ > > > static unsigned int mmc0_1_pins[] = { 56, 57 }; > > static unsigned int mmc0_2_pins[] = { 58, 59 }; > > static unsigned int mmc0_3_pins[] = { 60, 61, 62, 63 }; > > > > { > > .name = "mmc0-1:0", > > .pins = mmc0_1_pins, > > .num_pins = ARRAY_SIZE(mmc0_1_pins), > > }, > > { > > .name = "mmc0-3:2", > > .pins = mmc0_2_pins, > > .num_pins = ARRAY_SIZE(mmc0_2_pins), > > }, > > { > > .name = "mmc0-7:4", > > .pins = mmc0_3_pins, > > .num_pins = ARRAY_SIZE(mmc0_3_pins), > > }, > > > > So a board that happens to use a 2-bit bus would define: > > > > static struct pinmux_map pmx_mapping[] = { > > { > > .functions = {"mmc0-1:0"}, > > .dev_name = "tegra-sdhci.0", > > }, > > }; > > > > But a board that happens to use an 8-bit bus would define: > > > > static struct pinmux_map pmx_mapping[] = { > > { > > .functions = {"mmc0-1:0", "mmc0-3:2", "mmc0-7:4"}, > > .dev_name = "tegra-sdhci.0", > > }, > > }; > > NOW I *think* I get it. > > So we're basically talking about the function mapping API here. Yes, basically. In more detail, I'm talking about making the "functions" exposed to clients of the pinmux be a different set than the "functions" implemented by the pinmux driver. Perhaps we should call the former "mappings" not make that clear; I see you did just below! > So the idea is that one mapping can reference several functions. Yes. > So with this scheme actually both ways of doing this would be > possible IIUC. Yes, it's certainly possible for a board/machine/... to define mappings that expose the bus in the chunks that the HW supports, or aggregate the whole thing. But, I think that's slightly missing the point. I would expect: The pinmux driver itself to *always* expose functions as the raw chunks that the HW supports (whether HW supports individual pins, or groups of pins, whether multiple pins/groups end up being used as a bus or not). For Tegra's keyboard controller, that might be the following functions (actual names/widths probably incorrect; just for example): row[3:0] row[7:4] row[9:8] col[3:0] col[7:4] Recall that Tegra's keyboard controller module can be used with many different combinations of those sets of pins. I assert the pinmux driver itself needs to work identically, and expose the same data (functions, pins) without any knowledge of the board it's being used on. The mappings, provided by and specific to a particular board/machine would always expose only the exact configurations actually used on that board: mapping device: "tegra-kbc" mapping name: "config a" mapping function list: row[7:4], row[3:0], col[3:0] (note how I added a "mapping name" field here; more on this below. This is related to mapping and function names being different things) As a single mapping that the driver will just get and use. Note that if the driver is complex, and can dynamically switch between different setups at run-time on a specific board, then the mappings list might still contain multiple entries for the same device, which the device driver could get all of and switch between. However, I'd expect that to be pretty rare: mapping device: "tegra-kbc" mapping name: "config a" mapping function list: row[7:4], row[3:0], col[3:0] mapping device: "tegra-kbc" mapping name: "config b" mapping function list: row[3:0], col[3:0] > Note that this will be *quite* tricky to implement, since you have > an array of undetermined size with elements of undetermined size > in .functions = {"mmc0-1:0", "mmc0-3:2", "mmc0-7:4"}. > > You will need atleast to move the functions out of the mapping > something like this: > > static char *mmc0_10[] = { "mmc0-1:0", "mmc0-3:2", "mmc0-7:4" }; > > static struct pinmux_map pmx_mapping[] = { > { > .functions = &mmc0_10, > .functions_len = ARRAY_SIZE(mmc0_10); > .dev_name = "tegra-sdhci.0", > }, > }; > > Which sort of kludges up the syntax for all the simple cases that will > also have to add ARRAY_SIZE() and a separate string array for > each of its single-function maps. > > So it has the downside of complicating the simple maps. Yes, you're right. I think I have a simpler solution that degenerates to the same syntax as in your current patch. Starting with your original: static struct pinmux_map pmx_mapping[] = { { .dev_name = "tegra-sdhci.0", .function = "mmc0-1:0", }, (where devices look up mappings by ".function", among other options) We then add a new "mapping name" field; you'll see why soon: static struct pinmux_map pmx_mapping[] = { { .dev_name = "tegra-sdhci.0", .map_name = "2 bit", .function = "mmc0-1:0", }, (where we now use ".map_name" for searching the list instead of ".function", which ties into my comment above about having explicit different sets of names for mapping entries and low-level pinmux driver internal function names. We can still set ".map_name" = NULL and omit it in the simple case where drivers search based on ".dev_name" and don't specify any specific .map_name to search for, and don't have multiple options for mappings they can switch between. Now, we can have multiple entries with the same .map_name: static struct pinmux_map pmx_mapping[] = { { .dev_name = "tegra-sdhci.0", .map_name = "4 bit", # Note this is 4 now .function = "mmc0-1:0", }, { .dev_name = "tegra-sdhci.0", .map_name = "4 bit", .function = "mmc0-3:2", }, This means that if a client request map name "4 bit", then both functions "mmc0-1:0" and "mmc0-3:2" are part of that mapping. If there is only a single logical mapping, and hence .map_name is omitted, there could still be multiple entries in the list for the same dev_name; all entries with .map_name==NULL are considered the same name. Putting the above in database terms, the primary key (albeit not unique) is the tuple (.dev_name, .map_name), and the set of .function values for all rows with that same primary key are logically part of the same mapping. (Yes, this isn't correct normalized form for a database, but it makes the simpler cases simpler to represent) We can still have multiple .map_name options for a given .dev_name, by simply combining the previous two examples: static struct pinmux_map pmx_mapping[] = { { .dev_name = "tegra-sdhci.0", .map_name = "2 bit", .function = "mmc0-1:0", }, { .dev_name = "tegra-sdhci.0", .map_name = "4 bit", .function = "mmc0-1:0", }, { .dev_name = "tegra-sdhci.0", .map_name = "4 bit", .function = "mmc0-3:2", }, And as a final addition to this example, suppose we have a second MMC controller, it can also have a "2 bit" map_name entry, since I'd like to assert that ".map_name" be interpreted relative/within/for a given .dev_name in most cases; lookup only by .map_name without a .dev_name should be rare. { .dev_name = "tegra-sdhci.1", .map_name = "2 bit", .function = "mmc1-1:0", }, In terms of implementation, this is still pretty easy; all we do when reserving or activating a given mapping is to walk the entire list, find *all* entries that match the client's search terms, and operate on all of them, instead of stopping at the first one. struct pinmux, as returned by pinmux_get(), would need some adjustments to store either an array of map entries, or just the .map_name of the found entry/entries, so the loop could be repeated later. > What I think we need to understand at this point is what will be the > most common case: that a mapping selects a single or multiple > functions. If the case is rare I'd opt for my scheme (i.e. defining > one function per bus width) to keep it simple, whereas if a vast > majority of the function tables and mappings for say Tegra will be > way simpler this way, it'll be worth the increase in complexity > of the function mapping API. So sorry but just to hammer home my point, the disadvantages of the pinmux driver itself (as opposed to the mapping list) aggregating multiple pins or groups-of-pins into functions for each supported combination are: * Potential explosion of functions exposed. * The same point, but think about a SW bit-banged bus consisting of 8 GPIOs for the bus and 100 pins on the device. Each permutation of 8 out of 100 is scary... I'd love a board to be able to represent a single mapping for a particular board's set of GPIOs in this case, but not for the pinmux driver to expose all possibilities! * By having the pinmux driver expose the pins/groups in exactly the way the HW supports, the pinmux driver is purely a simple hardware driver, and doesn't know anything much beyond "program this pin/group to be this function". All the logic of aggregating sets of pins/groups-of-pins into mappings is defined by the board/machine/... when it sets up the mapping table. I like keeping the pinmux driver simpler, and having the boards describe functions in terms of which pins/groups should be set to which function, rather than picking for a pre-defined large list of all possibilities. > > ... although struct pinmux_map would probably need to grow a separate > > name field > > I don't get this, the member "function" is a name of the function: > > struct pinmux_map { > const char *function; > (...) > }; Hopefully I explained this well above; it's the difference between a function name known to the pinmux driver vs. a mapping name known to the pinmux clients. > > to match against pinmux_get's 2nd parameter, rather than > > assuming the driver's function names and the mapping's function names > > were the same. > > > > Such a new name field would also introduce a second namespace that'd > > satisfy my next concern. > > > >> > ** How can the board/machine name pins? That should be a function of the > >> > chip (i.e. pinmux driver), since that's where the pins are located. A > >> > chip's pins don't have different names on different boards; the names of > >> > the signals on the PCB connected to those pins are defined by the board, > >> > but not the names of the actual pins themselves. > >> > >> Actually I don't really like the use of the concept of board and machine > >> for chip packages either. But if I say that without devicetrees it > >> could be something like arch/arm/plat-nomadik/package-db8500.c > >> defining the pins. Then it is still coming from the outside, for > >> a particular platform, for a particular chip package. > >> > >> Then the init function in that file gets called on relevant systems. > >> > >> As you can see in the example implementation for U300 I actually > >> do this in the driver itself by including the machine.h file to that one. > >> So this driver first registers pins, then functions. > >> > >> I think there is broad consensus that this should come in from > >> the device tree in the future. And if it comes from the device tree, it's > >> still the say arch/arm/plat-nomadik/package-db8500.c file that looks > >> up the pins from the device tree and registers them, just it gets the > >> data from the outside. > > > > I think discussion of DeviceTree here is a bit of a distraction; if the > > pinmux core APIs imply the correct data model, it doesn't seem especially > > relevant. > > OK... maybe too much distraction here. All the DT noise > makes me dizzy too :-P > > > For the pinmux drivers themselves, I expect the SoC's pinmux driver to > > define the set of pins available, and the set of functions available. > > Yes, for those that map 1-to-1 for a certain platform. Personally, I think in all cases. For a pinmux driver that can apply to different platforms (e.g. the same register set applies to n different SoCs with different pin layouts), I think the pinmux driver itself can be parameterized with the mapping from register bits/fields to function names exported to the pinmux core. The parameterization could come from platform data, DeviceTree, ... > I'm a little bit sceptical due to the risk of getting into a situation > where every implementation claims to be special and ending up > having to consolidate a lot of stuff that really wasn't that special a few > years down the road, like the ARM tree right now. But maybe that's > our destiny so no big deal :-) > > > There will be a single static set of pins/functions for any Tegra 2 > > device. Tegra 3 will have a different set of pins/functions since the > > HW changed quite a bit in this area. As such, DeviceTree doesn't really > > help much, since there will always be a custom driver per SoC, and the > > data a given driver uses will not vary between boards, so DeviceTree > > helps little. > > Yep I get it. s/device tree/board file/g and the same reasoning holds > for anything coming from the platform/package/machine data. > > > On the other hand, if you've got a very generic pinmux controller, and > > it gets included in N (versions of) SoCs with the same register layout, > > but the actual pin names related to each register bit > > are different, then I can see it making sense for the pinmux driver > > itself to parameterize itself with data from DeviceTree. > > We have that situation already in Ux500. c.f.: > arch/arm/mach-ux500/pins-db5500.h > arch/arm/mach-ux500/pins-db8500.h > > Two ASICs, same set of hardware registers, but vastly different pin > assignments. OK, does the idea I floated a little above work; having the pinmux driver itself get its register->function-name mapping from some data structure provided to the pinmux driver? As you'll no doubt realize, I think that works:-) > > Either way > > though, this is purely an implementation detail within the pinmux driver, > > and not something the core would even be aware of. > > OK with me for now atleast, I don't know about other people's > ambitions. That sounds like you agree:-) > > However, I do think the board-specific list of struct pinmux_map will > > come from DeviceTree. This will vary from board to board with potentially > > little commonality. DeviceTree seems a great solution to keep all that > > varying data out of the kernel. The core pinmux code could well include > > the common code to parse the DeviceTree for this data. > > Agreed. Great. > >> > ** struct pinmux_map requires that each function name be globally unique, > >> > since one can only specify "function" not "function on a specific chip". > >> > This can't be a requirement; what if there are two instances of the same > >> > chip on the board (think some expansion chip attached to a memory-mapped > >> > bus rather than the primary SoC itself). > >> > >> Namespace clashes are a problem but the way I see it basically a > >> problem with how you design the namespace. It's just a string and > >> the document is just an example. > > > > The issue here is that the pinmux driver defines a single namespace for > > functions, and struct pinmux_map's definition passes that single namespace > > through, since the value of .function comes from that same namespace, and > > is also the search key for the list of driver-visible functions. > > So this would be the case if you say had two identical chips on I2C, both > of which could m[u]x some their pins. Yes. > That (in the sense of the present design) would be an argument to mandate > to always pass the function names from platform data to drivers that > handle more than once instance, so the function names are always > unique. > > (This is more about understanding, really, tell me if I got it wrong.) I think function names should be interpreted only within the context of a specific pinmux driver instance. I think mapping table entry names should generally be used in conjunction with a device name for lookup, so it's clear which device's mappings you're searching for. Now, the final question is, given a mapping entry: { .dev_name = "tegra-sdhci.0", .map_name = "2 bit", .function = "mmc0-1:0", }, How do you know which pinmux driver to go to when activating the function? I think the answer is to expand the mapping table again, to be: { .dev_name = "tegra-sdhci.0", .map_name = "2 bit", .pmx_dev_name = "tegra-pinmux.0", .function = "mmc0-1:0", }, The above entry means: For device "tegra-sdhci.0", when it wants to activate its pinmux mapping named "2 bit", (where that name is optional if there's only 1 for the device) go to pinmux driver names "tegra-pinmux.0", and activate function "mmc0-1:0" there. Each registered pinmux driver would need a name. To cover the case of multiple instances of the exact same driver, the driver's name could e.g. encode I2C bus number and address for example "foochip@0.1a". I think that gpiolib names gpiochips along those lines? The naming that ASoC (ALSA for Systems on Chip; sound/soc/*) uses for I2C devices such as codecs works just like this. > > As I alluded to above, if struct pinmux_map had a separate field for the > > search key (for pinmux_get's use) v.s. the set of driver functions that > > key maps to, then there can be separate namespaces, and a place to resolve > > conflicts: > > > > struct pinmux_map_entry { > > struct pinmux_dev *pmxdev > > const char *function; // pmxdev's specific function name-space > > }; > > This looks scary to me: we're already trying to avoid putting > struct device * into the map in favor of using dev_name to look > it up. > > How do you intend to get struct pinmux_dev * into the map > entry? Yes, having "struct pinmux_dev *" here isn't a great idea. Using a device's name works much better. > Remember that the map is a static thing that is defined at > compile-time, whereas struct pinmux_dev * is something > that appears at runtime. > > It seems smarter to me to use some naming convention, > but it definately need to keep track of instances, so we > need something like the device_name() from the > pinmux_dev:s struct device, that will be unique for each > instance, then match on that. OK, I think you're suggesting basically what I wrote above; my email reading lookahead buffer needs to be larger:-) > But maybe if you make a patch I can understand this better, > I might just not see how this could work. > > > struct pinmux_map { > > const char *name; // global driver-visible function name-space > > struct device *dev; > > This struct device *dev is curretly unused btw. > > I even made my map a __initdata in the U300 case, and I think > that should be the norm, this would only be useful in runtime, > maybe with device trees. Hmm. I think the mapping table ends up being used at runtime a lot; I think it's reasonable for drivers to be able to expect to pinmux_get() at any time. New pinmux devices could be added/removed at arbitrary times, e.g. if a hotplugged PCI or USB device contained pinmuxing capabilities. > > const char *dev_name; > > int n_ functions; > > const struct pinmux_map_entry * functions; > > }; > > > > Thinking about this at a high level, this seems equivalent to the GPIO > > driver: > > Sorry, not following. Which GPIO driver? Are you referring to > gpiolib? Yes. > > There are two name-spaces there: > > > > 1) One used by clients: A global name-space, equivalent to > > pinmux_map.name. > > > > 2) One used by GPIO drivers: A per-driver name-space, equivalent to > > pinmux_map_entry.function. > > > > ... and the GPIO core maps from (1) to (2) above internally. > > > > In the pinmux case, since everything is named by a string rather than a > > linear number range, the mapping process needs the pinmux_map_entry.function > > field, rather than just substracting a GPIO driver's base GPIO number. > > I see you refer to the way that the global GPIO ID is mapped to an > offset into the respective GPIO driver by subtracting the offset of each > GPIO chip. (Correct me if wrong.) Yes. > I think I understand what you mean, but I would claim that that an > unsigned integer number space has quite different semantics from a > namespace for them to be compared in that way. For example > it is very simple and quick to map one into the other by a simple > subtraction of an offset. > > In this case, with the design above you need to know two > namespaces instead of one when writing your platform data, > and that makes things more complex in my view. Well, I suppose two namespaces is indeed more complex than 1. However, in the simple case of no duplicates, you can probably get away with using the same names in each namespace... And having the option to use two separate namespaces solves a bunch of problems I've talked about above. > So instead of using a simple 1D string "chip0::mmc0" to identify > some function in a specific driver, you use the tuple > ("chip0", "mmc0") and I think it's getting complex and hard to > express as struct members, but that may be because I don't > quite see how clever this is. > > To me it seems plausible that the easiest way > to do this is that if a driver wants a private namespace > it just defines a separator such as "::" in above and we provide > some simple string split function to get chipname and function > name from that. > > Non-surprsingly, that is exactly what the drivers/leds subsystem > commonly does to identify LEDs on different chips: Interesting. Having a mapping be: { .dev_name = "tegra-sdhci.0", .map_name = "2 bit", .function = " tegra-pinmux.0::mmc0-1:0", }, Instead of: { .dev_name = "tegra-sdhci.0", .map_name = "2 bit", .pmx_dev_name = "tegra-pinmux.0", .function = "mmc0-1:0", }, Seems fine to me. However, having the pinmux drivers statically define their prefixes in strings at compile-time doesn't seem correct; it prevents one from having multiple instances of the same pinmux/led driver; having separate fields makes this pretty easy, and avoids having to dynamically construct prefixed strings at runtime. > drivers/leds$ grep '::' * > dell-led.c: .name = "dell::lid", > leds-ams-delta.c: .name = "ams-delta::camera", > leds-ams-delta.c: .name = "ams-delta::advert", > leds-ams-delta.c: .name = "ams-delta::email", > leds-ams-delta.c: .name = "ams-delta::handsfree", > leds-ams-delta.c: .name = "ams-delta::voicemail", > leds-ams-delta.c: .name = "ams-delta::voice", > leds-clevo-mail.c: .name = "clevo::mail", > leds-cobalt-qube.c: .name = "qube::front", > leds-cobalt-raq.c: .name = "raq::web", > leds-cobalt-raq.c: .name = "raq::power-off", > leds-net48xx.c: .name = "net48xx::error", > leds-wrap.c: .name = "wrap::power", > leds-wrap.c: .name = "wrap::error", > leds-wrap.c: .name = "wrap::extra", > > So can't we use such a convention and some helpers > like drivers/leds does instead of trying to actually match > split the namespace to C struct members? It's still a > namespace, it's just not expressed in C, but in string > structure. > > If the first part (before the "::") shall be unique per > chip instance, that can be generated in runtime from > the device name or some specific .init_name to make > sure it does not vary too much. > > If you're uncertain what to think about this I can implement > this namespacing scheme for U300 so we have some example? > > So to summarize there are two related areas of discussion > here: > > 1. Whether a pinmux map shall map one or 1..N functions > 2. How to handle per-driver instance namespacing of functions > > In both cases I'm currently using simple strings and claiming > that by namespacing these strings cleverly we can avoid > complexity. So my answer to these are: > > 1. Use several functions with ovelapping maps, just name > them differently > 2. Use a string convention and namespace by using > platform/machine/package data and string conventions > such as a "::" separator OK. I think I've address what I see as the disadvantages in those solutions in my rather long-wided discussions above:-) > While I *think* (and DO correct me!) that you would argue: > > 1. Make it possible to map several functions to a single > device map Yes. > 2. Namespace device instances by different map field > members referring to specific instances Yes. > Is this correctly understood, even if we may not agree? > > Thanks, > Linus Walleij -- nvpublic
On Thu, Jun 16, 2011 at 9:10 PM, Stephen Warren <swarren@nvidia.com> wrote: > Linus Walleij wrote at Thursday, June 16, 2011 6:47 AM: >> NOW I *think* I get it. >> >> So we're basically talking about the function mapping API here. > > Yes, basically. > > In more detail, I'm talking about making the "functions" exposed to > clients of the pinmux be a different set than the "functions" > implemented by the pinmux driver. Perhaps we should call the former > "mappings" not make that clear; I see you did just below! OK I get it now I believe. > The mappings, provided by and specific to a particular board/machine > would always expose only the exact configurations actually used on that > board: > > mapping device: "tegra-kbc" > mapping name: "config a" > mapping function list: row[7:4], row[3:0], col[3:0] > > (note how I added a "mapping name" field here; more on this below. This > is related to mapping and function names being different things) OK so one mapping reference several functions in this way, I see. >> You will need atleast to move the functions out of the mapping >> something like this: >> >> static char *mmc0_10[] = { "mmc0-1:0", "mmc0-3:2", "mmc0-7:4" }; >> >> static struct pinmux_map pmx_mapping[] = { >> { >> .functions = &mmc0_10, >> .functions_len = ARRAY_SIZE(mmc0_10); >> .dev_name = "tegra-sdhci.0", >> }, >> }; >> >> Which sort of kludges up the syntax for all the simple cases that will >> also have to add ARRAY_SIZE() and a separate string array for >> each of its single-function maps. >> >> So it has the downside of complicating the simple maps. > > Yes, you're right. I think I have a simpler solution that degenerates to > the same syntax as in your current patch. Starting with your original: > > static struct pinmux_map pmx_mapping[] = { > { > .dev_name = "tegra-sdhci.0", > .function = "mmc0-1:0", > }, > > (where devices look up mappings by ".function", among other options) > > We then add a new "mapping name" field; you'll see why soon: > > static struct pinmux_map pmx_mapping[] = { > { > .dev_name = "tegra-sdhci.0", > .map_name = "2 bit", > .function = "mmc0-1:0", > }, > > (where we now use ".map_name" for searching the list instead of > ".function", which ties into my comment above about having explicit > different sets of names for mapping entries and low-level pinmux driver > internal function names. > > We can still set ".map_name" = NULL and omit it in the simple case where > drivers search based on ".dev_name" and don't specify any specific > .map_name to search for, and don't have multiple options for mappings > they can switch between. > > Now, we can have multiple entries with the same .map_name: > > static struct pinmux_map pmx_mapping[] = { > { > .dev_name = "tegra-sdhci.0", > .map_name = "4 bit", # Note this is 4 now > .function = "mmc0-1:0", > }, > { > .dev_name = "tegra-sdhci.0", > .map_name = "4 bit", > .function = "mmc0-3:2", > }, > > This means that if a client request map name "4 bit", then both functions > "mmc0-1:0" and "mmc0-3:2" are part of that mapping. (...) > In terms of implementation, this is still pretty easy; all we do when > reserving or activating a given mapping is to walk the entire list, find > *all* entries that match the client's search terms, and operate on all of > them, instead of stopping at the first one. So: pmx = pinmux_get(dev, "4 bit"); in this case would reserve pins for two functions on pinmux_get() and activate two different functions after one another when we pinmux_enable(pmx) on this, "mmc0-1:0" and "mmc0-3:2" in some undefined order (inferenced across namespace). I don't think it's as simple as you say, this gets hairy pretty quickly: Since my previous pinmux_get() would create a struct pinmux * cookie for each map entry, assuming a 1:1 relationship between map entries and pinmuxes, we now break this, and you need to introduce more complex logic to search through the pinmux_list and dynamically add more functions to each entry with a matching map_name. Then you need to take care of the case where acquiring pins for one of the functions fail: then you need to go back and free the already acquired pins in the error path. I tried quickly to see if I could code this up, and it got very complex real quick, sadly. Maybe if I can just get to iterate a v4 first, I could venture down this track. But if you can code up the patch for this, I'm pretty much game for it! > struct pinmux, as returned by pinmux_get(), would need some adjustments > to store either an array of map entries, or just the .map_name of the > found entry/entries, so the loop could be repeated later. Yes if we can just write the code for it I buy into it. :-) > So sorry but just to hammer home my point, the disadvantages of the pinmux > driver itself (as opposed to the mapping list) aggregating multiple pins > or groups-of-pins into functions for each supported combination are: > > * Potential explosion of functions exposed. Yes I understand this, what I need to know if this is a problem in real life, i.e. that it's not just a function explosion in theory on some 5000 pin chip with a plethora of mux capabilities, but on real chips existing now, so it's worth all the extra abstraction and code incurred by this. But I do trust you if you say it's a real problem on the Tegra. > * The same point, but think about a SW bit-banged bus consisting of 8 > GPIOs for the bus and 100 pins on the device. Each permutation of 8 out > of 100 is scary... I'd love a board to be able to represent a single > mapping for a particular board's set of GPIOs in this case, but not for > the pinmux driver to expose all possibilities! Is this a real world problem or a theoretical one? Seems like the latter, and as stated in the documentation this subsystem is not about solving discrete maths permutation spaces, but practical enumerations. > * By having the pinmux driver expose the pins/groups in exactly the way > the HW supports, the pinmux driver is purely a simple hardware driver, > and doesn't know anything much beyond "program this pin/group to be this > function". All the logic of aggregating sets of pins/groups-of-pins into > mappings is defined by the board/machine/... when it sets up the mapping > table. I like keeping the pinmux driver simpler, and having the boards > describe functions in terms of which pins/groups should be set to which > function, rather than picking for a pre-defined large list of all > possibilities. This is a real good argument and I buy it wholesale, if the associated cost in code complexity does not run amok. > For a pinmux driver that can apply to different platforms (e.g. the same > register set applies to n different SoCs with different pin layouts), I > think the pinmux driver itself can be parameterized with the mapping from > register bits/fields to function names exported to the pinmux core. The > parameterization could come from platform data, DeviceTree, ... (...) >> We have that situation already in Ux500. c.f.: >> arch/arm/mach-ux500/pins-db5500.h >> arch/arm/mach-ux500/pins-db8500.h >> >> Two ASICs, same set of hardware registers, but vastly different pin >> assignments. > > OK, does the idea I floated a little above work; having the pinmux driver > itself get its register->function-name mapping from some data structure > provided to the pinmux driver? Yes I think so. We can adapt to that. [next subject] > I think mapping table entry names should generally be used in conjunction > with a device name for lookup, so it's clear which device's mappings > you're searching for. > > Now, the final question is, given a mapping entry: > > { > .dev_name = "tegra-sdhci.0", > .map_name = "2 bit", > .function = "mmc0-1:0", > }, > > How do you know which pinmux driver to go to when activating the function? > > I think the answer is to expand the mapping table again, to be: > > { > .dev_name = "tegra-sdhci.0", > .map_name = "2 bit", > .pmx_dev_name = "tegra-pinmux.0", > .function = "mmc0-1:0", > }, > > The above entry means: > > For device "tegra-sdhci.0", > when it wants to activate its pinmux mapping named "2 bit", > (where that name is optional if there's only 1 for the device) > go to pinmux driver names "tegra-pinmux.0", > and activate function "mmc0-1:0" there. > > Each registered pinmux driver would need a name. To cover the case of > multiple instances of the exact same driver, the driver's name could > e.g. encode I2C bus number and address for example "foochip@0.1a". I > think that gpiolib names gpiochips along those lines? The naming that > ASoC (ALSA for Systems on Chip; sound/soc/*) uses for I2C devices > such as codecs works just like this. OK sounds reasonable. But like we have an optional struct device *dev in the mapping as an alternative to .dev_name we should have an optional .pmx_dev too. But I get the idea, and it'll probably work. >> Non-surprsingly, that is exactly what the drivers/leds subsystem >> commonly does to identify LEDs on different chips: > > Interesting. Having a mapping be: > > { > .dev_name = "tegra-sdhci.0", > .map_name = "2 bit", > .function = " tegra-pinmux.0::mmc0-1:0", > }, > > Instead of: > > { > .dev_name = "tegra-sdhci.0", > .map_name = "2 bit", > .pmx_dev_name = "tegra-pinmux.0", > .function = "mmc0-1:0", > }, > > Seems fine to me. No, the other idea with a pmx_dev_name is much better, I was misguided in this. That namespace style only gets hard to maintain I think. > However, having the pinmux drivers statically define their prefixes in > strings at compile-time doesn't seem correct; it prevents one from having > multiple instances of the same pinmux/led driver; having separate fields > makes this pretty easy, and avoids having to dynamically construct > prefixed strings at runtime. You are right. >> While I *think* (and DO correct me!) that you would argue: >> >> 1. Make it possible to map several functions to a single >> device map > > Yes. > >> 2. Namespace device instances by different map field >> members referring to specific instances > > Yes. Atleast we know what the remaining problem space is now, surely we can iron this out. But I think I need some working code for that. Now I will post some v4 version of the framework, with some minor corrections and bugfixes, then we can do this larger redesign on top of that, patches welcome! I'll put in a git link. Thanks. Linus Walleij
Linus Walleij wrote at Monday, June 27, 2011 8:35 AM: > On Thu, Jun 16, 2011 at 9:10 PM, Stephen Warren <swarren@nvidia.com> wrote: ... > > Now, we can have multiple entries with the same .map_name: > > > > static struct pinmux_map pmx_mapping[] = { > > { > > .dev_name = "tegra-sdhci.0", > > .map_name = "4 bit", # Note this is 4 now > > .function = "mmc0-1:0", > > }, > > { > > .dev_name = "tegra-sdhci.0", > > .map_name = "4 bit", > > .function = "mmc0-3:2", > > }, > > > > This means that if a client request map name "4 bit", then both functions > > "mmc0-1:0" and "mmc0-3:2" are part of that mapping. > > (...) > > > In terms of implementation, this is still pretty easy; all we do when > > reserving or activating a given mapping is to walk the entire list, find > > *all* entries that match the client's search terms, and operate on all of > > them, instead of stopping at the first one. > > So: > pmx = pinmux_get(dev, "4 bit"); > > in this case would reserve pins for two functions on pinmux_get() and > activate two different functions after one another when > we pinmux_enable(pmx) on this, "mmc0-1:0" and "mmc0-3:2" in some > undefined order (inferenced across namespace). > > I don't think it's as simple as you say, this gets hairy pretty quickly: > > Since my previous pinmux_get() would create a struct pinmux * cookie > for each map entry, assuming a 1:1 relationship between map entries > and pinmuxes, we now break this, and you need to introduce > more complex logic to search through the pinmux_list and dynamically > add more functions to each entry with a matching map_name. In the patch you posted, pinmux_get looked up a single specific pinmux_map* and stored it in the struct pinmux. You're right that continuing to do something similar means needing to store an array of pinmux_map* in the struct pinmux, and dynamically adding more and more entries as you search through the mapping table. A bit painful. However, if instead of that, you store the original parameters to pinmux_get() in struct pinmux, there's nothing to dynamically store there. Instead, the implementation of pinmux_get and pinmux_enable is very roughly: pinmux_get: Allocate struct pinmux, fill it in: list_for_each_entry(mapping table) if (matches search terms) check_not_already_in_use acquire_pins() // Note: no break here pinmux_enable: // Note we redo the whole search here based on the original terms // rather than having pinmux_get pre-calculate the search result list_for_each_entry(mapping table) if (matches search terms) ops->enable() In fact, I'd imagine that the list_for_each_entry call above could be placed into a utility function that implements the searching logic, to keep the code in one place, with a function parameter: pinmux_search(terms, callback): list_for_each_entry(mapping table) if (matches search terms) callback() pinmux_get_body(): check_not_already_in_use acquire_pins() pinmux_get(terms): Allocate struct pinmux, fill it in: pinmux_search(terms, pinmux_get_body) pinmux_enable_body(): ops->enable() pinmux_enable(): pinmux_search(terms, pinmux_enable_body) Alternatively, each map entry contains a list node that's set up. The list is set up by pinmux_get, and contains all mapping entries associated with the original search terms. Struct pinmux points at the head of the list. pinmux_enable then just needs to iterate over that list, not the whole mapping array, and doesn't need to implement the search algorithm. That might actually be simpler. Since each mapping entry would only be get'd once, there'd be no conflicts with multiple things trying to use the one list node at once. > Then you need to take care of the case where acquiring pins for > one of the functions fail: then you need to go back and free the > already acquired pins in the error path. That's not that hard; just iterate over the whole list again, applying a function that undoes what you did before. It's just like the error handling code that already exists in acquire_pins for example. The only hard part might be stopping when you get to the point you already got to in the first loop, but actually that's just a matter of passing in a "stop at this index" parameter to the pinmux_search() I mentioned above. > I tried quickly to see if I could code this up, and it got very complex > real quick, sadly. Maybe if I can just get to iterate a v4 first, I > could venture down this track. > > But if you can code up the patch for this, I'm pretty much game for > it! Sure, I can add the extra looping to the code if you want. It'd be easiest if you set up the data structures in the initial patch such that all the fields exist in the mapping table (i.e. how I showed in the concept header files I posted). That way, there'd be no type or semantic changes in my patch, just support for acting on N instead of just 1 mapping entry at a time. ... > > * The same point, but think about a SW bit-banged bus consisting of 8 > > GPIOs for the bus and 100 pins on the device. Each permutation of 8 out > > of 100 is scary... I'd love a board to be able to represent a single > > mapping for a particular board's set of GPIOs in this case, but not for > > the pinmux driver to expose all possibilities! > > Is this a real world problem or a theoretical one? Seems like the > latter, and as stated in the documentation this subsystem is not about > solving discrete maths permutation spaces, but practical enumerations. That's theoretical in terms of the boards/code I'm currently dealing with. However, solving this seems pretty simple, and I can't imagine nobody will ever want to do that.
On Mon, Jun 13, 2011 at 01:57:36PM -0600, Grant Likely wrote: > On Mon, Jun 13, 2011 at 10:58 AM, Linus Walleij > > +This get/enable/disable/put sequence can just as well be handled by bus drivers > > +if you don't want each and every driver to handle it and you know the > > +arrangement on your bus. > I would *strongly* recommend against individual device drivers > accessing the pinmux api. This is system level configuration code, > and should be handled at the system level. There can also be advantages to putting the pin into the designed mode without the driver being loaded from the electrical point of view. For example, selecting appropriate pull values for pads can cut down on power consumption.
2011/6/14 Linus Walleij <linus.walleij@stericsson.com>: > From: Linus Walleij <linus.walleij@linaro.org> > > This creates a subsystem for handling of pinmux devices. These are > devices that enable and disable groups of pins on primarily PGA and > BGA type of chip packages and common in embedded systems. > > This is being done to depopulate the arch/arm/* directory of such > custom drivers and try to abstract the infrastructure they all > need. See the Documentation/pinmux.txt file that is part of this > patch for more details. > > Cc: Grant Likely <grant.likely@secretlab.ca> > Cc: Stephen Warren <swarren@nvidia.com> > Cc: Joe Perches <joe@perches.com> > Cc: Russell King <linux@arm.linux.org.uk> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > ChangeLog v2->v3: > - Renamed subsystem folder to "pinctrl" since we will likely > want to keep other pin control such as biasing in this > subsystem too, so let us keep to something generic even though > we're mainly doing pinmux now. > - As a consequence, register pins as an abstract entity separate > from the pinmux. The muxing functions will claim pins out of the > pin pool and make sure they do not collide. Pins can now be > named by the pinctrl core. > - Converted the pin lookup from a static array into a radix tree, > I agreed with Grant Likely to try to avoid any static allocation > (which is crap for device tree stuff) so I just rewrote this > to be dynamic, just like irq number descriptors. The > platform-wide definition of number of pins goes away - this is > now just the sum total of the pins registered to the subsystem. > - Make sure mappings with only a function name and no device > works properly. > --- > Documentation/ABI/testing/sysfs-class-pinmux | 11 + > Documentation/pinctrl.txt | 397 ++++++++++ > MAINTAINERS | 5 + > drivers/Kconfig | 4 + > drivers/Makefile | 2 + > drivers/pinctrl/Kconfig | 29 + > drivers/pinctrl/Makefile | 6 + > drivers/pinctrl/core.c | 1028 ++++++++++++++++++++++++++ > include/linux/pinctrl/machine.h | 57 ++ > include/linux/pinctrl/pinmux.h | 180 +++++ > 10 files changed, 1719 insertions(+), 0 deletions(-) > create mode 100644 Documentation/ABI/testing/sysfs-class-pinmux > create mode 100644 Documentation/pinctrl.txt > create mode 100644 drivers/pinctrl/Kconfig > create mode 100644 drivers/pinctrl/Makefile > create mode 100644 drivers/pinctrl/core.c > create mode 100644 include/linux/pinctrl/machine.h > create mode 100644 include/linux/pinctrl/pinmux.h > > diff --git a/Documentation/ABI/testing/sysfs-class-pinmux b/Documentation/ABI/testing/sysfs-class-pinmux > new file mode 100644 > index 0000000..c2ea843 > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-class-pinmux > @@ -0,0 +1,11 @@ > +What: /sys/class/pinmux/.../name > +Date: May 2011 > +KernelVersion: 3.1 has this been ready for 3.1? we have been planning to write pinmux driver based on this framework. > +Contact: Linus Walleij <linus.walleij@linaro.org> > +Description: > + Each pinmux directory will contain a field called > + name. This holds a string identifying the pinmux for > + display purposes. > + > + NOTE: this will be empty if no suitable name is provided > + by platform or pinmux drivers. > diff --git a/Documentation/pinctrl.txt b/Documentation/pinctrl.txt > new file mode 100644 > index 0000000..c4b6f48 > --- /dev/null > +++ b/Documentation/pinctrl.txt > @@ -0,0 +1,397 @@ > +PINCTRL (PIN CONTROL) subsystem > +This document outlines the pin control subsystem in Linux > + > +This subsystem deals with: > + > +- Multiplexing of pins, pads, fingers (etc) see below for details > + > +The intention is to also deal with: > + > +- Software-controlled biasing and driving mode specific pins, such as > + pull-up/down, open drain etc, load capacitance configuration when controlled > + by software, etc. > + > + > +Top-level interface > +=================== > + > +Definition of PIN: > + > +- PINS are equal to pads or fingers or whatever packaging output you want to > + mux and these are denoted by unsigned integers in the range 0..MAX_INT. Every > + pin on your system (or atleast every pin that can be muxed) should have a > + unique number. The numberspace can span several chips if you have more chips > + on your system that can be subject to muxing. > + > +All pins, pads, fingers, balls or other controllable units on the system shall > +be given a unique number in the global number space beginning on zero. > + > +All pins on the system need to be registered, typically by board code or a > +device tree. Board/machine code will #include <linux/pinctrl/machine.h> > +and use one of: > + > + pinctrl_register_pins_sparse(); > + pinctrl_register_pins_dense(); > + > +To register all the pins on the system. Both are supplied with a list of > +descriptor items, the difference is that the _dense version will also > +register anonymous pins for each "hole" in the pin map, from zero to the > +supplied extra argument giving the number of pins. > + > +Here is an example of a PGA (Pin Grid Array) chip seen from underneath: > + > + A B C D E F G H > + > + 8 o o o o o o o o > + > + 7 o o o o o o o o > + > + 6 o o o o o o o o > + > + 5 o o o o o o o o > + > + 4 o o o o o o o o > + > + 3 o o o o o o o o > + > + 2 o o o o o o o o > + > + 1 o o o o o o o o > + > +To register and name all the pins on this package we can do this in a board > +file: > + > +#include <linux/pinctrl/machine.h> > + > +const struct pinctrl_pin_desc __initdata my_pins[] = { > + PINCTRL_PIN(0, "A1"), > + PINCTRL_PIN(1, "A2"), > + PINCTRL_PIN(2, "A3"), > + ... > + PINCTRL_PIN(61, "H6"), > + PINCTRL_PIN(62, "H7"), > + PINCTRL_PIN(63, "H8"), > +}; > + > +int __init register_pins(void) > +{ > + pinctrl_register_pins_dense(&my_pins, ARRAY_SIZE(my_pins), 64); > +} > + > +Pins usually have fancier names than this. You can find these in the dataheet > +for your chip. Notice that the core machine.h file provides a fancy macro > +called PINCTRL_PIN() to create the struct entries. > + > + > +PINMUX interfaces > +================= > + > +These calls use the pinmux_* naming prefix. No other calls should use that > +prefix. > + > + > +What is pinmuxing? > +================== > + > +Pinmux, also known as padmux, ballmux, alternate functions or mission modes > +is a way for chip vendors producing some kind of electrical packages to use > +a certain physical bin (ball, pad, finger, etc) for multiple mutually exclusive > +functions, depending on the application. By "application" in this context > +we usually mean a way of soldering or wiring the package into an electronic > +system, even though the framework makes it possible to handle also change the > +function at runtime. > + > +Here is an example of a PGA (Pin Grid Array) chip seen from underneath: > + > + A B C D E F G H > + +---+ > + 8 | o | o o o o o o o > + | | > + 7 | o | o o o o o o o > + | | > + 6 | o | o o o o o o o > + +---+---+ > + 5 | o | o | o o o o o o > + +---+---+ +---+ > + 4 o o o o o o | o | o > + | | > + 3 o o o o o o | o | o > + | | > + 2 o o o o o o | o | o > + | | > + 1 o o o o o o | o | o > + +---+ > + > +This is not tetris. The game to think of is chess. Not all PGA/BGA packages > +are chessboard-like, big ones have "holes" in some arrangement according to > +different design patterns, but we're using this as a simple example. Of the > +pins you see some will be taken by things like a few VCC and GND to feed power > +to the chip, and quite a few will be taken by large ports like an external > +memory interface. The remaining pins will often be subject to pin multiplexing. > + > +The example 8x8 PGA package above will have pin numbers 0 thru 63 assigned to > +its physical pins. It will name the pins { A1, A2, A3 ... H6, H7, H8 } using > +pinctrl_register_pins_[sparse|dense]() and a suitable data set as shown > +earlier. > + > +In this 8x8 BGA package the pins { A8, A7, A6, A5 } can be used as an SPI port > +(these are four pins: CLK, RXD, TXD, FRM). In that case, pin B5 can be used as > +some general-purpose GPIO pin. However, in another setting, pins { A5, B5 } can > +be used as an I2C port (these are just two pins: SCL, SDA). Needless to say, > +we cannot use the SPI port and I2C port at the same time. However in the inside > +of the package the silicon performing the SPI logic can alternatively be routed > +out on pins { G4, G3, G2, G1 }. > + > +This way the silicon blocks present inside the chip can be multiplexed "muxed" > +out on different pin ranges. Often contemporary SoC (systems on chip) will > +contain several I2C, SPI, SDIO/MMC, etc silicon blocks that can be routed to > +different pins by pinmux settings. > + > +Since general-purpose I/O pins (GPIO) are typically always in shortage, it is > +common to be able to use almost any pin as a GPIO pin if it is not currently > +in use by some other I/O port. > + > + > +Pinmux conventions > +================== > + > +The purpose of the pinmux subsystem is to abstract and provide pinmux settings > +to the devices you choose to instantiate in your machine configuration. It is > +inspired by the clk, GPIO and regulator subsystems, so devices will request > +their mux setting, but it's also possible to request a single pin for e.g. > +GPIO. > + > +Definitions: > + > +- FUNCTIONS can be switched in and out by a driver residing with the pinmux > + subsystem in the drivers/pinmux/* directory of the kernel. The pinmux driver > + knows the possible functions. In the example above you can identify three > + pinmux functions, two for spi and one for i2c. > + > +- FUNCTIONS are assumed to be enumerable from zero in a one-dimensional array. > + In this case the array could be something like: { spi0-0, spi0-1, i2c0-0 } > + for the three available settings. The knowledge of this one-dimensional array > + and it's machine-specific particulars is kept inside the pinmux driver, > + from the outside only these enumerators are known, and the driver core > + can request the name or the list of pins belonging to a certain enumerator. > + > +- FUNCTIONS are MAPPED to a certain device by the board file, device tree or > + similar machine setup configuration mechanism, similar to how regulators are > + connected to devices, usually by name. In the example case we can define > + that this particular machine shall use device spi0 with pinmux setting > + spi0-1 and i2c0 on i2c0-1, something like the two-tuple: > + { {spi0, spi0-1}, {i2c0, i2c0-1} } > + > +- FUNCTIONS are provided on a first-come first-serve basis, so if some other > + device mux setting or GPIO pin request has already taken your physical pin, > + you will be denied the use of it. To get (activate) a new setting, the old > + one has to be put (deactivated) first. > + > +Sometimes the documentation and hardware registers will be oriented around > +pads (or "fingers") rather than pins - these are the soldering surfaces on the > +silicon inside the package, and may or may not match the actual number of > +pins/balls underneath the capsule. Pick some enumeration that makes sense to > +you. Define enumerators only for the pins you can control if that makes sense. > + > + > +Pinmux drivers > +============== > + > +The driver will for all calls be provided an offset pin number into its own > +pin range. If you have 2 chips with 8x8 pins, the first chips pins will have > +numbers 0 thru 63 and the second one pins 64 thru 127, but the driver for the > +second chip will be passed numbers in the range 0 thru 63 anyway, base offset > +subtracted. > + > +Pinmux drivers are required to supply a few callback functions, some are > +optional. Usually the enable() and disable() functions are implemented, > +writing values into some certain registers to activate a certain mux setting > +for a certain pin. > + > +A simple driver for the above example will work by setting bits 0, 1 or 2 > +into some register mamed MUX, so it enumerates its available settings and > +their pin assignments, and expose them like this: > + > +#include <linux/pinctrl/pinmux.h> > + > +struct foo_pmx_func { > + char *name; > + const unsigned int *pins; > + const unsigned num_pins; > +}; > + > +static unsigned int spi0_0_pins[] = { 0, 8, 16, 24 }; > +static unsigned int i2c0_pins[] = { 24, 25 }; > +static unsigned int spi0_1_pins[] = { 38, 46, 54, 62 }; > + > +static struct foo_pmx_func myfuncs[] = { > + { > + .name = "spi0-0", > + .pins = spi0_0_pins, > + .num_pins = ARRAY_SIZE(spi0_1_pins), > + }, > + { > + .name = "i2c0", > + .pins = i2c0_pins, > + .num_pins = ARRAY_SIZE(i2c0_pins), > + }, > + { > + .name = "spi0-1", > + .pins = spi0_1_pins, > + .num_pins = ARRAY_SIZE(spi0_1_pins), > + }, > +}; > + > +int foo_list(struct pinmux_dev *pmxdev, unsigned selector) > +{ > + if (selector >= ARRAY_SIZE(myfuncs)) > + return -EINVAL; > + return 0; > +} > + > +const char *foo_get_fname(struct pinmux_dev *pmxdev, unsigned selector) > +{ > + if (selector >= ARRAY_SIZE(myfuncs)) > + return NULL; > + return myfuncs[selector].name; > +} > + > +static int foo_get_pins(struct pinmux_dev *pmxdev, unsigned selector, > + unsigned ** const pins, unsigned * const num_pins) > +{ > + if (selector >= ARRAY_SIZE(myfuncs)) > + return -EINVAL; > + *pins = myfuncs[selector].pins; > + *num_pins = myfuncs[selector].num_pins; > + return 0; > +} > + > + > +int foo_enable(struct pinmux_dev *pmxdev, unsigned selector) > +{ > + if (selector < ARRAY_SIZE(myfuncs)) > + write((read(MUX)|(1<<selector)), MUX) > + return 0; > + } > + return -EINVAL; > +} > + > +int foo_disable(struct pinmux_dev *pmxdev, unsigned selector) > +{ > + if (selector < ARRAY_SIZE(myfuncs)) > + write((read(MUX) & ~(1<<selector)), MUX) > + return 0; > + } > + return -EINVAL; > +} > + > +struct pinmux_ops ops = { > + .list_functions = foo_list, > + .get_function_name = foo_get_fname, > + .get_function_pins = foo_get_pins, > + .enable = foo_enable, > + .disable = foo_disable, > +}; > + > +Now the able reader will say: "wait - the driver needs to make sure it > +can set this and that bit at the same time, because else it will collide > +and wreak havoc in my electronics, and make sure noone else is using the > +other setting that it's incompatible with". > + > +In the example activating muxing 0 and 1 at the same time setting bits > +0 and 1, uses one pin in common so they would collide. > + > +The beauty of the pinmux subsystem is that since it keeps track of all > +pins and who is using them, it will already have denied an impossible > +request like that, so the driver does not need to worry about such > +things - when it gets a selector passed in, the pinmux subsystem makes > +sure no other device or GPIO assignment is already using the selected > +pins. > + > +The above functions are mandatory to implement for a pinmux driver. > + > +The function list could become long, especially if you can convert every > +individual pin into a GPIO pin independent of any other pins, then your > +function array can become 64 entries for each GPIO setting and then the > +device functions. For this reason there is an additional function you > +can implement to enable only GPIO on an individual pin: gpio_enable(). > + > + > +Pinmux board/machine configuration > +================================== > + > +Boards and machines define how a certain complete running system is put > +together, including how GPIOs and devices are muxed, how regulators are > +constrained and how the clock tree looks. Of course pinmux settings are also > +part of this. > + > +A pinmux config for a machine looks pretty much like a simple regulator > +configuration, so for the example array above we want to enable i2c and > +spi on the second function mapping: > + > +#include <linux/pinctrl/machine.h> > + > +static struct pinmux_map pmx_mapping[] = { > + { > + .function = "spi0-1", > + .dev_name = "foo-spi.0", > + }, > + { > + .function = "i2c0", > + .dev_name = "foo-i2c.0", > + }, > +}; > + > +Since the above construct is pretty common there is a helper macro to make > +it even more compact: > + > +static struct pinmux_map pmx_mapping[] = { > + PINMUX_MAP("spi0-1", "foo-spi.0"), > + PINMUX_MAP("i2c0", "foo-i2c.0"), > +}; > + > +The dev_name here matches to the unique device name that can be used to look > +up the device struct (just like with clockdev or regulators). The function name > +must match a function provided by the pinmux driver handling this pin range. > +You register this pinmux mapping to the pinmux subsystem by simply: > + > + ret = pinmux_register_mappings(&pmx_mapping, ARRAY_SIZE(pmx_mapping)); > + > + > +Pinmux requests from drivers > +============================ > + > +A driver may request a certain mux to be activated, usually just the default > +mux like this: > + > +#include <linux/pinctrl/pinmux.h> > + > +foo_probe() > +{ > + /* Allocate a state holder named "state" etc */ > + struct pinmux pmx; > + > + pmx = pinmux_get(&device, NULL); > + if IS_ERR(pmx) > + return PTR_ERR(pmx); > + pinmux_enable(pmx); > + > + state->pmx = pmx; > +} > + > +foo_remove() > +{ > + pinmux_disable(state->pmx); > + pinmux_put(state->pmx); > +} > + > +If you want a specific mux setting and not just the first one found for this > +device you can specify a specific mux setting, for example in the above example > +the second i2c0 setting: pinmux_get(&device, "spi0-2"); > + > +This get/enable/disable/put sequence can just as well be handled by bus drivers > +if you don't want each and every driver to handle it and you know the > +arrangement on your bus. > + > +The pins are allocated for your device when you issue the pinmux_get() call, > +after this you should be able to see this in the debugfs listing of all pins. > diff --git a/MAINTAINERS b/MAINTAINERS > index 29801f7..5caea5a 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -4933,6 +4933,11 @@ L: linux-mtd@lists.infradead.org > S: Maintained > F: drivers/mtd/devices/phram.c > > +PINMUX SUBSYSTEM > +M: Linus Walleij <linus.walleij@linaro.org> > +S: Maintained > +F: drivers/pinmux/ > + > PKTCDVD DRIVER > M: Peter Osterlund <petero2@telia.com> > S: Maintained > diff --git a/drivers/Kconfig b/drivers/Kconfig > index 3bb154d..6998d78 100644 > --- a/drivers/Kconfig > +++ b/drivers/Kconfig > @@ -56,6 +56,10 @@ source "drivers/pps/Kconfig" > > source "drivers/ptp/Kconfig" > > +# pinctrl before gpio - gpio drivers may need it > + > +source "drivers/pinctrl/Kconfig" > + > source "drivers/gpio/Kconfig" > > source "drivers/w1/Kconfig" > diff --git a/drivers/Makefile b/drivers/Makefile > index 09f3232..a590a01 100644 > --- a/drivers/Makefile > +++ b/drivers/Makefile > @@ -5,6 +5,8 @@ > # Rewritten to use lists instead of if-statements. > # > > +# GPIO must come after pinctrl as gpios may need to mux pins etc > +obj-y += pinctrl/ > obj-y += gpio/ > obj-$(CONFIG_PCI) += pci/ > obj-$(CONFIG_PARISC) += parisc/ > diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig > new file mode 100644 > index 0000000..8050fdf > --- /dev/null > +++ b/drivers/pinctrl/Kconfig > @@ -0,0 +1,29 @@ > +# > +# PINCTRL infrastructure and drivers > +# > + > +menuconfig PINCTRL > + bool "PINCTRL Support" > + depends on SYSFS && EXPERIMENTAL > + help > + This enables the PINCTRL subsystem for controlling pins > + on chip packages, for example multiplexing pins on primarily > + PGA and BGA packages for systems on chip. > + > + If unsure, say N. > + > +if PINCTRL > + > +config DEBUG_PINCTRL > + bool "Debug PINCTRL calls" > + depends on DEBUG_KERNEL > + help > + Say Y here to add some extra checks and diagnostics to PINCTRL calls. > + > +config PINMUX_U300 > + bool "U300 pinmux driver" > + depends on ARCH_U300 > + help > + Say Y here to enable the U300 pinmux driver > + > +endif > diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile > new file mode 100644 > index 0000000..44d8933 > --- /dev/null > +++ b/drivers/pinctrl/Makefile > @@ -0,0 +1,6 @@ > +# generic pinmux support > + > +ccflags-$(CONFIG_DEBUG_PINMUX) += -DDEBUG > + > +obj-$(CONFIG_PINCTRL) += core.o > +obj-$(CONFIG_PINMUX_U300) += pinmux-u300.o > diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c > new file mode 100644 > index 0000000..8fd1437 > --- /dev/null > +++ b/drivers/pinctrl/core.c > @@ -0,0 +1,1028 @@ > +/* > + * Core driver for the pinmux subsystem > + * > + * Copyright (C) 2011 ST-Ericsson SA > + * Written on behalf of Linaro for ST-Ericsson > + * Based on bits of regulator core, gpio core and clk core > + * > + * Author: Linus Walleij <linus.walleij@linaro.org> > + * > + * License terms: GNU General Public License (GPL) version 2 > + */ > +#define pr_fmt(fmt) "pinctrl core: " fmt > + > +#include <linux/kernel.h> > +#include <linux/init.h> > +#include <linux/device.h> > +#include <linux/slab.h> > +#include <linux/radix-tree.h> > +#include <linux/err.h> > +#include <linux/list.h> > +#include <linux/mutex.h> > +#include <linux/spinlock.h> > +#include <linux/sysfs.h> > +#include <linux/debugfs.h> > +#include <linux/seq_file.h> > +#include <linux/pinctrl/machine.h> > +#include <linux/pinctrl/pinmux.h> > + > +/* Global list of pinmuxes */ > +static DEFINE_MUTEX(pinmux_list_mutex); > +static LIST_HEAD(pinmux_list); > + > +/* Global list of pinmux devices */ > +static DEFINE_MUTEX(pinmuxdev_list_mutex); > +static LIST_HEAD(pinmuxdev_list); > + > +/** > + * struct pin_desc - pin descriptor for each physical pin in the arch > + * @pmxdev: corresponding pinmux device > + * @requested: whether the pin is already requested by pinmux or not > + * @name: a name for the pin, e.g. the name of the pin/pad/finger on a > + * datasheet or such > + * @function: a named muxing function for the pin that will be passed to > + * subdrivers and shown in debugfs etc > + */ > +struct pin_desc { > + struct pinmux_dev *pmxdev; > + bool requested; > + char name[16]; > + char function[16]; > +}; > +/* Global lookup of per-pin descriptors, one for each physical pin */ > +static DEFINE_SPINLOCK(pin_desc_tree_lock); > +static RADIX_TREE(pin_desc_tree, GFP_KERNEL); > +static unsigned int num_pins = 0; > + > +/** > + * struct pinmux - per-device pinmux state holder > + * @node: global list node - only for internal use > + * @dev: the device using this pinmux > + * @pmxdev: the pinmux device controlling this pinmux > + * @map: corresponding pinmux map active for this pinmux setting > + * @usecount: the number of active users of this mux setting, used to keep > + * track of nested use cases > + * @pins: an array of discrete physical pins used in this mapping, taken > + * from the global pin enumeration space (copied from pinmux map) > + * @num_pins: the number of pins in this mapping array, i.e. the number of > + * elements in .pins so we can iterate over that array (copied from > + * pinmux map) > + * @pmxdev: pinmux device handling this pinmux > + * @pmxdev_selector: the selector for the pinmux device handling this pinmux > + * @mutex: a lock for the pinmux state holder > + */ > +struct pinmux { > + struct list_head node; > + struct device *dev; > + struct pinmux_map const *map; > + unsigned usecount; > + struct pinmux_dev *pmxdev; > + unsigned pmxdev_selector; > + struct mutex mutex; > +}; > + > +int pin_is_valid(int pin) > +{ > + return pin >= 0 && pin < num_pins; > +} > +EXPORT_SYMBOL_GPL(pin_is_valid); > + > +static ssize_t pinmux_name_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct pinmux_dev *pmxdev = dev_get_drvdata(dev); > + > + return sprintf(buf, "%s\n", pmxdev_get_name(pmxdev)); > +} > + > +static struct device_attribute pinmux_dev_attrs[] = { > + __ATTR(name, 0444, pinmux_name_show, NULL), > + __ATTR_NULL, > +}; > + > +static void pinmux_dev_release(struct device *dev) > +{ > + struct pinmux_dev *pmxdev = dev_get_drvdata(dev); > + kfree(pmxdev); > +} > + > +static struct class pinmux_class = { > + .name = "pinmux", > + .dev_release = pinmux_dev_release, > + .dev_attrs = pinmux_dev_attrs, > +}; > + > +/* Deletes a range of pin descriptors */ > +static void pinctrl_free_pindescs(struct pinctrl_pin_desc const *pins, > + unsigned num_pins) > +{ > + int i; > + > + for (i = 0; i < num_pins; i++) { > + struct pin_desc *pindesc; > + > + spin_lock(&pin_desc_tree_lock); > + pindesc = radix_tree_lookup(&pin_desc_tree, pins[i].number); > + if (pindesc != NULL) { > + radix_tree_delete(&pin_desc_tree, pins[i].number); > + num_pins --; > + } > + spin_unlock(&pin_desc_tree_lock); > + kfree(pindesc); > + } > +} > + > +static int pinctrl_register_one_pin(unsigned number, const char *name) > +{ > + struct pin_desc *pindesc; > + > + spin_lock(&pin_desc_tree_lock); > + pindesc = radix_tree_lookup(&pin_desc_tree, number); > + spin_unlock(&pin_desc_tree_lock); > + > + if (pindesc != NULL) { > + pr_err("pin %d already registered\n", number); > + return -EINVAL; > + } > + > + pindesc = kzalloc(sizeof(*pindesc), GFP_KERNEL); > + if (pindesc == NULL) > + return -ENOMEM; > + > + /* Copy optional basic pin info */ > + if (name) { > + strncpy(pindesc->name, name, 16); > + pindesc->name[15] = '\0'; > + } > + > + spin_lock(&pin_desc_tree_lock); > + radix_tree_insert(&pin_desc_tree, number, pindesc); > + num_pins ++; > + spin_unlock(&pin_desc_tree_lock); > + return 0; > +} > + > +/* Passing in 0 num_pins means "sparse" */ > +static int pinctrl_register_pins(struct pinctrl_pin_desc const *pins, > + unsigned num_descs, unsigned num_pins) > +{ > + unsigned i; > + int ret = 0; > + > + for (i = 0; i < num_descs; i++) { > + ret = pinctrl_register_one_pin(pins[i].number, pins[i].name); > + if (ret) > + return ret; > + } > + > + if (num_pins == 0) > + return 0; > + > + /* > + * If we are registerering dense pinlists, fill in all holes with > + * anonymous pins. > + */ > + for (i = 0; i < num_pins; i++) { > + char pinname[16]; > + struct pin_desc *pindesc; > + > + spin_lock(&pin_desc_tree_lock); > + pindesc = radix_tree_lookup(&pin_desc_tree, i); > + spin_unlock(&pin_desc_tree_lock); > + /* Already registered this one, take next */ > + if (pindesc) > + continue; > + > + snprintf(pinname, 15, "anonymous %u", i); > + pinname[15] = '\0'; > + > + ret = pinctrl_register_one_pin(i, pinname); > + if (ret) > + return ret; > + } > + > + return 0; > +} > + > +/** > + * pinctrl_register_pins_sparse() - register a range of pins for a > + * board/machine with potential holes in the pin map. The pins in > + * the holes will not be usable. > + * @pins: a range of pins to register > + * @num_descs: the number of pins descriptors passed in through the previous > + * pointer > + */ > +int pinctrl_register_pins_sparse(struct pinctrl_pin_desc const *pins, > + unsigned num_descs) > +{ > + int ret; > + > + ret = pinctrl_register_pins(pins, num_descs, 0); > + if (ret) > + pinctrl_free_pindescs(pins, num_descs); > + return ret; > + > +} > +EXPORT_SYMBOL_GPL(pinctrl_register_pins); > + > +/** > + * pinctrl_register_pins_dense() - register a range of pins for a > + * board/machine, if there are holes in the pin map, they will be > + * allocated by anonymous pins. > + * @pins: a range of pins to register > + * @num_descs: the number of pins descriptors passed in through the previous > + * pointer > + * @num_pins: the total number of pins including holes in the pin map and > + * any "air" at the end of the map, all pins from 0 to this number > + * will be allocated, the ones that does not have descriptors passed > + * in will be marked "anonymous" > + */ > +int pinctrl_register_pins_dense(struct pinctrl_pin_desc const *pins, > + unsigned num_descs, unsigned num_pins) > +{ > + int ret; > + unsigned i; > + > + ret = pinctrl_register_pins(pins, num_descs, num_pins); > + if (ret) { > + for (i = 0; i < num_pins; i++) { > + struct pin_desc *pindesc; > + > + spin_lock(&pin_desc_tree_lock); > + pindesc = radix_tree_lookup(&pin_desc_tree, i); > + if (pindesc != NULL) { > + radix_tree_delete(&pin_desc_tree, i); > + num_pins --; > + } > + spin_unlock(&pin_desc_tree_lock); > + kfree(pindesc); > + } > + } > + return ret; > +} > +EXPORT_SYMBOL_GPL(pinctrl_register_pins_sparse); > + > +/** > + * pin_request() - request a single pin to be muxed in, typically for GPIO > + * @pin: the pin number in the global pin space > + * @function: a functional name to give to this pin, passed to the driver > + * so it knows what function to mux in, e.g. the string "gpioNN" > + * means that you want to mux in the pin for use as GPIO number NN > + * @gpio: if this request concerns a single GPIO pin > + */ > +static int pin_request(int pin, const char *function, bool gpio) > +{ > + struct pin_desc *desc; > + struct pinmux_dev *pmxdev; > + const struct pinmux_ops *ops; > + int status = -EINVAL; > + unsigned long flags; > + > + pr_debug("request pin %d for %s\n", pin, function); > + if (!pin_is_valid(pin)) { > + pr_err("pin is invalid\n"); > + return -EINVAL; > + } > + > + if (!function) { > + pr_err("no function name given\n"); > + return -EINVAL; > + } > + > + spin_lock_irqsave(&pin_desc_tree_lock, flags); > + desc = radix_tree_lookup(&pin_desc_tree, pin); > + if (desc == NULL) { > + pr_err("pin is not registered so it cannot be requested\n"); > + goto out; > + } > + pmxdev = desc->pmxdev; > + if (desc->requested) { > + pr_err("pin already requested\n"); > + goto out; > + } > + if (!pmxdev) { > + pr_warn("no pinmux device is handling pin %d\n", pin); > + goto out; > + } > + ops = pmxdev->desc->ops; > + > + /* Let each pin increase references to this module */ > + if (!try_module_get(pmxdev->owner)) { > + pr_err("could not increase module refcount for pin %d\n", pin); > + status = -EINVAL; > + goto out; > + } > + > + /* > + * If there is no kind of request function for the pin we just assume > + * we got it by default and proceed. > + */ > + if (gpio && ops->gpio_request_enable) > + /* This requests and enables a single GPIO pin */ > + status = ops->gpio_request_enable(pmxdev, > + pin - pmxdev->desc->base); > + else if (ops->request) > + status = ops->request(pmxdev, > + pin - pmxdev->desc->base); > + else > + status = 0; > + > + if (status) { > + pr_err("->request on device %s failed " > + "for pin %d (offset %d)\n", > + pmxdev->desc->name, pin, > + pin - pmxdev->desc->base); > + goto out; > + } > + > + desc->requested = true; > + strncpy(desc->function, function, 16); > + desc->function[15] = '\0'; > + > +out: > + spin_unlock_irqrestore(&pin_desc_tree_lock, flags); > + if (status) > + pr_err("pin-%d (%s) status %d\n", > + pin, function ? : "?", status); > + > + return status; > +} > + > +/** > + * pin_free() - release a single muxed in pin so something else can be muxed in > + * instead > + * @pin: the pin to free > + */ > +static void pin_free(int pin) > +{ > + struct pin_desc *desc; > + struct pinmux_dev *pmxdev; > + unsigned long flags; > + > + if (!pin_is_valid(pin)) > + return; > + > + spin_lock_irqsave(&pin_desc_tree_lock, flags); > + desc = radix_tree_lookup(&pin_desc_tree, pin); > + if (desc == NULL) { > + pr_err("pin is not registered so it cannot be freed\n"); > + goto out; > + } > + pmxdev = desc->pmxdev; > + if (pmxdev) { > + const struct pinmux_ops *ops = pmxdev->desc->ops; > + > + if (ops->free) > + ops->free(pmxdev, pin - pmxdev->desc->base); > + } > + desc->requested = false; > + desc->function[0] = '\0'; > + module_put(pmxdev->owner); > +out: > + spin_unlock_irqrestore(&pin_desc_tree_lock, flags); > +} > + > +/** > + * pinmux_request_gpio() - request a single pin to be muxed in to be used > + * as a GPIO pin > + * @pin: the pin to mux in as GPIO > + * @gpio: the corresponding GPIO pin number > + */ > +int pinmux_request_gpio(int pin, unsigned gpio) > +{ > + char gpiostr[16]; > + > + snprintf(gpiostr, 15, "gpio%d", gpio); > + return pin_request(pin, gpiostr, true); > +} > +EXPORT_SYMBOL_GPL(pinmux_request_gpio); > + > +/** > + * pinmux_free_gpio() - free a single pin, currently muxed in to be used > + * as a GPIO pin > + * @pin: the pin to mux out from GPIO > + */ > +void pinmux_free_gpio(int pin) > +{ > + return pin_free(pin); > +} > +EXPORT_SYMBOL_GPL(pinmux_free_gpio); > + > +int pinmux_register_mappings(struct pinmux_map const *maps, unsigned num_maps) > +{ > + int ret = 0; > + int i; > + > + pr_debug("add %d functions\n", num_maps); > + for (i = 0; i < num_maps; i++) { > + struct pinmux *pmx; > + > + /* Sanity check the mapping */ > + if (!maps[i].function) { > + pr_err("failed to register map %d - no function ID given\n", i); > + ret = -EINVAL; > + goto out; > + } > + if (!maps[i].dev && !maps[i].dev_name) { > + pr_err("failed to register map %d - no device or device name given\n", i); > + ret = -EINVAL; > + goto out; > + } > + > + /* > + * create the state cookie holder struct pinmux for each > + * mapping, this is what consumers will get when requesting > + * a pinmux handle with pinmux_get() > + */ > + pmx = kzalloc(sizeof(struct pinmux), GFP_KERNEL); > + if (pmx == NULL) { > + ret = -ENOMEM; > + goto out; > + } > + mutex_init(&pmx->mutex); > + pmx->map = &maps[i]; > + > + /* Add the pinmux */ > + mutex_lock(&pinmux_list_mutex); > + list_add(&pmx->node, &pinmux_list); > + mutex_unlock(&pinmux_list_mutex); > + pr_debug("add function %s\n", maps[i].function); > + } > + > +out: > + return ret; > +} > + > +/** > + * acquire_pins() - acquire all the pins for a certain funcion on a certain > + * pinmux device > + * @pmxdev: the device to take the function on > + * @selector: the selector to acquire the pins for > + */ > +int acquire_pins(struct pinmux_dev *pmxdev, unsigned selector) > +{ > + const struct pinmux_ops *ops = pmxdev->desc->ops; > + unsigned *pins; > + unsigned num_pins; > + const char *func = ops->get_function_name(pmxdev, selector); > + int ret; > + int i; > + > + ret = ops->get_function_pins(pmxdev, selector, &pins, &num_pins); > + if (ret) > + return ret; > + > + /* Try to allocate all pins in this pinmux map, one by one */ > + for (i = 0; i < num_pins; i++) { > + ret = pin_request(pins[i], func, false); > + if (ret) { > + pr_err("could not get pin %d for function %s " > + "on device %s - conflicting mux mappings?\n", > + pins[i], func ? : "(undefined)", > + pmxdev->desc->name); > + /* On error release all taken pins */ > + i--; /* this pin just failed */ > + for (; i >= 0; i--) > + pin_free(pins[i]); > + return -ENODEV; > + } > + } > + return 0; > +} > + > +/** > + * pinmux_get() - retrieves the pinmux for a certain device > + * @dev: the device to get the pinmux for > + * @func: an optional mux name or NULL, the name is only needed > + * if a single device has multiple pinmux settings (i.e. if the > + * same device can be muxed out on different sets of pins) or if > + * you need an anonymous pinmux (not tied to any specific device) > + */ > +struct pinmux *pinmux_get(struct device *dev, const char *func) > +{ > + struct pinmux_map const *map = NULL; > + struct pinmux_dev *pmxdev = NULL; > + const char *devname = NULL; > + struct pinmux *pmx; > + bool found = false; > + int ret = -ENODEV; > + > + /* We must have dev or ID or both */ > + if (!dev && !func) > + return ERR_PTR(-EINVAL); > + > + mutex_lock(&pinmux_list_mutex); > + > + if (dev) > + devname = dev_name(dev); > + > + /* Locate the pinmux map */ > + list_for_each_entry(pmx, &pinmux_list, node) { > + map = pmx->map; > + > + /* If an function is given, it MUST match */ > + if ((func != NULL) && strcmp(map->function, func)) > + continue; > + > + /* > + * This is for the case where no device name is given, we > + * already know that the function name matches from above > + * code. > + */ > + if (!map->dev_name && (func != NULL)) { > + found = true; > + break; > + } > + > + /* If the mapping has a device set up it must match */ > + if (map->dev_name && > + (!devname || !strcmp(map->dev_name, devname))) { > + /* MATCH! */ > + found = true; > + break; > + } > + } > + > + mutex_unlock(&pinmux_list_mutex); > + > + if (!found) { > + pr_err("could not find mux map for device %s, ID %s\n", > + devname ? : "(anonymous)", func ? : "(undefined)"); > + goto out; > + } > + > + /* Make sure that noone else is using this function mapping */ > + mutex_lock(&pmx->mutex); > + if (pmx->dev) { > + if (pmx->dev != dev) { > + mutex_unlock(&pmx->mutex); > + pr_err("mapping already in use device %s, ID %s\n", > + devname ? : "(anonymous)", func ? : "(undefined)"); > + goto out; > + } else { > + /* We already fetched this and requested pins */ > + mutex_unlock(&pmx->mutex); > + ret = 0; > + goto out; > + } > + } > + mutex_unlock(&pmx->mutex); > + > + > + /* > + * Iterate over the drivers so see which ones that may handle this > + * specific muxing. NOTE: there can be only one as of now. > + */ > + list_for_each_entry(pmxdev, &pinmuxdev_list, node) { > + const struct pinmux_ops *ops = pmxdev->desc->ops; > + unsigned selector = 0; > + > + /* See if this pmxdev has this function */ > + while (ops->list_functions(pmxdev, selector) >= 0) { > + const char *fname = ops->get_function_name(pmxdev, > + selector); > + > + if (!strcmp(map->function, fname)) { > + ret = acquire_pins(pmxdev, selector); > + if (ret) > + goto out; > + /* Found it! */ > + mutex_lock(&pmx->mutex); > + pmx->dev = dev; > + pmx->pmxdev = pmxdev; > + pmx->pmxdev_selector = selector; > + mutex_unlock(&pmx->mutex); > + ret = 0; > + goto out; > + } > + selector++; > + } > + } > + /* We couldn't find the driver for this pinmux */ > + ret = -ENODEV; > + > +out: > + if (ret) > + pmx = ERR_PTR(ret); > + > + return pmx; > +} > +EXPORT_SYMBOL_GPL(pinmux_get); > + > +/** > + * pinmux_put() - release a previously claimed pinmux > + * @pmx: a pinmux previously claimed by pinmux_get() > + */ > +void pinmux_put(struct pinmux *pmx) > +{ > + if (pmx == NULL) > + return; > + mutex_lock(&pmx->mutex); > + if (pmx->usecount) > + pr_warn("pinmux: releasing pinmux with active users!\n"); > + pmx->dev = NULL; > + pmx->pmxdev = NULL; > + pmx->pmxdev_selector = 0; > + mutex_unlock(&pmx->mutex); > +} > +EXPORT_SYMBOL_GPL(pinmux_put); > + > +/** > + * pinmux_enable() - enable a certain pinmux setting > + * @pmx: the pinmux to enable, previously claimed by pinmux_get() > + */ > +int pinmux_enable(struct pinmux *pmx) > +{ > + int ret = 0; > + > + if (pmx == NULL) > + return -EINVAL; > + mutex_lock(&pmx->mutex); > + if (pmx->usecount++ == 0) { > + struct pinmux_dev *pmxdev = pmx->pmxdev; > + const struct pinmux_ops *ops = pmxdev->desc->ops; > + > + ret = ops->enable(pmxdev, pmx->pmxdev_selector); > + if (ret) > + pmx->usecount--; > + } > + mutex_unlock(&pmx->mutex); > + return ret; > +} > +EXPORT_SYMBOL_GPL(pinmux_enable); > + > +/** > + * pinmux_disable() - disable a certain pinmux setting > + * @pmx: the pinmux to disable, previously claimed by pinmux_get() > + */ > +void pinmux_disable(struct pinmux *pmx) > +{ > + if (pmx == NULL) > + return; > + > + mutex_lock(&pmx->mutex); > + if (--pmx->usecount == 0) { > + struct pinmux_dev *pmxdev = pmx->pmxdev; > + const struct pinmux_ops *ops = pmxdev->desc->ops; > + > + ops->disable(pmxdev, pmx->pmxdev_selector); > + } > + mutex_unlock(&pmx->mutex); > +} > +EXPORT_SYMBOL_GPL(pinmux_disable); > + > +/** > + * pinmux_config() - configure a certain pinmux setting > + * @pmx: the pinmux setting to configure > + * @param: the parameter to configure > + * @data: extra data to be passed to the configuration, also works as a > + * pointer to data returned from the function on success > + */ > +int pinmux_config(struct pinmux *pmx, u16 param, unsigned long *data) > +{ > + struct pinmux_dev *pmxdev; > + const struct pinmux_ops *ops; > + int ret = 0; > + > + if (pmx == NULL) > + return -ENODEV; > + > + pmxdev = pmx->pmxdev; > + ops = pmxdev->desc->ops; > + > + /* This operation is not mandatory to implement */ > + if (ops->config) { > + mutex_lock(&pmx->mutex); > + ret = ops->config(pmxdev, pmx->pmxdev_selector, param, data); > + mutex_unlock(&pmx->mutex); > + } > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(pinmux_config); > + > +static void pinmux_unclaim_pindescs(int first, int last) > +{ > + int i; > + > + for (i = first; i < last; i++) { > + struct pin_desc *pindesc; > + > + spin_lock(&pin_desc_tree_lock); > + pindesc = radix_tree_lookup(&pin_desc_tree, i); > + spin_unlock(&pin_desc_tree_lock); > + if (pindesc) { > + /* Mark as unclaimed by drivers or functions */ > + pindesc->pmxdev = NULL; > + pindesc->function[0] = '\0'; > + } > + } > +} > + > +/* Helper to claim the individual descs for each pin on a pinmux device */ > +static int pinmux_claim_pindescs(struct pinmux_desc *pmxdesc, > + struct pinmux_dev *pmxdev) > +{ > + int i; > + > + /* Put self as handler of the indicated pin range */ > + for (i = pmxdesc->base; i < (pmxdesc->base + pmxdesc->npins); i++) { > + struct pin_desc *pindesc; > + > + /* Check that none of the pins are already there */ > + spin_lock(&pin_desc_tree_lock); > + pindesc = radix_tree_lookup(&pin_desc_tree, i); > + spin_unlock(&pin_desc_tree_lock); > + if (pindesc == NULL) { > + dev_err(&pmxdev->dev, "pin %d is not registered, yet " > + "attempted to add pinmux driver for it\n", i); > + return -EINVAL; > + } > + if (pindesc->pmxdev != NULL) { > + dev_err(&pmxdev->dev, "pin %d taken by other pinmux " > + "device\n", i); > + return -EINVAL; > + } > + pindesc->pmxdev = pmxdev; > + } > + return 0; > +} > + > +/** > + * pinmux_register() - register a pinmux device > + * @pmxdesc: descriptor for this pinmux > + * @dev: parent device for this pinmux > + * @driver_data: private pinmux data for this pinmux > + */ > +struct pinmux_dev *pinmux_register(struct pinmux_desc *pmxdesc, > + struct device *dev, void *driver_data) > +{ > + static atomic_t pinmux_no = ATOMIC_INIT(0); > + struct pinmux_dev *pmxdev; > + int ret; > + > + if (pmxdesc == NULL) > + return ERR_PTR(-EINVAL); > + if (pmxdesc->name == NULL || pmxdesc->ops == NULL) > + return ERR_PTR(-EINVAL); > + /* These functions are mandatory */ > + if (!pmxdesc->ops->list_functions || > + !pmxdesc->ops->get_function_name || > + !pmxdesc->ops->enable || > + !pmxdesc->ops->disable) > + return ERR_PTR(-EINVAL); > + > + pmxdev = kzalloc(sizeof(struct pinmux_dev), GFP_KERNEL); > + if (pmxdev == NULL) > + return ERR_PTR(-ENOMEM); > + > + mutex_lock(&pinmuxdev_list_mutex); > + pmxdev->owner = pmxdesc->owner; > + pmxdev->desc = pmxdesc; > + pmxdev->driver_data = driver_data; > + > + /* Register device with sysfs */ > + pmxdev->dev.class = &pinmux_class; > + pmxdev->dev.parent = dev; > + dev_set_name(&pmxdev->dev, "pinmux.%d", > + atomic_inc_return(&pinmux_no) - 1); > + ret = device_register(&pmxdev->dev); > + if (ret != 0) { > + put_device(&pmxdev->dev); > + kfree(pmxdev); > + goto out_err; > + } > + dev_set_drvdata(&pmxdev->dev, pmxdev); > + > + ret = pinmux_claim_pindescs(pmxdesc, pmxdev); > + if (ret) > + goto out_err; > + > + list_add(&pmxdev->node, &pinmuxdev_list); > + mutex_unlock(&pinmuxdev_list_mutex); > + return pmxdev; > + > +out_err: > + mutex_unlock(&pinmuxdev_list_mutex); > + put_device(&pmxdev->dev); > + kfree(pmxdev); > + return ERR_PTR(ret); > +} > +EXPORT_SYMBOL_GPL(pinmux_register); > + > +/** > + * pinmux_unregister() - unregister pinmux > + * @pmxdev: pinmux to unregister > + * > + * Called by pinmux drivers to unregister a pinmux. > + */ > +void pinmux_unregister(struct pinmux_dev *pmxdev) > +{ > + if (pmxdev == NULL) > + return; > + > + mutex_lock(&pinmuxdev_list_mutex); > + list_del(&pmxdev->node); > + device_unregister(&pmxdev->dev); > + mutex_unlock(&pinmuxdev_list_mutex); > + pinmux_unclaim_pindescs(pmxdev->desc->base, > + pmxdev->desc->base + pmxdev->desc->npins); > +} > +EXPORT_SYMBOL_GPL(pinmux_unregister); > + > +#ifdef CONFIG_DEBUG_FS > + > +static int pins_show(struct seq_file *s, void *what) > +{ > + unsigned pin; > + > + seq_puts(s, "Pins:\n"); > + spin_lock(&pin_desc_tree_lock); > + for (pin = 0; pin < num_pins; pin++) { > + struct pin_desc *desc; > + > + desc = radix_tree_lookup(&pin_desc_tree, pin); > + > + seq_printf(s, "pin %d (%s)\n", pin, > + desc->name ? desc->name : "(unnamed)"); > + } > + spin_unlock(&pin_desc_tree_lock); > + > + return 0; > +} > + > +static int pinmux_devices_show(struct seq_file *s, void *what) > +{ > + struct pinmux_dev *pmxdev; > + > + seq_puts(s, "Available pinmux settings per pinmux device:\n"); > + list_for_each_entry(pmxdev, &pinmuxdev_list, node) { > + const struct pinmux_ops *ops = pmxdev->desc->ops; > + unsigned selector = 0; > + > + seq_printf(s, "Device %s:\n", pmxdev->desc->name); > + while (ops->list_functions(pmxdev, selector) >= 0) { > + unsigned *pins; > + unsigned num_pins; > + const char *func = ops->get_function_name(pmxdev, > + selector); > + int ret; > + int i; > + > + ret = ops->get_function_pins(pmxdev, selector, > + &pins, &num_pins); > + > + if (ret) > + seq_printf(s, "%s [ERROR GETTING PINS]\n", > + func); > + > + else { > + seq_printf(s, "function: %s, pins = [ ", func); > + for (i = 0; i < num_pins; i++) > + seq_printf(s, "%d ", pins[i]); > + seq_puts(s, "]\n"); > + } > + > + selector++; > + > + } > + } > + > + return 0; > +} > + > +static int pinmux_maps_show(struct seq_file *s, void *what) > +{ > + struct pinmux *pmx; > + const struct pinmux_map *map; > + > + seq_puts(s, "Pinmux maps:\n"); > + list_for_each_entry(pmx, &pinmux_list, node) { > + map = pmx->map; > + > + seq_printf(s, "map: %s -> %s\n", map->function, > + pmx->dev ? dev_name(pmx->dev) : "(unassigned)"); > + } > + > + return 0; > +} > + > +static int pinmux_pins_show(struct seq_file *s, void *what) > +{ > + unsigned pin; > + > + seq_puts(s, "Pinmux settings per pin\n"); > + seq_puts(s, "Format: pin (name): pinmuxfunction pinmuxdriver\n"); > + spin_lock(&pin_desc_tree_lock); > + for (pin = 0; pin < num_pins; pin++) { > + struct pin_desc *desc; > + struct pinmux_dev *pmxdev; > + > + desc = radix_tree_lookup(&pin_desc_tree, pin); > + pmxdev = desc->pmxdev; > + > + seq_printf(s, "pin %d (%s): %s", pin, > + desc->name ? desc->name : "(unnamed)", > + desc->requested ? desc->function : "(unclaimed)"); > + > + if (pmxdev && pmxdev->desc->ops->dbg_show) > + pmxdev->desc->ops->dbg_show(pmxdev, s, > + pin - pmxdev->desc->base); > + > + seq_puts(s, "\n"); > + } > + spin_unlock(&pin_desc_tree_lock); > + > + return 0; > +} > + > +static int pins_open(struct inode *inode, struct file *file) > +{ > + return single_open(file, pins_show, NULL); > +} > + > +static int pinmux_devices_open(struct inode *inode, struct file *file) > +{ > + return single_open(file, pinmux_devices_show, NULL); > +} > + > +static int pinmux_maps_open(struct inode *inode, struct file *file) > +{ > + return single_open(file, pinmux_maps_show, NULL); > +} > + > +static int pinmux_pins_open(struct inode *inode, struct file *file) > +{ > + return single_open(file, pinmux_pins_show, NULL); > +} > + > +static const struct file_operations pins_ops = { > + .open = pins_open, > + .read = seq_read, > + .llseek = seq_lseek, > + .release = single_release, > +}; > + > +static const struct file_operations pinmux_devices_ops = { > + .open = pinmux_devices_open, > + .read = seq_read, > + .llseek = seq_lseek, > + .release = single_release, > +}; > + > +static const struct file_operations pinmux_maps_ops = { > + .open = pinmux_maps_open, > + .read = seq_read, > + .llseek = seq_lseek, > + .release = single_release, > +}; > + > +static const struct file_operations pinmux_pins_ops = { > + .open = pinmux_pins_open, > + .read = seq_read, > + .llseek = seq_lseek, > + .release = single_release, > +}; > + > +static struct dentry *debugfs_root; > + > +static void pinctrl_init_debugfs(void) > +{ > + debugfs_root = debugfs_create_dir("pinctrl", NULL); > + if (IS_ERR(debugfs_root) || !debugfs_root) { > + pr_warn("failed to create debugfs directory\n"); > + debugfs_root = NULL; > + return; > + } > + > + debugfs_create_file("pins", S_IFREG | S_IRUGO, > + debugfs_root, NULL, &pins_ops); > + debugfs_create_file("pinmux-devices", S_IFREG | S_IRUGO, > + debugfs_root, NULL, &pinmux_devices_ops); > + debugfs_create_file("pinmux-maps", S_IFREG | S_IRUGO, > + debugfs_root, NULL, &pinmux_maps_ops); > + debugfs_create_file("pinmux-pins", S_IFREG | S_IRUGO, > + debugfs_root, NULL, &pinmux_pins_ops); > +} > + > +#else /* CONFIG_DEBUG_FS */ > + > +static void pinctrl_init_debugfs(void) > +{ > +} > + > +#endif > + > +static int __init pinctrl_init(void) > +{ > + int ret; > + > + ret = class_register(&pinmux_class); > + pr_info("initialized pinctrl subsystem\n"); > + > + pinctrl_init_debugfs(); > + return ret; > +} > + > +/* init early since many drivers really need to initialized pinmux early */ > +core_initcall(pinctrl_init); > diff --git a/include/linux/pinctrl/machine.h b/include/linux/pinctrl/machine.h > new file mode 100644 > index 0000000..1bc29f7 > --- /dev/null > +++ b/include/linux/pinctrl/machine.h > @@ -0,0 +1,57 @@ > +/* > + * Machine interface for the pinctrl subsystem. > + * > + * Copyright (C) 2011 ST-Ericsson SA > + * Written on behalf of Linaro for ST-Ericsson > + * Based on bits of regulator core, gpio core and clk core > + * > + * Author: Linus Walleij <linus.walleij@linaro.org> > + * > + * License terms: GNU General Public License (GPL) version 2 > + */ > +#ifndef __LINUX_PINMUX_MACHINE_H > +#define __LINUX_PINMUX_MACHINE_H > + > +/** > + * struct pinmux_map - boards/machines shall provide this map for devices > + * @function: a functional name for this mapping so it can be passed down > + * to the driver to invoke that function and be referenced by this ID > + * in e.g. pinmux_get() > + * @dev: the device using this specific mapping, may be NULL if you provide > + * .dev_name instead (this is more common) > + * @dev_name: the name of the device using this specific mapping, the name > + * must be the same that will return your struct device* > + */ > +struct pinmux_map { > + const char *function; > + struct device *dev; > + const char *dev_name; > +}; > + > +/* Convenience macro to set a simple map from a function to a named device */ > +#define PINMUX_MAP(a, b) { .function = a, .dev_name = b } > + > +/** > + * struct pinctrl_pin_desc - boards/machines provide information on their > + * pins, pads or other muxable units in this struct > + * @number: unique pin number from the global pin number space > + * @name: a name for this pin > + */ > +struct pinctrl_pin_desc { > + unsigned number; > + const char *name; > +}; > + > +/* Convenience macro to define a single named or anonymous pin descriptor */ > +#define PINCTRL_PIN(a, b) { .number = a, .name = b } > +#define PINCTRL_PIN_ANON(a) { .number = a } > + > +extern int pinctrl_register_pins_sparse(struct pinctrl_pin_desc const *pins, > + unsigned num_descs); > +extern int pinctrl_register_pins_dense(struct pinctrl_pin_desc const *pins, > + unsigned num_descs, unsigned num_pins); > +extern int pinctrl_register_anon_pins(unsigned first, unsigned last); > +extern int pinmux_register_mappings(struct pinmux_map const *map, > + unsigned num_maps); > + > +#endif > diff --git a/include/linux/pinctrl/pinmux.h b/include/linux/pinctrl/pinmux.h > new file mode 100644 > index 0000000..7b40893 > --- /dev/null > +++ b/include/linux/pinctrl/pinmux.h > @@ -0,0 +1,180 @@ > +/* > + * Interface the pinmux subsystem > + * > + * Copyright (C) 2011 ST-Ericsson SA > + * Written on behalf of Linaro for ST-Ericsson > + * Based on bits of regulator core, gpio core and clk core > + * > + * Author: Linus Walleij <linus.walleij@linaro.org> > + * > + * License terms: GNU General Public License (GPL) version 2 > + */ > +#ifndef __LINUX_PINCTRL_PINMUX_H > +#define __LINUX_PINCTRL_PINMUX_H > + > +#include <linux/list.h> > +#include <linux/seq_file.h> > + > +struct pinmux; > + > +#ifdef CONFIG_PINCTRL > + > +struct pinmux_dev; > + > +/** > + * struct pinmux_ops - pinmux operations, to be implemented by drivers > + * @request: called by the core to see if a certain pin can be muxed in > + * and made available in a certain mux setting The driver is allowed > + * to answer "no" by returning a negative error code > + * @list_functions: list the number of selectable named functions available > + * in this pinmux driver, the core will begin on 0 and call this > + * repeatedly as long as it returns >= 0 to enumerate mux settings > + * @get_function_name: return the function name of the muxing selector, > + * called by the core to figure out which mux setting it shall map a > + * certain device to > + * @get_function_pins: return an array of pins corresponding to a certain > + * function selector in @pins, and the size of the array in @num_pins > + * @enable: enable a certain muxing enumerator. The driver does not need to > + * figure out whether enabling this function conflicts some other use > + * of the pins, such collisions are handled by the pinmux subsystem > + * @disable: disable a certain muxing enumerator > + * @config: custom configuration function for a certain muxing enumerator - > + * this works a bit like an ioctl() and can pass in and return arbitrary > + * configuration data to the pinmux > + * @gpio_request_enable: requests and enables GPIO on a certain pin. > + * Implement this only if you can mux every pin individually as GPIO. If > + * your gpio assignments are grouped, so you cannot control the GPIO > + * muxing of every indvidual pin. > + * @dbg_show: optional debugfs display hook that will provide per-device > + * info for a certain pin in debugfs > + */ > +struct pinmux_ops { > + int (*request) (struct pinmux_dev *pmxdev, unsigned offset); > + int (*free) (struct pinmux_dev *pmxdev, unsigned offset); > + int (*list_functions) (struct pinmux_dev *pmxdev, unsigned selector); > + const char *(*get_function_name) (struct pinmux_dev *pmxdev, > + unsigned selector); > + int (*get_function_pins) (struct pinmux_dev *pmxdev, unsigned selector, > + unsigned ** const pins, unsigned * const num_pins); > + int (*enable) (struct pinmux_dev *pmxdev, unsigned selector); > + void (*disable) (struct pinmux_dev *pmxdev, unsigned selector); > + int (*config) (struct pinmux_dev *pmxdev, unsigned selector, > + u16 param, unsigned long *data); > + int (*gpio_request_enable) (struct pinmux_dev *pmxdev, unsigned offset); > + void (*dbg_show) (struct pinmux_dev *pmxdev, struct seq_file *s, > + unsigned offset); > +}; > + > +/** > + * struct pinmux_desc - pinmux descriptor, register this to pinmux subsystem > + * @name: name for the pinmux > + * @ops: pinmux operation table > + * @owner: module providing the pinmux, used for refcounting > + * @base: the number of the first pin handled by this pinmux, in the global > + * pin space, subtracted from a given pin to get the offset into the range > + * of a certain pinmux > + * @npins: the number of pins handled by this pinmux - note that > + * this is the number of possible pin settings, if your driver handles > + * 8 pins that each can be muxed in 3 different ways, you reserve 24 > + * pins in the global pin space and set this to 24 > + */ > +struct pinmux_desc { > + const char *name; > + struct pinmux_ops *ops; > + struct module *owner; > + int base; > + int npins; > +}; > + > +/** > + * struct pinmux_dev - pinmux class device > + * @desc: the descriptor supplied when initializing this pinmux > + * @node: node to include this pinmux in the global pinmux list > + * @dev: the device entry for this pinmux > + * @owner: module providing the pinmux, used for refcounting > + * @driver_data: driver data for drivers registering to the subsystem > + * > + * This should be dereferenced and used by the pinmux core ONLY > + */ > +struct pinmux_dev { > + struct pinmux_desc *desc; > + struct list_head node; > + struct device dev; > + struct module *owner; > + void *driver_data; > +}; > + > +/* These should only be used from drives */ > +static inline const char *pmxdev_get_name(struct pinmux_dev *pmxdev) > +{ > + /* We're not allowed to register devices without name */ > + return pmxdev->desc->name; > +} > + > +static inline void *pmxdev_get_drvdata(struct pinmux_dev *pmxdev) > +{ > + return pmxdev->driver_data; > +} > + > +/* External interface to pinmux */ > +extern int pin_is_valid(int pin); > +extern int pinmux_request_gpio(int pin, unsigned gpio); > +extern void pinmux_free_gpio(int pin); > +extern struct pinmux *pinmux_get(struct device *dev, const char *func); > +extern void pinmux_put(struct pinmux *pmx); > +extern int pinmux_enable(struct pinmux *pmx); > +extern void pinmux_disable(struct pinmux *pmx); > +extern int pinmux_config(struct pinmux *pmx, u16 param, unsigned long *data); > +extern struct pinmux_dev *pinmux_register(struct pinmux_desc *pmxdesc, > + struct device *dev, void *driver_data); > +extern void pinmux_unregister(struct pinmux_dev *pmxdev); > + > +#else /* !CONFIG_PINMUX */ > + > +static inline int pin_is_valid(int pin) > +{ > + return pin >= 0; > +} > + > +static inline int pinmux_request_gpio(int pin, unsigned gpio) > +{ > + return 0; > +} > + > +static inline void pinmux_free_gpio(int pin) > +{ > +} > + > +static inline int pinmux_register_mappings(struct pinmux_map const *map, > + unsigned num_maps) > +{ > + return 0; > +} > + > +static inline struct pinmux *pinmux_get(struct device *dev, const char *func) > +{ > + return NULL; > +} > + > +static inline void pinmux_put(struct pinmux *pmx) > +{ > +} > + > +static inline int pinmux_enable(struct pinmux *pmx) > +{ > + return 0; > +} > + > +static inline void pinmux_disable(struct pinmux *pmx) > +{ > +} > + > +static inline int pinmux_config(struct pinmux *pmx, u16 param, > + unsigned long *data) > +{ > + return 0; > +} > + > +#endif /* CONFIG_PINMUX */ > + > +#endif /* __LINUX_PINCTRL_PINMUX_H */ > -- > 1.7.3.2 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >
Hi Linus, >> diff --git a/Documentation/ABI/testing/sysfs-class-pinmux b/Documentation/ABI/testing/sysfs-class-pinmux >> new file mode 100644 >> index 0000000..c2ea843 >> --- /dev/null >> +++ b/Documentation/ABI/testing/sysfs-class-pinmux >> @@ -0,0 +1,11 @@ >> +What: /sys/class/pinmux/.../name >> +Date: May 2011 >> +KernelVersion: 3.1 > > has this been ready for 3.1? we have been planning to write pinmux > driver based on this framework. What is the status of pincfg subsystem? what's the plan about merging this subsystem? > >> +Contact: Linus Walleij <linus.walleij@linaro.org> >> +Description: >> + Each pinmux directory will contain a field called >> + name. This holds a string identifying the pinmux for >> + display purposes. >> + >> + NOTE: this will be empty if no suitable name is provided >> + by platform or pinmux drivers. >> diff --git a/Documentation/pinctrl.txt b/Documentation/pinctrl.txt >> new file mode 100644 >> index 0000000..c4b6f48 >> --- /dev/null >> +++ b/Documentation/pinctrl.txt >> @@ -0,0 +1,397 @@ >> +PINCTRL (PIN CONTROL) subsystem >> +This document outlines the pin control subsystem in Linux >> + >> +This subsystem deals with: >> + -barry
On Thu, Jul 14, 2011 at 7:57 AM, Barry Song <21cnbao@gmail.com> wrote: > has this been ready for 3.1? we have been planning to write pinmux > driver based on this framework. On Mon, Aug 8, 2011 at 4:28 AM, Barry Song <21cnbao@gmail.com> wrote: > What is the status of pincfg subsystem? what's the plan about merging > this subsystem? I failed, we needed feedback from some more players. If you need this framework, please review and ACK the patch set when you're happy with it so I know I have users who are happy with it as it looks. v4 is out today atleast. If there are no big objections so I have to re-architect it again, I might be able to ask Stephen to pull it into linux-next. Thanks, Linus Walleij
On Sat, Jul 9, 2011 at 12:23 PM, Mark Brown <broonie@opensource.wolfsonmicro.com> wrote: > On Mon, Jun 13, 2011 at 01:57:36PM -0600, Grant Likely wrote: >> On Mon, Jun 13, 2011 at 10:58 AM, Linus Walleij >> >> I would *strongly* recommend against individual device drivers >> accessing the pinmux api. This is system level configuration code, >> and should be handled at the system level. > > There can also be advantages to putting the pin into the designed mode > without the driver being loaded from the electrical point of view. For > example, selecting appropriate pull values for pads can cut down on > power consumption. Since the pin control subsystem is reference counting wrt mux settings, one does not exclude the other. So for example a driver may or may not grab a certain set-up of pins and the core platform may do the same, but when they start to request different conflicting things the subsystem will complain, as is apropriate. So this is a very pure driver framework, without policy, it just does what it's told, and will prevent undefined and impossible settings. Atleast that's the idea. Thanks, Linus Walleij
On Thu, Jun 16, 2011 at 2:47 PM, Linus Walleij <linus.walleij@linaro.org> wrote: > So to summarize there are two related areas of discussion > here: > > 1. Whether a pinmux map shall map one or 1..N functions > 2. How to handle per-driver instance namespacing of functions > > In both cases I'm currently using simple strings and claiming > that by namespacing these strings cleverly we can avoid > complexity. So my answer to these are: > > 1. Use several functions with ovelapping maps, just name > them differently > 2. Use a string convention and namespace by using > platform/machine/package data and string conventions > such as a "::" separator > > While I *think* (and DO correct me!) that you would argue: > > 1. Make it possible to map several functions to a single > device map > 2. Namespace device instances by different map field > members referring to specific instances > > Is this correctly understood, even if we may not agree? I have now after being massaged by Grant changed opinion on (2) and each pin controller (e.g. pinmux) instance has it's struct device * or pinctrl_dev_name field in the mapping table, so I hope you will find that part solved in an acceptable way in the v4 patch set. So we'd solved 50% of our disagreements. (Please verify!) So remains (1). I hope you will ACK the patch set if I fix this also... I'm thinking about good ways to solve it, reading through your old mails, new suggestions based on the new patch set are welcome. Thanks, Linus Walleij
diff --git a/Documentation/ABI/testing/sysfs-class-pinmux b/Documentation/ABI/testing/sysfs-class-pinmux new file mode 100644 index 0000000..c2ea843 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-class-pinmux @@ -0,0 +1,11 @@ +What: /sys/class/pinmux/.../name +Date: May 2011 +KernelVersion: 3.1 +Contact: Linus Walleij <linus.walleij@linaro.org> +Description: + Each pinmux directory will contain a field called + name. This holds a string identifying the pinmux for + display purposes. + + NOTE: this will be empty if no suitable name is provided + by platform or pinmux drivers. diff --git a/Documentation/pinctrl.txt b/Documentation/pinctrl.txt new file mode 100644 index 0000000..c4b6f48 --- /dev/null +++ b/Documentation/pinctrl.txt @@ -0,0 +1,397 @@ +PINCTRL (PIN CONTROL) subsystem +This document outlines the pin control subsystem in Linux + +This subsystem deals with: + +- Multiplexing of pins, pads, fingers (etc) see below for details + +The intention is to also deal with: + +- Software-controlled biasing and driving mode specific pins, such as + pull-up/down, open drain etc, load capacitance configuration when controlled + by software, etc. + + +Top-level interface +=================== + +Definition of PIN: + +- PINS are equal to pads or fingers or whatever packaging output you want to + mux and these are denoted by unsigned integers in the range 0..MAX_INT. Every + pin on your system (or atleast every pin that can be muxed) should have a + unique number. The numberspace can span several chips if you have more chips + on your system that can be subject to muxing. + +All pins, pads, fingers, balls or other controllable units on the system shall +be given a unique number in the global number space beginning on zero. + +All pins on the system need to be registered, typically by board code or a +device tree. Board/machine code will #include <linux/pinctrl/machine.h> +and use one of: + + pinctrl_register_pins_sparse(); + pinctrl_register_pins_dense(); + +To register all the pins on the system. Both are supplied with a list of +descriptor items, the difference is that the _dense version will also +register anonymous pins for each "hole" in the pin map, from zero to the +supplied extra argument giving the number of pins. + +Here is an example of a PGA (Pin Grid Array) chip seen from underneath: + + A B C D E F G H + + 8 o o o o o o o o + + 7 o o o o o o o o + + 6 o o o o o o o o + + 5 o o o o o o o o + + 4 o o o o o o o o + + 3 o o o o o o o o + + 2 o o o o o o o o + + 1 o o o o o o o o + +To register and name all the pins on this package we can do this in a board +file: + +#include <linux/pinctrl/machine.h> + +const struct pinctrl_pin_desc __initdata my_pins[] = { + PINCTRL_PIN(0, "A1"), + PINCTRL_PIN(1, "A2"), + PINCTRL_PIN(2, "A3"), + ... + PINCTRL_PIN(61, "H6"), + PINCTRL_PIN(62, "H7"), + PINCTRL_PIN(63, "H8"), +}; + +int __init register_pins(void) +{ + pinctrl_register_pins_dense(&my_pins, ARRAY_SIZE(my_pins), 64); +} + +Pins usually have fancier names than this. You can find these in the dataheet +for your chip. Notice that the core machine.h file provides a fancy macro +called PINCTRL_PIN() to create the struct entries. + + +PINMUX interfaces +================= + +These calls use the pinmux_* naming prefix. No other calls should use that +prefix. + + +What is pinmuxing? +================== + +Pinmux, also known as padmux, ballmux, alternate functions or mission modes +is a way for chip vendors producing some kind of electrical packages to use +a certain physical bin (ball, pad, finger, etc) for multiple mutually exclusive +functions, depending on the application. By "application" in this context +we usually mean a way of soldering or wiring the package into an electronic +system, even though the framework makes it possible to handle also change the +function at runtime. + +Here is an example of a PGA (Pin Grid Array) chip seen from underneath: + + A B C D E F G H + +---+ + 8 | o | o o o o o o o + | | + 7 | o | o o o o o o o + | | + 6 | o | o o o o o o o + +---+---+ + 5 | o | o | o o o o o o + +---+---+ +---+ + 4 o o o o o o | o | o + | | + 3 o o o o o o | o | o + | | + 2 o o o o o o | o | o + | | + 1 o o o o o o | o | o + +---+ + +This is not tetris. The game to think of is chess. Not all PGA/BGA packages +are chessboard-like, big ones have "holes" in some arrangement according to +different design patterns, but we're using this as a simple example. Of the +pins you see some will be taken by things like a few VCC and GND to feed power +to the chip, and quite a few will be taken by large ports like an external +memory interface. The remaining pins will often be subject to pin multiplexing. + +The example 8x8 PGA package above will have pin numbers 0 thru 63 assigned to +its physical pins. It will name the pins { A1, A2, A3 ... H6, H7, H8 } using +pinctrl_register_pins_[sparse|dense]() and a suitable data set as shown +earlier. + +In this 8x8 BGA package the pins { A8, A7, A6, A5 } can be used as an SPI port +(these are four pins: CLK, RXD, TXD, FRM). In that case, pin B5 can be used as +some general-purpose GPIO pin. However, in another setting, pins { A5, B5 } can +be used as an I2C port (these are just two pins: SCL, SDA). Needless to say, +we cannot use the SPI port and I2C port at the same time. However in the inside +of the package the silicon performing the SPI logic can alternatively be routed +out on pins { G4, G3, G2, G1 }. + +This way the silicon blocks present inside the chip can be multiplexed "muxed" +out on different pin ranges. Often contemporary SoC (systems on chip) will +contain several I2C, SPI, SDIO/MMC, etc silicon blocks that can be routed to +different pins by pinmux settings. + +Since general-purpose I/O pins (GPIO) are typically always in shortage, it is +common to be able to use almost any pin as a GPIO pin if it is not currently +in use by some other I/O port. + + +Pinmux conventions +================== + +The purpose of the pinmux subsystem is to abstract and provide pinmux settings +to the devices you choose to instantiate in your machine configuration. It is +inspired by the clk, GPIO and regulator subsystems, so devices will request +their mux setting, but it's also possible to request a single pin for e.g. +GPIO. + +Definitions: + +- FUNCTIONS can be switched in and out by a driver residing with the pinmux + subsystem in the drivers/pinmux/* directory of the kernel. The pinmux driver + knows the possible functions. In the example above you can identify three + pinmux functions, two for spi and one for i2c. + +- FUNCTIONS are assumed to be enumerable from zero in a one-dimensional array. + In this case the array could be something like: { spi0-0, spi0-1, i2c0-0 } + for the three available settings. The knowledge of this one-dimensional array + and it's machine-specific particulars is kept inside the pinmux driver, + from the outside only these enumerators are known, and the driver core + can request the name or the list of pins belonging to a certain enumerator. + +- FUNCTIONS are MAPPED to a certain device by the board file, device tree or + similar machine setup configuration mechanism, similar to how regulators are + connected to devices, usually by name. In the example case we can define + that this particular machine shall use device spi0 with pinmux setting + spi0-1 and i2c0 on i2c0-1, something like the two-tuple: + { {spi0, spi0-1}, {i2c0, i2c0-1} } + +- FUNCTIONS are provided on a first-come first-serve basis, so if some other + device mux setting or GPIO pin request has already taken your physical pin, + you will be denied the use of it. To get (activate) a new setting, the old + one has to be put (deactivated) first. + +Sometimes the documentation and hardware registers will be oriented around +pads (or "fingers") rather than pins - these are the soldering surfaces on the +silicon inside the package, and may or may not match the actual number of +pins/balls underneath the capsule. Pick some enumeration that makes sense to +you. Define enumerators only for the pins you can control if that makes sense. + + +Pinmux drivers +============== + +The driver will for all calls be provided an offset pin number into its own +pin range. If you have 2 chips with 8x8 pins, the first chips pins will have +numbers 0 thru 63 and the second one pins 64 thru 127, but the driver for the +second chip will be passed numbers in the range 0 thru 63 anyway, base offset +subtracted. + +Pinmux drivers are required to supply a few callback functions, some are +optional. Usually the enable() and disable() functions are implemented, +writing values into some certain registers to activate a certain mux setting +for a certain pin. + +A simple driver for the above example will work by setting bits 0, 1 or 2 +into some register mamed MUX, so it enumerates its available settings and +their pin assignments, and expose them like this: + +#include <linux/pinctrl/pinmux.h> + +struct foo_pmx_func { + char *name; + const unsigned int *pins; + const unsigned num_pins; +}; + +static unsigned int spi0_0_pins[] = { 0, 8, 16, 24 }; +static unsigned int i2c0_pins[] = { 24, 25 }; +static unsigned int spi0_1_pins[] = { 38, 46, 54, 62 }; + +static struct foo_pmx_func myfuncs[] = { + { + .name = "spi0-0", + .pins = spi0_0_pins, + .num_pins = ARRAY_SIZE(spi0_1_pins), + }, + { + .name = "i2c0", + .pins = i2c0_pins, + .num_pins = ARRAY_SIZE(i2c0_pins), + }, + { + .name = "spi0-1", + .pins = spi0_1_pins, + .num_pins = ARRAY_SIZE(spi0_1_pins), + }, +}; + +int foo_list(struct pinmux_dev *pmxdev, unsigned selector) +{ + if (selector >= ARRAY_SIZE(myfuncs)) + return -EINVAL; + return 0; +} + +const char *foo_get_fname(struct pinmux_dev *pmxdev, unsigned selector) +{ + if (selector >= ARRAY_SIZE(myfuncs)) + return NULL; + return myfuncs[selector].name; +} + +static int foo_get_pins(struct pinmux_dev *pmxdev, unsigned selector, + unsigned ** const pins, unsigned * const num_pins) +{ + if (selector >= ARRAY_SIZE(myfuncs)) + return -EINVAL; + *pins = myfuncs[selector].pins; + *num_pins = myfuncs[selector].num_pins; + return 0; +} + + +int foo_enable(struct pinmux_dev *pmxdev, unsigned selector) +{ + if (selector < ARRAY_SIZE(myfuncs)) + write((read(MUX)|(1<<selector)), MUX) + return 0; + } + return -EINVAL; +} + +int foo_disable(struct pinmux_dev *pmxdev, unsigned selector) +{ + if (selector < ARRAY_SIZE(myfuncs)) + write((read(MUX) & ~(1<<selector)), MUX) + return 0; + } + return -EINVAL; +} + +struct pinmux_ops ops = { + .list_functions = foo_list, + .get_function_name = foo_get_fname, + .get_function_pins = foo_get_pins, + .enable = foo_enable, + .disable = foo_disable, +}; + +Now the able reader will say: "wait - the driver needs to make sure it +can set this and that bit at the same time, because else it will collide +and wreak havoc in my electronics, and make sure noone else is using the +other setting that it's incompatible with". + +In the example activating muxing 0 and 1 at the same time setting bits +0 and 1, uses one pin in common so they would collide. + +The beauty of the pinmux subsystem is that since it keeps track of all +pins and who is using them, it will already have denied an impossible +request like that, so the driver does not need to worry about such +things - when it gets a selector passed in, the pinmux subsystem makes +sure no other device or GPIO assignment is already using the selected +pins. + +The above functions are mandatory to implement for a pinmux driver. + +The function list could become long, especially if you can convert every +individual pin into a GPIO pin independent of any other pins, then your +function array can become 64 entries for each GPIO setting and then the +device functions. For this reason there is an additional function you +can implement to enable only GPIO on an individual pin: gpio_enable(). + + +Pinmux board/machine configuration +================================== + +Boards and machines define how a certain complete running system is put +together, including how GPIOs and devices are muxed, how regulators are +constrained and how the clock tree looks. Of course pinmux settings are also +part of this. + +A pinmux config for a machine looks pretty much like a simple regulator +configuration, so for the example array above we want to enable i2c and +spi on the second function mapping: + +#include <linux/pinctrl/machine.h> + +static struct pinmux_map pmx_mapping[] = { + { + .function = "spi0-1", + .dev_name = "foo-spi.0", + }, + { + .function = "i2c0", + .dev_name = "foo-i2c.0", + }, +}; + +Since the above construct is pretty common there is a helper macro to make +it even more compact: + +static struct pinmux_map pmx_mapping[] = { + PINMUX_MAP("spi0-1", "foo-spi.0"), + PINMUX_MAP("i2c0", "foo-i2c.0"), +}; + +The dev_name here matches to the unique device name that can be used to look +up the device struct (just like with clockdev or regulators). The function name +must match a function provided by the pinmux driver handling this pin range. +You register this pinmux mapping to the pinmux subsystem by simply: + + ret = pinmux_register_mappings(&pmx_mapping, ARRAY_SIZE(pmx_mapping)); + + +Pinmux requests from drivers +============================ + +A driver may request a certain mux to be activated, usually just the default +mux like this: + +#include <linux/pinctrl/pinmux.h> + +foo_probe() +{ + /* Allocate a state holder named "state" etc */ + struct pinmux pmx; + + pmx = pinmux_get(&device, NULL); + if IS_ERR(pmx) + return PTR_ERR(pmx); + pinmux_enable(pmx); + + state->pmx = pmx; +} + +foo_remove() +{ + pinmux_disable(state->pmx); + pinmux_put(state->pmx); +} + +If you want a specific mux setting and not just the first one found for this +device you can specify a specific mux setting, for example in the above example +the second i2c0 setting: pinmux_get(&device, "spi0-2"); + +This get/enable/disable/put sequence can just as well be handled by bus drivers +if you don't want each and every driver to handle it and you know the +arrangement on your bus. + +The pins are allocated for your device when you issue the pinmux_get() call, +after this you should be able to see this in the debugfs listing of all pins. diff --git a/MAINTAINERS b/MAINTAINERS index 29801f7..5caea5a 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -4933,6 +4933,11 @@ L: linux-mtd@lists.infradead.org S: Maintained F: drivers/mtd/devices/phram.c +PINMUX SUBSYSTEM +M: Linus Walleij <linus.walleij@linaro.org> +S: Maintained +F: drivers/pinmux/ + PKTCDVD DRIVER M: Peter Osterlund <petero2@telia.com> S: Maintained diff --git a/drivers/Kconfig b/drivers/Kconfig index 3bb154d..6998d78 100644 --- a/drivers/Kconfig +++ b/drivers/Kconfig @@ -56,6 +56,10 @@ source "drivers/pps/Kconfig" source "drivers/ptp/Kconfig" +# pinctrl before gpio - gpio drivers may need it + +source "drivers/pinctrl/Kconfig" + source "drivers/gpio/Kconfig" source "drivers/w1/Kconfig" diff --git a/drivers/Makefile b/drivers/Makefile index 09f3232..a590a01 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -5,6 +5,8 @@ # Rewritten to use lists instead of if-statements. # +# GPIO must come after pinctrl as gpios may need to mux pins etc +obj-y += pinctrl/ obj-y += gpio/ obj-$(CONFIG_PCI) += pci/ obj-$(CONFIG_PARISC) += parisc/ diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig new file mode 100644 index 0000000..8050fdf --- /dev/null +++ b/drivers/pinctrl/Kconfig @@ -0,0 +1,29 @@ +# +# PINCTRL infrastructure and drivers +# + +menuconfig PINCTRL + bool "PINCTRL Support" + depends on SYSFS && EXPERIMENTAL + help + This enables the PINCTRL subsystem for controlling pins + on chip packages, for example multiplexing pins on primarily + PGA and BGA packages for systems on chip. + + If unsure, say N. + +if PINCTRL + +config DEBUG_PINCTRL + bool "Debug PINCTRL calls" + depends on DEBUG_KERNEL + help + Say Y here to add some extra checks and diagnostics to PINCTRL calls. + +config PINMUX_U300 + bool "U300 pinmux driver" + depends on ARCH_U300 + help + Say Y here to enable the U300 pinmux driver + +endif diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile new file mode 100644 index 0000000..44d8933 --- /dev/null +++ b/drivers/pinctrl/Makefile @@ -0,0 +1,6 @@ +# generic pinmux support + +ccflags-$(CONFIG_DEBUG_PINMUX) += -DDEBUG + +obj-$(CONFIG_PINCTRL) += core.o +obj-$(CONFIG_PINMUX_U300) += pinmux-u300.o diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c new file mode 100644 index 0000000..8fd1437 --- /dev/null +++ b/drivers/pinctrl/core.c @@ -0,0 +1,1028 @@ +/* + * Core driver for the pinmux subsystem + * + * Copyright (C) 2011 ST-Ericsson SA + * Written on behalf of Linaro for ST-Ericsson + * Based on bits of regulator core, gpio core and clk core + * + * Author: Linus Walleij <linus.walleij@linaro.org> + * + * License terms: GNU General Public License (GPL) version 2 + */ +#define pr_fmt(fmt) "pinctrl core: " fmt + +#include <linux/kernel.h> +#include <linux/init.h> +#include <linux/device.h> +#include <linux/slab.h> +#include <linux/radix-tree.h> +#include <linux/err.h> +#include <linux/list.h> +#include <linux/mutex.h> +#include <linux/spinlock.h> +#include <linux/sysfs.h> +#include <linux/debugfs.h> +#include <linux/seq_file.h> +#include <linux/pinctrl/machine.h> +#include <linux/pinctrl/pinmux.h> + +/* Global list of pinmuxes */ +static DEFINE_MUTEX(pinmux_list_mutex); +static LIST_HEAD(pinmux_list); + +/* Global list of pinmux devices */ +static DEFINE_MUTEX(pinmuxdev_list_mutex); +static LIST_HEAD(pinmuxdev_list); + +/** + * struct pin_desc - pin descriptor for each physical pin in the arch + * @pmxdev: corresponding pinmux device + * @requested: whether the pin is already requested by pinmux or not + * @name: a name for the pin, e.g. the name of the pin/pad/finger on a + * datasheet or such + * @function: a named muxing function for the pin that will be passed to + * subdrivers and shown in debugfs etc + */ +struct pin_desc { + struct pinmux_dev *pmxdev; + bool requested; + char name[16]; + char function[16]; +}; +/* Global lookup of per-pin descriptors, one for each physical pin */ +static DEFINE_SPINLOCK(pin_desc_tree_lock); +static RADIX_TREE(pin_desc_tree, GFP_KERNEL); +static unsigned int num_pins = 0; + +/** + * struct pinmux - per-device pinmux state holder + * @node: global list node - only for internal use + * @dev: the device using this pinmux + * @pmxdev: the pinmux device controlling this pinmux + * @map: corresponding pinmux map active for this pinmux setting + * @usecount: the number of active users of this mux setting, used to keep + * track of nested use cases + * @pins: an array of discrete physical pins used in this mapping, taken + * from the global pin enumeration space (copied from pinmux map) + * @num_pins: the number of pins in this mapping array, i.e. the number of + * elements in .pins so we can iterate over that array (copied from + * pinmux map) + * @pmxdev: pinmux device handling this pinmux + * @pmxdev_selector: the selector for the pinmux device handling this pinmux + * @mutex: a lock for the pinmux state holder + */ +struct pinmux { + struct list_head node; + struct device *dev; + struct pinmux_map const *map; + unsigned usecount; + struct pinmux_dev *pmxdev; + unsigned pmxdev_selector; + struct mutex mutex; +}; + +int pin_is_valid(int pin) +{ + return pin >= 0 && pin < num_pins; +} +EXPORT_SYMBOL_GPL(pin_is_valid); + +static ssize_t pinmux_name_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct pinmux_dev *pmxdev = dev_get_drvdata(dev); + + return sprintf(buf, "%s\n", pmxdev_get_name(pmxdev)); +} + +static struct device_attribute pinmux_dev_attrs[] = { + __ATTR(name, 0444, pinmux_name_show, NULL), + __ATTR_NULL, +}; + +static void pinmux_dev_release(struct device *dev) +{ + struct pinmux_dev *pmxdev = dev_get_drvdata(dev); + kfree(pmxdev); +} + +static struct class pinmux_class = { + .name = "pinmux", + .dev_release = pinmux_dev_release, + .dev_attrs = pinmux_dev_attrs, +}; + +/* Deletes a range of pin descriptors */ +static void pinctrl_free_pindescs(struct pinctrl_pin_desc const *pins, + unsigned num_pins) +{ + int i; + + for (i = 0; i < num_pins; i++) { + struct pin_desc *pindesc; + + spin_lock(&pin_desc_tree_lock); + pindesc = radix_tree_lookup(&pin_desc_tree, pins[i].number); + if (pindesc != NULL) { + radix_tree_delete(&pin_desc_tree, pins[i].number); + num_pins --; + } + spin_unlock(&pin_desc_tree_lock); + kfree(pindesc); + } +} + +static int pinctrl_register_one_pin(unsigned number, const char *name) +{ + struct pin_desc *pindesc; + + spin_lock(&pin_desc_tree_lock); + pindesc = radix_tree_lookup(&pin_desc_tree, number); + spin_unlock(&pin_desc_tree_lock); + + if (pindesc != NULL) { + pr_err("pin %d already registered\n", number); + return -EINVAL; + } + + pindesc = kzalloc(sizeof(*pindesc), GFP_KERNEL); + if (pindesc == NULL) + return -ENOMEM; + + /* Copy optional basic pin info */ + if (name) { + strncpy(pindesc->name, name, 16); + pindesc->name[15] = '\0'; + } + + spin_lock(&pin_desc_tree_lock); + radix_tree_insert(&pin_desc_tree, number, pindesc); + num_pins ++; + spin_unlock(&pin_desc_tree_lock); + return 0; +} + +/* Passing in 0 num_pins means "sparse" */ +static int pinctrl_register_pins(struct pinctrl_pin_desc const *pins, + unsigned num_descs, unsigned num_pins) +{ + unsigned i; + int ret = 0; + + for (i = 0; i < num_descs; i++) { + ret = pinctrl_register_one_pin(pins[i].number, pins[i].name); + if (ret) + return ret; + } + + if (num_pins == 0) + return 0; + + /* + * If we are registerering dense pinlists, fill in all holes with + * anonymous pins. + */ + for (i = 0; i < num_pins; i++) { + char pinname[16]; + struct pin_desc *pindesc; + + spin_lock(&pin_desc_tree_lock); + pindesc = radix_tree_lookup(&pin_desc_tree, i); + spin_unlock(&pin_desc_tree_lock); + /* Already registered this one, take next */ + if (pindesc) + continue; + + snprintf(pinname, 15, "anonymous %u", i); + pinname[15] = '\0'; + + ret = pinctrl_register_one_pin(i, pinname); + if (ret) + return ret; + } + + return 0; +} + +/** + * pinctrl_register_pins_sparse() - register a range of pins for a + * board/machine with potential holes in the pin map. The pins in + * the holes will not be usable. + * @pins: a range of pins to register + * @num_descs: the number of pins descriptors passed in through the previous + * pointer + */ +int pinctrl_register_pins_sparse(struct pinctrl_pin_desc const *pins, + unsigned num_descs) +{ + int ret; + + ret = pinctrl_register_pins(pins, num_descs, 0); + if (ret) + pinctrl_free_pindescs(pins, num_descs); + return ret; + +} +EXPORT_SYMBOL_GPL(pinctrl_register_pins); + +/** + * pinctrl_register_pins_dense() - register a range of pins for a + * board/machine, if there are holes in the pin map, they will be + * allocated by anonymous pins. + * @pins: a range of pins to register + * @num_descs: the number of pins descriptors passed in through the previous + * pointer + * @num_pins: the total number of pins including holes in the pin map and + * any "air" at the end of the map, all pins from 0 to this number + * will be allocated, the ones that does not have descriptors passed + * in will be marked "anonymous" + */ +int pinctrl_register_pins_dense(struct pinctrl_pin_desc const *pins, + unsigned num_descs, unsigned num_pins) +{ + int ret; + unsigned i; + + ret = pinctrl_register_pins(pins, num_descs, num_pins); + if (ret) { + for (i = 0; i < num_pins; i++) { + struct pin_desc *pindesc; + + spin_lock(&pin_desc_tree_lock); + pindesc = radix_tree_lookup(&pin_desc_tree, i); + if (pindesc != NULL) { + radix_tree_delete(&pin_desc_tree, i); + num_pins --; + } + spin_unlock(&pin_desc_tree_lock); + kfree(pindesc); + } + } + return ret; +} +EXPORT_SYMBOL_GPL(pinctrl_register_pins_sparse); + +/** + * pin_request() - request a single pin to be muxed in, typically for GPIO + * @pin: the pin number in the global pin space + * @function: a functional name to give to this pin, passed to the driver + * so it knows what function to mux in, e.g. the string "gpioNN" + * means that you want to mux in the pin for use as GPIO number NN + * @gpio: if this request concerns a single GPIO pin + */ +static int pin_request(int pin, const char *function, bool gpio) +{ + struct pin_desc *desc; + struct pinmux_dev *pmxdev; + const struct pinmux_ops *ops; + int status = -EINVAL; + unsigned long flags; + + pr_debug("request pin %d for %s\n", pin, function); + if (!pin_is_valid(pin)) { + pr_err("pin is invalid\n"); + return -EINVAL; + } + + if (!function) { + pr_err("no function name given\n"); + return -EINVAL; + } + + spin_lock_irqsave(&pin_desc_tree_lock, flags); + desc = radix_tree_lookup(&pin_desc_tree, pin); + if (desc == NULL) { + pr_err("pin is not registered so it cannot be requested\n"); + goto out; + } + pmxdev = desc->pmxdev; + if (desc->requested) { + pr_err("pin already requested\n"); + goto out; + } + if (!pmxdev) { + pr_warn("no pinmux device is handling pin %d\n", pin); + goto out; + } + ops = pmxdev->desc->ops; + + /* Let each pin increase references to this module */ + if (!try_module_get(pmxdev->owner)) { + pr_err("could not increase module refcount for pin %d\n", pin); + status = -EINVAL; + goto out; + } + + /* + * If there is no kind of request function for the pin we just assume + * we got it by default and proceed. + */ + if (gpio && ops->gpio_request_enable) + /* This requests and enables a single GPIO pin */ + status = ops->gpio_request_enable(pmxdev, + pin - pmxdev->desc->base); + else if (ops->request) + status = ops->request(pmxdev, + pin - pmxdev->desc->base); + else + status = 0; + + if (status) { + pr_err("->request on device %s failed " + "for pin %d (offset %d)\n", + pmxdev->desc->name, pin, + pin - pmxdev->desc->base); + goto out; + } + + desc->requested = true; + strncpy(desc->function, function, 16); + desc->function[15] = '\0'; + +out: + spin_unlock_irqrestore(&pin_desc_tree_lock, flags); + if (status) + pr_err("pin-%d (%s) status %d\n", + pin, function ? : "?", status); + + return status; +} + +/** + * pin_free() - release a single muxed in pin so something else can be muxed in + * instead + * @pin: the pin to free + */ +static void pin_free(int pin) +{ + struct pin_desc *desc; + struct pinmux_dev *pmxdev; + unsigned long flags; + + if (!pin_is_valid(pin)) + return; + + spin_lock_irqsave(&pin_desc_tree_lock, flags); + desc = radix_tree_lookup(&pin_desc_tree, pin); + if (desc == NULL) { + pr_err("pin is not registered so it cannot be freed\n"); + goto out; + } + pmxdev = desc->pmxdev; + if (pmxdev) { + const struct pinmux_ops *ops = pmxdev->desc->ops; + + if (ops->free) + ops->free(pmxdev, pin - pmxdev->desc->base); + } + desc->requested = false; + desc->function[0] = '\0'; + module_put(pmxdev->owner); +out: + spin_unlock_irqrestore(&pin_desc_tree_lock, flags); +} + +/** + * pinmux_request_gpio() - request a single pin to be muxed in to be used + * as a GPIO pin + * @pin: the pin to mux in as GPIO + * @gpio: the corresponding GPIO pin number + */ +int pinmux_request_gpio(int pin, unsigned gpio) +{ + char gpiostr[16]; + + snprintf(gpiostr, 15, "gpio%d", gpio); + return pin_request(pin, gpiostr, true); +} +EXPORT_SYMBOL_GPL(pinmux_request_gpio); + +/** + * pinmux_free_gpio() - free a single pin, currently muxed in to be used + * as a GPIO pin + * @pin: the pin to mux out from GPIO + */ +void pinmux_free_gpio(int pin) +{ + return pin_free(pin); +} +EXPORT_SYMBOL_GPL(pinmux_free_gpio); + +int pinmux_register_mappings(struct pinmux_map const *maps, unsigned num_maps) +{ + int ret = 0; + int i; + + pr_debug("add %d functions\n", num_maps); + for (i = 0; i < num_maps; i++) { + struct pinmux *pmx; + + /* Sanity check the mapping */ + if (!maps[i].function) { + pr_err("failed to register map %d - no function ID given\n", i); + ret = -EINVAL; + goto out; + } + if (!maps[i].dev && !maps[i].dev_name) { + pr_err("failed to register map %d - no device or device name given\n", i); + ret = -EINVAL; + goto out; + } + + /* + * create the state cookie holder struct pinmux for each + * mapping, this is what consumers will get when requesting + * a pinmux handle with pinmux_get() + */ + pmx = kzalloc(sizeof(struct pinmux), GFP_KERNEL); + if (pmx == NULL) { + ret = -ENOMEM; + goto out; + } + mutex_init(&pmx->mutex); + pmx->map = &maps[i]; + + /* Add the pinmux */ + mutex_lock(&pinmux_list_mutex); + list_add(&pmx->node, &pinmux_list); + mutex_unlock(&pinmux_list_mutex); + pr_debug("add function %s\n", maps[i].function); + } + +out: + return ret; +} + +/** + * acquire_pins() - acquire all the pins for a certain funcion on a certain + * pinmux device + * @pmxdev: the device to take the function on + * @selector: the selector to acquire the pins for + */ +int acquire_pins(struct pinmux_dev *pmxdev, unsigned selector) +{ + const struct pinmux_ops *ops = pmxdev->desc->ops; + unsigned *pins; + unsigned num_pins; + const char *func = ops->get_function_name(pmxdev, selector); + int ret; + int i; + + ret = ops->get_function_pins(pmxdev, selector, &pins, &num_pins); + if (ret) + return ret; + + /* Try to allocate all pins in this pinmux map, one by one */ + for (i = 0; i < num_pins; i++) { + ret = pin_request(pins[i], func, false); + if (ret) { + pr_err("could not get pin %d for function %s " + "on device %s - conflicting mux mappings?\n", + pins[i], func ? : "(undefined)", + pmxdev->desc->name); + /* On error release all taken pins */ + i--; /* this pin just failed */ + for (; i >= 0; i--) + pin_free(pins[i]); + return -ENODEV; + } + } + return 0; +} + +/** + * pinmux_get() - retrieves the pinmux for a certain device + * @dev: the device to get the pinmux for + * @func: an optional mux name or NULL, the name is only needed + * if a single device has multiple pinmux settings (i.e. if the + * same device can be muxed out on different sets of pins) or if + * you need an anonymous pinmux (not tied to any specific device) + */ +struct pinmux *pinmux_get(struct device *dev, const char *func) +{ + struct pinmux_map const *map = NULL; + struct pinmux_dev *pmxdev = NULL; + const char *devname = NULL; + struct pinmux *pmx; + bool found = false; + int ret = -ENODEV; + + /* We must have dev or ID or both */ + if (!dev && !func) + return ERR_PTR(-EINVAL); + + mutex_lock(&pinmux_list_mutex); + + if (dev) + devname = dev_name(dev); + + /* Locate the pinmux map */ + list_for_each_entry(pmx, &pinmux_list, node) { + map = pmx->map; + + /* If an function is given, it MUST match */ + if ((func != NULL) && strcmp(map->function, func)) + continue; + + /* + * This is for the case where no device name is given, we + * already know that the function name matches from above + * code. + */ + if (!map->dev_name && (func != NULL)) { + found = true; + break; + } + + /* If the mapping has a device set up it must match */ + if (map->dev_name && + (!devname || !strcmp(map->dev_name, devname))) { + /* MATCH! */ + found = true; + break; + } + } + + mutex_unlock(&pinmux_list_mutex); + + if (!found) { + pr_err("could not find mux map for device %s, ID %s\n", + devname ? : "(anonymous)", func ? : "(undefined)"); + goto out; + } + + /* Make sure that noone else is using this function mapping */ + mutex_lock(&pmx->mutex); + if (pmx->dev) { + if (pmx->dev != dev) { + mutex_unlock(&pmx->mutex); + pr_err("mapping already in use device %s, ID %s\n", + devname ? : "(anonymous)", func ? : "(undefined)"); + goto out; + } else { + /* We already fetched this and requested pins */ + mutex_unlock(&pmx->mutex); + ret = 0; + goto out; + } + } + mutex_unlock(&pmx->mutex); + + + /* + * Iterate over the drivers so see which ones that may handle this + * specific muxing. NOTE: there can be only one as of now. + */ + list_for_each_entry(pmxdev, &pinmuxdev_list, node) { + const struct pinmux_ops *ops = pmxdev->desc->ops; + unsigned selector = 0; + + /* See if this pmxdev has this function */ + while (ops->list_functions(pmxdev, selector) >= 0) { + const char *fname = ops->get_function_name(pmxdev, + selector); + + if (!strcmp(map->function, fname)) { + ret = acquire_pins(pmxdev, selector); + if (ret) + goto out; + /* Found it! */ + mutex_lock(&pmx->mutex); + pmx->dev = dev; + pmx->pmxdev = pmxdev; + pmx->pmxdev_selector = selector; + mutex_unlock(&pmx->mutex); + ret = 0; + goto out; + } + selector++; + } + } + /* We couldn't find the driver for this pinmux */ + ret = -ENODEV; + +out: + if (ret) + pmx = ERR_PTR(ret); + + return pmx; +} +EXPORT_SYMBOL_GPL(pinmux_get); + +/** + * pinmux_put() - release a previously claimed pinmux + * @pmx: a pinmux previously claimed by pinmux_get() + */ +void pinmux_put(struct pinmux *pmx) +{ + if (pmx == NULL) + return; + mutex_lock(&pmx->mutex); + if (pmx->usecount) + pr_warn("pinmux: releasing pinmux with active users!\n"); + pmx->dev = NULL; + pmx->pmxdev = NULL; + pmx->pmxdev_selector = 0; + mutex_unlock(&pmx->mutex); +} +EXPORT_SYMBOL_GPL(pinmux_put); + +/** + * pinmux_enable() - enable a certain pinmux setting + * @pmx: the pinmux to enable, previously claimed by pinmux_get() + */ +int pinmux_enable(struct pinmux *pmx) +{ + int ret = 0; + + if (pmx == NULL) + return -EINVAL; + mutex_lock(&pmx->mutex); + if (pmx->usecount++ == 0) { + struct pinmux_dev *pmxdev = pmx->pmxdev; + const struct pinmux_ops *ops = pmxdev->desc->ops; + + ret = ops->enable(pmxdev, pmx->pmxdev_selector); + if (ret) + pmx->usecount--; + } + mutex_unlock(&pmx->mutex); + return ret; +} +EXPORT_SYMBOL_GPL(pinmux_enable); + +/** + * pinmux_disable() - disable a certain pinmux setting + * @pmx: the pinmux to disable, previously claimed by pinmux_get() + */ +void pinmux_disable(struct pinmux *pmx) +{ + if (pmx == NULL) + return; + + mutex_lock(&pmx->mutex); + if (--pmx->usecount == 0) { + struct pinmux_dev *pmxdev = pmx->pmxdev; + const struct pinmux_ops *ops = pmxdev->desc->ops; + + ops->disable(pmxdev, pmx->pmxdev_selector); + } + mutex_unlock(&pmx->mutex); +} +EXPORT_SYMBOL_GPL(pinmux_disable); + +/** + * pinmux_config() - configure a certain pinmux setting + * @pmx: the pinmux setting to configure + * @param: the parameter to configure + * @data: extra data to be passed to the configuration, also works as a + * pointer to data returned from the function on success + */ +int pinmux_config(struct pinmux *pmx, u16 param, unsigned long *data) +{ + struct pinmux_dev *pmxdev; + const struct pinmux_ops *ops; + int ret = 0; + + if (pmx == NULL) + return -ENODEV; + + pmxdev = pmx->pmxdev; + ops = pmxdev->desc->ops; + + /* This operation is not mandatory to implement */ + if (ops->config) { + mutex_lock(&pmx->mutex); + ret = ops->config(pmxdev, pmx->pmxdev_selector, param, data); + mutex_unlock(&pmx->mutex); + } + + return 0; +} +EXPORT_SYMBOL_GPL(pinmux_config); + +static void pinmux_unclaim_pindescs(int first, int last) +{ + int i; + + for (i = first; i < last; i++) { + struct pin_desc *pindesc; + + spin_lock(&pin_desc_tree_lock); + pindesc = radix_tree_lookup(&pin_desc_tree, i); + spin_unlock(&pin_desc_tree_lock); + if (pindesc) { + /* Mark as unclaimed by drivers or functions */ + pindesc->pmxdev = NULL; + pindesc->function[0] = '\0'; + } + } +} + +/* Helper to claim the individual descs for each pin on a pinmux device */ +static int pinmux_claim_pindescs(struct pinmux_desc *pmxdesc, + struct pinmux_dev *pmxdev) +{ + int i; + + /* Put self as handler of the indicated pin range */ + for (i = pmxdesc->base; i < (pmxdesc->base + pmxdesc->npins); i++) { + struct pin_desc *pindesc; + + /* Check that none of the pins are already there */ + spin_lock(&pin_desc_tree_lock); + pindesc = radix_tree_lookup(&pin_desc_tree, i); + spin_unlock(&pin_desc_tree_lock); + if (pindesc == NULL) { + dev_err(&pmxdev->dev, "pin %d is not registered, yet " + "attempted to add pinmux driver for it\n", i); + return -EINVAL; + } + if (pindesc->pmxdev != NULL) { + dev_err(&pmxdev->dev, "pin %d taken by other pinmux " + "device\n", i); + return -EINVAL; + } + pindesc->pmxdev = pmxdev; + } + return 0; +} + +/** + * pinmux_register() - register a pinmux device + * @pmxdesc: descriptor for this pinmux + * @dev: parent device for this pinmux + * @driver_data: private pinmux data for this pinmux + */ +struct pinmux_dev *pinmux_register(struct pinmux_desc *pmxdesc, + struct device *dev, void *driver_data) +{ + static atomic_t pinmux_no = ATOMIC_INIT(0); + struct pinmux_dev *pmxdev; + int ret; + + if (pmxdesc == NULL) + return ERR_PTR(-EINVAL); + if (pmxdesc->name == NULL || pmxdesc->ops == NULL) + return ERR_PTR(-EINVAL); + /* These functions are mandatory */ + if (!pmxdesc->ops->list_functions || + !pmxdesc->ops->get_function_name || + !pmxdesc->ops->enable || + !pmxdesc->ops->disable) + return ERR_PTR(-EINVAL); + + pmxdev = kzalloc(sizeof(struct pinmux_dev), GFP_KERNEL); + if (pmxdev == NULL) + return ERR_PTR(-ENOMEM); + + mutex_lock(&pinmuxdev_list_mutex); + pmxdev->owner = pmxdesc->owner; + pmxdev->desc = pmxdesc; + pmxdev->driver_data = driver_data; + + /* Register device with sysfs */ + pmxdev->dev.class = &pinmux_class; + pmxdev->dev.parent = dev; + dev_set_name(&pmxdev->dev, "pinmux.%d", + atomic_inc_return(&pinmux_no) - 1); + ret = device_register(&pmxdev->dev); + if (ret != 0) { + put_device(&pmxdev->dev); + kfree(pmxdev); + goto out_err; + } + dev_set_drvdata(&pmxdev->dev, pmxdev); + + ret = pinmux_claim_pindescs(pmxdesc, pmxdev); + if (ret) + goto out_err; + + list_add(&pmxdev->node, &pinmuxdev_list); + mutex_unlock(&pinmuxdev_list_mutex); + return pmxdev; + +out_err: + mutex_unlock(&pinmuxdev_list_mutex); + put_device(&pmxdev->dev); + kfree(pmxdev); + return ERR_PTR(ret); +} +EXPORT_SYMBOL_GPL(pinmux_register); + +/** + * pinmux_unregister() - unregister pinmux + * @pmxdev: pinmux to unregister + * + * Called by pinmux drivers to unregister a pinmux. + */ +void pinmux_unregister(struct pinmux_dev *pmxdev) +{ + if (pmxdev == NULL) + return; + + mutex_lock(&pinmuxdev_list_mutex); + list_del(&pmxdev->node); + device_unregister(&pmxdev->dev); + mutex_unlock(&pinmuxdev_list_mutex); + pinmux_unclaim_pindescs(pmxdev->desc->base, + pmxdev->desc->base + pmxdev->desc->npins); +} +EXPORT_SYMBOL_GPL(pinmux_unregister); + +#ifdef CONFIG_DEBUG_FS + +static int pins_show(struct seq_file *s, void *what) +{ + unsigned pin; + + seq_puts(s, "Pins:\n"); + spin_lock(&pin_desc_tree_lock); + for (pin = 0; pin < num_pins; pin++) { + struct pin_desc *desc; + + desc = radix_tree_lookup(&pin_desc_tree, pin); + + seq_printf(s, "pin %d (%s)\n", pin, + desc->name ? desc->name : "(unnamed)"); + } + spin_unlock(&pin_desc_tree_lock); + + return 0; +} + +static int pinmux_devices_show(struct seq_file *s, void *what) +{ + struct pinmux_dev *pmxdev; + + seq_puts(s, "Available pinmux settings per pinmux device:\n"); + list_for_each_entry(pmxdev, &pinmuxdev_list, node) { + const struct pinmux_ops *ops = pmxdev->desc->ops; + unsigned selector = 0; + + seq_printf(s, "Device %s:\n", pmxdev->desc->name); + while (ops->list_functions(pmxdev, selector) >= 0) { + unsigned *pins; + unsigned num_pins; + const char *func = ops->get_function_name(pmxdev, + selector); + int ret; + int i; + + ret = ops->get_function_pins(pmxdev, selector, + &pins, &num_pins); + + if (ret) + seq_printf(s, "%s [ERROR GETTING PINS]\n", + func); + + else { + seq_printf(s, "function: %s, pins = [ ", func); + for (i = 0; i < num_pins; i++) + seq_printf(s, "%d ", pins[i]); + seq_puts(s, "]\n"); + } + + selector++; + + } + } + + return 0; +} + +static int pinmux_maps_show(struct seq_file *s, void *what) +{ + struct pinmux *pmx; + const struct pinmux_map *map; + + seq_puts(s, "Pinmux maps:\n"); + list_for_each_entry(pmx, &pinmux_list, node) { + map = pmx->map; + + seq_printf(s, "map: %s -> %s\n", map->function, + pmx->dev ? dev_name(pmx->dev) : "(unassigned)"); + } + + return 0; +} + +static int pinmux_pins_show(struct seq_file *s, void *what) +{ + unsigned pin; + + seq_puts(s, "Pinmux settings per pin\n"); + seq_puts(s, "Format: pin (name): pinmuxfunction pinmuxdriver\n"); + spin_lock(&pin_desc_tree_lock); + for (pin = 0; pin < num_pins; pin++) { + struct pin_desc *desc; + struct pinmux_dev *pmxdev; + + desc = radix_tree_lookup(&pin_desc_tree, pin); + pmxdev = desc->pmxdev; + + seq_printf(s, "pin %d (%s): %s", pin, + desc->name ? desc->name : "(unnamed)", + desc->requested ? desc->function : "(unclaimed)"); + + if (pmxdev && pmxdev->desc->ops->dbg_show) + pmxdev->desc->ops->dbg_show(pmxdev, s, + pin - pmxdev->desc->base); + + seq_puts(s, "\n"); + } + spin_unlock(&pin_desc_tree_lock); + + return 0; +} + +static int pins_open(struct inode *inode, struct file *file) +{ + return single_open(file, pins_show, NULL); +} + +static int pinmux_devices_open(struct inode *inode, struct file *file) +{ + return single_open(file, pinmux_devices_show, NULL); +} + +static int pinmux_maps_open(struct inode *inode, struct file *file) +{ + return single_open(file, pinmux_maps_show, NULL); +} + +static int pinmux_pins_open(struct inode *inode, struct file *file) +{ + return single_open(file, pinmux_pins_show, NULL); +} + +static const struct file_operations pins_ops = { + .open = pins_open, + .read = seq_read, + .llseek = seq_lseek, + .release = single_release, +}; + +static const struct file_operations pinmux_devices_ops = { + .open = pinmux_devices_open, + .read = seq_read, + .llseek = seq_lseek, + .release = single_release, +}; + +static const struct file_operations pinmux_maps_ops = { + .open = pinmux_maps_open, + .read = seq_read, + .llseek = seq_lseek, + .release = single_release, +}; + +static const struct file_operations pinmux_pins_ops = { + .open = pinmux_pins_open, + .read = seq_read, + .llseek = seq_lseek, + .release = single_release, +}; + +static struct dentry *debugfs_root; + +static void pinctrl_init_debugfs(void) +{ + debugfs_root = debugfs_create_dir("pinctrl", NULL); + if (IS_ERR(debugfs_root) || !debugfs_root) { + pr_warn("failed to create debugfs directory\n"); + debugfs_root = NULL; + return; + } + + debugfs_create_file("pins", S_IFREG | S_IRUGO, + debugfs_root, NULL, &pins_ops); + debugfs_create_file("pinmux-devices", S_IFREG | S_IRUGO, + debugfs_root, NULL, &pinmux_devices_ops); + debugfs_create_file("pinmux-maps", S_IFREG | S_IRUGO, + debugfs_root, NULL, &pinmux_maps_ops); + debugfs_create_file("pinmux-pins", S_IFREG | S_IRUGO, + debugfs_root, NULL, &pinmux_pins_ops); +} + +#else /* CONFIG_DEBUG_FS */ + +static void pinctrl_init_debugfs(void) +{ +} + +#endif + +static int __init pinctrl_init(void) +{ + int ret; + + ret = class_register(&pinmux_class); + pr_info("initialized pinctrl subsystem\n"); + + pinctrl_init_debugfs(); + return ret; +} + +/* init early since many drivers really need to initialized pinmux early */ +core_initcall(pinctrl_init); diff --git a/include/linux/pinctrl/machine.h b/include/linux/pinctrl/machine.h new file mode 100644 index 0000000..1bc29f7 --- /dev/null +++ b/include/linux/pinctrl/machine.h @@ -0,0 +1,57 @@ +/* + * Machine interface for the pinctrl subsystem. + * + * Copyright (C) 2011 ST-Ericsson SA + * Written on behalf of Linaro for ST-Ericsson + * Based on bits of regulator core, gpio core and clk core + * + * Author: Linus Walleij <linus.walleij@linaro.org> + * + * License terms: GNU General Public License (GPL) version 2 + */ +#ifndef __LINUX_PINMUX_MACHINE_H +#define __LINUX_PINMUX_MACHINE_H + +/** + * struct pinmux_map - boards/machines shall provide this map for devices + * @function: a functional name for this mapping so it can be passed down + * to the driver to invoke that function and be referenced by this ID + * in e.g. pinmux_get() + * @dev: the device using this specific mapping, may be NULL if you provide + * .dev_name instead (this is more common) + * @dev_name: the name of the device using this specific mapping, the name + * must be the same that will return your struct device* + */ +struct pinmux_map { + const char *function; + struct device *dev; + const char *dev_name; +}; + +/* Convenience macro to set a simple map from a function to a named device */ +#define PINMUX_MAP(a, b) { .function = a, .dev_name = b } + +/** + * struct pinctrl_pin_desc - boards/machines provide information on their + * pins, pads or other muxable units in this struct + * @number: unique pin number from the global pin number space + * @name: a name for this pin + */ +struct pinctrl_pin_desc { + unsigned number; + const char *name; +}; + +/* Convenience macro to define a single named or anonymous pin descriptor */ +#define PINCTRL_PIN(a, b) { .number = a, .name = b } +#define PINCTRL_PIN_ANON(a) { .number = a } + +extern int pinctrl_register_pins_sparse(struct pinctrl_pin_desc const *pins, + unsigned num_descs); +extern int pinctrl_register_pins_dense(struct pinctrl_pin_desc const *pins, + unsigned num_descs, unsigned num_pins); +extern int pinctrl_register_anon_pins(unsigned first, unsigned last); +extern int pinmux_register_mappings(struct pinmux_map const *map, + unsigned num_maps); + +#endif diff --git a/include/linux/pinctrl/pinmux.h b/include/linux/pinctrl/pinmux.h new file mode 100644 index 0000000..7b40893 --- /dev/null +++ b/include/linux/pinctrl/pinmux.h @@ -0,0 +1,180 @@ +/* + * Interface the pinmux subsystem + * + * Copyright (C) 2011 ST-Ericsson SA + * Written on behalf of Linaro for ST-Ericsson + * Based on bits of regulator core, gpio core and clk core + * + * Author: Linus Walleij <linus.walleij@linaro.org> + * + * License terms: GNU General Public License (GPL) version 2 + */ +#ifndef __LINUX_PINCTRL_PINMUX_H +#define __LINUX_PINCTRL_PINMUX_H + +#include <linux/list.h> +#include <linux/seq_file.h> + +struct pinmux; + +#ifdef CONFIG_PINCTRL + +struct pinmux_dev; + +/** + * struct pinmux_ops - pinmux operations, to be implemented by drivers + * @request: called by the core to see if a certain pin can be muxed in + * and made available in a certain mux setting The driver is allowed + * to answer "no" by returning a negative error code + * @list_functions: list the number of selectable named functions available + * in this pinmux driver, the core will begin on 0 and call this + * repeatedly as long as it returns >= 0 to enumerate mux settings + * @get_function_name: return the function name of the muxing selector, + * called by the core to figure out which mux setting it shall map a + * certain device to + * @get_function_pins: return an array of pins corresponding to a certain + * function selector in @pins, and the size of the array in @num_pins + * @enable: enable a certain muxing enumerator. The driver does not need to + * figure out whether enabling this function conflicts some other use + * of the pins, such collisions are handled by the pinmux subsystem + * @disable: disable a certain muxing enumerator + * @config: custom configuration function for a certain muxing enumerator - + * this works a bit like an ioctl() and can pass in and return arbitrary + * configuration data to the pinmux + * @gpio_request_enable: requests and enables GPIO on a certain pin. + * Implement this only if you can mux every pin individually as GPIO. If + * your gpio assignments are grouped, so you cannot control the GPIO + * muxing of every indvidual pin. + * @dbg_show: optional debugfs display hook that will provide per-device + * info for a certain pin in debugfs + */ +struct pinmux_ops { + int (*request) (struct pinmux_dev *pmxdev, unsigned offset); + int (*free) (struct pinmux_dev *pmxdev, unsigned offset); + int (*list_functions) (struct pinmux_dev *pmxdev, unsigned selector); + const char *(*get_function_name) (struct pinmux_dev *pmxdev, + unsigned selector); + int (*get_function_pins) (struct pinmux_dev *pmxdev, unsigned selector, + unsigned ** const pins, unsigned * const num_pins); + int (*enable) (struct pinmux_dev *pmxdev, unsigned selector); + void (*disable) (struct pinmux_dev *pmxdev, unsigned selector); + int (*config) (struct pinmux_dev *pmxdev, unsigned selector, + u16 param, unsigned long *data); + int (*gpio_request_enable) (struct pinmux_dev *pmxdev, unsigned offset); + void (*dbg_show) (struct pinmux_dev *pmxdev, struct seq_file *s, + unsigned offset); +}; + +/** + * struct pinmux_desc - pinmux descriptor, register this to pinmux subsystem + * @name: name for the pinmux + * @ops: pinmux operation table + * @owner: module providing the pinmux, used for refcounting + * @base: the number of the first pin handled by this pinmux, in the global + * pin space, subtracted from a given pin to get the offset into the range + * of a certain pinmux + * @npins: the number of pins handled by this pinmux - note that + * this is the number of possible pin settings, if your driver handles + * 8 pins that each can be muxed in 3 different ways, you reserve 24 + * pins in the global pin space and set this to 24 + */ +struct pinmux_desc { + const char *name; + struct pinmux_ops *ops; + struct module *owner; + int base; + int npins; +}; + +/** + * struct pinmux_dev - pinmux class device + * @desc: the descriptor supplied when initializing this pinmux + * @node: node to include this pinmux in the global pinmux list + * @dev: the device entry for this pinmux + * @owner: module providing the pinmux, used for refcounting + * @driver_data: driver data for drivers registering to the subsystem + * + * This should be dereferenced and used by the pinmux core ONLY + */ +struct pinmux_dev { + struct pinmux_desc *desc; + struct list_head node; + struct device dev; + struct module *owner; + void *driver_data; +}; + +/* These should only be used from drives */ +static inline const char *pmxdev_get_name(struct pinmux_dev *pmxdev) +{ + /* We're not allowed to register devices without name */ + return pmxdev->desc->name; +} + +static inline void *pmxdev_get_drvdata(struct pinmux_dev *pmxdev) +{ + return pmxdev->driver_data; +} + +/* External interface to pinmux */ +extern int pin_is_valid(int pin); +extern int pinmux_request_gpio(int pin, unsigned gpio); +extern void pinmux_free_gpio(int pin); +extern struct pinmux *pinmux_get(struct device *dev, const char *func); +extern void pinmux_put(struct pinmux *pmx); +extern int pinmux_enable(struct pinmux *pmx); +extern void pinmux_disable(struct pinmux *pmx); +extern int pinmux_config(struct pinmux *pmx, u16 param, unsigned long *data); +extern struct pinmux_dev *pinmux_register(struct pinmux_desc *pmxdesc, + struct device *dev, void *driver_data); +extern void pinmux_unregister(struct pinmux_dev *pmxdev); + +#else /* !CONFIG_PINMUX */ + +static inline int pin_is_valid(int pin) +{ + return pin >= 0; +} + +static inline int pinmux_request_gpio(int pin, unsigned gpio) +{ + return 0; +} + +static inline void pinmux_free_gpio(int pin) +{ +} + +static inline int pinmux_register_mappings(struct pinmux_map const *map, + unsigned num_maps) +{ + return 0; +} + +static inline struct pinmux *pinmux_get(struct device *dev, const char *func) +{ + return NULL; +} + +static inline void pinmux_put(struct pinmux *pmx) +{ +} + +static inline int pinmux_enable(struct pinmux *pmx) +{ + return 0; +} + +static inline void pinmux_disable(struct pinmux *pmx) +{ +} + +static inline int pinmux_config(struct pinmux *pmx, u16 param, + unsigned long *data) +{ + return 0; +} + +#endif /* CONFIG_PINMUX */ + +#endif /* __LINUX_PINCTRL_PINMUX_H */