Messages in this thread | | | Date | Wed, 22 Feb 2023 11:35:34 +0100 | Subject | Re: [PATCH v5 1/2] dt-bindings: i2c: Add support for ASPEED i2Cv2 | From | Krzysztof Kozlowski <> |
| |
On 22/02/2023 11:31, Ryan Chen wrote: >> Board B >>> -------------------------------------------------------- >> -------------------------------------------------------- >>> | i2c bus#1(master/slave) <--------------------> fingerprint.(can be unplug) >> <--------------------> i2c bus#x (master/slave) | >>> | i2c bus#2(master) -> tmp i2c device | >> | | >>> | i2c bus#3(master) -> adc i2c device | | >> | >>> -------------------------------------------------------- >> -------------------------------------------------------- >>> In this case i2c bus#1 need enable timeout, avoid suddenly unplug the >> connector. That slave will keep state to drive clock stretching. >>> So it is specific enable in i2c bus#1. Others is not needed enable timeout. >>> Does this draw is more clear in scenario? >> >> I2C bus #1 works in slave mode? So you always need it for slave work? > > Yes, it is both slave/master mode. It is always dual role. Slave must always work. > Due to another board master will send.
I meant that you need this property when it works in slave mode? It would be then redundant to have in DT as it is implied by the mode.
> >>> >>>>> >>>>> So in those reason add this timeout design in controller. >>>> >>>> You need to justify why DT is correct place for this property. DT is >>>> not for configuring OS, but to describe hardware. I gave you one >>>> possibility >>>> - why different boards would like to set this property. You said it >>>> is not board specific, thus all boards will have it (or none of them). >>>> Without any other reason, this is not a DT property. Drop. >>>> >>>>>> >>>>>>> >>>>>>>>> + >>>>>>>>> + byte-mode: >>>>>>>>> + type: boolean >>>>>>>>> + description: Force i2c driver use byte mode transmit >>>>>>>> >>>>>>>> Drop, not a DT property. >>>>>>>> >>>>>>>>> + >>>>>>>>> + buff-mode: >>>>>>>>> + type: boolean >>>>>>>>> + description: Force i2c driver use buffer mode transmit >>>>>>>> >>>>>>>> Drop, not a DT property. >>>>>>>> >>>>>>> The controller support 3 different for transfer. >>>>>>> Byte mode: it means step by step to issue transfer. >>>>>>> Example i2c read, each step will issue interrupt then enable next step. >>>>>>> Sr (start read) | D | D | D | P >>>>>>> Buffer mode: it means, the data can prepare into buffer register, >>>>>>> then Trigger transfer. So Sr D D D P, only have only 1 interrupt handling. >>>>>>> The DMA mode most like with buffer mode, The differ is data >>>>>>> prepare in DRAM, than trigger transfer. >>>>>>> >>>>>>> So, should I modify to >>>>>>> aspeed,byte: >>>>>>> type: boolean >>>>>>> description: Enable i2c controller transfer with byte mode >>>>>>> >>>>>>> aspeed,buff: >>>>>>> type: boolean >>>>>>> description: Enable i2c controller transfer with buff mode >>>>>> >>>>>> 1. No, these are not bools but enum in such case. >>>>> >>>>> Thanks, will modify following. >>>>> aspeed,xfer_mode: >>>>> enum: [0, 1, 2] >>>>> description: >>>>> 0: byte mode, 1: buff_mode, 2: dma_mode >>>> >>>> Just keep it text - byte, buffered, dma >>>> >>>>> >>>>>> 2. And why exactly this is board-specific? >>>>> >>>>> No, it not depends on board design. It is only for register control >>>>> for >>>> controller transfer behave. >>>>> The controller support 3 different trigger mode for transfer. >>>>> Assign bus#1 ~ 3 : dma tranfer and assign bus#4 ~ 6 : buffer mode >>>>> transfer, That can reduce the dram usage. >>>> >>>> Then anyway it does not look like property for Devicetree. DT >>>> describes hardware, not OS behavior. >>> >>> The same draw, in this case, i2c bus#1 that is multi-master transfer >> architecture. >>> Both will inactive with trunk data. That cane enable i2c#1 use DMA transfer >> to reduce CPU utilized. >>> Others (bus#2/3) can keep byte/buff mode. >> >> Isn't then current bus configuration for I2C#1 known to the driver? >> Jeremy asked few other questions around here... > > No, The driver don't know currently board configuration.
It knows whether it is working in multi-master/slave mode.
Best regards, Krzysztof
| |