Re:

Re:

[Josh Boyer]

On Sun, 13 Jan 2008 23:32:47 -0800 (PST)
Naveen Sattigeri <naveen_j_s@yahoo.com> wrote:

> Please correct if I am wrong.The kernel 2.6.10 should
> be patched for latest NAND mtd drivers.
> I have tried the latest patch 2008-January.txt but it
> was not successful because probably the patch is for
> latest kernel.
> Can I get the patch for 2.6.20?

http://linux-mtd.infradead.org/source.html#kernelversions

> What else I should do to detect the compact flash as
> MTD device using a host or using NFS.

You can't.  Compact flash is not real flash, nor does it have an
interface to present the real flash.

http://linux-mtd.infradead.org/faq/jffs2.html#L_stick_jffs2

josh

Re:

[Mike Frysinger]

On Tue, Feb 2, 2010 at 21:56, Stanley.Miao wrote:
> During some BSP development process, I found some tools in mtd-utils
> contain legacy interfaces and it is not suitable with the current linux
> kernel. I post my patches here in case somebody else need them.

these are good, but i think people might want to keep support for
older kernels around.  so you'd have the code try the newer ioctls
first and if those fail, fall back to the old ones.
-mike

Re:

[stanley.miao]

Mike Frysinger wrote:
> On Tue, Feb 2, 2010 at 21:56, Stanley.Miao wrote:
>   
>> During some BSP development process, I found some tools in mtd-utils
>> contain legacy interfaces and it is not suitable with the current linux
>> kernel. I post my patches here in case somebody else need them.
>>     
>
> these are good, but i think people might want to keep support for
> older kernels around.  so you'd have the code try the newer ioctls
> first and if those fail, fall back to the old ones.
> -mike
>
>   
Yeah, we should keep it compatible ASAP. But in the conflict occasion, I 
think, the
new kernel should get the higher priorities than the older kernel. The 
people who
use the old kernels can use the old version mtd-utils(not very old, 
1.3.1 or earlier).

Stanley.
 

Re !

[BRITISH COLUMBIA]

Your email has been awarded 1,263,584.00 GBP (One Million Two Hundred and Sixtythree Thousand,Five Hundred and Eightyfour Pounds Sterling) By British Columbia Lottery,Do send us Names, Address, Tel and Occupation for processing.

(no subject)

[Sascha Hauer]

The following adds i.MX53 nand support and generally devicetree
based probing for i.MX5 boards. The first three patches should go
via mtd, the last patch optionally aswell if all agree.

Sascha

The following changes since commit f8f5701bdaf9134b1f90e5044a82c66324d2073f:

  Linux 3.5-rc1 (2012-06-02 18:29:26 -0700)

are available in the git repository at:

  git://git.pengutronix.de/git/imx/linux-2.6.git imx/nand-mx53

for you to fetch changes up to d55d1479a3bfaedbb9f0c6c956f4dff6bb6d6d61:

  ARM i.MX5: Add nand oftree support (2012-06-06 12:20:24 +0200)

----------------------------------------------------------------
Sascha Hauer (4):
      mtd nand mxc_nand: Use managed resources
      mtd nand mxc_nand: swap iomem resource order
      mtd nand mxc_nand: add i.MX53 support
      ARM i.MX5: Add nand oftree support

 arch/arm/boot/dts/imx51.dtsi                  |    7 ++
 arch/arm/boot/dts/imx53.dtsi                  |    7 ++
 arch/arm/mach-imx/clk-imx51-imx53.c           |    2 +
 arch/arm/plat-mxc/devices/platform-mxc_nand.c |   11 +-
 drivers/mtd/nand/mxc_nand.c                   |  137 ++++++++++++++-----------
 5 files changed, 97 insertions(+), 67 deletions(-)

[PATCH 1/4] mtd nand mxc_nand: Use managed resources

[Sascha Hauer]

To make the error path simpler and to make subsequent patches
easier.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mtd/nand/mxc_nand.c |   70 ++++++++++++++-----------------------------
 1 file changed, 23 insertions(+), 47 deletions(-)

diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
index c58e6a9..a1136f3 100644
--- a/drivers/mtd/nand/mxc_nand.c
+++ b/drivers/mtd/nand/mxc_nand.c
@@ -1323,8 +1323,8 @@ static int __init mxcnd_probe(struct platform_device *pdev)
 	int err = 0;
 
 	/* Allocate memory for MTD device structure and private data */
-	host = kzalloc(sizeof(struct mxc_nand_host) + NAND_MAX_PAGESIZE +
-			NAND_MAX_OOBSIZE, GFP_KERNEL);
+	host = devm_kzalloc(&pdev->dev, sizeof(struct mxc_nand_host) +
+			NAND_MAX_PAGESIZE + NAND_MAX_OOBSIZE, GFP_KERNEL);
 	if (!host)
 		return -ENOMEM;
 
@@ -1351,26 +1351,17 @@ static int __init mxcnd_probe(struct platform_device *pdev)
 	this->read_buf = mxc_nand_read_buf;
 	this->verify_buf = mxc_nand_verify_buf;
 
-	host->clk = clk_get(&pdev->dev, "nfc");
-	if (IS_ERR(host->clk)) {
-		err = PTR_ERR(host->clk);
-		goto eclk;
-	}
-
-	clk_prepare_enable(host->clk);
-	host->clk_act = 1;
+	host->clk = devm_clk_get(&pdev->dev, "nfc");
+	if (IS_ERR(host->clk))
+		return PTR_ERR(host->clk);
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res) {
-		err = -ENODEV;
-		goto eres;
-	}
+	if (!res)
+		return -ENODEV;
 
-	host->base = ioremap(res->start, resource_size(res));
-	if (!host->base) {
-		err = -ENOMEM;
-		goto eres;
-	}
+	host->base = devm_request_and_ioremap(&pdev->dev, res);
+	if (!host->base)
+		return -ENOMEM;
 
 	host->main_area0 = host->base;
 
@@ -1378,7 +1369,7 @@ static int __init mxcnd_probe(struct platform_device *pdev)
 	if (err > 0)
 		err = mxcnd_probe_pdata(host);
 	if (err < 0)
-		goto eirq;
+		return err;
 
 	if (host->devtype_data->regs_offset)
 		host->regs = host->base + host->devtype_data->regs_offset;
@@ -1395,15 +1386,11 @@ static int __init mxcnd_probe(struct platform_device *pdev)
 
 	if (host->devtype_data->needs_ip) {
 		res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
-		if (!res) {
-			err = -ENODEV;
-			goto eirq;
-		}
-		host->regs_ip = ioremap(res->start, resource_size(res));
-		if (!host->regs_ip) {
-			err = -ENOMEM;
-			goto eirq;
-		}
+		if (!res)
+			return -ENODEV;
+		host->regs_ip = devm_request_and_ioremap(&pdev->dev, res);
+		if (!host->regs_ip)
+			return -ENOMEM;
 	}
 
 	if (host->pdata.hw_ecc) {
@@ -1437,9 +1424,13 @@ static int __init mxcnd_probe(struct platform_device *pdev)
 	 */
 	host->devtype_data->irq_control(host, 0);
 
-	err = request_irq(host->irq, mxc_nfc_irq, IRQF_DISABLED, DRIVER_NAME, host);
+	err = devm_request_irq(&pdev->dev, host->irq, mxc_nfc_irq,
+			IRQF_DISABLED, DRIVER_NAME, host);
 	if (err)
-		goto eirq;
+		return err;
+
+	clk_prepare_enable(host->clk);
+	host->clk_act = 1;
 
 	/*
 	 * Now that we "own" the interrupt make sure the interrupt mask bit is
@@ -1491,15 +1482,7 @@ static int __init mxcnd_probe(struct platform_device *pdev)
 	return 0;
 
 escan:
-	free_irq(host->irq, host);
-eirq:
-	if (host->regs_ip)
-		iounmap(host->regs_ip);
-	iounmap(host->base);
-eres:
-	clk_put(host->clk);
-eclk:
-	kfree(host);
+	clk_disable_unprepare(host->clk);
 
 	return err;
 }
@@ -1508,16 +1491,9 @@ static int __devexit mxcnd_remove(struct platform_device *pdev)
 {
 	struct mxc_nand_host *host = platform_get_drvdata(pdev);
 
-	clk_put(host->clk);
-
 	platform_set_drvdata(pdev, NULL);
 
 	nand_release(&host->mtd);
-	free_irq(host->irq, host);
-	if (host->regs_ip)
-		iounmap(host->regs_ip);
-	iounmap(host->base);
-	kfree(host);
 
 	return 0;
 }
-- 
1.7.10

[PATCH 2/4] mtd nand mxc_nand: swap iomem resource order

[Sascha Hauer]

The i.MX v3 nand controller (i.MX5) needs two memory resources.
Traditionally we have the AXI resource first. For sorting in this
driver into the devicetree it feels much more natural to have the
IP resource first. This patch swaps the ordering of these two
resources.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/arm/plat-mxc/devices/platform-mxc_nand.c |   11 ++++----
 drivers/mtd/nand/mxc_nand.c                   |   35 ++++++++++++++-----------
 2 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/arch/arm/plat-mxc/devices/platform-mxc_nand.c b/arch/arm/plat-mxc/devices/platform-mxc_nand.c
index 1568f39..95b75cc 100644
--- a/arch/arm/plat-mxc/devices/platform-mxc_nand.c
+++ b/arch/arm/plat-mxc/devices/platform-mxc_nand.c
@@ -63,10 +63,6 @@ struct platform_device *__init imx_add_mxc_nand(
 	/* AXI has to come first, that's how the mxc_nand driver expect it */
 	struct resource res[] = {
 		{
-			.start = data->axibase,
-			.end = data->axibase + SZ_16K - 1,
-			.flags = IORESOURCE_MEM,
-		}, {
 			.start = data->iobase,
 			.end = data->iobase + data->iosize - 1,
 			.flags = IORESOURCE_MEM,
@@ -74,10 +70,13 @@ struct platform_device *__init imx_add_mxc_nand(
 			.start = data->irq,
 			.end = data->irq,
 			.flags = IORESOURCE_IRQ,
+		}, {
+			.start = data->axibase,
+			.end = data->axibase + SZ_16K - 1,
+			.flags = IORESOURCE_MEM,
 		},
 	};
 	return imx_add_platform_device("mxc_nand", data->id,
-			res + !data->axibase,
-			ARRAY_SIZE(res) - !data->axibase,
+			res, ARRAY_SIZE(res) - !data->axibase,
 			pdata, sizeof(*pdata));
 }
diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
index a1136f3..cc0f6e7 100644
--- a/drivers/mtd/nand/mxc_nand.c
+++ b/drivers/mtd/nand/mxc_nand.c
@@ -1355,7 +1355,25 @@ static int __init mxcnd_probe(struct platform_device *pdev)
 	if (IS_ERR(host->clk))
 		return PTR_ERR(host->clk);
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	err = mxcnd_probe_dt(host);
+	if (err > 0)
+		err = mxcnd_probe_pdata(host);
+	if (err < 0)
+		return err;
+
+	if (host->devtype_data->needs_ip) {
+		res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+		if (!res)
+			return -ENODEV;
+		host->regs_ip = devm_request_and_ioremap(&pdev->dev, res);
+		if (!host->regs_ip)
+			return -ENOMEM;
+
+		res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	} else {
+		res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	}
+
 	if (!res)
 		return -ENODEV;
 
@@ -1365,12 +1383,6 @@ static int __init mxcnd_probe(struct platform_device *pdev)
 
 	host->main_area0 = host->base;
 
-	err = mxcnd_probe_dt(host);
-	if (err > 0)
-		err = mxcnd_probe_pdata(host);
-	if (err < 0)
-		return err;
-
 	if (host->devtype_data->regs_offset)
 		host->regs = host->base + host->devtype_data->regs_offset;
 	host->spare0 = host->base + host->devtype_data->spare0_offset;
@@ -1384,15 +1396,6 @@ static int __init mxcnd_probe(struct platform_device *pdev)
 	this->ecc.size = 512;
 	this->ecc.layout = host->devtype_data->ecclayout_512;
 
-	if (host->devtype_data->needs_ip) {
-		res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
-		if (!res)
-			return -ENODEV;
-		host->regs_ip = devm_request_and_ioremap(&pdev->dev, res);
-		if (!host->regs_ip)
-			return -ENOMEM;
-	}
-
 	if (host->pdata.hw_ecc) {
 		this->ecc.calculate = mxc_nand_calculate_ecc;
 		this->ecc.hwctl = mxc_nand_enable_hwecc;
-- 
1.7.10

[PATCH 3/4] mtd nand mxc_nand: add i.MX53 support

[Sascha Hauer]

The only relevant change between i.MX51 and i.MX53 is that
a bitfield is shifted one bit to the left.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mtd/nand/mxc_nand.c |   48 +++++++++++++++++++++++++++++++++++++------
 1 file changed, 42 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
index cc0f6e7..1aae564 100644
--- a/drivers/mtd/nand/mxc_nand.c
+++ b/drivers/mtd/nand/mxc_nand.c
@@ -43,8 +43,8 @@
 
 #define nfc_is_v21()		(cpu_is_mx25() || cpu_is_mx35())
 #define nfc_is_v1()		(cpu_is_mx31() || cpu_is_mx27() || cpu_is_mx21())
-#define nfc_is_v3_2()		(cpu_is_mx51() || cpu_is_mx53())
-#define nfc_is_v3()		nfc_is_v3_2()
+#define nfc_is_v3_2a()		cpu_is_mx51()
+#define nfc_is_v3_2b()		cpu_is_mx53()
 
 /* Addresses for NFC registers */
 #define NFC_V1_V2_BUF_SIZE		(host->regs + 0x00)
@@ -122,7 +122,7 @@
 #define NFC_V3_CONFIG2_2CMD_PHASES		(1 << 4)
 #define NFC_V3_CONFIG2_NUM_ADDR_PHASE0		(1 << 5)
 #define NFC_V3_CONFIG2_ECC_MODE_8		(1 << 6)
-#define NFC_V3_CONFIG2_PPB(x)			(((x) & 0x3) << 7)
+#define NFC_V3_CONFIG2_PPB(x, shift)		(((x) & 0x3) << shift)
 #define NFC_V3_CONFIG2_NUM_ADDR_PHASE1(x)	(((x) & 0x3) << 12)
 #define NFC_V3_CONFIG2_INT_MSK			(1 << 15)
 #define NFC_V3_CONFIG2_ST_CMD(x)		(((x) & 0xff) << 24)
@@ -174,6 +174,7 @@ struct mxc_nand_devtype_data {
 	int spare_len;
 	int eccbytes;
 	int eccsize;
+	int ppb_shift;
 };
 
 struct mxc_nand_host {
@@ -1001,7 +1002,9 @@ static void preset_v3(struct mtd_info *mtd)
 	}
 
 	if (mtd->writesize) {
-		config2 |= NFC_V3_CONFIG2_PPB(ffs(mtd->erasesize / mtd->writesize) - 6);
+		config2 |= NFC_V3_CONFIG2_PPB(
+				ffs(mtd->erasesize / mtd->writesize) - 6,
+				host->devtype_data->ppb_shift);
 		host->eccsize = get_eccsize(mtd);
 		if (host->eccsize == 8)
 			config2 |= NFC_V3_CONFIG2_ECC_MODE_8;
@@ -1213,7 +1216,7 @@ static const struct mxc_nand_devtype_data imx25_nand_devtype_data = {
 	.eccsize = 0,
 };
 
-/* v3: i.MX51, i.MX53 */
+/* v3.2a: i.MX51 */
 static const struct mxc_nand_devtype_data imx51_nand_devtype_data = {
 	.preset = preset_v3,
 	.send_cmd = send_cmd_v3,
@@ -1237,6 +1240,34 @@ static const struct mxc_nand_devtype_data imx51_nand_devtype_data = {
 	.spare_len = 64,
 	.eccbytes = 0,
 	.eccsize = 0,
+	.ppb_shift = 7,
+};
+
+/* v3.2b: i.MX53 */
+static const struct mxc_nand_devtype_data imx53_nand_devtype_data = {
+	.preset = preset_v3,
+	.send_cmd = send_cmd_v3,
+	.send_addr = send_addr_v3,
+	.send_page = send_page_v3,
+	.send_read_id = send_read_id_v3,
+	.get_dev_status = get_dev_status_v3,
+	.check_int = check_int_v3,
+	.irq_control = irq_control_v3,
+	.get_ecc_status = get_ecc_status_v3,
+	.ecclayout_512 = &nandv2_hw_eccoob_smallpage,
+	.ecclayout_2k = &nandv2_hw_eccoob_largepage,
+	.ecclayout_4k = &nandv2_hw_eccoob_smallpage, /* XXX: needs fix */
+	.select_chip = mxc_nand_select_chip_v1_v3,
+	.correct_data = mxc_nand_correct_data_v2_v3,
+	.irqpending_quirk = 0,
+	.needs_ip = 1,
+	.regs_offset = 0,
+	.spare0_offset = 0x1000,
+	.axi_offset = 0x1e00,
+	.spare_len = 64,
+	.eccbytes = 0,
+	.eccsize = 0,
+	.ppb_shift = 8,
 };
 
 #ifdef CONFIG_OF_MTD
@@ -1253,6 +1284,9 @@ static const struct of_device_id mxcnd_dt_ids[] = {
 	}, {
 		.compatible = "fsl,imx51-nand",
 		.data = &imx51_nand_devtype_data,
+	}, {
+		.compatible = "fsl,imx53-nand",
+		.data = &imx53_nand_devtype_data,
 	},
 	{ /* sentinel */ }
 };
@@ -1306,8 +1340,10 @@ static int __init mxcnd_probe_pdata(struct mxc_nand_host *host)
 			host->devtype_data = &imx27_nand_devtype_data;
 	} else if (nfc_is_v21()) {
 		host->devtype_data = &imx25_nand_devtype_data;
-	} else if (nfc_is_v3_2()) {
+	} else if (nfc_is_v3_2a()) {
 		host->devtype_data = &imx51_nand_devtype_data;
+	} else if (nfc_is_v3_2b()) {
+		host->devtype_data = &imx53_nand_devtype_data;
 	} else
 		BUG();
 
-- 
1.7.10

[PATCH 4/4] ARM i.MX5: Add nand oftree support

[Sascha Hauer]

This adds snippets to the i.MX51/53 devicetrees for the nand
flash controller.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/arm/boot/dts/imx51.dtsi        |    7 +++++++
 arch/arm/boot/dts/imx53.dtsi        |    7 +++++++
 arch/arm/mach-imx/clk-imx51-imx53.c |    2 ++
 3 files changed, 16 insertions(+)

diff --git a/arch/arm/boot/dts/imx51.dtsi b/arch/arm/boot/dts/imx51.dtsi
index bfa65ab..39eb88e 100644
--- a/arch/arm/boot/dts/imx51.dtsi
+++ b/arch/arm/boot/dts/imx51.dtsi
@@ -259,6 +259,13 @@
 				status = "disabled";
 			};
 
+			nand@83fdb000 {
+				compatible = "fsl,imx51-nand";
+				reg = <0x83fdb000 0x1000 0xcfff0000 0x10000>;
+				interrupts = <8>;
+				status = "disabled";
+			};
+
 			ssi3: ssi@83fe8000 {
 				compatible = "fsl,imx51-ssi", "fsl,imx21-ssi";
 				reg = <0x83fe8000 0x4000>;
diff --git a/arch/arm/boot/dts/imx53.dtsi b/arch/arm/boot/dts/imx53.dtsi
index e3e8694..2b5caf9 100644
--- a/arch/arm/boot/dts/imx53.dtsi
+++ b/arch/arm/boot/dts/imx53.dtsi
@@ -314,6 +314,13 @@
 				status = "disabled";
 			};
 
+			nand@63fdb000 {
+				compatible = "fsl,imx53-nand";
+				reg = <0x63fdb000 0x1000 0xf7ff0000 0x10000>;
+				interrupts = <8>;
+				status = "disabled";
+			};
+
 			ssi3: ssi@63fe8000 {
 				compatible = "fsl,imx53-ssi", "fsl,imx21-ssi";
 				reg = <0x63fe8000 0x4000>;
diff --git a/arch/arm/mach-imx/clk-imx51-imx53.c b/arch/arm/mach-imx/clk-imx51-imx53.c
index fcd94f3..7b525c1 100644
--- a/arch/arm/mach-imx/clk-imx51-imx53.c
+++ b/arch/arm/mach-imx/clk-imx51-imx53.c
@@ -357,6 +357,7 @@ int __init mx51_clocks_init(unsigned long rate_ckil, unsigned long rate_osc,
 	clk_register_clkdev(clk[ssi1_ipg_gate], NULL, "83fcc000.ssi");
 	clk_register_clkdev(clk[ssi2_ipg_gate], NULL, "70014000.ssi");
 	clk_register_clkdev(clk[ssi3_ipg_gate], NULL, "83fe8000.ssi");
+	clk_register_clkdev(clk[nfc_gate], NULL, "83fdb000.nand");
 
 	/* set the usboh3 parent to pll2_sw */
 	clk_set_parent(clk[usboh3_sel], clk[pll2_sw]);
@@ -446,6 +447,7 @@ int __init mx53_clocks_init(unsigned long rate_ckil, unsigned long rate_osc,
 	clk_register_clkdev(clk[ssi1_ipg_gate], NULL, "63fcc000.ssi");
 	clk_register_clkdev(clk[ssi2_ipg_gate], NULL, "50014000.ssi");
 	clk_register_clkdev(clk[ssi3_ipg_gate], NULL, "63fd0000.ssi");
+	clk_register_clkdev(clk[nfc_gate], NULL, "63fdb000.nand");
 
 	/* set SDHC root clock to 200MHZ*/
 	clk_set_rate(clk[esdhc_a_podf], 200000000);
-- 
1.7.10

[PATCH] i.MX5 nand support

[Sascha Hauer]

Damned, forgot subject (again)

Sascha

On Wed, Jun 06, 2012 at 12:33:12PM +0200, Sascha Hauer wrote:
> The following adds i.MX53 nand support and generally devicetree
> based probing for i.MX5 boards. The first three patches should go
> via mtd, the last patch optionally aswell if all agree.
> 
> Sascha
> 
> The following changes since commit f8f5701bdaf9134b1f90e5044a82c66324d2073f:
> 
>   Linux 3.5-rc1 (2012-06-02 18:29:26 -0700)
> 
> are available in the git repository at:
> 
>   git://git.pengutronix.de/git/imx/linux-2.6.git imx/nand-mx53
> 
> for you to fetch changes up to d55d1479a3bfaedbb9f0c6c956f4dff6bb6d6d61:
> 
>   ARM i.MX5: Add nand oftree support (2012-06-06 12:20:24 +0200)
> 
> ----------------------------------------------------------------
> Sascha Hauer (4):
>       mtd nand mxc_nand: Use managed resources
>       mtd nand mxc_nand: swap iomem resource order
>       mtd nand mxc_nand: add i.MX53 support
>       ARM i.MX5: Add nand oftree support
> 
>  arch/arm/boot/dts/imx51.dtsi                  |    7 ++
>  arch/arm/boot/dts/imx53.dtsi                  |    7 ++
>  arch/arm/mach-imx/clk-imx51-imx53.c           |    2 +
>  arch/arm/plat-mxc/devices/platform-mxc_nand.c |   11 +-
>  drivers/mtd/nand/mxc_nand.c                   |  137 ++++++++++++++-----------
>  5 files changed, 97 insertions(+), 67 deletions(-)
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re:

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 1679 bytes --]

On Wed, 2012-06-06 at 12:33 +0200, Sascha Hauer wrote:
> The following adds i.MX53 nand support and generally devicetree
> based probing for i.MX5 boards. The first three patches should go
> via mtd, the last patch optionally aswell if all agree.
> 
> Sascha
> 
> The following changes since commit f8f5701bdaf9134b1f90e5044a82c66324d2073f:
> 
>   Linux 3.5-rc1 (2012-06-02 18:29:26 -0700)
> 
> are available in the git repository at:
> 
>   git://git.pengutronix.de/git/imx/linux-2.6.git imx/nand-mx53
> 
> for you to fetch changes up to d55d1479a3bfaedbb9f0c6c956f4dff6bb6d6d61:
> 
>   ARM i.MX5: Add nand oftree support (2012-06-06 12:20:24 +0200)

Do you want this to go via the MTD tree? Would you be able to collect
acks for the arch/arm bits? Meanwhile, please, take a look at these
sparse warnings added by this patch-set and detected by aiaiai:

--------------------------------------------------------------------------------

Successfully built configuration "arm-mxc-imx_defconfig,arm,arm-unknown-linux-gnueabi-", results:

--- before_patching.log
+++ after_patching.log
@@ @@
+drivers/mtd/nand/mxc_nand.c:1289:26: warning: incorrect type in initializer (different modifiers) [sparse]
+drivers/mtd/nand/mxc_nand.c:1289:26:    expected void *data [sparse]
+drivers/mtd/nand/mxc_nand.c:1289:26:    got struct mxc_nand_devtype_data static const [toplevel] *<noident> [sparse]
+drivers/mtd/nand/mxc_nand.c:1289:3: warning: initialization discards 'const' qualifier from pointer target type [enabled by default]

--------------------------------------------------------------------------------


-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 4/4] ARM i.MX5: Add nand oftree support

[Shawn Guo]

On Wed, Jun 06, 2012 at 12:33:16PM +0200, Sascha Hauer wrote:
> This adds snippets to the i.MX51/53 devicetrees for the nand
> flash controller.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

Acked-by: Shawn Guo <shawn.guo@linaro.org>

Re: [PATCH 4/4] ARM i.MX5: Add nand oftree support

[Sascha Hauer]

On Thu, Jun 07, 2012 at 10:29:54AM +0800, Shawn Guo wrote:
> On Wed, Jun 06, 2012 at 12:33:16PM +0200, Sascha Hauer wrote:
> > This adds snippets to the i.MX51/53 devicetrees for the nand
> > flash controller.
> > 
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> 
> Acked-by: Shawn Guo <shawn.guo@linaro.org>

Thanks. Will address Artems comments and resend.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re:

[Sascha Hauer]

Hi Artem,

On Wed, Jun 06, 2012 at 05:39:07PM +0300, Artem Bityutskiy wrote:
> On Wed, 2012-06-06 at 12:33 +0200, Sascha Hauer wrote:
> > The following adds i.MX53 nand support and generally devicetree
> > based probing for i.MX5 boards. The first three patches should go
> > via mtd, the last patch optionally aswell if all agree.
> > 
> > Sascha
> > 
> > The following changes since commit f8f5701bdaf9134b1f90e5044a82c66324d2073f:
> > 
> >   Linux 3.5-rc1 (2012-06-02 18:29:26 -0700)
> > 
> > are available in the git repository at:
> > 
> >   git://git.pengutronix.de/git/imx/linux-2.6.git imx/nand-mx53
> > 
> > for you to fetch changes up to d55d1479a3bfaedbb9f0c6c956f4dff6bb6d6d61:
> > 
> >   ARM i.MX5: Add nand oftree support (2012-06-06 12:20:24 +0200)
> 
> Do you want this to go via the MTD tree? Would you be able to collect
> acks for the arch/arm bits? Meanwhile, please, take a look at these
> sparse warnings added by this patch-set and detected by aiaiai:
> 
> --------------------------------------------------------------------------------
> 
> Successfully built configuration "arm-mxc-imx_defconfig,arm,arm-unknown-linux-gnueabi-", results:
> 
> --- before_patching.log
> +++ after_patching.log
> @@ @@
> +drivers/mtd/nand/mxc_nand.c:1289:26: warning: incorrect type in initializer (different modifiers) [sparse]
> +drivers/mtd/nand/mxc_nand.c:1289:26:    expected void *data [sparse]
> +drivers/mtd/nand/mxc_nand.c:1289:26:    got struct mxc_nand_devtype_data static const [toplevel] *<noident> [sparse]
> +drivers/mtd/nand/mxc_nand.c:1289:3: warning: initialization discards 'const' qualifier from pointer target type [enabled by default]

Fixing these warnings in the nand driver does not seem to be the correct
approach. Initializing mxc_nand_devtype_data as const seems sane, the
problem is that struct of_device_id expects a void * instead of a const
void *. A patch fixing this is outstanding here:

http://permalink.gmane.org/gmane.linux.drivers.devicetree/15069

(this will also fix the other sparse warnings from this driver)

I asked Uwe to resend this.

So I only added Shawns Ack to the arm-i.MX part, you can pull this into
the mtd tree:


The following changes since commit f8f5701bdaf9134b1f90e5044a82c66324d2073f:

  Linux 3.5-rc1 (2012-06-02 18:29:26 -0700)

are available in the git repository at:

  git://git.pengutronix.de/git/imx/linux-2.6.git tags/mtd-imx53-nand-support

for you to fetch changes up to 25d097d575d7c06b76e4e6e2488718976b70c432:

  ARM i.MX5: Add nand oftree support (2012-06-07 11:59:19 +0200)

----------------------------------------------------------------
Nand support for i.MX53 and devicetree snippets for i.MX5 nand

----------------------------------------------------------------
Sascha Hauer (4):
      mtd nand mxc_nand: Use managed resources
      mtd nand mxc_nand: swap iomem resource order
      mtd nand mxc_nand: add i.MX53 support
      ARM i.MX5: Add nand oftree support

 arch/arm/boot/dts/imx51.dtsi                  |    7 ++
 arch/arm/boot/dts/imx53.dtsi                  |    7 ++
 arch/arm/mach-imx/clk-imx51-imx53.c           |    2 +
 arch/arm/plat-mxc/devices/platform-mxc_nand.c |   11 +-
 drivers/mtd/nand/mxc_nand.c                   |  137 ++++++++++++++-----------
 5 files changed, 97 insertions(+), 67 deletions(-)

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: Re:

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 582 bytes --]

On Thu, 2012-06-07 at 12:11 +0200, Sascha Hauer wrote:
> Fixing these warnings in the nand driver does not seem to be the correct
> approach. Initializing mxc_nand_devtype_data as const seems sane, the
> problem is that struct of_device_id expects a void * instead of a const
> void *. A patch fixing this is outstanding here:
> 
> http://permalink.gmane.org/gmane.linux.drivers.devicetree/15069
> 
> (this will also fix the other sparse warnings from this driver)
> 
> I asked Uwe to resend this.

Pushed to l2-mtd.git, thanks!

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

RE:

[Tarzon, Megan]

________________________________
From: Tarzon, Megan
Sent: Thursday, October 30, 2014 12:40 AM
To: Tarzon, Megan
Subject:

Good day.
l am the Chief Risk Officer and Executive Director of China Guangfa Bank in Hong Kong. I want to present you as the owner of 49.5 million USD In my bank since i am the only one aware of the funds due to my investigations. signify your interest by replying to this Email: morrowhkmorro1@rogers.com
James Morrow.

CONFIDENTIALITY NOTICE: This communication and any attachments may contain confidential or privileged information for the use by the designated recipient(s) named above.   If you are not the intended recipient, you are hereby notified that you have received this communication in error and that any review, disclosure, dissemination, distribution or copying of it or the attachments is strictly prohibited.  If you have received this communication in error, please contact the sender  and destroy all copies of the communication and attachments.  Thank you. MSG:104-123

Re:

[Richard Weinberger]

On Fri, Feb 26, 2016 at 2:19 AM, Fredrick Prashanth John Berchmans
<fredrickprashanth@gmail.com> wrote:
> We are using UBIFS on our NOR flash.
> We are observing that a lot of times the filesystem goes to read-only
> unable to recover.
> Most of the time its due to
> a) ubifs_scan_a_node failing due to bad crc or unclean free space.
> b) ubifs_leb_write failing to erase due to erase timeout
>
> [ The above would have happened due to unclean power cuts. In our
> environment this happens often ]
>
> I checked the code in jffs2. Looking at jffs2 code it looks like jffs2
> tolerates the above two
> failures and moves on without mounting read-only.
> Is my understanding right ?
>
> Could we change the ubifs_scan_a_node to skip corrupted bytes and move
> to next node,
> instead of returning error ?

Not without a detailed analysis what exactly is going on.
It sounds more like ad-hoc hack. :-)

-- 
Thanks,
//richard

Re:

[Boris Brezillon]

Hi Mario,

Not sure how you created this patchset, but you miss a Subject, and the
diff-stat.

Please use

git format-patch -o <output-dir> -3 --cover-letter

to generate patches, then fill the cover letter in.

Once your cover letter is ready, you can send the patches with

git send-email --to ... --cc ... <output-dir>/*.patch

Regards,

Boris

Le Fri, 19 May 2017 18:37:28 -0300,
"Mario J. Rugiero" <mrugiero@gmail.com> a écrit :

> Some manufacturers use different layouts than MTD for the NAND, creating
> incompatibilities when going from a vendor-specific kernel to mainline.
> In particular, NAND devices for AllWinner boards write non-FF values to
> the bad block marker, and thus false positives arise when detecting bad
> blocks with the MTD driver. Sometimes there are enough false positives
> to make the device unusable.
> A proposed solution is NAND scrubbing, something a user who knows what
> she's doing (TM) could do to avoid this. It consists in erasing blocks
> disregarding the BBM. Since the user must know what she's doing, the
> only way to enable this feature is through a per-chip debugfs entry.
> 

RE,

[Miss Juliet Muhammad]

I have a deal for you, in your region.

RE,

[Miss Juliet Muhammad]

I have a deal for you, in your region.

Re:

[Richard Weinberger]

On Fri, Dec 21, 2018 at 4:24 PM kenneth johansson <kenjo@kenjo.org> wrote:
>
> From 9815710fa078241c683de1b49d9a0c9631502e17 Mon Sep 17 00:00:00 2001
> From: Kenneth Johansson <kenjo@kenjo.org>
> Date: Fri, 21 Dec 2018 15:46:24 +0100
> Subject: [PATCH] mtd: rawnand: nandsim: Add support to disable subpage writes.
> X-IMAPbase: 1545405463 2
> Status: O
> X-UID: 1
>
> This is needed if you try to use an already existing ubifs image that
> is created for hardware that do not support subpage write.
>
> It is not enough that you can select what nandchip to emulate as the
> subpage support might not exist in the actual nand driver.

Is this really needed? Usually using ubiattach's -O parameter does the trick.

-- 
Thanks,
//richard

Re:

[Miquel Raynal]

Hi,

kr.kim@skyhighmemory.com wrote on Thu,  7 Mar 2024 15:07:29 +0900:

> Feat: Add SkyHigh Memory Patch code
> 
> Add SPI Nand Patch code of SkyHigh Memory
> - Add company dependent code with 'skyhigh.c'
> - Insert into 'core.c' so that 'always ECC on'

Patch formatting is still messed up.

> commit 6061b97a830af8cb5fd0917e833e779451f9046a (HEAD -> master)
> Author: KR Kim <kr.kim@skyhighmemory.com>
> Date:   Thu Mar 7 13:24:11 2024 +0900
> 
>     SPI Nand Patch code of SkyHigh Momory
> 
>     Signed-off-by: KR Kim <kr.kim@skyhighmemory.com>
> 
> From 6061b97a830af8cb5fd0917e833e779451f9046a Mon Sep 17 00:00:00 2001
> From: KR Kim <kr.kim@skyhighmemory.com>
> Date: Thu, 7 Mar 2024 13:24:11 +0900
> Subject: [PATCH] SPI Nand Patch code of SkyHigh Memory
> 
> ---
>  drivers/mtd/nand/spi/Makefile  |   2 +-
>  drivers/mtd/nand/spi/core.c    |   7 +-
>  drivers/mtd/nand/spi/skyhigh.c | 155 +++++++++++++++++++++++++++++++++
>  include/linux/mtd/spinand.h    |   3 +
>  4 files changed, 165 insertions(+), 2 deletions(-)
>  mode change 100644 => 100755 drivers/mtd/nand/spi/Makefile
>  mode change 100644 => 100755 drivers/mtd/nand/spi/core.c
>  create mode 100644 drivers/mtd/nand/spi/skyhigh.c
>  mode change 100644 => 100755 include/linux/mtd/spinand.h
> 
> diff --git a/drivers/mtd/nand/spi/Makefile b/drivers/mtd/nand/spi/Makefile
> old mode 100644
> new mode 100755
> index 19cc77288ebb..1e61ab21893a
> --- a/drivers/mtd/nand/spi/Makefile
> +++ b/drivers/mtd/nand/spi/Makefile
> @@ -1,4 +1,4 @@
>  # SPDX-License-Identifier: GPL-2.0
>  spinand-objs := core.o alliancememory.o ato.o esmt.o foresee.o gigadevice.o macronix.o
> -spinand-objs += micron.o paragon.o toshiba.o winbond.o xtx.o
> +spinand-objs += micron.o paragon.o skyhigh.o toshiba.o winbond.o xtx.o
>  obj-$(CONFIG_MTD_SPI_NAND) += spinand.o
> diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
> old mode 100644
> new mode 100755
> index e0b6715e5dfe..e3f0a7544ba4
> --- a/drivers/mtd/nand/spi/core.c
> +++ b/drivers/mtd/nand/spi/core.c
> @@ -34,7 +34,7 @@ static int spinand_read_reg_op(struct spinand_device *spinand, u8 reg, u8 *val)
>  	return 0;
>  }
>  
> -static int spinand_write_reg_op(struct spinand_device *spinand, u8 reg, u8 val)
> +int spinand_write_reg_op(struct spinand_device *spinand, u8 reg, u8 val)

Please do this in a separate commit.

>  {
>  	struct spi_mem_op op = SPINAND_SET_FEATURE_OP(reg,
>  						      spinand->scratchbuf);
> @@ -196,6 +196,10 @@ static int spinand_init_quad_enable(struct spinand_device *spinand)
>  static int spinand_ecc_enable(struct spinand_device *spinand,
>  			      bool enable)
>  {
> +	/* SHM : always ECC enable */
> +	if (spinand->flags & SPINAND_ON_DIE_ECC_MANDATORY)
> +		return 0;

Silently always enabling ECC is not possible. If you cannot disable the
on-die engine, then:
- you should prevent any other engine type to be used
- you should error out if a raw access is requested
- these chips are broken, IMO

> +
>  	return spinand_upd_cfg(spinand, CFG_ECC_ENABLE,
>  			       enable ? CFG_ECC_ENABLE : 0);
>  }
> @@ -945,6 +949,7 @@ static const struct spinand_manufacturer *spinand_manufacturers[] = {
>  	&macronix_spinand_manufacturer,
>  	&micron_spinand_manufacturer,
>  	&paragon_spinand_manufacturer,
> +	&skyhigh_spinand_manufacturer,
>  	&toshiba_spinand_manufacturer,
>  	&winbond_spinand_manufacturer,
>  	&xtx_spinand_manufacturer,
> diff --git a/drivers/mtd/nand/spi/skyhigh.c b/drivers/mtd/nand/spi/skyhigh.c
> new file mode 100644
> index 000000000000..92e7572094ff
> --- /dev/null
> +++ b/drivers/mtd/nand/spi/skyhigh.c
> @@ -0,0 +1,155 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2022 SkyHigh Memory Limited
> + *
> + * Author: Takahiro Kuwano <takahiro.kuwano@infineon.com>
> + */
> +
> +#include <linux/device.h>
> +#include <linux/kernel.h>
> +#include <linux/mtd/spinand.h>
> +
> +#define SPINAND_MFR_SKYHIGH		0x01
> +
> +#define SKYHIGH_STATUS_ECC_1TO2_BITFLIPS	(1 << 4)
> +#define SKYHIGH_STATUS_ECC_3TO6_BITFLIPS	(2 << 4)
> +#define SKYHIGH_STATUS_ECC_UNCOR_ERROR  	(3 << 4)
> +
> +#define SKYHIGH_CONFIG_PROTECT_EN	BIT(1)
> +
> +static SPINAND_OP_VARIANTS(read_cache_variants,
> +		SPINAND_PAGE_READ_FROM_CACHE_QUADIO_OP(0, 4, NULL, 0),
> +		SPINAND_PAGE_READ_FROM_CACHE_X4_OP(0, 1, NULL, 0),
> +		SPINAND_PAGE_READ_FROM_CACHE_DUALIO_OP(0, 2, NULL, 0),
> +		SPINAND_PAGE_READ_FROM_CACHE_X2_OP(0, 1, NULL, 0),
> +		SPINAND_PAGE_READ_FROM_CACHE_OP(true, 0, 1, NULL, 0),
> +		SPINAND_PAGE_READ_FROM_CACHE_OP(false, 0, 1, NULL, 0));
> +
> +static SPINAND_OP_VARIANTS(write_cache_variants,
> +		SPINAND_PROG_LOAD_X4(true, 0, NULL, 0),
> +		SPINAND_PROG_LOAD(true, 0, NULL, 0));
> +
> +static SPINAND_OP_VARIANTS(update_cache_variants,
> +		SPINAND_PROG_LOAD_X4(false, 0, NULL, 0),
> +		SPINAND_PROG_LOAD(false, 0, NULL, 0));
> +
> +static int skyhigh_spinand_ooblayout_ecc(struct mtd_info *mtd, int section,
> +					 struct mtd_oob_region *region)
> +{
> +	if (section)
> +		return -ERANGE;
> +
> +	/* SkyHigh's ecc parity is stored in the internal hidden area and is not needed for them. */

		     ECC		     an

"needed" is wrong here. Just stop after "area"


> +	region->length = 0;
> +	region->offset = mtd->oobsize;
> +
> +	return 0;
> +}
> +
> +static int skyhigh_spinand_ooblayout_free(struct mtd_info *mtd, int section,
> +					  struct mtd_oob_region *region)
> +{
> +	if (section)
> +		return -ERANGE;
> +
> +	region->length = mtd->oobsize - 2;
> +	region->offset = 2;
> +
> +	return 0;
> +}
> +
> +static const struct mtd_ooblayout_ops skyhigh_spinand_ooblayout = {
> +	.ecc = skyhigh_spinand_ooblayout_ecc,
> +	.free = skyhigh_spinand_ooblayout_free,
> +};
> +
> +static int skyhigh_spinand_ecc_get_status(struct spinand_device *spinand,
> +				  u8 status)
> +{
> +	/* SHM
> +	 * 00 : No bit-flip
> +	 * 01 : 1-2 errors corrected
> +	 * 10 : 3-6 errors corrected         
> +	 * 11 : uncorrectable
> +	 */

Thanks for the comment but the switch case looks rather
straightforward, it is self-sufficient in this case.

> +
> +	switch (status & STATUS_ECC_MASK) {
> +	case STATUS_ECC_NO_BITFLIPS:
> +		return 0;
> +
> +	case SKYHIGH_STATUS_ECC_1TO2_BITFLIPS:
> +		return 2;
> +
> + 	case SKYHIGH_STATUS_ECC_3TO6_BITFLIPS:
> +		return 6; 
> +
> + 	case SKYHIGH_STATUS_ECC_UNCOR_ERROR:
> +		return -EBADMSG;;
> +
> +	default:
> +		break;

I guess you can directly call return -EINVAL here?

> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static const struct spinand_info skyhigh_spinand_table[] = {
> +	SPINAND_INFO("S35ML01G301",
> +		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x15),
> +		     NAND_MEMORG(1, 2048, 64, 64, 1024, 20, 1, 1, 1),
> +		     NAND_ECCREQ(6, 32),
> +		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
> +					      &write_cache_variants,
> +					      &update_cache_variants),
> +		     SPINAND_ON_DIE_ECC_MANDATORY,
> +		     SPINAND_ECCINFO(&skyhigh_spinand_ooblayout,
> +		     		     skyhigh_spinand_ecc_get_status)),
> +	SPINAND_INFO("S35ML01G300",
> +		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x14),
> +		     NAND_MEMORG(1, 2048, 128, 64, 1024, 20, 1, 1, 1),
> +		     NAND_ECCREQ(6, 32),
> +		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
> +					      &write_cache_variants,
> +					      &update_cache_variants),
> +		     SPINAND_ON_DIE_ECC_MANDATORY,
> +		     SPINAND_ECCINFO(&skyhigh_spinand_ooblayout,
> +		     		     skyhigh_spinand_ecc_get_status)),
> +	SPINAND_INFO("S35ML02G300",
> +		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x25),
> +		     NAND_MEMORG(1, 2048, 128, 64, 2048, 40, 2, 1, 1),
> +		     NAND_ECCREQ(6, 32),
> +		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
> +					      &write_cache_variants,
> +					      &update_cache_variants),
> +		     SPINAND_ON_DIE_ECC_MANDATORY,
> +		     SPINAND_ECCINFO(&skyhigh_spinand_ooblayout,
> +		     		     skyhigh_spinand_ecc_get_status)),
> +	SPINAND_INFO("S35ML04G300",
> +		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x35),
> +		     NAND_MEMORG(1, 2048, 128, 64, 4096, 80, 2, 1, 1),
> +		     NAND_ECCREQ(6, 32),
> +		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
> +					      &write_cache_variants,
> +					      &update_cache_variants),
> +		     SPINAND_ON_DIE_ECC_MANDATORY,
> +		     SPINAND_ECCINFO(&skyhigh_spinand_ooblayout,
> +		     		     skyhigh_spinand_ecc_get_status)),
> +};
> +
> +static int skyhigh_spinand_init(struct spinand_device *spinand)
> +{
> +	return spinand_write_reg_op(spinand, REG_BLOCK_LOCK,
> +				    SKYHIGH_CONFIG_PROTECT_EN);

Is this really relevant? Isn't there an API for the block lock
mechanism?

> +}
> +
> +static const struct spinand_manufacturer_ops skyhigh_spinand_manuf_ops = {
> +	.init = skyhigh_spinand_init,
> + };
> +
> +const struct spinand_manufacturer skyhigh_spinand_manufacturer = {
> +	.id = SPINAND_MFR_SKYHIGH,
> +	.name = "SkyHigh",
> +	.chips = skyhigh_spinand_table,
> +	.nchips = ARRAY_SIZE(skyhigh_spinand_table),
> +	.ops = &skyhigh_spinand_manuf_ops,
> +};
> diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
> old mode 100644
> new mode 100755
> index badb4c1ac079..0e135076df24
> --- a/include/linux/mtd/spinand.h
> +++ b/include/linux/mtd/spinand.h
> @@ -268,6 +268,7 @@ extern const struct spinand_manufacturer gigadevice_spinand_manufacturer;
>  extern const struct spinand_manufacturer macronix_spinand_manufacturer;
>  extern const struct spinand_manufacturer micron_spinand_manufacturer;
>  extern const struct spinand_manufacturer paragon_spinand_manufacturer;
> +extern const struct spinand_manufacturer skyhigh_spinand_manufacturer;
>  extern const struct spinand_manufacturer toshiba_spinand_manufacturer;
>  extern const struct spinand_manufacturer winbond_spinand_manufacturer;
>  extern const struct spinand_manufacturer xtx_spinand_manufacturer;
> @@ -312,6 +313,7 @@ struct spinand_ecc_info {
>  
>  #define SPINAND_HAS_QE_BIT		BIT(0)
>  #define SPINAND_HAS_CR_FEAT_BIT		BIT(1)
> +#define SPINAND_ON_DIE_ECC_MANDATORY	BIT(2)	/* SHM */

If we go this route, then "mandatory" is not relevant here, we shall
convey the fact that the on-die ECC engine cannot be disabled and as
mentioned above, there are other impacts.

>  
>  /**
>   * struct spinand_ondie_ecc_conf - private SPI-NAND on-die ECC engine structure
> @@ -518,5 +520,6 @@ int spinand_match_and_init(struct spinand_device *spinand,
>  
>  int spinand_upd_cfg(struct spinand_device *spinand, u8 mask, u8 val);
>  int spinand_select_target(struct spinand_device *spinand, unsigned int target);
> +int spinand_write_reg_op(struct spinand_device *spinand, u8 reg, u8 val);
>  
>  #endif /* __LINUX_MTD_SPINAND_H */


Thanks,
Miquèl

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

RE: Re:

[Kyeongrho.Kim]

Hi Miquel,
Thank you for your comment.
I tried to match the patch format, but it seems to be not enough yet. 
Can you send me a good sample for the patch format?
Thanks,
KR
-----Original Message-----
From: Miquel Raynal <miquel.raynal@bootlin.com> 
Sent: Thursday, March 7, 2024 5:01 PM
To: Kyeongrho.Kim <kr.kim@skyhighmemory.com>
Cc: richard@nod.at; vigneshr@ti.com; mmkurbanov@salutedevices.com; ddrokosov@sberdevices.ru; gch981213@gmail.com; michael@walle.cc; broonie@kernel.org; mika.westerberg@linux.intel.com; acelan.kao@canonical.com; linux-kernel@vger.kernel.org; linux-mtd@lists.infradead.org; Mohamed Sardi <moh.sardi@skyhighmemory.com>; Changsub.Shim <changsub.shim@skyhighmemory.com>
Subject: Re:

Hi,

kr.kim@skyhighmemory.com wrote on Thu,  7 Mar 2024 15:07:29 +0900:

> Feat: Add SkyHigh Memory Patch code
> 
> Add SPI Nand Patch code of SkyHigh Memory
> - Add company dependent code with 'skyhigh.c'
> - Insert into 'core.c' so that 'always ECC on'

Patch formatting is still messed up.

> commit 6061b97a830af8cb5fd0917e833e779451f9046a (HEAD -> master)
> Author: KR Kim <kr.kim@skyhighmemory.com>
> Date:   Thu Mar 7 13:24:11 2024 +0900
> 
>     SPI Nand Patch code of SkyHigh Momory
> 
>     Signed-off-by: KR Kim <kr.kim@skyhighmemory.com>
> 
> From 6061b97a830af8cb5fd0917e833e779451f9046a Mon Sep 17 00:00:00 2001
> From: KR Kim <kr.kim@skyhighmemory.com>
> Date: Thu, 7 Mar 2024 13:24:11 +0900
> Subject: [PATCH] SPI Nand Patch code of SkyHigh Memory
> 
> ---
>  drivers/mtd/nand/spi/Makefile  |   2 +-
>  drivers/mtd/nand/spi/core.c    |   7 +-
>  drivers/mtd/nand/spi/skyhigh.c | 155 +++++++++++++++++++++++++++++++++
>  include/linux/mtd/spinand.h    |   3 +
>  4 files changed, 165 insertions(+), 2 deletions(-)  mode change 
> 100644 => 100755 drivers/mtd/nand/spi/Makefile  mode change 100644 => 
> 100755 drivers/mtd/nand/spi/core.c  create mode 100644 
> drivers/mtd/nand/spi/skyhigh.c  mode change 100644 => 100755 
> include/linux/mtd/spinand.h
> 
> diff --git a/drivers/mtd/nand/spi/Makefile 
> b/drivers/mtd/nand/spi/Makefile old mode 100644 new mode 100755 index 
> 19cc77288ebb..1e61ab21893a
> --- a/drivers/mtd/nand/spi/Makefile
> +++ b/drivers/mtd/nand/spi/Makefile
> @@ -1,4 +1,4 @@
>  # SPDX-License-Identifier: GPL-2.0
>  spinand-objs := core.o alliancememory.o ato.o esmt.o foresee.o 
> gigadevice.o macronix.o -spinand-objs += micron.o paragon.o toshiba.o 
> winbond.o xtx.o
> +spinand-objs += micron.o paragon.o skyhigh.o toshiba.o winbond.o 
> +xtx.o
>  obj-$(CONFIG_MTD_SPI_NAND) += spinand.o diff --git 
> a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c old mode 
> 100644 new mode 100755 index e0b6715e5dfe..e3f0a7544ba4
> --- a/drivers/mtd/nand/spi/core.c
> +++ b/drivers/mtd/nand/spi/core.c
> @@ -34,7 +34,7 @@ static int spinand_read_reg_op(struct spinand_device *spinand, u8 reg, u8 *val)
>  	return 0;
>  }
>  
> -static int spinand_write_reg_op(struct spinand_device *spinand, u8 
> reg, u8 val)
> +int spinand_write_reg_op(struct spinand_device *spinand, u8 reg, u8 
> +val)

Please do this in a separate commit.

>  {
>  	struct spi_mem_op op = SPINAND_SET_FEATURE_OP(reg,
>  						      spinand->scratchbuf);
> @@ -196,6 +196,10 @@ static int spinand_init_quad_enable(struct 
> spinand_device *spinand)  static int spinand_ecc_enable(struct spinand_device *spinand,
>  			      bool enable)
>  {
> +	/* SHM : always ECC enable */
> +	if (spinand->flags & SPINAND_ON_DIE_ECC_MANDATORY)
> +		return 0;

Silently always enabling ECC is not possible. If you cannot disable the on-die engine, then:
- you should prevent any other engine type to be used
- you should error out if a raw access is requested
- these chips are broken, IMO

> +
>  	return spinand_upd_cfg(spinand, CFG_ECC_ENABLE,
>  			       enable ? CFG_ECC_ENABLE : 0);  } @@ -945,6 +949,7 @@ static 
> const struct spinand_manufacturer *spinand_manufacturers[] = {
>  	&macronix_spinand_manufacturer,
>  	&micron_spinand_manufacturer,
>  	&paragon_spinand_manufacturer,
> +	&skyhigh_spinand_manufacturer,
>  	&toshiba_spinand_manufacturer,
>  	&winbond_spinand_manufacturer,
>  	&xtx_spinand_manufacturer,
> diff --git a/drivers/mtd/nand/spi/skyhigh.c 
> b/drivers/mtd/nand/spi/skyhigh.c new file mode 100644 index 
> 000000000000..92e7572094ff
> --- /dev/null
> +++ b/drivers/mtd/nand/spi/skyhigh.c
> @@ -0,0 +1,155 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2022 SkyHigh Memory Limited
> + *
> + * Author: Takahiro Kuwano <takahiro.kuwano@infineon.com>  */
> +
> +#include <linux/device.h>
> +#include <linux/kernel.h>
> +#include <linux/mtd/spinand.h>
> +
> +#define SPINAND_MFR_SKYHIGH		0x01
> +
> +#define SKYHIGH_STATUS_ECC_1TO2_BITFLIPS	(1 << 4)
> +#define SKYHIGH_STATUS_ECC_3TO6_BITFLIPS	(2 << 4)
> +#define SKYHIGH_STATUS_ECC_UNCOR_ERROR  	(3 << 4)
> +
> +#define SKYHIGH_CONFIG_PROTECT_EN	BIT(1)
> +
> +static SPINAND_OP_VARIANTS(read_cache_variants,
> +		SPINAND_PAGE_READ_FROM_CACHE_QUADIO_OP(0, 4, NULL, 0),
> +		SPINAND_PAGE_READ_FROM_CACHE_X4_OP(0, 1, NULL, 0),
> +		SPINAND_PAGE_READ_FROM_CACHE_DUALIO_OP(0, 2, NULL, 0),
> +		SPINAND_PAGE_READ_FROM_CACHE_X2_OP(0, 1, NULL, 0),
> +		SPINAND_PAGE_READ_FROM_CACHE_OP(true, 0, 1, NULL, 0),
> +		SPINAND_PAGE_READ_FROM_CACHE_OP(false, 0, 1, NULL, 0));
> +
> +static SPINAND_OP_VARIANTS(write_cache_variants,
> +		SPINAND_PROG_LOAD_X4(true, 0, NULL, 0),
> +		SPINAND_PROG_LOAD(true, 0, NULL, 0));
> +
> +static SPINAND_OP_VARIANTS(update_cache_variants,
> +		SPINAND_PROG_LOAD_X4(false, 0, NULL, 0),
> +		SPINAND_PROG_LOAD(false, 0, NULL, 0));
> +
> +static int skyhigh_spinand_ooblayout_ecc(struct mtd_info *mtd, int section,
> +					 struct mtd_oob_region *region)
> +{
> +	if (section)
> +		return -ERANGE;
> +
> +	/* SkyHigh's ecc parity is stored in the internal hidden area and is 
> +not needed for them. */

		     ECC		     an

"needed" is wrong here. Just stop after "area"


> +	region->length = 0;
> +	region->offset = mtd->oobsize;
> +
> +	return 0;
> +}
> +
> +static int skyhigh_spinand_ooblayout_free(struct mtd_info *mtd, int section,
> +					  struct mtd_oob_region *region) {
> +	if (section)
> +		return -ERANGE;
> +
> +	region->length = mtd->oobsize - 2;
> +	region->offset = 2;
> +
> +	return 0;
> +}
> +
> +static const struct mtd_ooblayout_ops skyhigh_spinand_ooblayout = {
> +	.ecc = skyhigh_spinand_ooblayout_ecc,
> +	.free = skyhigh_spinand_ooblayout_free, };
> +
> +static int skyhigh_spinand_ecc_get_status(struct spinand_device *spinand,
> +				  u8 status)
> +{
> +	/* SHM
> +	 * 00 : No bit-flip
> +	 * 01 : 1-2 errors corrected
> +	 * 10 : 3-6 errors corrected         
> +	 * 11 : uncorrectable
> +	 */

Thanks for the comment but the switch case looks rather straightforward, it is self-sufficient in this case.

> +
> +	switch (status & STATUS_ECC_MASK) {
> +	case STATUS_ECC_NO_BITFLIPS:
> +		return 0;
> +
> +	case SKYHIGH_STATUS_ECC_1TO2_BITFLIPS:
> +		return 2;
> +
> + 	case SKYHIGH_STATUS_ECC_3TO6_BITFLIPS:
> +		return 6;
> +
> + 	case SKYHIGH_STATUS_ECC_UNCOR_ERROR:
> +		return -EBADMSG;;
> +
> +	default:
> +		break;

I guess you can directly call return -EINVAL here?

> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static const struct spinand_info skyhigh_spinand_table[] = {
> +	SPINAND_INFO("S35ML01G301",
> +		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x15),
> +		     NAND_MEMORG(1, 2048, 64, 64, 1024, 20, 1, 1, 1),
> +		     NAND_ECCREQ(6, 32),
> +		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
> +					      &write_cache_variants,
> +					      &update_cache_variants),
> +		     SPINAND_ON_DIE_ECC_MANDATORY,
> +		     SPINAND_ECCINFO(&skyhigh_spinand_ooblayout,
> +		     		     skyhigh_spinand_ecc_get_status)),
> +	SPINAND_INFO("S35ML01G300",
> +		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x14),
> +		     NAND_MEMORG(1, 2048, 128, 64, 1024, 20, 1, 1, 1),
> +		     NAND_ECCREQ(6, 32),
> +		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
> +					      &write_cache_variants,
> +					      &update_cache_variants),
> +		     SPINAND_ON_DIE_ECC_MANDATORY,
> +		     SPINAND_ECCINFO(&skyhigh_spinand_ooblayout,
> +		     		     skyhigh_spinand_ecc_get_status)),
> +	SPINAND_INFO("S35ML02G300",
> +		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x25),
> +		     NAND_MEMORG(1, 2048, 128, 64, 2048, 40, 2, 1, 1),
> +		     NAND_ECCREQ(6, 32),
> +		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
> +					      &write_cache_variants,
> +					      &update_cache_variants),
> +		     SPINAND_ON_DIE_ECC_MANDATORY,
> +		     SPINAND_ECCINFO(&skyhigh_spinand_ooblayout,
> +		     		     skyhigh_spinand_ecc_get_status)),
> +	SPINAND_INFO("S35ML04G300",
> +		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x35),
> +		     NAND_MEMORG(1, 2048, 128, 64, 4096, 80, 2, 1, 1),
> +		     NAND_ECCREQ(6, 32),
> +		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
> +					      &write_cache_variants,
> +					      &update_cache_variants),
> +		     SPINAND_ON_DIE_ECC_MANDATORY,
> +		     SPINAND_ECCINFO(&skyhigh_spinand_ooblayout,
> +		     		     skyhigh_spinand_ecc_get_status)), };
> +
> +static int skyhigh_spinand_init(struct spinand_device *spinand) {
> +	return spinand_write_reg_op(spinand, REG_BLOCK_LOCK,
> +				    SKYHIGH_CONFIG_PROTECT_EN);

Is this really relevant? Isn't there an API for the block lock mechanism?

> +}
> +
> +static const struct spinand_manufacturer_ops skyhigh_spinand_manuf_ops = {
> +	.init = skyhigh_spinand_init,
> + };
> +
> +const struct spinand_manufacturer skyhigh_spinand_manufacturer = {
> +	.id = SPINAND_MFR_SKYHIGH,
> +	.name = "SkyHigh",
> +	.chips = skyhigh_spinand_table,
> +	.nchips = ARRAY_SIZE(skyhigh_spinand_table),
> +	.ops = &skyhigh_spinand_manuf_ops,
> +};
> diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h 
> old mode 100644 new mode 100755 index badb4c1ac079..0e135076df24
> --- a/include/linux/mtd/spinand.h
> +++ b/include/linux/mtd/spinand.h
> @@ -268,6 +268,7 @@ extern const struct spinand_manufacturer 
> gigadevice_spinand_manufacturer;  extern const struct 
> spinand_manufacturer macronix_spinand_manufacturer;  extern const 
> struct spinand_manufacturer micron_spinand_manufacturer;  extern const 
> struct spinand_manufacturer paragon_spinand_manufacturer;
> +extern const struct spinand_manufacturer 
> +skyhigh_spinand_manufacturer;
>  extern const struct spinand_manufacturer 
> toshiba_spinand_manufacturer;  extern const struct 
> spinand_manufacturer winbond_spinand_manufacturer;  extern const 
> struct spinand_manufacturer xtx_spinand_manufacturer; @@ -312,6 +313,7 
> @@ struct spinand_ecc_info {
>  
>  #define SPINAND_HAS_QE_BIT		BIT(0)
>  #define SPINAND_HAS_CR_FEAT_BIT		BIT(1)
> +#define SPINAND_ON_DIE_ECC_MANDATORY	BIT(2)	/* SHM */

If we go this route, then "mandatory" is not relevant here, we shall convey the fact that the on-die ECC engine cannot be disabled and as mentioned above, there are other impacts.

>  
>  /**
>   * struct spinand_ondie_ecc_conf - private SPI-NAND on-die ECC engine 
> structure @@ -518,5 +520,6 @@ int spinand_match_and_init(struct 
> spinand_device *spinand,
>  
>  int spinand_upd_cfg(struct spinand_device *spinand, u8 mask, u8 val);  
> int spinand_select_target(struct spinand_device *spinand, unsigned int 
> target);
> +int spinand_write_reg_op(struct spinand_device *spinand, u8 reg, u8 
> +val);
>  
>  #endif /* __LINUX_MTD_SPINAND_H */


Thanks,
Miquèl
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

RE: Re:

[Kyeongrho.Kim]

(I send again this mail with plain text not HTML.)

Dear Miquel,
Please see my reply in below email.
And please comment if you have any others.
Thanks,
KR

-----Original Message-----
From: Miquel Raynal <mailto:miquel.raynal@bootlin.com> 
Sent: Thursday, March 7, 2024 5:01 PM
To: Kyeongrho.Kim <mailto:kr.kim@skyhighmemory.com>
Cc: mailto:richard@nod.at; mailto:vigneshr@ti.com; mailto:mmkurbanov@salutedevices.com; mailto:ddrokosov@sberdevices.ru; mailto:gch981213@gmail.com; mailto:michael@walle.cc; mailto:broonie@kernel.org; mailto:mika.westerberg@linux.intel.com; mailto:acelan.kao@canonical.com; mailto:linux-kernel@vger.kernel.org; mailto:linux-mtd@lists.infradead.org; Mohamed Sardi <mailto:moh.sardi@skyhighmemory.com>; Changsub.Shim <mailto:changsub.shim@skyhighmemory.com>
Subject: Re:

Hi,

mailto:kr.kim@skyhighmemory.com wrote on Thu,  7 Mar 2024 15:07:29 +0900:

> Feat: Add SkyHigh Memory Patch code
> 
> Add SPI Nand Patch code of SkyHigh Memory
> - Add company dependent code with 'skyhigh.c'
> - Insert into 'core.c' so that 'always ECC on'

Patch formatting is still messed up.

> commit 6061b97a830af8cb5fd0917e833e779451f9046a (HEAD -> master)
> Author: KR Kim <mailto:kr.kim@skyhighmemory.com>
> Date:   Thu Mar 7 13:24:11 2024 +0900
> 
>     SPI Nand Patch code of SkyHigh Momory
> 
>     Signed-off-by: KR Kim <mailto:kr.kim@skyhighmemory.com>
> 
> From 6061b97a830af8cb5fd0917e833e779451f9046a Mon Sep 17 00:00:00 2001
> From: KR Kim <mailto:kr.kim@skyhighmemory.com>
> Date: Thu, 7 Mar 2024 13:24:11 +0900
> Subject: [PATCH] SPI Nand Patch code of SkyHigh Memory
> 
> ---
>  drivers/mtd/nand/spi/Makefile  |   2 +-
>  drivers/mtd/nand/spi/core.c    |   7 +-
>  drivers/mtd/nand/spi/skyhigh.c | 155 +++++++++++++++++++++++++++++++++
>  include/linux/mtd/spinand.h    |   3 +
>  4 files changed, 165 insertions(+), 2 deletions(-)  mode change 
> 100644 => 100755 drivers/mtd/nand/spi/Makefile  mode change 100644 => 
> 100755 drivers/mtd/nand/spi/core.c  create mode 100644 
> drivers/mtd/nand/spi/skyhigh.c  mode change 100644 => 100755 
> include/linux/mtd/spinand.h
> 
> diff --git a/drivers/mtd/nand/spi/Makefile 
> b/drivers/mtd/nand/spi/Makefile old mode 100644 new mode 100755 index 
> 19cc77288ebb..1e61ab21893a
> --- a/drivers/mtd/nand/spi/Makefile
> +++ b/drivers/mtd/nand/spi/Makefile
> @@ -1,4 +1,4 @@
>  # SPDX-License-Identifier: GPL-2.0
>  spinand-objs := core.o alliancememory.o ato.o esmt.o foresee.o 
> gigadevice.o macronix.o -spinand-objs += micron.o paragon.o toshiba.o 
> winbond.o xtx.o
> +spinand-objs += micron.o paragon.o skyhigh.o toshiba.o winbond.o 
> +xtx.o
>  obj-$(CONFIG_MTD_SPI_NAND) += spinand.o diff --git 
> a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c old mode 
> 100644 new mode 100755 index e0b6715e5dfe..e3f0a7544ba4
> --- a/drivers/mtd/nand/spi/core.c
> +++ b/drivers/mtd/nand/spi/core.c
> @@ -34,7 +34,7 @@ static int spinand_read_reg_op(struct spinand_device *spinand, u8 reg, u8 *val)
>     return 0;
>  }
>  
> -static int spinand_write_reg_op(struct spinand_device *spinand, u8 
> reg, u8 val)
> +int spinand_write_reg_op(struct spinand_device *spinand, u8 reg, u8 
> +val)

Please do this in a separate commit.
[SHM] May I know why we need to do a separate commit?
Please elaborate for the reason.
>  {
>     struct spi_mem_op op = SPINAND_SET_FEATURE_OP(reg,
>                                         spinand->scratchbuf);
> @@ -196,6 +196,10 @@ static int spinand_init_quad_enable(struct 
> spinand_device *spinand)  static int spinand_ecc_enable(struct spinand_device *spinand,
>                       bool enable)
>  {
> +   /* SHM : always ECC enable */
> +   if (spinand->flags & SPINAND_ON_DIE_ECC_MANDATORY)
> +         return 0;

Silently always enabling ECC is not possible. If you cannot disable the on-die engine, then:
- you should prevent any other engine type to be used
- you should error out if a raw access is requested
- these chips are broken, IMO
[SHM] I understand that you are concern.
We have already reviewed 'Always ECC on' to see if there was any problem in many aspects and confirmed that there was no problem.

> +
>     return spinand_upd_cfg(spinand, CFG_ECC_ENABLE,
>                        enable ? CFG_ECC_ENABLE : 0);  } @@ -945,6 +949,7 @@ static 
> const struct spinand_manufacturer *spinand_manufacturers[] = {
>     &macronix_spinand_manufacturer,
>     &micron_spinand_manufacturer,
>     &paragon_spinand_manufacturer,
> +   &skyhigh_spinand_manufacturer,
>     &toshiba_spinand_manufacturer,
>     &winbond_spinand_manufacturer,
>     &xtx_spinand_manufacturer,
> diff --git a/drivers/mtd/nand/spi/skyhigh.c 
> b/drivers/mtd/nand/spi/skyhigh.c new file mode 100644 index 
> 000000000000..92e7572094ff
> --- /dev/null
> +++ b/drivers/mtd/nand/spi/skyhigh.c
> @@ -0,0 +1,155 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2022 SkyHigh Memory Limited
> + *
> + * Author: Takahiro Kuwano <mailto:takahiro.kuwano@infineon.com>  */
> +
> +#include <linux/device.h>
> +#include <linux/kernel.h>
> +#include <linux/mtd/spinand.h>
> +
> +#define SPINAND_MFR_SKYHIGH      0x01
> +
> +#define SKYHIGH_STATUS_ECC_1TO2_BITFLIPS     (1 << 4)
> +#define SKYHIGH_STATUS_ECC_3TO6_BITFLIPS     (2 << 4)
> +#define SKYHIGH_STATUS_ECC_UNCOR_ERROR        (3 << 4)
> +
> +#define SKYHIGH_CONFIG_PROTECT_EN BIT(1)
> +
> +static SPINAND_OP_VARIANTS(read_cache_variants,
> +         SPINAND_PAGE_READ_FROM_CACHE_QUADIO_OP(0, 4, NULL, 0),
> +         SPINAND_PAGE_READ_FROM_CACHE_X4_OP(0, 1, NULL, 0),
> +         SPINAND_PAGE_READ_FROM_CACHE_DUALIO_OP(0, 2, NULL, 0),
> +         SPINAND_PAGE_READ_FROM_CACHE_X2_OP(0, 1, NULL, 0),
> +         SPINAND_PAGE_READ_FROM_CACHE_OP(true, 0, 1, NULL, 0),
> +         SPINAND_PAGE_READ_FROM_CACHE_OP(false, 0, 1, NULL, 0));
> +
> +static SPINAND_OP_VARIANTS(write_cache_variants,
> +         SPINAND_PROG_LOAD_X4(true, 0, NULL, 0),
> +         SPINAND_PROG_LOAD(true, 0, NULL, 0));
> +
> +static SPINAND_OP_VARIANTS(update_cache_variants,
> +         SPINAND_PROG_LOAD_X4(false, 0, NULL, 0),
> +         SPINAND_PROG_LOAD(false, 0, NULL, 0));
> +
> +static int skyhigh_spinand_ooblayout_ecc(struct mtd_info *mtd, int section,
> +                           struct mtd_oob_region *region)
> +{
> +   if (section)
> +         return -ERANGE;
> +
> +   /* SkyHigh's ecc parity is stored in the internal hidden area and is 
> +not needed for them. */

                 ECC                an

"needed" is wrong here. Just stop after "area"


> +   region->length = 0;
> +   region->offset = mtd->oobsize;
> +
> +   return 0;
> +}
> +
> +static int skyhigh_spinand_ooblayout_free(struct mtd_info *mtd, int section,
> +                             struct mtd_oob_region *region) {
> +   if (section)
> +         return -ERANGE;
> +
> +   region->length = mtd->oobsize - 2;
> +   region->offset = 2;
> +
> +   return 0;
> +}
> +
> +static const struct mtd_ooblayout_ops skyhigh_spinand_ooblayout = {
> +   .ecc = skyhigh_spinand_ooblayout_ecc,
> +   .free = skyhigh_spinand_ooblayout_free, };
> +
> +static int skyhigh_spinand_ecc_get_status(struct spinand_device *spinand,
> +                       u8 status)
> +{
> +   /* SHM
> +   * 00 : No bit-flip
> +   * 01 : 1-2 errors corrected
> +   * 10 : 3-6 errors corrected         
> +   * 11 : uncorrectable
> +   */

Thanks for the comment but the switch case looks rather straightforward, it is self-sufficient in this case.

> +
> +   switch (status & STATUS_ECC_MASK) {
> +   case STATUS_ECC_NO_BITFLIPS:
> +         return 0;
> +
> +   case SKYHIGH_STATUS_ECC_1TO2_BITFLIPS:
> +         return 2;
> +
> +   case SKYHIGH_STATUS_ECC_3TO6_BITFLIPS:
> +         return 6;
> +
> +   case SKYHIGH_STATUS_ECC_UNCOR_ERROR:
> +         return -EBADMSG;;
> +
> +   default:
> +         break;

I guess you can directly call return -EINVAL here?

> +   }
> +
> +   return -EINVAL;
> +}
> +
> +static const struct spinand_info skyhigh_spinand_table[] = {
> +   SPINAND_INFO("S35ML01G301",
> +              SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x15),
> +              NAND_MEMORG(1, 2048, 64, 64, 1024, 20, 1, 1, 1),
> +              NAND_ECCREQ(6, 32),
> +              SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
> +                                 &write_cache_variants,
> +                                 &update_cache_variants),
> +              SPINAND_ON_DIE_ECC_MANDATORY,
> +              SPINAND_ECCINFO(&skyhigh_spinand_ooblayout,
> +                           skyhigh_spinand_ecc_get_status)),
> +   SPINAND_INFO("S35ML01G300",
> +              SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x14),
> +              NAND_MEMORG(1, 2048, 128, 64, 1024, 20, 1, 1, 1),
> +              NAND_ECCREQ(6, 32),
> +              SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
> +                                 &write_cache_variants,
> +                                 &update_cache_variants),
> +              SPINAND_ON_DIE_ECC_MANDATORY,
> +              SPINAND_ECCINFO(&skyhigh_spinand_ooblayout,
> +                           skyhigh_spinand_ecc_get_status)),
> +   SPINAND_INFO("S35ML02G300",
> +              SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x25),
> +              NAND_MEMORG(1, 2048, 128, 64, 2048, 40, 2, 1, 1),
> +              NAND_ECCREQ(6, 32),
> +              SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
> +                                 &write_cache_variants,
> +                                 &update_cache_variants),
> +              SPINAND_ON_DIE_ECC_MANDATORY,
> +              SPINAND_ECCINFO(&skyhigh_spinand_ooblayout,
> +                           skyhigh_spinand_ecc_get_status)),
> +   SPINAND_INFO("S35ML04G300",
> +              SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0x35),
> +              NAND_MEMORG(1, 2048, 128, 64, 4096, 80, 2, 1, 1),
> +              NAND_ECCREQ(6, 32),
> +              SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
> +                                 &write_cache_variants,
> +                                 &update_cache_variants),
> +              SPINAND_ON_DIE_ECC_MANDATORY,
> +              SPINAND_ECCINFO(&skyhigh_spinand_ooblayout,
> +                           skyhigh_spinand_ecc_get_status)), };
> +
> +static int skyhigh_spinand_init(struct spinand_device *spinand) {
> +   return spinand_write_reg_op(spinand, REG_BLOCK_LOCK,
> +                         SKYHIGH_CONFIG_PROTECT_EN);

Is this really relevant? Isn't there an API for the block lock mechanism?
[SHM] SHM device should be done ‘Config Protect Enable’ first for unlock.
I changed to use the 'spinand_lock_block' function instead of the 'spinand_write_reg_op' function.

> +}
> +
> +static const struct spinand_manufacturer_ops skyhigh_spinand_manuf_ops = {
> +   .init = skyhigh_spinand_init,
> + };
> +
> +const struct spinand_manufacturer skyhigh_spinand_manufacturer = {
> +   .id = SPINAND_MFR_SKYHIGH,
> +   .name = "SkyHigh",
> +   .chips = skyhigh_spinand_table,
> +   .nchips = ARRAY_SIZE(skyhigh_spinand_table),
> +   .ops = &skyhigh_spinand_manuf_ops,
> +};
> diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h 
> old mode 100644 new mode 100755 index badb4c1ac079..0e135076df24
> --- a/include/linux/mtd/spinand.h
> +++ b/include/linux/mtd/spinand.h
> @@ -268,6 +268,7 @@ extern const struct spinand_manufacturer 
> gigadevice_spinand_manufacturer;  extern const struct 
> spinand_manufacturer macronix_spinand_manufacturer;  extern const 
> struct spinand_manufacturer micron_spinand_manufacturer;  extern const 
> struct spinand_manufacturer paragon_spinand_manufacturer;
> +extern const struct spinand_manufacturer 
> +skyhigh_spinand_manufacturer;
>  extern const struct spinand_manufacturer 
> toshiba_spinand_manufacturer;  extern const struct 
> spinand_manufacturer winbond_spinand_manufacturer;  extern const 
> struct spinand_manufacturer xtx_spinand_manufacturer; @@ -312,6 +313,7 
> @@ struct spinand_ecc_info {
>  
>  #define SPINAND_HAS_QE_BIT        BIT(0)
>  #define SPINAND_HAS_CR_FEAT_BIT         BIT(1)
> +#define SPINAND_ON_DIE_ECC_MANDATORY   BIT(2) /* SHM */

If we go this route, then "mandatory" is not relevant here, we shall convey the fact that the on-die ECC engine cannot be disabled and as mentioned above, there are other impacts.
[SHM] Please elaborate in more specific what I should do.
>  
>  /**
>   * struct spinand_ondie_ecc_conf - private SPI-NAND on-die ECC engine 
> structure @@ -518,5 +520,6 @@ int spinand_match_and_init(struct 
> spinand_device *spinand,
>  
>  int spinand_upd_cfg(struct spinand_device *spinand, u8 mask, u8 val);  
> int spinand_select_target(struct spinand_device *spinand, unsigned int 
> target);
> +int spinand_write_reg_op(struct spinand_device *spinand, u8 reg, u8 
> +val);
>  
>  #endif /* __LINUX_MTD_SPINAND_H */


Thanks,
Miquèl
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/