|
From: | Gerd Hoffmann |
Subject: | Re: [Qemu-devel] [PATCH 1/5] usb-ccid: add CCID bus |
Date: | Wed, 15 Dec 2010 13:23:11 +0100 |
User-agent: | Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.9) Gecko/20100827 Red Hat/3.1.3-1.el6 Thunderbird/3.1.3 |
On 12/12/10 19:37, Alon Levy wrote:
A CCID device is a smart card reader. It is a USB device, defined at [1]. This patch introduces the usb-ccid device that is a ccid bus. Next patches will introduce two card types to use it, a passthru card and an emulated card.
Looks good overall, just some minor nits / questions:
+struct CCIDCardState { + DeviceState qdev;
Add "uint32_t slot" here?Also adding a bus property for it would be good, even though it can't be anything but '0' right now, to prepare the interfaces for the day when we'll support more than a single card slot and thus can attach multiple cards to the ccid bus.
+static VMStateDescription ccid_vmstate = { + .name = CCID_DEV_NAME, + .version_id = 1, + .minimum_version_id = 1, + .post_load = ccid_post_load, + .pre_save = ccid_pre_save, + .fields = (VMStateField []) { + VMSTATE_STRUCT(dev, USBCCIDState, 1, usb_device_vmstate, USBDevice),
Can we please disable this for now? USB migration support doesn't exist, and when we add it chances are that it isn't compatible with what you have here.
+static struct USBDeviceInfo ccid_info = { + .product_desc = "QEMU USB CCID", + .qdev.name = CCID_DEV_NAME, + .qdev.size = sizeof(USBCCIDState), + .qdev.vmsd =&ccid_vmstate,
Best leave the vmstate structs in the code (with a comment) and just zap this line which winds it up.
thanks, Gerd
[Prev in Thread] | Current Thread | [Next in Thread] |