[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4553f82f-d5aa-8c31-5a6a-108582810b14@dev.rtsoft.ru>
Date: Mon, 31 Oct 2016 21:21:21 +0300
From: Maxim Syrchin <syrchin@....rtsoft.ru>
To: "Frkuska, Joshua" <joshua_frkuska@...tor.com>,
linux-i2c@...r.kernel.org
Cc: wsa@...-dreams.de, peda@...ntia.se, Jiada_Wang@...tor.com,
linux-kernel@...r.kernel.org,
"Zapolskiy, Vladimir" <vladimir_zapolskiy@...tor.com>,
"Baxter, Jim" <Jim_Baxter@...tor.com>
Subject: Re: [PATCH] i2c: imx: add slave support. v2 status
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
>>>>
>>
>
Powered by blists - more mailing lists