On Wed, Oct 03, 2012 at 20:40:46, Peter Meerwald wrote:
>
> some minor nitpicks below
>
> > + *
> > + * 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
>
> should probably be: "... & read the positions from ..."?
Ok I will correct it.
>
> > + * 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++;
> > + }
>
> extra braces above
As per coding style
This does not apply if only one branch of a conditional statement is a single
statement; in the latter case use braces in both branches:
>
> > +
> > + /* 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
>
> should be dev, not info
Ok I will correct it.
>
> > + * @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
> > + */
>
> I do not understand the statement "Called with one ..."
elm_decode_bch_error_page() API will called from nand driver if and only if
errors are present while reading page. Errors can be reported in one or
multiple error vectors.
I will reword it as.
Called with one or more error reported vectors & vectors with error reported
is updated in err_vec[].error_reported
>
[snip]
> > +
> > +#define BCH8_ECC_OOB_BYTES 13
> > +/* RBL requires 14 byte even though BCH8 uses only 13 byte */
>
> not sure what RBL is?
RBL stands for Rom boot Loader
>
> > +#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 */
> >
>
> --
>
> Peter Meerwald
> +43-664-2444418 (mobile)
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
