[PATCH v3 0/2] max77693: added device tree support

[PATCH v3 0/2] max77693: added device tree support

[Andrzej Hajda]

Hi,

The first patch cleans up the driver from unneccesary
wakeup handling.
The second patch adds device tree support to the driver.

Regards
Andrzej

Andrzej Hajda (2):
  max77693: remove device wakeup from driver
  max77693: added device tree support

 drivers/mfd/max77693.c               | 18 ++++++++----------
 include/linux/mfd/max77693-private.h |  1 -
 include/linux/mfd/max77693.h         |  2 --
 3 files changed, 8 insertions(+), 13 deletions(-)

-- 
1.8.1.2

[PATCH v3 1/2] max77693: remove device wakeup from driver

[Andrzej Hajda]

The patch removes wakeup related code from
the driver and plaftorm data - it is already
handled by i2c core using I2C_CLIENT_WAKE flag
from struct i2c_board_info. As a result MFD
itself do not requires platform data.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/mfd/max77693.c               | 10 ----------
 include/linux/mfd/max77693-private.h |  1 -
 include/linux/mfd/max77693.h         |  2 --
 3 files changed, 13 deletions(-)

diff --git a/drivers/mfd/max77693.c b/drivers/mfd/max77693.c
index 9e60fed..27f5da3 100644
--- a/drivers/mfd/max77693.c
+++ b/drivers/mfd/max77693.c
@@ -110,15 +110,9 @@ static int max77693_i2c_probe(struct i2c_client *i2c,
 			      const struct i2c_device_id *id)
 {
 	struct max77693_dev *max77693;
-	struct max77693_platform_data *pdata = i2c->dev.platform_data;
 	u8 reg_data;
 	int ret = 0;
 
-	if (!pdata) {
-		dev_err(&i2c->dev, "No platform data found.\n");
-		return -EINVAL;
-	}
-
 	max77693 = devm_kzalloc(&i2c->dev,
 			sizeof(struct max77693_dev), GFP_KERNEL);
 	if (max77693 == NULL)
@@ -138,8 +132,6 @@ static int max77693_i2c_probe(struct i2c_client *i2c,
 		return ret;
 	}
 
-	max77693->wakeup = pdata->wakeup;
-
 	ret = max77693_read_reg(max77693->regmap, MAX77693_PMIC_REG_PMIC_ID2,
 				&reg_data);
 	if (ret < 0) {
@@ -179,8 +171,6 @@ static int max77693_i2c_probe(struct i2c_client *i2c,
 	if (ret < 0)
 		goto err_mfd;
 
-	device_init_wakeup(max77693->dev, pdata->wakeup);
-
 	return ret;
 
 err_mfd:
diff --git a/include/linux/mfd/max77693-private.h b/include/linux/mfd/max77693-private.h
index 244fb0d..3e050b9 100644
--- a/include/linux/mfd/max77693-private.h
+++ b/include/linux/mfd/max77693-private.h
@@ -323,7 +323,6 @@ struct max77693_dev {
 
 	int irq;
 	int irq_gpio;
-	bool wakeup;
 	struct mutex irqlock;
 	int irq_masks_cur[MAX77693_IRQ_GROUP_NR];
 	int irq_masks_cache[MAX77693_IRQ_GROUP_NR];
diff --git a/include/linux/mfd/max77693.h b/include/linux/mfd/max77693.h
index 676f0f3..3f3dc45 100644
--- a/include/linux/mfd/max77693.h
+++ b/include/linux/mfd/max77693.h
@@ -64,8 +64,6 @@ struct max77693_muic_platform_data {
 };
 
 struct max77693_platform_data {
-	int wakeup;
-
 	/* regulator data */
 	struct max77693_regulator_data *regulators;
 	int num_regulators;
-- 
1.8.1.2

[PATCH v3 2/2] max77693: added device tree support

[Andrzej Hajda]

This patch adds only of_match_table.
There are no device specific properties.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/mfd/max77693.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/mfd/max77693.c b/drivers/mfd/max77693.c
index 27f5da3..e0b11a9 100644
--- a/drivers/mfd/max77693.c
+++ b/drivers/mfd/max77693.c
@@ -225,11 +225,19 @@ static const struct dev_pm_ops max77693_pm = {
 	.resume = max77693_resume,
 };
 
+#ifdef CONFIG_OF
+static struct of_device_id max77693_dt_match[] = {
+	{.compatible = "maxim,max77693"},
+	{},
+};
+#endif
+
 static struct i2c_driver max77693_i2c_driver = {
 	.driver = {
 		   .name = "max77693",
 		   .owner = THIS_MODULE,
 		   .pm = &max77693_pm,
+		   .of_match_table = of_match_ptr(max77693_dt_match),
 	},
 	.probe = max77693_i2c_probe,
 	.remove = max77693_i2c_remove,
-- 
1.8.1.2

Re: [PATCH v3 2/2] max77693: added device tree support

[Mark Rutland]

On Wed, Aug 21, 2013 at 05:53:34PM +0100, Andrzej Hajda wrote:
> This patch adds only of_match_table.
> There are no device specific properties.

Could you clarify what functionality this enables and what it doesn't,
please?

This doesn't seem to enable support for the regulators described in the
binding [1] (which from the looks of it needs proof-reading and possibly
rework).

Are there any changes we might need in future to either support new
functionality or to generalise the binding. e.g. do we need a regulator
for the LED? 

Given the binding has never been supported, are we happy now that it
best represents the hardware, or are there avenues of improvement
*before* it becomes ABI?

Thanks,
Mark.

[1] Documentation/devicetree/bindings/mfd/max77693.txt

> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/mfd/max77693.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/mfd/max77693.c b/drivers/mfd/max77693.c
> index 27f5da3..e0b11a9 100644
> --- a/drivers/mfd/max77693.c
> +++ b/drivers/mfd/max77693.c
> @@ -225,11 +225,19 @@ static const struct dev_pm_ops max77693_pm = {
>  	.resume = max77693_resume,
>  };
>  
> +#ifdef CONFIG_OF
> +static struct of_device_id max77693_dt_match[] = {
> +	{.compatible = "maxim,max77693"},
> +	{},
> +};
> +#endif
> +
>  static struct i2c_driver max77693_i2c_driver = {
>  	.driver = {
>  		   .name = "max77693",
>  		   .owner = THIS_MODULE,
>  		   .pm = &max77693_pm,
> +		   .of_match_table = of_match_ptr(max77693_dt_match),
>  	},
>  	.probe = max77693_i2c_probe,
>  	.remove = max77693_i2c_remove,
> -- 
> 1.8.1.2
> 
> 

Re: [PATCH v3 2/2] max77693: added device tree support

[Tomasz Figa]

Hi Andrzej,

On Wednesday 21 of August 2013 18:53:34 Andrzej Hajda wrote:
> This patch adds only of_match_table.
> There are no device specific properties.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/mfd/max77693.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/mfd/max77693.c b/drivers/mfd/max77693.c
> index 27f5da3..e0b11a9 100644
> --- a/drivers/mfd/max77693.c
> +++ b/drivers/mfd/max77693.c
> @@ -225,11 +225,19 @@ static const struct dev_pm_ops max77693_pm = {
>  	.resume = max77693_resume,
>  };
> 
> +#ifdef CONFIG_OF
> +static struct of_device_id max77693_dt_match[] = {
> +	{.compatible = "maxim,max77693"},
> +	{},
> +};
> +#endif
> +
>  static struct i2c_driver max77693_i2c_driver = {
>  	.driver = {
>  		   .name = "max77693",
>  		   .owner = THIS_MODULE,
>  		   .pm = &max77693_pm,
> +		   .of_match_table = of_match_ptr(max77693_dt_match),

As far as I'm aware of, you don't need explicit OF match table for I2C 
devices, because the I2C OF core can use the array of struct i2c_device_id 
pointed by .id_table field of struct i2c_driver.

I'm not sure if a separate OF table isn't preferred, though, so your patch 
might be fine.

Best regards,
Tomasz

Re: [PATCH v3 2/2] max77693: added device tree support

[Tomasz Figa]

Hi Mark,

On Friday 23 of August 2013 15:14:33 Mark Rutland wrote:
> On Wed, Aug 21, 2013 at 05:53:34PM +0100, Andrzej Hajda wrote:
> > This patch adds only of_match_table.
> > There are no device specific properties.
> 
> Could you clarify what functionality this enables and what it doesn't,
> please?

This patch simply adds explicit OF match table for this device. Before, 
the driver could be matched only by a fallback to i2c_device_id table.

> This doesn't seem to enable support for the regulators described in the
> binding [1] (which from the looks of it needs proof-reading and possibly
> rework).
> 
> Are there any changes we might need in future to either support new
> functionality or to generalise the binding. e.g. do we need a regulator
> for the LED?
> 
> Given the binding has never been supported, are we happy now that it
> best represents the hardware, or are there avenues of improvement
> *before* it becomes ABI?

Well, documentation of the binding has been present in kernel tree since 
June, but I too think that we should review it and make sure it makes 
sense. I'll try to get some information on this chip at work, on Monday.

Best regards,
Tomasz

Re: [PATCH v3 2/2] max77693: added device tree support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 796 bytes --]

On Fri, Aug 23, 2013 at 10:08:39PM +0200, Tomasz Figa wrote:
> On Wednesday 21 of August 2013 18:53:34 Andrzej Hajda wrote:

> > +#ifdef CONFIG_OF
> > +static struct of_device_id max77693_dt_match[] = {
> > +	{.compatible = "maxim,max77693"},
> > +	{},
> > +};
> > +#endif

> As far as I'm aware of, you don't need explicit OF match table for I2C 
> devices, because the I2C OF core can use the array of struct i2c_device_id 
> pointed by .id_table field of struct i2c_driver.

> I'm not sure if a separate OF table isn't preferred, though, so your patch 
> might be fine.

It's still good practice to explicitly define a binding since that
gives a vendor prefix and there are overlaps out there in chip vendor
namings - for example both Wolfson and Wondermedia use "WMxxxx".

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v3 1/2] max77693: remove device wakeup from driver

[Andrzej Hajda]

On 08/21/2013 06:53 PM, Andrzej Hajda wrote:
> The patch removes wakeup related code from
> the driver and plaftorm data - it is already
> handled by i2c core using I2C_CLIENT_WAKE flag
> from struct i2c_board_info. As a result MFD
> itself do not requires platform data.
>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/mfd/max77693.c               | 10 ----------
>  include/linux/mfd/max77693-private.h |  1 -
>  include/linux/mfd/max77693.h         |  2 --
>  3 files changed, 13 deletions(-)
>
> diff --git a/drivers/mfd/max77693.c b/drivers/mfd/max77693.c
> index 9e60fed..27f5da3 100644
> --- a/drivers/mfd/max77693.c
> +++ b/drivers/mfd/max77693.c
> @@ -110,15 +110,9 @@ static int max77693_i2c_probe(struct i2c_client *i2c,
>  			      const struct i2c_device_id *id)
>  {
>  	struct max77693_dev *max77693;
> -	struct max77693_platform_data *pdata = i2c->dev.platform_data;
>  	u8 reg_data;
>  	int ret = 0;
>  
> -	if (!pdata) {
> -		dev_err(&i2c->dev, "No platform data found.\n");
> -		return -EINVAL;
> -	}
> -
>  	max77693 = devm_kzalloc(&i2c->dev,
>  			sizeof(struct max77693_dev), GFP_KERNEL);
>  	if (max77693 == NULL)
> @@ -138,8 +132,6 @@ static int max77693_i2c_probe(struct i2c_client *i2c,
>  		return ret;
>  	}
>  
> -	max77693->wakeup = pdata->wakeup;
> -
>  	ret = max77693_read_reg(max77693->regmap, MAX77693_PMIC_REG_PMIC_ID2,
>  				&reg_data);
>  	if (ret < 0) {
> @@ -179,8 +171,6 @@ static int max77693_i2c_probe(struct i2c_client *i2c,
>  	if (ret < 0)
>  		goto err_mfd;
>  
> -	device_init_wakeup(max77693->dev, pdata->wakeup);
> -
>  	return ret;
>  
>  err_mfd:
> diff --git a/include/linux/mfd/max77693-private.h b/include/linux/mfd/max77693-private.h
> index 244fb0d..3e050b9 100644
> --- a/include/linux/mfd/max77693-private.h
> +++ b/include/linux/mfd/max77693-private.h
> @@ -323,7 +323,6 @@ struct max77693_dev {
>  
>  	int irq;
>  	int irq_gpio;
> -	bool wakeup;
>  	struct mutex irqlock;
>  	int irq_masks_cur[MAX77693_IRQ_GROUP_NR];
>  	int irq_masks_cache[MAX77693_IRQ_GROUP_NR];
> diff --git a/include/linux/mfd/max77693.h b/include/linux/mfd/max77693.h
> index 676f0f3..3f3dc45 100644
> --- a/include/linux/mfd/max77693.h
> +++ b/include/linux/mfd/max77693.h
> @@ -64,8 +64,6 @@ struct max77693_muic_platform_data {
>  };
>  
>  struct max77693_platform_data {
> -	int wakeup;
> -
>  	/* regulator data */
>  	struct max77693_regulator_data *regulators;
>  	int num_regulators;

Ping.

Regards
Andrzej

Re: [PATCH v3 2/2] max77693: added device tree support

[Andrzej Hajda]

On 08/21/2013 06:53 PM, Andrzej Hajda wrote:
> This patch adds only of_match_table.
> There are no device specific properties.
>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/mfd/max77693.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/drivers/mfd/max77693.c b/drivers/mfd/max77693.c
> index 27f5da3..e0b11a9 100644
> --- a/drivers/mfd/max77693.c
> +++ b/drivers/mfd/max77693.c
> @@ -225,11 +225,19 @@ static const struct dev_pm_ops max77693_pm = {
>  	.resume = max77693_resume,
>  };
>  
> +#ifdef CONFIG_OF
> +static struct of_device_id max77693_dt_match[] = {
> +	{.compatible = "maxim,max77693"},
> +	{},
> +};
> +#endif
> +
>  static struct i2c_driver max77693_i2c_driver = {
>  	.driver = {
>  		   .name = "max77693",
>  		   .owner = THIS_MODULE,
>  		   .pm = &max77693_pm,
> +		   .of_match_table = of_match_ptr(max77693_dt_match),
>  	},
>  	.probe = max77693_i2c_probe,
>  	.remove = max77693_i2c_remove,
Ping.

Regards
Andrzej

Re: [PATCH v3 2/2] max77693: added device tree support

[Lee Jones]

On Wed, 21 Aug 2013, Andrzej Hajda wrote:

> This patch adds only of_match_table.
> There are no device specific properties.
> 
> Signed-off-by: Andrzej Hajda <a.hajda-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Reviewed-by: Sylwester Nawrocki <s.nawrocki-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Kyungmin Park <kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
>  drivers/mfd/max77693.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/mfd/max77693.c b/drivers/mfd/max77693.c
> index 27f5da3..e0b11a9 100644
> --- a/drivers/mfd/max77693.c
> +++ b/drivers/mfd/max77693.c
> @@ -225,11 +225,19 @@ static const struct dev_pm_ops max77693_pm = {
>  	.resume = max77693_resume,
>  };
>  
> +#ifdef CONFIG_OF
> +static struct of_device_id max77693_dt_match[] = {
> +	{.compatible = "maxim,max77693"},

Please add spaces before ".comp.." and after "...693""

> +	{},
> +};
> +#endif
> +
>  static struct i2c_driver max77693_i2c_driver = {
>  	.driver = {
>  		   .name = "max77693",
>  		   .owner = THIS_MODULE,
>  		   .pm = &max77693_pm,
> +		   .of_match_table = of_match_ptr(max77693_dt_match),
>  	},
>  	.probe = max77693_i2c_probe,
>  	.remove = max77693_i2c_remove,

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 2/2] max77693: added device tree support

[Andrzej Hajda]

This patch adds only of_match_table.
There are no device specific properties.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
Hi,

Spaces added.

Regards
Andrzej
---
 drivers/mfd/max77693.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/mfd/max77693.c b/drivers/mfd/max77693.c
index 27f5da3..e0b11a9 100644
--- a/drivers/mfd/max77693.c
+++ b/drivers/mfd/max77693.c
@@ -225,11 +225,19 @@ static const struct dev_pm_ops max77693_pm = {
 	.resume = max77693_resume,
 };
 
+#ifdef CONFIG_OF
+static struct of_device_id max77693_dt_match[] = {
+	{ .compatible = "maxim,max77693" },
+	{},
+};
+#endif
+
 static struct i2c_driver max77693_i2c_driver = {
 	.driver = {
 		   .name = "max77693",
 		   .owner = THIS_MODULE,
 		   .pm = &max77693_pm,
+		   .of_match_table = of_match_ptr(max77693_dt_match),
 	},
 	.probe = max77693_i2c_probe,
 	.remove = max77693_i2c_remove,
-- 
1.8.1.2

Re: [PATCH v3 1/2] max77693: remove device wakeup from driver

[Lee Jones]

> The patch removes wakeup related code from
> the driver and plaftorm data - it is already
> handled by i2c core using I2C_CLIENT_WAKE flag
> from struct i2c_board_info. As a result MFD
> itself do not requires platform data.

I have expanded this to use more than 46 chars of the line
buffer. Please also do this for future submissions.

> Signed-off-by: Andrzej Hajda <a.hajda-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Kyungmin Park <kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
>  drivers/mfd/max77693.c               | 10 ----------
>  include/linux/mfd/max77693-private.h |  1 -
>  include/linux/mfd/max77693.h         |  2 --
>  3 files changed, 13 deletions(-)

Applied, thanks.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 2/2] max77693: added device tree support

[Lee Jones]

On Fri, 27 Sep 2013, Andrzej Hajda wrote:

> This patch adds only of_match_table.
> There are no device specific properties.
> 
> Signed-off-by: Andrzej Hajda <a.hajda-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Reviewed-by: Sylwester Nawrocki <s.nawrocki-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Kyungmin Park <kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
> Hi,
> 
> Spaces added.
> 
> Regards
> Andrzej
> ---
>  drivers/mfd/max77693.c | 8 ++++++++
>  1 file changed, 8 insertions(+)

Applied, thanks.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html