qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [Qemu-devel] [PATCH v4 03/14] nand: Switch to byte-base


From: Kevin Wolf
Subject: Re: [Qemu-block] [Qemu-devel] [PATCH v4 03/14] nand: Switch to byte-based block access
Date: Tue, 3 May 2016 09:45:56 +0200
User-agent: Mutt/1.5.21 (2010-09-15)

Am 02.05.2016 um 23:09 hat Eric Blake geschrieben:
> On 05/02/2016 09:35 AM, Kevin Wolf wrote:
> > Am 29.04.2016 um 22:08 hat Eric Blake geschrieben:
> >> Sector-based blk_write() should die; switch to byte-based
> >> blk_pwrite() instead.  Likewise for blk_read().
> >>
> >> This file is doing some complex computations to map various
> >> flash page sizes (256, 512, and 2048) atop generic uses of
> >> 512-byte sector operations.  Perhaps someone will want to tidy
> >> up the file for fewer gymnastics in managing addresses and
> >> offsets, and less wasteful visits of 256-byte pages, but it
> >> was out of scope for this series, where I just went with the
> >> mechanical conversion.
> >>
> >> Signed-off-by: Eric Blake <address@hidden>
> >> ---
> >>  hw/block/nand.c | 36 +++++++++++++++++++++++-------------
> >>  1 file changed, 23 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/hw/block/nand.c b/hw/block/nand.c
> >> index 29c6596..2703ff4 100644
> >> --- a/hw/block/nand.c
> 
> >>      } else {
> >>          off = PAGE_START(s->addr) + (s->addr & PAGE_MASK) + s->offset;
> >>          sector = off >> 9;
> >>          soff = off & 0x1ff;
> >> -        if (blk_read(s->blk, sector, iobuf, PAGE_SECTORS + 2) < 0) {
> >> +        if (blk_pread(s->blk, sector << BDRV_SECTOR_BITS, iobuf,
> >> +                     (PAGE_SECTORS + 2) << BDRV_SECTOR_BITS) < 0) {
> 
> Botched indentation here, too.
> 
> >>              printf("%s: read error in sector %" PRIu64 "\n", __func__, 
> >> sector);
> >>              return;
> >>          }
> >>
> >>          mem_and(iobuf + soff, s->io, s->iolen);
> >>
> >> -        if (blk_write(s->blk, sector, iobuf, PAGE_SECTORS + 2) < 0) {
> >> +        if (blk_write(s->blk, sector << BDRV_SECTOR_BITS, iobuf,
> >> +                      (PAGE_SECTORS + 2) << BDRV_SECTOR_BITS, 0) < 0) {
> > 
> > You forgot to actually change which function is called here.
> 
> I _did_ warn that some of my patches were not compile tested (wonder why
> I forgot to warn on this one, when I did warn on 4/14).  Of course,
> patch 14/14 would have caused a compile error if I had been compiling
> this. So, what exactly do I need to start compiling these files, and
> ensure I'm not doing dead code edits?

I just did a full compile (i.e. no --target-list), that covered it. This
is the only thing I saw, though maybe I missed a warning because somehow
-Werror didn't seem to be in effect. I did the test build with clang,
maybe configure behaves different there.

Kevin

Attachment: pgpm9cSAM2nXJ.pgp
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]