lkml.org 
[lkml]   [2003]   [Jul]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] (2.4.22-pre6 BK) New (IDE) driver: SGI IOC4
Christopher Wedgwood wrote:
> +static inline void
> +IDE_DBG(int debuglevel, char *format, ...)
> +{
> + va_list ap;
> + char tempstr[256];
> +
> + if (sgiioc4_debug >= debuglevel) {
> + va_start(ap, format);
> + vsprintf(tempstr, format, ap);
> + printk(tempstr);
> + va_end(ap);
> + }
> +}

I'm impressed that varargs static inline works. (not complaining, just
shocked :))


> +static inline void
> +xide_delay(long ticks)
> +{
> + if (!ticks)
> + return;
> +
> + current->state = TASK_INTERRUPTIBLE;
> + schedule_timeout(ticks);
> + return;
> +}

ITYM TASK_UNINTERRUPTIBLE, because you definitely don't handle being
interrupted... :)



> +unsigned int __init
> +pci_init_sgiioc4(struct pci_dev *dev, const char *name)
> +{
> + unsigned int class_rev;
> +
> + pci_read_config_dword(dev, PCI_CLASS_REVISION, &class_rev);
> + class_rev &= 0xff;

this appears to be dead code.


> + pci_enable_device(dev);

you _definitely_ want to check for errors.


> +void
> +sgiioc4_maskproc(ide_drive_t * drive, int mask)
> +{
> + ide_hwif_t *hwif = HWIF(drive);
> + if (IDE_CONTROL_REG)
> + hwif->OUTB(mask ? (drive->ctl | 2) : (drive->ctl & ~2),
> + IDE_CONTROL_REG);
> +}

I'm pretty sure you have a control reg :) Test should not be necessary.


> +void __init
> +ide_init_sgiioc4(ide_hwif_t * hwif)
> +{
> + hwif->autodma = 1;
> + hwif->index = 0; /* Channel 0 */
> + hwif->channel = 0;
> + hwif->atapi_dma = 1;
> + hwif->ultra_mask = 0x0; /* Disable Ultra DMA */
> + hwif->mwdma_mask = 0x2; /* Multimode-2 DMA */
> + hwif->swdma_mask = 0x2;
> + hwif->identify = NULL;
> + hwif->tuneproc = NULL; /* Sets timing for PIO mode */
> + hwif->speedproc = NULL; /* Sets timing for DMA &/or PIO modes */
> + hwif->selectproc = NULL; /* Use the default selection routine to select drive */
> + hwif->reset_poll = NULL; /* No HBA specific reset_poll needed */
> + hwif->pre_reset = NULL; /* No HBA specific pre_set needed */
> + hwif->resetproc = &sgiioc4_resetproc; /* Reset the IOC4 DMA engine, clear interrupts etc */
> + hwif->intrproc = NULL; /* Enable or Disable interrupt from drive */
> + hwif->maskproc = &sgiioc4_maskproc; /* Mask on/off NIEN register */
> + hwif->quirkproc = NULL;
> + hwif->busproc = NULL;

style: Under Linux we normally don't explicitly set to NULL.



> +static int
> +sgiioc4_ide_dma_lostirq(ide_drive_t * drive)
> +{
> + if (HWIF(drive)->resetproc != NULL)
> + HWIF(drive)->resetproc(drive);
> + return __ide_dma_lostirq(drive);
> +}

don't you know if you have a resetproc? :)


> +static u8
> +sgiioc4_INB(unsigned long port)
> +{
> + u8 reg = (u8) inb(port);
> +
> + if ((port & 0xFFF) == 0x11C) { /* Status register of IOC4 */
> + if (reg & 0x50) { /* Not busy...check for interrupt */

hmmm, you probably want OK_STAT macro invocation that's used throughout
the IDE code.


> +/* Creates a dma map for the scatter-gather list entries */
> +void __init
> +ide_dma_sgiioc4(ide_hwif_t * hwif, unsigned long dma_base)
> +{
> + int num_ports = sizeof (ioc4_dma_regs_t);
> +
> + IDE_DBG(0, "%s: BM-DMA at 0x%04lx-0x%04lx\n", hwif->name, dma_base,
> + dma_base + num_ports - 1);
> + if (check_region(dma_base, num_ports)) {
> + IDE_DBG(0,
> + "ide_dma_sgiioc4(%s) -- Error, Port Addresses 0x%p to 0x%p ALREADY in use\n",
> + hwif->name, dma_base, dma_base + num_ports - 1);
> + return;
> + }
> + request_region(dma_base, num_ports, hwif->name);

deprecated from the get-go... eliminate check_region, and test
request_region return value.


> + hwif->dma_base = dma_base;
> + hwif->dmatable_cpu = pci_alloc_consistent(hwif->pci_dev,
> + IOC4_PRD_ENTRIES * IOC4_PRD_BYTES, /* 1 Page */
> + &hwif->dmatable_dma);
> + if (hwif->dmatable_cpu == NULL)
> + goto dma_alloc_failure;
> +
> + hwif->sg_table = kmalloc(sizeof (struct scatterlist) * IOC4_PRD_ENTRIES,
> + GFP_KERNEL);
> + if (hwif->sg_table == NULL) {
> + pci_free_consistent(hwif->pci_dev,
> + IOC4_PRD_ENTRIES * IOC4_PRD_BYTES,
> + hwif->dmatable_cpu, hwif->dmatable_dma);
> + goto dma_alloc_failure;
> + }
> +
> +
> + hwif->dma_base2 = (unsigned long) pci_alloc_consistent(hwif->pci_dev,
> + IOC4_IDE_CACHELINE_SIZE,
> + (dma_addr_t*)&(hwif->dma_status));
> + if (!(hwif->dma_base2)) {
> + IDE_DBG(0,
> + "ide_dma_sgiioc4(%s) - Couldnot allocate map for Ending DMA \n",
> + hwif->name);
> + }

no error handling for this last alloc, unlike the previous two?


> +/* Sets the ATAPI Drive to do DMA in Multimode-2 */
> +void
> +sgiioc4_config_drive_for_dma(ide_drive_t * drive)
> +{
> + u8 status_val;
> + ide_hwif_t *hwif = HWIF(drive);
> +
> + hwif->OUTB(drive->select.all, IDE_SELECT_REG);
> + hwif->OUTB(drive->ctl | 2, IDE_CONTROL_REG);
> + hwif->OUTB(0x0, IDE_DATA_REG);
> + hwif->OUTB(0x0, IDE_SECTOR_REG);
> + hwif->OUTB(0x0, IDE_LCYL_REG);
> + hwif->OUTB(0x0, IDE_HCYL_REG);
> + hwif->OUTB(0x22, IDE_NSECTOR_REG);
> + hwif->OUTB(0x03, IDE_FEATURE_REG);
> + hwif->OUTB(0xEF, IDE_COMMAND_REG);
> +
> + do {
> + xide_delay(HZ);
> + status_val = hwif->INB(IDE_STATUS_REG);
> + } while (status_val & 0x80);
> +
> + if (status_val == 0x50) {
> + IDE_DBG(1, "Successfully set %s in Multimode-2 DMA mode \n",
> + drive->name);
> + drive->using_dma = 1;
> + } else { /* Go to PIO mode if this occurs */
> + IDE_DBG(0,
> + "Couldnot set %s in Multimode-2 DMA mode | status 0x%x | Drive %s using PIO \n",
> + drive->name, status_val, drive->name);
> + drive->using_dma = 0;
> + }
> + return;
> +}

isn't there a standard function to do this?

This is the standard method of setting device transfer mode.


> +/* Creates the scatter gather list, DMA Table */
> +unsigned int
> +sgiioc4_build_dma_table(ide_drive_t * drive, struct request *rq, int ddir)
> +{
> + ide_hwif_t *hwif = HWIF(drive);
> + unsigned int *table = hwif->dmatable_cpu;
> + unsigned int count = 0, i = 1;
> + struct scatterlist *sg;
> +
> + if (rq->cmd == IDE_DRIVE_TASKFILE)
> + hwif->sg_nents = i = sgiioc4_ide_raw_build_sglist(hwif, rq);
> + else
> + hwif->sg_nents = i = sgiioc4_ide_build_sglist(hwif, rq, ddir);
> + if (!i)
> + return 0; /* sglist of length Zero */
> +
> + sg = hwif->sg_table;
> + while (i && sg_dma_len(sg)) {
> + dma_addr_t cur_addr;
> + int cur_len;
> + cur_addr = sg_dma_address(sg);
> + cur_len = sg_dma_len(sg);
> +
> + while (cur_len) {
> + if (count++ >= IOC4_PRD_ENTRIES) {
> + printk("%s: DMA table too small\n",
> + drive->name);
> + goto use_pio_instead;
> + } else {
> + uint32_t xcount, bcount =
> + 0x10000 - (cur_addr & 0xffff);
> +
> + if (bcount > cur_len)
> + bcount = cur_len;
> +
> + /* put the addr, length in the IOC4 dma-table format */
> + *table = 0x0;
> + table++;
> + *table = cpu_to_be32(cur_addr);
> + table++;
> + *table = 0x0;
> + table++;
> +
> + xcount = bcount & 0xffff;
> + *table = cpu_to_be32(xcount);
> + table++;
> +
> + cur_addr += bcount;
> + cur_len -= bcount;
> + }
> + }
> +
> + sg++;
> + i--;
> + }
> +
> + if (count) {
> + table--;
> + *table |= cpu_to_be32(0x80000000);
> + return count;
> + }
> + use_pio_instead:
> + pci_unmap_sg(hwif->pci_dev,
> + hwif->sg_table, hwif->sg_nents, hwif->sg_dma_direction);
> + hwif->sg_dma_active = 0;
> + return 0; /* revert to PIO for this request */
> +}

hrm... did all this code really need duplicating from ide-dma.c?


> +static int
> +sgiioc4_clearirq(ide_drive_t * drive)
> +{
> + u32 intr_reg;
> + ide_hwif_t *hwif = HWIF(drive);
> + ide_ioreg_t other_ir =
> + hwif->io_ports[IDE_IRQ_OFFSET] + (IOC4_INTR_REG << 2);
> +
> + /* Code to check for PCI error conditions */
> + intr_reg = hwif->INL(other_ir);
> + if (intr_reg & 0x03) {
> + /* Valid IOC4-IDE interrupt */
> + u8 stat = hwif->INB(IDE_STATUS_REG);
> + int count = 0;
> + do {
> + xide_delay(count);
> + stat = hwif->INB(IDE_STATUS_REG); /* Removes Interrupt from IDE Device */
> + } while ((stat & 0x80) && (count++ < 1024));
> +
> + if (intr_reg & 0x02) {
> + /* Error when transferring DMA data on PCI bus */
> + uint32_t pci_err_addr_low, pci_err_addr_high,
> + pci_stat_cmd_reg;
> +
> + pci_err_addr_low =
> + hwif->INL(hwif->io_ports[IDE_IRQ_OFFSET]);
> + pci_err_addr_high =
> + hwif->INL(hwif->io_ports[IDE_IRQ_OFFSET] + 4);
> + pci_read_config_dword(hwif->pci_dev, PCI_COMMAND,
> + &pci_stat_cmd_reg);
> + IDE_DBG(0,
> + "sgiioc4_clearirq(%s) : PCI Bus Error when doing DMA : status-cmd reg is 0x%x \n",
> + drive->name, pci_stat_cmd_reg);
> + IDE_DBG(0,
> + "sgiioc4_clearirq(%s) : PCI Error Address is 0x%x%x \n",
> + drive->name, pci_err_addr_high,
> + pci_err_addr_low);
> + /* Clear the PCI Error indicator */
> + pci_write_config_dword(hwif->pci_dev, PCI_COMMAND,
> + 0x00000146);
> + }
> + hwif->OUTL(0x03, other_ir); /* Clear the Interrupt, Error bits on the IOC4 */
> +
> + intr_reg = hwif->INL(other_ir);
> + }
> + return intr_reg;
> +}

nice... this code is actually applicable to several other PCI host
controllers, too.




> +/* Weeds out non-IDE interrupts to the IOC4 */
> +#define ide_ack_intr(hwif) ((hwif)->hw.ack_intr ? (hwif)->hw.ack_intr(hwif) : 1)

does this deal with shared interrupts too?



-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 13:46    [W:0.058 / U:0.016 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site