[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-trivial] [Qemu-devel] [PATCH v1 1/5] block/nand: Factor out co
From: |
Peter Crosthwaite |
Subject: |
Re: [Qemu-trivial] [Qemu-devel] [PATCH v1 1/5] block/nand: Factor out common code |
Date: |
Thu, 1 Aug 2013 09:49:54 +1000 |
On Wed, Jun 19, 2013 at 6:09 PM, Michael Tokarev <address@hidden> wrote:
> 18.06.2013 15:08, address@hidden wrote:
>> From: Peter Crosthwaite <address@hidden>
>>
>> Most of this computation of s->iolen is the same for both the if and
>> else paths here. Factor out the common parts outside the if.
>>
>> Cc: address@hidden
>>
>> Signed-off-by: Peter Crosthwaite <address@hidden>
>> ---
>>
>> hw/block/nand.c | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/block/nand.c b/hw/block/nand.c
>> index 087ca14..6309f93 100644
>> --- a/hw/block/nand.c
>> +++ b/hw/block/nand.c
>> @@ -272,10 +272,10 @@ static void nand_command(NANDFlashState *s)
>> break;
>> offset = s->addr & ((1 << s->addr_shift) - 1);
>> s->blk_load(s, s->addr, offset);
>> - if (s->gnd)
>> - s->iolen = (1 << s->page_shift) - offset;
>> - else
>> - s->iolen = (1 << s->page_shift) + (1 << s->oob_shift) - offset;
>> + s->iolen = (1 << s->page_shift) - offset;
>> + if (!s->gnd) {
>> + s->iolen += 1 << s->oob_shift;
>> + }
>
> Hm. Can s->iolen become negative here?
>
> addr_shift can be either less or greather than page_shift. When addr_shift
> is larger than page_shift (addr_shift=16, page_shift=11), offset may be up
> to 65535, in which case s->iolen may be -63487, even without additional
> oob_shift.
>
> I dunno if this is a concern, just.. asking.
>
> Besides, exactly the same expression is used in nand_getio() down this file,
> maybe a common function is in order? :)
>
Factored this out. Just added it to nand_blk_load_XXX().
Regards,
Peter
> And looking at this file, I think it deserves some good type changes. For
> example, all these _shift are unsigned, and most are actually used as masks,
> in form (1<<shift), instead of directly (except of erase_shift).
>
> Thanks,
>
> /mjt
>
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-trivial] [Qemu-devel] [PATCH v1 1/5] block/nand: Factor out common code,
Peter Crosthwaite <=