From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751810AbbLUNTK (ORCPT ); Mon, 21 Dec 2015 08:19:10 -0500 Received: from smtp687.redcondor.net ([208.80.206.87]:42283 "EHLO smtp687.redcondor.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751436AbbLUNTG (ORCPT ); Mon, 21 Dec 2015 08:19:06 -0500 X-RC-FROM: Subject: Re: [PATCH 1/3] ata: sata_dwc_460ex: use "dmas" DT property to find dma channel To: =?UTF-8?B?TcOlbnMgUnVsbGfDpXJk?= References: <1450221935-6034-1-git-send-email-mans@mansr.com> <567541EE.9010308@candw.ms> <56758F33.20804@candw.ms> <5675A84F.2070208@candw.ms> <5675BB2F.6060107@candw.ms> <5675C452.2080206@candw.ms> <5676E906.1060603@candw.ms> <5677D447.40906@candw.ms> Cc: Andy Shevchenko , Viresh Kumar , Andy Shevchenko , Tejun Heo , linux-ide@vger.kernel.org, "linux-kernel@vger.kernel.org" From: Julian Margetson Message-ID: <5677FC31.1030305@candw.ms> Date: Mon, 21 Dec 2015 09:18:41 -0400 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 8bit X-DLP-ENABLED: 137.118.22.64/27 X-MAG-OUTBOUND: greymail.redcondor.net@137.118.22.64/27 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 12/21/2015 8:16 AM, Måns Rullgård wrote: > Julian Margetson writes: > >> On 12/21/2015 4:40 AM, Andy Shevchenko wrote: >>> +Viresh >>> >>> On Mon, Dec 21, 2015 at 2:58 AM, Måns Rullgård wrote: >>>> Andy Shevchenko writes: >>>> >>>>> On Sun, Dec 20, 2015 at 8:49 PM, Måns Rullgård wrote: >>>>>> Julian Margetson writes: >>>>>>> On 12/20/2015 1:11 PM, Måns Rullgård wrote: >>>>>>>> Julian Margetson writes: >>>>>>> [ 48.769671] ata3.00: failed command: READ FPDMA QUEUED >>>>>> Well, that didn't help. I still think it's part of the problem, but >>>>>> something else must be wrong as well. The various Master Select fields >>>>>> look like a good place to start. >>>>> Master number (which is here would be either 1 or 0) should not affect >>>>> as long as they are connected to the same AHB bus (I would be >>>>> surprised if they are not). >>>> I think they are not. The relevant part of the block diagram for the >>>> 460EX looks something like this: >>>> >>>> +-----+ >>>> | CPU | >>>> +-----+ >>>> | >>>> +---------------+ >>>> | BUS | >>>> +---------------+ >>>> | | >>>> +-----+ +-----+ >>>> | DMA | | RAM | >>>> +-----+ +-----+ >>>> | >>>> +------+ >>>> | SATA | >>>> +------+ >>>> >>>> The DMA-SATA link is private and ignores the address, which is the only >>>> reason the driver can possibly work (it's programming a CPU virtual >>>> address there). >>> If you look at the original code the SMS and DMS are programmed >>> statically independent on DMA direction, so LLP is programmed always >>> to master 1. I don't think your scheme is reflecting this right. I >>> could imagine two AHB buses, one of them connects CPU, SATA and RAM, >>> and the other CPU and DMA. >>> >>> In any case on all Intel SoCs and AVR32, and as far as I can tell on >>> Spear13xx (Viresh?) there is not a case, that's why I hardly imagine >>> that the problem is in master numbers by themselves. >>> >>>>>> Also, the manual says the LLP_SRC_EN >>>>>> and LLP_DST_EN flags should be cleared on the last in a chain of blocks. >>>>>> The old sata_dwc driver does this whereas dw_dma does not. >>>>> Easy to fix, however I can't get how it might affect. >>>> From the Atmel doc: >>>> >>>> In Table 17-1 on page 185, all other combinations of LLPx.LOC = 0, >>>> CTLx.LLP_S_EN, CFGx.RELOAD_SR, CTLx.LLP_D_EN, and CFGx.RELOAD_DS are >>>> illegal, and causes indeterminate or erroneous behavior. >>> I will check Synospys documentation later on. >>> >>>> Most likely nothing happens, but I think it ought to be fixed. In fact, >>>> I have a patch already. >>> Good. Send with Fixes tag if it's upstream ready. >>> >>>> Come to think of it, I have an AVR32 dev somewhere. Maybe I should dust >>>> it off. >>> I have ATNGW100. >>> >>> 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 ' or 'git rm ' >> 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) ^ make[2]: *** [drivers/ata/sata_dwc_460ex.o] Error 1 make[1]: *** [drivers/ata] Error 2 make: *** [drivers] Error 2 make: *** Waiting for unfinished jobs....