On 2021/03/28 13:40, Patrick Wildt wrote:
> Am Sun, Mar 28, 2021 at 10:53:53AM +0100 schrieb Stuart Henderson:
> > On 2021/03/25 00:14, Stuart Henderson wrote:
> > > On 2021/03/25 00:30, Patrick Wildt wrote:
> > > > Without having looked at anything, it might be worth looking at the most
> > > > recent mail in this thread:
> > > >
> > > > 'Re: [PATCH] umb(4) fix for X20 (DW5821e) in Dell Latitude 7300'
> > > >
> > >
> > > oh, usb runs through all drivers looking for a VID/PID match before
> > > then running through all looking for a class match? I didn't realise
> > > (thought it would try driver-by-driver with first VID/PID then class)
> > > but that does make sense.
> > >
> > > Updated diff below adding my pid/vid and tweaking some comments. My card
> > > attaches to umb with this. I've only added it commented-out to the manual
> > > for now until I see it actually pass traffic.
> > >
> > > So this fixes Bryan's, at least improves mine (will look for another
> > > sim / sim-adapter tomorrow), and seems targetted enough to not risk
> > > fallout with existing working devices.
> > >
> > > I think this makes sense to commit. OK with me if you'd like to commit
> > > it Gerhard (I think it was your diff originally).
> >
> > So this isn't enough for the Huawei yet but it doesn't make things
> > any worse, and helps for DW5821e.
> >
> > If Gerhard isn't around, can I commit this bit so I'm not wrangling
> > things which should be separate commits? OK?
>
> Yes, please do. ok patrick@
Thanks, done.
Since it looks like we're going to need additional quirks to get Huawei
to work and having three separate vid/pid tables seems silly, here's a
diff to change to using a single pid/vid table covering the matches and
the existing fccauth quirk.
Tested with EM7455 (fcc auth required; works as before) and Huawei
(attaches to the correct config descriptor; packet transfer not working
but this is not unexpected).
Index: if_umb.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/if_umb.c,v
retrieving revision 1.38
diff -u -p -r1.38 if_umb.c
--- if_umb.c 28 Mar 2021 12:08:58 -0000 1.38
+++ if_umb.c 28 Mar 2021 13:10:56 -0000
@@ -224,34 +224,34 @@ const struct cfattach umb_ca = {
int umb_delay = 4000;
-/*
- * Normally, MBIM devices are detected by their interface class and subclass.
- * But for some models that have multiple configurations, it is better to
- * match by vendor and product id so that we can select the desired
- * configuration ourselves, e.g. to override a class-based match to another
- * driver.
- *
- * OTOH, some devices identify themselves as an MBIM device but fail to speak
- * the MBIM protocol.
- */
-struct umb_products {
+struct umb_quirk {
struct usb_devno dev;
- int confno;
+ u_int32_t umb_flags;
+ int umb_confno;
+ int umb_match;
};
-const struct umb_products umb_devs[] = {
- { { USB_VENDOR_DELL, USB_PRODUCT_DELL_DW5821E }, 2 },
- { { USB_VENDOR_HUAWEI, USB_PRODUCT_HUAWEI_ME906S }, 3 },
+const struct umb_quirk umb_quirks[] = {
+ { { USB_VENDOR_DELL, USB_PRODUCT_DELL_DW5821E },
+ 0,
+ 2,
+ UMATCH_VENDOR_PRODUCT
+ },
+
+ { { USB_VENDOR_HUAWEI, USB_PRODUCT_HUAWEI_ME906S },
+ 0,
+ 3,
+ UMATCH_VENDOR_PRODUCT
+ },
+
+ { { USB_VENDOR_SIERRA, USB_PRODUCT_SIERRA_EM7455 },
+ UMBFLG_FCC_AUTH_REQUIRED,
+ 0,
+ 0
+ },
};
#define umb_lookup(vid, pid) \
- ((const struct umb_products *)usb_lookup(umb_devs, vid, pid))
-
-/*
- * These devices require an "FCC Authentication" command.
- */
-const struct usb_devno umb_fccauth_devs[] = {
- { USB_VENDOR_SIERRA, USB_PRODUCT_SIERRA_EM7455 },
-};
+ ((const struct umb_quirk *)usb_lookup(umb_quirks, vid, pid))
uint8_t umb_qmi_alloc_cid[] = {
0x01,
@@ -283,10 +283,12 @@ int
umb_match(struct device *parent, void *match, void *aux)
{
struct usb_attach_arg *uaa = aux;
+ const struct umb_quirk *quirk;
usb_interface_descriptor_t *id;
- if (umb_lookup(uaa->vendor, uaa->product) != NULL)
- return UMATCH_VENDOR_PRODUCT;
+ quirk = umb_lookup(uaa->vendor, uaa->product);
+ if (quirk != NULL && quirk->umb_match)
+ return (quirk->umb_match);
if (!uaa->iface)
return UMATCH_NONE;
if ((id = usbd_get_interface_descriptor(uaa->iface)) == NULL)
@@ -317,6 +319,7 @@ umb_attach(struct device *parent, struct
{
struct umb_softc *sc = (struct umb_softc *)self;
struct usb_attach_arg *uaa = aux;
+ const struct umb_quirk *quirk;
usbd_status status;
struct usbd_desc_iter iter;
const usb_descriptor_t *desc;
@@ -339,13 +342,27 @@ umb_attach(struct device *parent, struct
sc->sc_ctrl_ifaceno = uaa->ifaceno;
ml_init(&sc->sc_tx_ml);
+ quirk = umb_lookup(uaa->vendor, uaa->product);
+ if (quirk != NULL && quirk->umb_flags) {
+ DPRINTF("%s: setting flags 0x%x from quirk\n", DEVNAM(sc),
+ quirk->umb_flags);
+ sc->sc_flags |= quirk->umb_flags;
+ }
+
+ /*
+ * Normally, MBIM devices are detected by their interface class and
+ * subclass. But for some models that have multiple configurations, it
+ * is better to match by vendor and product id so that we can select
+ * the desired configuration ourselves, e.g. to override a class-based
+ * match to another driver.
+ */
if (uaa->configno < 0) {
- /*
- * In case the device was matched by VID/PID instead of
- * InterfaceClass/InterfaceSubClass, we have to pick the
- * correct configuration ourself.
- */
- uaa->configno = umb_lookup(uaa->vendor, uaa->product)->confno;
+ if (quirk == NULL) {
+ printf("%s: unknown configuration for vid/pid match\n",
+ DEVNAM(sc));
+ goto fail;
+ }
+ uaa->configno = quirk->umb_confno;
DPRINTF("%s: switching to config #%d\n", DEVNAM(sc),
uaa->configno);
status = usbd_set_config_no(sc->sc_udev, uaa->configno, 1);
@@ -357,7 +374,7 @@ umb_attach(struct device *parent, struct
usbd_delay_ms(sc->sc_udev, 200);
/*
- * Need to do some manual setups that usbd_probe_and_attach()
+ * Need to do some manual setup that usbd_probe_and_attach()
* would do for us otherwise.
*/
uaa->nifaces = uaa->device->cdesc->bNumInterfaces;
@@ -435,10 +452,8 @@ umb_attach(struct device *parent, struct
printf("%s: missing MBIM descriptor\n", DEVNAM(sc));
goto fail;
}
- if (usb_lookup(umb_fccauth_devs, uaa->vendor, uaa->product)) {
- sc->sc_flags |= UMBFLG_FCC_AUTH_REQUIRED;
+ if (sc->sc_flags & UMBFLG_FCC_AUTH_REQUIRED)
sc->sc_cid = -1;
- }
for (i = 0; i < uaa->nifaces; i++) {
if (usbd_iface_claimed(sc->sc_udev, i))
