[jira] [Commented] (MYNEWT-865) hal i2c lockup on nrf platform if timeout is too short
[ https://issues.apache.org/jira/browse/MYNEWT-865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16263029#comment-16263029 ] ASF subversion and git services commented on MYNEWT-865: Commit 67b40a7480b208508a59a723db1516ee94f452a7 in mynewt-core's branch refs/heads/master from [~marko] [ https://gitbox.apache.org/repos/asf?p=mynewt-core.git;h=67b40a7 ] Merge pull request #670 from mkiiskila/MYNEWT-865 MYNEWT-865; flush RXD if needed before starting read operation. > hal i2c lockup on nrf platform if timeout is too short > -- > > Key: MYNEWT-865 > URL: https://issues.apache.org/jira/browse/MYNEWT-865 > Project: Mynewt > Issue Type: Bug > Security Level: Public(Viewable by anyone) > Components: HAL >Affects Versions: v1_2_0_rel >Reporter: William San Filippo >Assignee: Marko Kiiskila > Fix For: v1_3_0_rel > > > The nordic TWI (i2c) interface locked up when a too short timeout was > applied. Not sure of all the details here but I believe another transaction > was started and that this transaction caused the TWI interface to become > unresponsive. > The basic issue here is that the timeout is in os ticks and it is possible, > given the current code implementation, that there is basically no timeout > applied as a timeout of 1 os tick will give you a timeout of 0 to 1 os tick > (in msecs). > Another issue is that the code does not attempt to calculate whether the > timeout is too short given the length of i2c data to be sent, the clock > frequency and the time per os tick. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (MYNEWT-865) hal i2c lockup on nrf platform if timeout is too short
[ https://issues.apache.org/jira/browse/MYNEWT-865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16263028#comment-16263028 ] ASF subversion and git services commented on MYNEWT-865: Commit 67b40a7480b208508a59a723db1516ee94f452a7 in mynewt-core's branch refs/heads/master from [~marko] [ https://gitbox.apache.org/repos/asf?p=mynewt-core.git;h=67b40a7 ] Merge pull request #670 from mkiiskila/MYNEWT-865 MYNEWT-865; flush RXD if needed before starting read operation. > hal i2c lockup on nrf platform if timeout is too short > -- > > Key: MYNEWT-865 > URL: https://issues.apache.org/jira/browse/MYNEWT-865 > Project: Mynewt > Issue Type: Bug > Security Level: Public(Viewable by anyone) > Components: HAL >Affects Versions: v1_2_0_rel >Reporter: William San Filippo >Assignee: Marko Kiiskila > Fix For: v1_3_0_rel > > > The nordic TWI (i2c) interface locked up when a too short timeout was > applied. Not sure of all the details here but I believe another transaction > was started and that this transaction caused the TWI interface to become > unresponsive. > The basic issue here is that the timeout is in os ticks and it is possible, > given the current code implementation, that there is basically no timeout > applied as a timeout of 1 os tick will give you a timeout of 0 to 1 os tick > (in msecs). > Another issue is that the code does not attempt to calculate whether the > timeout is too short given the length of i2c data to be sent, the clock > frequency and the time per os tick. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (MYNEWT-865) hal i2c lockup on nrf platform if timeout is too short
[ https://issues.apache.org/jira/browse/MYNEWT-865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16261007#comment-16261007 ] Marko Kiiskila commented on MYNEWT-865: --- That seems unlikely; as you can see from the tx side code, it does not leave the routine until either a) data was sent successfully, b) timeout and we have written to TASKS_STOP or c) ERROR register was set. Unlike with RX, there's no event marked to happen at byte boundary afterwards. On the other hand, I was not able to find from Nordic's documentation the fact that RXD had to be read to unstall the RX. This as opposed to just clearing the event register, which I thought would be sufficient. > hal i2c lockup on nrf platform if timeout is too short > -- > > Key: MYNEWT-865 > URL: https://issues.apache.org/jira/browse/MYNEWT-865 > Project: Mynewt > Issue Type: Bug > Security Level: Public(Viewable by anyone) > Components: HAL >Affects Versions: v1_2_0_rel >Reporter: William San Filippo >Assignee: Marko Kiiskila > Fix For: v1_3_0_rel > > > The nordic TWI (i2c) interface locked up when a too short timeout was > applied. Not sure of all the details here but I believe another transaction > was started and that this transaction caused the TWI interface to become > unresponsive. > The basic issue here is that the timeout is in os ticks and it is possible, > given the current code implementation, that there is basically no timeout > applied as a timeout of 1 os tick will give you a timeout of 0 to 1 os tick > (in msecs). > Another issue is that the code does not attempt to calculate whether the > timeout is too short given the length of i2c data to be sent, the clock > frequency and the time per os tick. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (MYNEWT-865) hal i2c lockup on nrf platform if timeout is too short
[ https://issues.apache.org/jira/browse/MYNEWT-865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16260144#comment-16260144 ] Simon Ratner commented on MYNEWT-865: - [~marko] Is a similar change needed for TXD on the write side? > hal i2c lockup on nrf platform if timeout is too short > -- > > Key: MYNEWT-865 > URL: https://issues.apache.org/jira/browse/MYNEWT-865 > Project: Mynewt > Issue Type: Bug > Security Level: Public(Viewable by anyone) > Components: HAL >Affects Versions: v1_2_0_rel >Reporter: William San Filippo >Assignee: Marko Kiiskila > Fix For: v1_3_0_rel > > > The nordic TWI (i2c) interface locked up when a too short timeout was > applied. Not sure of all the details here but I believe another transaction > was started and that this transaction caused the TWI interface to become > unresponsive. > The basic issue here is that the timeout is in os ticks and it is possible, > given the current code implementation, that there is basically no timeout > applied as a timeout of 1 os tick will give you a timeout of 0 to 1 os tick > (in msecs). > Another issue is that the code does not attempt to calculate whether the > timeout is too short given the length of i2c data to be sent, the clock > frequency and the time per os tick. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (MYNEWT-865) hal i2c lockup on nrf platform if timeout is too short
[
https://issues.apache.org/jira/browse/MYNEWT-865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16260125#comment-16260125
]
Simon Ratner commented on MYNEWT-865:
-
[~marko] Agree that fine grained timeout is probably not necessary, as long as
it is always non-zero. And i see now that the check is for
strictly-greater-than in:
{noformat}
if (os_time_get() - start > timo) { ... }
{noformat}
> hal i2c lockup on nrf platform if timeout is too short
> --
>
> Key: MYNEWT-865
> URL: https://issues.apache.org/jira/browse/MYNEWT-865
> Project: Mynewt
> Issue Type: Bug
> Security Level: Public(Viewable by anyone)
> Components: HAL
>Affects Versions: v1_2_0_rel
>Reporter: William San Filippo
>Assignee: Marko Kiiskila
> Fix For: v1_3_0_rel
>
>
> The nordic TWI (i2c) interface locked up when a too short timeout was
> applied. Not sure of all the details here but I believe another transaction
> was started and that this transaction caused the TWI interface to become
> unresponsive.
> The basic issue here is that the timeout is in os ticks and it is possible,
> given the current code implementation, that there is basically no timeout
> applied as a timeout of 1 os tick will give you a timeout of 0 to 1 os tick
> (in msecs).
> Another issue is that the code does not attempt to calculate whether the
> timeout is too short given the length of i2c data to be sent, the clock
> frequency and the time per os tick.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (MYNEWT-865) hal i2c lockup on nrf platform if timeout is too short
[ https://issues.apache.org/jira/browse/MYNEWT-865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16260057#comment-16260057 ] Marko Kiiskila commented on MYNEWT-865: --- Bit of a correction regarding timeout parameter: passing 1 means that we'll wait 1-2 ticks. Not 0-1 ticks. My opinion is that we don't need granular timeouts here. The timeout parameter is to catch cases where e.g. there is no device present with this address present, not so much to cancel requests to slow i2c devices. That being said, there was an issue when cancelling read requests when data was being RX'd. We told TWI to cancel the transfer on the next byte boundary, but did not drain the incoming data afterwards. I was able to tickle this case once I started stopping the core in debugger in the middle of a transaction. > hal i2c lockup on nrf platform if timeout is too short > -- > > Key: MYNEWT-865 > URL: https://issues.apache.org/jira/browse/MYNEWT-865 > Project: Mynewt > Issue Type: Bug > Security Level: Public(Viewable by anyone) > Components: HAL >Affects Versions: v1_2_0_rel >Reporter: William San Filippo >Assignee: Marko Kiiskila > Fix For: v1_3_0_rel > > > The nordic TWI (i2c) interface locked up when a too short timeout was > applied. Not sure of all the details here but I believe another transaction > was started and that this transaction caused the TWI interface to become > unresponsive. > The basic issue here is that the timeout is in os ticks and it is possible, > given the current code implementation, that there is basically no timeout > applied as a timeout of 1 os tick will give you a timeout of 0 to 1 os tick > (in msecs). > Another issue is that the code does not attempt to calculate whether the > timeout is too short given the length of i2c data to be sent, the clock > frequency and the time per os tick. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (MYNEWT-865) hal i2c lockup on nrf platform if timeout is too short
[ https://issues.apache.org/jira/browse/MYNEWT-865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16260058#comment-16260058 ] ASF GitHub Bot commented on MYNEWT-865: --- mkiiskila opened a new pull request #670: MYNEWT-865; flush RXD if needed before starting read operation. URL: https://github.com/apache/mynewt-core/pull/670 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > hal i2c lockup on nrf platform if timeout is too short > -- > > Key: MYNEWT-865 > URL: https://issues.apache.org/jira/browse/MYNEWT-865 > Project: Mynewt > Issue Type: Bug > Security Level: Public(Viewable by anyone) > Components: HAL >Affects Versions: v1_2_0_rel >Reporter: William San Filippo >Assignee: Marko Kiiskila > Fix For: v1_3_0_rel > > > The nordic TWI (i2c) interface locked up when a too short timeout was > applied. Not sure of all the details here but I believe another transaction > was started and that this transaction caused the TWI interface to become > unresponsive. > The basic issue here is that the timeout is in os ticks and it is possible, > given the current code implementation, that there is basically no timeout > applied as a timeout of 1 os tick will give you a timeout of 0 to 1 os tick > (in msecs). > Another issue is that the code does not attempt to calculate whether the > timeout is too short given the length of i2c data to be sent, the clock > frequency and the time per os tick. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (MYNEWT-865) hal i2c lockup on nrf platform if timeout is too short
[ https://issues.apache.org/jira/browse/MYNEWT-865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16249945#comment-16249945 ] Simon Ratner commented on MYNEWT-865: - Not sure if "lockup" is an accurate description, but every subsequent transaction fails. Also, the issue isn't just that the timeout calculation is inaccurate. Even if the timeout is valid and legitimate (the i2c peripheral didn't respond or responds late), need to make sure subsequent transactions don't continue failing, i.e. any unexpected state is flushed from the bus before the next transaction. > hal i2c lockup on nrf platform if timeout is too short > -- > > Key: MYNEWT-865 > URL: https://issues.apache.org/jira/browse/MYNEWT-865 > Project: Mynewt > Issue Type: Bug > Security Level: Public(Viewable by anyone) > Components: HAL >Affects Versions: v1_2_0_rel >Reporter: William San Filippo >Assignee: Marko Kiiskila > Fix For: v1_3_0_rel > > > The nordic TWI (i2c) interface locked up when a too short timeout was > applied. Not sure of all the details here but I believe another transaction > was started and that this transaction caused the TWI interface to become > unresponsive. > The basic issue here is that the timeout is in os ticks and it is possible, > given the current code implementation, that there is basically no timeout > applied as a timeout of 1 os tick will give you a timeout of 0 to 1 os tick > (in msecs). > Another issue is that the code does not attempt to calculate whether the > timeout is too short given the length of i2c data to be sent, the clock > frequency and the time per os tick. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
