linux-kernel - Re: [PATCH] i2c/busses: (mpc) Add support for SMBUS_READ_BLOCK

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <1321466950.2309.390.camel@groeck-laptop>
Date:	Wed, 16 Nov 2011 10:09:10 -0800
From:	Guenter Roeck <guenter.roeck@...csson.com>
To:	York Sun <yorksun@...escale.com>
CC:	Tabi Timur-B04825 <B04825@...escale.com>,
	"linux-i2c@...r.kernel.org" <linux-i2c@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"B29983@...escale.com" <B29983@...escale.com>
Subject: Re: [PATCH] i2c/busses: (mpc) Add support for SMBUS_READ_BLOCK_DATA

On Wed, 2011-11-16 at 12:55 -0500, York Sun wrote:
> On Wed, 2011-11-16 at 09:36 -0800, Guenter Roeck wrote:
> > On Wed, 2011-11-16 at 12:28 -0500, York Sun wrote:
> > > I am not sure if this mail will reach the list as I am not subscribed.
> > > 
> > > In the v2 patch (actually both version), you wrote
> > > 
> > > > +		byte = readb(i2c->base + MPC_I2C_DR);
> > > > +
> > > > +		/* Adjust length if first received byte is length */
> > > > +		if (i == 0 && recv_len) {
> > > > +			if (byte == 0 || byte > I2C_SMBUS_BLOCK_MAX)
> > > > +				return -EPROTO;
> > > > +			length += byte;
> > > > +			/*
> > > > +			 * For block reads, generate txack here if data length
> > > > +			 * is 1 byte (total length is 2 bytes).
> > > > +			 */
> > > > +			if (length == 2)
> > > > +				writeccr(i2c, CCR_MIEN | CCR_MEN | CCR_MSTA
> > > > +					 | CCR_TXAK);
> > > > +		}
> > > > +		data[i] = byte;
> > > 
> > > I am wondering if the first byte should be left out for data[] if the
> > > recv_len is non-zero. If I understand correctly, this patch is to
> > > support of SMBus with multiple byte read. The SMBus is different from
> > > I2C bus on multiple byte. The first data is byte count vs slave data for
> > > I2C. So you will receive all data preceded by the byte count, which is
> > > one more than your loop.
> > > 
> > York,
> > 
> > The calling code expects the data length in data[0], and the actual data
> > in data[1] .. data[<byte_count>]. The initial value for length is 1; the
> > byte count is added to it, so <byte count + 1> bytes are placed into the
> > buffer.
> > 
> > Thanks,
> > Guenter
> 
> Thanks for explanation. I am more confused by the length += byte now.
> For I2C bus, if you need length of byte, just keep reading until you get
> all of them. Of course you need to deal with the ACK. For SMBus, it is
> similar but you shouldn't read more after the byte count which is in the
> first data. If you want to read length of data but the block size is
> bigger than length, you should call block read at first place. If the
> block size is smaller than length, why increase the length? Does your
> SMBus controller only support fixed block size and not support single
> byte read? If it does, I would do
> 
> Block, Block, Block, byte, byte... until length of data
> 
York,

you lost me there. The added code only applies to SMBus block reads
(recv_len is true), for which the length of the to-be-received block is
in the first data byte, and must be added to the total number of bytes
to receive. The implementation is exactly the same in all other drivers
supporting the RECV_LEN flag.

Guenter


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/