Hi, On Mon, Dec 9, 2024 at 9:28 PM Viken Dadhaniya <quic_vdadhani@xxxxxxxxxxx> wrote: > > On 12/4/2024 10:55 PM, Doug Anderson wrote: > > Hi, > > > > On Wed, Dec 4, 2024 at 7:03 AM Viken Dadhaniya > > <quic_vdadhani@xxxxxxxxxxx> 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@xxxxxxxxxxx> > >> Signed-off-by: Mukesh Kumar Savaliya <quic_msavaliy@xxxxxxxxxxx> > >> Signed-off-by: Viken Dadhaniya <quic_vdadhani@xxxxxxxxxxx> > >> --- > >> .../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. Can't you detect it's in GSI mode without any device tree property like the code does today? -Doug