On 12/03/2015 17:16, Gabriel L. Somlo wrote:
> static bool fw_cfg_data_mem_valid(void *opaque, hwaddr addr,
> unsigned size, bool is_write)
> {
> +//FIXME: additional checks before we nuke fw_cfg_data_mem_write() ?
yes, "&& !is_write". Or just leave an empty fw_cfg_data_mem_write.
> return addr == 0;
> }
>
> @@ -336,7 +314,7 @@ static void fw_cfg_comb_write(void *opaque, hwaddr addr,
> {
> switch (size) {
> case 1:
> - fw_cfg_write(opaque, (uint8_t)value);
> + //FIXME: unused, fix fw_cfg_comb_mem_ops then remove case
> break;
> case 2:
> fw_cfg_select(opaque, (uint16_t)value);
> @@ -347,6 +325,7 @@ static void fw_cfg_comb_write(void *opaque, hwaddr addr,
> static bool fw_cfg_comb_valid(void *opaque, hwaddr addr,
> unsigned size, bool is_write)
> {
> +//FIXME: update checks before removing size==1 fw_cfg_comb_write() case
> return (size == 1) || (is_write && size == 2);
Same here: "size == 1 && !is_write", or just leave an empty "case" after
removing the call to fw_cfg_write.
> }
>
> @@ -358,6 +337,7 @@ static const MemoryRegionOps fw_cfg_ctl_mem_ops = {
>
> static const MemoryRegionOps fw_cfg_data_mem_ops = {
> .read = fw_cfg_data_mem_read,
> +//FIXME: nuke this after updating fw_cfg_data_mem_valid()
If you want, but it's not necessary.
Paolo
> .write = fw_cfg_data_mem_write,
> .endianness = DEVICE_BIG_ENDIAN,
> .valid = {
> @@ -458,7 +438,6 @@ static void *fw_cfg_modify_bytes_read(FWCfgState *s,
> uint16_t key,
> s->entries[arch][key].data = data;
> s->entries[arch][key].len = len;
> s->entries[arch][key].callback_opaque = NULL;
> - s->entries[arch][key].callback = NULL;
>
> return ptr;
> }
> @@ -502,23 +481,6 @@ void fw_cfg_add_i64(FWCfgState *s, uint16_t key,
> uint64_t value)
> fw_cfg_add_bytes(s, key, copy, sizeof(value));
> }
>
> -void fw_cfg_add_callback(FWCfgState *s, uint16_t key, FWCfgCallback callback,
> - void *callback_opaque, void *data, size_t len)
> -{
> - int arch = !!(key & FW_CFG_ARCH_LOCAL);
> -
> - assert(key & FW_CFG_WRITE_CHANNEL);
> -
> - key &= FW_CFG_ENTRY_MASK;
> -
> - assert(key < FW_CFG_MAX_ENTRY && len <= UINT32_MAX);
> -
> - s->entries[arch][key].data = data;
> - s->entries[arch][key].len = (uint32_t)len;
> - s->entries[arch][key].callback_opaque = callback_opaque;
> - s->entries[arch][key].callback = callback;
> -}
> -
> void fw_cfg_add_file_callback(FWCfgState *s, const char *filename,
> FWCfgReadCallback callback, void
> *callback_opaque,
> void *data, size_t len)
> diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
> index 6d8a8ac..b2e10c2 100644
> --- a/include/hw/nvram/fw_cfg.h
> +++ b/include/hw/nvram/fw_cfg.h
> @@ -69,8 +69,6 @@ void fw_cfg_add_string(FWCfgState *s, uint16_t key, const
> char *value);
> void fw_cfg_add_i16(FWCfgState *s, uint16_t key, uint16_t value);
> void fw_cfg_add_i32(FWCfgState *s, uint16_t key, uint32_t value);
> void fw_cfg_add_i64(FWCfgState *s, uint16_t key, uint64_t value);
> -void fw_cfg_add_callback(FWCfgState *s, uint16_t key, FWCfgCallback callback,
> - void *callback_opaque, void *data, size_t len);
> void fw_cfg_add_file(FWCfgState *s, const char *filename, void *data,
> size_t len);
> void fw_cfg_add_file_callback(FWCfgState *s, const char *filename,
>
>
