On 03/21/2012 03:35 AM, Dong Aisheng wrote: > On Wed, Mar 21, 2012 at 01:44:39AM +0800, Stephen Warren wrote: >> Implement pinctrl_ops dt_node_to_map() and dt_free_map(). These allow >> complete specification of the desired pinmux configuration using device >> tree. >> >> Signed-off-by: Stephen Warren <swar...@wwwdotorg.org> >> --- >> v2: Rebase on of_property_for_each_string() API changes. >> --- > Nice code and a good example to people. > > A small suggestion below: >> +static int add_map_configs(struct pinctrl_map **map, unsigned *num_maps, >> + const char *group, unsigned long *configs, >> + unsigned num_configs) >> +{ >> + unsigned i = *num_maps; >> + unsigned long *dup_configs; >> + int ret; >> + >> + dup_configs = kmemdup(configs, num_configs * sizeof(*dup_configs), >> + GFP_KERNEL); >> + if (!dup_configs) >> + return -ENOMEM; >> + >> + ret = add_map(map, num_maps); >> + if (ret < 0) >> + return ret; >> + >> + (*map)[i].type = PIN_MAP_TYPE_CONFIGS_GROUP; > > It still does not support PIN_MAP_TYPE_CONFIGS_PIN, right?
Yes. This is mainly due to a pinctrl core limitation. The core only supports muxing on groups, so even though the Tegra30 HW supports muxing per pin, the driver must define a group for each pin. Given that, it's simplest just to do all the pin config on those same groups. If/when the pinctrl core supports muxing per pin, we can take advantage of this within the Tegra pinctrl driver without affecting the binding at all. >> + for_each_child_of_node(np_config, np) { >> + ret = of_property_read_string(np, "nvidia,function", &function); >> + if (ret < 0) >> + function = NULL; >> + >> + for (i = 0; i < ARRAY_SIZE(cfg_params); i++) { >> + ret = of_property_read_u32(np, cfg_params[i].property, >> + &val); >> + if (!ret) { >> + config = TEGRA_PINCONF_PACK( >> + cfg_params[i].param, val); >> + ret = add_config(&configs, &num_configs, >> + config); >> + if (ret < 0) >> + goto error; >> + } >> + } >> + >> + of_property_for_each_string(np, "nvidia,pins", prop, group) { > > If we calculate out the strings count and allocate corresponding size array, > we may not > need to keep krealloc the maps and configs array size for each entry. > And this may be a little higher efficient. That's true. However, it'd require the code to loop once to determine how many properties are present and how many entries there are in the pin list. Then, loop again to actually construct the mapping table array. This is all added complexity that doesn't affect correctness. I'd rather get the simple code going first, and then refine it later if there turns out to be a performance issue. _______________________________________________ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss