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

[Alon Levy]

On Fri, Oct 22, 2010 at 03:49:38PM +0200, Markus Armbruster wrote:
> Alon Levy <address@hidden> 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 <address@hidden>
> > ---
> >  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 ?
> 

Thanks, will fix.

> > +    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.
> 

Thanks again. I'll change the DPRINTF to use the device instance (I think
I have it wherever I use DPRINTF), that will eliminate the static.

> > +    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)