[v3,5/5] mtd: rawnand: denali: remove hard-coded DENALI_DEFAULT_OOB_SKIP_BYTES

Message ID 20191220113155.28177-6-yamada.masahiro@socionext.com
State New
Headers show
Series
  • mtd: rawnand: denali: a bungle of denali patches that is cleanly applicable
Related show

Commit Message

Masahiro Yamada Dec. 20, 2019, 11:31 a.m.
As commit 0d55c668b218 (mtd: rawnand: denali: set SPARE_AREA_SKIP_BYTES
register to 8 if unset") says, there were three solutions discussed:

  [1] Add a DT property to specify the skipped bytes in OOB
  [2] Associate the preferred value with compatible
  [3] Hard-code the default value in the driver

At that time, [3] was chosen because I did not have enough information
about the other platforms than UniPhier.

That commit also says "The preferred value may vary by platform. If so,
please trade up to a different solution." My intention was to replace
[3] with [2], not keep both [2] and [3].

Now that we have switched to [2] for SOCFPGA's SPARE_AREA_SKIP_BYTES=2,
[3] should be removed. This should be OK because denali_pci.c just
gets back to the original behavior.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

---

Changes in v3:
  - New patch

Changes in v2: None

 drivers/mtd/nand/raw/denali.c | 21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

-- 
2.17.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Comments

Miquel Raynal Jan. 14, 2020, 5:05 p.m. | #1
On Fri, 2019-12-20 at 11:31:55 UTC, Masahiro Yamada wrote:
> As commit 0d55c668b218 (mtd: rawnand: denali: set SPARE_AREA_SKIP_BYTES

> register to 8 if unset") says, there were three solutions discussed:

> 

>   [1] Add a DT property to specify the skipped bytes in OOB

>   [2] Associate the preferred value with compatible

>   [3] Hard-code the default value in the driver

> 

> At that time, [3] was chosen because I did not have enough information

> about the other platforms than UniPhier.

> 

> That commit also says "The preferred value may vary by platform. If so,

> please trade up to a different solution." My intention was to replace

> [3] with [2], not keep both [2] and [3].

> 

> Now that we have switched to [2] for SOCFPGA's SPARE_AREA_SKIP_BYTES=2,

> [3] should be removed. This should be OK because denali_pci.c just

> gets back to the original behavior.

> 

> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>


Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.

Miquel

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Patch

diff --git a/drivers/mtd/nand/raw/denali.c b/drivers/mtd/nand/raw/denali.c
index b6c463d02167..fafd0a0aa8e2 100644
--- a/drivers/mtd/nand/raw/denali.c
+++ b/drivers/mtd/nand/raw/denali.c
@@ -21,7 +21,6 @@ 
 #include "denali.h"
 
 #define DENALI_NAND_NAME    "denali-nand"
-#define DENALI_DEFAULT_OOB_SKIP_BYTES	8
 
 /* for Indexed Addressing */
 #define DENALI_INDEXED_CTRL	0x00
@@ -1302,22 +1301,16 @@  int denali_init(struct denali_controller *denali)
 
 	/*
 	 * Set how many bytes should be skipped before writing data in OOB.
-	 * If a non-zero value has already been configured, update it in HW.
-	 * If a non-zero value has already been set (by firmware or something),
-	 * just use it. Otherwise, set the driver's default.
+	 * If a platform requests a non-zero value, set it to the register.
+	 * Otherwise, read the value out, expecting it has already been set up
+	 * by firmware.
 	 */
-	if (denali->oob_skip_bytes) {
+	if (denali->oob_skip_bytes)
 		iowrite32(denali->oob_skip_bytes,
 			  denali->reg + SPARE_AREA_SKIP_BYTES);
-	} else {
-		denali->oob_skip_bytes =
-			ioread32(denali->reg + SPARE_AREA_SKIP_BYTES);
-		if (!denali->oob_skip_bytes) {
-			denali->oob_skip_bytes = DENALI_DEFAULT_OOB_SKIP_BYTES;
-			iowrite32(denali->oob_skip_bytes,
-				  denali->reg + SPARE_AREA_SKIP_BYTES);
-		}
-	}
+	else
+		denali->oob_skip_bytes = ioread32(denali->reg +
+						  SPARE_AREA_SKIP_BYTES);
 
 	iowrite32(0, denali->reg + TRANSFER_SPARE_REG);
 	iowrite32(GENMASK(denali->nbanks - 1, 0), denali->reg + RB_PIN_ENABLED);