On 2014-01-02 18:38, Hartley Sweeten wrote:
On Monday, December 30, 2013 6:37 PM, Rostislav Lisovy wrote:
[snip]
+static int mf6x4_dio_insn_bits(struct comedi_device *dev,
+ struct comedi_subdevice *s,
+ struct comedi_insn *insn, unsigned int *data)
+{
+ struct mf6x4_private *devpriv = dev->private;
+
+ switch (s->type) {
+ case COMEDI_SUBD_DI: /* DIN */
+ data[1] = ioread16(devpriv->bar1_mem + MF6X4_DIN_reg) & MF6X4_DIN_mask;
+ break;
+
+ case COMEDI_SUBD_DO: /* DOUT */
+ /* data[0] -- mask
+ data[1] -- actual value */
+ if (data[0]) {
+ s->state &= ~data[0];
+ s->state |= (data[0] & data[1]);
Please use comedi_dio_update_state() to handle this boilerplate code.
+
+ iowrite16(s->state & MF6X4_DOUT_mask,
+ devpriv->bar1_mem + MF6X4_DOUT_reg);
+
+ data[1] = s->state;
+ }
+ break;
+ }
+
+ return insn->n;
+}
Please split this function into a mf6x4_di_insn_bits() and mf6x4_do_insn_bits().
That will remove an indent level and make the usage a bit clearer.
I'll also add that the `data[1] = s->state;` should be done even if
data[0] is zero, i.e. it should be moved after the end of the `if`
statement.
--
-=( Ian Abbott @ MEV Ltd. E-mail: <abbotti@xxxxxxxxx> )=-
-=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=-
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
