[PATCH 1/3] DMA: PL330: Infer transfer direction from transfer request instead of platform data

[Thomas Abraham]

Hi Rob,

On 24 August 2011 17:14, Rob Herring <robherring2 at gmail.com> wrote:
> Thomas,
>
> On 08/22/2011 04:59 PM, Thomas Abraham wrote:
>> The transfer direction for a channel can be inferred from the transfer
>> request and the need for specifying transfer direction in platfrom data
>> can be eliminated. So the structure definition 'struct dma_pl330_peri'
>> is no longer required.
>>
>> With the 'struct dma_pl330_peri' removed, the dma controller transfer
>> capabilities cannot be inferred any longer. Hence, the dma controller
>> capabilities is specified using platforme data.
>>
>> Cc: Jassi Brar <jassisinghbrar at gmail.com>
>> Cc: Boojin Kim <boojin.kim at samsung.com>
>> Signed-off-by: Thomas Abraham <thomas.abraham at linaro.org>
>> ---
>>  drivers/dma/pl330.c        |   56 ++++++++------------------------------------
>>  include/linux/amba/pl330.h |   14 +++--------
>>  2 files changed, 14 insertions(+), 56 deletions(-)
>>
>> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
>> index 3a0baac..6592b9a 100644
>> --- a/drivers/dma/pl330.c
>> +++ b/drivers/dma/pl330.c

[...]

>> @@ -872,27 +855,7 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id)
>>
>>       for (i = 0; i < num_chan; i++) {
>>               pch = &pdmac->peripherals[i];
>> -             if (pdat) {
>> -                     struct dma_pl330_peri *peri = &pdat->peri[i];
>> -
>> -                     switch (peri->rqtype) {
>> -                     case MEMTOMEM:
>> -                             dma_cap_set(DMA_MEMCPY, pd->cap_mask);
>> -                             break;
>> -                     case MEMTODEV:
>> -                     case DEVTOMEM:
>> -                             dma_cap_set(DMA_SLAVE, pd->cap_mask);
>> -                             dma_cap_set(DMA_CYCLIC, pd->cap_mask);
>> -                             break;
>> -                     default:
>> -                             dev_err(&adev->dev, "DEVTODEV Not Supported\n");
>> -                             continue;
>> -                     }
>> -                     pch->chan.private = peri;
>> -             } else {
>> -                     dma_cap_set(DMA_MEMCPY, pd->cap_mask);
>> -                     pch->chan.private = NULL;
>> -             }
>> +             pch->chan.private = (pdat) ? &pdat->peri_id[i] : NULL;
>
> Don't need parentheses here.

Ok. I will fix this in the next version of this patchset.

>
>>
>>               INIT_LIST_HEAD(&pch->work_list);
>>               spin_lock_init(&pch->lock);
>> @@ -907,6 +870,7 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id)
>>       }
>>
>>       pd->dev = &adev->dev;
>> +     pd->cap_mask = pdat->cap_mask;
>
> You are re-introducing the requirement to have platform data which I
> just made optional. For mem to mem transfers, there is no reason to have
> platform data. In my case, we only support mem to mem transfers.
>
> How about:
> pd->cap_mask = pdat ? pdat->cap_mask : DMA_MEMCPY;

All of your changes to make platform data optional is still retained
in this patch. But, as you said, the above change that I did to get
the capabilities from pdata would not work when no pdata is supplied
(as in the case mem-to-mem transfers). Thanks for pointing this out.

This can be fixed as below

if (pdat)
        pd->cap_mask = pdat->cap_mask;
else
        dma_cap_set(DMA_MEMCPY, pd->cap_mask);

>
> Also, should you be using dma_cap_set here?

Yes. That would be required. I will do these changes and resubmit the
patch. Thanks for your review.

Regards,
Thomas.

>
> Rob
>

[...]