Re: [PATCH 1/3] ata: sata_dwc_460ex: use "dmas" DT property to find dma channel

["Måns Rullgård" <mans@mansr.com>]

Julian Margetson <runaway@candw.ms> 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 <mans@mansr.com> wrote:
>>> Andy Shevchenko <andy.shevchenko@gmail.com> writes:
>>>
>>>> On Sun, Dec 20, 2015 at 8:49 PM, Måns Rullgård <mans@mansr.com> wrote:
>>>>> Julian Margetson <runaway@candw.ms> writes:
>>>>>> On 12/20/2015 1:11 PM, Måns Rullgård wrote:
>>>>>>> Julian Margetson <runaway@candw.ms> 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 <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

-- 
Måns Rullgård