From: Huang Shijie <b32955@freescale.com>
To: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Cc: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>,
linux-mtd@lists.infradead.org,
David Woodhouse <David.Woodhouse@intel.com>
Subject: Re: [PATCH] mtd/nand: don't use {read,write}_buf for 8-bit transfers
Date: Thu, 28 Feb 2013 18:33:46 +0800 [thread overview]
Message-ID: <512F328A.3070403@freescale.com> (raw)
In-Reply-To: <1361977852-18233-1-git-send-email-u.kleine-koenig@pengutronix.de>
于 2013年02月27日 23:10, Uwe Kleine-König 写道:
> According to the Open NAND Flash Interface Specification (ONFI) Revision
> 3.1 "Parameters are always transferred on the lower 8-bits of the data
> bus." for the Get Features and Set Features commands.
>
> So using read_buf and write_buf is wrong for 16-bit wide nand chips as
> they use I/O[15:0]. The Get Features command is easily fixed using 4
> times the read_byte callback. For Set Features error out as there is no
> write_byte callback.
>
> Signed-off-by: Uwe Kleine-König<u.kleine-koenig@pengutronix.de>
> ---
> Hello,
>
> note this is only compile tested and I don't have a 16-bit wide nand, so I
> don't even saw a failure.
>
> The problem exists since commit
>
> 7db03ec (mtd: add helpers to set/get features for ONFI nand)
>
> which introduced the two functions.
>
> Still I'd like to know how I can write a byte (or a sequence of bytes) as this
> is necessary for locking the otp are of micron chips. Well, I could implement
> it for 8-bit chips only, but this isn't very satisfying.
>
> Do we need to add a write_byte callback to struct nand_chip? Or is there
> a way to do write a byte that I'm just too blind to see?
>
> Is this patch relevant for stable? Probably not!?
>
> Best regards
> Uwe
>
> drivers/mtd/nand/nand_base.c | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index 4321415..abfd8ca 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -2706,7 +2706,7 @@ static int nand_block_markbad(struct mtd_info *mtd, loff_t ofs)
> }
>
> /**
> - * nand_onfi_set_features- [REPLACEABLE] set features for ONFI nand
> + * nand_onfi_set_features - [REPLACEABLE] set features for ONFI nand
> * @mtd: MTD device structure
> * @chip: nand chip info structure
> * @addr: feature address.
> @@ -2720,6 +2720,11 @@ static int nand_onfi_set_features(struct mtd_info *mtd, struct nand_chip *chip,
> if (!chip->onfi_version)
> return -EINVAL;
>
> + if (chip->options& NAND_BUSWIDTH_16) {
> + pr_warn("onfi set feature command buggy for 16-bit chips\n");
> + return -ENOTSUPP;
> + }
> +
> chip->cmdfunc(mtd, NAND_CMD_SET_FEATURES, addr, -1);
> chip->write_buf(mtd, subfeature_param, ONFI_SUBFEATURE_PARAM_LEN);
> status = chip->waitfunc(mtd, chip);
> @@ -2729,7 +2734,7 @@ static int nand_onfi_set_features(struct mtd_info *mtd, struct nand_chip *chip,
> }
>
> /**
> - * nand_onfi_get_features- [REPLACEABLE] get features for ONFI nand
> + * nand_onfi_get_features - [REPLACEABLE] get features for ONFI nand
> * @mtd: MTD device structure
> * @chip: nand chip info structure
> * @addr: feature address.
> @@ -2738,6 +2743,8 @@ static int nand_onfi_set_features(struct mtd_info *mtd, struct nand_chip *chip,
> static int nand_onfi_get_features(struct mtd_info *mtd, struct nand_chip *chip,
> int addr, uint8_t *subfeature_param)
> {
> + int i;
> +
> if (!chip->onfi_version)
> return -EINVAL;
>
> @@ -2745,7 +2752,8 @@ static int nand_onfi_get_features(struct mtd_info *mtd, struct nand_chip *chip,
> memset(subfeature_param, 0, ONFI_SUBFEATURE_PARAM_LEN);
>
> chip->cmdfunc(mtd, NAND_CMD_GET_FEATURES, addr, -1);
> - chip->read_buf(mtd, subfeature_param, ONFI_SUBFEATURE_PARAM_LEN);
> + for (i = 0; i< ONFI_SUBFEATURE_PARAM_LEN; ++i)
> + *subfeature_param++ = chip->read_byte(mtd);
> return 0;
> }
>
Acked-by: Huang Shijie <b32955@freescale.com>
prev parent reply other threads:[~2013-02-28 10:34 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-02-27 15:10 [PATCH] mtd/nand: don't use {read,write}_buf for 8-bit transfers Uwe Kleine-König
2013-02-28 2:47 ` Huang Shijie
2013-02-28 9:30 ` Uwe Kleine-König
2013-02-28 10:48 ` Uwe Kleine-König
2013-03-01 3:34 ` Huang Shijie
2013-03-01 8:50 ` Uwe Kleine-König
2013-03-01 8:59 ` Huang Shijie
2013-03-01 9:20 ` Matthieu CASTET
2013-03-01 9:59 ` Uwe Kleine-König
2013-03-01 14:00 ` Matthieu CASTET
2013-02-28 10:33 ` Huang Shijie [this message]
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=512F328A.3070403@freescale.com \
--to=b32955@freescale.com \
--cc=David.Woodhouse@intel.com \
--cc=artem.bityutskiy@linux.intel.com \
--cc=linux-mtd@lists.infradead.org \
--cc=u.kleine-koenig@pengutronix.de \
/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.