[PATCH 3/6 v2] mfd: stmpe: prope properly from the device tree

[PATCH 3/6 v2] mfd: stmpe: prope properly from the device tree

[Linus Walleij]

The current STMPE I2C probing code does not really match the
compatible strings - it matches node names happening to give
the right device name. Instead, let's introduce some real
compatible matching, more complex, more accurate. Make the
driver depend on OF since all platforms using it are DT-only.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- Make driver depend on OF and remove some ifdeffery that way.
- Squash OF match table to single lines.
- Merge of-specific probe function into probe().
- Print a small nag if DT is using the node name trick to make
  the driver probe.
---
 drivers/mfd/Kconfig     |  1 +
 drivers/mfd/stmpe-i2c.c | 31 ++++++++++++++++++++++++++++++-
 2 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 33834120d057..29be025d5835 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -675,6 +675,7 @@ config MFD_DB8500_PRCMU
 config MFD_STMPE
 	bool "STMicroelectronics STMPE"
 	depends on (I2C=y || SPI_MASTER=y)
+	depends on OF
 	select MFD_CORE
 	help
 	  Support for the STMPE family of I/O Expanders from
diff --git a/drivers/mfd/stmpe-i2c.c b/drivers/mfd/stmpe-i2c.c
index 0da02e11d58e..da15dda04df9 100644
--- a/drivers/mfd/stmpe-i2c.c
+++ b/drivers/mfd/stmpe-i2c.c
@@ -14,6 +14,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/types.h>
+#include <linux/of_device.h>
 #include "stmpe.h"
 
 static int i2c_reg_read(struct stmpe *stmpe, u8 reg)
@@ -52,15 +53,42 @@ static struct stmpe_client_info i2c_ci = {
 	.write_block = i2c_block_write,
 };
 
+static const struct of_device_id stmpe_of_match[] = {
+	{ .compatible = "st,stmpe610", .data = (void *)STMPE610, },
+	{ .compatible = "st,stmpe801", .data = (void *)STMPE801, },
+	{ .compatible = "st,stmpe811", .data = (void *)STMPE811, },
+	{ .compatible = "st,stmpe1601", .data = (void *)STMPE1601, },
+	{ .compatible = "st,stmpe1801", .data = (void *)STMPE1801, },
+	{ .compatible = "st,stmpe2401", .data = (void *)STMPE2401, },
+	{ .compatible = "st,stmpe2403", .data = (void *)STMPE2403, },
+	{},
+};
+MODULE_DEVICE_TABLE(of, stmpe_of_match);
+
 static int
 stmpe_i2c_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
 {
+	int partnum;
+	const struct of_device_id *of_id;
+
 	i2c_ci.data = (void *)id;
 	i2c_ci.irq = i2c->irq;
 	i2c_ci.client = i2c;
 	i2c_ci.dev = &i2c->dev;
 
-	return stmpe_probe(&i2c_ci, id->driver_data);
+	of_id = of_match_device(stmpe_of_match, &i2c->dev);
+	if (!of_id) {
+		/*
+		 * This happens when the I2C ID matches the node name
+		 * but no real compatible string has been given.
+		 */
+		dev_info(&i2c->dev, "matching on node name, compatible is preferred\n");
+		partnum = id->driver_data;
+		return -ENODEV;
+	} else
+		partnum = (int)of_id->data;
+
+	return stmpe_probe(&i2c_ci, partnum);
 }
 
 static int stmpe_i2c_remove(struct i2c_client *i2c)
@@ -89,6 +117,7 @@ static struct i2c_driver stmpe_i2c_driver = {
 #ifdef CONFIG_PM
 		.pm = &stmpe_dev_pm_ops,
 #endif
+		.of_match_table = stmpe_of_match,
 	},
 	.probe		= stmpe_i2c_probe,
 	.remove		= stmpe_i2c_remove,
-- 
1.9.0

Re: [PATCH 3/6 v2] mfd: stmpe: prope properly from the device tree

[Lee Jones]

Can you fix the $SUBJECT line please?

> The current STMPE I2C probing code does not really match the
> compatible strings - it matches node names happening to give
> the right device name. Instead, let's introduce some real
> compatible matching, more complex, more accurate. Make the
> driver depend on OF since all platforms using it are DT-only.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v1->v2:
> - Make driver depend on OF and remove some ifdeffery that way.
> - Squash OF match table to single lines.
> - Merge of-specific probe function into probe().
> - Print a small nag if DT is using the node name trick to make
>   the driver probe.
> ---
>  drivers/mfd/Kconfig     |  1 +
>  drivers/mfd/stmpe-i2c.c | 31 ++++++++++++++++++++++++++++++-
>  2 files changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 33834120d057..29be025d5835 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -675,6 +675,7 @@ config MFD_DB8500_PRCMU
>  config MFD_STMPE
>  	bool "STMicroelectronics STMPE"
>  	depends on (I2C=y || SPI_MASTER=y)
> +	depends on OF
>  	select MFD_CORE
>  	help
>  	  Support for the STMPE family of I/O Expanders from
> diff --git a/drivers/mfd/stmpe-i2c.c b/drivers/mfd/stmpe-i2c.c
> index 0da02e11d58e..da15dda04df9 100644
> --- a/drivers/mfd/stmpe-i2c.c
> +++ b/drivers/mfd/stmpe-i2c.c
> @@ -14,6 +14,7 @@
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/types.h>
> +#include <linux/of_device.h>
>  #include "stmpe.h"
>  
>  static int i2c_reg_read(struct stmpe *stmpe, u8 reg)
> @@ -52,15 +53,42 @@ static struct stmpe_client_info i2c_ci = {
>  	.write_block = i2c_block_write,
>  };
>  
> +static const struct of_device_id stmpe_of_match[] = {
> +	{ .compatible = "st,stmpe610", .data = (void *)STMPE610, },
> +	{ .compatible = "st,stmpe801", .data = (void *)STMPE801, },
> +	{ .compatible = "st,stmpe811", .data = (void *)STMPE811, },
> +	{ .compatible = "st,stmpe1601", .data = (void *)STMPE1601, },
> +	{ .compatible = "st,stmpe1801", .data = (void *)STMPE1801, },
> +	{ .compatible = "st,stmpe2401", .data = (void *)STMPE2401, },
> +	{ .compatible = "st,stmpe2403", .data = (void *)STMPE2403, },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, stmpe_of_match);
> +
>  static int
>  stmpe_i2c_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
>  {
> +	int partnum;
> +	const struct of_device_id *of_id;
> +
>  	i2c_ci.data = (void *)id;
>  	i2c_ci.irq = i2c->irq;
>  	i2c_ci.client = i2c;
>  	i2c_ci.dev = &i2c->dev;
>  
> -	return stmpe_probe(&i2c_ci, id->driver_data);
> +	of_id = of_match_device(stmpe_of_match, &i2c->dev);
> +	if (!of_id) {
> +		/*
> +		 * This happens when the I2C ID matches the node name
> +		 * but no real compatible string has been given.
> +		 */
> +		dev_info(&i2c->dev, "matching on node name, compatible is preferred\n");
> +		partnum = id->driver_data;
> +		return -ENODEV;

This changes the semantics since v1.

I don't think you want to return error at all.

> +	} else
> +		partnum = (int)of_id->data;
> +
> +	return stmpe_probe(&i2c_ci, partnum);
>  }
>  
>  static int stmpe_i2c_remove(struct i2c_client *i2c)
> @@ -89,6 +117,7 @@ static struct i2c_driver stmpe_i2c_driver = {
>  #ifdef CONFIG_PM
>  		.pm = &stmpe_dev_pm_ops,
>  #endif
> +		.of_match_table = stmpe_of_match,
>  	},
>  	.probe		= stmpe_i2c_probe,
>  	.remove		= stmpe_i2c_remove,

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog