[PATCH v3 3/6] mtd: set the ecc step size for master/slave mtd_info

[Gupta, Pekon]

> 
> On Mon, Aug 12, 2013 at 05:44:57PM +0800, Huang Shijie wrote:
> > 于 2013年08月12日 17:24, Gupta, Pekon 写道:
> > >[Pekon]: Following Artem's recommendations.. Can this be named as
> > >chip->ecc.size = ecc_step_size.
> > >then chip->ecc.bytes  = ecc_syndrome_size (or ecc_code_size)
> > yes, we can rename these fields in the future with another patch set.
> 
> Internal naming can be changed in the future if needed, but we really
> need to get a good sysfs name now, since that becomes ABI. And it would
> also be good to get this to be clearly consistent internally too.
> 
> So the main decision to be made here: what to name the sysfs field? It
> currently stands as ecc_step, but for clarity to users (and for avoiding
> internal confusion with nand_chip.ecc.steps), I think ecc_step_size is
> just as good or better.
> 
> Then, we might as well have the mtd_info field be
> mtd_info.ecc_step_size. So we would have the following names.
> 
> For ECC step size:
> 
>   /sys/class/mtd/mtdX/ecc_step_size
>   mtd_info.ecc_step_size
>   nand_chip.ecc.size    <--- candidate for renaming to ...ecc.step_size
> 
Agree.

> For ECC strength:
> 
>   /sys/class/mtd/mtdX/ecc_strength
>   mtd_info.ecc_strength
>   nand_chip.ecc.strength
> 
Agree.

> Other fields to consider for internal naming:
> 
>   # Number of ECC steps per page
>   nand_chip.ecc.steps
mtd_info.ecc_steps  (if needed in mtd_info)

> 
>   # Number of ECC bytes per ECC step
>   nand_chip.ecc.bytes
mtd_info.ecc_code_size
nand_chip.ecc.bytes    <--- candidate for renaming to ...ecc.code_size

> Please, let's get an agreement on one of these, so we hopefully only
> need at most a v4 patch series:
> 
> (1) Huang's current patch set (v3)
> (2) My proposed naming above
> (3) A third option? (AFAICT, (1) or (2) should be satisfactory)
> 
> If we go with (2), the v4 patch series only needs to take care of the
> new sysfs naming (plus documentation) and the new mtd_info field naming.
> Any internal consistency patches (e.g., for nand_ecc_ctrl) can come as
> follow-ups.
> 
> > >In addition, chip->ecc.bytes should also be helpful for userspace
> > >utility to determine how much bytes to reserve in spare-area for ECC.
> > >So exposing that as sysfs entry is also good.
> > >
> > I do not need the chip->ecc.bytes. :)
> > For me, export the chip->ecc.size is enough.
> 
> A better argument against this is that ecc.bytes does not necessarily
> have utility across many types of NAND drivers by itself, since ECC
> layouts differ. And at that point, we're trying to duplicate the
> behavior of ioctl(ECCGETLAYOUT) (which notably has run out of room and
> isn't 100% informative anymore).
> 
> > So you can submit a patch if you need this field.
> 
> Yes, if you have good reason for exporting it, send a patch and a good
> argument. And it would need to explain why we can't just use
> ECCGETLAYOUT.
> 
Thanks for the info..
I wasn't fully aware of ioctl(ECCGETLAYOUT), I'll explore it further.

Also, is there any documentation or URL explaining all ioctl for MTD?

with regards, pekon

> Brian