[PATCH 2/4] mtd: devices: elm: Add support for ELM error correction
Peter Korsgaard
jacmet at sunsite.dk
Mon Oct 15 15:40:47 EDT 2012
>>>>> Philip, Avinash <avinashphilip at ti.com> writes:
> Platforms containing the ELM module can be used to correct errors
> reported by BCH 4, 8 & 16 bit ECC scheme. For now only 4 & 8 bit
> support is added.
This sounds odd to me. What about something like:
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.
> +++ 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_OMAP2) += elm.o
You seem to only use it in 4/4 if CONFIG_MTD_NAND_OMAP_BCH is set, so it
probably makes more sense to use that symbol to not needlessly include
it if it won't be used.
> +++ b/drivers/mtd/devices/elm.c
> @@ -0,0 +1,440 @@
> +/*
> + * 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_CONFIG 0x020
> +#define ELM_PAGE_CTRL 0x080
> +#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_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_MASK 0x1fff
> +
> +#define ELM_ECC_SIZE 0x7ff
> +
> +#define SYNDROME_FRAGMENT_REG_SIZE 0x40
> +#define ERROR_LOCATION_SIZE 0x100
> +#define MAX_BCH_ELM_ERROR 16
> +#define ELM_FRAGMENT_REG 7
> +
> +typedef u32 syn_t[ELM_FRAGMENT_REG];
> +typedef u32 elm_error_t[MAX_BCH_ELM_ERROR];
> +
> +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(void *offset, u32 val)
> +{
> + writel(val, offset);
> +}
> +
> +static u32 elm_read_reg(void *offset)
> +{
> + return readl(offset);
> +}
As written these read/write wrappers don't add anything. How about
passing struct elm_info and offset as an integer so you can drop
elm_base from all call sites, E.G.:
static void elm_write_reg(struct elm_info *info, int offset, u32 val)
{
writel(val, info->elm_base + offset);
}
> +
> +/**
> + * elm_config - Configure ELM module
> + * @info: elm info
> + */
> +static void elm_config(struct elm_info *info)
> +{
> + u32 reg_val;
> +
> + reg_val = (info->bch_type & ECC_BCH_LEVEL_MASK) | (ELM_ECC_SIZE << 16);
> + elm_write_reg(info->elm_base + ELM_LOCATION_CONFIG, reg_val);
> +}
> +
> +/**
> + * elm_configure_page_mode - Enable/Disable page mode
> + * @info: elm info
> + * @index: index number of syndrome fragment vector
> + * @enable: enable/disable flag for page mode
> + *
> + * Enable page mode for syndrome fragment index
> + */
> +static void elm_configure_page_mode(struct elm_info *info, int index,
> + bool enable)
> +{
> + u32 reg_val;
> +
> + reg_val = elm_read_reg(info->elm_base + ELM_PAGE_CTRL);
> + if (enable)
> + reg_val |= BIT(index); /* enable page mode */
> + else
> + reg_val &= ~BIT(index); /* disable page mode */
> +
> + elm_write_reg(info->elm_base + ELM_PAGE_CTRL, reg_val);
> +}
> +
> +static void rearrange_bch4(u8 *syndrome)
> +{
> + /*
> + * BCH4 has 52 bit used for ecc, but OOB stored with
> + * nibble 0 appended, removes appended 0 nibble
> + */
> + u64 *dst = (u64 *)syndrome;
> + *dst >>= 4;
> +}
> +
> +/**
> + * elm_reverse_eccdata - Reverse ecc data
> + * @info: elm info
> + * @ecc_data: buffer for calculated ecc
> + * @syndrome: buffer for syndrome polynomial
> + *
> + * ecc_data has to be reversed for syndrome vector
> + */
> +static void elm_reverse_eccdata(struct elm_info *info, u8 *ecc_data,
> + u8 *syndrome)
> +{
> + int i;
> + int bch_size = info->bch_type ? BCH8_ECC_OOB_BYTES : BCH4_SIZE;
> +
> + for (i = 0; i < bch_size; i++)
> + syndrome[bch_size - 1 - i] = ecc_data[i];
> +
> + /*
> + * syndrome polynomial to be rearranged for BCH 4 ecc scheme as 52
> + * bits being used and 4 bits being appended in syndrome vector
> + */
> + if (info->bch_type == BCH4_ECC)
> + rearrange_bch4(syndrome);
> +}
> +
> +/**
> + * elm_load_syndrome - Load ELM syndrome reg
> + * @info: elm info
> + * @index: syndrome fragment index
> + * @syndrome: Syndrome polynomial
> + *
> + * Load syndrome fragment registers with syndrome polynomial
> + */
> +static void elm_load_syndrome(struct elm_info *info, int index, u8 *syndrome)
> +{
> + int i;
> + int max = info->bch_type ? BCH8_SYNDROME_SIZE : BCH4_SYNDROME_SIZE;
> + void *syn_frag_base = info->elm_base + ELM_SYNDROME_FRAGMENT_0;
> + syn_t *syn_fragment;
> + u32 reg_val;
> +
> + elm_configure_page_mode(info, index, true);
> + syn_fragment = syn_frag_base + SYNDROME_FRAGMENT_REG_SIZE * index;
> +
> + for (i = 0; i < max; i++) {
> + reg_val = syndrome[0] | syndrome[1] << 8 | syndrome[2] << 16 |
> + syndrome[3] << 24;
> + elm_write_reg(*syn_fragment + i, reg_val);
> + /* Update syndrome polynomial pointer */
> + syndrome += 4;
> + }
> +}
> +
> +/**
> + * elm_start_processing - start elm syndrome processing
> + * @info: elm info
> + * @err_vec: elm error vectors
> + *
> + * Set syndrome valid bit for syndrome fragment registers for which
> + * elm syndrome fragment registers are loaded. This enables elm module
> + * to start processing syndrome vectors.
> + */
> +static void elm_start_processing(struct elm_info *info,
> + struct elm_errorvec *err_vec)
> +{
> + int i;
> + void *offset;
> + u32 reg_val;
> +
> + /*
> + * Set syndrome vector valid so that ELM module will process it for
> + * vectors error is reported
> + */
> + for (i = 0; i < ERROR_VECTOR_MAX; i++) {
> + if (err_vec[i].error_reported) {
> + offset = info->elm_base + ELM_SYNDROME_FRAGMENT_6 + i *
> + SYNDROME_FRAGMENT_REG_SIZE;
> + reg_val = elm_read_reg(offset);
> + reg_val |= ELM_SYNDROME_VALID;
> + elm_write_reg(offset, reg_val);
> + }
> + }
> +}
> +
> +/**
> + * elm_error_correction - locate correctable error position
> + * @info: elm info
> + * @err_vec: elm error vectors
> + *
> + * On completion of processing by elm module, error location status
> + * register updated with correctable/uncorrectable error information.
> + * In case of correctable errors, number of errors located from
> + * elm location status register & read the these many positions from
> + * elm error location register.
> + */
> +static void elm_error_correction(struct elm_info *info,
> + struct elm_errorvec *err_vec)
> +{
> + int i, j, errors = 0;
> + void *err_loc_base = info->elm_base + ELM_ERROR_LOCATION_0;
> + elm_error_t *err_loc;
> + void *offset;
> + u32 reg_val;
> +
> + for (i = 0; i < ERROR_VECTOR_MAX; i++) {
> + /* check error reported */
> + if (err_vec[i].error_reported) {
> + offset = info->elm_base + ELM_LOCATION_STATUS +
> + i * ERROR_LOCATION_SIZE;
> + reg_val = elm_read_reg(offset);
> + /* check correctable error or not */
> + if (reg_val & ECC_CORRECTABLE_MASK) {
> + err_loc = err_loc_base +
> + i * ERROR_LOCATION_SIZE;
> + /* read count of correctable errors */
> + err_vec[i].error_count = reg_val &
> + ECC_NB_ERRORS_MASK;
> +
> + /* update the error locations in error vector */
> + for (j = 0; j < err_vec[i].error_count; j++) {
> +
> + reg_val = elm_read_reg(*err_loc + j);
> + err_vec[i].error_loc[j] = reg_val &
> + ECC_ERROR_LOCATION_MASK;
> + }
> +
> + errors += err_vec[i].error_count;
> + } else {
> + err_vec[i].error_uncorrectable++;
> + }
> +
> + /* clearing interrupts for processed error vectors */
> + elm_write_reg(info->elm_base + ELM_IRQSTATUS, BIT(i));
> +
> + /* disable page mode */
> + elm_configure_page_mode(info, i, false);
> + }
> + }
> +
> + return;
> +}
> +
> +/**
> + * elm_decode_bch_error_page - Locate error position
> + * @info: elm info
> + * @ecc_calc: calculated ECC bytes from GPMC
> + * @err_vec: elm error vectors
> + *
> + * Called with one or greater reported and is vectors with error reported
> + * is updated in err_vec[].error_reported
> + */
> +void elm_decode_bch_error_page(struct device *dev, u8 *ecc_calc,
> + struct elm_errorvec *err_vec)
> +{
> + int i;
> + u8 syndrome[BCH_MAX_ECC_BYTES_PER_SECTOR] = {0}, *syn_p;
Why do you need to keep the entire syndrome around? You seem to only use
it between elm_reverse_eccdata() and elm_load_syndrome(), so it could as
well be BCH8_ECC_OOB_BYTES long (or rather a multiple of sizeof(u32).
It would also be good to do the shuffeling directly in elm_load_syndrome
so you don't need the extra copy.
> + struct elm_info *info = dev_get_drvdata(dev);
> + u32 reg_val;
> +
> + /* enable page mode interrupt */
> + reg_val = elm_read_reg(info->elm_base + ELM_IRQSTATUS);
> + elm_write_reg(info->elm_base + ELM_IRQSTATUS,
> + reg_val & INTR_STATUS_PAGE_VALID);
> + elm_write_reg(info->elm_base + ELM_IRQENABLE, INTR_EN_PAGE_MASK);
> +
> + syn_p = syndrome;
> + for (i = 0; i < ERROR_VECTOR_MAX; i++) {
> + if (err_vec[i].error_reported) {
> + elm_reverse_eccdata(info, ecc_calc, syn_p);
> + elm_load_syndrome(info, i, syn_p);
> + }
> +
> + ecc_calc += info->bch_type ? BCH8_SIZE : BCH4_SIZE;
> + syn_p += OOB_SECTOR_SIZE;
> + }
> +
> + elm_start_processing(info, err_vec);
> +
> + /*
> + * In case of error reported, wait for ELM module to finish
> + * locating error correction
> + */
> + wait_for_completion(&info->elm_completion);
> +
> + /* disable page mode interrupt */
> + reg_val = elm_read_reg(info->elm_base + ELM_IRQENABLE);
> + elm_write_reg(info->elm_base + ELM_IRQENABLE,
> + reg_val & ~INTR_EN_PAGE_MASK);
> + elm_error_correction(info, err_vec);
> +}
> +EXPORT_SYMBOL(elm_decode_bch_error_page);
> +
> +static irqreturn_t elm_isr(int this_irq, void *dev_id)
> +{
> + u32 reg_val;
> + struct elm_info *info = dev_id;
> +
> + reg_val = elm_read_reg(info->elm_base + ELM_IRQSTATUS);
> +
> + /* all error vectors processed */
> + if (reg_val & INTR_STATUS_PAGE_VALID) {
> + elm_write_reg(info->elm_base + ELM_IRQSTATUS,
> + reg_val & INTR_STATUS_PAGE_VALID);
> + complete(&info->elm_completion);
> + return IRQ_HANDLED;
> + }
> +
> + return IRQ_NONE;
> +}
> +
> +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;
> + }
> + }
> +
> + return NULL;
> +}
> +EXPORT_SYMBOL(elm_request);
> +
> +static int __devinit elm_probe(struct platform_device *pdev)
> +{
> + int ret = 0;
> + struct resource *res, *irq;
> + struct elm_info *info;
> +
> + info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
> + if (!info) {
> + dev_err(&pdev->dev, "failed to allocate memory\n");
> + return -ENOMEM;
> + }
> +
> + info->dev = &pdev->dev;
> +
> + irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> + if (!irq) {
> + dev_err(&pdev->dev, "no irq resource defined\n");
> + return -ENODEV;
> + }
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res) {
> + dev_err(&pdev->dev, "no memory resource defined\n");
> + return -ENODEV;
> + }
> +
> +
> + info->elm_base = devm_request_and_ioremap(&pdev->dev, res);
> + if (!info->elm_base)
> + return -EADDRNOTAVAIL;
> +
> +
> + ret = devm_request_irq(&pdev->dev, irq->start, elm_isr, 0,
> + pdev->name, info);
> + if (ret) {
> + dev_err(&pdev->dev, "failure requesting irq %i\n", irq->start);
> + return ret;
> + }
> +
> + pm_runtime_enable(&pdev->dev);
> + if (pm_runtime_get_sync(&pdev->dev)) {
> + ret = -EINVAL;
> + pm_runtime_disable(&pdev->dev);
> + dev_err(&pdev->dev, "can't enable clock\n");
> + return ret;
> + }
> +
> + init_completion(&info->elm_completion);
> + INIT_LIST_HEAD(&info->list);
> + list_add(&info->list, &elm_devices);
> + platform_set_drvdata(pdev, info);
> + return ret;
> +}
> +
> +static int __devexit elm_remove(struct platform_device *pdev)
> +{
> + pm_runtime_put_sync(&pdev->dev);
> + pm_runtime_disable(&pdev->dev);
> + platform_set_drvdata(pdev, NULL);
> + return 0;
> +}
> +
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id elm_of_match[] = {
> + { .compatible = "ti,elm" },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, elm_of_match);
> +#endif
> +
> +static struct platform_driver elm_driver = {
> + .driver = {
> + .name = "elm",
> + .of_match_table = of_match_ptr(elm_of_match),
> + .owner = THIS_MODULE,
> + },
> + .probe = elm_probe,
> + .remove = __devexit_p(elm_remove),
> +};
> +
> +module_platform_driver(elm_driver);
> +
> +MODULE_DESCRIPTION("ELM driver for BCH error correction");
> +MODULE_AUTHOR("Texas Instruments");
> +MODULE_ALIAS("platform: elm");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/platform_data/elm.h b/include/linux/platform_data/elm.h
> new file mode 100644
> index 0000000..eb53163
> --- /dev/null
> +++ b/include/linux/platform_data/elm.h
> @@ -0,0 +1,60 @@
> +/*
> + * BCH 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.
> + *
> + */
> +
> +#ifndef __ELM_H
> +#define __ELM_H
> +
> +enum bch_ecc {
> + BCH4_ECC = 0,
> + BCH8_ECC,
> + BCH16_ECC,
It probably makes more sense to not provide the enum value for BCH16 as
you don't support it.
> +};
> +
> +/* ELM support 8 error syndrome process */
> +#define ERROR_VECTOR_MAX 8
> +#define OOB_SECTOR_SIZE 16
> +
> +#define BCH_MAX_ECC_BYTES_PER_SECTOR (OOB_SECTOR_SIZE * ERROR_VECTOR_MAX)
> +
> +#define BCH8_ECC_OOB_BYTES 13
> +/* RBL requires 14 byte even though BCH8 uses only 13 byte */
> +#define BCH8_SIZE (BCH8_ECC_OOB_BYTES + 1)
> +#define BCH4_SIZE 7
> +
> +#define BCH8_SYNDROME_SIZE 4 /* 13 bytes of ecc */
> +#define BCH4_SYNDROME_SIZE 2 /* 7 bytes of ecc */
> +
> +/**
> + * struct elm_errorvec - error vector for elm
> + * @error_reported: set true for vectors error is reported
> + *
> + * @error_count: number of correctable errors in the sector
> + * @error_uncorrectable: number of uncorrectable errors
> + * @error_loc: buffer for error location
> + *
> + */
> +struct elm_errorvec {
> + bool error_reported;
> + int error_count;
> + int error_uncorrectable;
> + int error_loc[ERROR_VECTOR_MAX];
> +};
> +
> +void elm_decode_bch_error_page(struct device *dev, u8 *ecc_calc,
> + struct elm_errorvec *err_vec);
> +struct device *elm_request(enum bch_ecc bch_type);
> +#endif /* __ELM_H */
> --
> 1.7.0.4
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
--
Bye, Peter Korsgaard
More information about the linux-mtd
mailing list