On Mon, Dec 07, 2020 at 08:21:03PM +0100, Sergio Paracuellos wrote:
> +static struct pinctrl_desc rt2880_pctrl_desc = {
> + .owner = THIS_MODULE,
> + .name = "rt2880-pinmux",
> + .pctlops = &rt2880_pctrl_ops,
> + .pmxops = &rt2880_pmx_group_ops,
> +};
> +
> +static struct rt2880_pmx_func gpio_func = {
> + .name = "gpio",
> +};
> +
> +static int rt2880_pinmux_index(struct rt2880_priv *p)
This function name is not great. I assumed that it would return the
index.
> +{
> + struct rt2880_pmx_func **f;
Get rid of this "f" variable and use "p->func" instead.
> + struct rt2880_pmx_group *mux = p->groups;
> + int i, j, c = 0;
> +
> + /* count the mux functions */
> + while (mux->name) {
> + p->group_count++;
> + mux++;
> + }
> +
> + /* allocate the group names array needed by the gpio function */
> + p->group_names = devm_kcalloc(p->dev, p->group_count,
> + sizeof(char *), GFP_KERNEL);
> + if (!p->group_names)
> + return -1;
Return proper error codes in this function. s/-1/-ENOMEM/
> +
> + for (i = 0; i < p->group_count; i++) {
> + p->group_names[i] = p->groups[i].name;
> + p->func_count += p->groups[i].func_count;
> + }
> +
> + /* we have a dummy function[0] for gpio */
> + p->func_count++;
> +
> + /* allocate our function and group mapping index buffers */
> + f = p->func = devm_kcalloc(p->dev,
> + p->func_count,
> + sizeof(*p->func),
> + GFP_KERNEL);
> + gpio_func.groups = devm_kcalloc(p->dev, p->group_count, sizeof(int),
> + GFP_KERNEL);
> + if (!f || !gpio_func.groups)
> + return -1;
> +
> + /* add a backpointer to the function so it knows its group */
> + gpio_func.group_count = p->group_count;
> + for (i = 0; i < gpio_func.group_count; i++)
> + gpio_func.groups[i] = i;
> +
> + f[c] = &gpio_func;
> + c++;
> +
> + /* add remaining functions */
> + for (i = 0; i < p->group_count; i++) {
> + for (j = 0; j < p->groups[i].func_count; j++) {
> + f[c] = &p->groups[i].func[j];
> + f[c]->groups = devm_kzalloc(p->dev, sizeof(int),
> + GFP_KERNEL);
Add a NULL check.
> + f[c]->groups[0] = i;
> + f[c]->group_count = 1;
> + c++;
> + }
> + }
> + return 0;
> +}
> +
> +static int rt2880_pinmux_pins(struct rt2880_priv *p)
> +{
> + int i, j;
> +
> + /*
> + * loop over the functions and initialize the pins array.
> + * also work out the highest pin used.
> + */
> + for (i = 0; i < p->func_count; i++) {
> + int pin;
> +
> + if (!p->func[i]->pin_count)
> + continue;
> +
> + p->func[i]->pins = devm_kcalloc(p->dev,
> + p->func[i]->pin_count,
> + sizeof(int),
> + GFP_KERNEL);
This can fit on two lines.
p->func[i]->pins = devm_kcalloc(p->dev, p->func[i]->pin_count,
sizeof(int), GFP_KERNEL);
> + for (j = 0; j < p->func[i]->pin_count; j++)
> + p->func[i]->pins[j] = p->func[i]->pin_first + j;
> +
> + pin = p->func[i]->pin_first + p->func[i]->pin_count;
> + if (pin > p->max_pins)
> + p->max_pins = pin;
> + }
> +
> + /* the buffer that tells us which pins are gpio */
> + p->gpio = devm_kcalloc(p->dev, p->max_pins, sizeof(u8), GFP_KERNEL);
> + /* the pads needed to tell pinctrl about our pins */
> + p->pads = devm_kcalloc(p->dev, p->max_pins,
> + sizeof(struct pinctrl_pin_desc), GFP_KERNEL);
> + if (!p->pads || !p->gpio) {
> + dev_err(p->dev, "Failed to allocate gpio data\n");
Delete this error message. #checkpatch.pl
> + return -ENOMEM;
> + }
> +
> + memset(p->gpio, 1, sizeof(u8) * p->max_pins);
> + for (i = 0; i < p->func_count; i++) {
> + if (!p->func[i]->pin_count)
> + continue;
> +
> + for (j = 0; j < p->func[i]->pin_count; j++)
> + p->gpio[p->func[i]->pins[j]] = 0;
> + }
> +
> + /* pin 0 is always a gpio */
> + p->gpio[0] = 1;
> +
> + /* set the pads */
> + for (i = 0; i < p->max_pins; i++) {
> + /* strlen("ioXY") + 1 = 5 */
> + char *name = devm_kzalloc(p->dev, 5, GFP_KERNEL);
> +
char *name;
name = kasprintf(GFP_KERNEL, "io%d", i);
> + if (!name)
> + return -ENOMEM;
> + snprintf(name, 5, "io%d", i);
> + p->pads[i].number = i;
> + p->pads[i].name = name;
> + }
> + p->desc->pins = p->pads;
> + p->desc->npins = p->max_pins;
> +
> + return 0;
> +}
> +
> +static int rt2880_pinmux_probe(struct platform_device *pdev)
> +{
> + struct rt2880_priv *p;
> + struct pinctrl_dev *dev;
> +
> + if (!rt2880_pinmux_data)
> + return -ENOTSUPP;
> +
> + /* setup the private data */
> + p = devm_kzalloc(&pdev->dev, sizeof(struct rt2880_priv), GFP_KERNEL);
> + if (!p)
> + return -ENOMEM;
> +
> + p->dev = &pdev->dev;
> + p->desc = &rt2880_pctrl_desc;
> + p->groups = rt2880_pinmux_data;
> + platform_set_drvdata(pdev, p);
> +
> + /* init the device */
> + if (rt2880_pinmux_index(p)) {
> + dev_err(&pdev->dev, "failed to load index\n");
> + return -EINVAL;
Preserve the error code:
err = rt2880_pinmux_index(p);
if (err) {
dev_err(&pdev->dev, "failed to load index\n");
return err;
}
> + }
> + if (rt2880_pinmux_pins(p)) {
> + dev_err(&pdev->dev, "failed to load pins\n");
> + return -EINVAL;
Same.
> + }
> + dev = pinctrl_register(p->desc, &pdev->dev, p);
> + if (IS_ERR(dev))
> + return PTR_ERR(dev);
> +
> + return 0;
> +}
regards,
dan carpenter
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
