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

[Dong, Chuanxiao]

Hi Chris,

> -----Original Message-----
> From: Chris Ball [mailto:cjb@xxxxxxxxxx]
> Sent: Friday, January 21, 2011 11:28 PM
> To: Dong, Chuanxiao
> Cc: linux-mmc@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> akpm@xxxxxxxxxxxxxxxxxxxx
> Subject: Re: [PATCH v3 1/1]mmc: implemented eMMC4.4 enhanced area feature
> 
> 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?
Sure, Chris. I will take care of such things in my future submissions. Of cause including the next v4 enhanced area feature patch. :) I also remember to fix the compile warnings. Thanks a lot, Chris.

Thanks
Chuanxiao
--
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/