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.
Nope, remove them all, just use the structure names instead please.
> 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.
They might be really old, this rule has been around for quite some time.
> Are we restricted from doing *any* typedefs at all? If not, could you
> give me a good guideline to follow?
You can only use a typedef for a driver if you are typedefing a function
pointer type that is used in a number of places. For example, the USB
core has a urb callback function pointer type:
typedef void (*usb_complete_t)(struct urb *);
That's the only thing that is allowed for drivers.
> > And what bout the TODO file? The first 3 items are not completed. Why
> > even have a TODO file if you aren't going to look at it? :(
>
> I'd like to expand it to include as much as we can, so when people ask
> me "when will our drivers be done?" or "what can I work on?" I have
> something they can read. :) It also helps us fix up the code we haven't
> submitted yet, and if we can get it into shape before you have to see
> it, that saves you a lot of time checking it as well.
Please feel free to add to it, I never object to that.
> I've definitely underestimated the work to do on cleanup because we
> weren't aware of --strict, but I thought we had gotten rid of all of the
> proc entries and moved anything important to sysfs/debugfs. The major
> numbers issue was next.
If you have moved all proc entries, then remove that item in the TODO
list.
And even without --strict, checkpatch points out a lot of issues
remaining.
thanks,
greg k-h
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
