Skip to contentSkip to conversation

staging:iio:ad7280a: Use managed interfaces

PATCH In review Himangi Saraogi 3 participants 5 messages Jul 19, 2014

Subsystems (4)
Staging IIO Analog Devices IIO drivers Staging - industrial io
lore
Share & tools
lore

Himangi Saraogi

author
#0 lore raw

PATCH 1/1 staging:iio:ad7280a: Use managed interfaces

This patch moves data allocated using unmanaged interfaces to managed interfaces like devm_kzalloc, devm_iio_device_register, devm_kasprintf, devm_request_threaded_irq and does away with the calls to free the allocated memory in the probe and remove functions. Some labels in the probe function are also removed.

Signed-off-by Himangi Saraogi <[email protected]>

---
I am not very sure if devmifying request_irq is a good idea as it leads to the freeing of the interrupt after the ad7280_write.

 drivers/staging/iio/adc/ad7280a.c | 57 ++++++++++++++-------------------------
 1 file changed, 20 insertions(+), 37 deletions(-)
drivers/staging/iio/adc/ad7280a.c +20 -37
index d215edf..3706b3e 100644
@@ -486,8 +486,8 @@ static int ad7280_channel_init(struct ad7280_state *st)
 {
 	int dev, ch, cnt;
 
-	st->channels = kcalloc((st->slave_num + 1) * 12 + 2,
-			       sizeof(*st->channels), GFP_KERNEL);
+	st->channels = devm_kcalloc(&st->spi->dev, (st->slave_num + 1) * 12 + 2,
+				    sizeof(*st->channels), GFP_KERNEL);
 	if (st->channels == NULL)
 		return -ENOMEM;
 

@@ -547,8 +547,9 @@ static int ad7280_attr_init(struct ad7280_state *st)
 {
 	int dev, ch, cnt;
 
-	st->iio_attr = kzalloc(sizeof(*st->iio_attr) * (st->slave_num + 1) *
-				AD7280A_CELLS_PER_DEV * 2, GFP_KERNEL);
+	st->iio_attr = devm_kzalloc(&st->spi->dev, sizeof(*st->iio_attr) *
+				    (st->slave_num + 1) *
+				    AD7280A_CELLS_PER_DEV * 2, GFP_KERNEL);
 	if (st->iio_attr == NULL)
 		return -ENOMEM;
 

@@ -564,7 +565,7 @@ static int ad7280_attr_init(struct ad7280_state *st)
 			st->iio_attr[cnt].dev_attr.store =
 				ad7280_store_balance_sw;
 			st->iio_attr[cnt].dev_attr.attr.name =
-				kasprintf(GFP_KERNEL,
+				devm_kasprintf(&st->spi->dev, GFP_KERNEL,
 					"in%d-in%d_balance_switch_en",
 					(dev * AD7280A_CELLS_PER_DEV) + ch,
 					(dev * AD7280A_CELLS_PER_DEV) + ch + 1);

@@ -581,7 +582,8 @@ static int ad7280_attr_init(struct ad7280_state *st)
 			st->iio_attr[cnt].dev_attr.store =
 				ad7280_store_balance_timer;
 			st->iio_attr[cnt].dev_attr.attr.name =
-				kasprintf(GFP_KERNEL, "in%d-in%d_balance_timer",
+				devm_kasprintf(&st->spi->dev, GFP_KERNEL,
+					"in%d-in%d_balance_timer",
 					(dev * AD7280A_CELLS_PER_DEV) + ch,
 					(dev * AD7280A_CELLS_PER_DEV) + ch + 1);
 			ad7280_attributes[cnt] =

@@ -900,48 +902,36 @@ static int ad7280_probe(struct spi_device *spi)
 
 	ret = ad7280_attr_init(st);
 	if (ret < 0)
-		goto error_free_channels;
+		return ret;
 
-	ret = iio_device_register(indio_dev);
+	ret = devm_iio_device_register(&spi->dev, indio_dev);
 	if (ret)
-		goto error_free_attr;
+		return ret;
 
 	if (spi->irq > 0) {
 		ret = ad7280_write(st, AD7280A_DEVADDR_MASTER,
 				   AD7280A_ALERT, 1,
 				   AD7280A_ALERT_RELAY_SIG_CHAIN_DOWN);
 		if (ret)
-			goto error_unregister;
+			return ret;
 
 		ret = ad7280_write(st, AD7280A_DEVADDR(st->slave_num),
 				   AD7280A_ALERT, 0,
 				   AD7280A_ALERT_GEN_STATIC_HIGH |
 				   (pdata->chain_last_alert_ignore & 0xF));
 		if (ret)
-			goto error_unregister;
-
-		ret = request_threaded_irq(spi->irq,
-					   NULL,
-					   ad7280_event_handler,
-					   IRQF_TRIGGER_FALLING |
-					   IRQF_ONESHOT,
-					   indio_dev->name,
-					   indio_dev);
+			return ret;
+
+		ret = devm_request_threaded_irq(&spi->dev, spi->irq, NULL,
+						ad7280_event_handler,
+						IRQF_TRIGGER_FALLING |
+						IRQF_ONESHOT, indio_dev->name,
+						indio_dev);
 		if (ret)
-			goto error_unregister;
+			return ret;
 	}
 
 	return 0;
-error_unregister:
-	iio_device_unregister(indio_dev);
-
-error_free_attr:
-	kfree(st->iio_attr);
-
-error_free_channels:
-	kfree(st->channels);
-
-	return ret;
 }
 
 static int ad7280_remove(struct spi_device *spi)

@@ -949,16 +939,9 @@ static int ad7280_remove(struct spi_device *spi)
 	struct iio_dev *indio_dev = spi_get_drvdata(spi);
 	struct ad7280_state *st = iio_priv(indio_dev);
 
-	if (spi->irq > 0)
-		free_irq(spi->irq, indio_dev);
-	iio_device_unregister(indio_dev);
-
 	ad7280_write(st, AD7280A_DEVADDR_MASTER, AD7280A_CONTROL_HB, 1,
 			AD7280A_CTRL_HB_PWRDN_SW | st->ctrl_hb);
 
-	kfree(st->channels);
-	kfree(st->iio_attr);
-
 	return 0;
 }
Signature · 1 line
1.9.1
maintainer
#1 lore raw
Quoted 7 lines · Himangi Saraogi

This patch moves data allocated using unmanaged interfaces to managed interfaces like devm_kzalloc, devm_iio_device_register, devm_kasprintf, devm_request_threaded_irq and does away with the calls to free the allocated memory in the probe and remove functions. Some labels in the probe function are also removed.

Signed-off-by Himangi Saraogi <[email protected]>

cc'ing Lars who tends to look after Analog Devices drivers and more specifically was involved in the thread that lead to this patch.

Also, the devm_kasprintf does not appear to be in upstream so this patch should have made an dependency clear.

https://lkml.org/lkml/2014/7/16/667

What is the intended merge path for that patch?

Quoted 3 lines

---
I am not very sure if devmifying request_irq is a good idea as it leads to the freeing of the interrupt after the ad7280_write.

Agreed. I wouldn't do it!

Otherwise, all looks fine.

Quoted 128 lines
  drivers/staging/iio/adc/ad7280a.c | 57 ++++++++++++++-------------------------
  1 file changed, 20 insertions(+), 37 deletions(-)
drivers/staging/iio/adc/ad7280a.c +20 -37
index d215edf..3706b3e 100644
@@ -486,8 +486,8 @@ static int ad7280_channel_init(struct ad7280_state *st)
  {
  	int dev, ch, cnt;
 
-	st->channels = kcalloc((st->slave_num + 1) * 12 + 2,
-			       sizeof(*st->channels), GFP_KERNEL);
+	st->channels = devm_kcalloc(&st->spi->dev, (st->slave_num + 1) * 12 + 2,
+				    sizeof(*st->channels), GFP_KERNEL);
  	if (st->channels == NULL)
  		return -ENOMEM;
 

@@ -547,8 +547,9 @@ static int ad7280_attr_init(struct ad7280_state *st)
  {
  	int dev, ch, cnt;
 
-	st->iio_attr = kzalloc(sizeof(*st->iio_attr) * (st->slave_num + 1) *
-				AD7280A_CELLS_PER_DEV * 2, GFP_KERNEL);
+	st->iio_attr = devm_kzalloc(&st->spi->dev, sizeof(*st->iio_attr) *
+				    (st->slave_num + 1) *
+				    AD7280A_CELLS_PER_DEV * 2, GFP_KERNEL);
  	if (st->iio_attr == NULL)
  		return -ENOMEM;
 

@@ -564,7 +565,7 @@ static int ad7280_attr_init(struct ad7280_state *st)
  			st->iio_attr[cnt].dev_attr.store =
  				ad7280_store_balance_sw;
  			st->iio_attr[cnt].dev_attr.attr.name =
-				kasprintf(GFP_KERNEL,
+				devm_kasprintf(&st->spi->dev, GFP_KERNEL,
  					"in%d-in%d_balance_switch_en",
  					(dev * AD7280A_CELLS_PER_DEV) + ch,
  					(dev * AD7280A_CELLS_PER_DEV) + ch + 1);

@@ -581,7 +582,8 @@ static int ad7280_attr_init(struct ad7280_state *st)
  			st->iio_attr[cnt].dev_attr.store =
  				ad7280_store_balance_timer;
  			st->iio_attr[cnt].dev_attr.attr.name =
-				kasprintf(GFP_KERNEL, "in%d-in%d_balance_timer",
+				devm_kasprintf(&st->spi->dev, GFP_KERNEL,
+					"in%d-in%d_balance_timer",
  					(dev * AD7280A_CELLS_PER_DEV) + ch,
  					(dev * AD7280A_CELLS_PER_DEV) + ch + 1);
  			ad7280_attributes[cnt] =

@@ -900,48 +902,36 @@ static int ad7280_probe(struct spi_device *spi)
 
  	ret = ad7280_attr_init(st);
  	if (ret < 0)
-		goto error_free_channels;
+		return ret;
 
-	ret = iio_device_register(indio_dev);
+	ret = devm_iio_device_register(&spi->dev, indio_dev);
  	if (ret)
-		goto error_free_attr;
+		return ret;
 
  	if (spi->irq > 0) {
  		ret = ad7280_write(st, AD7280A_DEVADDR_MASTER,
  				   AD7280A_ALERT, 1,
  				   AD7280A_ALERT_RELAY_SIG_CHAIN_DOWN);
  		if (ret)
-			goto error_unregister;
+			return ret;
 
  		ret = ad7280_write(st, AD7280A_DEVADDR(st->slave_num),
  				   AD7280A_ALERT, 0,
  				   AD7280A_ALERT_GEN_STATIC_HIGH |
  				   (pdata->chain_last_alert_ignore & 0xF));
  		if (ret)
-			goto error_unregister;
-
-		ret = request_threaded_irq(spi->irq,
-					   NULL,
-					   ad7280_event_handler,
-					   IRQF_TRIGGER_FALLING |
-					   IRQF_ONESHOT,
-					   indio_dev->name,
-					   indio_dev);
+			return ret;
+
+		ret = devm_request_threaded_irq(&spi->dev, spi->irq, NULL,
+						ad7280_event_handler,
+						IRQF_TRIGGER_FALLING |
+						IRQF_ONESHOT, indio_dev->name,
+						indio_dev);
  		if (ret)
-			goto error_unregister;
+			return ret;
  	}
 
  	return 0;
-error_unregister:
-	iio_device_unregister(indio_dev);
-
-error_free_attr:
-	kfree(st->iio_attr);
-
-error_free_channels:
-	kfree(st->channels);
-
-	return ret;
  }
 
  static int ad7280_remove(struct spi_device *spi)

@@ -949,16 +939,9 @@ static int ad7280_remove(struct spi_device *spi)
  	struct iio_dev *indio_dev = spi_get_drvdata(spi);
  	struct ad7280_state *st = iio_priv(indio_dev);
 
-	if (spi->irq > 0)
-		free_irq(spi->irq, indio_dev);
-	iio_device_unregister(indio_dev);
-
  	ad7280_write(st, AD7280A_DEVADDR_MASTER, AD7280A_CONTROL_HB, 1,
  			AD7280A_CTRL_HB_PWRDN_SW | st->ctrl_hb);
 
-	kfree(st->channels);
-	kfree(st->iio_attr);
-
  	return 0;
  }
#2 lore raw
Quoted 12 lines · Jonathan Cameron
Quoted 7 lines · Himangi Saraogi

This patch moves data allocated using unmanaged interfaces to managed interfaces like devm_kzalloc, devm_iio_device_register, devm_kasprintf, devm_request_threaded_irq and does away with the calls to free the allocated memory in the probe and remove functions. Some labels in the probe function are also removed.

Signed-off-by Himangi Saraogi <[email protected]>

cc'ing Lars who tends to look after Analog Devices drivers and more specifically was involved in the thread that lead to this patch.

Quoted 6 lines

Also, the devm_kasprintf does not appear to be in upstream so this patch should have made an dependency clear.

https://lkml.org/lkml/2014/7/16/667

What is the intended merge path for that patch?

Greg said that he would be taking it. He already took the devm_kasprintf patch.

julia

Quoted 144 lines
Quoted 5 lines

---
I am not very sure if devmifying request_irq is a good idea as it leads to the freeing of the interrupt after the ad7280_write.

Agreed. I wouldn't do it!

Otherwise, all looks fine.

Quoted 134 lines
  drivers/staging/iio/adc/ad7280a.c | 57
++++++++++++++-------------------------
  1 file changed, 20 insertions(+), 37 deletions(-)
drivers/staging/iio/adc/ad7280a.c +6 -5
index d215edf..3706b3e 100644
@@ -486,8 +486,8 @@ static int ad7280_channel_init(struct ad7280_state *st)
  {
  	int dev, ch, cnt;
 
-	st->channels = kcalloc((st->slave_num + 1) * 12 + 2,
-			       sizeof(*st->channels), GFP_KERNEL);
+	st->channels = devm_kcalloc(&st->spi->dev, (st->slave_num + 1) * 12 +
 2,
+				    sizeof(*st->channels), GFP_KERNEL);
  	if (st->channels == NULL)
  		return -ENOMEM;

@@ -547,8 +547,9 @@ static int ad7280_attr_init(struct ad7280_state *st)
  {
  	int dev, ch, cnt;
 
-	st->iio_attr = kzalloc(sizeof(*st->iio_attr) * (st->slave_num + 1) *
-				AD7280A_CELLS_PER_DEV * 2, GFP_KERNEL);
+	st->iio_attr = devm_kzalloc(&st->spi->dev, sizeof(*st->iio_attr) *
+				    (st->slave_num + 1) *
+				    AD7280A_CELLS_PER_DEV * 2, GFP_KERNEL);
  	if (st->iio_attr == NULL)
  		return -ENOMEM;
 

@@ -564,7 +565,7 @@ static int ad7280_attr_init(struct ad7280_state *st)
  			st->iio_attr[cnt].dev_attr.store =
  				ad7280_store_balance_sw;
  			st->iio_attr[cnt].dev_attr.attr.name =
-				kasprintf(GFP_KERNEL,
+				devm_kasprintf(&st->spi->dev, GFP_KERNEL,
  					"in%d-in%d_balance_switch_en",
  					(dev * AD7280A_CELLS_PER_DEV) + ch,
  					(dev * AD7280A_CELLS_PER_DEV) + ch +

1);

drivers/staging/iio/adc/ad7280a.c +2 -1
@@ -581,7 +582,8 @@ static int ad7280_attr_init(struct ad7280_state *st)
  			st->iio_attr[cnt].dev_attr.store =
  				ad7280_store_balance_timer;
  			st->iio_attr[cnt].dev_attr.attr.name =
-				kasprintf(GFP_KERNEL,
 "in%d-in%d_balance_timer",
+				devm_kasprintf(&st->spi->dev, GFP_KERNEL,
+					"in%d-in%d_balance_timer",
  					(dev * AD7280A_CELLS_PER_DEV) + ch,
  					(dev * AD7280A_CELLS_PER_DEV) + ch +

1);
ad7280_attributes[cnt] =

drivers/staging/iio/adc/ad7280a.c +12 -31
@@ -900,48 +902,36 @@ static int ad7280_probe(struct spi_device *spi)
 
  	ret = ad7280_attr_init(st);
  	if (ret < 0)
-		goto error_free_channels;
+		return ret;
 
-	ret = iio_device_register(indio_dev);
+	ret = devm_iio_device_register(&spi->dev, indio_dev);
  	if (ret)
-		goto error_free_attr;
+		return ret;
 
  	if (spi->irq > 0) {
  		ret = ad7280_write(st, AD7280A_DEVADDR_MASTER,
  				   AD7280A_ALERT, 1,
  				   AD7280A_ALERT_RELAY_SIG_CHAIN_DOWN);
  		if (ret)
-			goto error_unregister;
+			return ret;
 
  		ret = ad7280_write(st, AD7280A_DEVADDR(st->slave_num),
  				   AD7280A_ALERT, 0,
  				   AD7280A_ALERT_GEN_STATIC_HIGH |
  				   (pdata->chain_last_alert_ignore & 0xF));
  		if (ret)
-			goto error_unregister;
-
-		ret = request_threaded_irq(spi->irq,
-					   NULL,
-					   ad7280_event_handler,
-					   IRQF_TRIGGER_FALLING |
-					   IRQF_ONESHOT,
-					   indio_dev->name,
-					   indio_dev);
+			return ret;
+
+		ret = devm_request_threaded_irq(&spi->dev, spi->irq, NULL,
+						ad7280_event_handler,
+						IRQF_TRIGGER_FALLING |
+						IRQF_ONESHOT, indio_dev->name,
+						indio_dev);
  		if (ret)
-			goto error_unregister;
+			return ret;
  	}
 
  	return 0;
-error_unregister:
-	iio_device_unregister(indio_dev);
-
-error_free_attr:
-	kfree(st->iio_attr);
-
-error_free_channels:
-	kfree(st->channels);
-
-	return ret;
  }
 
  static int ad7280_remove(struct spi_device *spi)

@@ -949,16 +939,9 @@ static int ad7280_remove(struct spi_device *spi)
  	struct iio_dev *indio_dev = spi_get_drvdata(spi);
  	struct ad7280_state *st = iio_priv(indio_dev);
 
-	if (spi->irq > 0)
-		free_irq(spi->irq, indio_dev);
-	iio_device_unregister(indio_dev);
-
  	ad7280_write(st, AD7280A_DEVADDR_MASTER, AD7280A_CONTROL_HB, 1,
  			AD7280A_CTRL_HB_PWRDN_SW | st->ctrl_hb);
 
-	kfree(st->channels);
-	kfree(st->iio_attr);
-
  	return 0;
  }
maintainer
#3 lore raw
Quoted 19 lines · Julia Lawall
Quoted 15 lines · Jonathan Cameron
Quoted 7 lines · Himangi Saraogi

This patch moves data allocated using unmanaged interfaces to managed interfaces like devm_kzalloc, devm_iio_device_register, devm_kasprintf, devm_request_threaded_irq and does away with the calls to free the allocated memory in the probe and remove functions. Some labels in the probe function are also removed.

Signed-off-by Himangi Saraogi <[email protected]>

cc'ing Lars who tends to look after Analog Devices drivers and more specifically was involved in the thread that lead to this patch.

Also, the devm_kasprintf does not appear to be in upstream so this patch should have made an dependency clear.

Quoted 6 lines
Quoted 3 lines

https://lkml.org/lkml/2014/7/16/667

What is the intended merge path for that patch?

Greg said that he would be taking it. He already took the devm_kasprintf patch.

Cool. That's fine (and now you mention it - he did say that in the thread I just forgot). Still, it did want mentioning in the description for this patch as well.

Quoted 152 lines

julia

Quoted 135 lines
Quoted 5 lines

---
I am not very sure if devmifying request_irq is a good idea as it leads to the freeing of the interrupt after the ad7280_write.

Agreed. I wouldn't do it!

Otherwise, all looks fine.

Quoted 127 lines
   drivers/staging/iio/adc/ad7280a.c | 57
++++++++++++++-------------------------
   1 file changed, 20 insertions(+), 37 deletions(-)
drivers/staging/iio/adc/ad7280a.c +6 -5
index d215edf..3706b3e 100644
@@ -486,8 +486,8 @@ static int ad7280_channel_init(struct ad7280_state *st)
   {
   	int dev, ch, cnt;
 
-	st->channels = kcalloc((st->slave_num + 1) * 12 + 2,
-			       sizeof(*st->channels), GFP_KERNEL);
+	st->channels = devm_kcalloc(&st->spi->dev, (st->slave_num + 1) * 12 +
 2,
+				    sizeof(*st->channels), GFP_KERNEL);
   	if (st->channels == NULL)
   		return -ENOMEM;

@@ -547,8 +547,9 @@ static int ad7280_attr_init(struct ad7280_state *st)
   {
   	int dev, ch, cnt;
 
-	st->iio_attr = kzalloc(sizeof(*st->iio_attr) * (st->slave_num + 1) *
-				AD7280A_CELLS_PER_DEV * 2, GFP_KERNEL);
+	st->iio_attr = devm_kzalloc(&st->spi->dev, sizeof(*st->iio_attr) *
+				    (st->slave_num + 1) *
+				    AD7280A_CELLS_PER_DEV * 2, GFP_KERNEL);
   	if (st->iio_attr == NULL)
   		return -ENOMEM;
 

@@ -564,7 +565,7 @@ static int ad7280_attr_init(struct ad7280_state *st)
   			st->iio_attr[cnt].dev_attr.store =
   				ad7280_store_balance_sw;
   			st->iio_attr[cnt].dev_attr.attr.name =
-				kasprintf(GFP_KERNEL,
+				devm_kasprintf(&st->spi->dev, GFP_KERNEL,
   					"in%d-in%d_balance_switch_en",
   					(dev * AD7280A_CELLS_PER_DEV) + ch,
   					(dev * AD7280A_CELLS_PER_DEV) + ch +

1);

drivers/staging/iio/adc/ad7280a.c +2 -1
@@ -581,7 +582,8 @@ static int ad7280_attr_init(struct ad7280_state *st)
   			st->iio_attr[cnt].dev_attr.store =
   				ad7280_store_balance_timer;
   			st->iio_attr[cnt].dev_attr.attr.name =
-				kasprintf(GFP_KERNEL,
 "in%d-in%d_balance_timer",
+				devm_kasprintf(&st->spi->dev, GFP_KERNEL,
+					"in%d-in%d_balance_timer",
   					(dev * AD7280A_CELLS_PER_DEV) + ch,
   					(dev * AD7280A_CELLS_PER_DEV) + ch +

1);
   			ad7280_attributes[cnt] =
drivers/staging/iio/adc/ad7280a.c +12 -28
@@ -900,48 +902,36 @@ static int ad7280_probe(struct spi_device *spi)
 
   	ret = ad7280_attr_init(st);
   	if (ret < 0)
-		goto error_free_channels;
+		return ret;
 
-	ret = iio_device_register(indio_dev);
+	ret = devm_iio_device_register(&spi->dev, indio_dev);
   	if (ret)
-		goto error_free_attr;
+		return ret;
 
   	if (spi->irq > 0) {
   		ret = ad7280_write(st, AD7280A_DEVADDR_MASTER,
   				   AD7280A_ALERT, 1,
   				   AD7280A_ALERT_RELAY_SIG_CHAIN_DOWN);
   		if (ret)
-			goto error_unregister;
+			return ret;
 
   		ret = ad7280_write(st, AD7280A_DEVADDR(st->slave_num),
   				   AD7280A_ALERT, 0,
   				   AD7280A_ALERT_GEN_STATIC_HIGH |
   				   (pdata->chain_last_alert_ignore & 0xF));
   		if (ret)
-			goto error_unregister;
-
-		ret = request_threaded_irq(spi->irq,
-					   NULL,
-					   ad7280_event_handler,
-					   IRQF_TRIGGER_FALLING |
-					   IRQF_ONESHOT,
-					   indio_dev->name,
-					   indio_dev);
+			return ret;
+
+		ret = devm_request_threaded_irq(&spi->dev, spi->irq, NULL,
+						ad7280_event_handler,
+						IRQF_TRIGGER_FALLING |
+						IRQF_ONESHOT, indio_dev->name,
+						indio_dev);
   		if (ret)
-			goto error_unregister;
+			return ret;
   	}
 
   	return 0;
-error_unregister:
-	iio_device_unregister(indio_dev);
-
-error_free_attr:
-	kfree(st->iio_attr);
-
-error_free_channels:
-	kfree(st->channels);
-
-	return ret;
   }
 
   static int ad7280_remove(struct spi_device *spi)

@@ -949,16 +939,9 @@ static int ad7280_remove(struct spi_device *spi)
   	struct iio_dev *indio_dev = spi_get_drvdata(spi);
   	struct ad7280_state *st = iio_priv(indio_dev);
 
-	if (spi->irq > 0)
-		free_irq(spi->irq, indio_dev);
-	iio_device_unregister(indio_dev);
-
   	ad7280_write(st, AD7280A_DEVADDR_MASTER, AD7280A_CONTROL_HB, 1,
   			AD7280A_CTRL_HB_PWRDN_SW | st->ctrl_hb);
Quoted 9 lines

drivers/staging/iio/adc/ad7280a.c static int ad7280_remove(struct spi_device *spi) · line 947

Quoted 7 lines
drivers/staging/iio/adc/ad7280a.c +0 -3
-	kfree(st->channels);
-	kfree(st->iio_attr);
-
   	return 0;
   }

--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#4 lore raw
Quoted 22 lines · Jonathan Cameron
Quoted 21 lines · Julia Lawall
Quoted 17 lines · Jonathan Cameron
Quoted 7 lines · Himangi Saraogi

This patch moves data allocated using unmanaged interfaces to managed interfaces like devm_kzalloc, devm_iio_device_register, devm_kasprintf, devm_request_threaded_irq and does away with the calls to free the allocated memory in the probe and remove functions. Some labels in the probe function are also removed.

Signed-off-by Himangi Saraogi <[email protected]>

cc'ing Lars who tends to look after Analog Devices drivers and more specifically was involved in the thread that lead to this patch.

Also, the devm_kasprintf does not appear to be in upstream so this patch should have made an dependency clear.

https://lkml.org/lkml/2014/7/16/667

Quoted 7 lines
Quoted 4 lines
Quoted 1 line

What is the intended merge path for that patch?

Greg said that he would be taking it. He already took the devm_kasprintf patch.

Cool. That's fine (and now you mention it - he did say that in the thread I just forgot). Still, it did want mentioning in the description for this patch as well.

Indeed. Sorry about that.

julia

Quoted 161 lines
Quoted 159 lines

julia

Quoted 142 lines
Quoted 5 lines

---
I am not very sure if devmifying request_irq is a good idea as it leads to the freeing of the interrupt after the ad7280_write.

Agreed. I wouldn't do it!

Otherwise, all looks fine.

Quoted 134 lines
   drivers/staging/iio/adc/ad7280a.c | 57
++++++++++++++-------------------------
   1 file changed, 20 insertions(+), 37 deletions(-)
drivers/staging/iio/adc/ad7280a.c +2 -2
index d215edf..3706b3e 100644
@@ -486,8 +486,8 @@ static int ad7280_channel_init(struct ad7280_state
 *st)
   {
   	int dev, ch, cnt;
 
-	st->channels = kcalloc((st->slave_num + 1) * 12 + 2,
-			       sizeof(*st->channels), GFP_KERNEL);
+	st->channels = devm_kcalloc(&st->spi->dev, (st->slave_num + 1) * 12 +
 2,
+				    sizeof(*st->channels), GFP_KERNEL);
   	if (st->channels == NULL)

   		return -ENOMEM;
drivers/staging/iio/adc/ad7280a.c +6 -3
@@ -547,8 +547,9 @@ static int ad7280_attr_init(struct ad7280_state *st)
   {
   	int dev, ch, cnt;
 
-	st->iio_attr = kzalloc(sizeof(*st->iio_attr) * (st->slave_num + 1) *
-				AD7280A_CELLS_PER_DEV * 2, GFP_KERNEL);
+	st->iio_attr = devm_kzalloc(&st->spi->dev, sizeof(*st->iio_attr) *
+				    (st->slave_num + 1) *
+				    AD7280A_CELLS_PER_DEV * 2, GFP_KERNEL);
   	if (st->iio_attr == NULL)
   		return -ENOMEM;
 

@@ -564,7 +565,7 @@ static int ad7280_attr_init(struct ad7280_state *st)
   			st->iio_attr[cnt].dev_attr.store =
   				ad7280_store_balance_sw;
   			st->iio_attr[cnt].dev_attr.attr.name =
-				kasprintf(GFP_KERNEL,
+				devm_kasprintf(&st->spi->dev, GFP_KERNEL,
   					"in%d-in%d_balance_switch_en",
   					(dev * AD7280A_CELLS_PER_DEV)
+ ch,
   					(dev * AD7280A_CELLS_PER_DEV)
+ ch +

1);

drivers/staging/iio/adc/ad7280a.c +4 -1
@@ -581,7 +582,8 @@ static int ad7280_attr_init(struct ad7280_state *st)
   			st->iio_attr[cnt].dev_attr.store =
   				ad7280_store_balance_timer;
   			st->iio_attr[cnt].dev_attr.attr.name =
-				kasprintf(GFP_KERNEL,
 "in%d-in%d_balance_timer",
+				devm_kasprintf(&st->spi->dev, GFP_KERNEL,
+					"in%d-in%d_balance_timer",
   					(dev * AD7280A_CELLS_PER_DEV)
+ ch,
   					(dev * AD7280A_CELLS_PER_DEV)
+ ch +

1);
   			ad7280_attributes[cnt] =
drivers/staging/iio/adc/ad7280a.c +12 -24
@@ -900,48 +902,36 @@ static int ad7280_probe(struct spi_device *spi)
 
   	ret = ad7280_attr_init(st);
   	if (ret < 0)
-		goto error_free_channels;
+		return ret;
 
-	ret = iio_device_register(indio_dev);
+	ret = devm_iio_device_register(&spi->dev, indio_dev);
   	if (ret)
-		goto error_free_attr;
+		return ret;
 
   	if (spi->irq > 0) {
   		ret = ad7280_write(st, AD7280A_DEVADDR_MASTER,
   				   AD7280A_ALERT, 1,
   				   AD7280A_ALERT_RELAY_SIG_CHAIN_DOWN);
   		if (ret)
-			goto error_unregister;
+			return ret;
 
   		ret = ad7280_write(st, AD7280A_DEVADDR(st->slave_num),
   				   AD7280A_ALERT, 0,
   				   AD7280A_ALERT_GEN_STATIC_HIGH |
   				   (pdata->chain_last_alert_ignore &
 0xF));
   		if (ret)
-			goto error_unregister;
-
-		ret = request_threaded_irq(spi->irq,
-					   NULL,
-					   ad7280_event_handler,
-					   IRQF_TRIGGER_FALLING |
-					   IRQF_ONESHOT,
-					   indio_dev->name,
-					   indio_dev);
+			return ret;
+
+		ret = devm_request_threaded_irq(&spi->dev, spi->irq, NULL,
+						ad7280_event_handler,
+						IRQF_TRIGGER_FALLING |
+						IRQF_ONESHOT, indio_dev->name,
+						indio_dev);
   		if (ret)
-			goto error_unregister;
+			return ret;
   	}
 
   	return 0;
-error_unregister:
-	iio_device_unregister(indio_dev);
-
-error_free_attr:
-	kfree(st->iio_attr);
-
-error_free_channels:
-	kfree(st->channels);
-
-	return ret;
   }

static int ad7280_remove(struct spi_device *spi)

drivers/staging/iio/adc/ad7280a.c +0 -4
@@ -949,16 +939,9 @@ static int ad7280_remove(struct spi_device *spi)
   	struct iio_dev *indio_dev = spi_get_drvdata(spi);
   	struct ad7280_state *st = iio_priv(indio_dev);
 
-	if (spi->irq > 0)
-		free_irq(spi->irq, indio_dev);
-	iio_device_unregister(indio_dev);
-
   	ad7280_write(st, AD7280A_DEVADDR_MASTER, AD7280A_CONTROL_HB,
 1,
   			AD7280A_CTRL_HB_PWRDN_SW | st->ctrl_hb);
Quoted 9 lines

drivers/staging/iio/adc/ad7280a.c static int ad7280_remove(struct spi_device *spi) · line 946

Quoted 7 lines
drivers/staging/iio/adc/ad7280a.c +0 -3
-	kfree(st->channels);
-	kfree(st->iio_attr);
-
   	return 0;
   }

--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html