mbox series

[0/5] mtd: rawnand: qcom: Add support for QSPI nand

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

Message

Md Sadre Alam Oct. 10, 2020, 5:31 a.m. UTC
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(-)

Comments

Miquel Raynal Oct. 28, 2020, 9:48 a.m. UTC | #1
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
Md Sadre Alam Oct. 28, 2020, 6:24 p.m. UTC | #2
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
Miquel Raynal Oct. 29, 2020, 7:53 a.m. UTC | #3
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 Oct. 29, 2020, 8:37 a.m. UTC | #4
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
Boris Brezillon Oct. 29, 2020, 8:38 a.m. UTC | #5
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/