[PATCH bluetooth-next 1/3] mac802154: cleanup concurrent check

[PATCH bluetooth-next 1/3] mac802154: cleanup concurrent check

[Alexander Aring]

This patch cleanups the checking of different mac phy depended values by
handling depended mac settings per hw support flag in one condition.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 net/mac802154/iface.c | 20 ++++++--------------
 1 file changed, 6 insertions(+), 14 deletions(-)

diff --git a/net/mac802154/iface.c b/net/mac802154/iface.c
index 9b8e264..3f6a3fe 100644
--- a/net/mac802154/iface.c
+++ b/net/mac802154/iface.c
@@ -174,24 +174,16 @@ ieee802154_check_mac_settings(struct ieee802154_local *local,
 	}
 
 	if (local->hw.flags & IEEE802154_HW_AFILT) {
-		if (wpan_dev->pan_id != nwpan_dev->pan_id)
-			return -EBUSY;
-
-		if (wpan_dev->short_addr != nwpan_dev->short_addr)
-			return -EBUSY;
-
-		if (wpan_dev->extended_addr != nwpan_dev->extended_addr)
+		if (wpan_dev->pan_id != nwpan_dev->pan_id ||
+		    wpan_dev->short_addr != nwpan_dev->short_addr ||
+		    wpan_dev->extended_addr != nwpan_dev->extended_addr)
 			return -EBUSY;
 	}
 
 	if (local->hw.flags & IEEE802154_HW_CSMA_PARAMS) {
-		if (wpan_dev->min_be != nwpan_dev->min_be)
-			return -EBUSY;
-
-		if (wpan_dev->max_be != nwpan_dev->max_be)
-			return -EBUSY;
-
-		if (wpan_dev->csma_retries != nwpan_dev->csma_retries)
+		if (wpan_dev->min_be != nwpan_dev->min_be ||
+		    wpan_dev->max_be != nwpan_dev->max_be ||
+		    wpan_dev->csma_retries != nwpan_dev->csma_retries)
 			return -EBUSY;
 	}
 
-- 
2.3.4

[PATCH bluetooth-next 2/3] at86rf230: remove unnecessary spinlock

[Alexander Aring]

This spinlock isn't necessary because if we are in TX_ON/TX_ARET_ON
state the transceiver can't be interrupted e.g. by receiving a frame
when a SHR was detected. In this time the transceiver doesn't leave
the TX_ON/TX_ARET_ON state until the tx complete irq change the state
into RX_AACK_ON again. This means a receiving interrupt in state
TX_ON/TX_ARET_ON can't happen and is_tx is protected by transceiver.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 drivers/net/ieee802154/at86rf230.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/net/ieee802154/at86rf230.c b/drivers/net/ieee802154/at86rf230.c
index cc5efa1..1278dd5 100644
--- a/drivers/net/ieee802154/at86rf230.c
+++ b/drivers/net/ieee802154/at86rf230.c
@@ -25,7 +25,6 @@
 #include <linux/irq.h>
 #include <linux/gpio.h>
 #include <linux/delay.h>
-#include <linux/spinlock.h>
 #include <linux/spi/spi.h>
 #include <linux/spi/at86rf230.h>
 #include <linux/regmap.h>
@@ -96,8 +95,6 @@ struct at86rf230_local {
 	unsigned long cal_timeout;
 	s8 max_frame_retries;
 	bool is_tx;
-	/* spinlock for is_tx protection */
-	spinlock_t lock;
 	u8 tx_retry;
 	struct sk_buff *tx_skb;
 	struct at86rf230_state_change tx;
@@ -878,10 +875,8 @@ at86rf230_rx_trac_check(void *context)
 static void
 at86rf230_irq_trx_end(struct at86rf230_local *lp)
 {
-	spin_lock(&lp->lock);
 	if (lp->is_tx) {
 		lp->is_tx = 0;
-		spin_unlock(&lp->lock);
 
 		if (lp->tx_aret)
 			at86rf230_async_state_change(lp, &lp->irq,
@@ -894,7 +889,6 @@ at86rf230_irq_trx_end(struct at86rf230_local *lp)
 						     at86rf230_tx_complete,
 						     true);
 	} else {
-		spin_unlock(&lp->lock);
 		at86rf230_async_read_reg(lp, RG_TRX_STATE, &lp->irq,
 					 at86rf230_rx_trac_check, true);
 	}
@@ -964,9 +958,7 @@ at86rf230_write_frame(void *context)
 	u8 *buf = ctx->buf;
 	int rc;
 
-	spin_lock(&lp->lock);
 	lp->is_tx = 1;
-	spin_unlock(&lp->lock);
 
 	buf[0] = CMD_FB | CMD_WRITE;
 	buf[1] = skb->len + 2;
@@ -1698,7 +1690,6 @@ static int at86rf230_probe(struct spi_device *spi)
 	if (rc < 0)
 		goto free_dev;
 
-	spin_lock_init(&lp->lock);
 	init_completion(&lp->state_complete);
 
 	spi_set_drvdata(spi, lp);
-- 
2.3.4

Re: [PATCH bluetooth-next 2/3] at86rf230: remove unnecessary spinlock

[Marcel Holtmann]

Hi Alex,

> This spinlock isn't necessary because if we are in TX_ON/TX_ARET_ON
> state the transceiver can't be interrupted e.g. by receiving a frame
> when a SHR was detected. In this time the transceiver doesn't leave
> the TX_ON/TX_ARET_ON state until the tx complete irq change the state
> into RX_AACK_ON again. This means a receiving interrupt in state
> TX_ON/TX_ARET_ON can't happen and is_tx is protected by transceiver.
> 
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> ---
> drivers/net/ieee802154/at86rf230.c | 9 ---------
> 1 file changed, 9 deletions(-)

patch has been applied to bluetooth-next tree.

Regards

Marcel

[PATCH bluetooth-next 3/3] at86rf230: fix is_tx while error handling

[Alexander Aring]

This patch fix the error handling when is_tx is true. The error handling
tries to get the transceiver into RX_AACK_ON mode then we need to be
sure that is_tx is false.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 drivers/net/ieee802154/at86rf230.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ieee802154/at86rf230.c b/drivers/net/ieee802154/at86rf230.c
index 1278dd5..5ad46f7 100644
--- a/drivers/net/ieee802154/at86rf230.c
+++ b/drivers/net/ieee802154/at86rf230.c
@@ -457,6 +457,7 @@ at86rf230_async_error_recover(void *context)
 	struct at86rf230_state_change *ctx = context;
 	struct at86rf230_local *lp = ctx->lp;
 
+	lp->is_tx = 0;
 	at86rf230_async_state_change(lp, ctx, STATE_RX_AACK_ON, NULL, false);
 	ieee802154_wake_queue(lp->hw);
 }
-- 
2.3.4

Re: [PATCH bluetooth-next 3/3] at86rf230: fix is_tx while error handling

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 593 bytes --]

On 03/26/2015 12:46 PM, Alexander Aring wrote:
> This patch fix the error handling when is_tx is true. The error handling
> tries to get the transceiver into RX_AACK_ON mode then we need to be
> sure that is_tx is false.

Generally speaking, first fix errors, then clean up and improve code.

regards,
Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH bluetooth-next 3/3] at86rf230: fix is_tx while error handling

[Alexander Aring]

On Thu, Mar 26, 2015 at 12:54:51PM +0100, Marc Kleine-Budde wrote:
> On 03/26/2015 12:46 PM, Alexander Aring wrote:
> > This patch fix the error handling when is_tx is true. The error handling
> > tries to get the transceiver into RX_AACK_ON mode then we need to be
> > sure that is_tx is false.
> 
> Generally speaking, first fix errors, then clean up and improve code.
> 

should I resend again with a new sequence of this patch series?

I know it's some mixed things which I detect while hacking the nl802154
crypto foo. But I will remember Marc's words before sending next series.
:-)

btw:
I think if an spi_async error occurs then something goes really wrong
and a rescue by doing a TRX_OFF -> RX_AACK_ON doesn't help also anymore.

We need then something like rf_kill framework.

- Alex

Re: [PATCH bluetooth-next 3/3] at86rf230: fix is_tx while error handling

[Marcel Holtmann]

Hi Alex,

>>> This patch fix the error handling when is_tx is true. The error handling
>>> tries to get the transceiver into RX_AACK_ON mode then we need to be
>>> sure that is_tx is false.
>> 
>> Generally speaking, first fix errors, then clean up and improve code.
>> 
> 
> should I resend again with a new sequence of this patch series?
> 
> I know it's some mixed things which I detect while hacking the nl802154
> crypto foo. But I will remember Marc's words before sending next series.
> :-)

I am fine either way. I can apply the patches as you sent them.

Regards

Marcel

Re: [PATCH bluetooth-next 3/3] at86rf230: fix is_tx while error handling

[Marcel Holtmann]

Hi Alex,

> This patch fix the error handling when is_tx is true. The error handling
> tries to get the transceiver into RX_AACK_ON mode then we need to be
> sure that is_tx is false.
> 
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> ---
> drivers/net/ieee802154/at86rf230.c | 1 +
> 1 file changed, 1 insertion(+)

patch has been applied to bluetooth-next tree.

Regards

Marcel

Re: [PATCH bluetooth-next 1/3] mac802154: cleanup concurrent check

[Marcel Holtmann]

Hi Alex,

> This patch cleanups the checking of different mac phy depended values by
> handling depended mac settings per hw support flag in one condition.
> 
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> ---
> net/mac802154/iface.c | 20 ++++++--------------
> 1 file changed, 6 insertions(+), 14 deletions(-)

patch has been applied to bluetooth-next tree.

Regards

Marcel