| Message ID | 20250307-winbond-6-14-rc1-octal-v1-1-45c1e074ad74@bootlin.com |
|---|---|
| State | New |
| Headers | show |
| Series | mtd: spinand: Add octal support | expand |
On 3/7/25 3:08 PM, Miquel Raynal wrote: > SPI operations have been initially described through macros implicitly > implying the use of a single SPI SDR bus. Macros for supporting dual and > quad I/O transfers have been added on top, generally inspired by vendor > vendor naming, followed by DTR operations. Soon we might see octal > and even octal DTR operations as well (including the opcode byte). > > Let's clarify what the macro really means by describing the expected bus > topology in the reset macro name. > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > --- > drivers/mtd/nand/spi/core.c | 2 +- > include/linux/mtd/spinand.h | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c > index 7a578f3522be93d30a9132cf01831515db77ae38..ea6b48242ad4a4e51c713907ce5cc55022cdb569 100644 > --- a/drivers/mtd/nand/spi/core.c > +++ b/drivers/mtd/nand/spi/core.c > @@ -586,7 +586,7 @@ static int spinand_read_id_op(struct spinand_device *spinand, u8 naddr, > > static int spinand_reset_op(struct spinand_device *spinand) > { > - struct spi_mem_op op = SPINAND_RESET_OP; > + struct spi_mem_op op = SPINAND_RESET_1S_0_0_OP; > int ret; > > ret = spi_mem_exec_op(spinand->spimem, &op); > diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h > index 0da8a1c7740ef5cbbdb2a74858465136638224f4..4f47adbe4566d7813ffd8fbfaddd1a85d88d0208 100644 > --- a/include/linux/mtd/spinand.h > +++ b/include/linux/mtd/spinand.h > @@ -20,7 +20,7 @@ > * Standard SPI NAND flash operations > */ > > -#define SPINAND_RESET_OP \ > +#define SPINAND_RESET_1S_0_0_OP \ Hi, Miquel, Have you seen any reset op with address or data? If not, I'm not really sure whether we shall change the name for these basic operations. Changing them to 1S-0-0 may also indicate that there are resets with address or data fields, which I find confusing. I think the change is good for reads and writes. I'll check further in the series and let you know. Cheers, ta > SPI_MEM_OP(SPI_MEM_OP_CMD(0xff, 1), \ > SPI_MEM_OP_NO_ADDR, \ > SPI_MEM_OP_NO_DUMMY, \ >
Hi Tudor, >> -#define SPINAND_RESET_OP \ >> +#define SPINAND_RESET_1S_0_0_OP \ > Hi, Miquel, > > Have you seen any reset op with address or data? If not, I'm not really > sure whether we shall change the name for these basic operations. > > Changing them to 1S-0-0 may also indicate that there are resets with > address or data fields, which I find confusing. > > I think the change is good for reads and writes. I'll check further in > the series and let you know. I want to rename this macro for two reasons: - We might see in the near future the addition of 8D-0-0 ops (I plan on working on it). - I would like some kind of harmony among these macros. Now, whether is should be named like I proposed or just SPINAND_RESET_OP_1S, I have no strong preference and I can change that in an upcoming version. Which one would you prefer? Cheers, Miquèl
On 07.03.2025 17:45, Miquel Raynal wrote: > Hi Tudor, Hi! > >>> -#define SPINAND_RESET_OP \ >>> +#define SPINAND_RESET_1S_0_0_OP \ >> Hi, Miquel, >> >> Have you seen any reset op with address or data? If not, I'm not really >> sure whether we shall change the name for these basic operations. >> >> Changing them to 1S-0-0 may also indicate that there are resets with >> address or data fields, which I find confusing. >> >> I think the change is good for reads and writes. I'll check further in >> the series and let you know. > > I want to rename this macro for two reasons: > - We might see in the near future the addition of 8D-0-0 ops (I plan on > working on it). > - I would like some kind of harmony among these macros. > > Now, whether is should be named like I proposed or just > SPINAND_RESET_OP_1S, I have no strong preference and I can change that > in an upcoming version. > > Which one would you prefer? I don't know. Which one is backed up by a standard? JESD216F defines "(An-Bn-Cn): Command mode nomenclature used to indicate the number of active pins used for the instruction (A), address (B), and data (C), and the data rate used for each. Data rates(n) can be single (S) and dual (D)." Also, "(x-y-z) nomenclature is equivalent to(AS-BS-CS) unless otherwise noted." What's an "active pin"? Then if I look at JESD251-1.01 and JESD251C, (An-Bn-Cn) is referred to as "protocol mode". Write Enable, which is just an instruction command with no address or data, is seen as a required command in both 4D-4D-4D and 8D-8D-8D protocol modes, and it's defined as a "1.A" transaction format command. And the transaction format is: ''' The following transaction formats are used in Profile 1.0 mode 8D-8D-8D: Format 1.A: Command and Command Extension Format 1.B: Command, Command Extension, 4-byte Address, ‘n’ Latency Cycles, and Read Data Format 1.C: Command, Command Extension, and 4-byte Address Format 1.D: Command, Command Extension, 4-byte Address, and Write Data ''' So according to these standards maybe we shall refer to it as: WREN-8D-8D-8D-1A? This seems less intuitive than 8D-0-0, but I think it all depends on what's an "active pin". I think it describes the protocol mode, and not what's actually sent on the line. As we saw, WREN is considered an 8D-8D-8D command, and not an 8D-0-0 command. For dual mode, which is not covered by xSPI, I guess we can use the single spi transaction formats 0.{A,B,C,...}. How do you feel about a OP-An-Bn-Cn-transaction-format, where A, B, C is {1, 2, 4, 8} n is {S, D} transaction format is {0,1,2,3}{A,B,C,...} Care must be taken care of at the transaction format, as I see there are a few, depending on the xSPI profile and protocol mode. Cheers, ta
Hello Tudor, On 10/03/2025 at 12:14:06 +02, Tudor Ambarus <tudor.ambarus@linaro.org> wrote: > On 07.03.2025 17:45, Miquel Raynal wrote: >> Hi Tudor, > > Hi! > >> >>>> -#define SPINAND_RESET_OP \ >>>> +#define SPINAND_RESET_1S_0_0_OP \ >>> Hi, Miquel, >>> >>> Have you seen any reset op with address or data? If not, I'm not really >>> sure whether we shall change the name for these basic operations. >>> >>> Changing them to 1S-0-0 may also indicate that there are resets with >>> address or data fields, which I find confusing. >>> >>> I think the change is good for reads and writes. I'll check further in >>> the series and let you know. >> >> I want to rename this macro for two reasons: >> - We might see in the near future the addition of 8D-0-0 ops (I plan on >> working on it). >> - I would like some kind of harmony among these macros. >> >> Now, whether is should be named like I proposed or just >> SPINAND_RESET_OP_1S, I have no strong preference and I can change that >> in an upcoming version. >> >> Which one would you prefer? > > I don't know. Which one is backed up by a standard? Well, as far as I know, none of the SPI NAND devices follow a very specific standard. Most vendors in practice follow similar conventions, but they all deviate a bit from it. > JESD216F defines > "(An-Bn-Cn): Command mode nomenclature used to indicate the number of > active pins used for the instruction (A), address (B), and data (C), and > the data rate used for each. Data rates(n) can be single (S) and dual (D)." > > Also, "(x-y-z) nomenclature is equivalent to(AS-BS-CS) unless otherwise > noted." > > What's an "active pin"? > > Then if I look at JESD251-1.01 and JESD251C, (An-Bn-Cn) is referred to > as "protocol mode". Write Enable, which is just an instruction command > with no address or data, is seen as a required command in both 4D-4D-4D > and 8D-8D-8D protocol modes, and it's defined as a "1.A" transaction > format command. And the transaction format is: > ''' > The following transaction formats are used in Profile 1.0 mode 8D-8D-8D: > Format 1.A: Command and Command Extension > Format 1.B: Command, Command Extension, 4-byte Address, ‘n’ Latency > Cycles, and Read Data > Format 1.C: Command, Command Extension, and 4-byte Address > Format 1.D: Command, Command Extension, 4-byte Address, and Write Data > ''' > > So according to these standards maybe we shall refer to it as: > WREN-8D-8D-8D-1A? This seems less intuitive than 8D-0-0, but I think it > all depends on what's an "active pin". I think it describes the protocol > mode, and not what's actually sent on the line. As we saw, WREN is > considered an 8D-8D-8D command, and not an 8D-0-0 command. > > For dual mode, which is not covered by xSPI, I guess we can use the > single spi transaction formats 0.{A,B,C,...}. > > How do you feel about a OP-An-Bn-Cn-transaction-format, > where A, B, C is {1, 2, 4, 8} > n is {S, D} > transaction format is {0,1,2,3}{A,B,C,...} > > Care must be taken care of at the transaction format, as I see there are > a few, depending on the xSPI profile and protocol mode. I must admit I really dislike the transaction format abbreviation because it is specific to a spec we do not follow and is not explicit. I have a strong preference towards keeping just "An-Bn-Cn", like I did in this series. If people are in doubt, they can check what is in the op, it is self explanatory. I chose this in the first place because it is more explicit than just "An" which may feel like a cropped acronym. Furthermore, most octal capable datasheets I've looked at seem to use the "An-Bn-Cn" format, even for single opcodes. So unless there are strong arguments against, I'd prefer to keep the current formatting. TBH, it is not set in stone and can still evolve later if that's needed. Thanks, Miquèl
Hi, Miquel, On 3/19/25 5:32 PM, Miquel Raynal wrote: >>>>> -#define SPINAND_RESET_OP \ >>>>> +#define SPINAND_RESET_1S_0_0_OP \ >>>> Hi, Miquel, >>>> >>>> Have you seen any reset op with address or data? If not, I'm not really >>>> sure whether we shall change the name for these basic operations. >>>> >>>> Changing them to 1S-0-0 may also indicate that there are resets with >>>> address or data fields, which I find confusing. >>>> >>>> I think the change is good for reads and writes. I'll check further in >>>> the series and let you know. >>> >>> I want to rename this macro for two reasons: >>> - We might see in the near future the addition of 8D-0-0 ops (I plan on >>> working on it). >>> - I would like some kind of harmony among these macros. >>> >>> Now, whether is should be named like I proposed or just >>> SPINAND_RESET_OP_1S, I have no strong preference and I can change that >>> in an upcoming version. >>> >>> Which one would you prefer? >> >> I don't know. Which one is backed up by a standard? > > Well, as far as I know, none of the SPI NAND devices follow a very > specific standard. Most vendors in practice follow similar conventions, > but they all deviate a bit from it. > >> JESD216F defines >> "(An-Bn-Cn): Command mode nomenclature used to indicate the number of >> active pins used for the instruction (A), address (B), and data (C), and >> the data rate used for each. Data rates(n) can be single (S) and dual (D)." >> >> Also, "(x-y-z) nomenclature is equivalent to(AS-BS-CS) unless otherwise >> noted." >> >> What's an "active pin"? >> >> Then if I look at JESD251-1.01 and JESD251C, (An-Bn-Cn) is referred to >> as "protocol mode". Write Enable, which is just an instruction command >> with no address or data, is seen as a required command in both 4D-4D-4D >> and 8D-8D-8D protocol modes, and it's defined as a "1.A" transaction >> format command. And the transaction format is: >> ''' >> The following transaction formats are used in Profile 1.0 mode 8D-8D-8D: >> Format 1.A: Command and Command Extension >> Format 1.B: Command, Command Extension, 4-byte Address, ‘n’ Latency >> Cycles, and Read Data >> Format 1.C: Command, Command Extension, and 4-byte Address >> Format 1.D: Command, Command Extension, 4-byte Address, and Write Data >> ''' >> >> So according to these standards maybe we shall refer to it as: >> WREN-8D-8D-8D-1A? This seems less intuitive than 8D-0-0, but I think it >> all depends on what's an "active pin". I think it describes the protocol >> mode, and not what's actually sent on the line. As we saw, WREN is >> considered an 8D-8D-8D command, and not an 8D-0-0 command. >> >> For dual mode, which is not covered by xSPI, I guess we can use the >> single spi transaction formats 0.{A,B,C,...}. >> >> How do you feel about a OP-An-Bn-Cn-transaction-format, >> where A, B, C is {1, 2, 4, 8} >> n is {S, D} >> transaction format is {0,1,2,3}{A,B,C,...} >> >> Care must be taken care of at the transaction format, as I see there are >> a few, depending on the xSPI profile and protocol mode. > > I must admit I really dislike the transaction format abbreviation because I'm not in love with the "transaction format" either. > it is specific to a spec we do not follow and is not explicit. I have a > strong preference towards keeping just "An-Bn-Cn", like I did in this Okay. > series. If people are in doubt, they can check what is in the op, it is > self explanatory. I chose this in the first place because it is more > explicit than just "An" which may feel like a cropped > acronym. Furthermore, most octal capable datasheets I've looked at seem > to use the "An-Bn-Cn" format, even for single opcodes. So for RESET, WREN, WRDI and other opcodes that don't require address and data, are those datasheets referring to these opcodes as 1-1-1 commands? Can you point me to one of these datasheets, please? In SPI NOR, WREN is just WREN, regardless of the number of lines it is sent on, 1, 4, or 8. How would you model it here, 1-0-0, 4-0-0, 8-0-0 using the same opcode? > > So unless there are strong arguments against, I'd prefer to keep the > current formatting. TBH, it is not set in stone and can still evolve > later if that's needed. I find the naming scheme An-Bn-Cn good, I'm arguing that it's not needed everywhere, opcode only commands are better off with it, I think. Cheers, ta
>>> How do you feel about a OP-An-Bn-Cn-transaction-format, >>> where A, B, C is {1, 2, 4, 8} >>> n is {S, D} >>> transaction format is {0,1,2,3}{A,B,C,...} >>> >>> Care must be taken care of at the transaction format, as I see there are >>> a few, depending on the xSPI profile and protocol mode. >> >> I must admit I really dislike the transaction format abbreviation because > > I'm not in love with the "transaction format" either. > >> it is specific to a spec we do not follow and is not explicit. I have a >> strong preference towards keeping just "An-Bn-Cn", like I did in this > > Okay. > >> series. If people are in doubt, they can check what is in the op, it is >> self explanatory. I chose this in the first place because it is more >> explicit than just "An" which may feel like a cropped >> acronym. Furthermore, most octal capable datasheets I've looked at seem >> to use the "An-Bn-Cn" format, even for single opcodes. > > So for RESET, WREN, WRDI and other opcodes that don't require address > and data, are those datasheets referring to these opcodes as 1-1-1 commands? > Can you point me to one of these datasheets, please? All Winbond datasheets use this convention: https://www.winbond.com/export/sites/winbond/datasheet/W35N01JW_Datasheet_Brief.pdf See 7.1.2 Instruction Set Table p.25. I checked Macronix and Gigadevice datasheets, they simply never "name" these opcodes clearly. > In SPI NOR, WREN is just WREN, regardless of the number of lines it is > sent on, 1, 4, or 8. How would you model it here, 1-0-0, 4-0-0, 8-0-0 > using the same opcode? I guess, yes. >> So unless there are strong arguments against, I'd prefer to keep the >> current formatting. TBH, it is not set in stone and can still evolve >> later if that's needed. > > I find the naming scheme An-Bn-Cn good, I'm arguing that it's not needed > everywhere, opcode only commands are better off with it, I think. Ok, I can drop the -0-0 in this case if you prefer. Thanks, Miquèl
On 3/20/25 11:31 AM, Miquel Raynal wrote: > >>>> How do you feel about a OP-An-Bn-Cn-transaction-format, >>>> where A, B, C is {1, 2, 4, 8} >>>> n is {S, D} >>>> transaction format is {0,1,2,3}{A,B,C,...} >>>> >>>> Care must be taken care of at the transaction format, as I see there are >>>> a few, depending on the xSPI profile and protocol mode. >>> >>> I must admit I really dislike the transaction format abbreviation because >> >> I'm not in love with the "transaction format" either. >> >>> it is specific to a spec we do not follow and is not explicit. I have a >>> strong preference towards keeping just "An-Bn-Cn", like I did in this >> >> Okay. >> >>> series. If people are in doubt, they can check what is in the op, it is >>> self explanatory. I chose this in the first place because it is more >>> explicit than just "An" which may feel like a cropped >>> acronym. Furthermore, most octal capable datasheets I've looked at seem >>> to use the "An-Bn-Cn" format, even for single opcodes. >> >> So for RESET, WREN, WRDI and other opcodes that don't require address >> and data, are those datasheets referring to these opcodes as 1-1-1 commands? >> Can you point me to one of these datasheets, please? > > All Winbond datasheets use this convention: > https://www.winbond.com/export/sites/winbond/datasheet/W35N01JW_Datasheet_Brief.pdf > See 7.1.2 Instruction Set Table p.25. > Nice, thanks! > I checked Macronix and Gigadevice datasheets, they simply never "name" > these opcodes clearly. okay > >> In SPI NOR, WREN is just WREN, regardless of the number of lines it is >> sent on, 1, 4, or 8. How would you model it here, 1-0-0, 4-0-0, 8-0-0 >> using the same opcode? > > I guess, yes. > >>> So unless there are strong arguments against, I'd prefer to keep the >>> current formatting. TBH, it is not set in stone and can still evolve >>> later if that's needed. >> >> I find the naming scheme An-Bn-Cn good, I'm arguing that it's not needed >> everywhere, opcode only commands are better off with it, I think. > > Ok, I can drop the -0-0 in this case if you prefer. Not needed. Just wanted to make sure we have a unified way of dealing with the protocol modes. Having WREN-1-0-0 and WREN-8d-0-0 is not that bad after all, it allows you to specify the number of lines directly, without mangling the op afterwards. Reviewed-by: Tudor Ambarus <tudor.ambarus@linaro.org> Thanks for the patience. I'll check the other patches next week. Cheers, ta
diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c index 7a578f3522be93d30a9132cf01831515db77ae38..ea6b48242ad4a4e51c713907ce5cc55022cdb569 100644 --- a/drivers/mtd/nand/spi/core.c +++ b/drivers/mtd/nand/spi/core.c @@ -586,7 +586,7 @@ static int spinand_read_id_op(struct spinand_device *spinand, u8 naddr, static int spinand_reset_op(struct spinand_device *spinand) { - struct spi_mem_op op = SPINAND_RESET_OP; + struct spi_mem_op op = SPINAND_RESET_1S_0_0_OP; int ret; ret = spi_mem_exec_op(spinand->spimem, &op); diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h index 0da8a1c7740ef5cbbdb2a74858465136638224f4..4f47adbe4566d7813ffd8fbfaddd1a85d88d0208 100644 --- a/include/linux/mtd/spinand.h +++ b/include/linux/mtd/spinand.h @@ -20,7 +20,7 @@ * Standard SPI NAND flash operations */ -#define SPINAND_RESET_OP \ +#define SPINAND_RESET_1S_0_0_OP \ SPI_MEM_OP(SPI_MEM_OP_CMD(0xff, 1), \ SPI_MEM_OP_NO_ADDR, \ SPI_MEM_OP_NO_DUMMY, \
SPI operations have been initially described through macros implicitly implying the use of a single SPI SDR bus. Macros for supporting dual and quad I/O transfers have been added on top, generally inspired by vendor vendor naming, followed by DTR operations. Soon we might see octal and even octal DTR operations as well (including the opcode byte). Let's clarify what the macro really means by describing the expected bus topology in the reset macro name. Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> --- drivers/mtd/nand/spi/core.c | 2 +- include/linux/mtd/spinand.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)