Date:	Mon, 21 Dec 2015 15:24:23 +0000
From:	Måns Rullgård <mans@...sr.com>
To:	Julian Margetson <runaway@...dw.ms>
Cc:	Andy Shevchenko <andy.shevchenko@...il.com>,
	Viresh Kumar <viresh.linux@...il.com>,
	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
	Tejun Heo <tj@...nel.org>, linux-ide@...r.kernel.org,
	"linux-kernel\@vger.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/3] ata: sata_dwc_460ex: use "dmas" DT property to find dma channel

Julian Margetson <runaway@...dw.ms> writes:

> On 12/21/2015 9:24 AM, Måns Rullgård wrote:
>> Julian Margetson <runaway@...dw.ms> writes:
>>
>>>>>> P.S. Anyway we have to ask Julian to try the kernel with
>>>>>> 8b3444852a2b58129 reverted.
>>>>>>
>>>>> git revert 8b3444852a2b58129
>>>>> error: could not revert 8b34448... sata_dwc_460ex: move to generic DMA driver
>>>>> hint: after resolving the conflicts, mark the corrected paths
>>>>> hint: with 'git add <paths>' or 'git rm <paths>'
>>>>> hint: and commit the result with 'git commit'
>>>> Yeah, that won't work since there are numerous changes afterward.  Just
>>>> revert the entire file back to 4.0 like this:
>>>>
>>>> $ git checkout v4.0 drivers/ata/sata_dwc_460ex.c
>>>>
>>>   CC [M]  drivers/ata/sata_dwc_460ex.o
>>> drivers/ata/sata_dwc_460ex.c:467:36: error: macro
>>> "dma_request_channel" requires 3 arguments, but only 1 given
>>>   static int dma_request_channel(void)
>>>                                      ^
>>> drivers/ata/sata_dwc_460ex.c:468:1: error: expected ‘=’, ‘,’,
>>> ‘;’, ‘asm’ or ‘__attribute__’ before ‘{’ token
>>>   {
>>>   ^
>>> drivers/ata/sata_dwc_460ex.c: In function ‘dma_dwc_xfer_setup’:
>>> drivers/ata/sata_dwc_460ex.c:758:31: error: macro
>>> "dma_request_channel" requires 3 arguments, but only 1 given
>>>    dma_ch = dma_request_channel();
>>>                                 ^
>>> drivers/ata/sata_dwc_460ex.c:758:11: error: ‘dma_request_channel’
>>> undeclared (first use in this function)
>>>    dma_ch = dma_request_channel();
>>>             ^
>>> drivers/ata/sata_dwc_460ex.c:758:11: note: each undeclared identifier
>>> is reported only once for each function it appears in
>>> drivers/ata/sata_dwc_460ex.c: In function ‘sata_dwc_dma_filter’:
>>> drivers/ata/sata_dwc_460ex.c:1282:35: error: ‘struct
>>> sata_dwc_device_port’ has no member named ‘dws’
>>>    struct dw_dma_slave *dws = hsdevp->dws;
>>>                                     ^
>>> drivers/ata/sata_dwc_460ex.c: In function ‘sata_dwc_port_start’:
>>> drivers/ata/sata_dwc_460ex.c:1325:17: warning: unused variable
>>> ‘mask’ [-Wunused-variable]
>>>    dma_cap_mask_t mask;
>>>                   ^
>>> drivers/ata/sata_dwc_460ex.c: At top level:
>>> drivers/ata/sata_dwc_460ex.c:345:28: warning: ‘sata_dwc_dma_dws’
>>> defined but not used [-Wunused-variable]
>>>   static struct dw_dma_slave sata_dwc_dma_dws = {
>>>                              ^
>>> drivers/ata/sata_dwc_460ex.c:1279:13: warning:
>>> ‘sata_dwc_dma_filter’ defined but not used [-Wunused-function]
>>>   static bool sata_dwc_dma_filter(struct dma_chan *chan, void *param)
>>>               ^
>> Those messages do not match the contents of the file from v4.0.
>> For your convenience, here's the file as it should be.
>>
>> $ sha1sum drivers/ata/sata_dwc_460ex.c
>> 0f54dfa3a91591101f5de434c3a631a5cd20ff1a  drivers/ata/sata_dwc_460ex.c
>
> [   16.119186] BUG: spinlock recursion on CPU#0, kworker/u2:1/85
> [   16.124935]  lock: 0xedd2f910, .magic: dead4ead, .owner: kworker/u2:1/85, .owner_cpu: 0
> [   16.132947] CPU: 0 PID: 85 Comm: kworker/u2:1 Not tainted 4.4.0-rc5-Sam460ex-dirty #3
> [   16.140793] Workqueue: events_unbound async_run_entry_fn
> [   16.146119] Call Trace:
> [   16.148582] [ee3cf8c0] [c0049238] do_raw_spin_lock+0x4c/0x100 (unreliable)
> [   16.155491] [ee3cf8e0] [c068af98] _raw_spin_lock_irqsave+0x2c/0x38
> [   16.161721] [ee3cf8f0] [f6a0fd98] sata_dwc_exec_command_by_tag.constprop.9+0x80/0xb4 [sata_dwc_460ex]
> [   16.170954] [ee3cf920] [f6a108c0] sata_dwc_qc_issue+0x6a4/0x6c4 [sata_dwc_460ex]
> [   16.178380] [ee3cf9d0] [c043bdf8] ata_qc_issue+0x338/0x3a0
> [   16.183883] [ee3cfa00] [c0440c84] ata_scsi_translate+0xf4/0x150
> [   16.189813] [ee3cfa20] [c0444080] ata_scsi_queuecmd+0x1e8/0x238
> [   16.195750] [ee3cfa40] [c042511c] scsi_dispatch_cmd+0xd4/0x110
> [   16.201602] [ee3cfa50] [c0427a9c] scsi_request_fn+0x52c/0x55c

Oh, that one again.  My patch still applies.  Here it is as applied to
that revision of the file.

>From what I can tell, that bug has always been there.  Probably nobody
ever tested the driver in a PREEMPT or SMP build, nor with lock
debugging enabled.

-- 
Måns Rullgård

View attachment "0001-ata-sata_dwc_460ex-remove-incorrect-locking.patch" of type "text/x-diff" (1376 bytes)

Powered by blists - more mailing lists