Re: [PATCH v5 4/8] mtd: rawnand: Add nand_extract_bits()

[Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx>]

On Tue, 19 May 2020 09:45:45 +0200
Miquel Raynal <miquel.raynal@xxxxxxxxxxx> wrote:

> There are cases where ECC bytes are not byte-aligned. Indeed, BCH
> implies using a number of ECC bits, which are not always a multiple of
> 8. We then need a helper like nand_extract_bits() to extract these
> syndromes from a buffer.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@xxxxxxxxxxx>
> ---
>  drivers/mtd/nand/raw/nand_base.c | 42 ++++++++++++++++++++++++++++++++
>  include/linux/mtd/rawnand.h      |  4 +++
>  2 files changed, 46 insertions(+)
> 
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index 65e9b2fa2fc5..14387b967e8b 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -274,6 +274,48 @@ static int check_offs_len(struct nand_chip *chip, loff_t ofs, uint64_t len)
>  	return ret;
>  }
>  
> +/** nand_extract_bits - Copy unaligned bits from one buffer to another one

It should be

/**
 * nand_extract_bits - Copy unaligned bits from one buffer to another

Once addressed you can add

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

If someone reads that and has enough energy, it'd be great to have a
generic bitcpy() function (looks like the fbdev [1] subsystem has a
private implementation too).

> + * @dst: destination buffer
> + * @dst_off: bit offset at which the writing starts
> + * @src: source buffer
> + * @src_off: bit offset at which the reading starts
> + * @nbits: number of bits to copy from @src to @dst
> + *
> + * Copy bits from one memory region to another (overlap authorized).
> + */
> +void nand_extract_bits(u8 *dst, unsigned int dst_off, const u8 *src,
> +		       unsigned int src_off, unsigned int nbits)
> +{
> +	unsigned int tmp, n;
> +
> +	dst += dst_off / 8;
> +	dst_off %= 8;
> +	src += src_off / 8;
> +	src_off %= 8;
> +
> +	while (nbits) {
> +		n = min3(8 - dst_off, 8 - src_off, nbits);
> +
> +		tmp = (*src >> src_off) & GENMASK(n - 1, 0);
> +		*dst &= ~GENMASK(n - 1 + dst_off, dst_off);
> +		*dst |= tmp << dst_off;
> +
> +		dst_off += n;
> +		if (dst_off >= 8) {
> +			dst++;
> +			dst_off -= 8;
> +		}
> +
> +		src_off += n;
> +		if (src_off >= 8) {
> +			src++;
> +			src_off -= 8;
> +		}
> +
> +		nbits -= n;
> +	}
> +}
> +
>  /**
>   * nand_select_target() - Select a NAND target (A.K.A. die)
>   * @chip: NAND chip object
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index 0f45b6984ad1..45dd57e2a223 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -1412,6 +1412,10 @@ int nand_gpio_waitrdy(struct nand_chip *chip, struct gpio_desc *gpiod,
>  void nand_select_target(struct nand_chip *chip, unsigned int cs);
>  void nand_deselect_target(struct nand_chip *chip);
>  
> +/* Bitops */
> +void nand_extract_bits(u8 *dst, unsigned int dst_off, const u8 *src,
> +		       unsigned int src_off, unsigned int nbits);
> +
>  /**
>   * nand_get_data_buf() - Get the internal page buffer
>   * @chip: NAND chip object

[1]https://elixir.bootlin.com/linux/latest/source/drivers/video/fbdev/amifb.c#L2598

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