[PATCH 1/5] W1: w1-gpio - fix incorrect __init/__exit markups

[PATCH 1/5] W1: w1-gpio - fix incorrect __init/__exit markups

[Dmitry Torokhov]

We should not be using __init/__exit markups on probe() and remove()
methods unless platform_device_probe() is used, because other methods
allow unbinding device through sysfs and these methods should not be
discarded.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---

This was just compiled - no hardware.

 drivers/w1/masters/w1-gpio.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/w1/masters/w1-gpio.c b/drivers/w1/masters/w1-gpio.c
index 85b363a..799dafd 100644
--- a/drivers/w1/masters/w1-gpio.c
+++ b/drivers/w1/masters/w1-gpio.c
@@ -72,7 +72,7 @@ static int w1_gpio_probe_dt(struct platform_device *pdev)
 	return 0;
 }
 
-static int __init w1_gpio_probe(struct platform_device *pdev)
+static int w1_gpio_probe(struct platform_device *pdev)
 {
 	struct w1_bus_master *master;
 	struct w1_gpio_platform_data *pdata;
@@ -158,7 +158,7 @@ static int __init w1_gpio_probe(struct platform_device *pdev)
 	return err;
 }
 
-static int __exit w1_gpio_remove(struct platform_device *pdev)
+static int w1_gpio_remove(struct platform_device *pdev)
 {
 	struct w1_bus_master *master = platform_get_drvdata(pdev);
 	struct w1_gpio_platform_data *pdata = pdev->dev.platform_data;
@@ -210,7 +210,7 @@ static struct platform_driver w1_gpio_driver = {
 		.of_match_table = of_match_ptr(w1_gpio_dt_ids),
 	},
 	.probe = w1_gpio_probe,
-	.remove	= __exit_p(w1_gpio_remove),
+	.remove = w1_gpio_remove,
 	.suspend = w1_gpio_suspend,
 	.resume = w1_gpio_resume,
 };
-- 
1.7.11.7

[PATCH 2/5] W1: w1-gpio - switch to using dev_pm_ops

[Dmitry Torokhov]

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/w1/masters/w1-gpio.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/w1/masters/w1-gpio.c b/drivers/w1/masters/w1-gpio.c
index 799dafd..c45b9ae 100644
--- a/drivers/w1/masters/w1-gpio.c
+++ b/drivers/w1/masters/w1-gpio.c
@@ -176,11 +176,10 @@ static int w1_gpio_remove(struct platform_device *pdev)
 	return 0;
 }
 
-#ifdef CONFIG_PM
-
-static int w1_gpio_suspend(struct platform_device *pdev, pm_message_t state)
+#ifdef CONFIG_PM_SLEEP
+static int w1_gpio_suspend(struct device *dev)
 {
-	struct w1_gpio_platform_data *pdata = pdev->dev.platform_data;
+	const struct w1_gpio_platform_data *pdata = dev_get_platdata(dev);
 
 	if (pdata->enable_external_pullup)
 		pdata->enable_external_pullup(0);
@@ -188,31 +187,28 @@ static int w1_gpio_suspend(struct platform_device *pdev, pm_message_t state)
 	return 0;
 }
 
-static int w1_gpio_resume(struct platform_device *pdev)
+static int w1_gpio_resume(struct device *dev)
 {
-	struct w1_gpio_platform_data *pdata = pdev->dev.platform_data;
+	const struct w1_gpio_platform_data *pdata = dev_get_platdata(dev);
 
 	if (pdata->enable_external_pullup)
 		pdata->enable_external_pullup(1);
 
 	return 0;
 }
-
-#else
-#define w1_gpio_suspend	NULL
-#define w1_gpio_resume	NULL
 #endif
 
+static SIMPLE_DEV_PM_OPS(w1_gpio_pm_ops, w1_gpio_suspend, w1_gpio_resume);
+
 static struct platform_driver w1_gpio_driver = {
 	.driver = {
 		.name	= "w1-gpio",
 		.owner	= THIS_MODULE,
+		.pm	= &w1_gpio_pm_ops,
 		.of_match_table = of_match_ptr(w1_gpio_dt_ids),
 	},
 	.probe = w1_gpio_probe,
 	.remove = w1_gpio_remove,
-	.suspend = w1_gpio_suspend,
-	.resume = w1_gpio_resume,
 };
 
 module_platform_driver(w1_gpio_driver);
-- 
1.7.11.7

[PATCH 3/5] W1: w1-gpio - guard DT IDs with CONFIG_OF

[Dmitry Torokhov]

This fixes the following warning:

  CC      drivers/w1/masters/w1-gpio.o
drivers/w1/masters/w1-gpio.c:50:28: warning: ‘w1_gpio_dt_ids’ defined but not used [-Wunused-variable]

Also provide stub for w1_gpio_probe_dt() if device tree support is
disabled.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/w1/masters/w1-gpio.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/w1/masters/w1-gpio.c b/drivers/w1/masters/w1-gpio.c
index c45b9ae..aa97a96 100644
--- a/drivers/w1/masters/w1-gpio.c
+++ b/drivers/w1/masters/w1-gpio.c
@@ -47,6 +47,8 @@ static u8 w1_gpio_read_bit(void *data)
 	return gpio_get_value(pdata->pin) ? 1 : 0;
 }
 
+#ifdef CONFIG_OF
+
 static struct of_device_id w1_gpio_dt_ids[] = {
 	{ .compatible = "w1-gpio" },
 	{}
@@ -72,6 +74,15 @@ static int w1_gpio_probe_dt(struct platform_device *pdev)
 	return 0;
 }
 
+#else
+
+static inline int w1_gpio_probe_dt(struct platform_device *pdev)
+{
+	return -ENOSYS;
+}
+
+#endif
+
 static int w1_gpio_probe(struct platform_device *pdev)
 {
 	struct w1_bus_master *master;
-- 
1.7.11.7

[PATCH 4/5] W1: w1-gpio - rework handling of platform data

[Dmitry Torokhov]

The platform data in the dveice structure does not belong to the driver
and so it should not be trying to alter it, but instead use a local pointer
and populate it with a local copy in case we are dealing with device tree
setup.

Also allow mixed setups where platform data coexists with device tree and
prefer kernel-supplied data (it may be easier to fiddle in kernel structure
before committing final result to device tree).

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/w1/masters/w1-gpio.c | 93 +++++++++++++++++++++++++-------------------
 1 file changed, 53 insertions(+), 40 deletions(-)

diff --git a/drivers/w1/masters/w1-gpio.c b/drivers/w1/masters/w1-gpio.c
index aa97a96..ee6b6e3 100644
--- a/drivers/w1/masters/w1-gpio.c
+++ b/drivers/w1/masters/w1-gpio.c
@@ -23,28 +23,33 @@
 #include "../w1.h"
 #include "../w1_int.h"
 
+struct w1_gpio {
+	struct w1_bus_master master;
+	const struct w1_gpio_platform_data *pdata;
+};
+
 static void w1_gpio_write_bit_dir(void *data, u8 bit)
 {
-	struct w1_gpio_platform_data *pdata = data;
+	struct w1_gpio *w1_gpio = data;
 
 	if (bit)
-		gpio_direction_input(pdata->pin);
+		gpio_direction_input(w1_gpio->pdata->pin);
 	else
-		gpio_direction_output(pdata->pin, 0);
+		gpio_direction_output(w1_gpio->pdata->pin, 0);
 }
 
 static void w1_gpio_write_bit_val(void *data, u8 bit)
 {
-	struct w1_gpio_platform_data *pdata = data;
+	struct w1_gpio *w1_gpio = data;
 
-	gpio_set_value(pdata->pin, bit);
+	gpio_set_value(w1_gpio->pdata->pin, bit);
 }
 
 static u8 w1_gpio_read_bit(void *data)
 {
-	struct w1_gpio_platform_data *pdata = data;
+	struct w1_gpio *w1_gpio = data;
 
-	return gpio_get_value(pdata->pin) ? 1 : 0;
+	return gpio_get_value(w1_gpio->pdata->pin) ? 1 : 0;
 }
 
 #ifdef CONFIG_OF
@@ -55,38 +60,43 @@ static struct of_device_id w1_gpio_dt_ids[] = {
 };
 MODULE_DEVICE_TABLE(of, w1_gpio_dt_ids);
 
-static int w1_gpio_probe_dt(struct platform_device *pdev)
+static struct w1_gpio_platform_data *
+w1_gpio_probe_dt(struct platform_device *pdev)
 {
-	struct w1_gpio_platform_data *pdata = pdev->dev.platform_data;
+	struct w1_gpio_platform_data *pdata;
 	struct device_node *np = pdev->dev.of_node;
 
+	if (!np)
+		return ERR_PTR(-ENOENT);
+
 	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
 	if (!pdata)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 
 	if (of_get_property(np, "linux,open-drain", NULL))
 		pdata->is_open_drain = 1;
 
 	pdata->pin = of_get_gpio(np, 0);
 	pdata->ext_pullup_enable_pin = of_get_gpio(np, 1);
-	pdev->dev.platform_data = pdata;
 
-	return 0;
+	return pdata;
 }
 
 #else
 
-static inline int w1_gpio_probe_dt(struct platform_device *pdev)
+static inline struct w1_gpio_platform_data *
+w1_gpio_probe_dt(struct platform_device *pdev)
 {
-	return -ENOSYS;
+	return NULL;
 }
 
 #endif
 
 static int w1_gpio_probe(struct platform_device *pdev)
 {
-	struct w1_bus_master *master;
-	struct w1_gpio_platform_data *pdata;
+	const struct w1_gpio_platform_data *pdata =
+					dev_get_platdata(&pdev->dev);
+	struct w1_gpio *w1_gpio;
 	struct pinctrl *pinctrl;
 	int err;
 
@@ -94,23 +104,20 @@ static int w1_gpio_probe(struct platform_device *pdev)
 	if (IS_ERR(pinctrl))
 		dev_warn(&pdev->dev, "unable to select pin group\n");
 
-	if (of_have_populated_dt()) {
-		err = w1_gpio_probe_dt(pdev);
-		if (err < 0) {
-			dev_err(&pdev->dev, "Failed to parse DT\n");
-			return err;
-		}
-	}
-
-	pdata = pdev->dev.platform_data;
+	if (!pdata)
+		pdata = w1_gpio_probe_dt(pdev);
 
 	if (!pdata) {
 		dev_err(&pdev->dev, "No configuration data\n");
 		return -ENXIO;
+	} else if (IS_ERR(pdata)) {
+		err = PTR_ERR(pdata);
+		dev_err(&pdev->dev, "Failed to parse DT\n");
+		return err;
 	}
 
-	master = kzalloc(sizeof(struct w1_bus_master), GFP_KERNEL);
-	if (!master) {
+	w1_gpio = kzalloc(sizeof(struct w1_gpio), GFP_KERNEL);
+	if (!w1_gpio) {
 		dev_err(&pdev->dev, "Out of memory\n");
 		return -ENOMEM;
 	}
@@ -131,18 +138,20 @@ static int w1_gpio_probe(struct platform_device *pdev)
 		}
 	}
 
-	master->data = pdata;
-	master->read_bit = w1_gpio_read_bit;
+	w1_gpio->pdata = pdata;
+
+	w1_gpio->master.data = w1_gpio;
+	w1_gpio->master.read_bit = w1_gpio_read_bit;
 
 	if (pdata->is_open_drain) {
 		gpio_direction_output(pdata->pin, 1);
-		master->write_bit = w1_gpio_write_bit_val;
+		w1_gpio->master.write_bit = w1_gpio_write_bit_val;
 	} else {
 		gpio_direction_input(pdata->pin);
-		master->write_bit = w1_gpio_write_bit_dir;
+		w1_gpio->master.write_bit = w1_gpio_write_bit_dir;
 	}
 
-	err = w1_add_master_device(master);
+	err = w1_add_master_device(&w1_gpio->master);
 	if (err) {
 		dev_err(&pdev->dev, "w1_add_master device failed\n");
 		goto free_gpio_ext_pu;
@@ -154,7 +163,7 @@ static int w1_gpio_probe(struct platform_device *pdev)
 	if (gpio_is_valid(pdata->ext_pullup_enable_pin))
 		gpio_set_value(pdata->ext_pullup_enable_pin, 1);
 
-	platform_set_drvdata(pdev, master);
+	platform_set_drvdata(pdev, w1_gpio);
 
 	return 0;
 
@@ -164,15 +173,15 @@ static int w1_gpio_probe(struct platform_device *pdev)
  free_gpio:
 	gpio_free(pdata->pin);
  free_master:
-	kfree(master);
+	kfree(w1_gpio);
 
 	return err;
 }
 
 static int w1_gpio_remove(struct platform_device *pdev)
 {
-	struct w1_bus_master *master = platform_get_drvdata(pdev);
-	struct w1_gpio_platform_data *pdata = pdev->dev.platform_data;
+	struct w1_gpio *w1_gpio = platform_get_drvdata(pdev);
+	const struct w1_gpio_platform_data *pdata = w1_gpio->pdata;
 
 	if (pdata->enable_external_pullup)
 		pdata->enable_external_pullup(0);
@@ -180,9 +189,9 @@ static int w1_gpio_remove(struct platform_device *pdev)
 	if (gpio_is_valid(pdata->ext_pullup_enable_pin))
 		gpio_set_value(pdata->ext_pullup_enable_pin, 0);
 
-	w1_remove_master_device(master);
+	w1_remove_master_device(&w1_gpio->master);
 	gpio_free(pdata->pin);
-	kfree(master);
+	kfree(w1_gpio);
 
 	return 0;
 }
@@ -190,7 +199,9 @@ static int w1_gpio_remove(struct platform_device *pdev)
 #ifdef CONFIG_PM_SLEEP
 static int w1_gpio_suspend(struct device *dev)
 {
-	const struct w1_gpio_platform_data *pdata = dev_get_platdata(dev);
+	struct platform_device *pdev = to_platform_device(dev);
+	struct w1_gpio *w1_gpio = platform_get_drvdata(pdev);
+	const struct w1_gpio_platform_data *pdata = w1_gpio->pdata;
 
 	if (pdata->enable_external_pullup)
 		pdata->enable_external_pullup(0);
@@ -200,7 +211,9 @@ static int w1_gpio_suspend(struct device *dev)
 
 static int w1_gpio_resume(struct device *dev)
 {
-	const struct w1_gpio_platform_data *pdata = dev_get_platdata(dev);
+	struct platform_device *pdev = to_platform_device(dev);
+	struct w1_gpio *w1_gpio = platform_get_drvdata(pdev);
+	const struct w1_gpio_platform_data *pdata = w1_gpio->pdata;
 
 	if (pdata->enable_external_pullup)
 		pdata->enable_external_pullup(1);
-- 
1.7.11.7

[PATCH 5/5] W1: w1-gpio - switch to using managed resources (devm)

[Dmitry Torokhov]

This simplifies error unwinding and device teardown.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/w1/masters/w1-gpio.c | 31 ++++++++++---------------------
 1 file changed, 10 insertions(+), 21 deletions(-)

diff --git a/drivers/w1/masters/w1-gpio.c b/drivers/w1/masters/w1-gpio.c
index ee6b6e3..19439f8 100644
--- a/drivers/w1/masters/w1-gpio.c
+++ b/drivers/w1/masters/w1-gpio.c
@@ -116,25 +116,26 @@ static int w1_gpio_probe(struct platform_device *pdev)
 		return err;
 	}
 
-	w1_gpio = kzalloc(sizeof(struct w1_gpio), GFP_KERNEL);
+	w1_gpio = devm_kzalloc(&pdev->dev, sizeof(struct w1_gpio), GFP_KERNEL);
 	if (!w1_gpio) {
 		dev_err(&pdev->dev, "Out of memory\n");
 		return -ENOMEM;
 	}
 
-	err = gpio_request(pdata->pin, "w1");
+	err = devm_gpio_request(&pdev->dev, pdata->pin, "w1");
 	if (err) {
 		dev_err(&pdev->dev, "gpio_request (pin) failed\n");
-		goto free_master;
+		return err;
 	}
 
 	if (gpio_is_valid(pdata->ext_pullup_enable_pin)) {
-		err = gpio_request_one(pdata->ext_pullup_enable_pin,
-				       GPIOF_INIT_LOW, "w1 pullup");
+		err = devm_gpio_request_one(&pdev->dev,
+					    pdata->ext_pullup_enable_pin,
+					    GPIOF_INIT_LOW, "w1 pullup");
 		if (err < 0) {
-			dev_err(&pdev->dev, "gpio_request_one "
-					"(ext_pullup_enable_pin) failed\n");
-			goto free_gpio;
+			dev_err(&pdev->dev,
+				"gpio_request_one (ext_pullup_enable_pin) failed\n");
+			return err;
 		}
 	}
 
@@ -154,7 +155,7 @@ static int w1_gpio_probe(struct platform_device *pdev)
 	err = w1_add_master_device(&w1_gpio->master);
 	if (err) {
 		dev_err(&pdev->dev, "w1_add_master device failed\n");
-		goto free_gpio_ext_pu;
+		return err;
 	}
 
 	if (pdata->enable_external_pullup)
@@ -166,16 +167,6 @@ static int w1_gpio_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, w1_gpio);
 
 	return 0;
-
- free_gpio_ext_pu:
-	if (gpio_is_valid(pdata->ext_pullup_enable_pin))
-		gpio_free(pdata->ext_pullup_enable_pin);
- free_gpio:
-	gpio_free(pdata->pin);
- free_master:
-	kfree(w1_gpio);
-
-	return err;
 }
 
 static int w1_gpio_remove(struct platform_device *pdev)
@@ -190,8 +181,6 @@ static int w1_gpio_remove(struct platform_device *pdev)
 		gpio_set_value(pdata->ext_pullup_enable_pin, 0);
 
 	w1_remove_master_device(&w1_gpio->master);
-	gpio_free(pdata->pin);
-	kfree(w1_gpio);
 
 	return 0;
 }
-- 
1.7.11.7

Re: [PATCH 4/5] W1: w1-gpio - rework handling of platform data

[Ville Syrjälä]

On Fri, Feb 22, 2013 at 11:58:39PM -0800, Dmitry Torokhov wrote:
> The platform data in the dveice structure does not belong to the driver
> and so it should not be trying to alter it, but instead use a local pointer
> and populate it with a local copy in case we are dealing with device tree
> setup.
> 
> Also allow mixed setups where platform data coexists with device tree and
> prefer kernel-supplied data (it may be easier to fiddle in kernel structure
> before committing final result to device tree).
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>  drivers/w1/masters/w1-gpio.c | 93 +++++++++++++++++++++++++-------------------
>  1 file changed, 53 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/w1/masters/w1-gpio.c b/drivers/w1/masters/w1-gpio.c
> index aa97a96..ee6b6e3 100644
> --- a/drivers/w1/masters/w1-gpio.c
> +++ b/drivers/w1/masters/w1-gpio.c
> @@ -23,28 +23,33 @@
>  #include "../w1.h"
>  #include "../w1_int.h"
>  
> +struct w1_gpio {
> +	struct w1_bus_master master;
> +	const struct w1_gpio_platform_data *pdata;
> +};

I don't understand why you need this wrapper struct. It doesn't
seem to serve much purpose, other than to cause code churn in
the patch.

But feel free to ignore me if you think it's useful. get_maintainer.pl
doesn't even list me anymore, and that's a good thing since I haven't
even used this driver in years.

-- 
Ville Syrjälä
syrjala@sci.fi
http://www.sci.fi/~syrjala/

Re: [PATCH 4/5] W1: w1-gpio - rework handling of platform data

[Dmitry Torokhov]

On Sat, Feb 23, 2013 at 06:55:45PM +0200, Ville Syrjälä wrote:
> On Fri, Feb 22, 2013 at 11:58:39PM -0800, Dmitry Torokhov wrote:
> > The platform data in the dveice structure does not belong to the driver
> > and so it should not be trying to alter it, but instead use a local pointer
> > and populate it with a local copy in case we are dealing with device tree
> > setup.
> > 
> > Also allow mixed setups where platform data coexists with device tree and
> > prefer kernel-supplied data (it may be easier to fiddle in kernel structure
> > before committing final result to device tree).
> > 
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > ---
> >  drivers/w1/masters/w1-gpio.c | 93 +++++++++++++++++++++++++-------------------
> >  1 file changed, 53 insertions(+), 40 deletions(-)
> > 
> > diff --git a/drivers/w1/masters/w1-gpio.c b/drivers/w1/masters/w1-gpio.c
> > index aa97a96..ee6b6e3 100644
> > --- a/drivers/w1/masters/w1-gpio.c
> > +++ b/drivers/w1/masters/w1-gpio.c
> > @@ -23,28 +23,33 @@
> >  #include "../w1.h"
> >  #include "../w1_int.h"
> >  
> > +struct w1_gpio {
> > +	struct w1_bus_master master;
> > +	const struct w1_gpio_platform_data *pdata;
> > +};
> 
> I don't understand why you need this wrapper struct. It doesn't
> seem to serve much purpose, other than to cause code churn in
> the patch.

Hm, yes, you are right, I should be able to get rid of it...

Thanks.

-- 
Dmitry

[PATCH v2 1/5] W1: w1-gpio - fix incorrect __init/__exit markups

[Dmitry Torokhov]

We should not be using __init/__exit markups on probe() and remove()
methods unless platform_device_probe() is used, because other methods
allow unbinding device through sysfs and these methods should not be
discarded.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---

Paatch #4 was adjusted according to Ville's comments.

 drivers/w1/masters/w1-gpio.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/w1/masters/w1-gpio.c b/drivers/w1/masters/w1-gpio.c
index 85b363a..799dafd 100644
--- a/drivers/w1/masters/w1-gpio.c
+++ b/drivers/w1/masters/w1-gpio.c
@@ -72,7 +72,7 @@ static int w1_gpio_probe_dt(struct platform_device *pdev)
 	return 0;
 }
 
-static int __init w1_gpio_probe(struct platform_device *pdev)
+static int w1_gpio_probe(struct platform_device *pdev)
 {
 	struct w1_bus_master *master;
 	struct w1_gpio_platform_data *pdata;
@@ -158,7 +158,7 @@ static int __init w1_gpio_probe(struct platform_device *pdev)
 	return err;
 }
 
-static int __exit w1_gpio_remove(struct platform_device *pdev)
+static int w1_gpio_remove(struct platform_device *pdev)
 {
 	struct w1_bus_master *master = platform_get_drvdata(pdev);
 	struct w1_gpio_platform_data *pdata = pdev->dev.platform_data;
@@ -210,7 +210,7 @@ static struct platform_driver w1_gpio_driver = {
 		.of_match_table = of_match_ptr(w1_gpio_dt_ids),
 	},
 	.probe = w1_gpio_probe,
-	.remove	= __exit_p(w1_gpio_remove),
+	.remove = w1_gpio_remove,
 	.suspend = w1_gpio_suspend,
 	.resume = w1_gpio_resume,
 };
-- 
1.7.11.7

[PATCH 2/5] W1: w1-gpio - switch to using dev_pm_ops

[Dmitry Torokhov]

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/w1/masters/w1-gpio.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/w1/masters/w1-gpio.c b/drivers/w1/masters/w1-gpio.c
index 799dafd..c45b9ae 100644
--- a/drivers/w1/masters/w1-gpio.c
+++ b/drivers/w1/masters/w1-gpio.c
@@ -176,11 +176,10 @@ static int w1_gpio_remove(struct platform_device *pdev)
 	return 0;
 }
 
-#ifdef CONFIG_PM
-
-static int w1_gpio_suspend(struct platform_device *pdev, pm_message_t state)
+#ifdef CONFIG_PM_SLEEP
+static int w1_gpio_suspend(struct device *dev)
 {
-	struct w1_gpio_platform_data *pdata = pdev->dev.platform_data;
+	const struct w1_gpio_platform_data *pdata = dev_get_platdata(dev);
 
 	if (pdata->enable_external_pullup)
 		pdata->enable_external_pullup(0);
@@ -188,31 +187,28 @@ static int w1_gpio_suspend(struct platform_device *pdev, pm_message_t state)
 	return 0;
 }
 
-static int w1_gpio_resume(struct platform_device *pdev)
+static int w1_gpio_resume(struct device *dev)
 {
-	struct w1_gpio_platform_data *pdata = pdev->dev.platform_data;
+	const struct w1_gpio_platform_data *pdata = dev_get_platdata(dev);
 
 	if (pdata->enable_external_pullup)
 		pdata->enable_external_pullup(1);
 
 	return 0;
 }
-
-#else
-#define w1_gpio_suspend	NULL
-#define w1_gpio_resume	NULL
 #endif
 
+static SIMPLE_DEV_PM_OPS(w1_gpio_pm_ops, w1_gpio_suspend, w1_gpio_resume);
+
 static struct platform_driver w1_gpio_driver = {
 	.driver = {
 		.name	= "w1-gpio",
 		.owner	= THIS_MODULE,
+		.pm	= &w1_gpio_pm_ops,
 		.of_match_table = of_match_ptr(w1_gpio_dt_ids),
 	},
 	.probe = w1_gpio_probe,
 	.remove = w1_gpio_remove,
-	.suspend = w1_gpio_suspend,
-	.resume = w1_gpio_resume,
 };
 
 module_platform_driver(w1_gpio_driver);
-- 
1.7.11.7

[PATCH 3/5] W1: w1-gpio - guard DT IDs with CONFIG_OF

[Dmitry Torokhov]

This fixes the following warning:

  CC      drivers/w1/masters/w1-gpio.o
drivers/w1/masters/w1-gpio.c:50:28: warning: ‘w1_gpio_dt_ids’ defined but not used [-Wunused-variable]

Also provide stub for w1_gpio_probe_dt() if device tree support is
disabled.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/w1/masters/w1-gpio.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/w1/masters/w1-gpio.c b/drivers/w1/masters/w1-gpio.c
index c45b9ae..aa97a96 100644
--- a/drivers/w1/masters/w1-gpio.c
+++ b/drivers/w1/masters/w1-gpio.c
@@ -47,6 +47,8 @@ static u8 w1_gpio_read_bit(void *data)
 	return gpio_get_value(pdata->pin) ? 1 : 0;
 }
 
+#ifdef CONFIG_OF
+
 static struct of_device_id w1_gpio_dt_ids[] = {
 	{ .compatible = "w1-gpio" },
 	{}
@@ -72,6 +74,15 @@ static int w1_gpio_probe_dt(struct platform_device *pdev)
 	return 0;
 }
 
+#else
+
+static inline int w1_gpio_probe_dt(struct platform_device *pdev)
+{
+	return -ENOSYS;
+}
+
+#endif
+
 static int w1_gpio_probe(struct platform_device *pdev)
 {
 	struct w1_bus_master *master;
-- 
1.7.11.7

[PATCH 4/5] W1: w1-gpio - rework handling of platform data

[Dmitry Torokhov]

The platform data in the dveice structure does not belong to the driver
and so it should not be trying to alter it, but instead use a local pointer
and populate it with a local copy in case we are dealing with device tree
setup.

Also allow mixed setups where platform data coexists with device tree and
prefer kernel-supplied data (it may be easier to fiddle in kernel structure
before committing final result to device tree).

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/w1/masters/w1-gpio.c | 44 +++++++++++++++++++++++++-------------------
 1 file changed, 25 insertions(+), 19 deletions(-)

diff --git a/drivers/w1/masters/w1-gpio.c b/drivers/w1/masters/w1-gpio.c
index aa97a96..465ce52 100644
--- a/drivers/w1/masters/w1-gpio.c
+++ b/drivers/w1/masters/w1-gpio.c
@@ -55,30 +55,34 @@ static struct of_device_id w1_gpio_dt_ids[] = {
 };
 MODULE_DEVICE_TABLE(of, w1_gpio_dt_ids);
 
-static int w1_gpio_probe_dt(struct platform_device *pdev)
+static struct w1_gpio_platform_data *
+w1_gpio_probe_dt(struct platform_device *pdev)
 {
-	struct w1_gpio_platform_data *pdata = pdev->dev.platform_data;
+	struct w1_gpio_platform_data *pdata;
 	struct device_node *np = pdev->dev.of_node;
 
+	if (!np)
+		return ERR_PTR(-ENOENT);
+
 	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
 	if (!pdata)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 
 	if (of_get_property(np, "linux,open-drain", NULL))
 		pdata->is_open_drain = 1;
 
 	pdata->pin = of_get_gpio(np, 0);
 	pdata->ext_pullup_enable_pin = of_get_gpio(np, 1);
-	pdev->dev.platform_data = pdata;
 
-	return 0;
+	return pdata;
 }
 
 #else
 
-static inline int w1_gpio_probe_dt(struct platform_device *pdev)
+static inline struct w1_gpio_platform_data *
+w1_gpio_probe_dt(struct platform_device *pdev)
 {
-	return -ENOSYS;
+	return NULL;
 }
 
 #endif
@@ -94,19 +98,17 @@ static int w1_gpio_probe(struct platform_device *pdev)
 	if (IS_ERR(pinctrl))
 		dev_warn(&pdev->dev, "unable to select pin group\n");
 
-	if (of_have_populated_dt()) {
-		err = w1_gpio_probe_dt(pdev);
-		if (err < 0) {
-			dev_err(&pdev->dev, "Failed to parse DT\n");
-			return err;
-		}
-	}
-
-	pdata = pdev->dev.platform_data;
+	pdata = dev_get_platdata(&pdev->dev);
+	if (!pdata)
+		pdata = w1_gpio_probe_dt(pdev);
 
 	if (!pdata) {
 		dev_err(&pdev->dev, "No configuration data\n");
 		return -ENXIO;
+	} else if (IS_ERR(pdata)) {
+		err = PTR_ERR(pdata);
+		dev_err(&pdev->dev, "Failed to parse DT\n");
+		return err;
 	}
 
 	master = kzalloc(sizeof(struct w1_bus_master), GFP_KERNEL);
@@ -172,7 +174,7 @@ static int w1_gpio_probe(struct platform_device *pdev)
 static int w1_gpio_remove(struct platform_device *pdev)
 {
 	struct w1_bus_master *master = platform_get_drvdata(pdev);
-	struct w1_gpio_platform_data *pdata = pdev->dev.platform_data;
+	const struct w1_gpio_platform_data *pdata = master->data;
 
 	if (pdata->enable_external_pullup)
 		pdata->enable_external_pullup(0);
@@ -190,7 +192,9 @@ static int w1_gpio_remove(struct platform_device *pdev)
 #ifdef CONFIG_PM_SLEEP
 static int w1_gpio_suspend(struct device *dev)
 {
-	const struct w1_gpio_platform_data *pdata = dev_get_platdata(dev);
+	struct platform_device *pdev = to_platform_device(dev);
+	struct w1_bus_master *master = platform_get_drvdata(pdev);
+	const struct w1_gpio_platform_data *pdata = master->data;
 
 	if (pdata->enable_external_pullup)
 		pdata->enable_external_pullup(0);
@@ -200,7 +204,9 @@ static int w1_gpio_suspend(struct device *dev)
 
 static int w1_gpio_resume(struct device *dev)
 {
-	const struct w1_gpio_platform_data *pdata = dev_get_platdata(dev);
+	struct platform_device *pdev = to_platform_device(dev);
+	struct w1_bus_master *master = platform_get_drvdata(pdev);
+	const struct w1_gpio_platform_data *pdata = master->data;
 
 	if (pdata->enable_external_pullup)
 		pdata->enable_external_pullup(1);
-- 
1.7.11.7

[PATCH 5/5] W1: w1-gpio - switch to using managed resources (devm)

[Dmitry Torokhov]

This simplifies error unwinding and device teardown.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/w1/masters/w1-gpio.c | 32 +++++++++++---------------------
 1 file changed, 11 insertions(+), 21 deletions(-)

diff --git a/drivers/w1/masters/w1-gpio.c b/drivers/w1/masters/w1-gpio.c
index 465ce52..464b1a8 100644
--- a/drivers/w1/masters/w1-gpio.c
+++ b/drivers/w1/masters/w1-gpio.c
@@ -111,25 +111,27 @@ static int w1_gpio_probe(struct platform_device *pdev)
 		return err;
 	}
 
-	master = kzalloc(sizeof(struct w1_bus_master), GFP_KERNEL);
+	master = devm_kzalloc(&pdev->dev,
+			      sizeof(struct w1_bus_master), GFP_KERNEL);
 	if (!master) {
 		dev_err(&pdev->dev, "Out of memory\n");
 		return -ENOMEM;
 	}
 
-	err = gpio_request(pdata->pin, "w1");
+	err = devm_gpio_request(&pdev->dev, pdata->pin, "w1");
 	if (err) {
 		dev_err(&pdev->dev, "gpio_request (pin) failed\n");
-		goto free_master;
+		return err;
 	}
 
 	if (gpio_is_valid(pdata->ext_pullup_enable_pin)) {
-		err = gpio_request_one(pdata->ext_pullup_enable_pin,
-				       GPIOF_INIT_LOW, "w1 pullup");
+		err = devm_gpio_request_one(&pdev->dev,
+					    pdata->ext_pullup_enable_pin,
+					    GPIOF_INIT_LOW, "w1 pullup");
 		if (err < 0) {
-			dev_err(&pdev->dev, "gpio_request_one "
-					"(ext_pullup_enable_pin) failed\n");
-			goto free_gpio;
+			dev_err(&pdev->dev,
+				"gpio_request_one (ext_pullup_enable_pin) failed\n");
+			return err;
 		}
 	}
 
@@ -147,7 +149,7 @@ static int w1_gpio_probe(struct platform_device *pdev)
 	err = w1_add_master_device(master);
 	if (err) {
 		dev_err(&pdev->dev, "w1_add_master device failed\n");
-		goto free_gpio_ext_pu;
+		return err;
 	}
 
 	if (pdata->enable_external_pullup)
@@ -159,16 +161,6 @@ static int w1_gpio_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, master);
 
 	return 0;
-
- free_gpio_ext_pu:
-	if (gpio_is_valid(pdata->ext_pullup_enable_pin))
-		gpio_free(pdata->ext_pullup_enable_pin);
- free_gpio:
-	gpio_free(pdata->pin);
- free_master:
-	kfree(master);
-
-	return err;
 }
 
 static int w1_gpio_remove(struct platform_device *pdev)
@@ -183,8 +175,6 @@ static int w1_gpio_remove(struct platform_device *pdev)
 		gpio_set_value(pdata->ext_pullup_enable_pin, 0);
 
 	w1_remove_master_device(master);
-	gpio_free(pdata->pin);
-	kfree(master);
 
 	return 0;
 }
-- 
1.7.11.7

Re: [PATCH v2 1/5] W1: w1-gpio - fix incorrect __init/__exit markups

[Greg Kroah-Hartman]

On Sun, Feb 24, 2013 at 10:59:33PM -0800, Dmitry Torokhov wrote:
> We should not be using __init/__exit markups on probe() and remove()
> methods unless platform_device_probe() is used, because other methods
> allow unbinding device through sysfs and these methods should not be
> discarded.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> 
> Paatch #4 was adjusted according to Ville's comments.
> 
>  drivers/w1/masters/w1-gpio.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Thanks, but this is already in the tree.

greg k-h

Re: [PATCH 3/5] W1: w1-gpio - guard DT IDs with CONFIG_OF

[Greg Kroah-Hartman]

On Sun, Feb 24, 2013 at 10:59:35PM -0800, Dmitry Torokhov wrote:
> This fixes the following warning:
> 
>   CC      drivers/w1/masters/w1-gpio.o
> drivers/w1/masters/w1-gpio.c:50:28: warning: ‘w1_gpio_dt_ids’ defined but not used [-Wunused-variable]
> 
> Also provide stub for w1_gpio_probe_dt() if device tree support is
> disabled.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>  drivers/w1/masters/w1-gpio.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)

Already fixed.