[1/4] mtd: spi-nor: move the SPI NOR commands to a new header file

Commit Message

Huang Shijie Nov. 26, 2013, 6:32 a.m. UTC

This patch adds a new header :spi-nor.h,
and moves all the SPI NOR commands and relative macros into this new header.

This hearder can be used by the m25p80.c and other spi-nor controller,
such as Freescale's Quadspi.

Signed-off-by: Huang Shijie <[e-mail]>
---
 drivers/mtd/devices/m25p80.c |   50 +--------------------------------------
 include/linux/mtd/spi-nor.h  |   53 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 49 deletions(-)
 create mode 100644 include/linux/mtd/spi-nor.h

Comments

pekon gupta Nov. 26, 2013, 7:42 a.m. UTC | #1

Hi Shijie,

Few feedbacks inline.. 

> From: Huang Shijie [mailto:[e-mail]]
> This patch adds a new header :spi-nor.h,
> and moves all the SPI NOR commands and relative macros into this new
> header.
> 
(1)
It's better to take all Flash vendor specific opcodes from DT.. 
With new framework we should do away with macros for each vendor,
instead let each board file provide this information based on flash device
which is mounted on it.

(2)
And also, I suggest not to touch m25p80 in anyway, bcoz it stable driver
used by many. So unless spi-nor framework is accepted and is stable we
m25p80 should continue working as it is.
Thus your patches should be independent of any existing driver or other
frameworks. This way it might get accepted fast. Then later you can provide
a way to attach m25p80 to spi-nor framework.
Thoughts ?


with regards, pekon

Huang Shijie Nov. 26, 2013, 8:53 a.m. UTC | #2

Hi Pekon:
> (1)
> It's better to take all Flash vendor specific opcodes from DT..
firstly, some arch may does not support the DT, such as x86.

second, it makes people crazy if we put the opcodes in the DT :)
> With new framework we should do away with macros for each vendor,
> instead let each board file provide this information based on flash device
> which is mounted on it.
>
> (2)
> And also, I suggest not to touch m25p80 in anyway, bcoz it stable driver
> used by many. So unless spi-nor framework is accepted and is stable we
> m25p80 should continue working as it is.
> Thus your patches should be independent of any existing driver or other
> frameworks. This way it might get accepted fast. Then later you can provide
> a way to attach m25p80 to spi-nor framework.
> Thoughts ?
the merge window is closed not long ago, and we have rc1 now.

Isn't it the BEST time to change the m25p80?

thanks
Huang Shijie

Angus Clark Nov. 26, 2013, 2:48 p.m. UTC | #3

On 11/26/2013 08:53 AM, Huang Shijie wrote:
> Hi Pekon:
>> (1)
>> It's better to take all Flash vendor specific opcodes from DT..
> firstly, some arch may does not support the DT, such as x86.

Indeed, although the 'flash_platform_data' structure could be extended to
provide similar functionality, if that were deemed the right way to go.  However...

> 
> second, it makes people crazy if we put the opcodes in the DT :)

I would agree.  Where possible, I think it's preferable for the driver to probe
device properties at runtime.  One benefit, often overlooked, is that it allows
board manufacturers to second-source parts without having to rebuild, and
possibly re-certify, binaries.

>> With new framework we should do away with macros for each vendor,
>> instead let each board file provide this information based on flash
>> device
>> which is mounted on it.
>>
>> (2)
>> And also, I suggest not to touch m25p80 in anyway, bcoz it stable driver
>> used by many. So unless spi-nor framework is accepted and is stable we
>> m25p80 should continue working as it is.
>> Thus your patches should be independent of any existing driver or other
>> frameworks. This way it might get accepted fast. Then later you can
>> provide
>> a way to attach m25p80 to spi-nor framework.
>> Thoughts ?
> the merge window is closed not long ago, and we have rc1 now.
> 
> Isn't it the BEST time to change the m25p80?
> 

The spi-nor framework has great potential, but I think you should allow some
time for other driver developers to comment, and the framework to mature.
Without greater support, it risks becoming another single-use framework.

Cheers,

Angus

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

Patch

diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index 7dc2c14..4703aa4 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -34,55 +34,7 @@ 
 
 #include <linux/spi/spi.h>
 #include <linux/spi/flash.h>
-
-/* Flash opcodes. */
-#define	OPCODE_WREN		0x06	/* Write enable */
-#define	OPCODE_RDSR		0x05	/* Read status register */
-#define	OPCODE_WRSR		0x01	/* Write status register 1 byte */
-#define	OPCODE_NORM_READ	0x03	/* Read data bytes (low frequency) */
-#define	OPCODE_FAST_READ	0x0b	/* Read data bytes (high frequency) */
-#define	OPCODE_QUAD_READ        0x6b    /* Read data bytes */
-#define	OPCODE_PP		0x02	/* Page program (up to 256 bytes) */
-#define	OPCODE_BE_4K		0x20	/* Erase 4KiB block */
-#define	OPCODE_BE_4K_PMC	0xd7	/* Erase 4KiB block on PMC chips */
-#define	OPCODE_BE_32K		0x52	/* Erase 32KiB block */
-#define	OPCODE_CHIP_ERASE	0xc7	/* Erase whole flash chip */
-#define	OPCODE_SE		0xd8	/* Sector erase (usually 64KiB) */
-#define	OPCODE_RDID		0x9f	/* Read JEDEC ID */
-#define	OPCODE_RDCR             0x35    /* Read configuration register */
-
-/* 4-byte address opcodes - used on Spansion and some Macronix flashes. */
-#define	OPCODE_NORM_READ_4B	0x13	/* Read data bytes (low frequency) */
-#define	OPCODE_FAST_READ_4B	0x0c	/* Read data bytes (high frequency) */
-#define	OPCODE_QUAD_READ_4B	0x6c    /* Read data bytes */
-#define	OPCODE_PP_4B		0x12	/* Page program (up to 256 bytes) */
-#define	OPCODE_SE_4B		0xdc	/* Sector erase (usually 64KiB) */
-
-/* Used for SST flashes only. */
-#define	OPCODE_BP		0x02	/* Byte program */
-#define	OPCODE_WRDI		0x04	/* Write disable */
-#define	OPCODE_AAI_WP		0xad	/* Auto address increment word program */
-
-/* Used for Macronix and Winbond flashes. */
-#define	OPCODE_EN4B		0xb7	/* Enter 4-byte mode */
-#define	OPCODE_EX4B		0xe9	/* Exit 4-byte mode */
-
-/* Used for Spansion flashes only. */
-#define	OPCODE_BRWR		0x17	/* Bank register write */
-
-/* Status Register bits. */
-#define	SR_WIP			1	/* Write in progress */
-#define	SR_WEL			2	/* Write enable latch */
-/* meaning of other SR_* bits may differ between vendors */
-#define	SR_BP0			4	/* Block protect 0 */
-#define	SR_BP1			8	/* Block protect 1 */
-#define	SR_BP2			0x10	/* Block protect 2 */
-#define	SR_SRWD			0x80	/* SR write protect */
-
-#define SR_QUAD_EN_MX           0x40    /* Macronix Quad I/O */
-
-/* Configuration Register bits. */
-#define CR_QUAD_EN_SPAN		0x2     /* Spansion Quad I/O */
+#include <linux/mtd/spi-nor.h>
 
 /* Define max times to check status register before we give up. */
 #define	MAX_READY_WAIT_JIFFIES	(40 * HZ)	/* M25P16 specs 40s max chip erase */
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
new file mode 100644
index 0000000..ab2ea1e
--- /dev/null
+++ b/include/linux/mtd/spi-nor.h
@@ -0,0 +1,53 @@ 
+#ifndef __LINUX_MTD_SPI_NOR_H
+#define __LINUX_MTD_SPI_NOR_H
+
+/* Flash opcodes. */
+#define	OPCODE_WREN		0x06	/* Write enable */
+#define	OPCODE_RDSR		0x05	/* Read status register */
+#define	OPCODE_WRSR		0x01	/* Write status register 1 byte */
+#define	OPCODE_NORM_READ	0x03	/* Read data bytes (low frequency) */
+#define	OPCODE_FAST_READ	0x0b	/* Read data bytes (high frequency) */
+#define	OPCODE_QUAD_READ        0x6b    /* Read data bytes */
+#define	OPCODE_PP		0x02	/* Page program (up to 256 bytes) */
+#define	OPCODE_BE_4K		0x20	/* Erase 4KiB block */
+#define	OPCODE_BE_4K_PMC	0xd7	/* Erase 4KiB block on PMC chips */
+#define	OPCODE_BE_32K		0x52	/* Erase 32KiB block */
+#define	OPCODE_CHIP_ERASE	0xc7	/* Erase whole flash chip */
+#define	OPCODE_SE		0xd8	/* Sector erase (usually 64KiB) */
+#define	OPCODE_RDID		0x9f	/* Read JEDEC ID */
+#define	OPCODE_RDCR             0x35    /* Read configuration register */
+
+/* 4-byte address opcodes - used on Spansion and some Macronix flashes. */
+#define	OPCODE_NORM_READ_4B	0x13	/* Read data bytes (low frequency) */
+#define	OPCODE_FAST_READ_4B	0x0c	/* Read data bytes (high frequency) */
+#define	OPCODE_QUAD_READ_4B	0x6c    /* Read data bytes */
+#define	OPCODE_PP_4B		0x12	/* Page program (up to 256 bytes) */
+#define	OPCODE_SE_4B		0xdc	/* Sector erase (usually 64KiB) */
+
+/* Used for SST flashes only. */
+#define	OPCODE_BP		0x02	/* Byte program */
+#define	OPCODE_WRDI		0x04	/* Write disable */
+#define	OPCODE_AAI_WP		0xad	/* Auto address increment word program */
+
+/* Used for Macronix and Winbond flashes. */
+#define	OPCODE_EN4B		0xb7	/* Enter 4-byte mode */
+#define	OPCODE_EX4B		0xe9	/* Exit 4-byte mode */
+
+/* Used for Spansion flashes only. */
+#define	OPCODE_BRWR		0x17	/* Bank register write */
+
+/* Status Register bits. */
+#define	SR_WIP			1	/* Write in progress */
+#define	SR_WEL			2	/* Write enable latch */
+/* meaning of other SR_* bits may differ between vendors */
+#define	SR_BP0			4	/* Block protect 0 */
+#define	SR_BP1			8	/* Block protect 1 */
+#define	SR_BP2			0x10	/* Block protect 2 */
+#define	SR_SRWD			0x80	/* SR write protect */
+
+#define SR_QUAD_EN_MX           0x40    /* Macronix Quad I/O */
+
+/* Configuration Register bits. */
+#define CR_QUAD_EN_SPAN		0x2     /* Spansion Quad I/O */
+
+#endif