On Sat, Mar 09, 2013 at 07:02:05PM +0000, David Woodhouse wrote:
> On Sat, 2013-03-09 at 09:49 +0100, Sebastian Hesselbarth wrote:
> > I don't have a strong opinion on that, but I prefer not to have the list
> > statically in the SoC specific drivers. I think counting the number of
> > unique functions for each SoC specific driver once and verify the above
> > heuristic (fewer unique functions than pins) is still valid. Then drop
> > the krealloc and leave the array the way it is allocated on devm_kzalloc.
>
> Yeah. If you stick a check in the loop and make it warn if it *would*
> have run over the end of the array, that sounds like it ought to be
> fine. Something like this, perhaps? Still untested but otherwise
> Signed-off-by: David Woodhouse <david.woodho...@intel.com>
>
> diff --git a/drivers/pinctrl/mvebu/pinctrl-mvebu.c
> b/drivers/pinctrl/mvebu/pinctrl-mvebu.c
> index c689c04..55d55d5 100644
> --- a/drivers/pinctrl/mvebu/pinctrl-mvebu.c
> +++ b/drivers/pinctrl/mvebu/pinctrl-mvebu.c
> @@ -478,7 +478,8 @@ static struct pinctrl_ops mvebu_pinctrl_ops = {
> .dt_free_map = mvebu_pinctrl_dt_free_map,
> };
>
> -static int _add_function(struct mvebu_pinctrl_function *funcs, const char
> *name)
> +static int _add_function(struct mvebu_pinctrl_function *funcs, int nr_funcs,
> + const char *name)
> {
> while (funcs->num_groups) {
> /* function already there */
> @@ -487,7 +488,11 @@ static int _add_function(struct mvebu_pinctrl_function
> *funcs, const char *name)
> return -EEXIST;
> }
> funcs++;
> + nr_funcs--;
> }
> + if (!nr_funcs)
shouldn't this be:
if (nr_funcs <= 0)
thx,
Jason.
> + return -EOVERFLOW;
> +
> funcs->name = name;
> funcs->num_groups = 1;
> return 0;
> @@ -501,7 +506,7 @@ static int mvebu_pinctrl_build_functions(struct
> platform_device *pdev,
> int n, s;
>
> /* we allocate functions for number of pins and hope
> - * there are less unique functions than pins available */
> + * there are fewer unique functions than pins available */
> funcs = devm_kzalloc(&pdev->dev, pctl->desc.npins *
> sizeof(struct mvebu_pinctrl_function), GFP_KERNEL);
> if (!funcs)
> @@ -510,26 +515,27 @@ static int mvebu_pinctrl_build_functions(struct
> platform_device *pdev,
> for (n = 0; n < pctl->num_groups; n++) {
> struct mvebu_pinctrl_group *grp = &pctl->groups[n];
> for (s = 0; s < grp->num_settings; s++) {
> + int ret;
> +
> /* skip unsupported settings on this variant */
> if (pctl->variant &&
> !(pctl->variant & grp->settings[s].variant))
> continue;
>
> /* check for unique functions and count groups */
> - if (_add_function(funcs, grp->settings[s].name))
> + ret = _add_function(funcs, pctl->desc.npins,
> + grp->settings[s].name);
> + if (ret == -EOVERFLOW)
> + dev_err(&pdev->dev,
> + "More functions than pins(%d)\n",
> + pctl->desc.npins);
> + if (ret)
> continue;
>
> num++;
> }
> }
>
> - /* with the number of unique functions and it's groups known,
> - reallocate functions and assign group names */
> - funcs = krealloc(funcs, num * sizeof(struct mvebu_pinctrl_function),
> - GFP_KERNEL);
> - if (!funcs)
> - return -ENOMEM;
> -
> pctl->num_functions = num;
> pctl->functions = funcs;
>
>
>
> --
> dwmw2
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
