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

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi Boris,

Boris Brezillon <boris.brezillon@collabora.com> wrote on Tue, 19 May
2020 10:48:27 +0200:

> On Tue, 19 May 2020 09:45:45 +0200
> Miquel Raynal <miquel.raynal@bootlin.com> 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@bootlin.com>
> > ---
> >  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@collabora.com>

Oh yeah, right.

I'll fix it when applying.

> 
> 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).

Absolutely!

> 
> > + * @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

Thanks,
Miquèl

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