RE: [PATCH v27 2/4] dt-bindings: i2c: ast2600-i2c.yaml: Add global-regs and transfer-mode properties

Hello Jeremy,
	Thanks the review.

> Subject: Re: [PATCH v27 2/4] dt-bindings: i2c: ast2600-i2c.yaml: Add global-regs
> and transfer-mode properties
> 
> Hi Ryan,
> 
> > The AST2600 I2C controller supports three transfer modes (byte,
> > buffer, DMA). Add "aspeed,transfer-mode" so DT can select the
> > preferred transfer method per controller instance.
> 
> This patch does not add an aspeed,transfer-mode property.
Will update use aspeed,enable-dma
> 
> > Also add the "aspeed,global-regs"
> > phandle to reference the AST2600 global registers syscon/regmap used
> > by the controller.
> >
> > These properties apply only to the AST2600 binding and are not part of
> > the legacy binding, which uses a mixed controller/target register
> > layout and does not have the split register blocks or these new
> > configuration registers. Legacy DTs remain unchanged.
> >
> > Signed-off-by: Ryan Chen <[email protected]>
> > ---
> > Changes in v27:
> > - change aspeed,transfer-mode to aspeed,enable-dma.
> 
> What about all the previous changes?
Will update
Remove aspeed,transfer-mode instead aspeed,enable-dma
> 
> > ---
> >  .../devicetree/bindings/i2c/aspeed,ast2600-i2c.yaml     | 17
> > +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/i2c/aspeed,ast2600-i2c.yaml
> > b/Documentation/devicetree/bindings/i2c/aspeed,ast2600-i2c.yaml
> > index de2c359037da..38da6fc6424f 100644
> > --- a/Documentation/devicetree/bindings/i2c/aspeed,ast2600-i2c.yaml
> > +++ b/Documentation/devicetree/bindings/i2c/aspeed,ast2600-i2c.yaml
> > @@ -37,6 +37,21 @@ properties:
> >    resets:
> >      maxItems: 1
> >
> > +  aspeed,enable-dma:
> > +    type: boolean
> > +    description: |
> > +      I2C bus enable dma mode transfer.
> > +
> > +      ASPEED ast2600 platform equipped with 16 I2C controllers that
> > +share a
> > +      single DMA engine. DTS files can specify the data transfer mode
> > +to/from
> > +      the device, either DMA or programmed I/O.
> 
> As we had discussed: this does not define the transfer mode, only whether
> DMA is available to the peripheral.
> 
> Why mention the 16 i2c controllers here?
> 
> Please keep this description simple and relevant to the specific purpose of the
> property.

Will update with following.
description: Enable DMA for transfers on this I2C bus.
> 
> > +
> > +  aspeed,global-regs:
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> > +    description:
> > +      Phandle reference to the i2c global syscon node, containing the
> > +      SoC-common i2c register set.
> > +
> >  required:
> >    - reg
> >    - compatible
> > @@ -59,4 +74,6 @@ examples:
> >          resets = <&syscon ASPEED_RESET_I2C>;
> >          clock-frequency = <100000>;
> >          interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>;
> > +        aspeed,global-regs = <&i2c_global>;
> > +        aspeed,transfer-mode = "buffer";
> 
> This example does not match the binding.
Will remove aspeed,transfer-mode = "buffer";
Add with aspeed,enable-dma

> 
> Cheers,
> 
> 
> Jeremy