From: Sergei Shtylyov on
Hello.

Rupjyoti Sarmah wrote:

> This patch enables the on-chip DWC SATA controller of the AppliedMicro processor 460EX.
>
> Signed-off-by: Rupjyoti Sarmah <rsarmah(a)appliedmicro.com>
> Signed-off-by: Mark Miesfeld <mmiesfeld(a)appliedmicro.com>
> Signed-off-by: Prodyut Hazarika <phazarika(a)appliedmicro.com>
> ---
> arch/powerpc/boot/dts/canyonlands.dts | 8 +
> drivers/ata/Kconfig | 9 +
> drivers/ata/Makefile | 1 +
> drivers/ata/sata_dwc.c | 1827 +++++++++++++++++++++++++++++++++
> 4 files changed, 1845 insertions(+), 0 deletions(-)
> create mode 100644 drivers/ata/sata_dwc.c
>
> diff --git a/arch/powerpc/boot/dts/canyonlands.dts b/arch/powerpc/boot/dts/canyonlands.dts
> index cd56bb5..d3b2c99 100644
> --- a/arch/powerpc/boot/dts/canyonlands.dts
> +++ b/arch/powerpc/boot/dts/canyonlands.dts
> @@ -163,6 +163,14 @@
> interrupts = <0x1e 4>;
> };
>
> + SATA0: sata(a)bffd1000 {
> + compatible = "amcc,sata-460ex";
> + reg = <4 0xbffd1000 0x800 4 0xbffd0800 0x400>;
> + interrupt-parent = <&UIC3>;
> + interrupts = <0x0 0x4 /* SATA */
> + 0x5 0x4>; /* AHBDMA */
> + };
> +
> POB0: opb {
> compatible = "ibm,opb-460ex", "ibm,opb";
> #address-cells = <1>;
>

Please put the device tree update in a separate patch to go thru the
PowerPC tree.

MBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Jeff Garzik on
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(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Alan Cox on
On Wed, 19 May 2010 10:49:59 +0900
Jassi Brar <jassisinghbrar(a)gmail.com> wrote:

> On Thu, May 6, 2010 at 2:57 AM, Rupjyoti Sarmah <rsarmah(a)amcc.com> wrote:
> > This patch enables the on-chip DWC SATA controller of the AppliedMicro processor 460EX.
>
> The controller seems to be a thrid party IP (from Synopsys) in your
> SoC and there are many chances the IP will appear in some other
> SOCs too. This implementation doesn't seem to take care of that
> scenario.

I'd question if that is worth it seriously. At the point there are
multiple users of the device you know what it will look like. Until then
you neither know if the work is needed nor what will require abstracting.

Lazy evaluation is good for code too ;)

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