[PATCH 1/4] mtd: nand: gpio: Determine bus width automatically

[PATCH 1/4] mtd: nand: gpio: Determine bus width automatically

[Alexander Shiyan]

This patch provide automatically determine of bus width. If resource size,
supplied to the driver more than 1 byte, the NAND_BUSWIDTH_AUTO option
will be used in the MTD core.

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

diff --git a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
index 36ef07d..287b8b8 100644
--- a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
@@ -8,15 +8,14 @@ Required properties:
 - compatible : "gpio-control-nand"
 - reg : should specify localbus chip select and size used for the chip.  The
   resource describes the data bus connected to the NAND flash and all accesses
-  are made in native endianness.
+  are made in native endianness. Bus width of the device is determined
+  automatically if size > 1. If size = 1, 8 bit bus width will be used.
 - #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.
 
 Optional properties:
-- 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
   read registers (tR).  If not present then a default of 20us is used.
 - gpio-control-nand,io-sync-reg : A 64-bit physical address for a read
diff --git a/drivers/mtd/nand/gpio.c b/drivers/mtd/nand/gpio.c
index 800a1cc..f144b80 100644
--- a/drivers/mtd/nand/gpio.c
+++ b/drivers/mtd/nand/gpio.c
@@ -109,14 +109,9 @@ static int gpio_nand_get_config_of(const struct device *dev,
 	if (!dev->of_node)
 		return -ENODEV;
 
-	if (!of_property_read_u32(dev->of_node, "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;
-		}
-	}
+	/* Deprecated since 3.11-rc2 */
+	if (of_find_property(dev->of_node, "bank-width", NULL))
+		dev_notice(dev, "Property \"bank-width\" is deprecated");
 
 	plat->gpio_rdy = of_get_gpio(dev->of_node, 0);
 	plat->gpio_nce = of_get_gpio(dev->of_node, 1);
@@ -223,6 +218,14 @@ 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;
+
+	gpiomtd->plat.options &= ~(NAND_BUSWIDTH_16 | NAND_BUSWIDTH_AUTO);
+	if (resource_size(res) > 1)
+		gpiomtd->plat.options |= NAND_BUSWIDTH_AUTO;
+
 	res = gpio_nand_get_io_sync(pdev);
 	if (res) {
 		gpiomtd->io_sync = devm_ioremap_resource(&pdev->dev, res);
@@ -230,10 +233,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

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

[Alexander Shiyan]

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 f144b80..7f16962 100644
--- a/drivers/mtd/nand/gpio.c
+++ b/drivers/mtd/nand/gpio.c
@@ -181,14 +181,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;
@@ -233,34 +239,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;
 	}
 
@@ -275,8 +280,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;
@@ -295,8 +299,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

[PATCH 3/4] mtd: nand: gpio: Return real nand_scan() error code on fail and simplify error path

[Alexander Shiyan]

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

diff --git a/drivers/mtd/nand/gpio.c b/drivers/mtd/nand/gpio.c
index 7f16962..7b1dbd3 100644
--- a/drivers/mtd/nand/gpio.c
+++ b/drivers/mtd/nand/gpio.c
@@ -206,7 +206,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 ret;
 
 	if (!pdev->dev.of_node && !pdev->dev.platform_data)
 		return -EINVAL;
@@ -282,23 +282,20 @@ static int gpio_nand_probe(struct platform_device *pdev)
 
 	gpio_nand_set_wp(gpiomtd, 1);
 
-	if (nand_scan(&gpiomtd->mtd_info, 1)) {
-		ret = -ENXIO;
-		goto err_wp;
+	ret = nand_scan(&gpiomtd->mtd_info, 1);
+	if (!ret) {
+		if (gpiomtd->plat.adjust_parts)
+			gpiomtd->plat.adjust_parts(&gpiomtd->plat,
+						   gpiomtd->mtd_info.size);
+
+		ppdata.of_node = pdev->dev.of_node;
+		ret = mtd_device_parse_register(&gpiomtd->mtd_info, NULL,
+						&ppdata, gpiomtd->plat.parts,
+						gpiomtd->plat.num_parts);
+		if (!ret)
+			return 0;
 	}
 
-	if (gpiomtd->plat.adjust_parts)
-		gpiomtd->plat.adjust_parts(&gpiomtd->plat,
-					   gpiomtd->mtd_info.size);
-
-	ppdata.of_node = pdev->dev.of_node;
-	ret = mtd_device_parse_register(&gpiomtd->mtd_info, NULL, &ppdata,
-					gpiomtd->plat.parts,
-					gpiomtd->plat.num_parts);
-	if (!ret)
-		return 0;
-
-err_wp:
 	gpio_nand_set_wp(gpiomtd, 0);
 
 	return ret;
-- 
1.8.1.5

[PATCH 4/4] 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  |  16 ++--
 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                            | 104 +++++++++++++++------
 include/linux/mtd/nand-gpio.h                      |  12 ++-
 8 files changed, 109 insertions(+), 61 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
index 287b8b8..5c88241 100644
--- a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
@@ -12,8 +12,11 @@ Required properties:
   automatically if size > 1. If size =1, 8 bit bus width is used.
 - #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.
+- gpio-ale : specifies the ALE gpio pin.
+- gpio-cle : specifies the CLE gpio pin.
+- gpio-nwp : specifies the NWP gpio pin (Optional).
+- gpios-nce : specifies the NCE gpio pin or several NCE GPIOs for multichip NAND.
+- gpios-rdy : specifies the RDY gpio pin or several RDY GPIOs for multichip NAND.
 
 Optional properties:
 - chip-delay : chip dependent delay for transferring data from array to
@@ -34,11 +37,10 @@ gpio-nand@1,0 {
 	reg = <1 0x0000 0x2>;
 	#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 */>;
+	gpio-ale = <&banka 3 0>;
+	gpio-cle = <&banka 4 0>;
+	gpios-nce = <&banka 2 0>;
+	gpios-rdy = <&banka 1 0>;
 
 	partition@0 {
 	...
diff --git a/arch/arm/boot/dts/picoxcell-pc7302-pc3x2.dts b/arch/arm/boot/dts/picoxcell-pc7302-pc3x2.dts
index 1297414d..fec32ea 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 */>;
+				gpio-ale = <&banka 3 0>;
+				gpio-cle = <&banka 4 0>;
+				gpios-nce = <&banka 2 0>;
+				gpios-rdy = <&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..ef19635 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 */>;
+				gpio-ale = <&banka 3 0>;
+				gpio-cle = <&banka 4 0>;
+				gpios-nce = <&banka 2 0>;
+				gpios-rdy = <&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 5867aeb..c1fa4bd 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 7b1dbd3..c7beeeb 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
@@ -105,19 +125,34 @@ static int gpio_nand_get_config_of(const struct device *dev,
 				   struct gpio_nand_platdata *plat)
 {
 	u32 val;
+	int i;
 
 	if (!dev->of_node)
 		return -ENODEV;
 
-	/* Deprecated since 3.11-rc2 */
+	/* Deprecated since 3.11-rc2, should be removed around 3.17 */
 	if (of_find_property(dev->of_node, "bank-width", NULL))
 		dev_notice(dev, "Property \"bank-width\" is deprecated");
-
-	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(dev->of_node, "gpios", NULL)) {
+		dev_notice(dev, "Property \"gpios\" is deprecated");
+
+		plat->gpio_rdy[0] = of_get_gpio(dev->of_node, 0);
+		plat->gpio_nce[0] = 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);
+	} else {
+		plat->gpio_ale = of_get_named_gpio(dev->of_node, "gpio-ale", 0);
+		plat->gpio_cle = of_get_named_gpio(dev->of_node, "gpio-cle", 0);
+		plat->gpio_nwp = of_get_named_gpio(dev->of_node, "gpio-nwp", 0);
+
+		for (i = 1; i < MAX_NAND_PER_CHIP; i++) {
+			plat->gpio_nce[i] = of_get_named_gpio(dev->of_node,
+							      "gpios-nce", i);
+			plat->gpio_rdy[i] = of_get_named_gpio(dev->of_node,
+							      "gpios-rdy", i);
+		}
+	}
 
 	if (!of_property_read_u32(dev->of_node, "chip-delay", &val))
 		plat->chip_delay = val;
@@ -190,12 +225,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 = 1; 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;
 }
@@ -206,7 +244,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;
+	int i, ret, found = 0;
 
 	if (!pdev->dev.of_node && !pdev->dev.platform_data)
 		return -EINVAL;
@@ -239,18 +277,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)
@@ -261,19 +287,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
+				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;
@@ -282,7 +330,7 @@ static int gpio_nand_probe(struct platform_device *pdev)
 
 	gpio_nand_set_wp(gpiomtd, 1);
 
-	ret = nand_scan(&gpiomtd->mtd_info, 1);
+	ret = nand_scan(&gpiomtd->mtd_info, found);
 	if (!ret) {
 		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..4edd9d1 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	2
+
 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

Re: [PATCH 1/4] mtd: nand: gpio: Determine bus width automatically

[Brian Norris]

On Tue, Jul 23, 2013 at 5:28 AM, Alexander Shiyan <shc_work@mail.ru> wrote:
> This patch provide automatically determine of bus width. If resource size,
> supplied to the driver more than 1 byte, the NAND_BUSWIDTH_AUTO option
> will be used in the MTD core.

I presume this depends on the bugfix I sent for NAND_BUSWIDTH_AUTO /
nand_set_defaults()? That's worth noting (as I am right now), for
whenever someone gets around to merging this.

> Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> ---
>  .../devicetree/bindings/mtd/gpio-control-nand.txt  |  5 ++---

Considering the changes in device-tree bindings, it's good to CC the
device-tree list, I believe.

>  drivers/mtd/nand/gpio.c                            | 23 +++++++++++-----------
>  2 files changed, 13 insertions(+), 15 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
> index 36ef07d..287b8b8 100644
> --- a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
> +++ b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
> @@ -8,15 +8,14 @@ Required properties:
>  - compatible : "gpio-control-nand"
>  - reg : should specify localbus chip select and size used for the chip.  The
>    resource describes the data bus connected to the NAND flash and all accesses
> -  are made in native endianness.
> +  are made in native endianness. Bus width of the device is determined
> +  automatically if size > 1. If size = 1, 8 bit bus width will be used.
>  - #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.
>
>  Optional properties:
> -- bank-width : Width (in bytes) of the device.  If not present, the width
> -  defaults to 1 byte.

Do you really want to remove this property entirely? I'm not sure what
the policy is on this. And it may still be useful to leave in the
documentation, since older drivers (and potentially non-Linux OS?) may
still need the property. Maybe you can mark it as optional, and that
it may be ignored entirely.

>  - 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,io-sync-reg : A 64-bit physical address for a read
> diff --git a/drivers/mtd/nand/gpio.c b/drivers/mtd/nand/gpio.c
> index 800a1cc..f144b80 100644
> --- a/drivers/mtd/nand/gpio.c
> +++ b/drivers/mtd/nand/gpio.c
> @@ -109,14 +109,9 @@ static int gpio_nand_get_config_of(const struct device *dev,
>         if (!dev->of_node)
>                 return -ENODEV;
>
> -       if (!of_property_read_u32(dev->of_node, "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;
> -               }
> -       }
> +       /* Deprecated since 3.11-rc2 */

I doubt this patch will be included in 3.11-rc2 :) This is material
for the next merge window.

> +       if (of_find_property(dev->of_node, "bank-width", NULL))
> +               dev_notice(dev, "Property \"bank-width\" is deprecated");

If you don't totally kill this property (per my comments above), then
you probably don't want this message either. It's probably safe to
just ignore the property, if we can reliably auto-detect it instead.

>         plat->gpio_rdy = of_get_gpio(dev->of_node, 0);
>         plat->gpio_nce = of_get_gpio(dev->of_node, 1);
> @@ -223,6 +218,14 @@ 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;
> +
> +       gpiomtd->plat.options &= ~(NAND_BUSWIDTH_16 | NAND_BUSWIDTH_AUTO);
> +       if (resource_size(res) > 1)
> +               gpiomtd->plat.options |= NAND_BUSWIDTH_AUTO;
> +
>         res = gpio_nand_get_io_sync(pdev);
>         if (res) {
>                 gpiomtd->io_sync = devm_ioremap_resource(&pdev->dev, res);
> @@ -230,10 +233,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;

Brian

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

Re: [PATCH 1/4] mtd: nand: gpio: Determine bus width automatically

[Brian Norris]

On Tue, Jul 23, 2013 at 5:28 AM, Alexander Shiyan <shc_work@mail.ru> wrote:
> This patch provide automatically determine of bus width. If resource size,
> supplied to the driver more than 1 byte, the NAND_BUSWIDTH_AUTO option
> will be used in the MTD core.

I presume this depends on the bugfix I sent for NAND_BUSWIDTH_AUTO /
nand_set_defaults()? That's worth noting (as I am right now), for
whenever someone gets around to merging this.

> Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> ---
>  .../devicetree/bindings/mtd/gpio-control-nand.txt  |  5 ++---

Considering the changes in device-tree bindings, it's good to CC the
device-tree list, I believe.

>  drivers/mtd/nand/gpio.c                            | 23 +++++++++++-----------
>  2 files changed, 13 insertions(+), 15 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
> index 36ef07d..287b8b8 100644
> --- a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
> +++ b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
> @@ -8,15 +8,14 @@ Required properties:
>  - compatible : "gpio-control-nand"
>  - reg : should specify localbus chip select and size used for the chip.  The
>    resource describes the data bus connected to the NAND flash and all accesses
> -  are made in native endianness.
> +  are made in native endianness. Bus width of the device is determined
> +  automatically if size > 1. If size = 1, 8 bit bus width will be used.
>  - #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.
>
>  Optional properties:
> -- bank-width : Width (in bytes) of the device.  If not present, the width
> -  defaults to 1 byte.

Do you really want to remove this property entirely? I'm not sure what
the policy is on this. And it may still be useful to leave in the
documentation, since older drivers (and potentially non-Linux OS?) may
still need the property. Maybe you can mark it as optional, and that
it may be ignored entirely.

>  - 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,io-sync-reg : A 64-bit physical address for a read
> diff --git a/drivers/mtd/nand/gpio.c b/drivers/mtd/nand/gpio.c
> index 800a1cc..f144b80 100644
> --- a/drivers/mtd/nand/gpio.c
> +++ b/drivers/mtd/nand/gpio.c
> @@ -109,14 +109,9 @@ static int gpio_nand_get_config_of(const struct device *dev,
>         if (!dev->of_node)
>                 return -ENODEV;
>
> -       if (!of_property_read_u32(dev->of_node, "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;
> -               }
> -       }
> +       /* Deprecated since 3.11-rc2 */

I doubt this patch will be included in 3.11-rc2 :) This is material
for the next merge window.

> +       if (of_find_property(dev->of_node, "bank-width", NULL))
> +               dev_notice(dev, "Property \"bank-width\" is deprecated");

If you don't totally kill this property (per my comments above), then
you probably don't want this message either. It's probably safe to
just ignore the property, if we can reliably auto-detect it instead.

>         plat->gpio_rdy = of_get_gpio(dev->of_node, 0);
>         plat->gpio_nce = of_get_gpio(dev->of_node, 1);
> @@ -223,6 +218,14 @@ 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;
> +
> +       gpiomtd->plat.options &= ~(NAND_BUSWIDTH_16 | NAND_BUSWIDTH_AUTO);
> +       if (resource_size(res) > 1)
> +               gpiomtd->plat.options |= NAND_BUSWIDTH_AUTO;
> +
>         res = gpio_nand_get_io_sync(pdev);
>         if (res) {
>                 gpiomtd->io_sync = devm_ioremap_resource(&pdev->dev, res);
> @@ -230,10 +233,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;

Brian

Re: [PATCH 1/4] mtd: nand: gpio: Determine bus width automatically

[Brian Norris]

Resending to the new DT mailing list. (Guys, this isn't advertised
very well, and your MAINTAINERS patch hasn't been applied to Linus'
tree yet. Why the rapid transition?)

On Tue, Jul 23, 2013 at 11:42 PM, Brian Norris
<computersforpeace@gmail.com> wrote:
> On Tue, Jul 23, 2013 at 5:28 AM, Alexander Shiyan <shc_work@mail.ru> wrote:
>> This patch provide automatically determine of bus width. If resource size,
>> supplied to the driver more than 1 byte, the NAND_BUSWIDTH_AUTO option
>> will be used in the MTD core.
>
> I presume this depends on the bugfix I sent for NAND_BUSWIDTH_AUTO /
> nand_set_defaults()? That's worth noting (as I am right now), for
> whenever someone gets around to merging this.
>
>> Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
>> ---
>>  .../devicetree/bindings/mtd/gpio-control-nand.txt  |  5 ++---
>
> Considering the changes in device-tree bindings, it's good to CC the
> device-tree list, I believe.
>
>>  drivers/mtd/nand/gpio.c                            | 23 +++++++++++-----------
>>  2 files changed, 13 insertions(+), 15 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
>> index 36ef07d..287b8b8 100644
>> --- a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
>> +++ b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
>> @@ -8,15 +8,14 @@ Required properties:
>>  - compatible : "gpio-control-nand"
>>  - reg : should specify localbus chip select and size used for the chip.  The
>>    resource describes the data bus connected to the NAND flash and all accesses
>> -  are made in native endianness.
>> +  are made in native endianness. Bus width of the device is determined
>> +  automatically if size > 1. If size = 1, 8 bit bus width will be used.
>>  - #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.
>>
>>  Optional properties:
>> -- bank-width : Width (in bytes) of the device.  If not present, the width
>> -  defaults to 1 byte.
>
> Do you really want to remove this property entirely? I'm not sure what
> the policy is on this. And it may still be useful to leave in the
> documentation, since older drivers (and potentially non-Linux OS?) may
> still need the property. Maybe you can mark it as optional, and that
> it may be ignored entirely.
>
>>  - 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,io-sync-reg : A 64-bit physical address for a read
>> diff --git a/drivers/mtd/nand/gpio.c b/drivers/mtd/nand/gpio.c
>> index 800a1cc..f144b80 100644
>> --- a/drivers/mtd/nand/gpio.c
>> +++ b/drivers/mtd/nand/gpio.c
>> @@ -109,14 +109,9 @@ static int gpio_nand_get_config_of(const struct device *dev,
>>         if (!dev->of_node)
>>                 return -ENODEV;
>>
>> -       if (!of_property_read_u32(dev->of_node, "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;
>> -               }
>> -       }
>> +       /* Deprecated since 3.11-rc2 */
>
> I doubt this patch will be included in 3.11-rc2 :) This is material
> for the next merge window.
>
>> +       if (of_find_property(dev->of_node, "bank-width", NULL))
>> +               dev_notice(dev, "Property \"bank-width\" is deprecated");
>
> If you don't totally kill this property (per my comments above), then
> you probably don't want this message either. It's probably safe to
> just ignore the property, if we can reliably auto-detect it instead.
>
>>         plat->gpio_rdy = of_get_gpio(dev->of_node, 0);
>>         plat->gpio_nce = of_get_gpio(dev->of_node, 1);
>> @@ -223,6 +218,14 @@ 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;
>> +
>> +       gpiomtd->plat.options &= ~(NAND_BUSWIDTH_16 | NAND_BUSWIDTH_AUTO);
>> +       if (resource_size(res) > 1)
>> +               gpiomtd->plat.options |= NAND_BUSWIDTH_AUTO;
>> +
>>         res = gpio_nand_get_io_sync(pdev);
>>         if (res) {
>>                 gpiomtd->io_sync = devm_ioremap_resource(&pdev->dev, res);
>> @@ -230,10 +233,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;
>
> Brian

Re: [PATCH 1/4] mtd: nand: gpio: Determine bus width automatically

[Brian Norris]

Resending to the new DT mailing list. (Guys, this isn't advertised
very well, and your MAINTAINERS patch hasn't been applied to Linus'
tree yet. Why the rapid transition?)

On Tue, Jul 23, 2013 at 11:42 PM, Brian Norris
<computersforpeace@gmail.com> wrote:
> On Tue, Jul 23, 2013 at 5:28 AM, Alexander Shiyan <shc_work@mail.ru> wrote:
>> This patch provide automatically determine of bus width. If resource size,
>> supplied to the driver more than 1 byte, the NAND_BUSWIDTH_AUTO option
>> will be used in the MTD core.
>
> I presume this depends on the bugfix I sent for NAND_BUSWIDTH_AUTO /
> nand_set_defaults()? That's worth noting (as I am right now), for
> whenever someone gets around to merging this.
>
>> Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
>> ---
>>  .../devicetree/bindings/mtd/gpio-control-nand.txt  |  5 ++---
>
> Considering the changes in device-tree bindings, it's good to CC the
> device-tree list, I believe.
>
>>  drivers/mtd/nand/gpio.c                            | 23 +++++++++++-----------
>>  2 files changed, 13 insertions(+), 15 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
>> index 36ef07d..287b8b8 100644
>> --- a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
>> +++ b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
>> @@ -8,15 +8,14 @@ Required properties:
>>  - compatible : "gpio-control-nand"
>>  - reg : should specify localbus chip select and size used for the chip.  The
>>    resource describes the data bus connected to the NAND flash and all accesses
>> -  are made in native endianness.
>> +  are made in native endianness. Bus width of the device is determined
>> +  automatically if size > 1. If size = 1, 8 bit bus width will be used.
>>  - #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.
>>
>>  Optional properties:
>> -- bank-width : Width (in bytes) of the device.  If not present, the width
>> -  defaults to 1 byte.
>
> Do you really want to remove this property entirely? I'm not sure what
> the policy is on this. And it may still be useful to leave in the
> documentation, since older drivers (and potentially non-Linux OS?) may
> still need the property. Maybe you can mark it as optional, and that
> it may be ignored entirely.
>
>>  - 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,io-sync-reg : A 64-bit physical address for a read
>> diff --git a/drivers/mtd/nand/gpio.c b/drivers/mtd/nand/gpio.c
>> index 800a1cc..f144b80 100644
>> --- a/drivers/mtd/nand/gpio.c
>> +++ b/drivers/mtd/nand/gpio.c
>> @@ -109,14 +109,9 @@ static int gpio_nand_get_config_of(const struct device *dev,
>>         if (!dev->of_node)
>>                 return -ENODEV;
>>
>> -       if (!of_property_read_u32(dev->of_node, "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;
>> -               }
>> -       }
>> +       /* Deprecated since 3.11-rc2 */
>
> I doubt this patch will be included in 3.11-rc2 :) This is material
> for the next merge window.
>
>> +       if (of_find_property(dev->of_node, "bank-width", NULL))
>> +               dev_notice(dev, "Property \"bank-width\" is deprecated");
>
> If you don't totally kill this property (per my comments above), then
> you probably don't want this message either. It's probably safe to
> just ignore the property, if we can reliably auto-detect it instead.
>
>>         plat->gpio_rdy = of_get_gpio(dev->of_node, 0);
>>         plat->gpio_nce = of_get_gpio(dev->of_node, 1);
>> @@ -223,6 +218,14 @@ 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;
>> +
>> +       gpiomtd->plat.options &= ~(NAND_BUSWIDTH_16 | NAND_BUSWIDTH_AUTO);
>> +       if (resource_size(res) > 1)
>> +               gpiomtd->plat.options |= NAND_BUSWIDTH_AUTO;
>> +
>>         res = gpio_nand_get_io_sync(pdev);
>>         if (res) {
>>                 gpiomtd->io_sync = devm_ioremap_resource(&pdev->dev, res);
>> @@ -230,10 +233,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;
>
> Brian

Re: [PATCH 1/4] mtd: nand: gpio: Determine bus width automatically

[Brian Norris]

On Tue, Jul 23, 2013 at 11:52 PM, Brian Norris
<computersforpeace@gmail.com> wrote:
> and your MAINTAINERS patch hasn't been applied to Linus'
> tree yet

I stand corrected on this point. It was applied a few hours ago.

Regards,
Brian

Re: [PATCH 1/4] mtd: nand: gpio: Determine bus width automatically

[Brian Norris]

On Tue, Jul 23, 2013 at 11:52 PM, Brian Norris
<computersforpeace@gmail.com> wrote:
> and your MAINTAINERS patch hasn't been applied to Linus'
> tree yet

I stand corrected on this point. It was applied a few hours ago.

Regards,
Brian

Re: [PATCH 1/4] mtd: nand: gpio: Determine bus width automatically

[Alexander Shiyan]

On Tue, 23 Jul 2013 23:42:08 -0700
Brian Norris <computersforpeace@gmail.com> wrote:

> On Tue, Jul 23, 2013 at 5:28 AM, Alexander Shiyan <shc_work@mail.ru> wrote:
> > This patch provide automatically determine of bus width. If resource size,
> > supplied to the driver more than 1 byte, the NAND_BUSWIDTH_AUTO option
> > will be used in the MTD core.
> 
> I presume this depends on the bugfix I sent for NAND_BUSWIDTH_AUTO /
> nand_set_defaults()? That's worth noting (as I am right now), for
> whenever someone gets around to merging this.

OK.

[...]
> > diff --git a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
> > index 36ef07d..287b8b8 100644
> > --- a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
> > +++ b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
> > @@ -8,15 +8,14 @@ Required properties:
> >  - compatible : "gpio-control-nand"
> >  - reg : should specify localbus chip select and size used for the chip.  The
> >    resource describes the data bus connected to the NAND flash and all accesses
> > -  are made in native endianness.
> > +  are made in native endianness. Bus width of the device is determined
> > +  automatically if size > 1. If size = 1, 8 bit bus width will be used.
> >  - #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.
> >
> >  Optional properties:
> > -- bank-width : Width (in bytes) of the device.  If not present, the width
> > -  defaults to 1 byte.
> 
> Do you really want to remove this property entirely? I'm not sure what
> the policy is on this. And it may still be useful to leave in the
> documentation, since older drivers (and potentially non-Linux OS?) may
> still need the property. Maybe you can mark it as optional, and that
> it may be ignored entirely.

I redid it, I'll do option "bank-width" optional.

[...]
> > +       if (of_find_property(dev->of_node, "bank-width", NULL))
> > +               dev_notice(dev, "Property \"bank-width\" is deprecated");
> 
> If you don't totally kill this property (per my comments above), then
> you probably don't want this message either. It's probably safe to
> just ignore the property, if we can reliably auto-detect it instead.

What do you say about such changes in the patch 4/4?
Thanks.

[...]

-- 
Alexander Shiyan <shc_work@mail.ru>

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

Re: [PATCH 1/4] mtd: nand: gpio: Determine bus width automatically

[Alexander Shiyan]

On Tue, 23 Jul 2013 23:42:08 -0700
Brian Norris <computersforpeace@gmail.com> wrote:

> On Tue, Jul 23, 2013 at 5:28 AM, Alexander Shiyan <shc_work@mail.ru> wrote:
> > This patch provide automatically determine of bus width. If resource size,
> > supplied to the driver more than 1 byte, the NAND_BUSWIDTH_AUTO option
> > will be used in the MTD core.
> 
> I presume this depends on the bugfix I sent for NAND_BUSWIDTH_AUTO /
> nand_set_defaults()? That's worth noting (as I am right now), for
> whenever someone gets around to merging this.

OK.

[...]
> > diff --git a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
> > index 36ef07d..287b8b8 100644
> > --- a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
> > +++ b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
> > @@ -8,15 +8,14 @@ Required properties:
> >  - compatible : "gpio-control-nand"
> >  - reg : should specify localbus chip select and size used for the chip.  The
> >    resource describes the data bus connected to the NAND flash and all accesses
> > -  are made in native endianness.
> > +  are made in native endianness. Bus width of the device is determined
> > +  automatically if size > 1. If size = 1, 8 bit bus width will be used.
> >  - #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.
> >
> >  Optional properties:
> > -- bank-width : Width (in bytes) of the device.  If not present, the width
> > -  defaults to 1 byte.
> 
> Do you really want to remove this property entirely? I'm not sure what
> the policy is on this. And it may still be useful to leave in the
> documentation, since older drivers (and potentially non-Linux OS?) may
> still need the property. Maybe you can mark it as optional, and that
> it may be ignored entirely.

I redid it, I'll do option "bank-width" optional.

[...]
> > +       if (of_find_property(dev->of_node, "bank-width", NULL))
> > +               dev_notice(dev, "Property \"bank-width\" is deprecated");
> 
> If you don't totally kill this property (per my comments above), then
> you probably don't want this message either. It's probably safe to
> just ignore the property, if we can reliably auto-detect it instead.

What do you say about such changes in the patch 4/4?
Thanks.

[...]

-- 
Alexander Shiyan <shc_work@mail.ru>

Re: [PATCH 1/4] mtd: nand: gpio: Determine bus width automatically

[Brian Norris]

On Wed, Jul 24, 2013 at 6:28 AM, Alexander Shiyan <shc_work@mail.ru> wrote:
> On Tue, 23 Jul 2013 23:42:08 -0700
> Brian Norris <computersforpeace@gmail.com> wrote:
>
>> On Tue, Jul 23, 2013 at 5:28 AM, Alexander Shiyan <shc_work@mail.ru> wrote:
>> > diff --git a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
>> > index 36ef07d..287b8b8 100644
>> > --- a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
>> > +++ b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
>> > @@ -8,15 +8,14 @@ Required properties:
>> >  - compatible : "gpio-control-nand"
>> >  - reg : should specify localbus chip select and size used for the chip.  The
>> >    resource describes the data bus connected to the NAND flash and all accesses
>> > -  are made in native endianness.
>> > +  are made in native endianness. Bus width of the device is determined
>> > +  automatically if size > 1. If size = 1, 8 bit bus width will be used.
>> >  - #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.
>> >
>> >  Optional properties:
>> > -- bank-width : Width (in bytes) of the device.  If not present, the width
>> > -  defaults to 1 byte.
>>
>> Do you really want to remove this property entirely? I'm not sure what
>> the policy is on this. And it may still be useful to leave in the
>> documentation, since older drivers (and potentially non-Linux OS?) may
>> still need the property. Maybe you can mark it as optional, and that
>> it may be ignored entirely.
>
> I redid it, I'll do option "bank-width" optional.
>
> [...]
>> > +       if (of_find_property(dev->of_node, "bank-width", NULL))
>> > +               dev_notice(dev, "Property \"bank-width\" is deprecated");
>>
>> If you don't totally kill this property (per my comments above), then
>> you probably don't want this message either. It's probably safe to
>> just ignore the property, if we can reliably auto-detect it instead.
>
> What do you say about such changes in the patch 4/4?
> Thanks.

I guess you're referring to this hunk from patch 4/4?

-       /* Deprecated since 3.11-rc2 */
+       /* Deprecated since 3.11-rc2, should be removed around 3.17 */

This hunk really seems out of place in patch 4. If you even want this
change, it should go in with patch 1. I don't see how it is related to
patch 4.

But the key point is that I don't think you should try to detect (and
print notices about) the bank-width property at all. As long as the
documentation clearly states that it is optional (and that some
systems may autodetect it), I think that is sufficient not to print
any warnings. You should consider that some user may want to retain
this DT binding for some other non-Linux OS (or for some range of new
and old kernel versions) so that they need this bank-width property.

Another way to look at this is that it doesn't hurt Linux at all for
there to be an extra DT property for a device. But it can hurt the
bootloader if it needs to support other OS's/drivers that don't
autodetect bank-width property.

On the other hand, if you're going to include this kind of
schedule-related comment at all, then it should at least be correct. I
can tell you a few things for sure:
(1) This code is not going into any of the 3.11 release candidates
(2) It doesn't really make sense to a user to see references to "-rc2"
when reading Linux source code. That's very much a process-related
detail that should not go in here.

Brian

Re: [PATCH 1/4] mtd: nand: gpio: Determine bus width automatically

[Brian Norris]

On Wed, Jul 24, 2013 at 6:28 AM, Alexander Shiyan <shc_work@mail.ru> wrote:
> On Tue, 23 Jul 2013 23:42:08 -0700
> Brian Norris <computersforpeace@gmail.com> wrote:
>
>> On Tue, Jul 23, 2013 at 5:28 AM, Alexander Shiyan <shc_work@mail.ru> wrote:
>> > diff --git a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
>> > index 36ef07d..287b8b8 100644
>> > --- a/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
>> > +++ b/Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
>> > @@ -8,15 +8,14 @@ Required properties:
>> >  - compatible : "gpio-control-nand"
>> >  - reg : should specify localbus chip select and size used for the chip.  The
>> >    resource describes the data bus connected to the NAND flash and all accesses
>> > -  are made in native endianness.
>> > +  are made in native endianness. Bus width of the device is determined
>> > +  automatically if size > 1. If size = 1, 8 bit bus width will be used.
>> >  - #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.
>> >
>> >  Optional properties:
>> > -- bank-width : Width (in bytes) of the device.  If not present, the width
>> > -  defaults to 1 byte.
>>
>> Do you really want to remove this property entirely? I'm not sure what
>> the policy is on this. And it may still be useful to leave in the
>> documentation, since older drivers (and potentially non-Linux OS?) may
>> still need the property. Maybe you can mark it as optional, and that
>> it may be ignored entirely.
>
> I redid it, I'll do option "bank-width" optional.
>
> [...]
>> > +       if (of_find_property(dev->of_node, "bank-width", NULL))
>> > +               dev_notice(dev, "Property \"bank-width\" is deprecated");
>>
>> If you don't totally kill this property (per my comments above), then
>> you probably don't want this message either. It's probably safe to
>> just ignore the property, if we can reliably auto-detect it instead.
>
> What do you say about such changes in the patch 4/4?
> Thanks.

I guess you're referring to this hunk from patch 4/4?

-       /* Deprecated since 3.11-rc2 */
+       /* Deprecated since 3.11-rc2, should be removed around 3.17 */

This hunk really seems out of place in patch 4. If you even want this
change, it should go in with patch 1. I don't see how it is related to
patch 4.

But the key point is that I don't think you should try to detect (and
print notices about) the bank-width property at all. As long as the
documentation clearly states that it is optional (and that some
systems may autodetect it), I think that is sufficient not to print
any warnings. You should consider that some user may want to retain
this DT binding for some other non-Linux OS (or for some range of new
and old kernel versions) so that they need this bank-width property.

Another way to look at this is that it doesn't hurt Linux at all for
there to be an extra DT property for a device. But it can hurt the
bootloader if it needs to support other OS's/drivers that don't
autodetect bank-width property.

On the other hand, if you're going to include this kind of
schedule-related comment at all, then it should at least be correct. I
can tell you a few things for sure:
(1) This code is not going into any of the 3.11 release candidates
(2) It doesn't really make sense to a user to see references to "-rc2"
when reading Linux source code. That's very much a process-related
detail that should not go in here.

Brian

Re: [PATCH 1/4] mtd: nand: gpio: Determine bus width automatically

[Brian Norris]

(snip conversation)

BTW Alexander, please be sure to CC the appropriate list
(devicetree@vger.kernel.org) for the resend of your DT-related patches
(I initially used the wrong list). They should have better input on
the process of adding/removing/deprecating device properties. And if
they don't have any opinion, then it's probably fine to do this
whatever way you/we see fit :)

Brian

Re: [PATCH 1/4] mtd: nand: gpio: Determine bus width automatically

[Brian Norris]

(snip conversation)

BTW Alexander, please be sure to CC the appropriate list
(devicetree@vger.kernel.org) for the resend of your DT-related patches
(I initially used the wrong list). They should have better input on
the process of adding/removing/deprecating device properties. And if
they don't have any opinion, then it's probably fine to do this
whatever way you/we see fit :)

Brian

Re: [PATCH 1/4] mtd: nand: gpio: Determine bus width automatically

[Ezequiel Garcia]

On Tue, Jul 23, 2013 at 11:42:08PM -0700, Brian Norris wrote:
> On Tue, Jul 23, 2013 at 5:28 AM, Alexander Shiyan <shc_work@mail.ru> wrote:
> > This patch provide automatically determine of bus width. If resource size,
> > supplied to the driver more than 1 byte, the NAND_BUSWIDTH_AUTO option
> > will be used in the MTD core.
> 
> I presume this depends on the bugfix I sent for NAND_BUSWIDTH_AUTO /
> nand_set_defaults()? That's worth noting (as I am right now), for
> whenever someone gets around to merging this.
> 
> > Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> > ---
> >  .../devicetree/bindings/mtd/gpio-control-nand.txt  |  5 ++---
> 
> Considering the changes in device-tree bindings, it's good to CC the
> device-tree list, I believe.
> 

Adding the new devicetree list in Cc.

Keep in mind you might want to split the DT binding part of the patch
into a separate patch. For small patches like this one, it's probably
not a big deal.

-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

Re: [PATCH 1/4] mtd: nand: gpio: Determine bus width automatically

[Ezequiel Garcia]

On Tue, Jul 23, 2013 at 11:42:08PM -0700, Brian Norris wrote:
> On Tue, Jul 23, 2013 at 5:28 AM, Alexander Shiyan <shc_work@mail.ru> wrote:
> > This patch provide automatically determine of bus width. If resource size,
> > supplied to the driver more than 1 byte, the NAND_BUSWIDTH_AUTO option
> > will be used in the MTD core.
> 
> I presume this depends on the bugfix I sent for NAND_BUSWIDTH_AUTO /
> nand_set_defaults()? That's worth noting (as I am right now), for
> whenever someone gets around to merging this.
> 
> > Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> > ---
> >  .../devicetree/bindings/mtd/gpio-control-nand.txt  |  5 ++---
> 
> Considering the changes in device-tree bindings, it's good to CC the
> device-tree list, I believe.
> 

Adding the new devicetree list in Cc.

Keep in mind you might want to split the DT binding part of the patch
into a separate patch. For small patches like this one, it's probably
not a big deal.

-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com