Re: [Qemu-devel] [PATCH 1/2] usb-ccid: add CCID bus

[Markus Armbruster <armbru@redhat.com>]

Alon Levy <alevy@redhat.com> writes:

> 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.
>
>  [1] http://www.usb.org/developers/devclass_docs/DWG_Smart-Card_CCID_Rev110.
>
> Signed-off-by: Alon Levy <alevy@redhat.com>
> ---
>  Makefile.objs |    1 +
>  configure     |   12 +
>  hw/ccid.h     |   34 ++
>  hw/usb-ccid.c | 1349 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 1396 insertions(+), 0 deletions(-)
>  create mode 100644 hw/ccid.h
>  create mode 100644 hw/usb-ccid.c
[...]
> diff --git a/hw/usb-ccid.c b/hw/usb-ccid.c
> new file mode 100644
> index 0000000..a7b4c3f
> --- /dev/null
> +++ b/hw/usb-ccid.c
> @@ -0,0 +1,1349 @@
[...]
> +struct CCIDBus {
> +    BusState qbus;
> +    USBCCIDState *ccid;

Where does ccid point to?

> +};
[...]
> +static int ccid_initfn(USBDevice *dev)
> +{
> +    USBCCIDState *s = DO_UPCAST(USBCCIDState, dev, dev);
> +
> +    s->bus = ccid_bus_new(&dev->qdev);
> +    s->card = NULL;
> +    s->cardinfo = NULL;
> +    s->bus->ccid = s;

Looks like it points back to the device providing the bus.  Why not use
bus->qbus->parent ?

> +    s->migration_state = MIGRATION_NONE;
> +    dev->auto_attach = s->auto_attach;
> +    debug = s->debug;

Wait a sec!  Each CCID device has its own property "debug" (defined
below), but they all copy to the same static debug on initialization.
In other words, the device initialized last wins.  Ugh.

> +    s->migration_target_ip = 0;
> +    s->migration_target_port = 0;
> +    s->dev.speed = USB_SPEED_FULL;
> +    s->notify_slot_change = false;
> +    s->powered = true;
> +    s->pending_answers_num = 0;
> +    s->last_answer_error = 0;
> +    s->bulk_in_pending_start = 0;
> +    s->bulk_in_pending_end = 0;
> +    s->current_bulk_in = NULL;
> +    ccid_reset_error_status(s);
> +    s->bulk_out_pos = 0;
> +    ccid_reset_parameters(s);
> +    ccid_reset(s);
> +    return 0;
> +}
[...]
> +static struct USBDeviceInfo ccid_info = {
> +    .product_desc   = "QEMU USB CCID",
> +    .qdev.name      = CCID_DEV_NAME,
> +    .qdev.size      = sizeof(USBCCIDState),
> +    .qdev.vmsd      = &ccid_vmstate,
> +    .init           = ccid_initfn,
> +    .handle_packet  = usb_generic_handle_packet,
> +    .handle_reset   = ccid_handle_reset,
> +    .handle_control = ccid_handle_control,
> +    .handle_data    = ccid_handle_data,
> +    .handle_destroy = ccid_handle_destroy,
> +    .usbdevice_name = "ccid",
> +    .qdev.props     = (Property[]) {
> +        DEFINE_PROP_UINT8("auto_attach", USBCCIDState, auto_attach, 0),
> +        DEFINE_PROP_UINT8("debug", USBCCIDState, debug, 0),
> +        DEFINE_PROP_END_OF_LIST(),
> +    },
> +};
> +
> +
> +static void ccid_register_devices(void)
> +{
> +    usb_qdev_register(&ccid_info);
> +}
> +device_init(ccid_register_devices)