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

2012-12-12 Thread Sekhar Nori 
On 12/10/2012 12:13 PM, Philip, Avinash wrote:
> On Fri, Dec 07, 2012 at 16:07:23, Nori, Sekhar wrote:
>> On 11/29/2012 5:16 PM, Philip, Avinash wrote:

[...]

>>> +struct device *elm_request(enum bch_ecc bch_type)
>>> +{
>>> +   struct elm_info *info;
>>> +
>>> +   list_for_each_entry(info, _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.

Right. You essentially want to assign an ELM instance to work with a
given instance of GPMC and that could be done statically too. Just pass
phandle of ELM node in GPMC DT data?

> 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.

I still think a request/free API is bit too much for something that will
turn out to be a simple 1-to-1 match anyway. Can you please look at the
phandle suggestion above? I am no DT expert, but I think that will work
for your use case.

Thanks,
Sekhar
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2012-12-12 Thread Sekhar Nori 
On 12/10/2012 12:13 PM, Philip, Avinash wrote:
 On Fri, Dec 07, 2012 at 16:07:23, Nori, Sekhar wrote:
 On 11/29/2012 5:16 PM, Philip, Avinash wrote:

[...]

 +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.

Right. You essentially want to assign an ELM instance to work with a
given instance of GPMC and that could be done statically too. Just pass
phandle of ELM node in GPMC DT data?

 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.

I still think a request/free API is bit too much for something that will
turn out to be a simple 1-to-1 match anyway. Can you please look at the
phandle suggestion above? I am no DT expert, but I think that will work
for your use case.

Thanks,
Sekhar
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2012-12-11 Thread Philip, Avinash 
On Tue, Dec 11, 2012 at 14:33:56, Grant Likely wrote:
> On Thu, 29 Nov 2012 17:16:33 +0530, "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 
> > Cc: Grant Likely 
> > Cc: Rob Herring 
> > Cc: Rob Landley 
> > ---
> > Changes since v2:
> > - Remove __devinit & __devexit annotations
> > 
> > Changes since v1:
> > - Change build attribute to CONFIG_MTD_NAND_OMAP_BCH
> > - Reduced indentation using by passing elm_info , offset
> >   to elm_read & elm_write
> > - Removed syndrome manipulation functions.
> > 
> > :00 100644 000... b88ee83... A  
> > Documentation/devicetree/bindings/mtd/elm.txt
> > :100644 100644 395733a... 369a194... M  drivers/mtd/devices/Makefile
> > :00 100644 000... d2667f3... A  drivers/mtd/devices/elm.c
> > :00 100644 000... d4fce31... A  
> > include/linux/platform_data/elm.h
> >  Documentation/devicetree/bindings/mtd/elm.txt |   17 +
> >  drivers/mtd/devices/Makefile  |4 +-
> >  drivers/mtd/devices/elm.c |  418 
> > +
> >  include/linux/platform_data/elm.h |   54 
> >  4 files changed, 493 insertions(+), 1 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/mtd/elm.txt 
> > b/Documentation/devicetree/bindings/mtd/elm.txt
> > new file mode 100644
> > index 000..b88ee83
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mtd/elm.txt
> > @@ -0,0 +1,17 @@
> > +Error location module
> > +
> > +Required properties:
> > +- compatible: Must be "ti,elm"
> 
> Compatible string is too generic. Need to specify a specific SoC here.
> ie: "ti,omap3430-elm"

I will change to "ti,am33xx-elm" in next version.

Thanks
Avinash


> 
> Otherwise the binding looks fine. I haven't reviewed the code though.
> 
> g.
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2012-12-11 Thread Grant Likely 
On Thu, 29 Nov 2012 17:16:33 +0530, "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 
> Cc: Grant Likely 
> Cc: Rob Herring 
> Cc: Rob Landley 
> ---
> Changes since v2:
>   - Remove __devinit & __devexit annotations
> 
> Changes since v1:
>   - Change build attribute to CONFIG_MTD_NAND_OMAP_BCH
>   - Reduced indentation using by passing elm_info , offset
> to elm_read & elm_write
>   - Removed syndrome manipulation functions.
> 
> :00 100644 000... b88ee83... A
> Documentation/devicetree/bindings/mtd/elm.txt
> :100644 100644 395733a... 369a194... Mdrivers/mtd/devices/Makefile
> :00 100644 000... d2667f3... Adrivers/mtd/devices/elm.c
> :00 100644 000... d4fce31... A
> include/linux/platform_data/elm.h
>  Documentation/devicetree/bindings/mtd/elm.txt |   17 +
>  drivers/mtd/devices/Makefile  |4 +-
>  drivers/mtd/devices/elm.c |  418 
> +
>  include/linux/platform_data/elm.h |   54 
>  4 files changed, 493 insertions(+), 1 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/elm.txt 
> b/Documentation/devicetree/bindings/mtd/elm.txt
> new file mode 100644
> index 000..b88ee83
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/elm.txt
> @@ -0,0 +1,17 @@
> +Error location module
> +
> +Required properties:
> +- compatible: Must be "ti,elm"

Compatible string is too generic. Need to specify a specific SoC here.
ie: "ti,omap3430-elm"

Otherwise the binding looks fine. I haven't reviewed the code though.

g.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2012-12-11 Thread Grant Likely 
On Thu, 29 Nov 2012 17:16:33 +0530, Philip, Avinash avinashphi...@ti.com 
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 avinashphi...@ti.com
 Cc: Grant Likely grant.lik...@secretlab.ca
 Cc: Rob Herring rob.herr...@calxeda.com
 Cc: Rob Landley r...@landley.net
 ---
 Changes since v2:
   - Remove __devinit  __devexit annotations
 
 Changes since v1:
   - Change build attribute to CONFIG_MTD_NAND_OMAP_BCH
   - Reduced indentation using by passing elm_info , offset
 to elm_read  elm_write
   - Removed syndrome manipulation functions.
 
 :00 100644 000... b88ee83... A
 Documentation/devicetree/bindings/mtd/elm.txt
 :100644 100644 395733a... 369a194... Mdrivers/mtd/devices/Makefile
 :00 100644 000... d2667f3... Adrivers/mtd/devices/elm.c
 :00 100644 000... d4fce31... A
 include/linux/platform_data/elm.h
  Documentation/devicetree/bindings/mtd/elm.txt |   17 +
  drivers/mtd/devices/Makefile  |4 +-
  drivers/mtd/devices/elm.c |  418 
 +
  include/linux/platform_data/elm.h |   54 
  4 files changed, 493 insertions(+), 1 deletions(-)
 
 diff --git a/Documentation/devicetree/bindings/mtd/elm.txt 
 b/Documentation/devicetree/bindings/mtd/elm.txt
 new file mode 100644
 index 000..b88ee83
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/mtd/elm.txt
 @@ -0,0 +1,17 @@
 +Error location module
 +
 +Required properties:
 +- compatible: Must be ti,elm

Compatible string is too generic. Need to specify a specific SoC here.
ie: ti,omap3430-elm

Otherwise the binding looks fine. I haven't reviewed the code though.

g.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2012-12-11 Thread Philip, Avinash 
On Tue, Dec 11, 2012 at 14:33:56, Grant Likely wrote:
 On Thu, 29 Nov 2012 17:16:33 +0530, Philip, Avinash avinashphi...@ti.com 
 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 avinashphi...@ti.com
  Cc: Grant Likely grant.lik...@secretlab.ca
  Cc: Rob Herring rob.herr...@calxeda.com
  Cc: Rob Landley r...@landley.net
  ---
  Changes since v2:
  - Remove __devinit  __devexit annotations
  
  Changes since v1:
  - Change build attribute to CONFIG_MTD_NAND_OMAP_BCH
  - Reduced indentation using by passing elm_info , offset
to elm_read  elm_write
  - Removed syndrome manipulation functions.
  
  :00 100644 000... b88ee83... A  
  Documentation/devicetree/bindings/mtd/elm.txt
  :100644 100644 395733a... 369a194... M  drivers/mtd/devices/Makefile
  :00 100644 000... d2667f3... A  drivers/mtd/devices/elm.c
  :00 100644 000... d4fce31... A  
  include/linux/platform_data/elm.h
   Documentation/devicetree/bindings/mtd/elm.txt |   17 +
   drivers/mtd/devices/Makefile  |4 +-
   drivers/mtd/devices/elm.c |  418 
  +
   include/linux/platform_data/elm.h |   54 
   4 files changed, 493 insertions(+), 1 deletions(-)
  
  diff --git a/Documentation/devicetree/bindings/mtd/elm.txt 
  b/Documentation/devicetree/bindings/mtd/elm.txt
  new file mode 100644
  index 000..b88ee83
  --- /dev/null
  +++ b/Documentation/devicetree/bindings/mtd/elm.txt
  @@ -0,0 +1,17 @@
  +Error location module
  +
  +Required properties:
  +- compatible: Must be ti,elm
 
 Compatible string is too generic. Need to specify a specific SoC here.
 ie: ti,omap3430-elm

I will change to ti,am33xx-elm in next version.

Thanks
Avinash


 
 Otherwise the binding looks fine. I haven't reviewed the code though.
 
 g.
 
 

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2012-12-09 Thread 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 
> > Cc: Grant Likely 
> > Cc: Rob Herring 
> > Cc: Rob Landley 

[...]
/**
> > + * 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, _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
> 

N�r��yb�X��ǧv�^�)޺{.n�+{zX����ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i

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

2012-12-09 Thread 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 avinashphi...@ti.com
  Cc: Grant Likely grant.lik...@secretlab.ca
  Cc: Rob Herring rob.herr...@calxeda.com
  Cc: Rob Landley r...@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
 

N�r��yb�X��ǧv�^�)޺{.n�+{zX����ܨ}���Ơz�j:+v���zZ+��+zf���h���~i���z��w���?��)ߢf��^jǫy�m��@A�a���
0��h���i

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

2012-12-07 Thread Sekhar Nori 
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 
> Cc: Grant Likely 
> Cc: Rob Herring 
> Cc: Rob Landley 
> ---
> Changes since v2:
>   - Remove __devinit & __devexit annotations
> 
> Changes since v1:
>   - Change build attribute to CONFIG_MTD_NAND_OMAP_BCH
>   - Reduced indentation using by passing elm_info , offset
> to elm_read & elm_write
>   - Removed syndrome manipulation functions.
> 
> :00 100644 000... b88ee83... A
> Documentation/devicetree/bindings/mtd/elm.txt
> :100644 100644 395733a... 369a194... Mdrivers/mtd/devices/Makefile
> :00 100644 000... d2667f3... Adrivers/mtd/devices/elm.c
> :00 100644 000... d4fce31... A
> include/linux/platform_data/elm.h
>  Documentation/devicetree/bindings/mtd/elm.txt |   17 +
>  drivers/mtd/devices/Makefile  |4 +-
>  drivers/mtd/devices/elm.c |  418 
> +
>  include/linux/platform_data/elm.h |   54 
>  4 files changed, 493 insertions(+), 1 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/elm.txt 
> b/Documentation/devicetree/bindings/mtd/elm.txt
> new file mode 100644
> index 000..b88ee83
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/elm.txt
> @@ -0,0 +1,17 @@
> +Error location module
> +
> +Required properties:
> +- compatible: Must be "ti,elm"
> +- reg: physical base address and size of the registers map.
> +- interrupts: Interrupt number for the elm.
> +- interrupt-parent: The parent interrupt controller
> +
> +Optional properties:
> +- ti,hwmods: Name of the hwmod associated to the elm
> +
> +Example:
> +elm: elm@0 {
> + compatible  = "ti,elm";
> + reg = <0x4808 0x2000>;
> + interrupts = <4>;
> +};
> diff --git a/drivers/mtd/devices/Makefile b/drivers/mtd/devices/Makefile
> index 395733a..369a194 100644
> --- a/drivers/mtd/devices/Makefile
> +++ b/drivers/mtd/devices/Makefile
> @@ -17,8 +17,10 @@ obj-$(CONFIG_MTD_LART) += lart.o
>  obj-$(CONFIG_MTD_BLOCK2MTD)  += block2mtd.o
>  obj-$(CONFIG_MTD_DATAFLASH)  += mtd_dataflash.o
>  obj-$(CONFIG_MTD_M25P80) += m25p80.o
> +obj-$(CONFIG_MTD_NAND_OMAP_BCH)  += elm.o
>  obj-$(CONFIG_MTD_SPEAR_SMI)  += spear_smi.o
>  obj-$(CONFIG_MTD_SST25L) += sst25l.o
>  obj-$(CONFIG_MTD_BCM47XXSFLASH)  += bcm47xxsflash.o
>  
> -CFLAGS_docg3.o   += -I$(src)
> \ No newline at end of file
> +
> +CFLAGS_docg3.o   += -I$(src)
> diff --git a/drivers/mtd/devices/elm.c b/drivers/mtd/devices/elm.c
> new file mode 100644
> index 000..d2667f3
> --- /dev/null
> +++ b/drivers/mtd/devices/elm.c
> @@ -0,0 +1,418 @@
> +/*
> + * Error Location Module
> + *
> + * Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com/
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define ELM_IRQSTATUS0x018
> +#define ELM_IRQENABLE0x01c
> +#define ELM_LOCATION_CONFIG  0x020
> +#define ELM_PAGE_CTRL0x080
> +#define ELM_SYNDROME_FRAGMENT_0  0x400
> +#define ELM_SYNDROME_FRAGMENT_6  0x418
> +#define ELM_LOCATION_STATUS  0x800
> +#define ELM_ERROR_LOCATION_0 0x880
> +
> +/* ELM Interrupt Status Register */
> +#define INTR_STATUS_PAGE_VALID   BIT(8)
> +
> +/* ELM Interrupt Enable Register */
> +#define INTR_EN_PAGE_MASKBIT(8)
> +
> +/* ELM Location Configuration Register */
> +#define ECC_BCH_LEVEL_MASK   0x3
> +
> +/* ELM syndrome */
> +#define ELM_SYNDROME_VALID   BIT(16)
> +
> +/* ELM_LOCATION_STATUS Register */
> +#define ECC_CORRECTABLE_MASK BIT(8)
> +#define ECC_NB_ERRORS_MASK   0x1f
> +
> +/* ELM_ERROR_LOCATION_0-15 Registers */
> +#define ECC_ERROR_LOCATION_MASK  0x1fff
> +
> +#define ELM_ECC_SIZE 0x7ff
> +
> +#define SYNDROME_FRAGMENT_REG_SIZE   0x40
> +#define ERROR_LOCATION_SIZE  0x100
> +
> +struct elm_info {
> + struct device *dev;
> + void __iomem *elm_base;
> + struct completion elm_completion;
> + struct list_head list;
> + enum bch_ecc bch_type;
> +};
> +
> +static

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

2012-12-07 Thread Sekhar Nori 
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 avinashphi...@ti.com
 Cc: Grant Likely grant.lik...@secretlab.ca
 Cc: Rob Herring rob.herr...@calxeda.com
 Cc: Rob Landley r...@landley.net
 ---
 Changes since v2:
   - Remove __devinit  __devexit annotations
 
 Changes since v1:
   - Change build attribute to CONFIG_MTD_NAND_OMAP_BCH
   - Reduced indentation using by passing elm_info , offset
 to elm_read  elm_write
   - Removed syndrome manipulation functions.
 
 :00 100644 000... b88ee83... A
 Documentation/devicetree/bindings/mtd/elm.txt
 :100644 100644 395733a... 369a194... Mdrivers/mtd/devices/Makefile
 :00 100644 000... d2667f3... Adrivers/mtd/devices/elm.c
 :00 100644 000... d4fce31... A
 include/linux/platform_data/elm.h
  Documentation/devicetree/bindings/mtd/elm.txt |   17 +
  drivers/mtd/devices/Makefile  |4 +-
  drivers/mtd/devices/elm.c |  418 
 +
  include/linux/platform_data/elm.h |   54 
  4 files changed, 493 insertions(+), 1 deletions(-)
 
 diff --git a/Documentation/devicetree/bindings/mtd/elm.txt 
 b/Documentation/devicetree/bindings/mtd/elm.txt
 new file mode 100644
 index 000..b88ee83
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/mtd/elm.txt
 @@ -0,0 +1,17 @@
 +Error location module
 +
 +Required properties:
 +- compatible: Must be ti,elm
 +- reg: physical base address and size of the registers map.
 +- interrupts: Interrupt number for the elm.
 +- interrupt-parent: The parent interrupt controller
 +
 +Optional properties:
 +- ti,hwmods: Name of the hwmod associated to the elm
 +
 +Example:
 +elm: elm@0 {
 + compatible  = ti,elm;
 + reg = 0x4808 0x2000;
 + interrupts = 4;
 +};
 diff --git a/drivers/mtd/devices/Makefile b/drivers/mtd/devices/Makefile
 index 395733a..369a194 100644
 --- a/drivers/mtd/devices/Makefile
 +++ b/drivers/mtd/devices/Makefile
 @@ -17,8 +17,10 @@ obj-$(CONFIG_MTD_LART) += lart.o
  obj-$(CONFIG_MTD_BLOCK2MTD)  += block2mtd.o
  obj-$(CONFIG_MTD_DATAFLASH)  += mtd_dataflash.o
  obj-$(CONFIG_MTD_M25P80) += m25p80.o
 +obj-$(CONFIG_MTD_NAND_OMAP_BCH)  += elm.o
  obj-$(CONFIG_MTD_SPEAR_SMI)  += spear_smi.o
  obj-$(CONFIG_MTD_SST25L) += sst25l.o
  obj-$(CONFIG_MTD_BCM47XXSFLASH)  += bcm47xxsflash.o
  
 -CFLAGS_docg3.o   += -I$(src)
 \ No newline at end of file
 +
 +CFLAGS_docg3.o   += -I$(src)
 diff --git a/drivers/mtd/devices/elm.c b/drivers/mtd/devices/elm.c
 new file mode 100644
 index 000..d2667f3
 --- /dev/null
 +++ b/drivers/mtd/devices/elm.c
 @@ -0,0 +1,418 @@
 +/*
 + * Error Location Module
 + *
 + * Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com/
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License as published by
 + * the Free Software Foundation; either version 2 of the License, or
 + * (at your option) any later version.
 + *
 + * This program is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
 + * GNU General Public License for more details.
 + *
 + */
 +
 +#include linux/platform_device.h
 +#include linux/module.h
 +#include linux/interrupt.h
 +#include linux/io.h
 +#include linux/of.h
 +#include linux/pm_runtime.h
 +#include linux/platform_data/elm.h
 +
 +#define ELM_IRQSTATUS0x018
 +#define ELM_IRQENABLE0x01c
 +#define ELM_LOCATION_CONFIG  0x020
 +#define ELM_PAGE_CTRL0x080
 +#define ELM_SYNDROME_FRAGMENT_0  0x400
 +#define ELM_SYNDROME_FRAGMENT_6  0x418
 +#define ELM_LOCATION_STATUS  0x800
 +#define ELM_ERROR_LOCATION_0 0x880
 +
 +/* ELM Interrupt Status Register */
 +#define INTR_STATUS_PAGE_VALID   BIT(8)
 +
 +/* ELM Interrupt Enable Register */
 +#define INTR_EN_PAGE_MASKBIT(8)
 +
 +/* ELM Location Configuration Register */
 +#define ECC_BCH_LEVEL_MASK   0x3
 +
 +/* ELM syndrome */
 +#define ELM_SYNDROME_VALID   BIT(16)
 +
 +/* ELM_LOCATION_STATUS Register */
 +#define ECC_CORRECTABLE_MASK BIT(8)
 +#define ECC_NB_ERRORS_MASK   0x1f
 +
 +/* ELM_ERROR_LOCATION_0-15 Registers */
 +#define ECC_ERROR_LOCATION_MASK  0x1fff
 +
 +#define ELM_ECC_SIZE 0x7ff
 +
 +#define SYNDROME_FRAGMENT_REG_SIZE   0x40
 +#define ERROR_LOCATION_SIZE  0x100
 +
 +struct elm_info {
 + struct device *dev;
 + void __iomem *elm_base;
 + struct completion elm_completion;
 + struct list_head list;
 +

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

2012-11-29 Thread Philip, Avinash 
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 
Cc: Grant Likely 
Cc: Rob Herring 
Cc: Rob Landley 
---
Changes since v2:
- Remove __devinit & __devexit annotations

Changes since v1:
- Change build attribute to CONFIG_MTD_NAND_OMAP_BCH
- Reduced indentation using by passing elm_info , offset
  to elm_read & elm_write
- Removed syndrome manipulation functions.

:00 100644 000... b88ee83... A  
Documentation/devicetree/bindings/mtd/elm.txt
:100644 100644 395733a... 369a194... M  drivers/mtd/devices/Makefile
:00 100644 000... d2667f3... A  drivers/mtd/devices/elm.c
:00 100644 000... d4fce31... A  include/linux/platform_data/elm.h
 Documentation/devicetree/bindings/mtd/elm.txt |   17 +
 drivers/mtd/devices/Makefile  |4 +-
 drivers/mtd/devices/elm.c |  418 +
 include/linux/platform_data/elm.h |   54 
 4 files changed, 493 insertions(+), 1 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/elm.txt 
b/Documentation/devicetree/bindings/mtd/elm.txt
new file mode 100644
index 000..b88ee83
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/elm.txt
@@ -0,0 +1,17 @@
+Error location module
+
+Required properties:
+- compatible: Must be "ti,elm"
+- reg: physical base address and size of the registers map.
+- interrupts: Interrupt number for the elm.
+- interrupt-parent: The parent interrupt controller
+
+Optional properties:
+- ti,hwmods: Name of the hwmod associated to the elm
+
+Example:
+elm: elm@0 {
+   compatible  = "ti,elm";
+   reg = <0x4808 0x2000>;
+   interrupts = <4>;
+};
diff --git a/drivers/mtd/devices/Makefile b/drivers/mtd/devices/Makefile
index 395733a..369a194 100644
--- a/drivers/mtd/devices/Makefile
+++ b/drivers/mtd/devices/Makefile
@@ -17,8 +17,10 @@ obj-$(CONFIG_MTD_LART)   += lart.o
 obj-$(CONFIG_MTD_BLOCK2MTD)+= block2mtd.o
 obj-$(CONFIG_MTD_DATAFLASH)+= mtd_dataflash.o
 obj-$(CONFIG_MTD_M25P80)   += m25p80.o
+obj-$(CONFIG_MTD_NAND_OMAP_BCH)+= elm.o
 obj-$(CONFIG_MTD_SPEAR_SMI)+= spear_smi.o
 obj-$(CONFIG_MTD_SST25L)   += sst25l.o
 obj-$(CONFIG_MTD_BCM47XXSFLASH)+= bcm47xxsflash.o
 
-CFLAGS_docg3.o += -I$(src)
\ No newline at end of file
+
+CFLAGS_docg3.o += -I$(src)
diff --git a/drivers/mtd/devices/elm.c b/drivers/mtd/devices/elm.c
new file mode 100644
index 000..d2667f3
--- /dev/null
+++ b/drivers/mtd/devices/elm.c
@@ -0,0 +1,418 @@
+/*
+ * Error Location Module
+ *
+ * Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com/
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define ELM_IRQSTATUS  0x018
+#define ELM_IRQENABLE  0x01c
+#define ELM_LOCATION_CONFIG0x020
+#define ELM_PAGE_CTRL  0x080
+#define ELM_SYNDROME_FRAGMENT_00x400
+#define ELM_SYNDROME_FRAGMENT_60x418
+#define ELM_LOCATION_STATUS0x800
+#define ELM_ERROR_LOCATION_0   0x880
+
+/* ELM Interrupt Status Register */
+#define INTR_STATUS_PAGE_VALID BIT(8)
+
+/* ELM Interrupt Enable Register */
+#define INTR_EN_PAGE_MASK  BIT(8)
+
+/* ELM Location Configuration Register */
+#define ECC_BCH_LEVEL_MASK 0x3
+
+/* ELM syndrome */
+#define ELM_SYNDROME_VALID BIT(16)
+
+/* ELM_LOCATION_STATUS Register */
+#define ECC_CORRECTABLE_MASK   BIT(8)
+#define ECC_NB_ERRORS_MASK 0x1f
+
+/* ELM_ERROR_LOCATION_0-15 Registers */
+#define ECC_ERROR_LOCATION_MASK0x1fff
+
+#define ELM_ECC_SIZE   0x7ff
+
+#define SYNDROME_FRAGMENT_REG_SIZE 0x40
+#define ERROR_LOCATION_SIZE0x100
+
+struct elm_info {
+   struct device *dev;
+   void __iomem *elm_base;
+   struct completion elm_completion;
+   struct list_head list;
+   enum bch_ecc bch_type;
+};
+
+static LIST_HEAD(elm_devices);
+
+static void elm_write_reg(struct elm_info *info, int offset, u32 val)
+{
+   writel(val, info->elm_base + offset);
+}
+
+static u32 elm_read_reg(struct elm_info *info, int offset)
+{
+   return readl(info->elm_base + offset);
+}
+
+/**
+ * elm_config - Configure ELM module
+ * @info:  elm info

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

2012-11-29 Thread Philip, Avinash 
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 avinashphi...@ti.com
Cc: Grant Likely grant.lik...@secretlab.ca
Cc: Rob Herring rob.herr...@calxeda.com
Cc: Rob Landley r...@landley.net
---
Changes since v2:
- Remove __devinit  __devexit annotations

Changes since v1:
- Change build attribute to CONFIG_MTD_NAND_OMAP_BCH
- Reduced indentation using by passing elm_info , offset
  to elm_read  elm_write
- Removed syndrome manipulation functions.

:00 100644 000... b88ee83... A  
Documentation/devicetree/bindings/mtd/elm.txt
:100644 100644 395733a... 369a194... M  drivers/mtd/devices/Makefile
:00 100644 000... d2667f3... A  drivers/mtd/devices/elm.c
:00 100644 000... d4fce31... A  include/linux/platform_data/elm.h
 Documentation/devicetree/bindings/mtd/elm.txt |   17 +
 drivers/mtd/devices/Makefile  |4 +-
 drivers/mtd/devices/elm.c |  418 +
 include/linux/platform_data/elm.h |   54 
 4 files changed, 493 insertions(+), 1 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/elm.txt 
b/Documentation/devicetree/bindings/mtd/elm.txt
new file mode 100644
index 000..b88ee83
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/elm.txt
@@ -0,0 +1,17 @@
+Error location module
+
+Required properties:
+- compatible: Must be ti,elm
+- reg: physical base address and size of the registers map.
+- interrupts: Interrupt number for the elm.
+- interrupt-parent: The parent interrupt controller
+
+Optional properties:
+- ti,hwmods: Name of the hwmod associated to the elm
+
+Example:
+elm: elm@0 {
+   compatible  = ti,elm;
+   reg = 0x4808 0x2000;
+   interrupts = 4;
+};
diff --git a/drivers/mtd/devices/Makefile b/drivers/mtd/devices/Makefile
index 395733a..369a194 100644
--- a/drivers/mtd/devices/Makefile
+++ b/drivers/mtd/devices/Makefile
@@ -17,8 +17,10 @@ obj-$(CONFIG_MTD_LART)   += lart.o
 obj-$(CONFIG_MTD_BLOCK2MTD)+= block2mtd.o
 obj-$(CONFIG_MTD_DATAFLASH)+= mtd_dataflash.o
 obj-$(CONFIG_MTD_M25P80)   += m25p80.o
+obj-$(CONFIG_MTD_NAND_OMAP_BCH)+= elm.o
 obj-$(CONFIG_MTD_SPEAR_SMI)+= spear_smi.o
 obj-$(CONFIG_MTD_SST25L)   += sst25l.o
 obj-$(CONFIG_MTD_BCM47XXSFLASH)+= bcm47xxsflash.o
 
-CFLAGS_docg3.o += -I$(src)
\ No newline at end of file
+
+CFLAGS_docg3.o += -I$(src)
diff --git a/drivers/mtd/devices/elm.c b/drivers/mtd/devices/elm.c
new file mode 100644
index 000..d2667f3
--- /dev/null
+++ b/drivers/mtd/devices/elm.c
@@ -0,0 +1,418 @@
+/*
+ * Error Location Module
+ *
+ * Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com/
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include linux/platform_device.h
+#include linux/module.h
+#include linux/interrupt.h
+#include linux/io.h
+#include linux/of.h
+#include linux/pm_runtime.h
+#include linux/platform_data/elm.h
+
+#define ELM_IRQSTATUS  0x018
+#define ELM_IRQENABLE  0x01c
+#define ELM_LOCATION_CONFIG0x020
+#define ELM_PAGE_CTRL  0x080
+#define ELM_SYNDROME_FRAGMENT_00x400
+#define ELM_SYNDROME_FRAGMENT_60x418
+#define ELM_LOCATION_STATUS0x800
+#define ELM_ERROR_LOCATION_0   0x880
+
+/* ELM Interrupt Status Register */
+#define INTR_STATUS_PAGE_VALID BIT(8)
+
+/* ELM Interrupt Enable Register */
+#define INTR_EN_PAGE_MASK  BIT(8)
+
+/* ELM Location Configuration Register */
+#define ECC_BCH_LEVEL_MASK 0x3
+
+/* ELM syndrome */
+#define ELM_SYNDROME_VALID BIT(16)
+
+/* ELM_LOCATION_STATUS Register */
+#define ECC_CORRECTABLE_MASK   BIT(8)
+#define ECC_NB_ERRORS_MASK 0x1f
+
+/* ELM_ERROR_LOCATION_0-15 Registers */
+#define ECC_ERROR_LOCATION_MASK0x1fff
+
+#define ELM_ECC_SIZE   0x7ff
+
+#define SYNDROME_FRAGMENT_REG_SIZE 0x40
+#define ERROR_LOCATION_SIZE0x100
+
+struct elm_info {
+   struct device *dev;
+   void __iomem *elm_base;
+   struct completion elm_completion;
+   struct list_head list;
+   enum bch_ecc bch_type;
+};
+
+static LIST_HEAD(elm_devices);
+
+static void elm_write_reg(struct elm_info *info, int offset, u32 val)
+{
+   writel(val, info-elm_base +