[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-869) net/nimble/host unittest failures
[
https://issues.apache.org/jira/browse/MYNEWT-869?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16259894#comment-16259894
]
ASF GitHub Bot commented on MYNEWT-869:
---
ccollins476ad closed pull request #665: MYNEWT-869 net/nimble/host unittest
failures
URL: https://github.com/apache/mynewt-core/pull/665
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/apps/bletiny/src/main.c b/apps/bletiny/src/main.c
index 8445ff4ff..fe7a0be9c 100755
--- a/apps/bletiny/src/main.c
+++ b/apps/bletiny/src/main.c
@@ -1983,9 +1983,7 @@ bletiny_l2cap_send(uint16_t conn_handle, uint16_t idx,
uint16_t bytes)
rc = ble_l2cap_send(coc->chan, sdu_tx);
if (rc) {
console_printf("Could not send data rc=%d\n", rc);
-if (rc == BLE_HS_EBUSY) {
-os_mbuf_free_chain(sdu_tx);
-}
+os_mbuf_free_chain(sdu_tx);
}
return rc;
diff --git a/apps/btshell/src/main.c b/apps/btshell/src/main.c
index db3c1b306..d6a4df865 100755
--- a/apps/btshell/src/main.c
+++ b/apps/btshell/src/main.c
@@ -1985,9 +1985,7 @@ btshell_l2cap_send(uint16_t conn_handle, uint16_t idx,
uint16_t bytes)
rc = ble_l2cap_send(coc->chan, sdu_tx);
if (rc) {
console_printf("Could not send data rc=%d\n", rc);
-if (rc == BLE_HS_EBUSY) {
-os_mbuf_free_chain(sdu_tx);
-}
+os_mbuf_free_chain(sdu_tx);
}
return rc;
diff --git a/net/nimble/host/src/ble_l2cap.c b/net/nimble/host/src/ble_l2cap.c
index 04ee4cad5..1c7e27a2a 100644
--- a/net/nimble/host/src/ble_l2cap.c
+++ b/net/nimble/host/src/ble_l2cap.c
@@ -160,6 +160,10 @@ int ble_l2cap_disconnect(struct ble_l2cap_chan *chan)
return ble_l2cap_sig_disconnect(chan);
}
+/**
+ * Transmits a packet over an L2CAP channel. This function only consumes the
+ * supplied mbuf on success.
+ */
int
ble_l2cap_send(struct ble_l2cap_chan *chan, struct os_mbuf *sdu)
{
diff --git a/net/nimble/host/src/ble_l2cap_coc.c
b/net/nimble/host/src/ble_l2cap_coc.c
index e24359922..537e1e045 100644
--- a/net/nimble/host/src/ble_l2cap_coc.c
+++ b/net/nimble/host/src/ble_l2cap_coc.c
@@ -476,6 +476,10 @@ ble_l2cap_coc_recv_ready(struct ble_l2cap_chan *chan,
struct os_mbuf *sdu_rx)
ble_hs_unlock();
}
+/**
+ * Transmits a packet over a connection-oriented channel. This function only
+ * consumes the supplied mbuf on success.
+ */
int
ble_l2cap_coc_send(struct ble_l2cap_chan *chan, struct os_mbuf *sdu_tx)
{
@@ -487,12 +491,12 @@ ble_l2cap_coc_send(struct ble_l2cap_chan *chan, struct
os_mbuf *sdu_tx)
return BLE_HS_EBUSY;
}
-tx->sdu = sdu_tx;
-
if (OS_MBUF_PKTLEN(sdu_tx) > tx->mtu) {
return BLE_HS_EBADDATA;
}
+tx->sdu = sdu_tx;
+
return ble_l2cap_coc_continue_tx(chan);
}
diff --git a/net/nimble/host/test/src/ble_gatts_read_test.c
b/net/nimble/host/test/src/ble_gatts_read_test.c
index f0d0efa1d..92ef8b189 100644
--- a/net/nimble/host/test/src/ble_gatts_read_test.c
+++ b/net/nimble/host/test/src/ble_gatts_read_test.c
@@ -84,8 +84,6 @@ ble_gatts_read_test_misc_init(uint16_t *out_conn_handle)
TEST_ASSERT_FATAL(ble_gatts_read_test_chr_2_val_handle ==
ble_gatts_read_test_chr_2_def_handle + 1);
-ble_gatts_start();
-
ble_hs_test_util_create_conn(2, ble_gatts_read_test_peer_addr, NULL, NULL);
if (out_conn_handle != NULL) {
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
> net/nimble/host unittest failures
> -
>
> Key: MYNEWT-869
> URL: https://issues.apache.org/jira/browse/MYNEWT-869
> Project: Mynewt
> Issue Type: Bug
> Security Level: Public(Viewable by anyone)
>Reporter: Marko Kiiskila
>Assignee: Christopher Collins
>
> set the following syscfg variables:
> OS_MEMPOOL_CHECK:
> description: 'Whether to do stack sanity check of mempool operations'
> value: 1
> OS_MEMPOOL_POISON:
> description: 'Whether to do write known pattern to freed memory'
> value: 1
> (gdb) run
> Starting program:
> /Users/marko/src2/incubator-mynewt-blinky/repos/apache-mynewt-core/bin/targets/unittest/net_nimble_host_test/app/net/nimble/host/test/net_nimble_host_test.elf
>
> [New Thread 0x1303 of process 64395]
> warning: unhandled dyld version (15)
> [pass] ble_att_clt_suite/ble_att_clt_test_tx_find_info
> [pass]
