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

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi Stefan,

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

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

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

all the conditions from here and below should be:

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

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

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

Ditto

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

Ditto

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

Ditto

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

Ditto

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

Ditto

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

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

Again, sorry to only see this now.

Thanks,
Miquèl

---

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

    fixup! net: mac802154: Set durations automatically

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