Re: [PATCH v4 2/3] dw_dmac: max_mem_width limits value for SRC/DST_TR_WIDregister

[Hein Tibosch]

On 9/3/2012 4:59 PM, Viresh Kumar wrote:
> On 3 September 2012 14:19, Andy Shevchenko <andy.shevchenko@xxxxxxxxx> wrote:
>> On Mon, Sep 3, 2012 at 11:30 AM, Viresh Kumar <viresh.kumar@xxxxxxxxxx> wrote:
>>> Which register are you talking about? This configuration is outside of DMAC
>>> controller and i am not sure if dw DMAC controller can do 128 or 256
>>> bit transfers.
>> SRC_WIDTH & DST_WIDTH in CTLx. The field are 3 bit long. Acceptable
>> values from 0 to 5.
>> 2 corresponds to 32 bit transfers.
> The field is 3 bit long but only allowable values are 0,1,2 & 3... This is what
> i can check in my copy of dw_dmac manual.
>
> 4 and 5 aren't valid values.

About today's remarks about the patch series:

1. The first draft of the patches worked with the max allowable value for
the SRC_WIDTH & DST_WIDTH fields: 0,1,2,3... Viresh thought it was not
transparent enough, he suggested to make it simpler with a binary choice of
32- or 64-bits, defaulting to 64-bits.
But Andy is right: there are versions supporting 256-bit wide memory transfers.
I'd also go for this previous solution and use: "min(max_mem_width, width)"

The only problem is that one doesn't want to change arch code for other
platforms (ARM) so I proposed: let "max_mem_width=0" mean: leave it up to
the driver, for now 3 : 64-bits.

2. In another version I made 'max_mem_width' a member of 'dw_dma_platform_data'
because I also see it as 'constant' for all dma slaves.
But the dw_dmac controller can be used for multiple (types of) memories
and in that case, maybe a limit per slave might be desirable? My knowledge
of DMA-hardware doesn't reach far enough to judge that.

I'd say: for now let it become a member of 'dw_dma_platform_data' because
it's the max value of a register field.

3. Felipe Balbi: why don't we ask the DW IP for its maximum allowed value of
SRC_WIDTH & DST_WIDTH (on the memory side)? Sure, would be elegant!
I contacted Synopsys this week but as my company doesn't have a contract
with them, they won't share any secrets. And before sharing secrets, I'd need
to sign a non-disclosure contract with them...
I was particularly interested in the register called 'DMA ID Register' at
offset 0x3a8, which has no description in the Atmel sheets.

Alternatively, we could do a small dma-memcpy-test at start-up and try all
values from 5 (or 7) down to 2. The first value that works correctly will be
used as the maximum.
But that'll take some CPU time, because with invalid settings the memcpy
will timeout. I wouldn't mind to try this to see if it is a solution.


Hein
--
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/