Artem Kachitchkine wrote: > >> 1) it has notions about USB "disconnect" and "reconnect". > > Just extra code that will never be used unless device is hotpluggable. > Is pcser hotpluggable?
Yes. But, the way it works is quite different. All PCMCIA device retain some "state" on disconnect (free on last use, more or less). The device disconnect is handled out of the context of the core serial driver, but instead the PCMCIA driver gets a special callback that is called to register the fact that the device is now absent. PCMCIA is really weird in some respects -- I wouldn't use it as a "model" for how to do things. :-) > >> 3) "rx" is a callback in the driver. How does a DSD indicate arrival of >> data? > > GSD registers callbacks (tx, rx, status), DSD invokes rx_cb(), GSD > calls ds_rx() to retrieve the data. Okay. I can't find the prototypes for the functions a driver should call in any of the headers. Maybe I'm looking in the wrong place? (I did find usbser_rx_cb(), but it looks like that isn't an exported declaration. Wait, it looks like the usb framework registers the callback on the DSD's data structure? Weird.) > >> 4) The "tx" spec says "* data transmit: DSD is *required* to accept mblk >> for transfer". How does one exert backpressure in the stream to prevent >> loss of data or runaway memory consumption? > > Flow control. So flow control is handled OOB instead of in-band. Interesting. > >> 5) The notion of separate "start" and "stop" and directions seems >> weird, at least to me. > > Data flows in both directions. No, what I don't understand is why you have these entry points at all. Once the parameters are set, I would have expected the device to just start receiving. Likewise, once an mblk is queued for transmit, I would have expected the device to send it, without need for an explicit activation routine. > >> 6) There is no documentation or clear information in the headers for how >> to write a driver. > > Yeah, probably because the interfaces are not yet public. As a small > contribution, I wrote a 4 part blog about this: > > http://blogs.sun.com/artem/entry/usb_serial_drivers_part_1 > http://blogs.sun.com/artem/entry/usb_serial_drivers_part_2 > http://blogs.sun.com/artem/entry/usb_serial_drivers_part_3 > http://blogs.sun.com/artem/entry/usb_serial_drivers_part_4 > >> 7) There does not appear to be support for timely handling of modem >> interrupts > > Define timely. DSD can invoke status callback whenever there's a line > change. Timely as in, while still in high-level interrupt context. Can I call the status callback in high level interrupt context? Somehow I'm doubtful -- the usbser_status_cb routine does putnextctl and putctl, which I _strongly_ suspect should not be called with a high-level interrupt held. We're talking microsecond resolution here. A fast implementation of PPS should be ~50usec. I also note that usbser lacks any calls to ddi_hardpps(). Again, since it is intended for USB, that might not be a big issue. > >> 8) There are of course other "hacks" needed > > Sure. They'll be needed for any framework, not just this one. Right. Do you want to add them to this framework? I can see that usbser could evolve to a more general purpose serial framework. Its not entirely clear to me whether this is desirable or not. There are things it does that are definitely tuned for USB. The separate rx and tx taskq's are an excellent example of this. > >> 9) CPR/Suspend resume? Power management? Why?!? I would have though >> these would have been handled in the DSD without interaction from GSD. > > Maybe. Maybe not. It just happens that USB DSD's use usbser_attach() > and usbser_detach() as the attach(9E)/detach(9E) entry points, so when > the kernel does CPR, it enters GSD code first, which then propagates > it down to DSD. Ew. Looking at this a bit more, the attach logic is full of USB-specificness. Including, btw the ai_usb_events member of ds_attach_info_t. I think this decision is poor for other reasons. As a good example, consider multi-port serial devices which might also have parallel ports on them. A single driver could host both types of ports, handling them as different minor nodes without problem. But to do so, it definitely needs to do its own attach logic. By the way, this also underscores another thing -- which is that I believe DSDs need to manage the mapping of minor number to port number and instance. Right now that detail is locked away in usbser, which is probably good for usb devices, but it is a poor abstraction for a variety of other kinds of serial devices (some which may have unusual numbering schemes, may require other port types like parallel, or alternate modes like synchronous serial comms.) > >> 10) Finally, the whole framework resides in USB specific directories > > Non-issue. Moving stuff around is easy. Its an issue where the ARCs are concerned. :-) We'd have to rename usbser to something like "gser" or "gsd", and change a bunch of the API to make it less USB centric. I'm not opposed to the idea of doing this, I just don't want to make light of it either. One good thing about doing this the way I'm doing is that I don't have to worry about breaking a bunch of USB serial devices for which I lack hardware. :-) I can convert existing legacy drivers to the new framework as I have time. ;-) > >> Anyway, at this point, I suspect what you have is fairly ideal for for >> USB, but doesn't port well to the needs of onboard and PCI devices. I >> suggest we not try to consolidate this along with my "gser" driver, but >> instead keep them separate, as they are well optimized to different >> needs. It does mean I probably need to bother to give much thought to >> supporting USB devices though -- you seem to have that pretty well under >> control. :-) > > Sure. I wasn't advocating usbser reuse. Just seemed that one of your > main motivations was to avoid code duplication ;-) Of course I prefer to avoid duplication. Its just not clear to me that usbser will do what I need it to do without major surgery. I'm a little concerned about trying to undertake such surgery myself. I suppose I could rename & copy usbser, and start from there. It isn't clear to me whether this will be faster for me or not. I understand my own framework, but I suspect that there is a lot of detail in yours that I'm not familiar with, and I'm also not too familiar with Solaris' USB DDI. So figuring out how to map a typical UART to your framework might take me a while. Additionally, I have to figure out how to deal with the short-comings like lack of ddi_hardpps(), and deal with high-level interrupts. (And yes, this means I'm designing my framework to explicitly cope with high-level interrupt handlers calling a few functions. In some cases I'm planning on slightly abusing the fact that I can call my gser_rx() a few times in succession to drain the FIFO while in high level interrupt context before my soft interrupt handler runs to move the collection of all the data upstream.) -- Garrett > > -Artem. -- Garrett D'Amore, Principal Software Engineer Tadpole Computer / Computing Technologies Division, General Dynamics C4 Systems http://www.tadpolecomputer.com/ Phone: 951 325-2134 Fax: 951 325-2191 _______________________________________________ opensolaris-code mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/opensolaris-code
