LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Dmitry Osipenko <digetx@gmail.com>
To: Boris Brezillon <boris.brezillon@bootlin.com>
Cc: Stefan Agner <stefan@agner.ch>,
	dwmw2@infradead.org, computersforpeace@gmail.com,
	marek.vasut@gmail.com, robh+dt@kernel.org, mark.rutland@arm.com,
	thierry.reding@gmail.com, dev@lynxeye.de,
	miquel.raynal@bootlin.com, richard@nod.at, marcel@ziswiler.com,
	krzk@kernel.org, benjamin.lindqvist@endian.se,
	jonathanh@nvidia.com, pdeschrijver@nvidia.com,
	pgaikwad@nvidia.com, mirza.krak@gmail.com,
	linux-mtd@lists.infradead.org, linux-tegra@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 4/6] mtd: rawnand: add NVIDIA Tegra NAND Flash controller driver
Date: Mon, 11 Jun 2018 16:10:21 +0300	[thread overview]
Message-ID: <6764263.x6FQro3A0J@dimapc> (raw)
In-Reply-To: <20180611135013.7bb5b59b@bbrezillon>

On Monday, 11 June 2018 14:50:13 MSK Boris Brezillon wrote:
> On Mon, 11 Jun 2018 14:45:45 +0300
> 
> Dmitry Osipenko <digetx@gmail.com> wrote:
> > On Sunday, 10 June 2018 18:32:02 MSK Boris Brezillon wrote:
> > > On Sun, 10 Jun 2018 18:00:06 +0300
> > > 
> > > Dmitry Osipenko <digetx@gmail.com> wrote:
> > > > > >> That seems a lot of work for a code path I do not intend to ever
> > > > > >> use
> > > > > >> 
> > > > > >> :-)
> > > > > > 
> > > > > > Are you sure that resetting HW resets the timing and other
> > > > > > registers
> > > > > > configuration? Reset implementation is HW-specific, like for
> > > > > > example
> > > > > > in a
> > > > > > case of a video decoder the registers state is re-intialized on HW
> > > > > > reset,
> > > > > > but registers configuration is untouched in a case of resetting
> > > > > > GPU.
> > > > > > I'd
> > > > > > suggest to check whether NAND controller resetting affects the HW
> > > > > > configuration.
> > > > > 
> > > > > It seems all registers are set back to their documented reset value:
> > > > > 
> > > > > [boot loader/ROM initialized values]
> > > > > [    1.270253] tegra-nand 70008000.nand: Tegra NAND controller
> > > > > register
> > > > > dump
> > > > > [    1.277051] tegra-nand 70008000.nand: COMMAND: 0x66880104
> > > > > [    1.282457] tegra-nand 70008000.nand: STATUS: 0x00000101
> > > > > [    1.287763] tegra-nand 70008000.nand: ISR: 0x01000120
> > > > > [    1.292818] tegra-nand 70008000.nand: IER: 0x00000000
> > > > > [    1.297863] tegra-nand 70008000.nand: CONFIG: 0x00840000
> > > > > [    1.303181] tegra-nand 70008000.nand: TIMING: 0x05040000
> > > > > [    1.308486] tegra-nand 70008000.nand: TIMING2: 0x00000003
> > > > > [    1.313897] tegra-nand 70008000.nand: CMD_REG1: 0x00000000
> > > > > [    1.319377] tegra-nand 70008000.nand: CMD_REG2: 0x00000030
> > > > > [    1.324868] tegra-nand 70008000.nand: ADDR_REG1: 0x03000000
> > > > > [    1.330435] tegra-nand 70008000.nand: ADDR_REG2: 0x00000000
> > > > > [    1.336011] tegra-nand 70008000.nand: DMA_MST_CTRL: 0x04100004
> > > > > [    1.341838] tegra-nand 70008000.nand: DMA_CFG_A: 0x00000fff
> > > > > [    1.347415] tegra-nand 70008000.nand: DMA_CFG_B: 0x0000001b
> > > > > [    1.352991] tegra-nand 70008000.nand: FIFO_CTRL: 0x0000aa00
> > > > > [reset]
> > > > > [    1.358559] tegra-nand 70008000.nand: Tegra NAND controller
> > > > > register
> > > > > dump
> > > > > [    1.365352] tegra-nand 70008000.nand: COMMAND: 0x00800004
> > > > > [    1.370744] tegra-nand 70008000.nand: STATUS: 0x00000101
> > > > > [    1.376060] tegra-nand 70008000.nand: ISR: 0x00000100
> > > > > [    1.381105] tegra-nand 70008000.nand: IER: 0x00000000
> > > > > [    1.386161] tegra-nand 70008000.nand: CONFIG: 0x10030000
> > > > > [    1.391466] tegra-nand 70008000.nand: TIMING: 0x00000000
> > > > > [    1.396782] tegra-nand 70008000.nand: TIMING2: 0x00000000
> > > > > [    1.402174] tegra-nand 70008000.nand: CMD_REG1: 0x00000000
> > > > > [    1.407664] tegra-nand 70008000.nand: CMD_REG2: 0x00000000
> > > > > [    1.413156] tegra-nand 70008000.nand: ADDR_REG1: 0x00000000
> > > > > [    1.418722] tegra-nand 70008000.nand: ADDR_REG2: 0x00000000
> > > > > [    1.424297] tegra-nand 70008000.nand: DMA_MST_CTRL: 0x24000000
> > > > > [    1.430123] tegra-nand 70008000.nand: DMA_CFG_A: 0x00000000
> > > > > [    1.435698] tegra-nand 70008000.nand: DMA_CFG_B: 0x00000000
> > > > > [    1.441264] tegra-nand 70008000.nand: FIFO_CTRL: 0x0000aa00
> > > > 
> > > > Alright, then indeed it's not really worth to bother with HW resetting
> > > > here. Probably only a kernel module reload or a reboot will help if HW
> > > > is
> > > > hung. Maybe NAND controller / chip recovering is something that NAND
> > > > core
> > > > should be handling in a such case by providing a
> > > > nand_controller_reset()
> > > > hook?
> > > 
> > > I don't see what the core could do to help with that. We'd end up with
> > > a new hook implemented by the controller that would be called by the
> > > controller driver when it knows it's safe to reset the controller. So,
> > > why bother exposing that in the core?
> > 
> > Giving a driver more flexibility is always a good thing. I'm not really
> > familiar with mtd/ and maybe indeed it doesn't make much sense to move HW
> > resetting to NAND core, though it looked to me that it should be always
> > safe for NAND core to initiate HW resetting after IO failure and hence
> > would be cleaner and nicer to have a unified HW reset management rather
> > than to have each driver to do its own thing.
> 
> No really, the NAND core can't know when it's appropriate to reset the
> controller, and what this reset will do, hence it doesn't know if the
> chips connected to the controller should also be reset.

Okay!

  reply	other threads:[~2018-06-11 13:10 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-31 22:16 [PATCH v3 0/6] mtd: rawnand: add NVIDIA Tegra NAND flash support Stefan Agner
2018-05-31 22:16 ` [PATCH v3 1/6] mtd: rawnand: add Reed-Solomon error correction algorithm Stefan Agner
2018-06-01  7:26   ` Boris Brezillon
2018-06-01  9:25     ` Boris Brezillon
2018-06-01 13:34       ` Stefan Agner
2018-06-01 13:43         ` Boris Brezillon
2018-05-31 22:16 ` [PATCH v3 2/6] mtd: rawnand: add an option to specify NAND chip as a boot device Stefan Agner
2018-06-01  7:26   ` Boris Brezillon
2018-06-05 20:11   ` Rob Herring
2018-06-06  7:28     ` Boris Brezillon
2018-05-31 22:16 ` [PATCH v3 3/6] mtd: rawnand: tegra: add devicetree binding Stefan Agner
2018-06-01  7:30   ` Boris Brezillon
2018-06-05 20:19     ` Dmitry Osipenko
2018-06-06 10:39       ` Thierry Reding
2018-06-06 10:45         ` Boris Brezillon
2018-06-06 11:07           ` Thierry Reding
2018-06-06 12:14             ` Stefan Agner
2018-06-06 12:31               ` Boris Brezillon
2018-06-05 20:13   ` Rob Herring
2018-05-31 22:16 ` [PATCH v3 4/6] mtd: rawnand: add NVIDIA Tegra NAND Flash controller driver Stefan Agner
2018-06-01  9:20   ` Dmitry Osipenko
2018-06-08 21:51     ` Stefan Agner
2018-06-09  5:52       ` Boris Brezillon
2018-06-09  6:23         ` Stefan Agner
2018-06-09  6:41           ` Boris Brezillon
2018-06-09  6:46             ` Boris Brezillon
2018-06-09  6:55               ` Boris Brezillon
2018-06-09  6:51             ` Stefan Agner
2018-06-09 12:21       ` Dmitry Osipenko
2018-06-10 11:09         ` Stefan Agner
2018-06-10 15:00           ` Dmitry Osipenko
2018-06-10 15:32             ` Boris Brezillon
2018-06-11 11:45               ` Dmitry Osipenko
2018-06-11 11:50                 ` Boris Brezillon
2018-06-11 13:10                   ` Dmitry Osipenko [this message]
2018-06-04 17:16   ` Randolph Maaßen
2018-06-04 20:56     ` Stefan Agner
2018-06-09  5:55   ` Boris Brezillon
2018-06-09  7:18   ` Boris Brezillon
2018-05-31 22:16 ` [PATCH v3 5/6] ARM: dts: tegra: add Tegra20 NAND flash controller node Stefan Agner
2018-05-31 22:16 ` [PATCH v3 6/6] ARM: dts: tegra: enable NAND flash on Colibri T20 Stefan Agner

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=6764263.x6FQro3A0J@dimapc \
    --to=digetx@gmail.com \
    --cc=benjamin.lindqvist@endian.se \
    --cc=boris.brezillon@bootlin.com \
    --cc=computersforpeace@gmail.com \
    --cc=dev@lynxeye.de \
    --cc=devicetree@vger.kernel.org \
    --cc=dwmw2@infradead.org \
    --cc=jonathanh@nvidia.com \
    --cc=krzk@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=marcel@ziswiler.com \
    --cc=marek.vasut@gmail.com \
    --cc=mark.rutland@arm.com \
    --cc=miquel.raynal@bootlin.com \
    --cc=mirza.krak@gmail.com \
    --cc=pdeschrijver@nvidia.com \
    --cc=pgaikwad@nvidia.com \
    --cc=richard@nod.at \
    --cc=robh+dt@kernel.org \
    --cc=stefan@agner.ch \
    --cc=thierry.reding@gmail.com \
    --subject='Re: [PATCH v3 4/6] mtd: rawnand: add NVIDIA Tegra NAND Flash controller driver' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).