On Wed, Mar 23, 2011 at 05:28:29AM -0400, Alan Cox wrote:
> On Tue, 22 Mar 2011 20:43:47 -0700
> Guenter Roeck <guenter.roeck@xxxxxxxxxxxx> wrote:
>
> > This patch adds support for the I2C interface of the Devantech USB-ISS
> > Multifunction adapter.
> >
> > Signed-off-by: Guenter Roeck <guenter.roeck@xxxxxxxxxxxx>
> > ---
> > The driver has one problem: It competes with the cdc_acm driver for device
> > access. Copying the usb mailing list in the hope that someone can tell me
> > if there is a way to prevent this from happening.
>
> From the look of the code you actually want to be sitting on top of
> cdc_acm.
>
Ok, I'll see if I can do that.
>
> > +/* Send command to device, and get response. */
> > +static int devantech_usb_transfer(struct i2c_devantech_iss *dev, int len)
> > +{
> > + int ret, actual;
> > +
> > + if (len <= 0 || len > ISS_MAX_TRANSFER_LEN)
> > + return -EINVAL;
> > +
> > + ret = usb_bulk_msg(dev->usb_dev,
> > + usb_sndbulkpipe(dev->usb_dev, dev->ep_out),
> > + dev->buffer, len, &actual,
> > + ISS_USB_TIMEOUT);
> > +
> > + if (!ret) {
> > + ret = usb_bulk_msg(dev->usb_dev,
> > + usb_rcvbulkpipe(dev->usb_dev, dev->ep_in),
> > + dev->buffer, sizeof(dev->buffer),
> > + &actual, ISS_USB_TIMEOUT);
> > + if (!ret && actual > 0)
> > + ret = actual;
> > + }
> > + return ret;
> > +}
>
> The whole driver seems to boil down to sending and receiving message
> blocks to the bulk endpoint - yes ?
>
Yes
> > + dev->usb_dev = usb_get_dev(interface_to_usbdev(interface));
> > + dev->interface = interface;
> > +
> > + /* setup i2c adapter description */
> > + dev->adapter.owner = THIS_MODULE;
> > + dev->adapter.class = I2C_CLASS_HWMON;
>
> Doesn't seem to be HWMON ?
>
You lost me there. Why not ? Many if not all I2C bus drivers register as
I2C_CLASS_HWMON.
>
> The rest looks like the device doesn't pretend to be CDC ACM class, but
> is CDC-ACM class with the "device" the other end talking to the other end
> of the serial port.
>
> In fact it would be an interesting test to open the serial port and write
> your commands to it from user space to see what happens. If I read the
> code right it'll work fine.
>
Yes, it should work. That is what is done in Windows.
> From a tty perspective it would probably make sense to adjust the driver
> to be a line discipline sat on CDC-ACM.
>
> I am *very* wary of removing it from the CDC_ACM side because people may
> be using the CDC-ACM userspace directly for existing projects and would
> get a nasty surprise and breakage.
>
Same here. Not likely for existing projects (the device is brand new),
but at some point someone might want to use it.
> If setting the tty to raw 8bit noecho and firing commands over the
> virtual interface works then turning your driver into an ldisc is pretty
> trivial. The ldisc interfaces provide data received and allow data writes
> through the relevant tty.
>
I'll try.
Thanks,
Guenter
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
