[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/2] usb-ccid: add CCID bus
From: |
Alon Levy |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] usb-ccid: add CCID bus |
Date: |
Sat, 23 Oct 2010 17:45:52 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
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)