On Fri, 07 Apr 2017, Linus Walleij wrote:
> On Sun, Apr 2, 2017 at 10:42 PM, Paul Cercueil <paul@xxxxxxxxxxxxxxx> wrote:
>
> > This driver handles pin configuration and pin muxing for the
> > JZ4740 and JZ4780 SoCs from Ingenic.
> >
> > Signed-off-by: Paul Cercueil <paul@xxxxxxxxxxxxxxx>
> (...)
> > + 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.
I'd like to hear what the OP has to say about why this is necessary.
However, as a first glimpse, I'm dead against exporting MFD
functionality to outside of the subsystem.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
