Hi Vinod,
On 29/09/2020 11.06, Peter Ujfalusi wrote:
>
> I know that you want this to be as generic as much as it is possible,
> but do we really want to?
> GPIv2 will also handle I2S peripheral, other vendor's similar solution
> would require different sets of parameters unique to their IPs?
>
> How we are going to handle similar setups for DMA which is used for
> networking, SPI/I2C/I2S/NAND/display/capture, etc?
>
> Imho these settings are really part of the peripheral's domain and not
> the DMA. It is just a small detail that instead of direct register
> writes, your setup is using the DMA descriptors to write.
> It is similar to what I use as metadata (part of the descriptor belongs
> and owned by the client driver).
>
> I think it would be better to have:
>
> enum dmaengine_peripheral {
> DMAENGINE_PERIPHERAL_GPI_SPI = 1,
> DMAENGINE_PERIPHERAL_GPI_UART,
> DMAENGINE_PERIPHERAL_GPI_I2C,
> DMAENGINE_PERIPHERAL_XYZ_SPI,
> DMAENGINE_PERIPHERAL_XYZ_AASRC,
> DMAENGINE_PERIPHERAL_ABC_CAM,
> ...
> DMAENGINE_PERIPHERAL_LAST,
> };
>
> enum dmaengine_peripheral peripheral_type;
> void *peripheral_config;
TI have an AASRC (Audio Asynchronous Sample Rate Converted) in j721e and
to configure the DMA side (AASRC_PDMA) we need special configuration
parameters passed from the AASRC driver to the DMA channel.
This peripheral config extension would be perfect for it, but the
parameters I would need is not generic in any ways.
The other thing which might need to be considered is to have src/dst
pair of this. When we do DMA_DEV_TO_DEV, it would help to figure out
which side we should apply which config (if you have the same type of
device on both ends with different config?).
> and that's it. The set_config is specific to GPI.
> It can be debated where the structs should be defined, in the generic
> dmaengine.h or in include/linux/dma/ as controller specific
> (gpi_peripheral.h) or a generic one, like dmaengine_peripheral.h
>
> The SPI/I2C/UART client of yours would pass the GPI specific struct as
> in any case it has to know what is the DMA it is serviced by.
>
>> +};
>> /**
>> * struct dma_slave_config - dma slave channel runtime config
>> * @direction: whether the data shall go in or out on this slave
>> @@ -418,6 +506,8 @@ enum dma_slave_buswidth {
>> * @slave_id: Slave requester id. Only valid for slave channels. The dma
>> * slave peripheral will have unique id as dma requester which need to be
>> * pass as slave config.
>> + * @peripheral: peripheral configuration for programming peripheral for
>> + * dmaengine transfer
>> *
>> * This struct is passed in as configuration data to a DMA engine
>> * in order to set up a certain channel for DMA transport at runtime.
>> @@ -443,6 +533,7 @@ struct dma_slave_config {
>> u32 dst_port_window_size;
>> bool device_fc;
>> unsigned int slave_id;
>> + struct dmaengine_peripheral_config *peripheral;
>> };
>>
>> /**
>>
>
> - Péter
>
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>
- Péter
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
