Re: [PATCH V2 2/4] mfd: pv88080: MFD core support

[Linus Walleij]

On Thu, Oct 27, 2016 at 3:03 AM, Eric Jeong
<eric.jeong.opensource@xxxxxxxxxxx> wrote:

> +++ b/drivers/mfd/pv88080-i2c.c
> +
> +static const struct of_device_id pv88080_of_match_table[] = {
> +       { .compatible = "pvs,pv88080",    .data = (void *)TYPE_PV88080_AA },
> +       { .compatible = "pvs,pv88080-aa", .data = (void *)TYPE_PV88080_AA },
> +       { .compatible = "pvs,pv88080-ba", .data = (void *)TYPE_PV88080_BA },
> +       { },
> +};
> +MODULE_DEVICE_TABLE(of, pv88080_of_match_table);

Actually you are doing something weird.

The only thing you put in your device tree is this I guess.

That means that the GPIO chip does *not* have a device tree
entry, so you cannot reference the GPIOs there with &phandle
notation.

Please look around a bit to see how other OF-MFDs do it: they
register and populate by struct mfd_cell using the
.of_compatible member so that subdevices get
their own DT nodes, which is necessary for nodes providing
resources such as GPIOs, regulators and clocks, lest you
cannot reference them in your device tree!

Therefore I think all your subdevices should instantiate from
device tree with compatible matching as well, not as
platform devices.

> +struct pv88080_pdata {
> +       int (*init)(struct pv88080 *pv88080);
> +       int irq_base;
> +       int gpio_base;

NAK.

Get irq from the device tree, assign gpio base dynamically.

> +       struct regulator_init_data *regulators[PV88080_MAX_REGULATORS];

I suspect also this should come from the device tree.

Yours,
Linus Walleij