[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4BEDC72B.10507@pobox.com>
Date: Fri, 14 May 2010 17:56:59 -0400
From: Jeff Garzik <jgarzik@...ox.com>
To: Rupjyoti Sarmah <rsarmah@...c.com>
CC: linux-ide@...r.kernel.org, linux-kernel@...r.kernel.org,
sr@...x.de, linuxppc-dev@...abs.org,
Sergei Shtylyov <sshtylyov@...sta.com>,
Benjamin Herrenschmidt <benh@...nel.crashing.org>
Subject: Re: [PATCH]460EX on-chip SATA driver <Kernel 2.6.33> < resubmission
>
On 05/05/2010 01:57 PM, Rupjyoti Sarmah wrote:
> +static void clear_interrupt_bit(struct sata_dwc_device *hsdev, u32 bit)
> +{
> + out_le32(&hsdev->sata_dwc_regs->intpr,
> + in_le32(&hsdev->sata_dwc_regs->intpr));
> +}
> +
> +static u32 qcmd_tag_to_mask(u8 tag)
> +{
> + return 0x00000001<< (tag& 0x1f);
> +}
> +
> +/* See ahci.c */
> +static void sata_dwc_error_intr(struct ata_port *ap,
> + struct sata_dwc_device *hsdev, uint intpr)
> +{
> + struct sata_dwc_device_port *hsdevp = HSDEVP_FROM_AP(ap);
> + struct ata_eh_info *ehi =&ap->link.eh_info;
> + unsigned int err_mask = 0, action = 0;
> + struct ata_queued_cmd *qc;
> + u32 serror;
> + u8 status, tag;
> + u32 err_reg;
> +
> + ata_ehi_clear_desc(ehi);
> +
> + serror = core_scr_read(SCR_ERROR);
> + status = ap->ops->sff_check_status(ap);
> +
> + err_reg = in_le32(&(host_pvt.sata_dma_regs->interrupt_status.error.\
> + low));
> + tag = ap->link.active_tag;
> +
> + dev_err(ap->dev, "%s SCR_ERROR=0x%08x intpr=0x%08x status=0x%08x "
> + "dma_intp=%d pending=%d issued=%d dma_err_status=0x%08x\n",
> + __func__, serror, intpr, status, host_pvt.dma_interrupt_count,
> + hsdevp->dma_pending[tag], hsdevp->cmd_issued[tag], err_reg);
> +
> + /* Clear error register and interrupt bit */
> + clear_serror();
> + clear_interrupt_bit(hsdev, SATA_DWC_INTPR_ERR);
> +
> + /* This is the only error happening now. TODO check for exact error */
> + err_mask |= AC_ERR_HOST_BUS;
> + action |= ATA_EH_RESET;
this is a rather poor error mapping.
> +/******************************************************************************
> + * Function : sata_dwc_isr
> + * arguments : irq, void *dev_instance, struct pt_regs *regs
> + * Return value : irqreturn_t - status of IRQ
> + * This Interrupt handler called via port ops registered function.
> + * .irq_handler = sata_dwc_isr
> + *****************************************************************************/
> +static irqreturn_t sata_dwc_isr(int irq, void *dev_instance)
> +{
> + struct ata_host *host = (struct ata_host *)dev_instance;
> + struct sata_dwc_device *hsdev = HSDEV_FROM_HOST(host);
> + struct ata_port *ap;
> + struct ata_queued_cmd *qc;
> + unsigned long flags;
> + u8 status, tag;
> + int handled, num_processed, port = 0;
> + uint intpr, sactive, sactive2, tag_mask;
> + struct sata_dwc_device_port *hsdevp;
> + host_pvt.sata_dwc_sactive_issued = 0;
> +
> + spin_lock_irqsave(&host->lock, flags);
> +
> + /* Read the interrupt register */
> + intpr = in_le32(&hsdev->sata_dwc_regs->intpr);
> +
> + ap = host->ports[port];
> + hsdevp = HSDEVP_FROM_AP(ap);
> +
> + dev_dbg(ap->dev, "%s intpr=0x%08x active_tag=%d\n", __func__, intpr,
> + ap->link.active_tag);
> +
> + /* Check for error interrupt */
> + if (intpr& SATA_DWC_INTPR_ERR) {
> + sata_dwc_error_intr(ap, hsdev, intpr);
> + handled = 1;
> + goto DONE;
> + }
> +
> + /* Check for DMA SETUP FIS (FP DMA) interrupt */
> + if (intpr& SATA_DWC_INTPR_NEWFP) {
> + clear_interrupt_bit(hsdev, SATA_DWC_INTPR_NEWFP);
> +
> + tag = (u8)(in_le32(&hsdev->sata_dwc_regs->fptagr));
> + dev_dbg(ap->dev, "%s: NEWFP tag=%d\n", __func__, tag);
> + if (hsdevp->cmd_issued[tag] != SATA_DWC_CMD_ISSUED_PEND)
> + dev_warn(ap->dev, "CMD tag=%d not pending?\n", tag);
> +
> + host_pvt.sata_dwc_sactive_issued |= qcmd_tag_to_mask(tag);
> +
> + qc = ata_qc_from_tag(ap, tag);
> + /* Start FP DMA for NCQ command. At this point the tag is the
> + * active tag. It is the tag that matches the command about to
> + * be completed.
> + */
> + qc->ap->link.active_tag = tag;
> + sata_dwc_bmdma_start_by_tag(qc, tag);
> +
> + handled = 1;
> + goto DONE;
> + }
> +
> + sactive = core_scr_read(SCR_ACTIVE);
> + tag_mask = (host_pvt.sata_dwc_sactive_issued | sactive) ^ sactive;
> +
> + /* If no sactive issued and tag_mask is zero then this is not NCQ */
> + if (host_pvt.sata_dwc_sactive_issued == 0&& tag_mask == 0) {
> + if (ap->link.active_tag == ATA_TAG_POISON)
> + tag = 0;
> + else
> + tag = ap->link.active_tag;
> + qc = ata_qc_from_tag(ap, tag);
> +
> + /* DEV interrupt w/ no active qc? */
> + if (unlikely(!qc || (qc->tf.flags& ATA_TFLAG_POLLING))) {
> + dev_err(ap->dev, "%s interrupt with no active qc "
> + "qc=%p\n", __func__, qc);
> + ap->ops->sff_check_status(ap);
> + handled = 1;
> + goto DONE;
> + }
> +
> + status = ap->ops->sff_check_status(ap);
> +
> + qc->ap->link.active_tag = tag;
> + hsdevp->cmd_issued[tag] = SATA_DWC_CMD_ISSUED_NOT;
> +
> + if (status& ATA_ERR) {
> + dev_dbg(ap->dev, "interrupt ATA_ERR (0x%x)\n", status);
> + sata_dwc_qc_complete(ap, qc, 1);
> + handled = 1;
> + goto DONE;
> + }
> +
> + dev_dbg(ap->dev, "%s non-NCQ cmd interrupt, protocol: %s\n",
> + __func__, ata_get_cmd_descript(qc->tf.protocol));
> +DRVSTILLBUSY:
> + if (ata_is_dma(qc->tf.protocol)) {
> + /* Each DMA transaction produces 2 interrupts. The DMAC
> + * transfer complete interrupt and the SATA controller
> + * operation done interrupt. The command should be
> + * completed only after both interrupts are seen.
> + */
> + host_pvt.dma_interrupt_count++;
> + if (hsdevp->dma_pending[tag] == \
> + SATA_DWC_DMA_PENDING_NONE) {
> + dev_err(ap->dev, "%s: DMA not pending "
> + "intpr=0x%08x status=0x%08x pending"
> + "=%d\n", __func__, intpr, status,
> + hsdevp->dma_pending[tag]);
> + }
> +
> + if ((host_pvt.dma_interrupt_count % 2) == 0)
> + sata_dwc_dma_xfer_complete(ap, 1);
> + } else if (ata_is_pio(qc->tf.protocol)) {
> + ata_sff_hsm_move(ap, qc, status, 0);
> + handled = 1;
> + goto DONE;
> + } else {
> + if (unlikely(sata_dwc_qc_complete(ap, qc, 1)))
> + goto DRVSTILLBUSY;
> + }
> +
> + handled = 1;
> + goto DONE;
> + }
> +
> + /*
> + * This is a NCQ command. At this point we need to figure out for which
> + * tags we have gotten a completion interrupt. One interrupt may serve
> + * as completion for more than one operation when commands are queued
> + * (NCQ). We need to process each completed command.
> + */
> +
> + /* process completed commands */
> + sactive = core_scr_read(SCR_ACTIVE);
> + tag_mask = (host_pvt.sata_dwc_sactive_issued | sactive) ^ sactive;
> +
> + if (sactive != 0 || (host_pvt.sata_dwc_sactive_issued)> 1 || \
> + tag_mask> 1) {
> + dev_dbg(ap->dev, "%s NCQ:sactive=0x%08x sactive_issued=0x%08x"
> + "tag_mask=0x%08x\n", __func__, sactive,
> + host_pvt.sata_dwc_sactive_issued, tag_mask);
> + }
> +
> + if ((tag_mask | (host_pvt.sata_dwc_sactive_issued)) != \
> + (host_pvt.sata_dwc_sactive_issued)) {
> + dev_warn(ap->dev, "Bad tag mask? sactive=0x%08x "
> + "(host_pvt.sata_dwc_sactive_issued)=0x%08x tag_mask"
> + "=0x%08x\n", sactive, host_pvt.sata_dwc_sactive_issued,
> + tag_mask);
> + }
> +
> + /* read just to clear ... not bad if currently still busy */
> + status = ap->ops->sff_check_status(ap);
> + dev_dbg(ap->dev, "%s ATA status register=0x%x\n", __func__, status);
> +
> + tag = 0;
> + num_processed = 0;
> + while (tag_mask) {
> + num_processed++;
> + while (!(tag_mask& 0x00000001)) {
> + tag++;
> + tag_mask<<= 1;
> + }
> + tag_mask&= (~0x00000001);
> + qc = ata_qc_from_tag(ap, tag);
> +
> + /* To be picked up by completion functions */
> + qc->ap->link.active_tag = tag;
> + hsdevp->cmd_issued[tag] = SATA_DWC_CMD_ISSUED_NOT;
> +
> + /* Let libata/scsi layers handle error */
> + if (status& ATA_ERR) {
> + dev_dbg(ap->dev, "%s ATA_ERR (0x%x)\n", __func__,
> + status);
> + sata_dwc_qc_complete(ap, qc, 1);
> + handled = 1;
> + goto DONE;
> + }
> +
> + /* Process completed command */
> + dev_dbg(ap->dev, "%s NCQ command, protocol: %s\n", __func__,
> + ata_get_cmd_descript(qc->tf.protocol));
> + if (ata_is_dma(qc->tf.protocol)) {
> + host_pvt.dma_interrupt_count++;
> + if (hsdevp->dma_pending[tag] == \
> + SATA_DWC_DMA_PENDING_NONE)
> + dev_warn(ap->dev, "%s: DMA not pending?\n",
> + __func__);
> + if ((host_pvt.dma_interrupt_count % 2) == 0)
> + sata_dwc_dma_xfer_complete(ap, 1);
> + } else {
> + if (unlikely(sata_dwc_qc_complete(ap, qc, 1)))
> + goto STILLBUSY;
> + }
> + continue;
> +
> +STILLBUSY:
> + ap->stats.idle_irq++;
> + dev_warn(ap->dev, "STILL BUSY IRQ ata%d: irq trap\n",
> + ap->print_id);
> + } /* while tag_mask */
why not use ata_qc_complete_multiple?
> +static void sata_dwc_clear_dmacr(struct sata_dwc_device_port *hsdevp, u8 tag)
> +{
> + struct sata_dwc_device *hsdev = HSDEV_FROM_HSDEVP(hsdevp);
> +
> + if (hsdevp->dma_pending[tag] == SATA_DWC_DMA_PENDING_RX) {
> + out_le32(&(hsdev->sata_dwc_regs->dmacr),
> + SATA_DWC_DMACR_RX_CLEAR(
> + in_le32(&(hsdev->sata_dwc_regs->dmacr))));
> + } else if (hsdevp->dma_pending[tag] == SATA_DWC_DMA_PENDING_TX) {
> + out_le32(&(hsdev->sata_dwc_regs->dmacr),
> + SATA_DWC_DMACR_TX_CLEAR(
> + in_le32(&(hsdev->sata_dwc_regs->dmacr))));
> + } else {
> + /* This should not happen, it indicates the driver is out of
> + * sync. If it does happen, clear dmacr anyway.
> + */
> + dev_err(host_pvt.dwc_dev, "%s DMA protocol RX and"
> + "TX DMA not pending tag=0x%02x pending=%d"
> + " dmacr: 0x%08x\n", __func__, tag,
> + hsdevp->dma_pending[tag],
> + in_le32(&(hsdev->sata_dwc_regs->dmacr)));
> + out_le32(&(hsdev->sata_dwc_regs->dmacr),
> + SATA_DWC_DMACR_TXRXCH_CLEAR);
> + }
> +}
> +
> +static void sata_dwc_dma_xfer_complete(struct ata_port *ap, u32 check_status)
> +{
> + struct ata_queued_cmd *qc;
> + struct sata_dwc_device_port *hsdevp = HSDEVP_FROM_AP(ap);
> + struct sata_dwc_device *hsdev = HSDEV_FROM_AP(ap);
> + u8 tag = 0;
> +
> + tag = ap->link.active_tag;
> + qc = ata_qc_from_tag(ap, tag);
Note that qc may equal NULL!
> +#ifdef DEBUG_NCQ
> + if (tag> 0) {
> + dev_info(ap->dev, "%s tag=%u cmd=0x%02x dma dir=%s proto=%s "
> + "dmacr=0x%08x\n", __func__, qc->tag, qc->tf.command,
> + ata_get_cmd_descript(qc->dma_dir),
> + ata_get_cmd_descript(qc->tf.protocol),
> + in_le32(&(hsdev->sata_dwc_regs->dmacr)));
> + }
> +#endif
> +
> + if (ata_is_dma(qc->tf.protocol)) {
> + if (hsdevp->dma_pending[tag] == SATA_DWC_DMA_PENDING_NONE) {
> + dev_err(ap->dev, "%s DMA protocol RX and TX DMA not "
> + "pending dmacr: 0x%08x\n", __func__,
> + in_le32(&(hsdev->sata_dwc_regs->dmacr)));
> + }
> +
> + hsdevp->dma_pending[tag] = SATA_DWC_DMA_PENDING_NONE;
> + sata_dwc_qc_complete(ap, qc, check_status);
> + ap->link.active_tag = ATA_TAG_POISON;
> + } else {
> + sata_dwc_qc_complete(ap, qc, check_status);
> + }
> +}
> +
> +static int sata_dwc_qc_complete(struct ata_port *ap, struct ata_queued_cmd *qc,
> + u32 check_status)
> +{
> + u8 status = 0;
> + int i = 0;
> + u32 mask = 0x0;
> + u8 tag = qc->tag;
> + struct sata_dwc_device_port *hsdevp = HSDEVP_FROM_AP(ap);
> + u32 serror;
> + host_pvt.sata_dwc_sactive_queued = 0;
> + dev_dbg(ap->dev, "%s checkstatus? %x\n", __func__, check_status);
> +
> + if (hsdevp->dma_pending[tag] == SATA_DWC_DMA_PENDING_TX)
> + dev_err(ap->dev, "TX DMA PENDING\n");
> + else if (hsdevp->dma_pending[tag] == SATA_DWC_DMA_PENDING_RX)
> + dev_err(ap->dev, "RX DMA PENDING\n");
> +
> + if (check_status) {
> + i = 0;
> + do {
> + /* check main status, clearing INTRQ */
> + status = ap->ops->sff_check_status(ap);
> + if (status& ATA_BUSY) {
> + dev_dbg(ap->dev, "STATUS BUSY (0x%02x) [%d]\n",
> + status, i);
> + }
> + if (++i> 10)
> + break;
> + } while (status& ATA_BUSY);
> +
> + status = ap->ops->sff_check_status(ap);
> + if (unlikely(status& ATA_BUSY))
> + dev_err(ap->dev, "QC complete cmd=0x%02x STATUS BUSY"
> + " (0x%02x)[%d]\n", qc->tf.command, status, i);
> + serror = core_scr_read(SCR_ERROR);
> + if (serror& SATA_DWC_SERROR_ERR_BITS)
> + dev_err(ap->dev, "****** SERROR=0x%08x ******\n",
> + serror);
> + }
> + dev_dbg(ap->dev, "QC complete cmd=0x%02x status=0x%02x ata%u:"
> + " protocol=%d\n", qc->tf.command, status, ap->print_id,
> + qc->tf.protocol);
> +
> + /* clear active bit */
> + mask = (~(qcmd_tag_to_mask(tag)));
> + host_pvt.sata_dwc_sactive_queued = (host_pvt.sata_dwc_sactive_queued) \
> + & mask;
> + host_pvt.sata_dwc_sactive_issued = (host_pvt.sata_dwc_sactive_issued) \
> + & mask;
> +
> + /* Complete taskfile transaction (does not read SCR registers) */
> + ata_qc_complete(qc);
> +
> + return 0;
why is this loop in qc_complete? That is highly unusual.
> +/******************************************************************************
> + * Function : sata_dwc_qc_prep
> + * arguments : ata_queued_cmd *qc
> + * Return value : None
> + * qc_prep for a particular queued command
> + *****************************************************************************/
> +#if 1
> +static void sata_dwc_qc_prep(struct ata_queued_cmd *qc)
> +{
> + if ((qc->dma_dir == DMA_NONE) || (qc->tf.protocol == ATA_PROT_PIO))
> + return;
> +
> +#ifdef DEBUG_NCQ
> + if (qc->tag> 0)
> + dev_info(qc->ap->dev, "%s: qc->tag=%d ap->active_tag=0x%08x\n",
> + __func__, tag, qc->ap->link.active_tag);
> +
> + return ;
> +#endif
> +}
> +#endif
> +static void sata_dwc_error_handler(struct ata_port *ap)
> +{
> + ap->link.flags |= ATA_LFLAG_NO_HRST;
> + ata_sff_error_handler(ap);
> + ap->link.flags&= ~ATA_LFLAG_NO_HRST;
why do you mess with ATA_LFLAG_NO_HRST in this way?
> + /* Get SATA interrupt number */
> + irq = irq_of_parse_and_map(ofdev->node, 0);
> + if (irq == NO_IRQ) {
> + dev_err(&ofdev->dev, "no SATA DMA irq\n");
> + err = -ENODEV;
> + goto error_out;
> + }
> +
> + /*
> + * Now, register with libATA core, this will also initiate the
> + * device discovery process, invoking our port_start() handler&
> + * error_handler() to execute a dummy Softreset EH session
> + */
> + ata_host_activate(host, irq, sata_dwc_isr, 0,&sata_dwc_sht);
handle ata_host_activate failure
--
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/
Powered by blists - more mailing lists