lkml.org 
[lkml]   [2005]   [Aug]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: Telecom Clock driver for MPCBL0010 ATCA compute blade.
On 8/30/05, Mark Gross <mgross@linux.intel.com> wrote:
> On Tuesday 30 August 2005 13:31, Mark Gross wrote:
> > On Tuesday 30 August 2005 12:16, Marcelo Tosatti wrote:
> > >
> > > Mark,
> > >
> > > Please fix identation accordingly to CodingStyle and repost, it
> > > looks quite ugly at the moment.
> > >
> > Sorry about that.
> >
>
> My email client is f-ing with me. See attached.
>

ok, a few small comments :


+/* sysFS interface definition:

Isn't it just called "sysfs" without the caps?

+Uppon loading the driver will create a sysfs directory under class/misc/tlclk.

s/Uppon/Upon/

+This directory exports the following interfaces. There opperation is
documented

Line is longer than 80 chars - there are a few more such long lines,
not going to point them all out, just one example. Ohh, and
"opperation" should be "operation".

+All sysfs interaces are integers in hex format, i.e echo 99 > refalign

s/interaces/interfaces/

+#if CONFIG_DEBUG_KERNEL

I believe this should be "#ifdef CONFIG_DEBUG_KERNEL" or "#if
defined(CONFIG_DEBUG_KERNEL)" or you'll run afoul of the -Wundef
crowd.

+ int ret_val = 0;

There seems to be a preference for the name "retval" in the kernel source.

+ val = (unsigned char)arg;


That cast looks pointless.

+tlclk_read(struct file * filp, char __user * buf, size_t count, loff_t * f_pos)


tlclk_read(struct file *filp, char __user *buf, size_t count, loff_t *f_pos)
"*" next to the variable name is generally the preffered coding style
(several cases of this, only pointing out one).

+ val = (unsigned char)tmp;

pointless cast. Do take a look at all your casts and check if they are
really needed and remove them if not.

+ * This is also the probe opperation to avoid driver use on

s/opperation/operation/

+/* switchover_timer.function = switchover_timeout; */

+/* switchover_timer.data = 0; */

Why submit a driver with commented out code? If this is not supposed
to be there, then just remove it. If it needs to be added later, then
submit a patch later to add it.
Some people may disagree with me here, but that's my oppinion.

+ out3:


labels belong at column 0 (zero).


--
Jesper Juhl <jesper.juhl@gmail.com>
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-08-30 23:22    [W:0.100 / U:2.936 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site