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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20150320082238.GB2071@katana>
Date:	Fri, 20 Mar 2015 09:22:39 +0100
From:	Wolfram Sang <wsa@...-dreams.de>
To:	Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
Cc:	linux-i2c@...r.kernel.org, linux-sh@...r.kernel.org,
	Magnus Damm <magnus.damm@...il.com>,
	Simon Horman <horms@...ge.net.au>,
	Laurent Pinchart <laurent.pinchart@...asonboard.com>,
	Geert Uytterhoeven <geert@...ux-m68k.org>,
	Andrey Danin <danindrey@...l.ru>,
	Marc Dietrich <marvin24@....de>,
	Debora Grosse <debora@....com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/3] Documentation: i2c: describe the new slave mode

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


Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)