Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1380219 > unrolled thread

[PATCH 10/12] mtd: nand: read ECC algorithm from the new field

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 10/12] mtd: nand: read ECC algorithm from the new field Rafał Miłecki <zajec5@gmail.com> - 2016-04-15 22:00 +0200
    Re: [PATCH 10/12] mtd: nand: read ECC algorithm from the new field Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-04-16 10:00 +0200
      Re: [PATCH 10/12] mtd: nand: read ECC algorithm from the new field Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-04-16 10:20 +0200
      Re: [PATCH 10/12] mtd: nand: read ECC algorithm from the new field Rafał Miłecki <zajec5@gmail.com> - 2016-04-17 18:30 +0200
        Re: [PATCH 10/12] mtd: nand: read ECC algorithm from the new field Rafał Miłecki <zajec5@gmail.com> - 2016-04-17 19:00 +0200
        Re: [PATCH 10/12] mtd: nand: read ECC algorithm from the new field Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-04-17 19:00 +0200
          Re: [PATCH 10/12] mtd: nand: read ECC algorithm from the new field Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-04-17 19:00 +0200

#1380219 — [PATCH 10/12] mtd: nand: read ECC algorithm from the new field

Now we have all drivers properly setting this new field we can start
using it and proceed with deprecating NAND_ECC_SOFT_BCH.

Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
---
 drivers/mtd/nand/nand_base.c | 98 ++++++++++++++++++++++++--------------------
 1 file changed, 53 insertions(+), 45 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index e1f3cf8..ffd1b32 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -4154,7 +4154,7 @@ int nand_scan_tail(struct mtd_info *mtd)
 	/*
 	 * If no default placement scheme is given, select an appropriate one.
 	 */
-	if (!mtd->ooblayout && (ecc->mode != NAND_ECC_SOFT_BCH)) {
+	if (!mtd->ooblayout && ecc->algo != NAND_ECC_BCH) {
 		switch (mtd->oobsize) {
 		case 8:
 		case 16:
@@ -4248,51 +4248,59 @@ int nand_scan_tail(struct mtd_info *mtd)
 		ecc->algo = NAND_ECC_HAMMING;
 
 	case NAND_ECC_SOFT:
-		ecc->calculate = nand_calculate_ecc;
-		ecc->correct = nand_correct_data;
-		ecc->read_page = nand_read_page_swecc;
-		ecc->read_subpage = nand_read_subpage;
-		ecc->write_page = nand_write_page_swecc;
-		ecc->read_page_raw = nand_read_page_raw;
-		ecc->write_page_raw = nand_write_page_raw;
-		ecc->read_oob = nand_read_oob_std;
-		ecc->write_oob = nand_write_oob_std;
-		if (!ecc->size)
-			ecc->size = 256;
-		ecc->bytes = 3;
-		ecc->strength = 1;
-		break;
-
 	case NAND_ECC_SOFT_BCH:
-		if (!mtd_nand_has_bch()) {
-			WARN(1, "CONFIG_MTD_NAND_ECC_BCH not enabled\n");
-			ret = -EINVAL;
-			goto err_free;
-		}
-		ecc->calculate = nand_bch_calculate_ecc;
-		ecc->correct = nand_bch_correct_data;
-		ecc->read_page = nand_read_page_swecc;
-		ecc->read_subpage = nand_read_subpage;
-		ecc->write_page = nand_write_page_swecc;
-		ecc->read_page_raw = nand_read_page_raw;
-		ecc->write_page_raw = nand_write_page_raw;
-		ecc->read_oob = nand_read_oob_std;
-		ecc->write_oob = nand_write_oob_std;
-		/*
-		 * Board driver should supply ecc.size and ecc.strength values
-		 * to select how many bits are correctable. Otherwise, default
-		 * to 4 bits for large page devices.
-		 */
-		if (!ecc->size && (mtd->oobsize >= 64)) {
-			ecc->size = 512;
-			ecc->strength = 4;
-		}
+		switch (ecc->algo) {
+		case NAND_ECC_HAMMING:
+			ecc->calculate = nand_calculate_ecc;
+			ecc->correct = nand_correct_data;
+			ecc->read_page = nand_read_page_swecc;
+			ecc->read_subpage = nand_read_subpage;
+			ecc->write_page = nand_write_page_swecc;
+			ecc->read_page_raw = nand_read_page_raw;
+			ecc->write_page_raw = nand_write_page_raw;
+			ecc->read_oob = nand_read_oob_std;
+			ecc->write_oob = nand_write_oob_std;
+			if (!ecc->size)
+				ecc->size = 256;
+			ecc->bytes = 3;
+			ecc->strength = 1;
+			break;
+		case NAND_ECC_BCH:
+			if (!mtd_nand_has_bch()) {
+				WARN(1, "CONFIG_MTD_NAND_ECC_BCH not enabled\n");
+				ret = -EINVAL;
+				goto err_free;
+			}
+			ecc->calculate = nand_bch_calculate_ecc;
+			ecc->correct = nand_bch_correct_data;
+			ecc->read_page = nand_read_page_swecc;
+			ecc->read_subpage = nand_read_subpage;
+			ecc->write_page = nand_write_page_swecc;
+			ecc->read_page_raw = nand_read_page_raw;
+			ecc->write_page_raw = nand_write_page_raw;
+			ecc->read_oob = nand_read_oob_std;
+			ecc->write_oob = nand_write_oob_std;
+			/*
+			* Board driver should supply ecc.size and ecc.strength
+			* values to select how many bits are correctable.
+			* Otherwise, default to 4 bits for large page devices.
+			*/
+			if (!ecc->size && (mtd->oobsize >= 64)) {
+				ecc->size = 512;
+				ecc->strength = 4;
+			}
 
-		/* See nand_bch_init() for details. */
-		ecc->bytes = 0;
-		ecc->priv = nand_bch_init(mtd);
-		if (!ecc->priv) {
-			WARN(1, "BCH ECC initialization failed!\n");
+			/* See nand_bch_init() for details. */
+			ecc->bytes = 0;
+			ecc->priv = nand_bch_init(mtd);
+			if (!ecc->priv) {
+				WARN(1, "BCH ECC initialization failed!\n");
+				ret = -EINVAL;
+				goto err_free;
+			}
+			break;
+		default:
+			WARN(1, "Unsupported ECC algorithm!\n");
 			ret = -EINVAL;
 			goto err_free;
 		}
@@ -4478,7 +4486,7 @@ void nand_release(struct mtd_info *mtd)
 {
 	struct nand_chip *chip = mtd_to_nand(mtd);
 
-	if (chip->ecc.mode == NAND_ECC_SOFT_BCH)
+	if (chip->ecc.algo == NAND_ECC_BCH)
 		nand_bch_free((struct nand_bch_control *)chip->ecc.priv);
 
 	mtd_device_unregister(mtd);
-- 
1.8.4.5

[toc] | [next] | [standalone]

#1380502

On Fri, 15 Apr 2016 21:54:10 +0200
Rafał Miłecki <zajec5@gmail.com> wrote:

> Now we have all drivers properly setting this new field we can start
> using it and proceed with deprecating NAND_ECC_SOFT_BCH.
> 
> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
> ---
>  drivers/mtd/nand/nand_base.c | 98 ++++++++++++++++++++++++--------------------
>  1 file changed, 53 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index e1f3cf8..ffd1b32 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -4154,7 +4154,7 @@ int nand_scan_tail(struct mtd_info *mtd)
>  	/*
>  	 * If no default placement scheme is given, select an appropriate one.
>  	 */
> -	if (!mtd->ooblayout && (ecc->mode != NAND_ECC_SOFT_BCH)) {
> +	if (!mtd->ooblayout && ecc->algo != NAND_ECC_BCH) {

Should be:

	if (!mtd->ooblayout && ecc->algo == NAND_ECC_SOFT &&
	    ecc->algo != NAND_ECC_BCH) {

Otherwise you're also taking the NAND_ECC_HW + NAND_ECC_BCH into
account. 

>  		switch (mtd->oobsize) {
>  		case 8:
>  		case 16:
> @@ -4248,51 +4248,59 @@ int nand_scan_tail(struct mtd_info *mtd)
>  		ecc->algo = NAND_ECC_HAMMING;
>  
>  	case NAND_ECC_SOFT:
> -		ecc->calculate = nand_calculate_ecc;
> -		ecc->correct = nand_correct_data;
> -		ecc->read_page = nand_read_page_swecc;
> -		ecc->read_subpage = nand_read_subpage;
> -		ecc->write_page = nand_write_page_swecc;
> -		ecc->read_page_raw = nand_read_page_raw;
> -		ecc->write_page_raw = nand_write_page_raw;
> -		ecc->read_oob = nand_read_oob_std;
> -		ecc->write_oob = nand_write_oob_std;
> -		if (!ecc->size)
> -			ecc->size = 256;
> -		ecc->bytes = 3;
> -		ecc->strength = 1;
> -		break;
> -
>  	case NAND_ECC_SOFT_BCH:

Shouldn't we drop this case?

> -		if (!mtd_nand_has_bch()) {
> -			WARN(1, "CONFIG_MTD_NAND_ECC_BCH not enabled\n");
> -			ret = -EINVAL;
> -			goto err_free;
> -		}
> -		ecc->calculate = nand_bch_calculate_ecc;
> -		ecc->correct = nand_bch_correct_data;
> -		ecc->read_page = nand_read_page_swecc;
> -		ecc->read_subpage = nand_read_subpage;
> -		ecc->write_page = nand_write_page_swecc;
> -		ecc->read_page_raw = nand_read_page_raw;
> -		ecc->write_page_raw = nand_write_page_raw;
> -		ecc->read_oob = nand_read_oob_std;
> -		ecc->write_oob = nand_write_oob_std;
> -		/*
> -		 * Board driver should supply ecc.size and ecc.strength values
> -		 * to select how many bits are correctable. Otherwise, default
> -		 * to 4 bits for large page devices.
> -		 */
> -		if (!ecc->size && (mtd->oobsize >= 64)) {
> -			ecc->size = 512;
> -			ecc->strength = 4;
> -		}
> +		switch (ecc->algo) {

Please put this logic in a sub-function, nand_scan_tail() is already
quite long and hard to read, let's try to not define sub switch-case
blocks inside existing ones.

> +		case NAND_ECC_HAMMING:
> +			ecc->calculate = nand_calculate_ecc;
> +			ecc->correct = nand_correct_data;
> +			ecc->read_page = nand_read_page_swecc;
> +			ecc->read_subpage = nand_read_subpage;
> +			ecc->write_page = nand_write_page_swecc;
> +			ecc->read_page_raw = nand_read_page_raw;
> +			ecc->write_page_raw = nand_write_page_raw;
> +			ecc->read_oob = nand_read_oob_std;
> +			ecc->write_oob = nand_write_oob_std;
> +			if (!ecc->size)
> +				ecc->size = 256;
> +			ecc->bytes = 3;
> +			ecc->strength = 1;
> +			break;
> +		case NAND_ECC_BCH:
> +			if (!mtd_nand_has_bch()) {
> +				WARN(1, "CONFIG_MTD_NAND_ECC_BCH not enabled\n");
> +				ret = -EINVAL;
> +				goto err_free;
> +			}
> +			ecc->calculate = nand_bch_calculate_ecc;
> +			ecc->correct = nand_bch_correct_data;
> +			ecc->read_page = nand_read_page_swecc;
> +			ecc->read_subpage = nand_read_subpage;
> +			ecc->write_page = nand_write_page_swecc;
> +			ecc->read_page_raw = nand_read_page_raw;
> +			ecc->write_page_raw = nand_write_page_raw;
> +			ecc->read_oob = nand_read_oob_std;
> +			ecc->write_oob = nand_write_oob_std;
> +			/*
> +			* Board driver should supply ecc.size and ecc.strength
> +			* values to select how many bits are correctable.
> +			* Otherwise, default to 4 bits for large page devices.
> +			*/
> +			if (!ecc->size && (mtd->oobsize >= 64)) {
> +				ecc->size = 512;
> +				ecc->strength = 4;
> +			}
>  
> -		/* See nand_bch_init() for details. */
> -		ecc->bytes = 0;
> -		ecc->priv = nand_bch_init(mtd);
> -		if (!ecc->priv) {
> -			WARN(1, "BCH ECC initialization failed!\n");
> +			/* See nand_bch_init() for details. */
> +			ecc->bytes = 0;
> +			ecc->priv = nand_bch_init(mtd);
> +			if (!ecc->priv) {
> +				WARN(1, "BCH ECC initialization failed!\n");
> +				ret = -EINVAL;
> +				goto err_free;
> +			}
> +			break;
> +		default:
> +			WARN(1, "Unsupported ECC algorithm!\n");
>  			ret = -EINVAL;
>  			goto err_free;
>  		}
> @@ -4478,7 +4486,7 @@ void nand_release(struct mtd_info *mtd)
>  {
>  	struct nand_chip *chip = mtd_to_nand(mtd);
>  
> -	if (chip->ecc.mode == NAND_ECC_SOFT_BCH)
> +	if (chip->ecc.algo == NAND_ECC_BCH)

Again, should be:

	if (chip->ecc.mode = NAND_ECC_SOFT &&
	    chip->ecc.algo == NAND_ECC_BCH)

>  		nand_bch_free((struct nand_bch_control *)chip->ecc.priv);
>  
>  	mtd_device_unregister(mtd);



-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[toc] | [prev] | [next] | [standalone]

#1380508

On Sat, 16 Apr 2016 09:58:07 +0200
Boris Brezillon <boris.brezillon@free-electrons.com> wrote:

> On Fri, 15 Apr 2016 21:54:10 +0200
> Rafał Miłecki <zajec5@gmail.com> wrote:
> 
> > Now we have all drivers properly setting this new field we can start
> > using it and proceed with deprecating NAND_ECC_SOFT_BCH.
> > 
> > Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
> > ---
> >  drivers/mtd/nand/nand_base.c | 98 ++++++++++++++++++++++++--------------------
> >  1 file changed, 53 insertions(+), 45 deletions(-)
> > 

[...]

> > @@ -4248,51 +4248,59 @@ int nand_scan_tail(struct mtd_info *mtd)
> >  		ecc->algo = NAND_ECC_HAMMING;
> >  
> >  	case NAND_ECC_SOFT:
> > -		ecc->calculate = nand_calculate_ecc;
> > -		ecc->correct = nand_correct_data;
> > -		ecc->read_page = nand_read_page_swecc;
> > -		ecc->read_subpage = nand_read_subpage;
> > -		ecc->write_page = nand_write_page_swecc;
> > -		ecc->read_page_raw = nand_read_page_raw;
> > -		ecc->write_page_raw = nand_write_page_raw;
> > -		ecc->read_oob = nand_read_oob_std;
> > -		ecc->write_oob = nand_write_oob_std;
> > -		if (!ecc->size)
> > -			ecc->size = 256;
> > -		ecc->bytes = 3;
> > -		ecc->strength = 1;
> > -		break;
> > -
> >  	case NAND_ECC_SOFT_BCH:
> 
> Shouldn't we drop this case?

Nevermind, I see you're dropping it in the last patch.


-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[toc] | [prev] | [next] | [standalone]

#1380776

On 16 April 2016 at 09:58, Boris Brezillon
<boris.brezillon@free-electrons.com> wrote:
> On Fri, 15 Apr 2016 21:54:10 +0200
> Rafał Miłecki <zajec5@gmail.com> wrote:
>
>> Now we have all drivers properly setting this new field we can start
>> using it and proceed with deprecating NAND_ECC_SOFT_BCH.
>>
>> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
>> ---
>>  drivers/mtd/nand/nand_base.c | 98 ++++++++++++++++++++++++--------------------
>>  1 file changed, 53 insertions(+), 45 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
>> index e1f3cf8..ffd1b32 100644
>> --- a/drivers/mtd/nand/nand_base.c
>> +++ b/drivers/mtd/nand/nand_base.c
>> @@ -4154,7 +4154,7 @@ int nand_scan_tail(struct mtd_info *mtd)
>>       /*
>>        * If no default placement scheme is given, select an appropriate one.
>>        */
>> -     if (!mtd->ooblayout && (ecc->mode != NAND_ECC_SOFT_BCH)) {
>> +     if (!mtd->ooblayout && ecc->algo != NAND_ECC_BCH) {
>
> Should be:
>
>         if (!mtd->ooblayout && ecc->algo == NAND_ECC_SOFT &&
>             ecc->algo != NAND_ECC_BCH) {
>
> Otherwise you're also taking the NAND_ECC_HW + NAND_ECC_BCH into
> account.

Pretty much yes, but without
[PATCH 11/12] of: mtd: drop support for NAND_ECC_SOFT_BCH as "soft_bch" mapping
we may be getting NAND_ECC_SOFT as well as NAND_ECC_SOFT_BCH. And
11/12 can't be applied before 10/12. We'll need to support both values
for this very short moment (after 10/12 but before 11/12).

[toc] | [prev] | [next] | [standalone]

#1380778

On 17 April 2016 at 18:50, Boris Brezillon
<boris.brezillon@free-electrons.com> wrote:
> On Sun, 17 Apr 2016 18:27:40 +0200
> Rafał Miłecki <zajec5@gmail.com> wrote:
>
>> On 16 April 2016 at 09:58, Boris Brezillon
>> <boris.brezillon@free-electrons.com> wrote:
>> > On Fri, 15 Apr 2016 21:54:10 +0200
>> > Rafał Miłecki <zajec5@gmail.com> wrote:
>> >
>> >> Now we have all drivers properly setting this new field we can start
>> >> using it and proceed with deprecating NAND_ECC_SOFT_BCH.
>> >>
>> >> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
>> >> ---
>> >>  drivers/mtd/nand/nand_base.c | 98 ++++++++++++++++++++++++--------------------
>> >>  1 file changed, 53 insertions(+), 45 deletions(-)
>> >>
>> >> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
>> >> index e1f3cf8..ffd1b32 100644
>> >> --- a/drivers/mtd/nand/nand_base.c
>> >> +++ b/drivers/mtd/nand/nand_base.c
>> >> @@ -4154,7 +4154,7 @@ int nand_scan_tail(struct mtd_info *mtd)
>> >>       /*
>> >>        * If no default placement scheme is given, select an appropriate one.
>> >>        */
>> >> -     if (!mtd->ooblayout && (ecc->mode != NAND_ECC_SOFT_BCH)) {
>> >> +     if (!mtd->ooblayout && ecc->algo != NAND_ECC_BCH) {
>> >
>> > Should be:
>> >
>> >         if (!mtd->ooblayout && ecc->algo == NAND_ECC_SOFT &&
>> >             ecc->algo != NAND_ECC_BCH) {
>> >
>> > Otherwise you're also taking the NAND_ECC_HW + NAND_ECC_BCH into
>> > account.
>>
>> Pretty much yes, but without
>> [PATCH 11/12] of: mtd: drop support for NAND_ECC_SOFT_BCH as "soft_bch" mapping
>> we may be getting NAND_ECC_SOFT as well as NAND_ECC_SOFT_BCH. And
>> 11/12 can't be applied before 10/12. We'll need to support both values
>> for this very short moment (after 10/12 but before 11/12).
>
>
> Why can't you move patch 11 before patch 10. The DT parsing code has
> already been patched to set both ecc->algo to NAND_ECC_BCH and
> ecc->mode to NAND_ECC_SOFT, so, to it seems perfectly safe to reorder
> those changes to me (and change the test as suggested).
>
> Am I missing something?

Without patch 10/12 we still check "mode" instead of "algo" in
nand_base.c. It means it does matter if there is NAND_ECC_SOFT or
NAND_ECC_SOFT_BCH used. We can't simply make of_get_nand_ecc_mode stop
returning NAND_ECC_SOFT_BCH without adjusting nand_base.c (as in
10/12) first.

-- 
Rafał

[toc] | [prev] | [next] | [standalone]

#1380779

On Sun, 17 Apr 2016 18:27:40 +0200
Rafał Miłecki <zajec5@gmail.com> wrote:

> On 16 April 2016 at 09:58, Boris Brezillon
> <boris.brezillon@free-electrons.com> wrote:
> > On Fri, 15 Apr 2016 21:54:10 +0200
> > Rafał Miłecki <zajec5@gmail.com> wrote:
> >
> >> Now we have all drivers properly setting this new field we can start
> >> using it and proceed with deprecating NAND_ECC_SOFT_BCH.
> >>
> >> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
> >> ---
> >>  drivers/mtd/nand/nand_base.c | 98 ++++++++++++++++++++++++--------------------
> >>  1 file changed, 53 insertions(+), 45 deletions(-)
> >>
> >> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> >> index e1f3cf8..ffd1b32 100644
> >> --- a/drivers/mtd/nand/nand_base.c
> >> +++ b/drivers/mtd/nand/nand_base.c
> >> @@ -4154,7 +4154,7 @@ int nand_scan_tail(struct mtd_info *mtd)
> >>       /*
> >>        * If no default placement scheme is given, select an appropriate one.
> >>        */
> >> -     if (!mtd->ooblayout && (ecc->mode != NAND_ECC_SOFT_BCH)) {
> >> +     if (!mtd->ooblayout && ecc->algo != NAND_ECC_BCH) {
> >
> > Should be:
> >
> >         if (!mtd->ooblayout && ecc->algo == NAND_ECC_SOFT &&
> >             ecc->algo != NAND_ECC_BCH) {
> >
> > Otherwise you're also taking the NAND_ECC_HW + NAND_ECC_BCH into
> > account.
> 
> Pretty much yes, but without
> [PATCH 11/12] of: mtd: drop support for NAND_ECC_SOFT_BCH as "soft_bch" mapping
> we may be getting NAND_ECC_SOFT as well as NAND_ECC_SOFT_BCH. And
> 11/12 can't be applied before 10/12. We'll need to support both values
> for this very short moment (after 10/12 but before 11/12).


Why can't you move patch 11 before patch 10. The DT parsing code has
already been patched to set both ecc->algo to NAND_ECC_BCH and
ecc->mode to NAND_ECC_SOFT, so, to it seems perfectly safe to reorder
those changes to me (and change the test as suggested).

Am I missing something?

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[toc] | [prev] | [next] | [standalone]

#1380780

On Sun, 17 Apr 2016 18:50:05 +0200
Boris Brezillon <boris.brezillon@free-electrons.com> wrote:

> On Sun, 17 Apr 2016 18:27:40 +0200
> Rafał Miłecki <zajec5@gmail.com> wrote:
> 
> > On 16 April 2016 at 09:58, Boris Brezillon
> > <boris.brezillon@free-electrons.com> wrote:
> > > On Fri, 15 Apr 2016 21:54:10 +0200
> > > Rafał Miłecki <zajec5@gmail.com> wrote:
> > >
> > >> Now we have all drivers properly setting this new field we can start
> > >> using it and proceed with deprecating NAND_ECC_SOFT_BCH.
> > >>
> > >> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
> > >> ---
> > >>  drivers/mtd/nand/nand_base.c | 98 ++++++++++++++++++++++++--------------------
> > >>  1 file changed, 53 insertions(+), 45 deletions(-)
> > >>
> > >> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> > >> index e1f3cf8..ffd1b32 100644
> > >> --- a/drivers/mtd/nand/nand_base.c
> > >> +++ b/drivers/mtd/nand/nand_base.c
> > >> @@ -4154,7 +4154,7 @@ int nand_scan_tail(struct mtd_info *mtd)
> > >>       /*
> > >>        * If no default placement scheme is given, select an appropriate one.
> > >>        */
> > >> -     if (!mtd->ooblayout && (ecc->mode != NAND_ECC_SOFT_BCH)) {
> > >> +     if (!mtd->ooblayout && ecc->algo != NAND_ECC_BCH) {
> > >
> > > Should be:
> > >
> > >         if (!mtd->ooblayout && ecc->algo == NAND_ECC_SOFT &&
> > >             ecc->algo != NAND_ECC_BCH) {
> > >
> > > Otherwise you're also taking the NAND_ECC_HW + NAND_ECC_BCH into
> > > account.
> > 
> > Pretty much yes, but without
> > [PATCH 11/12] of: mtd: drop support for NAND_ECC_SOFT_BCH as "soft_bch" mapping
> > we may be getting NAND_ECC_SOFT as well as NAND_ECC_SOFT_BCH. And
> > 11/12 can't be applied before 10/12. We'll need to support both values
> > for this very short moment (after 10/12 but before 11/12).
> 
> 
> Why can't you move patch 11 before patch 10. The DT parsing code has
> already been patched to set both ecc->algo to NAND_ECC_BCH and
> ecc->mode to NAND_ECC_SOFT, so, to it seems perfectly safe to reorder
> those changes to me (and change the test as suggested).
> 
> Am I missing something?
> 

Forget what I just said. How about merging those 2 patches into a
single one?

BTW, I did not complain so far, but you're trying to split every
single change into it's own patch, and I don't think it's required for
such simple change as assigning ecc->algo to NAND_ECC_BCH wherever
ecc->algo is set to NAND_ECC_SOFT_BCH (even if the patch touches
different drivers).
And the problem we're facing here is partly caused by this attempt to
split every change in its own patch.

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web