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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <518397C60809E147AF5323E0420B992E3EA1FA14@DBDE01.ent.ti.com>
Date:	Mon, 10 Dec 2012 06:43:17 +0000
From:	"Philip, Avinash" <avinashphilip@...com>
To:	"Nori, Sekhar" <nsekhar@...com>
CC:	"dwmw2@...radead.org" <dwmw2@...radead.org>,
	"artem.bityutskiy@...ux.intel.com" <artem.bityutskiy@...ux.intel.com>,
	"Mohammed, Afzal" <afzal@...com>,
	"tony@...mide.com" <tony@...mide.com>,
	"broonie@...nsource.wolfsonmicro.com" 
	<broonie@...nsource.wolfsonmicro.com>,
	"rmk+kernel@....linux.org.uk" <rmk+kernel@....linux.org.uk>,
	"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
	"linux-mtd@...ts.infradead.org" <linux-mtd@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"devicetree-discuss@...ts.ozlabs.org" 
	<devicetree-discuss@...ts.ozlabs.org>,
	"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
	"linux-omap@...r.kernel.org" <linux-omap@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"Hebbar, Gururaja" <gururaja.hebbar@...com>,
	"ivan.djelic@...rot.com" <ivan.djelic@...rot.com>,
	Grant Likely <grant.likely@...retlab.ca>,
	Rob Herring <rob.herring@...xeda.com>,
	Rob Landley <rob@...dley.net>
Subject: RE: [PATCH v3 2/3] mtd: devices: elm: Add support for ELM error
 correction

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@...com>
> > Cc: Grant Likely <grant.likely@...retlab.ca>
> > Cc: Rob Herring <rob.herring@...xeda.com>
> > Cc: Rob Landley <rob@...dley.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
>