Re: [PATCH v2 3/6] mtd: nand: gpmi: may use minimum required ecc for 744 oobsize NAND

From: Huang Shijie
Date: Tue Aug 18 2015 - 02:41:09 EST


On Mon, Aug 17, 2015 at 10:24:49PM -0500, Han Xu wrote:
> By default NAND driver will choose the highest ecc strength that oob
> could contain, in this case, for some 8K+744 NAND flash, the ecc
> strength will be up to 52bit, which beyonds the i.MX6QDL BCH capability
> (40bit).
>
> This patch allows the NAND driver try to use minimum required ecc
> strength if it failed to use the highest ecc, even without explicitly
> claiming "fsl,use-minimum-ecc" in dts.
>
> Signed-off-by: Han Xu <b45815@xxxxxxxxxxxxx>
> ---
> drivers/mtd/nand/gpmi-nand/gpmi-nand.c | 24 +++++++++++++-----------
> 1 file changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> index dfd0ba1..01d24dd 100644
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> @@ -136,7 +136,7 @@ static inline bool gpmi_check_ecc(struct gpmi_nand_data *this)
> *
> * We may have available oob space in this case.
> */
> -static bool set_geometry_by_ecc_info(struct gpmi_nand_data *this)
> +static int set_geometry_by_ecc_info(struct gpmi_nand_data *this)
> {
> struct bch_geometry *geo = &this->bch_geometry;
> struct mtd_info *mtd = &this->mtd;
> @@ -145,7 +145,7 @@ static bool set_geometry_by_ecc_info(struct gpmi_nand_data *this)
> unsigned int block_mark_bit_offset;
>
> if (!(chip->ecc_strength_ds > 0 && chip->ecc_step_ds > 0))
> - return false;
> + return -EINVAL;
>
> switch (chip->ecc_step_ds) {
> case SZ_512:
> @@ -158,19 +158,19 @@ static bool set_geometry_by_ecc_info(struct gpmi_nand_data *this)
> dev_err(this->dev,
> "unsupported nand chip. ecc bits : %d, ecc size : %d\n",
> chip->ecc_strength_ds, chip->ecc_step_ds);
> - return false;
> + return -EINVAL;
> }
> geo->ecc_chunk_size = chip->ecc_step_ds;
> geo->ecc_strength = round_up(chip->ecc_strength_ds, 2);
> if (!gpmi_check_ecc(this))
> - return false;
> + return -EINVAL;
>
> /* Keep the C >= O */
> if (geo->ecc_chunk_size < mtd->oobsize) {
> dev_err(this->dev,
> "unsupported nand chip. ecc size: %d, oob size : %d\n",
> chip->ecc_step_ds, mtd->oobsize);
> - return false;
> + return -EINVAL;
> }
>
> /* The default value, see comment in the legacy_set_geometry(). */
> @@ -242,7 +242,7 @@ static bool set_geometry_by_ecc_info(struct gpmi_nand_data *this)
> + ALIGN(geo->ecc_chunk_count, 4);
>
> if (!this->swap_block_mark)
> - return true;
> + return 0;
>
> /* For bit swap. */
> block_mark_bit_offset = mtd->writesize * 8 -
> @@ -251,7 +251,7 @@ static bool set_geometry_by_ecc_info(struct gpmi_nand_data *this)
>
> geo->block_mark_byte_offset = block_mark_bit_offset / 8;
> geo->block_mark_bit_offset = block_mark_bit_offset % 8;
> - return true;
> + return 0;
> }
>
> static int legacy_set_geometry(struct gpmi_nand_data *this)
> @@ -366,10 +366,12 @@ static int legacy_set_geometry(struct gpmi_nand_data *this)
>
> int common_nfc_set_geometry(struct gpmi_nand_data *this)
> {
> - if (of_property_read_bool(this->dev->of_node, "fsl,use-minimum-ecc")
> - && set_geometry_by_ecc_info(this))
> - return 0;
> - return legacy_set_geometry(this);
> +

Please remove the empty line.

> + if ((of_property_read_bool(this->dev->of_node, "fsl,use-minimum-ecc"))
> + || legacy_set_geometry(this))
If the legacy_set_geometry() fails, it will print out an error message.
could you please also change the error log?

thanks
Huang Shijie

> + return set_geometry_by_ecc_info(this);
> +
> + return 0;
> }

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