Re: [PATCH v2 2/2] i2c: xiic: Support forcing single-master in DT

From: Wolfram Sang <wsa@the-dreams.de>
To: Laine Jaakko EXT <ext-jaakko.laine@vaisala.com>
Cc: "michal.simek@xilinx.com" <michal.simek@xilinx.com>,
	"shubhrajyoti.datta@gmail.com" <shubhrajyoti.datta@gmail.com>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"linux-i2c@vger.kernel.org" <linux-i2c@vger.kernel.org>
Subject: Re: [PATCH v2 2/2] i2c: xiic: Support forcing single-master in DT
Date: Wed, 26 Aug 2020 14:05:06 +0200	[thread overview]
Message-ID: <20200826120506.GD1081@ninjato> (raw)
In-Reply-To: <AM0PR06MB51855B0C24B5D1F938A9C18ED4540@AM0PR06MB5185.eurprd06.prod.outlook.com>

[-- Attachment #1.1: Type: text/plain, Size: 2848 bytes --]

Hi Jaako,

> > You could even initiate a recovery procedure if it is a device pulling
> > SDA low.
> 
> In the case we actually observed xiic got permanently stuck, when I2C bus
> was temporarily and indirectly affected by external voltage pulse. This can
> happen in single-master or multi-master bus. It does seem that no extra
> recovery behavior is needed in this case. If we are able to determine that bus
> is not actually busy, we can start using it. Indicating that bus is single-master
> was a handy way to determine that. This issue was bad for us because
> even if SDA was not grounded anymore, bus was still indicated to be busy by
> FPGA register and driver would not continue.

Okay, it was just a suggestion, definately not something I will require
to be added in this patch. Your reasoning makes sense to me.

> In multi-master case you would need some kind of timeout after which bus
> bus_is_busy is ignored and recovery attempted. This is ugly since it would be
> a non-standard behavior and intrusive to other masters on bus.

Yes, this is an issue. My best bet is to use the adapter->timeout value
for that because it is configurable and already used as kind-of I2C
timeout otherwise as well.

> In single-master case, if some slave device on bus would spontaneously
> pull SDA to ground when clock line is not pulsed, bus_is_busy could be triggered.
> In this case we could attempt some kind of recovery behavior. I guess this often
> means attempting to pulse the clock line to get the slave to release SDA.

Yes, as defined in the I2C specification even.

> In my knowledge pulsing the clock line can help if slave device on bus has missed
> some clock signal edges (or is answering with more bits than expected for some reason)
> and is holding SDA down in an attempt to communicate a data bit to i2c -master.
> Extra pulses in clock line can then allow the slave to finish transmission and stop
> pulling SDA low. I however doubt that this type of recovery would be likely to help
> if I2C -slave spontaneously pulls SDA low. This would however be a very badly
> misbehaving slave -device, so it´s hard to speculate what it will do and what specific
> recovery might help.

As the above standard says, you can try sending pulses but if that does
not help, then you should reset the device. In my experience, most I2C
devices are not wired to be reset externally, so we have the pulse
toggling mechanism in the I2C core to try as much as we can.

> There is too much speculation for me to attempt or test bus recovery in this case,
> so I would leave it out of this change. If somebody notices and is able to test  a case,
> where some specific extra recovery would be helpful, I would suggest to considered it
> later.

Totally fine with me.

Thanks,

   Wolfram

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel