Re: [PATCH v2 00/10] i2c: xiic: Add features, bug fixes.

From: Marek Vasut <marex@denx.de>
To: Raviteja Narayanam <rna@xilinx.com>,
	Michal Simek <michals@xilinx.com>,
	"linux-i2c@vger.kernel.org" <linux-i2c@vger.kernel.org>
Cc: "linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	git <git@xilinx.com>, "joe@perches.com" <joe@perches.com>
Subject: Re: [PATCH v2 00/10] i2c: xiic: Add features, bug fixes.
Date: Mon, 19 Jul 2021 20:00:19 +0200	[thread overview]
Message-ID: <45aa8d2b-a077-32a2-0608-8f20a5b807a8@denx.de> (raw)
In-Reply-To: <SN6PR02MB40933E99A241952502B69F41CAE19@SN6PR02MB4093.namprd02.prod.outlook.com>

On 7/19/21 12:09 PM, Raviteja Narayanam wrote:

Hi,

[...]

>>>> -Add 'standard mode' feature for reads > 255 bytes.
>>>> -Add 'smbus block read' functionality.
>>>> -Add 'xlnx,axi-iic-2.1' new IP version support.
>>>> -Switch to 'AXI I2C standard mode' for i2c reads in affected IP versions.
>>>> -Remove 'local_irq_save/restore' calls as discussed here:
>> https://www.spinics.net/lists/linux-i2c/msg46483.html.
>>>> -Some trivial fixes.
>>>>
>>>> Changes in v2:
>>>> -Grouped the commits as fixes first and then features.
>>>> -The first 4 commits fix the dynamic mode broken feature.
>>>> -Corrected the indentation in coding style issues.
>>>>
>>>> Michal Simek (1):
>>>>     i2c: xiic: Fix coding style issues
>>>>
>>>> Raviteja Narayanam (7):
>>>>     i2c: xiic: Fix Tx Interrupt path for grouped messages
>>>>     i2c: xiic: Add standard mode support for > 255 byte read transfers
>>>>     i2c: xiic: Switch to Xiic standard mode for i2c-read
>>>>     i2c: xiic: Remove interrupt enable/disable in Rx path
>>>>     dt-bindings: i2c: xiic: Add 'xlnx,axi-iic-2.1' to compatible
>>>>     i2c: xiic: Update compatible with new IP version
>>>>     i2c: xiic: Add smbus_block_read functionality
>>>>
>>>> Shubhrajyoti Datta (2):
>>>>     i2c: xiic: Return value of xiic_reinit
>>>>     i2c: xiic: Fix the type check for xiic_wakeup
>>>>
>>>>    .../bindings/i2c/xlnx,xps-iic-2.00.a.yaml     |   4 +-
>>>>    drivers/i2c/busses/i2c-xiic.c                 | 593 ++++++++++++++----
>>>>    2 files changed, 487 insertions(+), 110 deletions(-)
>>>>
>>>
>>> Acked-by: Michal Simek <michal.simek@xilinx.com>
>>
>> I just tested this patchset on next-20210716 and the XIIC failures are still
>> present, see:
> 
> The probe of ' atmel_mxt_ts' failed as per the error. May I know the details of
> your test case if you tweaked any i2ctransfers/added delays.

It is still the same test case from a year ago -- Atmel MXT touchscreen 
controller connected to XIIC I2C IP in ZynqMP FPGA, both drivers are 
compiled into the kernel. Also, it is not the "new" XIIC IP revision, 
but older one from Vivado 2019 or so.

> If it failed without adding anything, then please check whether the vivado design constraints
> are correctly applied or not.

They are, we already checked multiple times and the FPGA part is OK.

> Also check if the other devices on the bus are detected and i2ctransfer command is successful on them.

Note that this problem is very likely a race condition in the XIIC 
driver, so a trivial test like i2ctransfer on idle system from userspace 
is unlikely to trigger it. When the system is under heavy load e.g. 
during the kernel boot, that is when these corner cases start showing up.

> It would be helpful to know if the device ' atmel_mxt_ts' is successfully probed with next-20210716
> without applying this patchset.

Sometimes, the XIIC driver in current mainline Linux suffers from race 
conditions on SMP, so it depends.

The MXT driver also has to be patched to avoid longer than 255 byte 
transfers, because that is currently broken with XIIC.

> I have tested this again on our boards with eeprom and other sensors, this is working fine for us.

Can you share details of how those tests were performed ?