From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S943994AbcJaSVZ (ORCPT ); Mon, 31 Oct 2016 14:21:25 -0400 Received: from mail.dev.rtsoft.ru ([213.79.90.226]:38332 "EHLO mail.dev.rtsoft.ru" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S942679AbcJaSVX (ORCPT ); Mon, 31 Oct 2016 14:21:23 -0400 DKIM-Filter: OpenDKIM Filter v2.10.3 mail.dev.rtsoft.ru 4157C42262 Subject: Re: [PATCH] i2c: imx: add slave support. v2 status To: "Frkuska, Joshua" , linux-i2c@vger.kernel.org References: <1e9f10a4-6ea2-3dd9-159e-fd70f7c40224@mentor.com> <2404b260-4bd6-8ff6-3898-c9c63aaf6855@dev.rtsoft.ru> <0f7d5f1c-5889-27f3-f4e0-fc79960b8bf2@mentor.com> Cc: wsa@the-dreams.de, peda@axentia.se, Jiada_Wang@mentor.com, linux-kernel@vger.kernel.org, "Zapolskiy, Vladimir" , "Baxter, Jim" From: Maxim Syrchin Message-ID: <4553f82f-d5aa-8c31-5a6a-108582810b14@dev.rtsoft.ru> Date: Mon, 31 Oct 2016 21:21:21 +0300 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <0f7d5f1c-5889-27f3-f4e0-fc79960b8bf2@mentor.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hello, Please find some comments below. 31.10.2016 5:14, Frkuska, Joshua пишет: > Hello Maxim, > > Thank you very much for the intermediate patch. I am in the process of > reviewing it. Please let me clarify a few questions I have. > > 1. What alternative to "bus busy/bus free/IBB" polling do you have in > mind? This seems like a reasonable approach to me. We didn't find any suitable alternatives. The only one we're considered was using timeout on receive (which is kind of polling of course) > 2. What are the major points you consider in need of refactoring? As you can see we have implemented FSM in slave thread. - Due to lack of time all master functionality had not been included in State Machine. - wait_event_timeout() calls are used in every event handler (obviosly it is better to have only one wait function) - Need to review state switching code > 3. You mention race conditions being fixed in this version relating to > bus-locking by the slave and breaking slave transactions by the > master. Does this mean mixed slave/master mode works to your > satisfaction? If not, what work still needs to be done here? Yes mixed slave/master mode works ok. It had passed long-lasting stress tests (async message exchange of two imx6 boards connected together by i2c bus ) > 4. You mention the need for a slave locking test and a work-around > (checking IMX_I2C_I2DR and IBB) being in-place. Why is this > work-around not sufficient? By the time we discovered I2DR workaround we went far from version 2 of driver and it wasn't tested. I'm sure that I2DR workaround will improve stability, but I do not know if it will fix all issues (i.e. passing of stress tests ) Best Regards, Maxim Syrchin > Thanks again, > > Joshua > > > On 10/28/2016 04:38 AM, Maxim Syrchin wrote: >> Hi, >> Sorry for huge delay in answering. Unfortunately we don't have enough >> resources now to prepare clean enough patch to be accepted by community. >> Please find the latest version attached. Driver has passed stress >> tests, but looks like it need seriuos refactoring (it is >> unnecessarily complicated). >> We still have polling in slave code. Since imx doesn't generate >> interrupt on "bus busy"/"bus free" events we have to test IBB bit in >> polling loop. >> Comparing to v2 version several race conditions were fixed (bus >> locking by slave, breaking slave transaction by starting master >> xfer). v2 is working pretty good in slave-only or master-only mode. >> It is reasonable to add slave locking test - sometimes imx6 hw >> doesn't release data line. As workaround we use dummy reading of >> IMX_I2C_I2DR if driver is in slave mode and IBB bit is set for a >> long time. >> >> Thanks, >> Maxim Syrchin >> >> >> 27.10.2016 10:31, Frkuska, Joshua пишет: >>> Hi Maxim, Dmitriy, Wolfram, >>> >>> If there is no immediate plan for a third release of the below patch >>> set, would it be possible to continue with merging v2 after >>> addressing the remaining concerns? >>> >>> >>> Thank you and regards, >>> >>> Joshua >>>> Hi Maxim, >>>> >>>> On 2016-03-04 11:06:10 in the thread "Re: [PATCH] i2c: imx: add >>>> slave support. v2" >>>> referenced here: https://patchwork.ozlabs.org/patch/573353/ you said: >>>>> Hi Wolfram, >>>>> I'm now working on creating new driver version. I think I'll be >>>>> able to >>>>> sent it soon. >>>> Do you still plan to release a driver update for an i2c imx driver >>>> slave support? >>>> >>>> Best regards, >>>> Jim Baxter >>>> >> >