[PATCH v3 2/3] mtd: devices: elm: Add support for ELM error correction

[Philip, Avinash]

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 at ti.com>
> > Cc: Grant Likely <grant.likely at secretlab.ca>
> > Cc: Rob Herring <rob.herring at calxeda.com>
> > Cc: Rob Landley <rob at 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
>