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

[Jeremy Kerr <jk@codeconstruct.com.au>]

Hi Ryan & Rob,

> > > +  aspeed,transfer-mode:
> > > +    description: |
> > > +      ASPEED ast2600 platform equipped with 16 I2C controllers each i2c controller
> > > +      have 1 byte transfer buffer(byte mode), 32 bytes buffer(buffer mode), and
> > > +      share a DMA engine.
> > > +      Select I2C transfer mode for this controller. Supported values are:
> > > +        - "byte": Use 1 byte for i2c transmit (1-byte buffer).
> > > +        - "buffer": Use buffer (32-byte buffer) for i2c transmit. (default)
> > > +                    Better performance then byte mode.
> > 
> > Good, I like worse performance so I can use byte mode.
> Thanks your review.
> Will remove performance statement.

I don't think that really addresses Rob's point there.

The selection of mode is somewhat a driver implementation decision (and
so would not belong in a DT binding) - *except* that there are
considerations around the use of hardware DMA channels, as covered in
earlier review.

[My understanding is that the mode needs to be defined here to select
which i2c devices have a DMA channel allocated to them. I also
think that byte mode may be useful in some scenarios, but that
consideration certainly does not belong in the DT binding spec]

So, how about we refine this to *just* the hardware-specific component:
whether a DMA channel is allocated. A driver implementation can then
select the appropriate mode (dma, byte or buffer), depending on
implementation-specific details.

In that case, we would just have a boolean property, like:

    aspeed,i2c-dma-enabled;

- to signify that this controller may use a DMA channel. The choice of
actual mode is left up to the driver implementation.

Rob, would that suit better?

This way, we don't have ambiguity on "buffer" default vs. absent
property, and we're no longer specifying actual driver behaviour in the
DT.

Cheers,


Jeremy