linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/4] mfd: max77686: Remove unneeded non-OF code in driver
@ 2017-01-13 13:34 Javier Martinez Canillas
  2017-01-13 13:34 ` [PATCH v2 1/4] mfd: max77686: Don't attempt to get i2c_device_id .data Javier Martinez Canillas
                   ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: Javier Martinez Canillas @ 2017-01-13 13:34 UTC (permalink / raw)
  To: linux-kernel
  Cc: Laxman Dewangan, Javier Martinez Canillas, Krzysztof Kozlowski,
	Chanwoo Choi, Bartlomiej Zolnierkiewicz, Lee Jones

Hello Lee,

This series contains cleanups for the max77686 PMIC MFD driver.
The driver is only used in DT platforms and so all the code
related with the i2c_device_id table can be removed.

Best regards,
Javier

Changes in v2:
- Add Laxman's Acked-by tag to patch 1/4.
- Add Krzysztof's Reviewed-by and Tested-by tags to patch 1/4.
- Add Laxman's Acked-by tag to patch 2/4.
- Mention in commit message that an unneeded check for match is removed.
- Add Laxman's Acked-by tag to patch 3/4.
- Add Krzysztof's Reviewed-by and Tested-by tags to patch 3/4.
- Add Laxman's Acked-by tag to patch 4/4.
- Add Krzysztof's Reviewed-by and Tested-by tags to patch 4/4.

Javier Martinez Canillas (4):
  mfd: max77686: Don't attempt to get i2c_device_id .data
  mfd: max77686: Use of_device_get_match_data() helper
  mfd: max77686: Use the struct i2c_driver .probe_new instead of .probe
  mfd: max77686: Remove I2C device ID table

 drivers/mfd/max77686.c | 25 ++++---------------------
 1 file changed, 4 insertions(+), 21 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v2 1/4] mfd: max77686: Don't attempt to get i2c_device_id .data
  2017-01-13 13:34 [PATCH v2 0/4] mfd: max77686: Remove unneeded non-OF code in driver Javier Martinez Canillas
@ 2017-01-13 13:34 ` Javier Martinez Canillas
  2017-01-16  6:30   ` Chanwoo Choi
  2017-01-23 11:35   ` Lee Jones
  2017-01-13 13:34 ` [PATCH v2 2/4] mfd: max77686: Use of_device_get_match_data() helper Javier Martinez Canillas
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 16+ messages in thread
From: Javier Martinez Canillas @ 2017-01-13 13:34 UTC (permalink / raw)
  To: linux-kernel
  Cc: Laxman Dewangan, Javier Martinez Canillas, Krzysztof Kozlowski,
	Chanwoo Choi, Bartlomiej Zolnierkiewicz, Lee Jones

The driver is only used in platforms that have DT support so always the
I2C device .data will be get from the matched OF node and never will be
from the I2C device ID table.

Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
Acked-by: Laxman Dewangan <ldewangan@nvidia.com>
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
Tested-by: Krzysztof Kozlowski <krzk@kernel.org>

---

Changes in v2:
- Add Laxman's Acked-by tag to patch 1/4.
- Add Krzysztof's Reviewed-by and Tested-by tags to patch 1/4.

 drivers/mfd/max77686.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/mfd/max77686.c b/drivers/mfd/max77686.c
index 7b68ed72e9cb..ddae3bf3e46c 100644
--- a/drivers/mfd/max77686.c
+++ b/drivers/mfd/max77686.c
@@ -188,14 +188,11 @@ static int max77686_i2c_probe(struct i2c_client *i2c,
 	if (!max77686)
 		return -ENOMEM;
 
-	if (i2c->dev.of_node) {
-		match = of_match_node(max77686_pmic_dt_match, i2c->dev.of_node);
-		if (!match)
-			return -EINVAL;
-
-		max77686->type = (unsigned long)match->data;
-	} else
-		max77686->type = id->driver_data;
+	match = of_match_node(max77686_pmic_dt_match, i2c->dev.of_node);
+	if (!match)
+		return -EINVAL;
+
+	max77686->type = (unsigned long)match->data;
 
 	i2c_set_clientdata(i2c, max77686);
 	max77686->dev = &i2c->dev;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 2/4] mfd: max77686: Use of_device_get_match_data() helper
  2017-01-13 13:34 [PATCH v2 0/4] mfd: max77686: Remove unneeded non-OF code in driver Javier Martinez Canillas
  2017-01-13 13:34 ` [PATCH v2 1/4] mfd: max77686: Don't attempt to get i2c_device_id .data Javier Martinez Canillas
@ 2017-01-13 13:34 ` Javier Martinez Canillas
  2017-01-13 14:09   ` Krzysztof Kozlowski
                     ` (2 more replies)
  2017-01-13 13:34 ` [PATCH v2 3/4] mfd: max77686: Use the struct i2c_driver .probe_new instead of .probe Javier Martinez Canillas
  2017-01-13 13:34 ` [PATCH v2 4/4] mfd: max77686: Remove I2C device ID table Javier Martinez Canillas
  3 siblings, 3 replies; 16+ messages in thread
From: Javier Martinez Canillas @ 2017-01-13 13:34 UTC (permalink / raw)
  To: linux-kernel
  Cc: Laxman Dewangan, Javier Martinez Canillas, Krzysztof Kozlowski,
	Chanwoo Choi, Bartlomiej Zolnierkiewicz, Lee Jones

Use the generic helper to get the matched of_device_id .data, instead of
open coding it.

The driver was checking if matching the OF node with the driver's OF table
was failing, but this doesn't make too much sense since this can't happen
in practice. The fact the probe function was called, means OF registered a
device with a valid compatible string so a of_device_get_match_data() call
will always succeed. So just remove this unneeded check.

Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
Acked-by: Laxman Dewangan <ldewangan@nvidia.com>

---

Changes in v2:
- Add Laxman's Acked-by tag to patch 2/4.
- Mention in commit message that an unneeded check for match is removed.

 drivers/mfd/max77686.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/mfd/max77686.c b/drivers/mfd/max77686.c
index ddae3bf3e46c..33dd09493605 100644
--- a/drivers/mfd/max77686.c
+++ b/drivers/mfd/max77686.c
@@ -34,6 +34,7 @@
 #include <linux/mfd/max77686-private.h>
 #include <linux/err.h>
 #include <linux/of.h>
+#include <linux/of_device.h>
 
 static const struct mfd_cell max77686_devs[] = {
 	{ .name = "max77686-pmic", },
@@ -175,7 +176,6 @@ static int max77686_i2c_probe(struct i2c_client *i2c,
 			      const struct i2c_device_id *id)
 {
 	struct max77686_dev *max77686 = NULL;
-	const struct of_device_id *match;
 	unsigned int data;
 	int ret = 0;
 	const struct regmap_config *config;
@@ -188,13 +188,8 @@ static int max77686_i2c_probe(struct i2c_client *i2c,
 	if (!max77686)
 		return -ENOMEM;
 
-	match = of_match_node(max77686_pmic_dt_match, i2c->dev.of_node);
-	if (!match)
-		return -EINVAL;
-
-	max77686->type = (unsigned long)match->data;
-
 	i2c_set_clientdata(i2c, max77686);
+	max77686->type = (unsigned long)of_device_get_match_data(&i2c->dev);
 	max77686->dev = &i2c->dev;
 	max77686->i2c = i2c;
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 3/4] mfd: max77686: Use the struct i2c_driver .probe_new instead of .probe
  2017-01-13 13:34 [PATCH v2 0/4] mfd: max77686: Remove unneeded non-OF code in driver Javier Martinez Canillas
  2017-01-13 13:34 ` [PATCH v2 1/4] mfd: max77686: Don't attempt to get i2c_device_id .data Javier Martinez Canillas
  2017-01-13 13:34 ` [PATCH v2 2/4] mfd: max77686: Use of_device_get_match_data() helper Javier Martinez Canillas
@ 2017-01-13 13:34 ` Javier Martinez Canillas
  2017-01-16  6:37   ` Chanwoo Choi
  2017-01-23 11:36   ` Lee Jones
  2017-01-13 13:34 ` [PATCH v2 4/4] mfd: max77686: Remove I2C device ID table Javier Martinez Canillas
  3 siblings, 2 replies; 16+ messages in thread
From: Javier Martinez Canillas @ 2017-01-13 13:34 UTC (permalink / raw)
  To: linux-kernel
  Cc: Laxman Dewangan, Javier Martinez Canillas, Krzysztof Kozlowski,
	Chanwoo Choi, Bartlomiej Zolnierkiewicz, Lee Jones

If a driver is only used in DT platforms, there's no need to get the
i2c_device_id as an argument of the probe function. Since this data
can be get from the matching of_device_id.

There's a temporary .probe_new field in struct i2c_driver that can be
used as probe callback for the case when i2c_device_id won't be used.

Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
Acked-by: Laxman Dewangan <ldewangan@nvidia.com>
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
Tested-by: Krzysztof Kozlowski <krzk@kernel.org>

---

Changes in v2:
- Add Laxman's Acked-by tag to patch 3/4.
- Add Krzysztof's Reviewed-by and Tested-by tags to patch 3/4.

 drivers/mfd/max77686.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/mfd/max77686.c b/drivers/mfd/max77686.c
index 33dd09493605..896c1bf85acc 100644
--- a/drivers/mfd/max77686.c
+++ b/drivers/mfd/max77686.c
@@ -172,8 +172,7 @@ static const struct of_device_id max77686_pmic_dt_match[] = {
 };
 MODULE_DEVICE_TABLE(of, max77686_pmic_dt_match);
 
-static int max77686_i2c_probe(struct i2c_client *i2c,
-			      const struct i2c_device_id *id)
+static int max77686_i2c_probe(struct i2c_client *i2c)
 {
 	struct max77686_dev *max77686 = NULL;
 	unsigned int data;
@@ -294,7 +293,7 @@ static struct i2c_driver max77686_i2c_driver = {
 		   .pm = &max77686_pm,
 		   .of_match_table = of_match_ptr(max77686_pmic_dt_match),
 	},
-	.probe = max77686_i2c_probe,
+	.probe_new = max77686_i2c_probe,
 	.id_table = max77686_i2c_id,
 };
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 4/4] mfd: max77686: Remove I2C device ID table
  2017-01-13 13:34 [PATCH v2 0/4] mfd: max77686: Remove unneeded non-OF code in driver Javier Martinez Canillas
                   ` (2 preceding siblings ...)
  2017-01-13 13:34 ` [PATCH v2 3/4] mfd: max77686: Use the struct i2c_driver .probe_new instead of .probe Javier Martinez Canillas
@ 2017-01-13 13:34 ` Javier Martinez Canillas
  2017-01-16  6:46   ` Chanwoo Choi
  2017-01-23 11:40   ` Lee Jones
  3 siblings, 2 replies; 16+ messages in thread
From: Javier Martinez Canillas @ 2017-01-13 13:34 UTC (permalink / raw)
  To: linux-kernel
  Cc: Laxman Dewangan, Javier Martinez Canillas, Krzysztof Kozlowski,
	Chanwoo Choi, Bartlomiej Zolnierkiewicz, Lee Jones

The driver is only used in DT platforms so there's no need to
have an i2c_device_id table.

Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
Acked-by: Laxman Dewangan <ldewangan@nvidia.com>
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
Tested-by: Krzysztof Kozlowski <krzk@kernel.org>

---

Changes in v2:
- Add Laxman's Acked-by tag to patch 4/4.
- Add Krzysztof's Reviewed-by and Tested-by tags to patch 4/4.

 drivers/mfd/max77686.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/mfd/max77686.c b/drivers/mfd/max77686.c
index 896c1bf85acc..b0e8e13c0049 100644
--- a/drivers/mfd/max77686.c
+++ b/drivers/mfd/max77686.c
@@ -241,13 +241,6 @@ static int max77686_i2c_probe(struct i2c_client *i2c)
 	return 0;
 }
 
-static const struct i2c_device_id max77686_i2c_id[] = {
-	{ "max77686", TYPE_MAX77686 },
-	{ "max77802", TYPE_MAX77802 },
-	{ }
-};
-MODULE_DEVICE_TABLE(i2c, max77686_i2c_id);
-
 #ifdef CONFIG_PM_SLEEP
 static int max77686_suspend(struct device *dev)
 {
@@ -294,7 +287,6 @@ static struct i2c_driver max77686_i2c_driver = {
 		   .of_match_table = of_match_ptr(max77686_pmic_dt_match),
 	},
 	.probe_new = max77686_i2c_probe,
-	.id_table = max77686_i2c_id,
 };
 
 module_i2c_driver(max77686_i2c_driver);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/4] mfd: max77686: Use of_device_get_match_data() helper
  2017-01-13 13:34 ` [PATCH v2 2/4] mfd: max77686: Use of_device_get_match_data() helper Javier Martinez Canillas
@ 2017-01-13 14:09   ` Krzysztof Kozlowski
  2017-01-16  6:34   ` Chanwoo Choi
  2017-01-23 11:36   ` Lee Jones
  2 siblings, 0 replies; 16+ messages in thread
From: Krzysztof Kozlowski @ 2017-01-13 14:09 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: linux-kernel, Laxman Dewangan, Krzysztof Kozlowski, Chanwoo Choi,
	Bartlomiej Zolnierkiewicz, Lee Jones

On Fri, Jan 13, 2017 at 10:34:06AM -0300, Javier Martinez Canillas wrote:
> Use the generic helper to get the matched of_device_id .data, instead of
> open coding it.
> 
> The driver was checking if matching the OF node with the driver's OF table
> was failing, but this doesn't make too much sense since this can't happen
> in practice. The fact the probe function was called, means OF registered a
> device with a valid compatible string so a of_device_get_match_data() call
> will always succeed. So just remove this unneeded check.
> 
> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
> Acked-by: Laxman Dewangan <ldewangan@nvidia.com>
> 
> ---
> 
> Changes in v2:
> - Add Laxman's Acked-by tag to patch 2/4.
> - Mention in commit message that an unneeded check for match is removed.
> 
>  drivers/mfd/max77686.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
Tested-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/4] mfd: max77686: Don't attempt to get i2c_device_id .data
  2017-01-13 13:34 ` [PATCH v2 1/4] mfd: max77686: Don't attempt to get i2c_device_id .data Javier Martinez Canillas
@ 2017-01-16  6:30   ` Chanwoo Choi
  2017-01-23 11:35   ` Lee Jones
  1 sibling, 0 replies; 16+ messages in thread
From: Chanwoo Choi @ 2017-01-16  6:30 UTC (permalink / raw)
  To: Javier Martinez Canillas, linux-kernel
  Cc: Laxman Dewangan, Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz,
	Lee Jones

Hi,

On 2017년 01월 13일 22:34, Javier Martinez Canillas wrote:
> The driver is only used in platforms that have DT support so always the
> I2C device .data will be get from the matched OF node and never will be
> from the I2C device ID table.
> 
> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
> Acked-by: Laxman Dewangan <ldewangan@nvidia.com>
> Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
> Tested-by: Krzysztof Kozlowski <krzk@kernel.org>
> 
> ---
> 
> Changes in v2:
> - Add Laxman's Acked-by tag to patch 1/4.
> - Add Krzysztof's Reviewed-by and Tested-by tags to patch 1/4.
> 
>  drivers/mfd/max77686.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/mfd/max77686.c b/drivers/mfd/max77686.c
> index 7b68ed72e9cb..ddae3bf3e46c 100644
> --- a/drivers/mfd/max77686.c
> +++ b/drivers/mfd/max77686.c
> @@ -188,14 +188,11 @@ static int max77686_i2c_probe(struct i2c_client *i2c,
>  	if (!max77686)
>  		return -ENOMEM;
>  
> -	if (i2c->dev.of_node) {
> -		match = of_match_node(max77686_pmic_dt_match, i2c->dev.of_node);
> -		if (!match)
> -			return -EINVAL;
> -
> -		max77686->type = (unsigned long)match->data;
> -	} else
> -		max77686->type = id->driver_data;
> +	match = of_match_node(max77686_pmic_dt_match, i2c->dev.of_node);
> +	if (!match)
> +		return -EINVAL;
> +
> +	max77686->type = (unsigned long)match->data;
>  
>  	i2c_set_clientdata(i2c, max77686);
>  	max77686->dev = &i2c->dev;
> 

Looks good to me.
Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>

-- 
Best Regards,
Chanwoo Choi
S/W Center, Samsung Electronics

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/4] mfd: max77686: Use of_device_get_match_data() helper
  2017-01-13 13:34 ` [PATCH v2 2/4] mfd: max77686: Use of_device_get_match_data() helper Javier Martinez Canillas
  2017-01-13 14:09   ` Krzysztof Kozlowski
@ 2017-01-16  6:34   ` Chanwoo Choi
  2017-01-23 11:36   ` Lee Jones
  2 siblings, 0 replies; 16+ messages in thread
From: Chanwoo Choi @ 2017-01-16  6:34 UTC (permalink / raw)
  To: Javier Martinez Canillas, linux-kernel
  Cc: Laxman Dewangan, Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz,
	Lee Jones

Hi,

On 2017년 01월 13일 22:34, Javier Martinez Canillas wrote:
> Use the generic helper to get the matched of_device_id .data, instead of
> open coding it.
> 
> The driver was checking if matching the OF node with the driver's OF table
> was failing, but this doesn't make too much sense since this can't happen
> in practice. The fact the probe function was called, means OF registered a
> device with a valid compatible string so a of_device_get_match_data() call
> will always succeed. So just remove this unneeded check.
> 
> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
> Acked-by: Laxman Dewangan <ldewangan@nvidia.com>
> 
> ---
> 
> Changes in v2:
> - Add Laxman's Acked-by tag to patch 2/4.
> - Mention in commit message that an unneeded check for match is removed.
> 
>  drivers/mfd/max77686.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/mfd/max77686.c b/drivers/mfd/max77686.c
> index ddae3bf3e46c..33dd09493605 100644
> --- a/drivers/mfd/max77686.c
> +++ b/drivers/mfd/max77686.c
> @@ -34,6 +34,7 @@
>  #include <linux/mfd/max77686-private.h>
>  #include <linux/err.h>
>  #include <linux/of.h>
> +#include <linux/of_device.h>
>  
>  static const struct mfd_cell max77686_devs[] = {
>  	{ .name = "max77686-pmic", },
> @@ -175,7 +176,6 @@ static int max77686_i2c_probe(struct i2c_client *i2c,
>  			      const struct i2c_device_id *id)
>  {
>  	struct max77686_dev *max77686 = NULL;
> -	const struct of_device_id *match;
>  	unsigned int data;
>  	int ret = 0;
>  	const struct regmap_config *config;
> @@ -188,13 +188,8 @@ static int max77686_i2c_probe(struct i2c_client *i2c,
>  	if (!max77686)
>  		return -ENOMEM;
>  
> -	match = of_match_node(max77686_pmic_dt_match, i2c->dev.of_node);
> -	if (!match)
> -		return -EINVAL;
> -
> -	max77686->type = (unsigned long)match->data;
> -
>  	i2c_set_clientdata(i2c, max77686);
> +	max77686->type = (unsigned long)of_device_get_match_data(&i2c->dev);
>  	max77686->dev = &i2c->dev;
>  	max77686->i2c = i2c;
>  
> 

Looks good to me.
Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>

-- 
Best Regards,
Chanwoo Choi
S/W Center, Samsung Electronics

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 3/4] mfd: max77686: Use the struct i2c_driver .probe_new instead of .probe
  2017-01-13 13:34 ` [PATCH v2 3/4] mfd: max77686: Use the struct i2c_driver .probe_new instead of .probe Javier Martinez Canillas
@ 2017-01-16  6:37   ` Chanwoo Choi
  2017-01-23 11:36   ` Lee Jones
  1 sibling, 0 replies; 16+ messages in thread
From: Chanwoo Choi @ 2017-01-16  6:37 UTC (permalink / raw)
  To: Javier Martinez Canillas, linux-kernel
  Cc: Laxman Dewangan, Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz,
	Lee Jones

Hi,
On 2017년 01월 13일 22:34, Javier Martinez Canillas wrote:
> If a driver is only used in DT platforms, there's no need to get the
> i2c_device_id as an argument of the probe function. Since this data
> can be get from the matching of_device_id.
> 
> There's a temporary .probe_new field in struct i2c_driver that can be
> used as probe callback for the case when i2c_device_id won't be used.
> 
> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
> Acked-by: Laxman Dewangan <ldewangan@nvidia.com>
> Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
> Tested-by: Krzysztof Kozlowski <krzk@kernel.org>
> 
> ---
> 
> Changes in v2:
> - Add Laxman's Acked-by tag to patch 3/4.
> - Add Krzysztof's Reviewed-by and Tested-by tags to patch 3/4.
> 
>  drivers/mfd/max77686.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mfd/max77686.c b/drivers/mfd/max77686.c
> index 33dd09493605..896c1bf85acc 100644
> --- a/drivers/mfd/max77686.c
> +++ b/drivers/mfd/max77686.c
> @@ -172,8 +172,7 @@ static const struct of_device_id max77686_pmic_dt_match[] = {
>  };
>  MODULE_DEVICE_TABLE(of, max77686_pmic_dt_match);
>  
> -static int max77686_i2c_probe(struct i2c_client *i2c,
> -			      const struct i2c_device_id *id)
> +static int max77686_i2c_probe(struct i2c_client *i2c)
>  {
>  	struct max77686_dev *max77686 = NULL;
>  	unsigned int data;
> @@ -294,7 +293,7 @@ static struct i2c_driver max77686_i2c_driver = {
>  		   .pm = &max77686_pm,
>  		   .of_match_table = of_match_ptr(max77686_pmic_dt_match),
>  	},
> -	.probe = max77686_i2c_probe,
> +	.probe_new = max77686_i2c_probe,
>  	.id_table = max77686_i2c_id,
>  };
>  
> 

Looks good to me.
Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>

-- 
Best Regards,
Chanwoo Choi
S/W Center, Samsung Electronics

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 4/4] mfd: max77686: Remove I2C device ID table
  2017-01-13 13:34 ` [PATCH v2 4/4] mfd: max77686: Remove I2C device ID table Javier Martinez Canillas
@ 2017-01-16  6:46   ` Chanwoo Choi
  2017-01-16 12:30     ` Javier Martinez Canillas
  2017-01-23 11:40   ` Lee Jones
  1 sibling, 1 reply; 16+ messages in thread
From: Chanwoo Choi @ 2017-01-16  6:46 UTC (permalink / raw)
  To: Javier Martinez Canillas, linux-kernel
  Cc: Laxman Dewangan, Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz,
	Lee Jones

Hi,

I think that this patch better to squash with patch3.
After applying the patch3, this driver doesn't use
the max77686_i2c_id table.

On 2017년 01월 13일 22:34, Javier Martinez Canillas wrote:
> The driver is only used in DT platforms so there's no need to
> have an i2c_device_id table.
> 
> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
> Acked-by: Laxman Dewangan <ldewangan@nvidia.com>
> Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
> Tested-by: Krzysztof Kozlowski <krzk@kernel.org>
> 
> ---
> 
> Changes in v2:
> - Add Laxman's Acked-by tag to patch 4/4.
> - Add Krzysztof's Reviewed-by and Tested-by tags to patch 4/4.
> 
>  drivers/mfd/max77686.c | 8 --------
>  1 file changed, 8 deletions(-)
> 
> diff --git a/drivers/mfd/max77686.c b/drivers/mfd/max77686.c
> index 896c1bf85acc..b0e8e13c0049 100644
> --- a/drivers/mfd/max77686.c
> +++ b/drivers/mfd/max77686.c
> @@ -241,13 +241,6 @@ static int max77686_i2c_probe(struct i2c_client *i2c)
>  	return 0;
>  }
>  
> -static const struct i2c_device_id max77686_i2c_id[] = {
> -	{ "max77686", TYPE_MAX77686 },
> -	{ "max77802", TYPE_MAX77802 },
> -	{ }
> -};
> -MODULE_DEVICE_TABLE(i2c, max77686_i2c_id);
> -
>  #ifdef CONFIG_PM_SLEEP
>  static int max77686_suspend(struct device *dev)
>  {
> @@ -294,7 +287,6 @@ static struct i2c_driver max77686_i2c_driver = {
>  		   .of_match_table = of_match_ptr(max77686_pmic_dt_match),
>  	},
>  	.probe_new = max77686_i2c_probe,
> -	.id_table = max77686_i2c_id,
>  };
>  
>  module_i2c_driver(max77686_i2c_driver);
> 


-- 
Best Regards,
Chanwoo Choi
S/W Center, Samsung Electronics

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 4/4] mfd: max77686: Remove I2C device ID table
  2017-01-16  6:46   ` Chanwoo Choi
@ 2017-01-16 12:30     ` Javier Martinez Canillas
  2017-01-23 11:39       ` Lee Jones
  0 siblings, 1 reply; 16+ messages in thread
From: Javier Martinez Canillas @ 2017-01-16 12:30 UTC (permalink / raw)
  To: Chanwoo Choi, linux-kernel
  Cc: Laxman Dewangan, Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz,
	Lee Jones

Hello Chanwoo,

Thanks for the review.

On 01/16/2017 03:46 AM, Chanwoo Choi wrote:
> Hi,
> 
> I think that this patch better to squash with patch3.
> After applying the patch3, this driver doesn't use
> the max77686_i2c_id table.
> 

I usually prefer to separate each change in a different patch,
AFAICT moving to use the .probe_new callback and removing the
I2C id table are two different changes.

I can squash if that's the correct approach though, I will let
Lee decide.

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/4] mfd: max77686: Don't attempt to get i2c_device_id .data
  2017-01-13 13:34 ` [PATCH v2 1/4] mfd: max77686: Don't attempt to get i2c_device_id .data Javier Martinez Canillas
  2017-01-16  6:30   ` Chanwoo Choi
@ 2017-01-23 11:35   ` Lee Jones
  1 sibling, 0 replies; 16+ messages in thread
From: Lee Jones @ 2017-01-23 11:35 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: linux-kernel, Laxman Dewangan, Krzysztof Kozlowski, Chanwoo Choi,
	Bartlomiej Zolnierkiewicz

On Fri, 13 Jan 2017, Javier Martinez Canillas wrote:

> The driver is only used in platforms that have DT support so always the
> I2C device .data will be get from the matched OF node and never will be
> from the I2C device ID table.
> 
> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
> Acked-by: Laxman Dewangan <ldewangan@nvidia.com>
> Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
> Tested-by: Krzysztof Kozlowski <krzk@kernel.org>
> 
> ---
> 
> Changes in v2:
> - Add Laxman's Acked-by tag to patch 1/4.
> - Add Krzysztof's Reviewed-by and Tested-by tags to patch 1/4.
> 
>  drivers/mfd/max77686.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)

Applied, thanks.

(this patch supersedes the previous one -- no action required)

> diff --git a/drivers/mfd/max77686.c b/drivers/mfd/max77686.c
> index 7b68ed72e9cb..ddae3bf3e46c 100644
> --- a/drivers/mfd/max77686.c
> +++ b/drivers/mfd/max77686.c
> @@ -188,14 +188,11 @@ static int max77686_i2c_probe(struct i2c_client *i2c,
>  	if (!max77686)
>  		return -ENOMEM;
>  
> -	if (i2c->dev.of_node) {
> -		match = of_match_node(max77686_pmic_dt_match, i2c->dev.of_node);
> -		if (!match)
> -			return -EINVAL;
> -
> -		max77686->type = (unsigned long)match->data;
> -	} else
> -		max77686->type = id->driver_data;
> +	match = of_match_node(max77686_pmic_dt_match, i2c->dev.of_node);
> +	if (!match)
> +		return -EINVAL;
> +
> +	max77686->type = (unsigned long)match->data;
>  
>  	i2c_set_clientdata(i2c, max77686);
>  	max77686->dev = &i2c->dev;

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/4] mfd: max77686: Use of_device_get_match_data() helper
  2017-01-13 13:34 ` [PATCH v2 2/4] mfd: max77686: Use of_device_get_match_data() helper Javier Martinez Canillas
  2017-01-13 14:09   ` Krzysztof Kozlowski
  2017-01-16  6:34   ` Chanwoo Choi
@ 2017-01-23 11:36   ` Lee Jones
  2 siblings, 0 replies; 16+ messages in thread
From: Lee Jones @ 2017-01-23 11:36 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: linux-kernel, Laxman Dewangan, Krzysztof Kozlowski, Chanwoo Choi,
	Bartlomiej Zolnierkiewicz

On Fri, 13 Jan 2017, Javier Martinez Canillas wrote:

> Use the generic helper to get the matched of_device_id .data, instead of
> open coding it.
> 
> The driver was checking if matching the OF node with the driver's OF table
> was failing, but this doesn't make too much sense since this can't happen
> in practice. The fact the probe function was called, means OF registered a
> device with a valid compatible string so a of_device_get_match_data() call
> will always succeed. So just remove this unneeded check.
> 
> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
> Acked-by: Laxman Dewangan <ldewangan@nvidia.com>
> 
> ---
> 
> Changes in v2:
> - Add Laxman's Acked-by tag to patch 2/4.
> - Mention in commit message that an unneeded check for match is removed.
> 
>  drivers/mfd/max77686.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)

Applied, thanks.

> diff --git a/drivers/mfd/max77686.c b/drivers/mfd/max77686.c
> index ddae3bf3e46c..33dd09493605 100644
> --- a/drivers/mfd/max77686.c
> +++ b/drivers/mfd/max77686.c
> @@ -34,6 +34,7 @@
>  #include <linux/mfd/max77686-private.h>
>  #include <linux/err.h>
>  #include <linux/of.h>
> +#include <linux/of_device.h>
>  
>  static const struct mfd_cell max77686_devs[] = {
>  	{ .name = "max77686-pmic", },
> @@ -175,7 +176,6 @@ static int max77686_i2c_probe(struct i2c_client *i2c,
>  			      const struct i2c_device_id *id)
>  {
>  	struct max77686_dev *max77686 = NULL;
> -	const struct of_device_id *match;
>  	unsigned int data;
>  	int ret = 0;
>  	const struct regmap_config *config;
> @@ -188,13 +188,8 @@ static int max77686_i2c_probe(struct i2c_client *i2c,
>  	if (!max77686)
>  		return -ENOMEM;
>  
> -	match = of_match_node(max77686_pmic_dt_match, i2c->dev.of_node);
> -	if (!match)
> -		return -EINVAL;
> -
> -	max77686->type = (unsigned long)match->data;
> -
>  	i2c_set_clientdata(i2c, max77686);
> +	max77686->type = (unsigned long)of_device_get_match_data(&i2c->dev);
>  	max77686->dev = &i2c->dev;
>  	max77686->i2c = i2c;
>  

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 3/4] mfd: max77686: Use the struct i2c_driver .probe_new instead of .probe
  2017-01-13 13:34 ` [PATCH v2 3/4] mfd: max77686: Use the struct i2c_driver .probe_new instead of .probe Javier Martinez Canillas
  2017-01-16  6:37   ` Chanwoo Choi
@ 2017-01-23 11:36   ` Lee Jones
  1 sibling, 0 replies; 16+ messages in thread
From: Lee Jones @ 2017-01-23 11:36 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: linux-kernel, Laxman Dewangan, Krzysztof Kozlowski, Chanwoo Choi,
	Bartlomiej Zolnierkiewicz

On Fri, 13 Jan 2017, Javier Martinez Canillas wrote:

> If a driver is only used in DT platforms, there's no need to get the
> i2c_device_id as an argument of the probe function. Since this data
> can be get from the matching of_device_id.
> 
> There's a temporary .probe_new field in struct i2c_driver that can be
> used as probe callback for the case when i2c_device_id won't be used.
> 
> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
> Acked-by: Laxman Dewangan <ldewangan@nvidia.com>
> Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
> Tested-by: Krzysztof Kozlowski <krzk@kernel.org>
> 
> ---
> 
> Changes in v2:
> - Add Laxman's Acked-by tag to patch 3/4.
> - Add Krzysztof's Reviewed-by and Tested-by tags to patch 3/4.
> 
>  drivers/mfd/max77686.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)

Applied, thanks.

> diff --git a/drivers/mfd/max77686.c b/drivers/mfd/max77686.c
> index 33dd09493605..896c1bf85acc 100644
> --- a/drivers/mfd/max77686.c
> +++ b/drivers/mfd/max77686.c
> @@ -172,8 +172,7 @@ static const struct of_device_id max77686_pmic_dt_match[] = {
>  };
>  MODULE_DEVICE_TABLE(of, max77686_pmic_dt_match);
>  
> -static int max77686_i2c_probe(struct i2c_client *i2c,
> -			      const struct i2c_device_id *id)
> +static int max77686_i2c_probe(struct i2c_client *i2c)
>  {
>  	struct max77686_dev *max77686 = NULL;
>  	unsigned int data;
> @@ -294,7 +293,7 @@ static struct i2c_driver max77686_i2c_driver = {
>  		   .pm = &max77686_pm,
>  		   .of_match_table = of_match_ptr(max77686_pmic_dt_match),
>  	},
> -	.probe = max77686_i2c_probe,
> +	.probe_new = max77686_i2c_probe,
>  	.id_table = max77686_i2c_id,
>  };
>  

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 4/4] mfd: max77686: Remove I2C device ID table
  2017-01-16 12:30     ` Javier Martinez Canillas
@ 2017-01-23 11:39       ` Lee Jones
  0 siblings, 0 replies; 16+ messages in thread
From: Lee Jones @ 2017-01-23 11:39 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: Chanwoo Choi, linux-kernel, Laxman Dewangan, Krzysztof Kozlowski,
	Bartlomiej Zolnierkiewicz

On Mon, 16 Jan 2017, Javier Martinez Canillas wrote:

> Hello Chanwoo,
> 
> Thanks for the review.
> 
> On 01/16/2017 03:46 AM, Chanwoo Choi wrote:
> > Hi,
> > 
> > I think that this patch better to squash with patch3.
> > After applying the patch3, this driver doesn't use
> > the max77686_i2c_id table.
> > 
> 
> I usually prefer to separate each change in a different patch,
> AFAICT moving to use the .probe_new callback and removing the
> I2C id table are two different changes.
> 
> I can squash if that's the correct approach though, I will let
> Lee decide.

This should really be a single patch, as the previous one leaves this
code redundant, but ... meh!

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 4/4] mfd: max77686: Remove I2C device ID table
  2017-01-13 13:34 ` [PATCH v2 4/4] mfd: max77686: Remove I2C device ID table Javier Martinez Canillas
  2017-01-16  6:46   ` Chanwoo Choi
@ 2017-01-23 11:40   ` Lee Jones
  1 sibling, 0 replies; 16+ messages in thread
From: Lee Jones @ 2017-01-23 11:40 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: linux-kernel, Laxman Dewangan, Krzysztof Kozlowski, Chanwoo Choi,
	Bartlomiej Zolnierkiewicz

On Fri, 13 Jan 2017, Javier Martinez Canillas wrote:

> The driver is only used in DT platforms so there's no need to
> have an i2c_device_id table.
> 
> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
> Acked-by: Laxman Dewangan <ldewangan@nvidia.com>
> Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
> Tested-by: Krzysztof Kozlowski <krzk@kernel.org>
> 
> ---
> 
> Changes in v2:
> - Add Laxman's Acked-by tag to patch 4/4.
> - Add Krzysztof's Reviewed-by and Tested-by tags to patch 4/4.
> 
>  drivers/mfd/max77686.c | 8 --------
>  1 file changed, 8 deletions(-)

Applied, thanks.

> diff --git a/drivers/mfd/max77686.c b/drivers/mfd/max77686.c
> index 896c1bf85acc..b0e8e13c0049 100644
> --- a/drivers/mfd/max77686.c
> +++ b/drivers/mfd/max77686.c
> @@ -241,13 +241,6 @@ static int max77686_i2c_probe(struct i2c_client *i2c)
>  	return 0;
>  }
>  
> -static const struct i2c_device_id max77686_i2c_id[] = {
> -	{ "max77686", TYPE_MAX77686 },
> -	{ "max77802", TYPE_MAX77802 },
> -	{ }
> -};
> -MODULE_DEVICE_TABLE(i2c, max77686_i2c_id);
> -
>  #ifdef CONFIG_PM_SLEEP
>  static int max77686_suspend(struct device *dev)
>  {
> @@ -294,7 +287,6 @@ static struct i2c_driver max77686_i2c_driver = {
>  		   .of_match_table = of_match_ptr(max77686_pmic_dt_match),
>  	},
>  	.probe_new = max77686_i2c_probe,
> -	.id_table = max77686_i2c_id,
>  };
>  
>  module_i2c_driver(max77686_i2c_driver);

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2017-01-23 11:36 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-13 13:34 [PATCH v2 0/4] mfd: max77686: Remove unneeded non-OF code in driver Javier Martinez Canillas
2017-01-13 13:34 ` [PATCH v2 1/4] mfd: max77686: Don't attempt to get i2c_device_id .data Javier Martinez Canillas
2017-01-16  6:30   ` Chanwoo Choi
2017-01-23 11:35   ` Lee Jones
2017-01-13 13:34 ` [PATCH v2 2/4] mfd: max77686: Use of_device_get_match_data() helper Javier Martinez Canillas
2017-01-13 14:09   ` Krzysztof Kozlowski
2017-01-16  6:34   ` Chanwoo Choi
2017-01-23 11:36   ` Lee Jones
2017-01-13 13:34 ` [PATCH v2 3/4] mfd: max77686: Use the struct i2c_driver .probe_new instead of .probe Javier Martinez Canillas
2017-01-16  6:37   ` Chanwoo Choi
2017-01-23 11:36   ` Lee Jones
2017-01-13 13:34 ` [PATCH v2 4/4] mfd: max77686: Remove I2C device ID table Javier Martinez Canillas
2017-01-16  6:46   ` Chanwoo Choi
2017-01-16 12:30     ` Javier Martinez Canillas
2017-01-23 11:39       ` Lee Jones
2017-01-23 11:40   ` Lee Jones

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).