[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <523acc68-2e85-4e33-b87f-8400411ac00b@quicinc.com>
Date: Wed, 4 Sep 2024 23:53:45 +0530
From: Mukesh Kumar Savaliya <quic_msavaliy@...cinc.com>
To: Bryan O'Donoghue <bryan.odonoghue@...aro.org>, <konrad.dybcio@...aro.org>,
<andersson@...nel.org>, <andi.shyti@...nel.org>,
<linux-arm-msm@...r.kernel.org>, <dmaengine@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <linux-i2c@...r.kernel.org>
CC: <quic_vdadhani@...cinc.com>
Subject: Re: [PATCH v1 2/4] dma: gpi: Add Lock and Unlock TRE support to
access SE exclusively
On 8/29/2024 3:35 PM, Bryan O'Donoghue wrote:
> On 29/08/2024 10:24, Mukesh Kumar Savaliya wrote:
>> GSI DMA provides specific TREs namely Lock and Unlock TRE, which
>> provides mutual exclusive access to SE from any of the subsystem
>> (E.g. Apps, TZ, ADSP etc). Lock prevents other subsystems from
>> concurrently performing DMA transfers and avoids disturbance to
>> data path. Basically lock the SE for particular subsystem, complete
>> the transfer, unlock the SE.
>>
>> Apply Lock TRE for the first transfer of shared SE and Apply Unlock
>> TRE for the last transfer.
>>
>> Also change MAX_TRE macro to 5 from 3 because of the two additional TREs.
>>
>> Signed-off-by: Mukesh Kumar Savaliya <quic_msavaliy@...cinc.com>
>> ---
>> drivers/dma/qcom/gpi.c | 37 +++++++++++++++++++++++++++++++-
>> include/linux/dma/qcom-gpi-dma.h | 6 ++++++
>> 2 files changed, 42 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/dma/qcom/gpi.c b/drivers/dma/qcom/gpi.c
>> index e6ebd688d746..ba11b2641ab6 100644
>> --- a/drivers/dma/qcom/gpi.c
>> +++ b/drivers/dma/qcom/gpi.c
>> @@ -2,6 +2,7 @@
>> /*
>> * Copyright (c) 2017-2020, The Linux Foundation. All rights reserved.
>> * Copyright (c) 2020, Linaro Limited
>> + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights
>> reserved.
>> */
>> #include <dt-bindings/dma/qcom-gpi.h>
>> @@ -65,6 +66,14 @@
>> /* DMA TRE */
>> #define TRE_DMA_LEN GENMASK(23, 0)
>> +/* Lock TRE */
>> +#define TRE_I2C_LOCK BIT(0)
>> +#define TRE_MINOR_TYPE GENMASK(19, 16)
>> +#define TRE_MAJOR_TYPE GENMASK(23, 20)
>> +
>> +/* Unlock TRE */
>> +#define TRE_I2C_UNLOCK BIT(8)
>> +
>> /* Register offsets from gpi-top */
>> #define GPII_n_CH_k_CNTXT_0_OFFS(n, k) (0x20000 + (0x4000 * (n))
>> + (0x80 * (k)))
>> #define GPII_n_CH_k_CNTXT_0_EL_SIZE GENMASK(31, 24)
>> @@ -516,7 +525,7 @@ struct gpii {
>> bool ieob_set;
>> };
>> -#define MAX_TRE 3
>> +#define MAX_TRE 5
>> struct gpi_desc {
>> struct virt_dma_desc vd;
>> @@ -1637,6 +1646,19 @@ static int gpi_create_i2c_tre(struct gchan
>> *chan, struct gpi_desc *desc,
>> struct gpi_tre *tre;
>> unsigned int i;
>> + /* create lock tre for first tranfser */
>> + if (i2c->shared_se && i2c->first_msg) {
>> + tre = &desc->tre[tre_idx];
>> + tre_idx++;
>> +
>> + tre->dword[0] = 0;
>> + tre->dword[1] = 0;
>> + tre->dword[2] = 0;
>> + tre->dword[3] = u32_encode_bits(1, TRE_I2C_LOCK);
>> + tre->dword[3] |= u32_encode_bits(0, TRE_MINOR_TYPE);
>> + tre->dword[3] |= u32_encode_bits(3, TRE_MAJOR_TYPE);
>> + }
>> +
>> /* first create config tre if applicable */
>> if (i2c->set_config) {
>> tre = &desc->tre[tre_idx];
>> @@ -1695,6 +1717,19 @@ static int gpi_create_i2c_tre(struct gchan
>> *chan, struct gpi_desc *desc,
>> tre->dword[3] |= u32_encode_bits(1, TRE_FLAGS_IEOT);
>> }
>> + /* Unlock tre for last transfer */
>> + if (i2c->shared_se && i2c->last_msg && i2c->op != I2C_READ) {
>> + tre = &desc->tre[tre_idx];
>> + tre_idx++;
>> +
>> + tre->dword[0] = 0;
>> + tre->dword[1] = 0;
>> + tre->dword[2] = 0;
>> + tre->dword[3] = u32_encode_bits(1, TRE_I2C_UNLOCK);
>> + tre->dword[3] |= u32_encode_bits(1, TRE_MINOR_TYPE);
>> + tre->dword[3] |= u32_encode_bits(3, TRE_MAJOR_TYPE);
>> + }
>> +
>
> What happens if the first transfer succeeds => bus lock but the last
> transfer fails => !unlock ?
>
> Is the SE left in a locked state ?
>
Here, it's GSI running the transfer descriptors and if it fails, we do
dmaengine_terminate_sync() at i2c client side. It clears the descriptors
and unmaps the buffer. SE remains in unlocked like initial state.
> ---
> bod
Powered by blists - more mailing lists