Subject: Re: [PATCH wpan-next v2 2/2] net: ieee802154: Move the address structure earlier and provide a kdoc

From: Miquel Raynal <[email-masked]>
Date: Mon, 31 Jan 2022 15:09:27 +0100

Hi Miquel,

[email-masked] wrote on Mon, 31 Jan 2022 11:46:45 +0100:

> Hi Alexander,
> 
> [email-masked] wrote on Sun, 30 Jan 2022 16:09:00 -0500:
> 
> > Hi,
> > 
> > On Fri, Jan 28, 2022 at 6:20 AM Miquel Raynal <[email-masked]> wrote:  
> > >
> > > From: David Girault <[email-masked]>
> > >
> > > 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 <[email-masked]>
> > > [[email-masked]: Isolate this change from a bigger commit and
> > >                             reword the comment]
> > > Signed-off-by: Miquel Raynal <[email-masked]>
> > > ---
> > >  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