diff mbox

[02/10] mtd: add a helper to check the SLC/MLC nand chip

Message ID 1376286173-12581-3-git-send-email-b32955@freescale.com
State Accepted
Commit 1d0ed69ddd714b6e2a974f42896463366923ded6
Headers show

Commit Message

Huang Shijie Aug. 12, 2013, 5:42 a.m. UTC
Add a helper to check if a nand chip is SLC or MLC.
This helper makes the code more readable.

Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 drivers/mtd/nand/denali.c    |    2 +-
 drivers/mtd/nand/nand_base.c |   14 ++++++--------
 include/linux/mtd/nand.h     |    9 +++++++++
 3 files changed, 16 insertions(+), 9 deletions(-)

Comments

Brian Norris Aug. 13, 2013, 12:52 a.m. UTC | #1
On Mon, Aug 12, 2013 at 01:42:45PM +0800, Huang Shijie wrote:
> Add a helper to check if a nand chip is SLC or MLC.
> This helper makes the code more readable.
> 
> Signed-off-by: Huang Shijie <b32955@freescale.com>
> ---
>  drivers/mtd/nand/denali.c    |    2 +-
>  drivers/mtd/nand/nand_base.c |   14 ++++++--------
>  include/linux/mtd/nand.h     |    9 +++++++++
>  3 files changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
> index 2ed2bb3..645721e 100644
> --- a/drivers/mtd/nand/denali.c
> +++ b/drivers/mtd/nand/denali.c
> @@ -1520,7 +1520,7 @@ int denali_init(struct denali_nand_info *denali)
>  	 * so just let controller do 15bit ECC for MLC and 8bit ECC for
>  	 * SLC if possible.
>  	 * */
> -	if (denali->nand.cellinfo & NAND_CI_CELLTYPE_MSK &&
> +	if (!nand_is_slc(&denali->nand) &&

According to my recommendations in patch 1, this would only need to be:

  if (denali->nand.bits_per_cell > 1 && ...)

IMO, that is plenty readable, then we don't need the helper.

>  			(denali->mtd.oobsize > (denali->bbtskipbytes +
>  			ECC_15BITS * (denali->mtd.writesize /
>  			ECC_SECTOR_SIZE)))) {
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index ee1aa52..fd5117d 100644

(ditto elsewhere)

Thanks,
Brian
Huang Shijie Aug. 13, 2013, 2:35 a.m. UTC | #2
于 2013年08月13日 08:52, Brian Norris 写道:
>> -	if (denali->nand.cellinfo&  NAND_CI_CELLTYPE_MSK&&
>> >  +	if (!nand_is_slc(&denali->nand)&&
> According to my recommendations in patch 1, this would only need to be:
>
>    if (denali->nand.bits_per_cell>  1&&  ...)
>
> IMO, that is plenty readable, then we don't need the helper.
>

The drivers may also check the SLC/MLC.

IMO, using a helper is more readable :)

Huang Shijie
Brian Norris Aug. 13, 2013, 2:52 a.m. UTC | #3
On Mon, Aug 12, 2013 at 7:35 PM, Huang Shijie <b32955@freescale.com> wrote:
> 于 2013年08月13日 08:52, Brian Norris 写道:
>>>
>>> -       if (denali->nand.cellinfo&  NAND_CI_CELLTYPE_MSK&&
>>> >  +    if (!nand_is_slc(&denali->nand)&&
>>
>> According to my recommendations in patch 1, this would only need to be:
>>
>>    if (denali->nand.bits_per_cell>  1&&  ...)
>>
>>
>> IMO, that is plenty readable, then we don't need the helper.
>
> The drivers may also check the SLC/MLC.
>
> IMO, using a helper is more readable :)

OK, the helper is still fine. I just thought that bits_per_cell has
direct meaning in itself (whereas "cellinfo & NAND_CI_CELLTYPE_MSK"
doesn't).

Brian
diff mbox

Patch

diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index 2ed2bb3..645721e 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -1520,7 +1520,7 @@  int denali_init(struct denali_nand_info *denali)
 	 * so just let controller do 15bit ECC for MLC and 8bit ECC for
 	 * SLC if possible.
 	 * */
-	if (denali->nand.cellinfo & NAND_CI_CELLTYPE_MSK &&
+	if (!nand_is_slc(&denali->nand) &&
 			(denali->mtd.oobsize > (denali->bbtskipbytes +
 			ECC_15BITS * (denali->mtd.writesize /
 			ECC_SECTOR_SIZE)))) {
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index ee1aa52..fd5117d 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3105,8 +3105,7 @@  static void nand_decode_ext_id(struct mtd_info *mtd, struct nand_chip *chip,
 	 * ID to decide what to do.
 	 */
 	if (id_len == 6 && id_data[0] == NAND_MFR_SAMSUNG &&
-			(chip->cellinfo & NAND_CI_CELLTYPE_MSK) &&
-			id_data[5] != 0x00) {
+			!nand_is_slc(chip) && id_data[5] != 0x00) {
 		/* Calc pagesize */
 		mtd->writesize = 2048 << (extid & 0x03);
 		extid >>= 2;
@@ -3138,7 +3137,7 @@  static void nand_decode_ext_id(struct mtd_info *mtd, struct nand_chip *chip,
 			(((extid >> 1) & 0x04) | (extid & 0x03));
 		*busw = 0;
 	} else if (id_len == 6 && id_data[0] == NAND_MFR_HYNIX &&
-			(chip->cellinfo & NAND_CI_CELLTYPE_MSK)) {
+			!nand_is_slc(chip)) {
 		unsigned int tmp;
 
 		/* Calc pagesize */
@@ -3201,7 +3200,7 @@  static void nand_decode_ext_id(struct mtd_info *mtd, struct nand_chip *chip,
 		 * - ID byte 5, bit[7]:    1 -> BENAND, 0 -> raw SLC
 		 */
 		if (id_len >= 6 && id_data[0] == NAND_MFR_TOSHIBA &&
-				!(chip->cellinfo & NAND_CI_CELLTYPE_MSK) &&
+				nand_is_slc(chip) &&
 				(id_data[5] & 0x7) == 0x6 /* 24nm */ &&
 				!(id_data[4] & 0x80) /* !BENAND */) {
 			mtd->oobsize = 32 * mtd->writesize >> 9;
@@ -3262,11 +3261,11 @@  static void nand_decode_bbm_options(struct mtd_info *mtd,
 	 * Micron devices with 2KiB pages and on SLC Samsung, Hynix, Toshiba,
 	 * AMD/Spansion, and Macronix.  All others scan only the first page.
 	 */
-	if ((chip->cellinfo & NAND_CI_CELLTYPE_MSK) &&
+	if (!nand_is_slc(chip) &&
 			(maf_id == NAND_MFR_SAMSUNG ||
 			 maf_id == NAND_MFR_HYNIX))
 		chip->bbt_options |= NAND_BBT_SCANLASTPAGE;
-	else if ((!(chip->cellinfo & NAND_CI_CELLTYPE_MSK) &&
+	else if ((nand_is_slc(chip) &&
 				(maf_id == NAND_MFR_SAMSUNG ||
 				 maf_id == NAND_MFR_HYNIX ||
 				 maf_id == NAND_MFR_TOSHIBA ||
@@ -3742,8 +3741,7 @@  int nand_scan_tail(struct mtd_info *mtd)
 	chip->ecc.total = chip->ecc.steps * chip->ecc.bytes;
 
 	/* Allow subpage writes up to ecc.steps. Not possible for MLC flash */
-	if (!(chip->options & NAND_NO_SUBPAGE_WRITE) &&
-	    !(chip->cellinfo & NAND_CI_CELLTYPE_MSK)) {
+	if (!(chip->options & NAND_NO_SUBPAGE_WRITE) && nand_is_slc(chip)) {
 		switch (chip->ecc.steps) {
 		case 2:
 			mtd->subpage_sft = 1;
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index bf743ba..3c5aae6 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -798,4 +798,13 @@  static inline int onfi_get_sync_timing_mode(struct nand_chip *chip)
 	return le16_to_cpu(chip->onfi_params.src_sync_timing_mode);
 }
 
+/*
+ * Check if it is a SLC nand.
+ * The !nand_is_slc() can be used to check the MLC/TLC nand chips.
+ * We do not distinguish the MLC and TLC now.
+ */
+static inline bool nand_is_slc(struct nand_chip *chip)
+{
+	return !(chip->cellinfo & NAND_CI_CELLTYPE_MSK);
+}
 #endif /* __LINUX_MTD_NAND_H */