Re: [PATCH v1 1/7] dt-bindings: i2c: qcom,i2c-geni: Document DT properties for QUP firmware loading

Viken Dadhaniya
2024-12-10T05:28:17Z

On 12/4/2024 10:55 PM, Doug Anderson wrote:
> Hi,
>
> On Wed, Dec 4, 2024 at 7:03 AM Viken Dadhaniya
> <quic_vdadhani@quicinc.com> wrote:
>>
>> Document the 'qcom,load-firmware' and 'qcom,xfer-mode' properties to
>> support SE(Serial Engine) firmware loading from the protocol driver and to
>> select the data transfer mode, either GPI DMA (Generic Packet Interface)
>> or non-GPI mode (PIO/CPU DMA).
>>
>> I2C controller can operate in one of two modes based on the
>> 'qcom,xfer-mode' property, and the firmware is loaded accordingly.
>>
>> Co-developed-by: Mukesh Kumar Savaliya <quic_msavaliy@quicinc.com>
>> Signed-off-by: Mukesh Kumar Savaliya <quic_msavaliy@quicinc.com>
>> Signed-off-by: Viken Dadhaniya <quic_vdadhani@quicinc.com>
>> ---
>> .../devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml | 11 +++++++++++
>> 1 file changed, 11 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..a26f34fce1bb 100644
>> --- a/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml
>> +++ b/Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml
>> @@ -66,6 +66,15 @@ properties:
>> required-opps:
>> maxItems: 1
>>
>> + qcom,load-firmware:
>> + type: boolean
>> + description: Optional property to load SE (serial engine) Firmware from protocol driver.
>> +
>> + qcom,xfer-mode:
>> + description: Value 1,2 and 3 represents FIFO, CPU DMA and GSI DMA mode respectively.
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + enum: [1, 2, 3]
>
> I'm a little confused about this. I'll admit I haven't fully analyzed
> your patch with actual code in it, but in the past "CPU DMA" mode and
> "FIFO" mode were compatible with each other and then it was up to the
> driver to decide which of the two modes made sense in any given
> situation. For instance, last I looked at the i2c driver it tried to
> use DMA for large transfers and FIFO for small transfers. The SPI
> driver also has some cases where it will use DMA mode and then
> fallback to FIFO mode.
>
> ...so what exactly is the point of differentiating between "FIFO" and
> "CPU DMA" mode here?

Yes, correct, Will update in V2.
I plan to add 2 modes, GSI and non-GSI(PIO or DMA based on length).

>
> Then when it comes to "GSI DMA" mode, my understanding is that the
> firmware for "GSI DMA" mode is always loaded by Trustzone because the
> whole point is that the GSI mode arbitrates between multiple clients.
> Presumably if the firmware already loaded the GSI firmware then the
> code would just detect that case. ...so there shouldn't need to be any
> reason to specify GSI mode here either, right?
>
> -Doug

GSI firmware is loaded from TZ per QUP, but to use GSI mode,
we need to configure the SE to use GSI mode by writing into SE register
QUPV3_SE_GENI_DMA_MODE_EN and SE_GSI_EVENT_EN. This register is
used to configure data transfer mode for Serial Engine.