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

2 messages, 2 authors, 2015-12-21 · open the first message on its own page

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

From: Måns Rullgård <hidden>
Date: 2015-12-21 00:58:14

Andy Shevchenko [off-list ref] writes:
On Sun, Dec 20, 2015 at 8:49 PM, Måns Rullgård [off-list ref] wrote:
quoted
Julian Margetson [off-list ref] writes:
quoted
On 12/20/2015 1:11 PM, Måns Rullgård wrote:
quoted
Julian Margetson [off-list ref] writes:
quoted
quoted
[   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).
quoted
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.

Most likely nothing happens, but I think it ought to be fixed.  In fact,
I have a patch already.

Come to think of it, I have an AVR32 dev somewhere.  Maybe I should dust
it off.

-- 
Måns Rullgård

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

From: Andy Shevchenko <hidden>
Date: 2015-12-21 08:40:05

+Viresh

On Mon, Dec 21, 2015 at 2:58 AM, Måns Rullgård [off-list ref] wrote:
Andy Shevchenko [off-list ref] writes:
quoted
On Sun, Dec 20, 2015 at 8:49 PM, Måns Rullgård [off-list ref] wrote:
quoted
Julian Margetson [off-list ref] writes:
quoted
On 12/20/2015 1:11 PM, Måns Rullgård wrote:
quoted
Julian Margetson [off-list ref] writes:
quoted
quoted
[   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.
quoted
quoted
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.

-- 
With Best Regards,
Andy Shevchenko
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help