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
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
