All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lucas Stach <dev@lynxeye.de>
To: Brian Norris <computersforpeace@gmail.com>
Cc: David Woodhouse <dwmw2@infradead.org>,
	Thierry Reding <thierry.reding@gmail.com>,
	Stephen Warren <swarren@wwwdotorg.org>,
	Rob Herring <robh+dt@kernel.org>, Pawel Moll <pawel.moll@arm.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Alexandre Courbot <gnurou@gmail.com>,
	Boris BREZILLON <boris.brezillon@free-electrons.com>,
	Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>,
	Stefan Agner <stefan@agner.ch>,
	Marcel Ziswiler <marcel@ziswiler.com>,
	devicetree@vger.kernel.org, linux-tegra@vger.kernel.org,
	 linux-mtd@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [Patch v3 2/5] mtd: nand: add NVIDIA Tegra NAND Flash controller driver
Date: Mon, 27 Jul 2015 21:19:57 +0200	[thread overview]
Message-ID: <1438024797.2313.11.camel@lynxeye.de> (raw)
In-Reply-To: <20150721212710.GN24125@google.com>

Am Dienstag, den 21.07.2015, 14:27 -0700 schrieb Brian Norris:

[...]
> 
> > +		return 0;
> > +	}
> > +
> > +	if (value & DEC_STATUS_ERR_COUNT_MASK) {
> > +		value = (value & DEC_STATUS_ERR_COUNT_MASK) >>
> > +			DEC_STATUS_ERR_COUNT_SHIFT;
> 
> What does this ERR_COUNT represent? The total bitflips seen in this
> page? The max seen per ECC region? Or something else?
> 
> > +		mtd->ecc_stats.corrected += value;
> 
> I ask because this ^^^ should be accounting the total bitflips for the
> page, if possible...
> 
> > +		return value;
> 
> ...and this ^^^ should be returning the max bitflips seen in a single
> correction region (see the mtd_read() API). There is a subtle
> difference.
> 
I've checked this again and unfortunately the DEC_STATUS_ERR_COUNT seems
to be completely bogus. It is certainly only set when there are
bitflips, but it doesn't represent a valid value.

I have another register that seems to accurately reflect the maximum
number of bitflips in any of the 512b ECC blocks, so I'm able to return
the correct amount of bitflips to the upper level.
The downside is that the hardware already does the calculation of max
bitflips, so I'm unable to count bitflips correctly for the stats. All I
can do is to return a pessimistic value that assumes that all 512b
blocks had the returned number of bitflips. Could this be a problem?

Regards,
Lucas

WARNING: multiple messages have this Message-ID (diff)
From: Lucas Stach <dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
To: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: David Woodhouse <dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>,
	Thierry Reding
	<thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>,
	Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>,
	Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>,
	Alexandre Courbot
	<gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Boris BREZILLON
	<boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>,
	Ezequiel Garcia
	<ezequiel-30ULvvUtt6G51wMPkGsGjgyUoB5FGQPZ@public.gmane.org>,
	Stefan Agner <stefan-XLVq0VzYD2Y@public.gmane.org>,
	Marcel Ziswiler <marcel-mitwqZ+T+m9Wk0Htik3J/w@public.gmane.org>,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Subject: Re: [Patch v3 2/5] mtd: nand: add NVIDIA Tegra NAND Flash controller driver
Date: Mon, 27 Jul 2015 21:19:57 +0200	[thread overview]
Message-ID: <1438024797.2313.11.camel@lynxeye.de> (raw)
In-Reply-To: <20150721212710.GN24125-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>

Am Dienstag, den 21.07.2015, 14:27 -0700 schrieb Brian Norris:

[...]
> 
> > +		return 0;
> > +	}
> > +
> > +	if (value & DEC_STATUS_ERR_COUNT_MASK) {
> > +		value = (value & DEC_STATUS_ERR_COUNT_MASK) >>
> > +			DEC_STATUS_ERR_COUNT_SHIFT;
> 
> What does this ERR_COUNT represent? The total bitflips seen in this
> page? The max seen per ECC region? Or something else?
> 
> > +		mtd->ecc_stats.corrected += value;
> 
> I ask because this ^^^ should be accounting the total bitflips for the
> page, if possible...
> 
> > +		return value;
> 
> ...and this ^^^ should be returning the max bitflips seen in a single
> correction region (see the mtd_read() API). There is a subtle
> difference.
> 
I've checked this again and unfortunately the DEC_STATUS_ERR_COUNT seems
to be completely bogus. It is certainly only set when there are
bitflips, but it doesn't represent a valid value.

I have another register that seems to accurately reflect the maximum
number of bitflips in any of the 512b ECC blocks, so I'm able to return
the correct amount of bitflips to the upper level.
The downside is that the hardware already does the calculation of max
bitflips, so I'm unable to count bitflips correctly for the stats. All I
can do is to return a pessimistic value that assumes that all 512b
blocks had the returned number of bitflips. Could this be a problem?

Regards,
Lucas

WARNING: multiple messages have this Message-ID (diff)
From: dev@lynxeye.de (Lucas Stach)
To: linux-arm-kernel@lists.infradead.org
Subject: [Patch v3 2/5] mtd: nand: add NVIDIA Tegra NAND Flash controller driver
Date: Mon, 27 Jul 2015 21:19:57 +0200	[thread overview]
Message-ID: <1438024797.2313.11.camel@lynxeye.de> (raw)
In-Reply-To: <20150721212710.GN24125@google.com>

Am Dienstag, den 21.07.2015, 14:27 -0700 schrieb Brian Norris:

[...]
> 
> > +		return 0;
> > +	}
> > +
> > +	if (value & DEC_STATUS_ERR_COUNT_MASK) {
> > +		value = (value & DEC_STATUS_ERR_COUNT_MASK) >>
> > +			DEC_STATUS_ERR_COUNT_SHIFT;
> 
> What does this ERR_COUNT represent? The total bitflips seen in this
> page? The max seen per ECC region? Or something else?
> 
> > +		mtd->ecc_stats.corrected += value;
> 
> I ask because this ^^^ should be accounting the total bitflips for the
> page, if possible...
> 
> > +		return value;
> 
> ...and this ^^^ should be returning the max bitflips seen in a single
> correction region (see the mtd_read() API). There is a subtle
> difference.
> 
I've checked this again and unfortunately the DEC_STATUS_ERR_COUNT seems
to be completely bogus. It is certainly only set when there are
bitflips, but it doesn't represent a valid value.

I have another register that seems to accurately reflect the maximum
number of bitflips in any of the 512b ECC blocks, so I'm able to return
the correct amount of bitflips to the upper level.
The downside is that the hardware already does the calculation of max
bitflips, so I'm unable to count bitflips correctly for the stats. All I
can do is to return a pessimistic value that assumes that all 512b
blocks had the returned number of bitflips. Could this be a problem?

Regards,
Lucas

  parent reply	other threads:[~2015-07-27 19:19 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-10 18:29 [Patch v3 0/5] Tegra 2 NAND Flash Support Lucas Stach
2015-05-10 18:29 ` Lucas Stach
2015-05-10 18:29 ` Lucas Stach
2015-05-10 18:29 ` [Patch v3 1/5] mtd: nand: tegra: add devicetree binding Lucas Stach
2015-05-10 18:29   ` Lucas Stach
2015-05-10 18:29   ` Lucas Stach
2015-07-21 21:05   ` Brian Norris
2015-07-21 21:05     ` Brian Norris
2015-07-21 21:05     ` Brian Norris
2015-07-22 20:15     ` Lucas Stach
2015-07-22 20:15       ` Lucas Stach
2015-07-22 20:15       ` Lucas Stach
2015-07-22 22:32       ` Brian Norris
2015-07-22 22:32         ` Brian Norris
2015-07-22 22:32         ` Brian Norris
2015-05-10 18:29 ` [Patch v3 2/5] mtd: nand: add NVIDIA Tegra NAND Flash controller driver Lucas Stach
2015-05-10 18:29   ` Lucas Stach
2015-05-10 18:29   ` Lucas Stach
2015-07-21 21:27   ` Brian Norris
2015-07-21 21:27     ` Brian Norris
2015-07-21 21:27     ` Brian Norris
2015-07-22 20:42     ` Lucas Stach
2015-07-22 20:42       ` Lucas Stach
2015-07-22 20:42       ` Lucas Stach
2015-07-22 23:10       ` Brian Norris
2015-07-22 23:10         ` Brian Norris
2015-07-22 23:10         ` Brian Norris
2015-07-27 19:12         ` Lucas Stach
2015-07-27 19:12           ` Lucas Stach
2015-07-27 19:12           ` Lucas Stach
2015-07-27 19:19     ` Lucas Stach [this message]
2015-07-27 19:19       ` Lucas Stach
2015-07-27 19:19       ` Lucas Stach
2015-07-27 20:52       ` Brian Norris
2015-07-27 20:52         ` Brian Norris
2015-07-27 20:52         ` Brian Norris
2015-05-10 18:30 ` [Patch v3 3/5] clk: tegra20: init NDFLASH clock to sensible rate Lucas Stach
2015-05-10 18:30   ` Lucas Stach
2015-05-10 18:30   ` Lucas Stach
2015-05-10 18:30 ` [Patch v3 4/5] ARM: tegra: add Tegra20 NAND flash controller node Lucas Stach
2015-05-10 18:30   ` Lucas Stach
2015-05-10 18:30   ` Lucas Stach
2015-05-10 18:30 ` [Patch v3 5/5] ARM: tegra: enable NAND flash on Colibri T20 Lucas Stach
2015-05-10 18:30   ` Lucas Stach
2015-05-10 18:30   ` Lucas Stach
2015-07-21 21:07   ` Brian Norris
2015-07-21 21:07     ` Brian Norris
2015-07-21 21:07     ` 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=1438024797.2313.11.camel@lynxeye.de \
    --to=dev@lynxeye.de \
    --cc=boris.brezillon@free-electrons.com \
    --cc=computersforpeace@gmail.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dwmw2@infradead.org \
    --cc=ezequiel@vanguardiasur.com.ar \
    --cc=gnurou@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=marcel@ziswiler.com \
    --cc=mark.rutland@arm.com \
    --cc=pawel.moll@arm.com \
    --cc=robh+dt@kernel.org \
    --cc=stefan@agner.ch \
    --cc=swarren@wwwdotorg.org \
    --cc=thierry.reding@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.