| Message ID | 1602307902-16761-1-git-send-email-mdalam@codeaurora.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | mtd: rawnand: qcom: Add support for QSPI nand | expand |
Hello, Md Sadre Alam <mdalam@codeaurora.org> wrote on Sat, 10 Oct 2020 11:01:37 +0530: > QPIC 2.0 supports Serial NAND support in addition to all features and > commands in QPIC 1.0 for parallel NAND. Parallel and Serial NAND cannot > operate simultaneously. QSPI nand devices will connect to QPIC IO_MACRO > block of QPIC controller. There is a separate IO_MACRO clock for IO_MACRO > block. Default IO_MACRO block divide the input clock by 4. so if IO_MACRO > input clock is 320MHz then on bus it will be 80MHz, so QSPI nand device > should also support this frequency. > > QPIC provides 4 data pins to QSPI nand. In standard SPI mode (x1 mode) data > transfer will occur on only 2 pins one pin for Serial data in and one for > serial data out. In QUAD SPI mode (x4 mode) data transfer will occur at all > the four data lines. QPIC controller supports command for x1 mode and x4 mode. > > Md Sadre Alam (5): > dt-bindings: qcom_nandc: IPQ5018 QPIC NAND documentation > mtd: rawnand: qcom: Add initial support for qspi nand > mtd: rawnand: qcom: Read QPIC version > mtd: rawnand: qcom: Enable support for erase,read & write for serial > nand. > mtd: rawnand: qcom: Add support for serial training. > > .../devicetree/bindings/mtd/qcom_nandc.txt | 3 + > drivers/mtd/nand/raw/nand_ids.c | 13 + > drivers/mtd/nand/raw/qcom_nandc.c | 502 ++++++++++++++++++++- > 3 files changed, 494 insertions(+), 24 deletions(-) > I'm sorry but this series clearly breaks the current layering. I cannot authorize SPI-NAND code to fall into the raw NAND subsystem. As both typologies cannot be used at the same time, I guess you should have another driver handling this feature under the spi/ subsystem + a few declarations in the SPI-NAND devices list. Thanks, Miquèl
On 2020-10-28 15:18, Miquel Raynal wrote: > Hello, > > Md Sadre Alam <mdalam@codeaurora.org> wrote on Sat, 10 Oct 2020 > 11:01:37 +0530: > >> QPIC 2.0 supports Serial NAND support in addition to all features and >> commands in QPIC 1.0 for parallel NAND. Parallel and Serial NAND >> cannot >> operate simultaneously. QSPI nand devices will connect to QPIC >> IO_MACRO >> block of QPIC controller. There is a separate IO_MACRO clock for >> IO_MACRO >> block. Default IO_MACRO block divide the input clock by 4. so if >> IO_MACRO >> input clock is 320MHz then on bus it will be 80MHz, so QSPI nand >> device >> should also support this frequency. >> >> QPIC provides 4 data pins to QSPI nand. In standard SPI mode (x1 mode) >> data >> transfer will occur on only 2 pins one pin for Serial data in and one >> for >> serial data out. In QUAD SPI mode (x4 mode) data transfer will occur >> at all >> the four data lines. QPIC controller supports command for x1 mode and >> x4 mode. >> >> Md Sadre Alam (5): >> dt-bindings: qcom_nandc: IPQ5018 QPIC NAND documentation >> mtd: rawnand: qcom: Add initial support for qspi nand >> mtd: rawnand: qcom: Read QPIC version >> mtd: rawnand: qcom: Enable support for erase,read & write for serial >> nand. >> mtd: rawnand: qcom: Add support for serial training. >> >> .../devicetree/bindings/mtd/qcom_nandc.txt | 3 + >> drivers/mtd/nand/raw/nand_ids.c | 13 + >> drivers/mtd/nand/raw/qcom_nandc.c | 502 >> ++++++++++++++++++++- >> 3 files changed, 494 insertions(+), 24 deletions(-) >> > > I'm sorry but this series clearly breaks the current layering. I cannot > authorize SPI-NAND code to fall into the raw NAND subsystem. > I am agree with you, we should not add SPI-NAND changes inside raw NAND subsystem. > As both typologies cannot be used at the same time, I guess you should > have another driver handling this feature under the spi/ subsystem + > a few declarations in the SPI-NAND devices list. > Initially I was started writing separate driver under SPI-NAND subsystem then I realized that more than 85% of raw/qcom_nand.c code getting duplicated. That's why I have added this SPI-NAND change in raw/qcom_nand.c since more than 85% of code will be reused. If I will add this change inside SPI-NAND subsystem then much of raw/qcom_nand.c code will get duplicated. Would it be ok ? > Thanks, > Miquèl
Hello, mdalam@codeaurora.org wrote on Wed, 28 Oct 2020 23:54:23 +0530: > On 2020-10-28 15:18, Miquel Raynal wrote: > > Hello, > > > > Md Sadre Alam <mdalam@codeaurora.org> wrote on Sat, 10 Oct 2020 > > 11:01:37 +0530: > > > >> QPIC 2.0 supports Serial NAND support in addition to all features and > >> commands in QPIC 1.0 for parallel NAND. Parallel and Serial NAND >> cannot > >> operate simultaneously. QSPI nand devices will connect to QPIC >> IO_MACRO > >> block of QPIC controller. There is a separate IO_MACRO clock for >> IO_MACRO > >> block. Default IO_MACRO block divide the input clock by 4. so if >> IO_MACRO > >> input clock is 320MHz then on bus it will be 80MHz, so QSPI nand >> device > >> should also support this frequency. > >> >> QPIC provides 4 data pins to QSPI nand. In standard SPI mode (x1 mode) >> data > >> transfer will occur on only 2 pins one pin for Serial data in and one >> for > >> serial data out. In QUAD SPI mode (x4 mode) data transfer will occur >> at all > >> the four data lines. QPIC controller supports command for x1 mode and >> x4 mode. > >> >> Md Sadre Alam (5): > >> dt-bindings: qcom_nandc: IPQ5018 QPIC NAND documentation > >> mtd: rawnand: qcom: Add initial support for qspi nand > >> mtd: rawnand: qcom: Read QPIC version > >> mtd: rawnand: qcom: Enable support for erase,read & write for serial > >> nand. > >> mtd: rawnand: qcom: Add support for serial training. > >> >> .../devicetree/bindings/mtd/qcom_nandc.txt | 3 + > >> drivers/mtd/nand/raw/nand_ids.c | 13 + > >> drivers/mtd/nand/raw/qcom_nandc.c | 502 >> ++++++++++++++++++++- > >> 3 files changed, 494 insertions(+), 24 deletions(-) > >> > > I'm sorry but this series clearly breaks the current layering. I cannot > > authorize SPI-NAND code to fall into the raw NAND subsystem. > > > > I am agree with you, we should not add SPI-NAND changes inside > raw NAND subsystem. > > > As both typologies cannot be used at the same time, I guess you should > > have another driver handling this feature under the spi/ subsystem + > > a few declarations in the SPI-NAND devices list. > > > > Initially I was started writing separate driver under SPI-NAND subsystem then I > realized that more than 85% of raw/qcom_nand.c code getting duplicated. > > That's why I have added this SPI-NAND change in raw/qcom_nand.c since > more than 85% of code will be reused. > > If I will add this change inside SPI-NAND subsystem then much of > raw/qcom_nand.c code will get duplicated. Would it be ok ? What about moving the generic code to drivers/mtd/nand/common/ and referring to it from drivers/mtd/nand/raw/qcom_nand.c and drivers/spi/spi-qcom.c (or such)? Thanks, Miquèl
Miquel Raynal <miquel.raynal@bootlin.com> wrote on Thu, 29 Oct 2020 08:53:44 +0100: > Hello, > > mdalam@codeaurora.org wrote on Wed, 28 Oct 2020 23:54:23 +0530: > > > On 2020-10-28 15:18, Miquel Raynal wrote: > > > Hello, > > > > > > Md Sadre Alam <mdalam@codeaurora.org> wrote on Sat, 10 Oct 2020 > > > 11:01:37 +0530: > > > > > >> QPIC 2.0 supports Serial NAND support in addition to all features and > > >> commands in QPIC 1.0 for parallel NAND. Parallel and Serial NAND >> cannot > > >> operate simultaneously. QSPI nand devices will connect to QPIC >> IO_MACRO > > >> block of QPIC controller. There is a separate IO_MACRO clock for >> IO_MACRO > > >> block. Default IO_MACRO block divide the input clock by 4. so if >> IO_MACRO > > >> input clock is 320MHz then on bus it will be 80MHz, so QSPI nand >> device > > >> should also support this frequency. > > >> >> QPIC provides 4 data pins to QSPI nand. In standard SPI mode (x1 mode) >> data > > >> transfer will occur on only 2 pins one pin for Serial data in and one >> for > > >> serial data out. In QUAD SPI mode (x4 mode) data transfer will occur >> at all > > >> the four data lines. QPIC controller supports command for x1 mode and >> x4 mode. > > >> >> Md Sadre Alam (5): > > >> dt-bindings: qcom_nandc: IPQ5018 QPIC NAND documentation > > >> mtd: rawnand: qcom: Add initial support for qspi nand > > >> mtd: rawnand: qcom: Read QPIC version > > >> mtd: rawnand: qcom: Enable support for erase,read & write for serial > > >> nand. > > >> mtd: rawnand: qcom: Add support for serial training. > > >> >> .../devicetree/bindings/mtd/qcom_nandc.txt | 3 + > > >> drivers/mtd/nand/raw/nand_ids.c | 13 + > > >> drivers/mtd/nand/raw/qcom_nandc.c | 502 >> ++++++++++++++++++++- > > >> 3 files changed, 494 insertions(+), 24 deletions(-) > > >> > > I'm sorry but this series clearly breaks the current layering. I cannot > > > authorize SPI-NAND code to fall into the raw NAND subsystem. > > > > > > > I am agree with you, we should not add SPI-NAND changes inside > > raw NAND subsystem. > > > > > As both typologies cannot be used at the same time, I guess you should > > > have another driver handling this feature under the spi/ subsystem + > > > a few declarations in the SPI-NAND devices list. > > > > > > > Initially I was started writing separate driver under SPI-NAND subsystem then I > > realized that more than 85% of raw/qcom_nand.c code getting duplicated. > > > > That's why I have added this SPI-NAND change in raw/qcom_nand.c since > > more than 85% of code will be reused. > > > > If I will add this change inside SPI-NAND subsystem then much of > > raw/qcom_nand.c code will get duplicated. Would it be ok ? > > What about moving the generic code to drivers/mtd/nand/common/ and > referring to it from drivers/mtd/nand/raw/qcom_nand.c and > drivers/spi/spi-qcom.c (or such)? Actually, perhaps drivers/memory/ is the right location for the generic bits. Thanks, Miquèl
On Thu, 29 Oct 2020 08:53:44 +0100 Miquel Raynal <miquel.raynal@bootlin.com> wrote: > Hello, > > mdalam@codeaurora.org wrote on Wed, 28 Oct 2020 23:54:23 +0530: > > > On 2020-10-28 15:18, Miquel Raynal wrote: > > > Hello, > > > > > > Md Sadre Alam <mdalam@codeaurora.org> wrote on Sat, 10 Oct 2020 > > > 11:01:37 +0530: > > > > > >> QPIC 2.0 supports Serial NAND support in addition to all features and > > >> commands in QPIC 1.0 for parallel NAND. Parallel and Serial NAND >> cannot > > >> operate simultaneously. QSPI nand devices will connect to QPIC >> IO_MACRO > > >> block of QPIC controller. There is a separate IO_MACRO clock for >> IO_MACRO > > >> block. Default IO_MACRO block divide the input clock by 4. so if >> IO_MACRO > > >> input clock is 320MHz then on bus it will be 80MHz, so QSPI nand >> device > > >> should also support this frequency. > > >> >> QPIC provides 4 data pins to QSPI nand. In standard SPI mode (x1 mode) >> data > > >> transfer will occur on only 2 pins one pin for Serial data in and one >> for > > >> serial data out. In QUAD SPI mode (x4 mode) data transfer will occur >> at all > > >> the four data lines. QPIC controller supports command for x1 mode and >> x4 mode. > > >> >> Md Sadre Alam (5): > > >> dt-bindings: qcom_nandc: IPQ5018 QPIC NAND documentation > > >> mtd: rawnand: qcom: Add initial support for qspi nand > > >> mtd: rawnand: qcom: Read QPIC version > > >> mtd: rawnand: qcom: Enable support for erase,read & write for serial > > >> nand. > > >> mtd: rawnand: qcom: Add support for serial training. > > >> >> .../devicetree/bindings/mtd/qcom_nandc.txt | 3 + > > >> drivers/mtd/nand/raw/nand_ids.c | 13 + > > >> drivers/mtd/nand/raw/qcom_nandc.c | 502 >> ++++++++++++++++++++- > > >> 3 files changed, 494 insertions(+), 24 deletions(-) > > >> > > I'm sorry but this series clearly breaks the current layering. I cannot > > > authorize SPI-NAND code to fall into the raw NAND subsystem. > > > > > > > I am agree with you, we should not add SPI-NAND changes inside > > raw NAND subsystem. > > > > > As both typologies cannot be used at the same time, I guess you should > > > have another driver handling this feature under the spi/ subsystem + > > > a few declarations in the SPI-NAND devices list. > > > > > > > Initially I was started writing separate driver under SPI-NAND subsystem then I > > realized that more than 85% of raw/qcom_nand.c code getting duplicated. > > > > That's why I have added this SPI-NAND change in raw/qcom_nand.c since > > more than 85% of code will be reused. > > > > If I will add this change inside SPI-NAND subsystem then much of > > raw/qcom_nand.c code will get duplicated. Would it be ok ? > > What about moving the generic code to drivers/mtd/nand/common/ and Yeah, I don't think drivers/mtd/nand/common/ is the right place to put this common code TBH, and I don't really see what's to be shared between the NAND controller and SPI controller drivers. If it's just helpers to access the registers, those can probably live in drivers/soc/qcom/qcom_qpic2.c. > referring to it from drivers/mtd/nand/raw/qcom_nand.c and > drivers/spi/spi-qcom.c (or such)? > > Thanks, > Miquèl > > ______________________________________________________ > Linux MTD discussion mailing list > http://lists.infradead.org/mailman/listinfo/linux-mtd/
QPIC 2.0 supports Serial NAND support in addition to all features and commands in QPIC 1.0 for parallel NAND. Parallel and Serial NAND cannot operate simultaneously. QSPI nand devices will connect to QPIC IO_MACRO block of QPIC controller. There is a separate IO_MACRO clock for IO_MACRO block. Default IO_MACRO block divide the input clock by 4. so if IO_MACRO input clock is 320MHz then on bus it will be 80MHz, so QSPI nand device should also support this frequency. QPIC provides 4 data pins to QSPI nand. In standard SPI mode (x1 mode) data transfer will occur on only 2 pins one pin for Serial data in and one for serial data out. In QUAD SPI mode (x4 mode) data transfer will occur at all the four data lines. QPIC controller supports command for x1 mode and x4 mode. Md Sadre Alam (5): dt-bindings: qcom_nandc: IPQ5018 QPIC NAND documentation mtd: rawnand: qcom: Add initial support for qspi nand mtd: rawnand: qcom: Read QPIC version mtd: rawnand: qcom: Enable support for erase,read & write for serial nand. mtd: rawnand: qcom: Add support for serial training. .../devicetree/bindings/mtd/qcom_nandc.txt | 3 + drivers/mtd/nand/raw/nand_ids.c | 13 + drivers/mtd/nand/raw/qcom_nandc.c | 502 ++++++++++++++++++++- 3 files changed, 494 insertions(+), 24 deletions(-)