On Wed, Oct 24, 2012 at 01:23:40AM -0400, Devendra Naga wrote: > > Signed-off-by: Devendra Naga <devendra.aaru@xxxxxxxxx> You forgot to put some description in the body of the changelog for this patch. Overall, it looks fine, one comment though: > u16 count; /* serial number of dump */ > - CsrTime timestamp; /* host's system time at capture */ > - s16 requestor; /* request: 0=auto dump, 1=manual */ > + u32 timestamp; /* host's system time at capture */ > + s16 requestor; /* request: 0=auto dump, 1=manual */ That requestor line doesn't need to be changed here, right? Care to leave that alone? > u16 chip_ver; > u32 fw_ver; > - u16 *zone[HIP_CDUMP_NUM_ZONES]; > + u16 *zone[HIP_CDUMP_NUM_ZONES]; Same here. I know it's tempting to fix up other things you find wrong, but please, only do one thing per patch, it makes it easier to review. thanks, greg k-h _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
