On Fri, Nov 15, 2013 at 07:48:30PM +0000, Paul Zimmerman wrote:
> > From: Greg KH [mailto:[email protected]]
> > Sent: Thursday, November 14, 2013 8:33 PM
> >
> > On Thu, Nov 14, 2013 at 02:50:24PM -0800, Paul Zimmerman wrote:
> > > DWC2 driver should be in good enough shape to move out of staging
> > >
> > > Signed-off-by: Paul Zimmerman <[email protected]>
> > > ---
> > > Greg, is this the proper method for moving a driver out of staging?
> >
> > Yes, that's the way to do it.
> >
> > But, are all of the TODO entries really done? I don't want to have that
> > file in drivers/usb/dwc2/ as that doesn't make much sense, right?
>
> I didn't realize the TODO file was only for things that needed to be
> fixed before the driver can be moved from staging. In that case I
> wouldn't have added some of the items, like the Raspberry Pi stuff or
> the DT stuff.
>
> Other than those, I think the only remaining item is the first one.
> Himangi's patch addressed that one, but I'm not sure if it fixes
> all the concerns that Dan had.
>
I've added the staging mailing list.
Potential use after free:
drivers/staging/dwc2/hcd_ddma.c:1120 dwc2_complete_non_isoc_xfer_ddma() warn:
'qtd' was already freed.
It's complaining because "qtd" gets freed on some error paths in
dwc2_process_non_isoc_desc().
DWC2_PARAM_TEST() is not a very good name. Maybe:
if (OUT_OF_BOUNDS(val, 0, 1)) {
Some of the dwc2_set_param_max_packet_count() type functions could
be simplified a little if the "valid" variable were removed. We don't
care about "retval" in any of those functions so that could be removed
as well from all of them.
The NO_FS_PHY_HW_CHECKS could be be removed.
u16 dwc2_get_otg_version(struct dwc2_hsotg *hsotg)
{
return (u16)(hsotg->core_params->otg_ver == 1 ? 0x0200 : 0x0103);
^^^^^
Remove the superfluous cast.
}
Rename dwc2_check_core_status() to dwc2_controller_connected() and make it
return a bool.
Remove the ifdef in dwc2_op_state_str().
drivers/staging/dwc2/hcd.c
370 retval = dwc2_hcd_qtd_add(hsotg, qtd, (struct dwc2_qh
**)ep_handle,
371 mem_flags);
372 if (retval < 0) {
373 dev_err(hsotg->dev,
374 "DWC OTG HCD URB Enqueue failed adding QTD.
Error status %d\n",
375 retval);
376 kfree(qtd);
377 return retval;
378 }
379
380 intr_mask = readl(hsotg->regs + GINTMSK);
381 if (!(intr_mask & GINTSTS_SOF) && retval == 0) {
^^^^^^^^^^^
This is always here.
382 enum dwc2_transaction_type tr_type;
383
384 if (qtd->qh->ep_type == USB_ENDPOINT_XFER_BULK &&
385 !(qtd->urb->flags & URB_GIVEBACK_ASAP))
386 /*
387 * Do not schedule SG transactions until qtd has
388 * URB_GIVEBACK_ASAP set
389 */
390 return 0;
391
392 spin_lock_irqsave(&hsotg->lock, flags);
393 tr_type = dwc2_hcd_select_transactions(hsotg);
394 if (tr_type != DWC2_TRANSACTION_NONE)
395 dwc2_hcd_queue_transactions(hsotg, tr_type);
396 spin_unlock_irqrestore(&hsotg->lock, flags);
397 }
398
399 return retval;
Just "return 0;" here.
400 }
I'm half way through looking at the driver and that's all I've found so
far. Looks pretty good to me so far.
regards,
dan carpenter
_______________________________________________
devel mailing list
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
