Re: [RFC] dmaengine: Add metadata_ops for dma_async_tx_descriptor

From: Peter Ujfalusi
Date: Fri Aug 17 2018 - 02:31:02 EST


Radhey,

On 2018-08-16 16:29, Radhey Shyam Pandey wrote:
>> +/**
>> + * enum dma_desc_metadata_mode - per descriptor metadata mode types
>> supported
>> + * @DESC_METADATA_CLIENT - the metadata buffer is allocated/provided by
>> the
>> + * client driver and it is attached (via the dmaengine_desc_attach_metadata()
>> + * helper) to the descriptor.
>> + * @DESC_METADATA_EMBEDDED - the metadata buffer is
>> allocated/managed by the DMA
> Just a thought - We can rename it to DESC_METADATA_ENGINE?
> i.e metadata allocation place - > dma client/engine.

Sounds good.

>
>> + * driver. The client driver can ask for the pointer, maximum size and the
>> + * currently used size of the metadata and can directly updata or read it.
> /s/updata/update
>
>> + * dmaengine_desc_get_metadata_ptr() and
>> dmaengine_desc_set_metadata_len() is
>> + * provided as helper functions.
> It will be helpful if we add description for both DESC_METADATA_EMBEDDED
> modes i.e DMA_DEV_TO_MEM and MEM_TO_DEV types. I think in DEV_TO_MEM
> we don't need to set_metadata_len(). Length will provided by DMA engine.

I agree, it is better to extend the explanation on the two modes.

...

>> +static inline int _desc_check_and_set_metadata_mode(
>> + struct dma_async_tx_descriptor *desc, enum
>> dma_desc_metadata_mode mode)
>> +{
>> + /* Make sure that the metadata mode is not mixed */
>> + if (!desc->desc_metadata_mode) {
> Minor nit - we can refactor this code to have failure path early.

I don't think it would help readability, to move the failure case first:

if (desc->desc_metadata_mode &&
(desc->desc_metadata_mode != mode)) {
return -EINVAL;
} else if (!desc->desc_metadata_mode) {
if (dmaengine_is_metadata_mode_supported(desc->chan, mode))
desc->desc_metadata_mode = mode;
else
return -ENOTSUPP;
}

return 0;

>
>> + if (dmaengine_is_metadata_mode_supported(desc->chan,
>> mode))
>> + desc->desc_metadata_mode = mode;
>> + else
>> + return -ENOTSUPP;
>> + } else if (desc->desc_metadata_mode != mode) {
>> + return -EINVAL;
>> + }
>> +
>> + return 0;
>> +}

- PÃter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki