lkml.org 
[lkml]   [2012]   [Dec]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRE: [PATCH v3 2/3] mtd: devices: elm: Add support for ELM error correction
    Date
    On Fri, Dec 07, 2012 at 16:07:23, Nori, Sekhar wrote:
    > On 11/29/2012 5:16 PM, Philip, Avinash wrote:
    > > The ELM hardware module can be used to speedup BCH 4/8/16 ECC scheme
    > > error correction.
    > > For now only 4 & 8 bit support is added
    > >
    > > Signed-off-by: Philip, Avinash <avinashphilip@ti.com>
    > > Cc: Grant Likely <grant.likely@secretlab.ca>
    > > Cc: Rob Herring <rob.herring@calxeda.com>
    > > Cc: Rob Landley <rob@landley.net>

    [...]
    /**
    > > + * elm_config - Configure ELM module
    > > + * @info: elm info
    > > + */
    > > +static void elm_config(struct elm_info *info)
    >
    > This is called "config", but there is no configuration information
    > passed which looks odd..

    The config information is bch_type, encapsulated in struct elm_info.

    >
    > > +{
    > > + u32 reg_val;
    > > +
    > > + reg_val = (info->bch_type & ECC_BCH_LEVEL_MASK) | (ELM_ECC_SIZE << 16);
    > > + elm_write_reg(info, ELM_LOCATION_CONFIG, reg_val);
    > > +}
    >
    > Is there a use case where BCH type needs to be changed after NAND has
    > been probed?

    No, I think kernel handles the entire NAND part with a single ecc layout.
    Hence there is no run time BCH switching. But ELM driver should support BCH
    4 & 8. Selection of BCH information comes from DT of NAND driver.

    As NAND driver supporting BCH4 & 8 ecc scheme ELM module support
    configuration of both. Configuration of ELM module should done as part
    of NAND driver probing.


    > You will have to erase any existing data written to NAND if
    > you change the ECC type. That sounds destructive enough to avoid such a
    > thing.

    There is no support for BCH switching after NAND driver probing.

    [...]
    > > +struct device *elm_request(enum bch_ecc bch_type)
    > > +{
    > > + struct elm_info *info;
    > > +
    > > + list_for_each_entry(info, &elm_devices, list) {
    > > + if (info && info->dev) {
    > > + info->bch_type = bch_type;
    > > + elm_config(info);
    > > + return info->dev;
    > > + }
    > > + }
    >
    > This will always return the first ELM device probed since you never
    > remove the allocated device from the list.

    But now I realized that, there is no mechanism of freeing the requested
    resource.

    So I will add mechanism to request ELM module successfully only if ELM
    module is not requested already and add mechanism to free it, on NAND
    driver module unload (loadable module support). This way ELM driver
    can achieve multi instance support.

    > I wonder why you really need a list?

    The prime motivation for the list is the driver should support multi
    instances of ELM by removing global symbols.

    Thanks
    Avinash

    >
    > Thanks,
    > Sekhar
    >

    \
     
     \ /
      Last update: 2012-12-10 08:01    [W:3.352 / U:0.016 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site