Re: [PATCH v1] spi: spi-mtk-nor: Unify write buffer on/off

[Bayi Cheng (程八意) <bayi.cheng@xxxxxxxxxxxx>]

On Tue, 2022-11-15 at 20:46 +0800, Bayi Cheng wrote:
> From: bayi cheng <bayi.cheng@xxxxxxxxxxxx>
> 
> The logical structures of mtk_nor_write_buffer_enable and
> mtk_nor_write_buffer_disable are very similar, So it is necessary to
> combine them into one.
> 
> Signed-off-by: bayi cheng <bayi.cheng@xxxxxxxxxxxx>
> ---
> Change in v1:
>   -Delete mtk_nor_write_buffer_enable.
>   -Delete mtk_nor_write_buffer_disable.
>   -Add mtk_nor_setup_write_buffer.
> ---
> ---
>  drivers/spi/spi-mtk-nor.c | 40 ++++++++++++++++---------------------
> --
>  1 file changed, 16 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/spi/spi-mtk-nor.c b/drivers/spi/spi-mtk-nor.c
> index d167699a1a96..e8b355f5be56 100644
> --- a/drivers/spi/spi-mtk-nor.c
> +++ b/drivers/spi/spi-mtk-nor.c
> @@ -443,36 +443,28 @@ static int mtk_nor_read_pio(struct mtk_nor *sp,
> const struct spi_mem_op *op)
>  	return ret;
>  }
>  
> -static int mtk_nor_write_buffer_enable(struct mtk_nor *sp)
> +static int mtk_nor_setup_write_buffer(struct mtk_nor *sp, bool on)
>  {
>  	int ret;
>  	u32 val;
>  
> -	if (sp->wbuf_en)
> +	if (!(sp->wbuf_en ^ on))
>  		return 0;
>  
>  	val = readl(sp->base + MTK_NOR_REG_CFG2);
> -	writel(val | MTK_NOR_WR_BUF_EN, sp->base + MTK_NOR_REG_CFG2);
> -	ret = readl_poll_timeout(sp->base + MTK_NOR_REG_CFG2, val,
> -				 val & MTK_NOR_WR_BUF_EN, 0, 10000);
> -	if (!ret)
> -		sp->wbuf_en = true;
> -	return ret;
> -}
> -
> -static int mtk_nor_write_buffer_disable(struct mtk_nor *sp)
> -{
> -	int ret;
> -	u32 val;
> +	if (on) {
> +		writel(val | MTK_NOR_WR_BUF_EN, sp->base +
> MTK_NOR_REG_CFG2);
> +		ret = readl_poll_timeout(sp->base + MTK_NOR_REG_CFG2,
> val,
> +					 val & MTK_NOR_WR_BUF_EN, 0,
> 10000);
> +	} else {
> +		writel(val & ~MTK_NOR_WR_BUF_EN, sp->base +
> MTK_NOR_REG_CFG2);
> +		ret = readl_poll_timeout(sp->base + MTK_NOR_REG_CFG2,
> val,
> +					 !(val & MTK_NOR_WR_BUF_EN), 0,
> 10000);
> +	}
>  
> -	if (!sp->wbuf_en)
> -		return 0;
> -	val = readl(sp->base + MTK_NOR_REG_CFG2);
> -	writel(val & ~MTK_NOR_WR_BUF_EN, sp->base + MTK_NOR_REG_CFG2);
> -	ret = readl_poll_timeout(sp->base + MTK_NOR_REG_CFG2, val,
> -				 !(val & MTK_NOR_WR_BUF_EN), 0, 10000);
>  	if (!ret)
> -		sp->wbuf_en = false;
> +		sp->wbuf_en = on;
> +
>  	return ret;
>  }
>  
> @@ -482,7 +474,7 @@ static int mtk_nor_pp_buffered(struct mtk_nor
> *sp, const struct spi_mem_op *op)
>  	u32 val;
>  	int ret, i;
>  
> -	ret = mtk_nor_write_buffer_enable(sp);
> +	ret = mtk_nor_setup_write_buffer(sp, true);
>  	if (ret < 0)
>  		return ret;
>  
> @@ -501,7 +493,7 @@ static int mtk_nor_pp_unbuffered(struct mtk_nor
> *sp,
>  	const u8 *buf = op->data.buf.out;
>  	int ret;
>  
> -	ret = mtk_nor_write_buffer_disable(sp);
> +	ret = mtk_nor_setup_write_buffer(sp, false);
>  	if (ret < 0)
>  		return ret;
>  	writeb(buf[0], sp->base + MTK_NOR_REG_WDATA);
> @@ -608,7 +600,7 @@ static int mtk_nor_exec_op(struct spi_mem *mem,
> const struct spi_mem_op *op)
>  	}
>  
>  	if ((op->data.dir == SPI_MEM_DATA_IN) &&
> mtk_nor_match_read(op)) {
> -		ret = mtk_nor_write_buffer_disable(sp);
> +		ret = mtk_nor_setup_write_buffer(sp, false);
>  		if (ret < 0)
>  			return ret;
>  		mtk_nor_setup_bus(sp, op);

Hi Angelo, Hi David, Hi Matthias,

Just a gentle ping on this.
Could you please review this patch and give us some suggestion?

Thanks,
Bayi