First off, thanks for your helpful response! :)
On 05/18/2013 10:53 AM, Alan Stern wrote:
On Fri, 17 May 2013, Daniel Santos wrote:
Hello. I'm working on a driver for a USB to SPI bridge chip
(Microchip's MCP2210) that acts as a HID device (generic subclass). Of
course, it can be used from user-space via hid-generic, but I'm not
really interested in that. I started this work from somebody else's
implementation that was a HID-based driver, but didn't like the
requirement to allocate 6k of data for the report for each 64 byte
rep/req so I started over as a plane-jane USB driver. I suppose this
was a bit naive, having never written a USB driver before, I guess I
relish challenges. Also, I'm intending to use this driver on a
low-memory embedded device.
So I have the basics of my driver all working, sending 64 byte interrupt
URB requests (as per the datasheet -
http://ww1.microchip.com/downloads/en/DeviceDoc/22288A.pdf) and
receiving 64 byte response URBs,
That doesn't make sense. Your driver submits URBs to the USB stack and
waits for them to complete; it doesn't receive URBs from anywhere.
Maybe you meant that your driver receives 64-byte response packets.
Yes, my apologies, I didn't state that very clearly. I meant that I
submit a "request" URB to the in interrupt endpoint as well a "response"
URB to the out interrupt endpoint.This is where I'm currently submitting
these (for now just assume can_sleep is always zero, it's broken otherwise):
static int ctl_submit_req(struct mcp2210_command *cmd, int can_sleep)
{
struct mcp2210_ctl_command *ctl_cmd = (struct mcp2210_ctl_command
*)cmd;
struct mcp2210_device *dev;
const gfp_t gfp_flags = can_sleep ? GFP_KERNEL : GFP_ATOMIC;
unsigned long irqflags;
int ret = 0;
if (!cmd || !cmd->dev || cmd->type != &mcp2210_cmd_type_ctrl)
return -EINVAL;
dev = cmd->dev;
memcpy(dev->in.buffer, &ctl_cmd->req, sizeof(ctl_cmd->req));
dev->in.int_urb->complete = cmd->type->complete_req;
dev->out.int_urb->complete = cmd->type->complete_rep;
/* lock command in case we get preempted here and a completion handler
* is called */
/* FIXME: this makes no sense when can_sleep != 0 */
spin_lock_irqsave(&cmd->spinlock, irqflags);
ret = usb_submit_urb(dev->in.int_urb, gfp_flags);
if (ret) {
dev_err(&dev->udev->dev,
"usb_submit_urb failed for request URB %d", ret);
goto exit_unlock;
}
ret = usb_submit_urb(dev->out.int_urb, gfp_flags);
if (ret) {
dev_err(&dev->udev->dev,
"usb_submit_urb failed for response URB %d", ret);
usb_kill_urb(dev->in.int_urb);
goto exit_unlock;
}
cmd->state = MCP2210_CMD_STATE_SUBMITTED;
exit_unlock:
spin_unlock_irqrestore(&cmd->spinlock, irqflags);
return ret;
}
But that doesn't make much sense either; it would require an extremely
unusual HID device to send 64-byte responses. For example, a mouse or
a keyboard generally sends responses in the 3- to 5-byte range.
Yes, I know. This isn't even a device which interfaces with humans! I
presume Microchip implemented it as such so that it could be interfaced
from userspace with the generic HID drivers available on all modern
platforms. I think I understand their design decision, but from my
standpoint, it seems introduce a lot of needless complexity. Section
3.0 (page 11) of the datasheet
(http://ww1.microchip.com/downloads/en/DeviceDoc/22288A.pdf) describes
the USB command/response pairs it supports and all of them are 64 bytes,
most padded with lots of zeros. Then again, it's a $2.10 IC.
I guess this is the reason I've shied away from Mathew King's basic
alpha driver (https://github.com/MathewKing/mcp2210-linux), with 64 byte
messages. For some reason, I originally thought that it was allocated
6k of memory for each HID report,
(https://github.com/MathewKing/mcp2210-linux/blob/master/mcp2210-core.c#L102)
but after actually testing it (on x86_64), it's only 2456 bytes.
I guess this introduces a new (maybe better) question. If I instead
convert this back into a proper usbhid driver (instead of a plane USB
interface driver as it is now), can I allocate my struct hid_report and
struct hid_field just once and reuse them instead of allocating new ones
for each command/response pair? The original point of doing this as a
USB interface driver was to reduce memory requirements.
The traffic you see is all standard HID requests.
Yeah, that's what I figured -- something I wasn't doing.
My other thought was that I was perhaps I'm just not waiting long enough
before submitting the response URB (maybe the chip needs a little time
to process the request and just returns all zeros when it doesn't have
anything to say yet?)
This depends on how you are using it. The HID protocol includes a
request whereby the host tells the device not to send data if there
have not been any changes.
Well, I have an almost descent grasp writing plane USB interface
drivers, I suppose I need to learn the HID protocol now.My understanding
of the device is that it will only give you valid responses if a valid
request has been sent prior. Maybe I'm ignoring the fact that it's
still a compliant HID device and wont even give me that unless I do as
HID drivers do.
If your device runs at high speed then 125 us is correct. If the
device runs at full speed then 1 means 1 ms.
Yes it does, so at least I'm correct here! :)
Anyway, if somebody has some advice, I would greatly appreciate it. I'm
combing through the USB 2 spec to try to find where the HID protocol is
covered, but maybe that's an entirely different spec? Advice greatly
appreciated!!
See <http://www.usb.org/developers/hidpage/>.
Alan Stern
Thanks! Been reading, lots to cover!
Daniel
--
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
