Re: [RFC,v4,3/5] spi: mtk: Add mediatek SPI Nand Flash interface driver

[xiangsheng.hou <xiangsheng.hou@mediatek.com>]

Hi Miquel,

On Thu, 2021-12-09 at 11:20 +0100, Miquel Raynal wrote:
> Hi Xiangsheng,
> 
> xiangsheng.hou@mediatek.com wrote on Tue, 30 Nov 2021 16:32:00 +0800:
> 
> > 
> > +
> > +static int mtk_snfi_config(struct nand_device *nand,
> > +			   struct mtk_snfi *snfi)
> > +{
> > +	struct mtk_ecc_engine *eng = mtk_snfi_to_ecc_engine(snfi);
> > +	u32 val;
> > +
> > +	switch (nanddev_page_size(nand)) {
> > +	case 512:
> > +		val = PAGEFMT_512_2K | PAGEFMT_SEC_SEL_512;
> > +		break;
> > +	case KB(2):
> > +		if (eng->section_size == 512)
> > +			val = PAGEFMT_2K_4K | PAGEFMT_SEC_SEL_512;
> > +		else
> > +			val = PAGEFMT_512_2K;
> > +		break;
> > +	case KB(4):
> > +		if (eng->section_size == 512)
> > +			val = PAGEFMT_4K_8K | PAGEFMT_SEC_SEL_512;
> > +		else
> > +			val = PAGEFMT_2K_4K;
> > +		break;
> > +	case KB(8):
> > +		if (eng->section_size == 512)
> > +			val = PAGEFMT_8K_16K | PAGEFMT_SEC_SEL_512;
> > +		else
> > +			val = PAGEFMT_4K_8K;
> > +		break;
> > +	case KB(16):
> > +		val = PAGEFMT_8K_16K;
> > +		break;
> > +	default:
> > +		dev_err(snfi->dev, "invalid page len: %d\n",
> > +			nanddev_page_size(nand));
> > +		return -EINVAL;
> > +	}
> > +
> > +	val |= eng->oob_per_section_idx << PAGEFMT_SPARE_SHIFT;
> > +	val |= eng->oob_free << PAGEFMT_FDM_SHIFT;
> > +	val |= eng->oob_free_protected << PAGEFMT_FDM_ECC_SHIFT;
> > +	writel(val, snfi->regs + NFI_PAGEFMT);
> 
> Shouldn't this be calculated only once?

Yes, The mtk_snfi_config function can be only called in prepare_io_req
at the first time. I will add a variable to indicate config done or not
to avoid calculate repeatedly.

> 
> > +
> > +	return 0;
> > +}
> > +
> > +static int mtk_snfi_ecc_init_ctx(struct nand_device *nand)
> > +{
> > +	struct nand_ecc_engine_ops *ops = mtk_ecc_get_pipelined_ops();
> > +
> > +	return ops->init_ctx(nand);
> > +}
> > +
> > +static void mtk_snfi_ecc_cleanup_ctx(struct nand_device *nand)
> > +{
> > +	struct nand_ecc_engine_ops *ops = mtk_ecc_get_pipelined_ops();
> > +
> > +	ops->cleanup_ctx(nand);
> > +}
> > +
> > +static int mtk_snfi_ecc_prepare_io_req(struct nand_device *nand,
> > +				       struct nand_page_io_req *req)
> > +{
> > +	struct nand_ecc_engine_ops *ops = mtk_ecc_get_pipelined_ops();
> > +	struct mtk_snfi *snfi = mtk_nand_to_spi(nand);
> > +	int ret;
> > +
> > +	ret = mtk_snfi_config(nand, snfi);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return ops->prepare_io_req(nand, req);
> > +}
> > +
> > +static int mtk_snfi_ecc_finish_io_req(struct nand_device *nand,
> > +				      struct nand_page_io_req *req)
> > +{
> > +	struct nand_ecc_engine_ops *ops = mtk_ecc_get_pipelined_ops();
> > +	struct mtk_ecc_engine *eng = nand_to_ecc_ctx(nand);
> > +	struct mtk_snfi *snfi = mtk_nand_to_spi(nand);
> > +
> > +	if (req->mode != MTD_OPS_RAW)
> > +		eng->read_empty = readl(snfi->regs + NFI_STA) &
> > STA_EMP_PAGE;
> > +
> > +	return ops->finish_io_req(nand, req);
> > +}
> > +
> > +
> > +MODULE_LICENSE("GPL v2");
> > +MODULE_AUTHOR("Xiangsheng Hou <xiangsheng.hou@mediatek.com>");
> > +MODULE_DESCRIPTION("Mediatek SPI Nand Flash interface driver");
> 
> Otherwise looks good, I believe you can drop the RFC prefix now.
> 

I will prepare the formal patch and send for review after internal
review and test.

Thanks
Xiangsheng Hou
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek