[PATCH 01/11] mfd: 88pm860x: drop devm_kfree of devm_kzalloc'd data

[PATCH 01/11] mfd: 88pm860x: drop devm_kfree of devm_kzalloc'd data

[Jingoo Han]

devm_kfree() allocates memory that is released when a driver detaches.
Thus, there is no reason to explicitly call devm_kfree() in probe or remove
functions.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/mfd/88pm860x-core.c |   12 +++---------
 1 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/mfd/88pm860x-core.c b/drivers/mfd/88pm860x-core.c
index 893fc1b..31ca555 100644
--- a/drivers/mfd/88pm860x-core.c
+++ b/drivers/mfd/88pm860x-core.c
@@ -1144,17 +1144,15 @@ static int pm860x_probe(struct i2c_client *client,
 			return -ENOMEM;
 		ret = pm860x_dt_init(node, &client->dev, pdata);
 		if (ret)
-			goto err;
+			return ret;
 	} else if (!pdata) {
 		pr_info("No platform data in %s!\n", __func__);
 		return -EINVAL;
 	}
 
 	chip = kzalloc(sizeof(struct pm860x_chip), GFP_KERNEL);
-	if (chip == NULL) {
-		ret = -ENOMEM;
-		goto err;
-	}
+	if (chip == NULL)
+		return -ENOMEM;
 
 	chip->id = verify_addr(client);
 	chip->regmap = regmap_init_i2c(client, &pm860x_regmap_config);
@@ -1194,10 +1192,6 @@ static int pm860x_probe(struct i2c_client *client,
 
 	pm860x_device_init(chip, pdata);
 	return 0;
-err:
-	if (node)
-		devm_kfree(&client->dev, pdata);
-	return ret;
 }
 
 static int pm860x_remove(struct i2c_client *client)
-- 
1.7.2.5

[PATCH 02/11] mfd: lm3533: use devm_gpio_request_one()

[Jingoo Han]

Use devm_gpio_request_one() to make cleanup paths more simple.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/mfd/lm3533-core.c |    8 ++------
 1 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/mfd/lm3533-core.c b/drivers/mfd/lm3533-core.c
index ceebf2c..4b7e6da 100644
--- a/drivers/mfd/lm3533-core.c
+++ b/drivers/mfd/lm3533-core.c
@@ -496,8 +496,8 @@ static int lm3533_device_init(struct lm3533 *lm3533)
 	dev_set_drvdata(lm3533->dev, lm3533);
 
 	if (gpio_is_valid(lm3533->gpio_hwen)) {
-		ret = gpio_request_one(lm3533->gpio_hwen, GPIOF_OUT_INIT_LOW,
-								"lm3533-hwen");
+		ret = devm_gpio_request_one(lm3533->dev, lm3533->gpio_hwen,
+					GPIOF_OUT_INIT_LOW, "lm3533-hwen");
 		if (ret < 0) {
 			dev_err(lm3533->dev,
 				"failed to request HWEN GPIO %d\n",
@@ -528,8 +528,6 @@ err_unregister:
 	mfd_remove_devices(lm3533->dev);
 err_disable:
 	lm3533_disable(lm3533);
-	if (gpio_is_valid(lm3533->gpio_hwen))
-		gpio_free(lm3533->gpio_hwen);
 
 	return ret;
 }
@@ -542,8 +540,6 @@ static void lm3533_device_exit(struct lm3533 *lm3533)
 
 	mfd_remove_devices(lm3533->dev);
 	lm3533_disable(lm3533);
-	if (gpio_is_valid(lm3533->gpio_hwen))
-		gpio_free(lm3533->gpio_hwen);
 }
 
 static bool lm3533_readable_register(struct device *dev, unsigned int reg)
-- 
1.7.2.5

[PATCH 03/11] mfd: aat2870: use devm_gpio_request_one() and devm_kzalloc()

[Jingoo Han]

Use devm_gpio_request_one() and devm_kzalloc() to make cleanup paths
more simple.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/mfd/aat2870-core.c |   20 ++++++--------------
 1 files changed, 6 insertions(+), 14 deletions(-)

diff --git a/drivers/mfd/aat2870-core.c b/drivers/mfd/aat2870-core.c
index f1beb49..dfdb0a2 100644
--- a/drivers/mfd/aat2870-core.c
+++ b/drivers/mfd/aat2870-core.c
@@ -367,12 +367,12 @@ static int aat2870_i2c_probe(struct i2c_client *client,
 	int i, j;
 	int ret = 0;
 
-	aat2870 = kzalloc(sizeof(struct aat2870_data), GFP_KERNEL);
+	aat2870 = devm_kzalloc(&client->dev, sizeof(struct aat2870_data),
+				GFP_KERNEL);
 	if (!aat2870) {
 		dev_err(&client->dev,
 			"Failed to allocate memory for aat2870\n");
-		ret = -ENOMEM;
-		goto out;
+		return -ENOMEM;
 	}
 
 	aat2870->dev = &client->dev;
@@ -400,12 +400,12 @@ static int aat2870_i2c_probe(struct i2c_client *client,
 		aat2870->init(aat2870);
 
 	if (aat2870->en_pin >= 0) {
-		ret = gpio_request_one(aat2870->en_pin, GPIOF_OUT_INIT_HIGH,
-				       "aat2870-en");
+		ret = devm_gpio_request_one(&client->dev, aat2870->en_pin,
+					GPIOF_OUT_INIT_HIGH, "aat2870-en");
 		if (ret < 0) {
 			dev_err(&client->dev,
 				"Failed to request GPIO %d\n", aat2870->en_pin);
-			goto out_kfree;
+			return ret;
 		}
 	}
 
@@ -436,11 +436,6 @@ static int aat2870_i2c_probe(struct i2c_client *client,
 
 out_disable:
 	aat2870_disable(aat2870);
-	if (aat2870->en_pin >= 0)
-		gpio_free(aat2870->en_pin);
-out_kfree:
-	kfree(aat2870);
-out:
 	return ret;
 }
 
@@ -452,11 +447,8 @@ static int aat2870_i2c_remove(struct i2c_client *client)
 
 	mfd_remove_devices(aat2870->dev);
 	aat2870_disable(aat2870);
-	if (aat2870->en_pin >= 0)
-		gpio_free(aat2870->en_pin);
 	if (aat2870->uninit)
 		aat2870->uninit(aat2870);
-	kfree(aat2870);
 
 	return 0;
 }
-- 
1.7.2.5

[PATCH 04/11] mfd: intel_msic: use devm_gpio_request_one()

[Jingoo Han]

Use devm_gpio_request_one() to make cleanup paths more simple.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/mfd/intel_msic.c |   10 ++--------
 1 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/mfd/intel_msic.c b/drivers/mfd/intel_msic.c
index 1804331..5be3b5e 100644
--- a/drivers/mfd/intel_msic.c
+++ b/drivers/mfd/intel_msic.c
@@ -323,7 +323,8 @@ static int intel_msic_init_devices(struct intel_msic *msic)
 	if (pdata->ocd) {
 		unsigned gpio = pdata->ocd->gpio;
 
-		ret = gpio_request_one(gpio, GPIOF_IN, "ocd_gpio");
+		ret = devm_gpio_request_one(&pdev->dev, gpio,
+					GPIOF_IN, "ocd_gpio");
 		if (ret) {
 			dev_err(&pdev->dev, "failed to register OCD GPIO\n");
 			return ret;
@@ -332,7 +333,6 @@ static int intel_msic_init_devices(struct intel_msic *msic)
 		ret = gpio_to_irq(gpio);
 		if (ret < 0) {
 			dev_err(&pdev->dev, "no IRQ number for OCD GPIO\n");
-			gpio_free(gpio);
 			return ret;
 		}
 
@@ -359,8 +359,6 @@ static int intel_msic_init_devices(struct intel_msic *msic)
 
 fail:
 	mfd_remove_devices(&pdev->dev);
-	if (pdata->ocd)
-		gpio_free(pdata->ocd->gpio);
 
 	return ret;
 }
@@ -368,12 +366,8 @@ fail:
 static void intel_msic_remove_devices(struct intel_msic *msic)
 {
 	struct platform_device *pdev = msic->pdev;
-	struct intel_msic_platform_data *pdata = pdev->dev.platform_data;
 
 	mfd_remove_devices(&pdev->dev);
-
-	if (pdata->ocd)
-		gpio_free(pdata->ocd->gpio);
 }
 
 static int intel_msic_probe(struct platform_device *pdev)
-- 
1.7.2.5

[PATCH 05/11] mfd: twl6040: use devm_gpio_request_one() and devm_request_threaded_irq()

[Jingoo Han]

Use devm_gpio_request_one() and devm_request_threaded_irq() to make
cleanup paths and  more simple.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/mfd/twl6040.c |   26 +++++++-------------------
 1 files changed, 7 insertions(+), 19 deletions(-)

diff --git a/drivers/mfd/twl6040.c b/drivers/mfd/twl6040.c
index f361bf3..2d38512 100644
--- a/drivers/mfd/twl6040.c
+++ b/drivers/mfd/twl6040.c
@@ -586,8 +586,8 @@ static int twl6040_probe(struct i2c_client *client,
 		twl6040->audpwron = -EINVAL;
 
 	if (gpio_is_valid(twl6040->audpwron)) {
-		ret = gpio_request_one(twl6040->audpwron, GPIOF_OUT_INIT_LOW,
-				       "audpwron");
+		ret = devm_gpio_request_one(twl6040->dev, twl6040->audpwron,
+					GPIOF_OUT_INIT_LOW, "audpwron");
 		if (ret)
 			goto gpio_err;
 	}
@@ -596,14 +596,14 @@ static int twl6040_probe(struct i2c_client *client,
 			IRQF_ONESHOT, 0, &twl6040_irq_chip,
 			&twl6040->irq_data);
 	if (ret < 0)
-		goto irq_init_err;
+		goto gpio_err;
 
 	twl6040->irq_ready = regmap_irq_get_virq(twl6040->irq_data,
 					       TWL6040_IRQ_READY);
 	twl6040->irq_th = regmap_irq_get_virq(twl6040->irq_data,
 					       TWL6040_IRQ_TH);
 
-	ret = request_threaded_irq(twl6040->irq_ready, NULL,
+	ret = devm_request_threaded_irq(twl6040->dev, twl6040->irq_ready, NULL,
 				   twl6040_readyint_handler, IRQF_ONESHOT,
 				   "twl6040_irq_ready", twl6040);
 	if (ret) {
@@ -611,12 +611,12 @@ static int twl6040_probe(struct i2c_client *client,
 		goto readyirq_err;
 	}
 
-	ret = request_threaded_irq(twl6040->irq_th, NULL,
+	ret = devm_request_threaded_irq(twl6040->dev, twl6040->irq_th, NULL,
 				   twl6040_thint_handler, IRQF_ONESHOT,
 				   "twl6040_irq_th", twl6040);
 	if (ret) {
 		dev_err(twl6040->dev, "Thermal IRQ request failed: %d\n", ret);
-		goto thirq_err;
+		goto readyirq_err;
 	}
 
 	/* dual-access registers controlled by I2C only */
@@ -676,19 +676,12 @@ static int twl6040_probe(struct i2c_client *client,
 	ret = mfd_add_devices(&client->dev, -1, twl6040->cells, children,
 			      NULL, 0, NULL);
 	if (ret)
-		goto mfd_err;
+		goto readyirq_err;
 
 	return 0;
 
-mfd_err:
-	free_irq(twl6040->irq_th, twl6040);
-thirq_err:
-	free_irq(twl6040->irq_ready, twl6040);
 readyirq_err:
 	regmap_del_irq_chip(twl6040->irq, twl6040->irq_data);
-irq_init_err:
-	if (gpio_is_valid(twl6040->audpwron))
-		gpio_free(twl6040->audpwron);
 gpio_err:
 	regulator_bulk_disable(TWL6040_NUM_SUPPLIES, twl6040->supplies);
 power_err:
@@ -706,11 +699,6 @@ static int twl6040_remove(struct i2c_client *client)
 	if (twl6040->power_count)
 		twl6040_power(twl6040, 0);
 
-	if (gpio_is_valid(twl6040->audpwron))
-		gpio_free(twl6040->audpwron);
-
-	free_irq(twl6040->irq_ready, twl6040);
-	free_irq(twl6040->irq_th, twl6040);
 	regmap_del_irq_chip(twl6040->irq, twl6040->irq_data);
 
 	mfd_remove_devices(&client->dev);
-- 
1.7.2.5

[PATCH 06/11] mfd: omap-usb-host: use devm_gpio_request_one()

[Jingoo Han]

Use devm_gpio_request_one() to make cleanup paths more simple.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/mfd/omap-usb-host.c |   23 ++---------------------
 1 files changed, 2 insertions(+), 21 deletions(-)

diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
index 6b5edf6..2104d66 100644
--- a/drivers/mfd/omap-usb-host.c
+++ b/drivers/mfd/omap-usb-host.c
@@ -437,11 +437,11 @@ static void omap_usbhs_init(struct device *dev)
 
 	if (pdata->phy_reset) {
 		if (gpio_is_valid(pdata->reset_gpio_port[0]))
-			gpio_request_one(pdata->reset_gpio_port[0],
+			devm_gpio_request_one(dev, pdata->reset_gpio_port[0],
 					 GPIOF_OUT_INIT_LOW, "USB1 PHY reset");
 
 		if (gpio_is_valid(pdata->reset_gpio_port[1]))
-			gpio_request_one(pdata->reset_gpio_port[1],
+			devm_gpio_request_one(dev, pdata->reset_gpio_port[1],
 					 GPIOF_OUT_INIT_LOW, "USB2 PHY reset");
 
 		/* Hold the PHY in RESET for enough time till DIR is high */
@@ -492,21 +492,6 @@ static void omap_usbhs_init(struct device *dev)
 	}
 }
 
-static void omap_usbhs_deinit(struct device *dev)
-{
-	struct usbhs_hcd_omap		*omap = dev_get_drvdata(dev);
-	struct usbhs_omap_platform_data	*pdata = omap->pdata;
-
-	if (pdata->phy_reset) {
-		if (gpio_is_valid(pdata->reset_gpio_port[0]))
-			gpio_free(pdata->reset_gpio_port[0]);
-
-		if (gpio_is_valid(pdata->reset_gpio_port[1]))
-			gpio_free(pdata->reset_gpio_port[1]);
-	}
-}
-
-
 /**
  * usbhs_omap_probe - initialize TI-based HCDs
  *
@@ -709,8 +694,6 @@ static int usbhs_omap_probe(struct platform_device *pdev)
 	return 0;
 
 err_alloc:
-	omap_usbhs_deinit(&pdev->dev);
-
 	for (i = 0; i < omap->nports; i++) {
 		if (!IS_ERR(omap->utmi_clk[i]))
 			clk_put(omap->utmi_clk[i]);
@@ -755,8 +738,6 @@ static int usbhs_omap_remove(struct platform_device *pdev)
 	struct usbhs_hcd_omap *omap = platform_get_drvdata(pdev);
 	int i;
 
-	omap_usbhs_deinit(&pdev->dev);
-
 	for (i = 0; i < omap->nports; i++) {
 		if (!IS_ERR(omap->utmi_clk[i]))
 			clk_put(omap->utmi_clk[i]);
-- 
1.7.2.5

[PATCH 07/11] mfd: menelaus: use devm_request_irq() and devm_kzalloc()

[Jingoo Han]

Use devm_request_irq() and devm_kzalloc() to make cleanup paths
more simple.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/mfd/menelaus.c |   23 +++++++++--------------
 1 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/drivers/mfd/menelaus.c b/drivers/mfd/menelaus.c
index 998ce8c..28bac1c 100644
--- a/drivers/mfd/menelaus.c
+++ b/drivers/mfd/menelaus.c
@@ -1197,7 +1197,8 @@ static int menelaus_probe(struct i2c_client *client,
 		return -ENODEV;
 	}
 
-	menelaus = kzalloc(sizeof *menelaus, GFP_KERNEL);
+	menelaus = devm_kzalloc(&client->dev, sizeof(*menelaus),
+				GFP_KERNEL);
 	if (!menelaus)
 		return -ENOMEM;
 
@@ -1210,8 +1211,7 @@ static int menelaus_probe(struct i2c_client *client,
 	rev = menelaus_read_reg(MENELAUS_REV);
 	if (rev < 0) {
 		pr_err(DRIVER_NAME ": device not found");
-		err = -ENODEV;
-		goto fail1;
+		return -ENODEV;
 	}
 
 	/* Ack and disable all Menelaus interrupts */
@@ -1226,12 +1226,12 @@ static int menelaus_probe(struct i2c_client *client,
 	menelaus_write_reg(MENELAUS_MCT_CTRL1, 0x73);
 
 	if (client->irq > 0) {
-		err = request_irq(client->irq, menelaus_irq, 0,
-				  DRIVER_NAME, menelaus);
+		err = devm_request_irq(&client->dev, client->irq, menelaus_irq,
+					0, DRIVER_NAME, menelaus);
 		if (err) {
 			dev_dbg(&client->dev,  "can't get IRQ %d, err %d\n",
 					client->irq, err);
-			goto fail1;
+			return err;
 		}
 	}
 
@@ -1242,7 +1242,7 @@ static int menelaus_probe(struct i2c_client *client,
 
 	val = menelaus_read_reg(MENELAUS_VCORE_CTRL1);
 	if (val < 0)
-		goto fail2;
+		goto fail1;
 	if (val & (1 << 7))
 		menelaus->vcore_hw_mode = 1;
 	else
@@ -1251,17 +1251,14 @@ static int menelaus_probe(struct i2c_client *client,
 	if (menelaus_pdata != NULL && menelaus_pdata->late_init != NULL) {
 		err = menelaus_pdata->late_init(&client->dev);
 		if (err < 0)
-			goto fail2;
+			goto fail1;
 	}
 
 	menelaus_rtc_init(menelaus);
 
 	return 0;
-fail2:
-	free_irq(client->irq, menelaus);
-	flush_work(&menelaus->work);
 fail1:
-	kfree(menelaus);
+	flush_work(&menelaus->work);
 	return err;
 }
 
@@ -1269,9 +1266,7 @@ static int __exit menelaus_remove(struct i2c_client *client)
 {
 	struct menelaus_chip	*menelaus = i2c_get_clientdata(client);
 
-	free_irq(client->irq, menelaus);
 	flush_work(&menelaus->work);
-	kfree(menelaus);
 	the_menelaus = NULL;
 	return 0;
 }
-- 
1.7.2.5

[PATCH 08/11] mfd: ezx-pcap: use devm_request_irq() and devm_kzalloc()

[Jingoo Han]

Use devm_request_irq() and devm_kzalloc() to make cleanup paths
more simple.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/mfd/ezx-pcap.c |   16 +++++-----------
 1 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/mfd/ezx-pcap.c b/drivers/mfd/ezx-pcap.c
index b7a61f0..8dea3a9 100644
--- a/drivers/mfd/ezx-pcap.c
+++ b/drivers/mfd/ezx-pcap.c
@@ -403,7 +403,6 @@ static int ezx_pcap_remove(struct spi_device *spi)
 	/* cleanup ADC */
 	adc_irq = pcap_to_irq(pcap, (pdata->config & PCAP_SECOND_PORT) ?
 				PCAP_IRQ_ADCDONE2 : PCAP_IRQ_ADCDONE);
-	free_irq(adc_irq, pcap);
 	mutex_lock(&pcap->adc_mutex);
 	for (i = 0; i < PCAP_ADC_MAXQ; i++)
 		kfree(pcap->adc_queue[i]);
@@ -415,8 +414,6 @@ static int ezx_pcap_remove(struct spi_device *spi)
 
 	destroy_workqueue(pcap->workqueue);
 
-	kfree(pcap);
-
 	return 0;
 }
 
@@ -431,7 +428,7 @@ static int ezx_pcap_probe(struct spi_device *spi)
 	if (!pdata)
 		goto ret;
 
-	pcap = kzalloc(sizeof(*pcap), GFP_KERNEL);
+	pcap = devm_kzalloc(&spi->dev, sizeof(*pcap), GFP_KERNEL);
 	if (!pcap) {
 		ret = -ENOMEM;
 		goto ret;
@@ -448,7 +445,7 @@ static int ezx_pcap_probe(struct spi_device *spi)
 	spi->mode = SPI_MODE_0 | (pdata->config & PCAP_CS_AH ? SPI_CS_HIGH : 0);
 	ret = spi_setup(spi);
 	if (ret)
-		goto free_pcap;
+		goto ret;
 
 	pcap->spi = spi;
 
@@ -458,7 +455,7 @@ static int ezx_pcap_probe(struct spi_device *spi)
 	if (!pcap->workqueue) {
 		ret = -ENOMEM;
 		dev_err(&spi->dev, "can't create pcap thread\n");
-		goto free_pcap;
+		goto ret;
 	}
 
 	/* redirect interrupts to AP, except adcdone2 */
@@ -491,7 +488,8 @@ static int ezx_pcap_probe(struct spi_device *spi)
 	adc_irq = pcap_to_irq(pcap, (pdata->config & PCAP_SECOND_PORT) ?
 					PCAP_IRQ_ADCDONE2 : PCAP_IRQ_ADCDONE);
 
-	ret = request_irq(adc_irq, pcap_adc_irq, 0, "ADC", pcap);
+	ret = devm_request_irq(&spi->dev, adc_irq, pcap_adc_irq, 0,
+				"ADC", pcap);
 	if (ret)
 		goto free_irqchip;
 
@@ -510,15 +508,11 @@ static int ezx_pcap_probe(struct spi_device *spi)
 
 remove_subdevs:
 	device_for_each_child(&spi->dev, NULL, pcap_remove_subdev);
-/* free_adc: */
-	free_irq(adc_irq, pcap);
 free_irqchip:
 	for (i = pcap->irq_base; i < (pcap->irq_base + PCAP_NIRQS); i++)
 		irq_set_chip_and_handler(i, NULL, NULL);
 /* destroy_workqueue: */
 	destroy_workqueue(pcap->workqueue);
-free_pcap:
-	kfree(pcap);
 ret:
 	return ret;
 }
-- 
1.7.2.5

[PATCH 09/11] mfd: da903x: use devm_request_irq() and devm_kzalloc()

[Jingoo Han]

Use devm_request_irq() and devm_kzalloc() to make cleanup paths
more simple.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/mfd/da903x.c |   19 ++++++-------------
 1 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/drivers/mfd/da903x.c b/drivers/mfd/da903x.c
index 05176cd..f1a316e 100644
--- a/drivers/mfd/da903x.c
+++ b/drivers/mfd/da903x.c
@@ -499,7 +499,8 @@ static int da903x_probe(struct i2c_client *client,
 	unsigned int tmp;
 	int ret;
 
-	chip = kzalloc(sizeof(struct da903x_chip), GFP_KERNEL);
+	chip = devm_kzalloc(&client->dev, sizeof(struct da903x_chip),
+				GFP_KERNEL);
 	if (chip == NULL)
 		return -ENOMEM;
 
@@ -515,33 +516,27 @@ static int da903x_probe(struct i2c_client *client,
 
 	ret = chip->ops->init_chip(chip);
 	if (ret)
-		goto out_free_chip;
+		return ret;
 
 	/* mask and clear all IRQs */
 	chip->events_mask = 0xffffffff;
 	chip->ops->mask_events(chip, chip->events_mask);
 	chip->ops->read_events(chip, &tmp);
 
-	ret = request_irq(client->irq, da903x_irq_handler,
+	ret = devm_request_irq(&client->dev, client->irq, da903x_irq_handler,
 			IRQF_TRIGGER_FALLING,
 			"da903x", chip);
 	if (ret) {
 		dev_err(&client->dev, "failed to request irq %d\n",
 				client->irq);
-		goto out_free_chip;
+		return ret;
 	}
 
 	ret = da903x_add_subdevs(chip, pdata);
 	if (ret)
-		goto out_free_irq;
+		return ret;
 
 	return 0;
-
-out_free_irq:
-	free_irq(client->irq, chip);
-out_free_chip:
-	kfree(chip);
-	return ret;
 }
 
 static int da903x_remove(struct i2c_client *client)
@@ -549,8 +544,6 @@ static int da903x_remove(struct i2c_client *client)
 	struct da903x_chip *chip = i2c_get_clientdata(client);
 
 	da903x_remove_subdevs(chip);
-	free_irq(client->irq, chip);
-	kfree(chip);
 	return 0;
 }
 
-- 
1.7.2.5

[PATCH 10/11] mfd: tps65010: use devm_request_irq() and devm_kzalloc()

[Jingoo Han]

Use devm_request_irq() and devm_kzalloc() to make cleanup paths
more simple.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/mfd/tps65010.c |   15 +++++----------
 1 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/mfd/tps65010.c b/drivers/mfd/tps65010.c
index da2691f..a5438cc 100644
--- a/drivers/mfd/tps65010.c
+++ b/drivers/mfd/tps65010.c
@@ -525,11 +525,8 @@ static int __exit tps65010_remove(struct i2c_client *client)
 			dev_dbg(&client->dev, "board %s %s err %d\n",
 				"teardown", client->name, status);
 	}
-	if (client->irq > 0)
-		free_irq(client->irq, tps);
 	cancel_delayed_work_sync(&tps->work);
 	debugfs_remove(tps->file);
-	kfree(tps);
 	the_tps = NULL;
 	return 0;
 }
@@ -549,7 +546,7 @@ static int tps65010_probe(struct i2c_client *client,
 	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
 		return -EINVAL;
 
-	tps = kzalloc(sizeof *tps, GFP_KERNEL);
+	tps = devm_kzalloc(&client->dev, sizeof(*tps), GFP_KERNEL);
 	if (!tps)
 		return -ENOMEM;
 
@@ -562,12 +559,13 @@ static int tps65010_probe(struct i2c_client *client,
 	 * so this driver uses falling-edge triggers instead.
 	 */
 	if (client->irq > 0) {
-		status = request_irq(client->irq, tps65010_irq,
-				     IRQF_TRIGGER_FALLING, DRIVER_NAME, tps);
+		status = devm_request_irq(&client->dev, client->irq,
+					tps65010_irq, IRQF_TRIGGER_FALLING,
+					DRIVER_NAME, tps);
 		if (status < 0) {
 			dev_dbg(&client->dev, "can't get IRQ %d, err %d\n",
 					client->irq, status);
-			goto fail1;
+			return status;
 		}
 		/* annoying race here, ideally we'd have an option
 		 * to claim the irq now and enable it later.
@@ -667,9 +665,6 @@ static int tps65010_probe(struct i2c_client *client,
 	}
 
 	return 0;
-fail1:
-	kfree(tps);
-	return status;
 }
 
 static const struct i2c_device_id tps65010_id[] = {
-- 
1.7.2.5

[PATCH 11/11] mfd: tc3589x: use devm_request_irq() and devm_kzalloc()

[Jingoo Han]

Use devm_request_irq() and devm_kzalloc() to make cleanup paths
more simple.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/mfd/tc3589x.c |   21 ++++++---------------
 1 files changed, 6 insertions(+), 15 deletions(-)

diff --git a/drivers/mfd/tc3589x.c b/drivers/mfd/tc3589x.c
index ecc092c..4cb92bb 100644
--- a/drivers/mfd/tc3589x.c
+++ b/drivers/mfd/tc3589x.c
@@ -350,7 +350,8 @@ static int tc3589x_probe(struct i2c_client *i2c,
 				     | I2C_FUNC_SMBUS_I2C_BLOCK))
 		return -EIO;
 
-	tc3589x = kzalloc(sizeof(struct tc3589x), GFP_KERNEL);
+	tc3589x = devm_kzalloc(&i2c->dev, sizeof(struct tc3589x),
+				GFP_KERNEL);
 	if (!tc3589x)
 		return -ENOMEM;
 
@@ -366,33 +367,27 @@ static int tc3589x_probe(struct i2c_client *i2c,
 
 	ret = tc3589x_chip_init(tc3589x);
 	if (ret)
-		goto out_free;
+		return ret;
 
 	ret = tc3589x_irq_init(tc3589x, np);
 	if (ret)
-		goto out_free;
+		return ret;
 
 	ret = request_threaded_irq(tc3589x->i2c->irq, NULL, tc3589x_irq,
 				   IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
 				   "tc3589x", tc3589x);
 	if (ret) {
 		dev_err(tc3589x->dev, "failed to request IRQ: %d\n", ret);
-		goto out_free;
+		return ret;
 	}
 
 	ret = tc3589x_device_init(tc3589x);
 	if (ret) {
 		dev_err(tc3589x->dev, "failed to add child devices\n");
-		goto out_freeirq;
+		return ret;
 	}
 
 	return 0;
-
-out_freeirq:
-	free_irq(tc3589x->i2c->irq, tc3589x);
-out_free:
-	kfree(tc3589x);
-	return ret;
 }
 
 static int tc3589x_remove(struct i2c_client *client)
@@ -401,10 +396,6 @@ static int tc3589x_remove(struct i2c_client *client)
 
 	mfd_remove_devices(tc3589x->dev);
 
-	free_irq(tc3589x->i2c->irq, tc3589x);
-
-	kfree(tc3589x);
-
 	return 0;
 }
 
-- 
1.7.2.5

Re: [PATCH 07/11] mfd: menelaus: use devm_request_irq() and devm_kzalloc()

[Dmitry Torokhov]

Hi Jongoo,

On Wed, Feb 20, 2013 at 03:12:38PM +0900, Jingoo Han wrote:
> Use devm_request_irq() and devm_kzalloc() to make cleanup paths
> more simple.
> 

...

> @@ -1269,9 +1266,7 @@ static int __exit menelaus_remove(struct i2c_client *client)
>  {
>  	struct menelaus_chip	*menelaus = i2c_get_clientdata(client);
>  
> -	free_irq(client->irq, menelaus);
>  	flush_work(&menelaus->work);
> -	kfree(menelaus);
>  	the_menelaus = NULL;
>  	return 0;

This conversion is certainly wrong - you really want to disable IRQ and
then wait for the scheduled work to finish before freeing memory. Here
you flush work but nothing stops IRQ from firing and scheduling that
work again.

Please, be *extra* careful with devm_request_irq() conversions.

Thanks.

-- 
Dmitry

Re: [PATCH 08/11] mfd: ezx-pcap: use devm_request_irq() and devm_kzalloc()

[Dmitry Torokhov]

On Wed, Feb 20, 2013 at 03:13:06PM +0900, Jingoo Han wrote:
> Use devm_request_irq() and devm_kzalloc() to make cleanup paths
> more simple.
> 
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> ---
>  drivers/mfd/ezx-pcap.c |   16 +++++-----------
>  1 files changed, 5 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/mfd/ezx-pcap.c b/drivers/mfd/ezx-pcap.c
> index b7a61f0..8dea3a9 100644
> --- a/drivers/mfd/ezx-pcap.c
> +++ b/drivers/mfd/ezx-pcap.c
> @@ -403,7 +403,6 @@ static int ezx_pcap_remove(struct spi_device *spi)
>  	/* cleanup ADC */
>  	adc_irq = pcap_to_irq(pcap, (pdata->config & PCAP_SECOND_PORT) ?
>  				PCAP_IRQ_ADCDONE2 : PCAP_IRQ_ADCDONE);
> -	free_irq(adc_irq, pcap);
>  	mutex_lock(&pcap->adc_mutex);
>  	for (i = 0; i < PCAP_ADC_MAXQ; i++)
>  		kfree(pcap->adc_queue[i]);
> @@ -415,8 +414,6 @@ static int ezx_pcap_remove(struct spi_device *spi)
>  
>  	destroy_workqueue(pcap->workqueue);
>  
> -	kfree(pcap);
> -

I am pretty sure this conversion is wrong as well. Pretty much
work/workqueue and devm_request_irq() do not mix.

Thanks.

-- 
Dmitry

Re: [PATCH 10/11] mfd: tps65010: use devm_request_irq() and devm_kzalloc()

[Dmitry Torokhov]

On Wed, Feb 20, 2013 at 03:14:05PM +0900, Jingoo Han wrote:
> Use devm_request_irq() and devm_kzalloc() to make cleanup paths
> more simple.
> 
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> ---
>  drivers/mfd/tps65010.c |   15 +++++----------
>  1 files changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/mfd/tps65010.c b/drivers/mfd/tps65010.c
> index da2691f..a5438cc 100644
> --- a/drivers/mfd/tps65010.c
> +++ b/drivers/mfd/tps65010.c
> @@ -525,11 +525,8 @@ static int __exit tps65010_remove(struct i2c_client *client)
>  			dev_dbg(&client->dev, "board %s %s err %d\n",
>  				"teardown", client->name, status);
>  	}
> -	if (client->irq > 0)
> -		free_irq(client->irq, tps);
>  	cancel_delayed_work_sync(&tps->work);
>  	debugfs_remove(tps->file);
> -	kfree(tps);
>  	the_tps = NULL;
>  	return 0;

Nope, again work and IRQ have to be handled in certain order.

Thanks.

-- 
Dmitry

Re: [PATCH 05/11] mfd: twl6040: use devm_gpio_request_one() and devm_request_threaded_irq()

[Dmitry Torokhov]

Hi Jingoo,

On Wed, Feb 20, 2013 at 03:11:38PM +0900, Jingoo Han wrote:
> Use devm_gpio_request_one() and devm_request_threaded_irq() to make
> cleanup paths and  more simple.
> 
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> ---
>  drivers/mfd/twl6040.c |   26 +++++++-------------------
>  1 files changed, 7 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/mfd/twl6040.c b/drivers/mfd/twl6040.c
> index f361bf3..2d38512 100644
> --- a/drivers/mfd/twl6040.c
> +++ b/drivers/mfd/twl6040.c
> @@ -586,8 +586,8 @@ static int twl6040_probe(struct i2c_client *client,
>  		twl6040->audpwron = -EINVAL;
>  
>  	if (gpio_is_valid(twl6040->audpwron)) {
> -		ret = gpio_request_one(twl6040->audpwron, GPIOF_OUT_INIT_LOW,
> -				       "audpwron");
> +		ret = devm_gpio_request_one(twl6040->dev, twl6040->audpwron,
> +					GPIOF_OUT_INIT_LOW, "audpwron");
>  		if (ret)
>  			goto gpio_err;
>  	}
> @@ -596,14 +596,14 @@ static int twl6040_probe(struct i2c_client *client,
>  			IRQF_ONESHOT, 0, &twl6040_irq_chip,
>  			&twl6040->irq_data);
>  	if (ret < 0)
> -		goto irq_init_err;
> +		goto gpio_err;
>  
>  	twl6040->irq_ready = regmap_irq_get_virq(twl6040->irq_data,
>  					       TWL6040_IRQ_READY);
>  	twl6040->irq_th = regmap_irq_get_virq(twl6040->irq_data,
>  					       TWL6040_IRQ_TH);
>  
> -	ret = request_threaded_irq(twl6040->irq_ready, NULL,
> +	ret = devm_request_threaded_irq(twl6040->dev, twl6040->irq_ready, NULL,
>  				   twl6040_readyint_handler, IRQF_ONESHOT,
>  				   "twl6040_irq_ready", twl6040);
>  	if (ret) {
> @@ -611,12 +611,12 @@ static int twl6040_probe(struct i2c_client *client,
>  		goto readyirq_err;
>  	}
>  
> -	ret = request_threaded_irq(twl6040->irq_th, NULL,
> +	ret = devm_request_threaded_irq(twl6040->dev, twl6040->irq_th, NULL,
>  				   twl6040_thint_handler, IRQF_ONESHOT,
>  				   "twl6040_irq_th", twl6040);
>  	if (ret) {
>  		dev_err(twl6040->dev, "Thermal IRQ request failed: %d\n", ret);
> -		goto thirq_err;
> +		goto readyirq_err;
>  	}
>  
>  	/* dual-access registers controlled by I2C only */
> @@ -676,19 +676,12 @@ static int twl6040_probe(struct i2c_client *client,
>  	ret = mfd_add_devices(&client->dev, -1, twl6040->cells, children,
>  			      NULL, 0, NULL);
>  	if (ret)
> -		goto mfd_err;
> +		goto readyirq_err;
>  
>  	return 0;
>  
> -mfd_err:
> -	free_irq(twl6040->irq_th, twl6040);
> -thirq_err:
> -	free_irq(twl6040->irq_ready, twl6040);
>  readyirq_err:
>  	regmap_del_irq_chip(twl6040->irq, twl6040->irq_data);
> -irq_init_err:
> -	if (gpio_is_valid(twl6040->audpwron))
> -		gpio_free(twl6040->audpwron);
>  gpio_err:
>  	regulator_bulk_disable(TWL6040_NUM_SUPPLIES, twl6040->supplies);
>  power_err:
> @@ -706,11 +699,6 @@ static int twl6040_remove(struct i2c_client *client)
>  	if (twl6040->power_count)
>  		twl6040_power(twl6040, 0);
>  
> -	if (gpio_is_valid(twl6040->audpwron))
> -		gpio_free(twl6040->audpwron);
> -
> -	free_irq(twl6040->irq_ready, twl6040);
> -	free_irq(twl6040->irq_th, twl6040);
>  	regmap_del_irq_chip(twl6040->irq, twl6040->irq_data);
>  
>  	mfd_remove_devices(&client->dev);

Are you sure it is OK to have sub-devices removed and regmap destroyed
with IRQs still active?

Thanks.

-- 
Dmitry

RE: [PATCH 07/11] mfd: menelaus: use devm_request_irq() and devm_kzalloc()

[Jingoo Han]

On Wednesday, February 20, 2013 4:31 PM, Dmitry Torokhov wrote:
> 
> Hi Jongoo,
> 
> On Wed, Feb 20, 2013 at 03:12:38PM +0900, Jingoo Han wrote:
> > Use devm_request_irq() and devm_kzalloc() to make cleanup paths
> > more simple.
> >
> 
> ...
> 
> > @@ -1269,9 +1266,7 @@ static int __exit menelaus_remove(struct i2c_client *client)
> >  {
> >  	struct menelaus_chip	*menelaus = i2c_get_clientdata(client);
> >
> > -	free_irq(client->irq, menelaus);
> >  	flush_work(&menelaus->work);
> > -	kfree(menelaus);
> >  	the_menelaus = NULL;
> >  	return 0;
> 
> This conversion is certainly wrong - you really want to disable IRQ and
> then wait for the scheduled work to finish before freeing memory. Here
> you flush work but nothing stops IRQ from firing and scheduling that
> work again.

Yes, you're right. 
I will use devm_free_irq() before flush_work().

> 
> Please, be *extra* careful with devm_request_irq() conversions.
> 
> Thanks.
> 
> --
> Dmitry

Re: [PATCH 08/11] mfd: ezx-pcap: use devm_request_irq() and devm_kzalloc()

[Jingoo Han]

On Wednesday, February 20, 2013 4:34 PM, Dmitry Torokhov wrote:
> 
> On Wed, Feb 20, 2013 at 03:13:06PM +0900, Jingoo Han wrote:
> > Use devm_request_irq() and devm_kzalloc() to make cleanup paths
> > more simple.
> >
> > Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> > ---
> >  drivers/mfd/ezx-pcap.c |   16 +++++-----------
> >  1 files changed, 5 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/mfd/ezx-pcap.c b/drivers/mfd/ezx-pcap.c
> > index b7a61f0..8dea3a9 100644
> > --- a/drivers/mfd/ezx-pcap.c
> > +++ b/drivers/mfd/ezx-pcap.c
> > @@ -403,7 +403,6 @@ static int ezx_pcap_remove(struct spi_device *spi)
> >  	/* cleanup ADC */
> >  	adc_irq = pcap_to_irq(pcap, (pdata->config & PCAP_SECOND_PORT) ?
> >  				PCAP_IRQ_ADCDONE2 : PCAP_IRQ_ADCDONE);
> > -	free_irq(adc_irq, pcap);
> >  	mutex_lock(&pcap->adc_mutex);
> >  	for (i = 0; i < PCAP_ADC_MAXQ; i++)
> >  		kfree(pcap->adc_queue[i]);
> > @@ -415,8 +414,6 @@ static int ezx_pcap_remove(struct spi_device *spi)
> >
> >  	destroy_workqueue(pcap->workqueue);
> >
> > -	kfree(pcap);
> > -
> 
> I am pretty sure this conversion is wrong as well. Pretty much
> work/workqueue and devm_request_irq() do not mix.

Yes, you're right. 
I will use devm_free_irq().

> 
> Thanks.
> 
> --
> Dmitry

Re: [PATCH 10/11] mfd: tps65010: use devm_request_irq() and devm_kzalloc()

[Jingoo Han]

On Wednesday, February 20, 2013 4:35 PM, Dmitry Torokhov wrote:

> -----Original Message-----
> From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com]
> Sent: Wednesday, February 20, 2013 4:35 PM
> To: Jingoo Han
> Cc: 'Samuel Ortiz'; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 10/11] mfd: tps65010: use devm_request_irq() and devm_kzalloc()
> 
> On Wed, Feb 20, 2013 at 03:14:05PM +0900, Jingoo Han wrote:
> > Use devm_request_irq() and devm_kzalloc() to make cleanup paths
> > more simple.
> >
> > Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> > ---
> >  drivers/mfd/tps65010.c |   15 +++++----------
> >  1 files changed, 5 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/mfd/tps65010.c b/drivers/mfd/tps65010.c
> > index da2691f..a5438cc 100644
> > --- a/drivers/mfd/tps65010.c
> > +++ b/drivers/mfd/tps65010.c
> > @@ -525,11 +525,8 @@ static int __exit tps65010_remove(struct i2c_client *client)
> >  			dev_dbg(&client->dev, "board %s %s err %d\n",
> >  				"teardown", client->name, status);
> >  	}
> > -	if (client->irq > 0)
> > -		free_irq(client->irq, tps);
> >  	cancel_delayed_work_sync(&tps->work);
> >  	debugfs_remove(tps->file);
> > -	kfree(tps);
> >  	the_tps = NULL;
> >  	return 0;
> 
> Nope, again work and IRQ have to be handled in certain order.

Yes, you're right. 
I will use devm_free_irq()

> 
> Thanks.
> 
> --
> Dmitry

Re: [PATCH 05/11] mfd: twl6040: use devm_gpio_request_one() and devm_request_threaded_irq()

[Jingoo Han]

On Wednesday, February 20, 2013 4:37 PM, Dmitry Torokhov wrote:
> 
> Hi Jingoo,
> 
> On Wed, Feb 20, 2013 at 03:11:38PM +0900, Jingoo Han wrote:
> > Use devm_gpio_request_one() and devm_request_threaded_irq() to make
> > cleanup paths and  more simple.
> >
> > Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> > ---
> >  drivers/mfd/twl6040.c |   26 +++++++-------------------
> >  1 files changed, 7 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/mfd/twl6040.c b/drivers/mfd/twl6040.c
> > index f361bf3..2d38512 100644
> > --- a/drivers/mfd/twl6040.c
> > +++ b/drivers/mfd/twl6040.c
> > @@ -586,8 +586,8 @@ static int twl6040_probe(struct i2c_client *client,
> >  		twl6040->audpwron = -EINVAL;
> >
> >  	if (gpio_is_valid(twl6040->audpwron)) {
> > -		ret = gpio_request_one(twl6040->audpwron, GPIOF_OUT_INIT_LOW,
> > -				       "audpwron");
> > +		ret = devm_gpio_request_one(twl6040->dev, twl6040->audpwron,
> > +					GPIOF_OUT_INIT_LOW, "audpwron");
> >  		if (ret)
> >  			goto gpio_err;
> >  	}
> > @@ -596,14 +596,14 @@ static int twl6040_probe(struct i2c_client *client,
> >  			IRQF_ONESHOT, 0, &twl6040_irq_chip,
> >  			&twl6040->irq_data);
> >  	if (ret < 0)
> > -		goto irq_init_err;
> > +		goto gpio_err;
> >
> >  	twl6040->irq_ready = regmap_irq_get_virq(twl6040->irq_data,
> >  					       TWL6040_IRQ_READY);
> >  	twl6040->irq_th = regmap_irq_get_virq(twl6040->irq_data,
> >  					       TWL6040_IRQ_TH);
> >
> > -	ret = request_threaded_irq(twl6040->irq_ready, NULL,
> > +	ret = devm_request_threaded_irq(twl6040->dev, twl6040->irq_ready, NULL,
> >  				   twl6040_readyint_handler, IRQF_ONESHOT,
> >  				   "twl6040_irq_ready", twl6040);
> >  	if (ret) {
> > @@ -611,12 +611,12 @@ static int twl6040_probe(struct i2c_client *client,
> >  		goto readyirq_err;
> >  	}
> >
> > -	ret = request_threaded_irq(twl6040->irq_th, NULL,
> > +	ret = devm_request_threaded_irq(twl6040->dev, twl6040->irq_th, NULL,
> >  				   twl6040_thint_handler, IRQF_ONESHOT,
> >  				   "twl6040_irq_th", twl6040);
> >  	if (ret) {
> >  		dev_err(twl6040->dev, "Thermal IRQ request failed: %d\n", ret);
> > -		goto thirq_err;
> > +		goto readyirq_err;
> >  	}
> >
> >  	/* dual-access registers controlled by I2C only */
> > @@ -676,19 +676,12 @@ static int twl6040_probe(struct i2c_client *client,
> >  	ret = mfd_add_devices(&client->dev, -1, twl6040->cells, children,
> >  			      NULL, 0, NULL);
> >  	if (ret)
> > -		goto mfd_err;
> > +		goto readyirq_err;
> >
> >  	return 0;
> >
> > -mfd_err:
> > -	free_irq(twl6040->irq_th, twl6040);
> > -thirq_err:
> > -	free_irq(twl6040->irq_ready, twl6040);
> >  readyirq_err:
> >  	regmap_del_irq_chip(twl6040->irq, twl6040->irq_data);
> > -irq_init_err:
> > -	if (gpio_is_valid(twl6040->audpwron))
> > -		gpio_free(twl6040->audpwron);
> >  gpio_err:
> >  	regulator_bulk_disable(TWL6040_NUM_SUPPLIES, twl6040->supplies);
> >  power_err:
> > @@ -706,11 +699,6 @@ static int twl6040_remove(struct i2c_client *client)
> >  	if (twl6040->power_count)
> >  		twl6040_power(twl6040, 0);
> >
> > -	if (gpio_is_valid(twl6040->audpwron))
> > -		gpio_free(twl6040->audpwron);
> > -
> > -	free_irq(twl6040->irq_ready, twl6040);
> > -	free_irq(twl6040->irq_th, twl6040);
> >  	regmap_del_irq_chip(twl6040->irq, twl6040->irq_data);
> >
> >  	mfd_remove_devices(&client->dev);
> 
> Are you sure it is OK to have sub-devices removed and regmap destroyed
> with IRQs still active?

Yes, you're right. 
I will use devm_free_irq().

> 
> Thanks.
> 
> --
> Dmitry

Re: [PATCH 07/11] mfd: menelaus: use devm_request_irq() and devm_kzalloc()

[Dmitry Torokhov]

On Wed, Feb 20, 2013 at 05:05:10PM +0900, Jingoo Han wrote:
> On Wednesday, February 20, 2013 4:31 PM, Dmitry Torokhov wrote:
> > 
> > Hi Jongoo,
> > 
> > On Wed, Feb 20, 2013 at 03:12:38PM +0900, Jingoo Han wrote:
> > > Use devm_request_irq() and devm_kzalloc() to make cleanup paths
> > > more simple.
> > >
> > 
> > ...
> > 
> > > @@ -1269,9 +1266,7 @@ static int __exit menelaus_remove(struct i2c_client *client)
> > >  {
> > >  	struct menelaus_chip	*menelaus = i2c_get_clientdata(client);
> > >
> > > -	free_irq(client->irq, menelaus);
> > >  	flush_work(&menelaus->work);
> > > -	kfree(menelaus);
> > >  	the_menelaus = NULL;
> > >  	return 0;
> > 
> > This conversion is certainly wrong - you really want to disable IRQ and
> > then wait for the scheduled work to finish before freeing memory. Here
> > you flush work but nothing stops IRQ from firing and scheduling that
> > work again.
> 
> Yes, you're right. 
> I will use devm_free_irq() before flush_work().

Why change it at all if you have to call it manually in both error
unwinding and menelaus_remove() cases?

BTW, that __exit markup on menelaus_remove() is surprising... I am
pretty sure it can be unbound via sysfs and so there will be a nasty
oops.

-- 
Dmitry

Re: [PATCH 07/11] mfd: menelaus: use devm_request_irq() and devm_kzalloc()

[Jingoo Han]

On Wednesday, February 20, 2013 5:10 PM, Dmitry Torokhov wrote:
> 
> On Wed, Feb 20, 2013 at 05:05:10PM +0900, Jingoo Han wrote:
> > On Wednesday, February 20, 2013 4:31 PM, Dmitry Torokhov wrote:
> > >
> > > Hi Jongoo,
> > >
> > > On Wed, Feb 20, 2013 at 03:12:38PM +0900, Jingoo Han wrote:
> > > > Use devm_request_irq() and devm_kzalloc() to make cleanup paths
> > > > more simple.
> > > >
> > >
> > > ...
> > >
> > > > @@ -1269,9 +1266,7 @@ static int __exit menelaus_remove(struct i2c_client *client)
> > > >  {
> > > >  	struct menelaus_chip	*menelaus = i2c_get_clientdata(client);
> > > >
> > > > -	free_irq(client->irq, menelaus);
> > > >  	flush_work(&menelaus->work);
> > > > -	kfree(menelaus);
> > > >  	the_menelaus = NULL;
> > > >  	return 0;
> > >
> > > This conversion is certainly wrong - you really want to disable IRQ and
> > > then wait for the scheduled work to finish before freeing memory. Here
> > > you flush work but nothing stops IRQ from firing and scheduling that
> > > work again.
> >
> > Yes, you're right.
> > I will use devm_free_irq() before flush_work().
> 
> Why change it at all if you have to call it manually in both error
> unwinding and menelaus_remove() cases?
> 
> BTW, that __exit markup on menelaus_remove() is surprising... I am
> pretty sure it can be unbound via sysfs and so there will be a nasty
> oops.

I see, I will not modify this menelaus mfd driver.
Thank you.

> 
> --
> Dmitry

Re: [PATCH 07/11] mfd: menelaus: use devm_request_irq() and devm_kzalloc()

[Dmitry Torokhov]

On Wed, Feb 20, 2013 at 05:17:30PM +0900, Jingoo Han wrote:
> On Wednesday, February 20, 2013 5:10 PM, Dmitry Torokhov wrote:
> > 
> > On Wed, Feb 20, 2013 at 05:05:10PM +0900, Jingoo Han wrote:
> > > On Wednesday, February 20, 2013 4:31 PM, Dmitry Torokhov wrote:
> > > >
> > > > Hi Jongoo,
> > > >
> > > > On Wed, Feb 20, 2013 at 03:12:38PM +0900, Jingoo Han wrote:
> > > > > Use devm_request_irq() and devm_kzalloc() to make cleanup paths
> > > > > more simple.
> > > > >
> > > >
> > > > ...
> > > >
> > > > > @@ -1269,9 +1266,7 @@ static int __exit menelaus_remove(struct i2c_client *client)
> > > > >  {
> > > > >  	struct menelaus_chip	*menelaus = i2c_get_clientdata(client);
> > > > >
> > > > > -	free_irq(client->irq, menelaus);
> > > > >  	flush_work(&menelaus->work);
> > > > > -	kfree(menelaus);
> > > > >  	the_menelaus = NULL;
> > > > >  	return 0;
> > > >
> > > > This conversion is certainly wrong - you really want to disable IRQ and
> > > > then wait for the scheduled work to finish before freeing memory. Here
> > > > you flush work but nothing stops IRQ from firing and scheduling that
> > > > work again.
> > >
> > > Yes, you're right.
> > > I will use devm_free_irq() before flush_work().
> > 
> > Why change it at all if you have to call it manually in both error
> > unwinding and menelaus_remove() cases?
> > 
> > BTW, that __exit markup on menelaus_remove() is surprising... I am
> > pretty sure it can be unbound via sysfs and so there will be a nasty
> > oops.
> 
> I see, I will not modify this menelaus mfd driver.
> Thank you.

Same goes for the other drivers that can't safely use devm_request_irq()
please.

Thanks.

-- 
Dmitry