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

[Julian Margetson]

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


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/