[PATCH net-next 0/3] Driver for TI CC2520 Radio chip

[PATCH net-next 0/3] Driver for TI CC2520 Radio chip

[Varka Bhadram]

This patch series adds the support for cc2520 radio and
provides device tree bindings for cc2520

Varka Bhadram (3):
  ieee802154: cc2520: driver for TI cc2520 radio
  ieee802154: cc2520: add driver to kernel build system
  devicetree: add devicetree bindings for cc2520 driver

 .../devicetree/bindings/net/ieee802154/cc2520.txt  |   26 +
 drivers/net/ieee802154/Kconfig                     |   11 +
 drivers/net/ieee802154/Makefile                    |    1 +
 drivers/net/ieee802154/cc2520.c                    |  805 ++++++++++++++++++++
 include/linux/spi/cc2520.h                         |  176 +++++
 5 files changed, 1019 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/ieee802154/cc2520.txt
 create mode 100644 drivers/net/ieee802154/cc2520.c
 create mode 100644 include/linux/spi/cc2520.h

-- 
1.7.9.5


------------------------------------------------------------------------------
HPCC Systems Open Source Big Data Platform from LexisNexis Risk Solutions
Find What Matters Most in Your Big Data with HPCC Systems
Open Source. Fast. Scalable. Simple. Ideal for Dirty Data.
Leverages Graph Analysis for Fast Processing & Easy Data Exploration
http://p.sf.net/sfu/hpccsystems

[PATCH net-next 1/3] ieee802154: cc2520: driver for TI cc2520 radio

[Varka Bhadram]

Signed-off-by: Varka Bhadram <varkab-If5XQcfNmg0@public.gmane.org>
---
 drivers/net/ieee802154/cc2520.c |  805 +++++++++++++++++++++++++++++++++++++++
 include/linux/spi/cc2520.h      |  176 +++++++++
 2 files changed, 981 insertions(+)
 create mode 100644 drivers/net/ieee802154/cc2520.c
 create mode 100644 include/linux/spi/cc2520.h

diff --git a/drivers/net/ieee802154/cc2520.c b/drivers/net/ieee802154/cc2520.c
new file mode 100644
index 0000000..e8b5993
--- /dev/null
+++ b/drivers/net/ieee802154/cc2520.c
@@ -0,0 +1,805 @@
+/* Driver for TI CC2520 802.15.4 Wireless-PAN Networking controller
+ *
+ * Copyright (C) 2014 Varka Bhadram <varkab-If5XQcfNmg0@public.gmane.org>
+ *		      Md.Jamal Mohiuddin <mjmohiuddin-If5XQcfNmg0@public.gmane.org>
+ *		      P Sowjanya <sowjanyap-If5XQcfNmg0@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/gpio.h>
+#include <linux/delay.h>
+#include <linux/spi/spi.h>
+#include <linux/spi/cc2520.h>
+#include <linux/workqueue.h>
+#include <linux/interrupt.h>
+
+#include <linux/skbuff.h>
+
+#include <linux/pinctrl/consumer.h>
+
+#include <linux/of_device.h>
+#include <linux/of_platform.h>
+#include <linux/of_gpio.h>
+
+#include <net/mac802154.h>
+#include <net/wpan-phy.h>
+
+#define	SPI_COMMAND_BUFFER	3
+#define	HIGH			1
+#define	LOW			0
+#define	STATE_IDLE		0
+
+/*Driver private information */
+struct cc2520_private {
+	struct spi_device *spi;
+
+	struct ieee802154_dev *dev;
+
+	u8 *buf;
+	struct mutex buffer_mutex;
+
+	unsigned is_tx:1;
+	int fifo_irq;
+
+	int fifo_pin;
+	int fifop_pin;
+
+	struct work_struct fifop_irqwork;
+
+	spinlock_t lock;
+
+	struct completion tx_complete;
+};
+
+/*Generic Functions*/
+static int cc2520_cmd_strobe(struct cc2520_private *priv,
+						u8 cmd)
+{
+	int ret;
+	u8 status = 0xff;
+	struct spi_message msg;
+	struct spi_transfer xfer = {
+		.len = 0,
+		.tx_buf = priv->buf,
+		.rx_buf = priv->buf,
+	};
+
+	spi_message_init(&msg);
+	spi_message_add_tail(&xfer, &msg);
+
+	mutex_lock(&priv->buffer_mutex);
+	priv->buf[xfer.len++] = cmd;
+	dev_vdbg(&priv->spi->dev,
+			"command strobe buf[0] = %02x\n",
+			priv->buf[0]);
+
+	ret = spi_sync(priv->spi, &msg);
+	if (!ret)
+		status = priv->buf[0];
+	dev_vdbg(&priv->spi->dev,
+			"buf[0] = %02x\n", priv->buf[0]);
+	mutex_unlock(&priv->buffer_mutex);
+
+	return ret;
+}
+
+static int cc2520_get_status(struct cc2520_private *priv,
+						u8 *status)
+{
+	int ret;
+	struct spi_message msg;
+	struct spi_transfer xfer = {
+		.len = 0,
+		.tx_buf = priv->buf,
+		.rx_buf = priv->buf,
+	};
+
+	spi_message_init(&msg);
+	spi_message_add_tail(&xfer, &msg);
+
+	mutex_lock(&priv->buffer_mutex);
+	priv->buf[xfer.len++] = CC2520_CMD_SNOP;
+	dev_vdbg(&priv->spi->dev,
+			"get status command buf[0] = %02x\n", priv->buf[0]);
+
+	ret = spi_sync(priv->spi, &msg);
+	if (!ret)
+		*status = priv->buf[0];
+	dev_vdbg(&priv->spi->dev,
+			"buf[0] = %02x\n", priv->buf[0]);
+	mutex_unlock(&priv->buffer_mutex);
+
+	return ret;
+}
+
+static int cc2520_write_register(struct cc2520_private *priv,
+					u8 reg, u8 value)
+{
+	int status;
+	struct spi_message msg;
+	struct spi_transfer xfer = {
+		.len = 0,
+		.tx_buf = priv->buf,
+		.rx_buf = priv->buf,
+	};
+
+	spi_message_init(&msg);
+	spi_message_add_tail(&xfer, &msg);
+
+	mutex_lock(&priv->buffer_mutex);
+
+	if (reg <= CC2520_FREG_MASK) {
+		priv->buf[xfer.len++] = CC2520_CMD_REGISTER_WRITE | reg;
+		priv->buf[xfer.len++] = value;
+	} else {
+		priv->buf[xfer.len++] = CC2520_CMD_MEMORY_WRITE;
+		priv->buf[xfer.len++] = reg;
+		priv->buf[xfer.len++] = value;
+	}
+	status = spi_sync(priv->spi, &msg);
+	if (msg.status)
+		status = msg.status;
+
+	mutex_unlock(&priv->buffer_mutex);
+
+	return status;
+}
+
+static int cc2520_read_register(struct cc2520_private *priv,
+				u8 reg, u8 *data)
+{
+
+	int status;
+	struct spi_message msg;
+	struct spi_transfer xfer1 = {
+		.len = 0,
+		.tx_buf = priv->buf,
+		.rx_buf = priv->buf,
+	};
+
+	struct spi_transfer xfer2 = {
+		.len = 1,
+		.rx_buf = data,
+	};
+
+	spi_message_init(&msg);
+	spi_message_add_tail(&xfer1, &msg);
+	spi_message_add_tail(&xfer2, &msg);
+
+	mutex_lock(&priv->buffer_mutex);
+	priv->buf[xfer1.len++] = CC2520_CMD_MEMORY_READ;
+	priv->buf[xfer1.len++] = reg;
+
+	status = spi_sync(priv->spi, &msg);
+	dev_dbg(&priv->spi->dev,
+			"spi status = %d\n", status);
+	if (msg.status)
+		status = msg.status;
+
+	mutex_unlock(&priv->buffer_mutex);
+
+	return status;
+}
+
+static int cc2520_write_txfifo(struct cc2520_private *priv,
+			u8 *data, u8 len)
+{
+	int status;
+
+	/*
+	 *length byte must include FCS even
+	 *if it is calculated in the hardware
+	 */
+	int len_byte = len + 2;
+
+	struct spi_message msg;
+
+	struct spi_transfer xfer_head = {
+		.len = 0,
+		.tx_buf = priv->buf,
+		.rx_buf = priv->buf,
+	};
+	struct spi_transfer xfer_len = {
+		.len = 1,
+		.tx_buf = &len_byte,
+	};
+	struct spi_transfer xfer_buf = {
+		.len = len,
+		.tx_buf = data,
+	};
+
+	spi_message_init(&msg);
+	spi_message_add_tail(&xfer_head, &msg);
+	spi_message_add_tail(&xfer_len, &msg);
+	spi_message_add_tail(&xfer_buf, &msg);
+
+	mutex_lock(&priv->buffer_mutex);
+	priv->buf[xfer_head.len++] = CC2520_CMD_TXBUF;
+	dev_vdbg(&priv->spi->dev,
+			"TX_FIFO cmd buf[0] = %02x\n", priv->buf[0]);
+
+	status = spi_sync(priv->spi, &msg);
+	dev_vdbg(&priv->spi->dev, "status = %d\n", status);
+	if (msg.status)
+		status = msg.status;
+	dev_vdbg(&priv->spi->dev, "status = %d\n", status);
+	dev_vdbg(&priv->spi->dev, "buf[0] = %02x\n", priv->buf[0]);
+	mutex_unlock(&priv->buffer_mutex);
+
+	return status;
+}
+
+static int cc2520_read_rxfifo(struct cc2520_private *priv,
+			u8 *data, u8 *len, u8 *lqi)
+{
+	int status;
+	struct spi_message msg;
+
+	struct spi_transfer xfer_head = {
+		.len = 0,
+		.tx_buf = priv->buf,
+		.rx_buf = priv->buf,
+	};
+	struct spi_transfer xfer_buf = {
+		.len = *len,
+		.rx_buf = data,
+	};
+
+	spi_message_init(&msg);
+	spi_message_add_tail(&xfer_head, &msg);
+	spi_message_add_tail(&xfer_buf, &msg);
+
+	mutex_lock(&priv->buffer_mutex);
+	priv->buf[xfer_head.len++] = CC2520_CMD_RXBUF;
+
+	dev_vdbg(&priv->spi->dev, "read rxfifo buf[0] = %02x\n", priv->buf[0]);
+	dev_vdbg(&priv->spi->dev, "buf[1] = %02x\n", priv->buf[1]);
+
+	status = spi_sync(priv->spi, &msg);
+	dev_vdbg(&priv->spi->dev, "status = %d\n", status);
+	if (msg.status)
+		status = msg.status;
+	dev_vdbg(&priv->spi->dev, "status = %d\n", status);
+	dev_vdbg(&priv->spi->dev,
+				"return status buf[0] = %02x\n", priv->buf[0]);
+	dev_vdbg(&priv->spi->dev, "length buf[1] = %02x\n", priv->buf[1]);
+
+	*lqi = data[priv->buf[1] - 1] & 0x7f;
+
+	mutex_unlock(&priv->buffer_mutex);
+
+	return status;
+}
+
+static int cc2520_start(struct ieee802154_dev *dev)
+{
+	return cc2520_cmd_strobe(dev->priv, CC2520_CMD_SRXON);
+}
+
+static void cc2520_stop(struct ieee802154_dev *dev)
+{
+	cc2520_cmd_strobe(dev->priv, CC2520_CMD_SRFOFF);
+}
+
+static int cc2520_tx(struct ieee802154_dev *dev, struct sk_buff *skb)
+{
+	struct cc2520_private *priv = dev->priv;
+	unsigned long flags;
+	int rc;
+	u8 status = 0;
+
+	rc = cc2520_cmd_strobe(priv, CC2520_CMD_SFLUSHTX);
+	if (rc)
+		goto err_tx;
+
+	rc = cc2520_write_txfifo(priv, skb->data, skb->len);
+	if (rc)
+		goto err_tx;
+
+	rc = cc2520_get_status(priv, &status);
+	if (rc)
+		goto err_tx;
+
+	if (status & CC2520_STATUS_TX_UNDERFLOW) {
+		dev_err(&priv->spi->dev, "cc2520 tx underflow exception\n");
+		goto err_tx;
+	}
+
+	spin_lock_irqsave(&priv->lock, flags);
+	BUG_ON(priv->is_tx);
+	priv->is_tx = 1;
+	spin_unlock_irqrestore(&priv->lock, flags);
+
+	rc = cc2520_cmd_strobe(priv, CC2520_CMD_STXONCCA);
+	if (rc)
+		goto err;
+
+	rc = wait_for_completion_interruptible(&priv->tx_complete);
+	if (rc < 0)
+		goto err;
+
+	cc2520_cmd_strobe(priv, CC2520_CMD_SFLUSHTX);
+	cc2520_cmd_strobe(priv, CC2520_CMD_SRXON);
+
+	return rc;
+err:
+	spin_lock_irqsave(&priv->lock, flags);
+	priv->is_tx = 0;
+	spin_unlock_irqrestore(&priv->lock, flags);
+err_tx:
+	return rc;
+}
+
+
+static int cc2520_rx(struct cc2520_private *priv)
+{
+	u8 len = 0, lqi = 0, bytes = 1;
+	struct sk_buff *skb;
+
+	cc2520_read_rxfifo(priv, &len, &bytes, &lqi);
+
+	skb = alloc_skb(len, GFP_KERNEL);
+	if (!skb)
+		return -ENOMEM;
+
+	cc2520_read_rxfifo(priv, skb_put(skb, len), &len, &lqi);
+	if (len < 2) {
+		kfree_skb(skb);
+		return -EINVAL;
+	}
+
+	skb_trim(skb, skb->len - 2);
+
+	ieee802154_rx_irqsafe(priv->dev, skb, lqi);
+
+	dev_vdbg(&priv->spi->dev, "RXFIFO: %x %x\n", len, lqi);
+
+	return 0;
+}
+
+static int cc2520_ed(struct ieee802154_dev *dev, u8 *level)
+{
+	struct cc2520_private *priv = dev->priv;
+	u8 status = 0xff;
+	u8 rssi;
+	int ret;
+
+	ret = cc2520_read_register(priv , CC2520_RSSISTAT, &status);
+	if (ret)
+		return ret;
+
+	if (status != RSSI_VALID) {
+		ret = -EINVAL;
+		return ret;
+	}
+
+	ret = cc2520_read_register(priv , CC2520_RSSI, &rssi);
+	if (ret)
+		return ret;
+
+	/* level = RSSI(rssi) - OFFSET [dBm] : offset is 76dBm*/
+	*level = rssi - RSSI_OFFSET;
+
+	return ret;
+}
+
+static int cc2520_set_channel(struct ieee802154_dev *dev,
+				int page, int channel)
+{
+	struct cc2520_private *priv = dev->priv;
+	int ret;
+
+	might_sleep();
+	dev_dbg(&priv->spi->dev, "trying to set channel\n");
+
+	BUG_ON(page != 0);
+	BUG_ON(channel < CC2520_MINCHANNEL);
+	BUG_ON(channel > CC2520_MAXCHANNEL);
+
+	ret = cc2520_write_register(priv, CC2520_FREQCTRL,
+					11 + 5*(channel - 11));
+
+	return ret;
+}
+
+static struct ieee802154_ops cc2520_ops = {
+	.owner = THIS_MODULE,
+	.start = cc2520_start,
+	.stop = cc2520_stop,
+	.xmit = cc2520_tx,
+	.ed = cc2520_ed,
+	.set_channel = cc2520_set_channel,
+};
+
+static int cc2520_register(struct cc2520_private *priv)
+{
+	int ret = -ENOMEM;
+
+	priv->dev = ieee802154_alloc_device(sizeof(*priv), &cc2520_ops);
+	if (!priv->dev)
+		goto err_ret;
+
+	priv->dev->priv = priv;
+	priv->dev->parent = &priv->spi->dev;
+	priv->dev->extra_tx_headroom = 0;
+
+	/* We do support only 2.4 Ghz */
+	priv->dev->phy->channels_supported[0] = 0x7FFF800;
+	priv->dev->flags = IEEE802154_HW_OMIT_CKSUM;
+
+	dev_vdbg(&priv->spi->dev, "registered cc2520\n");
+	ret = ieee802154_register_device(priv->dev);
+	if (ret)
+		goto err_free_device;
+
+	return 0;
+
+err_free_device:
+	ieee802154_free_device(priv->dev);
+err_ret:
+	return ret;
+}
+
+static void cc2520_unregister(struct cc2520_private *priv)
+{
+	ieee802154_unregister_device(priv->dev);
+	ieee802154_free_device(priv->dev);
+}
+
+static void cc2520_fifop_irqwork(struct work_struct *work)
+{
+	struct cc2520_private *priv
+		= container_of(work, struct cc2520_private, fifop_irqwork);
+
+	dev_dbg(&priv->spi->dev, "fifop interrupt received\n");
+
+	if (gpio_get_value(priv->fifo_pin))
+		cc2520_rx(priv);
+	else
+		dev_err(&priv->spi->dev, "rxfifo overflow\n");
+
+	cc2520_cmd_strobe(priv, CC2520_CMD_SFLUSHRX);
+	cc2520_cmd_strobe(priv, CC2520_CMD_SFLUSHRX);
+}
+
+static irqreturn_t cc2520_fifop_isr(int irq, void *data)
+{
+	struct cc2520_private *priv = data;
+
+	schedule_work(&priv->fifop_irqwork);
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t cc2520_sfd_isr(int irq, void *data)
+{
+	struct cc2520_private *priv = data;
+
+	spin_lock(&priv->lock);
+	if (priv->is_tx) {
+		dev_dbg(&priv->spi->dev, "SFD for TX\n");
+		priv->is_tx = 0;
+		complete(&priv->tx_complete);
+	} else
+		dev_dbg(&priv->spi->dev, "SFD for RX\n");
+	spin_unlock(&priv->lock);
+
+	return IRQ_HANDLED;
+}
+
+static int cc2520_hw_init(struct cc2520_private *priv)
+{
+	u8 status = 0, state = 0xff;
+	int ret;
+	int timeout = 100;
+
+	ret = cc2520_read_register(priv, CC2520_FSMSTAT1, &state);
+	if (ret)
+		goto err_ret;
+
+	if (state != STATE_IDLE) {
+		ret = -EINVAL;
+		return ret;
+	}
+
+	do {
+		ret = cc2520_get_status(priv, &status);
+		if (ret)
+			goto err_ret;
+
+		if (timeout-- <= 0) {
+			dev_err(&priv->spi->dev, "oscillator start failed!\n");
+			return ret;
+		}
+		udelay(1);
+	} while (!(status & CC2520_STATUS_XOSC32M_STABLE));
+
+	dev_vdbg(&priv->spi->dev, "oscillator successfully brought up\n");
+
+	/*Registers default value: section 28.1 in Datasheet*/
+	ret = cc2520_write_register(priv, CC2520_TXPOWER, 0xF7);
+	if (ret)
+		goto err_ret;
+
+	ret = cc2520_write_register(priv, CC2520_CCACTRL0, 0x1A);
+	if (ret)
+		goto err_ret;
+
+	ret = cc2520_write_register(priv, CC2520_MDMCTRL0, 0x85);
+	if (ret)
+		goto err_ret;
+
+	ret = cc2520_write_register(priv, CC2520_MDMCTRL1, 0x14);
+	if (ret)
+		goto err_ret;
+
+	ret = cc2520_write_register(priv, CC2520_RXCTRL, 0x3f);
+	if (ret)
+		goto err_ret;
+
+	ret = cc2520_write_register(priv, CC2520_FSCTRL, 0x5a);
+	if (ret)
+		goto err_ret;
+
+	ret = cc2520_write_register(priv, CC2520_FSCAL1, 0x2b);
+	if (ret)
+		goto err_ret;
+
+	ret = cc2520_write_register(priv, CC2520_AGCCTRL1, 0x11);
+	if (ret)
+		goto err_ret;
+
+	ret = cc2520_write_register(priv, CC2520_ADCTEST0, 0x10);
+	if (ret)
+		goto err_ret;
+
+	ret = cc2520_write_register(priv, CC2520_ADCTEST1, 0x0e);
+	if (ret)
+		goto err_ret;
+
+	ret = cc2520_write_register(priv, CC2520_ADCTEST2, 0x03);
+	if (ret)
+		goto err_ret;
+
+	ret = cc2520_write_register(priv, CC2520_FRMCTRL0, 0x60);
+	if (ret)
+		goto err_ret;
+
+	ret = cc2520_write_register(priv, CC2520_FRMCTRL1, 0x03);
+	if (ret)
+		goto err_ret;
+
+	ret = cc2520_write_register(priv, CC2520_FRMFILT0, 0x00);
+	if (ret)
+		goto err_ret;
+
+	ret = cc2520_write_register(priv, CC2520_FIFOPCTRL, 127);
+	if (ret)
+		goto err_ret;
+
+	return 0;
+
+err_ret:
+	return ret;
+}
+
+static struct cc2520_platform_data *
+cc2520_get_platform_data(struct spi_device *spi)
+{
+	struct cc2520_platform_data *pdata;
+	struct device_node __maybe_unused *np = spi->dev.of_node;
+	struct cc2520_private *priv = spi_get_drvdata(spi);
+
+	if (!IS_ENABLED(CONFIG_OF) || !np)
+		return spi->dev.platform_data;
+
+	pdata = devm_kzalloc(&spi->dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		goto done;
+
+	pdata->fifo = of_get_named_gpio(np, "fifo-gpio", 0);
+	priv->fifo_pin = pdata->fifo;
+
+	pdata->fifop = of_get_named_gpio(np, "fifop-gpio", 0);
+	priv->fifop_pin = pdata->fifop;
+
+	pdata->sfd = of_get_named_gpio(np, "sfd-gpio", 0);
+	pdata->cca = of_get_named_gpio(np, "cca-gpio", 0);
+	pdata->vreg = of_get_named_gpio(np, "vreg-gpio", 0);
+	pdata->reset = of_get_named_gpio(np, "reset-gpio", 0);
+
+	spi->dev.platform_data = pdata;
+
+done:
+	return pdata;
+}
+
+/*Driver probe function*/
+static int cc2520_probe(struct spi_device *spi)
+{
+	struct cc2520_private *priv;
+	struct pinctrl *pinctrl;
+	struct cc2520_platform_data *pdata;
+	struct device_node __maybe_unused *np = spi->dev.of_node;
+	int ret;
+
+	priv = kzalloc(sizeof(struct cc2520_private), GFP_KERNEL);
+	if (!priv) {
+		ret = -ENOMEM;
+		goto err_free_local;
+	}
+
+	spi_set_drvdata(spi, priv);
+
+	pinctrl = devm_pinctrl_get_select_default(&spi->dev);
+	if (IS_ERR(pinctrl))
+		dev_warn(&spi->dev,
+			"pinctrl pins are not configured from the driver");
+
+	pdata = cc2520_get_platform_data(spi);
+	if (!pdata) {
+		dev_err(&spi->dev, "no platform data\n");
+		return -EINVAL;
+	}
+
+	mutex_init(&priv->buffer_mutex);
+	INIT_WORK(&priv->fifop_irqwork, cc2520_fifop_irqwork);
+	spin_lock_init(&priv->lock);
+	init_completion(&priv->tx_complete);
+
+	priv->spi = spi;
+
+	priv->buf = kzalloc(SPI_COMMAND_BUFFER, GFP_KERNEL);
+	if (!priv->buf) {
+		ret = -ENOMEM;
+		goto err_free_buf;
+	}
+
+	/*Request all the gpio's*/
+	if (gpio_is_valid(pdata->fifo)) {
+		ret = devm_gpio_request_one(&spi->dev, pdata->fifo, 
+					GPIOF_IN, "fifo");
+		if (ret)
+			goto err_hw_init;
+	}
+
+	if (gpio_is_valid(pdata->cca)) {
+		ret = devm_gpio_request_one(&spi->dev, pdata->cca,
+					GPIOF_IN, "cca");
+		if (ret)
+			goto err_hw_init;
+	}
+
+	if (gpio_is_valid(pdata->fifop)) {
+		ret = devm_gpio_request_one(&spi->dev, pdata->fifop,
+					GPIOF_IN, "fifop");
+		if (ret)
+			goto err_hw_init;
+	}
+
+	if (gpio_is_valid(pdata->sfd)) {
+		ret = devm_gpio_request_one(&spi->dev, pdata->sfd,
+					GPIOF_IN, "sfd");
+		if (ret)
+			goto err_hw_init;
+	}
+
+	if (gpio_is_valid(pdata->reset)) {
+		ret = devm_gpio_request_one(&spi->dev, pdata->reset,
+					GPIOF_OUT_INIT_LOW, "reset");
+		if (ret)
+			goto err_hw_init;
+	}
+
+	if (gpio_is_valid(pdata->vreg)) {
+		ret = devm_gpio_request_one(&spi->dev, pdata->vreg,
+					GPIOF_OUT_INIT_LOW, "vreg");
+		if (ret)
+			goto err_hw_init;
+	}
+
+	gpio_set_value(pdata->vreg, HIGH);
+	udelay(100);
+
+	gpio_set_value(pdata->reset, HIGH);
+	udelay(200);
+
+	ret = cc2520_hw_init(priv);
+	if (ret)
+		goto err_hw_init;
+
+	/*Set up fifop interrupt */
+	priv->fifo_irq = gpio_to_irq(pdata->fifop);
+	ret = devm_request_irq(&spi->dev,
+				priv->fifo_irq,
+				cc2520_fifop_isr,
+				IRQF_TRIGGER_RISING,
+				dev_name(&spi->dev),
+				priv);
+	if (ret) {
+		dev_err(&spi->dev, "could not get fifop irq\n");
+		goto err_hw_init;
+	}
+
+	/*Set up sfd interrupt*/
+	ret = devm_request_irq(&spi->dev,
+				gpio_to_irq(pdata->sfd),
+				cc2520_sfd_isr,
+				IRQF_TRIGGER_FALLING,
+				dev_name(&spi->dev),
+				priv);
+	if (ret) {
+		dev_err(&spi->dev, "could not get sfd irq\n");
+		goto err_hw_init;
+	}
+
+	ret = cc2520_register(priv);
+	if (ret)
+		goto err_hw_init;
+
+	return 0;
+
+err_hw_init:
+	mutex_destroy(&priv->buffer_mutex);
+	flush_work(&priv->fifop_irqwork);
+
+err_free_buf:
+	kfree(priv->buf);
+
+err_free_local:
+	kfree(priv);
+
+	return ret;
+}
+
+/*Driver remove function*/
+static int cc2520_remove(struct spi_device *spi)
+{
+	struct cc2520_private *priv = spi_get_drvdata(spi);
+
+	cc2520_unregister(priv);
+
+	mutex_destroy(&priv->buffer_mutex);
+	flush_work(&priv->fifop_irqwork);
+
+	kfree(priv->buf);
+	kfree(priv);
+
+	return 0;
+}
+
+static const struct spi_device_id cc2520_ids[] = {
+	{"cc2520", 0},
+	{},
+};
+MODULE_DEVICE_TABLE(spi, cc2520_ids);
+
+static const struct of_device_id cc2520_of_ids[] = {
+	{.compatible = "ti,cc2520", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, cc2520_of_ids);
+
+/*SPI Driver Structure*/
+static struct spi_driver cc2520_driver = {
+	.driver = {
+		.name = "cc2520",
+		.bus = &spi_bus_type,
+		.owner = THIS_MODULE,
+		.of_match_table = of_match_ptr(cc2520_of_ids),
+	},
+	.id_table = cc2520_ids,
+	.probe = cc2520_probe,
+	.remove = cc2520_remove,
+};
+module_spi_driver(cc2520_driver);
+
+MODULE_DESCRIPTION("CC2520 Transceiver Driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/spi/cc2520.h b/include/linux/spi/cc2520.h
new file mode 100644
index 0000000..68a2c88
--- /dev/null
+++ b/include/linux/spi/cc2520.h
@@ -0,0 +1,176 @@
+#ifndef __CC2520_H
+#define __CC2520_H
+
+#define RSSI_VALID		0
+#define RSSI_OFFSET		78
+#define CC2520_FREG_MASK	0x3F
+
+struct cc2520_platform_data {
+	int fifo;
+	int fifop;
+	int cca;
+	int sfd;
+	int reset;
+	int vreg;
+};
+
+enum cc2520_memory_size {
+	CC2520_RAM_SIZE		= 640,
+	CC2520_FIFO_SIZE	= 128,
+};
+
+enum cc2520_address {
+	CC2520RAM_TXFIFO	= 0x100,
+	CC2520RAM_RXFIFO	= 0x180,
+	CC2520RAM_IEEEADDR	= 0x3EA,
+	CC2520RAM_PANID		= 0x3F2,
+	CC2520RAM_SHORTADDR	= 0x3F4,
+};
+
+/*status byte values */
+#define	CC2520_STATUS_XOSC32M_STABLE	(1 << 7)
+#define	CC2520_STATUS_RSSI_VALID	(1 << 6)
+#define	CC2520_STATUS_TX_UNDERFLOW	(1 << 3)
+
+/*IEEE 802.15.4 defined constants (2.4 GHz logical channels) */
+#define CC2520_MINCHANNEL		11
+#define CC2520_MAXCHANNEL		26
+#define CC2520_CHANNEL_SPACING		5
+
+/*command strobes*/
+#define	CC2520_CMD_SNOP		0x00
+#define	CC2520_CMD_IBUFLD		0x02
+#define	CC2520_CMD_SIBUFEX		0x03
+#define	CC2520_CMD_SSAMPLECCA		0x04
+#define	CC2520_CMD_SRES			0x0f
+#define	CC2520_CMD_MEMORY_MASK		0x0f
+#define	CC2520_CMD_MEMORY_READ		0x10
+#define	CC2520_CMD_MEMORY_WRITE		0x20
+#define	CC2520_CMD_RXBUF		0x30
+#define	CC2520_CMD_RXBUFCP		0x38
+#define	CC2520_CMD_RXBUFMOV		0x32
+#define	CC2520_CMD_TXBUF		0x3A
+#define	CC2520_CMD_TXBUFCP		0x3E
+#define	CC2520_CMD_RANDOM		0x3C
+#define	CC2520_CMD_SXOSCON		0x40
+#define	CC2520_CMD_STXCAL		0x41
+#define	CC2520_CMD_SRXON		0x42
+#define	CC2520_CMD_STXON		0x43
+#define	CC2520_CMD_STXONCCA		0x44
+#define	CC2520_CMD_SRFOFF		0x45
+#define	CC2520_CMD_SXOSCOFF		0x46
+#define	CC2520_CMD_SFLUSHRX		0x47
+#define	CC2520_CMD_SFLUSHTX		0x48
+#define	CC2520_CMD_SACK			0x49
+#define	CC2520_CMD_SACKPEND		0x4A
+#define	CC2520_CMD_SNACK		0x4B
+#define	CC2520_CMD_SRXMASKBITSET	0x4C
+#define	CC2520_CMD_SRXMASKBITCLR	0x4D
+#define	CC2520_CMD_RXMASKAND		0x4E
+#define	CC2520_CMD_RXMASKOR		0x4F
+#define	CC2520_CMD_MEMCP		0x50
+#define	CC2520_CMD_MEMCPR		0x52
+#define	CC2520_CMD_MEMXCP		0x54
+#define	CC2520_CMD_MEMXWR		0x56
+#define	CC2520_CMD_BCLR			0x58
+#define	CC2520_CMD_BSET			0x59
+#define	CC2520_CMD_CTR_UCTR		0x60
+#define	CC2520_CMD_CBCMAC		0x64
+#define	CC2520_CMD_UCBCMAC		0x66
+#define	CC2520_CMD_CCM			0x68
+#define	CC2520_CMD_UCCM			0x6A
+#define	CC2520_CMD_ECB			0x70
+#define	CC2520_CMD_ECBO			0x72
+#define	CC2520_CMD_ECBX			0x74
+#define	CC2520_CMD_INC			0x78
+#define	CC2520_CMD_ABORT		0x7F
+#define	CC2520_CMD_REGISTER_READ	0x80
+#define	CC2520_CMD_REGISTER_WRITE	0xC0
+
+/* status registers */
+#define	CC2520_CHIPID			0x40
+#define	CC2520_VERSION			0x42
+#define	CC2520_EXTCLOCK			0x44
+#define	CC2520_MDMCTRL0			0x46
+#define	CC2520_MDMCTRL1			0x47
+#define	CC2520_FREQEST			0x48
+#define	CC2520_RXCTRL			0x4A
+#define	CC2520_FSCTRL			0x4C
+#define	CC2520_FSCAL0			0x4E
+#define	CC2520_FSCAL1			0x4F
+#define	CC2520_FSCAL2			0x50
+#define	CC2520_FSCAL3			0x51
+#define	CC2520_AGCCTRL0			0x52
+#define	CC2520_AGCCTRL1			0x53
+#define	CC2520_AGCCTRL2			0x54
+#define	CC2520_AGCCTRL3			0x55
+#define	CC2520_ADCTEST0			0x56
+#define	CC2520_ADCTEST1			0x57
+#define	CC2520_ADCTEST2			0x58
+#define	CC2520_MDMTEST0			0x5A
+#define	CC2520_MDMTEST1			0x5B
+#define CC2520_DACTEST0			0x5C
+#define CC2520_DACTEST1			0x5D
+#define CC2520_ATEST			0x5E
+#define CC2520_DACTEST2			0x5F
+#define CC2520_PTEST0			0x60
+#define CC2520_PTEST1			0x61
+#define CC2520_RESERVED			0x62
+#define CC2520_DPUBIST			0x7A
+#define CC2520_ACTBIST			0x7C
+#define CC2520_RAMBIST			0x7E
+
+/*frame registers*/
+#define CC2520_FRMFILT0			0x00
+#define CC2520_FRMFILT1			0x01
+#define CC2520_SRCMATCH			0x02
+#define CC2520_SRCSHORTEN0		0x04
+#define CC2520_SRCSHORTEN1		0x05
+#define CC2520_SRCSHORTEN2		0x06
+#define CC2520_SRCEXTEN0		0x08
+#define CC2520_SRCEXTEN1		0x09
+#define CC2520_SRCEXTEN2		0x0A
+#define CC2520_FRMCTRL0			0x0C
+#define CC2520_FRMCTRL1			0x0D
+#define CC2520_RXENABLE0		0x0E
+#define CC2520_RXENABLE1		0x0F
+#define CC2520_EXCFLAG0			0x10
+#define CC2520_EXCFLAG1			0x11
+#define CC2520_EXCFLAG2			0x12
+#define CC2520_EXCMASKA0		0x14
+#define CC2520_EXCMASKA1		0x15
+#define CC2520_EXCMASKA2		0x16
+#define CC2520_EXCMASKB0		0x18
+#define CC2520_EXCMASKB1		0x19
+#define CC2520_EXCMASKB2		0x1A
+#define CC2520_EXCBINDX0		0x1C
+#define CC2520_EXCBINDX1		0x1D
+#define CC2520_EXCBINDY0		0x1E
+#define CC2520_EXCBINDY1		0x1F
+#define CC2520_GPIOCTRL0		0x20
+#define CC2520_GPIOCTRL1		0x21
+#define CC2520_GPIOCTRL2		0x22
+#define CC2520_GPIOCTRL3		0x23
+#define CC2520_GPIOCTRL4		0x24
+#define CC2520_GPIOCTRL5		0x25
+#define CC2520_GPIOPOLARITY		0x26
+#define CC2520_GPIOCTRL			0x28
+#define CC2520_DPUCON			0x2A
+#define CC2520_DPUSTAT			0x2C
+#define CC2520_FREQCTRL			0x2E
+#define CC2520_FREQTUNE			0x2F
+#define CC2520_TXPOWER			0x30
+#define CC2520_TXCTRL			0x31
+#define CC2520_FSMSTAT0			0x32
+#define CC2520_FSMSTAT1			0x33
+#define CC2520_FIFOPCTRL		0x34
+#define CC2520_FSMCTRL			0x35
+#define CC2520_CCACTRL0			0x36
+#define CC2520_CCACTRL1			0x37
+#define CC2520_RSSI			0x38
+#define CC2520_RSSISTAT			0x39
+#define CC2520_RXFIRST			0x3C
+#define CC2520_RXFIFOCNT		0x3E
+#define CC2520_TXFIFOCNT		0x3F
+
+#endif
-- 
1.7.9.5


------------------------------------------------------------------------------
HPCC Systems Open Source Big Data Platform from LexisNexis Risk Solutions
Find What Matters Most in Your Big Data with HPCC Systems
Open Source. Fast. Scalable. Simple. Ideal for Dirty Data.
Leverages Graph Analysis for Fast Processing & Easy Data Exploration
http://p.sf.net/sfu/hpccsystems

Re: [Linux-zigbee-devel] [PATCH net-next 1/3] ieee802154: cc2520: driver for TI cc2520 radio

[Alexander Aring]

Hi Varka,

On Mon, Jun 16, 2014 at 10:21:56AM +0530, Varka Bhadram wrote:
> 

Maybe some more information about this chip in the commit msg?

> Signed-off-by: Varka Bhadram <varkab@cdac.in>
> ---
>  drivers/net/ieee802154/cc2520.c |  805 +++++++++++++++++++++++++++++++++++++++
>  include/linux/spi/cc2520.h      |  176 +++++++++
>  2 files changed, 981 insertions(+)
>  create mode 100644 drivers/net/ieee802154/cc2520.c
>  create mode 100644 include/linux/spi/cc2520.h
> 
> diff --git a/drivers/net/ieee802154/cc2520.c b/drivers/net/ieee802154/cc2520.c
> new file mode 100644
> index 0000000..e8b5993
> --- /dev/null
> +++ b/drivers/net/ieee802154/cc2520.c
> @@ -0,0 +1,805 @@
> +/* Driver for TI CC2520 802.15.4 Wireless-PAN Networking controller
> + *
> + * Copyright (C) 2014 Varka Bhadram <varkab@cdac.in>
> + *		      Md.Jamal Mohiuddin <mjmohiuddin@cdac.in>
> + *		      P Sowjanya <sowjanyap@cdac.in>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/gpio.h>
> +#include <linux/delay.h>
> +#include <linux/spi/spi.h>
> +#include <linux/spi/cc2520.h>
> +#include <linux/workqueue.h>
> +#include <linux/interrupt.h>
> +
> +#include <linux/skbuff.h>
> +
> +#include <linux/pinctrl/consumer.h>
> +
> +#include <linux/of_device.h>
> +#include <linux/of_platform.h>
> +#include <linux/of_gpio.h>
> +
> +#include <net/mac802154.h>
> +#include <net/wpan-phy.h>
> +
> +#define	SPI_COMMAND_BUFFER	3
> +#define	HIGH			1
> +#define	LOW			0
> +#define	STATE_IDLE		0
> +
> +/*Driver private information */
> +struct cc2520_private {
> +	struct spi_device *spi;
> +
> +	struct ieee802154_dev *dev;
> +
> +	u8 *buf;
> +	struct mutex buffer_mutex;
> +
> +	unsigned is_tx:1;
> +	int fifo_irq;
> +
> +	int fifo_pin;
> +	int fifop_pin;
> +
> +	struct work_struct fifop_irqwork;
> +
> +	spinlock_t lock;
> +
> +	struct completion tx_complete;
> +};
> +
> +/*Generic Functions*/
> +static int cc2520_cmd_strobe(struct cc2520_private *priv,
> +						u8 cmd)
> +{
> +	int ret;
> +	u8 status = 0xff;
> +	struct spi_message msg;
> +	struct spi_transfer xfer = {
> +		.len = 0,
> +		.tx_buf = priv->buf,
> +		.rx_buf = priv->buf,
> +	};
> +
> +	spi_message_init(&msg);
> +	spi_message_add_tail(&xfer, &msg);

I see, you maybe looked at others drivers like at86rf230 and see what
they do there. I really don't like the lowlevel spi calls in the others
drivers. There are spi helper functions like 'spi_write_then_read' look
in drivers/spi/spi.c for the helper functions. Then you don't need the
buffer_mutex also.

> +
> +	mutex_lock(&priv->buffer_mutex);
> +	priv->buf[xfer.len++] = cmd;
> +	dev_vdbg(&priv->spi->dev,
> +			"command strobe buf[0] = %02x\n",
> +			priv->buf[0]);
> +
> +	ret = spi_sync(priv->spi, &msg);
> +	if (!ret)
> +		status = priv->buf[0];
> +	dev_vdbg(&priv->spi->dev,
> +			"buf[0] = %02x\n", priv->buf[0]);
> +	mutex_unlock(&priv->buffer_mutex);
> +
> +	return ret;
> +}
> +

....

> +
> +static void cc2520_unregister(struct cc2520_private *priv)
> +{
> +	ieee802154_unregister_device(priv->dev);
> +	ieee802154_free_device(priv->dev);
> +}

Only used in remove callback of module. It's small enough to do this in
the remove callback.

> +
> +static void cc2520_fifop_irqwork(struct work_struct *work)
> +{
> +	struct cc2520_private *priv
> +		= container_of(work, struct cc2520_private, fifop_irqwork);
> +
> +	dev_dbg(&priv->spi->dev, "fifop interrupt received\n");
> +
> +	if (gpio_get_value(priv->fifo_pin))
> +		cc2520_rx(priv);
> +	else
> +		dev_err(&priv->spi->dev, "rxfifo overflow\n");
> +
> +	cc2520_cmd_strobe(priv, CC2520_CMD_SFLUSHRX);
> +	cc2520_cmd_strobe(priv, CC2520_CMD_SFLUSHRX);
> +}
> +
> +static irqreturn_t cc2520_fifop_isr(int irq, void *data)
> +{
> +	struct cc2520_private *priv = data;
> +
> +	schedule_work(&priv->fifop_irqwork);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t cc2520_sfd_isr(int irq, void *data)
> +{
> +	struct cc2520_private *priv = data;
> +
> +	spin_lock(&priv->lock);

You need to use here spin_lock_irqsave and spin_unlock_irqrestore here.
Please verify you locking with PROVE_LOCKING enabled in your kernel.

In your xmit callback you use a irqsave spinlocks there. You need always
save to the "strongest" context which is a interrupt in your case.

> +	if (priv->is_tx) {
> +		dev_dbg(&priv->spi->dev, "SFD for TX\n");
> +		priv->is_tx = 0;
> +		complete(&priv->tx_complete);
> +	} else
> +		dev_dbg(&priv->spi->dev, "SFD for RX\n");

make brackets in the else branch if you have brackets in the "if" branch.

You don't need to lock all the things here I think:

--snip
	spin_lock_irqsave(&priv->lock, flags);
	if (priv->is_tx) {
		priv->is_tx = 0;
		spin_unlock_irqrestore(&priv->lock, flags);
		dev_dbg(&priv->spi->dev, "SFD for TX\n");
		complete(&priv->tx_complete);
	} else {
		spin_unlock_irqrestore(&priv->lock, flags);
		dev_dbg(&priv->spi->dev, "SFD for RX\n");
	}
--snap

> +	spin_unlock(&priv->lock);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int cc2520_hw_init(struct cc2520_private *priv)
> +{
> +	u8 status = 0, state = 0xff;
> +	int ret;
> +	int timeout = 100;
> +
> +	ret = cc2520_read_register(priv, CC2520_FSMSTAT1, &state);
> +	if (ret)
> +		goto err_ret;
> +
> +	if (state != STATE_IDLE) {
> +		ret = -EINVAL;
> +		return ret;

return -EINVAL? saves the brackets ;)

> +	}
> +
> +	do {
> +		ret = cc2520_get_status(priv, &status);
> +		if (ret)
> +			goto err_ret;
> +
> +		if (timeout-- <= 0) {
> +			dev_err(&priv->spi->dev, "oscillator start failed!\n");
> +			return ret;
> +		}
> +		udelay(1);
> +	} while (!(status & CC2520_STATUS_XOSC32M_STABLE));
> +
> +	dev_vdbg(&priv->spi->dev, "oscillator successfully brought up\n");
> +
> +	/*Registers default value: section 28.1 in Datasheet*/
> +	ret = cc2520_write_register(priv, CC2520_TXPOWER, 0xF7);
> +	if (ret)
> +		goto err_ret;
> +
> +	ret = cc2520_write_register(priv, CC2520_CCACTRL0, 0x1A);
> +	if (ret)
> +		goto err_ret;
> +
> +	ret = cc2520_write_register(priv, CC2520_MDMCTRL0, 0x85);
> +	if (ret)
> +		goto err_ret;
> +
> +	ret = cc2520_write_register(priv, CC2520_MDMCTRL1, 0x14);
> +	if (ret)
> +		goto err_ret;
> +
> +	ret = cc2520_write_register(priv, CC2520_RXCTRL, 0x3f);
> +	if (ret)
> +		goto err_ret;
> +
> +	ret = cc2520_write_register(priv, CC2520_FSCTRL, 0x5a);
> +	if (ret)
> +		goto err_ret;
> +
> +	ret = cc2520_write_register(priv, CC2520_FSCAL1, 0x2b);
> +	if (ret)
> +		goto err_ret;
> +
> +	ret = cc2520_write_register(priv, CC2520_AGCCTRL1, 0x11);
> +	if (ret)
> +		goto err_ret;
> +
> +	ret = cc2520_write_register(priv, CC2520_ADCTEST0, 0x10);
> +	if (ret)
> +		goto err_ret;
> +
> +	ret = cc2520_write_register(priv, CC2520_ADCTEST1, 0x0e);
> +	if (ret)
> +		goto err_ret;
> +
> +	ret = cc2520_write_register(priv, CC2520_ADCTEST2, 0x03);
> +	if (ret)
> +		goto err_ret;
> +
> +	ret = cc2520_write_register(priv, CC2520_FRMCTRL0, 0x60);
> +	if (ret)
> +		goto err_ret;
> +
> +	ret = cc2520_write_register(priv, CC2520_FRMCTRL1, 0x03);
> +	if (ret)
> +		goto err_ret;
> +
> +	ret = cc2520_write_register(priv, CC2520_FRMFILT0, 0x00);
> +	if (ret)
> +		goto err_ret;
> +
> +	ret = cc2520_write_register(priv, CC2520_FIFOPCTRL, 127);
> +	if (ret)
> +		goto err_ret;
> +
> +	return 0;
> +
> +err_ret:
> +	return ret;
> +}
> +
> +static struct cc2520_platform_data *
> +cc2520_get_platform_data(struct spi_device *spi)
> +{
> +	struct cc2520_platform_data *pdata;
> +	struct device_node __maybe_unused *np = spi->dev.of_node;
> +	struct cc2520_private *priv = spi_get_drvdata(spi);
> +
> +	if (!IS_ENABLED(CONFIG_OF) || !np)
> +		return spi->dev.platform_data;
> +
> +	pdata = devm_kzalloc(&spi->dev, sizeof(*pdata), GFP_KERNEL);
> +	if (!pdata)
> +		goto done;
> +
> +	pdata->fifo = of_get_named_gpio(np, "fifo-gpio", 0);
> +	priv->fifo_pin = pdata->fifo;
> +
> +	pdata->fifop = of_get_named_gpio(np, "fifop-gpio", 0);
> +	priv->fifop_pin = pdata->fifop;
> +
> +	pdata->sfd = of_get_named_gpio(np, "sfd-gpio", 0);
> +	pdata->cca = of_get_named_gpio(np, "cca-gpio", 0);
> +	pdata->vreg = of_get_named_gpio(np, "vreg-gpio", 0);
> +	pdata->reset = of_get_named_gpio(np, "reset-gpio", 0);
> +
> +	spi->dev.platform_data = pdata;
> +
> +done:
> +	return pdata;
> +}
> +
> +/*Driver probe function*/
> +static int cc2520_probe(struct spi_device *spi)
> +{
> +	struct cc2520_private *priv;
> +	struct pinctrl *pinctrl;
> +	struct cc2520_platform_data *pdata;
> +	struct device_node __maybe_unused *np = spi->dev.of_node;
> +	int ret;
> +
> +	priv = kzalloc(sizeof(struct cc2520_private), GFP_KERNEL);
> +	if (!priv) {
> +		ret = -ENOMEM;
> +		goto err_free_local;
> +	}

why not devm_ calls?

> +
> +	spi_set_drvdata(spi, priv);
> +
> +	pinctrl = devm_pinctrl_get_select_default(&spi->dev);
> +	if (IS_ERR(pinctrl))
> +		dev_warn(&spi->dev,
> +			"pinctrl pins are not configured from the driver");
> +
> +	pdata = cc2520_get_platform_data(spi);
> +	if (!pdata) {
> +		dev_err(&spi->dev, "no platform data\n");
> +		return -EINVAL;
memory leak of priv here.

> +	}
> +
> +	mutex_init(&priv->buffer_mutex);
> +	INIT_WORK(&priv->fifop_irqwork, cc2520_fifop_irqwork);

I really don't like also the workqueue here. The at86rf230 use also a
workqueue but the mrf24j40 uses 'devm_request_threaded_irq'. A threaded
irq can also handle sync spi calls.

> +	spin_lock_init(&priv->lock);
> +	init_completion(&priv->tx_complete);
> +
> +	priv->spi = spi;
> +
> +	priv->buf = kzalloc(SPI_COMMAND_BUFFER, GFP_KERNEL);
> +	if (!priv->buf) {
> +		ret = -ENOMEM;
> +		goto err_free_buf;
> +	}
> +
> +	/*Request all the gpio's*/
> +	if (gpio_is_valid(pdata->fifo)) {
> +		ret = devm_gpio_request_one(&spi->dev, pdata->fifo, 
> +					GPIOF_IN, "fifo");
> +		if (ret)
> +			goto err_hw_init;
> +	}
> +
> +	if (gpio_is_valid(pdata->cca)) {
> +		ret = devm_gpio_request_one(&spi->dev, pdata->cca,
> +					GPIOF_IN, "cca");
> +		if (ret)
> +			goto err_hw_init;
> +	}
> +
> +	if (gpio_is_valid(pdata->fifop)) {
> +		ret = devm_gpio_request_one(&spi->dev, pdata->fifop,
> +					GPIOF_IN, "fifop");
> +		if (ret)
> +			goto err_hw_init;
> +	}
> +
> +	if (gpio_is_valid(pdata->sfd)) {
> +		ret = devm_gpio_request_one(&spi->dev, pdata->sfd,
> +					GPIOF_IN, "sfd");
> +		if (ret)
> +			goto err_hw_init;
> +	}
> +
> +	if (gpio_is_valid(pdata->reset)) {
> +		ret = devm_gpio_request_one(&spi->dev, pdata->reset,
> +					GPIOF_OUT_INIT_LOW, "reset");
> +		if (ret)
> +			goto err_hw_init;
> +	}
> +
> +	if (gpio_is_valid(pdata->vreg)) {
> +		ret = devm_gpio_request_one(&spi->dev, pdata->vreg,
> +					GPIOF_OUT_INIT_LOW, "vreg");
> +		if (ret)
> +			goto err_hw_init;
> +	}

You should check on the not optional pins if you can request them. You
use in the above code the pins vreg, reset, fifo_irq, sfd. And this
failed if the gpio's are not valid.

means:

if (!gpio_is_valid(pdata->vreg)) {
	dev_err(....)
	ret = -EINVAL;
	goto ...;
}

on all pins which are needed to use your chip.

> +
> +	gpio_set_value(pdata->vreg, HIGH);
> +	udelay(100);
> +
> +	gpio_set_value(pdata->reset, HIGH);
> +	udelay(200);
> +
> +	ret = cc2520_hw_init(priv);
> +	if (ret)
> +		goto err_hw_init;
> +
> +	/*Set up fifop interrupt */
> +	priv->fifo_irq = gpio_to_irq(pdata->fifop);

why is fifo_irq in your "private data"? This is only used in this function.

> +	ret = devm_request_irq(&spi->dev,
> +				priv->fifo_irq,
> +				cc2520_fifop_isr,
> +				IRQF_TRIGGER_RISING,
> +				dev_name(&spi->dev),
> +				priv);
> +	if (ret) {
> +		dev_err(&spi->dev, "could not get fifop irq\n");
> +		goto err_hw_init;
> +	}
> +
> +	/*Set up sfd interrupt*/
> +	ret = devm_request_irq(&spi->dev,
> +				gpio_to_irq(pdata->sfd),
> +				cc2520_sfd_isr,
> +				IRQF_TRIGGER_FALLING,
> +				dev_name(&spi->dev),
> +				priv);
> +	if (ret) {
> +		dev_err(&spi->dev, "could not get sfd irq\n");
> +		goto err_hw_init;
> +	}
> +
> +	ret = cc2520_register(priv);
> +	if (ret)
> +		goto err_hw_init;
> +
> +	return 0;
> +
> +err_hw_init:
> +	mutex_destroy(&priv->buffer_mutex);
> +	flush_work(&priv->fifop_irqwork);
> +
> +err_free_buf:
> +	kfree(priv->buf);
> +
> +err_free_local:
> +	kfree(priv);
> +
> +	return ret;
> +}
> +
> +/*Driver remove function*/
> +static int cc2520_remove(struct spi_device *spi)
> +{
> +	struct cc2520_private *priv = spi_get_drvdata(spi);
> +
> +	cc2520_unregister(priv);
> +
> +	mutex_destroy(&priv->buffer_mutex);
> +	flush_work(&priv->fifop_irqwork);
> +
> +	kfree(priv->buf);
> +	kfree(priv);
> +
> +	return 0;
> +}
> +
> +static const struct spi_device_id cc2520_ids[] = {
> +	{"cc2520", 0},
You don't need to set the driver_data to 0. Simple:
	{ "cc2520", },

> +	{},
> +};
> +MODULE_DEVICE_TABLE(spi, cc2520_ids);
> +
> +static const struct of_device_id cc2520_of_ids[] = {
> +	{.compatible = "ti,cc2520", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, cc2520_of_ids);
> +
> +/*SPI Driver Structure*/
> +static struct spi_driver cc2520_driver = {
> +	.driver = {
> +		.name = "cc2520",
> +		.bus = &spi_bus_type,
> +		.owner = THIS_MODULE,
> +		.of_match_table = of_match_ptr(cc2520_of_ids),
> +	},
> +	.id_table = cc2520_ids,
> +	.probe = cc2520_probe,
> +	.remove = cc2520_remove,
> +};
> +module_spi_driver(cc2520_driver);
> +
> +MODULE_DESCRIPTION("CC2520 Transceiver Driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/spi/cc2520.h b/include/linux/spi/cc2520.h
> new file mode 100644
> index 0000000..68a2c88
> --- /dev/null
> +++ b/include/linux/spi/cc2520.h

In this location of header files are platform_data structs only. I think
you should leave the cc2520_platform_data in this file and move the rest
of declaration to you cc2520.c file.

> @@ -0,0 +1,176 @@
> +#ifndef __CC2520_H
> +#define __CC2520_H
> +
> +#define RSSI_VALID		0
> +#define RSSI_OFFSET		78
> +#define CC2520_FREG_MASK	0x3F
> +
> +struct cc2520_platform_data {
> +	int fifo;
> +	int fifop;
> +	int cca;
> +	int sfd;
> +	int reset;
> +	int vreg;
> +};
> +

- Alex

Re: [Linux-zigbee-devel] [PATCH net-next 1/3] ieee802154: cc2520: driver for TI cc2520 radio

[Varka Bhadram]

Hi Alex,

Thanks for the comments.

I will create a patch for the required changes with Reported-by 
Alexander Aring  <...> reply if this is okay for you.


On 06/16/2014 01:08 PM, Alexander Aring wrote:
> Hi Varka,
>
> On Mon, Jun 16, 2014 at 10:21:56AM +0530, Varka Bhadram wrote:
> Maybe some more information about this chip in the commit msg?
I will provide the proper commit messages in next version
>
> +/*Generic Functions*/
> +static int cc2520_cmd_strobe(struct cc2520_private *priv,
> +						u8 cmd)
> +{
> +	int ret;
> +	u8 status = 0xff;
> +	struct spi_message msg;
> +	struct spi_transfer xfer = {
> +		.len = 0,
> +		.tx_buf = priv->buf,
> +		.rx_buf = priv->buf,
> +	};
> +
> +	spi_message_init(&msg);
> +	spi_message_add_tail(&xfer, &msg);
> I see, you maybe looked at others drivers like at86rf230 and see what
> they do there. I really don't like the lowlevel spi calls in the others
> drivers. There are spi helper functions like 'spi_write_then_read' look
> in drivers/spi/spi.c for the helper functions. Then you don't need the
> buffer_mutex also.
In at86rf230 driver first it hold the mutex lock for buffer then 
__at86rf230_read_subreg()/ __at86rf230_write()
functions called. One more thing is that at the low level spi core is 
holding the lock for the buffer data.
We can remove the buffer mutex.
>> +
>> +	mutex_lock(&priv->buffer_mutex);
>> +	priv->buf[xfer.len++] = cmd;
>> +	dev_vdbg(&priv->spi->dev,
>> +			"command strobe buf[0] = %02x\n",
>> +			priv->buf[0]);
>> +
>> +	ret = spi_sync(priv->spi, &msg);
>> +	if (!ret)
>> +		status = priv->buf[0];
>> +	dev_vdbg(&priv->spi->dev,
>> +			"buf[0] = %02x\n", priv->buf[0]);
>> +	mutex_unlock(&priv->buffer_mutex);
>> +
>> +	return ret;
>> +}
>> +
> ....
>
>> +
>> +static void cc2520_unregister(struct cc2520_private *priv)
>> +{
>> +	ieee802154_unregister_device(priv->dev);
>> +	ieee802154_free_device(priv->dev);
>> +}
> Only used in remove callback of module. It's small enough to do this in
> the remove callback.
>
Ya its nice . We can save the cpu context switching time also....
>> +
>> +static void cc2520_fifop_irqwork(struct work_struct *work)
>> +{
>> +	struct cc2520_private *priv
>> +		= container_of(work, struct cc2520_private, fifop_irqwork);
>> +
>> +	dev_dbg(&priv->spi->dev, "fifop interrupt received\n");
>> +
>> +	if (gpio_get_value(priv->fifo_pin))
>> +		cc2520_rx(priv);
>> +	else
>> +		dev_err(&priv->spi->dev, "rxfifo overflow\n");
>> +
>> +	cc2520_cmd_strobe(priv, CC2520_CMD_SFLUSHRX);
>> +	cc2520_cmd_strobe(priv, CC2520_CMD_SFLUSHRX);
>> +}
>> +
>> +static irqreturn_t cc2520_fifop_isr(int irq, void *data)
>> +{
>> +	struct cc2520_private *priv = data;
>> +
>> +	schedule_work(&priv->fifop_irqwork);
>> +
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static irqreturn_t cc2520_sfd_isr(int irq, void *data)
>> +{
>> +	struct cc2520_private *priv = data;
>> +
>> +	spin_lock(&priv->lock);
> You need to use here spin_lock_irqsave and spin_unlock_irqrestore here.
> Please verify you locking with PROVE_LOCKING enabled in your kernel.
>
> In your xmit callback you use a irqsave spinlocks there. You need always
> save to the "strongest" context which is a interrupt in your case.
This type of mechanism is needed when you want to remember the interrupt 
status for the
IRQ/system.
>> +	if (priv->is_tx) {
>> +		dev_dbg(&priv->spi->dev, "SFD for TX\n");
>> +		priv->is_tx = 0;
>> +		complete(&priv->tx_complete);
>> +	} else
>> +		dev_dbg(&priv->spi->dev, "SFD for RX\n");
> make brackets in the else branch if you have brackets in the "if" branch.
>
> You don't need to lock all the things here I think:
>
> --snip
> 	spin_lock_irqsave(&priv->lock, flags);
> 	if (priv->is_tx) {
> 		priv->is_tx = 0;
> 		spin_unlock_irqrestore(&priv->lock, flags);
> 		dev_dbg(&priv->spi->dev, "SFD for TX\n");
> 		complete(&priv->tx_complete);
> 	} else {
> 		spin_unlock_irqrestore(&priv->lock, flags);
> 		dev_dbg(&priv->spi->dev, "SFD for RX\n");
> 	}
> --snap
>
Ya this can be the good approach...
>> +	spin_unlock(&priv->lock);
>> +
>> +	return IRQ_HANDLED
>> +/*Driver probe function*/
>> +static int cc2520_probe(struct spi_device *spi)
>> +{
>> +	struct cc2520_private *priv;
>> +	struct pinctrl *pinctrl;
>> +	struct cc2520_platform_data *pdata;
>> +	struct device_node __maybe_unused *np = spi->dev.of_node;
>> +	int ret;
>> +
>> +	priv = kzalloc(sizeof(struct cc2520_private), GFP_KERNEL);
>> +	if (!priv) {
>> +		ret = -ENOMEM;
>> +		goto err_free_local;
>> +	}
> why not devm_ calls?
I will surely change it to devm_....
>> +
>> +	spi_set_drvdata(spi, priv);
>> +
>> +	pinctrl = devm_pinctrl_get_select_default(&spi->dev);
>> +
>> +	if (gpio_is_valid(pdata->vreg)) {
>> +		ret = devm_gpio_request_one(&spi->dev, pdata->vreg,
>> +					GPIOF_OUT_INIT_LOW, "vreg");
>> +		if (ret)
>> +			goto err_hw_init;
>> +	}
> You should check on the not optional pins if you can request them. You
> use in the above code the pins vreg, reset, fifo_irq, sfd. And this
> failed if the gpio's are not valid.
>
> means:
>
> if (!gpio_is_valid(pdata->vreg)) {
> 	dev_err(....)
> 	ret = -EINVAL;
> 	goto ...;
> }
>
> on all pins which are needed to use your chip.
>
>> +
>> +	gpio_set_value(pdata->vreg, HIGH);
>> +	udelay(100);
>> +
>> +	gpio_set_value(pdata->reset, HIGH);
>> +	udelay(200);
>> +
>> +	ret = cc2520_hw_init(priv);
>> +	if (ret)
>> +		goto err_hw_init;
>> +
>> +	/*Set up fifop interrupt */
>> +	priv->fifo_irq = gpio_to_irq(pdata->fifop);
> why is fifo_irq in your "private data"? This is only used in this function.

Ya this is my mistake. Previously i used in other place also.

>> +	ret = devm_request_irq(&spi->dev,
>> +				priv->fifo_irq,
>> +				cc2520_fifop_isr,
>> +				IRQF_TRIGGER_RISING,
>> +				dev_name(&spi->dev),
>> +				priv);
>> +	if (ret) {
>> +		dev_err(&spi->dev, "could not get fifop irq\n");
>> +		goto err_hw_init;
>> +	}
>> +
>> +	/*Set up sfd interrupt*/
>> +	ret = devm_request_irq(&spi->dev,
>> +				gpio_to_irq(pdata->sfd),
>> +				cc2520_sfd_isr,
>> +				IRQF_TRIGGER_FALLING,
>> +				dev_name(&spi->dev),
>> +				priv);
>> +	if (ret) {
>> +		dev_err(&spi->dev, "could not get sfd irq\n");
>> +		goto err_hw_init;
>> +	}
>> +
>> +	ret = cc2520_register(priv);
>> +	if (ret)
>> +		goto err_hw_init;
>> +
>> +	return 0;
>> +
>> +err_hw_init:
>> +	mutex_destroy(&priv->buffer_mutex);
>> +	flush_work(&priv->fifop_irqwork);
>> +
>> +err_free_buf:
>> +	kfree(priv->buf);
>> +
>> +err_free_local:
>> +	kfree(priv);
>> +
>> +	return ret;
>> +}
>> +
>> +/*Driver remove function*/
>> +static int cc2520_remove(struct spi_device *spi)
>> +{
>> +	struct cc2520_private *priv = spi_get_drvdata(spi);
>> +
>> +	cc2520_unregister(priv);
>> +
>> +	mutex_destroy(&priv->buffer_mutex);
>> +	flush_work(&priv->fifop_irqwork);
>> +
>> +	kfree(priv->buf);
>> +	kfree(priv);
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct spi_device_id cc2520_ids[] = {
>> +	{"cc2520", 0},
> You don't need to set the driver_data to 0. Simple:
> 	{ "cc2520", },
this does not make any difference.
>> +	{},
>> +};
>> +MODULE_DEVICE_TABLE(spi, cc2520_ids);
>> +
>> +static const struct of_device_id cc2520_of_ids[] = {
>> +	{.compatible = "ti,cc2520", },
>> +	{},
>> +};
>> +MODULE_DEVICE_TABLE(of, cc2520_of_ids);
>> +
>> +/*SPI Driver Structure*/
>> +static struct spi_driver cc2520_driver = {
>> +	.driver = {
>> +		.name = "cc2520",
>> +		.bus = &spi_bus_type,
>> +		.owner = THIS_MODULE,
>> +		.of_match_table = of_match_ptr(cc2520_of_ids),
>> +	},
>> +	.id_table = cc2520_ids,
>> +	.probe = cc2520_probe,
>> +	.remove = cc2520_remove,
>> +};
>> +module_spi_driver(cc2520_driver);
>> +
>> +MODULE_DESCRIPTION("CC2520 Transceiver Driver");
>> +MODULE_LICENSE("GPL v2");
>> diff --git a/include/linux/spi/cc2520.h b/include/linux/spi/cc2520.h
>> new file mode 100644
>> index 0000000..68a2c88
>> --- /dev/null
>> +++ b/include/linux/spi/cc2520.h
> In this location of header files are platform_data structs only. I think
> you should leave the cc2520_platform_data in this file and move the rest
> of declaration to you cc2520.c file.
>
I will move the code
>> @@ -0,0 +1,176 @@
>> +#ifndef __CC2520_H
>> +#define __CC2520_H
>> +
>> +#define RSSI_VALID		0
>> +#define RSSI_OFFSET		78
>> +#define CC2520_FREG_MASK	0x3F
>> +
>> +struct cc2520_platform_data {
>> +	int fifo;
>> +	int fifop;
>> +	int cca;
>> +	int sfd;
>> +	int reset;
>> +	int vreg;
>> +};
>> +
> - Alex


-Varka Bhadram

-------------------------------------------------------------------------------------------------------------------------------
[ C-DAC is on Social-Media too. Kindly follow us at:
Facebook: https://www.facebook.com/CDACINDIA & Twitter: @cdacindia ]

This e-mail is for the sole use of the intended recipient(s) and may
contain confidential and privileged information. If you are not the
intended recipient, please contact the sender by reply e-mail and destroy
all copies and the original message. Any unauthorized review, use,
disclosure, dissemination, forwarding, printing or copying of this email
is strictly prohibited and appropriate legal action will be taken.
-------------------------------------------------------------------------------------------------------------------------------

Re: [Linux-zigbee-devel] [PATCH net-next 1/3] ieee802154: cc2520: driver for TI cc2520 radio

[Alexander Aring]

Hi,

On Mon, Jun 16, 2014 at 02:21:19PM +0530, Varka Bhadram wrote:
...
> >>+
> >>+static void cc2520_unregister(struct cc2520_private *priv)
> >>+{
> >>+	ieee802154_unregister_device(priv->dev);
> >>+	ieee802154_free_device(priv->dev);
> >>+}
> >Only used in remove callback of module. It's small enough to do this in
> >the remove callback.
> >

There is no context switch here. It's a little bit faster because you
don't put some things on the stack. Alternative would be to add a inline
keyword to this function.

> Ya its nice . We can save the cpu context switching time also....
> >>+
> >>+static void cc2520_fifop_irqwork(struct work_struct *work)
> >>+{
> >>+	struct cc2520_private *priv
> >>+		= container_of(work, struct cc2520_private, fifop_irqwork);
> >>+
> >>+	dev_dbg(&priv->spi->dev, "fifop interrupt received\n");
> >>+
> >>+	if (gpio_get_value(priv->fifo_pin))
> >>+		cc2520_rx(priv);
> >>+	else
> >>+		dev_err(&priv->spi->dev, "rxfifo overflow\n");
> >>+
> >>+	cc2520_cmd_strobe(priv, CC2520_CMD_SFLUSHRX);
> >>+	cc2520_cmd_strobe(priv, CC2520_CMD_SFLUSHRX);
> >>+}
> >>+
> >>+static irqreturn_t cc2520_fifop_isr(int irq, void *data)
> >>+{
> >>+	struct cc2520_private *priv = data;
> >>+
> >>+	schedule_work(&priv->fifop_irqwork);
> >>+
> >>+	return IRQ_HANDLED;
> >>+}
> >>+
> >>+static irqreturn_t cc2520_sfd_isr(int irq, void *data)
> >>+{
> >>+	struct cc2520_private *priv = data;
> >>+
> >>+	spin_lock(&priv->lock);
> >You need to use here spin_lock_irqsave and spin_unlock_irqrestore here.
> >Please verify you locking with PROVE_LOCKING enabled in your kernel.
> >
> >In your xmit callback you use a irqsave spinlocks there. You need always
> >save to the "strongest" context which is a interrupt in your case.
> This type of mechanism is needed when you want to remember the interrupt
> status for the
> IRQ/system.

Yes you need I spinlock here, but spin_lock isn't irqsave but you need a
spin_lock function which is irqsave. This is
spin_lock_irqsave/spin_unlock_irqrestore.

You use these function in xmit callback for locking and that makes no
sense. You need to use spin_lock_irqsave/spin_unlock_irqrestore there of
course. But using in one function
spin_lock_irqsave/spin_unlock_irqrestore and in the other function
spin_lock/spin_unlock for the same spinlock is wrong.
In your case the strongest context is an irq, so you need for your
locking irqsave functions.

Please compile your kernel with PROVE_LOCKING and test your driver, then
you will see some warnings about deadlocks.

> >>+	if (priv->is_tx) {
> >>+		dev_dbg(&priv->spi->dev, "SFD for TX\n");
> >>+		priv->is_tx = 0;
> >>+		complete(&priv->tx_complete);
> >>+	} else
> >>+		dev_dbg(&priv->spi->dev, "SFD for RX\n");
> >make brackets in the else branch if you have brackets in the "if" branch.
> >
> >You don't need to lock all the things here I think:
> >
> >--snip
> >	spin_lock_irqsave(&priv->lock, flags);
> >	if (priv->is_tx) {
> >		priv->is_tx = 0;
> >		spin_unlock_irqrestore(&priv->lock, flags);
> >		dev_dbg(&priv->spi->dev, "SFD for TX\n");
> >		complete(&priv->tx_complete);
> >	} else {
> >		spin_unlock_irqrestore(&priv->lock, flags);
> >		dev_dbg(&priv->spi->dev, "SFD for RX\n");
> >	}
> >--snap
> >
> Ya this can be the good approach...
> >>+	spin_unlock(&priv->lock);
> >>+
> >>+	return IRQ_HANDLED
> >>+/*Driver probe function*/
> >>+static int cc2520_probe(struct spi_device *spi)
> >>+{
> >>+	struct cc2520_private *priv;
> >>+	struct pinctrl *pinctrl;
> >>+	struct cc2520_platform_data *pdata;
> >>+	struct device_node __maybe_unused *np = spi->dev.of_node;
> >>+	int ret;
> >>+
> >>+	priv = kzalloc(sizeof(struct cc2520_private), GFP_KERNEL);
> >>+	if (!priv) {
> >>+		ret = -ENOMEM;
> >>+		goto err_free_local;
> >>+	}
> >why not devm_ calls?
> I will surely change it to devm_....
> >>+
> >>+	spi_set_drvdata(spi, priv);
> >>+
> >>+	pinctrl = devm_pinctrl_get_select_default(&spi->dev);
> >>+
> >>+	if (gpio_is_valid(pdata->vreg)) {
> >>+		ret = devm_gpio_request_one(&spi->dev, pdata->vreg,
> >>+					GPIOF_OUT_INIT_LOW, "vreg");
> >>+		if (ret)
> >>+			goto err_hw_init;
> >>+	}
> >You should check on the not optional pins if you can request them. You
> >use in the above code the pins vreg, reset, fifo_irq, sfd. And this

s/above/below

> >failed if the gpio's are not valid.
> >
> >means:
> >
> >if (!gpio_is_valid(pdata->vreg)) {
> >	dev_err(....)
> >	ret = -EINVAL;
> >	goto ...;
> >}
> >
> >on all pins which are needed to use your chip.
> >
> >>+
> >>+	gpio_set_value(pdata->vreg, HIGH);
> >>+	udelay(100);
> >>+
> >>+	gpio_set_value(pdata->reset, HIGH);
> >>+	udelay(200);
> >>+
> >>+	ret = cc2520_hw_init(priv);
> >>+	if (ret)
> >>+		goto err_hw_init;
> >>+
> >>+	/*Set up fifop interrupt */
> >>+	priv->fifo_irq = gpio_to_irq(pdata->fifop);
...
> >>+static const struct spi_device_id cc2520_ids[] = {
> >>+	{"cc2520", 0},
> >You don't need to set the driver_data to 0. Simple:
> >	{ "cc2520", },
> this does not make any difference.

Yes, but saves code. :-)

Usual prefer coding is:

"(no difference + more code) < (no difference + less code)"

- Alex

[PATCH net-next 2/3] ieee802154: cc2520: add driver to kernel build system

[Varka Bhadram]

Signed-off-by: Varka Bhadram <varkab-If5XQcfNmg0@public.gmane.org>
---
 drivers/net/ieee802154/Kconfig  |   11 +++++++++++
 drivers/net/ieee802154/Makefile |    1 +
 2 files changed, 12 insertions(+)

diff --git a/drivers/net/ieee802154/Kconfig b/drivers/net/ieee802154/Kconfig
index 3e89bea..680422f 100644
--- a/drivers/net/ieee802154/Kconfig
+++ b/drivers/net/ieee802154/Kconfig
@@ -51,3 +51,14 @@ config IEEE802154_MRF24J40
 
 	  This driver can also be built as a module. To do so, say M here.
 	  the module will be called 'mrf24j40'.
+
+config IEEE802154_CC2520
+	depends on IEEE802154_DRIVERS && MAC802154
+	tristate "cc2520 transceiver driver"
+	depends on SPI
+	---help---
+	  Say Y here to enable the CC2520 SPI 802.15.4 wireless
+	  controller.
+
+	  This driver can also be built as a module. To do so, say M here.
+	  the module will be called 'cc2520'.
diff --git a/drivers/net/ieee802154/Makefile b/drivers/net/ieee802154/Makefile
index abb0c08..655cb95 100644
--- a/drivers/net/ieee802154/Makefile
+++ b/drivers/net/ieee802154/Makefile
@@ -2,3 +2,4 @@ obj-$(CONFIG_IEEE802154_FAKEHARD) += fakehard.o
 obj-$(CONFIG_IEEE802154_FAKELB) += fakelb.o
 obj-$(CONFIG_IEEE802154_AT86RF230) += at86rf230.o
 obj-$(CONFIG_IEEE802154_MRF24J40) += mrf24j40.o
+obj-$(CONFIG_IEEE802154_CC2520) += cc2520.o
-- 
1.7.9.5


------------------------------------------------------------------------------
HPCC Systems Open Source Big Data Platform from LexisNexis Risk Solutions
Find What Matters Most in Your Big Data with HPCC Systems
Open Source. Fast. Scalable. Simple. Ideal for Dirty Data.
Leverages Graph Analysis for Fast Processing & Easy Data Exploration
http://p.sf.net/sfu/hpccsystems

[PATCH net-next 3/3] devicetree: add devicetree bindings for cc2520 driver

[Varka Bhadram]

Signed-off-by: Varka Bhadram <varkab-If5XQcfNmg0@public.gmane.org>
---
 .../devicetree/bindings/net/ieee802154/cc2520.txt  |   26 ++++++++++++++++++++
 1 file changed, 26 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/ieee802154/cc2520.txt

diff --git a/Documentation/devicetree/bindings/net/ieee802154/cc2520.txt b/Documentation/devicetree/bindings/net/ieee802154/cc2520.txt
new file mode 100644
index 0000000..f0a749d
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/ieee802154/cc2520.txt
@@ -0,0 +1,26 @@
+CC2520 IEEE 802.15.4 Compatible Radio
+
+Required properties:
+	- compatible:		should be "ti,cc2520"
+	- spi-max-frequency:	maximal bus speed (8000000), should be set to 4000000 depends
+				sync or async operation mode
+	- reg:			the chipselect index
+	- mode:			SPI mode
+Example:
+
+	cc2520@0 {
+		compatible = "ti,cc2520";
+		reg = <0>;
+		spi-max-frequency = <4000000>;
+		mode = <1>;
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&cc2520_cape_pins>;	/*GPIO pin muxing details*/
+
+		fifo-gpio = <&gpio1 18 0>;
+		fifop-gpio = <&gpio1 19 0>;
+		sfd-gpio = <&gpio1 13 0>;
+		cca-gpio = <&gpio1 16 0>;
+		vreg-gpio = <&gpio0 31 0>;
+		reset-gpio = <&gpio1 12 0>;
+	};
-- 
1.7.9.5


------------------------------------------------------------------------------
HPCC Systems Open Source Big Data Platform from LexisNexis Risk Solutions
Find What Matters Most in Your Big Data with HPCC Systems
Open Source. Fast. Scalable. Simple. Ideal for Dirty Data.
Leverages Graph Analysis for Fast Processing & Easy Data Exploration
http://p.sf.net/sfu/hpccsystems