Julian Margetson <runaway@xxxxxxxx> writes: > On 12/19/2015 1:05 PM, Måns Rullgård wrote: >> Andy Shevchenko <andy.shevchenko@xxxxxxxxx> writes: >> >>> On Sat, Dec 19, 2015 at 5:40 PM, Måns Rullgård <mans@xxxxxxxxx> wrote: >>> >>>> OK, I've found something. The dma setup errors are benign, caused by >>>> the driver calling dmaengine_prep_slave_sg() even for non-dma >>>> operations. >>> I suppose the following is a quick fix to avoid preparing descriptor >>> for non-DMA operations (not tested anyhow) >>> >>> a/drivers/ata/sata_dwc_460ex.c >>> +++ b/drivers/ata/sata_dwc_460ex.c >>> @@ -1041,6 +1041,9 @@ static void sata_dwc_qc_prep_by_tag(struct >>> ata_queued_cmd *qc, u8 tag) >>> __func__, ap->port_no, get_dma_dir_descript(qc->dma_dir), >>> qc->n_elem); >>> >>> + if (!is_slave_direction(qc->dma_dir)) >>> + return; >>> + >>> desc = dma_dwc_xfer_setup(qc); >>> if (!desc) { >>> dev_err(ap->dev, "%s: dma_dwc_xfer_setup returns NULL\n", >> I already have a better patch sitting here. >> >>>> The real error is the lock recursion that's reported >>>> later. I wasn't seeing it since I was running a UP non-preempt kernel. >>>> With lock debugging enabled, I get the same error. This patch should >>>> fix it. >>>> - spin_lock_irqsave(&ap->host->lock, flags); >>>> hsdevp->cmd_issued[tag] = cmd_issued; >>>> - spin_unlock_irqrestore(&ap->host->lock, flags); >>>> + >>> This will create a second empty line, though I don't care it is so minor. >> The patch leaves one blank line before the following block comment. I >> think it looks better that way. >> > > Still can't get the patch applied . Sorry, didn't realise it conflicted with an intervening patch I had in my tree. Try this one. -- Måns Rullgård
>From 97c1cdb8a6b933bad2c35b9461c2c15935f2a514 Mon Sep 17 00:00:00 2001 From: Mans Rullgard <mans@xxxxxxxxx> Date: Sat, 19 Dec 2015 15:26:23 +0000 Subject: [PATCH] ata: sata_dwc_460ex: remove incorrect locking This lock is already taken in ata_scsi_queuecmd() a few levels up the call stack so attempting to take it here is an error. Moreover, it is pointless in the first place since it only protects a single, atomic assignment. Signed-off-by: Mans Rullgard <mans@xxxxxxxxx> --- drivers/ata/sata_dwc_460ex.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/ata/sata_dwc_460ex.c b/drivers/ata/sata_dwc_460ex.c index 9985749..19d1c5e 100644 --- a/drivers/ata/sata_dwc_460ex.c +++ b/drivers/ata/sata_dwc_460ex.c @@ -995,15 +995,13 @@ static void sata_dwc_exec_command_by_tag(struct ata_port *ap, struct ata_taskfile *tf, u8 tag, u32 cmd_issued) { - unsigned long flags; struct sata_dwc_device_port *hsdevp = HSDEVP_FROM_AP(ap); dev_dbg(ap->dev, "%s cmd(0x%02x): %s tag=%d\n", __func__, tf->command, ata_get_cmd_descript(tf->command), tag); - spin_lock_irqsave(&ap->host->lock, flags); hsdevp->cmd_issued[tag] = cmd_issued; - spin_unlock_irqrestore(&ap->host->lock, flags); + /* * Clear SError before executing a new command. * sata_dwc_scr_write and read can not be used here. Clearing the PM -- 2.6.3