Re: [PATCH 3/5] pinctrl: add generic board-level pinctrl driver using mux framework

[Daniel Baluta]

Hi Frank,

Few comments inline:


> --- /dev/null
> +++ b/drivers/pinctrl/pinctrl-generic-mux.c
> @@ -0,0 +1,222 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Generic Pin Control Driver for Board-Level Mux Chips
> + * Copyright (C) 2026
>
Copyright 2026 NXP

<snip>

> +struct mux_pin_function {
> +	struct mux_state *mux_state;
> +};
> +
> +struct mux_pinctrl {
> +	struct device *dev;
> +	struct pinctrl_dev *pctl;
> +
> +	/* mutex protecting the lists */

what lists? 


> +static int
> +mux_pinmux_dt_node_to_map(struct pinctrl_dev *pctldev,
> +			  struct device_node *np_config,
> +			  struct pinctrl_map **map, unsigned int *num_maps)
> +{
> +	struct mux_pinctrl *mpctl = pinctrl_dev_get_drvdata(pctldev);
> +	struct mux_pin_function *function;
> +	struct device *dev = mpctl->dev;
> +	const char **pgnames;
> +	int selector;
> +	int group;
> +	int ret;
> +
> +	*map = devm_kcalloc(dev, 1, sizeof(**map), GFP_KERNEL);
> +	if (!map)
> +		return -ENOMEM;
>
if (!*map) ?

> +
> +	*num_maps = 0;
> +
> +	function = devm_kzalloc(dev, sizeof(*function), GFP_KERNEL);
> +	if (!function) {
> +		ret = -ENOMEM;
> +		goto err_func;
Do you really need this goto? Previously allocated memory is dev managed .
> +	}
> +
> +	pgnames = devm_kzalloc(dev, sizeof(*pgnames), GFP_KERNEL);
> +	if (!pgnames) {
> +		ret = -ENOMEM;
> +		goto err_pgnames;
>
Same here.

<snip>

> +err_mux_state_get:
> +	pinctrl_generic_remove_group(mpctl->pctl, group);
> +err_add_group:
> +	pinmux_generic_remove_function(mpctl->pctl, selector);
> +err_add_func:
> +	devm_kfree(dev, pgnames);
> +err_pgnames:
> +	devm_kfree(dev, function);
> +err_func:
> +	devm_kfree(dev, *map);
> +
> +	return ret;
> +}