From: "Måns Rullgård" <mans@mansr.com> To: Julian Margetson <runaway@candw.ms> Cc: Andy Shevchenko <andy.shevchenko@gmail.com>, Andy Shevchenko <andriy.shevchenko@linux.intel.com>, Tejun Heo <tj@kernel.org>, linux-ide@vger.kernel.org, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org> Subject: Re: [PATCH 1/3] ata: sata_dwc_460ex: use "dmas" DT property to find dma channel Date: Sat, 19 Dec 2015 17:19:53 +0000 [thread overview] Message-ID: <yw1xsi2y8ifa.fsf@unicorn.mansr.com> (raw) In-Reply-To: <56758F33.20804@candw.ms> (Julian Margetson's message of "Sat, 19 Dec 2015 13:09:07 -0400") [-- Attachment #1: Type: text/plain, Size: 1879 bytes --] Julian Margetson <runaway@candw.ms> writes: > On 12/19/2015 1:05 PM, Måns Rullgård wrote: >> Andy Shevchenko <andy.shevchenko@gmail.com> writes: >> >>> On Sat, Dec 19, 2015 at 5:40 PM, Måns Rullgård <mans@mansr.com> 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 [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-ata-sata_dwc_460ex-remove-incorrect-locking.patch --] [-- Type: text/x-diff, Size: 1373 bytes --] >From 97c1cdb8a6b933bad2c35b9461c2c15935f2a514 Mon Sep 17 00:00:00 2001 From: Mans Rullgard <mans@mansr.com> 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@mansr.com> --- 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
WARNING: multiple messages have this Message-ID (diff)
From: "Måns Rullgård" <mans@mansr.com> To: Julian Margetson <runaway@candw.ms> Cc: Andy Shevchenko <andy.shevchenko@gmail.com>, Andy Shevchenko <andriy.shevchenko@linux.intel.com>, Tejun Heo <tj@kernel.org>, linux-ide@vger.kernel.org, "linux-kernel\@vger.kernel.org" <linux-kernel@vger.kernel.org> Subject: Re: [PATCH 1/3] ata: sata_dwc_460ex: use "dmas" DT property to find dma channel Date: Sat, 19 Dec 2015 17:19:53 +0000 [thread overview] Message-ID: <yw1xsi2y8ifa.fsf@unicorn.mansr.com> (raw) In-Reply-To: <56758F33.20804@candw.ms> (Julian Margetson's message of "Sat, 19 Dec 2015 13:09:07 -0400") [-- Attachment #1: Type: text/plain, Size: 1879 bytes --] Julian Margetson <runaway@candw.ms> writes: > On 12/19/2015 1:05 PM, Måns Rullgård wrote: >> Andy Shevchenko <andy.shevchenko@gmail.com> writes: >> >>> On Sat, Dec 19, 2015 at 5:40 PM, Måns Rullgård <mans@mansr.com> 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 [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-ata-sata_dwc_460ex-remove-incorrect-locking.patch --] [-- Type: text/x-diff, Size: 1373 bytes --] >From 97c1cdb8a6b933bad2c35b9461c2c15935f2a514 Mon Sep 17 00:00:00 2001 From: Mans Rullgard <mans@mansr.com> 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@mansr.com> --- 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
next prev parent reply other threads:[~2015-12-19 17:19 UTC|newest] Thread overview: 154+ messages / expand[flat|nested] mbox.gz Atom feed top 2015-12-15 23:25 [PATCH 1/3] ata: sata_dwc_460ex: use "dmas" DT property to find dma channel Mans Rullgard 2015-12-15 23:25 ` [PATCH 2/3] ata: sata_dwc_460ex: add phy support Mans Rullgard 2015-12-16 11:14 ` Sergei Shtylyov 2015-12-16 11:24 ` Måns Rullgård 2015-12-15 23:25 ` [PATCH 3/3] ata: sata_dwc_460ex: get rid of global data Mans Rullgard 2015-12-17 15:06 ` Andy Shevchenko 2015-12-17 15:19 ` Måns Rullgård 2015-12-17 15:37 ` Andy Shevchenko 2015-12-17 15:57 ` Måns Rullgård 2015-12-15 23:34 ` [PATCH 1/3] ata: sata_dwc_460ex: use "dmas" DT property to find dma channel Måns Rullgård 2015-12-17 14:59 ` Andy Shevchenko 2015-12-17 15:13 ` Måns Rullgård 2015-12-17 15:55 ` Andy Shevchenko 2015-12-17 16:04 ` Måns Rullgård 2015-12-17 16:53 ` Andy Shevchenko 2015-12-17 17:57 ` Julian Margetson 2015-12-17 17:59 ` Måns Rullgård [not found] ` <567302E8.5050303@candw.ms> 2015-12-17 18:51 ` Måns Rullgård [not found] ` <5673061A.4070700@candw.ms> 2015-12-17 19:53 ` Måns Rullgård [not found] ` <56732C04.9040100@candw.ms> 2015-12-18 0:06 ` Måns Rullgård 2015-12-18 0:59 ` Julian Margetson 2015-12-18 1:38 ` Måns Rullgård 2015-12-18 11:48 ` Julian Margetson 2015-12-18 12:04 ` Måns Rullgård 2015-12-18 12:23 ` Andy Shevchenko 2015-12-18 12:49 ` Måns Rullgård [not found] ` <5674271B.9090308@candw.ms> 2015-12-18 17:18 ` Måns Rullgård 2015-12-18 18:48 ` Andy Shevchenko [not found] ` <56745BA4.1090607@candw.ms> 2015-12-18 22:33 ` Måns Rullgård 2015-12-18 22:49 ` Julian Margetson 2015-12-18 23:16 ` Måns Rullgård 2015-12-19 2:34 ` Andy Shevchenko 2015-12-19 11:39 ` Julian Margetson 2015-12-19 15:40 ` Måns Rullgård 2015-12-19 15:40 ` Måns Rullgård [not found] ` <567585CD.9080105@candw.ms> 2015-12-19 16:39 ` Måns Rullgård 2015-12-19 16:39 ` Måns Rullgård 2015-12-19 16:56 ` Andy Shevchenko 2015-12-19 17:05 ` Måns Rullgård 2015-12-19 17:05 ` Måns Rullgård 2015-12-19 17:09 ` Julian Margetson 2015-12-19 17:11 ` Andy Shevchenko 2015-12-19 17:19 ` Måns Rullgård [this message] 2015-12-19 17:19 ` Måns Rullgård 2015-12-19 18:56 ` Julian Margetson 2015-12-19 19:07 ` Måns Rullgård 2015-12-19 19:07 ` Måns Rullgård 2015-12-19 20:16 ` Julian Margetson 2015-12-19 20:39 ` Andy Shevchenko 2015-12-19 20:41 ` Måns Rullgård 2015-12-19 20:41 ` Måns Rullgård 2015-12-19 20:48 ` Julian Margetson 2015-12-19 20:55 ` Julian Margetson 2015-12-20 17:11 ` Måns Rullgård 2015-12-20 17:11 ` Måns Rullgård 2015-12-20 17:41 ` Andy Shevchenko 2015-12-20 17:54 ` Måns Rullgård 2015-12-20 17:54 ` Måns Rullgård 2015-12-20 17:44 ` Julian Margetson 2015-12-20 18:49 ` Måns Rullgård 2015-12-20 18:49 ` Måns Rullgård 2015-12-20 20:17 ` Andy Shevchenko 2015-12-20 20:55 ` Andy Shevchenko 2015-12-21 1:19 ` Måns Rullgård 2015-12-21 1:19 ` Måns Rullgård 2015-12-21 19:08 ` Andy Shevchenko 2015-12-21 19:27 ` Måns Rullgård 2015-12-21 19:27 ` Måns Rullgård 2015-12-21 20:54 ` Andy Shevchenko 2015-12-21 21:06 ` Måns Rullgård 2015-12-21 21:06 ` Måns Rullgård 2015-12-22 0:08 ` Måns Rullgård 2015-12-22 0:08 ` Måns Rullgård 2015-12-22 10:58 ` Andy Shevchenko 2016-01-06 16:26 ` Måns Rullgård 2016-01-06 16:26 ` Måns Rullgård 2016-01-06 17:36 ` Andy Shevchenko 2016-01-07 9:34 ` Andy Shevchenko 2016-01-07 18:32 ` Måns Rullgård 2016-01-07 18:32 ` Måns Rullgård 2016-01-08 8:57 ` Andy Shevchenko 2016-01-08 10:57 ` Måns Rullgård 2016-01-08 10:57 ` Måns Rullgård 2016-01-20 18:50 ` Måns Rullgård 2016-01-20 18:50 ` Måns Rullgård 2016-01-20 19:23 ` Andy Shevchenko 2016-01-20 19:24 ` Måns Rullgård 2016-01-20 19:24 ` Måns Rullgård 2016-01-20 19:38 ` Andy Shevchenko 2016-01-20 19:46 ` Måns Rullgård 2016-01-20 19:46 ` Måns Rullgård 2016-01-20 19:51 ` Andy Shevchenko 2016-01-20 20:07 ` Måns Rullgård 2016-01-20 20:07 ` Måns Rullgård 2016-01-22 10:04 ` Andy Shevchenko 2016-01-22 11:13 ` Måns Rullgård 2016-01-22 11:13 ` Måns Rullgård 2016-01-22 11:56 ` Andy Shevchenko 2016-01-22 12:05 ` Måns Rullgård 2016-01-22 12:05 ` Måns Rullgård 2016-01-22 12:15 ` Andy Shevchenko 2015-12-21 16:48 ` Andy Shevchenko 2015-12-21 17:20 ` Julian Margetson 2015-12-21 17:26 ` Julian Margetson 2015-12-21 17:55 ` Andy Shevchenko 2015-12-21 18:23 ` Julian Margetson 2015-12-21 18:27 ` Måns Rullgård 2015-12-21 18:27 ` Måns Rullgård 2015-12-21 19:08 ` Julian Margetson 2015-12-21 19:19 ` Julian Margetson 2015-12-21 19:27 ` Måns Rullgård 2015-12-21 19:27 ` Måns Rullgård 2015-12-21 19:47 ` Julian Margetson 2015-12-21 20:25 ` Andy Shevchenko 2015-12-21 20:29 ` Julian Margetson 2016-01-22 10:07 ` Andy Shevchenko 2015-12-21 20:33 ` Måns Rullgård 2015-12-21 20:33 ` Måns Rullgård 2015-12-21 18:25 ` Måns Rullgård 2015-12-21 18:25 ` Måns Rullgård 2015-12-21 0:47 ` Måns Rullgård 2015-12-21 0:47 ` Måns Rullgård 2015-12-21 0:53 ` Måns Rullgård 2015-12-21 0:53 ` Måns Rullgård 2015-12-21 0:58 ` Måns Rullgård 2015-12-21 0:58 ` Måns Rullgård 2015-12-21 8:40 ` Andy Shevchenko 2015-12-21 12:15 ` Måns Rullgård 2015-12-21 12:15 ` Måns Rullgård 2015-12-21 17:24 ` Andy Shevchenko 2015-12-21 18:16 ` Måns Rullgård 2015-12-21 18:16 ` Måns Rullgård 2015-12-21 19:23 ` Andy Shevchenko 2015-12-21 19:50 ` Måns Rullgård 2015-12-21 19:50 ` Måns Rullgård [not found] ` <5677D447.40906@candw.ms> 2015-12-21 12:16 ` Måns Rullgård 2015-12-21 12:16 ` Måns Rullgård 2015-12-21 13:18 ` Julian Margetson 2015-12-21 13:24 ` Måns Rullgård 2015-12-21 13:24 ` Måns Rullgård 2015-12-21 14:40 ` Julian Margetson 2015-12-21 15:24 ` Måns Rullgård 2015-12-21 15:24 ` Måns Rullgård 2015-12-21 16:44 ` Andy Shevchenko 2015-12-21 18:19 ` Måns Rullgård 2015-12-21 18:19 ` Måns Rullgård 2015-12-18 12:33 ` Julian Margetson 2015-12-18 12:38 ` Andy Shevchenko 2015-12-18 12:45 ` Måns Rullgård [not found] ` <56740F9F.5020500@candw.ms> 2015-12-18 14:24 ` Andy Shevchenko 2015-12-18 14:27 ` Måns Rullgård 2015-12-18 10:08 ` Andy Shevchenko 2015-12-18 11:24 ` Måns Rullgård 2015-12-17 14:58 ` Andy Shevchenko
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=yw1xsi2y8ifa.fsf@unicorn.mansr.com \ --to=mans@mansr.com \ --cc=andriy.shevchenko@linux.intel.com \ --cc=andy.shevchenko@gmail.com \ --cc=linux-ide@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=runaway@candw.ms \ --cc=tj@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.