Re: [Qemu-devel] [PATCH v23 03/11] usb-ccid: add CCID bus

[Blue Swirl]

On Mon, Mar 28, 2011 at 9:00 PM, Alon Levy <address@hidden> wrote:
> On Mon, Mar 28, 2011 at 08:44:59PM +0300, Blue Swirl wrote:
>> On Mon, Mar 28, 2011 at 5:31 PM, Jes Sorensen <address@hidden> wrote:
>> > On 03/28/11 16:28, Alon Levy wrote:
>> >> On Mon, Mar 28, 2011 at 02:01:01PM +0200, Jes Sorensen wrote:
>> >>> > On 03/23/11 14:19, 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.
>> >>>> > >
>> >>>> > >  [1] 
>> >>>> > > http://www.usb.org/developers/devclass_docs/DWG_Smart-Card_CCID_Rev110.
>> >>>> > >
>> >>>> > > Signed-off-by: Alon Levy <address@hidden>
>> >>> >
>> >>> > This looks ok to me now - I noticed that the comments in the header 
>> >>> > file
>> >>> > refers to u8/u16/u32 for types, even though the code uses
>> >>> > uint{8,16,32}_t, but I don't think that warrants a fix.
>> >> You mean the comments in hw/usb-ccid.c in the descriptor? that's just 
>> >> shorter.
>> >
>> > I wasn't sure if it was a leftover from some of the headers being used
>> > in the kernel. However I am fine with it - no need to change it, and it
>> > makes it shorter as you say.
>>
>> No, u8/u16 etc are only available for Linux. Please use standard
>> uint8_t etc. See also HACKING 2.1.
>
> They are just in the comments, like I said above. Just did a git grep to 
> verify,
> here is the output (snipped because there are many comments):
> U playa:qemu alon (usb_ccid.v24)$ git grep "\<u8\>" hw/ccid.h
> U playa:qemu alon (usb_ccid.v24)$ git grep "\<u16\>" hw/ccid.h
> U playa:qemu alon (usb_ccid.v24)$ git grep "\<u16\>" hw/usb-ccid.c
> hw/usb-ccid.c:        0x10, 0x01, /*  u16 bcdUSB; v1.1 */
> hw/usb-ccid.c:                    /*  u16 idVendor  */
> hw/usb-ccid.c:                    /*  u16 idProduct */
> hw/usb-ccid.c:                    /*  u16 bcdDevice */
> hw/usb-ccid.c:        0x5d, 0x00, /* u16 wTotalLength; 9+9+54+7+7+7 */
> hw/usb-ccid.c:        0x10, 0x01, /* u16 bcdCCID; CCID Specification Release 
> Number.
> hw/usb-ccid.c:                     * u16 wLcdLayout; XXYY Number of lines 
> (XX) and ch
> hw/usb-ccid.c:                    /* u16 ep_wMaxPacketSize; */
> hw/usb-ccid.c:        0x40, 0x00, /* u16 ep_wMaxPacketSize; */
> hw/usb-ccid.c:        0x40, 0x00, /* u16 ep_wMaxPacketSize; */
> U playa:qemu alon (usb_ccid.v24)$ git grep "\<u8\>" hw/usb-ccid.c
> hw/usb-ccid.c:        0x12,       /*  u8 bLength; */
> hw/usb-ccid.c:        USB_DT_DEVICE, /*  u8 bDescriptorType; Device */
> hw/usb-ccid.c:        0x00,       /*  u8  bDeviceClass; */
> hw/usb-ccid.c:        0x00,       /*  u8  bDeviceSubClass; */
> hw/usb-ccid.c:        0x00,       /*  u8  bDeviceProtocol; [ low/full speeds 
> only ] *
> [snip 46 more lines with comments]

Sorry for the noise.