[PATCH v6 0/3] Mediatek SPI-NOR flash driver

[PATCH v6 0/3] Mediatek SPI-NOR flash driver

[Bayi Cheng]

This series is based on v4.3-rc1 and l2-mtd.git [0] and erase_sector
implementation patch [1]

[0]: git://git.infradead.org/l2-mtd.git
[1]: http://lists.infradead.org/pipermail/linux-mtd/2015-October//062959.html

Change in v6:
1: delete mt8173_nor_do_rx
2: delete mt8173_nor_do_rx
3: add mt8173_nor_do_tx_rx for general usage
4: support nor flash with 6 IDs
5: delete mt8173_nor_erase_sector and use "nor->erase_opcode"
6: add mt8173_nor_set_addr to programming the address register
7: initialize the ppdata in mtk_nor_init

Change in v5:
1: add "status = "disable"" to device tree
2: add document "flash" node
3: fix some statement error in Kconfig file
4: fix alphabetical order error in makefile
5: delete the parament "mtd_info *mtd" in mt8173_nor structure
6: delete SPINOR_OP_WREN repeated calls
7: add mt8173_nor_do_tx & mt8173_nor_do_rx for them full potential
8: use a subnode to represent the flash

Change in v4:
1: delete the parament "write_enable" for mt8173_nor_write_reg
2: fix the build warning for calling mt8173_nor_write_single_byte

Change in v3:
1: use switch() to replace some if-else statement
2: use shifts to replace endianness statement
3: delete some unused macros
4: use auto-increment mechanism for single write
5: write address added to 32bytes

Change in v2:
1. Rebase to 4.3-rc1
2. propagate error code
3. delete mux clock and axi clock in dts file
4. descripts more exactly for binding file
5. change file names from mtk-nor.c to mtk_quadspi.c
6. delete some functions witch were used once time

Bayi Cheng (3):
  doc: dt: add documentation for Mediatek spi-nor controller
  mtd: mtk-nor: mtk serial flash controller driver
  arm64: dts: mt8173: Add nor flash node

 .../devicetree/bindings/mtd/mtk-quadspi.txt        |  41 ++
 arch/arm64/boot/dts/mediatek/mt8173.dtsi           |  16 +
 drivers/mtd/spi-nor/Kconfig                        |   7 +
 drivers/mtd/spi-nor/Makefile                       |   1 +
 drivers/mtd/spi-nor/mtk-quadspi.c                  | 475 +++++++++++++++++++++
 5 files changed, 540 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/mtk-quadspi.txt
 create mode 100644 drivers/mtd/spi-nor/mtk-quadspi.c

--
1.8.1.1.dirty 

--
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

[PATCH v6 1/3] doc: dt: add documentation for Mediatek spi-nor controller

[Bayi Cheng]

Add device tree binding documentation for serial flash with
Mediatek serial flash controller

Signed-off-by: Bayi Cheng <bayi.cheng@mediatek.com>
---
 .../devicetree/bindings/mtd/mtk-quadspi.txt        | 41 ++++++++++++++++++++++
 1 file changed, 41 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/mtk-quadspi.txt

diff --git a/Documentation/devicetree/bindings/mtd/mtk-quadspi.txt b/Documentation/devicetree/bindings/mtd/mtk-quadspi.txt
new file mode 100644
index 0000000..866b492
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/mtk-quadspi.txt
@@ -0,0 +1,41 @@
+* MTD SPI nor driver for MTK MT81xx (and similar) serial flash controller
+
+Required properties:
+- compatible: 	  should be "mediatek,mt8173-nor";
+- reg: 		  physical base address and length of the controller's register
+- clocks: 	  the phandle of the clock needed by the nor controller
+- clock-names: 	  the name of the clocks
+		  the clocks needed "spi" and "sf". "spi" is used for spi bus,
+		  and "sf" is used for controller, these are the clocks witch
+		  hardware needs to enabling nor flash and nor flash controller.
+		  See Documentation/devicetree/bindings/clock/clock-bindings.txt for details.
+- #address-cells: should be <1>
+- #size-cells:	  should be <0>
+
+The SPI Flash must be a child of the nor_flash node and must have a
+compatible property.
+
+Required properties:
+- compatible:	  May include a device-specific string consisting of the manufacturer
+		  and name of the chip. Must also include "jedec,spi-nor" for any
+		  SPI NOR flash that can be identified by the JEDEC READ ID opcode (0x9F).
+- reg :		  Chip-Select number
+
+Example:
+
+nor_flash: spi@1100d000 {
+	compatible = "mediatek,mt8173-nor";
+	reg = <0 0x1100d000 0 0xe0>;
+	clocks = <&pericfg CLK_PERI_SPI>,
+		 <&topckgen CLK_TOP_SPINFI_IFR_SEL>;
+	clock-names = "spi", "sf";
+	#address-cells = <1>;
+	#size-cells = <0>;
+	status = "disabled";
+
+	flash@0 {
+		compatible = "jedec,spi-nor";
+		reg = <0>;
+	};
+};
+
-- 
1.8.1.1.dirty

Re: [PATCH v6 1/3] doc: dt: add documentation for Mediatek spi-nor controller

[Rob Herring]

On Fri, Nov 06, 2015 at 11:48:07PM +0800, Bayi Cheng wrote:
> Add device tree binding documentation for serial flash with
> Mediatek serial flash controller
> 
> Signed-off-by: Bayi Cheng <bayi.cheng-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

> ---
>  .../devicetree/bindings/mtd/mtk-quadspi.txt        | 41 ++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mtd/mtk-quadspi.txt
> 
> diff --git a/Documentation/devicetree/bindings/mtd/mtk-quadspi.txt b/Documentation/devicetree/bindings/mtd/mtk-quadspi.txt
> new file mode 100644
> index 0000000..866b492
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/mtk-quadspi.txt
> @@ -0,0 +1,41 @@
> +* MTD SPI nor driver for MTK MT81xx (and similar) serial flash controller
> +
> +Required properties:
> +- compatible: 	  should be "mediatek,mt8173-nor";
> +- reg: 		  physical base address and length of the controller's register
> +- clocks: 	  the phandle of the clock needed by the nor controller
> +- clock-names: 	  the name of the clocks
> +		  the clocks needed "spi" and "sf". "spi" is used for spi bus,
> +		  and "sf" is used for controller, these are the clocks witch
> +		  hardware needs to enabling nor flash and nor flash controller.
> +		  See Documentation/devicetree/bindings/clock/clock-bindings.txt for details.
> +- #address-cells: should be <1>
> +- #size-cells:	  should be <0>
> +
> +The SPI Flash must be a child of the nor_flash node and must have a
> +compatible property.
> +
> +Required properties:
> +- compatible:	  May include a device-specific string consisting of the manufacturer
> +		  and name of the chip. Must also include "jedec,spi-nor" for any
> +		  SPI NOR flash that can be identified by the JEDEC READ ID opcode (0x9F).
> +- reg :		  Chip-Select number
> +
> +Example:
> +
> +nor_flash: spi@1100d000 {
> +	compatible = "mediatek,mt8173-nor";
> +	reg = <0 0x1100d000 0 0xe0>;
> +	clocks = <&pericfg CLK_PERI_SPI>,
> +		 <&topckgen CLK_TOP_SPINFI_IFR_SEL>;
> +	clock-names = "spi", "sf";
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	status = "disabled";
> +
> +	flash@0 {
> +		compatible = "jedec,spi-nor";
> +		reg = <0>;
> +	};
> +};
> +
> -- 
> 1.8.1.1.dirty
> 
--
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 v6 1/3] doc: dt: add documentation for Mediatek spi-nor controller

[Brian Norris]

On Fri, Nov 06, 2015 at 11:48:07PM +0800, Bayi Cheng wrote:
> Add device tree binding documentation for serial flash with
> Mediatek serial flash controller
> 
> Signed-off-by: Bayi Cheng <bayi.cheng-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> ---

Applied to l2-mtd.git/next (for 4.5). This will show up in
linux-next.git after the merge window.

Also squashed in a small diff (below), to fix up some language issues
and to refer the reader to the jedec,spi-nor.txt document.

>  .../devicetree/bindings/mtd/mtk-quadspi.txt        | 41 ++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mtd/mtk-quadspi.txt
> 
> diff --git a/Documentation/devicetree/bindings/mtd/mtk-quadspi.txt b/Documentation/devicetree/bindings/mtd/mtk-quadspi.txt
> new file mode 100644
> index 0000000..866b492
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/mtk-quadspi.txt
> @@ -0,0 +1,41 @@
> +* MTD SPI nor driver for MTK MT81xx (and similar) serial flash controller

The DT binding document shouldn't be talking about software (i.e.,
shouldn't be talking about "drivers").

> +
> +Required properties:
> +- compatible: 	  should be "mediatek,mt8173-nor";
> +- reg: 		  physical base address and length of the controller's register
> +- clocks: 	  the phandle of the clock needed by the nor controller
> +- clock-names: 	  the name of the clocks
> +		  the clocks needed "spi" and "sf". "spi" is used for spi bus,
> +		  and "sf" is used for controller, these are the clocks witch
> +		  hardware needs to enabling nor flash and nor flash controller.
> +		  See Documentation/devicetree/bindings/clock/clock-bindings.txt for details.
> +- #address-cells: should be <1>
> +- #size-cells:	  should be <0>
> +
> +The SPI Flash must be a child of the nor_flash node and must have a
> +compatible property.
> +
> +Required properties:
> +- compatible:	  May include a device-specific string consisting of the manufacturer
> +		  and name of the chip. Must also include "jedec,spi-nor" for any
> +		  SPI NOR flash that can be identified by the JEDEC READ ID opcode (0x9F).
> +- reg :		  Chip-Select number
> +
> +Example:
> +
> +nor_flash: spi@1100d000 {
> +	compatible = "mediatek,mt8173-nor";
> +	reg = <0 0x1100d000 0 0xe0>;
> +	clocks = <&pericfg CLK_PERI_SPI>,
> +		 <&topckgen CLK_TOP_SPINFI_IFR_SEL>;
> +	clock-names = "spi", "sf";
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	status = "disabled";
> +
> +	flash@0 {
> +		compatible = "jedec,spi-nor";
> +		reg = <0>;
> +	};
> +};
> +

diff --git a/Documentation/devicetree/bindings/mtd/mtk-quadspi.txt b/Documentation/devicetree/bindings/mtd/mtk-quadspi.txt
index 866b492c38d2..fb314f09861b 100644
--- a/Documentation/devicetree/bindings/mtd/mtk-quadspi.txt
+++ b/Documentation/devicetree/bindings/mtd/mtk-quadspi.txt
@@ -1,19 +1,19 @@
-* MTD SPI nor driver for MTK MT81xx (and similar) serial flash controller
+* Serial NOR flash controller for MTK MT81xx (and similar)
 
 Required properties:
 - compatible: 	  should be "mediatek,mt8173-nor";
 - reg: 		  physical base address and length of the controller's register
-- clocks: 	  the phandle of the clock needed by the nor controller
-- clock-names: 	  the name of the clocks
-		  the clocks needed "spi" and "sf". "spi" is used for spi bus,
+- clocks: 	  the phandle of the clocks needed by the nor controller
+- clock-names: 	  the names of the clocks
+		  the clocks should be named "spi" and "sf". "spi" is used for spi bus,
 		  and "sf" is used for controller, these are the clocks witch
 		  hardware needs to enabling nor flash and nor flash controller.
 		  See Documentation/devicetree/bindings/clock/clock-bindings.txt for details.
 - #address-cells: should be <1>
 - #size-cells:	  should be <0>
 
-The SPI Flash must be a child of the nor_flash node and must have a
-compatible property.
+The SPI flash must be a child of the nor_flash node and must have a
+compatible property. Also see jedec,spi-nor.txt.
 
 Required properties:
 - compatible:	  May include a device-specific string consisting of the manufacturer
--
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 v6 1/3] doc: dt: add documentation for Mediatek spi-nor controller

[bayi cheng]

On Wed, 2015-11-11 at 12:38 -0800, Brian Norris wrote:
> On Fri, Nov 06, 2015 at 11:48:07PM +0800, Bayi Cheng wrote:
> > Add device tree binding documentation for serial flash with
> > Mediatek serial flash controller
> > 
> > Signed-off-by: Bayi Cheng <bayi.cheng-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> > ---
> 
> Applied to l2-mtd.git/next (for 4.5). This will show up in
> linux-next.git after the merge window.
> 
> Also squashed in a small diff (below), to fix up some language issues
> and to refer the reader to the jedec,spi-nor.txt document.
> 

OK, I will fix it in next patch!

> >  .../devicetree/bindings/mtd/mtk-quadspi.txt        | 41 ++++++++++++++++++++++
> >  1 file changed, 41 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mtd/mtk-quadspi.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/mtd/mtk-quadspi.txt b/Documentation/devicetree/bindings/mtd/mtk-quadspi.txt
> > new file mode 100644
> > index 0000000..866b492
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mtd/mtk-quadspi.txt
> > @@ -0,0 +1,41 @@
> > +* MTD SPI nor driver for MTK MT81xx (and similar) serial flash controller
> 
> The DT binding document shouldn't be talking about software (i.e.,
> shouldn't be talking about "drivers").
> 

OK, I will fix it.

> > +
> > +Required properties:
> > +- compatible: 	  should be "mediatek,mt8173-nor";
> > +- reg: 		  physical base address and length of the controller's register
> > +- clocks: 	  the phandle of the clock needed by the nor controller
> > +- clock-names: 	  the name of the clocks
> > +		  the clocks needed "spi" and "sf". "spi" is used for spi bus,
> > +		  and "sf" is used for controller, these are the clocks witch
> > +		  hardware needs to enabling nor flash and nor flash controller.
> > +		  See Documentation/devicetree/bindings/clock/clock-bindings.txt for details.
> > +- #address-cells: should be <1>
> > +- #size-cells:	  should be <0>
> > +
> > +The SPI Flash must be a child of the nor_flash node and must have a
> > +compatible property.
> > +
> > +Required properties:
> > +- compatible:	  May include a device-specific string consisting of the manufacturer
> > +		  and name of the chip. Must also include "jedec,spi-nor" for any
> > +		  SPI NOR flash that can be identified by the JEDEC READ ID opcode (0x9F).
> > +- reg :		  Chip-Select number
> > +
> > +Example:
> > +
> > +nor_flash: spi@1100d000 {
> > +	compatible = "mediatek,mt8173-nor";
> > +	reg = <0 0x1100d000 0 0xe0>;
> > +	clocks = <&pericfg CLK_PERI_SPI>,
> > +		 <&topckgen CLK_TOP_SPINFI_IFR_SEL>;
> > +	clock-names = "spi", "sf";
> > +	#address-cells = <1>;
> > +	#size-cells = <0>;
> > +	status = "disabled";
> > +
> > +	flash@0 {
> > +		compatible = "jedec,spi-nor";
> > +		reg = <0>;
> > +	};
> > +};
> > +
> 
> diff --git a/Documentation/devicetree/bindings/mtd/mtk-quadspi.txt b/Documentation/devicetree/bindings/mtd/mtk-quadspi.txt
> index 866b492c38d2..fb314f09861b 100644
> --- a/Documentation/devicetree/bindings/mtd/mtk-quadspi.txt
> +++ b/Documentation/devicetree/bindings/mtd/mtk-quadspi.txt
> @@ -1,19 +1,19 @@
> -* MTD SPI nor driver for MTK MT81xx (and similar) serial flash controller
> +* Serial NOR flash controller for MTK MT81xx (and similar)
>  
>  Required properties:
>  - compatible: 	  should be "mediatek,mt8173-nor";
>  - reg: 		  physical base address and length of the controller's register
> -- clocks: 	  the phandle of the clock needed by the nor controller
> -- clock-names: 	  the name of the clocks
> -		  the clocks needed "spi" and "sf". "spi" is used for spi bus,
> +- clocks: 	  the phandle of the clocks needed by the nor controller
> +- clock-names: 	  the names of the clocks
> +		  the clocks should be named "spi" and "sf". "spi" is used for spi bus,
>  		  and "sf" is used for controller, these are the clocks witch
>  		  hardware needs to enabling nor flash and nor flash controller.
>  		  See Documentation/devicetree/bindings/clock/clock-bindings.txt for details.
>  - #address-cells: should be <1>
>  - #size-cells:	  should be <0>
>  
> -The SPI Flash must be a child of the nor_flash node and must have a
> -compatible property.
> +The SPI flash must be a child of the nor_flash node and must have a
> +compatible property. Also see jedec,spi-nor.txt.
>  
>  Required properties:
>  - compatible:	  May include a device-specific string consisting of the manufacturer

Thanks for your instruction! and I will fix it in the next patch!


--
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 v6 1/3] doc: dt: add documentation for Mediatek spi-nor controller

[Brian Norris]

On Fri, Nov 13, 2015 at 03:20:45PM +0800, Bayi Cheng wrote:
> On Wed, 2015-11-11 at 12:38 -0800, Brian Norris wrote:
> > Applied to l2-mtd.git/next (for 4.5). This will show up in
> > linux-next.git after the merge window.
> > 
> > Also squashed in a small diff (below), to fix up some language issues
> > and to refer the reader to the jedec,spi-nor.txt document.
> > 
> 
> OK, I will fix it in next patch!

No, you don't need to send another patch. I already merged just this one
(the DT documentation) with my own small fixes, with a plan to go into
4.5. You only need to send another version of patch 2 (the driver).

During the merge window, I'm stashing changes in l2-mtd.git's 'next'
branch, so we can keep stuff that's going to 4.5 separate from stuff
thats getting merged to 4.4 right now. You can see it here for now:

http://git.infradead.org/l2-mtd.git/shortlog/refs/heads/next

After the merge window closes (probably on Sunday), I'll bring this into
+master.

Regards,
Brian

[PATCH v6 2/3] mtd: mtk-nor: mtk serial flash controller driver

[Bayi Cheng]

add spi nor flash driver for mediatek controller

Signed-off-by: Bayi Cheng <bayi.cheng-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
---
 drivers/mtd/spi-nor/Kconfig       |   7 +
 drivers/mtd/spi-nor/Makefile      |   1 +
 drivers/mtd/spi-nor/mtk-quadspi.c | 475 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 483 insertions(+)
 create mode 100644 drivers/mtd/spi-nor/mtk-quadspi.c

diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
index 89bf4c1..f544bbe 100644
--- a/drivers/mtd/spi-nor/Kconfig
+++ b/drivers/mtd/spi-nor/Kconfig
@@ -7,6 +7,13 @@ menuconfig MTD_SPI_NOR
 
 if MTD_SPI_NOR
 
+config MTD_MT81xx_NOR
+	tristate "Mediatek MT81xx SPI NOR flash controller"
+	help
+	  This enables access to SPI NOR flash, using MT81xx SPI NOR flash
+	  controller. This controller does not support generic SPI BUS, it only
+	  supports SPI NOR Flash.
+
 config MTD_SPI_NOR_USE_4K_SECTORS
 	bool "Use small 4096 B erase sectors"
 	default y
diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
index e53333e..0bf3a7f8 100644
--- a/drivers/mtd/spi-nor/Makefile
+++ b/drivers/mtd/spi-nor/Makefile
@@ -1,3 +1,4 @@
 obj-$(CONFIG_MTD_SPI_NOR)	+= spi-nor.o
 obj-$(CONFIG_SPI_FSL_QUADSPI)	+= fsl-quadspi.o
+obj-$(CONFIG_MTD_MT81xx_NOR)    += mtk-quadspi.o
 obj-$(CONFIG_SPI_NXP_SPIFI)	+= nxp-spifi.o
diff --git a/drivers/mtd/spi-nor/mtk-quadspi.c b/drivers/mtd/spi-nor/mtk-quadspi.c
new file mode 100644
index 0000000..6582811
--- /dev/null
+++ b/drivers/mtd/spi-nor/mtk-quadspi.c
@@ -0,0 +1,475 @@
+/*
+ * Copyright (c) 2015 MediaTek Inc.
+ * Author: Bayi Cheng <bayi.cheng-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/ioport.h>
+#include <linux/math64.h>
+#include <linux/module.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/partitions.h>
+#include <linux/mtd/spi-nor.h>
+
+#define MTK_NOR_CMD_REG			0x00
+#define MTK_NOR_CNT_REG			0x04
+#define MTK_NOR_RDSR_REG		0x08
+#define MTK_NOR_RDATA_REG		0x0c
+#define MTK_NOR_RADR0_REG		0x10
+#define MTK_NOR_RADR1_REG		0x14
+#define MTK_NOR_RADR2_REG		0x18
+#define MTK_NOR_WDATA_REG		0x1c
+#define MTK_NOR_PRGDATA0_REG		0x20
+#define MTK_NOR_PRGDATA1_REG		0x24
+#define MTK_NOR_PRGDATA2_REG		0x28
+#define MTK_NOR_PRGDATA3_REG		0x2c
+#define MTK_NOR_PRGDATA4_REG		0x30
+#define MTK_NOR_PRGDATA5_REG		0x34
+#define MTK_NOR_SHREG0_REG		0x38
+#define MTK_NOR_SHREG1_REG		0x3c
+#define MTK_NOR_SHREG2_REG		0x40
+#define MTK_NOR_SHREG3_REG		0x44
+#define MTK_NOR_SHREG4_REG		0x48
+#define MTK_NOR_SHREG5_REG		0x4c
+#define MTK_NOR_SHREG6_REG		0x50
+#define MTK_NOR_SHREG7_REG		0x54
+#define MTK_NOR_SHREG8_REG		0x58
+#define MTK_NOR_SHREG9_REG		0x5c
+#define MTK_NOR_CFG1_REG		0x60
+#define MTK_NOR_CFG2_REG		0x64
+#define MTK_NOR_CFG3_REG		0x68
+#define MTK_NOR_STATUS0_REG		0x70
+#define MTK_NOR_STATUS1_REG		0x74
+#define MTK_NOR_STATUS2_REG		0x78
+#define MTK_NOR_STATUS3_REG		0x7c
+#define MTK_NOR_FLHCFG_REG		0x84
+#define MTK_NOR_TIME_REG		0x94
+#define MTK_NOR_PP_DATA_REG		0x98
+#define MTK_NOR_PREBUF_STUS_REG		0x9c
+#define MTK_NOR_DELSEL0_REG		0xa0
+#define MTK_NOR_DELSEL1_REG		0xa4
+#define MTK_NOR_INTRSTUS_REG		0xa8
+#define MTK_NOR_INTREN_REG		0xac
+#define MTK_NOR_CHKSUM_CTL_REG		0xb8
+#define MTK_NOR_CHKSUM_REG		0xbc
+#define MTK_NOR_CMD2_REG		0xc0
+#define MTK_NOR_WRPROT_REG		0xc4
+#define MTK_NOR_RADR3_REG		0xc8
+#define MTK_NOR_DUAL_REG		0xcc
+#define MTK_NOR_DELSEL2_REG		0xd0
+#define MTK_NOR_DELSEL3_REG		0xd4
+#define MTK_NOR_DELSEL4_REG		0xd8
+
+/* commands for mtk nor controller */
+#define MTK_NOR_READ_CMD		0x0
+#define MTK_NOR_RDSR_CMD		0x2
+#define MTK_NOR_PRG_CMD			0x4
+#define MTK_NOR_WR_CMD			0x10
+#define MTK_NOR_PIO_WR_CMD		0x90
+#define MTK_NOR_WRSR_CMD		0x20
+#define MTK_NOR_PIO_READ_CMD		0x81
+#define MTK_NOR_WR_BUF_ENABLE		0x1
+#define MTK_NOR_WR_BUF_DISABLE		0x0
+#define MTK_NOR_ENABLE_SF_CMD		0x30
+#define MTK_NOR_DUAD_ADDR_EN		0x8
+#define MTK_NOR_QUAD_READ_EN		0x4
+#define MTK_NOR_DUAL_ADDR_EN		0x2
+#define MTK_NOR_DUAL_READ_EN		0x1
+#define MTK_NOR_DUAL_DISABLE		0x0
+#define MTK_NOR_FAST_READ		0x1
+
+#define SFLASH_WRBUF_SIZE		128
+
+/* Can shift up to 48 bits (6 bytes) of TX/RX */
+#define MTK_NOR_MAX_SHIFT		6
+/* Helpers for accessing the program data / shift data registers */
+#define MTK_NOR_PRG_REG(n)		(MTK_NOR_PRGDATA0_REG + 4 * (n))
+#define MTK_NOR_SHREG(n)		(MTK_NOR_SHREG0_REG + 4 * (n))
+
+struct mt8173_nor {
+	struct spi_nor nor;
+	struct device *dev;
+	void __iomem *base;	/* nor flash base address */
+	struct clk *spi_clk;
+	struct clk *nor_clk;
+};
+
+static void mt8173_nor_set_read_mode(struct mt8173_nor *mt8173_nor)
+{
+	struct spi_nor *nor = &mt8173_nor->nor;
+
+	writeb(nor->read_opcode, mt8173_nor->base + MTK_NOR_PRGDATA3_REG);
+
+	switch (nor->flash_read) {
+	case SPI_NOR_FAST:
+		writeb(MTK_NOR_FAST_READ, mt8173_nor->base +
+		       MTK_NOR_CFG1_REG);
+		break;
+	case SPI_NOR_DUAL:
+		writeb(MTK_NOR_DUAL_READ_EN, mt8173_nor->base +
+		       MTK_NOR_DUAL_REG);
+		break;
+	case SPI_NOR_QUAD:
+		writeb(MTK_NOR_QUAD_READ_EN, mt8173_nor->base +
+		       MTK_NOR_DUAL_REG);
+		break;
+	default:
+		writeb(MTK_NOR_DUAL_DISABLE, mt8173_nor->base +
+		       MTK_NOR_DUAL_REG);
+		break;
+	}
+}
+
+static int mt8173_nor_execute_cmd(struct mt8173_nor *mt8173_nor, u8 cmdval)
+{
+	int reg;
+	u8 val = cmdval & 0x1f;
+
+	writeb(cmdval, mt8173_nor->base + MTK_NOR_CMD_REG);
+	return readl_poll_timeout(mt8173_nor->base + MTK_NOR_CMD_REG, reg,
+				  !(reg & val), 100, 10000);
+}
+
+static int mt8173_nor_do_tx_rx(struct mt8173_nor *mt8173_nor, u8 op,
+			       u8 *tx, int txlen, u8 *rx, int rxlen)
+{
+	int len = 1 + txlen + rxlen;
+	int i, ret, idx;
+
+	if (len > MTK_NOR_MAX_SHIFT + 1)
+		return -EINVAL;
+
+	writeb(len * 8, mt8173_nor->base + MTK_NOR_CNT_REG);
+
+	/* start at PRGDATA5, go down to PRGDATA0 */
+	idx = MTK_NOR_MAX_SHIFT - 1;
+
+	/* opcode */
+	writeb(op, mt8173_nor->base + MTK_NOR_PRG_REG(idx));
+	idx--;
+
+	/* program TX data */
+	for (i = 0; i < txlen; i++, idx--)
+		writeb(tx[i], mt8173_nor->base + MTK_NOR_PRG_REG(idx));
+
+	/* clear out rest of TX registers */
+	while (idx >= 0) {
+		writeb(0, mt8173_nor->base + MTK_NOR_PRG_REG(idx));
+		idx--;
+	}
+
+	ret = mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_PRG_CMD);
+	if (ret)
+		return ret;
+
+	/* restart at first RX byte */
+	idx = rxlen - 1;
+
+	/* read out RX data */
+	for (i = 0; i < rxlen; i++, idx--)
+		rx[i] = readb(mt8173_nor->base + MTK_NOR_SHREG(idx));
+
+	return 0;
+}
+
+/* Do a WRSR (Write Status Register) command */
+static int mt8173_nor_wr_sr(struct mt8173_nor *mt8173_nor, u8 sr)
+{
+	writeb(sr, mt8173_nor->base + MTK_NOR_PRGDATA5_REG);
+	writeb(8, mt8173_nor->base + MTK_NOR_CNT_REG);
+	return mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_WRSR_CMD);
+}
+
+static int mt8173_nor_write_buffer_enable(struct mt8173_nor *mt8173_nor)
+{
+	u8 reg;
+
+	/* the bit0 of MTK_NOR_CFG2_REG is pre-fetch buffer
+	 * 0: pre-fetch buffer use for read
+	 * 1: pre-fetch buffer use for page program
+	 */
+	writel(MTK_NOR_WR_BUF_ENABLE, mt8173_nor->base + MTK_NOR_CFG2_REG);
+	return readb_poll_timeout(mt8173_nor->base + MTK_NOR_CFG2_REG, reg,
+				  0x01 == (reg & 0x01), 100, 10000);
+}
+
+static int mt8173_nor_write_buffer_disable(struct mt8173_nor *mt8173_nor)
+{
+	u8 reg;
+
+	writel(MTK_NOR_WR_BUF_DISABLE, mt8173_nor->base + MTK_NOR_CFG2_REG);
+	return readb_poll_timeout(mt8173_nor->base + MTK_NOR_CFG2_REG, reg,
+				  MTK_NOR_WR_BUF_DISABLE == (reg & 0x1), 100,
+				  10000);
+}
+
+static void mt8173_nor_set_addr(struct mt8173_nor *mt8173_nor, u32 addr)
+{
+	int i;
+
+	for (i = 0; i < 3; i++) {
+		writeb(addr & 0xff, mt8173_nor->base + MTK_NOR_RADR0_REG + i * 4);
+		addr >>= 8;
+	}
+	/* Last register is non-contiguous */
+	writeb(addr & 0xff, mt8173_nor->base + MTK_NOR_RADR3_REG);
+}
+
+static int mt8173_nor_read(struct spi_nor *nor, loff_t from, size_t length,
+			   size_t *retlen, u_char *buffer)
+{
+	int i, ret;
+	int addr = (int)from;
+	u8 *buf = (u8 *)buffer;
+	struct mt8173_nor *mt8173_nor = nor->priv;
+
+	/* set mode for fast read mode ,dual mode or quad mode */
+	mt8173_nor_set_read_mode(mt8173_nor);
+	mt8173_nor_set_addr(mt8173_nor, addr);
+
+	for (i = 0; i < length; i++, (*retlen)++) {
+		ret = mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_PIO_READ_CMD);
+		if (ret < 0)
+			return ret;
+		buf[i] = readb(mt8173_nor->base + MTK_NOR_RDATA_REG);
+	}
+	return 0;
+}
+
+static int mt8173_nor_write_single_byte(struct mt8173_nor *mt8173_nor,
+					int addr, int length, u8 *data)
+{
+	int i, ret;
+
+	mt8173_nor_set_addr(mt8173_nor, addr);
+
+	for (i = 0; i < length; i++) {
+		ret = mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_PIO_WR_CMD);
+		if (ret < 0)
+			return ret;
+		writeb(*data++, mt8173_nor->base + MTK_NOR_WDATA_REG);
+	}
+	return 0;
+}
+
+static int mt8173_nor_write_buffer(struct mt8173_nor *mt8173_nor, int addr,
+				   const u8 *buf)
+{
+	int i, bufidx, data;
+
+	mt8173_nor_set_addr(mt8173_nor, addr);
+
+	bufidx = 0;
+	for (i = 0; i < SFLASH_WRBUF_SIZE; i += 4) {
+		data = buf[bufidx + 3]<<24 | buf[bufidx + 2]<<16 |
+		       buf[bufidx + 1]<<8 | buf[bufidx];
+		bufidx += 4;
+		writel(data, mt8173_nor->base + MTK_NOR_PP_DATA_REG);
+	}
+	return mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_WR_CMD);
+}
+
+static void mt8173_nor_write(struct spi_nor *nor, loff_t to, size_t len,
+			     size_t *retlen, const u_char *buf)
+{
+	int ret;
+	struct mt8173_nor *mt8173_nor = nor->priv;
+
+	ret = mt8173_nor_write_buffer_enable(mt8173_nor);
+	if (ret < 0)
+		dev_warn(mt8173_nor->dev, "write buffer enable failed!\n");
+
+	while (len >= SFLASH_WRBUF_SIZE) {
+		ret = mt8173_nor_write_buffer(mt8173_nor, to, buf);
+		if (ret < 0)
+			dev_err(mt8173_nor->dev, "write buffer failed!\n");
+		len -= SFLASH_WRBUF_SIZE;
+		to += SFLASH_WRBUF_SIZE;
+		buf += SFLASH_WRBUF_SIZE;
+		(*retlen) += SFLASH_WRBUF_SIZE;
+	}
+	ret = mt8173_nor_write_buffer_disable(mt8173_nor);
+	if (ret < 0)
+		dev_warn(mt8173_nor->dev, "write buffer disable failed!\n");
+
+	if (len) {
+		ret = mt8173_nor_write_single_byte(mt8173_nor, to, (int)len,
+						   (u8 *)buf);
+		if (ret < 0)
+			dev_err(mt8173_nor->dev, "write single byte failed!\n");
+		(*retlen) += len;
+	}
+}
+
+static int mt8173_nor_read_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len)
+{
+	int ret;
+	struct mt8173_nor *mt8173_nor = nor->priv;
+
+	switch (opcode) {
+	case SPINOR_OP_RDSR:
+		ret = mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_RDSR_CMD);
+		if (ret < 0)
+			return ret;
+		if (len == 1)
+			*buf = readb(mt8173_nor->base + MTK_NOR_RDSR_REG);
+		else
+			dev_err(mt8173_nor->dev, "len should be 1 for read status!\n");
+		break;
+	default:
+		ret = mt8173_nor_do_tx_rx(mt8173_nor, opcode, NULL, 0, buf, len);
+		break;
+	}
+	return ret;
+}
+
+static int mt8173_nor_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf,
+				int len)
+{
+	int ret;
+	struct mt8173_nor *mt8173_nor = nor->priv;
+
+	switch (opcode) {
+	case SPINOR_OP_WRSR:
+		/* We only handle 1 byte */
+		ret = mt8173_nor_wr_sr(mt8173_nor, *buf);
+		break;
+	default:
+		ret = mt8173_nor_do_tx_rx(mt8173_nor, opcode, buf, len, NULL, 0);
+		if (ret)
+			dev_warn(mt8173_nor->dev, "write reg failure!\n");
+		break;
+	}
+	return ret;
+}
+
+static int __init mtk_nor_init(struct mt8173_nor *mt8173_nor,
+			       struct device_node *flash_node)
+{
+	struct mtd_part_parser_data ppdata = {
+		.of_node = flash_node,
+	};
+	int ret;
+	struct spi_nor *nor;
+	/* initialize controller to accept commands */
+	writel(MTK_NOR_ENABLE_SF_CMD, mt8173_nor->base + MTK_NOR_WRPROT_REG);
+
+	nor = &mt8173_nor->nor;
+	nor->dev = mt8173_nor->dev;
+	nor->priv = mt8173_nor;
+	nor->flash_node = flash_node;
+
+	/* fill the hooks to spi nor */
+	nor->read = mt8173_nor_read;
+	nor->read_reg = mt8173_nor_read_reg;
+	nor->write = mt8173_nor_write;
+	nor->write_reg = mt8173_nor_write_reg;
+	nor->mtd.owner = THIS_MODULE;
+	nor->mtd.name = "mtk_nor";
+	/* initialized with NULL */
+	ret = spi_nor_scan(nor, NULL, SPI_NOR_DUAL);
+	if (ret)
+		return ret;
+
+	return mtd_device_parse_register(&nor->mtd, NULL, &ppdata, NULL, 0);
+}
+
+static int mtk_nor_drv_probe(struct platform_device *pdev)
+{
+	struct device_node *flash_np;
+	struct resource *res;
+	int ret;
+	struct mt8173_nor *mt8173_nor;
+
+	if (!pdev->dev.of_node) {
+		dev_err(&pdev->dev, "No DT found\n");
+		return -EINVAL;
+	}
+
+	mt8173_nor = devm_kzalloc(&pdev->dev, sizeof(*mt8173_nor), GFP_KERNEL);
+	if (!mt8173_nor)
+		return -ENOMEM;
+	platform_set_drvdata(pdev, mt8173_nor);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	mt8173_nor->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(mt8173_nor->base))
+		return PTR_ERR(mt8173_nor->base);
+
+	mt8173_nor->spi_clk = devm_clk_get(&pdev->dev, "spi");
+	if (IS_ERR(mt8173_nor->spi_clk)) {
+		ret = PTR_ERR(mt8173_nor->spi_clk);
+		goto nor_free;
+	}
+
+	mt8173_nor->nor_clk = devm_clk_get(&pdev->dev, "sf");
+	if (IS_ERR(mt8173_nor->nor_clk)) {
+		ret = PTR_ERR(mt8173_nor->nor_clk);
+		goto nor_free;
+	}
+
+	mt8173_nor->dev = &pdev->dev;
+	clk_prepare_enable(mt8173_nor->spi_clk);
+	clk_prepare_enable(mt8173_nor->nor_clk);
+
+	/* only support one attached flash */
+	flash_np = of_get_next_available_child(pdev->dev.of_node, NULL);
+	if (!flash_np) {
+		dev_err(&pdev->dev, "no SPI flash device to configure\n");
+		ret = -ENODEV;
+		goto nor_free;
+	}
+	ret = mtk_nor_init(mt8173_nor, flash_np);
+
+nor_free:
+	return ret;
+}
+
+static int mtk_nor_drv_remove(struct platform_device *pdev)
+{
+	struct mt8173_nor *mt8173_nor = platform_get_drvdata(pdev);
+
+	clk_disable_unprepare(mt8173_nor->spi_clk);
+	clk_disable_unprepare(mt8173_nor->nor_clk);
+	return 0;
+}
+
+static const struct of_device_id mtk_nor_of_ids[] = {
+	{ .compatible = "mediatek,mt8173-nor"},
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mtk_nor_of_ids);
+
+static struct platform_driver mtk_nor_driver = {
+	.probe = mtk_nor_drv_probe,
+	.remove = mtk_nor_drv_remove,
+	.driver = {
+		.name = "mtk-nor",
+		.of_match_table = mtk_nor_of_ids,
+	},
+};
+
+module_platform_driver(mtk_nor_driver);
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("MediaTek SPI NOR Flash Driver");
-- 
1.8.1.1.dirty

--
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

[PATCH] mtd: mtk-nor: fix compare_const_fl.cocci warnings

[kbuild test robot]

drivers/mtd/spi-nor/mtk-quadspi.c:223:6-28: Move constant to right.
drivers/mtd/spi-nor/mtk-quadspi.c:214:6-10: Move constant to right.

 Move constants to the right of binary operators.

Semantic patch information:
 Depends on personal taste in some cases.

Generated by: scripts/coccinelle/misc/compare_const_fl.cocci

CC: Bayi Cheng <bayi.cheng@mediatek.com>
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---

Please take the patch only if it's a positive warning. Thanks!

 mtk-quadspi.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- a/drivers/mtd/spi-nor/mtk-quadspi.c
+++ b/drivers/mtd/spi-nor/mtk-quadspi.c
@@ -211,7 +211,7 @@ static int mt8173_nor_write_buffer_enabl
 	 */
 	writel(MTK_NOR_WR_BUF_ENABLE, mt8173_nor->base + MTK_NOR_CFG2_REG);
 	return readb_poll_timeout(mt8173_nor->base + MTK_NOR_CFG2_REG, reg,
-				  0x01 == (reg & 0x01), 100, 10000);
+				  (reg & 0x01) == 0x01, 100, 10000);
 }
 
 static int mt8173_nor_write_buffer_disable(struct mt8173_nor *mt8173_nor)
@@ -220,7 +220,7 @@ static int mt8173_nor_write_buffer_disab
 
 	writel(MTK_NOR_WR_BUF_DISABLE, mt8173_nor->base + MTK_NOR_CFG2_REG);
 	return readb_poll_timeout(mt8173_nor->base + MTK_NOR_CFG2_REG, reg,
-				  MTK_NOR_WR_BUF_DISABLE == (reg & 0x1), 100,
+				  (reg & 0x1) == MTK_NOR_WR_BUF_DISABLE, 100,
 				  10000);
 }
 

Re: [PATCH v6 2/3] mtd: mtk-nor: mtk serial flash controller driver

[kbuild test robot]

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

Hi Bayi,

[auto build test ERROR on mtd/master]
[also build test ERROR on v4.3 next-20151106]

url:    https://github.com/0day-ci/linux/commits/Bayi-Cheng/Mediatek-SPI-NOR-flash-driver/20151106-235157
base:   git://git.infradead.org/linux-mtd.git master
config: i386-allmodconfig (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/mtd/spi-nor/mtk-quadspi.c: In function 'mtk_nor_init':
>> drivers/mtd/spi-nor/mtk-quadspi.c:381:5: error: 'struct spi_nor' has no member named 'flash_node'
     nor->flash_node = flash_node;
        ^
   drivers/mtd/spi-nor/mtk-quadspi.c:387:17: warning: assignment from incompatible pointer type [-Wincompatible-pointer-types]
     nor->write_reg = mt8173_nor_write_reg;
                    ^
>> drivers/mtd/spi-nor/mtk-quadspi.c:388:10: error: request for member 'owner' in something not a structure or union
     nor->mtd.owner = THIS_MODULE;
             ^
>> drivers/mtd/spi-nor/mtk-quadspi.c:389:10: error: request for member 'name' in something not a structure or union
     nor->mtd.name = "mtk_nor";
             ^
   drivers/mtd/spi-nor/mtk-quadspi.c:395:35: warning: passing argument 1 of 'mtd_device_parse_register' from incompatible pointer type [-Wincompatible-pointer-types]
     return mtd_device_parse_register(&nor->mtd, NULL, &ppdata, NULL, 0);
                                      ^
   In file included from drivers/mtd/spi-nor/mtk-quadspi.c:24:0:
   include/linux/mtd/mtd.h:372:12: note: expected 'struct mtd_info *' but argument is of type 'struct mtd_info **'
    extern int mtd_device_parse_register(struct mtd_info *mtd,
               ^

coccinelle warnings: (new ones prefixed by >>)

>> drivers/mtd/spi-nor/mtk-quadspi.c:223:6-28: Move constant to right.
   drivers/mtd/spi-nor/mtk-quadspi.c:214:6-10: Move constant to right.

Please review and possibly fold the followup patch.

vim +381 drivers/mtd/spi-nor/mtk-quadspi.c

   217	static int mt8173_nor_write_buffer_disable(struct mt8173_nor *mt8173_nor)
   218	{
   219		u8 reg;
   220	
   221		writel(MTK_NOR_WR_BUF_DISABLE, mt8173_nor->base + MTK_NOR_CFG2_REG);
   222		return readb_poll_timeout(mt8173_nor->base + MTK_NOR_CFG2_REG, reg,
 > 223					  MTK_NOR_WR_BUF_DISABLE == (reg & 0x1), 100,
   224					  10000);
   225	}
   226	
   227	static void mt8173_nor_set_addr(struct mt8173_nor *mt8173_nor, u32 addr)
   228	{
   229		int i;
   230	
   231		for (i = 0; i < 3; i++) {
   232			writeb(addr & 0xff, mt8173_nor->base + MTK_NOR_RADR0_REG + i * 4);
   233			addr >>= 8;
   234		}
   235		/* Last register is non-contiguous */
   236		writeb(addr & 0xff, mt8173_nor->base + MTK_NOR_RADR3_REG);
   237	}
   238	
   239	static int mt8173_nor_read(struct spi_nor *nor, loff_t from, size_t length,
   240				   size_t *retlen, u_char *buffer)
   241	{
   242		int i, ret;
   243		int addr = (int)from;
   244		u8 *buf = (u8 *)buffer;
   245		struct mt8173_nor *mt8173_nor = nor->priv;
   246	
   247		/* set mode for fast read mode ,dual mode or quad mode */
   248		mt8173_nor_set_read_mode(mt8173_nor);
   249		mt8173_nor_set_addr(mt8173_nor, addr);
   250	
   251		for (i = 0; i < length; i++, (*retlen)++) {
   252			ret = mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_PIO_READ_CMD);
   253			if (ret < 0)
   254				return ret;
   255			buf[i] = readb(mt8173_nor->base + MTK_NOR_RDATA_REG);
   256		}
   257		return 0;
   258	}
   259	
   260	static int mt8173_nor_write_single_byte(struct mt8173_nor *mt8173_nor,
   261						int addr, int length, u8 *data)
   262	{
   263		int i, ret;
   264	
   265		mt8173_nor_set_addr(mt8173_nor, addr);
   266	
   267		for (i = 0; i < length; i++) {
   268			ret = mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_PIO_WR_CMD);
   269			if (ret < 0)
   270				return ret;
   271			writeb(*data++, mt8173_nor->base + MTK_NOR_WDATA_REG);
   272		}
   273		return 0;
   274	}
   275	
   276	static int mt8173_nor_write_buffer(struct mt8173_nor *mt8173_nor, int addr,
   277					   const u8 *buf)
   278	{
   279		int i, bufidx, data;
   280	
   281		mt8173_nor_set_addr(mt8173_nor, addr);
   282	
   283		bufidx = 0;
   284		for (i = 0; i < SFLASH_WRBUF_SIZE; i += 4) {
   285			data = buf[bufidx + 3]<<24 | buf[bufidx + 2]<<16 |
   286			       buf[bufidx + 1]<<8 | buf[bufidx];
   287			bufidx += 4;
   288			writel(data, mt8173_nor->base + MTK_NOR_PP_DATA_REG);
   289		}
   290		return mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_WR_CMD);
   291	}
   292	
   293	static void mt8173_nor_write(struct spi_nor *nor, loff_t to, size_t len,
   294				     size_t *retlen, const u_char *buf)
   295	{
   296		int ret;
   297		struct mt8173_nor *mt8173_nor = nor->priv;
   298	
   299		ret = mt8173_nor_write_buffer_enable(mt8173_nor);
   300		if (ret < 0)
   301			dev_warn(mt8173_nor->dev, "write buffer enable failed!\n");
   302	
   303		while (len >= SFLASH_WRBUF_SIZE) {
   304			ret = mt8173_nor_write_buffer(mt8173_nor, to, buf);
   305			if (ret < 0)
   306				dev_err(mt8173_nor->dev, "write buffer failed!\n");
   307			len -= SFLASH_WRBUF_SIZE;
   308			to += SFLASH_WRBUF_SIZE;
   309			buf += SFLASH_WRBUF_SIZE;
   310			(*retlen) += SFLASH_WRBUF_SIZE;
   311		}
   312		ret = mt8173_nor_write_buffer_disable(mt8173_nor);
   313		if (ret < 0)
   314			dev_warn(mt8173_nor->dev, "write buffer disable failed!\n");
   315	
   316		if (len) {
   317			ret = mt8173_nor_write_single_byte(mt8173_nor, to, (int)len,
   318							   (u8 *)buf);
   319			if (ret < 0)
   320				dev_err(mt8173_nor->dev, "write single byte failed!\n");
   321			(*retlen) += len;
   322		}
   323	}
   324	
   325	static int mt8173_nor_read_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len)
   326	{
   327		int ret;
   328		struct mt8173_nor *mt8173_nor = nor->priv;
   329	
   330		switch (opcode) {
   331		case SPINOR_OP_RDSR:
   332			ret = mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_RDSR_CMD);
   333			if (ret < 0)
   334				return ret;
   335			if (len == 1)
   336				*buf = readb(mt8173_nor->base + MTK_NOR_RDSR_REG);
   337			else
   338				dev_err(mt8173_nor->dev, "len should be 1 for read status!\n");
   339			break;
   340		default:
   341			ret = mt8173_nor_do_tx_rx(mt8173_nor, opcode, NULL, 0, buf, len);
   342			break;
   343		}
   344		return ret;
   345	}
   346	
   347	static int mt8173_nor_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf,
   348					int len)
   349	{
   350		int ret;
   351		struct mt8173_nor *mt8173_nor = nor->priv;
   352	
   353		switch (opcode) {
   354		case SPINOR_OP_WRSR:
   355			/* We only handle 1 byte */
   356			ret = mt8173_nor_wr_sr(mt8173_nor, *buf);
   357			break;
   358		default:
   359			ret = mt8173_nor_do_tx_rx(mt8173_nor, opcode, buf, len, NULL, 0);
   360			if (ret)
   361				dev_warn(mt8173_nor->dev, "write reg failure!\n");
   362			break;
   363		}
   364		return ret;
   365	}
   366	
   367	static int __init mtk_nor_init(struct mt8173_nor *mt8173_nor,
   368				       struct device_node *flash_node)
   369	{
   370		struct mtd_part_parser_data ppdata = {
   371			.of_node = flash_node,
   372		};
   373		int ret;
   374		struct spi_nor *nor;
   375		/* initialize controller to accept commands */
   376		writel(MTK_NOR_ENABLE_SF_CMD, mt8173_nor->base + MTK_NOR_WRPROT_REG);
   377	
   378		nor = &mt8173_nor->nor;
   379		nor->dev = mt8173_nor->dev;
   380		nor->priv = mt8173_nor;
 > 381		nor->flash_node = flash_node;
   382	
   383		/* fill the hooks to spi nor */
   384		nor->read = mt8173_nor_read;
   385		nor->read_reg = mt8173_nor_read_reg;
   386		nor->write = mt8173_nor_write;
   387		nor->write_reg = mt8173_nor_write_reg;
 > 388		nor->mtd.owner = THIS_MODULE;
 > 389		nor->mtd.name = "mtk_nor";
   390		/* initialized with NULL */
   391		ret = spi_nor_scan(nor, NULL, SPI_NOR_DUAL);
   392		if (ret)

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 51507 bytes --]

Re: [PATCH v6 2/3] mtd: mtk-nor: mtk serial flash controller driver

[Brian Norris]

Hi Bayi,

A few small comments.

On Fri, Nov 06, 2015 at 11:48:08PM +0800, Bayi Cheng wrote:
> add spi nor flash driver for mediatek controller
> 
> Signed-off-by: Bayi Cheng <bayi.cheng@mediatek.com>
> ---
>  drivers/mtd/spi-nor/Kconfig       |   7 +
>  drivers/mtd/spi-nor/Makefile      |   1 +
>  drivers/mtd/spi-nor/mtk-quadspi.c | 475 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 483 insertions(+)
>  create mode 100644 drivers/mtd/spi-nor/mtk-quadspi.c


> diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
> index e53333e..0bf3a7f8 100644
> --- a/drivers/mtd/spi-nor/Makefile
> +++ b/drivers/mtd/spi-nor/Makefile
> @@ -1,3 +1,4 @@
>  obj-$(CONFIG_MTD_SPI_NOR)	+= spi-nor.o
>  obj-$(CONFIG_SPI_FSL_QUADSPI)	+= fsl-quadspi.o
> +obj-$(CONFIG_MTD_MT81xx_NOR)    += mtk-quadspi.o
>  obj-$(CONFIG_SPI_NXP_SPIFI)	+= nxp-spifi.o
> diff --git a/drivers/mtd/spi-nor/mtk-quadspi.c b/drivers/mtd/spi-nor/mtk-quadspi.c
> new file mode 100644
> index 0000000..6582811
> --- /dev/null
> +++ b/drivers/mtd/spi-nor/mtk-quadspi.c
> @@ -0,0 +1,475 @@

...

> +/* Can shift up to 48 bits (6 bytes) of TX/RX */
> +#define MTK_NOR_MAX_SHIFT		6

...

> +static int mt8173_nor_do_tx_rx(struct mt8173_nor *mt8173_nor, u8 op,
> +			       u8 *tx, int txlen, u8 *rx, int rxlen)
> +{
> +	int len = 1 + txlen + rxlen;
> +	int i, ret, idx;
> +
> +	if (len > MTK_NOR_MAX_SHIFT + 1)
> +		return -EINVAL;

So I see you adjusted these bounds to add 1, which inspired one of my
questions on the cover letter.

Why do you allow len=7, which means you'd program 7*8 to the COUNT
register, when the SoC manual says it has a max of 48? Is the manual
wrong?

I notice you added the '+ 1' to your driver, so it allows:

	do_tx_rx( txlen = 0 , rxlen = 6) /* e.g., for READ ID */

but that means your driver also allows:

	do_tx_rx( txlen = 6, rxlen = 0) /* ERROR: this will allow out of
					   bounds write on PRGDATA
					   register -1 */

If I understand correctly, the following constraints are more correct:

	/* Can only transmit 1 opcode and 5 other bytes */
	if (1 + txlen > MTK_NOR_MAX_SHIFT)
		return -EINVAL;

	/* Can only receive 6 bytes after the opcode */
	if (rxlen > MTK_NOR_MAX_SHIFT)
		return -EINVAL;

	/* Can only handle XXX bytes total */
	/* How many bytes is the max for register MTK_NOR_CNT_REG ? */
	if (len > XXX)
		return -EINVAL;

> +
> +	writeb(len * 8, mt8173_nor->base + MTK_NOR_CNT_REG);
> +
> +	/* start at PRGDATA5, go down to PRGDATA0 */
> +	idx = MTK_NOR_MAX_SHIFT - 1;
> +
> +	/* opcode */
> +	writeb(op, mt8173_nor->base + MTK_NOR_PRG_REG(idx));
> +	idx--;
> +
> +	/* program TX data */
> +	for (i = 0; i < txlen; i++, idx--)
> +		writeb(tx[i], mt8173_nor->base + MTK_NOR_PRG_REG(idx));
> +
> +	/* clear out rest of TX registers */
> +	while (idx >= 0) {
> +		writeb(0, mt8173_nor->base + MTK_NOR_PRG_REG(idx));
> +		idx--;
> +	}
> +
> +	ret = mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_PRG_CMD);
> +	if (ret)
> +		return ret;
> +
> +	/* restart at first RX byte */
> +	idx = rxlen - 1;
> +
> +	/* read out RX data */
> +	for (i = 0; i < rxlen; i++, idx--)
> +		rx[i] = readb(mt8173_nor->base + MTK_NOR_SHREG(idx));
> +
> +	return 0;
> +}
> +

...

> +static int __init mtk_nor_init(struct mt8173_nor *mt8173_nor,
> +			       struct device_node *flash_node)
> +{
> +	struct mtd_part_parser_data ppdata = {
> +		.of_node = flash_node,
> +	};
> +	int ret;
> +	struct spi_nor *nor;

Normally we'd have a blank line here.

> +	/* initialize controller to accept commands */
> +	writel(MTK_NOR_ENABLE_SF_CMD, mt8173_nor->base + MTK_NOR_WRPROT_REG);
> +
> +	nor = &mt8173_nor->nor;
> +	nor->dev = mt8173_nor->dev;
> +	nor->priv = mt8173_nor;
> +	nor->flash_node = flash_node;
> +
> +	/* fill the hooks to spi nor */
> +	nor->read = mt8173_nor_read;
> +	nor->read_reg = mt8173_nor_read_reg;
> +	nor->write = mt8173_nor_write;
> +	nor->write_reg = mt8173_nor_write_reg;
> +	nor->mtd.owner = THIS_MODULE;
> +	nor->mtd.name = "mtk_nor";
> +	/* initialized with NULL */
> +	ret = spi_nor_scan(nor, NULL, SPI_NOR_DUAL);
> +	if (ret)
> +		return ret;
> +
> +	return mtd_device_parse_register(&nor->mtd, NULL, &ppdata, NULL, 0);
> +}

...

Brian

Re: [PATCH v6 2/3] mtd: mtk-nor: mtk serial flash controller driver

[bayi cheng]

On Mon, 2015-11-09 at 19:01 -0800, Brian Norris wrote:
> Hi Bayi,
> 
> A few small comments.
> 
> On Fri, Nov 06, 2015 at 11:48:08PM +0800, Bayi Cheng wrote:
> > add spi nor flash driver for mediatek controller
> > 
> > Signed-off-by: Bayi Cheng <bayi.cheng-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> > ---
> >  drivers/mtd/spi-nor/Kconfig       |   7 +
> >  drivers/mtd/spi-nor/Makefile      |   1 +
> >  drivers/mtd/spi-nor/mtk-quadspi.c | 475 ++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 483 insertions(+)
> >  create mode 100644 drivers/mtd/spi-nor/mtk-quadspi.c
> 
> 
> > diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
> > index e53333e..0bf3a7f8 100644
> > --- a/drivers/mtd/spi-nor/Makefile
> > +++ b/drivers/mtd/spi-nor/Makefile
> > @@ -1,3 +1,4 @@
> >  obj-$(CONFIG_MTD_SPI_NOR)	+= spi-nor.o
> >  obj-$(CONFIG_SPI_FSL_QUADSPI)	+= fsl-quadspi.o
> > +obj-$(CONFIG_MTD_MT81xx_NOR)    += mtk-quadspi.o
> >  obj-$(CONFIG_SPI_NXP_SPIFI)	+= nxp-spifi.o
> > diff --git a/drivers/mtd/spi-nor/mtk-quadspi.c b/drivers/mtd/spi-nor/mtk-quadspi.c
> > new file mode 100644
> > index 0000000..6582811
> > --- /dev/null
> > +++ b/drivers/mtd/spi-nor/mtk-quadspi.c
> > @@ -0,0 +1,475 @@
> 
> ...
> 
> > +/* Can shift up to 48 bits (6 bytes) of TX/RX */
> > +#define MTK_NOR_MAX_SHIFT		6
> 
> ...
> 
> > +static int mt8173_nor_do_tx_rx(struct mt8173_nor *mt8173_nor, u8 op,
> > +			       u8 *tx, int txlen, u8 *rx, int rxlen)
> > +{
> > +	int len = 1 + txlen + rxlen;
> > +	int i, ret, idx;
> > +
> > +	if (len > MTK_NOR_MAX_SHIFT + 1)
> > +		return -EINVAL;
> 
> So I see you adjusted these bounds to add 1, which inspired one of my
> questions on the cover letter.
> 
> Why do you allow len=7, which means you'd program 7*8 to the COUNT
> register, when the SoC manual says it has a max of 48? Is the manual
> wrong?
> 
Hi Brian, you are right, the manual is wrong here, Actually, it has a
max of 56. when we want to read 6 IDs, we need transfer 1 byte command
and 6 bytes null address to nor flash, then we can read the six IDs from
SHREGx register.

> I notice you added the '+ 1' to your driver, so it allows:
> 
> 	do_tx_rx( txlen = 0 , rxlen = 6) /* e.g., for READ ID */
> 
> but that means your driver also allows:
> 
> 	do_tx_rx( txlen = 6, rxlen = 0) /* ERROR: this will allow out of
> 					   bounds write on PRGDATA
> 					   register -1 */
> 
> If I understand correctly, the following constraints are more correct:
> 
> 	/* Can only transmit 1 opcode and 5 other bytes */
> 	if (1 + txlen > MTK_NOR_MAX_SHIFT)
> 		return -EINVAL;
> 
> 	/* Can only receive 6 bytes after the opcode */
> 	if (rxlen > MTK_NOR_MAX_SHIFT)
> 		return -EINVAL;
> 
> 	/* Can only handle XXX bytes total */
> 	/* How many bytes is the max for register MTK_NOR_CNT_REG ? */
> 	if (len > XXX)
> 		return -EINVAL;
> 
yes, your constraints seems more correct, and I will adapt these lines
to next patch.
> > +
> > +	writeb(len * 8, mt8173_nor->base + MTK_NOR_CNT_REG);
> > +
> > +	/* start at PRGDATA5, go down to PRGDATA0 */
> > +	idx = MTK_NOR_MAX_SHIFT - 1;
> > +
> > +	/* opcode */
> > +	writeb(op, mt8173_nor->base + MTK_NOR_PRG_REG(idx));
> > +	idx--;
> > +
> > +	/* program TX data */
> > +	for (i = 0; i < txlen; i++, idx--)
> > +		writeb(tx[i], mt8173_nor->base + MTK_NOR_PRG_REG(idx));
> > +
> > +	/* clear out rest of TX registers */
> > +	while (idx >= 0) {
> > +		writeb(0, mt8173_nor->base + MTK_NOR_PRG_REG(idx));
> > +		idx--;
> > +	}
> > +
> > +	ret = mt8173_nor_execute_cmd(mt8173_nor, MTK_NOR_PRG_CMD);
> > +	if (ret)
> > +		return ret;
> > +
> > +	/* restart at first RX byte */
> > +	idx = rxlen - 1;
> > +
> > +	/* read out RX data */
> > +	for (i = 0; i < rxlen; i++, idx--)
> > +		rx[i] = readb(mt8173_nor->base + MTK_NOR_SHREG(idx));
> > +
> > +	return 0;
> > +}
> > +
> 
> ...
> 
> > +static int __init mtk_nor_init(struct mt8173_nor *mt8173_nor,
> > +			       struct device_node *flash_node)
> > +{
> > +	struct mtd_part_parser_data ppdata = {
> > +		.of_node = flash_node,
> > +	};
> > +	int ret;
> > +	struct spi_nor *nor;
> 
> Normally we'd have a blank line here.
> 
Ok, I will fix it, and thanks for your advice.

> > +	/* initialize controller to accept commands */
> > +	writel(MTK_NOR_ENABLE_SF_CMD, mt8173_nor->base + MTK_NOR_WRPROT_REG);
> > +
> > +	nor = &mt8173_nor->nor;
> > +	nor->dev = mt8173_nor->dev;
> > +	nor->priv = mt8173_nor;
> > +	nor->flash_node = flash_node;
> > +
> > +	/* fill the hooks to spi nor */
> > +	nor->read = mt8173_nor_read;
> > +	nor->read_reg = mt8173_nor_read_reg;
> > +	nor->write = mt8173_nor_write;
> > +	nor->write_reg = mt8173_nor_write_reg;
> > +	nor->mtd.owner = THIS_MODULE;
> > +	nor->mtd.name = "mtk_nor";
> > +	/* initialized with NULL */
> > +	ret = spi_nor_scan(nor, NULL, SPI_NOR_DUAL);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return mtd_device_parse_register(&nor->mtd, NULL, &ppdata, NULL, 0);
> > +}
> 
> ...
> 
> Brian
> 
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek


--
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 v6 2/3] mtd: mtk-nor: mtk serial flash controller driver

[Brian Norris]

One more small comment, since you're respinning this:

On Fri, Nov 06, 2015 at 11:48:08PM +0800, Bayi Cheng wrote:
> diff --git a/drivers/mtd/spi-nor/mtk-quadspi.c b/drivers/mtd/spi-nor/mtk-quadspi.c
> new file mode 100644
> index 0000000..6582811
> --- /dev/null
> +++ b/drivers/mtd/spi-nor/mtk-quadspi.c
> @@ -0,0 +1,475 @@

...

> +static int mtk_nor_drv_probe(struct platform_device *pdev)
> +{
> +	struct device_node *flash_np;
> +	struct resource *res;
> +	int ret;
> +	struct mt8173_nor *mt8173_nor;
> +
> +	if (!pdev->dev.of_node) {
> +		dev_err(&pdev->dev, "No DT found\n");
> +		return -EINVAL;
> +	}
> +
> +	mt8173_nor = devm_kzalloc(&pdev->dev, sizeof(*mt8173_nor), GFP_KERNEL);
> +	if (!mt8173_nor)
> +		return -ENOMEM;
> +	platform_set_drvdata(pdev, mt8173_nor);
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	mt8173_nor->base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(mt8173_nor->base))
> +		return PTR_ERR(mt8173_nor->base);
> +
> +	mt8173_nor->spi_clk = devm_clk_get(&pdev->dev, "spi");
> +	if (IS_ERR(mt8173_nor->spi_clk)) {
> +		ret = PTR_ERR(mt8173_nor->spi_clk);
> +		goto nor_free;
> +	}
> +
> +	mt8173_nor->nor_clk = devm_clk_get(&pdev->dev, "sf");
> +	if (IS_ERR(mt8173_nor->nor_clk)) {
> +		ret = PTR_ERR(mt8173_nor->nor_clk);
> +		goto nor_free;
> +	}
> +
> +	mt8173_nor->dev = &pdev->dev;
> +	clk_prepare_enable(mt8173_nor->spi_clk);
> +	clk_prepare_enable(mt8173_nor->nor_clk);

You enable the clocks here, but...

> +
> +	/* only support one attached flash */
> +	flash_np = of_get_next_available_child(pdev->dev.of_node, NULL);
> +	if (!flash_np) {

... you might bail out here if there is no available flash node, and you
never disable the clocks. (Same is true if we fail to detect the flash;
you leave the no-longer-needed clocks enabled.) Seems like maybe you
should disable clocks on failure.

> +		dev_err(&pdev->dev, "no SPI flash device to configure\n");
> +		ret = -ENODEV;
> +		goto nor_free;
> +	}
> +	ret = mtk_nor_init(mt8173_nor, flash_np);
> +
> +nor_free:
> +	return ret;
> +}

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 v6 2/3] mtd: mtk-nor: mtk serial flash controller driver

[bayi cheng]

On Wed, 2015-11-11 at 13:41 -0800, Brian Norris wrote:
> One more small comment, since you're respinning this:
> 
> On Fri, Nov 06, 2015 at 11:48:08PM +0800, Bayi Cheng wrote:
> > diff --git a/drivers/mtd/spi-nor/mtk-quadspi.c b/drivers/mtd/spi-nor/mtk-quadspi.c
> > new file mode 100644
> > index 0000000..6582811
> > --- /dev/null
> > +++ b/drivers/mtd/spi-nor/mtk-quadspi.c
> > @@ -0,0 +1,475 @@
> 
> ...
> 
> > +static int mtk_nor_drv_probe(struct platform_device *pdev)
> > +{
> > +	struct device_node *flash_np;
> > +	struct resource *res;
> > +	int ret;
> > +	struct mt8173_nor *mt8173_nor;
> > +
> > +	if (!pdev->dev.of_node) {
> > +		dev_err(&pdev->dev, "No DT found\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	mt8173_nor = devm_kzalloc(&pdev->dev, sizeof(*mt8173_nor), GFP_KERNEL);
> > +	if (!mt8173_nor)
> > +		return -ENOMEM;
> > +	platform_set_drvdata(pdev, mt8173_nor);
> > +
> > +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +	mt8173_nor->base = devm_ioremap_resource(&pdev->dev, res);
> > +	if (IS_ERR(mt8173_nor->base))
> > +		return PTR_ERR(mt8173_nor->base);
> > +
> > +	mt8173_nor->spi_clk = devm_clk_get(&pdev->dev, "spi");
> > +	if (IS_ERR(mt8173_nor->spi_clk)) {
> > +		ret = PTR_ERR(mt8173_nor->spi_clk);
> > +		goto nor_free;
> > +	}
> > +
> > +	mt8173_nor->nor_clk = devm_clk_get(&pdev->dev, "sf");
> > +	if (IS_ERR(mt8173_nor->nor_clk)) {
> > +		ret = PTR_ERR(mt8173_nor->nor_clk);
> > +		goto nor_free;
> > +	}
> > +
> > +	mt8173_nor->dev = &pdev->dev;
> > +	clk_prepare_enable(mt8173_nor->spi_clk);
> > +	clk_prepare_enable(mt8173_nor->nor_clk);
> 
> You enable the clocks here, but...
> 
> > +
> > +	/* only support one attached flash */
> > +	flash_np = of_get_next_available_child(pdev->dev.of_node, NULL);
> > +	if (!flash_np) {
> 
> ... you might bail out here if there is no available flash node, and you
> never disable the clocks. (Same is true if we fail to detect the flash;
> you leave the no-longer-needed clocks enabled.) Seems like maybe you
> should disable clocks on failure.

Yes, I have forgot to disable these clocks on failure. and I will fix it
in the next patch! Thanks!
> 
> > +		dev_err(&pdev->dev, "no SPI flash device to configure\n");
> > +		ret = -ENODEV;
> > +		goto nor_free;
> > +	}
> > +	ret = mtk_nor_init(mt8173_nor, flash_np);
> > +
> > +nor_free:
> > +	return ret;
> > +}
> 
> 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

[PATCH v6 3/3] arm64: dts: mt8173: Add nor flash node

[Bayi Cheng]

Add Mediatek nor flash node

Signed-off-by: Bayi Cheng <bayi.cheng@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8173.dtsi | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
index d18ee42..f5f08eb 100644
--- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
@@ -365,6 +365,22 @@
 			status = "disabled";
 		};
 
+		nor_flash: spi@1100d000 {
+			compatible = "mediatek,mt8173-nor";
+			reg = <0 0x1100d000 0 0xe0>;
+			clocks = <&pericfg CLK_PERI_SPI>,
+				 <&topckgen CLK_TOP_SPINFI_IFR_SEL>;
+			clock-names = "spi", "sf";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+
+			flash@0 {
+				compatible = "jedec,spi-nor";
+				reg = <0>;
+			};
+		};
+
 		i2c3: i2c3@11010000 {
 			compatible = "mediatek,mt8173-i2c";
 			reg = <0 0x11010000 0 0x70>,
-- 
1.8.1.1.dirty

Re: [PATCH v6 3/3] arm64: dts: mt8173: Add nor flash node

[Brian Norris]

On Fri, Nov 06, 2015 at 11:48:09PM +0800, Bayi Cheng wrote:
> Add Mediatek nor flash node
> 
> Signed-off-by: Bayi Cheng <bayi.cheng-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>

Acked-by: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

> ---
>  arch/arm64/boot/dts/mediatek/mt8173.dtsi | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> index d18ee42..f5f08eb 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> @@ -365,6 +365,22 @@
>  			status = "disabled";
>  		};
>  
> +		nor_flash: spi@1100d000 {
> +			compatible = "mediatek,mt8173-nor";
> +			reg = <0 0x1100d000 0 0xe0>;
> +			clocks = <&pericfg CLK_PERI_SPI>,
> +				 <&topckgen CLK_TOP_SPINFI_IFR_SEL>;
> +			clock-names = "spi", "sf";
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			status = "disabled";
> +
> +			flash@0 {
> +				compatible = "jedec,spi-nor";
> +				reg = <0>;
> +			};
> +		};
> +
>  		i2c3: i2c3@11010000 {
>  			compatible = "mediatek,mt8173-i2c";
>  			reg = <0 0x11010000 0 0x70>,
> -- 
> 1.8.1.1.dirty
> 
--
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 v6 0/3] Mediatek SPI-NOR flash driver

[Brian Norris]

Hi Bayi,

On Fri, Nov 06, 2015 at 11:48:06PM +0800, Bayi Cheng wrote:
> This series is based on v4.3-rc1 and l2-mtd.git [0] and erase_sector
> implementation patch [1]
> 
> [0]: git://git.infradead.org/l2-mtd.git
> [1]: http://lists.infradead.org/pipermail/linux-mtd/2015-October//062959.html
> 
> Change in v6:
> 1: delete mt8173_nor_do_rx
> 2: delete mt8173_nor_do_rx
> 3: add mt8173_nor_do_tx_rx for general usage
> 4: support nor flash with 6 IDs
> 5: delete mt8173_nor_erase_sector and use "nor->erase_opcode"
> 6: add mt8173_nor_set_addr to programming the address register
> 7: initialize the ppdata in mtk_nor_init

This series is looking a lot better to me. Thanks for incorporating (and
I hope fully reviewing and testing!) my suggested changes. I have a just
a few small comments that I might post to the driver patch, and if
that's all that's outstanding, I can fix them up myself before applying.

I believe you didn't completely answer all my questions from v5 though.
I'll repeat a bit here. Particularly, refer to [1].
I'll summarize; I understand that your common transmit/receive operation
works something like this:

Quoting from [1]:
> (1) total number of bits to send/receive goes in the COUNT register (so
>     far, as many as 7*8=56?)
> (2) opcode is written to PRGDATA5
> (3) other "transmit" data (like addresses), if any, are placed on PRGDATA4..0
> (4) command is sent (execute_cmd())
> (5) data is read back in SHREG{X..0}, if needed

My questions were:

(a) Why does mt8173_nor_set_read_mode() use PRGDATA3? That's not
    mentioned in the SoC manual, and it doesn't really match any of the
    steps above. Perhaps it's just a quirk of the controller's
    programming model?

(b) How do you determine X from step (5)?

Right now, your code seems to answer that X is "rxlen - 1". Correct?

If that's correct and if I put all of my understanding together
correctly, this means that you can actually shift out (in PRGDATA) up to
6 bytes (that is, 1 opcode and 5 tx bytes) and shift in (in SHREG) up to
7 bytes, except that the first byte is received during the opcode cycle,
and so it is discarded, and we effectively receive only 6 bytes.

Is that all correct? If so, then I think you still need to adjust the
boundary conditions in your do_tx_rx() function. (I'll comment on the
driver to point out the specifics.)

Regards,
Brian

[1] http://lists.infradead.org/pipermail/linux-mtd/2015-October/062951.html

Re: [PATCH v6 0/3] Mediatek SPI-NOR flash driver

[bayi cheng]

On Mon, 2015-11-09 at 18:46 -0800, Brian Norris wrote:
> Hi Bayi,
> 
> On Fri, Nov 06, 2015 at 11:48:06PM +0800, Bayi Cheng wrote:
> > This series is based on v4.3-rc1 and l2-mtd.git [0] and erase_sector
> > implementation patch [1]
> > 
> > [0]: git://git.infradead.org/l2-mtd.git
> > [1]: http://lists.infradead.org/pipermail/linux-mtd/2015-October//062959.html
> > 
> > Change in v6:
> > 1: delete mt8173_nor_do_rx
> > 2: delete mt8173_nor_do_rx
> > 3: add mt8173_nor_do_tx_rx for general usage
> > 4: support nor flash with 6 IDs
> > 5: delete mt8173_nor_erase_sector and use "nor->erase_opcode"
> > 6: add mt8173_nor_set_addr to programming the address register
> > 7: initialize the ppdata in mtk_nor_init
> 
> This series is looking a lot better to me. Thanks for incorporating (and
> I hope fully reviewing and testing!) my suggested changes. I have a just
> a few small comments that I might post to the driver patch, and if
> that's all that's outstanding, I can fix them up myself before applying.
> 
> I believe you didn't completely answer all my questions from v5 though.
> I'll repeat a bit here. Particularly, refer to [1].
> I'll summarize; I understand that your common transmit/receive operation
> works something like this:
> 
> Quoting from [1]:
> > (1) total number of bits to send/receive goes in the COUNT register (so
> >     far, as many as 7*8=56?)
> > (2) opcode is written to PRGDATA5
> > (3) other "transmit" data (like addresses), if any, are placed on PRGDATA4..0
> > (4) command is sent (execute_cmd())
> > (5) data is read back in SHREG{X..0}, if needed
> 
> My questions were:
> 
> (a) Why does mt8173_nor_set_read_mode() use PRGDATA3? That's not
>     mentioned in the SoC manual, and it doesn't really match any of the
>     steps above. Perhaps it's just a quirk of the controller's
>     programming model?
> 
yes, for this question, I have done some testes, If I change the
PRGDATA3 to PRGDATA5 for mt8173_nor_set_read_mode() like others
functions, then the controller will be hanged, and I have asked our
designer for double confirm.

> (b) How do you determine X from step (5)?
> 
> Right now, your code seems to answer that X is "rxlen - 1". Correct?
> 
yes, I have used "rxlen -1", because the first of nor flash output is
located at SHREG[0], in the other words, the output data starts at
SHREG[0] and go up to SHREG[relen -1]

> If that's correct and if I put all of my understanding together
> correctly, this means that you can actually shift out (in PRGDATA) up to
> 6 bytes (that is, 1 opcode and 5 tx bytes) and shift in (in SHREG) up to
> 7 bytes, except that the first byte is received during the opcode cycle,
> and so it is discarded, and we effectively receive only 6 bytes.
> 
> Is that all correct? If so, then I think you still need to adjust the
> boundary conditions in your do_tx_rx() function. (I'll comment on the
> driver to point out the specifics.)

Yes, you are right! and I will adjust the boundary conditions in
do_tx_rx() function.

By the way, could you tell me whether I need to publish a new patch? or
you can fix them up directly?
> 
> Regards,
> Brian
> 
> [1] http://lists.infradead.org/pipermail/linux-mtd/2015-October/062951.html

Re: [PATCH v6 0/3] Mediatek SPI-NOR flash driver

[Brian Norris]

On Wed, Nov 11, 2015 at 10:04:14PM +0800, Bayi Cheng wrote:
> On Mon, 2015-11-09 at 18:46 -0800, Brian Norris wrote:
> > I believe you didn't completely answer all my questions from v5 though.
> > I'll repeat a bit here. Particularly, refer to [1].
> > I'll summarize; I understand that your common transmit/receive operation
> > works something like this:
> > 
> > Quoting from [1]:
> > > (1) total number of bits to send/receive goes in the COUNT register (so
> > >     far, as many as 7*8=56?)
> > > (2) opcode is written to PRGDATA5
> > > (3) other "transmit" data (like addresses), if any, are placed on PRGDATA4..0
> > > (4) command is sent (execute_cmd())
> > > (5) data is read back in SHREG{X..0}, if needed
> > 
> > My questions were:
> > 
> > (a) Why does mt8173_nor_set_read_mode() use PRGDATA3? That's not
> >     mentioned in the SoC manual, and it doesn't really match any of the
> >     steps above. Perhaps it's just a quirk of the controller's
> >     programming model?
> > 
> yes, for this question, I have done some testes, If I change the
> PRGDATA3 to PRGDATA5 for mt8173_nor_set_read_mode() like others
> functions, then the controller will be hanged, and I have asked our
> designer for double confirm.

I wasn't suggesting to change this to PRGDATA5. I just was wondering why
the difference. It's not documented. (I suppose an acceptable answer is
just "because that's how the HW works.")

> > (b) How do you determine X from step (5)?
> > 
> > Right now, your code seems to answer that X is "rxlen - 1". Correct?
> > 
> yes, I have used "rxlen -1", because the first of nor flash output is
> located at SHREG[0], in the other words, the output data starts at
> SHREG[0] and go up to SHREG[relen -1]

But, we aren't reading from SHREG[0] first; we're reading backwards from
SHREG[rxlen - 1] down to SHREG[0]. It seems that's correct, right?

> > If that's correct and if I put all of my understanding together
> > correctly, this means that you can actually shift out (in PRGDATA) up to
> > 6 bytes (that is, 1 opcode and 5 tx bytes) and shift in (in SHREG) up to
> > 7 bytes, except that the first byte is received during the opcode cycle,
> > and so it is discarded, and we effectively receive only 6 bytes.
> > 
> > Is that all correct? If so, then I think you still need to adjust the
> > boundary conditions in your do_tx_rx() function. (I'll comment on the
> > driver to point out the specifics.)
> 
> Yes, you are right! and I will adjust the boundary conditions in
> do_tx_rx() function.

OK, good. BTW, can you make sure to rewrite the appropriate MAX macro(s)
to reflect the right values? It seems like maybe you'll want separate
macros for the maximum TX and RX -- and total (?), or is this just the
same as RX? -- since they seem to have different limits.

> By the way, could you tell me whether I need to publish a new patch? or
> you can fix them up directly?

I think there are a few more adjustments to make, so please just post a
new version of the driver only. The DT binding and DTS changes look good
to go now.

Regards,
Brian

> > [1] http://lists.infradead.org/pipermail/linux-mtd/2015-October/062951.html

Re: [PATCH v6 0/3] Mediatek SPI-NOR flash driver

[bayi cheng]

On Wed, 2015-11-11 at 12:26 -0800, Brian Norris wrote:
> On Wed, Nov 11, 2015 at 10:04:14PM +0800, Bayi Cheng wrote:
> > On Mon, 2015-11-09 at 18:46 -0800, Brian Norris wrote:
> > > I believe you didn't completely answer all my questions from v5 though.
> > > I'll repeat a bit here. Particularly, refer to [1].
> > > I'll summarize; I understand that your common transmit/receive operation
> > > works something like this:
> > > 
> > > Quoting from [1]:
> > > > (1) total number of bits to send/receive goes in the COUNT register (so
> > > >     far, as many as 7*8=56?)
> > > > (2) opcode is written to PRGDATA5
> > > > (3) other "transmit" data (like addresses), if any, are placed on PRGDATA4..0
> > > > (4) command is sent (execute_cmd())
> > > > (5) data is read back in SHREG{X..0}, if needed
> > > 
> > > My questions were:
> > > 
> > > (a) Why does mt8173_nor_set_read_mode() use PRGDATA3? That's not
> > >     mentioned in the SoC manual, and it doesn't really match any of the
> > >     steps above. Perhaps it's just a quirk of the controller's
> > >     programming model?
> > > 
> > yes, for this question, I have done some testes, If I change the
> > PRGDATA3 to PRGDATA5 for mt8173_nor_set_read_mode() like others
> > functions, then the controller will be hanged, and I have asked our
> > designer for double confirm.
> 
> I wasn't suggesting to change this to PRGDATA5. I just was wondering why
> the difference. It's not documented. (I suppose an acceptable answer is
> just "because that's how the HW works.")
> 
I have synced with our designer, and just as you said, that's how the HW
works, and the read operation is different is different from other
operation. By the way, I have double confirm our driver code, and I have
made a mistake for quad read operation, the quad command need use
PRGDATA4 Instead PRGDATA3.
PS: write PRGDATA4 0xEB(4bit I/O read mode) or 0x6B(4 bit output mode),
So I will modify mt8173_nor_set_read_mode function in next patch.


> > > (b) How do you determine X from step (5)?
> > > 
> > > Right now, your code seems to answer that X is "rxlen - 1". Correct?
> > > 
> > yes, I have used "rxlen -1", because the first of nor flash output is
> > located at SHREG[0], in the other words, the output data starts at
> > SHREG[0] and go up to SHREG[relen -1]
> 
> But, we aren't reading from SHREG[0] first; we're reading backwards from
> SHREG[rxlen - 1] down to SHREG[0]. It seems that's correct, right?

Yes, You are right!
> 
> > > If that's correct and if I put all of my understanding together
> > > correctly, this means that you can actually shift out (in PRGDATA) up to
> > > 6 bytes (that is, 1 opcode and 5 tx bytes) and shift in (in SHREG) up to
> > > 7 bytes, except that the first byte is received during the opcode cycle,
> > > and so it is discarded, and we effectively receive only 6 bytes.
> > > 
> > > Is that all correct? If so, then I think you still need to adjust the
> > > boundary conditions in your do_tx_rx() function. (I'll comment on the
> > > driver to point out the specifics.)
> > 
> > Yes, you are right! and I will adjust the boundary conditions in
> > do_tx_rx() function.
> 
> OK, good. BTW, can you make sure to rewrite the appropriate MAX macro(s)
> to reflect the right values? It seems like maybe you'll want separate
> macros for the maximum TX and RX -- and total (?), or is this just the
> same as RX? -- since they seem to have different limits.
> 

OK, I will use two MAX macros, one is for TX&RX, and the other is for 
the total.

> > By the way, could you tell me whether I need to publish a new patch? or
> > you can fix them up directly?
> 
> I think there are a few more adjustments to make, so please just post a
> new version of the driver only. The DT binding and DTS changes look good
> to go now.

Yes, I will publish a new patch next week, and Thanks again for your
grate help and support!

> 
> Regards,
> Brian
> 
> > > [1] http://lists.infradead.org/pipermail/linux-mtd/2015-October/062951.html
> 
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek