public inbox for linux-mtd@lists.infradead.org
 help / color / mirror / Atom feed
From: Janusz Krzysztofik <jmkrzyszt@gmail.com>
To: Boris Brezillon <boris.brezillon@bootlin.com>
Cc: Richard Weinberger <richard@nod.at>,
	Miquel Raynal <miquel.raynal@bootlin.com>,
	linux-mtd@lists.infradead.org,
	David Woodhouse <dwmw2@infradead.org>,
	Brian Norris <computersforpeace@gmail.com>,
	Marek Vasut <marek.vasut@gmail.com>, Han Xu <han.xu@nxp.com>,
	Masahiro Yamada <yamada.masahiro@socionext.com>,
	Tudor Ambarus <tudor.ambarus@microchip.com>,
	Harvey Hunt <harveyhuntnexus@gmail.com>,
	Xiaolei Li <xiaolei.li@mediatek.com>,
	Maxim Levitsky <maximlevitsky@gmail.com>,
	Marc Gonzalez <marc.w.gonzalez@free.fr>,
	Stefan Agner <stefan@agner.ch>,
	Janusz Krzysztofik <jmkrzyszt@gmail.com>
Subject: Re: [PATCH v2 13/22] mtd: rawnand: Pass the CS line to be selected in struct nand_operation
Date: Sat, 10 Nov 2018 02:30:04 +0100	[thread overview]
Message-ID: <2312482.kRjraIG7a0@z50> (raw)
In-Reply-To: <20181106150810.9569-14-boris.brezillon@bootlin.com>

Hi Boris,

On Tuesday, November 6, 2018 4:08:01 PM CET Boris Brezillon wrote:
> In order to deprecate the ->select_chip hook we need to pass the CS
> line a NAND operations are targeting. This is done through the
> addition of a cs field to the nand_operation struct.
> 
> We also need to keep track of the currently selected target to
> properly initialize op->cs, hence the ->cur_cs field addition to the
> nand_chip struct.
> 
> Note that op->cs is not assigned in nand_exec_op() because we might
> rework the way we execute NAND operations in the future (adopt a
> queuing mechanism instead of the serialization we have right now).
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
> ---
> Changes in v2:
> - Initialize ->cur_cs
> ---
>  drivers/mtd/nand/raw/internals.h  |  3 +++
>  drivers/mtd/nand/raw/nand_base.c  | 39 +++++++++++++++++--------------
>  drivers/mtd/nand/raw/nand_hynix.c |  4 ++--
>  include/linux/mtd/rawnand.h       | 11 ++++++++-
>  4 files changed, 37 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/internals.h b/drivers/mtd/nand/raw/internals.h
> index 6e2f61fbc5f0..b62728d5884b 100644
> --- a/drivers/mtd/nand/raw/internals.h
> +++ b/drivers/mtd/nand/raw/internals.h
> @@ -101,6 +101,9 @@ static inline int nand_exec_op(struct nand_chip *chip,
>  	if (!chip->exec_op)
>  		return -ENOTSUPP;
>  
> +	if (WARN_ON(op->cs >= chip->numchips))
> +		return -EINVAL;

This needs a fix to nand_scan_ident() like the following:

--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -5034,10 +5034,11 @@ static int nand_scan_ident(struct nand_chip *chip, unsigned int maxchips,
 
 	/* Set the default functions */
 	nand_set_defaults(chip);
 
 	/* Read the flash type */
+	chip->numchips = 1;
 	ret = nand_detect(chip, table);
 	if (ret) {
 		if (!(chip->options & NAND_SCAN_SILENT_NODEV))
 			pr_warn("No NAND device found\n");
 		nand_deselect_target(chip);


Otherwise nand_detect() fails for me in nand_exec_op() called from the very 
first nand_reset_op():

    WARNING: CPU: 0 PID: 1 at drivers/mtd/nand/raw/internals.h:104 nand_reset_op+0x100/0x17c
    ...
    nand: No NAND device found

With that issue fixed one way or another, you can add my:

	Tested-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>

to your whole series.

Thanks,
Janusz


PS.
I'm ready to submit a series which converts ams-delta-nand to use GPIO 
API for data I/O.  My only concerns is which branch should I rebase it on.

  reply	other threads:[~2018-11-10  1:28 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-06 15:07 [PATCH v2 00/22] mtd: rawnand: 3rd batch of cleanup Boris Brezillon
2018-11-06 15:07 ` [PATCH v2 01/22] mtd: rawnand: Stop passing mtd_info objects to internal functions Boris Brezillon
2018-11-06 15:07 ` [PATCH v2 02/22] mtd: rawnand: Reorganize code to avoid forward declarations Boris Brezillon
2018-11-06 15:07 ` [PATCH v2 03/22] mtd: rawnand: legacy: Drop useless test in nand_legacy_set_defaults() Boris Brezillon
2018-11-06 15:07 ` [PATCH v2 04/22] mtd: rawnand: Move nand_exec_op() to internal.h Boris Brezillon
2018-11-06 15:07 ` [PATCH v2 05/22] mtd: rawnand: Remove unused NAND_CONTROLLER_ALLOC flag Boris Brezillon
2018-11-06 15:07 ` [PATCH v2 06/22] mtd: rawnand: ams-delta: Allow this driver to be compiled when COMPILE_TEST=y Boris Brezillon
2018-11-06 16:56   ` Boris Brezillon
2018-11-06 15:07 ` [PATCH v2 07/22] mtd: rawnand: ams-delta: Add an SPDX tag to replace the license text Boris Brezillon
2018-11-06 15:07 ` [PATCH v2 08/22] mtd: rawnand: ams-delta: Fix various coding style issues Boris Brezillon
2018-11-06 15:07 ` [PATCH v2 09/22] mtd: rawnand: ams-delta: cleanup ams_delta_init() error path Boris Brezillon
2018-11-06 15:07 ` [PATCH v2 10/22] mtd: rawnand: ams-delta: Check mtd_device_register() return code Boris Brezillon
2018-11-06 15:07 ` [PATCH v2 11/22] mtd: rawnand: ams-delta: Explicitly inherit from nand_controller Boris Brezillon
2018-11-06 15:08 ` [PATCH v2 12/22] mtd: rawnand: Add nand_[de]select_target() helpers Boris Brezillon
2018-11-06 15:08 ` [PATCH v2 13/22] mtd: rawnand: Pass the CS line to be selected in struct nand_operation Boris Brezillon
2018-11-10  1:30   ` Janusz Krzysztofik [this message]
2018-11-10  8:04     ` Boris Brezillon
2018-11-10  9:33       ` Janusz Krzysztofik
2018-11-06 15:08 ` [PATCH v2 14/22] mtd: rawnand: Make ->select_chip() optional when ->exec_op() is implemented Boris Brezillon
2018-11-06 15:08 ` [PATCH v2 15/22] mtd: rawnand: fsmc: Stop implementing ->select_chip() Boris Brezillon
2018-11-06 15:08 ` [PATCH v2 16/22] mtd: rawnand: marvell: " Boris Brezillon
2018-11-06 15:08 ` [PATCH v2 17/22] mtd: rawnand: tegra: " Boris Brezillon
2018-11-06 15:08 ` [PATCH v2 18/22] mtd: rawnand: vf610: " Boris Brezillon
2018-11-06 15:08 ` [PATCH v2 19/22] mtd: rawnand: ams-delta: " Boris Brezillon
2018-11-06 15:08 ` [PATCH v2 20/22] mtd: rawnand: Deprecate the ->select_chip() hook Boris Brezillon
2018-11-06 15:08 ` [PATCH v2 21/22] mtd: rawnand: Move the ->exec_op() method to nand_controller_ops Boris Brezillon
2018-11-06 15:08 ` [PATCH v2 22/22] mtd: rawnand: Move ->setup_data_interface() " Boris Brezillon

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=2312482.kRjraIG7a0@z50 \
    --to=jmkrzyszt@gmail.com \
    --cc=boris.brezillon@bootlin.com \
    --cc=computersforpeace@gmail.com \
    --cc=dwmw2@infradead.org \
    --cc=han.xu@nxp.com \
    --cc=harveyhuntnexus@gmail.com \
    --cc=linux-mtd@lists.infradead.org \
    --cc=marc.w.gonzalez@free.fr \
    --cc=marek.vasut@gmail.com \
    --cc=maximlevitsky@gmail.com \
    --cc=miquel.raynal@bootlin.com \
    --cc=richard@nod.at \
    --cc=stefan@agner.ch \
    --cc=tudor.ambarus@microchip.com \
    --cc=xiaolei.li@mediatek.com \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox