at86rf230: fix phy settings while sleeping

This patch adds support for a new random csma backoffs settings when going into sleep state. This is recommended according at86rf2xx datasheets.

Signed-off-by Alexander Aring <[email protected]>

---
drivers/net/ieee802154/at86rf230.c | 11 +++++++++++

1 file changed, 11 insertions(+)

This code cleanups the start and stop callbacks by removing hw->priv and using the already dereferenced variable lp which is the same.

Signed-off-by Alexander Aring <[email protected]>

---
drivers/net/ieee802154/at86rf230.c | 4 ++--

1 file changed, 2 insertions(+), 2 deletions(-)

While in sleep state then we can't access the at86rf2xx registers. This patch checks if the transceiver is in sleep state before sending spi messages via regmap. Regmap is used on every driver ops callback except for receive and xmit handling, but while receive and xmit handling the phy should not be inside the sleep state.

Signed-off-by Alexander Aring <[email protected]>

---
change since v2:
- I detect an issue with cca ed level setting in v1, there was a return in the middle of awake and sleep transceiver. This results that the transceiver wasn't again turn into sleep state afterwards.
To avoid such behaviour I moved the awake and sleep into lowlevel register io functions. These functions "could" be called only when the phy is in sleep state.

 drivers/net/ieee802154/at86rf230.c | 88 +++++++++++++++++++++++++++++---------
 1 file changed, 68 insertions(+), 20 deletions(-)

drivers/net/ieee802154/at86rf230.c +0 -0

index 6b31f47..b839bbd 100644

drivers/net/ieee802154/at86rf230.c +67 -20

 	struct completion state_complete;
 	struct at86rf230_state_change state;

@@ -114,18 +115,66 @@ at86rf230_async_state_change(struct at86rf230_local *lp,
 			     const u8 state, void (*complete)(void *context),
 			     const bool irq_enable);
 
+static inline void
+at86rf230_sleep(struct at86rf230_local *lp)
+{
+	if (gpio_is_valid(lp->slp_tr)) {
+		gpio_set_value(lp->slp_tr, 1);
+		usleep_range(lp->data->t_off_to_sleep,
+			     lp->data->t_off_to_sleep + 10);
+		lp->sleep = true;
+	}
+}
+
+static inline void
+at86rf230_awake(struct at86rf230_local *lp)
+{
+	if (gpio_is_valid(lp->slp_tr)) {
+		gpio_set_value(lp->slp_tr, 0);
+		usleep_range(lp->data->t_sleep_to_off,
+			     lp->data->t_sleep_to_off + 100);
+		lp->sleep = false;
+	}
+}
+
 static inline int
 __at86rf230_write(struct at86rf230_local *lp,
 		  unsigned int addr, unsigned int data)
 {
-	return regmap_write(lp->regmap, addr, data);
+	bool sleep = lp->sleep;
+	int ret;
+
+	/* awake for register setting if sleep */
+	if (sleep)
+		at86rf230_awake(lp);
+
+	ret = regmap_write(lp->regmap, addr, data);
+
+	/* sleep again if was sleeping */
+	if (sleep)
+		at86rf230_sleep(lp);
+
+	return ret;
 }
 
 static inline int
 __at86rf230_read(struct at86rf230_local *lp,
 		 unsigned int addr, unsigned int *data)
 {
-	return regmap_read(lp->regmap, addr, data);
+	bool sleep = lp->sleep;
+	int ret;
+
+	/* awake for register setting if sleep */
+	if (sleep)
+		at86rf230_awake(lp);
+
+	ret = regmap_read(lp->regmap, addr, data);
+
+	/* sleep again if was sleeping */
+	if (sleep)
+		at86rf230_sleep(lp);
+
+	return ret;
 }
 
 static inline int

@@ -147,7 +196,20 @@ at86rf230_write_subreg(struct at86rf230_local *lp,
 		       unsigned int addr, unsigned int mask,
 		       unsigned int shift, unsigned int data)
 {
-	return regmap_update_bits(lp->regmap, addr, mask, data << shift);
+	bool sleep = lp->sleep;
+	int ret;
+
+	/* awake for register setting if sleep */
+	if (sleep)
+		at86rf230_awake(lp);
+
+	ret = regmap_update_bits(lp->regmap, addr, mask, data << shift);
+
+	/* sleep again if was sleeping */
+	if (sleep)
+		at86rf230_sleep(lp);
+
+	return ret;
 }
 
 static inline void

@@ -873,12 +935,7 @@ at86rf230_start(struct ieee802154_hw *hw)
 {
 	struct at86rf230_local *lp = hw->priv;
 
-	if (gpio_is_valid(lp->slp_tr)) {
-		gpio_set_value(lp->slp_tr, 0);
-		usleep_range(lp->data->t_sleep_to_off,
-			     lp->data->t_sleep_to_off + 100);
-	}
-
+	at86rf230_awake(lp);
 	enable_irq(lp->spi->irq);
 
 	return at86rf230_sync_state_change(hw->priv, STATE_RX_AACK_ON);

@@ -892,12 +949,7 @@ at86rf230_stop(struct ieee802154_hw *hw)
 	at86rf230_sync_state_change(hw->priv, STATE_FORCE_TRX_OFF);
 
 	disable_irq(lp->spi->irq);
-
-	if (gpio_is_valid(lp->slp_tr)) {
-		gpio_set_value(lp->slp_tr, 1);
-		usleep_range(lp->data->t_off_to_sleep,
-			     lp->data->t_off_to_sleep + 10);
-	}
+	at86rf230_sleep(lp);
 }
 
 static int

@@ -1672,11 +1724,7 @@ static int at86rf230_probe(struct spi_device *spi)
 	disable_irq(spi->irq);
 
 	/* going into sleep by default */
-	if (gpio_is_valid(slp_tr)) {
-		gpio_set_value(slp_tr, 1);
-		usleep_range(lp->data->t_off_to_sleep,
-			     lp->data->t_off_to_sleep + 10);
-	}
+	at86rf230_sleep(lp);
 
 	rc = ieee802154_register_hw(lp->hw);
 	if (rc)

Am Dienstag, den 16.06.2015, 11:07 +0200 schrieb Alexander Aring:

Quoted 20 lines

While in sleep state then we can't access the at86rf2xx registers. This patch checks if the transceiver is in sleep state before sending spi messages via regmap. Regmap is used on every driver ops callback except for receive and xmit handling, but while receive and xmit handling the phy should not be inside the sleep state.

Signed-off-by Alexander Aring <[email protected]>

---
change since v2:
- I detect an issue with cca ed level setting in v1, there was a return in the middle of awake and sleep transceiver. This results that the transceiver wasn't again turn into sleep state afterwards.
To avoid such behaviour I moved the awake and sleep into lowlevel register io functions. These functions "could" be called only when the phy is in sleep state.

 drivers/net/ieee802154/at86rf230.c | 88 +++++++++++++++++++++++++++++---------
 1 file changed, 68 insertions(+), 20 deletions(-)

drivers/net/ieee802154/at86rf230.c struct at86rf230_local { · line 93

Quoted 9 lines

drivers/net/ieee802154/at86rf230.c +1 -0

index 6b31f47..b839bbd 100644
@@ -90,6 +90,7 @@ struct at86rf230_local {
 	struct at86rf2xx_chip_data *data;
 	struct regmap *regmap;
 	int slp_tr;
+	bool sleep;

This should probably be a reference count instead of a simple bool. I suppose there are more locations in the driver where you want to keep the PHY awake.

Also this would allow you to throw away all those "if (sleep)" checks at the callers. Just call at86rf230_phy_wake_get() before you do something that needs the phy to be awake and increment the refcount there and have a similar ..._put() function to decrement the refcount. Then toggle the GPIO when the refcount is going 0->1 and the other way around.

I agree this is the common kernel soltution. I think is what we need is to put this logic then into 802.15.4 subsystem.

In the wpan_phy we put a refcounter and let them increment when the phy is "used by susbsytem" -> if was 0 then awake and decrement "when it's not used" -> if reach 0 then subsystem.

Currently all ops (except the xmit op, but this should not called when sleeping) are protected by rtnl mutex so the refcount can't be greater than one.

I will put it to my ToDo List, then we have some driver_ops resume/suspsend when the susbsystem will use the wpan_phy when it's sleeping and the driver needs to do something.

Nevertheless this patch looks for _now_ valid and introducing a refcount in 802.15.4 for the wpan_phy is better. At the moment I broke the behaviour when introduced the sleep state some days ago.

At Wednesday Marcel will send the last push request to net-next and I need to decide now if we do this fix OR reverting the introducing of sleep state.

I would leave this patch and when the times comes we introduce a better 802.15.4 wpan behaviour with refcounts later. The logic is just completely in driver layer right now.

While in sleep state then we can't access the at86rf2xx registers. This patch checks if the transceiver is in sleep state before sending spi messages via regmap. Regmap is used on every driver ops callback except for receive and xmit handling, but while receive and xmit handling the phy should not be inside the sleep state.

Signed-off-by Alexander Aring <[email protected]>

---
change since v2:
- I detect an issue with cca ed level setting in v1, there was a return in the middle of awake and sleep transceiver. This results that the transceiver wasn't again turn into sleep state afterwards.
To avoid such behaviour I moved the awake and sleep into lowlevel register io functions. These functions "could" be called only when the phy is in sleep state.

drivers/net/ieee802154/at86rf230.c | 88 +++++++++++++++++++++++++++++---------

1 file changed, 68 insertions(+), 20 deletions(-)