[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH V2 01/10] hw/sd.c: convert wp_groups in SDState
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PATCH V2 01/10] hw/sd.c: convert wp_groups in SDState to bitfield |
Date: |
Wed, 11 Apr 2012 11:12:56 +0100 |
On 5 April 2012 16:48, Igor Mitsyanko <address@hidden> wrote:
> @@ -536,8 +541,8 @@ static void sd_function_switch(SDState *sd, uint32_t arg)
>
> static inline int sd_wp_addr(SDState *sd, uint32_t addr)
> {
> - return sd->wp_groups[addr >>
> - (HWBLOCK_SHIFT + SECTOR_SHIFT + WPGROUP_SHIFT)];
> + return test_bit(addr >> (HWBLOCK_SHIFT + SECTOR_SHIFT + WPGROUP_SHIFT),
> + sd->wp_groups);
Looking at how often the expression "addr >> (HWBLOCK_SHIFT +
SECTOR_SHIFT + WPGROUP_SHIFT)"
turns up in this file, I suspect that it would be helpful to have
a function
static uint32_t addr_to_wpnum(uint64_t addr) {
return addr >> (HWBLOCK_SHIFT + SECTOR_SHIFT + WPGROUP_SHIFT);
}
But that should be in a separate patch, and it's optional.
> }
>
> static void sd_lock_command(SDState *sd)
> @@ -560,8 +565,8 @@ static void sd_lock_command(SDState *sd)
> sd->card_status |= LOCK_UNLOCK_FAILED;
> return;
> }
> - memset(sd->wp_groups, 0, sizeof(int) * (sd->size >>
> - (HWBLOCK_SHIFT + SECTOR_SHIFT + WPGROUP_SHIFT)));
> + bitmap_zero(sd->wp_groups, BITS_TO_LONGS((sd->size >> (HWBLOCK_SHIFT
> +
> + SECTOR_SHIFT + WPGROUP_SHIFT)) + 1));
This is wrong -- bitmap_zero() takes a bit count, so you don't need
to do BITS_TO_LONGS. Also where has the + 1 come from?
Otherwise looks good.
-- PMM
[Qemu-devel] [PATCH V2 03/10] hw/sd.c: make sd_dataready() return bool, Igor Mitsyanko, 2012/04/05
[Qemu-devel] [PATCH V2 07/10] SD card: introduce "if-idx" property for SD card objects, Igor Mitsyanko, 2012/04/05
[Qemu-devel] [PATCH V2 05/10] hw/sd.c: add SD card save/load support, Igor Mitsyanko, 2012/04/05
[Qemu-devel] [PATCH V2 10/10] hw/sd.c: introduce SD card "image" property and allow SD hot-insert, Igor Mitsyanko, 2012/04/05