2012/3/18 Sergei Shtylyov <sshtylyov@xxxxxxxxxx>:
> Hello.
>
> On 17-03-2012 16:17, Aleksey Babahin wrote:
>
> Sorry for some grammar nitpicking.
>
That you have to forgive me.
>
>> We should send special control command to tell device start or stop
>> transmitting a data.
>> In Bi-Directional mode this cmd`s are not required.
>
>
> s/this cmd`s/these cmd's/
>
>
>> ---
>> drivers/usb/serial/metro-usb.c | 44
>> ++++++++++++++++++++++++++++++++++++++++
>> 1 files changed, 44 insertions(+), 0 deletions(-)
>
>
>> diff --git a/drivers/usb/serial/metro-usb.c
>> b/drivers/usb/serial/metro-usb.c
>> index 6dba779..f9c73c5 100644
>> --- a/drivers/usb/serial/metro-usb.c
>> +++ b/drivers/usb/serial/metro-usb.c
>> @@ -56,6 +56,38 @@ MODULE_DEVICE_TABLE(usb, id_table);
>> /* Input parameter constants. */
>> static bool debug;
>>
>> +/* UNI-Directional mode commands for device configure */
>> +static char open_cmd = 0x80;
>> +static char close_cmd = 0xFF;
>
>
> Why these are not #define's or member of enum instead. Or at least
> *const*?
I will use #define.
>
>
>> +
>> +inline int metrousb_is_unidirectional_mode(struct usb_serial_port *port)
>> +{
>> + __u16 product_id = le16_to_cpu(
>> + port->serial->dev->descriptor.idProduct);
>> +
>> + return product_id == FOCUS_PRODUCT_ID_UNI;
>> +}
>> +
>> +static int metrousb_send_unidirectional_cmd(u8 cmd, struct
>> usb_serial_port *port)
>> +{
>> + int ret;
>> + int actual_len;
>> + u8 buf_cmd = cmd;
>
>
> You can't do DMA off stack -- you need to kmalloc() the buffer.
>
kmalloc() for one byte? Well, it is necessary to do so.
I get a problem when use static variable directly. usbmon say: "data
was sent", but the device does not do anything.
>
>> + ret = usb_interrupt_msg(port->serial->dev,
>> + usb_sndintpipe(port->serial->dev,
>> port->interrupt_out_endpointAddress),
>> + &buf_cmd, sizeof(buf_cmd),
>> + &actual_len, USB_CTRL_SET_TIMEOUT);
>
>
> This call can cause DMA of the buffer.
>
>> + if (ret< 0)
>> + return ret;
>> + else if (actual_len != sizeof(buf_cmd))
>> + return -EIO;
>> + return 0;
>> +}
>> +
>> static void metrousb_read_int_callback(struct urb *urb)
>> {
>> struct usb_serial_port *port = urb->context;
>> @@ -205,6 +240,15 @@ static int metrousb_open(struct tty_struct *tty,
>> struct usb_serial_port *port)
>> goto exit;
>> }
>>
>> + /* Send activate cmd to device */
>> + result = metrousb_send_unidirectional_cmd(open_cmd, port);
>> + if (result) {
>> + dev_err(&port->dev,
>> + "%s - failed to configure device for port
>> number=%d, error code=%d\n"
>> + , __FUNCTION__, port->number, result);
>
>
> Same comment about comma and __FUNCTION__ as in the previous patch.
>
Already fixed and waiting for resend the series.
--
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
