> Does a change to the README like so answer your question? > > To use this device, you will need: > 1) This driver. > 2) A userspace to use the interface(s) presented by this driver. ModemManager > (http://cgit.freedesktop.org/ModemManager/ModemManager/) can do this to some > extent; Qualcomm has a closed-source SDK, available under restrictive > license terms, that supports all features of the device. > 3) A firmware loader. This device comes up 'dumb' (that is, with no firmware on > it). Qualcomm ships a firmware loader as part of their SDK, and Matthew > Garrett has written an open-source one: > http://www.codon.org.uk/~mjg59/gobi_loader/ > 4) A firmware image. These are copyright Qualcomm and not redistributable. > Existing OEMs of Gobi modems might make these available on their websites. Ok > > > + for (pos = 4; pos + 3 < msgsize; pos += msize + 3) { > > > + msize = *(u16 *)(msg + pos + 1); > > > > Is this always guaranteed to be aligned ? > > No. Is it important that it be? (I've only tested this driver on Arm and x86). > If it is important, is there an idiomatic way to do this? get_unaligned_*() > > WTF ????? > > I feel embarassed about having missed these. This driver was (I suspect) ported Don't - I know what happens having done the same kind of thing with drivers I've stared at far too long to see them > > We have IOR/IOW/etc macros for building ioctl numbers and it should be > > using those. To start with that helps enormously for trace tools as it > > shows the directions and sizes in the ioctl code. > > These ioctl numbers have to stay the same in order to remain compatible with a > closed userspace SDK that we can't change. I think that is "not our problem" - point for discussion anyway. > > > + result = down_interruptible(&sem); > > > > Really wants to be a mutex so it'll work with the realtime stuff > > I don't understand this comment. Is there a document I should have read? Not that I know of The basic problem is that a semaphore is a counting object, it's very hard to answer the question "who is stopping me taking this semaphore" because there may be multiple takers in the general case. With a mutex you know who to blame, and for the realtime patches currently being moved into the kernel bit by bit this is vital in order to implement priority inheritance. > > So like validity - what lock stops it dying mid ioctl > > Again, not much. It seems I will need a lock on the whole device. A lot of our USB drivers went through the following rough cycle Crashes a lot on unload Lots of locking Deadlocks and crashes in obscure cases on unload Much patching with little progress Ripped out the locking and used struct kref and the kref_* functions to refcount instead You might want to look at those and see if they would fit your needs better ? > > > + wbuf = kmalloc(size + qmux_size, GFP_KERNEL); > > > > Maths overflow ? > > Oh, excellent catch! > > Does a per-device lock on the entire device (and an accompanying flattening of > 'struct qmidev' into 'struct qcusbnet', and replacement of qmidev's > clients_lock with a device lock) seem like a good solution to the concurrency > problems here? I wonder if refcounting it would be better but I've not sat and considered how well that fits this specific problem, merely thinking from general experience. Greg may also have some thoughts there as he's done far more USB than most people. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
