mbox series

[RFC,v1,0/4] Add a driver for Mediatek SPI Nand controller

Message ID 20210927053629.17847-1-xiangsheng.hou@mediatek.com (mailing list archive)
Headers show
Series Add a driver for Mediatek SPI Nand controller | expand

Message

Xiangsheng Hou Sept. 27, 2021, 5:36 a.m. UTC
Add a driver for Mediatek SPI Nand controller

Mediatek SPI Nand controller cosists of two parts: on-host HW ECC and
snfi(stand for spi nand flash interface). They can cowork with high
performance which called ECC nfi mode. The nfi stand for nand flash
interfacei(snfi a one part of nfi) which can support SPI Nand flash
and raw nand flash.

However, the snfi driver in spi subsytem need to be aware of nand
parameter(page/spare size) and ecc status(enable/disable) when work
at ECC nfi mode. The snfi driver in spi subsystem seems difficult to
know these.

Therefore, consider two ways to let snfi can get these information.
The RFC patch send to review whether they are suitable and which
solution maybe better.

RFC patch v1:
Add nfi register base at bch(ecc) dts node and config nand parameter
and ecc status into nfi registers in ecc driver, then parse these
information at snfi driver to use.

RFC patch v2:
Export some function in HW ECC driver and snfi driver.
In HW ECC driver, export function include get nand page/spare size, HW
ECC status(enable/disable) and fdm(oob free per sector in ooblayout) size.
In snfi driver need export empty page status which the nfi can be aware
when in ECC nfi mode(the spim framework can not return this information).

This series patch is the v1 solution, and only take mt7622 board for dts
node example.

Xiangsheng Hou (4):
  mtd: ecc: move mediatek HW ECC driver
  mtd: ecc: realize Mediatek HW ECC driver
  spi: add Mediatek SPI Nand controller driver
  arm64: dts: add snfi node for spi nand

 arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts  |   16 +
 arch/arm64/boot/dts/mediatek/mt7622.dtsi      |   15 +-
 drivers/mtd/nand/Kconfig                      |    9 +
 drivers/mtd/nand/Makefile                     |    1 +
 drivers/mtd/nand/core.c                       |    2 +-
 drivers/mtd/nand/ecc.c                        |   19 +
 drivers/mtd/nand/{raw => }/mtk_ecc.c          |  321 ++++-
 drivers/mtd/nand/raw/Kconfig                  |    1 +
 drivers/mtd/nand/raw/Makefile                 |    2 +-
 drivers/mtd/nand/raw/mtk_nand.c               |    2 +-
 drivers/spi/Kconfig                           |   11 +
 drivers/spi/Makefile                          |    1 +
 drivers/spi/spi-mtk-snfi.c                    | 1043 +++++++++++++++++
 .../nand/raw => include/linux/mtd}/mtk_ecc.h  |    0
 include/linux/mtd/nand.h                      |   12 +
 15 files changed, 1450 insertions(+), 5 deletions(-)
 rename drivers/mtd/nand/{raw => }/mtk_ecc.c (63%)
 create mode 100644 drivers/spi/spi-mtk-snfi.c
 rename {drivers/mtd/nand/raw => include/linux/mtd}/mtk_ecc.h (100%)

Comments

Miquel Raynal Oct. 8, 2021, 9:20 a.m. UTC | #1
Hello,

xiangsheng.hou@mediatek.com wrote on Mon, 27 Sep 2021 13:36:25 +0800:

> Add a driver for Mediatek SPI Nand controller
> 
> Mediatek SPI Nand controller cosists of two parts: on-host HW ECC and
> snfi(stand for spi nand flash interface). They can cowork with high
> performance which called ECC nfi mode. The nfi stand for nand flash
> interfacei(snfi a one part of nfi) which can support SPI Nand flash
> and raw nand flash.
> 
> However, the snfi driver in spi subsytem need to be aware of nand
> parameter(page/spare size) and ecc status(enable/disable) when work
> at ECC nfi mode. The snfi driver in spi subsystem seems difficult to
> know these.
> 
> Therefore, consider two ways to let snfi can get these information.
> The RFC patch send to review whether they are suitable and which
> solution maybe better.
> 
> RFC patch v1:
> Add nfi register base at bch(ecc) dts node and config nand parameter
> and ecc status into nfi registers in ecc driver, then parse these
> information at snfi driver to use.
> 
> RFC patch v2:
> Export some function in HW ECC driver and snfi driver.
> In HW ECC driver, export function include get nand page/spare size, HW
> ECC status(enable/disable) and fdm(oob free per sector in ooblayout) size.
> In snfi driver need export empty page status which the nfi can be aware
> when in ECC nfi mode(the spim framework can not return this information).
> 

I've looked at both versions that you provided and I thought about a
number of things that cannot be done like this:
- I believe the snfi is a regular SPI controller. I will let Mark
  confirm but I do not think we want to start writing SPI-NAND
  controllers. Instead we write SPI controllers and we provide SPI-mem
  operations (we've explained this in a previous ELC, the video is
  available on YouTube).
- You cannot add an MTK ECC algorithm. This is dedicated for sofware
  solutions only and as far as I understand your engine uses the BCH
  algorithm.
- When the ECC engine is pipelined, there is an additional complexity
  in interfacing it with a SPI controller (that's your case I believe).
  I have an example that is not yet upstream but I think worth looking
  at that I will send very soon (I will Cc: you on it).
- The DT description for those engines that has been described on the
  mailing list and will be enforced is:

// External engine

	&spi-controller {
                flash@0 {
                        compatible = "spi-nand";
                        reg = <0>;
                        nand-ecc-engine = <&ecc_engine>;
                        spi-max-frequency = <50000000>;
                        spi-tx-bus-width = <4>;
                        spi-rx-bus-width = <4>;
                };
        };

        ecc_engine: ecc@43c40000 {
                compatible = "mxic,nand-ecc-engine-rev3";
                reg = <0x43c40000 0x10000>;
        };
 

// Pipelined engine

	&spi-controller {
               nand-ecc-engine = <&ecc_engine>;
 
                flash@0 {
                        compatible = "spi-nand";
                        reg = <0>;
                        nand-ecc-engine = <&spi_controller>;
                        spi-max-frequency = <50000000>;
                        spi-tx-bus-width = <4>;
                        spi-rx-bus-width = <4>;
 		};
	};

        ecc_engine: ecc@43c40000 {
                compatible = "mxic,nand-ecc-engine-rev3";
                reg = <0x43c40000 0x10000>;
        };

Thanks,
Miquèl
Xiangsheng Hou Oct. 11, 2021, 11:31 a.m. UTC | #2
Hi Miquel,

On Fri, 2021-10-08 at 11:20 +0200, Miquel Raynal wrote:
> Hello,
> 
> xiangsheng.hou@mediatek.com wrote on Mon, 27 Sep 2021 13:36:25 +0800:
> 
> > Add a driver for Mediatek SPI Nand controller
> > 
> > Mediatek SPI Nand controller cosists of two parts: on-host HW ECC
> > and
> > snfi(stand for spi nand flash interface). They can cowork with high
> > performance which called ECC nfi mode. The nfi stand for nand flash
> > interfacei(snfi a one part of nfi) which can support SPI Nand flash
> > and raw nand flash.
> > 
> > However, the snfi driver in spi subsytem need to be aware of nand
> > parameter(page/spare size) and ecc status(enable/disable) when work
> > at ECC nfi mode. The snfi driver in spi subsystem seems difficult
> > to
> > know these.
> > 
> > Therefore, consider two ways to let snfi can get these information.
> > The RFC patch send to review whether they are suitable and which
> > solution maybe better.
> > 
> 
> I've looked at both versions that you provided and I thought about a
> number of things that cannot be done like this:
> - I believe the snfi is a regular SPI controller. I will let Mark
>   confirm but I do not think we want to start writing SPI-NAND
>   controllers. Instead we write SPI controllers and we provide SPI-
> mem
>   operations (we've explained this in a previous ELC, the video is
>   available on YouTube).

The snfi controller can support multiple SPI protocols, which can
support other SPI device in theory. However, the snfi need to know nand
parameter and ecc status(enable/disable) when work with MTK on-host HW
BCH ECC engine for nand flash.

Therefore, the RFC patch v1/v2 is try the way to get these information.

> - You cannot add an MTK ECC algorithm. This is dedicated for sofware
>   solutions only and as far as I understand your engine uses the BCH
>   algorithm.
> - When the ECC engine is pipelined, there is an additional complexity
>   in interfacing it with a SPI controller (that's your case I
> believe).
>   I have an example tintendhat is not yet upstream but I think worth
> looking
>   at that I will send very soon (I will Cc: you on it).

Thanks for your patch.

MTK HW ECC(bch) algorithm can work in pipelined and external.
However, the performance worse when work at ecternal which realize and
verify in local. Therefore, try the pipelined in RFC patch v1/v2.

And also realize the mtd/nand info can be get in spi driver which the
spi-mxic driver in your patch. This may solve most of difficulty that
encountered in the snfi driver that the RFC patch v1/v2 try to resolve.

I will prepare the RFC patch v3 with correct pipelined ecc engine
realization for your review.

Thanks
Xiangsheng Hou
Miquel Raynal Oct. 11, 2021, 2:31 p.m. UTC | #3
Hello,

xiangsheng.hou@mediatek.com wrote on Mon, 11 Oct 2021 19:31:28 +0800:

> Hi Miquel,
> 
> On Fri, 2021-10-08 at 11:20 +0200, Miquel Raynal wrote:
> > Hello,
> > 
> > xiangsheng.hou@mediatek.com wrote on Mon, 27 Sep 2021 13:36:25 +0800:
> >   
> > > Add a driver for Mediatek SPI Nand controller
> > > 
> > > Mediatek SPI Nand controller cosists of two parts: on-host HW ECC
> > > and
> > > snfi(stand for spi nand flash interface). They can cowork with high
> > > performance which called ECC nfi mode. The nfi stand for nand flash
> > > interfacei(snfi a one part of nfi) which can support SPI Nand flash
> > > and raw nand flash.
> > > 
> > > However, the snfi driver in spi subsytem need to be aware of nand
> > > parameter(page/spare size) and ecc status(enable/disable) when work
> > > at ECC nfi mode. The snfi driver in spi subsystem seems difficult
> > > to
> > > know these.
> > > 
> > > Therefore, consider two ways to let snfi can get these information.
> > > The RFC patch send to review whether they are suitable and which
> > > solution maybe better.
> > >   
> > 
> > I've looked at both versions that you provided and I thought about a
> > number of things that cannot be done like this:
> > - I believe the snfi is a regular SPI controller. I will let Mark
> >   confirm but I do not think we want to start writing SPI-NAND
> >   controllers. Instead we write SPI controllers and we provide SPI-
> > mem
> >   operations (we've explained this in a previous ELC, the video is
> >   available on YouTube).  
> 
> The snfi controller can support multiple SPI protocols, which can
> support other SPI device in theory. However, the snfi need to know nand
> parameter and ecc status(enable/disable) when work with MTK on-host HW
> BCH ECC engine for nand flash.
> 
> Therefore, the RFC patch v1/v2 is try the way to get these information.
> 
> > - You cannot add an MTK ECC algorithm. This is dedicated for sofware
> >   solutions only and as far as I understand your engine uses the BCH
> >   algorithm.
> > - When the ECC engine is pipelined, there is an additional complexity
> >   in interfacing it with a SPI controller (that's your case I
> > believe).
> >   I have an example tintendhat is not yet upstream but I think worth
> > looking
> >   at that I will send very soon (I will Cc: you on it).  
> 
> Thanks for your patch.
> 
> MTK HW ECC(bch) algorithm can work in pipelined and external.
> However, the performance worse when work at ecternal which realize and
> verify in local. Therefore, try the pipelined in RFC patch v1/v2.

Yes this is completely expected as the data must be moved twice instead
of once. I believe pipelined mode is harder to implement in software but
brings better performances.

> And also realize the mtd/nand info can be get in spi driver which the
> spi-mxic driver in your patch. This may solve most of difficulty that
> encountered in the snfi driver that the RFC patch v1/v2 try to resolve.

Great, that was indeed the goal.

> I will prepare the RFC patch v3 with correct pipelined ecc engine
> realization for your review.

Thanks,
Miquèl