Some minor style nits.
On Wed, Aug 06, 2014 at 01:35:32PM +0200, Luca Ellero wrote:
+static int ni6501_send_command(struct comedi_device *dev, int command,
+ const u8 *port, u8 *bitmap)
+{
+ struct usb_device *usb = comedi_to_usb_dev(dev);
+ struct ni6501_private *devpriv = dev->private;
+ int request_size, response_size;
+ u8 *tx = devpriv->usb_tx_buf;
+ int ret;
+
+ if (!tx || !port)
+ return -EINVAL;
+
+ if (command != SET_PORT_DIR && !bitmap)
^
Extra space character.
+ return -EINVAL;
+
+ down(&devpriv->sem);
+
+ switch (command) {
+ case READ_PORT:
+
+ request_size = sizeof(READ_PORT_REQUEST);
+ /* 4 additional bytes for READ_PORT request */
+ response_size = sizeof(GENERIC_RESPONSE) + 4;
+
+ memcpy(tx, READ_PORT_REQUEST, request_size);
+
+ tx[14] = port[0];
+
+ break;
+
+ case WRITE_PORT:
+
+ request_size = sizeof(WRITE_PORT_REQUEST);
+ response_size = sizeof(GENERIC_RESPONSE);
+
+ memcpy(tx, WRITE_PORT_REQUEST, request_size);
+
+ tx[14] = port[0];
+ tx[17] = bitmap[0];
+
+ break;
+
+ case SET_PORT_DIR:
+
+ request_size = sizeof(SET_PORT_DIR_REQUEST);
+ response_size = sizeof(GENERIC_RESPONSE);
+
+ memcpy(tx, SET_PORT_DIR_REQUEST, request_size);
+
+ tx[14] = port[0];
+ tx[15] = port[1];
+ tx[16] = port[2];
+
+ break;
+
+ default:
+ ret = -EINVAL;
+ goto end;
+ }
+
+ ret = usb_bulk_msg(usb,
+ usb_sndbulkpipe(usb,
+ devpriv->ep_tx->bEndpointAddress),
+ devpriv->usb_tx_buf,
+ request_size,
+ NULL,
+ NI6501_TIMEOUT);
+
Don't leave a blank line here before checking ret. Especially the
ni6501_dio_insn_bits() has too many blanks lines. See below.
+ if (ret)
+ goto end;
+
+ ret = usb_bulk_msg(usb,
+ usb_rcvbulkpipe(usb,
+ devpriv->ep_rx->bEndpointAddress),
+ devpriv->usb_rx_buf,
+ response_size,
+ NULL,
+ NI6501_TIMEOUT);
+
+ if (ret)
+ goto end;
+
+ /* Check if results are valid */
^
Extra space character.
[snip]
+static int ni6501_dio_insn_bits(struct comedi_device *dev,
+ struct comedi_subdevice *s,
+ struct comedi_insn *insn,
+ unsigned int *data)
+{
+ unsigned int mask;
+ int ret;
+ u8 port;
+ u8 bitmap;
+
+ mask = comedi_dio_update_state(s, data);
+
+ if (mask & 0xff) {
+ port = 0x00;
+ bitmap = s->state & 0xFF;
+
+ ret = ni6501_send_command(dev, WRITE_PORT, &port, &bitmap);
+
+ if (ret)
+ return ret;
+ }
+
+ if (mask & 0xff00) {
+ port = 0x01;
+ bitmap = (s->state >> 8) & 0xFF;
+
+ ret = ni6501_send_command(dev, WRITE_PORT, &port, &bitmap);
+
+ if (ret)
+ return ret;
+ }
+
+ if (mask & 0xff0000) {
+ port = 0x02;
+ bitmap = (s->state >> 16) & 0xFF;
+
+ ret = ni6501_send_command(dev, WRITE_PORT, &port, &bitmap);
+
+ if (ret)
+ return ret;
+ }
+
+ /* Read port 0 */
This comment doesn't add anything because port = 0x00 below.
+
+ port = 0x00;
+
+ ret = ni6501_send_command(dev, READ_PORT, &port, &bitmap);
+
+ if (ret)
+ return ret;
+
+ data[1] = bitmap;
+
You could do it like this:
data[1] = 0;
for (port = 0; port < 3; port++) {
ret = ni6501_send_command(dev, READ_PORT, &port, &bitmap);
if (ret)
return ret;
data[1] |= bitmap << port * 8;
}
regards,
dan carpenter
