On Fri, 2013-06-07 at 00:11 +0300, Dan Carpenter wrote: > On Thu, Jun 06, 2013 at 10:53:20AM -0700, Joe Perches wrote: > > On Thu, 2013-06-06 at 18:10 +0200, Johannes Schilling wrote: > > > From: Laura Lawniczak <laura.lawniczak@xxxxxxxxxxxxxx> > > [] > > > diff --git a/drivers/staging/keucr/init.c b/drivers/staging/keucr/init.c > > [] > > > @@ -19,13 +19,13 @@ int ENE_InitMedia(struct us_data *us) > > [] > > > - printk(KERN_INFO "MiscReg03 = %x\n", MiscReg03); > > > + dev_info(&us->pusb_dev->dev, "MiscReg03 = %x\n", MiscReg03); > > > > I suggest adding a few convenience macros to make > > this a bit shorter and more likely to fit on a single > > line without exceeding 80 cols. > > > > Adding something like: > > > > #define us_<level>(us, fmt, ...) \ > > dev_<level>(&us->pusb_dev->dev, fmt, ##__VA_ARGS__) > > > > so these uses become > > > > us_info(us, "MiscReg03\n", MiscReg03); > > > > which is shorter and more readable. > > > > Wait what? That's not a readable example. Why are we passing the > MiscReg03 variable when there is no %d in the string? 'cause I forgot to add in the " = %x", Oops. > You're a big fan of custom printk macros but it's better if people > can stick the to the normal ones if possible. In this case, just > using a local variable is enough: > > dev_dbg(us_dev, "scsi cmd %X --- SCSIOP_ALLOW_MEDIUM_REMOVAL\n", cmd); > > 79 characters. <shrug> Now you need another line to set the local var in each function. If there's only a few functions that'd be fine. > Anyway this could fixed in a later patch. True, but if there are a lot of files patched separately, putting the macro in an appropriate header first saves a lot of follow up patches. _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
