Re: [Qemu-devel] [PATCH v5 wave 1 2/4] fw-cfg: turn FW_CFG_FILE_SLOTS into a device property

[Eduardo Habkost]

On Wed, Jan 11, 2017 at 06:34:55PM +0100, Laszlo Ersek wrote:
[...]
>  static Property fw_cfg_io_properties[] = {
>      DEFINE_PROP_UINT32("iobase", FWCfgIoState, iobase, -1),
>      DEFINE_PROP_UINT32("dma_iobase", FWCfgIoState, dma_iobase, -1),
>      DEFINE_PROP_BOOL("dma_enabled", FWCfgIoState, parent_obj.dma_enabled,
>                       true),
> +    DEFINE_PROP_UINT16("file_slots", FWCfgIoState, parent_obj.file_slots,
> +                       FW_CFG_FILE_SLOTS_MIN),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> @@ -1027,6 +1066,13 @@ static void fw_cfg_io_realize(DeviceState *dev, Error 
> **errp)

I'm not sure what is more important here: following the QOM
property name convention using "-" instead of "_", or being
consistent with the other existing properties.

In either case, we could add a "x-" prefix to indicate it is not
supposed to be configured directly by the user.

[...]
> @@ -1063,6 +1109,8 @@ static Property fw_cfg_mem_properties[] = {
>      DEFINE_PROP_UINT32("data_width", FWCfgMemState, data_width, -1),
>      DEFINE_PROP_BOOL("dma_enabled", FWCfgMemState, parent_obj.dma_enabled,
>                       true),
> +    DEFINE_PROP_UINT16("file_slots", FWCfgMemState, parent_obj.file_slots,
> +                       FW_CFG_FILE_SLOTS_MIN),

It looks like you can add the property to the TYPE_FW_CFG parent
class instead of duplicating it on the subclasses. The existing
"dma_enabled" property could be moved there as well.

-- 
Eduardo