RE: [PATCH v5 1/2] dt-bindings: i2c: Add support for ASPEED i2Cv2

[Ryan Chen]

Hello Krzysztof,

> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>
> Sent: Thursday, February 23, 2023 5:29 PM
> To: Ryan Chen <ryan_chen@xxxxxxxxxxxxxx>; Rob Herring
> <robh+dt@xxxxxxxxxx>; Krzysztof Kozlowski
> <krzysztof.kozlowski+dt@xxxxxxxxxx>; Joel Stanley <joel@xxxxxxxxx>; Andrew
> Jeffery <andrew@xxxxxxxx>; Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>;
> openbmc@xxxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx;
> linux-aspeed@xxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v5 1/2] dt-bindings: i2c: Add support for ASPEED i2Cv2
> 
> On 22/02/2023 11:47, Ryan Chen wrote:
> >>>> 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.
> >
> > But timeout feature is also apply in master. It for avoid suddenly
> > slave miss(un-plug) Master can timeout and release the SDA/SCL, return.
> 
> OK, yet the property should describe the hardware, not the register feature you
> want to program. You need to properly model it in DT binding to represent
> hardware setup, not your desired Linux driver 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.
> >
> > But in DT can decide which i2c bus number can use dma or buffer mode
> transfer.
> > If in another i2c bus support master only, also can use dma to transfer trunk
> data to another slave.
> 
> and none of these were explained in commit msg or device description.
> 
Thanks your guidance. I will add all those discussion in next patches cover-letter.
Best regards,
Ryan Chen.