linux-mtd.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Artem Bityutskiy <dedekind1@gmail.com>
To: Brian Norris <norris@broadcom.com>
Cc: "linux-mtd@lists.infradead.org" <linux-mtd@lists.infradead.org>
Subject: Re: NAND ECC Layout, sysfs question
Date: Thu, 05 Aug 2010 21:18:04 +0300	[thread overview]
Message-ID: <1281032284.1175.45.camel@localhost.localdomain> (raw)
In-Reply-To: <4C5AFA88.3040009@broadcom.com>

On Thu, 2010-08-05 at 10:53 -0700, Brian Norris wrote:
> Hello,
> 
> I'm trying to replace the following structure (in mtd-abi.h) and its 
> corresponding ioctl with a larger, dynamic form exported via sysfs. 
> We're running out of room in the eccpos and oobfree arrays on larger chips:

I personally hate that internal information like eccpos is exposed to
user-space. I think a good an not error-prone SW should be written in a
way the ECC is completely hidden and handled by driver. And it is better
not to use OOB.

But there may be people who want to use OOB. But then ECC and free space
in OOB should be considered as virtual contiguous array, and the driver
places it properly. It is not user-spaces' business to know where ECC
bytes are placed. There was some OOB mode for this, do not remember its
name.

Do you really need to know ECC layout? What for? Would be nice if
everything but the above mode of working with OOB / ECC would be
considered as legacy and died...

> #define MTD_MAX_OOBFREE_ENTRIES 8
> /*
>   * ECC layout control structure. Exported to userspace for
>   * diagnosis and to allow creation of raw images
>   */
> 
> struct nand_ecclayout {
>          __u32 eccbytes;
>          __u32 eccpos[64];
>          __u32 oobavail;
>          struct nand_oobfree oobfree[MTD_MAX_OOBFREE_ENTRIES];
> };
> 
> First, is it a smart idea to use sysfs?
> 
> Second, if we go with sysfs, there seem to be many potential ways to 
> structure the "eccpos" and "oobfree" arrays. Bearing in mind the 
> convention of either one entry per file or a space-separated array of 
> values as well as the file limit of PAGE_SIZE (approx. 4096 bytes?), 
> there are several options with which I'm having difficulty. Formats 
> could be as follows:
> 1) Single file: 0 1 2 3 6 7 10 12
> 2) Single file: 0-3 6-7 10 12
> 3) Multiple files:
>      file 0: 0 3
>      file 1: 6 7
>      file 2: 10 10
>      file 3: 12 12
> It seemed to me like (3) was the cleanest while ensuring we fit under 
> PAGE_SIZE even if the OOB gets to be 1000+ bytes, so no one ever has to 
> come back and redo this :) Unfortunately, directories and a dynamic 
> number of attributes/files aren't really straightforward, hence my next 
> questions...
> 
> Third, if I were to create directories under the "mtdX" sysfs directory, 
> what's the best way to do this? Explicitly creating and filling in a 
> kobject? Or creating a new "device"? Or are there other, better options?
> 
> I've already coded parts of a few of these options, but they all have 
> various difficulties and can easily become ugly pieces of code (at least 
> with my limited skill at sysfs).

If you really have to do this, I think a new ioctl with some extra space
for future extentions is better. sysfs is not good choice, imo.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

  reply	other threads:[~2010-08-05 18:18 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-08-05 17:53 NAND ECC Layout, sysfs question Brian Norris
2010-08-05 18:18 ` Artem Bityutskiy [this message]
2010-08-07  0:11   ` [PATCH] mtd: Expand nand_ecc_layout, deprecate ioctl ECCGETLAYOUT Brian Norris
2010-08-18 18:06     ` [PATCH v2 0/2] Deprecate ECCGETLAYOUT Brian Norris
2010-08-18 20:37       ` [PATCH v3 1/2] mtd: Expand nand_ecc_layout, deprecate ioctl ECCGETLAYOUT Brian Norris
2010-08-20  0:50       ` [PATCH v2 0/2] Deprecate ECCGETLAYOUT Shinya Kuribayashi
2010-08-20 15:15         ` Brian Norris
2010-08-23  4:12           ` Shinya Kuribayashi
2010-08-24 10:45           ` Artem Bityutskiy
2010-08-25  1:12             ` [PATCH v4] mtd: nand: Expand nand_ecc_layout, deprecate ioctl ECCGETLAYOUT Brian Norris
2010-08-30 10:20               ` Artem Bityutskiy
2010-09-18 17:24                 ` Artem Bityutskiy
2010-09-18 20:03                   ` Brian Norris
2010-09-19  7:17                     ` Artem Bityutskiy
2010-09-20  6:28                   ` Brian Norris
2010-09-20  6:57                     ` [PATCH] mtd: Edit comments on deprecation of " Brian Norris
2010-09-20  8:49                       ` Artem Bityutskiy
2010-09-20  7:52                     ` [PATCH v4] mtd: nand: Expand nand_ecc_layout, deprecate " Artem Bityutskiy
2010-08-24 10:42       ` [PATCH v2 0/2] Deprecate ECCGETLAYOUT Artem Bityutskiy
2010-08-18 18:06     ` [PATCH v2 1/2] mtd: nand: Expand nand_ecc_layout, deprecate ioctl ECCGETLAYOUT Brian Norris
2010-08-18 18:06     ` [PATCH v2 2/2] " 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=1281032284.1175.45.camel@localhost.localdomain \
    --to=dedekind1@gmail.com \
    --cc=linux-mtd@lists.infradead.org \
    --cc=norris@broadcom.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;
as well as URLs for NNTP newsgroup(s).