GNOME Bugzilla – Bug 696701
[MM 0.8] MBM devices get QMI probed
Last modified: 2013-04-01 12:17:09 UTC
Created attachment 239958 [details] md300 probing with qmi They use cdc-wdm as well, but obviously aren't QMI-capable. Which makes me wonder, do we need 10 VersionInfo tries for QMI? Is there some way to prevent QMI probing of Ericsson devices through the udev tags we've already got for the MBM devices? It doesn't look like it hurts anything, it just really slows down probing of the Ericsson devices.
Please review the 'aleksander/explicit-qmi-probing' branch in git. The change is basically to make the plugins explicitly request QMI probing, as done already with AT and QCDM. If the plugin doesn't ask for QMI probing (e.g. the MBM plugin), it will just accept the port with 'unknown' type.
Branch looks good, and we should merge it, but it doesn't solve the problem, because the mbm plugin doesn't claim "wdm" ports and thus the Generic plugin asks for them to be QMI probed before the mbm plugin can claim them later after ACM0 has been probed and assigned to mbm. We need an additional patch like this: diff --git a/plugins/mbm/77-mm-ericsson-mbm.rules b/plugins/mbm/77-mm-ericsson-mbm.rules index 863204c..6f53ef0 100644 --- a/plugins/mbm/77-mm-ericsson-mbm.rules +++ b/plugins/mbm/77-mm-ericsson-mbm.rules @@ -1,7 +1,10 @@ # do not edit this file, it will be overwritten on update ACTION!="add|change", GOTO="mm_mbm_end" -SUBSYSTEM!="tty", GOTO="mm_mbm_end" +SUBSYSTEMS=="usb", GOTO="mm_mbm_check" +GOTO="mm_mbm_end" + +LABEL="mm_mbm_check" # Ericsson F3507g ATTRS{idVendor}=="0bdb", ATTRS{idProduct}=="1900", ENV{ID_MM_ERICSSON_MBM}="1" diff --git a/plugins/mbm/mm-plugin-mbm.c b/plugins/mbm/mm-plugin-mbm.c index d776265..6c23e7c 100644 --- a/plugins/mbm/mm-plugin-mbm.c +++ b/plugins/mbm/mm-plugin-mbm.c @@ -55,7 +55,7 @@ create_modem (MMPlugin *self, G_MODULE_EXPORT MMPlugin * mm_plugin_create (void) { - static const gchar *subsystems[] = { "tty", "net", NULL }; + static const gchar *subsystems[] = { "tty", "net", "usb", NULL }; static const gchar *udev_tags[] = { "ID_MM_ERICSSON_MBM", NULL The udev rule syntax is a bit ugly, but I don't remember how to do logical OR on subsystems; eg we want "SUBSYSTEMS==tty|usb". Any other modems you can think of that use the cdc-wdm driver?
(In reply to comment #2) > Branch looks good, and we should merge it, but it doesn't solve the problem, > because the mbm plugin doesn't claim "wdm" ports and thus the Generic plugin > asks for them to be QMI probed before the mbm plugin can claim them later after > ACM0 has been probed and assigned to mbm. We need an additional patch like > this: > > diff --git a/plugins/mbm/77-mm-ericsson-mbm.rules > b/plugins/mbm/77-mm-ericsson-mbm.rules > index 863204c..6f53ef0 100644 > --- a/plugins/mbm/77-mm-ericsson-mbm.rules > +++ b/plugins/mbm/77-mm-ericsson-mbm.rules > @@ -1,7 +1,10 @@ > # do not edit this file, it will be overwritten on update > > ACTION!="add|change", GOTO="mm_mbm_end" > -SUBSYSTEM!="tty", GOTO="mm_mbm_end" > +SUBSYSTEMS=="usb", GOTO="mm_mbm_check" Shouldn't it have an additional check like this one here? In order to do the "tty|usb" logic check: SUBSYSTEMS=="usb", GOTO="mm_mbm_check" > +GOTO="mm_mbm_end" > + > +LABEL="mm_mbm_check" > > # Ericsson F3507g > ATTRS{idVendor}=="0bdb", ATTRS{idProduct}=="1900", ENV{ID_MM_ERICSSON_MBM}="1" > diff --git a/plugins/mbm/mm-plugin-mbm.c b/plugins/mbm/mm-plugin-mbm.c > index d776265..6c23e7c 100644 > --- a/plugins/mbm/mm-plugin-mbm.c > +++ b/plugins/mbm/mm-plugin-mbm.c > @@ -55,7 +55,7 @@ create_modem (MMPlugin *self, > G_MODULE_EXPORT MMPlugin * > mm_plugin_create (void) > { > - static const gchar *subsystems[] = { "tty", "net", NULL }; > + static const gchar *subsystems[] = { "tty", "net", "usb", NULL }; > static const gchar *udev_tags[] = { > "ID_MM_ERICSSON_MBM", > NULL > > > The udev rule syntax is a bit ugly, but I don't remember how to do logical OR > on subsystems; eg we want "SUBSYSTEMS==tty|usb". > > Any other modems you can think of that use the cdc-wdm driver? MBIM modems will also use cdc-wdm, but we don't handle those yet.
(In reply to comment #3) > (In reply to comment #2) > > Branch looks good, and we should merge it, but it doesn't solve the problem, > > because the mbm plugin doesn't claim "wdm" ports and thus the Generic plugin > > asks for them to be QMI probed before the mbm plugin can claim them later after > > ACM0 has been probed and assigned to mbm. We need an additional patch like > > this: > > > > diff --git a/plugins/mbm/77-mm-ericsson-mbm.rules > > b/plugins/mbm/77-mm-ericsson-mbm.rules > > index 863204c..6f53ef0 100644 > > --- a/plugins/mbm/77-mm-ericsson-mbm.rules > > +++ b/plugins/mbm/77-mm-ericsson-mbm.rules > > @@ -1,7 +1,10 @@ > > # do not edit this file, it will be overwritten on update > > > > ACTION!="add|change", GOTO="mm_mbm_end" > > -SUBSYSTEM!="tty", GOTO="mm_mbm_end" > > +SUBSYSTEMS=="usb", GOTO="mm_mbm_check" > > Shouldn't it have an additional check like this one here? In order to do the > "tty|usb" logic check: > SUBSYSTEMS=="usb", GOTO="mm_mbm_check" I actually meant: SUBSYSTEMS=="tty", GOTO="mm_mbm_check"
(In reply to comment #2) > Branch looks good, and we should merge it Merged it.
(In reply to comment #4) > (In reply to comment #3) > > (In reply to comment #2) > > > Branch looks good, and we should merge it, but it doesn't solve the problem, > > > because the mbm plugin doesn't claim "wdm" ports and thus the Generic plugin > > > asks for them to be QMI probed before the mbm plugin can claim them later after > > > ACM0 has been probed and assigned to mbm. We need an additional patch like > > > this: > > > > > > diff --git a/plugins/mbm/77-mm-ericsson-mbm.rules > > > b/plugins/mbm/77-mm-ericsson-mbm.rules > > > index 863204c..6f53ef0 100644 > > > --- a/plugins/mbm/77-mm-ericsson-mbm.rules > > > +++ b/plugins/mbm/77-mm-ericsson-mbm.rules > > > @@ -1,7 +1,10 @@ > > > # do not edit this file, it will be overwritten on update > > > > > > ACTION!="add|change", GOTO="mm_mbm_end" > > > -SUBSYSTEM!="tty", GOTO="mm_mbm_end" > > > +SUBSYSTEMS=="usb", GOTO="mm_mbm_check" > > > > Shouldn't it have an additional check like this one here? In order to do the > > "tty|usb" logic check: > > SUBSYSTEMS=="usb", GOTO="mm_mbm_check" > > I actually meant: > SUBSYSTEMS=="tty", GOTO="mm_mbm_check" Well actually since the ttys also have a USB parent, the SUBSYSTEMS== will match both the TTYs and the WDM ports, so we end up with everything being correct it seems. I think we can just go with =="usb" and we're good, other than the ugly GOTOs.
Created attachment 240158 [details] [review] Patch. Does this patch work?
Fixed the issue with Dan's suggested changes, plus some additional change in MMBaseModem. All in git master now.