| Message ID | 20240417-mac_addr_at_probe-v1-1-67d6c9b3bc2b@bootlin.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | wifi: wilc1000: fix default mac address | expand |
Alexis Lothoré <alexis.lothore@bootlin.com> wrote: > net device registration is currently done in wilc_netdev_ifc_init but > other initialization operations are still done after this registration. > Since net device is assumed to be usable right after registration, it > should be the very last step of initialization. > > Move netdev registration at the very end of wilc_netdev_ifc_init to let > this function completely initialize netdevice before registering it. > > Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com> I see errors: ERROR: modpost: "wilc_load_mac_from_nv" [drivers/net/wireless/microchip/wilc1000/wilc1000-sdio.ko] undefined! ERROR: modpost: "wilc_netdev_ifc_init" [drivers/net/wireless/microchip/wilc1000/wilc1000-sdio.ko] undefined! ERROR: modpost: "wilc_load_mac_from_nv" [drivers/net/wireless/microchip/wilc1000/wilc1000-spi.ko] undefined! ERROR: modpost: "wilc_netdev_ifc_init" [drivers/net/wireless/microchip/wilc1000/wilc1000-spi.ko] undefined! make[2]: *** [scripts/Makefile.modpost:145: Module.symvers] Error 1 make[1]: *** [/home/kvalo/projects/personal/wireless-drivers/src/wireless-next/Makefile:1871: modpost] Error 2 make: *** [Makefile:240: __sub-make] Error 2 6 patches set to Changes Requested. 13633102 [1/6] wifi: wilc1000: set net device registration as last step during interface creation 13633103 [2/6] wifi: wilc1000: register net device only after bus being fully initialized 13633104 [3/6] wifi: wilc1000: set wilc_set_mac_address parameter as const 13633105 [4/6] wifi: wilc1000: add function to read mac address from eFuse 13633106 [5/6] wifi: wilc1000: make sdio deinit function really deinit the sdio card 13633107 [6/6] wifi: wilc1000: read MAC address from fuse at probe
Hello Kalle, On 5/14/24 14:45, Kalle Valo wrote: > Alexis Lothoré <alexis.lothore@bootlin.com> wrote: > >> net device registration is currently done in wilc_netdev_ifc_init but >> other initialization operations are still done after this registration. >> Since net device is assumed to be usable right after registration, it >> should be the very last step of initialization. >> >> Move netdev registration at the very end of wilc_netdev_ifc_init to let >> this function completely initialize netdevice before registering it. >> >> Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com> > > I see errors: > > ERROR: modpost: "wilc_load_mac_from_nv" [drivers/net/wireless/microchip/wilc1000/wilc1000-sdio.ko] undefined! > ERROR: modpost: "wilc_netdev_ifc_init" [drivers/net/wireless/microchip/wilc1000/wilc1000-sdio.ko] undefined! > ERROR: modpost: "wilc_load_mac_from_nv" [drivers/net/wireless/microchip/wilc1000/wilc1000-spi.ko] undefined! > ERROR: modpost: "wilc_netdev_ifc_init" [drivers/net/wireless/microchip/wilc1000/wilc1000-spi.ko] undefined! > make[2]: *** [scripts/Makefile.modpost:145: Module.symvers] Error 1 > make[1]: *** [/home/kvalo/projects/personal/wireless-drivers/src/wireless-next/Makefile:1871: modpost] Error 2 > make: *** [Makefile:240: __sub-make] Error 2 > > 6 patches set to Changes Requested. > > 13633102 [1/6] wifi: wilc1000: set net device registration as last step during interface creation > 13633103 [2/6] wifi: wilc1000: register net device only after bus being fully initialized > 13633104 [3/6] wifi: wilc1000: set wilc_set_mac_address parameter as const > 13633105 [4/6] wifi: wilc1000: add function to read mac address from eFuse > 13633106 [5/6] wifi: wilc1000: make sdio deinit function really deinit the sdio card > 13633107 [6/6] wifi: wilc1000: read MAC address from fuse at probe Shame on me, I missed those basic errors since I worked with drivers as built-in instead of modules. I'll update my workflow and send a v2. Thanks, Alexis
Alexis Lothoré <alexis.lothore@bootlin.com> writes: > Hello Kalle, > > On 5/14/24 14:45, Kalle Valo wrote: >> Alexis Lothoré <alexis.lothore@bootlin.com> wrote: >> >>> net device registration is currently done in wilc_netdev_ifc_init but >>> other initialization operations are still done after this registration. >>> Since net device is assumed to be usable right after registration, it >>> should be the very last step of initialization. >>> >>> Move netdev registration at the very end of wilc_netdev_ifc_init to let >>> this function completely initialize netdevice before registering it. >>> >>> Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com> >> >> I see errors: >> >> ERROR: modpost: "wilc_load_mac_from_nv" >> [drivers/net/wireless/microchip/wilc1000/wilc1000-sdio.ko] >> undefined! >> ERROR: modpost: "wilc_netdev_ifc_init" >> [drivers/net/wireless/microchip/wilc1000/wilc1000-sdio.ko] >> undefined! >> ERROR: modpost: "wilc_load_mac_from_nv" >> [drivers/net/wireless/microchip/wilc1000/wilc1000-spi.ko] undefined! >> ERROR: modpost: "wilc_netdev_ifc_init" >> [drivers/net/wireless/microchip/wilc1000/wilc1000-spi.ko] undefined! >> make[2]: *** [scripts/Makefile.modpost:145: Module.symvers] Error 1 >> make[1]: *** >> [/home/kvalo/projects/personal/wireless-drivers/src/wireless-next/Makefile:1871: >> modpost] Error 2 >> make: *** [Makefile:240: __sub-make] Error 2 >> >> 6 patches set to Changes Requested. >> >> 13633102 [1/6] wifi: wilc1000: set net device registration as last >> step during interface creation >> 13633103 [2/6] wifi: wilc1000: register net device only after bus >> being fully initialized >> 13633104 [3/6] wifi: wilc1000: set wilc_set_mac_address parameter as const >> 13633105 [4/6] wifi: wilc1000: add function to read mac address from eFuse >> 13633106 [5/6] wifi: wilc1000: make sdio deinit function really deinit the sdio card >> 13633107 [6/6] wifi: wilc1000: read MAC address from fuse at probe > > Shame on me, I missed those basic errors since I worked with drivers as built-in > instead of modules. I'll update my workflow and send a v2. No worries, but I'm surprised that Intel's kernel test robot didn't report anything.
diff --git a/drivers/net/wireless/microchip/wilc1000/netdev.c b/drivers/net/wireless/microchip/wilc1000/netdev.c index 73f56f7b002b..acc9b9a64552 100644 --- a/drivers/net/wireless/microchip/wilc1000/netdev.c +++ b/drivers/net/wireless/microchip/wilc1000/netdev.c @@ -965,16 +965,6 @@ struct wilc_vif *wilc_netdev_ifc_init(struct wilc *wl, const char *name, vif->priv.wdev.iftype = type; vif->priv.dev = ndev; - if (rtnl_locked) - ret = cfg80211_register_netdevice(ndev); - else - ret = register_netdev(ndev); - - if (ret) { - ret = -EFAULT; - goto error; - } - ndev->needs_free_netdev = true; vif->iftype = vif_type; vif->idx = wilc_get_available_idx(wl); @@ -985,13 +975,24 @@ struct wilc_vif *wilc_netdev_ifc_init(struct wilc *wl, const char *name, mutex_unlock(&wl->vif_mutex); synchronize_rcu(); - return vif; - -error: if (rtnl_locked) - cfg80211_unregister_netdevice(ndev); + ret = cfg80211_register_netdevice(ndev); else - unregister_netdev(ndev); + ret = register_netdev(ndev); + + if (ret) { + ret = -EFAULT; + goto error_remove_vif; + } + + return vif; + +error_remove_vif: + mutex_lock(&wl->vif_mutex); + list_del_rcu(&vif->list); + wl->vif_num -= 1; + mutex_unlock(&wl->vif_mutex); + synchronize_rcu(); free_netdev(ndev); return ERR_PTR(ret); }
net device registration is currently done in wilc_netdev_ifc_init but other initialization operations are still done after this registration. Since net device is assumed to be usable right after registration, it should be the very last step of initialization. Move netdev registration at the very end of wilc_netdev_ifc_init to let this function completely initialize netdevice before registering it. Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com> --- drivers/net/wireless/microchip/wilc1000/netdev.c | 31 ++++++++++++------------ 1 file changed, 16 insertions(+), 15 deletions(-)