[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <63cbf052-86cb-74d2-f189-2eedc05343ba@quicinc.com>
Date: Mon, 9 Dec 2024 11:08:07 +0530
From: Md Sadre Alam <quic_mdalam@...cinc.com>
To: Vinod Koul <vkoul@...nel.org>
CC: <thara.gopinath@...il.com>, <herbert@...dor.apana.org.au>,
<davem@...emloft.net>, <kees@...nel.org>, <robin.murphy@....com>,
<fenghua.yu@...el.com>, <av2082000@...il.com>,
<u.kleine-koenig@...gutronix.d>, <linux-crypto@...r.kernel.org>,
<linux-arm-msm@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<dmaengine@...r.kernel.org>, <quic_varada@...cinc.com>,
<quic_srichara@...cinc.com>
Subject: Re: [PATCH v4 02/11] dmaengine: qcom: bam_dma: add LOCK & UNLOCK flag
support
On 12/4/2024 6:15 PM, Vinod Koul wrote:
> 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@...cinc.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
Ok
>
>> +#define SW_VERSION_MINOR_MASK 0xf
>
> Can you define only masks and use FIELD_PREP etc to handle this
Ok
>
>> +#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
Ok
>
>> +
>> + 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...
Ok , will prepare separate patch for these flags in next revision.
>
>
>> };
>>
>> /**
>> --
>> 2.34.1
>
Powered by blists - more mailing lists