Message ID | 20211126113924.310459-21-miquel.raynal@bootlin.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | External ECC engines & Macronix support | expand |
Hi Miquel, I love your patch! Perhaps something to improve: [auto build test WARNING on mtd/nand/next] [also build test WARNING on broonie-spi/for-next mtd/mtd/next mtd/mtd/fixes v5.16-rc2 next-20211126] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Miquel-Raynal/External-ECC-engines-Macronix-support/20211126-195956 base: https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next config: xtensa-buildonly-randconfig-r006-20211125 (https://download.01.org/0day-ci/archive/20211127/202111270526.T8x5Nyq1-lkp@intel.com/config) compiler: xtensa-linux-gcc (GCC) 11.2.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/0804d6ccdf15e7a65743d048d01d876a54070b6b git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Miquel-Raynal/External-ECC-engines-Macronix-support/20211126-195956 git checkout 0804d6ccdf15e7a65743d048d01d876a54070b6b # save the config file to linux build tree mkdir build_dir COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=xtensa SHELL=/bin/bash drivers/spi/ If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All warnings (new ones prefixed by >>): In file included from drivers/spi/spi-mxic.c:16: >> include/linux/mtd/nand-ecc-mxic.h:34:1: warning: no previous prototype for 'mxic_ecc_get_pipelined_engine' [-Wmissing-prototypes] 34 | mxic_ecc_get_pipelined_engine(struct platform_device *spi_pdev) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> include/linux/mtd/nand-ecc-mxic.h:39:6: warning: no previous prototype for 'mxic_ecc_put_pipelined_engine' [-Wmissing-prototypes] 39 | void mxic_ecc_put_pipelined_engine(struct nand_ecc_engine *eng) {} | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ vim +/mxic_ecc_get_pipelined_engine +34 include/linux/mtd/nand-ecc-mxic.h a19f28121341b52 Miquel Raynal 2021-11-26 32 a19f28121341b52 Miquel Raynal 2021-11-26 33 struct nand_ecc_engine * a19f28121341b52 Miquel Raynal 2021-11-26 @34 mxic_ecc_get_pipelined_engine(struct platform_device *spi_pdev) a19f28121341b52 Miquel Raynal 2021-11-26 35 { a19f28121341b52 Miquel Raynal 2021-11-26 36 return ERR_PTR(-EOPNOTSUPP); a19f28121341b52 Miquel Raynal 2021-11-26 37 } a19f28121341b52 Miquel Raynal 2021-11-26 38 a19f28121341b52 Miquel Raynal 2021-11-26 @39 void mxic_ecc_put_pipelined_engine(struct nand_ecc_engine *eng) {} a19f28121341b52 Miquel Raynal 2021-11-26 40 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi Miquel,
I love your patch! Yet something to improve:
[auto build test ERROR on mtd/nand/next]
[also build test ERROR on broonie-spi/for-next mtd/mtd/next mtd/mtd/fixes v5.16-rc2 next-20211126]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Miquel-Raynal/External-ECC-engines-Macronix-support/20211126-195956
base: https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next
config: csky-buildonly-randconfig-r004-20211126 (https://download.01.org/0day-ci/archive/20211127/202111271552.AOgIHVI2-lkp@intel.com/config)
compiler: csky-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/0804d6ccdf15e7a65743d048d01d876a54070b6b
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Miquel-Raynal/External-ECC-engines-Macronix-support/20211126-195956
git checkout 0804d6ccdf15e7a65743d048d01d876a54070b6b
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=csky SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
csky-linux-ld: drivers/spi/spi-mxic.o: in function `mxic_spi_remove':
>> spi-mxic.c:(.text+0xba4): undefined reference to `nand_ecc_unregister_on_host_hw_engine'
csky-linux-ld: drivers/spi/spi-mxic.o: in function `mxic_ecc_put_pipelined_engine':
spi-mxic.c:(.text+0xbc8): undefined reference to `nand_ecc_unregister_on_host_hw_engine'
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff --git a/drivers/spi/spi-mxic.c b/drivers/spi/spi-mxic.c index 485a7f2afb44..a180165281e5 100644 --- a/drivers/spi/spi-mxic.c +++ b/drivers/spi/spi-mxic.c @@ -12,6 +12,8 @@ #include <linux/io.h> #include <linux/iopoll.h> #include <linux/module.h> +#include <linux/mtd/nand.h> +#include <linux/mtd/nand-ecc-mxic.h> #include <linux/platform_device.h> #include <linux/pm_runtime.h> #include <linux/spi/spi.h> @@ -167,6 +169,7 @@ #define HW_TEST(x) (0xe0 + ((x) * 4)) struct mxic_spi { + struct device *dev; struct clk *ps_clk; struct clk *send_clk; struct clk *send_dly_clk; @@ -177,6 +180,12 @@ struct mxic_spi { dma_addr_t dma; size_t size; } linear; + + struct { + bool use_pipelined_conf; + struct nand_ecc_engine *pipelined_engine; + void *ctx; + } ecc; }; static int mxic_spi_clk_enable(struct mxic_spi *mxic) @@ -400,7 +409,15 @@ static ssize_t mxic_spi_mem_dirmap_read(struct spi_mem_dirmap_desc *desc, LMODE_EN, mxic->regs + LRD_CTRL); - memcpy_fromio(buf, mxic->linear.map, len); + if (mxic->ecc.use_pipelined_conf && desc->info.op_tmpl.ecc_en) { + ret = mxic_ecc_process_data_pipelined(mxic->ecc.pipelined_engine, + NAND_PAGE_READ, + mxic->linear.dma + offs); + if (ret) + return ret; + } else { + memcpy_fromio(buf, mxic->linear.map, len); + } writel(INT_LRD_DIS, mxic->regs + INT_STS); writel(0, mxic->regs + LRD_CTRL); @@ -436,7 +453,15 @@ static ssize_t mxic_spi_mem_dirmap_write(struct spi_mem_dirmap_desc *desc, LMODE_EN, mxic->regs + LWR_CTRL); - memcpy_toio(mxic->linear.map, buf, len); + if (mxic->ecc.use_pipelined_conf && desc->info.op_tmpl.ecc_en) { + ret = mxic_ecc_process_data_pipelined(mxic->ecc.pipelined_engine, + NAND_PAGE_WRITE, + mxic->linear.dma + offs); + if (ret) + return ret; + } else { + memcpy_toio(mxic->linear.map, buf, len); + } writel(INT_LWR_DIS, mxic->regs + INT_STS); writel(0, mxic->regs + LWR_CTRL); @@ -615,6 +640,80 @@ static int mxic_spi_transfer_one(struct spi_master *master, return 0; } +/* ECC wrapper */ +static int mxic_spi_mem_ecc_init_ctx(struct nand_device *nand) +{ + struct nand_ecc_engine_ops *ops = mxic_ecc_get_pipelined_ops(); + struct mxic_spi *mxic = nand->ecc.engine->priv; + + mxic->ecc.use_pipelined_conf = true; + + return ops->init_ctx(nand); +} + +static void mxic_spi_mem_ecc_cleanup_ctx(struct nand_device *nand) +{ + struct nand_ecc_engine_ops *ops = mxic_ecc_get_pipelined_ops(); + struct mxic_spi *mxic = nand->ecc.engine->priv; + + mxic->ecc.use_pipelined_conf = false; + + ops->cleanup_ctx(nand); +} + +static int mxic_spi_mem_ecc_prepare_io_req(struct nand_device *nand, + struct nand_page_io_req *req) +{ + struct nand_ecc_engine_ops *ops = mxic_ecc_get_pipelined_ops(); + + return ops->prepare_io_req(nand, req); +} + +static int mxic_spi_mem_ecc_finish_io_req(struct nand_device *nand, + struct nand_page_io_req *req) +{ + struct nand_ecc_engine_ops *ops = mxic_ecc_get_pipelined_ops(); + + return ops->finish_io_req(nand, req); +} + +static struct nand_ecc_engine_ops mxic_spi_mem_ecc_engine_pipelined_ops = { + .init_ctx = mxic_spi_mem_ecc_init_ctx, + .cleanup_ctx = mxic_spi_mem_ecc_cleanup_ctx, + .prepare_io_req = mxic_spi_mem_ecc_prepare_io_req, + .finish_io_req = mxic_spi_mem_ecc_finish_io_req, +}; + +static void mxic_spi_mem_ecc_remove(struct mxic_spi *mxic) +{ + if (mxic->ecc.pipelined_engine) { + mxic_ecc_put_pipelined_engine(mxic->ecc.pipelined_engine); + nand_ecc_unregister_on_host_hw_engine(mxic->ecc.pipelined_engine); + } +} + +static int mxic_spi_mem_ecc_probe(struct platform_device *pdev, + struct mxic_spi *mxic) +{ + struct nand_ecc_engine *eng; + + if (!mxic_ecc_get_pipelined_ops()) + return -EOPNOTSUPP; + + eng = mxic_ecc_get_pipelined_engine(pdev); + if (IS_ERR(eng)) + return PTR_ERR(eng); + + eng->dev = &pdev->dev; + eng->integration = NAND_ECC_ENGINE_INTEGRATION_PIPELINED; + eng->ops = &mxic_spi_mem_ecc_engine_pipelined_ops; + eng->priv = mxic; + mxic->ecc.pipelined_engine = eng; + nand_ecc_register_on_host_hw_engine(eng); + + return 0; +} + static int __maybe_unused mxic_spi_runtime_suspend(struct device *dev) { struct spi_master *master = dev_get_drvdata(dev); @@ -660,6 +759,7 @@ static int mxic_spi_probe(struct platform_device *pdev) platform_set_drvdata(pdev, master); mxic = spi_master_get_devdata(master); + mxic->dev = &pdev->dev; master->dev.of_node = pdev->dev.of_node; @@ -705,6 +805,12 @@ static int mxic_spi_probe(struct platform_device *pdev) mxic_spi_hw_init(mxic); + ret = mxic_spi_mem_ecc_probe(pdev, mxic); + if (ret == -EPROBE_DEFER) { + pm_runtime_disable(&pdev->dev); + return ret; + } + ret = spi_register_master(master); if (ret) { dev_err(&pdev->dev, "spi_register_master failed\n"); @@ -717,8 +823,10 @@ static int mxic_spi_probe(struct platform_device *pdev) static int mxic_spi_remove(struct platform_device *pdev) { struct spi_master *master = platform_get_drvdata(pdev); + struct mxic_spi *mxic = spi_master_get_devdata(master); pm_runtime_disable(&pdev->dev); + mxic_spi_mem_ecc_remove(mxic); spi_unregister_master(master); return 0;
Some SPI-NAND chips do not have a proper on-die ECC engine providing error correction/detection. This is particularly an issue on embedded devices with limited resources because all the computations must happen in software, unless an external hardware engine is provided. These external engines are new and can be of two categories: external or pipelined. Macronix is providing both, the former being already supported. The second, however, is very SoC implementation dependent and must be instantiated by the SPI host controller directly. An entire subsystem has been contributed to support these engines which makes the insertion into another subsystem such as SPI quite straightforward without the need for a lot of specific functions. Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> --- drivers/spi/spi-mxic.c | 112 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 110 insertions(+), 2 deletions(-)