rcar-i2c: always reads a byte

rcar-i2c: always reads a byte

[Ben Dooks]

I have noticed the following behaviour with the i2c-rcar driver
with the following single i2c_msg structure:

	msg[0].addr = 0x12;
	msg[0].flags = I2C_M_RD;
	msg[0].len = 0;
	msg[0].buf = data;

The system issues an address transaction followed by a single byte
read.

I have tried changing the code to not ack the read after the address
interrupt, however this causes the i2c block to hang forever.

i2c_recv_irq():

	if (priv->pos + 1 >= msg->len) {
		rcar_i2c_bus_phase(priv, RCAR_BUS_PHASE_STOP);
		if (msg->len != 0)
			rcar_i2c_status_bit_clear(priv, MAT | MDR);
		else
			rcar_i2c_status_bit_clear(priv, MAT);
	} else {
		rcar_i2c_bus_phase(priv, RCAR_BUS_PHASE_DATA);
		rcar_i2c_recv_restart(priv);
	}

-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

Re: rcar-i2c: always reads a byte

[Wolfram Sang]

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

On Tue, Mar 11, 2014 at 04:15:02PM +0000, Ben Dooks wrote:
> I have noticed the following behaviour with the i2c-rcar driver
> with the following single i2c_msg structure:
> 
> 	msg[0].addr = 0x12;
> 	msg[0].flags = I2C_M_RD;
> 	msg[0].len = 0;
> 	msg[0].buf = data;
> 
> The system issues an address transaction followed by a single byte
> read.

This fits to my observation that the rcar driver cannot do SMBUS_QUICK
(e.g. via i2cdetect). I assumed that handling a length of 0 is broken. I
haven't had a deeper look yet, so I can't say much. So, you either go
for it yourself, or you wait until I posted my IIC enablement series for
Lager. Then, you can switch to the IIC core with the sh_mobile driver.
That one does SMBUS_QUICK correctly.

Regards,

   Wolfram


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

Re: rcar-i2c: always reads a byte

[Ben Dooks]

On 11/03/14 16:40, Wolfram Sang wrote:
> On Tue, Mar 11, 2014 at 04:15:02PM +0000, Ben Dooks wrote:
>> I have noticed the following behaviour with the i2c-rcar driver
>> with the following single i2c_msg structure:
>>
>> 	msg[0].addr = 0x12;
>> 	msg[0].flags = I2C_M_RD;
>> 	msg[0].len = 0;
>> 	msg[0].buf = data;
>>
>> The system issues an address transaction followed by a single byte
>> read.
>
> This fits to my observation that the rcar driver cannot do SMBUS_QUICK
> (e.g. via i2cdetect). I assumed that handling a length of 0 is broken. I
> haven't had a deeper look yet, so I can't say much. So, you either go
> for it yourself, or you wait until I posted my IIC enablement series for
> Lager. Then, you can switch to the IIC core with the sh_mobile driver.
> That one does SMBUS_QUICK correctly.

I'll have a deeper look into this, I think there may be a solution.

-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

Re: rcar-i2c: always reads a byte

[Wolfram Sang]

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


> I'll have a deeper look into this, I think there may be a solution.

There surely is :)


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

Re: rcar-i2c: always reads a byte

[Ben Dooks]

On 11/03/14 16:41, Ben Dooks wrote:
> On 11/03/14 16:40, Wolfram Sang wrote:
>> On Tue, Mar 11, 2014 at 04:15:02PM +0000, Ben Dooks wrote:
>>> I have noticed the following behaviour with the i2c-rcar driver
>>> with the following single i2c_msg structure:
>>>
>>>     msg[0].addr = 0x12;
>>>     msg[0].flags = I2C_M_RD;
>>>     msg[0].len = 0;
>>>     msg[0].buf = data;
>>>
>>> The system issues an address transaction followed by a single byte
>>> read.
>>
>> This fits to my observation that the rcar driver cannot do SMBUS_QUICK
>> (e.g. via i2cdetect). I assumed that handling a length of 0 is broken. I
>> haven't had a deeper look yet, so I can't say much. So, you either go
>> for it yourself, or you wait until I posted my IIC enablement series for
>> Lager. Then, you can switch to the IIC core with the sh_mobile driver.
>> That one does SMBUS_QUICK correctly.
>
> I'll have a deeper look into this, I think there may be a solution.

I've tried issuing the ESG + FSB  bits as a command, however that
did not work.

-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

Re: rcar-i2c: always reads a byte

[Wolfram Sang]

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


> >>>The system issues an address transaction followed by a single byte
> >>>read.
> >>
> >>This fits to my observation that the rcar driver cannot do SMBUS_QUICK
> >>(e.g. via i2cdetect). I assumed that handling a length of 0 is broken. I
> >>haven't had a deeper look yet, so I can't say much. So, you either go
> >>for it yourself, or you wait until I posted my IIC enablement series for
> >>Lager. Then, you can switch to the IIC core with the sh_mobile driver.
> >>That one does SMBUS_QUICK correctly.
> >
> >I'll have a deeper look into this, I think there may be a solution.
> 
> I've tried issuing the ESG + FSB  bits as a command, however that
> did not work.

So, I experimented today as well. I don't think it is possible to have
zero-length support. I'll cook up a patch to disable them. Come to think
of it, it should be even for stable since I assume SMBUS_QUICK_WRITE
will also put a byte on the bus. Not good! And I2C_M_NOSTART is also out
of the question with this hardware.


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

Re: rcar-i2c: always reads a byte

[Wolfram Sang]

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

> or you wait until I posted my IIC enablement series for Lager.

I'll post it this evening, forgot to mention that.


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

Re: rcar-i2c: always reads a byte

[Ben Dooks]

On 11/03/14 17:24, Wolfram Sang wrote:
>> or you wait until I posted my IIC enablement series for Lager.
>
> I'll post it this evening, forgot to mention that.

erm, I already posted patches for i2c on lager.


-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

Re: rcar-i2c: always reads a byte

[Wolfram Sang]

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

On Tue, Mar 11, 2014 at 05:31:59PM +0000, Ben Dooks wrote:
> On 11/03/14 17:24, Wolfram Sang wrote:
> >>or you wait until I posted my IIC enablement series for Lager.
> >
> >I'll post it this evening, forgot to mention that.
> 
> erm, I already posted patches for i2c on lager.

Yes, for busses 0-3. I'll send patches for busses 4-7. And those two
cores share the same pinmuxing, so you can switch from the current i2c
(rcar) core n to the IIC (sh_mobile) core n+4 easily.


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

Re: rcar-i2c: always reads a byte

[Michael Lawnick]

Hi,

Ben Dooks schrieb:
> I have noticed the following behaviour with the i2c-rcar driver
> with the following single i2c_msg structure:
>
> 	msg[0].addr = 0x12;
> 	msg[0].flags = I2C_M_RD;
> 	msg[0].len = 0;
> 	msg[0].buf = data;

isn't this illegal?
This should lead to 50% chance of blocked bus: After sending read-bit 
the slave ACK's and puts first data bit onto SDL. If it is '0', master 
won't be able to signal stop nor arbitrate the bus for other transaction 
until slave runs into internal timeout (*if* supported).
Only 9 SCL toggles will be able to free bus again.

But maybe I'm missing something.

KR
Michael

Re: rcar-i2c: always reads a byte

[Paul Carpenter]

Michael Lawnick wrote:
 > Hi,
 >
 > Ben Dooks schrieb:
 >> I have noticed the following behaviour with the i2c-rcar driver
 >> with the following single i2c_msg structure:
 >>
 >>     msg[0].addr = 0x12;
 >>     msg[0].flags = I2C_M_RD;
 >>     msg[0].len = 0;
 >>     msg[0].buf = data;
 >
 > isn't this illegal?
 > This should lead to 50% chance of blocked bus: After sending read-bit
 > the slave ACK's and puts first data bit onto SDL. If it is '0',

NO device should change the data line during SCL rising or SCL HIGH
this would cause STOP/START confusion.

The data line should be driven at detection of SCL falling (or just
after normally.

 > master
 > won't be able to signal stop nor arbitrate the bus for other transaction
 > until slave runs into internal timeout (*if* supported).

Master can signal STOP during SCL HIGH as normal.

 > Only 9 SCL toggles will be able to free bus again.

If there is not a short or other fault on SCL being held low, drive as
much as you like wont make any difference.

 > But maybe I'm missing something.

See above

 > KR
 > Michael

-- 
Paul Carpenter          | paul@pcserviceselectronics.co.uk
<http://www.pcserviceselectronics.co.uk/>    PC Services
<http://www.pcserviceselectronics.co.uk/pi/>  Raspberry Pi Add-ons
<http://www.pcserviceselectronics.co.uk/fonts/> Timing Diagram Font

Re: rcar-i2c: always reads a byte

[Michael Lawnick]

Paul Carpenter schrieb:
> Michael Lawnick wrote:
>   > Hi,
>   >
>   > Ben Dooks schrieb:
>   >> I have noticed the following behaviour with the i2c-rcar driver
>   >> with the following single i2c_msg structure:
>   >>
>   >>     msg[0].addr = 0x12;
>   >>     msg[0].flags = I2C_M_RD;
>   >>     msg[0].len = 0;
>   >>     msg[0].buf = data;
>   >
>   > isn't this illegal?
>   > This should lead to 50% chance of blocked bus: After sending read-bit
>   > the slave ACK's and puts first data bit onto SDL. If it is '0',
>
> NO device should change the data line during SCL rising or SCL HIGH
> this would cause STOP/START confusion.

Nobody talked about change while SCL high. SCL is high and SDA low for 
ACK. From slave's view it just keeps SDA low for first bit transmission 
while SCL goes low and is expected to go high again.
I'm sure to have seen this multiple times.

You may just test on ordinary devices like lm75.

KR
Michael

Re: rcar-i2c: always reads a byte

[Wolfram Sang]

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


> >I have noticed the following behaviour with the i2c-rcar driver
> >with the following single i2c_msg structure:
> >
> >	msg[0].addr = 0x12;
> >	msg[0].flags = I2C_M_RD;
> >	msg[0].len = 0;
> >	msg[0].buf = data;
> 
> isn't this illegal?
> This should lead to 50% chance of blocked bus: After sending
> read-bit the slave ACK's and puts first data bit onto SDL.

It is not illegal, SMBUS_QUICK is described in the SMBus specs and can
bee seen as a kind of 1-bit transfer. Yet, you should only issue it for
devices which require exactly this kind of transfer. Which is true for
most I2C transaction types.


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