Hi Linus, On 22 November 2011 01:17, Linus Walleij <linus.walleij@xxxxxxxxxxxxxx> wrote: > From: Linus Walleij <linus.walleij@xxxxxxxxxx> > > This add per-pin and per-group pin config interfaces for biasing, > driving and other such electronic properties. The intention is > clearly to enumerate all things you can do with pins, hoping that > these are enumerable. [...] > Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx> > --- > Documentation/pinctrl.txt | 105 +++++++++++- > drivers/pinctrl/Kconfig | 5 +- > drivers/pinctrl/Makefile | 1 + > drivers/pinctrl/core.c | 19 ++ > drivers/pinctrl/core.h | 10 + > drivers/pinctrl/pinconf.c | 366 +++++++++++++++++++++++++++++++++++++++ > drivers/pinctrl/pinconf.h | 41 +++++ > include/linux/pinctrl/pinconf.h | 209 ++++++++++++++++++++++ > include/linux/pinctrl/pinctrl.h | 10 +- > 9 files changed, 755 insertions(+), 11 deletions(-) > create mode 100644 drivers/pinctrl/pinconf.c > create mode 100644 drivers/pinctrl/pinconf.h > create mode 100644 include/linux/pinctrl/pinconf.h > [...] > diff --git a/drivers/pinctrl/core.h b/drivers/pinctrl/core.h > index 74dee43..4b30a28 100644 > --- a/drivers/pinctrl/core.h > +++ b/drivers/pinctrl/core.h > @@ -9,6 +9,10 @@ > * License terms: GNU General Public License (GPL) version 2 > */ > > +#include <linux/pinctrl/pinconf.h> > + > +struct pinctrl_gpio_range; > + > /** > * struct pinctrl_dev - pin control class device > * @node: node to include this pin controller in the global pin controller list > @@ -52,6 +56,8 @@ struct pinctrl_dev { > * @mux_requested: whether the pin is already requested by pinmux or not > * @mux_function: a named muxing function for the pin that will be passed to > * subdrivers and shown in debugfs etc > + * @config_lock: a lock to protect the pin configuration portions > + * @pin_configs: a list of configuration settings for this pin > */ > struct pin_desc { > struct pinctrl_dev *pctldev; > @@ -61,6 +67,10 @@ struct pin_desc { > #ifdef CONFIG_PINMUX > const char *mux_function; > #endif > +#ifdef CONFIG_PINCONF > + struct mutex config_lock; > + struct pin_config config; Some platforms (like exynos) can read back the current pin_config from the hardware registers. For such platforms, 'config' would consume additional space which will not be used. Can 'config' be made a pointer and memory for it allocated in 'pinctrl_register_one_pin' only if the pin-controller requires it. Maybe, 'struct pinctrl_desc' can have a additional member to describe some of its characteristics such as ability to read back pin configuration from registers. > +#endif > }; > > struct pinctrl_dev *get_pinctrl_dev_from_dev(struct device *dev, > diff --git a/drivers/pinctrl/pinconf.c b/drivers/pinctrl/pinconf.c > new file mode 100644 > index 0000000..dc59c04 > --- /dev/null > +++ b/drivers/pinctrl/pinconf.c [...] > +int pin_config(struct pinctrl_dev *pctldev, int pin, > + const struct pin_config_tuple *configs, > + unsigned num_configs) > +{ > + const struct pinconf_ops *ops = pctldev->desc->confops; > + struct pin_desc *desc; > + struct pin_config *conf; > + int ret; > + > + desc = pin_desc_get(pctldev, pin); > + if (desc == NULL) { > + dev_err(&pctldev->dev, "tried to configure unregistered pin\n"); > + return -EINVAL; > + } > + > + conf = &desc->config; > + > + if (!ops || !ops->pin_config) { > + dev_err(&pctldev->dev, "cannot configure pin, missing " > + "config function in driver\n"); > + return -EINVAL; > + } > + > + mutex_lock(&desc->config_lock); > + ret = ops->pin_config(pctldev, conf, pin, configs, num_configs); > + if (ret) { > + dev_err(&pctldev->dev, > + "unable to set pin configuration on pin %d\n", pin); > + return ret; > + } > + pinconf_update_states(conf, configs, num_configs); > + mutex_unlock(&desc->config_lock); > + > + return 0; > +} EXPORT_SYMBOL(pin_config); here ? > + > +int pin_config_group(struct pinctrl_dev *pctldev, const char *pin_group, > + const struct pin_config_tuple *configs, > + unsigned num_configs) > +{ > + const struct pinctrl_ops *pctlops = pctldev->desc->pctlops; > + const struct pinconf_ops *ops = pctldev->desc->confops; > + int selector; > + const unsigned *pins; > + unsigned num_pins; > + int ret; > + int i; > + > + if (!ops || (!ops->pin_config_group && !ops->pin_config)) { > + dev_err(&pctldev->dev, "cannot configure pin group, missing " > + "config function in driver\n"); > + return -EINVAL; > + } > + > + selector = pinctrl_get_group_selector(pctldev, pin_group); > + if (selector < 0) > + return selector; > + > + ret = pctlops->get_group_pins(pctldev, selector, &pins, &num_pins); > + if (ret) { > + dev_err(&pctldev->dev, "cannot configure pin group, error " > + "getting pins\n"); > + return ret; > + } > + > + /* > + * If the pin controller supports handling entire groups we use that > + * capability. > + */ > + if (ops->pin_config_group) { > + ret = ops->pin_config_group(pctldev, selector, > + configs, num_configs); > + > + /* Success, update per-pin state */ > + if (ret == 0) { > + for (i = 0; i < num_pins; i++) { > + struct pin_desc *desc; > + > + desc = pin_desc_get(pctldev, pins[i]); > + if (desc == NULL) { > + dev_err(&pctldev->dev, "error updating state\n"); > + return -EINVAL; > + } > + mutex_lock(&desc->config_lock); > + pinconf_update_states(&desc->config, > + configs, num_configs); > + mutex_unlock(&desc->config_lock); > + } > + } > + /* > + * If the pin controller prefer that a certain group be handled > + * pin-by-pin as well, it returns -EAGAIN. > + */ > + if (ret != -EAGAIN) > + return ret; > + } > + > + /* > + * If the controller cannot handle entire groups, we configure each pin > + * individually. > + */ > + for (i = 0; i < num_pins; i++) { > + ret = pin_config(pctldev, pins[i], configs, num_configs); > + if (ret < 0) > + return ret; > + } > + > + return 0; > +} EXPORT_SYMBOL(pin_config_group); here ? > + > +int pinconf_check_ops(const struct pinconf_ops *ops) > +{ [...] > diff --git a/include/linux/pinctrl/pinconf.h b/include/linux/pinctrl/pinconf.h > new file mode 100644 > index 0000000..45e424b > --- /dev/null > +++ b/include/linux/pinctrl/pinconf.h > @@ -0,0 +1,209 @@ > +/* > + * Interface the pinconfig portions of the pinctrl subsystem > + * > + * Copyright (C) 2011 ST-Ericsson SA > + * Written on behalf of Linaro for ST-Ericsson > + * This interface is used in the core to keep track of pins. > + * > + * Author: Linus Walleij <linus.walleij@xxxxxxxxxx> > + * > + * License terms: GNU General Public License (GPL) version 2 > + */ > +#ifndef __LINUX_PINCTRL_PINCONF_H > +#define __LINUX_PINCTRL_PINCONF_H > + [...] > +enum pin_config_param { > + PIN_CONFIG_BIAS_DISABLE, > + PIN_CONFIG_BIAS_HIGH_IMPEDANCE, > + PIN_CONFIG_BIAS_PULL_UP, > + PIN_CONFIG_BIAS_PULL_DOWN, > + PIN_CONFIG_BIAS_HIGH, > + PIN_CONFIG_BIAS_GROUND, > + PIN_CONFIG_DRIVE_PUSH_PULL, > + PIN_CONFIG_DRIVE_OPEN_DRAIN, > + PIN_CONFIG_DRIVE_OPEN_SOURCE, > + PIN_CONFIG_DRIVE_OFF, > + PIN_CONFIG_INPUT_SCHMITT, > + PIN_CONFIG_INPUT_DEBOUNCE, > + PIN_CONFIG_SLEW_RATE_RISING, > + PIN_CONFIG_SLEW_RATE_FALLING, > + PIN_CONFIG_POWER_SOURCE, > + PIN_CONFIG_LOW_POWER_MODE, > + PIN_CONFIG_WAKEUP, > + PIN_CONFIG_END, > +}; For all Samsung SoC's, PIN_CONFIG_BIAS_PULL_UP, PIN_CONFIG_BIAS_PULL_DOWN and PIN_CONFIG_DRIVE_PUSH_PULL would be sufficient. So the above list of config options is fine for all Samsung platforms. > + > +/** > + * struct pin_config_tuple - a composite parameter and argument config item > + * @param: the parameter to configure > + * @data: argument to the parameter, meaning depend on parameter > + */ [...] > +extern int pin_config(struct pinctrl_dev *pctldev, int pin, > + const struct pin_config_tuple *configs, > + unsigned num_configs); Is the 'pin' number expected to be local to the pin-controller represented by 'pctldev' ? There can be multiple pin-controllers in a system, so it would be easier if 'pin' parameter above is a system-wide pin number, and the caller of 'pin_config' need not know which pin-controller manages 'pin'. Ideally, 'pctldev' should not be included in the parameter list. > +extern int pin_config_group(struct pinctrl_dev *pctldev, const char *pin_group, > + const struct pin_config_tuple *configs, > + unsigned num_configs); > + [...] Thanks for this patch. Regards, Thomas. -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html