On Thu, Aug 16, 2018 at 01:28:54AM -0700, Patong Yang wrote: > On Thu, Aug 16, 2018 at 08:34:47AM +0200, Greg KH wrote: > > On Wed, Aug 15, 2018 at 10:56:47PM -0700, Patong Yang wrote: > > > Greg, > > > > > > Please see my response inline below. > > > > > > Patong > > > > > > > But there is a bigger problem here: > > > > > > > > > + xrusb_tty_driver = alloc_tty_driver(XRUSB_TTY_MINORS); > > > > > + if (!xrusb_tty_driver) > > > > > + return -ENOMEM; > > > > > > > > Why are you not using the usb serial core here? You need to do that, > > > > not try to provide your own custom tty driver. That way userspace > > > > programs will "just work" with your new device, no changes needed as > > > > your major/minor number and device name would be custom only for your > > > > device, which is not acceptable. > > > > > > The MaxLinear/USB serial devices support the CDC-ACM commands. > > > Therefore, we used the cdc-acm driver instead of the usb serial driver > > > as the starting point for developing the driver. We replaced "ACM" > > > with "XRUSB" throughout the driver. Would it be better if we just used > > > the same major/minor number as the CDC-ACM driver since it was based on > > > the cdc-acm driver? > > > > No, just use the cdc-acm driver itself and add your product/device id to > > it and it should work just fine. Why do you need to write a whole new > > driver at all? > > The basic TX/RX functionality works fine now with the cdc-acm driver. > That's the driver that is loaded because it's advertised as a cdc-acm > compatible device in the device descriptors. > > However, the cdc-acm driver (and spec) does not have support all of the > features in the MaxLinear/Exar USB UARTs. Hence the reason for a separate > and new driver. So your device should not be exposing itself as a cdc-acm driver if it is doing vendor-specific things, right? Ok, that's fine, but then you still need to tie into the usb-serial core, just use that and do not implement all of the duplicated tty handling logic that you have done here. You will end up with a ttyUSB* device node, which is what you, and your users want, not some custom-name that no program supports. thanks, greg k-h
