| Message ID | 20220112173312.764660-7-miquel.raynal@bootlin.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | IEEE 802.15.4 scan support | expand |
Hi, On Wed, 12 Jan 2022 at 12:33, Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > 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. > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > --- > include/net/cfg802154.h | 2 + > net/mac802154/cfg.c | 1 + > net/mac802154/main.c | 93 +++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 96 insertions(+) > > diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h > index 286709a9dd0b..52eefc4b5b4d 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_set_symbol_duration(struct wpan_phy *phy); > + > #endif /* __NET_CFG802154_H */ > diff --git a/net/mac802154/cfg.c b/net/mac802154/cfg.c > index 6969f1330ccd..ba57da07c08e 100644 > --- a/net/mac802154/cfg.c > +++ b/net/mac802154/cfg.c > @@ -113,6 +113,7 @@ int ieee802154_change_channel(struct wpan_phy *wpan_phy, u8 page, u8 channel) > if (!ret) { > wpan_phy->current_page = page; > wpan_phy->current_channel = channel; > + ieee802154_set_symbol_duration(wpan_phy); > } > > return ret; We also need to do it in ieee802154_register_hw()? > diff --git a/net/mac802154/main.c b/net/mac802154/main.c > index 77a4943f345f..88826c5aa4ba 100644 > --- a/net/mac802154/main.c > +++ b/net/mac802154/main.c > @@ -113,6 +113,99 @@ ieee802154_alloc_hw(size_t priv_data_len, const struct ieee802154_ops *ops) > } > EXPORT_SYMBOL(ieee802154_alloc_hw); > > +void ieee802154_set_symbol_duration(struct wpan_phy *phy) > +{ > + struct phy_page *page = &phy->supported.page[phy->current_page]; > + struct phy_channels *chan; > + unsigned int chunk; > + u32 duration = 0; > + > + for (chunk = 0; chunk < page->nchunks; chunk++) { > + if (page->chunk[chunk].channels & phy->current_channel) > + break; > + } > + > + if (chunk == page->nchunks) > + goto set_duration; > + > + 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 * 1000; * NSEC_PER_USEC? - Alex
Hi Alexander, alex.aring@gmail.com wrote on Wed, 12 Jan 2022 17:25:01 -0500: > Hi, > > On Wed, 12 Jan 2022 at 12:33, Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > 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. > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > --- > > include/net/cfg802154.h | 2 + > > net/mac802154/cfg.c | 1 + > > net/mac802154/main.c | 93 +++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 96 insertions(+) > > > > diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h > > index 286709a9dd0b..52eefc4b5b4d 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_set_symbol_duration(struct wpan_phy *phy); > > + > > #endif /* __NET_CFG802154_H */ > > diff --git a/net/mac802154/cfg.c b/net/mac802154/cfg.c > > index 6969f1330ccd..ba57da07c08e 100644 > > --- a/net/mac802154/cfg.c > > +++ b/net/mac802154/cfg.c > > @@ -113,6 +113,7 @@ int ieee802154_change_channel(struct wpan_phy *wpan_phy, u8 page, u8 channel) > > if (!ret) { > > wpan_phy->current_page = page; > > wpan_phy->current_channel = channel; > > + ieee802154_set_symbol_duration(wpan_phy); > > } > > > > return ret; > > We also need to do it in ieee802154_register_hw()? As you probably saw, my idea was to call for a channel change during the registration but you nacked that possibility so I'll indeed have to set the symbol duration manually there. > > diff --git a/net/mac802154/main.c b/net/mac802154/main.c > > index 77a4943f345f..88826c5aa4ba 100644 > > --- a/net/mac802154/main.c > > +++ b/net/mac802154/main.c > > @@ -113,6 +113,99 @@ ieee802154_alloc_hw(size_t priv_data_len, const struct ieee802154_ops *ops) > > } > > EXPORT_SYMBOL(ieee802154_alloc_hw); > > > > +void ieee802154_set_symbol_duration(struct wpan_phy *phy) > > +{ > > + struct phy_page *page = &phy->supported.page[phy->current_page]; > > + struct phy_channels *chan; > > + unsigned int chunk; > > + u32 duration = 0; > > + > > + for (chunk = 0; chunk < page->nchunks; chunk++) { > > + if (page->chunk[chunk].channels & phy->current_channel) .channels still being a bitfield, David allegedly reported that the above line should use "& BIT(phy->current_channel)". > > + break; > > + } > > + > > + if (chunk == page->nchunks) > > + goto set_duration; > > + > > + 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 * 1000; > > * NSEC_PER_USEC? Oh right, I grepped for USEC_TO_NSEC but the macro was named the other way around, thanks. Thanks, Miquèl
Hi, On Wed, 12 Jan 2022 at 12:33, Miquel Raynal <miquel.raynal@bootlin.com> wrote: ... > + case IEEE802154_4030KHZ_MEAN_PRF: > + duration = 3974; > + break; > + case IEEE802154_62890KHZ_MEAN_PRF: > + duration = 1018; > + break; > + default: > + break; > + } > + break; > + default: > + break; > + } > + > +set_duration: > + if (!duration) > + pr_debug("Unknown PHY symbol duration, the driver should be fixed\n"); Why should the driver be fixed? It's more this table which needs to be fixed? > + else > + phy->symbol_duration = duration; Can you also set the lifs/sifs period after the duration is known? - Alex
Hi Alexander, alex.aring@gmail.com wrote on Thu, 13 Jan 2022 18:36:15 -0500: > Hi, > > On Wed, 12 Jan 2022 at 12:33, Miquel Raynal <miquel.raynal@bootlin.com> wrote: > ... > > + case IEEE802154_4030KHZ_MEAN_PRF: > > + duration = 3974; > > + break; > > + case IEEE802154_62890KHZ_MEAN_PRF: > > + duration = 1018; > > + break; > > + default: > > + break; > > + } > > + break; > > + default: > > + break; > > + } > > + > > +set_duration: > > + if (!duration) > > + pr_debug("Unknown PHY symbol duration, the driver should be fixed\n"); > > Why should the driver be fixed? It's more this table which needs to be fixed? Right. > > > + else > > + phy->symbol_duration = duration; > > Can you also set the lifs/sifs period after the duration is known? Done. Thanks, Miquèl
diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h index 286709a9dd0b..52eefc4b5b4d 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_set_symbol_duration(struct wpan_phy *phy); + #endif /* __NET_CFG802154_H */ diff --git a/net/mac802154/cfg.c b/net/mac802154/cfg.c index 6969f1330ccd..ba57da07c08e 100644 --- a/net/mac802154/cfg.c +++ b/net/mac802154/cfg.c @@ -113,6 +113,7 @@ int ieee802154_change_channel(struct wpan_phy *wpan_phy, u8 page, u8 channel) if (!ret) { wpan_phy->current_page = page; wpan_phy->current_channel = channel; + ieee802154_set_symbol_duration(wpan_phy); } return ret; diff --git a/net/mac802154/main.c b/net/mac802154/main.c index 77a4943f345f..88826c5aa4ba 100644 --- a/net/mac802154/main.c +++ b/net/mac802154/main.c @@ -113,6 +113,99 @@ ieee802154_alloc_hw(size_t priv_data_len, const struct ieee802154_ops *ops) } EXPORT_SYMBOL(ieee802154_alloc_hw); +void ieee802154_set_symbol_duration(struct wpan_phy *phy) +{ + struct phy_page *page = &phy->supported.page[phy->current_page]; + struct phy_channels *chan; + unsigned int chunk; + u32 duration = 0; + + for (chunk = 0; chunk < page->nchunks; chunk++) { + if (page->chunk[chunk].channels & phy->current_channel) + break; + } + + if (chunk == page->nchunks) + goto set_duration; + + 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 * 1000; + break; + case IEEE802154_915_MHZ_BAND: + /* 915 MHz BPSK 802.15.4-2003: 40 ksym/s */ + duration = 25 * 1000; + 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 * 1000; + 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 * 1000; + 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 * 1000; + 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: + break; + default: + goto set_duration; + } + + /* 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; + } + +set_duration: + if (!duration) + pr_debug("Unknown PHY symbol duration, the driver should be fixed\n"); + else + phy->symbol_duration = duration; +} +EXPORT_SYMBOL(ieee802154_set_symbol_duration); + void ieee802154_free_hw(struct ieee802154_hw *hw) { struct ieee802154_local *local = hw_to_local(hw);
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. Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> --- include/net/cfg802154.h | 2 + net/mac802154/cfg.c | 1 + net/mac802154/main.c | 93 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 96 insertions(+)