On Wed, Mar 26, 2025 at 7:49 PM, Alexander Aring <aahringo@xxxxxxxxxx> wrote:
>
> Hi,
>
> On Wed, Mar 26, 2025 at 2:09 PM Ramon Fontes <ramonreisfontes@xxxxxxxxx> wrote:
> >
> > This establishes an initialization method for perm_extended_addr, aligning it with the approach used in mac80211_hwsim.
> >
>
> that is based on the phy index value instead of a random generated one?
Yes, that's based on the phy index value.
>
> > Signed-off-by: Ramon Fontes <ramonreisfontes@xxxxxxxxx>
> > ---
> > drivers/net/ieee802154/mac802154_hwsim.c | 18 +++++++++++++++++-
> > drivers/net/ieee802154/mac802154_hwsim.h | 2 ++
> > 2 files changed, 19 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ieee802154/mac802154_hwsim.c b/drivers/net/ieee802154/mac802154_hwsim.c
> > index 1cab20b5a..400cdac1f 100644
> > --- a/drivers/net/ieee802154/mac802154_hwsim.c
> > +++ b/drivers/net/ieee802154/mac802154_hwsim.c
> > @@ -41,6 +41,17 @@ enum hwsim_multicast_groups {
> > HWSIM_MCGRP_CONFIG,
> > };
> >
> > +__le64 addr_to_le64(u8 *addr) {
> > + return cpu_to_le64(((u64)addr[0] << 56) |
> > + ((u64)addr[1] << 48) |
> > + ((u64)addr[2] << 40) |
> > + ((u64)addr[3] << 32) |
> > + ((u64)addr[4] << 24) |
> > + ((u64)addr[5] << 16) |
> > + ((u64)addr[6] << 8) |
> > + ((u64)addr[7]));
> > +}
> > +
> > static const struct genl_multicast_group hwsim_mcgrps[] = {
> > [HWSIM_MCGRP_CONFIG] = { .name = "config", },
> > };
> > @@ -896,6 +907,7 @@ static int hwsim_subscribe_all_others(struct hwsim_phy *phy)
> > static int hwsim_add_one(struct genl_info *info, struct device *dev,
> > bool init)
> > {
> > + u8 addr[8];
>
> why not using directly
Yes, we don't need it.
>
> > struct ieee802154_hw *hw;
> > struct hwsim_phy *phy;
> > struct hwsim_pib *pib;
> > @@ -942,7 +954,11 @@ static int hwsim_add_one(struct genl_info *info, struct device *dev,
> > /* 950 MHz GFSK 802.15.4d-2009 */
> > hw->phy->supported.channels[6] |= 0x3ffc00;
> >
> > - ieee802154_random_extended_addr(&hw->phy->perm_extended_addr);
> > + memset(addr, 0, sizeof(addr));
> > + /* give a specific prefix to the address */
> > + addr[0] = 0x02;
> > + addr[7] = idx;
> > + hw->phy->perm_extended_addr = addr_to_le64(addr);
I think we can replace everything with only one line of code:
hw->phy->perm_extended_addr = cpu_to_le64(((u64)0x02 << 56) | ((u64)idx));
This does the trick! What do you think?
> >
> > /* hwsim phy channel 13 as default */
> > hw->phy->current_channel = 13;
> > diff --git a/drivers/net/ieee802154/mac802154_hwsim.h b/drivers/net/ieee802154/mac802154_hwsim.h
> > index 6c6e30e38..536d95eb1 100644
> > --- a/drivers/net/ieee802154/mac802154_hwsim.h
> > +++ b/drivers/net/ieee802154/mac802154_hwsim.h
> > @@ -1,6 +1,8 @@
> > #ifndef __MAC802154_HWSIM_H
> > #define __MAC802154_HWSIM_H
> >
> > +__le64 addr_to_le64(u8 *addr);
> > +
>
> This is a uapi header for netlink which is not yet delivered through
> kernel-headers installation.
>
> Why do we need this prototype declaration here?
We don't need it.
>
> Thanks.
>
> - Alex
>
