Re: [PATCH v3 1/1]mmc: implemented eMMC4.4 enhanced area feature

From: Chris Ball
Date: Fri Jan 21 2011 - 10:27:58 EST


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-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/