[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <bb9e6ba2-4aeb-40ee-b123-9c59a0efa098@kernel.org>
Date: Tue, 10 Sep 2024 11:54:01 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Mukesh Kumar Savaliya <quic_msavaliy@...cinc.com>,
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,
conor+dt@...nel.org, agross@...nel.org, devicetree@...r.kernel.org,
vkoul@...nel.org, linux@...blig.org, dan.carpenter@...aro.org,
Frank.Li@....com, konradybcio@...nel.org
Cc: quic_vdadhani@...cinc.com
Subject: Re: [PATCH v2 1/4] dt-bindindgs: i2c: qcom,i2c-geni: Document shared
flag
On 10/09/2024 11:09, Mukesh Kumar Savaliya wrote:
> Thanks Krzysztof.
>
> On 9/7/2024 2:34 PM, Krzysztof Kozlowski wrote:
>> On 06/09/2024 21:14, Mukesh Kumar Savaliya wrote:
>>> Adds qcom,shared-se flag usage. Use this when particular I2C serial
>>> controller needs to be shared between two subsystems.
>>
>> <form letter>
>> Please use scripts/get_maintainers.pl to get a list of necessary people
>> and lists to CC (and consider --no-git-fallback argument). It might
>> happen, that command when run on an older kernel, gives you outdated
>> entries. Therefore please be sure you base your patches on recent Linux
>> kernel.
>>
>> Tools like b4 or scripts/get_maintainer.pl provide you proper list of
>> people, so fix your workflow. Tools might also fail if you work on some
>> ancient tree (don't, instead use mainline) or work on fork of kernel
>> (don't, instead use mainline). Just use b4 and everything should be
>> fine, although remember about `b4 prep --auto-to-cc` if you added new
>> patches to the patchset.
>> </form letter>
>>
>> You already got this comment, so how many times it has to be repeated?
>> Your process is just wrong if you do not use the tools for this.
>>
> Sorry, I was already using scripts/get_maintainer.pl but i kept everyone
> into To list (That's my mistake here). I shall keep maintainers in TO
> list and rest in CC list.
No, To or Cc does not matter. Your list is just incomplete.
>
> Question: With <Form Letter> , are you asking to add letter in this
> first patch ? I have cover letter, but it will get removed when patch
> gets merged. Please help suggest and clarify.
No, it's just template. Form letter... I am just bored to repeat the
same comment.
>>
>>>
>>> SE = Serial Engine, meant for I2C controller here.
>>> TRE = Transfer Ring Element, refers to Queued Descriptor.
>>>
>>> Example :
>>> Two clients from different SS can share an I2C SE for same slave device
>>
>> What is SS?
>>
> SS = Subsystem (EE - Execution Environment, can be Apps
> processor/TZ/Modem/ADSP etc). Let me add this too in next patch.
Yes, please explain in the binding itself.
>>> OR their owned slave devices.
>>> Assume I2C Slave EEPROM device connected with I2C controller.
>>> Each client from ADSP SS and APPS Linux SS can perform i2c transactions.
>>> This gets serialized by lock TRE + DMA Transfers + Unlock TRE at HW level.
>>>
>>> Signed-off-by: Mukesh Kumar Savaliya <quic_msavaliy@...cinc.com>
>>> ---
>>> Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml | 4 ++++
>>> 1 file changed, 4 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml b/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml
>>> index 9f66a3bb1f80..ae423127f736 100644
>>> --- a/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml
>>> +++ b/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml
>>> @@ -60,6 +60,10 @@ properties:
>>> power-domains:
>>> maxItems: 1
>>>
>>> + qcom,shared-se:
>>> + description: True if I2C needs to be shared between two or more subsystems.
>>
>> What is a subsystem? With commit msg I still do not understand this.
> SS = Subsystem (EE - Execution Environment, can be Apps
> processor/TZ/Modem/ADSP etc). Let me add EE too with full form.
>> Maybe presence of hwlock defines it anyway, so this is redundant?
> No, this flag is required. As hwlock comes into picture if this flag is
Flag is required? By what? Sorry, you push your downstream solution to us.
> defined. So flag is acting as a condition to take hwlock TRE
> descriptor(transfer ring element). Hope i could answer your query.
Hm, not sure, maybe indeed hwlock would not be enough. However I think
existing binding misses hwlock property.
Best regards,
Krzysztof
Powered by blists - more mailing lists