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.

quoted

+};
 /**
  * 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