| Message ID | 1f8e85fd8e75170e1005473c075942cb3d4479fe.1430034929.git.baruch@tkos.co.il (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hello Baruch, On Sun, Apr 26, 2015 at 11:16:50AM +0300, Baruch Siach wrote: > diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c > index c650f0950b20..c05f5e8fef17 100644 > --- a/drivers/mtd/nand/mxc_nand.c > +++ b/drivers/mtd/nand/mxc_nand.c > @@ -840,22 +840,22 @@ static void copy_spare(struct mtd_info *mtd, bool bfrom) > for (i = 0; i < num_chunks - 1; i++) > memcpy32_fromio(d + i * oob_chunk_size, > s + i * sparebuf_size, > - oob_chunk_size); > + ALIGN(oob_chunk_size, 4)); If oob_chunk_size isn't 32-bit-aligned, d + i * oob_chunk_size isn't either for uneven i. That's not nice. I suggest to use memcpy16_fromio. Best regards Uwe
Hi Uwe, On Sun, Apr 26, 2015 at 09:52:11PM +0200, Uwe Kleine-König wrote: > On Sun, Apr 26, 2015 at 11:16:50AM +0300, Baruch Siach wrote: > > diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c > > index c650f0950b20..c05f5e8fef17 100644 > > --- a/drivers/mtd/nand/mxc_nand.c > > +++ b/drivers/mtd/nand/mxc_nand.c > > @@ -840,22 +840,22 @@ static void copy_spare(struct mtd_info *mtd, bool bfrom) > > for (i = 0; i < num_chunks - 1; i++) > > memcpy32_fromio(d + i * oob_chunk_size, > > s + i * sparebuf_size, > > - oob_chunk_size); > > + ALIGN(oob_chunk_size, 4)); > If oob_chunk_size isn't 32-bit-aligned, d + i * oob_chunk_size isn't > either for uneven i. That's not nice. I suggest to use memcpy16_fromio. memcpy32_fromio() was introduced by Sascha in 096bcc231fd2 (mtd: mxc_nand: use 32bit copy functions, 2012-05-29), replacing memcpy_fromio() to force 32bit io access. Are you sure the hypothetical memcpy16_fromio() would work? baruch
Hello Baruch, On Mon, Apr 27, 2015 at 07:46:18AM +0300, Baruch Siach wrote: > On Sun, Apr 26, 2015 at 09:52:11PM +0200, Uwe Kleine-König wrote: > > On Sun, Apr 26, 2015 at 11:16:50AM +0300, Baruch Siach wrote: > > > diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c > > > index c650f0950b20..c05f5e8fef17 100644 > > > --- a/drivers/mtd/nand/mxc_nand.c > > > +++ b/drivers/mtd/nand/mxc_nand.c > > > @@ -840,22 +840,22 @@ static void copy_spare(struct mtd_info *mtd, bool bfrom) > > > for (i = 0; i < num_chunks - 1; i++) > > > memcpy32_fromio(d + i * oob_chunk_size, > > > s + i * sparebuf_size, > > > - oob_chunk_size); > > > + ALIGN(oob_chunk_size, 4)); > > If oob_chunk_size isn't 32-bit-aligned, d + i * oob_chunk_size isn't > > either for uneven i. That's not nice. I suggest to use memcpy16_fromio. > > memcpy32_fromio() was introduced by Sascha in 096bcc231fd2 (mtd: mxc_nand: use > 32bit copy functions, 2012-05-29), replacing memcpy_fromio() to force 32bit io > access. Are you sure the hypothetical memcpy16_fromio() would work? according to the reference manual you can use 16-bit or 32-bit accesses. And giving that in some cases the amount of valid data (in bytes) is ? 2 (mod 4) 16-bit access seem to be the obvious thing. Also Sascha only wrote about byte accesses being problematic in 096bcc231fd2. So I'm expecting that 16-bit operators should be fine. Best regards Uwe
diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c index c650f0950b20..c05f5e8fef17 100644 --- a/drivers/mtd/nand/mxc_nand.c +++ b/drivers/mtd/nand/mxc_nand.c @@ -840,22 +840,22 @@ static void copy_spare(struct mtd_info *mtd, bool bfrom) for (i = 0; i < num_chunks - 1; i++) memcpy32_fromio(d + i * oob_chunk_size, s + i * sparebuf_size, - oob_chunk_size); + ALIGN(oob_chunk_size, 4)); /* the last chunk */ memcpy32_fromio(d + i * oob_chunk_size, s + i * sparebuf_size, - used_oobsize - i * oob_chunk_size); + ALIGN(used_oobsize - i * oob_chunk_size, 4)); } else { for (i = 0; i < num_chunks - 1; i++) memcpy32_toio(&s[i * sparebuf_size], &d[i * oob_chunk_size], - oob_chunk_size); + ALIGN(oob_chunk_size, 4)); /* the last chunk */ memcpy32_toio(&s[oob_chunk_size * sparebuf_size], &d[i * oob_chunk_size], - used_oobsize - i * oob_chunk_size); + ALIGN(used_oobsize - i * oob_chunk_size, 4)); } }
Copy to/from oob io area might not be aligned to 4 bytes. When 8 bit ECC is used, the buffer size is 26. Make sure we pass 4 bytes aligned buffer size to memcpy32_{to,from}io to avoid truncating the buffer. Signed-off-by: Baruch Siach <baruch@tkos.co.il> --- drivers/mtd/nand/mxc_nand.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)