Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Radhey Shyam Pandey <radheys@xilinx.com>
To:     Peter Ujfalusi <peter.ujfalusi@ti.com>,
        "dan.j.williams@intel.com" <dan.j.williams@intel.com>,
        "vkoul@kernel.org" <vkoul@kernel.org>
CC:     "dmaengine@vger.kernel.org" <dmaengine@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "lars@metafoo.de" <lars@metafoo.de>
Subject: RE: [RFC] dmaengine: Add metadata_ops for dma_async_tx_descriptor
Thread-Topic: [RFC] dmaengine: Add metadata_ops for dma_async_tx_descriptor
Thread-Index: AQHUNIa/aLzeL9Zb/0iQNP+9dl8PvKTCWczQ
Date:   Thu, 16 Aug 2018 13:29:12 +0000
Message-ID: <CY4PR0201MB36192D111C6EE8AC4E86C173C73E0@CY4PR0201MB3619.namprd02.prod.outlook.com>
References: <20180815105704.26498-1-peter.ujfalusi@ti.com>
In-Reply-To: <20180815105704.26498-1-peter.ujfalusi@ti.com>
Accept-Language: en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: xilinx.com does not designate
 permitted sender hosts)
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: aa093fd1-c00f-4b8c-6f74-08d6037c4104
X-MS-Exchange-CrossTenant-originalarrivaltime: 16 Aug 2018 13:29:12.0672
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 657af505-d5df-48d0-8300-c31994686c5c
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY4PR0201MB3395
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> -----Original Message-----
> From: Peter Ujfalusi <peter.ujfalusi@ti.com>
> Sent: Wednesday, August 15, 2018 4:27 PM
> To: dan.j.williams@intel.com; vkoul@kernel.org
> Cc: dmaengine@vger.kernel.org; linux-kernel@vger.kernel.org;
> lars@metafoo.de; Radhey Shyam Pandey <radheys@xilinx.com>
> Subject: [RFC] dmaengine: Add metadata_ops for dma_async_tx_descriptor
>=20
> The metadata is best described as side band data or parameters traveling
> alongside the data DMAd by the DMA engine. It is data
> which is understood by the peripheral and the peripheral driver only, the
> DMA engine see it only as data block and it is not interpreting it in any
> way.
>=20
> The metadata can be different per descriptor as it is a parameter for the
> data being transferred.
>=20
> If the DMA supports per descriptor metadata it can implement the attach,
> get_ptr/set_len callbacks.
>=20
> Client drivers must only use either attach or get_ptr/set_len to avoid
> miss configuration.
>=20
> Client driver can check if a given metadata mode is supported by the
> channel during probe time with
> dmaengine_is_metadata_mode_supported(chan, DESC_METADATA_CLIENT);
> dmaengine_is_metadata_mode_supported(chan,
> DESC_METADATA_EMBEDDED);
>=20
> and based on this information can use either mode.
>=20
> Wrappers are also added for the metadata_ops.
>=20
> To be used in DESC_METADATA_CLIENT mode:
> dmaengine_desc_attach_metadata()
>=20
> To be used in DESC_METADATA_EMBEDDED mode:
> dmaengine_desc_get_metadata_ptr()
> dmaengine_desc_set_metadata_len()
>=20
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> ---
>  include/linux/dmaengine.h | 112
> ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 112 insertions(+)
>=20
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index 3db833a8c542..2200f8985adf 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -231,6 +231,25 @@ typedef struct { DECLARE_BITMAP(bits,
> DMA_TX_TYPE_END); } dma_cap_mask_t;
>   * @bytes_transferred: byte counter
>   */
>=20
> +/**
> + * 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_meta=
data()
> + *  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?=20
i.e metadata allocation place - >  dma client/engine.

> + *  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=20
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.

> + *
> + * Note: the two mode is not compatible and clients must use one mode fo=
r a
> + * descriptor.
> + */
> +enum dma_desc_metadata_mode {
> +	DESC_METADATA_CLIENT =3D (1 << 0),
> +	DESC_METADATA_EMBEDDED =3D (1 << 1),
> +};
> +
>  struct dma_chan_percpu {
>  	/* stats */
>  	unsigned long memcpy_count;
> @@ -494,6 +513,18 @@ struct dmaengine_unmap_data {
>  	dma_addr_t addr[0];
>  };
>=20
> +struct dma_async_tx_descriptor;
> +
> +struct dma_descriptor_metadata_ops {
> +	int (*attach)(struct dma_async_tx_descriptor *desc, void *data,
> +		      size_t len);
> +
> +	void *(*get_ptr)(struct dma_async_tx_descriptor *desc,
> +			 size_t *payload_len, size_t *max_len);
> +	int (*set_len)(struct dma_async_tx_descriptor *desc,
> +		       size_t payload_len);
> +};
> +
>  /**
>   * struct dma_async_tx_descriptor - async transaction descriptor
>   * ---dma generic offload fields---
> @@ -523,6 +554,8 @@ struct dma_async_tx_descriptor {
>  	dma_async_tx_callback_result callback_result;
>  	void *callback_param;
>  	struct dmaengine_unmap_data *unmap;
> +	enum dma_desc_metadata_mode desc_metadata_mode;
> +	struct dma_descriptor_metadata_ops *metadata_ops;
>  #ifdef CONFIG_ASYNC_TX_ENABLE_CHANNEL_SWITCH
>  	struct dma_async_tx_descriptor *next;
>  	struct dma_async_tx_descriptor *parent;
> @@ -685,6 +718,7 @@ struct dma_filter {
>   * @global_node: list_head for global dma_device_list
>   * @filter: information for device/slave to filter function/param mappin=
g
>   * @cap_mask: one or more dma_capability flags
> + * @desc_metadata_modes: supported metadata modes by the DMA device
>   * @max_xor: maximum number of xor sources, 0 if no capability
>   * @max_pq: maximum number of PQ sources and PQ-continue capability
>   * @copy_align: alignment shift for memcpy operations
> @@ -749,6 +783,7 @@ struct dma_device {
>  	struct list_head global_node;
>  	struct dma_filter filter;
>  	dma_cap_mask_t  cap_mask;
> +	enum dma_desc_metadata_mode desc_metadata_modes;
>  	unsigned short max_xor;
>  	unsigned short max_pq;
>  	enum dmaengine_alignment copy_align;
> @@ -935,6 +970,83 @@ static inline struct dma_async_tx_descriptor
> *dmaengine_prep_dma_memcpy(
>  						    len, flags);
>  }
>=20
> +static inline bool dmaengine_is_metadata_mode_supported(struct dma_chan
> *chan,
> +		enum dma_desc_metadata_mode mode)
> +{
> +	return !!(chan->device->desc_metadata_modes & mode);
> +}
> +
> +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.

> +		if (dmaengine_is_metadata_mode_supported(desc->chan,
> mode))
> +			desc->desc_metadata_mode =3D mode;
> +		else
> +			return -ENOTSUPP;
> +	} else if (desc->desc_metadata_mode !=3D mode) {
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static inline int dmaengine_desc_attach_metadata(
> +		struct dma_async_tx_descriptor *desc, void *data, size_t len)
> +{
> +	int ret;
> +
> +	if (!desc)
> +		return -EINVAL;
> +
> +	ret =3D _desc_check_and_set_metadata_mode(desc,
> DESC_METADATA_CLIENT);
> +	if (ret)
> +		return ret;
> +
> +	if (!desc->metadata_ops || !desc->metadata_ops->attach)
> +		return -ENOTSUPP;
> +
> +	return desc->metadata_ops->attach(desc, data, len);
> +}
> +
> +static inline void *dmaengine_desc_get_metadata_ptr(
> +		struct dma_async_tx_descriptor *desc, size_t *payload_len,
> +		size_t *max_len)
> +{
> +	int ret;
> +
> +	if (!desc)
> +		return ERR_PTR(-EINVAL);
> +
> +	ret =3D _desc_check_and_set_metadata_mode(desc,
> DESC_METADATA_EMBEDDED);
> +	if (ret)
> +		return ERR_PTR(ret);
> +
> +	if (!desc->metadata_ops || !desc->metadata_ops->get_ptr)
> +		return ERR_PTR(-ENOTSUPP);
> +
> +	return desc->metadata_ops->get_ptr(desc, payload_len, max_len);
> +}
> +
> +static inline int dmaengine_desc_set_metadata_len(
> +		struct dma_async_tx_descriptor *desc, size_t payload_len)
> +{
> +	int ret;
> +
> +	if (!desc)
> +		return -EINVAL;
> +
> +	ret =3D _desc_check_and_set_metadata_mode(desc,
> DESC_METADATA_EMBEDDED);
> +	if (ret)
> +		return ret;
> +
> +	if (!desc->metadata_ops || !desc->metadata_ops->set_len)
> +		return -ENOTSUPP;
> +
> +	return desc->metadata_ops->set_len(desc, payload_len);
> +}
> +
>  /**
>   * dmaengine_terminate_all() - Terminate all active DMA transfers
>   * @chan: The channel for which to terminate the transfers
> --
> Peter
>=20
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki