miquel.raynal@xxxxxxxxxxx wrote on Thu, 28 Apr 2022 17:58:38 +0200:
> Hi Stefan,
>
> miquel.raynal@xxxxxxxxxxx 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@xxxxxxxxxxx>
> > ---
> > 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@xxxxxxxxxxx>
> Date: Thu Apr 28 17:57:59 2022 +0200
>
> fixup! net: mac802154: Set durations automatically
And I keep having it wrong. Here is the right fixup!...
commit 676aa77f1c279a90f521c1c05757c702e1538472 (HEAD)
Author: Miquel Raynal <miquel.raynal@xxxxxxxxxxx>
Date: Thu Apr 28 17:57:59 2022 +0200
fixup! net: mac802154: Set durations automatically
diff --git a/net/mac802154/main.c b/net/mac802154/main.c
index 5546ef86e231..bd7bdb1219dd 100644
--- a/net/mac802154/main.c
+++ b/net/mac802154/main.c
@@ -119,26 +119,26 @@ void ieee802154_configure_durations(struct wpan_phy *phy)
switch (phy->current_page) {
case 0:
- if (BIT(phy->current_page) & 0x1)
+ if (BIT(phy->current_channel) & 0x1)
/* 868 MHz BPSK 802.15.4-2003: 20 ksym/s */
duration = 50 * NSEC_PER_USEC;
- else if (phy->current_page & 0x7FE)
+ else if (BIT(phy->current_channel) & 0x7FE)
/* 915 MHz BPSK 802.15.4-2003: 40 ksym/s */
duration = 25 * NSEC_PER_USEC;
- else if (phy->current_page & 0x7FFF800)
+ else if (BIT(phy->current_channel) & 0x7FFF800)
/* 2400 MHz O-QPSK 802.15.4-2006: 62.5 ksym/s */
duration = 16 * NSEC_PER_USEC;
break;
case 2:
- if (BIT(phy->current_page) & 0x1)
+ if (BIT(phy->current_channel) & 0x1)
/* 868 MHz O-QPSK 802.15.4-2006: 25 ksym/s */
duration = 40 * NSEC_PER_USEC;
- else if (phy->current_page & 0x7FE)
+ else if (BIT(phy->current_channel) & 0x7FE)
/* 915 MHz O-QPSK 802.15.4-2006: 62.5 ksym/s */
duration = 16 * NSEC_PER_USEC;
break;
case 3:
- if (BIT(phy->current_page) & 0x3FFF)
+ if (BIT(phy->current_channel) & 0x3FFF)
/* 2.4 GHz CSS 802.15.4a-2007: 1/6 Msym/s */
duration = 6 * NSEC_PER_USEC;
break;
