| Message ID | 20210615003016.477-7-ryazanov.s.a@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | net: WWAN link creation improvements | expand |
Hello Chetan, On Tue, Jun 15, 2021 at 3:30 AM Sergey Ryazanov <ryazanov.s.a@gmail.com> wrote: > Since the last commit, the WWAN core will remove all our network > interfaces for us at the time of the WWAN netdev ops unregistering. > Therefore, we can safely drop the custom code that cleaning the list of > created netdevs. Anyway it no longer removes any netdev, since all > netdevs were removed earlier in the wwan_unregister_ops() call. Are you Ok with this change? I plan to submit a next version of the series. If you have any objections, I can address them in V2. BTW, if IOSM modems have a default data channel, I can add a separate patch to the series to create a default network interface for IOSM if you tell me which link id is used for the default data channel. > Signed-off-by: Sergey Ryazanov <ryazanov.s.a@gmail.com> > CC: M Chetan Kumar <m.chetan.kumar@intel.com> > CC: Intel Corporation <linuxwwan@intel.com> > --- > drivers/net/wwan/iosm/iosm_ipc_wwan.c | 15 +-------------- > 1 file changed, 1 insertion(+), 14 deletions(-) > > diff --git a/drivers/net/wwan/iosm/iosm_ipc_wwan.c b/drivers/net/wwan/iosm/iosm_ipc_wwan.c > index 1711b79fc616..bee9b278223d 100644 > --- a/drivers/net/wwan/iosm/iosm_ipc_wwan.c > +++ b/drivers/net/wwan/iosm/iosm_ipc_wwan.c > @@ -329,22 +329,9 @@ struct iosm_wwan *ipc_wwan_init(struct iosm_imem *ipc_imem, struct device *dev) > > void ipc_wwan_deinit(struct iosm_wwan *ipc_wwan) > { > - int if_id; > - > + /* This call will remove all child netdev(s) */ > wwan_unregister_ops(ipc_wwan->dev); > > - for (if_id = 0; if_id < ARRAY_SIZE(ipc_wwan->sub_netlist); if_id++) { > - struct iosm_netdev_priv *priv; > - > - priv = rcu_access_pointer(ipc_wwan->sub_netlist[if_id]); > - if (!priv) > - continue; > - > - rtnl_lock(); > - ipc_wwan_dellink(ipc_wwan, priv->netdev, NULL); > - rtnl_unlock(); > - } > - > mutex_destroy(&ipc_wwan->if_mutex); > > kfree(ipc_wwan); -- Sergey
Hi Sergey, > On Tue, Jun 15, 2021 at 3:30 AM Sergey Ryazanov > <ryazanov.s.a@gmail.com> wrote: > > Since the last commit, the WWAN core will remove all our network > > interfaces for us at the time of the WWAN netdev ops unregistering. > > Therefore, we can safely drop the custom code that cleaning the list > > of created netdevs. Anyway it no longer removes any netdev, since all > > netdevs were removed earlier in the wwan_unregister_ops() call. > > Are you Ok with this change? I plan to submit a next version of the series. If > you have any objections, I can address them in V2. Changes looks fine. > BTW, if IOSM modems have a default data channel, I can add a separate > patch to the series to create a default network interface for IOSM if you tell > me which link id is used for the default data channel. Link id 1 is always associated as default data channel. Thanks, Reviewed-by: M Chetan Kumar <m.chetan.kumar@intel.com>
On Sun, Jun 20, 2021 at 6:42 PM Kumar, M Chetan <m.chetan.kumar@intel.com> wrote: >> On Tue, Jun 15, 2021 at 3:30 AM Sergey Ryazanov >> <ryazanov.s.a@gmail.com> wrote: >>> Since the last commit, the WWAN core will remove all our network >>> interfaces for us at the time of the WWAN netdev ops unregistering. >>> Therefore, we can safely drop the custom code that cleaning the list >>> of created netdevs. Anyway it no longer removes any netdev, since all >>> netdevs were removed earlier in the wwan_unregister_ops() call. >> >> Are you Ok with this change? I plan to submit a next version of the series. If >> you have any objections, I can address them in V2. > > Changes looks fine. > >> BTW, if IOSM modems have a default data channel, I can add a separate >> patch to the series to create a default network interface for IOSM if you tell >> me which link id is used for the default data channel. > > Link id 1 is always associated as default data channel. Thank you, will add default interface creation with this Id in the V2 series. > Thanks, > Reviewed-by: M Chetan Kumar <m.chetan.kumar@intel.com> -- Sergey
Hi Chetan, On Sun, 20 Jun 2021 at 17:42, Kumar, M Chetan <m.chetan.kumar@intel.com> wrote: > > Hi Sergey, > > > On Tue, Jun 15, 2021 at 3:30 AM Sergey Ryazanov > > <ryazanov.s.a@gmail.com> wrote: > > > Since the last commit, the WWAN core will remove all our network > > > interfaces for us at the time of the WWAN netdev ops unregistering. > > > Therefore, we can safely drop the custom code that cleaning the list > > > of created netdevs. Anyway it no longer removes any netdev, since all > > > netdevs were removed earlier in the wwan_unregister_ops() call. > > > > Are you Ok with this change? I plan to submit a next version of the series. If > > you have any objections, I can address them in V2. > > Changes looks fine. > > > BTW, if IOSM modems have a default data channel, I can add a separate > > patch to the series to create a default network interface for IOSM if you tell > > me which link id is used for the default data channel. > > Link id 1 is always associated as default data channel. Quick question, Isn't your driver use MBIM session IDs? with session-ID 0 as the default one? Regards, Loic
Hi Loic, On 6/29/2021 7:44 PM, Loic Poulain wrote: >>> BTW, if IOSM modems have a default data channel, I can add a separate >>> patch to the series to create a default network interface for IOSM if you tell >>> me which link id is used for the default data channel. >> >> Link id 1 is always associated as default data channel. > > Quick question, Isn't your driver use MBIM session IDs? with > session-ID 0 as the default one? Link Id from 1 to 8 are treated as valid link ids. These ids are decremented by 1 to match session id. In this case link id 1 would be mapped to session id 0. So have requested link id 1 to be set as default data channel. Regards, Chetan
Hi Chetan, On Tue, 29 Jun 2021 at 16:56, Kumar, M Chetan <m.chetan.kumar@intel.com> wrote: > > Hi Loic, > > On 6/29/2021 7:44 PM, Loic Poulain wrote: > > >>> BTW, if IOSM modems have a default data channel, I can add a separate > >>> patch to the series to create a default network interface for IOSM if you tell > >>> me which link id is used for the default data channel. > >> > >> Link id 1 is always associated as default data channel. > > > > Quick question, Isn't your driver use MBIM session IDs? with > > session-ID 0 as the default one? > > Link Id from 1 to 8 are treated as valid link ids. These ids are > decremented by 1 to match session id. > > In this case link id 1 would be mapped to session id 0. So have > requested link id 1 to be set as default data channel. Oh ok, but why? it seems quite confusing, that means a user creating a MBIM session 0 has to create a link with ID 1? It seems to be quite specific to your driver, can't you simply handle ID 0 from user? to keep aligned with other drivers. Regards, Loic
Hi Loic, On 6/29/2021 8:59 PM, Loic Poulain wrote: > Hi Chetan, > > On Tue, 29 Jun 2021 at 16:56, Kumar, M Chetan <m.chetan.kumar@intel.com> wrote: >> >> Hi Loic, >> >> On 6/29/2021 7:44 PM, Loic Poulain wrote: >> >>>>> BTW, if IOSM modems have a default data channel, I can add a separate >>>>> patch to the series to create a default network interface for IOSM if you tell >>>>> me which link id is used for the default data channel. >>>> >>>> Link id 1 is always associated as default data channel. >>> >>> Quick question, Isn't your driver use MBIM session IDs? with >>> session-ID 0 as the default one? >> >> Link Id from 1 to 8 are treated as valid link ids. These ids are >> decremented by 1 to match session id. >> >> In this case link id 1 would be mapped to session id 0. So have >> requested link id 1 to be set as default data channel. > > Oh ok, but why? it seems quite confusing, that means a user creating a > MBIM session 0 has to create a link with ID 1? > It seems to be quite specific to your driver, can't you simply handle > ID 0 from user? to keep aligned with other drivers. Thought link id 0 is not a valid id so had considered it from 1 :( Sure, We will correct it to be intact with other drivers. Regards, Chetan
diff --git a/drivers/net/wwan/iosm/iosm_ipc_wwan.c b/drivers/net/wwan/iosm/iosm_ipc_wwan.c index 1711b79fc616..bee9b278223d 100644 --- a/drivers/net/wwan/iosm/iosm_ipc_wwan.c +++ b/drivers/net/wwan/iosm/iosm_ipc_wwan.c @@ -329,22 +329,9 @@ struct iosm_wwan *ipc_wwan_init(struct iosm_imem *ipc_imem, struct device *dev) void ipc_wwan_deinit(struct iosm_wwan *ipc_wwan) { - int if_id; - + /* This call will remove all child netdev(s) */ wwan_unregister_ops(ipc_wwan->dev); - for (if_id = 0; if_id < ARRAY_SIZE(ipc_wwan->sub_netlist); if_id++) { - struct iosm_netdev_priv *priv; - - priv = rcu_access_pointer(ipc_wwan->sub_netlist[if_id]); - if (!priv) - continue; - - rtnl_lock(); - ipc_wwan_dellink(ipc_wwan, priv->netdev, NULL); - rtnl_unlock(); - } - mutex_destroy(&ipc_wwan->if_mutex); kfree(ipc_wwan);
Since the last commit, the WWAN core will remove all our network interfaces for us at the time of the WWAN netdev ops unregistering. Therefore, we can safely drop the custom code that cleaning the list of created netdevs. Anyway it no longer removes any netdev, since all netdevs were removed earlier in the wwan_unregister_ops() call. Signed-off-by: Sergey Ryazanov <ryazanov.s.a@gmail.com> CC: M Chetan Kumar <m.chetan.kumar@intel.com> CC: Intel Corporation <linuxwwan@intel.com> --- drivers/net/wwan/iosm/iosm_ipc_wwan.c | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-)