Hi James, I want to route this patch by Laurent Pinchart, as he's also adding device tree support for a SoC using generic pinconf. Creating new bindings for every pin controller defining the same thing and implementing the same OF parsers is not sane. We need to: - Define generic pinconf bindings - Implement a common parser for these in drivers/pinctrl/pinconf-generic.c I'll merge your patches to <linux/pinctrl/pinctrl-generic.h> as a baseline. On Fri, May 24, 2013 at 6:21 PM, James Hogan <james.hogan@xxxxxxxxxx> wrote: (...) > +Optional subnode-properties: > +- function: A string containing the name of the function to mux to the pin or > + group. Valid values for function names are listed below, including which > + pingroups can be muxed to them. > +- tristate: Flag, put pin into high impedance state. > +- pull-up: Flag, pull pin high. > +- pull-down: Flag, pull pin low. > +- bus-hold: Flag, weak latch last value on tristate bus. > +- schmitt: Integer, enable or disable Schmitt trigger mode for the pins. > + 0: no hysteresis > + 1: schmitt trigger > +- slew-rate: Integer, control slew rate of pins. > + 0: slow (half frequency) > + 1: fast > +- drive-strength: Integer, control drive strength of pins in mA. > + 2: 2mA > + 4: 4mA > + 8: 8mA > + 12: 12mA So if you want to use these names they shall be made generic. Else you have to prefix everything with "tz1090,<property>" > +/* Describes pinconf properties/flags available from device tree */ > +static const struct cfg_param { > + const char *property; > + enum pin_config_param param; > + bool flag; > +} cfg_params[] = { > + {"tristate", PIN_CONFIG_BIAS_HIGH_IMPEDANCE, true}, > + {"pull-up", PIN_CONFIG_BIAS_PULL_UP, true}, > + {"pull-down", PIN_CONFIG_BIAS_PULL_DOWN, true}, > + {"bus-hold", PIN_CONFIG_BIAS_BUS_HOLD, true}, > + {"schmitt", PIN_CONFIG_INPUT_SCHMITT_ENABLE, true}, > + {"slew-rate", PIN_CONFIG_SLEW_RATE, false}, > + {"drive-strength", PIN_CONFIG_DRIVE_STRENGTH, false}, > +}; > + > +int tz1090_pinctrl_dt_subnode_to_map(struct device *dev, > + struct device_node *np, > + struct pinctrl_map **map, > + unsigned *reserved_maps, > + unsigned *num_maps) > +{ > + int ret, i; > + const char *function; > + u32 val; > + unsigned long config; > + unsigned long *configs = NULL; > + unsigned num_configs = 0; > + unsigned reserve; > + struct property *prop; > + const char *group; > + > + ret = of_property_read_string(np, "function", &function); > + if (ret < 0) { > + /* EINVAL=missing, which is fine since it's optional */ > + if (ret != -EINVAL) > + dev_err(dev, "could not parse property function\n"); > + function = NULL; > + } > + > + for (i = 0; i < ARRAY_SIZE(cfg_params); i++) { > + ret = of_property_read_u32(np, cfg_params[i].property, &val); > + /* flags don't have to have a value */ > + if (ret == -EOVERFLOW && cfg_params[i].flag) { > + val = 1; > + ret = 0; > + } > + if (!ret) { > + config = pinconf_to_config_packed(cfg_params[i].param, > + val); > + ret = add_config(dev, &configs, &num_configs, config); > + if (ret < 0) > + goto exit; > + /* EINVAL=missing, which is fine since it's optional */ > + } else if (ret != -EINVAL) { > + dev_err(dev, "could not parse property %s (%d)\n", > + cfg_params[i].property, ret); > + } > + } > + > + reserve = 0; > + if (function != NULL) > + reserve++; > + if (num_configs) > + reserve++; > + ret = of_property_count_strings(np, "pins"); > + if (ret < 0) { > + dev_err(dev, "could not parse property pins\n"); > + goto exit; > + } > + reserve *= ret; > + > + ret = reserve_map(dev, map, reserved_maps, num_maps, reserve); > + if (ret < 0) > + goto exit; > + > + of_property_for_each_string(np, "pins", prop, group) { > + if (function) { > + ret = add_map_mux(map, reserved_maps, num_maps, > + group, function); > + if (ret < 0) > + goto exit; > + } > + > + if (num_configs) { > + ret = add_map_configs(dev, map, reserved_maps, > + num_maps, group, configs, > + num_configs); > + if (ret < 0) > + goto exit; > + } > + } > + > + ret = 0; > + > +exit: > + kfree(configs); > + return ret; > +} This is looking good. Can you split out the pin config mapping in some way and put that into pinconf-generic.c? Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html