Hi Chuanxiao, On Fri, Jan 21, 2011 at 05:12:00PM +0800, Dong, Chuanxiao wrote: > I have used the checkpatch.pl to check this patch, and no error or warning found.... Oh, you're right! I wonder if that's because it's in a .txt file rather than a source file. Not sure how to get it to warn about this, then. > > Ah, I wasn't clear -- I meant the bytes/kilobytes units, not the data type > > sizes. e.g.: > > > > + size_t enhanced_area_size; /* Units: KB */ > I think the units of _offset and _size should be easy for user to understand, so just choose the Byte for _offset and Kbyte for _size... > And as spec said, the _size is (ENH_SIZE_MULT_2 << 16 + ENH_SIZE_MULT_1 << 8 + ENH_SIZE_MULT_0) x HC_WP_GRP_SIZE x HC_ERASE_GPR_SIZE x 512kBytes. It must be Kbytes aligned. That is another reason to choose it. I agree that we we should use these units, I'm just arguing that we might as well mention them in the comment area. The other card.h definitions mention their unit sizes too. At the moment your comments for card.h simply repeat the variable name, which isn't useful -- why not make them repeat some useful and not-necessarily-obvious information instead? Thanks, -- Chris Ball <cjb@xxxxxxxxxx> <http://printf.net/> One Laptop Per Child -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html