Skip to contentSkip to conversation

at86rf230: cleanups and add missing devicetree documentation

PATCH In review Alexander Aring 2 participants 8 messages Mar 26, 2014 – Mar 27, 2014 1 message not in the archive

Subsystems (5)
Networking Network drivers Device tree bindings SPI +1 more subsystems
lore
Share & tools
lore

Alexander Aring

author
#0 lore raw

cover at86rf230: cleanups and add missing devicetree documentation

Hi,

this patch series contains some cleanups for at86rf230 driver.

The patch series contains:

- Removing of the irq_type inside the platform data and use irq_get_trigger_type instead.
- Use of devm_* functions.
- At least we add a missed devicetree documentation. Sorry I didn't create one at intial devicetree support patch.

- Alex

Alexander Aring (6):
at86rf230: use dev_* print functions
at86rf230: use irq_get_trigger_type
at86rf230: remove irq_type in request_irq
at86rf230: use devm_request_irq
at86rf230: use devm_gpio_request_one
devicetree: add at86rf230 bindings

 .../bindings/net/ieee802154/at86rf230.txt          |  23 +++++
 drivers/net/ieee802154/at86rf230.c                 | 100 ++++++---------------
 include/linux/spi/at86rf230.h                      |  14 ---
 3 files changed, 51 insertions(+), 86 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/ieee802154/at86rf230.txt
Signature · 6 lines
1.9.1

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

PATCH 1/6 at86rf230: use dev_* print functions

This patch removes the pr_* functions and replace it with the dev_* print functions.

Signed-off-by Alexander Aring <[email protected]>
---
 drivers/net/ieee802154/at86rf230.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
drivers/net/ieee802154/at86rf230.c +5 -4
index e8004ef..df290a5 100644
@@ -557,11 +557,12 @@ at86rf230_state(struct ieee802154_dev *dev, int state)
 	    (desired_status == STATE_RX_AACK_ON && val == STATE_BUSY_RX_AACK))
 		return 0;
 
-	pr_err("unexpected state change: %d, asked for %d\n", val, state);
+	dev_err(&lp->spi->dev, "unexpected state change: %d, asked for %d\n",
+		val, state);
 	return -EBUSY;
 
 err:
-	pr_err("error: %d\n", rc);
+	dev_err(&lp->spi->dev, "error: %d\n", rc);
 	return rc;
 }
 

@@ -699,7 +700,7 @@ at86rf230_xmit(struct ieee802154_dev *dev, struct sk_buff *skb)
 err_rx:
 	at86rf230_start(dev);
 err:
-	pr_err("error: %d\n", rc);
+	dev_err(&lp->spi->dev, "error: %d\n", rc);
 
 	spin_lock_irqsave(&lp->lock, flags);
 	lp->is_tx = 0;

@@ -732,7 +733,7 @@ static int at86rf230_rx(struct at86rf230_local *lp)
 
 	return 0;
 err:
-	pr_debug("received frame is too small\n");
+	dev_dbg(&lp->spi->dev, "received frame is too small\n");
 
 	kfree_skb(skb);
 	return -EINVAL;
Signature · 1 line
1.9.1
maintainer
#2 lore raw

PATCH 2/6 at86rf230: use irq_get_trigger_type

In reply to Alexander Aring

This patch removes the platform data for the irq_type. We use instead the irq_get_trigger_type function to get these flags which should already configured by the interrupt controller.

Signed-off-by Alexander Aring <[email protected]>
---
 drivers/net/ieee802154/at86rf230.c | 28 ++++++++--------------------
 include/linux/spi/at86rf230.h      | 14 --------------
 2 files changed, 8 insertions(+), 34 deletions(-)
drivers/net/ieee802154/at86rf230.c +8 -20
index df290a5..b6aa741 100644
@@ -23,6 +23,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/interrupt.h>
+#include <linux/irq.h>
 #include <linux/gpio.h>
 #include <linux/delay.h>
 #include <linux/mutex.h>

@@ -971,8 +972,7 @@ static int at86rf230_irq_polarity(struct at86rf230_local *lp, int pol)
 
 static int at86rf230_hw_init(struct at86rf230_local *lp)
 {
-	struct at86rf230_platform_data *pdata = lp->spi->dev.platform_data;
-	int rc, irq_pol;
+	int rc, irq_pol, irq_type;
 	u8 status;
 	u8 csma_seed[2];
 

@@ -984,8 +984,9 @@ static int at86rf230_hw_init(struct at86rf230_local *lp)
 	if (rc)
 		return rc;
 
+	irq_type = irq_get_trigger_type(lp->spi->irq);
 	/* configure irq polarity, defaults to high active */
-	if (pdata->irq_type & (IRQF_TRIGGER_FALLING | IRQF_TRIGGER_LOW))
+	if (irq_type & (IRQF_TRIGGER_FALLING | IRQF_TRIGGER_LOW))
 		irq_pol = IRQ_ACTIVE_LOW;
 	else
 		irq_pol = IRQ_ACTIVE_HIGH;

@@ -1041,7 +1042,6 @@ static struct at86rf230_platform_data *
 at86rf230_get_pdata(struct spi_device *spi)
 {
 	struct at86rf230_platform_data *pdata;
-	const char *irq_type;
 
 	if (!IS_ENABLED(CONFIG_OF) || !spi->dev.of_node)
 		return spi->dev.platform_data;

@@ -1053,19 +1053,6 @@ at86rf230_get_pdata(struct spi_device *spi)
 	pdata->rstn = of_get_named_gpio(spi->dev.of_node, "reset-gpio", 0);
 	pdata->slp_tr = of_get_named_gpio(spi->dev.of_node, "sleep-gpio", 0);
 
-	pdata->irq_type = IRQF_TRIGGER_RISING;
-	of_property_read_string(spi->dev.of_node, "irq-type", &irq_type);
-	if (!strcmp(irq_type, "level-high"))
-		pdata->irq_type = IRQF_TRIGGER_HIGH;
-	else if (!strcmp(irq_type, "level-low"))
-		pdata->irq_type = IRQF_TRIGGER_LOW;
-	else if (!strcmp(irq_type, "edge-rising"))
-		pdata->irq_type = IRQF_TRIGGER_RISING;
-	else if (!strcmp(irq_type, "edge-falling"))
-		pdata->irq_type = IRQF_TRIGGER_FALLING;
-	else
-		dev_warn(&spi->dev, "wrong irq-type specified using edge-rising\n");
-
 	spi->dev.platform_data = pdata;
 done:
 	return pdata;

@@ -1080,7 +1067,7 @@ static int at86rf230_probe(struct spi_device *spi)
 	u8 part = 0, version = 0, status;
 	irq_handler_t irq_handler;
 	work_func_t irq_worker;
-	int rc;
+	int rc, irq_type;
 	const char *chip;
 	struct ieee802154_ops *ops = NULL;
 

@@ -1185,7 +1172,8 @@ static int at86rf230_probe(struct spi_device *spi)
 	dev->extra_tx_headroom = 0;
 	dev->flags = IEEE802154_HW_OMIT_CKSUM | IEEE802154_HW_AACK;
 
-	if (pdata->irq_type & (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING)) {
+	irq_type = irq_get_trigger_type(spi->irq);
+	if (irq_type & (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING)) {
 		irq_worker = at86rf230_irqwork;
 		irq_handler = at86rf230_isr;
 	} else {

@@ -1212,7 +1200,7 @@ static int at86rf230_probe(struct spi_device *spi)
 		goto err_hw_init;
 
 	rc = request_irq(spi->irq, irq_handler,
-			 IRQF_SHARED | pdata->irq_type,
+			 IRQF_SHARED | irq_type,
 			 dev_name(&spi->dev), lp);
 	if (rc)
 		goto err_hw_init;
include/linux/spi/at86rf230.h +0 -14
index aa327a8..b2b1afb 100644
@@ -26,20 +26,6 @@ struct at86rf230_platform_data {
 	int rstn;
 	int slp_tr;
 	int dig2;
-
-	/* Setting the irq_type will configure the driver to request
-	 * the platform irq trigger type according to the given value
-	 * and configure the interrupt polarity of the device to the
-	 * corresponding polarity.
-	 *
-	 * Allowed values are: IRQF_TRIGGER_RISING, IRQF_TRIGGER_FALLING,
-	 *                     IRQF_TRIGGER_HIGH and IRQF_TRIGGER_LOW
-	 *
-	 * Setting it to 0, the driver does not touch the trigger type
-	 * configuration of the interrupt and sets the interrupt polarity
-	 * of the device to high active (the default value).
-	 */
-	int irq_type;
 };
 
 #endif
Signature · 1 line
1.9.1

A message that is not in the archive (sent off-list or not yet received): 1395843117-16494-1-git-send-email-alex.aring-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org

author
#4 lore raw

PATCH 3/6 at86rf230: remove irq_type in request_irq

We don't need to set these values at request_irq. The interrupt line is already configured to same value like irq_get_trigger_type returned.

Signed-off-by Alexander Aring <[email protected]>
---
 drivers/net/ieee802154/at86rf230.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/net/ieee802154/at86rf230.c +1 -2
index b6aa741..e034376 100644
@@ -1199,8 +1199,7 @@ static int at86rf230_probe(struct spi_device *spi)
 	if (rc)
 		goto err_hw_init;
 
-	rc = request_irq(spi->irq, irq_handler,
-			 IRQF_SHARED | irq_type,
+	rc = request_irq(spi->irq, irq_handler, IRQF_SHARED,
 			 dev_name(&spi->dev), lp);
 	if (rc)
 		goto err_hw_init;
Signature · 9 lines
1.9.1


------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and their
applications. Written by three acclaimed leaders in the field,
this first edition is now available. Download your free book today!
http://p.sf.net/sfu/13534_NeoTech
author
#5 lore raw

PATCH 4/6 at86rf230: use devm_request_irq

In reply to a message not in the archive

This patch replace request_irq with devm_request_irq. With devm_request_irq we don't need to care about freeing the irq.

Signed-off-by Alexander Aring <[email protected]>
---
 drivers/net/ieee802154/at86rf230.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)
drivers/net/ieee802154/at86rf230.c +4 -7
index e034376..a3f9d91 100644
@@ -1199,24 +1199,22 @@ static int at86rf230_probe(struct spi_device *spi)
 	if (rc)
 		goto err_hw_init;
 
-	rc = request_irq(spi->irq, irq_handler, IRQF_SHARED,
-			 dev_name(&spi->dev), lp);
+	rc = devm_request_irq(&spi->dev, spi->irq, irq_handler, IRQF_SHARED,
+			      dev_name(&spi->dev), lp);
 	if (rc)
 		goto err_hw_init;
 
 	/* Read irq status register to reset irq line */
 	rc = at86rf230_read_subreg(lp, RG_IRQ_STATUS, 0xff, 0, &status);
 	if (rc)
-		goto err_irq;
+		goto err_hw_init;
 
 	rc = ieee802154_register_device(lp->dev);
 	if (rc)
-		goto err_irq;
+		goto err_hw_init;
 
 	return rc;
 
-err_irq:
-	free_irq(spi->irq, lp);
 err_hw_init:
 	flush_work(&lp->irqwork);
 	spi_set_drvdata(spi, NULL);

@@ -1239,7 +1237,6 @@ static int at86rf230_remove(struct spi_device *spi)
 
 	ieee802154_unregister_device(lp->dev);
 
-	free_irq(spi->irq, lp);
 	flush_work(&lp->irqwork);
 
 	if (gpio_is_valid(pdata->slp_tr))
Signature · 9 lines
1.9.1


------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and their
applications. Written by three acclaimed leaders in the field,
this first edition is now available. Download your free book today!
http://p.sf.net/sfu/13534_NeoTech
author
#6 lore raw

PATCH 5/6 at86rf230: use devm_gpio_request_one

In reply to a message not in the archive

This patch replace the gpio_request functions with devm_gpio_request_one functions. Then we don't need to take care about freeing gpios.

Signed-off-by Alexander Aring <[email protected]>
---
 drivers/net/ieee802154/at86rf230.c | 53 +++++++++-----------------------------
 1 file changed, 12 insertions(+), 41 deletions(-)
drivers/net/ieee802154/at86rf230.c +12 -41
index a3f9d91..1044a02 100644
@@ -1083,27 +1083,17 @@ static int at86rf230_probe(struct spi_device *spi)
 	}
 
 	if (gpio_is_valid(pdata->rstn)) {
-		rc = gpio_request(pdata->rstn, "rstn");
+		rc = devm_gpio_request_one(&spi->dev, pdata->rstn,
+					   GPIOF_OUT_INIT_HIGH, "rstn");
 		if (rc)
 			return rc;
 	}
 
 	if (gpio_is_valid(pdata->slp_tr)) {
-		rc = gpio_request(pdata->slp_tr, "slp_tr");
+		rc = devm_gpio_request_one(&spi->dev, pdata->slp_tr,
+					   GPIOF_OUT_INIT_LOW, "slp_tr");
 		if (rc)
-			goto err_slp_tr;
-	}
-
-	if (gpio_is_valid(pdata->rstn)) {
-		rc = gpio_direction_output(pdata->rstn, 1);
-		if (rc)
-			goto err_gpio_dir;
-	}
-
-	if (gpio_is_valid(pdata->slp_tr)) {
-		rc = gpio_direction_output(pdata->slp_tr, 0);
-		if (rc)
-			goto err_gpio_dir;
+			return rc;
 	}
 
 	/* Reset */

@@ -1117,13 +1107,12 @@ static int at86rf230_probe(struct spi_device *spi)
 
 	rc = __at86rf230_detect_device(spi, &man_id, &part, &version);
 	if (rc < 0)
-		goto err_gpio_dir;
+		return rc;
 
 	if (man_id != 0x001f) {
 		dev_err(&spi->dev, "Non-Atmel dev found (MAN_ID %02x %02x)\n",
 			man_id >> 8, man_id & 0xFF);
-		rc = -EINVAL;
-		goto err_gpio_dir;
+		return -EINVAL;
 	}
 
 	switch (part) {

@@ -1150,16 +1139,12 @@ static int at86rf230_probe(struct spi_device *spi)
 	}
 
 	dev_info(&spi->dev, "Detected %s chip version %d\n", chip, version);
-	if (!ops) {
-		rc = -ENOTSUPP;
-		goto err_gpio_dir;
-	}
+	if (!ops)
+		return -ENOTSUPP;
 
 	dev = ieee802154_alloc_device(sizeof(*lp), ops);
-	if (!dev) {
-		rc = -ENOMEM;
-		goto err_gpio_dir;
-	}
+	if (!dev)
+		return -ENOMEM;
 
 	lp = dev->priv;
 	lp->dev = dev;

@@ -1221,33 +1206,19 @@ err_hw_init:
 	mutex_destroy(&lp->bmux);
 	ieee802154_free_device(lp->dev);
 
-err_gpio_dir:
-	if (gpio_is_valid(pdata->slp_tr))
-		gpio_free(pdata->slp_tr);
-err_slp_tr:
-	if (gpio_is_valid(pdata->rstn))
-		gpio_free(pdata->rstn);
 	return rc;
 }
 
 static int at86rf230_remove(struct spi_device *spi)
 {
 	struct at86rf230_local *lp = spi_get_drvdata(spi);
-	struct at86rf230_platform_data *pdata = spi->dev.platform_data;
 
 	ieee802154_unregister_device(lp->dev);
-
 	flush_work(&lp->irqwork);
-
-	if (gpio_is_valid(pdata->slp_tr))
-		gpio_free(pdata->slp_tr);
-	if (gpio_is_valid(pdata->rstn))
-		gpio_free(pdata->rstn);
-
 	mutex_destroy(&lp->bmux);
 	ieee802154_free_device(lp->dev);
-
 	dev_dbg(&spi->dev, "unregistered at86rf230\n");
+
 	return 0;
 }
Signature · 9 lines
1.9.1


------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and their
applications. Written by three acclaimed leaders in the field,
this first edition is now available. Download your free book today!
http://p.sf.net/sfu/13534_NeoTech
maintainer
#7 lore raw

PATCH 6/6 devicetree: add at86rf230 bindings

In reply to Alexander Aring

This patch adds devicetree bindings for the at86rf230 IEEE 802.15.4 SPI device driver.

Signed-off-by Alexander Aring <[email protected]>
---
 .../bindings/net/ieee802154/at86rf230.txt          | 23 ++++++++++++++++++++++
 1 file changed, 23 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/ieee802154/at86rf230.txt
Documentation/devicetree/bindings/net/ieee802154/at86rf230.txt new file +23 -0
new file mode 100644
index 0000000..d3bbdded
@@ -0,0 +1,23 @@
+* AT86RF230 IEEE 802.15.4 *
+
+Required properties:
+  - compatible:		should be "atmel,at86rf230", "atmel,at86rf231",
+			"atmel,at86rf233" or "atmel,at86rf212"
+  - spi-max-frequency:	maximal bus speed, should be set to 7500000 depends
+			sync or async operation mode
+  - reg:		the chipselect index
+  - interrupts:		the interrupt generated by the device
+
+Optional properties:
+  - reset-gpio:		GPIO spec for the rstn pin
+  - sleep-gpio:		GPIO spec for the slp_tr pin
+
+Example:
+
+	at86rf231@0 {
+		compatible = "atmel,at86rf231";
+		spi-max-frequency = <7500000>;
+		reg = <0>;
+		interrupts = <19 1>;
+		interrupt-parent = <&gpio3>;
+	};
Signature · 1 line
1.9.1
maintainer
#8 lore raw
In reply to Alexander Aring

Hi David,

please drop this patch-series I found a small issue on while unloading the at86rf230 module.

We need to call devm_free_irq in remove callback before flush_work, otherwise the interrupt will use some already freed memory.

I will rework this patch serie and send a v2, sorry.

- Alex