linux-kernel - Re: [PATCH] comedi: Humusoft MF634 and MF624 DAQ cards driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <52CAA38C.9050307@mev.co.uk>
Date:	Mon, 6 Jan 2014 12:37:32 +0000
From:	Ian Abbott <abbotti@....co.uk>
To:	Hartley Sweeten <HartleyS@...ionengravers.com>,
	Rostislav Lisovy <lisovy@...il.com>,
	Ian Abbott <ian.abbott@....co.uk>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"devel@...verdev.osuosl.org" <devel@...verdev.osuosl.org>
CC:	"pisa@....felk.cvut.cz" <pisa@....felk.cvut.cz>
Subject: Re: [PATCH] comedi: Humusoft MF634 and MF624 DAQ cards driver

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@....co.uk>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/