Re: [PATCH v4 02/11] dmaengine: qcom: bam_dma: add LOCK & UNLOCK flag support

[Vinod Koul <vkoul@kernel.org>]

On 09-09-24, 14:56, Md Sadre Alam wrote:
> Add lock and unlock flag support on command descriptor.
> Once lock set in requester pipe, then the bam controller
> will lock all others pipe and process the request only
> from requester pipe. Unlocking only can be performed from
> the same pipe.
> 
> If DMA_PREP_LOCK flag passed in command descriptor then requester
> of this transaction wanted to lock the BAM controller for this
> transaction so BAM driver should set LOCK bit for the HW descriptor.
> 
> If DMA_PREP_UNLOCK flag passed in command descriptor then requester
> of this transaction wanted to unlock the BAM controller.so BAM driver
> should set UNLOCK bit for the HW descriptor.
> 
> BAM IP version 1.4.0 and above only supports this LOCK/UNLOCK
> feature. So adding check for the same and setting bam_pipe_lock
> based on BAM SW Version.
> 
> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
> ---
> 
> Change in [v4]
> 
> * Added BAM_SW_VERSION read for major & minor
>   version
> 
> * Added bam_pipe_lock flag 
> 
> Change in [v3]
> 
> * Moved lock/unlock bit set inside loop
> 
> Change in [v2]
> 
> * No change
>  
> Change in [v1]
> 
> * Added initial support for BAM pipe lock/unlock
> 
>  drivers/dma/qcom/bam_dma.c | 25 ++++++++++++++++++++++++-
>  include/linux/dmaengine.h  |  6 ++++++
>  2 files changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
> index 3a2965939531..d30416a26d8e 100644
> --- a/drivers/dma/qcom/bam_dma.c
> +++ b/drivers/dma/qcom/bam_dma.c
> @@ -53,11 +53,20 @@ struct bam_desc_hw {
>  
>  #define BAM_DMA_AUTOSUSPEND_DELAY 100
>  
> +#define SW_VERSION_MAJOR_SHIFT	28
> +#define SW_VERSION_MINOR_SHIFT	16
> +#define SW_VERSION_MAJOR_MASK	(0xf << SW_VERSION_MAJOR_SHIFT)

Use GENMASK for this

> +#define SW_VERSION_MINOR_MASK	0xf

Can you define only masks and use FIELD_PREP etc to handle this

> +#define SW_MAJOR_1	0x1
> +#define SW_VERSION_4	0x4
> +
>  #define DESC_FLAG_INT BIT(15)
>  #define DESC_FLAG_EOT BIT(14)
>  #define DESC_FLAG_EOB BIT(13)
>  #define DESC_FLAG_NWD BIT(12)
>  #define DESC_FLAG_CMD BIT(11)
> +#define DESC_FLAG_LOCK BIT(10)
> +#define DESC_FLAG_UNLOCK BIT(9)
>  
>  struct bam_async_desc {
>  	struct virt_dma_desc vd;
> @@ -393,6 +402,7 @@ struct bam_device {
>  	u32 ee;
>  	bool controlled_remotely;
>  	bool powered_remotely;
> +	bool bam_pipe_lock;
>  	u32 active_channels;
>  	u32 bam_sw_version;
>  
> @@ -696,9 +706,15 @@ static struct dma_async_tx_descriptor *bam_prep_slave_sg(struct dma_chan *chan,
>  		unsigned int curr_offset = 0;
>  
>  		do {
> -			if (flags & DMA_PREP_CMD)
> +			if (flags & DMA_PREP_CMD) {
>  				desc->flags |= cpu_to_le16(DESC_FLAG_CMD);
>  
> +				if (bdev->bam_pipe_lock && flags & DMA_PREP_LOCK)
> +					desc->flags |= cpu_to_le16(DESC_FLAG_LOCK);
> +				else if (bdev->bam_pipe_lock && flags & DMA_PREP_UNLOCK)
> +					desc->flags |= cpu_to_le16(DESC_FLAG_UNLOCK);
> +			}
> +
>  			desc->addr = cpu_to_le32(sg_dma_address(sg) +
>  						 curr_offset);
>  
> @@ -1242,6 +1258,7 @@ static int bam_dma_probe(struct platform_device *pdev)
>  {
>  	struct bam_device *bdev;
>  	const struct of_device_id *match;
> +	u32 sw_major, sw_minor;
>  	int ret, i;
>  
>  	bdev = devm_kzalloc(&pdev->dev, sizeof(*bdev), GFP_KERNEL);
> @@ -1305,6 +1322,12 @@ static int bam_dma_probe(struct platform_device *pdev)
>  
>  	bdev->bam_sw_version = readl_relaxed(bam_addr(bdev, 0, BAM_SW_VERSION));
>  	dev_info(bdev->dev, "BAM software version:0x%08x\n", bdev->bam_sw_version);
> +	sw_major = (bdev->bam_sw_version & SW_VERSION_MAJOR_MASK) >> SW_VERSION_MAJOR_SHIFT;
> +	sw_minor = (bdev->bam_sw_version >> SW_VERSION_MINOR_SHIFT) & SW_VERSION_MINOR_MASK;

FIELD_GET

> +
> +	if (sw_major == SW_MAJOR_1 && sw_minor >= SW_VERSION_4)
> +		bdev->bam_pipe_lock = true;
> +
>  
>  	ret = bam_init(bdev);
>  	if (ret)
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index b137fdb56093..70f23068bfdc 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -200,6 +200,10 @@ struct dma_vec {
>   *  transaction is marked with DMA_PREP_REPEAT will cause the new transaction
>   *  to never be processed and stay in the issued queue forever. The flag is
>   *  ignored if the previous transaction is not a repeated transaction.
> + *  @DMA_PREP_LOCK: tell the driver that there is a lock bit set on command
> + *  descriptor.
> + *  @DMA_PREP_UNLOCK: tell the driver that there is a un-lock bit set on command
> + *  descriptor.
>   */
>  enum dma_ctrl_flags {
>  	DMA_PREP_INTERRUPT = (1 << 0),
> @@ -212,6 +216,8 @@ enum dma_ctrl_flags {
>  	DMA_PREP_CMD = (1 << 7),
>  	DMA_PREP_REPEAT = (1 << 8),
>  	DMA_PREP_LOAD_EOT = (1 << 9),
> +	DMA_PREP_LOCK = (1 << 10),
> +	DMA_PREP_UNLOCK = (1 << 11),

this should be a separate patch to define how to use these new flags,
what it means etc...


>  };
>  
>  /**
> -- 
> 2.34.1

-- 
~Vinod