Hi Vinod,

On 25/08/2020 14.02, Vinod Koul wrote:

quoted

The only thing which might be an issue is that with the DMA_PREP_CMD the
config_data is dma_addr_t (via dmaengine_prep_slave_single).

Yes I came to same conclusion

quoted

quoted

I did have a prototype with metadata but didnt work very well, the
problem is it assumes metadata for tx/rx but here i send the data
everytime from client data.

Yes, the intended use case for metadata (per descriptor!) is for
channels where each transfer might have different metadata needed for
the given transfer (tx/rx).

In your case you have semi static peripheral configuration data, which
is not really changing between transfers.

A compromise would be to add:
void *peripheral_config;
to the dma_slave_config, move the set_config inside of the device
specific struct you are passing from a client to the core?

That sounds more saner to me and uses existing interfaces cleanly. I
think I like this option ;-)

The other option would be to use the descriptor metadata support and
that might be even cleaner.

In gpi_create_tre() via gpi_prep_slave_sg() you would set up the
desc->tre[1] and desc->tre[2] for TX
desc->tre[2] for RX
in the desc, you add a new variable, let's say first_tre and
set it to 1 for TX, 2 for RX.

If you need to send a config, you attach it via either way metadata
support allows you (get the pointer to desc->tre[0] or give a config
struct to the DMA driver.

In the metadata handler, you check if the transfer is TX, if it is, then
you update desc->tre[0] (or give the pointer to the client) and update
the first_tre to 0.

In issue_pending, or a small helper which can be used to start the
transfer you would do the queuing instead of prepare time:
for (i = gpi_desc->first_tre; i < MAX_TRE; i++)
	gpi_queue_xfer(gpii, gpii_chan,  &gpi_desc->tre[i], &wp);

With this change it should work neatly without any change to
dma_slave_config.

quoted

quoted

quoted

I'm concerned about the size increase of dma_slave_config (it grows by

quoted

30 bytes) and for DMAs with hundreds of channels (UDMA) it will add up

to a sizeable amount.

I agree that is indeed a valid concern, that is the reason I tagged this
as a RFC patch ;-)

I see the prep_cmd is a better approach for this, anyone else has better
suggestions?

Thanks for looking in.

- 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