diff mbox

[v5,1/8] mtd: spi-nor: copy the SPI NOR commands to a new header file

Message ID 1393238262-8622-2-git-send-email-b32955@freescale.com (mailing list archive)
State New, archived
Headers show

Commit Message

Huang Shijie Feb. 24, 2014, 10:37 a.m. UTC
This patch adds a new header :spi-nor.h,
and copies 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 <b32955@freescale.com>
---
 include/linux/mtd/spi-nor.h |   55 +++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 55 insertions(+), 0 deletions(-)
 create mode 100644 include/linux/mtd/spi-nor.h

Comments

Marek Vasut March 4, 2014, 10:13 p.m. UTC | #1
On Monday, February 24, 2014 at 11:37:35 AM, Huang Shijie wrote:
> This patch adds a new header :spi-nor.h,
> and copies 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 <b32955@freescale.com>

Please keep me on CC ;-)

I honestly have to wonder ... why do you not 'move' these definitions instead of 
copying them ?

Best regards,
Marek Vasut
Huang Shijie March 5, 2014, 2:59 a.m. UTC | #2
? 2014?03?05? 06:13, Marek Vasut ??:
> Please keep me on CC;-)
sorry, My fault :(


> I honestly have to wonder ... why do you not 'move' these definitions instead of
> copying them ?
>
In the first version , i did so:
http://marc.info/?l=linux-arm-kernel&m=138544933522564&w=2

if I need to resend the patch set again, i can change to move the 
commands to the new header.

thanks
Huang Shijie
Marek Vasut March 5, 2014, 3:43 a.m. UTC | #3
On Wednesday, March 05, 2014 at 03:59:56 AM, Huang Shijie wrote:
> ? 2014?03?05? 06:13, Marek Vasut ??:
> > Please keep me on CC;-)
> 
> sorry, My fault :(
> 
> > I honestly have to wonder ... why do you not 'move' these definitions
> > instead of copying them ?
> 
> In the first version , i did so:
> http://marc.info/?l=linux-arm-kernel&m=138544933522564&w=2
> 
> if I need to resend the patch set again, i can change to move the
> commands to the new header.

Why didn't you keep it like that? Was there some reason for that ?

Best regards,
Marek Vasut
Huang Shijie March 5, 2014, 5:45 a.m. UTC | #4
? 2014?03?05? 11:43, Marek Vasut ??:
> Why didn't you keep it like that? Was there some reason for that ?
http://marc.info/?l=linux-arm-kernel&m=138545182232220&w=4

Pekon suggestted  do not touch the m25p80.c.
So i just copy these commands to a new header.
Anyway, it's really not an important issue for me. :)



thanks
Huang Shijie
pekon gupta March 5, 2014, 7:24 a.m. UTC | #5
Hi Marek, Shijie,

>From: Huang Shijie [mailto:b32955@freescale.com]

>

>? 2014?03?05? 11:43, Marek Vasut ??:

>> Why didn't you keep it like that? Was there some reason for that ?

>http://marc.info/?l=linux-arm-kernel&m=138545182232220&w=4

>

>Pekon suggestted  do not touch the m25p80.c.

>So i just copy these commands to a new header.

>Anyway, it's really not an important issue for me. :)

>

My opinion was that we should not touch m25p80.c because:
(1)  As m25p80 is already working for various vendors. So unless multiple
   platforms start using SPI NOR framework, m25p80.c should remain untouched.
(2) we should not clutter header file with vendor specific op-codes as used in m25p80.
   I was of the opinion that instead of hard-coding the vendor specific info as
   MACROs, such information should be taken from DT or platform-data.

However, this was my thought during initial versions of SPI NOR patches.
But now if you think that SPI NOR is stable enough that same header info
can be reused without conflicts, then no problems.

(Apologies to Huang Shijie for re-work :-) ).

with regards, pekon
Marek Vasut March 5, 2014, 1:36 p.m. UTC | #6
On Wednesday, March 05, 2014 at 08:24:21 AM, Gupta, Pekon wrote:
> Hi Marek, Shijie,
> 
> >From: Huang Shijie [mailto:b32955@freescale.com]
> >
> >? 2014?03?05? 11:43, Marek Vasut ??:
> >> Why didn't you keep it like that? Was there some reason for that ?
> >
> >http://marc.info/?l=linux-arm-kernel&m=138545182232220&w=4
> >
> >Pekon suggestted  do not touch the m25p80.c.
> >So i just copy these commands to a new header.
> >Anyway, it's really not an important issue for me. :)
> 
> My opinion was that we should not touch m25p80.c because:
> (1)  As m25p80 is already working for various vendors. So unless multiple
>    platforms start using SPI NOR framework, m25p80.c should remain
> untouched. (2) we should not clutter header file with vendor specific
> op-codes as used in m25p80. I was of the opinion that instead of
> hard-coding the vendor specific info as MACROs, such information should be
> taken from DT or platform-data.
> 
> However, this was my thought during initial versions of SPI NOR patches.
> But now if you think that SPI NOR is stable enough that same header info
> can be reused without conflicts, then no problems.
> 
> (Apologies to Huang Shijie for re-work :-) ).

Ah, thanks for explaining, understood. Given that the m25p80.c is reworked later 
in the series anyway, I'm mostly OK with this patch ... even though I'd prefer 
to see no duplication throughout the series ;-)

Best regards,
Marek Vasut
Brian Norris April 9, 2014, 5:40 p.m. UTC | #7
On Mon, Feb 24, 2014 at 06:37:35PM +0800, Huang Shijie wrote:
> This patch adds a new header :spi-nor.h,
> and copies all the SPI NOR commands and relative macros into this new header.
...
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> new file mode 100644
> index 0000000..483fc2a
> --- /dev/null
> +++ b/include/linux/mtd/spi-nor.h
> @@ -0,0 +1,55 @@
> +#ifndef __LINUX_MTD_SPI_NOR_H
> +#define __LINUX_MTD_SPI_NOR_H
> +

You're missing a copyright and license header. Please submit one against
l2-mtd.git/spinor, preferably after the patch series I just sent out
that renames a few things here.

Regarding copyright assignments (also an issue with spi-nor.c): please
carry the original copyright from m25p80.c, and add your own or your
employer's wherever your work was a significant addition.

> +/* 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_DUAL_READ        0x3b    /* Read data bytes (Dual SPI) */
> +#define	OPCODE_QUAD_READ        0x6b    /* Read data bytes (Quad SPI) */
> +#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_DUAL_READ_4B	0x3c    /* Read data bytes (Dual SPI) */
> +#define	OPCODE_QUAD_READ_4B	0x6c    /* Read data bytes (Quad SPI) */
> +#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

Brian
diff mbox

Patch

diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
new file mode 100644
index 0000000..483fc2a
--- /dev/null
+++ b/include/linux/mtd/spi-nor.h
@@ -0,0 +1,55 @@ 
+#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_DUAL_READ        0x3b    /* Read data bytes (Dual SPI) */
+#define	OPCODE_QUAD_READ        0x6b    /* Read data bytes (Quad SPI) */
+#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_DUAL_READ_4B	0x3c    /* Read data bytes (Dual SPI) */
+#define	OPCODE_QUAD_READ_4B	0x6c    /* Read data bytes (Quad SPI) */
+#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