Re: [PATCH 2/3] Documentation: i2c: describe the new slave mode

[Wolfram Sang <wsa@the-dreams.de>]

[-- Attachment #1: Type: text/plain, Size: 2924 bytes --]

On Fri, Mar 20, 2015 at 08:42:14AM +0100, Uwe Kleine-König wrote:
> Hello Wolfram,
> 
> On Fri, Mar 20, 2015 at 08:30:13AM +0100, Wolfram Sang wrote:
> > 
> > > > +Finally, Linux can also be an I2C slave in case I2C controllers have slave
> > > > +support. Besides this HW requirement, one also needs a software backend
> > > I wouldn't have written "Finally, ...". While it's great that we have
> > > slave support now, being enthusiastic here looks strange if someone
> > > reads it while slave support has become "normal".
> > 
> > OK.
> > 
> > > > +providing the actual functionality. An example for this is the slave-eeprom
> > > > +driver, which acts as a dual memory driver. While another I2C master on the bus
> > > > +can access it like a regular eeprom, the Linux I2C slave can access the content
> > > > +via sysfs and retrieve/provide information as needed. The software backend
> > > > +driver and the I2C bus driver communicate via events. Here is a small graph
> > > > +visualizing the data flow and the means by which data is transported. The
> > > > +dotted line marks only one example. The backend could also use e.g. a character
> > > > +device, or use in-kernel mechanisms only, or something completely different:
> > > Or something self contained, so the userspace part is actually optional
> > > (but probably present most of the time).
> > 
> > With "in-kernel mechanisms" I meant self-contained. Maybe "be in-kernel
> > only"?
> I'm sure "in-kernel mechanisms" wasn't in the mail I replied to. (Hmm,
> or I must have missed that while reading.)

:) Still, I like "be in-kernel only" better, so I'll rephrase.

> > > Does that mean that I have to pass a valid address, or can I use NULL,
> > > too?
> > 
> > Is NULL a valid pointer to val?
> NULL is a pointer and you didn't wrote about "valid" above. I just

But NULL is not a pointer to val.

> wondered if the necessity just comes from the fact that the function
> takes 3 parameters and so you have to give it 3 (this wouldn't needed to
> be pointed out IMHO) or if the value must be valid (then the wording
> isn't optimal).

I'll try to rephrase.

> Is there a technical reason to require val to be valid?

Better be safe than sorry in case for future needs of 'val' I can't see
now.

> > You need to assume that if the next byte is requested, the previous byte
> > made it to the bus. So, you should do pre-increment in
> > I2C_SLAVE_READ_PROCESSED, not post-increment. I didn't want to write
> This might be a correctness problem, right? If we cannot fix it (with
> the current slave abstraction) should this be pointed out somewhere; at
> least in the eeprom driver as this will probably be the reference for
> the next backend?

Adding some more info to the eeprom driver sounds good. Updating this
paragraph with some infos from this discussion, too.

Thanks,

   Wolfram


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]