| 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
| ||
|
Message-ID: <AANLkTikSb-16eACaR16Uh2c7K=2yT3e+Vna=Z2Tt-x=6@mail.gmail.com>
Date: Tue, 7 Sep 2010 14:53:05 +0200
From: Maurus Cuelenaere <mcuelenaere@...il.com>
To: Masayuki Ohtake <masa-korg@....okisemi.com>
Cc: Randy Dunlap <randy.dunlap@...cle.com>,
Peter Korsgaard <peter.korsgaard@...co.com>,
Nicolas Ferre <nicolas.ferre@...el.com>,
Michal Nazarewicz <m.nazarewicz@...sung.com>,
linux-usb <linux-usb@...r.kernel.org>,
Laurent Pinchart <laurent.pinchart@...asonboard.com>,
Greg Kroah-Hartman <gregkh@...e.de>,
Fabien Chouteau <fabien.chouteau@...co.com>,
david Brownell <dbrownell@...rs.sourceforge.net>,
Christoph Egger <siccegge@...d.informatik.uni-erlangen.de>,
LKML <linux-kernel@...r.kernel.org>, MeeGo <meego-dev@...go.com>,
"Wang, Qi" <qi.wang@...el.com>,
"Wang, Yong Y" <yong.y.wang@...el.com>,
Andrew <andrew.chih.howe.khor@...el.com>,
Intel OTC <joel.clark@...el.com>,
"Foster, Margie" <margie.foster@...el.com>,
Arjan <arjan@...ux.intel.com>,
Toshiharu Okada <okada533@....okisemi.com>,
Takahiro Shimizu <shimizu394@....okisemi.com>,
Tomoya Morinaga <morinaga526@....okisemi.com>
Subject: Re: [PATCH] USB device driver of Topcliff PCH
2010/9/7 Masayuki Ohtake <masa-korg@....okisemi.com>
>
> This patch adds the USB device driver of Topcliff PCH.
> Topcliff PCH is the platform controller hub that is going to be used in
> Intel's upcoming general embedded platform. All IO peripherals in
> Topcliff PCH are actually devices sitting on AMBA bus.
> Topcliff PCH has USB device I/F. Using this I/F, it is able to access system
> devices connected to USB device.
>
> Signed-off-by: Masayuki Ohtake <masa-korg@....okisemi.com>
No need to mail all these people, just the maintainers and appropriate mailing
lists will do.
> ---
> drivers/usb/gadget/Kconfig | 24 +
> drivers/usb/gadget/Makefile | 2 +-
> drivers/usb/gadget/gadget_chips.h | 7 +
> drivers/usb/gadget/pch_udc.c | 3153 +++++++++++++++++++++++++++++++++++++
> drivers/usb/gadget/pch_udc.h | 495 ++++++
> 5 files changed, 3680 insertions(+), 1 deletions(-)
> create mode 100755 drivers/usb/gadget/pch_udc.c
> create mode 100755 drivers/usb/gadget/pch_udc.h
>
[snip]
> diff --git a/drivers/usb/gadget/pch_udc.c b/drivers/usb/gadget/pch_udc.c
> new file mode 100755
> index 0000000..2065c11
> --- /dev/null
> +++ b/drivers/usb/gadget/pch_udc.c
[snip]
> +
> +const char ep0_string[] = "ep0in";
> +static DEFINE_SPINLOCK(udc_stall_spinlock); /* stall spin lock */
> +static u32 pch_udc_base;
> +static union pch_udc_setup_data setup_data; /* received setup data */
> +static unsigned long ep0out_buf[64];
> +static dma_addr_t dma_addr;
> +struct pch_udc_dev *pch_udc; /* pointer to device object */
> +int speed_fs;
I'd put all these in struct phc_udc_dev or similar, so you could have multiple
controllers.
> +
> +module_param_named(speed_fs, speed_fs, bool, S_IRUGO);
> +MODULE_PARM_DESC(speed_fs, "true for Full speed operation");
> +MODULE_DESCRIPTION("OKISEMI PCH UDC - USB Device Controller");
> +MODULE_LICENSE("GPL");
> +
> +/**
> + * pch_udc_write_csr - Write the command and status registers.
> + * @val: value to be written to CSR register
> + * @addr: address of CSR register
> + */
> +inline void pch_udc_write_csr(unsigned long val, unsigned long addr)
Make all these functions static.
> +{
> + int count = MAX_LOOP;
> +
> + /* Wait till idle */
> + while ((count > 0) &&\
> + (ioread32((u32 *)(PCH_UDC_CSR_BUSY_ADDR + pch_udc_base)) &
> + PCH_UDC_CSR_BUSY))
Why not define PCH_UDC_* as (void __iomem*) so no casting is necessary.
> + count--;
> +
> + if (count < 0)
> + pr_debug("%s: wait error; count = %x", __func__, count);
> +
> + iowrite32(val, (u32 *)addr);
> + /* Wait till idle */
> + count = MAX_LOOP;
> + while ((count > 0) &&
> + (ioread32((u32 *)(PCH_UDC_CSR_BUSY_ADDR + pch_udc_base)) &
> + PCH_UDC_CSR_BUSY))
> + count--;
> +
> + if (count < 0)
> + pr_debug("%s: wait error; count = %x", __func__, count);
> +
> +}
> +
> +/**
> + * pch_udc_read_csr - Read the command and status registers.
> + * @addr: address of CSR register
> + * Returns
> + * content of CSR register
> + */
> +inline u32 pch_udc_read_csr(unsigned long addr)
void __iomem *addr
> +{
> + int count = MAX_LOOP;
> +
> + /* Wait till idle */
> + while ((count > 0) &&
> + (ioread32((u32 *)(PCH_UDC_CSR_BUSY_ADDR + pch_udc_base)) &
> + PCH_UDC_CSR_BUSY))
> + count--;
> +
> + if (count < 0)
> + pr_debug("%s: wait error; count = %x", __func__, count);
> + /* Dummy read */
> + ioread32((u32 *)addr);
> + count = MAX_LOOP;
> + /* Wait till idle */
> + while ((count > 0) &&
> + (ioread32((u32 *)(PCH_UDC_CSR_BUSY_ADDR + pch_udc_base)) &
> + PCH_UDC_CSR_BUSY))
> + count--;
> + /* actual read */
> + if (count < 0)
> + pr_debug("%s: wait error; count = %x", __func__, count);
> +
> + return ioread32((u32 *)addr);
> +}
> +
> +/**
> + * pch_udc_rmt_wakeup - Initiate for remote wakeup
> + * @dev: Reference to pch_udc_regs structure
> + */
> +inline void pch_udc_rmt_wakeup(struct pch_udc_regs *dev)
> +{
> + PCH_UDC_BIT_SET(&dev->devctl, 1 << UDC_DEVCTL_RES);
> + mdelay(1);
> + PCH_UDC_BIT_CLR(&dev->devctl, 1 << UDC_DEVCTL_RES);
> +}
> +
> +/**
> + * pch_udc_get_frame - Get the current frame from device status register
> + * @dev: Reference to pch_udc_regs structure
> + * Retern current frame
> + */
> +inline int pch_udc_get_frame(struct pch_udc_regs *dev)
> +{
> + u32 frame;
> +
> + frame = ioread32(&dev->devsts);
Why not just get rid of struct pch_udc_regs and do something like:
static inline u32 pch_readl(struct pch_udc_dev *dev, unsigned long reg)
{
return ioread32(dev->base_addr + reg);
}
(and similar for pch_writel)
> + return (frame & UDC_DEVSTS_TS_MASK) >> UDC_DEVSTS_TS_OFS;
> +}
[snip]
> +static struct usb_request *pch_udc_alloc_request(struct usb_ep *usbep,
> + gfp_t gfp)
> +{
> + struct pch_udc_request *req;
> + struct pch_udc_ep *ep;
> +
> + pr_debug("%s: enter", __func__);
> + if (usbep == NULL)
> + return NULL;
> +
> + ep = container_of(usbep, struct pch_udc_ep, ep);
> + pr_debug("%s: ep %s", __func__, usbep->name);
> + req = kzalloc(sizeof(struct pch_udc_request), gfp);
> + if (req == NULL) {
> + pr_debug("%s: no memory for request", __func__);
> + return NULL;
> + }
> + memset(req, 0, sizeof(struct pch_udc_request));
kzalloc does this..
> + req->req.dma = DMA_ADDR_INVALID;
> + INIT_LIST_HEAD(&req->queue);
> +
> + if (ep->dma != NULL) {
> + struct pch_udc_data_dma_desc *dma_desc;
> +
> + /* ep0 in requests are allocated from data pool here */
> + dma_desc = pci_pool_alloc(ep->dev->data_requests, gfp,
> + &req->td_data_phys);
> + if (NULL == dma_desc) {
> + kfree(req);
> + return NULL;
> + }
> +
> + pr_debug("%s: req = 0x%p dma_desc = 0x%p, "
> + "td_phys = 0x%08lx", __func__,
> + req, dma_desc, (unsigned long)req->td_data_phys);
> +
> + /* prevent from using desc. - set HOST BUSY */
> + dma_desc->status |= PCH_UDC_BS_HST_BSY;
> + dma_desc->dataptr = __constant_cpu_to_le32(DMA_ADDR_INVALID);
> + req->td_data = dma_desc;
> + req->td_data_last = dma_desc;
> + req->chain_len = 1;
> + }
> + return &req->req;
> +}
[snip]
> +
> +/**
> + * pch_udc_start_next_txrequest - This function starts
> + * the next transmission requirement
> + * @ep: Reference to the endpoint structure
> + */
> +static void pch_udc_start_next_txrequest(struct pch_udc_ep *ep)
> +{
> + struct pch_udc_request *req;
> +
> + pr_debug("%s: enter", __func__);
> + if (pch_udc_read_ep_control(ep->regs) & (1 << UDC_EPCTL_P))
> + return;
> +
> + if (!list_empty(&ep->queue)) {
Convert this into:
if (list_empty(&ep->queue))
return;
That way you get rid of the unnecessary spacing below
> + /* next request */
> + req = list_entry(ep->queue.next, struct pch_udc_request, queue);
> + if (req && !req->dma_going) {
> + pr_debug("%s: Set request: req=%p req->td_data=%p",
> + __func__, req, req->td_data);
> + if (req->td_data) {
> + struct pch_udc_data_dma_desc *td_data;
> +
> + while (pch_udc_read_ep_control(ep->regs) &
> + (1 << UDC_EPCTL_S))
> + udelay(100);
> +
> + req->dma_going = 1;
> + /* Clear the descriptor pointer */
> + pch_udc_ep_set_ddptr(ep->regs, 0);
> +
> + td_data = req->td_data;
> + while (1) {
> + td_data->status = (td_data->status &
> + ~PCH_UDC_BUFF_STS) |
> + PCH_UDC_BS_HST_RDY;
> + if ((td_data->status &
> + PCH_UDC_DMA_LAST) ==
> + PCH_UDC_DMA_LAST)
> + break;
> +
> + td_data =
> + (struct pch_udc_data_dma_desc *)\
> + phys_to_virt(td_data->next);
> + }
> + /* Write the descriptor pointer */
> + pch_udc_ep_set_ddptr(ep->regs,
> + req->td_data_phys);
> + pch_udc_set_dma(ep->dev->regs, DMA_DIR_TX);
> + /* Set the poll demand bit */
> + pch_udc_ep_set_pd(ep->regs);
> + pch_udc_enable_ep_interrupts(ep->dev->regs,
> + 1 << (ep->in ? ep->num :\
> + ep->num + UDC_EPINT_OUT_EP0));
> + pch_udc_ep_clear_nak(ep->regs);
> + }
> + }
> + }
> +}
> +
> +/**
> + * pch_udc_complete_transfer - This function completes a transfer
> + * @ep: Reference to the endpoint structure
> + */
> +static void pch_udc_complete_transfer(struct pch_udc_ep *ep)
> +{
> + struct pch_udc_request *req;
> +
> + pr_debug("%s: enter", __func__);
> + if (!list_empty(&ep->queue)) {
Same here
> + pr_debug("%s: list_entry", __func__);
> + req = list_entry(ep->queue.next, struct pch_udc_request, queue);
> + if (req && ((req->td_data_last->status & PCH_UDC_BUFF_STS) ==
> + PCH_UDC_BS_DMA_DONE)) {
> +#ifdef DMA_PPB_WITH_DESC_UPDATE
> + struct pch_udc_data_dma_desc *td_data = req->td_data;
> + for (i = 0; i < req->chain_len; i++) {
> + if ((td_data->status & PCH_UDC_RXTX_STS) !=
> + PCH_UDC_RTS_SUCC) {
> + pr_err("Invalid RXTX status (0x%08x)"
> + " epstatus=0x%08x\n",
> + (td_data->status &
> + PCH_UDC_RXTX_STS),
> + (int)(ep->epsts));
> + return;
> + }
> + td_data = (struct pch_udc_data_dma_desc *)
> + phys_to_virt(td_data->next);
> + }
> +#else
> + if ((req->td_data_last->status & PCH_UDC_RXTX_STS) !=
> + PCH_UDC_RTS_SUCC) {
> + pr_err("Invalid RXTX status (0x%08x)"
> + " epstatus=0x%08x\n",
> + (req->td_data_last->status &
> + PCH_UDC_RXTX_STS),
> + (int)(ep->epsts));
> + return;
> + }
> +#endif
> + req->req.actual = req->req.length;
> + req->td_data_last->status = PCH_UDC_BS_HST_BSY |
> + PCH_UDC_DMA_LAST;
> + req->td_data->status = PCH_UDC_BS_HST_BSY |
> + PCH_UDC_DMA_LAST;
> + /* complete req */
> + complete_req(ep, req, 0);
> + req->dma_going = 0;
> + if (!list_empty(&ep->queue)) {
> + while (pch_udc_read_ep_control(ep->regs) &
> + (1 << UDC_EPCTL_S))
> + udelay(100);
> +
> + pch_udc_ep_clear_nak(ep->regs);
> + pch_udc_enable_ep_interrupts(ep->dev->regs,
> + 1 << (ep->in ? ep->num : ep->num +
> + UDC_EPINT_OUT_EP0));
> + } else {
> + pch_udc_disable_ep_interrupts(ep->dev->regs,
> + 1 << (ep->in ? ep->num : ep->num +
> + UDC_EPINT_OUT_EP0));
> + }
> + }
> + }
> +}
[snip]
> +
> +/**
> + * pch_udc_svc_enum_interrupt - This function handles a USB speed enumeration
> + * done interrupt
> + * @dev: Reference to driver structure
> + */
> +void
> +pch_udc_svc_enum_interrupt(struct pch_udc_dev *dev)
> +{
> + u32 dev_stat, dev_speed;
> + u32 speed = USB_SPEED_FULL;
> +
> + pr_debug("%s: enter", __func__);
> + dev_stat = pch_udc_read_device_status(dev->regs);
> + dev_speed = (dev_stat & UDC_DEVSTS_ENUM_SPEED_MASK) >>
> + UDC_DEVSTS_ENUM_SPEED_OFS;
> +
> + pr_debug("%s: dev_speed = 0x%08x", __func__, dev_speed);
> +
> + if (dev_speed == UDC_DEVSTS_ENUM_SPEED_HIGH) {
> + pr_debug("HighSpeed");
> + speed = USB_SPEED_HIGH;
> + } else if (dev_speed == UDC_DEVSTS_ENUM_SPEED_FULL) {
> + pr_debug("FullSpeed");
> + speed = USB_SPEED_FULL;
> + } else if (dev_speed == UDC_DEVSTS_ENUM_SPEED_LOW) {
> + pr_debug("LowSpeed?");
> + speed = USB_SPEED_LOW;
> + } else {
> + pr_debug("FullSpeed?");
BUG() perhaps? Also, change this into a switch statement
> + }
> + dev->gadget.speed = speed;
> +
> + pch_udc_activate_control_ep(dev);
> +
> + /* enable ep0 interrupts */
> + pch_udc_enable_ep_interrupts(dev->regs, 1 << UDC_EPINT_IN_EP0 |
> + 1 << UDC_EPINT_OUT_EP0);
> + /* enable DMA */
> + pch_udc_set_dma(dev->regs, DMA_DIR_TX);
> + pch_udc_set_dma(dev->regs, DMA_DIR_RX);
> + pch_udc_ep_set_rrdy(dev->ep[UDC_EP0OUT_IDX].regs);
> +
> +
> + pr_debug("%s: EP mask set to %x", __func__,
> + ioread32(&dev->regs->epirqmsk));
> +}
[snip]
> +
> +int pch_udc_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> +{
> + unsigned long resource;
> + unsigned long len;
> + int retval = 0;
> + struct pch_udc_dev *dev;
> +
> + dev_dbg(&pdev->dev, "%s: enter", __func__);
> + /* one udc only */
> + if (pch_udc != NULL) {
> + dev_err(&pdev->dev, "%s: already probed", __func__);
> + return -EBUSY;
> + }
> +
> + /* init */
> + dev = kzalloc(sizeof(struct pch_udc_dev), GFP_KERNEL);
> + if (dev == NULL) {
> + dev_err(&pdev->dev, "%s: no memory for device structure",
> + __func__);
> + return -ENOMEM;
> + }
> + memset(dev, 0, sizeof(struct pch_udc_dev));
kzalloc already does this for you
> + /* pci setup */
> + if (pci_enable_device(pdev) < 0) {
> + kfree(dev);
> + dev_err(&pdev->dev, "%s: pci_enable_device failed", __func__);
> + return -ENODEV;
> + }
> + dev->active = 1;
> + pci_set_drvdata(pdev, dev);
> +
> + /* PCI resource allocation */
> + resource = pci_resource_start(pdev, 1);
> + len = pci_resource_len(pdev, 1);
> + dev_dbg(&pdev->dev, "%s: resource %lx, len %ld",
> + __func__, resource, len);
> +
> + if (request_mem_region(resource, len, KBUILD_MODNAME) == NULL) {
> + dev_err(&pdev->dev, "%s: pci device used already", __func__);
> + retval = -EBUSY;
> + goto finished;
> + }
> + dev->phys_addr = resource;
> + dev->mem_region = 1;
> +
> + dev->virt_addr = ioremap_nocache(resource, len);
> + if (dev->virt_addr == NULL) {
> + dev_err(&pdev->dev, "%s: device memory cannot be mapped",
> + __func__);
> + retval = -ENOMEM;
> + goto finished;
> + }
> + dev_dbg(&pdev->dev, "%s: device memory mapped at %x", __func__,
> + (int)dev->virt_addr);
> +
> + if (pdev->irq == 0) {
> + dev_err(&pdev->dev, "%s: irq not set", __func__);
> + retval = -ENODEV;
> + goto finished;
> + }
> +
> + pch_udc = dev;
> +
> + /* initialize the hardware */
> + if (pch_udc_pcd_init(dev) != 0)
> + goto finished;
> +
> + if (request_irq(pdev->irq, pch_udc_isr, IRQF_SHARED,
> + KBUILD_MODNAME, dev) != 0) {
> + dev_err(&pdev->dev, "%s: request_irq(%d) fail", __func__,
> + pdev->irq);
> + retval = -ENODEV;
> + goto finished;
> + }
> + dev->irq = pdev->irq;
> + dev->irq_registered = 1;
> +
> + pci_set_master(pdev);
> + pci_try_set_mwi(pdev);
> +
> + /* device struct setup */
> + spin_lock_init(&dev->lock);
> + dev->pdev = pdev;
> + dev->gadget.ops = &pch_udc_ops;
> +
> + retval = init_dma_pools(dev);
> + if (retval != 0)
> + goto finished;
> +
> + dev_set_name(&dev->gadget.dev, "gadget");
> + dev->gadget.dev.parent = &pdev->dev;
> + dev->gadget.dev.dma_mask = pdev->dev.dma_mask;
> + dev->gadget.dev.release = gadget_release;
> + dev->gadget.name = KBUILD_MODNAME;
> + dev->gadget.is_dualspeed = 1;
> +
> + retval = device_register(&dev->gadget.dev);
> + if (retval != 0)
> + goto finished;
> + dev->registered = 1;
> +
> + /* Put the device in disconnected state till a driver is bound */
> + pch_udc_set_disconnect(dev->regs);
> + return 0;
> +
> +finished:
> + pch_udc_remove(pdev);
> + return retval;
> +}
> +
> +static const struct pci_device_id pch_udc_pcidev_id[] = {
> + {
> + PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_PCH1_UDC),
> + .class = (PCI_CLASS_SERIAL_USB << 8) | 0xfe,
> + .class_mask = 0xffffffff,
> + },
> + { 0 },
> +};
> +
> +MODULE_DEVICE_TABLE(pci, pch_udc_pcidev_id);
> +
> +
> +static struct pci_driver pch_udc_driver = {
> + .name = KBUILD_MODNAME,
> + .id_table = pch_udc_pcidev_id,
> + .probe = pch_udc_probe,
> + .remove = pch_udc_remove,
> + .suspend = pch_udc_suspend,
> + .resume = pch_udc_resume,
> + .shutdown = pch_udc_shutdown,
Make all these functions static
> +};
> +
> +static int __init pch_udc_pci_init(void)
> +{
> + return pci_register_driver(&pch_udc_driver);
> +}
> +module_init(pch_udc_pci_init);
> +
> +static void __exit pch_udc_pci_exit(void)
> +{
> + pci_unregister_driver(&pch_udc_driver);
> +}
> +module_exit(pch_udc_pci_exit);
> diff --git a/drivers/usb/gadget/pch_udc.h b/drivers/usb/gadget/pch_udc.h
> new file mode 100755
> index 0000000..55c22ef
> --- /dev/null
> +++ b/drivers/usb/gadget/pch_udc.h
> @@ -0,0 +1,495 @@
> +/*
> + * Copyright (C) 2010 OKI SEMICONDUCTOR Co., LTD.
> + *
> + * 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; version 2 of the License.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307, USA.
> + */
> +
> +#ifndef _PCH_UDC_H_
> +#define _PCH_UDC_H_
> +
> +/* Address offset of Registers */
> +#define UDC_EPIN_REGS_ADDR 0x000
> +#define UDC_EPOUT_REGS_ADDR 0x200
> +#define UDC_EP_REG_OFS 0x20 /* Offset to next EP */
> +#define UDC_DEVCFG_ADDR 0x400
> +#define PCH_UDC_CSR_BUSY_ADDR 0x4f0
> +#define PCH_UDC_SRST_ADDR 0x4fc
> +#define UDC_CSR_ADDR 0x500
> +
> +/* Bit position in UDC CSR Busy status Register */
> +#define PCH_UDC_CSR_BUSY 1
> +/* Bit position in UDC Soft reset Register */
> +#define PCH_UDC_PSRST 1
> +#define PCH_UDC_SRST 0
> +
[snip]
> +
> +/**
> + * pch_udc_csrs - Structure to Endpoint configuration registers
> + */
> +struct pch_udc_csrs {
> + u32 ne[PCH_UDC_USED_EP_NUM * 2];
Why not do away with the structs and do something like:
#define PCH_UDC_CSR(ep) (UDC_CSR_ADDR + ep*4)
(and similar for the others)
> +/* Starting bit position */
> +#define UDC_CSR_NE_NUM_OFS 0
> +#define UDC_CSR_NE_DIR_OFS 4
> +#define UDC_CSR_NE_TYPE_OFS 5
> +#define UDC_CSR_NE_CFG_OFS 7
> +#define UDC_CSR_NE_INTF_OFS 11
> +#define UDC_CSR_NE_ALT_OFS 15
> +#define UDC_CSR_NE_MAX_PKT_OFS 19
> +/* Mask patern */
> +#define UDC_CSR_NE_NUM_MASK 0x0000000f
> +#define UDC_CSR_NE_DIR_MASK 0x00000010
> +#define UDC_CSR_NE_TYPE_MASK 0x00000060
> +#define UDC_CSR_NE_CFG_MASK 0x00000780
> +#define UDC_CSR_NE_INTF_MASK 0x00007800
> +#define UDC_CSR_NE_ALT_MASK 0x00078000
> +#define UDC_CSR_NE_MAX_PKT_MASK 0x3ff80000
> +};
> +
[snip]
> +
> +/**
> + * pch_udc_request - Structure holding a PCH USB device request
> + * @req embedded ep request
> + * @td_data_phys phys. address
> + * @td_data first dma desc. of chain
> + * @td_data_last last dma desc. of chain
> + * @queue associated queue
> + * @dma_going DMA in progress for request
> + * @dma_mapped DMA memory mapped for request
> + * @dma_done DMA completed for request
> + * @chain_len chain length
> + */
> +struct pch_udc_request /* request packet */
> +{
I don't see any reason to not put this in the main .c file?
(same for struct pch_udc_{ep,request})
> + struct usb_request req;
> + dma_addr_t td_data_phys;
> + struct pch_udc_data_dma_desc *td_data;
> + struct pch_udc_data_dma_desc *td_data_last;
> + struct list_head queue;
> + unsigned dma_going:1,
> + dma_mapped:1,
> + dma_done:1;
> + unsigned chain_len;
> +};
> +
[snip]
> +
> +struct pch_udc_dev {
> + struct usb_gadget gadget;
> + struct usb_gadget_driver *driver;
> + struct pci_dev *pdev;
> + /* all endpoints */
> + struct pch_udc_ep ep[PCH_UDC_EP_NUM];
> + spinlock_t lock;
> + unsigned active:1,
> + stall:1,
> + prot_stall:1,
> + irq_registered:1,
> + mem_region:1,
> + registered:1,
> + suspended:1,
> + connected:1,
> + set_cfg_not_acked:1,
> + waiting_zlp_ack:1;
> + struct pch_udc_csrs __iomem *csr;
> + struct pch_udc_regs __iomem *regs;
> + struct pch_udc_ep_regs __iomem *ep_regs;
These pointers just seem unnecessary, especially as you could easily construct
them in-place by adding the appropriate offset to your base address..
> + struct pci_pool *data_requests;
> + struct pci_pool *stp_requests;
> + unsigned long phys_addr;
> + void __iomem *virt_addr;
> + unsigned irq;
> + struct pch_udc_cfg_data cfg_data;
> +};
Powered by blists - more mailing lists