Re: i2c_burst_write API


Erwan Gouriou
 



On 3 April 2017 at 18:19, Jon Medhurst (Tixy) <tixy@...> wrote:
On Mon, 2017-04-03 at 18:10 +0200, Erwan Gouriou wrote:
> On 3 April 2017 at 17:53, Jon Medhurst (Tixy) <tixy@...> wrote:
>
> > On Mon, 2017-04-03 at 13:39 +0200, Erwan Gouriou wrote:
> > > On 3 April 2017 at 12:35, Jon Medhurst (Tixy) <tixy@...> wrote:
> > >
> > > > On Mon, 2017-04-03 at 11:55 +0200, Erwan Gouriou wrote:
> > > > [...]
> > > > > As you described, for burst write operation, only one message has to
> >
> > be
> > > > > S Addr Wr [A] subAddr [A] Data [A] Data [A] ... [A] Data [A] P
> > > > > (unlike for burst read where 2 messages are expected).
> > > > >
> > > > > If we keep the burst write API as is, we define an incorrect
> >
> > sequence of
> > > >
> > > > 2
> > > > > messages:
> > > > >     msg[0].buf = &start_addr;
> > > > >     msg[0].len = 1;
> > > > >     msg[0].flags = I2C_MSG_WRITE;
> > > > >
> > > > >     msg[1].buf = buf;
> > > > >     msg[1].len = num_bytes;
> > > > >     msg[1].flags = I2C_MSG_WRITE | I2C_MSG_STOP;
> > > >
> > > > In what way is that incorrect? msg[0] doesn't have I2C_MSG_STOP and
> > > > msg[1] doesn't have I2C_MSG_START or I2C_MSG_RESTART, so that is a
> > > > correct way of specifying the single write message you want to appear
> >
> > on
> > > > the i2c bus.
> > > >
> > > > My view (but this could be an error from my part), is that defining a
> > >
> > > message
> > > involves having a header byte (Slave Address | Write) being issued before
> > > payload.
> > > Payload being:
> > > *subaddress (or start address) in the first msg[0]
> > > *data in msg[1]
> > >
> > > Hence, when we define 2 messages, we get, if we set RESTART correctly
> > > msg[0]: S Addr Wr [A] subAddr [A] RS
> > > msg[1]: Addr Wr [A] Data [A] Data [A] ... [A] Data [A] P
> >
> > Why do you want to send a RESTART? In the doc you link to, in
> > Table 13 "Transfer when master is writing multiple bytes to slave",
> > it doesn't specify a restart. It lists the same sequence as Zephyr's
> > i2c_burst_write() function implements.
> >
> > A restart is needed for reads (Table 15) because you need a way of
> > changing from write mode (sending address) to read mode (reading bytes).
> > And that's what i2c_burst_read() does.
>
>
> > > Serialized, this gets:
> > > S Addr Wr [A] subAddr [A] RS *Addr Wr [A]* Data [A] Data [A] ... [A] Data
> > > [A] P
> >
> > And in message after your restart, the i2c device will possibly use the
> > first Data byte as the 'subAddr' to use, and then write your second Data
> > byte to that address, which is probably not what you want.
>
>
> > > While, as explained by Piotr, expected single message (for a burst write)
> > > is:
> > > S Addr Wr [A] subAddr [A] Data [A] Data [A] ... [A] Data [A] P
> >
> > Which is the same as the document for your device specifies (Table 13
> > again).
>
>
> Exact, I just don't see why we specify 2 messages in API (msg[0] and
> msg[1]),
> if we want driver to send one single message.
> There should be a reason, but I don't get it, and this was basically my
> initial
> question (sorry if this was not clear enough).

I expect the message is specified in two parts so we don't have to
create a buffer of arbitrary length and copy the data to it just so we
can prepend the address byte. (The SPI API has that flaw, and several
months ago there was an RFC to fix that, but I've not seen anything
since.)

The i2c interface at least has total flexibility on how you construct
messages.

Hi Jon
 
Ok, This is indeed a valid reason, even if a bit confusing.
What about making this explicit in the API?

Thanks anyway for the input.

Join devel@lists.zephyrproject.org to automatically receive all group messages.