| Message ID | 20220128110825.1120678-2-miquel.raynal@bootlin.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | ieee802154: Improve durations handling | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Guessing tree name failed - patch did not apply |
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
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
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
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
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
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
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
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
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
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
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);
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(-)