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

On Fri, Jun 20, 2014 at 03:58:00PM +0530, Varka Bhadram wrote:

Hi Alex,

On 06/20/2014 02:02 PM, Alexander Aring wrote:

quoted

Hi Varka,

sorry for my careful reviewing...,, but I had also some patch series
which was at v8 or something like that. :-)

mhhh.. More careful review produces the best code ...

quoted

On Fri, Jun 20, 2014 at 11:18:12AM +0530, Varka Bhadram wrote:

quoted

This patch adds the driver support for the cc2520 radio.

Driver support:
        - Tx and Rx of IEEE-802.15.4 packets.
        - Energy Detection on channel.
        - Setting the Channel for the radio. [b/w 11 - 26 channels]
        - Start and Stop the radio
        - h/w address filtering.

Signed-off-by: Varka Bhadram <redacted>
---
 drivers/net/ieee802154/cc2520.c | 1045 +++++++++++++++++++++++++++++++++++++++
 include/linux/spi/cc2520.h      |   26 +
 2 files changed, 1071 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..fff7fd2
--- /dev/null
+++ b/drivers/net/ieee802154/cc2520.c

@@ -0,0 +1,1045 @@
+/* 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.
+ *
+ */
+

...

quoted

+
+/* Driver private information */
+struct cc2520_private {
+	struct spi_device *spi;		/* spi device structure */
+	struct ieee802154_dev *dev;	/* Ieee802.15.4 device */

why lowercase IEEE in the comment? :-)

Ok.

quoted

quoted

+	u8 *buf;			/* SPI TX/Rx data buffer */
+	struct mutex buffer_mutex;	/* SPI buffer mutex */
+	unsigned is_tx:1;		/* Flag for sync b/w Tx and Rx */

bool/int?

I think there is no difference b/w 'unsigned' and 'unsigned int'. But it will be clear if we use 'unsigned int'

I meant the bitfield here. I only see bitfields if we they use that for
more than one bitfield. is_tx:1 is more a bool.

- Alex