Re: [PATCH v4 03/14] pinctrl-ingenic: add a pinctrl driver for the Ingenic jz47xx SoCs

Linus Walleij
2017-04-07T09:41:44Z

On Sun, Apr 2, 2017 at 10:42 PM, Paul Cercueil <paul@crapouillou.net> wrote:

> This driver handles pin configuration and pin muxing for the
> JZ4740 and JZ4780 SoCs from Ingenic.
>
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
(...)
> + select MFD_CORE
(...)
> +#include <linux/mfd/core.h>

That's unorthodox. Still quite pretty!
I would nee Lee Jones to say something about this, as it is
essentially hijacking MFD into the pinctrl subsystem.

> +static struct mfd_cell ingenic_pinctrl_mfd_cells[] = {
> + {
> + .id = 0,
> + .name = "GPIOA",
> + .of_compatible = "ingenic,gpio-bank-a",
> + },
> + {
> + .id = 1,
> + .name = "GPIOB",
> + .of_compatible = "ingenic,gpio-bank-b",
> + },
> + {
> + .id = 2,
> + .name = "GPIOC",
> + .of_compatible = "ingenic,gpio-bank-c",
> + },
> + {
> + .id = 3,
> + .name = "GPIOD",
> + .of_compatible = "ingenic,gpio-bank-d",
> + },
> + {
> + .id = 4,
> + .name = "GPIOE",
> + .of_compatible = "ingenic,gpio-bank-e",
> + },
> + {
> + .id = 5,
> + .name = "GPIOF",
> + .of_compatible = "ingenic,gpio-bank-f",
> + },
> +};
(...)
> + err = devm_mfd_add_devices(dev, 0, ingenic_pinctrl_mfd_cells,
> + ARRAY_SIZE(ingenic_pinctrl_mfd_cells), NULL, 0, NULL);
> + if (err) {
> + dev_err(dev, "Failed to add MFD devices\n");
> + return err;
> + }

I guess the alternative would be to reimplement the MFD structure.

Did you check the approach to use "simple-mfd" and just let the subnodes
spawn as devices that way? I guess you did and this adds something
necessary.

Yours,
Linus Walleij