Re: [PATCH 16/20] pinctrl: Refactor struct pinctrl handling in core.cvs pinmux.c

From: Dong Aisheng
Date: Fri Feb 24 2012 - 11:55:36 EST


On Mon, Feb 20, 2012 at 2:45 PM, Stephen Warren <swarren@xxxxxxxxxx> wrote:
> This change separates two aspects of struct pinctrl:
>
> a) The data representation of the parsed mapping table, into:
>
>   1) The top-level struct pinctrl object, a single entity returned
>      by pinctrl_get().
>
>   2) The parsed version of each mapping table entry, struct
>      pinctrl_setting, of which there is one per mapping table entry.
>
> b) The code that handles this; the code for (1) above is in core.c, and
>   the code to parse/execute each entry in (2) above is in pinmux.c, while
>   the iteration over multiple settings is lifted to core.c.
>
> This will allow the following future changes:
>
> 1) pinctrl_get() API rework, so that struct pinctrl represents all states
>   for the device, and the device can select between them without calling
>   put()/get() again.
>
> 2) To support that, a struct pinctrl_state object will be inserted into
>   the data model between the struct pinctrl and struct pinctrl_setting.
>
> 3) The mapping table will be extended to allow specification of pin config
>   settings too. To support this, struct pinctrl_setting will be enhanced
>   to store either mux settings or config settings, and functions will be
>   added to pinconf.c to parse/execute pin configuration settings.
>
> Signed-off-by: Stephen Warren <swarren@xxxxxxxxxx>

Looks good to me.
Please also with my tag.
Acked-by: Dong Aisheng <dong.aisheng@xxxxxxxxxx>

BTW, one minor thing, see below:
...
> -/**
> - * pinmux_search_function() - check pin control driver for a certain function
> - * @pctldev: device to check for function and position
> - * @map: function map containing the function and position to look for
> - * @func_selector: returns the applicable function selector if found
> - * @group_selector: returns the applicable group selector if found
> - *
> - * This will search the pinmux driver for an applicable
> - * function with a specific pin group, returns 0 if these can be mapped
> - * negative otherwise
> - */
> -static int pinmux_search_function(struct pinctrl_dev *pctldev,
> -                                 struct pinctrl_map const *map,
> -                                 unsigned *func_selector,
> -                                 unsigned *group_selector)
> +int pinmux_func_name_to_selector(struct pinctrl_dev *pctldev,
> +                                const char *function)
Static?

Regards
Dong Aisheng
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/