From: Boris Brezillon <bbrezillon@kernel.org>
To: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Boris Brezillon <boris.brezillon@bootlin.com>,
Richard Weinberger <richard@nod.at>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Marek Vasut <marek.vasut@gmail.com>,
linux-mtd <linux-mtd@lists.infradead.org>,
Miquel Raynal <miquel.raynal@bootlin.com>,
Brian Norris <computersforpeace@gmail.com>,
David Woodhouse <dwmw2@infradead.org>
Subject: Re: [PATCH] mtd: rawnand: check return code of nand_reset() and nand_readid_op()
Date: Mon, 21 Jan 2019 19:53:54 +0100 [thread overview]
Message-ID: <20190121195354.788199f5@bbrezillon> (raw)
In-Reply-To: <CAK7LNATUpyfnicBiSdL49dsFnvo+M=N9X-wY-J0efnRc-1_0gA@mail.gmail.com>
On Tue, 22 Jan 2019 00:57:43 +0900
Masahiro Yamada <yamada.masahiro@socionext.com> wrote:
>
> Maybe, is the following better?
Sounds good, even if the original commit message was fine too. I was
just pointing out that nand_scan() should, when possible, be passed
the real number of CS lines connected to the chip instead of the max
number of CS lines supported by the controller.
>
>
> ------------------>8-----------------------
> nand_scan_ident() iterates over maxchips to find as many homogeneous
> chips as possible.
>
> Since commit 2d472aba15ff ("mtd: nand: document the NAND controller/NAND
> chip DT representation"), new drivers should pass in the exact number of
> CS lines instead of possible max, but old platforms may still rely on
> nand_scan_ident() to detect the actual number of connected CS lines.
>
> In that case, this loop bails out when manufacturer or device ID
> unmatches. The reason of unmatch is most likely no chip is connected
> to that CS line. If so, nand_reset() should already have failed,
> and the following nand_readid_op() is pointless.
>
> Before ->exec_op hook was introduced, drivers had no way to tell
> the failure of NAND_CMD_RESET to the framework because the legacy
> ->cmdfunc() has void return type. Now drivers implementing ->exec_op
> hook can return the error code. You can save nand_readid_op() by
> checking the return value of nand_reset(). The return value of
> nand_readid_op() should be checked as well. If it fails, probably
> id[0] and id[1] are undefined values.
>
> Just for consistency, it should be sensible to check the return
> code in nand_do_write_oob() as well.
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
WARNING: multiple messages have this Message-ID (diff)
From: Boris Brezillon <bbrezillon@kernel.org>
To: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Marek Vasut <marek.vasut@gmail.com>,
Richard Weinberger <richard@nod.at>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Boris Brezillon <boris.brezillon@bootlin.com>,
linux-mtd <linux-mtd@lists.infradead.org>,
Miquel Raynal <miquel.raynal@bootlin.com>,
Brian Norris <computersforpeace@gmail.com>,
David Woodhouse <dwmw2@infradead.org>
Subject: Re: [PATCH] mtd: rawnand: check return code of nand_reset() and nand_readid_op()
Date: Mon, 21 Jan 2019 19:53:54 +0100 [thread overview]
Message-ID: <20190121195354.788199f5@bbrezillon> (raw)
In-Reply-To: <CAK7LNATUpyfnicBiSdL49dsFnvo+M=N9X-wY-J0efnRc-1_0gA@mail.gmail.com>
On Tue, 22 Jan 2019 00:57:43 +0900
Masahiro Yamada <yamada.masahiro@socionext.com> wrote:
>
> Maybe, is the following better?
Sounds good, even if the original commit message was fine too. I was
just pointing out that nand_scan() should, when possible, be passed
the real number of CS lines connected to the chip instead of the max
number of CS lines supported by the controller.
>
>
> ------------------>8-----------------------
> nand_scan_ident() iterates over maxchips to find as many homogeneous
> chips as possible.
>
> Since commit 2d472aba15ff ("mtd: nand: document the NAND controller/NAND
> chip DT representation"), new drivers should pass in the exact number of
> CS lines instead of possible max, but old platforms may still rely on
> nand_scan_ident() to detect the actual number of connected CS lines.
>
> In that case, this loop bails out when manufacturer or device ID
> unmatches. The reason of unmatch is most likely no chip is connected
> to that CS line. If so, nand_reset() should already have failed,
> and the following nand_readid_op() is pointless.
>
> Before ->exec_op hook was introduced, drivers had no way to tell
> the failure of NAND_CMD_RESET to the framework because the legacy
> ->cmdfunc() has void return type. Now drivers implementing ->exec_op
> hook can return the error code. You can save nand_readid_op() by
> checking the return value of nand_reset(). The return value of
> nand_readid_op() should be checked as well. If it fails, probably
> id[0] and id[1] are undefined values.
>
> Just for consistency, it should be sensible to check the return
> code in nand_do_write_oob() as well.
next prev parent reply other threads:[~2019-01-21 18:54 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-01-21 13:05 [PATCH] mtd: rawnand: check return code of nand_reset() and nand_readid_op() Masahiro Yamada
2019-01-21 13:05 ` Masahiro Yamada
2019-01-21 13:14 ` Boris Brezillon
2019-01-21 13:14 ` Boris Brezillon
2019-01-21 15:57 ` Masahiro Yamada
2019-01-21 15:57 ` Masahiro Yamada
2019-01-21 18:53 ` Boris Brezillon [this message]
2019-01-21 18:53 ` Boris Brezillon
2019-01-25 12:38 ` Miquel Raynal
2019-01-25 12:38 ` Miquel Raynal
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=20190121195354.788199f5@bbrezillon \
--to=bbrezillon@kernel.org \
--cc=boris.brezillon@bootlin.com \
--cc=computersforpeace@gmail.com \
--cc=dwmw2@infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mtd@lists.infradead.org \
--cc=marek.vasut@gmail.com \
--cc=miquel.raynal@bootlin.com \
--cc=richard@nod.at \
--cc=yamada.masahiro@socionext.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.