Staging: unisys: base drivers complete

Dan Carpenter dan.carpenter at oracle.com
Mon Sep 15 18:02:05 UTC 2014


On Mon, Sep 15, 2014 at 12:14:49PM -0500, Romer, Benjamin M wrote:
> > Have you actually ran the checkpatch.pl tool on this code?  You still
> > have a lot of cleanup to do (hint, typedefs for drivers are not
> > allowed...)
> 
> Yes, I've been using checkpatch.pl a lot, though admittedly I did not
> know about --strict. I'll start addressing the check issues as well as
> the warnings and errors. 
> 
> About the "WARNING: do not add new typedefs" messages that are
> generated, I have a question about what typedefs are permitted. It's
> easy enough to replace "typedef enum {...} x;" with "enum x {...}", but
> there are a lot of typedef struct declarations throughout the s-Par
> driver code used to give clearer names to internally-used structures. I
> don't know of any kernel-defined types being renamed anymore, and when I
> did a grep of the driver tree to see what other drivers were doing in
> this area, I found lots of drivers with "typedef struct" in them.
> 
> Are we restricted from doing *any* typedefs at all?

Pretty much.

> If not, could you give me a good guideline to follow?

http://yarchive.net/comp/linux/typedefs.html

This one is probably ok, except the name is crap:
typedef u64 GUEST_PHYSICAL_ADDRESS;
I hate that the names are all caps.
Also We might have a standard type for this?  I forget.

For structs, just use struct name throughout.  I do understand that you
want many of them to be opaque, but really it's a struct.  Just do a
forward declaration in the .h file and leave the implementation in the
.c file as you have it.

This file is really gross.
drivers/staging/unisys/common-spar/include/vmcallinterface.h
The comments and the pragmas.

This one is misleading.  Normally in the kernel bool is a C _Bool type.
#define BOOL int

I don't think we actually use the SPARSTOP_COMPLETE_FUNC typedef.  I
think function pointer typedefs are worthwhile because they are less
typing but this one can just be deleted obviously.

regards,
dan carpenter



More information about the devel mailing list