Re: [PATCH 05/10] mtd: rawnand: Rename the use_bufpoi variables

[Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx>]

On Fri, 24 Apr 2020 19:36:26 +0200
Miquel Raynal <miquel.raynal@xxxxxxxxxxx> wrote:

> Both in nand_do_read_ops() and nand_do_write_ops() there is a boolean
> called use_bufpoi which is set to true in case of unaligned request or
> when there is a need for a DMA-able buffer. It basically means "use a
> bounce buffer".
> 
> Depending on the value of use_bufpoi, the bufpoi variable is always
> used and will either point to the original buffer or to the nand_chip
> structure "internal data buffer" (this buffer is allocated with
> kmalloc() on purpose so that it will be DMA-compliant).
> 
> In all cases bufpoi is used so the boolean name is misleading. Rename
> use_bufpoi to be use_bouce_buf to be more accurate.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@xxxxxxxxxxx>

Reviewed-by: Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx>

I wonder if we shouldn't find a better name for bufpoi too. Not sure
what the poi means here (pointer?). So maybe just rename those into
read_buf, write_buf (since buf seems to be declared already).

> ---
>  drivers/mtd/nand/raw/nand_base.c | 34 ++++++++++++++++----------------
>  1 file changed, 17 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index 4d8a4a20df63..0e2dd4c1b44c 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -3166,7 +3166,7 @@ static int nand_do_read_ops(struct nand_chip *chip, loff_t from,
>  	uint32_t max_oobsize = mtd_oobavail(mtd, ops);
>  
>  	uint8_t *bufpoi, *oob, *buf;
> -	int use_bufpoi;
> +	int use_bounce_buf;
>  	unsigned int max_bitflips = 0;
>  	int retry_mode = 0;
>  	bool ecc_fail = false;
> @@ -3190,19 +3190,19 @@ static int nand_do_read_ops(struct nand_chip *chip, loff_t from,
>  		aligned = (bytes == mtd->writesize);
>  
>  		if (!aligned)
> -			use_bufpoi = 1;
> +			use_bounce_buf = 1;
>  		else if (chip->options & NAND_USE_DMA_BUFFER)
> -			use_bufpoi = !virt_addr_valid(buf) ||
> -				     !IS_ALIGNED((unsigned long)buf,
> -						 chip->buf_align);
> +			use_bounce_buf = !virt_addr_valid(buf) ||
> +					 !IS_ALIGNED((unsigned long)buf,
> +						     chip->buf_align);
>  		else
> -			use_bufpoi = 0;
> +			use_bounce_buf = 0;
>  
>  		/* Is the current page in the buffer? */
>  		if (realpage != chip->pagecache.page || oob) {
> -			bufpoi = use_bufpoi ? chip->data_buf : buf;
> +			bufpoi = use_bounce_buf ? chip->data_buf : buf;
>  
> -			if (use_bufpoi && aligned)
> +			if (use_bounce_buf && aligned)
>  				pr_debug("%s: using read bounce buffer for buf@%p\n",
>  						 __func__, buf);
>  
> @@ -3223,7 +3223,7 @@ static int nand_do_read_ops(struct nand_chip *chip, loff_t from,
>  				ret = chip->ecc.read_page(chip, bufpoi,
>  							  oob_required, page);
>  			if (ret < 0) {
> -				if (use_bufpoi)
> +				if (use_bounce_buf)
>  					/* Invalidate page cache */
>  					chip->pagecache.page = -1;
>  				break;
> @@ -3233,7 +3233,7 @@ static int nand_do_read_ops(struct nand_chip *chip, loff_t from,
>  			 * Copy back the data in the initial buffer when reading
>  			 * partial pages or when a bounce buffer is required.
>  			 */
> -			if (use_bufpoi) {
> +			if (use_bounce_buf) {
>  				if (!NAND_HAS_SUBPAGE_READ(chip) && !oob &&
>  				    !(mtd->ecc_stats.failed - ecc_failures) &&
>  				    (ops->mode != MTD_OPS_RAW)) {
> @@ -4015,23 +4015,23 @@ static int nand_do_write_ops(struct nand_chip *chip, loff_t to,
>  	while (1) {
>  		int bytes = mtd->writesize;
>  		uint8_t *wbuf = buf;
> -		int use_bufpoi;
> +		int use_bounce_buf;
>  		int part_pagewr = (column || writelen < mtd->writesize);
>  
>  		if (part_pagewr)
> -			use_bufpoi = 1;
> +			use_bounce_buf = 1;
>  		else if (chip->options & NAND_USE_DMA_BUFFER)
> -			use_bufpoi = !virt_addr_valid(buf) ||
> -				     !IS_ALIGNED((unsigned long)buf,
> -						 chip->buf_align);
> +			use_bounce_buf = !virt_addr_valid(buf) ||
> +					 !IS_ALIGNED((unsigned long)buf,
> +						     chip->buf_align);
>  		else
> -			use_bufpoi = 0;
> +			use_bounce_buf = 0;
>  
>  		/*
>  		 * Copy the data from the initial buffer when doing partial page
>  		 * writes or when a bounce buffer is required.
>  		 */
> -		if (use_bufpoi) {
> +		if (use_bounce_buf) {
>  			pr_debug("%s: using write bounce buffer for buf@%p\n",
>  					 __func__, buf);
>  			if (part_pagewr)


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/