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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CACRpkdZMhtiyk1iyBE-j7rAS6yhSCBsa57-VMnyL=tsUi9wQZg@mail.gmail.com>
Date:   Fri, 28 Oct 2016 14:18:13 +0200
From:   Linus Walleij <linus.walleij@...aro.org>
To:     Eric Jeong <eric.jeong.opensource@...semi.com>
Cc:     LINUX-KERNEL <linux-kernel@...r.kernel.org>,
        Lee Jones <lee.jones@...aro.org>,
        Alexandre Courbot <gnurou@...il.com>,
        DEVICETREE <devicetree@...r.kernel.org>,
        LINUX-GPIO <linux-gpio@...r.kernel.org>,
        Liam Girdwood <lgirdwood@...il.com>,
        Mark Brown <broonie@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Rob Herring <robh+dt@...nel.org>,
        Support Opensource <support.opensource@...semi.com>
Subject: Re: [PATCH V2 2/4] mfd: pv88080: MFD core support

On Thu, Oct 27, 2016 at 3:03 AM, Eric Jeong
<eric.jeong.opensource@...semi.com> 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