[PATCH wpan-next v3 0/4] ieee802154: Improve durations handling

[PATCH wpan-next v3 0/4] ieee802154: Improve durations handling

[Miquel Raynal]

These patches try to enhance the support of the various delays by adding
into the core the necessary logic to derive the actual symbol duration
(and then the lifs/sifs durations) depending on the protocol used. The
symbol duration type is also updated to fit smaller numbers.

Having the symbol durations properly set is a mandatory step in order to
use the scanning feature that will soon be introduced.

Changes since v2:
* Added the ca8210 driver fix.
* Fully dropped my rework of the way channels are advertised by device
  drivers. Adapted instead the main existing helper to derive durations
  based on the page/channel couple.

Miquel Raynal (4):
  net: ieee802154: ca8210: Fix lifs/sifs periods
  net: mac802154: Convert the symbol duration into nanoseconds
  net: mac802154: Set durations automatically
  net: ieee802154: Drop duration settings when the core does it already

 drivers/net/ieee802154/at86rf230.c | 33 ------------------
 drivers/net/ieee802154/atusb.c     | 33 ------------------
 drivers/net/ieee802154/ca8210.c    |  3 --
 drivers/net/ieee802154/mcr20a.c    |  5 ---
 include/net/cfg802154.h            |  6 ++--
 net/mac802154/cfg.c                |  1 +
 net/mac802154/main.c               | 54 +++++++++++++++++++++++++++---
 7 files changed, 55 insertions(+), 80 deletions(-)

-- 
2.27.0

[PATCH wpan-next v3 1/4] net: ieee802154: ca8210: Fix lifs/sifs periods

[Miquel Raynal]

These periods are expressed in time units (microseconds) while 40 and 12
are the number of symbol durations these periods will last. We need to
multiply them both with the symbol_duration in order to get these
values in microseconds.

Fixes: ded845a781a5 ("ieee802154: Add CA8210 IEEE 802.15.4 device driver")
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/net/ieee802154/ca8210.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
index f3438d3e104a..2bc730fd260e 100644
--- a/drivers/net/ieee802154/ca8210.c
+++ b/drivers/net/ieee802154/ca8210.c
@@ -2975,8 +2975,8 @@ static void ca8210_hw_setup(struct ieee802154_hw *ca8210_hw)
 	ca8210_hw->phy->cca.opt = NL802154_CCA_OPT_ENERGY_CARRIER_AND;
 	ca8210_hw->phy->cca_ed_level = -9800;
 	ca8210_hw->phy->symbol_duration = 16;
-	ca8210_hw->phy->lifs_period = 40;
-	ca8210_hw->phy->sifs_period = 12;
+	ca8210_hw->phy->lifs_period = 40 * ca8210_hw->phy->symbol_duration;
+	ca8210_hw->phy->sifs_period = 12 * ca8210_hw->phy->symbol_duration;
 	ca8210_hw->flags =
 		IEEE802154_HW_AFILT |
 		IEEE802154_HW_OMIT_CKSUM |
-- 
2.27.0

[PATCH wpan-next v3 2/4] net: mac802154: Convert the symbol duration into nanoseconds

[Miquel Raynal]

Tdsym is often given in the spec as pretty small numbers in microseconds
and hence was reflected in the code as symbol_duration and was stored as
a u8. Actually, for UWB PHYs, the symbol duration is given in
nanoseconds and are as precise as picoseconds. In order to handle better
these PHYs, change the type of symbol_duration to u32 and store this
value in nanoseconds.

All the users of this variable are updated in a mechanical way.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/net/ieee802154/at86rf230.c | 22 +++++++++++-----------
 drivers/net/ieee802154/atusb.c     | 22 +++++++++++-----------
 drivers/net/ieee802154/ca8210.c    |  2 +-
 drivers/net/ieee802154/mcr20a.c    |  8 ++++----
 include/net/cfg802154.h            |  4 ++--
 net/mac802154/main.c               |  8 ++++----
 6 files changed, 33 insertions(+), 33 deletions(-)

diff --git a/drivers/net/ieee802154/at86rf230.c b/drivers/net/ieee802154/at86rf230.c
index 4f5ef8a9a9a8..0264e43a1080 100644
--- a/drivers/net/ieee802154/at86rf230.c
+++ b/drivers/net/ieee802154/at86rf230.c
@@ -1075,24 +1075,24 @@ at86rf212_set_channel(struct at86rf230_local *lp, u8 page, u8 channel)
 	if (channel == 0) {
 		if (page == 0) {
 			/* SUB:0 and BPSK:0 -> BPSK-20 */
-			lp->hw->phy->symbol_duration = 50;
+			lp->hw->phy->symbol_duration = 50 * NSEC_PER_USEC;
 		} else {
 			/* SUB:1 and BPSK:0 -> BPSK-40 */
-			lp->hw->phy->symbol_duration = 25;
+			lp->hw->phy->symbol_duration = 25 * NSEC_PER_USEC;
 		}
 	} else {
 		if (page == 0)
 			/* SUB:0 and BPSK:1 -> OQPSK-100/200/400 */
-			lp->hw->phy->symbol_duration = 40;
+			lp->hw->phy->symbol_duration = 40 * NSEC_PER_USEC;
 		else
 			/* SUB:1 and BPSK:1 -> OQPSK-250/500/1000 */
-			lp->hw->phy->symbol_duration = 16;
+			lp->hw->phy->symbol_duration = 16 * NSEC_PER_USEC;
 	}
 
-	lp->hw->phy->lifs_period = IEEE802154_LIFS_PERIOD *
-				   lp->hw->phy->symbol_duration;
-	lp->hw->phy->sifs_period = IEEE802154_SIFS_PERIOD *
-				   lp->hw->phy->symbol_duration;
+	lp->hw->phy->lifs_period =
+		(IEEE802154_LIFS_PERIOD * lp->hw->phy->symbol_duration) / 1000;
+	lp->hw->phy->sifs_period =
+		(IEEE802154_SIFS_PERIOD * lp->hw->phy->symbol_duration) / 1000;
 
 	return at86rf230_write_subreg(lp, SR_CHANNEL, channel);
 }
@@ -1569,7 +1569,7 @@ at86rf230_detect_device(struct at86rf230_local *lp)
 		lp->data = &at86rf231_data;
 		lp->hw->phy->supported.channels[0] = 0x7FFF800;
 		lp->hw->phy->current_channel = 11;
-		lp->hw->phy->symbol_duration = 16;
+		lp->hw->phy->symbol_duration = 16 * NSEC_PER_USEC;
 		lp->hw->phy->supported.tx_powers = at86rf231_powers;
 		lp->hw->phy->supported.tx_powers_size = ARRAY_SIZE(at86rf231_powers);
 		lp->hw->phy->supported.cca_ed_levels = at86rf231_ed_levels;
@@ -1582,7 +1582,7 @@ at86rf230_detect_device(struct at86rf230_local *lp)
 		lp->hw->phy->supported.channels[0] = 0x00007FF;
 		lp->hw->phy->supported.channels[2] = 0x00007FF;
 		lp->hw->phy->current_channel = 5;
-		lp->hw->phy->symbol_duration = 25;
+		lp->hw->phy->symbol_duration = 25 * NSEC_PER_USEC;
 		lp->hw->phy->supported.lbt = NL802154_SUPPORTED_BOOL_BOTH;
 		lp->hw->phy->supported.tx_powers = at86rf212_powers;
 		lp->hw->phy->supported.tx_powers_size = ARRAY_SIZE(at86rf212_powers);
@@ -1594,7 +1594,7 @@ at86rf230_detect_device(struct at86rf230_local *lp)
 		lp->data = &at86rf233_data;
 		lp->hw->phy->supported.channels[0] = 0x7FFF800;
 		lp->hw->phy->current_channel = 13;
-		lp->hw->phy->symbol_duration = 16;
+		lp->hw->phy->symbol_duration = 16 * NSEC_PER_USEC;
 		lp->hw->phy->supported.tx_powers = at86rf233_powers;
 		lp->hw->phy->supported.tx_powers_size = ARRAY_SIZE(at86rf233_powers);
 		lp->hw->phy->supported.cca_ed_levels = at86rf233_ed_levels;
diff --git a/drivers/net/ieee802154/atusb.c b/drivers/net/ieee802154/atusb.c
index 23ee0b14cbfa..eb27d0fb4f5f 100644
--- a/drivers/net/ieee802154/atusb.c
+++ b/drivers/net/ieee802154/atusb.c
@@ -678,24 +678,24 @@ static int hulusb_set_channel(struct ieee802154_hw *hw, u8 page, u8 channel)
 	if (channel == 0) {
 		if (page == 0) {
 			/* SUB:0 and BPSK:0 -> BPSK-20 */
-			lp->hw->phy->symbol_duration = 50;
+			lp->hw->phy->symbol_duration = 50 * NSEC_PER_USEC;
 		} else {
 			/* SUB:1 and BPSK:0 -> BPSK-40 */
-			lp->hw->phy->symbol_duration = 25;
+			lp->hw->phy->symbol_duration = 25 * NSEC_PER_USEC;
 		}
 	} else {
 		if (page == 0)
 			/* SUB:0 and BPSK:1 -> OQPSK-100/200/400 */
-			lp->hw->phy->symbol_duration = 40;
+			lp->hw->phy->symbol_duration = 40 * NSEC_PER_USEC;
 		else
 			/* SUB:1 and BPSK:1 -> OQPSK-250/500/1000 */
-			lp->hw->phy->symbol_duration = 16;
+			lp->hw->phy->symbol_duration = 16 * NSEC_PER_USEC;
 	}
 
-	lp->hw->phy->lifs_period = IEEE802154_LIFS_PERIOD *
-				   lp->hw->phy->symbol_duration;
-	lp->hw->phy->sifs_period = IEEE802154_SIFS_PERIOD *
-				   lp->hw->phy->symbol_duration;
+	lp->hw->phy->lifs_period =
+		(IEEE802154_LIFS_PERIOD * lp->hw->phy->symbol_duration) / 1000;
+	lp->hw->phy->sifs_period =
+		(IEEE802154_SIFS_PERIOD * lp->hw->phy->symbol_duration) / 1000;
 
 	return atusb_write_subreg(lp, SR_CHANNEL, channel);
 }
@@ -916,7 +916,7 @@ static int atusb_get_and_conf_chip(struct atusb *atusb)
 		chip = "AT86RF230";
 		atusb->hw->phy->supported.channels[0] = 0x7FFF800;
 		atusb->hw->phy->current_channel = 11;	/* reset default */
-		atusb->hw->phy->symbol_duration = 16;
+		atusb->hw->phy->symbol_duration = 16 * NSEC_PER_USEC;
 		atusb->hw->phy->supported.tx_powers = atusb_powers;
 		atusb->hw->phy->supported.tx_powers_size = ARRAY_SIZE(atusb_powers);
 		hw->phy->supported.cca_ed_levels = atusb_ed_levels;
@@ -926,7 +926,7 @@ static int atusb_get_and_conf_chip(struct atusb *atusb)
 		chip = "AT86RF231";
 		atusb->hw->phy->supported.channels[0] = 0x7FFF800;
 		atusb->hw->phy->current_channel = 11;	/* reset default */
-		atusb->hw->phy->symbol_duration = 16;
+		atusb->hw->phy->symbol_duration = 16 * NSEC_PER_USEC;
 		atusb->hw->phy->supported.tx_powers = atusb_powers;
 		atusb->hw->phy->supported.tx_powers_size = ARRAY_SIZE(atusb_powers);
 		hw->phy->supported.cca_ed_levels = atusb_ed_levels;
@@ -938,7 +938,7 @@ static int atusb_get_and_conf_chip(struct atusb *atusb)
 		atusb->hw->phy->supported.channels[0] = 0x00007FF;
 		atusb->hw->phy->supported.channels[2] = 0x00007FF;
 		atusb->hw->phy->current_channel = 5;
-		atusb->hw->phy->symbol_duration = 25;
+		atusb->hw->phy->symbol_duration = 25 * NSEC_PER_USEC;
 		atusb->hw->phy->supported.lbt = NL802154_SUPPORTED_BOOL_BOTH;
 		atusb->hw->phy->supported.tx_powers = at86rf212_powers;
 		atusb->hw->phy->supported.tx_powers_size = ARRAY_SIZE(at86rf212_powers);
diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
index 2bc730fd260e..e88cdbf6673b 100644
--- a/drivers/net/ieee802154/ca8210.c
+++ b/drivers/net/ieee802154/ca8210.c
@@ -2974,7 +2974,7 @@ static void ca8210_hw_setup(struct ieee802154_hw *ca8210_hw)
 	ca8210_hw->phy->cca.mode = NL802154_CCA_ENERGY_CARRIER;
 	ca8210_hw->phy->cca.opt = NL802154_CCA_OPT_ENERGY_CARRIER_AND;
 	ca8210_hw->phy->cca_ed_level = -9800;
-	ca8210_hw->phy->symbol_duration = 16;
+	ca8210_hw->phy->symbol_duration = 16 * NSEC_PER_USEC;
 	ca8210_hw->phy->lifs_period = 40 * ca8210_hw->phy->symbol_duration;
 	ca8210_hw->phy->sifs_period = 12 * ca8210_hw->phy->symbol_duration;
 	ca8210_hw->flags =
diff --git a/drivers/net/ieee802154/mcr20a.c b/drivers/net/ieee802154/mcr20a.c
index 383231b85464..c925e629ddf3 100644
--- a/drivers/net/ieee802154/mcr20a.c
+++ b/drivers/net/ieee802154/mcr20a.c
@@ -975,9 +975,9 @@ static void mcr20a_hw_setup(struct mcr20a_local *lp)
 
 	dev_dbg(printdev(lp), "%s\n", __func__);
 
-	phy->symbol_duration = 16;
-	phy->lifs_period = 40 * phy->symbol_duration;
-	phy->sifs_period = 12 * phy->symbol_duration;
+	phy->symbol_duration = 16 * NSEC_PER_USEC;
+	phy->lifs_period = (40 * phy->symbol_duration) / NSEC_PER_USEC;
+	phy->sifs_period = (12 * phy->symbol_duration) / NSEC_PER_USEC;
 
 	hw->flags = IEEE802154_HW_TX_OMIT_CKSUM |
 			IEEE802154_HW_AFILT |
@@ -1006,7 +1006,7 @@ static void mcr20a_hw_setup(struct mcr20a_local *lp)
 	phy->current_page = 0;
 	/* MCR20A default reset value */
 	phy->current_channel = 20;
-	phy->symbol_duration = 16;
+	phy->symbol_duration = 16 * NSEC_PER_USEC;
 	phy->supported.tx_powers = mcr20a_powers;
 	phy->supported.tx_powers_size = ARRAY_SIZE(mcr20a_powers);
 	phy->cca_ed_level = phy->supported.cca_ed_levels[75];
diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h
index 6ed07844eb24..8a4b6a50452f 100644
--- a/include/net/cfg802154.h
+++ b/include/net/cfg802154.h
@@ -203,8 +203,8 @@ struct wpan_phy {
 
 	/* PHY depended MAC PIB values */
 
-	/* 802.15.4 acronym: Tdsym in usec */
-	u8 symbol_duration;
+	/* 802.15.4 acronym: Tdsym in nsec */
+	u32 symbol_duration;
 	/* lifs and sifs periods timing */
 	u16 lifs_period;
 	u16 sifs_period;
diff --git a/net/mac802154/main.c b/net/mac802154/main.c
index 520cedc594e1..53153367f9d0 100644
--- a/net/mac802154/main.c
+++ b/net/mac802154/main.c
@@ -131,10 +131,10 @@ static void ieee802154_setup_wpan_phy_pib(struct wpan_phy *wpan_phy)
 	 * Should be done when all drivers sets this value.
 	 */
 
-	wpan_phy->lifs_period = IEEE802154_LIFS_PERIOD *
-				wpan_phy->symbol_duration;
-	wpan_phy->sifs_period = IEEE802154_SIFS_PERIOD *
-				wpan_phy->symbol_duration;
+	wpan_phy->lifs_period =
+		(IEEE802154_LIFS_PERIOD * wpan_phy->symbol_duration) / 1000;
+	wpan_phy->sifs_period =
+		(IEEE802154_SIFS_PERIOD * wpan_phy->symbol_duration) / 1000;
 }
 
 int ieee802154_register_hw(struct ieee802154_hw *hw)
-- 
2.27.0

[PATCH wpan-next v3 3/4] net: mac802154: Set durations automatically

[Miquel Raynal]

As depicted in the IEEE 802.15.4 specification, modulation/bands are
tight to a number of page/channels so we can for most of them derive the
durations automatically.

The two locations that must call this new helper to set the variou
symbol durations are:
- when manually requesting a channel change though the netlink interface
- at PHY creation, once the device driver has set the default
  page/channel

If an information is missing, the symbol duration is not touched, a
debug message is eventually printed. This keeps the compatibility with
the unconverted drivers for which it was too complicated for me to find
their precise information. If they initially provided a symbol duration,
it would be kept. If they don't, the symbol duration value is left
untouched.

Once the symbol duration derived, the lifs and sifs durations are
updated as well.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 include/net/cfg802154.h |  2 ++
 net/mac802154/cfg.c     |  1 +
 net/mac802154/main.c    | 46 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 49 insertions(+)

diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h
index 8a4b6a50452f..49b4bcc24032 100644
--- a/include/net/cfg802154.h
+++ b/include/net/cfg802154.h
@@ -405,4 +405,6 @@ static inline const char *wpan_phy_name(struct wpan_phy *phy)
 	return dev_name(&phy->dev);
 }
 
+void ieee802154_configure_durations(struct wpan_phy *phy);
+
 #endif /* __NET_CFG802154_H */
diff --git a/net/mac802154/cfg.c b/net/mac802154/cfg.c
index fbeebe3bc31d..1e4a9f74ed43 100644
--- a/net/mac802154/cfg.c
+++ b/net/mac802154/cfg.c
@@ -118,6 +118,7 @@ ieee802154_set_channel(struct wpan_phy *wpan_phy, u8 page, u8 channel)
 	if (!ret) {
 		wpan_phy->current_page = page;
 		wpan_phy->current_channel = channel;
+		ieee802154_configure_durations(wpan_phy);
 	}
 
 	return ret;
diff --git a/net/mac802154/main.c b/net/mac802154/main.c
index 53153367f9d0..5546ef86e231 100644
--- a/net/mac802154/main.c
+++ b/net/mac802154/main.c
@@ -113,6 +113,50 @@ ieee802154_alloc_hw(size_t priv_data_len, const struct ieee802154_ops *ops)
 }
 EXPORT_SYMBOL(ieee802154_alloc_hw);
 
+void ieee802154_configure_durations(struct wpan_phy *phy)
+{
+	u32 duration = 0;
+
+	switch (phy->current_page) {
+	case 0:
+		if (BIT(phy->current_page) & 0x1)
+			/* 868 MHz BPSK 802.15.4-2003: 20 ksym/s */
+			duration = 50 * NSEC_PER_USEC;
+		else if (phy->current_page & 0x7FE)
+			/* 915 MHz BPSK	802.15.4-2003: 40 ksym/s */
+			duration = 25 * NSEC_PER_USEC;
+		else if (phy->current_page & 0x7FFF800)
+			/* 2400 MHz O-QPSK 802.15.4-2006: 62.5 ksym/s */
+			duration = 16 * NSEC_PER_USEC;
+		break;
+	case 2:
+		if (BIT(phy->current_page) & 0x1)
+			/* 868 MHz O-QPSK 802.15.4-2006: 25 ksym/s */
+			duration = 40 * NSEC_PER_USEC;
+		else if (phy->current_page & 0x7FE)
+			/* 915 MHz O-QPSK 802.15.4-2006: 62.5 ksym/s */
+			duration = 16 * NSEC_PER_USEC;
+		break;
+	case 3:
+		if (BIT(phy->current_page) & 0x3FFF)
+			/* 2.4 GHz CSS 802.15.4a-2007: 1/6 Msym/s */
+			duration = 6 * NSEC_PER_USEC;
+		break;
+	default:
+		break;
+	}
+
+	if (!duration) {
+		pr_debug("Unknown PHY symbol duration\n");
+		return;
+	}
+
+	phy->symbol_duration = duration;
+	phy->lifs_period = (IEEE802154_LIFS_PERIOD * phy->symbol_duration) / NSEC_PER_SEC;
+	phy->sifs_period = (IEEE802154_SIFS_PERIOD * phy->symbol_duration) / NSEC_PER_SEC;
+}
+EXPORT_SYMBOL(ieee802154_configure_durations);
+
 void ieee802154_free_hw(struct ieee802154_hw *hw)
 {
 	struct ieee802154_local *local = hw_to_local(hw);
@@ -157,6 +201,8 @@ int ieee802154_register_hw(struct ieee802154_hw *hw)
 
 	ieee802154_setup_wpan_phy_pib(local->phy);
 
+	ieee802154_configure_durations(local->phy);
+
 	if (!(hw->flags & IEEE802154_HW_CSMA_PARAMS)) {
 		local->phy->supported.min_csma_backoffs = 4;
 		local->phy->supported.max_csma_backoffs = 4;
-- 
2.27.0

[PATCH wpan-next v3 4/4] net: ieee802154: Drop duration settings when the core does it already

[Miquel Raynal]

The core now knows how to set the symbol duration in a few cases, when
drivers correctly advertise the protocols used on each channel. For
these drivers, there is no more need to bother with symbol duration,
lifs and sifs periods so just drop the duplicated code.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/net/ieee802154/at86rf230.c | 33 ------------------------------
 drivers/net/ieee802154/atusb.c     | 33 ------------------------------
 drivers/net/ieee802154/ca8210.c    |  3 ---
 drivers/net/ieee802154/mcr20a.c    |  5 -----
 4 files changed, 74 deletions(-)

diff --git a/drivers/net/ieee802154/at86rf230.c b/drivers/net/ieee802154/at86rf230.c
index 0264e43a1080..563031ce76f0 100644
--- a/drivers/net/ieee802154/at86rf230.c
+++ b/drivers/net/ieee802154/at86rf230.c
@@ -1064,36 +1064,6 @@ at86rf212_set_channel(struct at86rf230_local *lp, u8 page, u8 channel)
 	if (rc < 0)
 		return rc;
 
-	/* This sets the symbol_duration according frequency on the 212.
-	 * TODO move this handling while set channel and page in cfg802154.
-	 * We can do that, this timings are according 802.15.4 standard.
-	 * If we do that in cfg802154, this is a more generic calculation.
-	 *
-	 * This should also protected from ifs_timer. Means cancel timer and
-	 * init with a new value. For now, this is okay.
-	 */
-	if (channel == 0) {
-		if (page == 0) {
-			/* SUB:0 and BPSK:0 -> BPSK-20 */
-			lp->hw->phy->symbol_duration = 50 * NSEC_PER_USEC;
-		} else {
-			/* SUB:1 and BPSK:0 -> BPSK-40 */
-			lp->hw->phy->symbol_duration = 25 * NSEC_PER_USEC;
-		}
-	} else {
-		if (page == 0)
-			/* SUB:0 and BPSK:1 -> OQPSK-100/200/400 */
-			lp->hw->phy->symbol_duration = 40 * NSEC_PER_USEC;
-		else
-			/* SUB:1 and BPSK:1 -> OQPSK-250/500/1000 */
-			lp->hw->phy->symbol_duration = 16 * NSEC_PER_USEC;
-	}
-
-	lp->hw->phy->lifs_period =
-		(IEEE802154_LIFS_PERIOD * lp->hw->phy->symbol_duration) / 1000;
-	lp->hw->phy->sifs_period =
-		(IEEE802154_SIFS_PERIOD * lp->hw->phy->symbol_duration) / 1000;
-
 	return at86rf230_write_subreg(lp, SR_CHANNEL, channel);
 }
 
@@ -1569,7 +1539,6 @@ at86rf230_detect_device(struct at86rf230_local *lp)
 		lp->data = &at86rf231_data;
 		lp->hw->phy->supported.channels[0] = 0x7FFF800;
 		lp->hw->phy->current_channel = 11;
-		lp->hw->phy->symbol_duration = 16 * NSEC_PER_USEC;
 		lp->hw->phy->supported.tx_powers = at86rf231_powers;
 		lp->hw->phy->supported.tx_powers_size = ARRAY_SIZE(at86rf231_powers);
 		lp->hw->phy->supported.cca_ed_levels = at86rf231_ed_levels;
@@ -1582,7 +1551,6 @@ at86rf230_detect_device(struct at86rf230_local *lp)
 		lp->hw->phy->supported.channels[0] = 0x00007FF;
 		lp->hw->phy->supported.channels[2] = 0x00007FF;
 		lp->hw->phy->current_channel = 5;
-		lp->hw->phy->symbol_duration = 25 * NSEC_PER_USEC;
 		lp->hw->phy->supported.lbt = NL802154_SUPPORTED_BOOL_BOTH;
 		lp->hw->phy->supported.tx_powers = at86rf212_powers;
 		lp->hw->phy->supported.tx_powers_size = ARRAY_SIZE(at86rf212_powers);
@@ -1594,7 +1562,6 @@ at86rf230_detect_device(struct at86rf230_local *lp)
 		lp->data = &at86rf233_data;
 		lp->hw->phy->supported.channels[0] = 0x7FFF800;
 		lp->hw->phy->current_channel = 13;
-		lp->hw->phy->symbol_duration = 16 * NSEC_PER_USEC;
 		lp->hw->phy->supported.tx_powers = at86rf233_powers;
 		lp->hw->phy->supported.tx_powers_size = ARRAY_SIZE(at86rf233_powers);
 		lp->hw->phy->supported.cca_ed_levels = at86rf233_ed_levels;
diff --git a/drivers/net/ieee802154/atusb.c b/drivers/net/ieee802154/atusb.c
index eb27d0fb4f5f..f27a5f535808 100644
--- a/drivers/net/ieee802154/atusb.c
+++ b/drivers/net/ieee802154/atusb.c
@@ -667,36 +667,6 @@ static int hulusb_set_channel(struct ieee802154_hw *hw, u8 page, u8 channel)
 	if (rc < 0)
 		return rc;
 
-	/* This sets the symbol_duration according frequency on the 212.
-	 * TODO move this handling while set channel and page in cfg802154.
-	 * We can do that, this timings are according 802.15.4 standard.
-	 * If we do that in cfg802154, this is a more generic calculation.
-	 *
-	 * This should also protected from ifs_timer. Means cancel timer and
-	 * init with a new value. For now, this is okay.
-	 */
-	if (channel == 0) {
-		if (page == 0) {
-			/* SUB:0 and BPSK:0 -> BPSK-20 */
-			lp->hw->phy->symbol_duration = 50 * NSEC_PER_USEC;
-		} else {
-			/* SUB:1 and BPSK:0 -> BPSK-40 */
-			lp->hw->phy->symbol_duration = 25 * NSEC_PER_USEC;
-		}
-	} else {
-		if (page == 0)
-			/* SUB:0 and BPSK:1 -> OQPSK-100/200/400 */
-			lp->hw->phy->symbol_duration = 40 * NSEC_PER_USEC;
-		else
-			/* SUB:1 and BPSK:1 -> OQPSK-250/500/1000 */
-			lp->hw->phy->symbol_duration = 16 * NSEC_PER_USEC;
-	}
-
-	lp->hw->phy->lifs_period =
-		(IEEE802154_LIFS_PERIOD * lp->hw->phy->symbol_duration) / 1000;
-	lp->hw->phy->sifs_period =
-		(IEEE802154_SIFS_PERIOD * lp->hw->phy->symbol_duration) / 1000;
-
 	return atusb_write_subreg(lp, SR_CHANNEL, channel);
 }
 
@@ -916,7 +886,6 @@ static int atusb_get_and_conf_chip(struct atusb *atusb)
 		chip = "AT86RF230";
 		atusb->hw->phy->supported.channels[0] = 0x7FFF800;
 		atusb->hw->phy->current_channel = 11;	/* reset default */
-		atusb->hw->phy->symbol_duration = 16 * NSEC_PER_USEC;
 		atusb->hw->phy->supported.tx_powers = atusb_powers;
 		atusb->hw->phy->supported.tx_powers_size = ARRAY_SIZE(atusb_powers);
 		hw->phy->supported.cca_ed_levels = atusb_ed_levels;
@@ -926,7 +895,6 @@ static int atusb_get_and_conf_chip(struct atusb *atusb)
 		chip = "AT86RF231";
 		atusb->hw->phy->supported.channels[0] = 0x7FFF800;
 		atusb->hw->phy->current_channel = 11;	/* reset default */
-		atusb->hw->phy->symbol_duration = 16 * NSEC_PER_USEC;
 		atusb->hw->phy->supported.tx_powers = atusb_powers;
 		atusb->hw->phy->supported.tx_powers_size = ARRAY_SIZE(atusb_powers);
 		hw->phy->supported.cca_ed_levels = atusb_ed_levels;
@@ -938,7 +906,6 @@ static int atusb_get_and_conf_chip(struct atusb *atusb)
 		atusb->hw->phy->supported.channels[0] = 0x00007FF;
 		atusb->hw->phy->supported.channels[2] = 0x00007FF;
 		atusb->hw->phy->current_channel = 5;
-		atusb->hw->phy->symbol_duration = 25 * NSEC_PER_USEC;
 		atusb->hw->phy->supported.lbt = NL802154_SUPPORTED_BOOL_BOTH;
 		atusb->hw->phy->supported.tx_powers = at86rf212_powers;
 		atusb->hw->phy->supported.tx_powers_size = ARRAY_SIZE(at86rf212_powers);
diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
index e88cdbf6673b..fc74fa0f1ddd 100644
--- a/drivers/net/ieee802154/ca8210.c
+++ b/drivers/net/ieee802154/ca8210.c
@@ -2974,9 +2974,6 @@ static void ca8210_hw_setup(struct ieee802154_hw *ca8210_hw)
 	ca8210_hw->phy->cca.mode = NL802154_CCA_ENERGY_CARRIER;
 	ca8210_hw->phy->cca.opt = NL802154_CCA_OPT_ENERGY_CARRIER_AND;
 	ca8210_hw->phy->cca_ed_level = -9800;
-	ca8210_hw->phy->symbol_duration = 16 * NSEC_PER_USEC;
-	ca8210_hw->phy->lifs_period = 40 * ca8210_hw->phy->symbol_duration;
-	ca8210_hw->phy->sifs_period = 12 * ca8210_hw->phy->symbol_duration;
 	ca8210_hw->flags =
 		IEEE802154_HW_AFILT |
 		IEEE802154_HW_OMIT_CKSUM |
diff --git a/drivers/net/ieee802154/mcr20a.c b/drivers/net/ieee802154/mcr20a.c
index c925e629ddf3..2b4ba3f9ecd7 100644
--- a/drivers/net/ieee802154/mcr20a.c
+++ b/drivers/net/ieee802154/mcr20a.c
@@ -975,10 +975,6 @@ static void mcr20a_hw_setup(struct mcr20a_local *lp)
 
 	dev_dbg(printdev(lp), "%s\n", __func__);
 
-	phy->symbol_duration = 16 * NSEC_PER_USEC;
-	phy->lifs_period = (40 * phy->symbol_duration) / NSEC_PER_USEC;
-	phy->sifs_period = (12 * phy->symbol_duration) / NSEC_PER_USEC;
-
 	hw->flags = IEEE802154_HW_TX_OMIT_CKSUM |
 			IEEE802154_HW_AFILT |
 			IEEE802154_HW_PROMISCUOUS;
@@ -1006,7 +1002,6 @@ static void mcr20a_hw_setup(struct mcr20a_local *lp)
 	phy->current_page = 0;
 	/* MCR20A default reset value */
 	phy->current_channel = 20;
-	phy->symbol_duration = 16 * NSEC_PER_USEC;
 	phy->supported.tx_powers = mcr20a_powers;
 	phy->supported.tx_powers_size = ARRAY_SIZE(mcr20a_powers);
 	phy->cca_ed_level = phy->supported.cca_ed_levels[75];
-- 
2.27.0

Re: [PATCH wpan-next v3 1/4] net: ieee802154: ca8210: Fix lifs/sifs periods

[Stefan Schmidt]

Hello.

On 01.02.22 19:06, Miquel Raynal wrote:
> These periods are expressed in time units (microseconds) while 40 and 12
> are the number of symbol durations these periods will last. We need to
> multiply them both with the symbol_duration in order to get these
> values in microseconds.
> 
> Fixes: ded845a781a5 ("ieee802154: Add CA8210 IEEE 802.15.4 device driver")
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>   drivers/net/ieee802154/ca8210.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
> index f3438d3e104a..2bc730fd260e 100644
> --- a/drivers/net/ieee802154/ca8210.c
> +++ b/drivers/net/ieee802154/ca8210.c
> @@ -2975,8 +2975,8 @@ static void ca8210_hw_setup(struct ieee802154_hw *ca8210_hw)
>   	ca8210_hw->phy->cca.opt = NL802154_CCA_OPT_ENERGY_CARRIER_AND;
>   	ca8210_hw->phy->cca_ed_level = -9800;
>   	ca8210_hw->phy->symbol_duration = 16;
> -	ca8210_hw->phy->lifs_period = 40;
> -	ca8210_hw->phy->sifs_period = 12;
> +	ca8210_hw->phy->lifs_period = 40 * ca8210_hw->phy->symbol_duration;
> +	ca8210_hw->phy->sifs_period = 12 * ca8210_hw->phy->symbol_duration;
>   	ca8210_hw->flags =
>   		IEEE802154_HW_AFILT |
>   		IEEE802154_HW_OMIT_CKSUM |
> 

As mentioned in the discussion on the other thread I ripped this one out 
and applied it as fix to the wpan tree. Thanks!

regards
Stefan Schmidt

Re: [PATCH wpan-next v3 0/4] ieee802154: Improve durations handling

[Alexander Aring]

Hi,

On Tue, Feb 1, 2022 at 1:06 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> These patches try to enhance the support of the various delays by adding
> into the core the necessary logic to derive the actual symbol duration
> (and then the lifs/sifs durations) depending on the protocol used. The
> symbol duration type is also updated to fit smaller numbers.
>
> Having the symbol durations properly set is a mandatory step in order to
> use the scanning feature that will soon be introduced.
>
> Changes since v2:
> * Added the ca8210 driver fix.
> * Fully dropped my rework of the way channels are advertised by device
>   drivers. Adapted instead the main existing helper to derive durations
>   based on the page/channel couple.
>
> Miquel Raynal (4):
>   net: ieee802154: ca8210: Fix lifs/sifs periods
>   net: mac802154: Convert the symbol duration into nanoseconds
>   net: mac802154: Set durations automatically
>   net: ieee802154: Drop duration settings when the core does it already
>

Acked-by: Alexander Aring <aahringo@redhat.com>

Thanks Stefan. I agree "net: ieee802154: ca8210: Fix lifs/sifs
periods" should go into "wpan".

- Alex

Re: [PATCH wpan-next v3 0/4] ieee802154: Improve durations handling

[Stefan Schmidt]

Hello.

On 01.02.22 19:06, Miquel Raynal wrote:
> These patches try to enhance the support of the various delays by adding
> into the core the necessary logic to derive the actual symbol duration
> (and then the lifs/sifs durations) depending on the protocol used. The
> symbol duration type is also updated to fit smaller numbers.
> 
> Having the symbol durations properly set is a mandatory step in order to
> use the scanning feature that will soon be introduced.
> 
> Changes since v2:
> * Added the ca8210 driver fix.
> * Fully dropped my rework of the way channels are advertised by device
>    drivers. Adapted instead the main existing helper to derive durations
>    based on the page/channel couple.
> 
> Miquel Raynal (4):
>    net: ieee802154: ca8210: Fix lifs/sifs periods
>    net: mac802154: Convert the symbol duration into nanoseconds
>    net: mac802154: Set durations automatically
>    net: ieee802154: Drop duration settings when the core does it already
> 
>   drivers/net/ieee802154/at86rf230.c | 33 ------------------
>   drivers/net/ieee802154/atusb.c     | 33 ------------------
>   drivers/net/ieee802154/ca8210.c    |  3 --
>   drivers/net/ieee802154/mcr20a.c    |  5 ---
>   include/net/cfg802154.h            |  6 ++--
>   net/mac802154/cfg.c                |  1 +
>   net/mac802154/main.c               | 54 +++++++++++++++++++++++++++---
>   7 files changed, 55 insertions(+), 80 deletions(-)
> 


This patchset has been applied to the wpan-next tree and will be
part of the next pull request to net-next. Thanks!

regards
Stefan Schmidt

Re: [PATCH wpan-next v3 3/4] net: mac802154: Set durations automatically

[Miquel Raynal]

Hi Stefan,

miquel.raynal@bootlin.com wrote on Tue,  1 Feb 2022 19:06:28 +0100:

> As depicted in the IEEE 802.15.4 specification, modulation/bands are
> tight to a number of page/channels so we can for most of them derive the
> durations automatically.
> 
> The two locations that must call this new helper to set the variou
> symbol durations are:
> - when manually requesting a channel change though the netlink interface
> - at PHY creation, once the device driver has set the default
>   page/channel
> 
> If an information is missing, the symbol duration is not touched, a
> debug message is eventually printed. This keeps the compatibility with
> the unconverted drivers for which it was too complicated for me to find
> their precise information. If they initially provided a symbol duration,
> it would be kept. If they don't, the symbol duration value is left
> untouched.
> 
> Once the symbol duration derived, the lifs and sifs durations are
> updated as well.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  include/net/cfg802154.h |  2 ++
>  net/mac802154/cfg.c     |  1 +
>  net/mac802154/main.c    | 46 +++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 49 insertions(+)
> 
> diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h
> index 8a4b6a50452f..49b4bcc24032 100644
> --- a/include/net/cfg802154.h
> +++ b/include/net/cfg802154.h
> @@ -405,4 +405,6 @@ static inline const char *wpan_phy_name(struct wpan_phy *phy)
>  	return dev_name(&phy->dev);
>  }
>  
> +void ieee802154_configure_durations(struct wpan_phy *phy);
> +
>  #endif /* __NET_CFG802154_H */
> diff --git a/net/mac802154/cfg.c b/net/mac802154/cfg.c
> index fbeebe3bc31d..1e4a9f74ed43 100644
> --- a/net/mac802154/cfg.c
> +++ b/net/mac802154/cfg.c
> @@ -118,6 +118,7 @@ ieee802154_set_channel(struct wpan_phy *wpan_phy, u8 page, u8 channel)
>  	if (!ret) {
>  		wpan_phy->current_page = page;
>  		wpan_phy->current_channel = channel;
> +		ieee802154_configure_durations(wpan_phy);
>  	}
>  
>  	return ret;
> diff --git a/net/mac802154/main.c b/net/mac802154/main.c
> index 53153367f9d0..5546ef86e231 100644
> --- a/net/mac802154/main.c
> +++ b/net/mac802154/main.c
> @@ -113,6 +113,50 @@ ieee802154_alloc_hw(size_t priv_data_len, const struct ieee802154_ops *ops)
>  }
>  EXPORT_SYMBOL(ieee802154_alloc_hw);
>  
> +void ieee802154_configure_durations(struct wpan_phy *phy)
> +{
> +	u32 duration = 0;
> +
> +	switch (phy->current_page) {
> +	case 0:
> +		if (BIT(phy->current_page) & 0x1)

I am very sorry to spot this only now but this is wrong. 

all the conditions from here and below should be:

		if (BIT(phy->current_channel & <mask>))

The masks look good, the durations as well, but the conditions are
wrong.

> +			/* 868 MHz BPSK 802.15.4-2003: 20 ksym/s */
> +			duration = 50 * NSEC_PER_USEC;
> +		else if (phy->current_page & 0x7FE)

Ditto

> +			/* 915 MHz BPSK	802.15.4-2003: 40 ksym/s */
> +			duration = 25 * NSEC_PER_USEC;
> +		else if (phy->current_page & 0x7FFF800)

Ditto

> +			/* 2400 MHz O-QPSK 802.15.4-2006: 62.5 ksym/s */
> +			duration = 16 * NSEC_PER_USEC;
> +		break;
> +	case 2:
> +		if (BIT(phy->current_page) & 0x1)

Ditto

> +			/* 868 MHz O-QPSK 802.15.4-2006: 25 ksym/s */
> +			duration = 40 * NSEC_PER_USEC;
> +		else if (phy->current_page & 0x7FE)

Ditto

> +			/* 915 MHz O-QPSK 802.15.4-2006: 62.5 ksym/s */
> +			duration = 16 * NSEC_PER_USEC;
> +		break;
> +	case 3:
> +		if (BIT(phy->current_page) & 0x3FFF)

Ditto

> +			/* 2.4 GHz CSS 802.15.4a-2007: 1/6 Msym/s */
> +			duration = 6 * NSEC_PER_USEC;
> +		break;

I see it's "only" in wpan-next (781830c800dd "net: mac802154: Set
durations automatically") and was not yet pulled in the net-next
tree so please let me know what you prefer: I can either provide a
proper patch to fit it (without upstream Fixes reference), or you can
just apply this diff below and push -f the branch. Let me know what you
prefer.

Again, sorry to only see this now.

Thanks,
Miquèl

---

commit 4122765e5f982ed8f0ccea5fd813ef4d53d20a90 (HEAD)
Author: Miquel Raynal <miquel.raynal@bootlin.com>
Date:   Thu Apr 28 17:57:59 2022 +0200

    fixup! net: mac802154: Set durations automatically

diff --git a/net/mac802154/main.c b/net/mac802154/main.c
index 5546ef86e231..bbbdac6ee028 100644
--- a/net/mac802154/main.c
+++ b/net/mac802154/main.c
@@ -119,26 +119,26 @@ void ieee802154_configure_durations(struct wpan_phy *phy)
 
        switch (phy->current_page) {
        case 0:
-               if (BIT(phy->current_page) & 0x1)
+               if (BIT(phy->current_channel) & 0x1)
                        /* 868 MHz BPSK 802.15.4-2003: 20 ksym/s */
                        duration = 50 * NSEC_PER_USEC;
-               else if (phy->current_page & 0x7FE)
+               else if (phy->current_channel & 0x7FE)
                        /* 915 MHz BPSK 802.15.4-2003: 40 ksym/s */
                        duration = 25 * NSEC_PER_USEC;
-               else if (phy->current_page & 0x7FFF800)
+               else if (phy->current_channel & 0x7FFF800)
                        /* 2400 MHz O-QPSK 802.15.4-2006: 62.5 ksym/s */
                        duration = 16 * NSEC_PER_USEC;
                break;
        case 2:
-               if (BIT(phy->current_page) & 0x1)
+               if (BIT(phy->current_channel) & 0x1)
                        /* 868 MHz O-QPSK 802.15.4-2006: 25 ksym/s */
                        duration = 40 * NSEC_PER_USEC;
-               else if (phy->current_page & 0x7FE)
+               else if (phy->current_channel & 0x7FE)
                        /* 915 MHz O-QPSK 802.15.4-2006: 62.5 ksym/s */
                        duration = 16 * NSEC_PER_USEC;
                break;
        case 3:
-               if (BIT(phy->current_page) & 0x3FFF)
+               if (BIT(phy->current_channel) & 0x3FFF)
                        /* 2.4 GHz CSS 802.15.4a-2007: 1/6 Msym/s */
                        duration = 6 * NSEC_PER_USEC;
                break;

Re: [PATCH wpan-next v3 3/4] net: mac802154: Set durations automatically

[Stefan Schmidt]

Hello.

On 28.04.22 17:58, Miquel Raynal wrote:
> Hi Stefan,
> 
> miquel.raynal@bootlin.com wrote on Tue,  1 Feb 2022 19:06:28 +0100:
> 
>> As depicted in the IEEE 802.15.4 specification, modulation/bands are
>> tight to a number of page/channels so we can for most of them derive the
>> durations automatically.
>>
>> The two locations that must call this new helper to set the variou
>> symbol durations are:
>> - when manually requesting a channel change though the netlink interface
>> - at PHY creation, once the device driver has set the default
>>    page/channel
>>
>> If an information is missing, the symbol duration is not touched, a
>> debug message is eventually printed. This keeps the compatibility with
>> the unconverted drivers for which it was too complicated for me to find
>> their precise information. If they initially provided a symbol duration,
>> it would be kept. If they don't, the symbol duration value is left
>> untouched.
>>
>> Once the symbol duration derived, the lifs and sifs durations are
>> updated as well.
>>
>> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
>> ---
>>   include/net/cfg802154.h |  2 ++
>>   net/mac802154/cfg.c     |  1 +
>>   net/mac802154/main.c    | 46 +++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 49 insertions(+)
>>
>> diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h
>> index 8a4b6a50452f..49b4bcc24032 100644
>> --- a/include/net/cfg802154.h
>> +++ b/include/net/cfg802154.h
>> @@ -405,4 +405,6 @@ static inline const char *wpan_phy_name(struct wpan_phy *phy)
>>   	return dev_name(&phy->dev);
>>   }
>>   
>> +void ieee802154_configure_durations(struct wpan_phy *phy);
>> +
>>   #endif /* __NET_CFG802154_H */
>> diff --git a/net/mac802154/cfg.c b/net/mac802154/cfg.c
>> index fbeebe3bc31d..1e4a9f74ed43 100644
>> --- a/net/mac802154/cfg.c
>> +++ b/net/mac802154/cfg.c
>> @@ -118,6 +118,7 @@ ieee802154_set_channel(struct wpan_phy *wpan_phy, u8 page, u8 channel)
>>   	if (!ret) {
>>   		wpan_phy->current_page = page;
>>   		wpan_phy->current_channel = channel;
>> +		ieee802154_configure_durations(wpan_phy);
>>   	}
>>   
>>   	return ret;
>> diff --git a/net/mac802154/main.c b/net/mac802154/main.c
>> index 53153367f9d0..5546ef86e231 100644
>> --- a/net/mac802154/main.c
>> +++ b/net/mac802154/main.c
>> @@ -113,6 +113,50 @@ ieee802154_alloc_hw(size_t priv_data_len, const struct ieee802154_ops *ops)
>>   }
>>   EXPORT_SYMBOL(ieee802154_alloc_hw);
>>   
>> +void ieee802154_configure_durations(struct wpan_phy *phy)
>> +{
>> +	u32 duration = 0;
>> +
>> +	switch (phy->current_page) {
>> +	case 0:
>> +		if (BIT(phy->current_page) & 0x1)
> 
> I am very sorry to spot this only now but this is wrong.
> 
> all the conditions from here and below should be:
> 
> 		if (BIT(phy->current_channel & <mask>))
> 
> The masks look good, the durations as well, but the conditions are
> wrong.
> 
>> +			/* 868 MHz BPSK 802.15.4-2003: 20 ksym/s */
>> +			duration = 50 * NSEC_PER_USEC;
>> +		else if (phy->current_page & 0x7FE)
> 
> Ditto
> 
>> +			/* 915 MHz BPSK	802.15.4-2003: 40 ksym/s */
>> +			duration = 25 * NSEC_PER_USEC;
>> +		else if (phy->current_page & 0x7FFF800)
> 
> Ditto
> 
>> +			/* 2400 MHz O-QPSK 802.15.4-2006: 62.5 ksym/s */
>> +			duration = 16 * NSEC_PER_USEC;
>> +		break;
>> +	case 2:
>> +		if (BIT(phy->current_page) & 0x1)
> 
> Ditto
> 
>> +			/* 868 MHz O-QPSK 802.15.4-2006: 25 ksym/s */
>> +			duration = 40 * NSEC_PER_USEC;
>> +		else if (phy->current_page & 0x7FE)
> 
> Ditto
> 
>> +			/* 915 MHz O-QPSK 802.15.4-2006: 62.5 ksym/s */
>> +			duration = 16 * NSEC_PER_USEC;
>> +		break;
>> +	case 3:
>> +		if (BIT(phy->current_page) & 0x3FFF)
> 
> Ditto
> 
>> +			/* 2.4 GHz CSS 802.15.4a-2007: 1/6 Msym/s */
>> +			duration = 6 * NSEC_PER_USEC;
>> +		break;
> 
> I see it's "only" in wpan-next (781830c800dd "net: mac802154: Set
> durations automatically") and was not yet pulled in the net-next
> tree so please let me know what you prefer: I can either provide a
> proper patch to fit it (without upstream Fixes reference), or you can
> just apply this diff below and push -f the branch. Let me know what you
> prefer.

No forced push to a public branch in my public repo. Please provide a 
fixup patch that I can apply on top.

regards
Stefan Schmidt

Re: [PATCH wpan-next v3 3/4] net: mac802154: Set durations automatically

[Miquel Raynal]

miquel.raynal@bootlin.com wrote on Thu, 28 Apr 2022 17:58:38 +0200:

> Hi Stefan,
> 
> miquel.raynal@bootlin.com wrote on Tue,  1 Feb 2022 19:06:28 +0100:
> 
> > As depicted in the IEEE 802.15.4 specification, modulation/bands are
> > tight to a number of page/channels so we can for most of them derive the
> > durations automatically.
> > 
> > The two locations that must call this new helper to set the variou
> > symbol durations are:
> > - when manually requesting a channel change though the netlink interface
> > - at PHY creation, once the device driver has set the default
> >   page/channel
> > 
> > If an information is missing, the symbol duration is not touched, a
> > debug message is eventually printed. This keeps the compatibility with
> > the unconverted drivers for which it was too complicated for me to find
> > their precise information. If they initially provided a symbol duration,
> > it would be kept. If they don't, the symbol duration value is left
> > untouched.
> > 
> > Once the symbol duration derived, the lifs and sifs durations are
> > updated as well.
> > 
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  include/net/cfg802154.h |  2 ++
> >  net/mac802154/cfg.c     |  1 +
> >  net/mac802154/main.c    | 46 +++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 49 insertions(+)
> > 
> > diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h
> > index 8a4b6a50452f..49b4bcc24032 100644
> > --- a/include/net/cfg802154.h
> > +++ b/include/net/cfg802154.h
> > @@ -405,4 +405,6 @@ static inline const char *wpan_phy_name(struct wpan_phy *phy)
> >  	return dev_name(&phy->dev);
> >  }
> >  
> > +void ieee802154_configure_durations(struct wpan_phy *phy);
> > +
> >  #endif /* __NET_CFG802154_H */
> > diff --git a/net/mac802154/cfg.c b/net/mac802154/cfg.c
> > index fbeebe3bc31d..1e4a9f74ed43 100644
> > --- a/net/mac802154/cfg.c
> > +++ b/net/mac802154/cfg.c
> > @@ -118,6 +118,7 @@ ieee802154_set_channel(struct wpan_phy *wpan_phy, u8 page, u8 channel)
> >  	if (!ret) {
> >  		wpan_phy->current_page = page;
> >  		wpan_phy->current_channel = channel;
> > +		ieee802154_configure_durations(wpan_phy);
> >  	}
> >  
> >  	return ret;
> > diff --git a/net/mac802154/main.c b/net/mac802154/main.c
> > index 53153367f9d0..5546ef86e231 100644
> > --- a/net/mac802154/main.c
> > +++ b/net/mac802154/main.c
> > @@ -113,6 +113,50 @@ ieee802154_alloc_hw(size_t priv_data_len, const struct ieee802154_ops *ops)
> >  }
> >  EXPORT_SYMBOL(ieee802154_alloc_hw);
> >  
> > +void ieee802154_configure_durations(struct wpan_phy *phy)
> > +{
> > +	u32 duration = 0;
> > +
> > +	switch (phy->current_page) {
> > +	case 0:
> > +		if (BIT(phy->current_page) & 0x1)  
> 
> I am very sorry to spot this only now but this is wrong. 
> 
> all the conditions from here and below should be:
> 
> 		if (BIT(phy->current_channel & <mask>))
> 
> The masks look good, the durations as well, but the conditions are
> wrong.
> 
> > +			/* 868 MHz BPSK 802.15.4-2003: 20 ksym/s */
> > +			duration = 50 * NSEC_PER_USEC;
> > +		else if (phy->current_page & 0x7FE)  
> 
> Ditto
> 
> > +			/* 915 MHz BPSK	802.15.4-2003: 40 ksym/s */
> > +			duration = 25 * NSEC_PER_USEC;
> > +		else if (phy->current_page & 0x7FFF800)  
> 
> Ditto
> 
> > +			/* 2400 MHz O-QPSK 802.15.4-2006: 62.5 ksym/s */
> > +			duration = 16 * NSEC_PER_USEC;
> > +		break;
> > +	case 2:
> > +		if (BIT(phy->current_page) & 0x1)  
> 
> Ditto
> 
> > +			/* 868 MHz O-QPSK 802.15.4-2006: 25 ksym/s */
> > +			duration = 40 * NSEC_PER_USEC;
> > +		else if (phy->current_page & 0x7FE)  
> 
> Ditto
> 
> > +			/* 915 MHz O-QPSK 802.15.4-2006: 62.5 ksym/s */
> > +			duration = 16 * NSEC_PER_USEC;
> > +		break;
> > +	case 3:
> > +		if (BIT(phy->current_page) & 0x3FFF)  
> 
> Ditto
> 
> > +			/* 2.4 GHz CSS 802.15.4a-2007: 1/6 Msym/s */
> > +			duration = 6 * NSEC_PER_USEC;
> > +		break;  
> 
> I see it's "only" in wpan-next (781830c800dd "net: mac802154: Set
> durations automatically") and was not yet pulled in the net-next
> tree so please let me know what you prefer: I can either provide a
> proper patch to fit it (without upstream Fixes reference), or you can
> just apply this diff below and push -f the branch. Let me know what you
> prefer.
> 
> Again, sorry to only see this now.
> 
> Thanks,
> Miquèl
> 
> commit 4122765e5f982ed8f0ccea5fd813ef4d53d20a90 (HEAD)
> Author: Miquel Raynal <miquel.raynal@bootlin.com>
> Date:   Thu Apr 28 17:57:59 2022 +0200
> 
>     fixup! net: mac802154: Set durations automatically

And I keep having it wrong. Here is the right fixup!...

commit 676aa77f1c279a90f521c1c05757c702e1538472 (HEAD)
Author: Miquel Raynal <miquel.raynal@bootlin.com>
Date:   Thu Apr 28 17:57:59 2022 +0200

    fixup! net: mac802154: Set durations automatically

diff --git a/net/mac802154/main.c b/net/mac802154/main.c
index 5546ef86e231..bd7bdb1219dd 100644
--- a/net/mac802154/main.c
+++ b/net/mac802154/main.c
@@ -119,26 +119,26 @@ void ieee802154_configure_durations(struct wpan_phy *phy)
 
        switch (phy->current_page) {
        case 0:
-               if (BIT(phy->current_page) & 0x1)
+               if (BIT(phy->current_channel) & 0x1)
                        /* 868 MHz BPSK 802.15.4-2003: 20 ksym/s */
                        duration = 50 * NSEC_PER_USEC;
-               else if (phy->current_page & 0x7FE)
+               else if (BIT(phy->current_channel) & 0x7FE)
                        /* 915 MHz BPSK 802.15.4-2003: 40 ksym/s */
                        duration = 25 * NSEC_PER_USEC;
-               else if (phy->current_page & 0x7FFF800)
+               else if (BIT(phy->current_channel) & 0x7FFF800)
                        /* 2400 MHz O-QPSK 802.15.4-2006: 62.5 ksym/s */
                        duration = 16 * NSEC_PER_USEC;
                break;
        case 2:
-               if (BIT(phy->current_page) & 0x1)
+               if (BIT(phy->current_channel) & 0x1)
                        /* 868 MHz O-QPSK 802.15.4-2006: 25 ksym/s */
                        duration = 40 * NSEC_PER_USEC;
-               else if (phy->current_page & 0x7FE)
+               else if (BIT(phy->current_channel) & 0x7FE)
                        /* 915 MHz O-QPSK 802.15.4-2006: 62.5 ksym/s */
                        duration = 16 * NSEC_PER_USEC;
                break;
        case 3:
-               if (BIT(phy->current_page) & 0x3FFF)
+               if (BIT(phy->current_channel) & 0x3FFF)
                        /* 2.4 GHz CSS 802.15.4a-2007: 1/6 Msym/s */
                        duration = 6 * NSEC_PER_USEC;
                break;

Re: [PATCH wpan-next v3 3/4] net: mac802154: Set durations automatically

[Miquel Raynal]

Hi Stefan,

> > I see it's "only" in wpan-next (781830c800dd "net: mac802154: Set
> > durations automatically") and was not yet pulled in the net-next
> > tree so please let me know what you prefer: I can either provide a
> > proper patch to fit it (without upstream Fixes reference), or you can
> > just apply this diff below and push -f the branch. Let me know what you
> > prefer.  
> 
> No forced push to a public branch in my public repo. Please provide a fixup patch that I can apply on top.

Duly noted.

Thanks,
Miquèl