LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Chris Ball <cjb@laptop.org>
To: "Dong, Chuanxiao" <chuanxiao.dong@intel.com>
Cc: "linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"akpm@linux-foundation.org" <akpm@linux-foundation.org>
Subject: Re: [PATCH v3 1/1]mmc: implemented eMMC4.4 enhanced area feature
Date: Fri, 21 Jan 2011 15:27:50 +0000	[thread overview]
Message-ID: <20110121152750.GA32551@void.printf.net> (raw)
In-Reply-To: <5D8008F58939784290FAB48F549751983539F50558@shsmsx502.ccr.corp.intel.com>

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@laptop.org>   <http://printf.net/>
One Laptop Per Child

  reply	other threads:[~2011-01-21 15:27 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-01-20  7:55 Chuanxiao Dong
2011-01-20 16:12 ` Chris Ball
2011-01-21  3:06   ` Dong, Chuanxiao
2011-01-21  8:04     ` Chris Ball
2011-01-21  9:12       ` Dong, Chuanxiao
2011-01-21 15:27         ` Chris Ball [this message]
2011-01-21 17:43           ` Dong, Chuanxiao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110121152750.GA32551@void.printf.net \
    --to=cjb@laptop.org \
    --cc=akpm@linux-foundation.org \
    --cc=chuanxiao.dong@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mmc@vger.kernel.org \
    --subject='Re: [PATCH v3 1/1]mmc: implemented eMMC4.4 enhanced area feature' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).