[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20110110063909.GC3016@suse.de>
Date: Sun, 9 Jan 2011 22:39:09 -0800
From: Greg KH <gregkh@...e.de>
To: ahern.michael.t@...il.com
Cc: u.kleine-koenig@...gutronix.de, julia@...u.dk, nikai@...ai.net,
morgan.gatti@...il.com, devel@...verdev.osuosl.org,
linux-kernel@...r.kernel.org
Subject: Re: [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 <ahern.michael.t@...il.com>
On Mon, Jan 10, 2011 at 10:03:14AM +1100, ahern.michael.t@...il.com wrote:
> From: mah <ahern.michael.t@...il.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
--
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/
Powered by blists - more mailing lists