Skip to contentSkip to conversation

mac802154: iface and driver-ops cleanup

PATCH In review Alexander Aring 3 participants 35 messages Oct 28, 2014 – Oct 29, 2014

Subsystems (5)
lore
Share & tools
lore

Alexander Aring

author maintainer
#0 lore raw

cover mac802154: iface and driver-ops cleanup

This patch series contains cleanups for iface handling driver-ops and sysfs handling. Also removing dead code of current behaviour of local->running and use it as local->started behaviour like mac8011.

Alexander Aring (17):
mac802154: monitor: merge into iface implementation
mac802154: main: move open and close into iface
mac802154: declare struct ieee802154_ops as const
mac802154: ops: declare channel and page as u8
mac802154: introduce driver-ops header
mac802154: use driver-ops function wrappers
mac802154: remove might_sleep from driver layer
mac802154: remove driver ops in wpan-phy
mac802154: rework sdata state change to running
mac802154: rename running to started
mac802154: move local started handling
mac802154: add synchronization handling
mac802154: iface: remove assign to zero
mac802154: remove channel attributes from sdata
mac802154: move mac_params functions into mac_cmd
mac802154: cleanup open count handling
ieee802154: introduce sysfs file

 drivers/net/ieee802154/at86rf230.c |  13 +--
 drivers/net/ieee802154/cc2520.c    |   5 +-
 drivers/net/ieee802154/fakelb.c    |   8 +-
 drivers/net/ieee802154/mrf24j40.c  |   5 +-
 include/net/cfg802154.h            |   8 --
 include/net/mac802154.h            |   7 +-
 net/ieee802154/Makefile            |   2 +-
 net/ieee802154/core.c              |  73 +------------
 net/ieee802154/nl-mac.c            |  19 +---
 net/ieee802154/sysfs.c             |  94 ++++++++++++++++
 net/ieee802154/sysfs.h             |   9 ++
 net/mac802154/Makefile             |   2 +-
 net/mac802154/driver-ops.h         | 213 +++++++++++++++++++++++++++++++++++++
 net/mac802154/ieee802154_i.h       |  34 +++---
 net/mac802154/iface.c              | 119 +++++++++++++++++----
 net/mac802154/mac_cmd.c            |  22 ++++
 net/mac802154/main.c               | 166 +----------------------------
 net/mac802154/mib.c                |  56 ++--------
 net/mac802154/monitor.c            |  59 ----------
 net/mac802154/tx.c                 |   5 +-
 20 files changed, 488 insertions(+), 431 deletions(-)
 create mode 100644 net/ieee802154/sysfs.c
 create mode 100644 net/ieee802154/sysfs.h
 create mode 100644 net/mac802154/driver-ops.h
 delete mode 100644 net/mac802154/monitor.c
Signature · 1 line
2.1.2
author maintainer
#1 lore raw

PATCH 1/17 mac802154: monitor: merge into iface implementation

This patch removes the monitor implementation file and put all monitor stuff into iface file. It's now small enough to put all necessary handling into iface.

---
 net/mac802154/Makefile  |  2 +-
 net/mac802154/iface.c   | 30 +++++++++++++++++++++++++
 net/mac802154/monitor.c | 59 -------------------------------------------------
 3 files changed, 31 insertions(+), 60 deletions(-)
 delete mode 100644 net/mac802154/monitor.c
net/mac802154/Makefile +1 -1
index e68deba..2e497d0 100644
@@ -1,5 +1,5 @@
 obj-$(CONFIG_MAC802154)	+= mac802154.o
 mac802154-objs		:= main.o rx.o tx.o mac_cmd.o mib.o \
-			   monitor.o iface.o llsec.o util.o
+			   iface.o llsec.o util.o
 
 ccflags-y += -D__CHECK_ENDIAN__
net/mac802154/iface.c +30 -0
index 7e4bffc..dafb2c3 100644
@@ -320,6 +320,12 @@ static const struct net_device_ops mac802154_wpan_ops = {
 	.ndo_set_mac_address	= mac802154_wpan_mac_addr,
 };
 
+static const struct net_device_ops mac802154_monitor_ops = {
+	.ndo_open		= mac802154_slave_open,
+	.ndo_stop		= mac802154_slave_close,
+	.ndo_start_xmit		= ieee802154_monitor_start_xmit,
+};
+
 static void mac802154_wpan_free(struct net_device *dev)
 {
 	struct ieee802154_sub_if_data *sdata = IEEE802154_DEV_TO_SUB_IF(dev);
@@ -373,3 +379,27 @@ void mac802154_wpan_setup(struct net_device *dev)
 
 	mac802154_llsec_init(&sdata->sec);
 }
+
+void mac802154_monitor_setup(struct net_device *dev)
+{
+	struct ieee802154_sub_if_data *sdata;
+
+	dev->addr_len		= 0;
+	dev->hard_header_len	= 0;
+	dev->needed_tailroom	= 2; /* room for FCS */
+	dev->mtu		= IEEE802154_MTU;
+	dev->tx_queue_len	= 10;
+	dev->type		= ARPHRD_IEEE802154_MONITOR;
+	dev->flags		= IFF_NOARP | IFF_BROADCAST;
+	dev->watchdog_timeo	= 0;
+
+	dev->destructor		= free_netdev;
+	dev->netdev_ops		= &mac802154_monitor_ops;
+	dev->ml_priv		= &mac802154_mlme_reduced;
+
+	sdata = IEEE802154_DEV_TO_SUB_IF(dev);
+	sdata->type = IEEE802154_DEV_MONITOR;
+
+	sdata->chan = MAC802154_CHAN_NONE; /* not initialized */
+	sdata->page = 0;
+}
net/mac802154/monitor.c deleted +0 -59

59 lines not on this page · raw message

Signature · 1 line
2.1.2
#2 lore raw

Re: [PATCH bluetooth-next 01/17] mac802154: monitor: merge into iface implementation

Quoted 4 lines · Alexander Aring

This patch removes the monitor implementation file and put all monitor stuff into iface file. It's now small enough to put all necessary handling into iface.

Quoted 8 lines
---
  net/mac802154/Makefile  |  2 +-
  net/mac802154/iface.c   | 30 +++++++++++++++++++++++++
  net/mac802154/monitor.c | 59 -------------------------------------------------
  3 files changed, 31 insertions(+), 60 deletions(-)

delete mode 100644 net/mac802154/monitor.c

Hi Alex,

Doing nice work,changing the code view same as mac80211.

But What ever you do in previous patches you are undoing, like moving some of the code to iface file from their to main file like that... Please correct me if I am wrong.

Signature · 2 lines
Regards,
Varka Bhadram.
#3 lore raw

Re: [PATCH bluetooth-next 01/17] mac802154: monitor: merge into iface implementation

Quoted 14 lines · Varka Bhadram
Quoted 4 lines · Alexander Aring

This patch removes the monitor implementation file and put all monitor stuff into iface file. It's now small enough to put all necessary handling into iface.

Quoted 9 lines
---
  net/mac802154/Makefile  |  2 +-
  net/mac802154/iface.c   | 30 +++++++++++++++++++++++++
  net/mac802154/monitor.c | 59 
-------------------------------------------------
  3 files changed, 31 insertions(+), 60 deletions(-)

delete mode 100644 net/mac802154/monitor.c

Quoted 8 lines

Hi Alex,

Doing nice work,changing the code view same as mac80211.

But What ever you do in previous patches you are undoing, like moving some of the code to iface file from their to main file like that... Please correct me if I am wrong.

Good to have the cover-letter for all these changes...

Signature · 2 lines
Regards,
Varka Bhadram.
author maintainer
#4 lore raw

Re: [PATCH bluetooth-next 01/17] mac802154: monitor: merge into iface implementation

Hi Varka,

Quoted 19 lines · Varka Bhadram
Quoted 18 lines · Varka Bhadram
Quoted 4 lines · Alexander Aring

This patch removes the monitor implementation file and put all monitor stuff into iface file. It's now small enough to put all necessary handling into iface.

Quoted 9 lines
---
 net/mac802154/Makefile  |  2 +-
 net/mac802154/iface.c   | 30 +++++++++++++++++++++++++
 net/mac802154/monitor.c | 59
-------------------------------------------------
 3 files changed, 31 insertions(+), 60 deletions(-)
 delete mode 100644 net/mac802154/monitor.c

Hi Alex,

Doing nice work,changing the code view same as mac80211.

Quoted 8 lines
Quoted 6 lines

But What ever you do in previous patches you are undoing, like moving some of the
code to iface file from their to main file like that... Please correct me if I am wrong.

Good to have the cover-letter for all these changes...

I tried to grab me one file and then doing all stuff which is in my rework branch. First tx file then rx file, now with iface sending trivial patches has ended here.

You are right cover-letters looks much ugly and patches contains too much changes sometimes.

I want that all these changes are contain in the upcomming kernel release. I will try to improve my cover-letter in the next series. For me wrtiting a commit msg is more difficult than writing code and there will comming much patches which will do many change.

The current code base for this branch is very bad and when we don't doing anything here in this structual architecture then this branch will die. For me it was already died when I cames mainline.

I can't simple send a patch which removes all 802.15.4 code and adding a new implementation. We need to do this in small steps.

- Alex

author maintainer
#5 lore raw

PATCH 2/17 mac802154: main: move open and close into iface

In reply to Alexander Aring

These functions can be static inside the iface file, because it's not used anywhere else. This patch moves these functions into iface file.

---
 net/mac802154/ieee802154_i.h |  3 ---
 net/mac802154/iface.c        | 59 ++++++++++++++++++++++++++++++++++++++++++++
 net/mac802154/main.c         | 59 --------------------------------------------
 3 files changed, 59 insertions(+), 62 deletions(-)
net/mac802154/ieee802154_i.h +0 -3
index ac907d9..4408c46 100644
@@ -123,9 +123,6 @@ IEEE802154_DEV_TO_SUB_IF(const struct net_device *dev)
 extern struct ieee802154_reduced_mlme_ops mac802154_mlme_reduced;
 extern struct ieee802154_mlme_ops mac802154_mlme_wpan;
 
-int mac802154_slave_open(struct net_device *dev);
-int mac802154_slave_close(struct net_device *dev);
-
 void mac802154_monitor_setup(struct net_device *dev);
 netdev_tx_t
 ieee802154_monitor_start_xmit(struct sk_buff *skb, struct net_device *dev);
net/mac802154/iface.c +59 -0

71 lines not on this page · raw message

net/mac802154/main.c +0 -59

65 lines not on this page · raw message

Signature · 1 line
2.1.2
author maintainer
#6 lore raw

PATCH 3/17 mac802154: declare struct ieee802154_ops as const

In reply to Alexander Aring

The ieee802154_ops structure should be never changed during runtime. This patch declare this structure as const to avoid a runtime change.

---
 drivers/net/ieee802154/at86rf230.c | 2 +-
 drivers/net/ieee802154/cc2520.c    | 2 +-
 drivers/net/ieee802154/fakelb.c    | 2 +-
 drivers/net/ieee802154/mrf24j40.c  | 2 +-
 include/net/mac802154.h            | 2 +-
 net/mac802154/ieee802154_i.h       | 2 +-
 net/mac802154/main.c               | 2 +-
 7 files changed, 7 insertions(+), 7 deletions(-)
drivers/net/ieee802154/at86rf230.c +1 -1
index a155838..662efd3 100644
@@ -1214,7 +1214,7 @@ at86rf230_set_frame_retries(struct ieee802154_hw *hw, s8 retries)
 	return rc;
 }
 
-static struct ieee802154_ops at86rf230_ops = {
+static const struct ieee802154_ops at86rf230_ops = {
 	.owner = THIS_MODULE,
 	.xmit_async = at86rf230_xmit,
 	.ed = at86rf230_ed,
drivers/net/ieee802154/cc2520.c +1 -1
index a31b5b6..b479c9d 100644
@@ -631,7 +631,7 @@ cc2520_filter(struct ieee802154_hw *hw,
 	return 0;
 }
 
-static struct ieee802154_ops cc2520_ops = {
+static const struct ieee802154_ops cc2520_ops = {
 	.owner = THIS_MODULE,
 	.start = cc2520_start,
 	.stop = cc2520_stop,
drivers/net/ieee802154/fakelb.c +1 -1
index db0703f..2a97cbb 100644
@@ -129,7 +129,7 @@ fakelb_hw_stop(struct ieee802154_hw *hw) {
 	spin_unlock(&priv->lock);
 }
 
-static struct ieee802154_ops fakelb_ops = {
+static const struct ieee802154_ops fakelb_ops = {
 	.owner = THIS_MODULE,
 	.xmit_sync = fakelb_hw_xmit,
 	.ed = fakelb_hw_ed,
drivers/net/ieee802154/mrf24j40.c +1 -1
index 7abb237..f19cf58 100644
@@ -579,7 +579,7 @@ out:
 	return ret;
 }
 
-static struct ieee802154_ops mrf24j40_ops = {
+static const struct ieee802154_ops mrf24j40_ops = {
 	.owner = THIS_MODULE,
 	.xmit_sync = mrf24j40_tx,
 	.ed = mrf24j40_ed,
include/net/mac802154.h +1 -1
index 4c4642e..0ea44cd 100644
@@ -197,7 +197,7 @@ struct ieee802154_ops {
 
 /* Basic interface to register ieee802154 hwice */
 struct ieee802154_hw *
-ieee802154_alloc_hw(size_t priv_data_len, struct ieee802154_ops *ops);
+ieee802154_alloc_hw(size_t priv_data_len, const struct ieee802154_ops *ops);
 void ieee802154_free_hw(struct ieee802154_hw *hw);
 int ieee802154_register_hw(struct ieee802154_hw *hw);
 void ieee802154_unregister_hw(struct ieee802154_hw *hw);
net/mac802154/ieee802154_i.h +1 -1
index 4408c46..a379b97 100644
@@ -28,7 +28,7 @@
 /* mac802154 device private data */
 struct ieee802154_local {
 	struct ieee802154_hw hw;
-	struct ieee802154_ops *ops;
+	const struct ieee802154_ops *ops;
 
 	/* ieee802154 phy */
 	struct wpan_phy *phy;
net/mac802154/main.c +1 -1
index 2c6d772..632707b 100644
@@ -187,7 +187,7 @@ static void ieee802154_tasklet_handler(unsigned long data)
 }
 
 struct ieee802154_hw *
-ieee802154_alloc_hw(size_t priv_data_len, struct ieee802154_ops *ops)
+ieee802154_alloc_hw(size_t priv_data_len, const struct ieee802154_ops *ops)
 {
 	struct wpan_phy *phy;
 	struct ieee802154_local *local;
Signature · 1 line
2.1.2
Acked-by on v1 3/17
#7 lore raw

Re: [PATCH bluetooth-next 03/17] mac802154: declare struct ieee802154_ops as const

Quoted 5 lines · Alexander Aring

The ieee802154_ops structure should be never changed during runtime. This patch declare this structure as const to avoid a runtime change.

Quoted 9 lines
---
  drivers/net/ieee802154/at86rf230.c | 2 +-
  drivers/net/ieee802154/cc2520.c    | 2 +-
  drivers/net/ieee802154/fakelb.c    | 2 +-
  drivers/net/ieee802154/mrf24j40.c  | 2 +-
  include/net/mac802154.h            | 2 +-
  net/mac802154/ieee802154_i.h       | 2 +-
  net/mac802154/main.c               | 2 +-
  7 files changed, 7 insertions(+), 7 deletions(-)

For CC2520 driver

Signature · 2 lines
Regards,
Varka Bhadram.
author maintainer
#8 lore raw

PATCH 4/17 mac802154: ops: declare channel and page as u8

In reply to Alexander Aring

The range of channel and page fits into an unsigned byte range. This patch changes the set_channel parameter definitions for channel and page to u8.

---
 drivers/net/ieee802154/at86rf230.c | 8 ++++----
 drivers/net/ieee802154/cc2520.c    | 2 +-
 drivers/net/ieee802154/fakelb.c    | 2 +-
 drivers/net/ieee802154/mrf24j40.c  | 3 +--
 include/net/mac802154.h            | 5 ++---
 5 files changed, 9 insertions(+), 11 deletions(-)
drivers/net/ieee802154/at86rf230.c +4 -4
index 662efd3..5068c1b 100644
@@ -54,7 +54,7 @@ struct at86rf2xx_chip_data {
 	u16 t_tx_timeout;
 	int rssi_base_val;
 
-	int (*set_channel)(struct at86rf230_local *, int, int);
+	int (*set_channel)(struct at86rf230_local *, u8, u8);
 	int (*get_desense_steps)(struct at86rf230_local *, s32);
 };
 
@@ -1012,13 +1012,13 @@ at86rf230_stop(struct ieee802154_hw *hw)
 }
 
 static int
-at86rf23x_set_channel(struct at86rf230_local *lp, int page, int channel)
+at86rf23x_set_channel(struct at86rf230_local *lp, u8 page, u8 channel)
 {
 	return at86rf230_write_subreg(lp, SR_CHANNEL, channel);
 }
 
 static int
-at86rf212_set_channel(struct at86rf230_local *lp, int page, int channel)
+at86rf212_set_channel(struct at86rf230_local *lp, u8 page, u8 channel)
 {
 	int rc;
 
@@ -1043,7 +1043,7 @@ at86rf212_set_channel(struct at86rf230_local *lp, int page, int channel)
 }
 
 static int
-at86rf230_channel(struct ieee802154_hw *hw, int page, int channel)
+at86rf230_channel(struct ieee802154_hw *hw, u8 page, u8 channel)
 {
 	struct at86rf230_local *lp = hw->priv;
 	int rc;
drivers/net/ieee802154/cc2520.c +1 -1
index b479c9d..78ea2ca 100644
@@ -569,7 +569,7 @@ cc2520_ed(struct ieee802154_hw *hw, u8 *level)
 }
 
 static int
-cc2520_set_channel(struct ieee802154_hw *hw, int page, int channel)
+cc2520_set_channel(struct ieee802154_hw *hw, u8 page, u8 channel)
 {
 	struct cc2520_private *priv = hw->priv;
 	int ret;
drivers/net/ieee802154/fakelb.c +1 -1
index 2a97cbb..4092e70 100644
@@ -55,7 +55,7 @@ fakelb_hw_ed(struct ieee802154_hw *hw, u8 *level)
 }
 
 static int
-fakelb_hw_channel(struct ieee802154_hw *hw, int page, int channel)
+fakelb_hw_channel(struct ieee802154_hw *hw, u8 page, u8 channel)
 {
 	pr_debug("set channel to %d\n", channel);
 
drivers/net/ieee802154/mrf24j40.c +1 -2
index f19cf58..52b3d31 100644
@@ -423,8 +423,7 @@ static void mrf24j40_stop(struct ieee802154_hw *hw)
 	write_short_reg(devrec, REG_INTCON, val);
 }
 
-static int mrf24j40_set_channel(struct ieee802154_hw *hw,
-				int page, int channel)
+static int mrf24j40_set_channel(struct ieee802154_hw *hw, u8 page, u8 channel)
 {
 	struct mrf24j40 *devrec = hw->priv;
 	u8 val;
include/net/mac802154.h +2 -3
index 0ea44cd..85a4efc 100644
@@ -178,9 +178,8 @@ struct ieee802154_ops {
 	int		(*xmit_async)(struct ieee802154_hw *hw,
 				      struct sk_buff *skb);
 	int		(*ed)(struct ieee802154_hw *hw, u8 *level);
-	int		(*set_channel)(struct ieee802154_hw *hw,
-				       int page,
-				       int channel);
+	int		(*set_channel)(struct ieee802154_hw *hw, u8 page,
+				       u8 channel);
 	int		(*set_hw_addr_filt)(struct ieee802154_hw *hw,
 					    struct ieee802154_hw_addr_filt *filt,
 					    unsigned long changed);
Signature · 1 line
2.1.2
Acked-by on v1 4/17
#9 lore raw

Re: [PATCH bluetooth-next 04/17] mac802154: ops: declare channel and page as u8

Quoted 6 lines · Alexander Aring

The range of channel and page fits into an unsigned byte range. This patch changes the set_channel parameter definitions for channel and page to u8.

Quoted 8 lines
---
  drivers/net/ieee802154/at86rf230.c | 8 ++++----
  drivers/net/ieee802154/cc2520.c    | 2 +-
  drivers/net/ieee802154/fakelb.c    | 2 +-
  drivers/net/ieee802154/mrf24j40.c  | 3 +--
  include/net/mac802154.h            | 5 ++---
  5 files changed, 9 insertions(+), 11 deletions(-)

For cc2520 driver change

Signature · 2 lines
Regards,
Varka Bhadram.
author maintainer
#10 lore raw

PATCH 5/17 mac802154: introduce driver-ops header

In reply to Alexander Aring

This patch introduce a driver-ops header file with function wrappers to call the driver ops. These wrappers checking on right context information and warn if optional driver ops are called when these aren't implemented. This behaviour is like mac80211 driver-ops header file, just without function tracing calls.

---
 net/mac802154/driver-ops.h | 202 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 202 insertions(+)
 create mode 100644 net/mac802154/driver-ops.h
net/mac802154/driver-ops.h new file +202 -0
new file mode 100644
index 0000000..bf7980b
@@ -0,0 +1,202 @@
+#ifndef __MAC802154_DRVIER_OPS
+#define __MAC802154_DRIVER_OPS
+
+#include <linux/types.h>
+#include <linux/rtnetlink.h>
+
+#include <net/mac802154.h>
+
+#include "ieee802154_i.h"
+
+static inline int
+drv_xmit_async(struct ieee802154_local *local, struct sk_buff *skb)
+{
+	return local->ops->xmit_async(&local->hw, skb);
+}
+
+static inline int
+drv_xmit_sync(struct ieee802154_local *local, struct sk_buff *skb)
+{
+	/* don't allow other operations while sync xmit */
+	ASSERT_RTNL();
+
+	might_sleep();
+
+	return local->ops->xmit_sync(&local->hw, skb);
+}
+
+static inline int drv_start(struct ieee802154_local *local)
+{
+	might_sleep();
+
+	return local->ops->start(&local->hw);
+}
+
+static inline void drv_stop(struct ieee802154_local *local)
+{
+	might_sleep();
+
+	local->ops->stop(&local->hw);
+}
+
+static inline int drv_set_channel(struct ieee802154_local *local,
+				  const u8 page, const u8 channel)
+{
+	might_sleep();
+
+	return local->ops->set_channel(&local->hw, page, channel);
+}
+
+static inline int drv_set_tx_power(struct ieee802154_local *local,
+				   const s8 dbm)
+{
+	might_sleep();
+
+	if (!local->ops->set_txpower) {
+		WARN_ON(1);
+		return -EOPNOTSUPP;
+	}
+
+	return local->ops->set_txpower(&local->hw, dbm);
+}
+
+static inline int drv_set_cca_mode(struct ieee802154_local *local,
+				   const u8 cca_mode)
+{
+	might_sleep();
+
+	if (!local->ops->set_cca_mode) {
+		WARN_ON(1);
+		return -EOPNOTSUPP;
+	}
+
+	return local->ops->set_cca_mode(&local->hw, cca_mode);
+}
+
+static inline int drv_set_lbt_mode(struct ieee802154_local *local,
+				   const bool mode)
+{
+	might_sleep();
+
+	if (!local->ops->set_lbt) {
+		WARN_ON(1);
+		return -EOPNOTSUPP;
+	}
+
+	return local->ops->set_lbt(&local->hw, mode);
+}
+
+static inline int drv_set_cca_ed_level(struct ieee802154_local *local,
+				       const s32 ed_level)
+{
+	might_sleep();
+
+	if (!local->ops->set_cca_ed_level) {
+		WARN_ON(1);
+		return -EOPNOTSUPP;
+	}
+
+	return local->ops->set_cca_ed_level(&local->hw, ed_level);
+}
+
+static inline int drv_set_pan_id(struct ieee802154_local *local,
+				 const __le16 pan_id)
+{
+	struct ieee802154_hw_addr_filt filt;
+
+	might_sleep();
+
+	if (!local->ops->set_hw_addr_filt) {
+		WARN_ON(1);
+		return -EOPNOTSUPP;
+	}
+
+	filt.pan_id = pan_id;
+
+	return local->ops->set_hw_addr_filt(&local->hw, &filt,
+					    IEEE802154_AFILT_PANID_CHANGED);
+}
+
+static inline int drv_set_extended_addr(struct ieee802154_local *local,
+					const __le64 extended_addr)
+{
+	struct ieee802154_hw_addr_filt filt;
+
+	might_sleep();
+
+	if (!local->ops->set_hw_addr_filt) {
+		WARN_ON(1);
+		return -EOPNOTSUPP;
+	}
+
+	filt.ieee_addr = extended_addr;
+
+	return local->ops->set_hw_addr_filt(&local->hw, &filt,
+					    IEEE802154_AFILT_IEEEADDR_CHANGED);
+}
+
+static inline int drv_set_short_addr(struct ieee802154_local *local,
+				     const __le16 short_addr)
+{
+	struct ieee802154_hw_addr_filt filt;
+
+	might_sleep();
+
+	if (!local->ops->set_hw_addr_filt) {
+		WARN_ON(1);
+		return -EOPNOTSUPP;
+	}
+
+	filt.short_addr = short_addr;
+
+	return local->ops->set_hw_addr_filt(&local->hw, &filt,
+					    IEEE802154_AFILT_SADDR_CHANGED);
+}
+
+static inline int drv_set_pan_coord(struct ieee802154_local *local,
+				    const bool is_coord)
+{
+	struct ieee802154_hw_addr_filt filt;
+
+	might_sleep();
+
+	if (!local->ops->set_hw_addr_filt) {
+		WARN_ON(1);
+		return -EOPNOTSUPP;
+	}
+
+	filt.pan_coord = is_coord;
+
+	return local->ops->set_hw_addr_filt(&local->hw, &filt,
+					    IEEE802154_AFILT_PANC_CHANGED);
+}
+
+static inline int drv_set_csma_params(struct ieee802154_local *local,
+				      u8 min_be, u8 max_be,
+				      u8 max_csma_backoffs)
+{
+	might_sleep();
+
+	if (!local->ops->set_csma_params) {
+		WARN_ON(1);
+		return -EOPNOTSUPP;
+	}
+
+	return local->ops->set_csma_params(&local->hw, min_be, max_be,
+					   max_csma_backoffs);
+}
+
+static inline int drv_set_max_frame_retries(struct ieee802154_local *local,
+					    s8 max_frame_retries)
+{
+	might_sleep();
+
+	if (!local->ops->set_frame_retries) {
+		WARN_ON(1);
+		return -EOPNOTSUPP;
+	}
+
+	return local->ops->set_frame_retries(&local->hw, max_frame_retries);
+}
+
+#endif /* __MAC802154_DRVIER_OPS */
Signature · 1 line
2.1.2
author maintainer
#11 lore raw

PATCH 6/17 mac802154: use driver-ops function wrappers

In reply to Alexander Aring

This patch replaces all directly called driver ops by previous introduced driver-ops function wrappers.

---
 net/mac802154/iface.c | 25 ++++++++++++++-----------
 net/mac802154/main.c  | 13 +++++++------
 net/mac802154/mib.c   |  3 ++-
 net/mac802154/tx.c    |  5 +++--
 4 files changed, 26 insertions(+), 20 deletions(-)
net/mac802154/iface.c +14 -11
index 0b21413..025cd5a 100644
@@ -30,6 +30,7 @@
 #include <net/cfg802154.h>
 
 #include "ieee802154_i.h"
+#include "driver-ops.h"
 
 static int mac802154_wpan_update_llsec(struct net_device *dev)
 {
@@ -168,7 +169,7 @@ static int mac802154_slave_open(struct net_device *dev)
 	mutex_unlock(&sdata->local->iflist_mtx);
 
 	if (local->open_count++ == 0) {
-		res = local->ops->start(&local->hw);
+		res = drv_start(local);
 		WARN_ON(res);
 		if (res)
 			goto err;
@@ -186,6 +187,7 @@ static int mac802154_wpan_open(struct net_device *dev)
 {
 	int rc;
 	struct ieee802154_sub_if_data *sdata = IEEE802154_DEV_TO_SUB_IF(dev);
+	struct ieee802154_local *local = sdata->local;
 	struct wpan_phy *phy = sdata->local->phy;
 
 	rc = mac802154_slave_open(dev);
@@ -195,40 +197,41 @@ static int mac802154_wpan_open(struct net_device *dev)
 	mutex_lock(&phy->pib_lock);
 
 	if (phy->set_txpower) {
-		rc = phy->set_txpower(phy, sdata->mac_params.transmit_power);
+		rc = drv_set_tx_power(local, sdata->mac_params.transmit_power);
 		if (rc < 0)
 			goto out;
 	}
 
 	if (phy->set_lbt) {
-		rc = phy->set_lbt(phy, sdata->mac_params.lbt);
+		rc = drv_set_lbt_mode(local, sdata->mac_params.lbt);
 		if (rc < 0)
 			goto out;
 	}
 
 	if (phy->set_cca_mode) {
-		rc = phy->set_cca_mode(phy, sdata->mac_params.cca_mode);
+		rc = drv_set_cca_mode(local, sdata->mac_params.cca_mode);
 		if (rc < 0)
 			goto out;
 	}
 
 	if (phy->set_cca_ed_level) {
-		rc = phy->set_cca_ed_level(phy, sdata->mac_params.cca_ed_level);
+		rc = drv_set_cca_ed_level(local,
+					  sdata->mac_params.cca_ed_level);
 		if (rc < 0)
 			goto out;
 	}
 
 	if (phy->set_csma_params) {
-		rc = phy->set_csma_params(phy, sdata->mac_params.min_be,
-					  sdata->mac_params.max_be,
-					  sdata->mac_params.csma_retries);
+		rc = drv_set_csma_params(local, sdata->mac_params.min_be,
+					 sdata->mac_params.max_be,
+					 sdata->mac_params.csma_retries);
 		if (rc < 0)
 			goto out;
 	}
 
 	if (phy->set_frame_retries) {
-		rc = phy->set_frame_retries(phy,
-					    sdata->mac_params.frame_retries);
+		rc = drv_set_max_frame_retries(local,
+					       sdata->mac_params.frame_retries);
 		if (rc < 0)
 			goto out;
 	}
@@ -255,7 +258,7 @@ static int mac802154_slave_close(struct net_device *dev)
 	mutex_unlock(&sdata->local->iflist_mtx);
 
 	if (!--local->open_count)
-		local->ops->stop(&local->hw);
+		drv_stop(local);
 
 	return 0;
 }
net/mac802154/main.c +7 -6
index 632707b..24ecc09 100644
@@ -28,6 +28,7 @@
 #include <net/cfg802154.h>
 
 #include "ieee802154_i.h"
+#include "driver-ops.h"
 
 static int
 mac802154_netdev_register(struct wpan_phy *phy, struct net_device *dev)
@@ -124,28 +125,28 @@ static int mac802154_set_txpower(struct wpan_phy *phy, int db)
 {
 	struct ieee802154_local *local = wpan_phy_priv(phy);
 
-	return local->ops->set_txpower(&local->hw, db);
+	return drv_set_tx_power(local, db);
 }
 
 static int mac802154_set_lbt(struct wpan_phy *phy, bool on)
 {
 	struct ieee802154_local *local = wpan_phy_priv(phy);
 
-	return local->ops->set_lbt(&local->hw, on);
+	return drv_set_lbt_mode(local, on);
 }
 
 static int mac802154_set_cca_mode(struct wpan_phy *phy, u8 mode)
 {
 	struct ieee802154_local *local = wpan_phy_priv(phy);
 
-	return local->ops->set_cca_mode(&local->hw, mode);
+	return drv_set_cca_mode(local, mode);
 }
 
 static int mac802154_set_cca_ed_level(struct wpan_phy *phy, s32 level)
 {
 	struct ieee802154_local *local = wpan_phy_priv(phy);
 
-	return local->ops->set_cca_ed_level(&local->hw, level);
+	return drv_set_cca_ed_level(local, level);
 }
 
 static int mac802154_set_csma_params(struct wpan_phy *phy, u8 min_be,
@@ -153,14 +154,14 @@ static int mac802154_set_csma_params(struct wpan_phy *phy, u8 min_be,
 {
 	struct ieee802154_local *local = wpan_phy_priv(phy);
 
-	return local->ops->set_csma_params(&local->hw, min_be, max_be, retries);
+	return drv_set_csma_params(local, min_be, max_be, retries);
 }
 
 static int mac802154_set_frame_retries(struct wpan_phy *phy, s8 retries)
 {
 	struct ieee802154_local *local = wpan_phy_priv(phy);
 
-	return local->ops->set_frame_retries(&local->hw, retries);
+	return drv_set_max_frame_retries(local, retries);
 }
 
 static void ieee802154_tasklet_handler(unsigned long data)
net/mac802154/mib.c +2 -1
index 16baff1..3fbc217 100644
@@ -24,6 +24,7 @@
 #include <net/cfg802154.h>
 
 #include "ieee802154_i.h"
+#include "driver-ops.h"
 
 struct phy_chan_notify_work {
 	struct work_struct work;
@@ -170,7 +171,7 @@ static void phy_chan_notify(struct work_struct *work)
 	int res;
 
 	mutex_lock(&sdata->local->phy->pib_lock);
-	res = local->ops->set_channel(&local->hw, sdata->page, sdata->chan);
+	res = drv_set_channel(local, sdata->page, sdata->chan);
 	if (res) {
 		pr_debug("set_channel failed\n");
 	} else {
net/mac802154/tx.c +3 -2
index e857673..77973a8 100644
@@ -28,6 +28,7 @@
 #include <net/cfg802154.h>
 
 #include "ieee802154_i.h"
+#include "driver-ops.h"
 
 /* IEEE 802.15.4 transceivers can sleep during the xmit session, so process
  * packets through the workqueue.
@@ -55,7 +56,7 @@ static void ieee802154_xmit_worker(struct work_struct *work)
 	if (!netif_running(dev))
 		goto err_tx;
 
-	res = local->ops->xmit_sync(&local->hw, skb);
+	res = drv_xmit_sync(local, skb);
 	if (res)
 		goto err_tx;
 
@@ -96,7 +97,7 @@ ieee802154_tx(struct ieee802154_local *local, struct sk_buff *skb)
 
 	/* async is priority, otherwise sync is fallback */
 	if (local->ops->xmit_async) {
-		ret = local->ops->xmit_async(&local->hw, skb);
+		ret = drv_xmit_async(local, skb);
 		if (ret) {
 			ieee802154_wake_queue(&local->hw);
 			goto err_tx;
Signature · 1 line
2.1.2
author maintainer
#12 lore raw

PATCH 7/17 mac802154: remove might_sleep from driver layer

In reply to Alexander Aring

This patch removes all might_sleep calls from driver layer. This handling is already done by mac802154 layer.

---
 drivers/net/ieee802154/at86rf230.c | 3 ---
 drivers/net/ieee802154/cc2520.c    | 1 -
 drivers/net/ieee802154/fakelb.c    | 4 ----
 3 files changed, 8 deletions(-)
drivers/net/ieee802154/at86rf230.c +0 -3
index 5068c1b..b83ad0b 100644
@@ -993,7 +993,6 @@ at86rf230_xmit(struct ieee802154_hw *hw, struct sk_buff *skb)
 static int
 at86rf230_ed(struct ieee802154_hw *hw, u8 *level)
 {
-	might_sleep();
 	BUG_ON(!level);
 	*level = 0xbe;
 	return 0;
@@ -1048,8 +1047,6 @@ at86rf230_channel(struct ieee802154_hw *hw, u8 page, u8 channel)
 	struct at86rf230_local *lp = hw->priv;
 	int rc;
 
-	might_sleep();
-
 	if (page < 0 || page > 31 ||
 	    !(lp->hw->phy->channels_supported[page] & BIT(channel))) {
 		WARN_ON(1);
drivers/net/ieee802154/cc2520.c +0 -1
index 78ea2ca..c56d10c 100644
@@ -574,7 +574,6 @@ cc2520_set_channel(struct ieee802154_hw *hw, u8 page, u8 channel)
 	struct cc2520_private *priv = hw->priv;
 	int ret;
 
-	might_sleep();
 	dev_dbg(&priv->spi->dev, "trying to set channel\n");
 
 	BUG_ON(page != 0);
drivers/net/ieee802154/fakelb.c +0 -4
index 4092e70..6e62286 100644
@@ -47,7 +47,6 @@ struct fakelb_priv {
 static int
 fakelb_hw_ed(struct ieee802154_hw *hw, u8 *level)
 {
-	might_sleep();
 	BUG_ON(!level);
 	*level = 0xbe;
 
@@ -59,7 +58,6 @@ fakelb_hw_channel(struct ieee802154_hw *hw, u8 page, u8 channel)
 {
 	pr_debug("set channel to %d\n", channel);
 
-	might_sleep();
 	hw->phy->current_page = page;
 	hw->phy->current_channel = channel;
 
@@ -85,8 +83,6 @@ fakelb_hw_xmit(struct ieee802154_hw *hw, struct sk_buff *skb)
 	struct fakelb_dev_priv *priv = hw->priv;
 	struct fakelb_priv *fake = priv->fake;
 
-	might_sleep();
-
 	read_lock_bh(&fake->lock);
 	if (priv->list.next == priv->list.prev) {
 		/* we are the only one device */
Signature · 1 line
2.1.2
Acked-by on v1 7/17
#13 lore raw

Re: [PATCH bluetooth-next 07/17] mac802154: remove might_sleep from driver layer

Quoted 10 lines · Alexander Aring

This patch removes all might_sleep calls from driver layer. This handling is already done by mac802154 layer.

---
  drivers/net/ieee802154/at86rf230.c | 3 ---
  drivers/net/ieee802154/cc2520.c    | 1 -
  drivers/net/ieee802154/fakelb.c    | 4 ----
  3 files changed, 8 deletions(-)

For CC2520 driver

Signature · 2 lines
Regards,
Varka Bhadram.
author maintainer
#14 lore raw

Re: [PATCH bluetooth-next 07/17] mac802154: remove might_sleep from driver layer

Hi Varka,

Quoted 4 lines · Varka Bhadram
Quoted 3 lines · Alexander Aring

This patch removes all might_sleep calls from driver layer. This handling is already done by mac802154 layer.

Quoted 9 lines
Quoted 7 lines
---
 drivers/net/ieee802154/at86rf230.c | 3 ---
 drivers/net/ieee802154/cc2520.c    | 1 -
 drivers/net/ieee802154/fakelb.c    | 4 ----
 3 files changed, 8 deletions(-)

For CC2520 driver

I would Cc you here if you stand in the MAINTAINERS file for cc2520 driver. I asked you some months ago if you like to be there. I didn't saw any patches for that. I thought it's out of interest by you that you want to be maintainer for this driver. When these changes are very trivial like this one, it's okay for you when Marcel simple apply these patches without any ack? I already told him that we don't need to wait of Alan's ack for the mrf24j40 driver.

Nevertheless if you want to add really a Acked here and we need to wait then please insert you in the MAINTAINERS file. When I run get_maintainers it's currently me who is maintainer for the cc2520 driver.

If you want to take care of this driver, please see my improvements and fixes for the at86rf230. I am sure you could also take some code for the cc2520 from it. Also I would be very happy for async xmit handling or regmap support. Most of the cc2520 code is lowlevel spi api calls.

- Alex

#15 lore raw

Re: [PATCH bluetooth-next 07/17] mac802154: remove might_sleep from driver layer

Quoted 28 lines · Alexander Aring

Hi Varka,

Quoted 4 lines · Varka Bhadram
Quoted 3 lines · Alexander Aring

This patch removes all might_sleep calls from driver layer. This handling is already done by mac802154 layer.

Quoted 9 lines
Quoted 7 lines
---
  drivers/net/ieee802154/at86rf230.c | 3 ---
  drivers/net/ieee802154/cc2520.c    | 1 -
  drivers/net/ieee802154/fakelb.c    | 4 ----
  3 files changed, 8 deletions(-)

For CC2520 driver

I would Cc you here if you stand in the MAINTAINERS file for cc2520 driver. I asked you some months ago if you like to be there. I didn't saw any patches for that. I thought it's out of interest by you that you want to be maintainer for this driver. When these changes are very trivial like this one, it's okay for you when Marcel simple apply these patches without any ack? I already told him that we don't need to wait of Alan's ack for the mrf24j40 driver.

Nevertheless if you want to add really a Acked here and we need to wait then please insert you in the MAINTAINERS file. When I run get_maintainers it's currently me who is maintainer for the cc2520 driver.

Quoted 6 lines

If you want to take care of this driver, please see my improvements and fixes for the at86rf230. I am sure you could also take some code for the cc2520 from it. Also I would be very happy for async xmit handling or regmap support. Most of the cc2520 code is lowlevel spi api calls.

- Alex

I think i clarified you that i want to be the CC2520 driver maintainer.

I asked you to submit the patche for MAINTAINER file, you told that its not required.

If i run get_maintainer on the cc2520 driver , my name is there for the commit_signer

I will improve this driver regmap suuport.

Signature · 2 lines
Regards,
Varka Bhadram.
author maintainer
#16 lore raw

Re: [PATCH bluetooth-next 07/17] mac802154: remove might_sleep from driver layer

Hi Varka,

Quoted 34 lines · Varka Bhadram
Quoted 33 lines · Alexander Aring

Hi Varka,

Quoted 4 lines · Varka Bhadram
Quoted 3 lines · Alexander Aring

This patch removes all might_sleep calls from driver layer. This handling is already done by mac802154 layer.

Quoted 9 lines
Quoted 7 lines
---
 drivers/net/ieee802154/at86rf230.c | 3 ---
 drivers/net/ieee802154/cc2520.c    | 1 -
 drivers/net/ieee802154/fakelb.c    | 4 ----
 3 files changed, 8 deletions(-)

For CC2520 driver

I would Cc you here if you stand in the MAINTAINERS file for cc2520 driver. I asked you some months ago if you like to be there. I didn't saw any patches for that. I thought it's out of interest by you that you want to be maintainer for this driver. When these changes are very trivial like this one, it's okay for you when Marcel simple apply these patches without any ack? I already told him that we don't need to wait of Alan's ack for the mrf24j40 driver.

Nevertheless if you want to add really a Acked here and we need to wait then please insert you in the MAINTAINERS file. When I run get_maintainers it's currently me who is maintainer for the cc2520 driver.

If you want to take care of this driver, please see my improvements and fixes for the at86rf230. I am sure you could also take some code for the cc2520 from it. Also I would be very happy for async xmit handling or regmap support. Most of the cc2520 code is lowlevel spi api calls.

Quoted 7 lines
Quoted 1 line

- Alex

I think i clarified you that i want to be the CC2520 driver maintainer.

I asked you to submit the patche for MAINTAINER file, you told that its not required.

I looked up the conversion:

You:

"You mean to say that i need to add an entry in MAINTAINER file or ..?"

Me:

"not need, only if you like. I have no idea about the cc2520 driver."

need != required. For me this sentence is more... you really don't need that do be maintainer of this if you don't like that. But if you like to be maintainer, then send a patch.

Quoted 2 lines

If i run get_maintainer on the cc2520 driver , my name is there for the commit_signer

This is because get_maintainers lookups the last commits.

I run "./scripts/get_maintainer.pl -f drivers/net/ieee802154/cc2520.c" on my own.

Output is:

Alexander Aring <[email protected]> (maintainer:IEEE 802.15.4 SUB...) Grant Likely <[email protected]> (maintainer:OPEN FIRMWARE AND...) Rob Herring <[email protected]> (maintainer:OPEN FIRMWARE AND...) [email protected] (open list:IEEE 802.15.4 SUB...) [email protected] (open list:NETWORKING DRIVERS) [email protected] (open list) [email protected] (open list:OPEN FIRMWARE AND...)

there is no Varka here.

Quoted 2 lines

I will improve this driver regmap suuport.

ok.

- Alex

author maintainer
#17 lore raw

Re: [PATCH bluetooth-next 07/17] mac802154: remove might_sleep from driver layer

Hi Varka,

it's too early in the morning, apologize my english here.

Quoted 39 lines · Alexander Aring

Hi Varka,

Quoted 36 lines · Varka Bhadram
Quoted 34 lines · Alexander Aring

Hi Varka,

Quoted 4 lines · Varka Bhadram
Quoted 3 lines · Alexander Aring

This patch removes all might_sleep calls from driver layer. This handling is already done by mac802154 layer.

Quoted 9 lines
Quoted 7 lines
---
 drivers/net/ieee802154/at86rf230.c | 3 ---
 drivers/net/ieee802154/cc2520.c    | 1 -
 drivers/net/ieee802154/fakelb.c    | 4 ----
 3 files changed, 8 deletions(-)

For CC2520 driver

I would Cc you here if you stand in the MAINTAINERS file for cc2520 driver. I asked you some months ago if you like to be there. I didn't saw any patches for that. I thought it's out of interest by you that you want to be maintainer for this driver. When these changes are very trivial like this one, it's okay for you when Marcel simple apply these patches without any ack? I already told him that we don't need to wait of Alan's ack for the mrf24j40 driver.

Nevertheless if you want to add really a Acked here and we need to wait then please insert you in the MAINTAINERS file. When I run get_maintainers it's currently me who is maintainer for the cc2520 driver.

If you want to take care of this driver, please see my improvements and fixes for the at86rf230. I am sure you could also take some code for the cc2520 from it. Also I would be very happy for async xmit handling or regmap support. Most of the cc2520 code is lowlevel spi api calls.

- Alex

Quoted 8 lines
Quoted 5 lines

I think i clarified you that i want to be the CC2520 driver maintainer.

I asked you to submit the patche for MAINTAINER file, you told that its not required.

I looked up the conversion:

s/conversion/conversation/

Quoted 10 lines

You:

"You mean to say that i need to add an entry in MAINTAINER file or ..?"

Me:

"not need, only if you like. I have no idea about the cc2520 driver."

need != required. For me this sentence is more... you really don't need that do be maintainer of this if you don't like that. But if you like to

s/do/to

well, I think I cherry-pick some commits now to generate the next series.

- Alex

#18 lore raw

Re: [PATCH bluetooth-next 07/17] mac802154: remove might_sleep from driver layer

In reply to Alexander Aring
Quoted 49 lines · Alexander Aring

Hi Varka,

Quoted 34 lines · Varka Bhadram
Quoted 33 lines · Alexander Aring

Hi Varka,

Quoted 4 lines · Varka Bhadram
Quoted 3 lines · Alexander Aring

This patch removes all might_sleep calls from driver layer. This handling is already done by mac802154 layer.

Quoted 9 lines
Quoted 7 lines
---
  drivers/net/ieee802154/at86rf230.c | 3 ---
  drivers/net/ieee802154/cc2520.c    | 1 -
  drivers/net/ieee802154/fakelb.c    | 4 ----
  3 files changed, 8 deletions(-)

For CC2520 driver

I would Cc you here if you stand in the MAINTAINERS file for cc2520 driver. I asked you some months ago if you like to be there. I didn't saw any patches for that. I thought it's out of interest by you that you want to be maintainer for this driver. When these changes are very trivial like this one, it's okay for you when Marcel simple apply these patches without any ack? I already told him that we don't need to wait of Alan's ack for the mrf24j40 driver.

Nevertheless if you want to add really a Acked here and we need to wait then please insert you in the MAINTAINERS file. When I run get_maintainers it's currently me who is maintainer for the cc2520 driver.

If you want to take care of this driver, please see my improvements and fixes for the at86rf230. I am sure you could also take some code for the cc2520 from it. Also I would be very happy for async xmit handling or regmap support. Most of the cc2520 code is lowlevel spi api calls.

Quoted 6 lines
Quoted 1 line

- Alex

I think i clarified you that i want to be the CC2520 driver maintainer.

I asked you to submit the patche for MAINTAINER file, you told that its not required.

I looked up the conversion:

You:

"You mean to say that i need to add an entry in MAINTAINER file or ..?"

Quoted 7 lines

Me:

"not need, only if you like. I have no idea about the cc2520 driver."

need != required. For me this sentence is more... you really don't need that do be maintainer of this if you don't like that. But if you like to be maintainer, then send a patch.

I will send a patch for the MAINTAINER.

Quoted 18 lines
Quoted 2 lines

If i run get_maintainer on the cc2520 driver , my name is there for the commit_signer

This is because get_maintainers lookups the last commits.

I run "./scripts/get_maintainer.pl -f drivers/net/ieee802154/cc2520.c" on my own.

Output is:

Alexander Aring <[email protected]> (maintainer:IEEE 802.15.4 SUB...) Grant Likely <[email protected]> (maintainer:OPEN FIRMWARE AND...) Rob Herring <[email protected]> (maintainer:OPEN FIRMWARE AND...) [email protected] (open list:IEEE 802.15.4 SUB...) [email protected] (open list:NETWORKING DRIVERS) [email protected] (open list) [email protected] (open list:OPEN FIRMWARE AND...)

Quoted 1 line

there is no Varka here.

Ok.. I will update.

Quoted 5 lines
Quoted 2 lines

I will improve this driver regmap suuport.

ok.

- Alex

Thanks...

Signature · 2 lines
Regards,
Varka Bhadram.
#19 lore raw

Re: [PATCH bluetooth-next 07/17] mac802154: remove might_sleep from driver layer

Quoted 82 lines · Varka Bhadram
Quoted 49 lines · Alexander Aring

Hi Varka,

Quoted 34 lines · Varka Bhadram
Quoted 33 lines · Alexander Aring

Hi Varka,

Quoted 4 lines · Varka Bhadram
Quoted 3 lines · Alexander Aring

This patch removes all might_sleep calls from driver layer. This handling is already done by mac802154 layer.

Quoted 9 lines
Quoted 7 lines
---
  drivers/net/ieee802154/at86rf230.c | 3 ---
  drivers/net/ieee802154/cc2520.c    | 1 -
  drivers/net/ieee802154/fakelb.c    | 4 ----
  3 files changed, 8 deletions(-)

For CC2520 driver

I would Cc you here if you stand in the MAINTAINERS file for cc2520 driver. I asked you some months ago if you like to be there. I didn't saw any patches for that. I thought it's out of interest by you that you want to be maintainer for this driver. When these changes are very trivial like this one, it's okay for you when Marcel simple apply these patches without any ack? I already told him that we don't need to wait of Alan's ack for the mrf24j40 driver.

Nevertheless if you want to add really a Acked here and we need to wait then please insert you in the MAINTAINERS file. When I run get_maintainers it's currently me who is maintainer for the cc2520 driver.

If you want to take care of this driver, please see my improvements and fixes for the at86rf230. I am sure you could also take some code for the cc2520 from it. Also I would be very happy for async xmit handling or regmap support. Most of the cc2520 code is lowlevel spi api calls.

Quoted 6 lines
Quoted 1 line

- Alex

I think i clarified you that i want to be the CC2520 driver maintainer.

I asked you to submit the patche for MAINTAINER file, you told that its not required.

I looked up the conversion:

You:

"You mean to say that i need to add an entry in MAINTAINER file or ..?"

Quoted 7 lines

Me:

"not need, only if you like. I have no idea about the cc2520 driver."

need != required. For me this sentence is more... you really don't need that do be maintainer of this if you don't like that. But if you like to be maintainer, then send a patch.

I will send a patch for the MAINTAINER.

Quoted 18 lines
Quoted 2 lines

If i run get_maintainer on the cc2520 driver , my name is there for the commit_signer

This is because get_maintainers lookups the last commits.

I run "./scripts/get_maintainer.pl -f drivers/net/ieee802154/cc2520.c" on my own.

Output is:

Alexander Aring <[email protected]> (maintainer:IEEE 802.15.4 SUB...) Grant Likely <[email protected]> (maintainer:OPEN FIRMWARE AND...) Rob Herring <[email protected]> (maintainer:OPEN FIRMWARE AND...) [email protected] (open list:IEEE 802.15.4 SUB...) [email protected] (open list:NETWORKING DRIVERS) [email protected] (open list) [email protected] (open list:OPEN FIRMWARE AND...)

Quoted 1 line

there is no Varka here.

Ok.. I will update.

Quoted 8 lines
Quoted 5 lines
Quoted 2 lines

I will improve this driver regmap suuport.

ok.

- Alex

Thanks...

Put cc to me incase of cc2520 driver changes.

Wait for my Ack on those changes.

Signature · 2 lines
Regards,
Varka Bhadram.
author maintainer
#20 lore raw

PATCH 8/17 mac802154: remove driver ops in wpan-phy

In reply to Alexander Aring

This patch removes the driver ops callbacks inside of wpan_phy struct. It was used to check if a phy supports this driver ops call. We do this now via hardware flags.

---
 include/net/cfg802154.h |  8 -----
 net/ieee802154/nl-mac.c | 19 ++---------
 net/mac802154/iface.c   | 12 +++----
 net/mac802154/main.c    | 85 -------------------------------------------------
 4 files changed, 8 insertions(+), 116 deletions(-)
include/net/cfg802154.h +0 -8
index 5c67467..440b9be 100644
@@ -57,14 +57,6 @@ struct wpan_phy {
 					const char *name, int type);
 	void (*del_iface)(struct wpan_phy *phy, struct net_device *dev);
 
-	int (*set_txpower)(struct wpan_phy *phy, int db);
-	int (*set_lbt)(struct wpan_phy *phy, bool on);
-	int (*set_cca_mode)(struct wpan_phy *phy, u8 cca_mode);
-	int (*set_cca_ed_level)(struct wpan_phy *phy, int level);
-	int (*set_csma_params)(struct wpan_phy *phy, u8 min_be, u8 max_be,
-			       u8 retries);
-	int (*set_frame_retries)(struct wpan_phy *phy, s8 retries);
-
 	char priv[0] __aligned(NETDEV_ALIGN);
 };
 
net/ieee802154/nl-mac.c +2 -17
index fb6866d..abd0f31 100644
@@ -664,20 +664,6 @@ int ieee802154_set_macparams(struct sk_buff *skb, struct genl_info *info)
 
 	phy = ops->get_phy(dev);
 
-	if ((!phy->set_lbt && info->attrs[IEEE802154_ATTR_LBT_ENABLED]) ||
-	    (!phy->set_cca_mode && info->attrs[IEEE802154_ATTR_CCA_MODE]) ||
-	    (!phy->set_cca_ed_level &&
-	     info->attrs[IEEE802154_ATTR_CCA_ED_LEVEL]) ||
-	    (!phy->set_csma_params &&
-	     (info->attrs[IEEE802154_ATTR_CSMA_RETRIES] ||
-	      info->attrs[IEEE802154_ATTR_CSMA_MIN_BE] ||
-	      info->attrs[IEEE802154_ATTR_CSMA_MAX_BE])) ||
-	    (!phy->set_frame_retries &&
-	     info->attrs[IEEE802154_ATTR_FRAME_RETRIES])) {
-		rc = -EOPNOTSUPP;
-		goto out_phy;
-	}
-
 	ops->get_mac_params(dev, &params);
 
 	if (info->attrs[IEEE802154_ATTR_TXPOWER])
@@ -708,10 +694,9 @@ int ieee802154_set_macparams(struct sk_buff *skb, struct genl_info *info)
 
 	wpan_phy_put(phy);
 	dev_put(dev);
-	return rc;
 
-out_phy:
-	wpan_phy_put(phy);
+	return 0;
+
 out:
 	dev_put(dev);
 	return rc;
net/mac802154/iface.c +6 -6
index 025cd5a..300877a 100644
@@ -196,32 +196,32 @@ static int mac802154_wpan_open(struct net_device *dev)
 
 	mutex_lock(&phy->pib_lock);
 
-	if (phy->set_txpower) {
+	if (local->hw.flags & IEEE802154_HW_TXPOWER) {
 		rc = drv_set_tx_power(local, sdata->mac_params.transmit_power);
 		if (rc < 0)
 			goto out;
 	}
 
-	if (phy->set_lbt) {
+	if (local->hw.flags & IEEE802154_HW_LBT) {
 		rc = drv_set_lbt_mode(local, sdata->mac_params.lbt);
 		if (rc < 0)
 			goto out;
 	}
 
-	if (phy->set_cca_mode) {
+	if (local->hw.flags & IEEE802154_HW_CCA_MODE) {
 		rc = drv_set_cca_mode(local, sdata->mac_params.cca_mode);
 		if (rc < 0)
 			goto out;
 	}
 
-	if (phy->set_cca_ed_level) {
+	if (local->hw.flags & IEEE802154_HW_CCA_ED_LEVEL) {
 		rc = drv_set_cca_ed_level(local,
 					  sdata->mac_params.cca_ed_level);
 		if (rc < 0)
 			goto out;
 	}
 
-	if (phy->set_csma_params) {
+	if (local->hw.flags & IEEE802154_HW_CSMA_PARAMS) {
 		rc = drv_set_csma_params(local, sdata->mac_params.min_be,
 					 sdata->mac_params.max_be,
 					 sdata->mac_params.csma_retries);
@@ -229,7 +229,7 @@ static int mac802154_wpan_open(struct net_device *dev)
 			goto out;
 	}
 
-	if (phy->set_frame_retries) {
+	if (local->hw.flags & IEEE802154_HW_FRAME_RETRIES) {
 		rc = drv_set_max_frame_retries(local,
 					       sdata->mac_params.frame_retries);
 		if (rc < 0)
net/mac802154/main.c +0 -85

97 lines not on this page · raw message

Signature · 1 line
2.1.2
author maintainer
#21 lore raw

PATCH 9/17 mac802154: rework sdata state change to running

In reply to Alexander Aring

This patch reworks the handling for setting the state like mac80211. We use bit's instead a bool variable. The mutex is not needed because it use test and set bits which are atomic operations.

---
 net/mac802154/ieee802154_i.h | 12 +++++++++++-
 net/mac802154/iface.c        | 12 +++++-------
 2 files changed, 16 insertions(+), 8 deletions(-)
net/mac802154/ieee802154_i.h +11 -1
index a379b97..e34fe51 100644
@@ -67,6 +67,10 @@ enum {
 	IEEE802154_RX_MSG        = 1,
 };
 
+enum ieee802154_sdata_state_bits {
+	SDATA_STATE_RUNNING,
+};
+
 /* Slave interface definition.
  *
  * Slaves represent typical network interfaces available from userspace.
@@ -80,7 +84,7 @@ struct ieee802154_sub_if_data {
 	struct net_device *dev;
 
 	int type;
-	bool running;
+	unsigned long state;
 
 	spinlock_t mib_lock;
 
@@ -120,6 +124,12 @@ IEEE802154_DEV_TO_SUB_IF(const struct net_device *dev)
 	return netdev_priv(dev);
 }
 
+static inline bool
+ieee802154_sdata_running(struct ieee802154_sub_if_data *sdata)
+{
+	return test_bit(SDATA_STATE_RUNNING, &sdata->state);
+}
+
 extern struct ieee802154_reduced_mlme_ops mac802154_mlme_reduced;
 extern struct ieee802154_mlme_ops mac802154_mlme_wpan;
 
net/mac802154/iface.c +5 -7
index 300877a..e10fd78 100644
@@ -156,7 +156,7 @@ static int mac802154_slave_open(struct net_device *dev)
 		mutex_lock(&sdata->local->iflist_mtx);
 		list_for_each_entry(subif, &sdata->local->interfaces, list) {
 			if (subif != sdata && subif->type == sdata->type &&
-			    subif->running) {
+			    ieee802154_sdata_running(subif)) {
 				mutex_unlock(&sdata->local->iflist_mtx);
 				return -EBUSY;
 			}
@@ -164,9 +164,7 @@ static int mac802154_slave_open(struct net_device *dev)
 		mutex_unlock(&sdata->local->iflist_mtx);
 	}
 
-	mutex_lock(&sdata->local->iflist_mtx);
-	sdata->running = true;
-	mutex_unlock(&sdata->local->iflist_mtx);
+	set_bit(SDATA_STATE_RUNNING, &sdata->state);
 
 	if (local->open_count++ == 0) {
 		res = drv_start(local);
@@ -178,6 +176,8 @@ static int mac802154_slave_open(struct net_device *dev)
 	netif_start_queue(dev);
 	return 0;
 err:
+	/* might already be clear but that doesn't matter */
+	clear_bit(SDATA_STATE_RUNNING, &sdata->state);
 	sdata->local->open_count--;
 
 	return res;
@@ -253,9 +253,7 @@ static int mac802154_slave_close(struct net_device *dev)
 
 	netif_stop_queue(dev);
 
-	mutex_lock(&sdata->local->iflist_mtx);
-	sdata->running = false;
-	mutex_unlock(&sdata->local->iflist_mtx);
+	clear_bit(SDATA_STATE_RUNNING, &sdata->state);
 
 	if (!--local->open_count)
 		drv_stop(local);
Signature · 1 line
2.1.2
author maintainer
#22 lore raw

PATCH 10/17 mac802154: rename running to started

In reply to Alexander Aring

This variable should be handled like ieee80211_local struct of mac80211. We rename this variable to started now to have the same name convention.

---
 net/mac802154/ieee802154_i.h | 2 +-
 net/mac802154/main.c         | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)
net/mac802154/ieee802154_i.h +1 -1
index e34fe51..f52afd4 100644
@@ -54,7 +54,7 @@ struct ieee802154_local {
 	 * This flag should be modified under slaves_mtx and RTNL, so you can
 	 * read them using any of protection methods.
 	 */
-	bool running;
+	bool started;
 
 	struct tasklet_struct tasklet;
 	struct sk_buff_head skb_queue;
net/mac802154/main.c +3 -3
index 9fa9514..7458f71 100644
@@ -47,7 +47,7 @@ mac802154_netdev_register(struct wpan_phy *phy, struct net_device *dev)
 	SET_NETDEV_DEV(dev, &local->phy->dev);
 
 	mutex_lock(&local->iflist_mtx);
-	if (!local->running) {
+	if (!local->started) {
 		mutex_unlock(&local->iflist_mtx);
 		return -ENODEV;
 	}
@@ -238,7 +238,7 @@ int ieee802154_register_hw(struct ieee802154_hw *hw)
 	rtnl_lock();
 
 	mutex_lock(&local->iflist_mtx);
-	local->running = MAC802154_DEVICE_RUN;
+	local->started = MAC802154_DEVICE_RUN;
 	mutex_unlock(&local->iflist_mtx);
 
 	rtnl_unlock();
@@ -264,7 +264,7 @@ void ieee802154_unregister_hw(struct ieee802154_hw *hw)
 	rtnl_lock();
 
 	mutex_lock(&local->iflist_mtx);
-	local->running = MAC802154_DEVICE_STOPPED;
+	local->started = MAC802154_DEVICE_STOPPED;
 	mutex_unlock(&local->iflist_mtx);
 
 	list_for_each_entry_safe(sdata, next, &local->interfaces, list) {
Signature · 1 line
2.1.2
author maintainer
#23 lore raw

PATCH 11/17 mac802154: move local started handling

In reply to Alexander Aring

This patch removes the current handling of started boolean. This is actually dead code, because mac802154_netdev_register can't never be called before ieee802154_register_hw. This means that local->started is always be true when mac802154_netdev_register is called. Instead we using this now like mac80211 to indicate that an instance of sdata is running.

---
 net/mac802154/driver-ops.h   |  4 ++++
 net/mac802154/ieee802154_i.h |  7 -------
 net/mac802154/main.c         | 19 -------------------
 3 files changed, 4 insertions(+), 26 deletions(-)
net/mac802154/driver-ops.h +4 -0
index bf7980b..bb3ee03 100644
@@ -29,6 +29,8 @@ static inline int drv_start(struct ieee802154_local *local)
 {
 	might_sleep();
 
+	local->started = true;
+
 	return local->ops->start(&local->hw);
 }
 
@@ -37,6 +39,8 @@ static inline void drv_stop(struct ieee802154_local *local)
 	might_sleep();
 
 	local->ops->stop(&local->hw);
+
+	local->started = false;
 }
 
 static inline int drv_set_channel(struct ieee802154_local *local,
net/mac802154/ieee802154_i.h +0 -7
index f52afd4..bf0b5f6 100644
@@ -50,19 +50,12 @@ struct ieee802154_local {
 	 */
 	struct workqueue_struct	*workqueue;
 
-	/* SoftMAC device is registered and running. One can add subinterfaces.
-	 * This flag should be modified under slaves_mtx and RTNL, so you can
-	 * read them using any of protection methods.
-	 */
 	bool started;
 
 	struct tasklet_struct tasklet;
 	struct sk_buff_head skb_queue;
 };
 
-#define	MAC802154_DEVICE_STOPPED	0x00
-#define MAC802154_DEVICE_RUN		0x01
-
 enum {
 	IEEE802154_RX_MSG        = 1,
 };
net/mac802154/main.c +0 -19
index 7458f71..21dcc56 100644
@@ -46,13 +46,6 @@ mac802154_netdev_register(struct wpan_phy *phy, struct net_device *dev)
 
 	SET_NETDEV_DEV(dev, &local->phy->dev);
 
-	mutex_lock(&local->iflist_mtx);
-	if (!local->started) {
-		mutex_unlock(&local->iflist_mtx);
-		return -ENODEV;
-	}
-	mutex_unlock(&local->iflist_mtx);
-
 	err = register_netdev(dev);
 	if (err < 0)
 		return err;
@@ -235,14 +228,6 @@ int ieee802154_register_hw(struct ieee802154_hw *hw)
 	if (rc < 0)
 		goto out_wq;
 
-	rtnl_lock();
-
-	mutex_lock(&local->iflist_mtx);
-	local->started = MAC802154_DEVICE_RUN;
-	mutex_unlock(&local->iflist_mtx);
-
-	rtnl_unlock();
-
 	return 0;
 
 out_wq:
@@ -263,10 +248,6 @@ void ieee802154_unregister_hw(struct ieee802154_hw *hw)
 
 	rtnl_lock();
 
-	mutex_lock(&local->iflist_mtx);
-	local->started = MAC802154_DEVICE_STOPPED;
-	mutex_unlock(&local->iflist_mtx);
-
 	list_for_each_entry_safe(sdata, next, &local->interfaces, list) {
 		mutex_lock(&sdata->local->iflist_mtx);
 		list_del(&sdata->list);
Signature · 1 line
2.1.2
author maintainer
#24 lore raw

PATCH 12/17 mac802154: add synchronization handling

In reply to Alexander Aring

This patch adds synchronization handling in start and stop driver ops calls. This patch is mostly grab from mac80211 which was introduced by commit ea77f12f2cc0f31168f2e0259e65a22202ac4dc2 ("mac80211: remove tasklet enable/disable"). This is to be sure that we don't run into same issues.

---
 net/mac802154/driver-ops.h | 7 +++++++
 1 file changed, 7 insertions(+)
net/mac802154/driver-ops.h +7 -0
index bb3ee03..4b820cf 100644
@@ -30,6 +30,7 @@ static inline int drv_start(struct ieee802154_local *local)
 	might_sleep();
 
 	local->started = true;
+	smp_mb();
 
 	return local->ops->start(&local->hw);
 }
@@ -40,6 +41,12 @@ static inline void drv_stop(struct ieee802154_local *local)
 
 	local->ops->stop(&local->hw);
 
+	/* sync away all work on the tasklet before clearing started */
+	tasklet_disable(&local->tasklet);
+	tasklet_enable(&local->tasklet);
+
+	barrier();
+
 	local->started = false;
 }
 
Signature · 1 line
2.1.2
author maintainer
#25 lore raw

PATCH 13/17 mac802154: iface: remove assign to zero

In reply to Alexander Aring

These variables should already be zero, so we remove the extra assign to zero.

---
 net/mac802154/iface.c | 6 ------
 1 file changed, 6 deletions(-)
net/mac802154/iface.c +0 -6
index e10fd78..2423aa7 100644
@@ -409,7 +409,6 @@ void mac802154_wpan_setup(struct net_device *dev)
 	dev->tx_queue_len	= 300;
 	dev->type		= ARPHRD_IEEE802154;
 	dev->flags		= IFF_NOARP | IFF_BROADCAST;
-	dev->watchdog_timeo	= 0;
 
 	dev->destructor		= mac802154_wpan_free;
 	dev->netdev_ops		= &mac802154_wpan_ops;
@@ -419,7 +418,6 @@ void mac802154_wpan_setup(struct net_device *dev)
 	sdata->type = IEEE802154_DEV_WPAN;
 
 	sdata->chan = MAC802154_CHAN_NONE;
-	sdata->page = 0;
 
 	spin_lock_init(&sdata->mib_lock);
 	mutex_init(&sdata->sec_mtx);
@@ -444,14 +442,11 @@ void mac802154_monitor_setup(struct net_device *dev)
 {
 	struct ieee802154_sub_if_data *sdata;
 
-	dev->addr_len		= 0;
-	dev->hard_header_len	= 0;
 	dev->needed_tailroom	= 2; /* room for FCS */
 	dev->mtu		= IEEE802154_MTU;
 	dev->tx_queue_len	= 10;
 	dev->type		= ARPHRD_IEEE802154_MONITOR;
 	dev->flags		= IFF_NOARP | IFF_BROADCAST;
-	dev->watchdog_timeo	= 0;
 
 	dev->destructor		= free_netdev;
 	dev->netdev_ops		= &mac802154_monitor_ops;
@@ -461,5 +456,4 @@ void mac802154_monitor_setup(struct net_device *dev)
 	sdata->type = IEEE802154_DEV_MONITOR;
 
 	sdata->chan = MAC802154_CHAN_NONE; /* not initialized */
-	sdata->page = 0;
 }
Signature · 1 line
2.1.2
author maintainer
#26 lore raw

PATCH 14/17 mac802154: remove channel attributes from sdata

In reply to Alexander Aring

These channel attributes was part of "channel context switch while xmit" which was removed by commit dc67c6b30f36d57b70b70547a30e7a8432540c6f ("mac802154: tx: remove xmit channel context switch"). This patch removes these unnecessary variables and use the current_page and current_channel by wpan_phy struct now.

---
 net/mac802154/ieee802154_i.h |  3 ---
 net/mac802154/iface.c        |  4 ----
 net/mac802154/mib.c          | 55 ++++++++------------------------------------
 3 files changed, 9 insertions(+), 53 deletions(-)
net/mac802154/ieee802154_i.h +0 -3
index bf0b5f6..46c9fe7 100644
@@ -85,9 +85,6 @@ struct ieee802154_sub_if_data {
 	__le16 short_addr;
 	__le64 extended_addr;
 
-	u8 chan;
-	u8 page;
-
 	struct ieee802154_mac_params mac_params;
 
 	/* MAC BSN field */
net/mac802154/iface.c +0 -4
index 2423aa7..1c0274e 100644
@@ -417,8 +417,6 @@ void mac802154_wpan_setup(struct net_device *dev)
 	sdata = IEEE802154_DEV_TO_SUB_IF(dev);
 	sdata->type = IEEE802154_DEV_WPAN;
 
-	sdata->chan = MAC802154_CHAN_NONE;
-
 	spin_lock_init(&sdata->mib_lock);
 	mutex_init(&sdata->sec_mtx);
 
@@ -454,6 +452,4 @@ void mac802154_monitor_setup(struct net_device *dev)
 
 	sdata = IEEE802154_DEV_TO_SUB_IF(dev);
 	sdata->type = IEEE802154_DEV_MONITOR;
-
-	sdata->chan = MAC802154_CHAN_NONE; /* not initialized */
 }
net/mac802154/mib.c +9 -46

74 lines not on this page · raw message

Signature · 1 line
2.1.2
#27 lore raw

Re: [PATCH bluetooth-next 14/17] mac802154: remove channel attributes from sdata

Quoted 108 lines · Alexander Aring

These channel attributes was part of "channel context switch while xmit" which was removed by commit dc67c6b30f36d57b70b70547a30e7a8432540c6f ("mac802154: tx: remove xmit channel context switch"). This patch removes these unnecessary variables and use the current_page and current_channel by wpan_phy struct now.

---
  net/mac802154/ieee802154_i.h |  3 ---
  net/mac802154/iface.c        |  4 ----
  net/mac802154/mib.c          | 55 ++++++++------------------------------------
  3 files changed, 9 insertions(+), 53 deletions(-)
net/mac802154/ieee802154_i.h +0 -3
index bf0b5f6..46c9fe7 100644
@@ -85,9 +85,6 @@ struct ieee802154_sub_if_data {
  	__le16 short_addr;
  	__le64 extended_addr;
 
-	u8 chan;
-	u8 page;
-
  	struct ieee802154_mac_params mac_params;
 
  	/* MAC BSN field */
net/mac802154/iface.c +0 -4
index 2423aa7..1c0274e 100644
@@ -417,8 +417,6 @@ void mac802154_wpan_setup(struct net_device *dev)
  	sdata = IEEE802154_DEV_TO_SUB_IF(dev);
  	sdata->type = IEEE802154_DEV_WPAN;
 
-	sdata->chan = MAC802154_CHAN_NONE;
-
  	spin_lock_init(&sdata->mib_lock);
  	mutex_init(&sdata->sec_mtx);
 
@@ -454,6 +452,4 @@ void mac802154_monitor_setup(struct net_device *dev)
 
  	sdata = IEEE802154_DEV_TO_SUB_IF(dev);
  	sdata->type = IEEE802154_DEV_MONITOR;
-
-	sdata->chan = MAC802154_CHAN_NONE; /* not initialized */
  }
net/mac802154/mib.c +2 -38
index 3fbc217..0184fce 100644
@@ -26,11 +26,6 @@
  #include "ieee802154_i.h"
  #include "driver-ops.h"
 
-struct phy_chan_notify_work {
-	struct work_struct work;
-	struct net_device *dev;
-};
-
  struct hw_addr_filt_notify_work {
  	struct work_struct work;
  	struct net_device *dev;
@@ -161,54 +156,22 @@ u8 mac802154_dev_get_dsn(const struct net_device *dev)
  	return sdata->dsn++;
  }
 
-static void phy_chan_notify(struct work_struct *work)
-{
-	struct phy_chan_notify_work *nw = container_of(work,
-					  struct phy_chan_notify_work, work);
-	struct net_device *dev = nw->dev;
-	struct ieee802154_local *local = mac802154_slave_get_priv(dev);
-	struct ieee802154_sub_if_data *sdata = IEEE802154_DEV_TO_SUB_IF(dev);
-	int res;
-
-	mutex_lock(&sdata->local->phy->pib_lock);
-	res = drv_set_channel(local, sdata->page, sdata->chan);
-	if (res) {
-		pr_debug("set_channel failed\n");
-	} else {
-		sdata->local->phy->current_channel = sdata->chan;
-		sdata->local->phy->current_page = sdata->page;
-	}
-	mutex_unlock(&sdata->local->phy->pib_lock);
-
-	kfree(nw);
-}
-
  void mac802154_dev_set_page_channel(struct net_device *dev, u8 page, u8 chan)
  {
  	struct ieee802154_sub_if_data *sdata = IEEE802154_DEV_TO_SUB_IF(dev);
-	struct phy_chan_notify_work *work;
+	struct ieee802154_local *local = sdata->local;
+	int res;
 
  	BUG_ON(dev->type != ARPHRD_IEEE802154);
 
-	spin_lock_bh(&sdata->mib_lock);
-	sdata->page = page;
-	sdata->chan = chan;
-	spin_unlock_bh(&sdata->mib_lock);
-
-	mutex_lock(&sdata->local->phy->pib_lock);
-	if (sdata->local->phy->current_channel != sdata->chan ||
-	    sdata->local->phy->current_page != sdata->page) {
-		mutex_unlock(&sdata->local->phy->pib_lock);
-

net/mac802154/mib.c u8 mac802154_dev_get_dsn(const struct net_device *dev) · line 169

Quoted 10 lines
net/mac802154/mib.c +3 -7
-		work = kzalloc(sizeof(*work), GFP_ATOMIC);
-		if (!work)
-			return;
-
-		INIT_WORK(&work->work, phy_chan_notify);
-		work->dev = dev;
-		queue_work(sdata->local->workqueue, &work->work);
+	res = drv_set_channel(local, page, chan);
+	if (res) {
+		pr_debug("set_channel failed\n");

Why don't we use netdev_dbg() here...?

Quoted 9 lines
net/mac802154/mib.c +4 -1
  	} else {
-		mutex_unlock(&sdata->local->phy->pib_lock);
+		mutex_lock(&local->phy->pib_lock);
+		local->phy->current_channel = chan;
+		local->phy->current_page = page;
+		mutex_unlock(&local->phy->pib_lock);
  	}
  }
 
Signature · 2 lines
Regards,
Varka Bhadram.
author maintainer
#28 lore raw

Re: [PATCH bluetooth-next 14/17] mac802154: remove channel attributes from sdata

Hi Varka,

Quoted 1 line · Varka Bhadram

On 10/28/2014 10:51 PM, Alexander Aring wrote:

...

Quoted 7 lines
Quoted 7 lines
net/mac802154/mib.c +0 -7
-	    sdata->local->phy->current_page != sdata->page) {
-		mutex_unlock(&sdata->local->phy->pib_lock);
-
-		work = kzalloc(sizeof(*work), GFP_ATOMIC);
-		if (!work)
-			return;
-
Quoted 10 lines

net/mac802154/mib.c u8 mac802154_dev_get_dsn(const struct net_device *dev) · line 169

Quoted 6 lines
net/mac802154/mib.c +3 -3
-		INIT_WORK(&work->work, phy_chan_notify);
-		work->dev = dev;
-		queue_work(sdata->local->workqueue, &work->work);
+	res = drv_set_channel(local, page, chan);
+	if (res) {
+		pr_debug("set_channel failed\n");

Why don't we use netdev_dbg() here...?

Quoted 1 line

} else {

because there comming more patches and I am not at the finally cleanup. This code will be part of the deprecated netlink interface. I don't want to fix anything here. It should only work for the things which we support in the official userspace software and that is. Channel setting, interface add/del and setting address filter.

I don't want to send a patch series which contains more than 20 patches.

This patch I could also add an another patch for removing the worker here and I really don't have an idea why we have a worker here. This channel setting patch is more a rework. In further all phy settings are directly calls to the driver. MAC settings are stored at each interface and setted by a open/close or rather ifup and ifdown.

There will come a new netlink interface which use the framework like wireless. The old one will also be functional but we will remove it some or later.

Quoted 8 lines

net/mac802154/mib.c u8 mac802154_dev_get_dsn(const struct net_device *dev) · line 176

Quoted 7 lines
net/mac802154/mib.c +4 -1
-		mutex_unlock(&sdata->local->phy->pib_lock);
+		mutex_lock(&local->phy->pib_lock);
+		local->phy->current_channel = chan;
+		local->phy->current_page = page;
+		mutex_unlock(&local->phy->pib_lock);
 	}
 }

- Alex

#29 lore raw

Re: [PATCH bluetooth-next 14/17] mac802154: remove channel attributes from sdata

Quoted 22 lines · Alexander Aring
Quoted 6 lines
Quoted 6 lines
net/mac802154/mib.c +0 -6
-	    sdata->local->phy->current_page != sdata->page) {
-		mutex_unlock(&sdata->local->phy->pib_lock);
-
-		work = kzalloc(sizeof(*work), GFP_ATOMIC);
-		if (!work)
-			return;
Quoted 10 lines

net/mac802154/mib.c u8 mac802154_dev_get_dsn(const struct net_device *dev) · line 169

Quoted 7 lines
net/mac802154/mib.c +3 -4
-
-		INIT_WORK(&work->work, phy_chan_notify);
-		work->dev = dev;
-		queue_work(sdata->local->workqueue, &work->work);
+	res = drv_set_channel(local, page, chan);
+	if (res) {
+		pr_debug("set_channel failed\n");

Why don't we use netdev_dbg() here...?

Quoted 1 line
  	} else {

because there comming more patches and I am not at the finally cleanup. This code will be part of the deprecated netlink interface. I don't want to fix anything here. It should only work for the things which we support in the official userspace software and that is. Channel setting, interface add/del and setting address filter.

Quoted 7 lines

I don't want to send a patch series which contains more than 20 patches.

This patch I could also add an another patch for removing the worker here and I really don't have an idea why we have a worker here. This channel setting patch is more a rework. In further all phy settings are directly calls to the driver. MAC settings are stored at each interface and setted by a open/close or rather ifup and ifdown.

I think when we do ifup the open function will be called..?

Quoted 3 lines

There will come a new netlink interface which use the framework like wireless. The old one will also be functional but we will remove it some or later.

What is the New netlink interface..?

I would like to know.

Signature · 2 lines
Thanks and Regards,
Varka Bhadram.
author maintainer
#30 lore raw

Re: [PATCH bluetooth-next 14/17] mac802154: remove channel attributes from sdata

Quoted 25 lines · Varka Bhadram
Quoted 24 lines · Alexander Aring
Quoted 6 lines
Quoted 6 lines
net/mac802154/mib.c +0 -6
-	    sdata->local->phy->current_page != sdata->page) {
-		mutex_unlock(&sdata->local->phy->pib_lock);
-
-		work = kzalloc(sizeof(*work), GFP_ATOMIC);
-		if (!work)
-			return;
Quoted 10 lines

net/mac802154/mib.c u8 mac802154_dev_get_dsn(const struct net_device *dev) · line 169

Quoted 7 lines
net/mac802154/mib.c +3 -4
-
-		INIT_WORK(&work->work, phy_chan_notify);
-		work->dev = dev;
-		queue_work(sdata->local->workqueue, &work->work);
+	res = drv_set_channel(local, page, chan);
+	if (res) {
+		pr_debug("set_channel failed\n");

Why don't we use netdev_dbg() here...?

Quoted 1 line

} else {

because there comming more patches and I am not at the finally cleanup. This code will be part of the deprecated netlink interface. I don't want to fix anything here. It should only work for the things which we support in the official userspace software and that is. Channel setting, interface add/del and setting address filter.

I don't want to send a patch series which contains more than 20 patches.

Quoted 8 lines
Quoted 5 lines

This patch I could also add an another patch for removing the worker here and I really don't have an idea why we have a worker here. This channel setting patch is more a rework. In further all phy settings are directly calls to the driver. MAC settings are stored at each interface and setted by a open/close or rather ifup and ifdown.

I think when we do ifup the open function will be called..?

Yes, we have these multiple interfaces and these "can" have different MAC settings. When I say MAC here then it's something which needs an algorithmn. PHY settings doesn't need to run a algorithmn, like channel page settings.

Later if we do a ifup we should set all MAC settings which is handled by PHY. It's handled by phy because it's very time critical like ARET or CSMA. Also address filtering is a MAC settings which is handled by phy.

In the next patches we also remove the pib/mib lock and make locking via netif_running. That means we can't set MAC which are handled by PHY while netif is running. This is usual principle in netdev, like you can't change ethernet address while netif is running. Now when netif is running then the rx/tx paths are called and these variables becomes read only. We simple don't need any locking mechanism in hotpaths then.

If somebody want to change addresses short/extended/pan_id, ARET, CSMA. He need to do a ifdown first and then the user can change it.

On ifup all interfaces should be have the same MAC settings which are handled by PHY. I need to see if we really need the multiple interface thing or just implement a netlink call to change the interface type.

Only one use case for me is that we can have more than one monitor interface at the same time.

Quoted 7 lines
Quoted 3 lines

There will come a new netlink interface which use the framework like wireless. The old one will also be functional but we will remove it some or later.

What is the New netlink interface..?

I would like to know.

The current netlink interface is the most weird code in the current implementation and the official userspace software doesn't support all netlink commands. I tried to implement it but I gave up. The new one should be look like:

https://github.com/linux-wpan/linux-wpan-next/blob/wpan_rework_rfc/net/ieee802154/nl802154.c

mostly grab the framework from wireless.

Userspace tools are:

https://github.com/linux-wpan/wpan-tools

mostly grab the framework from iw wireless tools.

- Alex

author maintainer
#31 lore raw

PATCH 15/17 mac802154: move mac_params functions into mac_cmd

In reply to Alexander Aring

These functions can be static in mac_cmd file.

---
 net/mac802154/ieee802154_i.h |  5 -----
 net/mac802154/mac_cmd.c      | 22 ++++++++++++++++++++++
 2 files changed, 22 insertions(+), 5 deletions(-)
net/mac802154/ieee802154_i.h +0 -5
index 46c9fe7..7cebc98 100644
@@ -140,11 +140,6 @@ void mac802154_dev_set_pan_id(struct net_device *dev, __le16 val);
 void mac802154_dev_set_page_channel(struct net_device *dev, u8 page, u8 chan);
 u8 mac802154_dev_get_dsn(const struct net_device *dev);
 
-int mac802154_set_mac_params(struct net_device *dev,
-			     const struct ieee802154_mac_params *params);
-void mac802154_get_mac_params(struct net_device *dev,
-			      struct ieee802154_mac_params *params);
-
 int mac802154_get_params(struct net_device *dev,
 			 struct ieee802154_llsec_params *params);
 int mac802154_set_params(struct net_device *dev,
net/mac802154/mac_cmd.c +22 -0
index ed767f5..fc261ab 100644
@@ -82,6 +82,28 @@ static struct wpan_phy *mac802154_get_phy(const struct net_device *dev)
 	return to_phy(get_device(&sdata->local->phy->dev));
 }
 
+static int mac802154_set_mac_params(struct net_device *dev,
+				    const struct ieee802154_mac_params *params)
+{
+	struct ieee802154_sub_if_data *sdata = IEEE802154_DEV_TO_SUB_IF(dev);
+
+	mutex_lock(&sdata->local->iflist_mtx);
+	sdata->mac_params = *params;
+	mutex_unlock(&sdata->local->iflist_mtx);
+
+	return 0;
+}
+
+static void mac802154_get_mac_params(struct net_device *dev,
+				     struct ieee802154_mac_params *params)
+{
+	struct ieee802154_sub_if_data *sdata = IEEE802154_DEV_TO_SUB_IF(dev);
+
+	mutex_lock(&sdata->local->iflist_mtx);
+	*params = sdata->mac_params;
+	mutex_unlock(&sdata->local->iflist_mtx);
+}
+
 static struct ieee802154_llsec_ops mac802154_llsec_ops = {
 	.get_params = mac802154_get_params,
 	.set_params = mac802154_set_params,
Signature · 1 line
2.1.2
author maintainer
#32 lore raw

PATCH 16/17 mac802154: cleanup open count handling

In reply to Alexander Aring

This patch cleanups the open_count variable increment in open and close calls of netdev.

---
 net/mac802154/iface.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
net/mac802154/iface.c +4 -3
index 1c0274e..c0bf5f9 100644
@@ -166,19 +166,19 @@ static int mac802154_slave_open(struct net_device *dev)
 
 	set_bit(SDATA_STATE_RUNNING, &sdata->state);
 
-	if (local->open_count++ == 0) {
+	if (!local->open_count) {
 		res = drv_start(local);
 		WARN_ON(res);
 		if (res)
 			goto err;
 	}
 
+	local->open_count++;
 	netif_start_queue(dev);
 	return 0;
 err:
 	/* might already be clear but that doesn't matter */
 	clear_bit(SDATA_STATE_RUNNING, &sdata->state);
-	sdata->local->open_count--;
 
 	return res;
 }
@@ -252,10 +252,11 @@ static int mac802154_slave_close(struct net_device *dev)
 	ASSERT_RTNL();
 
 	netif_stop_queue(dev);
+	local->open_count--;
 
 	clear_bit(SDATA_STATE_RUNNING, &sdata->state);
 
-	if (!--local->open_count)
+	if (!local->open_count)
 		drv_stop(local);
 
 	return 0;
Signature · 1 line
2.1.2
author maintainer
#33 lore raw

PATCH 17/17 ieee802154: introduce sysfs file

In reply to Alexander Aring

This patch moves the sysfs handling in a own file. This is like wireless sysfs file handling.

---
 net/ieee802154/Makefile |  2 +-
 net/ieee802154/core.c   | 73 +++-----------------------------------
 net/ieee802154/sysfs.c  | 94 +++++++++++++++++++++++++++++++++++++++++++++++++
 net/ieee802154/sysfs.h  |  9 +++++
 4 files changed, 108 insertions(+), 70 deletions(-)
 create mode 100644 net/ieee802154/sysfs.c
 create mode 100644 net/ieee802154/sysfs.h
net/ieee802154/Makefile +1 -1
index e58c4cb..38354d4 100644
@@ -3,7 +3,7 @@ obj-$(CONFIG_IEEE802154_6LOWPAN) += ieee802154_6lowpan.o
 
 ieee802154_6lowpan-y := 6lowpan_rtnl.o reassembly.o
 ieee802154-y := netlink.o nl-mac.o nl-phy.o nl_policy.o core.o \
-                header_ops.o
+                header_ops.o sysfs.o
 af_802154-y := af_ieee802154.o raw.o dgram.o
 
 ccflags-y += -D__CHECK_ENDIAN__
net/ieee802154/core.c +4 -69

97 lines not on this page · raw message

net/ieee802154/sysfs.c new file +94 -0

94 lines not on this page · raw message

net/ieee802154/sysfs.h new file +9 -0
new file mode 100644
index 0000000..aa42e39
@@ -0,0 +1,9 @@
+#ifndef __IEEE802154_SYSFS_H
+#define __IEEE802154_SYSFS_H
+
+int wpan_phy_sysfs_init(void);
+void wpan_phy_sysfs_exit(void);
+
+extern struct class wpan_phy_class;
+
+#endif /* __IEEE802154_SYSFS_H */
Signature · 1 line
2.1.2
#34 lore raw
In reply to Alexander Aring

Hi Alex,

Quoted 44 lines

This patch series contains cleanups for iface handling driver-ops and sysfs handling. Also removing dead code of current behaviour of local->running and use it as local->started behaviour like mac8011.

Alexander Aring (17):
mac802154: monitor: merge into iface implementation
mac802154: main: move open and close into iface
mac802154: declare struct ieee802154_ops as const
mac802154: ops: declare channel and page as u8
mac802154: introduce driver-ops header
mac802154: use driver-ops function wrappers
mac802154: remove might_sleep from driver layer
mac802154: remove driver ops in wpan-phy
mac802154: rework sdata state change to running
mac802154: rename running to started
mac802154: move local started handling
mac802154: add synchronization handling
mac802154: iface: remove assign to zero
mac802154: remove channel attributes from sdata
mac802154: move mac_params functions into mac_cmd
mac802154: cleanup open count handling
ieee802154: introduce sysfs file

drivers/net/ieee802154/at86rf230.c |  13 +--
drivers/net/ieee802154/cc2520.c    |   5 +-
drivers/net/ieee802154/fakelb.c    |   8 +-
drivers/net/ieee802154/mrf24j40.c  |   5 +-
include/net/cfg802154.h            |   8 --
include/net/mac802154.h            |   7 +-
net/ieee802154/Makefile            |   2 +-
net/ieee802154/core.c              |  73 +------------
net/ieee802154/nl-mac.c            |  19 +---
net/ieee802154/sysfs.c             |  94 ++++++++++++++++
net/ieee802154/sysfs.h             |   9 ++
net/mac802154/Makefile             |   2 +-
net/mac802154/driver-ops.h         | 213 +++++++++++++++++++++++++++++++++++++
net/mac802154/ieee802154_i.h       |  34 +++---
net/mac802154/iface.c              | 119 +++++++++++++++++----
net/mac802154/mac_cmd.c            |  22 ++++
net/mac802154/main.c               | 166 +----------------------------
net/mac802154/mib.c                |  56 ++--------
net/mac802154/monitor.c            |  59 ----------
net/mac802154/tx.c                 |   5 +-
20 files changed, 488 insertions(+), 431 deletions(-)
Quoted 4 lines

create mode 100644 net/ieee802154/sysfs.c
create mode 100644 net/ieee802154/sysfs.h
create mode 100644 net/mac802154/driver-ops.h
delete mode 100644 net/mac802154/monitor.c

all 17 patches have been applied to bluetooth-next tree.

Regards

Marcel