| Message ID | 20220112173312.764660-9-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: > > 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, so > just drop the duplicated code. > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > --- > drivers/net/ieee802154/ca8210.c | 1 - > drivers/net/ieee802154/mcr20a.c | 2 -- > 2 files changed, 3 deletions(-) > > diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c > index 82b2a173bdbd..d3a9e4fe05f4 100644 > --- a/drivers/net/ieee802154/ca8210.c > +++ b/drivers/net/ieee802154/ca8210.c > @@ -2977,7 +2977,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 * 1000; > 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 8aa87e9bf92e..da2ab19cb5ee 100644 > --- a/drivers/net/ieee802154/mcr20a.c > +++ b/drivers/net/ieee802154/mcr20a.c > @@ -975,7 +975,6 @@ static void mcr20a_hw_setup(struct mcr20a_local *lp) > > dev_dbg(printdev(lp), "%s\n", __func__); > > - phy->symbol_duration = 16 * 1000; > phy->lifs_period = 40; > phy->sifs_period = 12; > > @@ -1010,7 +1009,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 * 1000; > 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]; What's about the atrf86230 driver? - Alex
Hi Alexander, alex.aring@gmail.com wrote on Wed, 12 Jan 2022 17:26:14 -0500: > Hi, > > On Wed, 12 Jan 2022 at 12:33, Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > 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, so > > just drop the duplicated code. > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > --- > > drivers/net/ieee802154/ca8210.c | 1 - > > drivers/net/ieee802154/mcr20a.c | 2 -- > > 2 files changed, 3 deletions(-) > > > > diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c > > index 82b2a173bdbd..d3a9e4fe05f4 100644 > > --- a/drivers/net/ieee802154/ca8210.c > > +++ b/drivers/net/ieee802154/ca8210.c > > @@ -2977,7 +2977,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 * 1000; > > 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 8aa87e9bf92e..da2ab19cb5ee 100644 > > --- a/drivers/net/ieee802154/mcr20a.c > > +++ b/drivers/net/ieee802154/mcr20a.c > > @@ -975,7 +975,6 @@ static void mcr20a_hw_setup(struct mcr20a_local *lp) > > > > dev_dbg(printdev(lp), "%s\n", __func__); > > > > - phy->symbol_duration = 16 * 1000; > > phy->lifs_period = 40; > > phy->sifs_period = 12; > > > > @@ -1010,7 +1009,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 * 1000; > > 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]; > > What's about the atrf86230 driver? I couldn't find reliable information about what this meant: /* SUB:0 and BPSK:0 -> BPSK-20 */ /* SUB:1 and BPSK:0 -> BPSK-40 */ /* SUB:0 and BPSK:1 -> OQPSK-100/200/400 */ /* SUB:1 and BPSK:1 -> OQPSK-250/500/1000 */ None of these comments match the spec so I don't know what to put there. If you know what these protocols are, I will immediately provide this information into the driver and ensure the core handles these durations properly before dropping the symbol_durations settings from the code. Thanks, Miquèl
Hi, On Thu, 13 Jan 2022 at 06:16, Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > Hi Alexander, > > alex.aring@gmail.com wrote on Wed, 12 Jan 2022 17:26:14 -0500: > > > Hi, > > > > On Wed, 12 Jan 2022 at 12:33, Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > > > 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, so > > > just drop the duplicated code. > > > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > > --- > > > drivers/net/ieee802154/ca8210.c | 1 - > > > drivers/net/ieee802154/mcr20a.c | 2 -- > > > 2 files changed, 3 deletions(-) > > > > > > diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c > > > index 82b2a173bdbd..d3a9e4fe05f4 100644 > > > --- a/drivers/net/ieee802154/ca8210.c > > > +++ b/drivers/net/ieee802154/ca8210.c > > > @@ -2977,7 +2977,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 * 1000; > > > 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 8aa87e9bf92e..da2ab19cb5ee 100644 > > > --- a/drivers/net/ieee802154/mcr20a.c > > > +++ b/drivers/net/ieee802154/mcr20a.c > > > @@ -975,7 +975,6 @@ static void mcr20a_hw_setup(struct mcr20a_local *lp) > > > > > > dev_dbg(printdev(lp), "%s\n", __func__); > > > > > > - phy->symbol_duration = 16 * 1000; > > > phy->lifs_period = 40; > > > phy->sifs_period = 12; > > > > > > @@ -1010,7 +1009,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 * 1000; > > > 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]; > > > > What's about the atrf86230 driver? > > I couldn't find reliable information about what this meant: > > /* SUB:0 and BPSK:0 -> BPSK-20 */ > /* SUB:1 and BPSK:0 -> BPSK-40 */ > /* SUB:0 and BPSK:1 -> OQPSK-100/200/400 */ > /* SUB:1 and BPSK:1 -> OQPSK-250/500/1000 */ > > None of these comments match the spec so I don't know what to put > there. If you know what these protocols are, I will immediately > provide this information into the driver and ensure the core handles > these durations properly before dropping the symbol_durations settings > from the code. I think those are from the transceiver datasheets (which are free to access). Can you not simply merge them or is there a conflict? - Alex
Hi Alexander, alex.aring@gmail.com wrote on Thu, 13 Jan 2022 18:34:00 -0500: > Hi, > > On Thu, 13 Jan 2022 at 06:16, Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > Hi Alexander, > > > > alex.aring@gmail.com wrote on Wed, 12 Jan 2022 17:26:14 -0500: > > > > > Hi, > > > > > > On Wed, 12 Jan 2022 at 12:33, Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > > > > > 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, so > > > > just drop the duplicated code. > > > > > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > > > --- > > > > drivers/net/ieee802154/ca8210.c | 1 - > > > > drivers/net/ieee802154/mcr20a.c | 2 -- > > > > 2 files changed, 3 deletions(-) > > > > > > > > diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c > > > > index 82b2a173bdbd..d3a9e4fe05f4 100644 > > > > --- a/drivers/net/ieee802154/ca8210.c > > > > +++ b/drivers/net/ieee802154/ca8210.c > > > > @@ -2977,7 +2977,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 * 1000; > > > > 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 8aa87e9bf92e..da2ab19cb5ee 100644 > > > > --- a/drivers/net/ieee802154/mcr20a.c > > > > +++ b/drivers/net/ieee802154/mcr20a.c > > > > @@ -975,7 +975,6 @@ static void mcr20a_hw_setup(struct mcr20a_local *lp) > > > > > > > > dev_dbg(printdev(lp), "%s\n", __func__); > > > > > > > > - phy->symbol_duration = 16 * 1000; > > > > phy->lifs_period = 40; > > > > phy->sifs_period = 12; > > > > > > > > @@ -1010,7 +1009,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 * 1000; > > > > 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]; > > > > > > What's about the atrf86230 driver? > > > > I couldn't find reliable information about what this meant: > > > > /* SUB:0 and BPSK:0 -> BPSK-20 */ > > /* SUB:1 and BPSK:0 -> BPSK-40 */ > > /* SUB:0 and BPSK:1 -> OQPSK-100/200/400 */ > > /* SUB:1 and BPSK:1 -> OQPSK-250/500/1000 */ > > > > None of these comments match the spec so I don't know what to put > > there. If you know what these protocols are, I will immediately > > provide this information into the driver and ensure the core handles > > these durations properly before dropping the symbol_durations settings > > from the code. > > I think those are from the transceiver datasheets (which are free to > access). Can you not simply merge them or is there a conflict? Actually I misread the driver, it supports several kind of chips with different channel settings and this disturbed me. I downloaded the datasheet and figured that the number after the protocol is the bit rate. This helped me to make the connection with what I already know, so both atusb and atrf86230 drivers have been converted too. Thanks, Miquèl
Hi, On Fri, 14 Jan 2022 at 05:21, Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > Hi Alexander, > > alex.aring@gmail.com wrote on Thu, 13 Jan 2022 18:34:00 -0500: > > > Hi, > > > > On Thu, 13 Jan 2022 at 06:16, Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > > > Hi Alexander, > > > > > > alex.aring@gmail.com wrote on Wed, 12 Jan 2022 17:26:14 -0500: > > > > > > > Hi, > > > > > > > > On Wed, 12 Jan 2022 at 12:33, Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > > > > > > > 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, so > > > > > just drop the duplicated code. > > > > > > > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > > > > --- > > > > > drivers/net/ieee802154/ca8210.c | 1 - > > > > > drivers/net/ieee802154/mcr20a.c | 2 -- > > > > > 2 files changed, 3 deletions(-) > > > > > > > > > > diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c > > > > > index 82b2a173bdbd..d3a9e4fe05f4 100644 > > > > > --- a/drivers/net/ieee802154/ca8210.c > > > > > +++ b/drivers/net/ieee802154/ca8210.c > > > > > @@ -2977,7 +2977,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 * 1000; > > > > > 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 8aa87e9bf92e..da2ab19cb5ee 100644 > > > > > --- a/drivers/net/ieee802154/mcr20a.c > > > > > +++ b/drivers/net/ieee802154/mcr20a.c > > > > > @@ -975,7 +975,6 @@ static void mcr20a_hw_setup(struct mcr20a_local *lp) > > > > > > > > > > dev_dbg(printdev(lp), "%s\n", __func__); > > > > > > > > > > - phy->symbol_duration = 16 * 1000; > > > > > phy->lifs_period = 40; > > > > > phy->sifs_period = 12; > > > > > > > > > > @@ -1010,7 +1009,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 * 1000; > > > > > 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]; > > > > > > > > What's about the atrf86230 driver? > > > > > > I couldn't find reliable information about what this meant: > > > > > > /* SUB:0 and BPSK:0 -> BPSK-20 */ > > > /* SUB:1 and BPSK:0 -> BPSK-40 */ > > > /* SUB:0 and BPSK:1 -> OQPSK-100/200/400 */ > > > /* SUB:1 and BPSK:1 -> OQPSK-250/500/1000 */ > > > > > > None of these comments match the spec so I don't know what to put > > > there. If you know what these protocols are, I will immediately > > > provide this information into the driver and ensure the core handles > > > these durations properly before dropping the symbol_durations settings > > > from the code. > > > > I think those are from the transceiver datasheets (which are free to > > access). Can you not simply merge them or is there a conflict? > > Actually I misread the driver, it supports several kind of chips with > different channel settings and this disturbed me. I downloaded the > datasheet and figured that the number after the protocol is the bit > rate. This helped me to make the connection with what I already know, > so both atusb and atrf86230 drivers have been converted too. and what is about hwsim? I think the table gets too large then... that's why I was thinking of moving that somehow to the regdb, however this is another project as I said and this way is fine. Maybe we use a kind of fallback then? The hwsim phy isn't really any 802.15.4 PHY, it's just memcpy() but it would be nice to be able to test scan with it. So far I understand it is already possible to make something with hwsim here but what about the zero symbol rate and the "waiting period" is zero? btw: Also for testing with hwsim and the missing features which currently exist. Can we implement some user space test program which replies (active scan) or sends periodically something out via AF_PACKET raw and a monitor interface that should work to test if it is working? Ideally we could do that very easily with scapy (not sure about their _upstream_ 802.15.4 support). I hope I got that right that there is still something missing but we could fake it in such a way (just for hwsim testing). Side note: tx via monitor over AF_PACKET raw is without fcs currently. - Alex
Hi Alexander, alex.aring@gmail.com wrote on Sun, 16 Jan 2022 18:21:16 -0500: > Hi, > > On Fri, 14 Jan 2022 at 05:21, Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > Hi Alexander, > > > > alex.aring@gmail.com wrote on Thu, 13 Jan 2022 18:34:00 -0500: > > > > > Hi, > > > > > > On Thu, 13 Jan 2022 at 06:16, Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > > > > > Hi Alexander, > > > > > > > > alex.aring@gmail.com wrote on Wed, 12 Jan 2022 17:26:14 -0500: > > > > > > > > > Hi, > > > > > > > > > > On Wed, 12 Jan 2022 at 12:33, Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > > > > > > > > > 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, so > > > > > > just drop the duplicated code. > > > > > > > > > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > > > > > --- > > > > > > drivers/net/ieee802154/ca8210.c | 1 - > > > > > > drivers/net/ieee802154/mcr20a.c | 2 -- > > > > > > 2 files changed, 3 deletions(-) > > > > > > > > > > > > diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c > > > > > > index 82b2a173bdbd..d3a9e4fe05f4 100644 > > > > > > --- a/drivers/net/ieee802154/ca8210.c > > > > > > +++ b/drivers/net/ieee802154/ca8210.c > > > > > > @@ -2977,7 +2977,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 * 1000; > > > > > > 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 8aa87e9bf92e..da2ab19cb5ee 100644 > > > > > > --- a/drivers/net/ieee802154/mcr20a.c > > > > > > +++ b/drivers/net/ieee802154/mcr20a.c > > > > > > @@ -975,7 +975,6 @@ static void mcr20a_hw_setup(struct mcr20a_local *lp) > > > > > > > > > > > > dev_dbg(printdev(lp), "%s\n", __func__); > > > > > > > > > > > > - phy->symbol_duration = 16 * 1000; > > > > > > phy->lifs_period = 40; > > > > > > phy->sifs_period = 12; > > > > > > > > > > > > @@ -1010,7 +1009,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 * 1000; > > > > > > 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]; > > > > > > > > > > What's about the atrf86230 driver? > > > > > > > > I couldn't find reliable information about what this meant: > > > > > > > > /* SUB:0 and BPSK:0 -> BPSK-20 */ > > > > /* SUB:1 and BPSK:0 -> BPSK-40 */ > > > > /* SUB:0 and BPSK:1 -> OQPSK-100/200/400 */ > > > > /* SUB:1 and BPSK:1 -> OQPSK-250/500/1000 */ > > > > > > > > None of these comments match the spec so I don't know what to put > > > > there. If you know what these protocols are, I will immediately > > > > provide this information into the driver and ensure the core handles > > > > these durations properly before dropping the symbol_durations settings > > > > from the code. > > > > > > I think those are from the transceiver datasheets (which are free to > > > access). Can you not simply merge them or is there a conflict? > > > > Actually I misread the driver, it supports several kind of chips with > > different channel settings and this disturbed me. I downloaded the > > datasheet and figured that the number after the protocol is the bit > > rate. This helped me to make the connection with what I already know, > > so both atusb and atrf86230 drivers have been converted too. > > and what is about hwsim? I think the table gets too large then... I'm sorry but I don't follow you here, what do you mean by "the table gets too large"? > that's why I was thinking of moving that somehow to the regdb, however > this is another project as I said and this way is fine. Maybe we use a > kind of fallback then? The hwsim phy isn't really any 802.15.4 PHY, > it's just memcpy() but it would be nice to be able to test scan with > it. So far I understand it is already possible to make something with > hwsim here but what about the zero symbol rate and the "waiting > period" is zero? Before this series: many drivers would not set the symbol duration properly. In this case the scan will likely not work because wait periods will be too short. But that's how it is, we miss some information. But for hwsim, I've handled a lot of situations so yes, there are still channels that won't have a proper symbol duration because I just don't know them, but for most of them (several pages) it will work like a charm. > > btw: > Also for testing with hwsim and the missing features which currently > exist. Can we implement some user space test program which replies > (active scan) or sends periodically something out via AF_PACKET raw > and a monitor interface that should work to test if it is working? We already have all this handled, no need for extra software. You can test active and passive scans between two hwsim devices already: # iwpan dev wpan0 beacons send interval 15 # iwpan dev wpan1 scan type active duration 1 # iwpan dev wpan0 beacons stop or # iwpan dev wpan0 beacons send interval 1 # iwpan dev wpan1 scan type passive duration 2 # iwpan dev wpan0 beacons stop > Ideally we could do that very easily with scapy (not sure about their > _upstream_ 802.15.4 support). I hope I got that right that there is > still something missing but we could fake it in such a way (just for > hwsim testing). I hope the above will match your expectations. Thanks, Miquèl
Hi, On Mon, 17 Jan 2022 at 04:12, Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > Hi Alexander, > > alex.aring@gmail.com wrote on Sun, 16 Jan 2022 18:21:16 -0500: > > > Hi, > > > > On Fri, 14 Jan 2022 at 05:21, Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > > > Hi Alexander, > > > > > > alex.aring@gmail.com wrote on Thu, 13 Jan 2022 18:34:00 -0500: > > > > > > > Hi, > > > > > > > > On Thu, 13 Jan 2022 at 06:16, Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > > > > > > > Hi Alexander, > > > > > > > > > > alex.aring@gmail.com wrote on Wed, 12 Jan 2022 17:26:14 -0500: > > > > > > > > > > > Hi, > > > > > > > > > > > > On Wed, 12 Jan 2022 at 12:33, Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > > > > > > > > > > > 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, so > > > > > > > just drop the duplicated code. > > > > > > > > > > > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > > > > > > --- > > > > > > > drivers/net/ieee802154/ca8210.c | 1 - > > > > > > > drivers/net/ieee802154/mcr20a.c | 2 -- > > > > > > > 2 files changed, 3 deletions(-) > > > > > > > > > > > > > > diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c > > > > > > > index 82b2a173bdbd..d3a9e4fe05f4 100644 > > > > > > > --- a/drivers/net/ieee802154/ca8210.c > > > > > > > +++ b/drivers/net/ieee802154/ca8210.c > > > > > > > @@ -2977,7 +2977,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 * 1000; > > > > > > > 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 8aa87e9bf92e..da2ab19cb5ee 100644 > > > > > > > --- a/drivers/net/ieee802154/mcr20a.c > > > > > > > +++ b/drivers/net/ieee802154/mcr20a.c > > > > > > > @@ -975,7 +975,6 @@ static void mcr20a_hw_setup(struct mcr20a_local *lp) > > > > > > > > > > > > > > dev_dbg(printdev(lp), "%s\n", __func__); > > > > > > > > > > > > > > - phy->symbol_duration = 16 * 1000; > > > > > > > phy->lifs_period = 40; > > > > > > > phy->sifs_period = 12; > > > > > > > > > > > > > > @@ -1010,7 +1009,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 * 1000; > > > > > > > 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]; > > > > > > > > > > > > What's about the atrf86230 driver? > > > > > > > > > > I couldn't find reliable information about what this meant: > > > > > > > > > > /* SUB:0 and BPSK:0 -> BPSK-20 */ > > > > > /* SUB:1 and BPSK:0 -> BPSK-40 */ > > > > > /* SUB:0 and BPSK:1 -> OQPSK-100/200/400 */ > > > > > /* SUB:1 and BPSK:1 -> OQPSK-250/500/1000 */ > > > > > > > > > > None of these comments match the spec so I don't know what to put > > > > > there. If you know what these protocols are, I will immediately > > > > > provide this information into the driver and ensure the core handles > > > > > these durations properly before dropping the symbol_durations settings > > > > > from the code. > > > > > > > > I think those are from the transceiver datasheets (which are free to > > > > access). Can you not simply merge them or is there a conflict? > > > > > > Actually I misread the driver, it supports several kind of chips with > > > different channel settings and this disturbed me. I downloaded the > > > datasheet and figured that the number after the protocol is the bit > > > rate. This helped me to make the connection with what I already know, > > > so both atusb and atrf86230 drivers have been converted too. > > > > and what is about hwsim? I think the table gets too large then... > > I'm sorry but I don't follow you here, what do you mean by "the table > gets too large"? > The switch/case statements getting large to support the channels which hwsim supports. > > that's why I was thinking of moving that somehow to the regdb, however > > this is another project as I said and this way is fine. Maybe we use a > > kind of fallback then? The hwsim phy isn't really any 802.15.4 PHY, > > it's just memcpy() but it would be nice to be able to test scan with > > it. So far I understand it is already possible to make something with > > hwsim here but what about the zero symbol rate and the "waiting > > period" is zero? > > Before this series: many drivers would not set the symbol duration > properly. In this case the scan will likely not work because wait > periods will be too short. But that's how it is, we miss some > information. > This is the case because not every transceiver was using lifs/sifs handling. > But for hwsim, I've handled a lot of situations so yes, there are > still channels that won't have a proper symbol duration because I just > don't know them, but for most of them (several pages) it will work like > a charm. > > > > > btw: > > Also for testing with hwsim and the missing features which currently > > exist. Can we implement some user space test program which replies > > (active scan) or sends periodically something out via AF_PACKET raw > > and a monitor interface that should work to test if it is working? > > We already have all this handled, no need for extra software. You can > test active and passive scans between two hwsim devices already: > > # iwpan dev wpan0 beacons send interval 15 > # iwpan dev wpan1 scan type active duration 1 > # iwpan dev wpan0 beacons stop > > or > > # iwpan dev wpan0 beacons send interval 1 > # iwpan dev wpan1 scan type passive duration 2 > # iwpan dev wpan0 beacons stop > > > Ideally we could do that very easily with scapy (not sure about their > > _upstream_ 802.15.4 support). I hope I got that right that there is > > still something missing but we could fake it in such a way (just for > > hwsim testing). > > I hope the above will match your expectations. > I need to think and read more about... in my mind is currently the following question: are not coordinators broadcasting that information only? Means, isn't that a job for a coordinator? Thanks. - Alex
Hi Alexander, > > > btw: > > > Also for testing with hwsim and the missing features which currently > > > exist. Can we implement some user space test program which replies > > > (active scan) or sends periodically something out via AF_PACKET raw > > > and a monitor interface that should work to test if it is working? > > > > We already have all this handled, no need for extra software. You can > > test active and passive scans between two hwsim devices already: > > > > # iwpan dev wpan0 beacons send interval 15 > > # iwpan dev wpan1 scan type active duration 1 > > # iwpan dev wpan0 beacons stop > > > > or > > > > # iwpan dev wpan0 beacons send interval 1 > > # iwpan dev wpan1 scan type passive duration 2 > > # iwpan dev wpan0 beacons stop > > > > > Ideally we could do that very easily with scapy (not sure about their > > > _upstream_ 802.15.4 support). I hope I got that right that there is > > > still something missing but we could fake it in such a way (just for > > > hwsim testing). > > > > I hope the above will match your expectations. > > > > I need to think and read more about... in my mind is currently the > following question: are not coordinators broadcasting that information > only? Means, isn't that a job for a coordinator? My understanding right now: - The spec states that coordinators only can send beacons and perform scans. - I don't yet have the necessary infrastructure to give coordinators more rights than regular devices or RFDs (but 40+ patches already, don't worry this is something we have in mind) - Right now this is the user to decide whether a device might answer beacon requests or not. This will soon become more limited but it greatly simplifies the logic for now. Thanks, Miquèl
Hi, On Tue, 18 Jan 2022 at 05:38, Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > Hi Alexander, > > > > > btw: > > > > Also for testing with hwsim and the missing features which currently > > > > exist. Can we implement some user space test program which replies > > > > (active scan) or sends periodically something out via AF_PACKET raw > > > > and a monitor interface that should work to test if it is working? > > > > > > We already have all this handled, no need for extra software. You can > > > test active and passive scans between two hwsim devices already: > > > > > > # iwpan dev wpan0 beacons send interval 15 > > > # iwpan dev wpan1 scan type active duration 1 > > > # iwpan dev wpan0 beacons stop > > > > > > or > > > > > > # iwpan dev wpan0 beacons send interval 1 > > > # iwpan dev wpan1 scan type passive duration 2 > > > # iwpan dev wpan0 beacons stop > > > > > > > Ideally we could do that very easily with scapy (not sure about their > > > > _upstream_ 802.15.4 support). I hope I got that right that there is > > > > still something missing but we could fake it in such a way (just for > > > > hwsim testing). > > > > > > I hope the above will match your expectations. > > > > > > > I need to think and read more about... in my mind is currently the > > following question: are not coordinators broadcasting that information > > only? Means, isn't that a job for a coordinator? > > My understanding right now: > - The spec states that coordinators only can send beacons and perform > scans. ok. > - I don't yet have the necessary infrastructure to give coordinators > more rights than regular devices or RFDs (but 40+ patches already, > don't worry this is something we have in mind) > - Right now this is the user to decide whether a device might answer > beacon requests or not. This will soon become more limited but it > greatly simplifies the logic for now. > There was always the idea behind it to make an "coordinator" interface type and there is a reason for that because things e.g. filtering becomes different than a non-coordinator interface type (known as node interface in wpan). At the end interface types should make a big difference in how the "role" inside the network should be, which you can also see in wireless as "station"/"access point" interface devices. A non full functional device should then also not be able to act as a coordinator e.g. it cannot create coordinator types. However we can still make some -EOPNOTSUPP if something in a different way should be done. This clearly breaks userspace and I am not sure if we should worry or not worry about it in the current state of 802.15.4... - Alex
Hi Alexander, alex.aring@gmail.com wrote on Tue, 18 Jan 2022 17:43:00 -0500: > Hi, > > On Tue, 18 Jan 2022 at 05:38, Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > Hi Alexander, > > > > > > > btw: > > > > > Also for testing with hwsim and the missing features which currently > > > > > exist. Can we implement some user space test program which replies > > > > > (active scan) or sends periodically something out via AF_PACKET raw > > > > > and a monitor interface that should work to test if it is working? > > > > > > > > We already have all this handled, no need for extra software. You can > > > > test active and passive scans between two hwsim devices already: > > > > > > > > # iwpan dev wpan0 beacons send interval 15 > > > > # iwpan dev wpan1 scan type active duration 1 > > > > # iwpan dev wpan0 beacons stop > > > > > > > > or > > > > > > > > # iwpan dev wpan0 beacons send interval 1 > > > > # iwpan dev wpan1 scan type passive duration 2 > > > > # iwpan dev wpan0 beacons stop > > > > > > > > > Ideally we could do that very easily with scapy (not sure about their > > > > > _upstream_ 802.15.4 support). I hope I got that right that there is > > > > > still something missing but we could fake it in such a way (just for > > > > > hwsim testing). > > > > > > > > I hope the above will match your expectations. > > > > > > > > > > I need to think and read more about... in my mind is currently the > > > following question: are not coordinators broadcasting that information > > > only? Means, isn't that a job for a coordinator? > > > > My understanding right now: > > - The spec states that coordinators only can send beacons and perform > > scans. > > ok. > > > - I don't yet have the necessary infrastructure to give coordinators > > more rights than regular devices or RFDs (but 40+ patches already, > > don't worry this is something we have in mind) > > - Right now this is the user to decide whether a device might answer > > beacon requests or not. This will soon become more limited but it > > greatly simplifies the logic for now. > > > > There was always the idea behind it to make an "coordinator" interface > type and there is a reason for that because things e.g. filtering > becomes different than a non-coordinator interface type (known as node > interface in wpan). > At the end interface types should make a big difference in how the > "role" inside the network should be, which you can also see in > wireless as "station"/"access point" interface devices. > > A non full functional device should then also not be able to act as a > coordinator e.g. it cannot create coordinator types. I've added a few more parameters to be able to reflect the type of device (ffd, rfd, rfd_r/tx) and also eventually its coordinator state. I've hacked into nl802154 to give these information to the user and let it device wether the device (if it's an ffd) should act as a coordinator. This is only a first step before we create a real PAN creation procedure of course. I've then adapted the following patches to follow check against the device/coordinator state to decide if an operation should be aborted or not. > However we can still make some -EOPNOTSUPP if something in a different > way should be done. This clearly breaks userspace and I am not sure if > we should worry or not worry about it in the current state of > 802.15.4... > > - Alex Thanks, Miquèl
Hi, On Wed, 19 Jan 2022 at 17:26, Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > Hi Alexander, > > alex.aring@gmail.com wrote on Tue, 18 Jan 2022 17:43:00 -0500: > > > Hi, > > > > On Tue, 18 Jan 2022 at 05:38, Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > > > Hi Alexander, > > > > > > > > > btw: > > > > > > Also for testing with hwsim and the missing features which currently > > > > > > exist. Can we implement some user space test program which replies > > > > > > (active scan) or sends periodically something out via AF_PACKET raw > > > > > > and a monitor interface that should work to test if it is working? > > > > > > > > > > We already have all this handled, no need for extra software. You can > > > > > test active and passive scans between two hwsim devices already: > > > > > > > > > > # iwpan dev wpan0 beacons send interval 15 > > > > > # iwpan dev wpan1 scan type active duration 1 > > > > > # iwpan dev wpan0 beacons stop > > > > > > > > > > or > > > > > > > > > > # iwpan dev wpan0 beacons send interval 1 > > > > > # iwpan dev wpan1 scan type passive duration 2 > > > > > # iwpan dev wpan0 beacons stop > > > > > > > > > > > Ideally we could do that very easily with scapy (not sure about their > > > > > > _upstream_ 802.15.4 support). I hope I got that right that there is > > > > > > still something missing but we could fake it in such a way (just for > > > > > > hwsim testing). > > > > > > > > > > I hope the above will match your expectations. > > > > > > > > > > > > > I need to think and read more about... in my mind is currently the > > > > following question: are not coordinators broadcasting that information > > > > only? Means, isn't that a job for a coordinator? > > > > > > My understanding right now: > > > - The spec states that coordinators only can send beacons and perform > > > scans. > > > > ok. > > > > > - I don't yet have the necessary infrastructure to give coordinators > > > more rights than regular devices or RFDs (but 40+ patches already, > > > don't worry this is something we have in mind) > > > - Right now this is the user to decide whether a device might answer > > > beacon requests or not. This will soon become more limited but it > > > greatly simplifies the logic for now. > > > > > > > There was always the idea behind it to make an "coordinator" interface > > type and there is a reason for that because things e.g. filtering > > becomes different than a non-coordinator interface type (known as node > > interface in wpan). > > At the end interface types should make a big difference in how the > > "role" inside the network should be, which you can also see in > > wireless as "station"/"access point" interface devices. > > > > A non full functional device should then also not be able to act as a > > coordinator e.g. it cannot create coordinator types. > > I've added a few more parameters to be able to reflect the type of > device (ffd, rfd, rfd_r/tx) and also eventually its coordinator state. > I've hacked into nl802154 to give these information to the user and let > it device wether the device (if it's an ffd) should act as a > coordinator. This is only a first step before we create a real PAN > creation procedure of course. > > I've then adapted the following patches to follow check against the > device/coordinator state to decide if an operation should be aborted or > not. Okay, I need to see the patches to say anything about that and how it fits. The problem still exists that we don't currently have any distinction to know if a device can do it or not. After this patch series every "node interface" is allowed to scan/send beacons, where a node interface should not be allowed to do it. If we change it later we break things. However there is still the question if we care about it because 802.15.4 is in such an early/experimental state. It was "experimental" as the Kconfig EXPERIMENTAL entry still existed. It was dropped because most people ignored this setting, that doesn't mean that 802.15.4 ever left the experimental state. - Alex
diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c index 82b2a173bdbd..d3a9e4fe05f4 100644 --- a/drivers/net/ieee802154/ca8210.c +++ b/drivers/net/ieee802154/ca8210.c @@ -2977,7 +2977,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 * 1000; 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 8aa87e9bf92e..da2ab19cb5ee 100644 --- a/drivers/net/ieee802154/mcr20a.c +++ b/drivers/net/ieee802154/mcr20a.c @@ -975,7 +975,6 @@ static void mcr20a_hw_setup(struct mcr20a_local *lp) dev_dbg(printdev(lp), "%s\n", __func__); - phy->symbol_duration = 16 * 1000; phy->lifs_period = 40; phy->sifs_period = 12; @@ -1010,7 +1009,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 * 1000; 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];
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, so just drop the duplicated code. Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> --- drivers/net/ieee802154/ca8210.c | 1 - drivers/net/ieee802154/mcr20a.c | 2 -- 2 files changed, 3 deletions(-)