On Tue, May 29, 2018 at 9:50 PM, Marcus Folkesson
<marcus.folkesson@xxxxxxxxx> wrote:
> Chip Card Interface Device (CCID) protocol is a USB protocol that
> allows a smartcard device to be connected to a computer via a card
> reader using a standard USB interface, without the need for each manufacturer
> of smartcards to provide its own reader or protocol.
>
> This gadget driver makes Linux show up as a CCID device to the host and let a
> userspace daemon act as the smartcard.
>
> This is useful when the Linux gadget itself should act as a cryptographic
> device or forward APDUs to an embedded smartcard device.
> + * Copyright (C) 2018 Marcus Folkesson <marcus.folkesson@xxxxxxxxx>
> + *
Redundant line
> +static DEFINE_IDA(ccidg_ida);
Where is it destroyed?
> + ccidg_class = NULL;
> + return PTR_ERR(ccidg_class);
Are you sure?
> + if (!list_empty(head)) {
> + req = list_first_entry(head, struct usb_request, list);
list_first_entry_or_null()
> + req->length = len;
Perhaps assign this obly if malloc successedeed ?
> + req->buf = kmalloc(len, GFP_ATOMIC);
> + if (req->buf == NULL) {
if (!req->buf) ?
> + usb_ep_free_request(ep, req);
> + return ERR_PTR(-ENOMEM);
> + }
> +static void ccidg_request_free(struct usb_request *req, struct usb_ep *ep)
> +{
> + if (req) {
Is it even possible?
What about
if (!req)
return;
?
> + kfree(req->buf);
> + usb_ep_free_request(ep, req);
> + }
> +}
> + *(__le32 *) req->buf = ccid_class_desc.dwDefaultClock;
Hmm... put_unaligned()? cpu_to_le32()? cpu_to_le32p()?
> + *(__le32 *) req->buf = ccid_class_desc.dwDataRate;
Ditto.
> + }
> + }
Indentation.
> + /* responded with data transfer or status phase? */
> + if (ret >= 0) {
Why not
if (ret < 0)
return ret;
?
> + }
> +
> + return ret;
> +}
> + atomic_set(&ccidg->online, 1);
> + return ret;
return 0; ?
> + struct f_ccidg *ccidg;
> + ccidg = container_of(inode->i_cdev, struct f_ccidg, cdev);
One line ?
> + xfer = (req->actual < count) ? req->actual : count;
min_t()
> + ret = wait_event_interruptible(bulk_dev->write_wq,
> + ((req = ccidg_req_get(ccidg, &bulk_dev->tx_idle))));
> +
> + if (ret < 0)
> + return -ERESTARTSYS;
Redundant blank line above.
> +static void ccidg_function_free(struct usb_function *f)
> +{
> + struct f_ccidg_opts *opts;
> + opts = container_of(f->fi, struct f_ccidg_opts, func_inst);
One line.
> + mutex_lock(&opts->lock);
> + --opts->refcnt;
-- will work
> + mutex_unlock(&opts->lock);
> +}
> + struct f_ccidg_opts *opts;
> + opts = container_of(fi, struct f_ccidg_opts, func_inst);
Perhaps one line ?
> + ++opts->refcnt;
X++ would work as well.
> + struct f_ccidg_opts *opts;
> +
> + opts = container_of(f, struct f_ccidg_opts, func_inst);
Perhaps one line?
> +#define CCID_PINSUPOORT_NONE 0x00
(0 << 0)
?
for sake of consistency
> +#define CCID_PINSUPOORT_VERIFICATION (1 << 1)
> +#define CCID_PINSUPOORT_MODIFICATION (1 << 2)
--
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
