lkml.org 
[lkml]   [2005]   [Jul]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH linux-2.6.13-rc3] SATA: rewritten sil24 driver
Tejun Heo wrote:
> Hello, Jeff.
>
> This is rewritten sil24 driver against v2.6.13-rc3. It seems to work
> and am currently running stress test on it (random raw read of
> concurrency 4, repeatitive mount/copy/checksup/unmount). I'll keep
> running stress test for at least 12 hours and let you know if
> something goes wrong. I've also tested basic error handling and it
> seems to work.

I've merged this into the 'sil24' branch of libata-dev.git, and moved
the original driver from Silicon Image into the 'sil24-original' branch.

So, please submit an incremental patch for any future changes to this
driver.

Comments below, need a few fixups before pushing upstream, most likely.

Also, a question: do you have hardware docs?


> diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
> --- a/drivers/scsi/Kconfig
> +++ b/drivers/scsi/Kconfig
> @@ -499,6 +499,14 @@ config SCSI_SATA_SIL
>
> If unsure, say N.
>
> +config SCSI_SATA_SIL24
> + tristate "Silicon Image 3124/3132 SATA support"
> + depends on SCSI_SATA && PCI && EXPERIMENTAL
> + help
> + This option enables support for Silicon Image 3124/3132 Serial ATA.
> +
> + If unsure, say N.
> +
> config SCSI_SATA_SIS
> tristate "SiS 964/180 SATA support"
> depends on SCSI_SATA && PCI && EXPERIMENTAL
> diff --git a/drivers/scsi/Makefile b/drivers/scsi/Makefile
> --- a/drivers/scsi/Makefile
> +++ b/drivers/scsi/Makefile
> @@ -126,6 +126,7 @@ obj-$(CONFIG_SCSI_ATA_PIIX) += libata.o
> obj-$(CONFIG_SCSI_SATA_PROMISE) += libata.o sata_promise.o
> obj-$(CONFIG_SCSI_SATA_QSTOR) += libata.o sata_qstor.o
> obj-$(CONFIG_SCSI_SATA_SIL) += libata.o sata_sil.o
> +obj-$(CONFIG_SCSI_SATA_SIL24) += libata.o sata_sil24.o
> obj-$(CONFIG_SCSI_SATA_VIA) += libata.o sata_via.o
> obj-$(CONFIG_SCSI_SATA_VITESSE) += libata.o sata_vsc.o
> obj-$(CONFIG_SCSI_SATA_SIS) += libata.o sata_sis.o
> diff --git a/drivers/scsi/sata_sil24.c b/drivers/scsi/sata_sil24.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/scsi/sata_sil24.c
> @@ -0,0 +1,786 @@
> +/*
> + * sata_sil24.c - Driver for Silicon Image 3124/3132 SATA-2 controllers
> + *
> + * Copyright 2005 Tejun Heo
> + *
> + * Based on preview driver from Silicon Image.
> + *
> + * NOTE: No NCQ/ATAPI support yet. The preview driver didn't support
> + * NCQ nor ATAPI, and, unfortunately, I couldn't find out how to make
> + * those work. Enabling those shouldn't be difficult. Basic
> + * structure is all there (in libata-dev tree). If you have any
> + * information about this hardware, please contact me or linux-ide.
> + * Info is needed on...
> + *
> + * - How to issue tagged commands and turn on sactive on issue accordingly.
> + * - Where to put an ATAPI command and how to tell the device to send it.
> + * - How to enable/use 64bit.
> + *
> + * 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, or (at your option) any
> + * later version.
> + *
> + * 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.
> + *
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/blkdev.h>
> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/dma-mapping.h>
> +#include <scsi/scsi_host.h>
> +#include "scsi.h"
> +#include <linux/libata.h>
> +#include <asm/io.h>
> +
> +#define DRV_NAME "sata_sil24"
> +#define DRV_VERSION "0.20" /* Silicon Image's preview driver was 0.10 */
> +
> +#define NR_PORTS 4
> +
> +/*
> + * Port request block (PRB) 32 bytes
> + */
> +struct sil24_prb {
> + u16 ctrl;
> + u16 prot;
> + u32 rx_cnt;
> + u8 fis[6 * 4];
> +};
> +
> +/*
> + * Scatter gather entry (SGE) 16 bytes
> + */
> +struct sil24_sge {
> + u64 addr;
> + u32 cnt;
> + u32 flags;
> +};
> +
> +/*
> + * Port multiplier
> + */
> +struct sil24_port_multiplier {
> + u32 diag;
> + u32 sactive;
> +};

I know this is unlikely, but just asking... you didn't test this with a
port multiplier, did you?


> +enum {
> + /*
> + * Global controller registers (128 bytes @ BAR0)
> + */
> + /* 32 bit regs */
> + HOST_SLOT_STAT = 0x00, /* 32 bit slot stat * 4 */
> + HOST_CTRL = 0x40,
> + HOST_IRQ_STAT = 0x44,
> + HOST_PHY_CFG = 0x48,
> + HOST_BIST_CTRL = 0x50,
> + HOST_BIST_PTRN = 0x54,
> + HOST_BIST_STAT = 0x58,
> + HOST_MEM_BIST_STAT = 0x5c,
> + HOST_FLASH_CMD = 0x70,
> + /* 8 bit regs */
> + HOST_FLASH_DATA = 0x74,
> + HOST_TRANSITION_DETECT = 0x75,
> + HOST_GPIO_CTRL = 0x76,
> + HOST_I2C_ADDR = 0x78, /* 32 bit */
> + HOST_I2C_DATA = 0x7c,
> + HOST_I2C_XFER_CNT = 0x7e,
> + HOST_I2C_CTRL = 0x7f,
> +
> + /* HOST_SLOT_STAT bits */
> + HOST_SSTAT_ATTN = (1 << 31),
> +
> + /*
> + * Port registers
> + * (8192 bytes @ +0x0000, +0x2000, +0x4000 and +0x6000 @ BAR2)
> + */
> + PORT_REGS_SIZE = 0x2000,
> + PORT_PRB = 0x0000, /* (32 bytes PRB + 16 bytes SGEs * 6) * 31 (3968 bytes) */
> + /* TF is overlayed w/ PRB regs in the preview driver,
> + * but it doesn't seem to work. */
> + PORT_TF = 0x0000,
> +
> + PORT_PM = 0x0f80, /* 8 bytes PM * 16 (128 bytes) */
> + /* 32 bit regs */
> + PORT_CTRL_STAT = 0x1000, /* write:ctrl, read:stat */
> + PORT_CTRL_CLR = 0x1004,
> + PORT_IRQ_STAT = 0x1008,
> + PORT_IRQ_ENABLE_SET = 0x1010,
> + PORT_IRQ_ENABLE_CLR = 0x1014,
> + PORT_ACTIVATE_UPPER_ADDR= 0x101c,
> + PORT_EXEC_FIFO = 0x1020,
> + PORT_CMD_ERR = 0x1024,
> + PORT_FIS_CFG = 0x1028,
> + PORT_FIFO_THRES = 0x102c,
> + /* 16 bit regs */
> + PORT_DECODE_ERR_CNT = 0x1040,
> + PORT_DECODE_ERR_THRESH = 0x1042,
> + PORT_CRC_ERR_CNT = 0x1044,
> + PORT_CRC_ERR_THRESH = 0x1046,
> + PORT_HSHK_ERR_CNT = 0x1048,
> + PORT_HSHK_ERR_THRESH = 0x104a,
> + /* 32 bit regs */
> + PORT_PHY_CFG = 0x1050,
> + PORT_SLOT_STAT = 0x1800,
> + PORT_CMD_ACTIVATE = 0x1c00, /* 64 bit cmd activate * 31 (248 bytes) */
> + PORT_EXEC_DIAG = 0x1e00, /* 32bit exec diag * 16 (64 bytes, 0-10 used on 3124) */
> + PORT_PSD_DIAG = 0x1e40, /* 32bit psd diag * 16 (64 bytes, 0-8 used on 3124) */
> + PORT_SCONTROL = 0x1f00,
> + PORT_SSTATUS = 0x1f04,
> + PORT_SERROR = 0x1f08,
> + PORT_SACTIVE = 0x1f0c,
> +
> + /* PORT_CTRL_STAT bits */
> + PORT_CS_PORT_RST = (1 << 0), /* port reset */
> + PORT_CS_DEV_RST = (1 << 1), /* device reset */
> + PORT_CS_INIT = (1 << 2), /* port initialize */
> + PORT_CS_IRQ_WOC = (1 << 3), /* interrupt write one to clear */
> + PORT_CS_RESUME = (1 << 4), /* port resume */
> + PORT_CS_32BIT_ACTV = (1 << 5), /* 32-bit activation */
> + PORT_CS_PM_EN = (1 << 6), /* port multiplier enable */
> + PORT_CS_RDY = (1 << 7), /* port ready to accept commands */
> +
> + /* PORT_IRQ_STAT/ENABLE_SET/CLR */
> + /* bits[11:0] are masked */
> + PORT_IRQ_COMPLETE = (1 << 0), /* command(s) completed */
> + PORT_IRQ_ERROR = (1 << 1), /* command execution error */
> + PORT_IRQ_PORTRDY_CHG = (1 << 2), /* port ready change */
> + PORT_IRQ_PWR_CHG = (1 << 3), /* power management change */
> + PORT_IRQ_PHYRDY_CHG = (1 << 4), /* PHY ready change */
> + PORT_IRQ_COMWAKE = (1 << 5), /* COMWAKE received */
> + PORT_IRQ_UNK_FIS = (1 << 6), /* Unknown FIS received */
> + PORT_IRQ_SDB_FIS = (1 << 11), /* SDB FIS received */
> +
> + /* bits[27:16] are unmasked (raw) */
> + PORT_IRQ_RAW_SHIFT = 16,
> + PORT_IRQ_MASKED_MASK = 0x7ff,
> + PORT_IRQ_RAW_MASK = (0x7ff << PORT_IRQ_RAW_SHIFT),
> +
> + /* ENABLE_SET/CLR specific, intr steering - 2 bit field */
> + PORT_IRQ_STEER_SHIFT = 30,
> + PORT_IRQ_STEER_MASK = (3 << PORT_IRQ_STEER_SHIFT),
> +
> + /* PORT_CMD_ERR constants */
> + PORT_CERR_DEV = 1, /* Error bit in D2H Register FIS */
> + PORT_CERR_SDB = 2, /* Error bit in SDB FIS */
> + PORT_CERR_DATA = 3, /* Error in data FIS not detected by dev */
> + PORT_CERR_SEND = 4, /* Initial cmd FIS transmission failure */
> + PORT_CERR_INCONSISTENT = 5, /* Protocol mismatch */
> + PORT_CERR_DIRECTION = 6, /* Data direction mismatch */
> + PORT_CERR_UNDERRUN = 7, /* Ran out of SGEs while writing */
> + PORT_CERR_OVERRUN = 8, /* Ran out of SGEs while reading */
> + PORT_CERR_PKT_PROT = 11, /* DIR invalid in 1st PIO setup of ATAPI */
> + PORT_CERR_SGT_BOUNDARY = 16, /* PLD ecode 00 - SGT not on qword boundary */
> + PORT_CERR_SGT_TGTABRT = 17, /* PLD ecode 01 - target abort */
> + PORT_CERR_SGT_MSTABRT = 18, /* PLD ecode 10 - master abort */
> + PORT_CERR_SGT_PCIPERR = 19, /* PLD ecode 11 - PCI parity err while fetching SGT */
> + PORT_CERR_CMD_BOUNDARY = 24, /* ctrl[15:13] 001 - PRB not on qword boundary */
> + PORT_CERR_CMD_TGTABRT = 25, /* ctrl[15:13] 010 - target abort */
> + PORT_CERR_CMD_MSTABRT = 26, /* ctrl[15:13] 100 - master abort */
> + PORT_CERR_CMD_PCIPERR = 27, /* ctrl[15:13] 110 - PCI parity err while fetching PRB */
> + PORT_CERR_XFR_UNDEF = 32, /* PSD ecode 00 - undefined */
> + PORT_CERR_XFR_TGTABRT = 33, /* PSD ecode 01 - target abort */
> + PORT_CERR_XFR_MSGABRT = 34, /* PSD ecode 10 - master abort */
> + PORT_CERR_XFR_PCIPERR = 35, /* PSD ecode 11 - PCI prity err during transfer */
> + PORT_CERR_SENDSERVICE = 36, /* FIS received whiel sending service */
> +
> + /*
> + * Other constants
> + */
> + SGE_TRM = (1 << 31), /* Last SGE in chain */
> + PRB_SOFT_RST = (1 << 7), /* Soft reset request (ign BSY?) */
> +
> + /* board id */
> + BID_SIL3124 = 0,
> + BID_SIL3132 = 1,
> +
> + IRQ_STAT_4PORTS = 0xf,
> +};
> +
> +struct sil24_cmd_block {
> + struct sil24_prb prb;
> + struct sil24_sge sge[LIBATA_MAX_PRD];
> +};
> +
> +/*
> + * ap->private_data
> + *
> + * The preview driver always returned 0 for status. We emulate it
> + * here from the previous interrupt.
> + */
> +struct sil24_port_priv {
> + void *port;
> + struct sil24_cmd_block *cmd_block; /* 32 cmd blocks */
> + dma_addr_t cmd_block_dma; /* DMA base addr for them */
> +};
> +
> +/* ap->host_set->private_data */
> +struct sil24_host_priv {
> + void *host_base; /* global controller control (128 bytes @BAR0) */
> + void *port_base; /* port registers (4 * 8192 bytes @BAR2) */
> +};
> +
> +static u8 sil24_check_status(struct ata_port *ap);
> +static u8 sil24_check_err(struct ata_port *ap);
> +static u32 sil24_scr_read(struct ata_port *ap, unsigned sc_reg);
> +static void sil24_scr_write(struct ata_port *ap, unsigned sc_reg, u32 val);
> +static void sil24_phy_reset(struct ata_port *ap);
> +static void sil24_qc_prep(struct ata_queued_cmd *qc);
> +static int sil24_qc_issue(struct ata_queued_cmd *qc);
> +static void sil24_irq_clear(struct ata_port *ap);
> +static void sil24_eng_timeout(struct ata_port *ap);
> +static irqreturn_t sil24_interrupt(int irq, void *dev_instance, struct pt_regs *regs);
> +static int sil24_port_start(struct ata_port *ap);
> +static void sil24_port_stop(struct ata_port *ap);
> +static void sil24_host_stop(struct ata_host_set *host_set);
> +static int sil24_init_one(struct pci_dev *pdev, const struct pci_device_id *ent);
> +
> +static struct pci_device_id sil24_pci_tbl[] = {
> + { 0x1095, 0x3124, PCI_ANY_ID, PCI_ANY_ID, 0, 0, BID_SIL3124 },
> + { 0x1095, 0x3132, PCI_ANY_ID, PCI_ANY_ID, 0, 0, BID_SIL3132 },
> +};
> +
> +static struct pci_driver sil24_pci_driver = {
> + .name = DRV_NAME,
> + .id_table = sil24_pci_tbl,
> + .probe = sil24_init_one,
> + .remove = ata_pci_remove_one, /* safe? */
> +};
> +
> +static Scsi_Host_Template sil24_sht = {
> + .module = THIS_MODULE,
> + .name = DRV_NAME,
> + .ioctl = ata_scsi_ioctl,
> + .queuecommand = ata_scsi_queuecmd,
> + .eh_strategy_handler = ata_scsi_error,
> + .can_queue = ATA_DEF_QUEUE,
> + .this_id = ATA_SHT_THIS_ID,
> + .sg_tablesize = LIBATA_MAX_PRD,
> + .max_sectors = ATA_MAX_SECTORS,
> + .cmd_per_lun = ATA_SHT_CMD_PER_LUN,
> + .emulated = ATA_SHT_EMULATED,
> + .use_clustering = ATA_SHT_USE_CLUSTERING,
> + .proc_name = DRV_NAME,
> + .dma_boundary = ATA_DMA_BOUNDARY,
> + .slave_configure = ata_scsi_slave_config,
> + .bios_param = ata_std_bios_param,
> + .ordered_flush = 1, /* NCQ not supported yet */
> +};
> +
> +static struct ata_port_operations sil24_ops = {
> + .port_disable = ata_port_disable,
> +
> + .check_status = sil24_check_status,
> + .check_altstatus = sil24_check_status,
> + .check_err = sil24_check_err,
> + .dev_select = ata_noop_dev_select,
> +
> + .phy_reset = sil24_phy_reset,
> +
> + .qc_prep = sil24_qc_prep,
> + .qc_issue = sil24_qc_issue,
> +
> + .eng_timeout = sil24_eng_timeout,
> +
> + .irq_handler = sil24_interrupt,
> + .irq_clear = sil24_irq_clear,
> +
> + .scr_read = sil24_scr_read,
> + .scr_write = sil24_scr_write,
> +
> + .port_start = sil24_port_start,
> + .port_stop = sil24_port_stop,
> + .host_stop = sil24_host_stop,
> +};
> +
> +static struct ata_port_info sil24_port_info[] = {
> + /* sil_3124 */
> + {
> + .sht = &sil24_sht,
> + .host_flags = ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
> + ATA_FLAG_SATA_RESET | ATA_FLAG_MMIO |
> + ATA_FLAG_PIO_DMA,
> + .pio_mask = 0x1f, /* pio0-4 */
> + .mwdma_mask = 0x07, /* mwdma0-2 */
> + .udma_mask = 0x3f, /* udma0-5 */
> + .port_ops = &sil24_ops,
> + },
> + /* sil_3132 */
> + {
> + .sht = &sil24_sht,
> + .host_flags = ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
> + ATA_FLAG_SATA_RESET | ATA_FLAG_MMIO |
> + ATA_FLAG_PIO_DMA,
> + .pio_mask = 0x1f, /* pio0-4 */
> + .mwdma_mask = 0x07, /* mwdma0-2 */
> + .udma_mask = 0x3f, /* udma0-5 */
> + .port_ops = &sil24_ops,
> + },
> +};
> +
> +static u8 sil24_check_status(struct ata_port *ap)
> +{
> + return ATA_DRDY;
> +}
> +
> +static u8 sil24_check_err(struct ata_port *ap)
> +{
> + return 0;
> +}

For these two functions, we need to grab the values for these registers
from the D2H Register FIS. These should not be constant values, they
are used in probing.


> +static int sil24_scr_map[] = {
> + [SCR_CONTROL] = 0,
> + [SCR_STATUS] = 1,
> + [SCR_ERROR] = 2,
> + [SCR_ACTIVE] = 3,
> +};
> +
> +static u32 sil24_scr_read(struct ata_port *ap, unsigned sc_reg)
> +{
> + void *scr_addr = (void *)ap->ioaddr.scr_addr;
> + if (sc_reg < ARRAY_SIZE(sil24_scr_map)) {
> + void *addr;
> + addr = scr_addr + sil24_scr_map[sc_reg] * 4;
> + return readl(scr_addr + sil24_scr_map[sc_reg] * 4);
> + }
> + return 0xffffffffU;
> +}
> +
> +static void sil24_scr_write(struct ata_port *ap, unsigned sc_reg, u32 val)
> +{
> + void *scr_addr = (void *)ap->ioaddr.scr_addr;
> + if (sc_reg < ARRAY_SIZE(sil24_scr_map)) {
> + void *addr;
> + addr = scr_addr + sil24_scr_map[sc_reg] * 4;
> + writel(val, scr_addr + sil24_scr_map[sc_reg] * 4);
> + }
> +}
> +
> +static void sil24_phy_reset(struct ata_port *ap)
> +{
> + __sata_phy_reset(ap);
> + /*
> + * No ATAPI yet. Just unconditionally indicate ATA device.
> + * If ATAPI device is attached, it will fail ATA_CMD_ID_ATA
> + * and libata core will ignore the device.
> + */
> + if (!(ap->flags & ATA_FLAG_PORT_DISABLED))
> + ap->device[0].class = ATA_DEV_ATA;
> +}

We need to use the standard probing code to figure this out.
ata_dev_classify(), etc.


> +static inline void sil24_fill_sg(struct ata_queued_cmd *qc,
> + struct sil24_cmd_block *cb)
> +{
> + struct scatterlist *sg = qc->sg;
> + struct sil24_sge *sge = cb->sge;
> + unsigned i;
> +
> + for (i = 0; i < qc->n_elem; i++, sg++, sge++) {
> + sge->addr = cpu_to_le64(sg_dma_address(sg));
> + sge->cnt = cpu_to_le32(sg_dma_len(sg));
> + sge->flags = 0;
> + sge->flags = i < qc->n_elem - 1 ? 0 : cpu_to_le32(SGE_TRM);
> + }
> +}
> +
> +static void sil24_qc_prep(struct ata_queued_cmd *qc)
> +{
> + struct ata_port *ap = qc->ap;
> + struct sil24_port_priv *pp = ap->private_data;
> + struct sil24_cmd_block *cb = pp->cmd_block + qc->tag;
> + struct sil24_prb *prb = &cb->prb;
> +
> + switch (qc->tf.protocol) {
> + case ATA_PROT_PIO:
> + case ATA_PROT_DMA:
> + case ATA_PROT_NODATA:
> + break;
> + default:
> + /* ATAPI isn't supported yet */
> + BUG();
> + }
> +
> + ata_tf_to_fis(&qc->tf, prb->fis, 0);
> +
> + if (qc->flags & ATA_QCFLAG_DMAMAP)
> + sil24_fill_sg(qc, cb);
> +}
> +
> +static int sil24_qc_issue(struct ata_queued_cmd *qc)
> +{
> + struct ata_port *ap = qc->ap;
> + struct sil24_port_priv *pp = ap->private_data;
> + dma_addr_t paddr = pp->cmd_block_dma + qc->tag * sizeof(*pp->cmd_block);
> +
> + writel((u32)paddr, pp->port + PORT_CMD_ACTIVATE);
> + return 0;
> +}
> +
> +static void sil24_irq_clear(struct ata_port *ap)
> +{
> + /* unused */
> +}

we need to fill this in.


> +static void sil24_reset_controller(struct ata_port *ap)
> +{
> + struct sil24_port_priv *pp = ap->private_data;
> + void *port = pp->port;
> + int cnt;
> + u32 tmp;
> +
> + printk(KERN_NOTICE DRV_NAME
> + " ata%u: resetting controller...\n", ap->id);
> +
> + /* Reset controller state. Is this correct? */
> + writel(PORT_CS_DEV_RST, port + PORT_CTRL_STAT);
> + readl(port + PORT_CTRL_STAT); /* sync */
> +
> + /* Max ~100ms */
> + for (cnt = 0; cnt < 1000; cnt++) {
> + udelay(100);
> + tmp = readl(port + PORT_CTRL_STAT);
> + if (!(tmp & PORT_CS_DEV_RST))
> + break;
> + }

Use mdelay. We should be able to sleep here?

Either way, we want to avoid long delays like this.


> + if (tmp & PORT_CS_DEV_RST)
> + printk(KERN_ERR DRV_NAME
> + " ata%u: failed to reset controller\n", ap->id);
> +}
> +
> +static void sil24_eng_timeout(struct ata_port *ap)
> +{
> + struct ata_queued_cmd *qc;
> +
> + qc = ata_qc_from_tag(ap, ap->active_tag);
> + if (!qc) {
> + printk(KERN_ERR "ata%u: BUG: tiemout without command\n",
> + ap->id);
> + return;
> + }
> +
> + /*
> + * hack alert! We cannot use the supplied completion
> + * function from inside the ->eh_strategy_handler() thread.
> + * libata is the only user of ->eh_strategy_handler() in
> + * any kernel, so the default scsi_done() assumes it is
> + * not being called from the SCSI EH.
> + */
> + printk(KERN_ERR "ata%u: command timeout\n", ap->id);
> + qc->scsidone = scsi_finish_command;
> + ata_qc_complete(qc, ATA_ERR);
> +
> + sil24_reset_controller(ap);
> +}
> +
> +static inline void sil24_host_intr(struct ata_port *ap)
> +{
> + struct ata_queued_cmd *qc = ata_qc_from_tag(ap, ap->active_tag);
> + struct sil24_port_priv *pp = ap->private_data;
> + void *port = pp->port;
> + u32 slot_stat;
> +
> + slot_stat = readl(port + PORT_SLOT_STAT);
> + if (!(slot_stat & HOST_SSTAT_ATTN)) {
> + if (qc)
> + ata_qc_complete(qc, 0);
> + } else {
> + u32 irq_stat, cmd_err, sstatus, serror;
> +
> + irq_stat = readl(port + PORT_IRQ_STAT);
> + cmd_err = readl(port + PORT_CMD_ERR);
> + sstatus = readl(port + PORT_SSTATUS);
> + serror = readl(port + PORT_SERROR);
> +
> + /* Clear IRQ/errors */
> + writel(irq_stat, port + PORT_IRQ_STAT);
> + if (cmd_err)
> + writel(cmd_err, port + PORT_CMD_ERR);
> + if (serror)
> + writel(serror, port + PORT_SERROR);
> +
> + printk(KERN_ERR DRV_NAME " ata%u: error interrupt on port%d\n"
> + " stat=0x%x irq=0x%x cmd_err=%d sstatus=0x%x serror=0x%x\n",
> + ap->id, ap->port_no, slot_stat, irq_stat, cmd_err, sstatus, serror);
> +
> + if (qc)
> + ata_qc_complete(qc, ATA_ERR);
> +
> + sil24_reset_controller(ap);
> + }
> +}

Two comments:
1) The "OK" test for command completion seems quite fragile. I think
the code may be -assuming- a command is completed. I'll have to check
the docs to see if HOST_IRQ_STAT + no-other-events truly guarantees an
indication of command completion.

2) Error handling should be moved out-of-line, in a separate function.


> +static irqreturn_t sil24_interrupt(int irq, void *dev_instance, struct pt_regs *regs)
> +{
> + struct ata_host_set *host_set = dev_instance;
> + struct sil24_host_priv *hpriv = host_set->private_data;
> + unsigned handled = 0;
> + u32 status;
> + int i;
> +
> + status = readl(hpriv->host_base + HOST_IRQ_STAT);
> +
> + if (!(status & IRQ_STAT_4PORTS))
> + goto out;

also check for status==0xffffffff to indicate PCI bus fault, or hardware
unplug.


> + spin_lock(&host_set->lock);
> +
> + for (i = 0; i < host_set->n_ports; i++)
> + if (status & (1 << i)) {
> + struct ata_port *ap = host_set->ports[i];
> + if (ap && !(ap->flags & ATA_FLAG_PORT_DISABLED))
> + sil24_host_intr(host_set->ports[i]);
> + else {
> + u32 tmp;
> + printk(KERN_WARNING DRV_NAME
> + ": spurious interrupt from port %d\n", i);
> + tmp = readl(hpriv->host_base + HOST_CTRL);
> + tmp &= ~(1 << i);
> + writel(tmp, hpriv->host_base + HOST_CTRL);

add a comment describing what these three lines of code do


> + }
> + handled++;
> + }
> +
> + spin_unlock(&host_set->lock);
> + out:
> + return IRQ_RETVAL(handled);
> +}
> +
> +static int sil24_port_start(struct ata_port *ap)
> +{
> + struct device *dev = ap->host_set->dev;
> + struct sil24_host_priv *hpriv = ap->host_set->private_data;
> + struct sil24_port_priv *pp;
> + struct sil24_cmd_block *cb;
> + size_t cb_size = sizeof(*cb);
> + dma_addr_t cb_dma;
> +
> + pp = kmalloc(sizeof(*pp), GFP_KERNEL);
> + if (!pp)
> + return -ENOMEM;
> + memset(pp, 0, sizeof(*pp));
> +
> + cb = dma_alloc_coherent(dev, cb_size, &cb_dma, GFP_KERNEL);
> + if (!cb) {
> + kfree(pp);
> + return -ENOMEM;
> + }
> + memset(cb, 0, cb_size);
> +
> + pp->port = hpriv->port_base + ap->port_no * PORT_REGS_SIZE;
> + pp->cmd_block = cb;
> + pp->cmd_block_dma = cb_dma;
> +
> + ap->private_data = pp;
> +
> + return 0;
> +}
> +
> +static void sil24_port_stop(struct ata_port *ap)
> +{
> + struct device *dev = ap->host_set->dev;
> + struct sil24_port_priv *pp = ap->private_data;
> + size_t cb_size = sizeof(*pp->cmd_block);
> +
> + dma_free_coherent(dev, cb_size, pp->cmd_block, pp->cmd_block_dma);
> + kfree(pp);
> +}
> +
> +static void sil24_host_stop(struct ata_host_set *host_set)
> +{
> + struct sil24_host_priv *hpriv = host_set->private_data;
> +
> + iounmap(hpriv->host_base);
> + iounmap(hpriv->port_base);
> + kfree(hpriv);
> +}
> +
> +static int sil24_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
> +{
> + static int printed_version = 0;
> + unsigned int board_id = (unsigned int)ent->driver_data;
> + struct ata_probe_ent *probe_ent = NULL;
> + struct sil24_host_priv *hpriv = NULL;
> + void *host_base = NULL, *port_base = NULL;
> + int i, rc;
> +
> + if (!printed_version++)
> + printk(KERN_DEBUG DRV_NAME " version " DRV_VERSION "\n");
> +
> + rc = pci_enable_device(pdev);
> + if (rc)
> + return rc;
> +
> + rc = pci_request_regions(pdev, DRV_NAME);
> + if (rc)
> + goto out_disable;
> +
> + rc = -ENOMEM;
> + /* ioremap mmio registers */
> + host_base = ioremap(pci_resource_start(pdev, 0),
> + pci_resource_len(pdev, 0));
> + if (!host_base)
> + goto out_free;
> + port_base = ioremap(pci_resource_start(pdev, 2),
> + pci_resource_len(pdev, 2));
> + if (!port_base)
> + goto out_free;
> + /* allocate & init probe_ent and hpriv */
> + probe_ent = kmalloc(sizeof(*probe_ent), GFP_KERNEL);
> + if (!probe_ent)
> + goto out_free;
> +
> + hpriv = kmalloc(sizeof(*hpriv), GFP_KERNEL);
> + if (!hpriv)
> + goto out_free;
> +
> + memset(probe_ent, 0, sizeof(*probe_ent));
> + probe_ent->dev = pci_dev_to_dev(pdev);
> + INIT_LIST_HEAD(&probe_ent->node);
> +
> + probe_ent->sht = sil24_port_info[board_id].sht;
> + probe_ent->host_flags = sil24_port_info[board_id].host_flags;
> + probe_ent->pio_mask = sil24_port_info[board_id].pio_mask;
> + probe_ent->udma_mask = sil24_port_info[board_id].udma_mask;
> + probe_ent->port_ops = sil24_port_info[board_id].port_ops;
> + probe_ent->n_ports = (board_id == BID_SIL3124) ? 4 : 2;
> +
> + probe_ent->irq = pdev->irq;
> + probe_ent->irq_flags = SA_SHIRQ;
> + probe_ent->mmio_base = port_base;
> + probe_ent->private_data = hpriv;
> +
> + memset(hpriv, 0, sizeof(*hpriv));
> + hpriv->host_base = host_base;
> + hpriv->port_base = port_base;
> +
> + /*
> + * Configure the device
> + */
> + /*
> + * FIXME: This device is certainly 64-bit capable. We just
> + * don't know how to use it. After fixing 32bit activation in
> + * this function, enable 64bit masks here.
> + */

elaboration?


> + rc = pci_set_dma_mask(pdev, DMA_32BIT_MASK);
> + if (rc) {
> + printk(KERN_ERR DRV_NAME "(%s): 32-bit DMA enable failed\n",
> + pci_name(pdev));
> + goto out_free;
> + }
> + rc = pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK);
> + if (rc) {
> + printk(KERN_ERR DRV_NAME "(%s): 32-bit consistent DMA enable failed\n",
> + pci_name(pdev));
> + goto out_free;
> + }
> +
> + /* GPIO off */
> + writel(0, host_base + HOST_FLASH_CMD);
> +
> + /* Mask interrupts during initialization */
> + writel(0, host_base + HOST_CTRL);

add a readl() to flush this write out to the PCI bus ("PCI posting")


> + for (i = 0; i < probe_ent->n_ports; i++) {
> + void *port = port_base + i * PORT_REGS_SIZE;
> + unsigned long portu = (unsigned long)port;
> + u32 tmp;
> + int cnt;
> +
> + probe_ent->port[i].cmd_addr = portu + PORT_TF;
> + probe_ent->port[i].ctl_addr = portu + PORT_TF + 0xa;
> + probe_ent->port[i].altstatus_addr = portu + PORT_TF + 0xa;
> + probe_ent->port[i].scr_addr = portu + PORT_SCONTROL;
> +
> + ata_std_ports(&probe_ent->port[i]);
> +
> + /* Initial PHY setting */
> + writel(0x20c, port + PORT_PHY_CFG);
> +
> + /* Clear port RST */
> + tmp = readl(port + PORT_CTRL_STAT);
> + if (tmp & PORT_CS_PORT_RST) {
> + writel(PORT_CS_PORT_RST, port + PORT_CTRL_CLR);
> + readl(port + PORT_CTRL_STAT); /* sync */
> + for (cnt = 0; cnt < 10; cnt++) {
> + msleep(10);
> + tmp = readl(port + PORT_CTRL_STAT);
> + if (!(tmp & PORT_CS_PORT_RST))
> + break;
> + }
> + if (tmp & PORT_CS_PORT_RST)
> + printk(KERN_ERR DRV_NAME
> + "(%s): failed to clear port RST\n",
> + pci_name(pdev));
> + }

this is already done in sata_phy_reset()?


> + /* Zero error counters. */
> + writel(0x8000, port + PORT_DECODE_ERR_THRESH);
> + writel(0x8000, port + PORT_CRC_ERR_THRESH);
> + writel(0x8000, port + PORT_HSHK_ERR_THRESH);
> + writel(0x0000, port + PORT_DECODE_ERR_CNT);
> + writel(0x0000, port + PORT_CRC_ERR_CNT);
> + writel(0x0000, port + PORT_HSHK_ERR_CNT);
> +
> + /* FIXME: 32bit activation? */
> + writel(0, port + PORT_ACTIVATE_UPPER_ADDR);
> + writel(PORT_CS_32BIT_ACTV, port + PORT_CTRL_STAT);
> +
> + /* Configure interrupts */
> + writel(0xffff, port + PORT_IRQ_ENABLE_CLR);
> + writel(PORT_IRQ_COMPLETE | PORT_IRQ_ERROR | PORT_IRQ_SDB_FIS,
> + port + PORT_IRQ_ENABLE_SET);
> +
> + /* Clear interrupts */
> + writel(0x0fff0fff, port + PORT_IRQ_STAT);
> + writel(PORT_CS_IRQ_WOC, port + PORT_CTRL_CLR);
> + }
> +
> + /* Turn on interrupts */
> + writel(IRQ_STAT_4PORTS, host_base + HOST_CTRL);
> +
> + pci_set_master(pdev);
> +
> + ata_device_add(probe_ent);

as with other drivers, add FIXME comment, indicating that the return
value should be checked, and if ata_device_add() failed.


> + kfree(probe_ent);
> + return 0;
> +
> + out_free:
> + if (host_base)
> + iounmap(host_base);
> + if (port_base)
> + iounmap(port_base);
> + kfree(probe_ent);
> + kfree(hpriv);
> + pci_release_regions(pdev);
> + out_disable:
> + pci_disable_device(pdev);
> + return rc;
> +}
> +
> +static int __init sil24_init(void)
> +{
> + return pci_module_init(&sil24_pci_driver);
> +}
> +
> +static void __exit sil24_exit(void)
> +{
> + pci_unregister_driver(&sil24_pci_driver);
> +}
> +
> +MODULE_AUTHOR("Tejun Heo");
> +MODULE_DESCRIPTION("Silicon Image 3124/3132 SATA low-level driver");
> +MODULE_LICENSE("GPL");
> +MODULE_DEVICE_TABLE(pci, sil24_pci_tbl);
> +
> +module_init(sil24_init);
> +module_exit(sil24_exit);
>

-
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-07-28 22:35    [W:0.095 / U:0.476 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site