On Fri, 12 Aug 2011 15:45:34 -0400
Alexandre Bounine <alexandre.boun...@idt.com> wrote:
> Add RapidIO mport driver for IDT TSI721 PCI Express-to-SRIO bridge device.
> The driver provides full set of callback functions defined for mport devices
> in RapidIO subsystem. It also is compatible with current version of RIONET
> driver (Ethernet over RapidIO messaging services).
>
> This patch is applicable to kernel versions starting from 2.6.39.
What a huge driver.
>
> ...
>
> --- /dev/null
> +++ b/drivers/rapidio/devices/Kconfig
> @@ -0,0 +1,10 @@
> +#
> +# RapidIO master port configuration
> +#
> +
> +config RAPIDIO_TSI721
> + bool "IDT Tsi721 PCI Express SRIO Controller support"
> + depends on RAPIDIO && PCI && PCIEPORTBUS
The dependency on PCI is redundant - PCIEPORTBUS already depends on
PCI. Doesn't matter much though.
> + default "n"
> + ---help---
> + Include support for IDT Tsi721 PCI Express Serial RapidIO controller.
>
> ...
>
> +static int tsi721_maint_dma(struct tsi721_device *priv, u32 sys_size,
> + u16 destid, u8 hopcount, u32 offset, int len,
> + u32 *data, int do_wr)
> +{
> + struct tsi721_dma_desc *bd_ptr;
> + u32 rd_count, swr_ptr, ch_stat;
> + int i, err = 0;
> + u32 op = do_wr ? MAINT_WR : MAINT_RD;
> +
> + if (offset > (RIO_MAINT_SPACE_SZ - len) || (len != sizeof(u32)))
> + return -EINVAL;
> +
> + bd_ptr = priv->bdma[TSI721_DMACH_MAINT].bd_base;
> +
> + rd_count = ioread32(
> + priv->regs + TSI721_DMAC_DRDCNT(TSI721_DMACH_MAINT));
> +
> + /* Initialize DMA descriptor */
> + bd_ptr[0].type_id = cpu_to_le32((DTYPE2 << 29) | (op << 19) | destid);
> + bd_ptr[0].bcount = cpu_to_le32((sys_size << 26) | 0x04);
> + bd_ptr[0].raddr_lo = cpu_to_le32((hopcount << 24) | offset);
> + bd_ptr[0].raddr_hi = 0;
> + if (do_wr)
> + bd_ptr[0].data[0] = cpu_to_be32p(data);
> + else
> + bd_ptr[0].data[0] = 0xffffffff;
> +
> + mb();
> +
> + /* Start DMA operation */
> + iowrite32(rd_count + 2,
> + priv->regs + TSI721_DMAC_DWRCNT(TSI721_DMACH_MAINT));
> + (void)ioread32(priv->regs + TSI721_DMAC_DWRCNT(TSI721_DMACH_MAINT));
> + i = 0;
> +
> + /* Wait until DMA transfer is finished */
> + while ((ch_stat = ioread32(priv->regs +
> + TSI721_DMAC_STS(TSI721_DMACH_MAINT))) & TSI721_DMAC_STS_RUN) {
> + udelay(10);
> + i++;
> + if (i >= 5000000) {
> + dev_dbg(&priv->pdev->dev,
> + "%s : DMA[%d] read timeout ch_status=%x\n",
> + __func__, TSI721_DMACH_MAINT, ch_stat);
> + if (!do_wr)
> + *data = 0xffffffff;
> + err = -EFAULT;
> + goto err_out;
> + }
> + }
Fifty seconds!?!?
EFAULT seems an inappropriate errno.
> + if (ch_stat & TSI721_DMAC_STS_ABORT) {
> + /* If DMA operation aborted due to error,
> + * reinitialize DMA channel
> + */
> + dev_dbg(&priv->pdev->dev, "%s : DMA ABORT ch_stat=%x\n",
> + __func__, ch_stat);
> + dev_dbg(&priv->pdev->dev, "OP=%d : destid=%x hc=%x off=%x\n",
> + do_wr ? MAINT_WR : MAINT_RD, destid, hopcount, offset);
> + iowrite32(TSI721_DMAC_INT_ALL,
> + priv->regs + TSI721_DMAC_INT(TSI721_DMACH_MAINT));
> + iowrite32(TSI721_DMAC_CTL_INIT,
> + priv->regs + TSI721_DMAC_CTL(TSI721_DMACH_MAINT));
> + udelay(10);
> + iowrite32(0, priv->regs +
> + TSI721_DMAC_DWRCNT(TSI721_DMACH_MAINT));
> + udelay(1);
> + if (!do_wr)
> + *data = 0xffffffff;
> + err = -EFAULT;
> + goto err_out;
> + }
> +
> + if (!do_wr)
> + *data = be32_to_cpu(bd_ptr[0].data[0]);
> +
> + /*
> + * Update descriptor status FIFO RD pointer.
> + * NOTE: Skipping check and clear FIFO entries because we are waiting
> + * for transfer to be completed.
> + */
> + swr_ptr = ioread32(priv->regs + TSI721_DMAC_DSWP(TSI721_DMACH_MAINT));
> + iowrite32(swr_ptr, priv->regs + TSI721_DMAC_DSRP(TSI721_DMACH_MAINT));
> +err_out:
> +
> + return err;
> +}
>
> ...
>
> +static int
> +tsi721_pw_handler(struct rio_mport *mport)
> +{
> + struct tsi721_device *priv = mport->priv;
> + u32 pw_stat;
> + u32 pw_buf[TSI721_RIO_PW_MSG_SIZE/sizeof(u32)];
> +
> +
> + pw_stat = ioread32(priv->regs + TSI721_RIO_PW_RX_STAT);
> +
> + if (pw_stat & TSI721_RIO_PW_RX_STAT_PW_VAL) {
> + pw_buf[0] = ioread32(priv->regs + TSI721_RIO_PW_RX_CAPT(0));
> + pw_buf[1] = ioread32(priv->regs + TSI721_RIO_PW_RX_CAPT(1));
> + pw_buf[2] = ioread32(priv->regs + TSI721_RIO_PW_RX_CAPT(2));
> + pw_buf[3] = ioread32(priv->regs + TSI721_RIO_PW_RX_CAPT(3));
> +
> + /* Queue PW message (if there is room in FIFO),
> + * otherwise discard it.
> + */
> + spin_lock(&priv->pw_fifo_lock);
> + if (kfifo_avail(&priv->pw_fifo) >= TSI721_RIO_PW_MSG_SIZE)
> + kfifo_in(&priv->pw_fifo, pw_buf,
> + TSI721_RIO_PW_MSG_SIZE);
> + else
> + priv->pw_discard_count++;
> + spin_unlock(&priv->pw_fifo_lock);
> + }
> +
> + /* Clear pending PW interrupts */
> + iowrite32(TSI721_RIO_PW_RX_STAT_PW_DISC | TSI721_RIO_PW_RX_STAT_PW_VAL,
> + priv->regs + TSI721_RIO_PW_RX_STAT);
> +
> + schedule_work(&priv->pw_work);
I see scheduled work being done, but no flush_scheduled_work() or
similar. This is often a bug, leading to code being executed after
device shutdown or even after rmmod.
> + return 0;
> +}
> +
> +static void tsi721_pw_dpc(struct work_struct *work)
> +{
> + struct tsi721_device *priv = container_of(work, struct tsi721_device,
> + pw_work);
> + unsigned long flags;
> + u32 msg_buffer[RIO_PW_MSG_SIZE/sizeof(u32)]; /* Use full size PW message
> + buffer for RIO layer */
> +
> + /*
> + * Process port-write messages
> + */
> + spin_lock_irqsave(&priv->pw_fifo_lock, flags);
> + while (kfifo_out(&priv->pw_fifo, (unsigned char *)msg_buffer,
> + TSI721_RIO_PW_MSG_SIZE)) {
> + /* Process one message */
> + spin_unlock_irqrestore(&priv->pw_fifo_lock, flags);
> +#ifdef DEBUG_PW
> + {
> + u32 i;
> + pr_debug("%s : Port-Write Message:", __func__);
> + for (i = 0; i < RIO_PW_MSG_SIZE/sizeof(u32); ) {
> + pr_debug("0x%02x: %08x %08x %08x %08x", i*4,
> + msg_buffer[i], msg_buffer[i + 1],
> + msg_buffer[i + 2], msg_buffer[i + 3]);
> + i += 4;
> + }
> + pr_debug("\n");
> + }
> +#endif
> + /* Pass the port-write message to RIO core for processing */
> + rio_inb_pwrite_handler((union rio_pw_msg *)msg_buffer);
> + spin_lock_irqsave(&priv->pw_fifo_lock, flags);
> + }
> + spin_unlock_irqrestore(&priv->pw_fifo_lock, flags);
> +}
The lock handling in this function is pretty ugly-looking. Is it correct?
>
> ...
>
> +static void tsi721_db_dpc(struct work_struct *work)
> +{
> + struct tsi721_device *priv = container_of(work, struct tsi721_device,
> + idb_work);
> + struct rio_mport *mport;
> + struct rio_dbell *dbell;
> + int found = 0;
> + u32 wr_ptr, rd_ptr;
> + u64 *idb_entry;
> + u32 regval;
> + union {
> + u64 msg;
> + u8 bytes[8];
> + } idb;
> +
> + /*
> + * Process queued inbound doorbells
> + */
> + mport = priv->mport;
> +
> + wr_ptr = ioread32(priv->regs + TSI721_IDQ_WP(IDB_QUEUE));
> + rd_ptr = ioread32(priv->regs + TSI721_IDQ_RP(IDB_QUEUE));
> +
> + while (wr_ptr != rd_ptr) {
> + idb_entry = (u64 *)(priv->idb_base +
> + (TSI721_IDB_ENTRY_SIZE * rd_ptr));
> + rd_ptr++;
> + idb.msg = *idb_entry;
Is this code correct on both little-endian and big-endian hardware?
> + *idb_entry = 0;
> +
> + /* Process one doorbell */
> + list_for_each_entry(dbell, &mport->dbells, node) {
> + if ((dbell->res->start <= DBELL_INF(idb.bytes)) &&
> + (dbell->res->end >= DBELL_INF(idb.bytes))) {
> + found = 1;
> + break;
> + }
> + }
> +
> + if (found) {
> + dbell->dinb(mport, dbell->dev_id, DBELL_SID(idb.bytes),
> + DBELL_TID(idb.bytes), DBELL_INF(idb.bytes));
> + } else {
> + dev_dbg(&priv->pdev->dev,
> + "spurious inb doorbell, sid %2.2x tid %2.2x"
> + " info %4.4x\n", DBELL_SID(idb.bytes),
> + DBELL_TID(idb.bytes), DBELL_INF(idb.bytes));
> + }
> + }
> +
> + iowrite32(rd_ptr & (IDB_QSIZE - 1),
> + priv->regs + TSI721_IDQ_RP(IDB_QUEUE));
> +
> + /* Re-enable IDB interrupts */
> + regval = ioread32(priv->regs + TSI721_SR_CHINTE(IDB_QUEUE));
> + regval |= TSI721_SR_CHINT_IDBQRCV;
> + iowrite32(regval,
> + priv->regs + TSI721_SR_CHINTE(IDB_QUEUE));
> +}
> +
>
> ...
>
> +static void tsi721_interrupts_init(struct tsi721_device *priv)
> +{
> + u32 intr;
> +
> + /* Enable IDB interrupts */
> + iowrite32(TSI721_SR_CHINT_ALL,
> + priv->regs + TSI721_SR_CHINT(IDB_QUEUE));
> + iowrite32(TSI721_SR_CHINT_IDBQRCV,
> + priv->regs + TSI721_SR_CHINTE(IDB_QUEUE));
> + iowrite32(TSI721_INT_SR2PC_CHAN(IDB_QUEUE),
> + priv->regs + TSI721_DEV_CHAN_INTE);
> +
> + /* Enable SRIO MAC interrupts */
> + iowrite32(TSI721_RIO_EM_DEV_INT_EN_INT,
> + priv->regs + TSI721_RIO_EM_DEV_INT_EN);
> +
> + if (priv->flags & TSI721_USING_MSIX)
> + intr = TSI721_DEV_INT_SRIO;
> + else
> + intr = TSI721_DEV_INT_SR2PC_CH | TSI721_DEV_INT_SRIO |
> + TSI721_DEV_INT_SMSG_CH;
> +
> + iowrite32(intr, priv->regs + TSI721_DEV_INTE);
> + (void)ioread32(priv->regs + TSI721_DEV_INTE);
Why include all of these void casts, btw?
> +}
> +
> +/**
> + * tsi721_request_msix - register interrupt service for MSI-X mode.
> + * @mport: RapidIO master port structure
> + *
> + * Registers MSI-X interrupt service routines for interrupts that are active
> + * immediately after mport initialization. Messaging interrupt service
> routines
> + * should be registered during corresponding open requests.
> + */
> +static int tsi721_request_msix(struct rio_mport *mport)
> +{
> + struct tsi721_device *priv = mport->priv;
> + int err = 0;
> +
> + err = request_irq(priv->msix[TSI721_VECT_IDB].vector,
> + tsi721_sr2pc_ch_msix, 0,
> + priv->msix[TSI721_VECT_IDB].irq_name, (void *)mport);
> + if (err)
> + goto out;
> +
> + err = request_irq(priv->msix[TSI721_VECT_PWRX].vector,
> + tsi721_srio_msix, 0,
> + priv->msix[TSI721_VECT_PWRX].irq_name, (void *)mport);
Did we leak the first IRQ here?
> +out:
> + return err;
> +}
> +
>
> ...
>
> +static int tsi721_enable_msix(struct tsi721_device *priv)
> +{
> + struct msix_entry entries[TSI721_VECT_MAX];
> + int err;
> + int i;
> +
> + entries[TSI721_VECT_IDB].entry = TSI721_MSIX_SR2PC_IDBQ_RCV(IDB_QUEUE);
> + entries[TSI721_VECT_PWRX].entry = TSI721_MSIX_SRIO_MAC_INT;
> +
> + /*
> + * Initialize MSI-X entries for Messaging Engine:
> + * this driver supports four RIO mailboxes (inbound and outbound)
> + * NOTE: Inbound message MBOX 0...4 use IB channels 4...7. Therefore
> + * offset +4 is added to IB MBOX number.
> + */
> + for (i = 0; i < RIO_MAX_MBOX; i++) {
> + entries[TSI721_VECT_IMB0_RCV + i].entry =
> + TSI721_MSIX_IMSG_DQ_RCV(i + 4);
> + entries[TSI721_VECT_IMB0_INT + i].entry =
> + TSI721_MSIX_IMSG_INT(i + 4);
> + entries[TSI721_VECT_OMB0_DONE + i].entry =
> + TSI721_MSIX_OMSG_DONE(i);
> + entries[TSI721_VECT_OMB0_INT + i].entry =
> + TSI721_MSIX_OMSG_INT(i);
> + }
> +
> + err = pci_enable_msix(priv->pdev, entries, ARRAY_SIZE(entries));
> + if (err) {
> + if (err > 0)
> + dev_info(&priv->pdev->dev,
> + "Only %d MSI-X vectors available, "
> + "not using MSI-X\n", err);
> + return err;
> + }
> +
> + /*
> + * Copy MSI-X vector information into tsi721 private structure
> + */
> + priv->msix[TSI721_VECT_IDB].vector = entries[TSI721_VECT_IDB].vector;
> + snprintf(priv->msix[TSI721_VECT_IDB].irq_name, IRQ_DEVICE_NAME_MAX,
> + DRV_NAME "-idb@pci:%s", pci_name(priv->pdev));
> + priv->msix[TSI721_VECT_PWRX].vector = entries[TSI721_VECT_PWRX].vector;
> + snprintf(priv->msix[TSI721_VECT_PWRX].irq_name, IRQ_DEVICE_NAME_MAX,
> + DRV_NAME "-pwrx@pci:%s", pci_name(priv->pdev));
> +
> + for (i = 0; i < RIO_MAX_MBOX; i++) {
> + priv->msix[TSI721_VECT_IMB0_RCV + i].vector =
> + entries[TSI721_VECT_IMB0_RCV + i].vector;
> + snprintf(priv->msix[TSI721_VECT_IMB0_RCV + i].irq_name,
> + IRQ_DEVICE_NAME_MAX, DRV_NAME "-imbr%d@pci:%s",
> + i, pci_name(priv->pdev));
> +
> + priv->msix[TSI721_VECT_IMB0_INT + i].vector =
> + entries[TSI721_VECT_IMB0_INT + i].vector;
> + snprintf(priv->msix[TSI721_VECT_IMB0_INT + i].irq_name,
> + IRQ_DEVICE_NAME_MAX, DRV_NAME "-imbi%d@pci:%s",
> + i, pci_name(priv->pdev));
> +
> + priv->msix[TSI721_VECT_OMB0_DONE + i].vector =
> + entries[TSI721_VECT_OMB0_DONE + i].vector;
> + snprintf(priv->msix[TSI721_VECT_OMB0_DONE + i].irq_name,
> + IRQ_DEVICE_NAME_MAX, DRV_NAME "-ombd%d@pci:%s",
> + i, pci_name(priv->pdev));
> +
> + priv->msix[TSI721_VECT_OMB0_INT + i].vector =
> + entries[TSI721_VECT_OMB0_INT + i].vector;
> + snprintf(priv->msix[TSI721_VECT_OMB0_INT + i].irq_name,
> + IRQ_DEVICE_NAME_MAX, DRV_NAME "-ombi%d@pci:%s",
> + i, pci_name(priv->pdev));
> + }
Did we need a dependency on CONFIG_PCI_MSI?
> + return 0;
> +}
> +
>
> ...
>
> +static int tsi721_bdma_ch_init(struct tsi721_device *priv, int chnum)
> +{
> + struct tsi721_dma_desc *bd_ptr;
> + u64 *sts_ptr;
> + dma_addr_t bd_phys, sts_phys;
> + int sts_size;
> + int bd_num = priv->bdma[chnum].bd_num;
> +
> + dev_dbg(&priv->pdev->dev, "Init Block DMA Engine, CH%d\n", chnum);
> +
> + /*
> + * Initialize DMA channel for maintenance requests
> + */
> +
> + /* Allocate space for DMA descriptors */
> + bd_ptr = dma_alloc_coherent(&priv->pdev->dev,
> + bd_num * sizeof(struct tsi721_dma_desc),
> + &bd_phys, GFP_KERNEL);
> + if (!bd_ptr)
> + return -ENOMEM;
> +
> + priv->bdma[chnum].bd_phys = bd_phys;
> + priv->bdma[chnum].bd_base = bd_ptr;
> +
> + memset(bd_ptr, 0, bd_num * sizeof(struct tsi721_dma_desc));
There it is again. Perhaps we need a dma_zalloc_coherent().
> + dev_dbg(&priv->pdev->dev, "DMA descriptors @ %p (phys = %llx)\n",
> + bd_ptr, (unsigned long long)bd_phys);
> +
> + /* Allocate space for descriptor status FIFO */
> + sts_size = (bd_num >= TSI721_DMA_MINSTSSZ) ?
> + bd_num : TSI721_DMA_MINSTSSZ;
> + sts_size = roundup_pow_of_two(sts_size);
> + sts_ptr = dma_alloc_coherent(&priv->pdev->dev,
> + sts_size * sizeof(struct tsi721_dma_sts),
> + &sts_phys, GFP_KERNEL);
> + if (!sts_ptr) {
> + /* Free space allocated for DMA descriptors */
> + dma_free_coherent(&priv->pdev->dev,
> + bd_num * sizeof(struct tsi721_dma_desc),
> + bd_ptr, bd_phys);
> + priv->bdma[chnum].bd_base = NULL;
> + return -ENOMEM;
> + }
> +
> + priv->bdma[chnum].sts_phys = sts_phys;
> + priv->bdma[chnum].sts_base = sts_ptr;
> + priv->bdma[chnum].sts_size = sts_size;
> +
> + memset(sts_ptr, 0, sts_size);
and again.
> + dev_dbg(&priv->pdev->dev,
> + "desc status FIFO @ %p (phys = %llx) size=0x%x\n",
> + sts_ptr, (unsigned long long)sts_phys, sts_size);
> +
> + /* Initialize DMA descriptors ring */
> + bd_ptr[bd_num - 1].type_id = cpu_to_le32(DTYPE3 << 29);
> + bd_ptr[bd_num - 1].next_lo = cpu_to_le32((u64)bd_phys &
> + TSI721_DMAC_DPTRL_MASK);
> + bd_ptr[bd_num - 1].next_hi = cpu_to_le32((u64)bd_phys >> 32);
> +
> + /* Setup DMA descriptor pointers */
> + iowrite32(((u64)bd_phys >> 32),
> + priv->regs + TSI721_DMAC_DPTRH(chnum));
> + iowrite32(((u64)bd_phys & TSI721_DMAC_DPTRL_MASK),
> + priv->regs + TSI721_DMAC_DPTRL(chnum));
> +
> + /* Setup descriptor status FIFO */
> + iowrite32(((u64)sts_phys >> 32),
> + priv->regs + TSI721_DMAC_DSBH(chnum));
> + iowrite32(((u64)sts_phys & TSI721_DMAC_DSBL_MASK),
> + priv->regs + TSI721_DMAC_DSBL(chnum));
> + iowrite32(TSI721_DMAC_DSSZ_SIZE(sts_size),
> + priv->regs + TSI721_DMAC_DSSZ(chnum));
> +
> + /* Clear interrupt bits */
> + iowrite32(TSI721_DMAC_INT_ALL,
> + priv->regs + TSI721_DMAC_INT(chnum));
> +
> + (void)ioread32(priv->regs + TSI721_DMAC_INT(chnum));
> +
> + /* Toggle DMA channel initialization */
> + iowrite32(TSI721_DMAC_CTL_INIT, priv->regs + TSI721_DMAC_CTL(chnum));
> + (void)ioread32(priv->regs + TSI721_DMAC_CTL(chnum));
> + udelay(10);
> +
> + return 0;
> +}
> +
>
> ...
>
> +struct tsi721_dma_desc {
> + __le32 type_id;
> +
> +#define TSI721_DMAD_DEVID 0x0000ffff
> +#define TSI721_DMAD_CRF 0x00010000
> +#define TSI721_DMAD_PRIO 0x00060000
> +#define TSI721_DMAD_RTYPE 0x00780000
> +#define TSI721_DMAD_IOF 0x08000000
> +#define TSI721_DMAD_DTYPE 0xe0000000
> +
> + __le32 bcount;
> +
> +#define TSI721_DMAD_BCOUNT1 0x03ffffff /* if DTYPE == 1 */
> +#define TSI721_DMAD_BCOUNT2 0x0000000f /* if DTYPE == 2 */
> +#define TSI721_DMAD_TT 0x0c000000
> +#define TSI721_DMAD_RADDR0 0xc0000000
> +
> + union {
> + __le32 raddr_lo; /* if DTYPE == (1 || 2) */
> + __le32 next_lo; /* if DTYPE == 3 */
> + };
> +
> +#define TSI721_DMAD_CFGOFF 0x00ffffff
> +#define TSI721_DMAD_HOPCNT 0xff000000
> +
> + union {
> + __le32 raddr_hi; /* if DTYPE == (1 || 2) */
> + __le32 next_hi; /* if DTYPE == 3 */
> + };
> +
> + union {
> + struct { /* if DTYPE == 1 */
> + __le32 bufptr_lo;
> + __le32 bufptr_hi;
> + __le32 s_dist;
> + __le32 s_size;
> + } t1;
> + __le32 data[4]; /* if DTYPE == 2 */
> + u32 reserved[4]; /* if DTYPE == 3 */
> + };
> +} __attribute__((aligned(32)));
We have the __aligned helper for this. (more below)
>
> ...
>
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
