lkml.org 
[lkml]   [2011]   [Jan]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 3/5] Coding style in unioxx5.c KERN_ facility level and coding style warnings as reported by checkpatch.pl Signed-off-by: Michael Ahern <>
    On Mon, Jan 10, 2011 at 10:03:14AM +1100, ahern.michael.t@gmail.com wrote:
    > From: mah <ahern.michael.t@gmail.com>
    >
    > ---

    You put the description and signed-off-by line on the subject: You need
    to put a blank line after the first line in your git commit to keep git
    send-email from messing this up.


    > drivers/staging/comedi/drivers/unioxx5.c | 65 +++++++++++++++++++----------
    > 1 files changed, 42 insertions(+), 23 deletions(-)
    >
    > diff --git a/drivers/staging/comedi/drivers/unioxx5.c b/drivers/staging/comedi/drivers/unioxx5.c
    > index 598884e..334f036 100644
    > --- a/drivers/staging/comedi/drivers/unioxx5.c
    > +++ b/drivers/staging/comedi/drivers/unioxx5.c
    > @@ -72,13 +72,17 @@ Devices: [Fastwel] UNIOxx-5 (unioxx5),
    > #define ALL_2_INPUT 0 /* config all digital channels to input */
    > #define ALL_2_OUTPUT 1 /* config all digital channels to output */
    >
    > -/* 'private' structure for each subdevice */
    > +/* 'private' unioxx5 structure for each subdevice
    > + usp_module_type - 12 modules. each can be 70L or 73L
    > + usp_extra_data - for saving previous written value for analog modules
    > + usp_prev_wr_val - previous written value
    > + usp_prev_cn_val - previous channel value */
    > struct unioxx5_subd_priv {
    > int usp_iobase;
    > - unsigned char usp_module_type[12]; /* 12 modules. each can be 70L or 73L */
    > - unsigned char usp_extra_data[12][4]; /* for saving previous written value for analog modules */
    > - unsigned char usp_prev_wr_val[3]; /* previous written value */
    > - unsigned char usp_prev_cn_val[3]; /* previous channel value */
    > + unsigned char usp_module_type[12]; /* see comment above */
    > + unsigned char usp_extra_data[12][4]; /* see comment above */
    > + unsigned char usp_prev_wr_val[3]; /* see comment above */
    > + unsigned char usp_prev_cn_val[3]; /* see comment above */

    No need for the "comment above" just put all of the comments in the
    comment. And if you do that, use the proper kernel-doc format for this,
    so it works properly.

    > static int unioxx5_attach(struct comedi_device *dev,
    > @@ -99,7 +103,8 @@ static int __unioxx5_digital_write(struct unioxx5_subd_priv *usp,
    > unsigned int *data, int channel, int minor);
    > static int __unioxx5_digital_read(struct unioxx5_subd_priv *usp,
    > unsigned int *data, int channel, int minor);
    > -/* static void __unioxx5_digital_config(struct unioxx5_subd_priv* usp, int mode); */
    > +/* static void __unioxx5_digital_config(struct unioxx5_subd_priv* usp,
    > + int mode); */
    > static int __unioxx5_analog_write(struct unioxx5_subd_priv *usp,
    > unsigned int *data, int channel, int minor);
    > static int __unioxx5_analog_read(struct unioxx5_subd_priv *usp,
    > @@ -139,8 +144,10 @@ static int unioxx5_attach(struct comedi_device *dev,
    > dev->iobase = iobase;
    > iobase += UNIOXX5_SUBDEV_BASE;
    >
    > - /* defining number of subdevices and getting they types (it must be 'g01') */
    > - for (i = n_subd = 0, ba = iobase; i < 4; i++, ba += UNIOXX5_SUBDEV_ODDS) {
    > + /* defining number of subdevices and getting types (it must be 'g01') */
    > + for (i = n_subd = 0, ba = iobase; i < 4;
    > + i++, ba += UNIOXX5_SUBDEV_ODDS) {
    > +
    > id = inb(ba + 0xE);
    > num = inb(ba + 0xF);
    >
    > @@ -169,7 +176,7 @@ static int unioxx5_attach(struct comedi_device *dev,
    > return -1;
    > }
    >
    > - printk("attached\n");
    > + printk(KERN_INFO "attached\n");
    > return 0;
    > }
    >
    > @@ -181,7 +188,8 @@ static int unioxx5_subdev_read(struct comedi_device *dev,
    > int channel, type;
    >
    > channel = CR_CHAN(insn->chanspec);
    > - type = usp->usp_module_type[channel / 2]; /* defining module type(analog or digital) */
    > + /* defining module type(analog or digital) */
    > + type = usp->usp_module_type[channel / 2];
    >
    > if (type == MODULE_DIGITAL) {
    > if (!__unioxx5_digital_read(usp, data, channel, dev->minor))
    > @@ -202,7 +210,8 @@ static int unioxx5_subdev_write(struct comedi_device *dev,
    > int channel, type;
    >
    > channel = CR_CHAN(insn->chanspec);
    > - type = usp->usp_module_type[channel / 2]; /* defining module type(analog or digital) */
    > + /* defining module type(analog or digital) */
    > + type = usp->usp_module_type[channel / 2];
    >
    > if (type == MODULE_DIGITAL) {
    > if (!__unioxx5_digital_write(usp, data, channel, dev->minor))
    > @@ -259,11 +268,16 @@ static int unioxx5_insn_config(struct comedi_device *dev,
    > /* *\
    > * sets channels buffer to 1(after this we are allowed to *
    > * change channel type on input or output) *
    > + *
    > + outb(flags, ...) - changes type of _one_ channel
    > + outb(0, ...) - sets channels bank to 0(allows directly input/output)
    > + usp-> ... flags - saves written value
    > + *
    > \* */
    > outb(1, usp->usp_iobase + 0);
    > - outb(flags, usp->usp_iobase + channel_offset); /* changes type of _one_ channel */
    > - outb(0, usp->usp_iobase + 0); /* sets channels bank to 0(allows directly input/output) */
    > - usp->usp_prev_cn_val[channel_offset - 1] = flags; /* saves written value */
    > + outb(flags, usp->usp_iobase + channel_offset); /* see comment above */
    > + outb(0, usp->usp_iobase + 0); /* see above */
    > + usp->usp_prev_cn_val[channel_offset - 1] = flags; /* see above */

    No need to put the "see above".

    thanks,

    greg k-h


    \
     
     \ /
      Last update: 2011-01-10 07:41    [W:0.034 / U:30.904 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site