linux-kernel - Re: [PATCH 3/3] USB: driver for Freescale QUICC Engine USB Host Controller

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 23 Sep 2008 22:31:29 +0400
From:	Anton Vorontsov <avorontsov@...mvista.com>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	dbrownell@...rs.sourceforge.net, greg@...ah.com,
	galak@...nel.crashing.org, timur@...escale.com,
	leoli@...escale.com, laurentp@...-semaphore.com,
	linuxppc-dev@...abs.org, linux-usb@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] USB: driver for Freescale QUICC Engine USB Host
	Controller

On Fri, Sep 19, 2008 at 04:23:24PM -0700, Andrew Morton wrote:
> On Thu, 18 Sep 2008 19:17:46 +0400
> Anton Vorontsov <avorontsov@...mvista.com> wrote:
> 
> > This patch adds support for the FHCI USB controller, as found
> > in the Freescale MPC836x and MPC832x processors. It can support
> > Full or Low speed modes.
> > 
> > Quite a lot the hardware is doing by itself (SOF generation, CRC
> > generation and checking), though scheduling and retransmission is on
> > software's shoulders.
> > 
> > This controller does not integrate the root hub, so this driver also
> > fakes one-port hub. External hub is required to support more than
> > one device.
> >
> > ...
> >
> 
> Please, no.

What exactly 'no'? ;-)

> > new file mode 100644
> > index 0000000..23716fa
> > --- /dev/null
> > +++ b/drivers/usb/host/fhci-cq.c
> > @@ -0,0 +1,105 @@
> > +/*
> > + * Freescale QUICC Engine USB Host Controller Driver
> > + *
> > + * Copyright (c) Freescale Semicondutor, Inc. 2006.
> > + *               Shlomi Gridish <gridish@...escale.com>
> > + *               Jerry Huang <Chang-Ming.Huang@...escale.com>
> > + * Copyright (c) Logic Product Development, Inc. 2007
> > + *               Peter Barada <peterb@...icpd.com>
> > + * Copyright (c) MontaVista Software, Inc. 2008.
> > + *               Anton Vorontsov <avorontsov@...mvista.com>
> > + *
> > + * This program is free software; you can redistribute  it and/or modify it
> > + * under  the terms of  the GNU General  Public License as published by the
> > + * Free Software Foundation;  either version 2 of the  License, or (at your
> > + * option) any later version.
> > + */
> > +
> > +/* circular queue structure */
> 
> Yet another private ringbuffer implementation.  Sigh.

Heh. I changed this to kfifo, though I don't quite like it,
since it feels heavy to hold mere pointers...

[...]
> > +++ b/drivers/usb/host/fhci-hcd.c
> > @@ -0,0 +1,798 @@
> > +/*
> > + * Freescale QUICC Engine USB Host Controller Driver
> > + *
> > + * Copyright (c) Freescale Semicondutor, Inc. 2006.
> > + *               Shlomi Gridish <gridish@...escale.com>
> > + *               Jerry Huang <Chang-Ming.Huang@...escale.com>
> > + * Copyright (c) Logic Product Development, Inc. 2007
> > + *               Peter Barada <peterb@...icpd.com>
> > + * Copyright (c) MontaVista Software, Inc. 2008.
> > + *               Anton Vorontsov <avorontsov@...mvista.com>
> > + *
> > + * This program is free software; you can redistribute  it and/or modify it
> > + * under  the terms of  the GNU General  Public License as published by the
> > + * Free Software Foundation;  either version 2 of the  License, or (at your
> > + * option) any later version.
> > + */
> > +
> > +#if defined(CONFIG_FHCI_DEBUG) && !defined(DEBUG)
> > +#define DEBUG
> > +#endif
> > +
> > +#include <linux/module.h>
> > +#include <linux/kernel.h>
> > +#include <linux/delay.h>
> > +#include <linux/errno.h>
> > +#include <linux/list.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/io.h>
> > +#include <linux/usb.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/of_gpio.h>
> > +#include <asm/qe.h>
> > +#include <asm/fsl_gtm.h>
> > +#include "../core/hcd.h"
> > +#include "fhci.h"
> > +#include "fhci-hub.c"
> > +#include "fhci-q.c"
> > +#include "fhci-dbg.c"
> > +#include "fhci-mem.c"
> > +#include "fhci-cq.c"
> > +#include "fhci-tds.c"
> > +#include "fhci-sched.c"
> 
> hack, gack, nack.
> 
> We know that USB likes to prevertedly #include C files all over the
> place, but this doesn't mean that it's a sane, tasteful or desirable
> thing to do.

Hm. I fixed this, but for lengthy drivers, where we want split the
driver in multiple logical files, this sometimes makes sense (for
example, we don't need externed functions. plus gcc automatically
might inline some of them, etc).


Thanks!

-- 
Anton Vorontsov
email: cbouatmailru@...il.com
irc://irc.freenode.net/bd2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/