[PATCH v5 1/3] mtd: nand: gpio: Add DT property to automatically determine bus width

[PATCH v5 1/3] mtd: nand: gpio: Add DT property to automatically determine bus width

[Alexander Shiyan]

This patch adds a property to automatically determine the NAND
bus width by CFI/ONFI information from chip. This property works
if the bus width is not specified explicitly.

Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
 .../devicetree/bindings/mtd/gpio-control-nand.txt        |  3 +++
 drivers/mtd/nand/gpio.c                                  | 16 ++++++++++++----
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
index 36ef07d..fe4e960 100644
--- a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
@@ -19,6 +19,9 @@ Optional properties:
   defaults to 1 byte.
 - chip-delay : chip dependent delay for transferring data from array to
   read registers (tR).  If not present then a default of 20us is used.
+- gpio-control-nand,bank-width-auto : Device bus width is determined
+  automatically by CFI/ONFI information from chip if "bank-width"
+  parameter is omitted (Boolean).
 - gpio-control-nand,io-sync-reg : A 64-bit physical address for a read
   location used to guard against bus reordering with regards to accesses to
   the GPIO's and the NAND flash data bus.  If present, then after changing
diff --git a/drivers/mtd/nand/gpio.c b/drivers/mtd/nand/gpio.c
index e826f89..8ec731d 100644
--- a/drivers/mtd/nand/gpio.c
+++ b/drivers/mtd/nand/gpio.c
@@ -116,6 +116,9 @@ static int gpio_nand_get_config_of(const struct device *dev,
 			dev_err(dev, "invalid bank-width %u\n", val);
 			return -EINVAL;
 		}
+	} else if (of_property_read_bool(dev->of_node,
+					 "gpio-control-nand,bank-width-auto")) {
+		plat->options |= NAND_BUSWIDTH_AUTO;
 	}
 
 	plat->gpio_rdy = of_get_gpio(dev->of_node, 0);
@@ -223,6 +226,15 @@ static int gpio_nand_probe(struct platform_device *pdev)
 	if (IS_ERR(chip->IO_ADDR_R))
 		return PTR_ERR(chip->IO_ADDR_R);
 
+	ret = gpio_nand_get_config(&pdev->dev, &gpiomtd->plat);
+	if (ret)
+		return ret;
+
+	/* Only 8-bit bus wide is possible if size is 1 */
+	if (resource_size(res) < 2)
+		gpiomtd->plat.options &= ~(NAND_BUSWIDTH_16 |
+					   NAND_BUSWIDTH_AUTO);
+
 	res = gpio_nand_get_io_sync(pdev);
 	if (res) {
 		gpiomtd->io_sync = devm_ioremap_resource(&pdev->dev, res);
@@ -230,10 +242,6 @@ static int gpio_nand_probe(struct platform_device *pdev)
 			return PTR_ERR(gpiomtd->io_sync);
 	}
 
-	ret = gpio_nand_get_config(&pdev->dev, &gpiomtd->plat);
-	if (ret)
-		return ret;
-
 	ret = devm_gpio_request(&pdev->dev, gpiomtd->plat.gpio_nce, "NAND NCE");
 	if (ret)
 		return ret;
-- 
1.8.1.5


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

[PATCH v5 2/3] mtd: nand: gpio: Use devm_gpio_request_one() where possible

[Alexander Shiyan]

Patch replaces devm_gpio_request() by devm_gpio_request_one()
function and adds a helper for set/clear WP signal. This change
simplify source code a bit.

Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
 drivers/mtd/nand/gpio.c | 37 ++++++++++++++++++++-----------------
 1 file changed, 20 insertions(+), 17 deletions(-)

diff --git a/drivers/mtd/nand/gpio.c b/drivers/mtd/nand/gpio.c
index 8ec731d..a3286c8 100644
--- a/drivers/mtd/nand/gpio.c
+++ b/drivers/mtd/nand/gpio.c
@@ -189,14 +189,20 @@ gpio_nand_get_io_sync(struct platform_device *pdev)
 	return platform_get_resource(pdev, IORESOURCE_MEM, 1);
 }
 
+static void gpio_nand_set_wp(struct gpiomtd *gpiomtd, int val)
+{
+	if (gpio_is_valid(gpiomtd->plat.gpio_nwp))
+		gpio_set_value(gpiomtd->plat.gpio_nwp, val);
+}
+
 static int gpio_nand_remove(struct platform_device *pdev)
 {
 	struct gpiomtd *gpiomtd = platform_get_drvdata(pdev);
 
 	nand_release(&gpiomtd->mtd_info);
 
-	if (gpio_is_valid(gpiomtd->plat.gpio_nwp))
-		gpio_set_value(gpiomtd->plat.gpio_nwp, 0);
+	gpio_nand_set_wp(gpiomtd, 0);
+
 	gpio_set_value(gpiomtd->plat.gpio_nce, 1);
 
 	return 0;
@@ -242,34 +248,33 @@ static int gpio_nand_probe(struct platform_device *pdev)
 			return PTR_ERR(gpiomtd->io_sync);
 	}
 
-	ret = devm_gpio_request(&pdev->dev, gpiomtd->plat.gpio_nce, "NAND NCE");
+	ret = devm_gpio_request_one(&pdev->dev, gpiomtd->plat.gpio_nce,
+				    GPIOF_OUT_INIT_HIGH, "NAND NCE");
 	if (ret)
 		return ret;
-	gpio_direction_output(gpiomtd->plat.gpio_nce, 1);
 
 	if (gpio_is_valid(gpiomtd->plat.gpio_nwp)) {
-		ret = devm_gpio_request(&pdev->dev, gpiomtd->plat.gpio_nwp,
-					"NAND NWP");
+		ret = devm_gpio_request_one(&pdev->dev, gpiomtd->plat.gpio_nwp,
+					    GPIOF_OUT_INIT_LOW, "NAND NWP");
 		if (ret)
 			return ret;
 	}
 
-	ret = devm_gpio_request(&pdev->dev, gpiomtd->plat.gpio_ale, "NAND ALE");
+	ret = devm_gpio_request_one(&pdev->dev, gpiomtd->plat.gpio_ale,
+				    GPIOF_OUT_INIT_LOW, "NAND ALE");
 	if (ret)
 		return ret;
-	gpio_direction_output(gpiomtd->plat.gpio_ale, 0);
 
-	ret = devm_gpio_request(&pdev->dev, gpiomtd->plat.gpio_cle, "NAND CLE");
+	ret = devm_gpio_request_one(&pdev->dev, gpiomtd->plat.gpio_cle,
+				    GPIOF_OUT_INIT_LOW, "NAND CLE");
 	if (ret)
 		return ret;
-	gpio_direction_output(gpiomtd->plat.gpio_cle, 0);
 
 	if (gpio_is_valid(gpiomtd->plat.gpio_rdy)) {
-		ret = devm_gpio_request(&pdev->dev, gpiomtd->plat.gpio_rdy,
-					"NAND RDY");
+		ret = devm_gpio_request_one(&pdev->dev, gpiomtd->plat.gpio_rdy,
+					    GPIOF_IN, "NAND RDY");
 		if (ret)
 			return ret;
-		gpio_direction_input(gpiomtd->plat.gpio_rdy);
 		chip->dev_ready = gpio_nand_devready;
 	}
 
@@ -284,8 +289,7 @@ static int gpio_nand_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, gpiomtd);
 
-	if (gpio_is_valid(gpiomtd->plat.gpio_nwp))
-		gpio_direction_output(gpiomtd->plat.gpio_nwp, 1);
+	gpio_nand_set_wp(gpiomtd, 1);
 
 	if (nand_scan(&gpiomtd->mtd_info, 1)) {
 		ret = -ENXIO;
@@ -304,8 +308,7 @@ static int gpio_nand_probe(struct platform_device *pdev)
 		return 0;
 
 err_wp:
-	if (gpio_is_valid(gpiomtd->plat.gpio_nwp))
-		gpio_set_value(gpiomtd->plat.gpio_nwp, 0);
+	gpio_nand_set_wp(gpiomtd, 0);
 
 	return ret;
 }
-- 
1.8.1.5


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

[PATCH v5 3/3] mtd: nand: gpio: Add support for multichip devices

[Alexander Shiyan]

This patch adds support for multichip NAND devices controlled through GPIOs.
To implement this, the properties of tree have been renamed. All current
boards and DTS files converted to use an updated driver. Also driver
temporarily keep support for DTS files which use the previous names scheme.

Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
 .../devicetree/bindings/mtd/gpio-control-nand.txt  |  37 +++++--
 arch/arm/boot/dts/picoxcell-pc7302-pc3x2.dts       |  10 +-
 arch/arm/boot/dts/picoxcell-pc7302-pc3x3.dts       |  10 +-
 arch/arm/mach-clps711x/board-autcpu12.c            |   4 +-
 arch/arm/mach-clps711x/board-p720t.c               |   4 +-
 arch/arm/mach-pxa/cm-x255.c                        |  10 +-
 drivers/mtd/nand/gpio.c                            | 111 +++++++++++++++------
 include/linux/mtd/nand-gpio.h                      |  12 ++-
 8 files changed, 133 insertions(+), 65 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
index fe4e960..cdc6680 100644
--- a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
@@ -11,10 +11,18 @@ Required properties:
   are made in native endianness.
 - #address-cells, #size-cells : Must be present if the device has sub-nodes
   representing partitions.
-- gpios : specifies the gpio pins to control the NAND device.  nwp is an
-  optional gpio and may be set to 0 if not present.
+- ale-gpios : specifies the ALE gpio pin.
+- cle-gpios : specifies the CLE gpio pin.
+- nce-gpios : specifies the NCE gpio pin or several NCE GPIOs for multichip NAND.
+
+Obsolete properties:
+- gpios : specifies the gpio pins to control the NAND device. rdy & nwp is an
+  optional gpios and may be set to 0 if not present. This property was be
+  replaced with xxx-gpios.
 
 Optional properties:
+- nwp-gpios : specifies the NWP gpio pin.
+- rdy-gpios : specifies the RDY gpio pin or several RDY GPIOs for multichip NAND.
 - bank-width : Width (in bytes) of the device.  If not present, the width
   defaults to 1 byte.
 - chip-delay : chip dependent delay for transferring data from array to
@@ -33,18 +41,31 @@ address space. See partition.txt for more detail.
 
 Examples:
 
-gpio-nand@1,0 {
+nand0: gpio-nand@10000000 {
+	/* 8 Bit NAND device with one chipselect */
 	compatible = "gpio-control-nand";
 	reg = <1 0x0000 0x2>;
+	ale-gpios = <&banka 1 0>;
+	cle-gpios = <&banka 2 0>;
+	nce-gpios = <&banka 3 0>;
+	rdy-gpios = <&bankb 0 0>;
 	#address-cells = <1>;
 	#size-cells = <1>;
-	gpios = <&banka 1 0	/* rdy */
-		 &banka 2 0 	/* nce */
-		 &banka 3 0 	/* ale */
-		 &banka 4 0 	/* cle */
-		 0		/* nwp */>;
 
 	partition@0 {
 	...
 	};
 };
+
+nand1: gpio-nand@10001000 {
+	/* 8/16 Bit NAND device with two chipselects */
+	compatible = "gpio-control-nand";
+	reg = <1 0x1000 0x2>;
+	gpio-control-nand,bank-width-auto;
+	ale-gpios = <&banka 4 0>;
+	cle-gpios = <&banka 5 0>;
+	nce-gpios = <&banka 6 0>, <&banka 7 0>;
+	rdy-gpios = <&bankb 1 0>, <&bankb 2 0>;
+	#address-cells = <1>;
+	#size-cells = <1>;
+};
diff --git a/arch/arm/boot/dts/picoxcell-pc7302-pc3x2.dts b/arch/arm/boot/dts/picoxcell-pc7302-pc3x2.dts
index 1297414..709eed6 100644
--- a/arch/arm/boot/dts/picoxcell-pc7302-pc3x2.dts
+++ b/arch/arm/boot/dts/picoxcell-pc7302-pc3x2.dts
@@ -44,12 +44,10 @@
 				bus-clock = <&pclk>, "bus";
 				gpio-control-nand,io-sync-reg =
 					<0x00000000 0x80220000>;
-
-				gpios = <&banka 1 0	/* rdy */
-					 &banka 2 0 	/* nce */
-					 &banka 3 0 	/* ale */
-					 &banka 4 0 	/* cle */
-					 0		/* nwp */>;
+				ale-gpios = <&banka 3 0>;
+				cle-gpios = <&banka 4 0>;
+				nce-gpios = <&banka 2 0>;
+				rdy-gpios = <&banka 1 0>;
 
 				boot@100000 {
 					label = "Boot";
diff --git a/arch/arm/boot/dts/picoxcell-pc7302-pc3x3.dts b/arch/arm/boot/dts/picoxcell-pc7302-pc3x3.dts
index 9e317a4..1438f9f 100644
--- a/arch/arm/boot/dts/picoxcell-pc7302-pc3x3.dts
+++ b/arch/arm/boot/dts/picoxcell-pc7302-pc3x3.dts
@@ -50,12 +50,10 @@
 				bus-clock = <&ebi_clk>, "bus";
 				gpio-control-nand,io-sync-reg =
 					<0x00000000 0x80220000>;
-
-				gpios = <&banka 1 0	/* rdy */
-					 &banka 2 0 	/* nce */
-					 &banka 3 0 	/* ale */
-					 &banka 4 0 	/* cle */
-					 0		/* nwp */>;
+				ale-gpios = <&banka 3 0>;
+				cle-gpios = <&banka 4 0>;
+				nce-gpios = <&banka 2 0>;
+				rdy-gpios = <&banka 1 0>;
 
 				boot@100000 {
 					label = "Boot";
diff --git a/arch/arm/mach-clps711x/board-autcpu12.c b/arch/arm/mach-clps711x/board-autcpu12.c
index f8d71a8..9dce1c2 100644
--- a/arch/arm/mach-clps711x/board-autcpu12.c
+++ b/arch/arm/mach-clps711x/board-autcpu12.c
@@ -120,8 +120,8 @@ static void __init autcpu12_adjust_parts(struct gpio_nand_platdata *pdata,
 }
 
 static struct gpio_nand_platdata autcpu12_nand_pdata __initdata = {
-	.gpio_rdy	= AUTCPU12_SMC_RDY,
-	.gpio_nce	= AUTCPU12_SMC_NCE,
+	.gpio_rdy[0]	= AUTCPU12_SMC_RDY,
+	.gpio_nce[0]	= AUTCPU12_SMC_NCE,
 	.gpio_ale	= AUTCPU12_SMC_ALE,
 	.gpio_cle	= AUTCPU12_SMC_CLE,
 	.gpio_nwp	= -1,
diff --git a/arch/arm/mach-clps711x/board-p720t.c b/arch/arm/mach-clps711x/board-p720t.c
index dd81b06..3774417 100644
--- a/arch/arm/mach-clps711x/board-p720t.c
+++ b/arch/arm/mach-clps711x/board-p720t.c
@@ -243,8 +243,8 @@ static struct mtd_partition p720t_nand_parts[] __initdata = {
 };
 
 static struct gpio_nand_platdata p720t_nand_pdata __initdata = {
-	.gpio_rdy	= -1,
-	.gpio_nce	= P720T_NAND_NCE,
+	.gpio_rdy[0]	= -1,
+	.gpio_nce[0]	= P720T_NAND_NCE,
 	.gpio_ale	= P720T_NAND_ALE,
 	.gpio_cle	= P720T_NAND_CLE,
 	.gpio_nwp	= -1,
diff --git a/arch/arm/mach-pxa/cm-x255.c b/arch/arm/mach-pxa/cm-x255.c
index be75147..3cb9f4d 100644
--- a/arch/arm/mach-pxa/cm-x255.c
+++ b/arch/arm/mach-pxa/cm-x255.c
@@ -198,11 +198,11 @@ static struct mtd_partition cmx255_nand_parts[] = {
 };
 
 static struct gpio_nand_platdata cmx255_nand_platdata = {
-	.gpio_nce = GPIO_NAND_CS,
-	.gpio_cle = GPIO_NAND_CLE,
-	.gpio_ale = GPIO_NAND_ALE,
-	.gpio_rdy = GPIO_NAND_RB,
-	.gpio_nwp = -1,
+	.gpio_nce[0]	= GPIO_NAND_CS,
+	.gpio_cle	= GPIO_NAND_CLE,
+	.gpio_ale	= GPIO_NAND_ALE,
+	.gpio_rdy[0]	= GPIO_NAND_RB,
+	.gpio_nwp	= -1,
 	.parts = cmx255_nand_parts,
 	.num_parts = ARRAY_SIZE(cmx255_nand_parts),
 	.chip_delay = 25,
diff --git a/drivers/mtd/nand/gpio.c b/drivers/mtd/nand/gpio.c
index a3286c8..ffc88b0 100644
--- a/drivers/mtd/nand/gpio.c
+++ b/drivers/mtd/nand/gpio.c
@@ -36,6 +36,7 @@ struct gpiomtd {
 	void __iomem		*io_sync;
 	struct mtd_info		mtd_info;
 	struct nand_chip	nand_chip;
+	int			chip_index;
 	struct gpio_nand_platdata plat;
 };
 
@@ -75,7 +76,8 @@ static void gpio_nand_cmd_ctrl(struct mtd_info *mtd, int cmd, unsigned int ctrl)
 	gpio_nand_dosync(gpiomtd);
 
 	if (ctrl & NAND_CTRL_CHANGE) {
-		gpio_set_value(gpiomtd->plat.gpio_nce, !(ctrl & NAND_NCE));
+		gpio_set_value(gpiomtd->plat.gpio_nce[gpiomtd->chip_index],
+			       !(ctrl & NAND_NCE));
 		gpio_set_value(gpiomtd->plat.gpio_cle, !!(ctrl & NAND_CLE));
 		gpio_set_value(gpiomtd->plat.gpio_ale, !!(ctrl & NAND_ALE));
 		gpio_nand_dosync(gpiomtd);
@@ -90,8 +92,26 @@ static void gpio_nand_cmd_ctrl(struct mtd_info *mtd, int cmd, unsigned int ctrl)
 static int gpio_nand_devready(struct mtd_info *mtd)
 {
 	struct gpiomtd *gpiomtd = gpio_nand_getpriv(mtd);
+	int idx = gpiomtd->chip_index;
 
-	return gpio_get_value(gpiomtd->plat.gpio_rdy);
+	if (!gpio_is_valid(gpiomtd->plat.gpio_rdy[idx]))
+		idx = 0;
+
+	return gpio_get_value(gpiomtd->plat.gpio_rdy[idx]);
+}
+
+static void gpio_nand_select_chip(struct mtd_info *mtd, int chipnr)
+{
+	struct gpiomtd *gpiomtd = gpio_nand_getpriv(mtd);
+
+	switch (chipnr) {
+	case -1:
+		gpio_nand_cmd_ctrl(mtd, NAND_CMD_NONE, 0 | NAND_CTRL_CHANGE);
+		break;
+	default:
+		gpiomtd->chip_index = chipnr;
+		break;
+	}
 }
 
 #ifdef CONFIG_OF
@@ -104,30 +124,47 @@ MODULE_DEVICE_TABLE(of, gpio_nand_id_table);
 static int gpio_nand_get_config_of(const struct device *dev,
 				   struct gpio_nand_platdata *plat)
 {
+	struct device_node *np = dev->of_node;
 	u32 val;
+	int i;
 
 	if (!dev->of_node)
 		return -ENODEV;
 
-	if (!of_property_read_u32(dev->of_node, "bank-width", &val)) {
+	if (!of_property_read_u32(np, "bank-width", &val)) {
 		if (val == 2) {
 			plat->options |= NAND_BUSWIDTH_16;
 		} else if (val != 1) {
 			dev_err(dev, "invalid bank-width %u\n", val);
 			return -EINVAL;
 		}
-	} else if (of_property_read_bool(dev->of_node,
+	} else if (of_property_read_bool(np,
 					 "gpio-control-nand,bank-width-auto")) {
 		plat->options |= NAND_BUSWIDTH_AUTO;
 	}
 
-	plat->gpio_rdy = of_get_gpio(dev->of_node, 0);
-	plat->gpio_nce = of_get_gpio(dev->of_node, 1);
-	plat->gpio_ale = of_get_gpio(dev->of_node, 2);
-	plat->gpio_cle = of_get_gpio(dev->of_node, 3);
-	plat->gpio_nwp = of_get_gpio(dev->of_node, 4);
+	if (of_find_property(np, "gpios", NULL)) {
+		dev_notice(dev, "Property \"gpios\" is deprecated");
+
+		plat->gpio_rdy[0] = of_get_gpio(np, 0);
+		plat->gpio_nce[0] = of_get_gpio(np, 1);
+		plat->gpio_ale = of_get_gpio(np, 2);
+		plat->gpio_cle = of_get_gpio(np, 3);
+		plat->gpio_nwp = of_get_gpio(np, 4);
+	} else {
+		plat->gpio_ale = of_get_named_gpio(np, "ale-gpios", 0);
+		plat->gpio_cle = of_get_named_gpio(np, "cle-gpios", 0);
+		plat->gpio_nwp = of_get_named_gpio(np, "nwp-gpios", 0);
+
+		for (i = 0; i < MAX_NAND_PER_CHIP; i++) {
+			plat->gpio_nce[i] = of_get_named_gpio(np,
+							      "nce-gpios", i);
+			plat->gpio_rdy[i] = of_get_named_gpio(np,
+							      "rdy-gpios", i);
+		}
+	}
 
-	if (!of_property_read_u32(dev->of_node, "chip-delay", &val))
+	if (!of_property_read_u32(np, "chip-delay", &val))
 		plat->chip_delay = val;
 
 	return 0;
@@ -198,12 +235,15 @@ static void gpio_nand_set_wp(struct gpiomtd *gpiomtd, int val)
 static int gpio_nand_remove(struct platform_device *pdev)
 {
 	struct gpiomtd *gpiomtd = platform_get_drvdata(pdev);
+	int i;
 
 	nand_release(&gpiomtd->mtd_info);
 
 	gpio_nand_set_wp(gpiomtd, 0);
 
-	gpio_set_value(gpiomtd->plat.gpio_nce, 1);
+	for (i = 0; i < MAX_NAND_PER_CHIP; i++)
+		if (gpio_is_valid(gpiomtd->plat.gpio_nce[i]))
+			gpio_set_value(gpiomtd->plat.gpio_nce[i], 1);
 
 	return 0;
 }
@@ -214,7 +254,7 @@ static int gpio_nand_probe(struct platform_device *pdev)
 	struct nand_chip *chip;
 	struct resource *res;
 	struct mtd_part_parser_data ppdata = {};
-	int ret = 0;
+	int i, ret, found = 0;
 
 	if (!pdev->dev.of_node && !dev_get_platdata(&pdev->dev))
 		return -EINVAL;
@@ -248,18 +288,6 @@ static int gpio_nand_probe(struct platform_device *pdev)
 			return PTR_ERR(gpiomtd->io_sync);
 	}
 
-	ret = devm_gpio_request_one(&pdev->dev, gpiomtd->plat.gpio_nce,
-				    GPIOF_OUT_INIT_HIGH, "NAND NCE");
-	if (ret)
-		return ret;
-
-	if (gpio_is_valid(gpiomtd->plat.gpio_nwp)) {
-		ret = devm_gpio_request_one(&pdev->dev, gpiomtd->plat.gpio_nwp,
-					    GPIOF_OUT_INIT_LOW, "NAND NWP");
-		if (ret)
-			return ret;
-	}
-
 	ret = devm_gpio_request_one(&pdev->dev, gpiomtd->plat.gpio_ale,
 				    GPIOF_OUT_INIT_LOW, "NAND ALE");
 	if (ret)
@@ -270,19 +298,41 @@ static int gpio_nand_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	if (gpio_is_valid(gpiomtd->plat.gpio_rdy)) {
-		ret = devm_gpio_request_one(&pdev->dev, gpiomtd->plat.gpio_rdy,
-					    GPIOF_IN, "NAND RDY");
+	if (gpio_is_valid(gpiomtd->plat.gpio_nwp)) {
+		ret = devm_gpio_request_one(&pdev->dev, gpiomtd->plat.gpio_nwp,
+					    GPIOF_OUT_INIT_LOW, "NAND NWP");
 		if (ret)
 			return ret;
-		chip->dev_ready = gpio_nand_devready;
 	}
 
+	for (i = 0; i < MAX_NAND_PER_CHIP; i++) {
+		if (!gpio_is_valid(gpiomtd->plat.gpio_nce[i]))
+			break;
+		if (devm_gpio_request_one(&pdev->dev, gpiomtd->plat.gpio_nce[i],
+					  GPIOF_OUT_INIT_HIGH, NULL))
+			break;
+
+		found++;
+
+		if (gpio_is_valid(gpiomtd->plat.gpio_rdy[i])) {
+			if (devm_gpio_request_one(&pdev->dev,
+						  gpiomtd->plat.gpio_rdy[i],
+						  GPIOF_IN, NULL))
+				gpiomtd->plat.gpio_rdy[i] = -1;
+			else if (!i)
+				chip->dev_ready = gpio_nand_devready;
+		}
+	}
+
+	if (!found)
+		return -EINVAL;
+
 	chip->IO_ADDR_W		= chip->IO_ADDR_R;
 	chip->ecc.mode		= NAND_ECC_SOFT;
 	chip->options		= gpiomtd->plat.options;
 	chip->chip_delay	= gpiomtd->plat.chip_delay;
 	chip->cmd_ctrl		= gpio_nand_cmd_ctrl;
+	chip->select_chip	= gpio_nand_select_chip;
 
 	gpiomtd->mtd_info.priv	= chip;
 	gpiomtd->mtd_info.owner	= THIS_MODULE;
@@ -291,10 +341,9 @@ static int gpio_nand_probe(struct platform_device *pdev)
 
 	gpio_nand_set_wp(gpiomtd, 1);
 
-	if (nand_scan(&gpiomtd->mtd_info, 1)) {
-		ret = -ENXIO;
+	ret = nand_scan(&gpiomtd->mtd_info, found);
+	if (ret)
 		goto err_wp;
-	}
 
 	if (gpiomtd->plat.adjust_parts)
 		gpiomtd->plat.adjust_parts(&gpiomtd->plat,
diff --git a/include/linux/mtd/nand-gpio.h b/include/linux/mtd/nand-gpio.h
index 51534e5..bb1d7e7 100644
--- a/include/linux/mtd/nand-gpio.h
+++ b/include/linux/mtd/nand-gpio.h
@@ -3,12 +3,14 @@
 
 #include <linux/mtd/nand.h>
 
+#define MAX_NAND_PER_CHIP	4
+
 struct gpio_nand_platdata {
-	int	gpio_nce;
-	int	gpio_nwp;
-	int	gpio_cle;
-	int	gpio_ale;
-	int	gpio_rdy;
+	int	gpio_ale;			/* ALE */
+	int	gpio_cle;			/* CLE */
+	int	gpio_nwp;			/* NWP (Optional) */
+	int	gpio_nce[MAX_NAND_PER_CHIP];	/* NCE */
+	int	gpio_rdy[MAX_NAND_PER_CHIP];	/* RDY (Optional) */
 	void	(*adjust_parts)(struct gpio_nand_platdata *, size_t);
 	struct mtd_partition *parts;
 	unsigned int num_parts;
-- 
1.8.1.5


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

Re: [PATCH v5 1/3] mtd: nand: gpio: Add DT property to automatically determine bus width

[Brian Norris]

+ Pekon, Ezequiel

Hi Alexander,

We're dealing with a similar issue in other drivers currently, and I
think it's worth straightening out the issue for all systems.

On Wed, Nov 13, 2013 at 03:58:02PM +0400, Alexander Shiyan wrote:
> This patch adds a property to automatically determine the NAND
> bus width by CFI/ONFI information from chip. This property works
> if the bus width is not specified explicitly.

This issue brings up a few questions in my mind, which are relevant to
device tree in general.

First of all, do we need a device tree property at all, for something
that is auto-detectable?

Related: is a device tree consumer (like Linux) supposed to be a
validator, or simply a best-effort? I'm considering the following case:
if Linux is allowed to auto-detect some property which also has a device
tree binding (e.g., "nand-bus-width", in
Documentation/devicetree/bindings/mtd/nand.txt), what should happen if
the binding happens to be incorrect? IOW, what if the device tree
specifies buswidth is x16, but Linux correctly detects it as x8?
Shouldn't we make the best effort to bring the hardware up, regardless
of what the device tree says?

So for something like this GPIO driver, I'm thinking that Linux should
just use NAND_BUSWIDTH_AUTO all the time [*]. But (as I'm hinting above)
that would allow DTB firmware implementors to be lazier and have a
technically-incorrect "nand-bus-width" or "bank-width" binding, since
they know it can reliably be detected and overridden by Linux.

[*] Except where resource_size(res) < 2, as Alexander already has in
this patch.

> Signed-off-by: Alexander Shiyan <shc_work-JGs/UdohzUI@public.gmane.org>
> ---
>  .../devicetree/bindings/mtd/gpio-control-nand.txt        |  3 +++
>  drivers/mtd/nand/gpio.c                                  | 16 ++++++++++++----
>  2 files changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
> index 36ef07d..fe4e960 100644
> --- a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
> +++ b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
> @@ -19,6 +19,9 @@ Optional properties:
>    defaults to 1 byte.
>  - chip-delay : chip dependent delay for transferring data from array to
>    read registers (tR).  If not present then a default of 20us is used.
> +- gpio-control-nand,bank-width-auto : Device bus width is determined
> +  automatically by CFI/ONFI information from chip if "bank-width"
> +  parameter is omitted (Boolean).

If we do resort to a new binding for auto-buswidth, it should be a
generic one that all NAND drivers can use. Maybe a separate boolean
"nand-bus-width-auto" and if it is present, then it overrules the
presence (or lack) of the "nand-bus-width" boolean property?
Or is it possible to extend "nand-bus-width" to allow the value of 0 to
mean automatic?

You may want to modify the of_get_nand_bus_width() helper (or add a new
one, of_nand_bus_width_auto()?) to drivers/of/of_mtd.c to assist with
this.

...BTW, it looks like we have a duplicate binding here: GPIO NAND
defines "bank-width" where generic NAND defines "nand-bus-width". Aren't
these essentially duplications? Can we support the generic binding in
gpio.c and discourage "bank-width"? Or is that just unnecessary effort?

>  - gpio-control-nand,io-sync-reg : A 64-bit physical address for a read
>    location used to guard against bus reordering with regards to accesses to
>    the GPIO's and the NAND flash data bus.  If present, then after changing

[...]

Brian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 1/3] mtd: nand: gpio: Add DT property to automatically determine bus width

[Brian Norris]

On Tue, Nov 26, 2013 at 05:21:58PM -0800, Brian Norris wrote:
> + Pekon, Ezequiel

+ Pekon, Ezequiel for real this time! Sorry... Everything else intact
/Brian

> 
> Hi Alexander,
> 
> We're dealing with a similar issue in other drivers currently, and I
> think it's worth straightening out the issue for all systems.
> 
> On Wed, Nov 13, 2013 at 03:58:02PM +0400, Alexander Shiyan wrote:
> > This patch adds a property to automatically determine the NAND
> > bus width by CFI/ONFI information from chip. This property works
> > if the bus width is not specified explicitly.
> 
> This issue brings up a few questions in my mind, which are relevant to
> device tree in general.
> 
> First of all, do we need a device tree property at all, for something
> that is auto-detectable?
> 
> Related: is a device tree consumer (like Linux) supposed to be a
> validator, or simply a best-effort? I'm considering the following case:
> if Linux is allowed to auto-detect some property which also has a device
> tree binding (e.g., "nand-bus-width", in
> Documentation/devicetree/bindings/mtd/nand.txt), what should happen if
> the binding happens to be incorrect? IOW, what if the device tree
> specifies buswidth is x16, but Linux correctly detects it as x8?
> Shouldn't we make the best effort to bring the hardware up, regardless
> of what the device tree says?
> 
> So for something like this GPIO driver, I'm thinking that Linux should
> just use NAND_BUSWIDTH_AUTO all the time [*]. But (as I'm hinting above)
> that would allow DTB firmware implementors to be lazier and have a
> technically-incorrect "nand-bus-width" or "bank-width" binding, since
> they know it can reliably be detected and overridden by Linux.
> 
> [*] Except where resource_size(res) < 2, as Alexander already has in
> this patch.
> 
> > Signed-off-by: Alexander Shiyan <shc_work-JGs/UdohzUI@public.gmane.org>
> > ---
> >  .../devicetree/bindings/mtd/gpio-control-nand.txt        |  3 +++
> >  drivers/mtd/nand/gpio.c                                  | 16 ++++++++++++----
> >  2 files changed, 15 insertions(+), 4 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
> > index 36ef07d..fe4e960 100644
> > --- a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
> > +++ b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
> > @@ -19,6 +19,9 @@ Optional properties:
> >    defaults to 1 byte.
> >  - chip-delay : chip dependent delay for transferring data from array to
> >    read registers (tR).  If not present then a default of 20us is used.
> > +- gpio-control-nand,bank-width-auto : Device bus width is determined
> > +  automatically by CFI/ONFI information from chip if "bank-width"
> > +  parameter is omitted (Boolean).
> 
> If we do resort to a new binding for auto-buswidth, it should be a
> generic one that all NAND drivers can use. Maybe a separate boolean
> "nand-bus-width-auto" and if it is present, then it overrules the
> presence (or lack) of the "nand-bus-width" boolean property?
> Or is it possible to extend "nand-bus-width" to allow the value of 0 to
> mean automatic?
> 
> You may want to modify the of_get_nand_bus_width() helper (or add a new
> one, of_nand_bus_width_auto()?) to drivers/of/of_mtd.c to assist with
> this.
> 
> ...BTW, it looks like we have a duplicate binding here: GPIO NAND
> defines "bank-width" where generic NAND defines "nand-bus-width". Aren't
> these essentially duplications? Can we support the generic binding in
> gpio.c and discourage "bank-width"? Or is that just unnecessary effort?
> 
> >  - gpio-control-nand,io-sync-reg : A 64-bit physical address for a read
> >    location used to guard against bus reordering with regards to accesses to
> >    the GPIO's and the NAND flash data bus.  If present, then after changing
> 
> [...]
> 
> Brian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 1/3] mtd: nand: gpio: Add DT property to automatically determine bus width

[Alexander Shiyan]

...
> We're dealing with a similar issue in other drivers currently, and I
> think it's worth straightening out the issue for all systems.
> 
> On Wed, Nov 13, 2013 at 03:58:02PM +0400, Alexander Shiyan wrote:
> > This patch adds a property to automatically determine the NAND
> > bus width by CFI/ONFI information from chip. This property works
> > if the bus width is not specified explicitly.
> 
> This issue brings up a few questions in my mind, which are relevant to
> device tree in general.
> 
> First of all, do we need a device tree property at all, for something
> that is auto-detectable?
> 
> Related: is a device tree consumer (like Linux) supposed to be a
> validator, or simply a best-effort? I'm considering the following case:
> if Linux is allowed to auto-detect some property which also has a device
> tree binding (e.g., "nand-bus-width", in
> Documentation/devicetree/bindings/mtd/nand.txt), what should happen if
> the binding happens to be incorrect? IOW, what if the device tree
> specifies buswidth is x16, but Linux correctly detects it as x8?
> Shouldn't we make the best effort to bring the hardware up, regardless
> of what the device tree says?
> 
> So for something like this GPIO driver, I'm thinking that Linux should
> just use NAND_BUSWIDTH_AUTO all the time [*]. But (as I'm hinting above)
> that would allow DTB firmware implementors to be lazier and have a
> technically-incorrect "nand-bus-width" or "bank-width" binding, since
> they know it can reliably be detected and overridden by Linux.
> 
> [*] Except where resource_size(res) < 2, as Alexander already has in
> this patch.
> 
> > Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> > ---
> >  .../devicetree/bindings/mtd/gpio-control-nand.txt        |  3 +++
> >  drivers/mtd/nand/gpio.c                                  | 16 ++++++++++++----
> >  2 files changed, 15 insertions(+), 4 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
> > index 36ef07d..fe4e960 100644
> > --- a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
> > +++ b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
> > @@ -19,6 +19,9 @@ Optional properties:
> >    defaults to 1 byte.
> >  - chip-delay : chip dependent delay for transferring data from array to
> >    read registers (tR).  If not present then a default of 20us is used.
> > +- gpio-control-nand,bank-width-auto : Device bus width is determined
> > +  automatically by CFI/ONFI information from chip if "bank-width"
> > +  parameter is omitted (Boolean).
> 
> If we do resort to a new binding for auto-buswidth, it should be a
> generic one that all NAND drivers can use. Maybe a separate boolean
> "nand-bus-width-auto" and if it is present, then it overrules the
> presence (or lack) of the "nand-bus-width" boolean property?
> Or is it possible to extend "nand-bus-width" to allow the value of 0 to
> mean automatic?

This was be my first implementation of this feature, but rejected...
http://permalink.gmane.org/gmane.linux.drivers.mtd/47302

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

Re: [PATCH v5 1/3] mtd: nand: gpio: Add DT property to automatically determine bus width

[Brian Norris]

Hi Alexander,

On Tue, Nov 26, 2013 at 8:21 PM, Alexander Shiyan <shc_work-JGs/UdohzUI@public.gmane.org> wrote:
> ...
>> We're dealing with a similar issue in other drivers currently, and I
>> think it's worth straightening out the issue for all systems.
>>
>> On Wed, Nov 13, 2013 at 03:58:02PM +0400, Alexander Shiyan wrote:
>> > This patch adds a property to automatically determine the NAND
>> > bus width by CFI/ONFI information from chip. This property works
>> > if the bus width is not specified explicitly.
>>
>> This issue brings up a few questions in my mind, which are relevant to
>> device tree in general.
>>
>> First of all, do we need a device tree property at all, for something
>> that is auto-detectable?
>>
>> Related: is a device tree consumer (like Linux) supposed to be a
>> validator, or simply a best-effort? I'm considering the following case:
>> if Linux is allowed to auto-detect some property which also has a device
>> tree binding (e.g., "nand-bus-width", in
>> Documentation/devicetree/bindings/mtd/nand.txt), what should happen if
>> the binding happens to be incorrect? IOW, what if the device tree
>> specifies buswidth is x16, but Linux correctly detects it as x8?
>> Shouldn't we make the best effort to bring the hardware up, regardless
>> of what the device tree says?
>>
>> So for something like this GPIO driver, I'm thinking that Linux should
>> just use NAND_BUSWIDTH_AUTO all the time [*]. But (as I'm hinting above)
>> that would allow DTB firmware implementors to be lazier and have a
>> technically-incorrect "nand-bus-width" or "bank-width" binding, since
>> they know it can reliably be detected and overridden by Linux.
>>
>> [*] Except where resource_size(res) < 2, as Alexander already has in
>> this patch.
>>
>> > Signed-off-by: Alexander Shiyan <shc_work-JGs/UdohzUI@public.gmane.org>
>> > ---
>> >  .../devicetree/bindings/mtd/gpio-control-nand.txt        |  3 +++
>> >  drivers/mtd/nand/gpio.c                                  | 16 ++++++++++++----
>> >  2 files changed, 15 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
>> > index 36ef07d..fe4e960 100644
>> > --- a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
>> > +++ b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
>> > @@ -19,6 +19,9 @@ Optional properties:
>> >    defaults to 1 byte.
>> >  - chip-delay : chip dependent delay for transferring data from array to
>> >    read registers (tR).  If not present then a default of 20us is used.
>> > +- gpio-control-nand,bank-width-auto : Device bus width is determined
>> > +  automatically by CFI/ONFI information from chip if "bank-width"
>> > +  parameter is omitted (Boolean).
>>
>> If we do resort to a new binding for auto-buswidth, it should be a
>> generic one that all NAND drivers can use. Maybe a separate boolean
>> "nand-bus-width-auto" and if it is present, then it overrules the
>> presence (or lack) of the "nand-bus-width" boolean property?
>> Or is it possible to extend "nand-bus-width" to allow the value of 0 to
>> mean automatic?
>
> This was be my first implementation of this feature, but rejected...
> http://permalink.gmane.org/gmane.linux.drivers.mtd/47302

Huh? Your original patch was not correct (it removed properties) and
it was not comprehensive in its description -- it didn't cover any of
the topics in my questions above. It wasn't rejected for any reason
related to my questions here.

My question is whether we can leave the documented
bus-width/bank-width binding in place, but just change the Linux
implementation to perform auto-detection instead of just using the
binding directly. And if the answer is "no", then we need a new
binding like your current patch, except that it should be usable for
all NAND.

Brian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v5 1/3] mtd: nand: gpio: Add DT property to automatically determine bus width

[Gupta, Pekon]

Hi Alexander,

> From:  Alexander Shiyan
> This patch adds a property to automatically determine the NAND
> bus width by CFI/ONFI information from chip. This property works
> if the bus width is not specified explicitly.
> 
> Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> ---
[...]

> --- a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
> +++ b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
> @@ -19,6 +19,9 @@ Optional properties:
>    defaults to 1 byte.
>  - chip-delay : chip dependent delay for transferring data from array to
>    read registers (tR).  If not present then a default of 20us is used.
> +- gpio-control-nand,bank-width-auto : Device bus width is determined
> +  automatically by CFI/ONFI information from chip if "bank-width"
> +  parameter is omitted (Boolean).
> 
There is one important thing to understand here is that when a READ_ID
or READ_PARAM command is issued to NAND device, the read_data is
is transmitted only on lower 8-bit data lines. This is as per ONFI standard.
Therefore, irrespective of whether the NAND device is x8 or x16,
ONFI parameter page is always read in 8-bit mode.. 
--------------------------
[*] Reference: ONFI spec version 3.1 (section 3.5.3. Target Initialization)
    "The Read ID and Read Parameter Page commands only use the lower 8-bits
     of the data bus. The host shall not issue commands that use a word
     data width on x16 devices until the host determines the device supports
     a 16-bit data bus width in the parameter page."
--------------------------

Now this forms the basis of NAND_BUSWIDTH_AUTO, which allows
ONFI parameter parsing '@@nand_flash_detect_onfi()' only in 8-bit
mode. And once the actual width is known, then it re-configures the
driver while returning from nand_scan_ident().

Thus, in summary: To read on-chip ONFI parameters, your driver
should always be in 8-bit mode.
Using this as basis, we tried eliminate NAND_BUSWIDTH_AUTO and
instead implemented this requirement directly as part of generic
NAND driver in nand_base.c: nand_flash_detect_onfi().

Please refer following patch and subsequent discussion on same.
(It might help you understand that we don't need to explicitly
set have NAND_BUSWIDTH_AUTO, instead we can make it implicit
in our code while scanning NAND devices)

http://lists.infradead.org/pipermail/linux-mtd/2013-November/050252.html


with regards, pekon

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

Re: [PATCH v5 1/3] mtd: nand: gpio: Add DT property to automatically determine bus width

[Alexander Shiyan]

> Hi Alexander,
> 
> We're dealing with a similar issue in other drivers currently, and I
> think it's worth straightening out the issue for all systems.
> 
> On Wed, Nov 13, 2013 at 03:58:02PM +0400, Alexander Shiyan wrote:
> > This patch adds a property to automatically determine the NAND
> > bus width by CFI/ONFI information from chip. This property works
> > if the bus width is not specified explicitly.
...

Hello Brian.
Can at least part [2/3] "mtd: nand: gpio: Use devm_gpio_request_one() where possible"
be applied at this time?
Thanks.

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

Re: [PATCH v5 1/3] mtd: nand: gpio: Add DT property to automatically determine bus width

[Ezequiel Garcia]

Hi Brian,

On Tue, Nov 26, 2013 at 05:23:38PM -0800, Brian Norris wrote:
[..]
> > 
> > If we do resort to a new binding for auto-buswidth, it should be a
> > generic one that all NAND drivers can use.

Why do we need yet another binding to describe something that's
completely discoverable?

I'm working on *removing* any need to set the bus width, either from the
driver or from the DT, so I see this patch as step backwards.

Can anyone help me understand if there's *any* valid use case where we
want to specify a-priori the bus width, considering it's completely
discoverable at run-time?
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 1/3] mtd: nand: gpio: Add DT property to automatically determine bus width

[Brian Norris]

On Fri, Nov 29, 2013 at 09:25:52AM -0300, Ezequiel Garcia wrote:
> On Tue, Nov 26, 2013 at 05:23:38PM -0800, Brian Norris wrote:
> [..]
> > > 
> > > If we do resort to a new binding for auto-buswidth, it should be a
> > > generic one that all NAND drivers can use.
> 
> Why do we need yet another binding to describe something that's
> completely discoverable?

The DT property is not specified only for the sake of the flash chip
itself, but for the sake of the controller which supports it. I suppose
we've kind of overloaded its usage, but it is not entirely
auto-detectable.

> I'm working on *removing* any need to set the bus width, either from the
> driver or from the DT, so I see this patch as step backwards.

Well, I disagree with the removal ;)

> Can anyone help me understand if there's *any* valid use case where we
> want to specify a-priori the bus width, considering it's completely
> discoverable at run-time?

I think the primary use case should be to reflect a limitation in the
hardware (besides just the flash chip). It can mean that the controller
itself only supports one bus width, or that the board is only wired up
for x8, for instance.

Brian

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

Re: [PATCH v5 1/3] mtd: nand: gpio: Add DT property to automatically determine bus width

[Brian Norris]

On Fri, Nov 29, 2013 at 12:56:04PM +0400, Alexander Shiyan wrote:
> > Hi Alexander,
> > 
> > We're dealing with a similar issue in other drivers currently, and I
> > think it's worth straightening out the issue for all systems.
> > 
> > On Wed, Nov 13, 2013 at 03:58:02PM +0400, Alexander Shiyan wrote:
> > > This patch adds a property to automatically determine the NAND
> > > bus width by CFI/ONFI information from chip. This property works
> > > if the bus width is not specified explicitly.
> ...
> 
> Hello Brian.
> Can at least part [2/3] "mtd: nand: gpio: Use devm_gpio_request_one() where possible"
> be applied at this time?

Probably. I was avoiding taking things out of order in the series for
now, but I'll take another look. If it looks good and applies
independently, then I'll take it.

Brian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 1/3] mtd: nand: gpio: Add DT property to automatically determine bus width

[Ezequiel Garcia]

Brian,

On Sat, Nov 30, 2013 at 01:15:56AM -0800, Brian Norris wrote:
> 
> > Can anyone help me understand if there's *any* valid use case where we
> > want to specify a-priori the bus width, considering it's completely
> > discoverable at run-time?
> 
> I think the primary use case should be to reflect a limitation in the
> hardware (besides just the flash chip). It can mean that the controller
> itself only supports one bus width, or that the board is only wired up
> for x8, for instance.
> 

What do you mean by "reflect a limitation" of the hardware?

Maybe I'm not really following you, but it sounds as you're mixing up
the NAND device buswidth and the memory controller buswidth. At least
I consider them as two different parameters for two different pieces
of hardware.

Consider OMAP, just as an example. We currently have two DT properties:

1. nand-bus-width: not related to OMAP but kernel wide. It's supposed
   to specify the devices buswidth.

2. gpmc,device-width: It specifies how the controller should be
   configured (it doesn't really specify hardware, but configuration,
   sadly).

I now realise maybe this piece of DT binding is not a good example of DT.

Anyway, once again: Why would we need to set "nand-bus-width" to specify
the flash device width given we can discover that as soon as the NAND
is detected?

Notice, that this discussion is independent of the discussion about removing
the NAND_BUSWIDTH_AUTO. It's just about removing a DT property for a
parameter that's runtime configurable.
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 1/3] mtd: nand: gpio: Add DT property to automatically determine bus width

[Brian Norris]

On Sat, Nov 30, 2013 at 08:17:22AM -0300, Ezequiel Garcia wrote:
> Brian,
> 
> On Sat, Nov 30, 2013 at 01:15:56AM -0800, Brian Norris wrote:
> > 
> > > Can anyone help me understand if there's *any* valid use case where we
> > > want to specify a-priori the bus width, considering it's completely
> > > discoverable at run-time?
> > 
> > I think the primary use case should be to reflect a limitation in the
> > hardware (besides just the flash chip). It can mean that the controller
> > itself only supports one bus width, or that the board is only wired up
> > for x8, for instance.
> > 
> 
> What do you mean by "reflect a limitation" of the hardware?

Read the following sentence.

Limitations: the hardware could be hooked up only for 8 data lines; or
the controller could support only one particular bus width. Either of
these can only be reflected in device tree.

> Maybe I'm not really following you, but it sounds as you're mixing up
> the NAND device buswidth and the memory controller buswidth. At least
> I consider them as two different parameters for two different pieces
> of hardware.

Yes, maybe I'm combining two properties, but we only have one way to
express this for now. This should be resolved.

> Consider OMAP, just as an example. We currently have two DT properties:
> 
> 1. nand-bus-width: not related to OMAP but kernel wide. It's supposed
>    to specify the devices buswidth.
> 
> 2. gpmc,device-width: It specifies how the controller should be
>    configured (it doesn't really specify hardware, but configuration,
>    sadly).
> 
> I now realise maybe this piece of DT binding is not a good example of DT.
> 
> Anyway, once again: Why would we need to set "nand-bus-width" to specify
> the flash device width given we can discover that as soon as the NAND
> is detected?

You've asked two different questions.

Question 1:
  Can anyone help me understand if there's *any* valid use case where we
  want to specify a-priori the bus width, considering it's completely
  discoverable at run-time?

Question 2:
  Why would we need to set "nand-bus-width" to specify the flash device
  width given we can discover that as soon as the NAND is detected?

The first question is about the NAND core in general, where I strongly
believe that a NAND driver has the right to specify "a-priori" what its
supported bus width(s) is/are.

The second question is specifically about the nand-bus-width DT
property. I think the property is somewhat broken, since it describes
the flash chip and not exactly the board wiring or controller
capabilities.

So to keep this thread focused on Alexander's patch:

+- gpio-control-nand,bank-width-auto : Device bus width is determined
+  automatically by CFI/ONFI information from chip if "bank-width"
+  parameter is omitted (Boolean).

This description fits squarely into describing the flash chip (not the
board wiring or controller limitations), and so it seems superfluous (in
agreement with Ezequiel here, right?). But we have this property already
for GPIO:

- bank-width : Width (in bytes) of the device.  If not present, the width
  defaults to 1 byte.

This seems to describe the flash chip as well, and paints us into a
corner where we can't determine if it was used because of
board/controller limitations or because of the flash chip. We can get
out of this through the use of an additional property of some kind.

I think to describe the controller/board properly, we probably need
something like this for some drivers:

  <manufacturer>,supported-bus-widths = <LIST>;

Where <LIST> can be one or more of 8 or 16, and it reflects any hardware
limitations outside of the flash chip (e.g., # of data lines; controller
limitations). And if <manufacturer>,supported-bus-widths is missing,
then we default to say that the hardware *only* supports the bus width
found in "nand-bus-width"/"bank-width"/similar. So we have this:

[I]
  <manufacturer>,supported-bus-widths = <8>, <16>;
  nand-bus-width = <don't care>;

  ===> This means the NAND driver could allow auto-detect of the flash
       buswidth. The "nand-bus-width" (or "bank-width") can actually be
       "deprecated" here.

[II]
  <manufacturer>,supported-bus-widths = <X>; // X = 8 or 16
  nand-bus-width = <Y>; // Y = 8 or 16 or missing

  ===> This means the NAND driver should only configure bus width to X.
       If Y is missing, we're OK. If X != Y, then the device tree is
       malformed.

[III]
  <manufacturer>,supported-bus-widths is missing;
  nand-bus-width = <Y>; // Y = 8 or 16 or missing

  ===> This means the NAND driver should only configure bus width to Y.

This covers the most general DT description, I think. But in the GPIO
case, the "controller" is so dead simple that we can probably represent
the "supported-bus-widths" simply through the # of connected data lines:

  gpio,data-lines = <X>;

where X = 8 or 16. If X=16, that means it can support either x8 or x16
(and hence, auto-detection in the NAND core), but if X=8, we only
support x8 and no autodetection.

Brian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 1/3] mtd: nand: gpio: Add DT property to automatically determine bus width

[Brian Norris]

On Sat, Nov 30, 2013 at 01:17:38AM -0800, Brian Norris wrote:
> On Fri, Nov 29, 2013 at 12:56:04PM +0400, Alexander Shiyan wrote:
> > Can at least part [2/3] "mtd: nand: gpio: Use devm_gpio_request_one() where possible"
> > be applied at this time?
> 
> Probably. I was avoiding taking things out of order in the series for
> now, but I'll take another look. If it looks good and applies
> independently, then I'll take it.

Patch 2 can't be applied independently. You can rebase and resend if you
want it applied.

We still haven't resolved patch 1. It doesn't seem like quite the right
binding.

Brian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 1/3] mtd: nand: gpio: Add DT property to automatically determine bus width

[Alexander Shiyan]

> On Sat, Nov 30, 2013 at 01:17:38AM -0800, Brian Norris wrote:
> > On Fri, Nov 29, 2013 at 12:56:04PM +0400, Alexander Shiyan wrote:
> > > Can at least part [2/3] "mtd: nand: gpio: Use devm_gpio_request_one() where possible"
> > > be applied at this time?
> > 
> > Probably. I was avoiding taking things out of order in the series for
> > now, but I'll take another look. If it looks good and applies
> > independently, then I'll take it.
> 
> Patch 2 can't be applied independently. You can rebase and resend if you
> want it applied.
> 
> We still haven't resolved patch 1. It doesn't seem like quite the right
> binding.

I am again inclined to believe that it is necessary to completely remove
the width parameter for the driver and always determine the bus width
automatically when the window size is more than 1 byte.

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