Received-SPF: pass (google.com: domain of linux-kernel+bounces-129069-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <669b516a-74c7-445a-b151-5463fe39b21b@quicinc.com>
Date: Wed, 3 Apr 2024 11:26:42 +0530
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2] i2c: i2c-qcom-geni: Add support to share an I2C SE
 from two subsystem
Content-Language: en-US
To: Konrad Dybcio <konrad.dybcio@linaro.org>, <andersson@kernel.org>,
        <andi.shyti@kernel.org>, <linux-arm-msm@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <linux-i2c@vger.kernel.org>,
        <dmaengine@vger.kernel.org>
CC: <quic_vdadhani@quicinc.com>, Vinod Koul <vkoul@kernel.org>
References: <20240402062131.9836-1-quic_msavaliy@quicinc.com>
 <51c84af2-73f7-4af4-8676-2276b6c7786d@linaro.org>
From: Mukesh Kumar Savaliya <quic_msavaliy@quicinc.com>
In-Reply-To: <51c84af2-73f7-4af4-8676-2276b6c7786d@linaro.org>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit

Thanks Konrad. I understood.

On 4/2/2024 7:54 PM, Konrad Dybcio wrote:
> On 2.04.2024 8:21 AM, Mukesh Kumar Savaliya wrote:
>> Add feature to share an I2C serial engine between two subsystems(SS) so
>> that individual clients from different subsystems can access the same bus.
>> For example single i2c slave device can be accessed by Client driver from
>> APPS OR modem subsystem image. Same way we can have slave being accessed
>> between APPS and TZ subsystems.
>>
>> This is possible in GSI mode where driver queues the TREs with required
>> descriptors and ensures to execute TREs in an mutually exclusive way.
>> Issue a "Lock TRE" command at the start of the transfer and an "Unlock TRE"
>> command at the end of the transfer. This prevents other subsystems from
>> concurrently performing DMA transfers and avoids disturbance to data path.
>> Change MAX_TRE macro to 5 from 3 because of these two additional TREs.
>>
>> Since the GPIOs are also shared for the i2c bus, do not touch GPIO
>> configuration while going to runtime suspend and only turn off the
>> clocks. This will allow other SS to continue to transfer the data.
>>
>> This feature needs to be controlled by DTSI flag to make it flexible
>> based on the usecase, hence during probe check the same from i2c driver.
>>
>> Export function geni_se_clks_off() to call explicitly instead of
>> geni_se_resources_off() to not modify TLMM configuration as other SS might
>> perform the transfer while APPS SS can go to sleep.
>>
>> Signed-off-by: Mukesh Kumar Savaliya <quic_msavaliy@quicinc.com>
>> ---
>> v1 -> v2:
>> - Addressed review comments.
> 
> The biggest one ("too many changes across the board") is still not
> addressed and the patch will not be further reviewed until that is done.
> 
> Each subsystem has different owners and each change requires an explanation
> (maintainers always "expect your patch to be wrong" and you need to
> convince them otherwise through commit messages)
> 
Sure, I got it. Will send patch dividing logically between i2c, dma.
I have already responded in just previous Mail to seek clarity as below.
It was :
"Please correct me if this is wrong. The overall change is for i2c in 
GSI DMA mode. This also requires changes in resource control like TLMM 
changes. But it's more like integrated feature.
Are you suggesting to make 3 sub-patches under same change ? "


> Konrad