LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Boris Brezillon <boris.brezillon@bootlin.com>
To: Paul Cercueil <paul@crapouillou.net>
Cc: David Woodhouse <dwmw2@infradead.org>,
	Brian Norris <computersforpeace@gmail.com>,
	Boris Brezillon <boris.brezillon@bolin.com>,
	Marek Vasut <marek.vasut@gmail.com>,
	Richard Weinberger <richard@nod.at>,
	Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	devicetree@vger.kernel.org, linux-mtd@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mtd: nand: Add support for reading ooblayout from device tree
Date: Sat, 12 May 2018 07:55:56 +0200	[thread overview]
Message-ID: <20180512075556.2e590e06@bbrezillon> (raw)
In-Reply-To: <20180511212912.1426-1-paul@crapouillou.net>

Hi Paul,

On Fri, 11 May 2018 23:29:12 +0200
Paul Cercueil <paul@crapouillou.net> wrote:

> By specifying the properties "mtd-oob-ecc" and "mtd-oob-free", it is
> now possible to specify from devicetree where the ECC data is located
> inside the OOB region.

Why would we want to do that? I mean, ECC/free regions are ECC
controller dependent (and NAND chip dependent for the OOB size part),
so there's no reason to describe it in the DT. And more importantly,
people are likely to get it wrong.

I'm curious, why do you need that?

Regards,

Boris

> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
>  Documentation/devicetree/bindings/mtd/nand.txt |  7 +++++
>  drivers/mtd/nand/raw/nand_base.c               | 42 ++++++++++++++++++++++++++
>  2 files changed, 49 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/nand.txt b/Documentation/devicetree/bindings/mtd/nand.txt
> index 8bb11d809429..118ea92787cb 100644
> --- a/Documentation/devicetree/bindings/mtd/nand.txt
> +++ b/Documentation/devicetree/bindings/mtd/nand.txt
> @@ -45,6 +45,13 @@ Optional NAND chip properties:
>  		     as reliable as possible.
>  - nand-rb: shall contain the native Ready/Busy ids.
>  
> +- nand-oob-ecc: <offset, length> couples of integers, specifying the offset
> +		     and length of the ECC data in the OOB region. There can be more
> +		     than one couple.
> +- nand-oob-free: <offset, length> couples of integers, specifying the offset
> +		     and length of a free-to-use area in the OOB region. There can be
> +		     more than one couple.
> +
>  The ECC strength and ECC step size properties define the correction capability
>  of a controller. Together, they say a controller can correct "{strength} bit
>  errors per {size} bytes".
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index 72f3a89da513..c905531effb0 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -213,6 +213,43 @@ static const struct mtd_ooblayout_ops nand_ooblayout_lp_hamming_ops = {
>  	.free = nand_ooblayout_free_lp_hamming,
>  };
>  
> +static int nand_oob_of(struct device_node *np, int section,
> +		       struct mtd_oob_region *oobregion, const char *prop)
> +{
> +	int ret = of_property_read_u32_index(np, prop,
> +			section * 2, &oobregion->offset);
> +	if (ret == -EOVERFLOW)
> +		return -ERANGE; /* We're done */
> +	if (ret)
> +		return ret;
> +
> +	ret = of_property_read_u32_index(np, prop,
> +			section * 2 + 1, &oobregion->length);
> +	if (ret == -EOVERFLOW)
> +		return -EINVAL; /* We must have an even number of integers */
> +
> +	return ret;
> +}
> +
> +static int nand_ooblayout_ecc_of(struct mtd_info *mtd, int section,
> +				 struct mtd_oob_region *oobregion)
> +{
> +	return nand_oob_of(mtd->dev.of_node, section,
> +			oobregion, "nand-oob-ecc");
> +}
> +
> +static int nand_ooblayout_free_of(struct mtd_info *mtd, int section,
> +				 struct mtd_oob_region *oobregion)
> +{
> +	return nand_oob_of(mtd->dev.of_node, section,
> +			oobregion, "nand-oob-free");
> +}
> +
> +static const struct mtd_ooblayout_ops nand_ooblayout_of_ops = {
> +	.ecc = nand_ooblayout_ecc_of,
> +	.free = nand_ooblayout_free_of,
> +};
> +
>  static int check_offs_len(struct mtd_info *mtd,
>  					loff_t ofs, uint64_t len)
>  {
> @@ -5843,6 +5880,11 @@ static int nand_dt_init(struct nand_chip *chip)
>  	if (of_property_read_bool(dn, "nand-ecc-maximize"))
>  		chip->ecc.options |= NAND_ECC_MAXIMIZE;
>  
> +	if (!chip->mtd.ooblayout &&
> +				of_property_read_bool(dn, "nand-oob-ecc") &&
> +				of_property_read_bool(dn, "nand-oob-free"))
> +		chip->mtd.ooblayout = &nand_ooblayout_of_ops;
> +
>  	return 0;
>  }
>  

  reply	other threads:[~2018-05-12  5:56 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-11 21:29 Paul Cercueil
2018-05-12  5:55 ` Boris Brezillon [this message]
     [not found] <20180512115551.56C6E20787@mail.bootlin.com>
2018-05-12 13:42 ` Boris Brezillon
2018-05-12 14:38   ` Paul Cercueil
2018-05-12 15:00     ` Boris Brezillon
2018-05-12 17:42       ` Paul Cercueil
2018-05-12 18:02         ` Boris Brezillon
2018-05-12 18:30           ` Paul Cercueil

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=20180512075556.2e590e06@bbrezillon \
    --to=boris.brezillon@bootlin.com \
    --cc=boris.brezillon@bolin.com \
    --cc=computersforpeace@gmail.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dwmw2@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=marek.vasut@gmail.com \
    --cc=mark.rutland@arm.com \
    --cc=paul@crapouillou.net \
    --cc=richard@nod.at \
    --cc=robh+dt@kernel.org \
    --subject='Re: [PATCH] mtd: nand: Add support for reading ooblayout from device tree' \
    /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).