On Sun, May 18, 2008 at 08:18:42PM +0100, Adrian McMenamin wrote: > +MODULE_AUTHOR("YAEGASHI Takeshi <t@xxxxxxxxx>"); > +MODULE_DESCRIPTION("SEGA Dreamcast controller driver"); > +MODULE_LICENSE("GPL"); > + MODULE_AUTHOR() is a bit ambiguous here. As this is user-visible by way of modinfo, it really doesn't make any sense to list someone who has nothing to do with the driver in its present form here. Especially since it's been 7 years since their last contribution or even basic list activity, I think it's safe to say that you are going to be the only one that users wish to contact about this. In this case, the copyright should suffice. You may wish to add a bit of a note there indicating where the driver came from, but there's not much point in going beyond that. Note that if you aren't comfortable with being in MODULE_AUTHOR(), you can also make this more generic and attribute it to the LinuxDC or Linux/SH teams respectively, as that's a bit more true to form as far as all copyright holders are concerned, and gives people a pointer to a list to go to with problems. It's probably more reasonable to treat MODULE_AUTHOR() as MODULE_MAINTAINER() rather than the literal original author, as the latter is of zero interest or relevance to users that are looking at this information on the other side of the fence. > +struct dc_pad { > + struct input_dev *dev; > + struct maple_device *mdev; > +}; > + This is pretty ugly, you should really fix up your private device pointer layering to get around having to use this structure at all. > +static struct maple_driver dc_pad_driver = { > + .function = MAPLE_FUNC_CONTROLLER, > + .connect = dc_pad_connect, > + .disconnect = dc_pad_disconnect, > + .drv = { > + .name = "Dreamcast_controller", Is there some particular reason why the driver name can't reflect the module name? There's no reason to have such sickeningly verbose driver names. > + .probe = probe_maple_controller, > + .remove = remove_maple_controller, > + }, > +}; > + These should be __devinit/__devexit at least, with the latter being __devexit_p() wrapped.. > +static int __init dc_pad_init(void) > +{ > + return maple_driver_register(&dc_pad_driver.drv); > +} > + > +static void __exit dc_pad_exit(void) > +{ > + driver_unregister(&dc_pad_driver.drv); > +} > + If you don't have a maple_driver_unregister(), stub one in, even if it's just a wrapper. -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html