On 12/09/2016 12:49, Lee Jones wrote:

On Mon, 12 Sep 2016, Maxime Ripard wrote:

quoted

On Mon, Sep 12, 2016 at 10:59:23AM +0100, Lee Jones wrote:

quoted

quoted

quoted

quoted

+static const struct of_device_id sun4i_gpadc_mfd_of_match[] = {
+	{
+		.compatible = "allwinner,sun4i-a10-ts",
+		.data = &sun4i_gpadc_mfd_cells,
+	}, {
+		.compatible = "allwinner,sun5i-a13-ts",
+		.data = &sun5i_gpadc_mfd_cells,
+	}, {
+		.compatible = "allwinner,sun6i-a31-ts",
+		.data = &sun6i_gpadc_mfd_cells,
+	}, { /* sentinel */ }
+};

Don't mix OF and MFD functionality.

Why don't you create a node for "iio_hwmon" and have
platform_of_populate() do your bidding?

We are using a stable binding which we cannot modify. This means, the DT
in its current state can only be modified to add features, which is not
the case of this driver (it is a rewriting of an existing driver which
uses the rtp node).

Then use .data = <defined model ID> and set up a switch() in .probe().

Uh? Why? It just adds a non-standard indirection, while using
of_match_device is very standard, and used extensively in Linux.

You still use of_match_device() to obtain the ID.

The "don't mix DT with the MFD API" is there to prevent some of the
nasty hacks I've seen previously.  This particular example doesn't
seem so bad, but it's a gateway to ridiculous hackery!

How am I supposed to get the .data without of_match_node then?
What's more hackish in using .data field for specific data for each
compatible than in using a random ID in .data and switching on it? The
result is exactly the same, the switching case being more verbose and
adding complexity to something that can be done in a straightforward manner.

Quentin