| Message ID | 20220128112002.1121320-3-miquel.raynal@bootlin.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | ieee802154: Internal moves | expand |
Hi, On Fri, Jan 28, 2022 at 6:20 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > From: David Girault <david.girault@qorvo.com> > > Move the address structure earlier in the cfg802154.h header in order to > use it in subsequent additions. Give this structure a header to better > explain its content. > > Signed-off-by: David Girault <david.girault@qorvo.com> > [miquel.raynal@bootlin.com: Isolate this change from a bigger commit and > reword the comment] > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > --- > include/net/cfg802154.h | 28 +++++++++++++++++++--------- > 1 file changed, 19 insertions(+), 9 deletions(-) > > diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h > index 4491e2724ff2..0b8b1812cea1 100644 > --- a/include/net/cfg802154.h > +++ b/include/net/cfg802154.h > @@ -29,6 +29,25 @@ struct ieee802154_llsec_key_id; > struct ieee802154_llsec_key; > #endif /* CONFIG_IEEE802154_NL802154_EXPERIMENTAL */ > > +/** > + * struct ieee802154_addr - IEEE802.15.4 device address > + * @mode: Address mode from frame header. Can be one of: > + * - @IEEE802154_ADDR_NONE > + * - @IEEE802154_ADDR_SHORT > + * - @IEEE802154_ADDR_LONG > + * @pan_id: The PAN ID this address belongs to > + * @short_addr: address if @mode is @IEEE802154_ADDR_SHORT > + * @extended_addr: address if @mode is @IEEE802154_ADDR_LONG > + */ > +struct ieee802154_addr { > + u8 mode; > + __le16 pan_id; > + union { > + __le16 short_addr; > + __le64 extended_addr; > + }; > +}; > + > struct cfg802154_ops { > struct net_device * (*add_virtual_intf_deprecated)(struct wpan_phy *wpan_phy, > const char *name, > @@ -277,15 +296,6 @@ static inline void wpan_phy_net_set(struct wpan_phy *wpan_phy, struct net *net) > write_pnet(&wpan_phy->_net, net); > } > > -struct ieee802154_addr { > - u8 mode; > - __le16 pan_id; > - union { > - __le16 short_addr; > - __le64 extended_addr; > - }; > -}; > - I don't see the sense of moving this around? Is there a compilation warning/error? - Alex
Hi Alexander, alex.aring@gmail.com wrote on Sun, 30 Jan 2022 16:09:00 -0500: > Hi, > > On Fri, Jan 28, 2022 at 6:20 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > From: David Girault <david.girault@qorvo.com> > > > > Move the address structure earlier in the cfg802154.h header in order to > > use it in subsequent additions. Give this structure a header to better > > explain its content. > > > > Signed-off-by: David Girault <david.girault@qorvo.com> > > [miquel.raynal@bootlin.com: Isolate this change from a bigger commit and > > reword the comment] > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > --- > > include/net/cfg802154.h | 28 +++++++++++++++++++--------- > > 1 file changed, 19 insertions(+), 9 deletions(-) > > > > diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h > > index 4491e2724ff2..0b8b1812cea1 100644 > > --- a/include/net/cfg802154.h > > +++ b/include/net/cfg802154.h > > @@ -29,6 +29,25 @@ struct ieee802154_llsec_key_id; > > struct ieee802154_llsec_key; > > #endif /* CONFIG_IEEE802154_NL802154_EXPERIMENTAL */ > > > > +/** > > + * struct ieee802154_addr - IEEE802.15.4 device address > > + * @mode: Address mode from frame header. Can be one of: > > + * - @IEEE802154_ADDR_NONE > > + * - @IEEE802154_ADDR_SHORT > > + * - @IEEE802154_ADDR_LONG > > + * @pan_id: The PAN ID this address belongs to > > + * @short_addr: address if @mode is @IEEE802154_ADDR_SHORT > > + * @extended_addr: address if @mode is @IEEE802154_ADDR_LONG > > + */ > > +struct ieee802154_addr { > > + u8 mode; > > + __le16 pan_id; > > + union { > > + __le16 short_addr; > > + __le64 extended_addr; > > + }; > > +}; > > + > > struct cfg802154_ops { > > struct net_device * (*add_virtual_intf_deprecated)(struct wpan_phy *wpan_phy, > > const char *name, > > @@ -277,15 +296,6 @@ static inline void wpan_phy_net_set(struct wpan_phy *wpan_phy, struct net *net) > > write_pnet(&wpan_phy->_net, net); > > } > > > > -struct ieee802154_addr { > > - u8 mode; > > - __le16 pan_id; > > - union { > > - __le16 short_addr; > > - __le64 extended_addr; > > - }; > > -}; > > - > > I don't see the sense of moving this around? Is there a compilation > warning/error? Not yet but we will need to move this structure around soon. This commit is like a 'preparation' step for the changes coming later. I can move this later if you prefer. Thanks, Miquèl
Hi Miquel, miquel.raynal@bootlin.com wrote on Mon, 31 Jan 2022 11:46:45 +0100: > Hi Alexander, > > alex.aring@gmail.com wrote on Sun, 30 Jan 2022 16:09:00 -0500: > > > Hi, > > > > On Fri, Jan 28, 2022 at 6:20 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > > > From: David Girault <david.girault@qorvo.com> > > > > > > Move the address structure earlier in the cfg802154.h header in order to > > > use it in subsequent additions. Give this structure a header to better > > > explain its content. > > > > > > Signed-off-by: David Girault <david.girault@qorvo.com> > > > [miquel.raynal@bootlin.com: Isolate this change from a bigger commit and > > > reword the comment] > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > > --- > > > include/net/cfg802154.h | 28 +++++++++++++++++++--------- > > > 1 file changed, 19 insertions(+), 9 deletions(-) > > > > > > diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h > > > index 4491e2724ff2..0b8b1812cea1 100644 > > > --- a/include/net/cfg802154.h > > > +++ b/include/net/cfg802154.h > > > @@ -29,6 +29,25 @@ struct ieee802154_llsec_key_id; > > > struct ieee802154_llsec_key; > > > #endif /* CONFIG_IEEE802154_NL802154_EXPERIMENTAL */ > > > > > > +/** > > > + * struct ieee802154_addr - IEEE802.15.4 device address > > > + * @mode: Address mode from frame header. Can be one of: > > > + * - @IEEE802154_ADDR_NONE > > > + * - @IEEE802154_ADDR_SHORT > > > + * - @IEEE802154_ADDR_LONG > > > + * @pan_id: The PAN ID this address belongs to > > > + * @short_addr: address if @mode is @IEEE802154_ADDR_SHORT > > > + * @extended_addr: address if @mode is @IEEE802154_ADDR_LONG > > > + */ > > > +struct ieee802154_addr { > > > + u8 mode; > > > + __le16 pan_id; > > > + union { > > > + __le16 short_addr; > > > + __le64 extended_addr; > > > + }; > > > +}; > > > + > > > struct cfg802154_ops { > > > struct net_device * (*add_virtual_intf_deprecated)(struct wpan_phy *wpan_phy, > > > const char *name, > > > @@ -277,15 +296,6 @@ static inline void wpan_phy_net_set(struct wpan_phy *wpan_phy, struct net *net) > > > write_pnet(&wpan_phy->_net, net); > > > } > > > > > > -struct ieee802154_addr { > > > - u8 mode; > > > - __le16 pan_id; > > > - union { > > > - __le16 short_addr; > > > - __le64 extended_addr; > > > - }; > > > -}; > > > - > > > > I don't see the sense of moving this around? Is there a compilation > > warning/error? > > Not yet but we will need to move this structure around soon. This > commit is like a 'preparation' step for the changes coming later. > > I can move this later if you prefer. Actually there is not actual need for moving this structure anymore. The number of changes applied on top of the original series have turned that move unnecessary. I still believe however that structures should, as far as possible, be defined at the top of headers files, instead of be defined right before where they will be immediately used when introduced. I'll cancel the move but I'll keep the addition of the kdoc which I think is useful. Thanks, Miquèl
Hi, On Mon, Jan 31, 2022 at 9:09 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > Hi Miquel, > > miquel.raynal@bootlin.com wrote on Mon, 31 Jan 2022 11:46:45 +0100: > > > Hi Alexander, > > > > alex.aring@gmail.com wrote on Sun, 30 Jan 2022 16:09:00 -0500: > > > > > Hi, > > > > > > On Fri, Jan 28, 2022 at 6:20 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > > > > > From: David Girault <david.girault@qorvo.com> > > > > > > > > Move the address structure earlier in the cfg802154.h header in order to > > > > use it in subsequent additions. Give this structure a header to better > > > > explain its content. > > > > > > > > Signed-off-by: David Girault <david.girault@qorvo.com> > > > > [miquel.raynal@bootlin.com: Isolate this change from a bigger commit and > > > > reword the comment] > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > > > --- > > > > include/net/cfg802154.h | 28 +++++++++++++++++++--------- > > > > 1 file changed, 19 insertions(+), 9 deletions(-) > > > > > > > > diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h > > > > index 4491e2724ff2..0b8b1812cea1 100644 > > > > --- a/include/net/cfg802154.h > > > > +++ b/include/net/cfg802154.h > > > > @@ -29,6 +29,25 @@ struct ieee802154_llsec_key_id; > > > > struct ieee802154_llsec_key; > > > > #endif /* CONFIG_IEEE802154_NL802154_EXPERIMENTAL */ > > > > > > > > +/** > > > > + * struct ieee802154_addr - IEEE802.15.4 device address > > > > + * @mode: Address mode from frame header. Can be one of: > > > > + * - @IEEE802154_ADDR_NONE > > > > + * - @IEEE802154_ADDR_SHORT > > > > + * - @IEEE802154_ADDR_LONG > > > > + * @pan_id: The PAN ID this address belongs to > > > > + * @short_addr: address if @mode is @IEEE802154_ADDR_SHORT > > > > + * @extended_addr: address if @mode is @IEEE802154_ADDR_LONG > > > > + */ > > > > +struct ieee802154_addr { > > > > + u8 mode; > > > > + __le16 pan_id; > > > > + union { > > > > + __le16 short_addr; > > > > + __le64 extended_addr; > > > > + }; > > > > +}; > > > > + > > > > struct cfg802154_ops { > > > > struct net_device * (*add_virtual_intf_deprecated)(struct wpan_phy *wpan_phy, > > > > const char *name, > > > > @@ -277,15 +296,6 @@ static inline void wpan_phy_net_set(struct wpan_phy *wpan_phy, struct net *net) > > > > write_pnet(&wpan_phy->_net, net); > > > > } > > > > > > > > -struct ieee802154_addr { > > > > - u8 mode; > > > > - __le16 pan_id; > > > > - union { > > > > - __le16 short_addr; > > > > - __le64 extended_addr; > > > > - }; > > > > -}; > > > > - > > > > > > I don't see the sense of moving this around? Is there a compilation > > > warning/error? > > > > Not yet but we will need to move this structure around soon. This > > commit is like a 'preparation' step for the changes coming later. > > > > I can move this later if you prefer. > > Actually there is not actual need for moving this structure anymore. > The number of changes applied on top of the original series have turned > that move unnecessary. I still believe however that structures should, > as far as possible, be defined at the top of headers files, instead of > be defined right before where they will be immediately used when > introduced. I'll cancel the move but I'll keep the addition of the kdoc > which I think is useful. > ok. - Alex
diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h index 4491e2724ff2..0b8b1812cea1 100644 --- a/include/net/cfg802154.h +++ b/include/net/cfg802154.h @@ -29,6 +29,25 @@ struct ieee802154_llsec_key_id; struct ieee802154_llsec_key; #endif /* CONFIG_IEEE802154_NL802154_EXPERIMENTAL */ +/** + * struct ieee802154_addr - IEEE802.15.4 device address + * @mode: Address mode from frame header. Can be one of: + * - @IEEE802154_ADDR_NONE + * - @IEEE802154_ADDR_SHORT + * - @IEEE802154_ADDR_LONG + * @pan_id: The PAN ID this address belongs to + * @short_addr: address if @mode is @IEEE802154_ADDR_SHORT + * @extended_addr: address if @mode is @IEEE802154_ADDR_LONG + */ +struct ieee802154_addr { + u8 mode; + __le16 pan_id; + union { + __le16 short_addr; + __le64 extended_addr; + }; +}; + struct cfg802154_ops { struct net_device * (*add_virtual_intf_deprecated)(struct wpan_phy *wpan_phy, const char *name, @@ -277,15 +296,6 @@ static inline void wpan_phy_net_set(struct wpan_phy *wpan_phy, struct net *net) write_pnet(&wpan_phy->_net, net); } -struct ieee802154_addr { - u8 mode; - __le16 pan_id; - union { - __le16 short_addr; - __le64 extended_addr; - }; -}; - struct ieee802154_llsec_key_id { u8 mode; u8 id;