All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Gupta, Pekon" <pekon@ti.com>
To: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
Cc: Brian Norris <computersforpeace@gmail.com>,
	"linux-omap@vger.kernel.org" <linux-omap@vger.kernel.org>,
	"linux-mtd@lists.infradead.org" <linux-mtd@lists.infradead.org>,
	"Balbi, Felipe" <balbi@ti.com>,
	"marek.belisko@gmail.com" <marek.belisko@gmail.com>
Subject: RE: [PATCH 0/1] Fix OMAP2 NAND ONFI device detection
Date: Wed, 6 Nov 2013 22:00:09 +0000	[thread overview]
Message-ID: <20980858CB6D3A4BAE95CA194937D5E73EA31BB3@DBDE04.ent.ti.com> (raw)
In-Reply-To: <20131106211833.GB12624@localhost>

> From: Ezequiel Garcia
[...]

> But: on the other hand, I'd really like you to convince me as to
> why is it so bad to require the DTB to have the proper GPMC bus width.
> 
No its not at all bad, all I want is either of the one way (not mixture of both).
- Either depend on DT completely (which is current case for all drivers)
- OR depend on ONFI and nand_flash_id[] for bus-width detection.


> Once again:
> 1. the NAND devices aren't hot-pluggable
> 2. the "user" (who is actually an engineer, not some regular dummy user)
> knows perfectly well the width of the device.
> 
> What's the problem with describing the hardware in the DT and saving us
> lots of runtime re-configuration trouble?

I agree with both your arguments above.
So shouldn't we kill NAND_BUSWIDTH_AUTO ?
And probably therefore  NAND_BUSWIDTH_AUTO isn't that popular.

Now what remains is ONFI probe, which should always happen in x8 mode.
So for that below patch should be sufficient ..

----------------------
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index ec1db1e..d1220fb 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2942,14 +2942,8 @@ static int nand_flash_detect_onfi(struct mtd_info *mtd, struct nand_chip *chip,
                chip->read_byte(mtd) != 'F' || chip->read_byte(mtd) != 'I')
                return 0;

-       /*
-        * ONFI must be probed in 8-bit mode or with NAND_BUSWIDTH_AUTO, not
-        * with NAND_BUSWIDTH_16
-        */
-       if (chip->options & NAND_BUSWIDTH_16) {
-               pr_err("ONFI cannot be probed in 16-bit mode; aborting\n");
-               return 0;
-       }
+       /* ONFI must be probed in 8-bit mode only */
+       nand_set_defaults(chip, 0);

        chip->cmdfunc(mtd, NAND_CMD_PARAM, 0, -1);
        for (i = 0; i < 3; i++) {
@@ -2962,7 +2956,7 @@ static int nand_flash_detect_onfi(struct mtd_info *mtd, struct nand_chip *chip,

        if (i == 3) {
                pr_err("Could not find valid ONFI parameter page; aborting\n");
-               return 0;
+               goto return_error;
        }

        /* Check version */
@@ -2980,7 +2974,7 @@ static int nand_flash_detect_onfi(struct mtd_info *mtd, struct nand_chip *chip,

        if (!chip->onfi_version) {
                pr_info("%s: unsupported ONFI version: %d\n", __func__, val);
-               return 0;
+               goto return_error;
        }

        sanitize_string(p->manufacturer, sizeof(p->manufacturer));
@@ -3033,6 +3027,12 @@ static int nand_flash_detect_onfi(struct mtd_info *mtd, struct nand_chip *chip,
        }

        return 1;
+
+return_error:
+       /* revert original bus-width */
+       nand_set_defaults( chip->options & NAND_BUSWIDTH_16);
+       return 0;
+
 }

 /*
------------------------- 


with regards, pekon

WARNING: multiple messages have this Message-ID (diff)
From: "Gupta, Pekon" <pekon@ti.com>
To: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
Cc: "marek.belisko@gmail.com" <marek.belisko@gmail.com>,
	"linux-omap@vger.kernel.org" <linux-omap@vger.kernel.org>,
	Brian Norris <computersforpeace@gmail.com>,
	"linux-mtd@lists.infradead.org" <linux-mtd@lists.infradead.org>,
	"Balbi, Felipe" <balbi@ti.com>
Subject: RE: [PATCH 0/1] Fix OMAP2 NAND ONFI device detection
Date: Wed, 6 Nov 2013 22:00:09 +0000	[thread overview]
Message-ID: <20980858CB6D3A4BAE95CA194937D5E73EA31BB3@DBDE04.ent.ti.com> (raw)
In-Reply-To: <20131106211833.GB12624@localhost>

> From: Ezequiel Garcia
[...]

> But: on the other hand, I'd really like you to convince me as to
> why is it so bad to require the DTB to have the proper GPMC bus width.
> 
No its not at all bad, all I want is either of the one way (not mixture of both).
- Either depend on DT completely (which is current case for all drivers)
- OR depend on ONFI and nand_flash_id[] for bus-width detection.


> Once again:
> 1. the NAND devices aren't hot-pluggable
> 2. the "user" (who is actually an engineer, not some regular dummy user)
> knows perfectly well the width of the device.
> 
> What's the problem with describing the hardware in the DT and saving us
> lots of runtime re-configuration trouble?

I agree with both your arguments above.
So shouldn't we kill NAND_BUSWIDTH_AUTO ?
And probably therefore  NAND_BUSWIDTH_AUTO isn't that popular.

Now what remains is ONFI probe, which should always happen in x8 mode.
So for that below patch should be sufficient ..

----------------------
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index ec1db1e..d1220fb 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2942,14 +2942,8 @@ static int nand_flash_detect_onfi(struct mtd_info *mtd, struct nand_chip *chip,
                chip->read_byte(mtd) != 'F' || chip->read_byte(mtd) != 'I')
                return 0;

-       /*
-        * ONFI must be probed in 8-bit mode or with NAND_BUSWIDTH_AUTO, not
-        * with NAND_BUSWIDTH_16
-        */
-       if (chip->options & NAND_BUSWIDTH_16) {
-               pr_err("ONFI cannot be probed in 16-bit mode; aborting\n");
-               return 0;
-       }
+       /* ONFI must be probed in 8-bit mode only */
+       nand_set_defaults(chip, 0);

        chip->cmdfunc(mtd, NAND_CMD_PARAM, 0, -1);
        for (i = 0; i < 3; i++) {
@@ -2962,7 +2956,7 @@ static int nand_flash_detect_onfi(struct mtd_info *mtd, struct nand_chip *chip,

        if (i == 3) {
                pr_err("Could not find valid ONFI parameter page; aborting\n");
-               return 0;
+               goto return_error;
        }

        /* Check version */
@@ -2980,7 +2974,7 @@ static int nand_flash_detect_onfi(struct mtd_info *mtd, struct nand_chip *chip,

        if (!chip->onfi_version) {
                pr_info("%s: unsupported ONFI version: %d\n", __func__, val);
-               return 0;
+               goto return_error;
        }

        sanitize_string(p->manufacturer, sizeof(p->manufacturer));
@@ -3033,6 +3027,12 @@ static int nand_flash_detect_onfi(struct mtd_info *mtd, struct nand_chip *chip,
        }

        return 1;
+
+return_error:
+       /* revert original bus-width */
+       nand_set_defaults( chip->options & NAND_BUSWIDTH_16);
+       return 0;
+
 }

 /*
------------------------- 


with regards, pekon

  reply	other threads:[~2013-11-06 22:00 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-30  9:53 [PATCH 0/1] Fix OMAP2 NAND ONFI device detection Ezequiel Garcia
2013-10-30  9:53 ` Ezequiel Garcia
2013-10-30  9:53 ` [PATCH 1/1] mtd: nand: omap2: Fix device detection path Ezequiel Garcia
2013-10-30  9:53   ` Ezequiel Garcia
2013-10-30 19:14 ` [PATCH 0/1] Fix OMAP2 NAND ONFI device detection Brian Norris
2013-10-30 19:14   ` Brian Norris
2013-10-30 21:18   ` Gupta, Pekon
2013-10-30 21:18     ` Gupta, Pekon
2013-10-30 23:19     ` Ezequiel Garcia
2013-10-30 23:19       ` Ezequiel Garcia
2013-11-06 20:54       ` Gupta, Pekon
2013-11-06 20:54         ` Gupta, Pekon
2013-11-06 21:18         ` Ezequiel Garcia
2013-11-06 21:18           ` Ezequiel Garcia
2013-11-06 22:00           ` Gupta, Pekon [this message]
2013-11-06 22:00             ` Gupta, Pekon
2013-11-06 22:38             ` Ezequiel Garcia
2013-11-06 22:38               ` Ezequiel Garcia
2013-11-30  8:56       ` Brian Norris
2013-11-30  8:56         ` Brian Norris

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=20980858CB6D3A4BAE95CA194937D5E73EA31BB3@DBDE04.ent.ti.com \
    --to=pekon@ti.com \
    --cc=balbi@ti.com \
    --cc=computersforpeace@gmail.com \
    --cc=ezequiel.garcia@free-electrons.com \
    --cc=linux-mtd@lists.infradead.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=marek.belisko@gmail.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.