On 9/10/24 12:53, Marek Vasut wrote: > On 9/10/24 12:08 PM, Alexis Lothoré wrote: >> On 9/9/24 21:29, Marek Vasut wrote: [...] >>> EXPORT_SYMBOL_GPL(wilc_cfg80211_init); >>> +int wilc_cfg80211_register(struct wilc *wilc) >>> +{ >>> + wilc->wiphy->features |= NL80211_FEATURE_SAE; >> >> Even if I get the general need, it feels weird to have parts of the wphy init >> performed in wilc_create_wiphy, and some parts (the features field) here. >> Wouldn't it work to just move wilc_create_wiphy content here, since wphy will >> not be usable anyway before eventually registering it ? > That's what I thought initially too, but look closely at wilc_create_wiphy(): > > struct wilc *wilc_create_wiphy(struct device *dev) > { > ... > struct wiphy *wiphy; > struct wilc *wl; > ... > wiphy = wiphy_new(&wilc_cfg80211_ops, sizeof(*wl)); > ... > wl = wiphy_priv(wiphy); // <----------- HERE , *wl is struct wilc > ... > return wl; > } > > That 'struct wilc' is allocated as part of wiphy_new() and used all around the > place before we reach wiphy_register() much later on. Meh, true. We could still let any part affecting the struct wilc in wilc_create_wiphy, and move any wphy configuration in wilc_cfg80211_register, but then I am not sure anymore if it makes things better. -- Alexis Lothoré, Bootlin Embedded Linux and Kernel engineering https://bootlin.com