linux-wpan.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH wpan-next v2 0/5] ieee802154: Improve durations handling
@ 2022-01-28 11:08 Miquel Raynal
  2022-01-28 11:08 ` [PATCH wpan-next v2 1/5] net: ieee802154: Improve the way supported channels are declared Miquel Raynal
                   ` (5 more replies)
  0 siblings, 6 replies; 24+ messages in thread
From: Miquel Raynal @ 2022-01-28 11:08 UTC (permalink / raw)
  To: Alexander Aring, Stefan Schmidt, linux-wpan
  Cc: David S. Miller, Jakub Kicinski, netdev, Michael Hennerich,
	Varka Bhadram, Xue Liu, Alan Ott, Miquel Raynal

These paches 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.

Miquel Raynal (5):
  net: ieee802154: Improve the way supported channels are declared
  net: ieee802154: Give more details to the core about the channel
    configurations
  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/adf7242.c         |   3 +-
 drivers/net/ieee802154/at86rf230.c       |  66 ++++++-------
 drivers/net/ieee802154/atusb.c           |  66 ++++++-------
 drivers/net/ieee802154/ca8210.c          |   7 +-
 drivers/net/ieee802154/cc2520.c          |   3 +-
 drivers/net/ieee802154/fakelb.c          |  43 ++++++---
 drivers/net/ieee802154/mac802154_hwsim.c |  76 ++++++++++++---
 drivers/net/ieee802154/mcr20a.c          |  11 +--
 drivers/net/ieee802154/mrf24j40.c        |   3 +-
 include/net/cfg802154.h                  |  60 +++++++++++-
 net/ieee802154/core.h                    |   2 +
 net/ieee802154/nl-phy.c                  |   8 +-
 net/ieee802154/nl802154.c                |  30 ++++--
 net/mac802154/cfg.c                      |   1 +
 net/mac802154/main.c                     | 113 ++++++++++++++++++++++-
 15 files changed, 361 insertions(+), 131 deletions(-)

-- 
2.27.0


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH wpan-next v2 1/5] net: ieee802154: Improve the way supported channels are declared
  2022-01-28 11:08 [PATCH wpan-next v2 0/5] ieee802154: Improve durations handling Miquel Raynal
@ 2022-01-28 11:08 ` Miquel Raynal
  2022-01-30 21:35   ` Alexander Aring
  2022-01-28 11:08 ` [PATCH wpan-next v2 2/5] net: ieee802154: Give more details to the core about the channel configurations Miquel Raynal
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 24+ messages in thread
From: Miquel Raynal @ 2022-01-28 11:08 UTC (permalink / raw)
  To: Alexander Aring, Stefan Schmidt, linux-wpan
  Cc: David S. Miller, Jakub Kicinski, netdev, Michael Hennerich,
	Varka Bhadram, Xue Liu, Alan Ott, Miquel Raynal

The idea here is to create a structure per set of channels so that we
can define much more than basic bitfields for these.

The structure is currently almost empty on purpose because this change
is supposed to be a mechanical update without additional information but
more details will be added in the following commits.

For each page, the core now has access to how many "chunks" of channels
are defined. Overall up to only 3 chunks have been defined so let's
hardcode this value to simplify a lot the handling. Then, for each
chunk, we define an independent bitfield of channels. As there are
several users of these bitfields, we also create the
cfg802154_get_supported_chans() helper to reconstruct the bitfield as it
was before when the only information that matters is identifying the
supported/unsupported channels.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/net/ieee802154/adf7242.c         |  3 +-
 drivers/net/ieee802154/at86rf230.c       | 12 ++++---
 drivers/net/ieee802154/atusb.c           | 12 ++++---
 drivers/net/ieee802154/ca8210.c          |  3 +-
 drivers/net/ieee802154/cc2520.c          |  3 +-
 drivers/net/ieee802154/fakelb.c          | 43 +++++++++++++++---------
 drivers/net/ieee802154/mac802154_hwsim.c | 43 +++++++++++++++---------
 drivers/net/ieee802154/mcr20a.c          |  3 +-
 drivers/net/ieee802154/mrf24j40.c        |  3 +-
 include/net/cfg802154.h                  | 13 +++++--
 net/ieee802154/core.h                    |  2 ++
 net/ieee802154/nl-phy.c                  |  8 +++--
 net/ieee802154/nl802154.c                | 30 +++++++++++++----
 13 files changed, 125 insertions(+), 53 deletions(-)

diff --git a/drivers/net/ieee802154/adf7242.c b/drivers/net/ieee802154/adf7242.c
index 7db9cbd0f5de..40c77a643b78 100644
--- a/drivers/net/ieee802154/adf7242.c
+++ b/drivers/net/ieee802154/adf7242.c
@@ -1211,7 +1211,8 @@ static int adf7242_probe(struct spi_device *spi)
 	hw->extra_tx_headroom = 0;
 
 	/* We support only 2.4 Ghz */
-	hw->phy->supported.channels[0] = 0x7FFF800;
+	hw->phy->supported.page[0].nchunks = 1;
+	hw->phy->supported.page[0].chunk[0].channels = 0x7FFF800;
 
 	hw->flags = IEEE802154_HW_OMIT_CKSUM |
 		    IEEE802154_HW_CSMA_PARAMS |
diff --git a/drivers/net/ieee802154/at86rf230.c b/drivers/net/ieee802154/at86rf230.c
index 4f5ef8a9a9a8..d3a16c2c7e37 100644
--- a/drivers/net/ieee802154/at86rf230.c
+++ b/drivers/net/ieee802154/at86rf230.c
@@ -1567,7 +1567,8 @@ at86rf230_detect_device(struct at86rf230_local *lp)
 	case 3:
 		chip = "at86rf231";
 		lp->data = &at86rf231_data;
-		lp->hw->phy->supported.channels[0] = 0x7FFF800;
+		lp->hw->phy->supported.page[0].nchunks = 1;
+		lp->hw->phy->supported.page[0].chunk[0].channels = 0x7FFF800;
 		lp->hw->phy->current_channel = 11;
 		lp->hw->phy->symbol_duration = 16;
 		lp->hw->phy->supported.tx_powers = at86rf231_powers;
@@ -1579,8 +1580,10 @@ at86rf230_detect_device(struct at86rf230_local *lp)
 		chip = "at86rf212";
 		lp->data = &at86rf212_data;
 		lp->hw->flags |= IEEE802154_HW_LBT;
-		lp->hw->phy->supported.channels[0] = 0x00007FF;
-		lp->hw->phy->supported.channels[2] = 0x00007FF;
+		lp->hw->phy->supported.page[0].nchunks = 1;
+		lp->hw->phy->supported.page[0].chunk[0].channels = 0x00007FF;
+		lp->hw->phy->supported.page[2].nchunks = 1;
+		lp->hw->phy->supported.page[2].chunk[0].channels = 0x00007FF;
 		lp->hw->phy->current_channel = 5;
 		lp->hw->phy->symbol_duration = 25;
 		lp->hw->phy->supported.lbt = NL802154_SUPPORTED_BOOL_BOTH;
@@ -1592,7 +1595,8 @@ at86rf230_detect_device(struct at86rf230_local *lp)
 	case 11:
 		chip = "at86rf233";
 		lp->data = &at86rf233_data;
-		lp->hw->phy->supported.channels[0] = 0x7FFF800;
+		lp->hw->phy->supported.page[0].nchunks = 1;
+		lp->hw->phy->supported.page[0].chunk[0].channels = 0x7FFF800;
 		lp->hw->phy->current_channel = 13;
 		lp->hw->phy->symbol_duration = 16;
 		lp->hw->phy->supported.tx_powers = at86rf233_powers;
diff --git a/drivers/net/ieee802154/atusb.c b/drivers/net/ieee802154/atusb.c
index 23ee0b14cbfa..38ebfacf2698 100644
--- a/drivers/net/ieee802154/atusb.c
+++ b/drivers/net/ieee802154/atusb.c
@@ -914,7 +914,8 @@ static int atusb_get_and_conf_chip(struct atusb *atusb)
 	switch (part_num) {
 	case 2:
 		chip = "AT86RF230";
-		atusb->hw->phy->supported.channels[0] = 0x7FFF800;
+		atusb->hw->phy->supported.page[0].nchunks = 1;
+		atusb->hw->phy->supported.page[0].chunk[0].channels = 0x7FFF800;
 		atusb->hw->phy->current_channel = 11;	/* reset default */
 		atusb->hw->phy->symbol_duration = 16;
 		atusb->hw->phy->supported.tx_powers = atusb_powers;
@@ -924,7 +925,8 @@ static int atusb_get_and_conf_chip(struct atusb *atusb)
 		break;
 	case 3:
 		chip = "AT86RF231";
-		atusb->hw->phy->supported.channels[0] = 0x7FFF800;
+		atusb->hw->phy->supported.page[0].nchunks = 1;
+		atusb->hw->phy->supported.page[0].chunk[0].channels = 0x7FFF800;
 		atusb->hw->phy->current_channel = 11;	/* reset default */
 		atusb->hw->phy->symbol_duration = 16;
 		atusb->hw->phy->supported.tx_powers = atusb_powers;
@@ -935,8 +937,10 @@ static int atusb_get_and_conf_chip(struct atusb *atusb)
 	case 7:
 		chip = "AT86RF212";
 		atusb->hw->flags |= IEEE802154_HW_LBT;
-		atusb->hw->phy->supported.channels[0] = 0x00007FF;
-		atusb->hw->phy->supported.channels[2] = 0x00007FF;
+		atusb->hw->phy->supported.page[0].nchunks = 1;
+		atusb->hw->phy->supported.page[0].chunk[0].channels = 0x00007FF;
+		atusb->hw->phy->supported.page[2].nchunks = 1;
+		atusb->hw->phy->supported.page[2].chunk[0].channels = 0x00007FF;
 		atusb->hw->phy->current_channel = 5;
 		atusb->hw->phy->symbol_duration = 25;
 		atusb->hw->phy->supported.lbt = NL802154_SUPPORTED_BOOL_BOTH;
diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
index f3438d3e104a..8e6545bbbe5d 100644
--- a/drivers/net/ieee802154/ca8210.c
+++ b/drivers/net/ieee802154/ca8210.c
@@ -2963,7 +2963,8 @@ static const s32 ca8210_ed_levels[CA8210_MAX_ED_LEVELS] = {
 static void ca8210_hw_setup(struct ieee802154_hw *ca8210_hw)
 {
 	/* Support channels 11-26 */
-	ca8210_hw->phy->supported.channels[0] = CA8210_VALID_CHANNELS;
+	ca8210_hw->phy->supported.page[0].nchunks = 1;
+	ca8210_hw->phy->supported.page[0].chunk[0].channels = CA8210_VALID_CHANNELS;
 	ca8210_hw->phy->supported.tx_powers_size = CA8210_MAX_TX_POWERS;
 	ca8210_hw->phy->supported.tx_powers = ca8210_tx_powers;
 	ca8210_hw->phy->supported.cca_ed_levels_size = CA8210_MAX_ED_LEVELS;
diff --git a/drivers/net/ieee802154/cc2520.c b/drivers/net/ieee802154/cc2520.c
index 89c046b204e0..587f050ef4e8 100644
--- a/drivers/net/ieee802154/cc2520.c
+++ b/drivers/net/ieee802154/cc2520.c
@@ -836,7 +836,8 @@ static int cc2520_register(struct cc2520_private *priv)
 	ieee802154_random_extended_addr(&priv->hw->phy->perm_extended_addr);
 
 	/* We do support only 2.4 Ghz */
-	priv->hw->phy->supported.channels[0] = 0x7FFF800;
+	priv->hw->phy->supported.page[0].nchunks = 1;
+	priv->hw->phy->supported.page[0].chunk[0].channels = 0x7FFF800;
 	priv->hw->flags = IEEE802154_HW_TX_OMIT_CKSUM | IEEE802154_HW_AFILT |
 			  IEEE802154_HW_PROMISCUOUS;
 
diff --git a/drivers/net/ieee802154/fakelb.c b/drivers/net/ieee802154/fakelb.c
index 523d13ee02bf..bc44d1f7551c 100644
--- a/drivers/net/ieee802154/fakelb.c
+++ b/drivers/net/ieee802154/fakelb.c
@@ -137,36 +137,49 @@ static int fakelb_add_one(struct device *dev)
 	phy = hw->priv;
 	phy->hw = hw;
 
+	hw->phy->supported.page[0].nchunks = 3;
 	/* 868 MHz BPSK	802.15.4-2003 */
-	hw->phy->supported.channels[0] |= 1;
+	hw->phy->supported.page[0].chunk[0].channels |= 1;
 	/* 915 MHz BPSK	802.15.4-2003 */
-	hw->phy->supported.channels[0] |= 0x7fe;
+	hw->phy->supported.page[0].chunk[1].channels |= 0x7fe;
 	/* 2.4 GHz O-QPSK 802.15.4-2003 */
-	hw->phy->supported.channels[0] |= 0x7FFF800;
+	hw->phy->supported.page[0].chunk[2].channels |= 0x7FFF800;
+
+	hw->phy->supported.page[1].nchunks = 2;
 	/* 868 MHz ASK 802.15.4-2006 */
-	hw->phy->supported.channels[1] |= 1;
+	hw->phy->supported.page[1].chunk[0].channels |= 1;
 	/* 915 MHz ASK 802.15.4-2006 */
-	hw->phy->supported.channels[1] |= 0x7fe;
+	hw->phy->supported.page[1].chunk[1].channels |= 0x7fe;
+
+	hw->phy->supported.page[2].nchunks = 2;
 	/* 868 MHz O-QPSK 802.15.4-2006 */
-	hw->phy->supported.channels[2] |= 1;
+	hw->phy->supported.page[2].chunk[0].channels |= 1;
 	/* 915 MHz O-QPSK 802.15.4-2006 */
-	hw->phy->supported.channels[2] |= 0x7fe;
+	hw->phy->supported.page[2].chunk[1].channels |= 0x7fe;
+
+	hw->phy->supported.page[3].nchunks = 1;
 	/* 2.4 GHz CSS 802.15.4a-2007 */
-	hw->phy->supported.channels[3] |= 0x3fff;
+	hw->phy->supported.page[3].chunk[0].channels |= 0x3fff;
+
+	hw->phy->supported.page[4].nchunks = 3;
 	/* UWB Sub-gigahertz 802.15.4a-2007 */
-	hw->phy->supported.channels[4] |= 1;
+	hw->phy->supported.page[4].chunk[0].channels |= 1;
 	/* UWB Low band 802.15.4a-2007 */
-	hw->phy->supported.channels[4] |= 0x1e;
+	hw->phy->supported.page[4].chunk[1].channels |= 0x1e;
 	/* UWB High band 802.15.4a-2007 */
-	hw->phy->supported.channels[4] |= 0xffe0;
+	hw->phy->supported.page[4].chunk[2].channels |= 0xffe0;
+
+	hw->phy->supported.page[5].nchunks = 2;
 	/* 750 MHz O-QPSK 802.15.4c-2009 */
-	hw->phy->supported.channels[5] |= 0xf;
+	hw->phy->supported.page[5].chunk[0].channels |= 0xf;
 	/* 750 MHz MPSK 802.15.4c-2009 */
-	hw->phy->supported.channels[5] |= 0xf0;
+	hw->phy->supported.page[5].chunk[1].channels |= 0xf0;
+
+	hw->phy->supported.page[6].nchunks = 2;
 	/* 950 MHz BPSK 802.15.4d-2009 */
-	hw->phy->supported.channels[6] |= 0x3ff;
+	hw->phy->supported.page[6].chunk[0].channels |= 0x3ff;
 	/* 950 MHz GFSK 802.15.4d-2009 */
-	hw->phy->supported.channels[6] |= 0x3ffc00;
+	hw->phy->supported.page[6].chunk[1].channels |= 0x3ffc00;
 
 	ieee802154_random_extended_addr(&hw->phy->perm_extended_addr);
 	/* fake phy channel 13 as default */
diff --git a/drivers/net/ieee802154/mac802154_hwsim.c b/drivers/net/ieee802154/mac802154_hwsim.c
index be77f489be13..8d99ed383b58 100644
--- a/drivers/net/ieee802154/mac802154_hwsim.c
+++ b/drivers/net/ieee802154/mac802154_hwsim.c
@@ -745,36 +745,49 @@ static int hwsim_add_one(struct genl_info *info, struct device *dev,
 	phy = hw->priv;
 	phy->hw = hw;
 
+	hw->phy->supported.page[0].nchunks = 3;
 	/* 868 MHz BPSK	802.15.4-2003 */
-	hw->phy->supported.channels[0] |= 1;
+	hw->phy->supported.page[0].chunk[0].channels |= 1;
 	/* 915 MHz BPSK	802.15.4-2003 */
-	hw->phy->supported.channels[0] |= 0x7fe;
+	hw->phy->supported.page[0].chunk[1].channels |= 0x7fe;
 	/* 2.4 GHz O-QPSK 802.15.4-2003 */
-	hw->phy->supported.channels[0] |= 0x7FFF800;
+	hw->phy->supported.page[0].chunk[2].channels |= 0x7FFF800;
+
+	hw->phy->supported.page[1].nchunks = 2;
 	/* 868 MHz ASK 802.15.4-2006 */
-	hw->phy->supported.channels[1] |= 1;
+	hw->phy->supported.page[1].chunk[0].channels |= 1;
 	/* 915 MHz ASK 802.15.4-2006 */
-	hw->phy->supported.channels[1] |= 0x7fe;
+	hw->phy->supported.page[1].chunk[1].channels |= 0x7fe;
+
+	hw->phy->supported.page[2].nchunks = 2;
 	/* 868 MHz O-QPSK 802.15.4-2006 */
-	hw->phy->supported.channels[2] |= 1;
+	hw->phy->supported.page[2].chunk[0].channels |= 1;
 	/* 915 MHz O-QPSK 802.15.4-2006 */
-	hw->phy->supported.channels[2] |= 0x7fe;
+	hw->phy->supported.page[2].chunk[1].channels |= 0x7fe;
+
+	hw->phy->supported.page[3].nchunks = 1;
 	/* 2.4 GHz CSS 802.15.4a-2007 */
-	hw->phy->supported.channels[3] |= 0x3fff;
+	hw->phy->supported.page[3].chunk[0].channels |= 0x3fff;
+
+	hw->phy->supported.page[4].nchunks = 3;
 	/* UWB Sub-gigahertz 802.15.4a-2007 */
-	hw->phy->supported.channels[4] |= 1;
+	hw->phy->supported.page[4].chunk[0].channels |= 1;
 	/* UWB Low band 802.15.4a-2007 */
-	hw->phy->supported.channels[4] |= 0x1e;
+	hw->phy->supported.page[4].chunk[1].channels |= 0x1e;
 	/* UWB High band 802.15.4a-2007 */
-	hw->phy->supported.channels[4] |= 0xffe0;
+	hw->phy->supported.page[4].chunk[2].channels |= 0xffe0;
+
+	hw->phy->supported.page[5].nchunks = 2;
 	/* 750 MHz O-QPSK 802.15.4c-2009 */
-	hw->phy->supported.channels[5] |= 0xf;
+	hw->phy->supported.page[5].chunk[0].channels |= 0xf;
 	/* 750 MHz MPSK 802.15.4c-2009 */
-	hw->phy->supported.channels[5] |= 0xf0;
+	hw->phy->supported.page[5].chunk[1].channels |= 0xf0;
+
+	hw->phy->supported.page[6].nchunks = 2;
 	/* 950 MHz BPSK 802.15.4d-2009 */
-	hw->phy->supported.channels[6] |= 0x3ff;
+	hw->phy->supported.page[6].chunk[0].channels |= 0x3ff;
 	/* 950 MHz GFSK 802.15.4d-2009 */
-	hw->phy->supported.channels[6] |= 0x3ffc00;
+	hw->phy->supported.page[6].chunk[1].channels |= 0x3ffc00;
 
 	ieee802154_random_extended_addr(&hw->phy->perm_extended_addr);
 
diff --git a/drivers/net/ieee802154/mcr20a.c b/drivers/net/ieee802154/mcr20a.c
index 383231b85464..80bed905acf9 100644
--- a/drivers/net/ieee802154/mcr20a.c
+++ b/drivers/net/ieee802154/mcr20a.c
@@ -1002,7 +1002,8 @@ static void mcr20a_hw_setup(struct mcr20a_local *lp)
 
 	phy->cca.mode = NL802154_CCA_ENERGY;
 
-	phy->supported.channels[0] = MCR20A_VALID_CHANNELS;
+	phy->supported.page[0].nchunks = 1;
+	phy->supported.page[0].chunk[0].channels = MCR20A_VALID_CHANNELS;
 	phy->current_page = 0;
 	/* MCR20A default reset value */
 	phy->current_channel = 20;
diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c
index ff83e00b77af..5a38f3077771 100644
--- a/drivers/net/ieee802154/mrf24j40.c
+++ b/drivers/net/ieee802154/mrf24j40.c
@@ -1287,7 +1287,8 @@ static int mrf24j40_probe(struct spi_device *spi)
 	spi_set_drvdata(spi, devrec);
 	devrec->hw = hw;
 	devrec->hw->parent = &spi->dev;
-	devrec->hw->phy->supported.channels[0] = CHANNEL_MASK;
+	devrec->hw->phy->supported.page[0].nchunks = 1;
+	devrec->hw->phy->supported.page[0].chunk[0].channels = CHANNEL_MASK;
 	devrec->hw->flags = IEEE802154_HW_TX_OMIT_CKSUM | IEEE802154_HW_AFILT |
 			    IEEE802154_HW_CSMA_PARAMS |
 			    IEEE802154_HW_PROMISCUOUS;
diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h
index 6ed07844eb24..ef49a23801c6 100644
--- a/include/net/cfg802154.h
+++ b/include/net/cfg802154.h
@@ -131,9 +131,18 @@ wpan_phy_supported_bool(bool b, enum nl802154_supported_bool_states st)
 	return false;
 }
 
+struct phy_channels {
+	u32 channels;
+};
+
+struct phy_page {
+	unsigned int nchunks;
+	struct phy_channels chunk[3];
+};
+
 struct wpan_phy_supported {
-	u32 channels[IEEE802154_MAX_PAGE + 1],
-	    cca_modes, cca_opts, iftypes;
+	struct phy_page page[IEEE802154_MAX_PAGE + 1];
+	u32 cca_modes, cca_opts, iftypes;
 	enum nl802154_supported_bool_states lbt;
 	u8 min_minbe, max_minbe, min_maxbe, max_maxbe,
 	   min_csma_backoffs, max_csma_backoffs;
diff --git a/net/ieee802154/core.h b/net/ieee802154/core.h
index 1c19f575d574..a0cf6feffc6a 100644
--- a/net/ieee802154/core.h
+++ b/net/ieee802154/core.h
@@ -47,4 +47,6 @@ struct cfg802154_registered_device *
 cfg802154_rdev_by_wpan_phy_idx(int wpan_phy_idx);
 struct wpan_phy *wpan_phy_idx_to_wpan_phy(int wpan_phy_idx);
 
+u32 cfg802154_get_supported_chans(struct wpan_phy *phy, unsigned int page);
+
 #endif /* __IEEE802154_CORE_H */
diff --git a/net/ieee802154/nl-phy.c b/net/ieee802154/nl-phy.c
index 359249ab77bf..c62040f3b4e0 100644
--- a/net/ieee802154/nl-phy.c
+++ b/net/ieee802154/nl-phy.c
@@ -31,6 +31,7 @@ static int ieee802154_nl_fill_phy(struct sk_buff *msg, u32 portid,
 	void *hdr;
 	int i, pages = 0;
 	u32 *buf = kcalloc(IEEE802154_MAX_PAGE + 1, sizeof(u32), GFP_KERNEL);
+	u32 chans;
 
 	pr_debug("%s\n", __func__);
 
@@ -48,8 +49,11 @@ static int ieee802154_nl_fill_phy(struct sk_buff *msg, u32 portid,
 	    nla_put_u8(msg, IEEE802154_ATTR_CHANNEL, phy->current_channel))
 		goto nla_put_failure;
 	for (i = 0; i <= IEEE802154_MAX_PAGE; i++) {
-		if (phy->supported.channels[i])
-			buf[pages++] = phy->supported.channels[i] | (i << 27);
+		chans = cfg802154_get_supported_chans(phy, i);
+		if (!chans)
+			continue;
+
+		buf[pages++] = chans | (i << 27);
 	}
 	if (pages &&
 	    nla_put(msg, IEEE802154_ATTR_CHANNEL_PAGE_LIST,
diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
index e0b072aecf0f..bd1015611a7e 100644
--- a/net/ieee802154/nl802154.c
+++ b/net/ieee802154/nl802154.c
@@ -320,6 +320,21 @@ nl802154_put_flags(struct sk_buff *msg, int attr, u32 mask)
 	return 0;
 }
 
+u32 cfg802154_get_supported_chans(struct wpan_phy *phy, unsigned int page)
+{
+	struct phy_page *ppage;
+	unsigned int chunk;
+	u32 supported = 0;
+
+	ppage = &phy->supported.page[page];
+
+	for (chunk = 0; chunk <= ppage->nchunks; chunk++)
+		supported |= ppage->chunk[chunk].channels;
+
+	return supported;
+}
+EXPORT_SYMBOL(cfg802154_get_supported_chans);
+
 static int
 nl802154_send_wpan_phy_channels(struct cfg802154_registered_device *rdev,
 				struct sk_buff *msg)
@@ -333,7 +348,7 @@ nl802154_send_wpan_phy_channels(struct cfg802154_registered_device *rdev,
 
 	for (page = 0; page <= IEEE802154_MAX_PAGE; page++) {
 		if (nla_put_u32(msg, NL802154_ATTR_SUPPORTED_CHANNEL,
-				rdev->wpan_phy.supported.channels[page]))
+				cfg802154_get_supported_chans(&rdev->wpan_phy, page)))
 			return -ENOBUFS;
 	}
 	nla_nest_end(msg, nl_page);
@@ -347,6 +362,7 @@ nl802154_put_capabilities(struct sk_buff *msg,
 {
 	const struct wpan_phy_supported *caps = &rdev->wpan_phy.supported;
 	struct nlattr *nl_caps, *nl_channels;
+	u32 chans;
 	int i;
 
 	nl_caps = nla_nest_start_noflag(msg, NL802154_ATTR_WPAN_PHY_CAPS);
@@ -358,10 +374,12 @@ nl802154_put_capabilities(struct sk_buff *msg,
 		return -ENOBUFS;
 
 	for (i = 0; i <= IEEE802154_MAX_PAGE; i++) {
-		if (caps->channels[i]) {
-			if (nl802154_put_flags(msg, i, caps->channels[i]))
-				return -ENOBUFS;
-		}
+		chans = cfg802154_get_supported_chans(&rdev->wpan_phy, i);
+		if (!chans)
+			continue;
+
+		if (nl802154_put_flags(msg, i, chans))
+			return -ENOBUFS;
 	}
 
 	nla_nest_end(msg, nl_channels);
@@ -965,7 +983,7 @@ static int nl802154_set_channel(struct sk_buff *skb, struct genl_info *info)
 
 	/* check 802.15.4 constraints */
 	if (page > IEEE802154_MAX_PAGE || channel > IEEE802154_MAX_CHANNEL ||
-	    !(rdev->wpan_phy.supported.channels[page] & BIT(channel)))
+	    !(cfg802154_get_supported_chans(&rdev->wpan_phy, page) & BIT(channel)))
 		return -EINVAL;
 
 	return rdev_set_channel(rdev, page, channel);
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH wpan-next v2 2/5] net: ieee802154: Give more details to the core about the channel configurations
  2022-01-28 11:08 [PATCH wpan-next v2 0/5] ieee802154: Improve durations handling Miquel Raynal
  2022-01-28 11:08 ` [PATCH wpan-next v2 1/5] net: ieee802154: Improve the way supported channels are declared Miquel Raynal
@ 2022-01-28 11:08 ` Miquel Raynal
  2022-01-28 11:08 ` [PATCH wpan-next v2 3/5] net: mac802154: Convert the symbol duration into nanoseconds Miquel Raynal
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 24+ messages in thread
From: Miquel Raynal @ 2022-01-28 11:08 UTC (permalink / raw)
  To: Alexander Aring, Stefan Schmidt, linux-wpan
  Cc: David S. Miller, Jakub Kicinski, netdev, Michael Hennerich,
	Varka Bhadram, Xue Liu, Alan Ott, Miquel Raynal

In order to let the core derive eg. the symbol duration for a given
channel, it needs to know which protocol is being used, on which band,
and eventually more details such as the mean PRF in the case of UWB.

Create the necessary enumerations to declare all of that. Include them
in the currently-almost-empty phy_channels structure which until now
only declared the supported channels as bitfields.

The PRF is declared in a union as this clearly is a parameter that does
not apply to most of the protocols. It is very likely that other
parameters will get added in the future to further define specific
protocols and the union will likely be a good location for them.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/net/ieee802154/at86rf230.c       | 29 ++++++++++++++---
 drivers/net/ieee802154/atusb.c           | 29 ++++++++++++++---
 drivers/net/ieee802154/ca8210.c          |  3 ++
 drivers/net/ieee802154/mac802154_hwsim.c | 33 +++++++++++++++++++
 drivers/net/ieee802154/mcr20a.c          |  3 ++
 include/net/cfg802154.h                  | 41 ++++++++++++++++++++++++
 6 files changed, 130 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ieee802154/at86rf230.c b/drivers/net/ieee802154/at86rf230.c
index d3a16c2c7e37..d3dc03926246 100644
--- a/drivers/net/ieee802154/at86rf230.c
+++ b/drivers/net/ieee802154/at86rf230.c
@@ -1569,6 +1569,8 @@ at86rf230_detect_device(struct at86rf230_local *lp)
 		lp->data = &at86rf231_data;
 		lp->hw->phy->supported.page[0].nchunks = 1;
 		lp->hw->phy->supported.page[0].chunk[0].channels = 0x7FFF800;
+		lp->hw->phy->supported.page[0].chunk[0].protocol = IEEE802154_OQPSK_PHY;
+		lp->hw->phy->supported.page[0].chunk[0].band = IEEE802154_2400_MHZ_BAND;
 		lp->hw->phy->current_channel = 11;
 		lp->hw->phy->symbol_duration = 16;
 		lp->hw->phy->supported.tx_powers = at86rf231_powers;
@@ -1580,10 +1582,27 @@ at86rf230_detect_device(struct at86rf230_local *lp)
 		chip = "at86rf212";
 		lp->data = &at86rf212_data;
 		lp->hw->flags |= IEEE802154_HW_LBT;
-		lp->hw->phy->supported.page[0].nchunks = 1;
-		lp->hw->phy->supported.page[0].chunk[0].channels = 0x00007FF;
-		lp->hw->phy->supported.page[2].nchunks = 1;
-		lp->hw->phy->supported.page[2].chunk[0].channels = 0x00007FF;
+
+		lp->hw->phy->supported.page[0].nchunks = 2;
+		/* SUB:0 and BPSK:0 -> BPSK-20 */
+		lp->hw->phy->supported.page[0].chunk[0].channels = 1;
+		lp->hw->phy->supported.page[0].chunk[0].protocol = IEEE802154_BPSK_PHY;
+		lp->hw->phy->supported.page[0].chunk[0].band = IEEE802154_868_MHZ_BAND;
+		/* SUB:0 and BPSK:1 -> OQPSK-100/200/400 */
+		lp->hw->phy->supported.page[0].chunk[1].channels = 0x00007FE;
+		lp->hw->phy->supported.page[0].chunk[1].protocol = IEEE802154_OQPSK_PHY;
+		lp->hw->phy->supported.page[0].chunk[1].band = IEEE802154_868_MHZ_BAND;
+
+		lp->hw->phy->supported.page[2].nchunks = 2;
+		/* SUB:1 and BPSK:0 -> BPSK-40 */
+		lp->hw->phy->supported.page[2].chunk[0].channels = 1;
+		lp->hw->phy->supported.page[2].chunk[0].protocol = IEEE802154_BPSK_PHY;
+		lp->hw->phy->supported.page[2].chunk[0].band = IEEE802154_915_MHZ_BAND;
+		/* SUB:1 and BPSK:1 -> OQPSK-250/500/1000 */
+		lp->hw->phy->supported.page[2].chunk[1].channels = 0x00007FE;
+		lp->hw->phy->supported.page[2].chunk[1].protocol = IEEE802154_OQPSK_PHY;
+		lp->hw->phy->supported.page[2].chunk[1].band = IEEE802154_915_MHZ_BAND;
+
 		lp->hw->phy->current_channel = 5;
 		lp->hw->phy->symbol_duration = 25;
 		lp->hw->phy->supported.lbt = NL802154_SUPPORTED_BOOL_BOTH;
@@ -1597,6 +1616,8 @@ at86rf230_detect_device(struct at86rf230_local *lp)
 		lp->data = &at86rf233_data;
 		lp->hw->phy->supported.page[0].nchunks = 1;
 		lp->hw->phy->supported.page[0].chunk[0].channels = 0x7FFF800;
+		lp->hw->phy->supported.page[0].chunk[0].protocol = IEEE802154_OQPSK_PHY;
+		lp->hw->phy->supported.page[0].chunk[0].band = IEEE802154_2400_MHZ_BAND;
 		lp->hw->phy->current_channel = 13;
 		lp->hw->phy->symbol_duration = 16;
 		lp->hw->phy->supported.tx_powers = at86rf233_powers;
diff --git a/drivers/net/ieee802154/atusb.c b/drivers/net/ieee802154/atusb.c
index 38ebfacf2698..7e8c9d6db7d7 100644
--- a/drivers/net/ieee802154/atusb.c
+++ b/drivers/net/ieee802154/atusb.c
@@ -916,6 +916,8 @@ static int atusb_get_and_conf_chip(struct atusb *atusb)
 		chip = "AT86RF230";
 		atusb->hw->phy->supported.page[0].nchunks = 1;
 		atusb->hw->phy->supported.page[0].chunk[0].channels = 0x7FFF800;
+		atusb->hw->phy->supported.page[0].chunk[0].protocol = IEEE802154_OQPSK_PHY;
+		atusb->hw->phy->supported.page[0].chunk[0].band = IEEE802154_2400_MHZ_BAND;
 		atusb->hw->phy->current_channel = 11;	/* reset default */
 		atusb->hw->phy->symbol_duration = 16;
 		atusb->hw->phy->supported.tx_powers = atusb_powers;
@@ -927,6 +929,8 @@ static int atusb_get_and_conf_chip(struct atusb *atusb)
 		chip = "AT86RF231";
 		atusb->hw->phy->supported.page[0].nchunks = 1;
 		atusb->hw->phy->supported.page[0].chunk[0].channels = 0x7FFF800;
+		atusb->hw->phy->supported.page[0].chunk[0].protocol = IEEE802154_OQPSK_PHY;
+		atusb->hw->phy->supported.page[0].chunk[0].band = IEEE802154_2400_MHZ_BAND;
 		atusb->hw->phy->current_channel = 11;	/* reset default */
 		atusb->hw->phy->symbol_duration = 16;
 		atusb->hw->phy->supported.tx_powers = atusb_powers;
@@ -937,10 +941,27 @@ static int atusb_get_and_conf_chip(struct atusb *atusb)
 	case 7:
 		chip = "AT86RF212";
 		atusb->hw->flags |= IEEE802154_HW_LBT;
-		atusb->hw->phy->supported.page[0].nchunks = 1;
-		atusb->hw->phy->supported.page[0].chunk[0].channels = 0x00007FF;
-		atusb->hw->phy->supported.page[2].nchunks = 1;
-		atusb->hw->phy->supported.page[2].chunk[0].channels = 0x00007FF;
+
+		atusb->hw->phy->supported.page[0].nchunks = 2;
+		/* SUB:0 and BPSK:0 -> BPSK-20 */
+		atusb->hw->phy->supported.page[0].chunk[0].channels = 1;
+		atusb->hw->phy->supported.page[0].chunk[0].protocol = IEEE802154_BPSK_PHY;
+		atusb->hw->phy->supported.page[0].chunk[0].band = IEEE802154_868_MHZ_BAND;
+		/* SUB:0 and BPSK:1 -> OQPSK-100/200/400 */
+		atusb->hw->phy->supported.page[0].chunk[1].channels = 0x00007FE;
+		atusb->hw->phy->supported.page[0].chunk[1].protocol = IEEE802154_OQPSK_PHY;
+		atusb->hw->phy->supported.page[0].chunk[1].band = IEEE802154_868_MHZ_BAND;
+
+		atusb->hw->phy->supported.page[2].nchunks = 2;
+		/* SUB:1 and BPSK:0 -> BPSK-40 */
+		atusb->hw->phy->supported.page[2].chunk[0].channels = 1;
+		atusb->hw->phy->supported.page[2].chunk[0].protocol = IEEE802154_BPSK_PHY;
+		atusb->hw->phy->supported.page[2].chunk[0].band = IEEE802154_915_MHZ_BAND;
+		/* SUB:1 and BPSK:1 -> OQPSK-250/500/1000 */
+		atusb->hw->phy->supported.page[2].chunk[1].channels = 0x00007FE;
+		atusb->hw->phy->supported.page[2].chunk[1].protocol = IEEE802154_OQPSK_PHY;
+		atusb->hw->phy->supported.page[2].chunk[1].band = IEEE802154_915_MHZ_BAND;
+
 		atusb->hw->phy->current_channel = 5;
 		atusb->hw->phy->symbol_duration = 25;
 		atusb->hw->phy->supported.lbt = NL802154_SUPPORTED_BOOL_BOTH;
diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
index 8e6545bbbe5d..b4ba38151576 100644
--- a/drivers/net/ieee802154/ca8210.c
+++ b/drivers/net/ieee802154/ca8210.c
@@ -2964,7 +2964,10 @@ static void ca8210_hw_setup(struct ieee802154_hw *ca8210_hw)
 {
 	/* Support channels 11-26 */
 	ca8210_hw->phy->supported.page[0].nchunks = 1;
+	/* 2.4 GHz O-QPSK */
 	ca8210_hw->phy->supported.page[0].chunk[0].channels = CA8210_VALID_CHANNELS;
+	ca8210_hw->phy->supported.page[0].chunk[0].protocol = IEEE802154_OQPSK_PHY;
+	ca8210_hw->phy->supported.page[0].chunk[0].band = IEEE802154_2400_MHZ_BAND;
 	ca8210_hw->phy->supported.tx_powers_size = CA8210_MAX_TX_POWERS;
 	ca8210_hw->phy->supported.tx_powers = ca8210_tx_powers;
 	ca8210_hw->phy->supported.cca_ed_levels_size = CA8210_MAX_ED_LEVELS;
diff --git a/drivers/net/ieee802154/mac802154_hwsim.c b/drivers/net/ieee802154/mac802154_hwsim.c
index 8d99ed383b58..dc08885b9ec9 100644
--- a/drivers/net/ieee802154/mac802154_hwsim.c
+++ b/drivers/net/ieee802154/mac802154_hwsim.c
@@ -748,46 +748,79 @@ static int hwsim_add_one(struct genl_info *info, struct device *dev,
 	hw->phy->supported.page[0].nchunks = 3;
 	/* 868 MHz BPSK	802.15.4-2003 */
 	hw->phy->supported.page[0].chunk[0].channels |= 1;
+	hw->phy->supported.page[0].chunk[0].protocol = IEEE802154_BPSK_PHY;
+	hw->phy->supported.page[0].chunk[0].band = IEEE802154_868_MHZ_BAND;
 	/* 915 MHz BPSK	802.15.4-2003 */
 	hw->phy->supported.page[0].chunk[1].channels |= 0x7fe;
+	hw->phy->supported.page[0].chunk[1].protocol = IEEE802154_BPSK_PHY;
+	hw->phy->supported.page[0].chunk[1].band = IEEE802154_915_MHZ_BAND;
 	/* 2.4 GHz O-QPSK 802.15.4-2003 */
 	hw->phy->supported.page[0].chunk[2].channels |= 0x7FFF800;
+	hw->phy->supported.page[0].chunk[2].protocol = IEEE802154_OQPSK_PHY;
+	hw->phy->supported.page[0].chunk[2].band = IEEE802154_2400_MHZ_BAND;
 
 	hw->phy->supported.page[1].nchunks = 2;
 	/* 868 MHz ASK 802.15.4-2006 */
 	hw->phy->supported.page[1].chunk[0].channels |= 1;
+	hw->phy->supported.page[1].chunk[0].protocol = IEEE802154_ASK_PHY;
+	hw->phy->supported.page[1].chunk[0].band = IEEE802154_868_MHZ_BAND;
 	/* 915 MHz ASK 802.15.4-2006 */
 	hw->phy->supported.page[1].chunk[1].channels |= 0x7fe;
+	hw->phy->supported.page[1].chunk[1].protocol = IEEE802154_ASK_PHY;
+	hw->phy->supported.page[1].chunk[1].band = IEEE802154_915_MHZ_BAND;
 
 	hw->phy->supported.page[2].nchunks = 2;
 	/* 868 MHz O-QPSK 802.15.4-2006 */
 	hw->phy->supported.page[2].chunk[0].channels |= 1;
+	hw->phy->supported.page[2].chunk[0].protocol = IEEE802154_OQPSK_PHY;
+	hw->phy->supported.page[2].chunk[0].band = IEEE802154_868_MHZ_BAND;
 	/* 915 MHz O-QPSK 802.15.4-2006 */
 	hw->phy->supported.page[2].chunk[1].channels |= 0x7fe;
+	hw->phy->supported.page[2].chunk[1].protocol = IEEE802154_OQPSK_PHY;
+	hw->phy->supported.page[2].chunk[1].band = IEEE802154_915_MHZ_BAND;
 
 	hw->phy->supported.page[3].nchunks = 1;
 	/* 2.4 GHz CSS 802.15.4a-2007 */
 	hw->phy->supported.page[3].chunk[0].channels |= 0x3fff;
+	hw->phy->supported.page[3].chunk[0].protocol = IEEE802154_CSS_PHY;
+	hw->phy->supported.page[3].chunk[0].band = IEEE802154_2400_MHZ_BAND;
 
 	hw->phy->supported.page[4].nchunks = 3;
 	/* UWB Sub-gigahertz 802.15.4a-2007 */
 	hw->phy->supported.page[4].chunk[0].channels |= 1;
+	hw->phy->supported.page[4].chunk[0].protocol = IEEE802154_HRP_UWB_PHY;
+	hw->phy->supported.page[4].chunk[0].band = IEEE802154_250_750_MHZ_BAND;
+	hw->phy->supported.page[4].chunk[0].prf = IEEE802154_62890KHZ_MEAN_PRF;
 	/* UWB Low band 802.15.4a-2007 */
 	hw->phy->supported.page[4].chunk[1].channels |= 0x1e;
+	hw->phy->supported.page[4].chunk[1].protocol = IEEE802154_HRP_UWB_PHY;
+	hw->phy->supported.page[4].chunk[1].band = IEEE802154_3100_4800_MHZ_BAND;
+	hw->phy->supported.page[4].chunk[1].prf = IEEE802154_62890KHZ_MEAN_PRF;
 	/* UWB High band 802.15.4a-2007 */
 	hw->phy->supported.page[4].chunk[2].channels |= 0xffe0;
+	hw->phy->supported.page[4].chunk[2].protocol = IEEE802154_HRP_UWB_PHY;
+	hw->phy->supported.page[4].chunk[2].band = IEEE802154_6000_10600_MHZ_BAND;
+	hw->phy->supported.page[4].chunk[2].prf = IEEE802154_62890KHZ_MEAN_PRF;
 
 	hw->phy->supported.page[5].nchunks = 2;
 	/* 750 MHz O-QPSK 802.15.4c-2009 */
 	hw->phy->supported.page[5].chunk[0].channels |= 0xf;
+	hw->phy->supported.page[5].chunk[0].protocol = IEEE802154_OQPSK_PHY;
+	hw->phy->supported.page[5].chunk[0].band = IEEE802154_750_MHZ_BAND;
 	/* 750 MHz MPSK 802.15.4c-2009 */
 	hw->phy->supported.page[5].chunk[1].channels |= 0xf0;
+	hw->phy->supported.page[5].chunk[1].protocol = IEEE802154_MQPSK_PHY;
+	hw->phy->supported.page[5].chunk[1].band = IEEE802154_750_MHZ_BAND;
 
 	hw->phy->supported.page[6].nchunks = 2;
 	/* 950 MHz BPSK 802.15.4d-2009 */
 	hw->phy->supported.page[6].chunk[0].channels |= 0x3ff;
+	hw->phy->supported.page[6].chunk[0].protocol = IEEE802154_BPSK_PHY;
+	hw->phy->supported.page[6].chunk[0].band = IEEE802154_950_MHZ_BAND;
 	/* 950 MHz GFSK 802.15.4d-2009 */
 	hw->phy->supported.page[6].chunk[1].channels |= 0x3ffc00;
+	hw->phy->supported.page[6].chunk[1].protocol = IEEE802154_GFSK_PHY;
+	hw->phy->supported.page[6].chunk[1].band = IEEE802154_950_MHZ_BAND;
 
 	ieee802154_random_extended_addr(&hw->phy->perm_extended_addr);
 
diff --git a/drivers/net/ieee802154/mcr20a.c b/drivers/net/ieee802154/mcr20a.c
index 80bed905acf9..e2c249aef430 100644
--- a/drivers/net/ieee802154/mcr20a.c
+++ b/drivers/net/ieee802154/mcr20a.c
@@ -1003,7 +1003,10 @@ static void mcr20a_hw_setup(struct mcr20a_local *lp)
 	phy->cca.mode = NL802154_CCA_ENERGY;
 
 	phy->supported.page[0].nchunks = 1;
+	/* 2.4 GHz O-QPSK */
 	phy->supported.page[0].chunk[0].channels = MCR20A_VALID_CHANNELS;
+	phy->supported.page[0].chunk[0].protocol = IEEE802154_OQPSK_PHY;
+	phy->supported.page[0].chunk[0].band = IEEE802154_2400_MHZ_BAND;
 	phy->current_page = 0;
 	/* MCR20A default reset value */
 	phy->current_channel = 20;
diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h
index ef49a23801c6..03c8008217fb 100644
--- a/include/net/cfg802154.h
+++ b/include/net/cfg802154.h
@@ -131,8 +131,49 @@ wpan_phy_supported_bool(bool b, enum nl802154_supported_bool_states st)
 	return false;
 }
 
+enum ieee802154_phy_protocols {
+	IEEE802154_UNKNOWN_PHY,
+	IEEE802154_BPSK_PHY,
+	IEEE802154_OQPSK_PHY,
+	IEEE802154_MQPSK_PHY,
+	IEEE802154_GFSK_PHY,
+	IEEE802154_ASK_PHY,
+	IEEE802154_CSS_PHY,
+	IEEE802154_HRP_UWB_PHY,
+
+	IEEE802154_MAX_PHY,
+};
+
+enum ieee802154_phy_bands {
+	IEEE802154_UNKNOWN_BAND,
+	IEEE802154_750_MHZ_BAND,
+	IEEE802154_868_MHZ_BAND,
+	IEEE802154_915_MHZ_BAND,
+	IEEE802154_950_MHZ_BAND,
+	IEEE802154_2400_MHZ_BAND,
+	IEEE802154_250_750_MHZ_BAND,
+	IEEE802154_3100_4800_MHZ_BAND,
+	IEEE802154_6000_10600_MHZ_BAND,
+
+	IEEE802154_MAX_BAND,
+};
+
+enum ieee802154_phy_mean_prfs {
+	IEEE802154_UNKNOWN_MEAN_PRF,
+	IEEE802154_16100KHZ_MEAN_PRF,
+	IEEE802154_4030KHZ_MEAN_PRF,
+	IEEE802154_62890KHZ_MEAN_PRF,
+
+	IEEE802154_MAX_PRF,
+};
+
 struct phy_channels {
 	u32 channels;
+	enum ieee802154_phy_protocols protocol;
+	enum ieee802154_phy_bands band;
+	union {
+		enum ieee802154_phy_mean_prfs prf;
+	};
 };
 
 struct phy_page {
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH wpan-next v2 3/5] net: mac802154: Convert the symbol duration into nanoseconds
  2022-01-28 11:08 [PATCH wpan-next v2 0/5] ieee802154: Improve durations handling Miquel Raynal
  2022-01-28 11:08 ` [PATCH wpan-next v2 1/5] net: ieee802154: Improve the way supported channels are declared Miquel Raynal
  2022-01-28 11:08 ` [PATCH wpan-next v2 2/5] net: ieee802154: Give more details to the core about the channel configurations Miquel Raynal
@ 2022-01-28 11:08 ` Miquel Raynal
  2022-01-28 13:00   ` Stefan Schmidt
  2022-01-28 11:08 ` [PATCH wpan-next v2 4/5] net: mac802154: Set durations automatically Miquel Raynal
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 24+ messages in thread
From: Miquel Raynal @ 2022-01-28 11:08 UTC (permalink / raw)
  To: Alexander Aring, Stefan Schmidt, linux-wpan
  Cc: David S. Miller, Jakub Kicinski, netdev, Michael Hennerich,
	Varka Bhadram, Xue Liu, Alan Ott, 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 d3dc03926246..b08f08475426 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);
 }
@@ -1572,7 +1572,7 @@ at86rf230_detect_device(struct at86rf230_local *lp)
 		lp->hw->phy->supported.page[0].chunk[0].protocol = IEEE802154_OQPSK_PHY;
 		lp->hw->phy->supported.page[0].chunk[0].band = IEEE802154_2400_MHZ_BAND;
 		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;
@@ -1604,7 +1604,7 @@ at86rf230_detect_device(struct at86rf230_local *lp)
 		lp->hw->phy->supported.page[2].chunk[1].band = IEEE802154_915_MHZ_BAND;
 
 		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);
@@ -1619,7 +1619,7 @@ at86rf230_detect_device(struct at86rf230_local *lp)
 		lp->hw->phy->supported.page[0].chunk[0].protocol = IEEE802154_OQPSK_PHY;
 		lp->hw->phy->supported.page[0].chunk[0].band = IEEE802154_2400_MHZ_BAND;
 		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 7e8c9d6db7d7..80382919520e 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);
 }
@@ -919,7 +919,7 @@ static int atusb_get_and_conf_chip(struct atusb *atusb)
 		atusb->hw->phy->supported.page[0].chunk[0].protocol = IEEE802154_OQPSK_PHY;
 		atusb->hw->phy->supported.page[0].chunk[0].band = IEEE802154_2400_MHZ_BAND;
 		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;
@@ -932,7 +932,7 @@ static int atusb_get_and_conf_chip(struct atusb *atusb)
 		atusb->hw->phy->supported.page[0].chunk[0].protocol = IEEE802154_OQPSK_PHY;
 		atusb->hw->phy->supported.page[0].chunk[0].band = IEEE802154_2400_MHZ_BAND;
 		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;
@@ -963,7 +963,7 @@ static int atusb_get_and_conf_chip(struct atusb *atusb)
 		atusb->hw->phy->supported.page[2].chunk[1].band = IEEE802154_915_MHZ_BAND;
 
 		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 b4ba38151576..8eb48a8014db 100644
--- a/drivers/net/ieee802154/ca8210.c
+++ b/drivers/net/ieee802154/ca8210.c
@@ -2978,7 +2978,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->sifs_period = 12;
 	ca8210_hw->flags =
diff --git a/drivers/net/ieee802154/mcr20a.c b/drivers/net/ieee802154/mcr20a.c
index e2c249aef430..34063b7e663e 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 |
@@ -1010,7 +1010,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 03c8008217fb..286709a9dd0b 100644
--- a/include/net/cfg802154.h
+++ b/include/net/cfg802154.h
@@ -253,8 +253,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


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH wpan-next v2 4/5] net: mac802154: Set durations automatically
  2022-01-28 11:08 [PATCH wpan-next v2 0/5] ieee802154: Improve durations handling Miquel Raynal
                   ` (2 preceding siblings ...)
  2022-01-28 11:08 ` [PATCH wpan-next v2 3/5] net: mac802154: Convert the symbol duration into nanoseconds Miquel Raynal
@ 2022-01-28 11:08 ` Miquel Raynal
  2022-01-28 11:08 ` [PATCH wpan-next v2 5/5] net: ieee802154: Drop duration settings when the core does it already Miquel Raynal
  2022-01-28 11:11 ` [PATCH wpan-next v2 0/5] ieee802154: Improve durations handling Miquel Raynal
  5 siblings, 0 replies; 24+ messages in thread
From: Miquel Raynal @ 2022-01-28 11:08 UTC (permalink / raw)
  To: Alexander Aring, Stefan Schmidt, linux-wpan
  Cc: David S. Miller, Jakub Kicinski, netdev, Michael Hennerich,
	Varka Bhadram, Xue Liu, Alan Ott, Miquel Raynal

Now that we have access to all the basic information to know which
symbol duration should be applied, let's set the symbol duration
automatically. The two locations that must call for the symbol duration
to be set are:
- when manually requesting a channel change though the netlink interface
- at PHY creation, ieee802154_alloc_hw() already calls
  ieee802154_change_channel() which will now update the symbol duration
  accordingly.

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    | 105 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 108 insertions(+)

diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h
index 286709a9dd0b..4491e2724ff2 100644
--- a/include/net/cfg802154.h
+++ b/include/net/cfg802154.h
@@ -455,4 +455,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..f08c34c27ea9 100644
--- a/net/mac802154/main.c
+++ b/net/mac802154/main.c
@@ -113,6 +113,109 @@ 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)
+{
+	struct phy_page *page = &phy->supported.page[phy->current_page];
+	struct phy_channels *chan;
+	bool valid_band = false;
+	unsigned int chunk;
+	u32 duration = 0;
+
+	for (chunk = 0; chunk < page->nchunks; chunk++) {
+		if (page->chunk[chunk].channels & BIT(phy->current_channel))
+			break;
+	}
+
+	if (chunk == page->nchunks) {
+		pr_debug("Wrong channels description\n");
+		return;
+	}
+
+	chan = &page->chunk[chunk];
+	switch (chan->protocol) {
+	case IEEE802154_BPSK_PHY:
+		switch (chan->band) {
+		case IEEE802154_868_MHZ_BAND:
+			/* 868 MHz BPSK	802.15.4-2003: 20 ksym/s */
+			duration = 50 * NSEC_PER_USEC;
+			break;
+		case IEEE802154_915_MHZ_BAND:
+			/* 915 MHz BPSK	802.15.4-2003: 40 ksym/s */
+			duration = 25 * NSEC_PER_USEC;
+			break;
+		default:
+			break;
+		}
+		break;
+	case IEEE802154_OQPSK_PHY:
+		switch (chan->band) {
+		case IEEE802154_868_MHZ_BAND:
+			/* 868 MHz O-QPSK 802.15.4-2006: 25 ksym/s */
+			duration = 40 * NSEC_PER_USEC;
+			break;
+		case IEEE802154_915_MHZ_BAND:
+		case IEEE802154_2400_MHZ_BAND:
+			/* 915/2400 MHz O-QPSK 802.15.4-2006: 62.5 ksym/s */
+			duration = 16 * NSEC_PER_USEC;
+			break;
+		default:
+			break;
+		}
+		break;
+	case IEEE802154_CSS_PHY:
+		switch (chan->band) {
+		case IEEE802154_2400_MHZ_BAND:
+			/* 2.4 GHz CSS 802.15.4a-2007: 1/6 Msym/s */
+			duration = 6 * NSEC_PER_USEC;
+			break;
+		default:
+			break;
+		}
+		break;
+	case IEEE802154_HRP_UWB_PHY:
+		switch (chan->band) {
+		case IEEE802154_250_750_MHZ_BAND:
+		case IEEE802154_3100_4800_MHZ_BAND:
+		case IEEE802154_6000_10600_MHZ_BAND:
+			valid_band = true;
+			break;
+		default:
+			break;
+		}
+
+		if (!valid_band)
+			break;
+
+		/* UWB 802.15.4a-2007: 993.6 or 1017.6 or 3974.4 ns */
+		switch (chan->prf) {
+		case IEEE802154_16100KHZ_MEAN_PRF:
+			duration = 994;
+			break;
+		case IEEE802154_4030KHZ_MEAN_PRF:
+			duration = 3974;
+			break;
+		case IEEE802154_62890KHZ_MEAN_PRF:
+			duration = 1018;
+			break;
+		default:
+			break;
+		}
+		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 +260,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


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH wpan-next v2 5/5] net: ieee802154: Drop duration settings when the core does it already
  2022-01-28 11:08 [PATCH wpan-next v2 0/5] ieee802154: Improve durations handling Miquel Raynal
                   ` (3 preceding siblings ...)
  2022-01-28 11:08 ` [PATCH wpan-next v2 4/5] net: mac802154: Set durations automatically Miquel Raynal
@ 2022-01-28 11:08 ` Miquel Raynal
  2022-02-01 17:40   ` Miquel Raynal
  2022-01-28 11:11 ` [PATCH wpan-next v2 0/5] ieee802154: Improve durations handling Miquel Raynal
  5 siblings, 1 reply; 24+ messages in thread
From: Miquel Raynal @ 2022-01-28 11:08 UTC (permalink / raw)
  To: Alexander Aring, Stefan Schmidt, linux-wpan
  Cc: David S. Miller, Jakub Kicinski, netdev, Michael Hennerich,
	Varka Bhadram, Xue Liu, Alan Ott, 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    |  1 -
 drivers/net/ieee802154/mcr20a.c    |  5 -----
 4 files changed, 72 deletions(-)

diff --git a/drivers/net/ieee802154/at86rf230.c b/drivers/net/ieee802154/at86rf230.c
index b08f08475426..9f3b4a8d151a 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);
 }
 
@@ -1572,7 +1542,6 @@ at86rf230_detect_device(struct at86rf230_local *lp)
 		lp->hw->phy->supported.page[0].chunk[0].protocol = IEEE802154_OQPSK_PHY;
 		lp->hw->phy->supported.page[0].chunk[0].band = IEEE802154_2400_MHZ_BAND;
 		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;
@@ -1604,7 +1573,6 @@ at86rf230_detect_device(struct at86rf230_local *lp)
 		lp->hw->phy->supported.page[2].chunk[1].band = IEEE802154_915_MHZ_BAND;
 
 		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);
@@ -1619,7 +1587,6 @@ at86rf230_detect_device(struct at86rf230_local *lp)
 		lp->hw->phy->supported.page[0].chunk[0].protocol = IEEE802154_OQPSK_PHY;
 		lp->hw->phy->supported.page[0].chunk[0].band = IEEE802154_2400_MHZ_BAND;
 		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 80382919520e..1a56073c1c52 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);
 }
 
@@ -919,7 +889,6 @@ static int atusb_get_and_conf_chip(struct atusb *atusb)
 		atusb->hw->phy->supported.page[0].chunk[0].protocol = IEEE802154_OQPSK_PHY;
 		atusb->hw->phy->supported.page[0].chunk[0].band = IEEE802154_2400_MHZ_BAND;
 		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;
@@ -932,7 +901,6 @@ static int atusb_get_and_conf_chip(struct atusb *atusb)
 		atusb->hw->phy->supported.page[0].chunk[0].protocol = IEEE802154_OQPSK_PHY;
 		atusb->hw->phy->supported.page[0].chunk[0].band = IEEE802154_2400_MHZ_BAND;
 		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;
@@ -963,7 +931,6 @@ static int atusb_get_and_conf_chip(struct atusb *atusb)
 		atusb->hw->phy->supported.page[2].chunk[1].band = IEEE802154_915_MHZ_BAND;
 
 		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 8eb48a8014db..baa4392d20c2 100644
--- a/drivers/net/ieee802154/ca8210.c
+++ b/drivers/net/ieee802154/ca8210.c
@@ -2978,7 +2978,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->sifs_period = 12;
 	ca8210_hw->flags =
diff --git a/drivers/net/ieee802154/mcr20a.c b/drivers/net/ieee802154/mcr20a.c
index 34063b7e663e..8d12c2968302 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;
@@ -1010,7 +1006,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


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH wpan-next v2 0/5] ieee802154: Improve durations handling
  2022-01-28 11:08 [PATCH wpan-next v2 0/5] ieee802154: Improve durations handling Miquel Raynal
                   ` (4 preceding siblings ...)
  2022-01-28 11:08 ` [PATCH wpan-next v2 5/5] net: ieee802154: Drop duration settings when the core does it already Miquel Raynal
@ 2022-01-28 11:11 ` Miquel Raynal
  5 siblings, 0 replies; 24+ messages in thread
From: Miquel Raynal @ 2022-01-28 11:11 UTC (permalink / raw)
  To: Alexander Aring, Stefan Schmidt, linux-wpan
  Cc: David S. Miller, Jakub Kicinski, netdev, Michael Hennerich,
	Varka Bhadram, Xue Liu, Alan Ott

Hello,

miquel.raynal@bootlin.com wrote on Fri, 28 Jan 2022 12:08:20 +0100:

> These paches 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.

I forgot to mention the changelog here, I've just reworded a commit
message and fixed a typo.

> 
> Miquel Raynal (5):
>   net: ieee802154: Improve the way supported channels are declared
>   net: ieee802154: Give more details to the core about the channel
>     configurations
>   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/adf7242.c         |   3 +-
>  drivers/net/ieee802154/at86rf230.c       |  66 ++++++-------
>  drivers/net/ieee802154/atusb.c           |  66 ++++++-------
>  drivers/net/ieee802154/ca8210.c          |   7 +-
>  drivers/net/ieee802154/cc2520.c          |   3 +-
>  drivers/net/ieee802154/fakelb.c          |  43 ++++++---
>  drivers/net/ieee802154/mac802154_hwsim.c |  76 ++++++++++++---
>  drivers/net/ieee802154/mcr20a.c          |  11 +--
>  drivers/net/ieee802154/mrf24j40.c        |   3 +-
>  include/net/cfg802154.h                  |  60 +++++++++++-
>  net/ieee802154/core.h                    |   2 +
>  net/ieee802154/nl-phy.c                  |   8 +-
>  net/ieee802154/nl802154.c                |  30 ++++--
>  net/mac802154/cfg.c                      |   1 +
>  net/mac802154/main.c                     | 113 ++++++++++++++++++++++-
>  15 files changed, 361 insertions(+), 131 deletions(-)
> 


Thanks,
Miquèl

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH wpan-next v2 3/5] net: mac802154: Convert the symbol duration into nanoseconds
  2022-01-28 11:08 ` [PATCH wpan-next v2 3/5] net: mac802154: Convert the symbol duration into nanoseconds Miquel Raynal
@ 2022-01-28 13:00   ` Stefan Schmidt
  0 siblings, 0 replies; 24+ messages in thread
From: Stefan Schmidt @ 2022-01-28 13:00 UTC (permalink / raw)
  To: Miquel Raynal, Alexander Aring, linux-wpan
  Cc: David S. Miller, Jakub Kicinski, netdev, Michael Hennerich,
	Varka Bhadram, Xue Liu, Alan Ott


Hello.

On 28.01.22 12:08, Miquel Raynal wrote:
> 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 ++++----

I get a error on the mcr20a hunk when applying this. I assume its due to 
the mcr20a fix that hit wpan, but is not yet in wpan-next.

The wpan pull request for the fixes has been send today. Once merged and 
the next merge of net to net-next happened I will pull that in here to 
resolve the conflict.

regards
Stefan Schmidt

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH wpan-next v2 1/5] net: ieee802154: Improve the way supported channels are declared
  2022-01-28 11:08 ` [PATCH wpan-next v2 1/5] net: ieee802154: Improve the way supported channels are declared Miquel Raynal
@ 2022-01-30 21:35   ` Alexander Aring
  2022-01-31 14:23     ` Miquel Raynal
  0 siblings, 1 reply; 24+ messages in thread
From: Alexander Aring @ 2022-01-30 21:35 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Stefan Schmidt, linux-wpan - ML, David S. Miller, Jakub Kicinski,
	open list:NETWORKING [GENERAL],
	Michael Hennerich, Varka Bhadram, Xue Liu, Alan Ott

Hi,

On Fri, Jan 28, 2022 at 6:08 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> The idea here is to create a structure per set of channels so that we
> can define much more than basic bitfields for these.
>
> The structure is currently almost empty on purpose because this change
> is supposed to be a mechanical update without additional information but
> more details will be added in the following commits.
>

In my opinion you want to put more information in this structure which
is not necessary and force the driver developer to add information
which is already there encoded in the page/channel bitfields. Why not
add helper functionality and get your "band" and "protocol" for a
page/channel combination?

- Alex

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH wpan-next v2 1/5] net: ieee802154: Improve the way supported channels are declared
  2022-01-30 21:35   ` Alexander Aring
@ 2022-01-31 14:23     ` Miquel Raynal
  2022-02-01  0:04       ` Alexander Aring
  0 siblings, 1 reply; 24+ messages in thread
From: Miquel Raynal @ 2022-01-31 14:23 UTC (permalink / raw)
  To: Alexander Aring
  Cc: Stefan Schmidt, linux-wpan - ML, David S. Miller, Jakub Kicinski,
	open list:NETWORKING [GENERAL],
	Michael Hennerich, Varka Bhadram, Xue Liu, Alan Ott

Hi Alexander,

alex.aring@gmail.com wrote on Sun, 30 Jan 2022 16:35:35 -0500:

> Hi,
> 
> On Fri, Jan 28, 2022 at 6:08 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > The idea here is to create a structure per set of channels so that we
> > can define much more than basic bitfields for these.
> >
> > The structure is currently almost empty on purpose because this change
> > is supposed to be a mechanical update without additional information but
> > more details will be added in the following commits.
> >  
> 
> In my opinion you want to put more information in this structure which
> is not necessary and force the driver developer to add information
> which is already there encoded in the page/channel bitfields.

The information I am looking forward to add is clearly not encoded in
the page/channel bitfields (these information are added in the
following patches). At least I don't see anywhere in the spec a
paragraph telling which protocol and band must be used as a function of
the page and channel information. So I improved the way channels are
declared to give more information than what we currently have.

BTW I see the wpan tools actually derive the protocol/band from the
channel/page information and I _really_ don't get it. I believe it only
works with hwsim but if it's not the case I would like to hear
more about it.

> Why not
> add helper functionality and get your "band" and "protocol" for a
> page/channel combination?

This information is as static as the channel/page information, so why
using two different channels to get it? This means two different places
where the channels must be described, which IMHO hardens the work for
device driver writers.

I however agree that the final presentation looks a bit more heavy to
the eyes, but besides the extra fat that this change brings, it is
rather easy to give the core all the information it needs in a rather
detailed and understandable way.

Thanks,
Miquèl

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH wpan-next v2 1/5] net: ieee802154: Improve the way supported channels are declared
  2022-01-31 14:23     ` Miquel Raynal
@ 2022-02-01  0:04       ` Alexander Aring
  2022-02-01 14:55         ` Miquel Raynal
  0 siblings, 1 reply; 24+ messages in thread
From: Alexander Aring @ 2022-02-01  0:04 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Stefan Schmidt, linux-wpan - ML, David S. Miller, Jakub Kicinski,
	open list:NETWORKING [GENERAL],
	Michael Hennerich, Varka Bhadram, Xue Liu, Alan Ott

Hi,

On Mon, Jan 31, 2022 at 9:23 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Alexander,
>
> alex.aring@gmail.com wrote on Sun, 30 Jan 2022 16:35:35 -0500:
>
> > Hi,
> >
> > On Fri, Jan 28, 2022 at 6:08 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > >
> > > The idea here is to create a structure per set of channels so that we
> > > can define much more than basic bitfields for these.
> > >
> > > The structure is currently almost empty on purpose because this change
> > > is supposed to be a mechanical update without additional information but
> > > more details will be added in the following commits.
> > >
> >
> > In my opinion you want to put more information in this structure which
> > is not necessary and force the driver developer to add information
> > which is already there encoded in the page/channel bitfields.
>
> The information I am looking forward to add is clearly not encoded in
> the page/channel bitfields (these information are added in the
> following patches). At least I don't see anywhere in the spec a
> paragraph telling which protocol and band must be used as a function of
> the page and channel information. So I improved the way channels are
> declared to give more information than what we currently have.
>

This makes no sense for me, because you are telling right now that a
page/channel combination depends on the transceiver.

> BTW I see the wpan tools actually derive the protocol/band from the
> channel/page information and I _really_ don't get it. I believe it only
> works with hwsim but if it's not the case I would like to hear
> more about it.
>

No, I remember the discussion with Christoffer Holmstedt, he described
it in his commit message "8.1.2 in IEEE 802.15.4-2011".
See wpan-tools commit 0af3e40bbd6da60cc000cfdfd13b9cdd8a20d717 ("info:
add frequency to channel listing for phy capabilities").

I think it is the chapter "Channel assignments"?

> > Why not
> > add helper functionality and get your "band" and "protocol" for a
> > page/channel combination?
>
> This information is as static as the channel/page information, so why
> using two different channels to get it? This means two different places
> where the channels must be described, which IMHO hardens the work for
> device driver writers.
>

device drivers writers can make mistakes here, they probably can only
set page/channel registers in their hardware and have no idea about
other things.

> I however agree that the final presentation looks a bit more heavy to
> the eyes, but besides the extra fat that this change brings, it is
> rather easy to give the core all the information it needs in a rather
> detailed and understandable way.

On the driver layer it should be as simple as possible. If you want to
have a static array for that init it in the phy register
functionality, however I think a simple lookup table should be enough
for that.
To make it more understandable I guess some people can introduce some
defines/etc to make a more sense behind setting static hex values.

- Alex

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH wpan-next v2 1/5] net: ieee802154: Improve the way supported channels are declared
  2022-02-01  0:04       ` Alexander Aring
@ 2022-02-01 14:55         ` Miquel Raynal
  2022-02-06 21:37           ` Alexander Aring
  0 siblings, 1 reply; 24+ messages in thread
From: Miquel Raynal @ 2022-02-01 14:55 UTC (permalink / raw)
  To: Alexander Aring
  Cc: Stefan Schmidt, linux-wpan - ML, David S. Miller, Jakub Kicinski,
	open list:NETWORKING [GENERAL],
	Michael Hennerich, Varka Bhadram, Xue Liu, Alan Ott

Hi Alexander,

alex.aring@gmail.com wrote on Mon, 31 Jan 2022 19:04:40 -0500:

> Hi,
> 
> On Mon, Jan 31, 2022 at 9:23 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > Hi Alexander,
> >
> > alex.aring@gmail.com wrote on Sun, 30 Jan 2022 16:35:35 -0500:
> >  
> > > Hi,
> > >
> > > On Fri, Jan 28, 2022 at 6:08 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:  
> > > >
> > > > The idea here is to create a structure per set of channels so that we
> > > > can define much more than basic bitfields for these.
> > > >
> > > > The structure is currently almost empty on purpose because this change
> > > > is supposed to be a mechanical update without additional information but
> > > > more details will be added in the following commits.
> > > >  
> > >
> > > In my opinion you want to put more information in this structure which
> > > is not necessary and force the driver developer to add information
> > > which is already there encoded in the page/channel bitfields.  
> >
> > The information I am looking forward to add is clearly not encoded in
> > the page/channel bitfields (these information are added in the
> > following patches). At least I don't see anywhere in the spec a
> > paragraph telling which protocol and band must be used as a function of
> > the page and channel information. So I improved the way channels are
> > declared to give more information than what we currently have.
> >  
> 
> This makes no sense for me, because you are telling right now that a
> page/channel combination depends on the transceiver.

That is exactly what I meant, and you made me realize that I overlooked
that information from the spec.

> > BTW I see the wpan tools actually derive the protocol/band from the
> > channel/page information and I _really_ don't get it. I believe it only
> > works with hwsim but if it's not the case I would like to hear
> > more about it.
> >  
> 
> No, I remember the discussion with Christoffer Holmstedt, he described
> it in his commit message "8.1.2 in IEEE 802.15.4-2011".
> See wpan-tools commit 0af3e40bbd6da60cc000cfdfd13b9cdd8a20d717 ("info:
> add frequency to channel listing for phy capabilities").
> 
> I think it is the chapter "Channel assignments"?

Oh yeah, now I get it. It's gonna be much simpler than what I thought.

In the 2020 spec this is § "10.1.3 Channel assignments".

> > > Why not
> > > add helper functionality and get your "band" and "protocol" for a
> > > page/channel combination?  
> >
> > This information is as static as the channel/page information, so why
> > using two different channels to get it? This means two different places
> > where the channels must be described, which IMHO hardens the work for
> > device driver writers.
> >  
> 
> device drivers writers can make mistakes here, they probably can only
> set page/channel registers in their hardware and have no idea about
> other things.
> 
> > I however agree that the final presentation looks a bit more heavy to
> > the eyes, but besides the extra fat that this change brings, it is
> > rather easy to give the core all the information it needs in a rather
> > detailed and understandable way.  
> 
> On the driver layer it should be as simple as possible. If you want to
> have a static array for that init it in the phy register
> functionality, however I think a simple lookup table should be enough
> for that.

Given the new information that I am currently processing, I believe the
array is not needed anymore, we can live with a minimal number of
additional helpers, like the one getting the PRF value for the UWB
PHYs. It's the only one I have in mind so far.

> To make it more understandable I guess some people can introduce some
> defines/etc to make a more sense behind setting static hex values.

I'll propose a new approach soon.

Thanks,
Miquèl

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH wpan-next v2 5/5] net: ieee802154: Drop duration settings when the core does it already
  2022-01-28 11:08 ` [PATCH wpan-next v2 5/5] net: ieee802154: Drop duration settings when the core does it already Miquel Raynal
@ 2022-02-01 17:40   ` Miquel Raynal
  2022-02-01 20:51     ` Stefan Schmidt
  0 siblings, 1 reply; 24+ messages in thread
From: Miquel Raynal @ 2022-02-01 17:40 UTC (permalink / raw)
  To: Alexander Aring, Stefan Schmidt, linux-wpan
  Cc: David S. Miller, Jakub Kicinski, netdev, Michael Hennerich,
	Varka Bhadram, Xue Liu, Alan Ott

Hi,

> --- a/drivers/net/ieee802154/ca8210.c
> +++ b/drivers/net/ieee802154/ca8210.c
> @@ -2978,7 +2978,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->sifs_period = 12;

I've missed that error                ^^

This driver should be fixed first (that's probably a copy/paste of the
error from the other driver which did the same).

As the rest of the series will depend on this fix (or conflict) we could
merge it through wpan-next anyway, if you don't mind, as it was there
since 2017 and these numbers had no real impact so far (I believe).

I just figure this out now while searching for leftovers after a rebase
operation, sorry.

Thanks,
Miquèl

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH wpan-next v2 5/5] net: ieee802154: Drop duration settings when the core does it already
  2022-02-01 17:40   ` Miquel Raynal
@ 2022-02-01 20:51     ` Stefan Schmidt
  2022-02-02  7:40       ` Miquel Raynal
  0 siblings, 1 reply; 24+ messages in thread
From: Stefan Schmidt @ 2022-02-01 20:51 UTC (permalink / raw)
  To: Miquel Raynal, Alexander Aring, linux-wpan
  Cc: David S. Miller, Jakub Kicinski, netdev, Michael Hennerich,
	Varka Bhadram, Xue Liu, Alan Ott


Hello.

On 01.02.22 18:40, Miquel Raynal wrote:
> Hi,
> 
>> --- a/drivers/net/ieee802154/ca8210.c
>> +++ b/drivers/net/ieee802154/ca8210.c
>> @@ -2978,7 +2978,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->sifs_period = 12;
> 
> I've missed that error                ^^
> 
> This driver should be fixed first (that's probably a copy/paste of the
> error from the other driver which did the same).
> 
> As the rest of the series will depend on this fix (or conflict) we could
> merge it through wpan-next anyway, if you don't mind, as it was there
> since 2017 and these numbers had no real impact so far (I believe).

Not sure I follow this logic. The fix you do is being removed in 4/4 of 
your v3 set again. So it would only be in place for these two in between 
commits.

As you laid out above this has been in place since 2017 and the number 
have no real impact. Getting the fix in wpan-next to remove it again two 
patches later would not be needed here.

If you would like to have this fixed for 5.16 and older stable kernels I 
could go ahead and apply it to wpan and let it trickle down into stable 
trees. We would have to deal with either a merge of net into net-next or 
with a merge conflicts when sending the pull request. Both can be done.

But given the circumstances above I have no problem to drop this fix 
completely and have it fixed implicitly with the rest of the patchset.

regards
Stefan Schmidt

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH wpan-next v2 5/5] net: ieee802154: Drop duration settings when the core does it already
  2022-02-01 20:51     ` Stefan Schmidt
@ 2022-02-02  7:40       ` Miquel Raynal
  2022-02-02 12:17         ` Stefan Schmidt
  0 siblings, 1 reply; 24+ messages in thread
From: Miquel Raynal @ 2022-02-02  7:40 UTC (permalink / raw)
  To: Stefan Schmidt
  Cc: Alexander Aring, linux-wpan, David S. Miller, Jakub Kicinski,
	netdev, Michael Hennerich, Varka Bhadram, Xue Liu, Alan Ott

Hi Stefan,

stefan@datenfreihafen.org wrote on Tue, 1 Feb 2022 21:51:04 +0100:

> Hello.
> 
> On 01.02.22 18:40, Miquel Raynal wrote:
> > Hi,
> >   
> >> --- a/drivers/net/ieee802154/ca8210.c
> >> +++ b/drivers/net/ieee802154/ca8210.c
> >> @@ -2978,7 +2978,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->sifs_period = 12;  
> > 
> > I've missed that error                ^^
> > 
> > This driver should be fixed first (that's probably a copy/paste of the
> > error from the other driver which did the same).
> > 
> > As the rest of the series will depend on this fix (or conflict) we could
> > merge it through wpan-next anyway, if you don't mind, as it was there
> > since 2017 and these numbers had no real impact so far (I believe).  
> 
> Not sure I follow this logic. The fix you do is being removed in 4/4 of your v3 set again. So it would only be in place for these two in between commits.

Exactly.

> As you laid out above this has been in place since 2017 and the number have no real impact. Getting the fix in wpan-next to remove it again two patches later would not be needed here.
> 
> If you would like to have this fixed for 5.16 and older stable kernels I could go ahead and apply it to wpan and let it trickle down into stable trees.

I'm fine "ignoring" the issue in stable kernels, it was just a warning
for you that this would happen otherwise, given the fact that this is
the second driver doing so (first fix has already been merged) and that
I just realized it now.

> We would have to deal with either a merge of net into net-next or with
> a merge conflicts when sending the pull request. Both can be done.
> 
> But given the circumstances above I have no problem to drop this fix completely and have it fixed implicitly with the rest of the patchset.

Fine by me!

Thanks,
Miquèl

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH wpan-next v2 5/5] net: ieee802154: Drop duration settings when the core does it already
  2022-02-02  7:40       ` Miquel Raynal
@ 2022-02-02 12:17         ` Stefan Schmidt
  2022-02-02 13:50           ` Miquel Raynal
  0 siblings, 1 reply; 24+ messages in thread
From: Stefan Schmidt @ 2022-02-02 12:17 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Alexander Aring, linux-wpan, David S. Miller, Jakub Kicinski,
	netdev, Michael Hennerich, Varka Bhadram, Xue Liu, Alan Ott

Hello.

On 02.02.22 08:40, Miquel Raynal wrote:
> Hi Stefan,
> 
> stefan@datenfreihafen.org wrote on Tue, 1 Feb 2022 21:51:04 +0100:
> 
>> Hello.
>>
>> On 01.02.22 18:40, Miquel Raynal wrote:
>>> Hi,
>>>    
>>>> --- a/drivers/net/ieee802154/ca8210.c
>>>> +++ b/drivers/net/ieee802154/ca8210.c
>>>> @@ -2978,7 +2978,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->sifs_period = 12;
>>>
>>> I've missed that error                ^^
>>>
>>> This driver should be fixed first (that's probably a copy/paste of the
>>> error from the other driver which did the same).
>>>
>>> As the rest of the series will depend on this fix (or conflict) we could
>>> merge it through wpan-next anyway, if you don't mind, as it was there
>>> since 2017 and these numbers had no real impact so far (I believe).
>>
>> Not sure I follow this logic. The fix you do is being removed in 4/4 of your v3 set again. So it would only be in place for these two in between commits.
> 
> Exactly.
> 
>> As you laid out above this has been in place since 2017 and the number have no real impact. Getting the fix in wpan-next to remove it again two patches later would not be needed here.
>>
>> If you would like to have this fixed for 5.16 and older stable kernels I could go ahead and apply it to wpan and let it trickle down into stable trees.
> 
> I'm fine "ignoring" the issue in stable kernels, it was just a warning
> for you that this would happen otherwise, given the fact that this is
> the second driver doing so (first fix has already been merged) and that
> I just realized it now.
> 
>> We would have to deal with either a merge of net into net-next or with
>> a merge conflicts when sending the pull request. Both can be done.
>>
>> But given the circumstances above I have no problem to drop this fix completely and have it fixed implicitly with the rest of the patchset.
> 
> Fine by me!

Let's do it like this.
You drop it from this series against wpan-next.
I will pull it out of the series and apply to wpan directly. That way we 
get it into the stable kernels as well. You already did the work so we 
should not waste it.
I will deal with the merge conflict get get between wpan/net and 
wpan-next/net-next on my side. Nothing to worry for you.

regards
Stefan Schmidt


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH wpan-next v2 5/5] net: ieee802154: Drop duration settings when the core does it already
  2022-02-02 12:17         ` Stefan Schmidt
@ 2022-02-02 13:50           ` Miquel Raynal
  2022-02-02 17:07             ` Stefan Schmidt
  0 siblings, 1 reply; 24+ messages in thread
From: Miquel Raynal @ 2022-02-02 13:50 UTC (permalink / raw)
  To: Stefan Schmidt
  Cc: Alexander Aring, linux-wpan, David S. Miller, Jakub Kicinski,
	netdev, Michael Hennerich, Varka Bhadram, Xue Liu, Alan Ott

Hi Stefan,

stefan@datenfreihafen.org wrote on Wed, 2 Feb 2022 13:17:39 +0100:

> Hello.
> 
> On 02.02.22 08:40, Miquel Raynal wrote:
> > Hi Stefan,
> > 
> > stefan@datenfreihafen.org wrote on Tue, 1 Feb 2022 21:51:04 +0100:
> >   
> >> Hello.
> >>
> >> On 01.02.22 18:40, Miquel Raynal wrote:  
> >>> Hi,  
> >>>    >>>> --- a/drivers/net/ieee802154/ca8210.c  
> >>>> +++ b/drivers/net/ieee802154/ca8210.c
> >>>> @@ -2978,7 +2978,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->sifs_period = 12;  
> >>>
> >>> I've missed that error                ^^
> >>>
> >>> This driver should be fixed first (that's probably a copy/paste of the
> >>> error from the other driver which did the same).
> >>>
> >>> As the rest of the series will depend on this fix (or conflict) we could
> >>> merge it through wpan-next anyway, if you don't mind, as it was there
> >>> since 2017 and these numbers had no real impact so far (I believe).  
> >>
> >> Not sure I follow this logic. The fix you do is being removed in 4/4 of your v3 set again. So it would only be in place for these two in between commits.  
> > 
> > Exactly.
> >   
> >> As you laid out above this has been in place since 2017 and the number have no real impact. Getting the fix in wpan-next to remove it again two patches later would not be needed here.
> >>
> >> If you would like to have this fixed for 5.16 and older stable kernels I could go ahead and apply it to wpan and let it trickle down into stable trees.  
> > 
> > I'm fine "ignoring" the issue in stable kernels, it was just a warning
> > for you that this would happen otherwise, given the fact that this is
> > the second driver doing so (first fix has already been merged) and that
> > I just realized it now.
> >   
> >> We would have to deal with either a merge of net into net-next or with
> >> a merge conflicts when sending the pull request. Both can be done.
> >>
> >> But given the circumstances above I have no problem to drop this fix completely and have it fixed implicitly with the rest of the patchset.  
> > 
> > Fine by me!  
> 
> Let's do it like this.
> You drop it from this series against wpan-next.
> I will pull it out of the series and apply to wpan directly. That way we get it into the stable kernels as well. You already did the work so we should not waste it.
> I will deal with the merge conflict get get between wpan/net and wpan-next/net-next on my side. Nothing to worry for you.

That's very kind, but don't feel forced to do that, I won't turn mad if
you finally decide that this requires too much handling for such a
short-in-time improvement ;)

Thanks,
Miquèl

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH wpan-next v2 5/5] net: ieee802154: Drop duration settings when the core does it already
  2022-02-02 13:50           ` Miquel Raynal
@ 2022-02-02 17:07             ` Stefan Schmidt
  0 siblings, 0 replies; 24+ messages in thread
From: Stefan Schmidt @ 2022-02-02 17:07 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Alexander Aring, linux-wpan, David S. Miller, Jakub Kicinski,
	netdev, Michael Hennerich, Varka Bhadram, Xue Liu, Alan Ott


Hello.

On 02.02.22 14:50, Miquel Raynal wrote:
> Hi Stefan,
> 
> stefan@datenfreihafen.org wrote on Wed, 2 Feb 2022 13:17:39 +0100:
> 
>> Hello.
>>
>> On 02.02.22 08:40, Miquel Raynal wrote:
>>> Hi Stefan,
>>>
>>> stefan@datenfreihafen.org wrote on Tue, 1 Feb 2022 21:51:04 +0100:
>>>    
>>>> Hello.
>>>>
>>>> On 01.02.22 18:40, Miquel Raynal wrote:
>>>>> Hi,
>>>>>     >>>> --- a/drivers/net/ieee802154/ca8210.c
>>>>>> +++ b/drivers/net/ieee802154/ca8210.c
>>>>>> @@ -2978,7 +2978,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->sifs_period = 12;
>>>>>
>>>>> I've missed that error                ^^
>>>>>
>>>>> This driver should be fixed first (that's probably a copy/paste of the
>>>>> error from the other driver which did the same).
>>>>>
>>>>> As the rest of the series will depend on this fix (or conflict) we could
>>>>> merge it through wpan-next anyway, if you don't mind, as it was there
>>>>> since 2017 and these numbers had no real impact so far (I believe).
>>>>
>>>> Not sure I follow this logic. The fix you do is being removed in 4/4 of your v3 set again. So it would only be in place for these two in between commits.
>>>
>>> Exactly.
>>>    
>>>> As you laid out above this has been in place since 2017 and the number have no real impact. Getting the fix in wpan-next to remove it again two patches later would not be needed here.
>>>>
>>>> If you would like to have this fixed for 5.16 and older stable kernels I could go ahead and apply it to wpan and let it trickle down into stable trees.
>>>
>>> I'm fine "ignoring" the issue in stable kernels, it was just a warning
>>> for you that this would happen otherwise, given the fact that this is
>>> the second driver doing so (first fix has already been merged) and that
>>> I just realized it now.
>>>    
>>>> We would have to deal with either a merge of net into net-next or with
>>>> a merge conflicts when sending the pull request. Both can be done.
>>>>
>>>> But given the circumstances above I have no problem to drop this fix completely and have it fixed implicitly with the rest of the patchset.
>>>
>>> Fine by me!
>>
>> Let's do it like this.
>> You drop it from this series against wpan-next.
>> I will pull it out of the series and apply to wpan directly. That way we get it into the stable kernels as well. You already did the work so we should not waste it.
>> I will deal with the merge conflict get get between wpan/net and wpan-next/net-next on my side. Nothing to worry for you.
> 
> That's very kind, but don't feel forced to do that, I won't turn mad if
> you finally decide that this requires too much handling for such a
> short-in-time improvement ;)

Nah, don't worry. I just applied it to wpan. It's the right thing to do 
as we simply do not know who is using the driving in what situation. 
Holding off a fix that you already did would be silly.

I deal with the merge conflict when it comes to it.

regards
Stefan Schmidt

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH wpan-next v2 1/5] net: ieee802154: Improve the way supported channels are declared
  2022-02-01 14:55         ` Miquel Raynal
@ 2022-02-06 21:37           ` Alexander Aring
  2022-02-07  7:49             ` Miquel Raynal
  0 siblings, 1 reply; 24+ messages in thread
From: Alexander Aring @ 2022-02-06 21:37 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Stefan Schmidt, linux-wpan - ML, David S. Miller, Jakub Kicinski,
	open list:NETWORKING [GENERAL],
	Michael Hennerich, Varka Bhadram, Xue Liu, Alan Ott

Hi,

On Tue, Feb 1, 2022 at 9:55 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
...
>
> Given the new information that I am currently processing, I believe the
> array is not needed anymore, we can live with a minimal number of
> additional helpers, like the one getting the PRF value for the UWB
> PHYs. It's the only one I have in mind so far.

I am not really sure if I understood now. So far those channel/page
combinations are the same because we have no special "type" value in
wpan_phy, what we currently support is the "normal" (I think they name
it legacy devices) phy type (no UWB, sun phy, whatever) and as Channel
Assignments says that it does not apply for those PHY's I think it
there are channel/page combinations which are different according to
the PHY "type". However we don't support them and I think there might
be an upcoming type field in wpan_phy which might be set only once at
registration time.

- Alex

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH wpan-next v2 1/5] net: ieee802154: Improve the way supported channels are declared
  2022-02-06 21:37           ` Alexander Aring
@ 2022-02-07  7:49             ` Miquel Raynal
  2022-02-20 23:05               ` Alexander Aring
  0 siblings, 1 reply; 24+ messages in thread
From: Miquel Raynal @ 2022-02-07  7:49 UTC (permalink / raw)
  To: Alexander Aring
  Cc: Stefan Schmidt, linux-wpan - ML, David S. Miller, Jakub Kicinski,
	open list:NETWORKING [GENERAL],
	Michael Hennerich, Varka Bhadram, Xue Liu, Alan Ott

Hi Alexander,

alex.aring@gmail.com wrote on Sun, 6 Feb 2022 16:37:23 -0500:

> Hi,
> 
> On Tue, Feb 1, 2022 at 9:55 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> ...
> >
> > Given the new information that I am currently processing, I believe the
> > array is not needed anymore, we can live with a minimal number of
> > additional helpers, like the one getting the PRF value for the UWB
> > PHYs. It's the only one I have in mind so far.  
> 
> I am not really sure if I understood now. So far those channel/page
> combinations are the same because we have no special "type" value in
> wpan_phy,

Yes, my assumption was more: I know there are only -legacy- phy types
supported, we will add another (or improve the current) way of defining
channels when we'll need to. Eg when improving UWB support.

> what we currently support is the "normal" (I think they name
> it legacy devices) phy type (no UWB, sun phy, whatever) and as Channel
> Assignments says that it does not apply for those PHY's I think it
> there are channel/page combinations which are different according to
> the PHY "type". However we don't support them and I think there might
> be an upcoming type field in wpan_phy which might be set only once at
> registration time.

An idea might be to create a callback that drivers might decide to
implement or not. If they implement it, the core might call it to get
further information about the channels. The core would provide a {page,
channel} couple and retrieve a structure with many information such as
the the frequency, the protocol, eventually the prf, etc.

Thanks,
Miquèl

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH wpan-next v2 1/5] net: ieee802154: Improve the way supported channels are declared
  2022-02-07  7:49             ` Miquel Raynal
@ 2022-02-20 23:05               ` Alexander Aring
  2022-03-02 13:21                 ` Miquel Raynal
  0 siblings, 1 reply; 24+ messages in thread
From: Alexander Aring @ 2022-02-20 23:05 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Stefan Schmidt, linux-wpan - ML, David S. Miller, Jakub Kicinski,
	open list:NETWORKING [GENERAL],
	Michael Hennerich, Varka Bhadram, Xue Liu, Alan Ott

Hi,

On Mon, Feb 7, 2022 at 2:49 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Alexander,
>
> alex.aring@gmail.com wrote on Sun, 6 Feb 2022 16:37:23 -0500:
>
> > Hi,
> >
> > On Tue, Feb 1, 2022 at 9:55 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > ...
> > >
> > > Given the new information that I am currently processing, I believe the
> > > array is not needed anymore, we can live with a minimal number of
> > > additional helpers, like the one getting the PRF value for the UWB
> > > PHYs. It's the only one I have in mind so far.
> >
> > I am not really sure if I understood now. So far those channel/page
> > combinations are the same because we have no special "type" value in
> > wpan_phy,
>
> Yes, my assumption was more: I know there are only -legacy- phy types
> supported, we will add another (or improve the current) way of defining
> channels when we'll need to. Eg when improving UWB support.
>
> > what we currently support is the "normal" (I think they name
> > it legacy devices) phy type (no UWB, sun phy, whatever) and as Channel
> > Assignments says that it does not apply for those PHY's I think it
> > there are channel/page combinations which are different according to
> > the PHY "type". However we don't support them and I think there might
> > be an upcoming type field in wpan_phy which might be set only once at
> > registration time.
>
> An idea might be to create a callback that drivers might decide to
> implement or not. If they implement it, the core might call it to get
> further information about the channels. The core would provide a {page,
> channel} couple and retrieve a structure with many information such as
> the the frequency, the protocol, eventually the prf, etc.
>

As I said before, for "many information" we should look at how
wireless is using that with regdb and extend it with 802.15.4
channels/etc. The kernel should only deal with an unique
identification of a database key for "regdb" which so far I see is a
combination of phy type, page id and channel id. Then from "somewhere"
also the country code gets involved into that and you get a subset of
what is available.

- Alex

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH wpan-next v2 1/5] net: ieee802154: Improve the way supported channels are declared
  2022-02-20 23:05               ` Alexander Aring
@ 2022-03-02 13:21                 ` Miquel Raynal
  2022-03-13 20:58                   ` Alexander Aring
  0 siblings, 1 reply; 24+ messages in thread
From: Miquel Raynal @ 2022-03-02 13:21 UTC (permalink / raw)
  To: Alexander Aring
  Cc: Stefan Schmidt, linux-wpan - ML, David S. Miller, Jakub Kicinski,
	open list:NETWORKING [GENERAL],
	Michael Hennerich, Varka Bhadram, Xue Liu, Alan Ott

Hi Alexander,

alex.aring@gmail.com wrote on Sun, 20 Feb 2022 18:05:39 -0500:

> Hi,
> 
> On Mon, Feb 7, 2022 at 2:49 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > Hi Alexander,
> >
> > alex.aring@gmail.com wrote on Sun, 6 Feb 2022 16:37:23 -0500:
> >  
> > > Hi,
> > >
> > > On Tue, Feb 1, 2022 at 9:55 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > > ...  
> > > >
> > > > Given the new information that I am currently processing, I believe the
> > > > array is not needed anymore, we can live with a minimal number of
> > > > additional helpers, like the one getting the PRF value for the UWB
> > > > PHYs. It's the only one I have in mind so far.  
> > >
> > > I am not really sure if I understood now. So far those channel/page
> > > combinations are the same because we have no special "type" value in
> > > wpan_phy,  
> >
> > Yes, my assumption was more: I know there are only -legacy- phy types
> > supported, we will add another (or improve the current) way of defining
> > channels when we'll need to. Eg when improving UWB support.
> >  
> > > what we currently support is the "normal" (I think they name
> > > it legacy devices) phy type (no UWB, sun phy, whatever) and as Channel
> > > Assignments says that it does not apply for those PHY's I think it
> > > there are channel/page combinations which are different according to
> > > the PHY "type". However we don't support them and I think there might
> > > be an upcoming type field in wpan_phy which might be set only once at
> > > registration time.  
> >
> > An idea might be to create a callback that drivers might decide to
> > implement or not. If they implement it, the core might call it to get
> > further information about the channels. The core would provide a {page,
> > channel} couple and retrieve a structure with many information such as
> > the the frequency, the protocol, eventually the prf, etc.
> >  
> 
> As I said before, for "many information" we should look at how
> wireless is using that with regdb and extend it with 802.15.4
> channels/etc. The kernel should only deal with an unique
> identification of a database key for "regdb" which so far I see is a
> combination of phy type, page id and channel id. Then from "somewhere"
> also the country code gets involved into that and you get a subset of
> what is available.

Do you want another implementation of regdb that would support the
802.15.4 world only (so far it is highly 802.11 oriented) ? Or is this
something that you would like to merge in the existing project?

Overall it can be useful to define what is allowed in different
countries but this will not save us from needing extra information from
the devices. Describing the channels and protocols (and PRFs) for an
UWB PHY has nothing to do with the regulatory database, it's just
listing what is supported by the device. The actual location where it
might be useful to have a regdb (but not mandatory at the beginning)
would be when changing channels to avoid messing with local
regulations, I believe?

Thanks,
Miquèl

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH wpan-next v2 1/5] net: ieee802154: Improve the way supported channels are declared
  2022-03-02 13:21                 ` Miquel Raynal
@ 2022-03-13 20:58                   ` Alexander Aring
  2022-03-18  9:09                     ` Miquel Raynal
  0 siblings, 1 reply; 24+ messages in thread
From: Alexander Aring @ 2022-03-13 20:58 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Stefan Schmidt, linux-wpan - ML, David S. Miller, Jakub Kicinski,
	open list:NETWORKING [GENERAL],
	Michael Hennerich, Varka Bhadram, Xue Liu, Alan Ott

Hi,

On Wed, Mar 2, 2022 at 8:21 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Alexander,
>
> alex.aring@gmail.com wrote on Sun, 20 Feb 2022 18:05:39 -0500:
>
> > Hi,
> >
> > On Mon, Feb 7, 2022 at 2:49 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > >
> > > Hi Alexander,
> > >
> > > alex.aring@gmail.com wrote on Sun, 6 Feb 2022 16:37:23 -0500:
> > >
> > > > Hi,
> > > >
> > > > On Tue, Feb 1, 2022 at 9:55 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > > > ...
> > > > >
> > > > > Given the new information that I am currently processing, I believe the
> > > > > array is not needed anymore, we can live with a minimal number of
> > > > > additional helpers, like the one getting the PRF value for the UWB
> > > > > PHYs. It's the only one I have in mind so far.
> > > >
> > > > I am not really sure if I understood now. So far those channel/page
> > > > combinations are the same because we have no special "type" value in
> > > > wpan_phy,
> > >
> > > Yes, my assumption was more: I know there are only -legacy- phy types
> > > supported, we will add another (or improve the current) way of defining
> > > channels when we'll need to. Eg when improving UWB support.
> > >
> > > > what we currently support is the "normal" (I think they name
> > > > it legacy devices) phy type (no UWB, sun phy, whatever) and as Channel
> > > > Assignments says that it does not apply for those PHY's I think it
> > > > there are channel/page combinations which are different according to
> > > > the PHY "type". However we don't support them and I think there might
> > > > be an upcoming type field in wpan_phy which might be set only once at
> > > > registration time.
> > >
> > > An idea might be to create a callback that drivers might decide to
> > > implement or not. If they implement it, the core might call it to get
> > > further information about the channels. The core would provide a {page,
> > > channel} couple and retrieve a structure with many information such as
> > > the the frequency, the protocol, eventually the prf, etc.
> > >
> >
> > As I said before, for "many information" we should look at how
> > wireless is using that with regdb and extend it with 802.15.4
> > channels/etc. The kernel should only deal with an unique
> > identification of a database key for "regdb" which so far I see is a
> > combination of phy type, page id and channel id. Then from "somewhere"
> > also the country code gets involved into that and you get a subset of
> > what is available.
>
> Do you want another implementation of regdb that would support the
> 802.15.4 world only (so far it is highly 802.11 oriented) ? Or is this
> something that you would like to merge in the existing project?
>

I think we should run the strategy like wpan-tools, fork it but leave
it open that probably they can be merged in future. How about that?

I don't like that it is wireless standard specific, it should be
specific to the standard which defines the regulation... As an
example, I remember that at86rf212 has some LBT (listen before
transmit) mode because of some duty cycle regulations in some
countries. The regdb should not contain if LBT should be used in a
country for specific sub 1Ghz range, etc. It should contain the duty
cycle allowance. That's an example of what I mean with "wireless
standard" and "regulation standard". However the regulation for sub
1Ghz is also a little bit crazy so far I see. :)

However I really don't know if this is extremely difficult to handle.
I would say this would be the better approach but if it doesn't work
do it wireless specific. So it's up to whoever wants to do the work?

> Overall it can be useful to define what is allowed in different
> countries but this will not save us from needing extra information from
> the devices. Describing the channels and protocols (and PRFs) for an
> UWB PHY has nothing to do with the regulatory database, it's just
> listing what is supported by the device. The actual location where it
> might be useful to have a regdb (but not mandatory at the beginning)
> would be when changing channels to avoid messing with local
> regulations, I believe?
>

I see, but I am not sure what additional information you need as
channel, page, phy type? And if you have those values in user space
you can get other information out of it, or not? Why does the kernel
need to handle more than necessary? Even there we can use helpers to
map those combinations to something else. Just avoid that drivers
declare those information what they already declared and introduce
helpers to whatever higher level information you want to get out of
it.

- Alex

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH wpan-next v2 1/5] net: ieee802154: Improve the way supported channels are declared
  2022-03-13 20:58                   ` Alexander Aring
@ 2022-03-18  9:09                     ` Miquel Raynal
  0 siblings, 0 replies; 24+ messages in thread
From: Miquel Raynal @ 2022-03-18  9:09 UTC (permalink / raw)
  To: Alexander Aring
  Cc: Stefan Schmidt, linux-wpan - ML, David S. Miller, Jakub Kicinski,
	open list:NETWORKING [GENERAL],
	Michael Hennerich, Varka Bhadram, Xue Liu, Alan Ott

Hi Alexander,

alex.aring@gmail.com wrote on Sun, 13 Mar 2022 16:58:01 -0400:

> Hi,
> 
> On Wed, Mar 2, 2022 at 8:21 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > Hi Alexander,
> >
> > alex.aring@gmail.com wrote on Sun, 20 Feb 2022 18:05:39 -0500:
> >  
> > > Hi,
> > >
> > > On Mon, Feb 7, 2022 at 2:49 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:  
> > > >
> > > > Hi Alexander,
> > > >
> > > > alex.aring@gmail.com wrote on Sun, 6 Feb 2022 16:37:23 -0500:
> > > >  
> > > > > Hi,
> > > > >
> > > > > On Tue, Feb 1, 2022 at 9:55 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > > > > ...  
> > > > > >
> > > > > > Given the new information that I am currently processing, I believe the
> > > > > > array is not needed anymore, we can live with a minimal number of
> > > > > > additional helpers, like the one getting the PRF value for the UWB
> > > > > > PHYs. It's the only one I have in mind so far.  
> > > > >
> > > > > I am not really sure if I understood now. So far those channel/page
> > > > > combinations are the same because we have no special "type" value in
> > > > > wpan_phy,  
> > > >
> > > > Yes, my assumption was more: I know there are only -legacy- phy types
> > > > supported, we will add another (or improve the current) way of defining
> > > > channels when we'll need to. Eg when improving UWB support.
> > > >  
> > > > > what we currently support is the "normal" (I think they name
> > > > > it legacy devices) phy type (no UWB, sun phy, whatever) and as Channel
> > > > > Assignments says that it does not apply for those PHY's I think it
> > > > > there are channel/page combinations which are different according to
> > > > > the PHY "type". However we don't support them and I think there might
> > > > > be an upcoming type field in wpan_phy which might be set only once at
> > > > > registration time.  
> > > >
> > > > An idea might be to create a callback that drivers might decide to
> > > > implement or not. If they implement it, the core might call it to get
> > > > further information about the channels. The core would provide a {page,
> > > > channel} couple and retrieve a structure with many information such as
> > > > the the frequency, the protocol, eventually the prf, etc.
> > > >  
> > >
> > > As I said before, for "many information" we should look at how
> > > wireless is using that with regdb and extend it with 802.15.4
> > > channels/etc. The kernel should only deal with an unique
> > > identification of a database key for "regdb" which so far I see is a
> > > combination of phy type, page id and channel id. Then from "somewhere"
> > > also the country code gets involved into that and you get a subset of
> > > what is available.  
> >
> > Do you want another implementation of regdb that would support the
> > 802.15.4 world only (so far it is highly 802.11 oriented) ? Or is this
> > something that you would like to merge in the existing project?
> >  
> 
> I think we should run the strategy like wpan-tools, fork it but leave
> it open that probably they can be merged in future. How about that?
> 
> I don't like that it is wireless standard specific, it should be
> specific to the standard which defines the regulation... As an
> example, I remember that at86rf212 has some LBT (listen before
> transmit) mode because of some duty cycle regulations in some
> countries. The regdb should not contain if LBT should be used in a
> country for specific sub 1Ghz range, etc. It should contain the duty
> cycle allowance. That's an example of what I mean with "wireless
> standard" and "regulation standard". However the regulation for sub
> 1Ghz is also a little bit crazy so far I see. :)
> 
> However I really don't know if this is extremely difficult to handle.
> I would say this would be the better approach but if it doesn't work
> do it wireless specific. So it's up to whoever wants to do the work?
> 
> > Overall it can be useful to define what is allowed in different
> > countries but this will not save us from needing extra information from
> > the devices. Describing the channels and protocols (and PRFs) for an
> > UWB PHY has nothing to do with the regulatory database, it's just
> > listing what is supported by the device. The actual location where it
> > might be useful to have a regdb (but not mandatory at the beginning)
> > would be when changing channels to avoid messing with local
> > regulations, I believe?
> >  
> 
> I see, but I am not sure what additional information you need as
> channel, page, phy type?

For a UWB PHY: the preamble code and the PRF, I believe.

> And if you have those values in user space
> you can get other information out of it, or not? Why does the kernel
> need to handle more than necessary? Even there we can use helpers to
> map those combinations to something else. Just avoid that drivers
> declare those information what they already declared and introduce
> helpers to whatever higher level information you want to get out of
> it.

I'll look into it soon.

Thanks,
Miquèl

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2022-03-18  9:09 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-28 11:08 [PATCH wpan-next v2 0/5] ieee802154: Improve durations handling Miquel Raynal
2022-01-28 11:08 ` [PATCH wpan-next v2 1/5] net: ieee802154: Improve the way supported channels are declared Miquel Raynal
2022-01-30 21:35   ` Alexander Aring
2022-01-31 14:23     ` Miquel Raynal
2022-02-01  0:04       ` Alexander Aring
2022-02-01 14:55         ` Miquel Raynal
2022-02-06 21:37           ` Alexander Aring
2022-02-07  7:49             ` Miquel Raynal
2022-02-20 23:05               ` Alexander Aring
2022-03-02 13:21                 ` Miquel Raynal
2022-03-13 20:58                   ` Alexander Aring
2022-03-18  9:09                     ` Miquel Raynal
2022-01-28 11:08 ` [PATCH wpan-next v2 2/5] net: ieee802154: Give more details to the core about the channel configurations Miquel Raynal
2022-01-28 11:08 ` [PATCH wpan-next v2 3/5] net: mac802154: Convert the symbol duration into nanoseconds Miquel Raynal
2022-01-28 13:00   ` Stefan Schmidt
2022-01-28 11:08 ` [PATCH wpan-next v2 4/5] net: mac802154: Set durations automatically Miquel Raynal
2022-01-28 11:08 ` [PATCH wpan-next v2 5/5] net: ieee802154: Drop duration settings when the core does it already Miquel Raynal
2022-02-01 17:40   ` Miquel Raynal
2022-02-01 20:51     ` Stefan Schmidt
2022-02-02  7:40       ` Miquel Raynal
2022-02-02 12:17         ` Stefan Schmidt
2022-02-02 13:50           ` Miquel Raynal
2022-02-02 17:07             ` Stefan Schmidt
2022-01-28 11:11 ` [PATCH wpan-next v2 0/5] ieee802154: Improve durations handling Miquel Raynal

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).