You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/05/10 07:05:07 UTC
[GitHub] [incubator-nuttx] xiaoxiang781216 opened a new pull request, #6226: Replace nxsem_timedwait_uninterruptible with nxsem_tickwait_uninterruptible
xiaoxiang781216 opened a new pull request, #6226:
URL: https://github.com/apache/incubator-nuttx/pull/6226
## Summary
to simplify the timeout logic
## Impact
Code refactor
## Testing
Pass CI
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6226: Replace nxsem_timedwait_uninterruptible with nxsem_tickwait_uninterruptible
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6226:
URL: https://github.com/apache/incubator-nuttx/pull/6226#discussion_r869077568
##########
arch/arm/src/am335x/am335x_i2c.c:
##########
@@ -546,48 +545,25 @@ static inline int am335x_i2c_sem_waitdone(struct am335x_i2c_priv_s *priv)
/* Signal the interrupt handler that we are waiting. NOTE: Interrupts
* are currently disabled but will be temporarily re-enabled below when
- * nxsem_timedwait() sleeps.
+ * nxsem_tickwait() sleeps.
*/
priv->intstate = INTSTATE_WAITING;
do
{
- /* Get the current time */
-
- clock_gettime(CLOCK_REALTIME, &abstime);
-
- /* Calculate a time in the future */
-
-#if CONFIG_AM335X_I2CTIMEOSEC > 0
- abstime.tv_sec += CONFIG_AM335X_I2CTIMEOSEC;
-#endif
-
- /* Add a value proportional to the number of bytes in the transfer */
+ /* Wait until either the transfer is complete or the timeout expires */
#ifdef CONFIG_AM335X_I2C_DYNTIMEO
- abstime.tv_nsec += 1000 * am335x_i2c_tousecs(priv->msgc, priv->msgv);
- if (abstime.tv_nsec >= 1000 * 1000 * 1000)
- {
- abstime.tv_sec++;
- abstime.tv_nsec -= 1000 * 1000 * 1000;
- }
-
-#elif CONFIG_AM335X_I2CTIMEOMS > 0
- abstime.tv_nsec += CONFIG_AM335X_I2CTIMEOMS * 1000 * 1000;
- if (abstime.tv_nsec >= 1000 * 1000 * 1000)
- {
- abstime.tv_sec++;
- abstime.tv_nsec -= 1000 * 1000 * 1000;
- }
+ ret = nxsem_tickwait(&priv->sem_isr,
+ USEC2TICK(am335x_i2c_tousecs(priv->msgc, priv->msgv));
+#else
+ ret = nxsem_tickwait(&priv->sem_isr,
+ CONFIG_AM335X_I2CTIMEOTICKS);
Review Comment:
Done.
##########
arch/arm/src/efm32/efm32_i2c.c:
##########
@@ -528,48 +528,24 @@ static useconds_t efm32_i2c_tousecs(int msgc, struct i2c_msg_s *msgs)
#ifndef CONFIG_I2C_POLLED
static inline int efm32_i2c_sem_waitdone(struct efm32_i2c_priv_s *priv)
{
- struct timespec abstime;
int ret;
do
{
- /* Get the current time */
-
- clock_gettime(CLOCK_REALTIME, &abstime);
-
- /* Calculate a time in the future */
-
-#if CONFIG_EFM32_I2CTIMEOSEC > 0
- abstime.tv_sec += CONFIG_EFM32_I2CTIMEOSEC;
-#endif
-
- /* Add a value proportional to the number of bytes in the transfer */
-
-#ifdef CONFIG_EFM32_I2C_DYNTIMEO
- abstime.tv_nsec += 1000 * efm32_i2c_tousecs(priv->msgc, priv->msgv);
- if (abstime.tv_nsec >= 1000 * 1000 * 1000)
- {
- abstime.tv_sec++;
- abstime.tv_nsec -= 1000 * 1000 * 1000;
- }
-
-#elif CONFIG_EFM32_I2CTIMEOMS > 0
- abstime.tv_nsec += CONFIG_EFM32_I2CTIMEOMS * 1000 * 1000;
- if (abstime.tv_nsec >= 1000 * 1000 * 1000)
- {
- abstime.tv_sec++;
- abstime.tv_nsec -= 1000 * 1000 * 1000;
- }
-#endif
-
/* Enable I2C interrupts */
efm32_i2c_putreg(priv, EFM32_I2C_IEN_OFFSET, I2C_IF_NACK | I2C_IF_ACK |
I2C_IF_MSTOP | I2C_IF_RXDATAV | I2C_IF_ERRORS);
/* Wait until either the transfer is complete or the timeout expires */
- ret = nxsem_timedwait_uninterruptible(&priv->sem_isr, &abstime);
+#ifdef CONFIG_EFM32_I2C_DYNTIMEO
+ ret = nxsem_tickwait_uninterruptible(&priv->sem_isr,
+ USEC2TICK(efm32_i2c_tousecs(priv->msgc, priv->msgv)));
+#else
+ ret = nxsem_tickwait_uninterruptible(&priv->sem_isr,
+ CONFIG_EFM32_I2CTIMEOTICKS);
Review Comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6226: Replace nxsem_timedwait_uninterruptible with nxsem_tickwait_uninterruptible
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6226:
URL: https://github.com/apache/incubator-nuttx/pull/6226#discussion_r873195292
##########
arch/arm/src/efm32/efm32_i2c.c:
##########
@@ -528,48 +528,24 @@ static useconds_t efm32_i2c_tousecs(int msgc, struct i2c_msg_s *msgs)
#ifndef CONFIG_I2C_POLLED
static inline int efm32_i2c_sem_waitdone(struct efm32_i2c_priv_s *priv)
{
- struct timespec abstime;
int ret;
do
{
- /* Get the current time */
-
- clock_gettime(CLOCK_REALTIME, &abstime);
-
- /* Calculate a time in the future */
-
-#if CONFIG_EFM32_I2CTIMEOSEC > 0
- abstime.tv_sec += CONFIG_EFM32_I2CTIMEOSEC;
-#endif
-
- /* Add a value proportional to the number of bytes in the transfer */
-
-#ifdef CONFIG_EFM32_I2C_DYNTIMEO
- abstime.tv_nsec += 1000 * efm32_i2c_tousecs(priv->msgc, priv->msgv);
- if (abstime.tv_nsec >= 1000 * 1000 * 1000)
- {
- abstime.tv_sec++;
- abstime.tv_nsec -= 1000 * 1000 * 1000;
- }
-
-#elif CONFIG_EFM32_I2CTIMEOMS > 0
- abstime.tv_nsec += CONFIG_EFM32_I2CTIMEOMS * 1000 * 1000;
- if (abstime.tv_nsec >= 1000 * 1000 * 1000)
- {
- abstime.tv_sec++;
- abstime.tv_nsec -= 1000 * 1000 * 1000;
- }
-#endif
-
/* Enable I2C interrupts */
efm32_i2c_putreg(priv, EFM32_I2C_IEN_OFFSET, I2C_IF_NACK | I2C_IF_ACK |
I2C_IF_MSTOP | I2C_IF_RXDATAV | I2C_IF_ERRORS);
/* Wait until either the transfer is complete or the timeout expires */
- ret = nxsem_timedwait_uninterruptible(&priv->sem_isr, &abstime);
+#ifdef CONFIG_EFM32_I2C_DYNTIMEO
Review Comment:
Here: https://github.com/apache/incubator-nuttx/pull/6277
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6226: Replace nxsem_timedwait_uninterruptible with nxsem_tickwait_uninterruptible
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6226:
URL: https://github.com/apache/incubator-nuttx/pull/6226#discussion_r872989683
##########
sched/semaphore/sem_tickwait.c:
##########
@@ -165,19 +145,27 @@ int nxsem_tickwait(FAR sem_t *sem, clock_t start, uint32_t delay)
*
****************************************************************************/
-int nxsem_tickwait_uninterruptible(FAR sem_t *sem, clock_t start,
- uint32_t delay)
+int nxsem_tickwait_uninterruptible(FAR sem_t *sem, uint32_t delay)
{
+ clock_t end = clock_systime_ticks() + delay;
int ret;
- do
+ for (; ; )
{
/* Take the semaphore (perhaps waiting) */
- ret = nxsem_tickwait(sem, start, delay);
+ int ret = nxsem_tickwait(sem, delay);
Review Comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #6226: Replace nxsem_timedwait_uninterruptible with nxsem_tickwait_uninterruptible
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #6226:
URL: https://github.com/apache/incubator-nuttx/pull/6226#issuecomment-1126202635
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6226: Replace nxsem_timedwait_uninterruptible with nxsem_tickwait_uninterruptible
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6226:
URL: https://github.com/apache/incubator-nuttx/pull/6226#discussion_r869620719
##########
sched/semaphore/sem_tickwait.c:
##########
@@ -165,18 +145,22 @@ int nxsem_tickwait(FAR sem_t *sem, clock_t start, uint32_t delay)
*
****************************************************************************/
-int nxsem_tickwait_uninterruptible(FAR sem_t *sem, clock_t start,
- uint32_t delay)
+int nxsem_tickwait_uninterruptible(FAR sem_t *sem, uint32_t delay)
{
+ clock_t start = clock_systime_ticks();
int ret;
do
{
/* Take the semaphore (perhaps waiting) */
- ret = nxsem_tickwait(sem, start, delay);
+ ret = nxsem_tickwait(sem, delay);
Review Comment:
@pkarashchenko Done, please review again.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6226: Replace nxsem_timedwait_uninterruptible with nxsem_tickwait_uninterruptible
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6226:
URL: https://github.com/apache/incubator-nuttx/pull/6226#discussion_r872981369
##########
arch/xtensa/src/esp32/esp32_i2c.c:
##########
@@ -723,31 +723,10 @@ static void esp32_i2c_reset_fsmc(struct esp32_i2c_priv_s *priv)
#ifndef CONFIG_I2C_POLLED
static int esp32_i2c_sem_waitdone(struct esp32_i2c_priv_s *priv)
{
- int ret;
- struct timespec abstime;
-
- /* Get the current absolute time and adds a offset as timeout */
-
- clock_gettime(CLOCK_REALTIME, &abstime);
-
-#if CONFIG_ESP32_I2CTIMEOSEC > 0
- abstime.tv_sec += CONFIG_ESP32_I2CTIMEOSEC;
-#endif
-
-#if CONFIG_ESP32_I2CTIMEOMS > 0
- abstime.tv_nsec += CONFIG_ESP32_I2CTIMEOMS * NSEC_PER_MSEC;
- if (abstime.tv_nsec >= 1000 * NSEC_PER_MSEC)
- {
- abstime.tv_sec++;
- abstime.tv_nsec -= 1000 * NSEC_PER_MSEC;
- }
-#endif
-
/* Wait on ISR semaphore */
- ret = nxsem_timedwait_uninterruptible(&priv->sem_isr, &abstime);
-
- return ret;
+ return nxsem_tickwait_uninterruptible(&priv->sem_isr,
+ SEC2TICK(CONFIG_ESP32_I2CTIMEOSEC) + MSEC2TICK(CONFIG_ESP32_I2CTIMEOMS));
Review Comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6226: Replace nxsem_timedwait_uninterruptible with nxsem_tickwait_uninterruptible
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6226:
URL: https://github.com/apache/incubator-nuttx/pull/6226#discussion_r869599520
##########
sched/semaphore/sem_tickwait.c:
##########
@@ -165,18 +145,22 @@ int nxsem_tickwait(FAR sem_t *sem, clock_t start, uint32_t delay)
*
****************************************************************************/
-int nxsem_tickwait_uninterruptible(FAR sem_t *sem, clock_t start,
- uint32_t delay)
+int nxsem_tickwait_uninterruptible(FAR sem_t *sem, uint32_t delay)
{
+ clock_t start = clock_systime_ticks();
int ret;
do
{
/* Take the semaphore (perhaps waiting) */
- ret = nxsem_tickwait(sem, start, delay);
+ ret = nxsem_tickwait(sem, delay);
Review Comment:
this code is bad. `delay` is not adjusted in case if `nxsem_tickwait` was interrupted, so for example in case of `nxsem_tickwait_uninterruptible(sem, 10);` the `nxsem_tickwait` gets interrupted at 9th tick, the `clock_systime_ticks() - start` == `9` and `9 < 10` so `nxsem_tickwait(sem, 10);` will be called the second time and will lead to `19` ticks delay instead of `10`.
The existing code was exactly handing such cases correctly.
##########
arch/risc-v/src/bl602/bl602_os_hal.c:
##########
@@ -1481,12 +1481,7 @@ int32_t bl_os_sem_take(void *semphr, uint32_t ticks)
return false;
}
- if (ticks)
- {
- bl_os_update_time(&timeout, ticks);
Review Comment:
why this call is removed?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6226: Replace nxsem_timedwait_uninterruptible with nxsem_tickwait_uninterruptible
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6226:
URL: https://github.com/apache/incubator-nuttx/pull/6226#discussion_r871240017
##########
sched/semaphore/sem_tickwait.c:
##########
@@ -165,19 +145,24 @@ int nxsem_tickwait(FAR sem_t *sem, clock_t start, uint32_t delay)
*
****************************************************************************/
-int nxsem_tickwait_uninterruptible(FAR sem_t *sem, clock_t start,
- uint32_t delay)
+int nxsem_tickwait_uninterruptible(FAR sem_t *sem, uint32_t delay)
{
- int ret;
+ clock_t end = clock_systime_ticks() + delay;
- do
+ for (; ; )
{
/* Take the semaphore (perhaps waiting) */
- ret = nxsem_tickwait(sem, start, delay);
+ int ret = nxsem_tickwait(sem, delay);
+ if (ret != -EINTR)
+ {
+ return ret;
+ }
+
+ delay = end - clock_systime_ticks();
+ if ((int32_t)delay < 0)
+ {
+ delay = 0;
Review Comment:
I see. That is to give a try for `nxsem_trywait(sem);` inside `nxsem_tickwait`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6226: Replace nxsem_timedwait_uninterruptible with nxsem_tickwait_uninterruptible
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6226:
URL: https://github.com/apache/incubator-nuttx/pull/6226#discussion_r872982238
##########
net/route/ramroute.h:
##########
@@ -200,19 +200,19 @@ void net_freeroute_ipv6(FAR struct net_route_ipv6_s *route);
void ramroute_ipv4_addlast(FAR struct net_route_ipv4_entry_s *entry,
FAR struct net_route_ipv4_queue_s *list);
FAR struct net_route_ipv4_entry_s *
- ramroute_ipv4_remfirst(struct net_route_ipv4_queue_s *list);
+ramroute_ipv4_remfirst(struct net_route_ipv4_queue_s *list);
FAR struct net_route_ipv4_entry_s *
- ramroute_ipv4_remafter(FAR struct net_route_ipv4_entry_s *entry,
+ramroute_ipv4_remafter(FAR struct net_route_ipv4_entry_s *entry,
FAR struct net_route_ipv4_queue_s *list);
#endif
#ifdef CONFIG_ROUTE_IPv6_RAMROUTE
void ramroute_ipv6_addlast(FAR struct net_route_ipv6_entry_s *entry,
FAR struct net_route_ipv6_queue_s *list);
FAR struct net_route_ipv6_entry_s *
- ramroute_ipv6_remfirst(struct net_route_ipv6_queue_s *list);
+ramroute_ipv6_remfirst(struct net_route_ipv6_queue_s *list);
FAR struct net_route_ipv6_entry_s *
- ramroute_ipv6_remafter(FAR struct net_route_ipv6_entry_s *entry,
+ramroute_ipv6_remafter(FAR struct net_route_ipv6_entry_s *entry,
struct net_route_ipv6_queue_s *list);
Review Comment:
Done.
##########
net/route/ramroute.h:
##########
@@ -200,19 +200,19 @@ void net_freeroute_ipv6(FAR struct net_route_ipv6_s *route);
void ramroute_ipv4_addlast(FAR struct net_route_ipv4_entry_s *entry,
FAR struct net_route_ipv4_queue_s *list);
FAR struct net_route_ipv4_entry_s *
- ramroute_ipv4_remfirst(struct net_route_ipv4_queue_s *list);
+ramroute_ipv4_remfirst(struct net_route_ipv4_queue_s *list);
FAR struct net_route_ipv4_entry_s *
- ramroute_ipv4_remafter(FAR struct net_route_ipv4_entry_s *entry,
+ramroute_ipv4_remafter(FAR struct net_route_ipv4_entry_s *entry,
FAR struct net_route_ipv4_queue_s *list);
#endif
#ifdef CONFIG_ROUTE_IPv6_RAMROUTE
void ramroute_ipv6_addlast(FAR struct net_route_ipv6_entry_s *entry,
FAR struct net_route_ipv6_queue_s *list);
FAR struct net_route_ipv6_entry_s *
- ramroute_ipv6_remfirst(struct net_route_ipv6_queue_s *list);
+ramroute_ipv6_remfirst(struct net_route_ipv6_queue_s *list);
Review Comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6226: Replace nxsem_timedwait_uninterruptible with nxsem_tickwait_uninterruptible
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6226:
URL: https://github.com/apache/incubator-nuttx/pull/6226#discussion_r872986133
##########
sched/semaphore/sem_tickwait.c:
##########
@@ -165,19 +145,27 @@ int nxsem_tickwait(FAR sem_t *sem, clock_t start, uint32_t delay)
*
****************************************************************************/
-int nxsem_tickwait_uninterruptible(FAR sem_t *sem, clock_t start,
- uint32_t delay)
+int nxsem_tickwait_uninterruptible(FAR sem_t *sem, uint32_t delay)
{
+ clock_t end = clock_systime_ticks() + delay;
int ret;
- do
+ for (; ; )
{
/* Take the semaphore (perhaps waiting) */
- ret = nxsem_tickwait(sem, start, delay);
+ int ret = nxsem_tickwait(sem, delay);
Review Comment:
```suggestion
ret = nxsem_tickwait(sem, delay);
```
to fix
```
Error: semaphore/sem_tickwait.c:157:11: error: declaration of 'ret' shadows a previous local [-Werror=shadow]
int ret = nxsem_tickwait(sem, delay);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6226: Replace nxsem_timedwait_uninterruptible with nxsem_tickwait_uninterruptible
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6226:
URL: https://github.com/apache/incubator-nuttx/pull/6226#discussion_r873148938
##########
arch/arm/src/efm32/efm32_i2c.c:
##########
@@ -528,48 +528,24 @@ static useconds_t efm32_i2c_tousecs(int msgc, struct i2c_msg_s *msgs)
#ifndef CONFIG_I2C_POLLED
static inline int efm32_i2c_sem_waitdone(struct efm32_i2c_priv_s *priv)
{
- struct timespec abstime;
int ret;
do
{
- /* Get the current time */
-
- clock_gettime(CLOCK_REALTIME, &abstime);
-
- /* Calculate a time in the future */
-
-#if CONFIG_EFM32_I2CTIMEOSEC > 0
- abstime.tv_sec += CONFIG_EFM32_I2CTIMEOSEC;
-#endif
-
- /* Add a value proportional to the number of bytes in the transfer */
-
-#ifdef CONFIG_EFM32_I2C_DYNTIMEO
- abstime.tv_nsec += 1000 * efm32_i2c_tousecs(priv->msgc, priv->msgv);
- if (abstime.tv_nsec >= 1000 * 1000 * 1000)
- {
- abstime.tv_sec++;
- abstime.tv_nsec -= 1000 * 1000 * 1000;
- }
-
-#elif CONFIG_EFM32_I2CTIMEOMS > 0
- abstime.tv_nsec += CONFIG_EFM32_I2CTIMEOMS * 1000 * 1000;
- if (abstime.tv_nsec >= 1000 * 1000 * 1000)
- {
- abstime.tv_sec++;
- abstime.tv_nsec -= 1000 * 1000 * 1000;
- }
-#endif
-
/* Enable I2C interrupts */
efm32_i2c_putreg(priv, EFM32_I2C_IEN_OFFSET, I2C_IF_NACK | I2C_IF_ACK |
I2C_IF_MSTOP | I2C_IF_RXDATAV | I2C_IF_ERRORS);
/* Wait until either the transfer is complete or the timeout expires */
- ret = nxsem_timedwait_uninterruptible(&priv->sem_isr, &abstime);
+#ifdef CONFIG_EFM32_I2C_DYNTIMEO
Review Comment:
I will create a patch addressed all typo error in the original code base. Please merge this one.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6226: Replace nxsem_timedwait_uninterruptible with nxsem_tickwait_uninterruptible
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6226:
URL: https://github.com/apache/incubator-nuttx/pull/6226#discussion_r869613053
##########
sched/semaphore/sem_tickwait.c:
##########
@@ -165,18 +145,22 @@ int nxsem_tickwait(FAR sem_t *sem, clock_t start, uint32_t delay)
*
****************************************************************************/
-int nxsem_tickwait_uninterruptible(FAR sem_t *sem, clock_t start,
- uint32_t delay)
+int nxsem_tickwait_uninterruptible(FAR sem_t *sem, uint32_t delay)
{
+ clock_t start = clock_systime_ticks();
int ret;
do
{
/* Take the semaphore (perhaps waiting) */
- ret = nxsem_tickwait(sem, start, delay);
+ ret = nxsem_tickwait(sem, delay);
Review Comment:
Yes, the last wait is longer than expect, let me modify it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #6226: Replace nxsem_timedwait_uninterruptible with nxsem_tickwait_uninterruptible
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #6226:
URL: https://github.com/apache/incubator-nuttx/pull/6226#issuecomment-1122719586
> Please ignore the follow warning:
>
> ```
> Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/drivers/wireless/nrf24l01.c:1889:11: error: Mixed case identifier found
> Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/drivers/wireless/nrf24l01.c:1892:11: error: Mixed case identifier found
> Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/drivers/wireless/nrf24l01.c:1896:11: error: Mixed case identifier found
> ```
>
> I will provide the separated patch fix it.
Fix here: https://github.com/apache/incubator-nuttx/pull/6232
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #6226: Replace nxsem_timedwait_uninterruptible with nxsem_tickwait_uninterruptible
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #6226:
URL: https://github.com/apache/incubator-nuttx/pull/6226#issuecomment-1124599496
@pkarashchenko could you review again?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6226: Replace nxsem_timedwait_uninterruptible with nxsem_tickwait_uninterruptible
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6226:
URL: https://github.com/apache/incubator-nuttx/pull/6226#discussion_r873148938
##########
arch/arm/src/efm32/efm32_i2c.c:
##########
@@ -528,48 +528,24 @@ static useconds_t efm32_i2c_tousecs(int msgc, struct i2c_msg_s *msgs)
#ifndef CONFIG_I2C_POLLED
static inline int efm32_i2c_sem_waitdone(struct efm32_i2c_priv_s *priv)
{
- struct timespec abstime;
int ret;
do
{
- /* Get the current time */
-
- clock_gettime(CLOCK_REALTIME, &abstime);
-
- /* Calculate a time in the future */
-
-#if CONFIG_EFM32_I2CTIMEOSEC > 0
- abstime.tv_sec += CONFIG_EFM32_I2CTIMEOSEC;
-#endif
-
- /* Add a value proportional to the number of bytes in the transfer */
-
-#ifdef CONFIG_EFM32_I2C_DYNTIMEO
- abstime.tv_nsec += 1000 * efm32_i2c_tousecs(priv->msgc, priv->msgv);
- if (abstime.tv_nsec >= 1000 * 1000 * 1000)
- {
- abstime.tv_sec++;
- abstime.tv_nsec -= 1000 * 1000 * 1000;
- }
-
-#elif CONFIG_EFM32_I2CTIMEOMS > 0
- abstime.tv_nsec += CONFIG_EFM32_I2CTIMEOMS * 1000 * 1000;
- if (abstime.tv_nsec >= 1000 * 1000 * 1000)
- {
- abstime.tv_sec++;
- abstime.tv_nsec -= 1000 * 1000 * 1000;
- }
-#endif
-
/* Enable I2C interrupts */
efm32_i2c_putreg(priv, EFM32_I2C_IEN_OFFSET, I2C_IF_NACK | I2C_IF_ACK |
I2C_IF_MSTOP | I2C_IF_RXDATAV | I2C_IF_ERRORS);
/* Wait until either the transfer is complete or the timeout expires */
- ret = nxsem_timedwait_uninterruptible(&priv->sem_isr, &abstime);
+#ifdef CONFIG_EFM32_I2C_DYNTIMEO
Review Comment:
I will create a patch addressed the typo error in the original code base. Please merge this one.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6226: Replace nxsem_timedwait_uninterruptible with nxsem_tickwait_uninterruptible
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6226:
URL: https://github.com/apache/incubator-nuttx/pull/6226#discussion_r872982264
##########
net/route/ramroute.h:
##########
@@ -200,19 +200,19 @@ void net_freeroute_ipv6(FAR struct net_route_ipv6_s *route);
void ramroute_ipv4_addlast(FAR struct net_route_ipv4_entry_s *entry,
FAR struct net_route_ipv4_queue_s *list);
FAR struct net_route_ipv4_entry_s *
- ramroute_ipv4_remfirst(struct net_route_ipv4_queue_s *list);
+ramroute_ipv4_remfirst(struct net_route_ipv4_queue_s *list);
Review Comment:
Done.
##########
net/route/ramroute.h:
##########
@@ -200,19 +200,19 @@ void net_freeroute_ipv6(FAR struct net_route_ipv6_s *route);
void ramroute_ipv4_addlast(FAR struct net_route_ipv4_entry_s *entry,
FAR struct net_route_ipv4_queue_s *list);
FAR struct net_route_ipv4_entry_s *
- ramroute_ipv4_remfirst(struct net_route_ipv4_queue_s *list);
+ramroute_ipv4_remfirst(struct net_route_ipv4_queue_s *list);
FAR struct net_route_ipv4_entry_s *
- ramroute_ipv4_remafter(FAR struct net_route_ipv4_entry_s *entry,
+ramroute_ipv4_remafter(FAR struct net_route_ipv4_entry_s *entry,
FAR struct net_route_ipv4_queue_s *list);
Review Comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6226: Replace nxsem_timedwait_uninterruptible with nxsem_tickwait_uninterruptible
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6226:
URL: https://github.com/apache/incubator-nuttx/pull/6226#discussion_r869077831
##########
arch/arm/src/am335x/am335x_i2c.c:
##########
@@ -546,48 +545,25 @@ static inline int am335x_i2c_sem_waitdone(struct am335x_i2c_priv_s *priv)
/* Signal the interrupt handler that we are waiting. NOTE: Interrupts
* are currently disabled but will be temporarily re-enabled below when
- * nxsem_timedwait() sleeps.
+ * nxsem_tickwait() sleeps.
*/
priv->intstate = INTSTATE_WAITING;
do
{
- /* Get the current time */
-
- clock_gettime(CLOCK_REALTIME, &abstime);
-
- /* Calculate a time in the future */
-
-#if CONFIG_AM335X_I2CTIMEOSEC > 0
- abstime.tv_sec += CONFIG_AM335X_I2CTIMEOSEC;
-#endif
-
- /* Add a value proportional to the number of bytes in the transfer */
+ /* Wait until either the transfer is complete or the timeout expires */
#ifdef CONFIG_AM335X_I2C_DYNTIMEO
- abstime.tv_nsec += 1000 * am335x_i2c_tousecs(priv->msgc, priv->msgv);
- if (abstime.tv_nsec >= 1000 * 1000 * 1000)
- {
- abstime.tv_sec++;
- abstime.tv_nsec -= 1000 * 1000 * 1000;
- }
-
-#elif CONFIG_AM335X_I2CTIMEOMS > 0
- abstime.tv_nsec += CONFIG_AM335X_I2CTIMEOMS * 1000 * 1000;
- if (abstime.tv_nsec >= 1000 * 1000 * 1000)
- {
- abstime.tv_sec++;
- abstime.tv_nsec -= 1000 * 1000 * 1000;
- }
+ ret = nxsem_tickwait(&priv->sem_isr,
+ USEC2TICK(am335x_i2c_tousecs(priv->msgc, priv->msgv));
Review Comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #6226: Replace nxsem_timedwait_uninterruptible with nxsem_tickwait_uninterruptible
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #6226:
URL: https://github.com/apache/incubator-nuttx/pull/6226#issuecomment-1122125054
Please ignore the follow warning:
```
Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/drivers/wireless/nrf24l01.c:1889:11: error: Mixed case identifier found
Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/drivers/wireless/nrf24l01.c:1892:11: error: Mixed case identifier found
Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/drivers/wireless/nrf24l01.c:1896:11: error: Mixed case identifier found
```
I will provide the separated patch fix it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6226: Replace nxsem_timedwait_uninterruptible with nxsem_tickwait_uninterruptible
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6226:
URL: https://github.com/apache/incubator-nuttx/pull/6226#discussion_r873151605
##########
arch/arm/src/efm32/efm32_i2c.c:
##########
@@ -528,48 +528,24 @@ static useconds_t efm32_i2c_tousecs(int msgc, struct i2c_msg_s *msgs)
#ifndef CONFIG_I2C_POLLED
static inline int efm32_i2c_sem_waitdone(struct efm32_i2c_priv_s *priv)
{
- struct timespec abstime;
int ret;
do
{
- /* Get the current time */
-
- clock_gettime(CLOCK_REALTIME, &abstime);
-
- /* Calculate a time in the future */
-
-#if CONFIG_EFM32_I2CTIMEOSEC > 0
- abstime.tv_sec += CONFIG_EFM32_I2CTIMEOSEC;
-#endif
-
- /* Add a value proportional to the number of bytes in the transfer */
-
-#ifdef CONFIG_EFM32_I2C_DYNTIMEO
- abstime.tv_nsec += 1000 * efm32_i2c_tousecs(priv->msgc, priv->msgv);
- if (abstime.tv_nsec >= 1000 * 1000 * 1000)
- {
- abstime.tv_sec++;
- abstime.tv_nsec -= 1000 * 1000 * 1000;
- }
-
-#elif CONFIG_EFM32_I2CTIMEOMS > 0
- abstime.tv_nsec += CONFIG_EFM32_I2CTIMEOMS * 1000 * 1000;
- if (abstime.tv_nsec >= 1000 * 1000 * 1000)
- {
- abstime.tv_sec++;
- abstime.tv_nsec -= 1000 * 1000 * 1000;
- }
-#endif
-
/* Enable I2C interrupts */
efm32_i2c_putreg(priv, EFM32_I2C_IEN_OFFSET, I2C_IF_NACK | I2C_IF_ACK |
I2C_IF_MSTOP | I2C_IF_RXDATAV | I2C_IF_ERRORS);
/* Wait until either the transfer is complete or the timeout expires */
- ret = nxsem_timedwait_uninterruptible(&priv->sem_isr, &abstime);
+#ifdef CONFIG_EFM32_I2C_DYNTIMEO
Review Comment:
Here is fix: https://github.com/apache/incubator-nuttx/pull/6274
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6226: Replace nxsem_timedwait_uninterruptible with nxsem_tickwait_uninterruptible
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6226:
URL: https://github.com/apache/incubator-nuttx/pull/6226#discussion_r873146066
##########
arch/arm/src/efm32/efm32_i2c.c:
##########
@@ -528,48 +528,24 @@ static useconds_t efm32_i2c_tousecs(int msgc, struct i2c_msg_s *msgs)
#ifndef CONFIG_I2C_POLLED
static inline int efm32_i2c_sem_waitdone(struct efm32_i2c_priv_s *priv)
{
- struct timespec abstime;
int ret;
do
{
- /* Get the current time */
-
- clock_gettime(CLOCK_REALTIME, &abstime);
-
- /* Calculate a time in the future */
-
-#if CONFIG_EFM32_I2CTIMEOSEC > 0
- abstime.tv_sec += CONFIG_EFM32_I2CTIMEOSEC;
-#endif
-
- /* Add a value proportional to the number of bytes in the transfer */
-
-#ifdef CONFIG_EFM32_I2C_DYNTIMEO
- abstime.tv_nsec += 1000 * efm32_i2c_tousecs(priv->msgc, priv->msgv);
- if (abstime.tv_nsec >= 1000 * 1000 * 1000)
- {
- abstime.tv_sec++;
- abstime.tv_nsec -= 1000 * 1000 * 1000;
- }
-
-#elif CONFIG_EFM32_I2CTIMEOMS > 0
- abstime.tv_nsec += CONFIG_EFM32_I2CTIMEOMS * 1000 * 1000;
- if (abstime.tv_nsec >= 1000 * 1000 * 1000)
- {
- abstime.tv_sec++;
- abstime.tv_nsec -= 1000 * 1000 * 1000;
- }
-#endif
-
/* Enable I2C interrupts */
efm32_i2c_putreg(priv, EFM32_I2C_IEN_OFFSET, I2C_IF_NACK | I2C_IF_ACK |
I2C_IF_MSTOP | I2C_IF_RXDATAV | I2C_IF_ERRORS);
/* Wait until either the transfer is complete or the timeout expires */
- ret = nxsem_timedwait_uninterruptible(&priv->sem_isr, &abstime);
+#ifdef CONFIG_EFM32_I2C_DYNTIMEO
Review Comment:
@xiaoxiang781216 I can merge, but just had a brief look and see that some places use `#ifdef CONFIG_EFM32_I2C_DYNTIMEOUT` and other `#ifdef CONFIG_EFM32_I2C_DYNTIMEO`.
Should be merge this as is and make a separate PR or can continue a little more here?
I think that we can simplify logic even more by changing `efm32_i2c_tousecs` to `efm32_i2c_toticks`. Same for the other places like `am335x_i2c_tousecs`, etc.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6226: Replace nxsem_timedwait_uninterruptible with nxsem_tickwait_uninterruptible
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6226:
URL: https://github.com/apache/incubator-nuttx/pull/6226#discussion_r872948941
##########
tools/nxstyle.c:
##########
@@ -748,7 +751,7 @@ static bool white_list(const char *ident, int lineno)
pptr++)
{
len = strlen(str);
- if (len2 >= len && strncmp(ident + len2 - len, str, len) == 0)
+ if (len2 >= len && strncmp(ident + len2 - len, str, len) == 0)
Review Comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #6226: Replace nxsem_timedwait_uninterruptible with nxsem_tickwait_uninterruptible
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #6226:
URL: https://github.com/apache/incubator-nuttx/pull/6226#issuecomment-1126089213
@xiaoxiang781216 please rebase on the latest mainline
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6226: Replace nxsem_timedwait_uninterruptible with nxsem_tickwait_uninterruptible
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6226:
URL: https://github.com/apache/incubator-nuttx/pull/6226#discussion_r873191279
##########
arch/arm/src/efm32/efm32_i2c.c:
##########
@@ -528,48 +528,24 @@ static useconds_t efm32_i2c_tousecs(int msgc, struct i2c_msg_s *msgs)
#ifndef CONFIG_I2C_POLLED
static inline int efm32_i2c_sem_waitdone(struct efm32_i2c_priv_s *priv)
{
- struct timespec abstime;
int ret;
do
{
- /* Get the current time */
-
- clock_gettime(CLOCK_REALTIME, &abstime);
-
- /* Calculate a time in the future */
-
-#if CONFIG_EFM32_I2CTIMEOSEC > 0
- abstime.tv_sec += CONFIG_EFM32_I2CTIMEOSEC;
-#endif
-
- /* Add a value proportional to the number of bytes in the transfer */
-
-#ifdef CONFIG_EFM32_I2C_DYNTIMEO
- abstime.tv_nsec += 1000 * efm32_i2c_tousecs(priv->msgc, priv->msgv);
- if (abstime.tv_nsec >= 1000 * 1000 * 1000)
- {
- abstime.tv_sec++;
- abstime.tv_nsec -= 1000 * 1000 * 1000;
- }
-
-#elif CONFIG_EFM32_I2CTIMEOMS > 0
- abstime.tv_nsec += CONFIG_EFM32_I2CTIMEOMS * 1000 * 1000;
- if (abstime.tv_nsec >= 1000 * 1000 * 1000)
- {
- abstime.tv_sec++;
- abstime.tv_nsec -= 1000 * 1000 * 1000;
- }
-#endif
-
/* Enable I2C interrupts */
efm32_i2c_putreg(priv, EFM32_I2C_IEN_OFFSET, I2C_IF_NACK | I2C_IF_ACK |
I2C_IF_MSTOP | I2C_IF_RXDATAV | I2C_IF_ERRORS);
/* Wait until either the transfer is complete or the timeout expires */
- ret = nxsem_timedwait_uninterruptible(&priv->sem_isr, &abstime);
+#ifdef CONFIG_EFM32_I2C_DYNTIMEO
Review Comment:
Do you plan to change `i2c_tousecs` to `i2c_toticks`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko merged pull request #6226: Replace nxsem_timedwait_uninterruptible with nxsem_tickwait_uninterruptible
Posted by GitBox <gi...@apache.org>.
pkarashchenko merged PR #6226:
URL: https://github.com/apache/incubator-nuttx/pull/6226
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6226: Replace nxsem_timedwait_uninterruptible with nxsem_tickwait_uninterruptible
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6226:
URL: https://github.com/apache/incubator-nuttx/pull/6226#discussion_r869059606
##########
arch/arm/src/am335x/am335x_i2c.c:
##########
@@ -546,48 +545,25 @@ static inline int am335x_i2c_sem_waitdone(struct am335x_i2c_priv_s *priv)
/* Signal the interrupt handler that we are waiting. NOTE: Interrupts
* are currently disabled but will be temporarily re-enabled below when
- * nxsem_timedwait() sleeps.
+ * nxsem_tickwait() sleeps.
*/
priv->intstate = INTSTATE_WAITING;
do
{
- /* Get the current time */
-
- clock_gettime(CLOCK_REALTIME, &abstime);
-
- /* Calculate a time in the future */
-
-#if CONFIG_AM335X_I2CTIMEOSEC > 0
- abstime.tv_sec += CONFIG_AM335X_I2CTIMEOSEC;
-#endif
-
- /* Add a value proportional to the number of bytes in the transfer */
+ /* Wait until either the transfer is complete or the timeout expires */
#ifdef CONFIG_AM335X_I2C_DYNTIMEO
- abstime.tv_nsec += 1000 * am335x_i2c_tousecs(priv->msgc, priv->msgv);
- if (abstime.tv_nsec >= 1000 * 1000 * 1000)
- {
- abstime.tv_sec++;
- abstime.tv_nsec -= 1000 * 1000 * 1000;
- }
-
-#elif CONFIG_AM335X_I2CTIMEOMS > 0
- abstime.tv_nsec += CONFIG_AM335X_I2CTIMEOMS * 1000 * 1000;
- if (abstime.tv_nsec >= 1000 * 1000 * 1000)
- {
- abstime.tv_sec++;
- abstime.tv_nsec -= 1000 * 1000 * 1000;
- }
+ ret = nxsem_tickwait(&priv->sem_isr,
+ USEC2TICK(am335x_i2c_tousecs(priv->msgc, priv->msgv));
Review Comment:
```suggestion
ret = nxsem_tickwait(&priv->sem_isr,
USEC2TICK(am335x_i2c_tousecs(priv->msgc,
priv->msgv));
```
##########
arch/arm/src/am335x/am335x_i2c.c:
##########
@@ -546,48 +545,25 @@ static inline int am335x_i2c_sem_waitdone(struct am335x_i2c_priv_s *priv)
/* Signal the interrupt handler that we are waiting. NOTE: Interrupts
* are currently disabled but will be temporarily re-enabled below when
- * nxsem_timedwait() sleeps.
+ * nxsem_tickwait() sleeps.
*/
priv->intstate = INTSTATE_WAITING;
do
{
- /* Get the current time */
-
- clock_gettime(CLOCK_REALTIME, &abstime);
-
- /* Calculate a time in the future */
-
-#if CONFIG_AM335X_I2CTIMEOSEC > 0
- abstime.tv_sec += CONFIG_AM335X_I2CTIMEOSEC;
-#endif
-
- /* Add a value proportional to the number of bytes in the transfer */
+ /* Wait until either the transfer is complete or the timeout expires */
#ifdef CONFIG_AM335X_I2C_DYNTIMEO
- abstime.tv_nsec += 1000 * am335x_i2c_tousecs(priv->msgc, priv->msgv);
- if (abstime.tv_nsec >= 1000 * 1000 * 1000)
- {
- abstime.tv_sec++;
- abstime.tv_nsec -= 1000 * 1000 * 1000;
- }
-
-#elif CONFIG_AM335X_I2CTIMEOMS > 0
- abstime.tv_nsec += CONFIG_AM335X_I2CTIMEOMS * 1000 * 1000;
- if (abstime.tv_nsec >= 1000 * 1000 * 1000)
- {
- abstime.tv_sec++;
- abstime.tv_nsec -= 1000 * 1000 * 1000;
- }
+ ret = nxsem_tickwait(&priv->sem_isr,
+ USEC2TICK(am335x_i2c_tousecs(priv->msgc, priv->msgv));
+#else
+ ret = nxsem_tickwait(&priv->sem_isr,
+ CONFIG_AM335X_I2CTIMEOTICKS);
Review Comment:
```suggestion
ret = nxsem_tickwait(&priv->sem_isr,
CONFIG_AM335X_I2CTIMEOTICKS);
```
##########
arch/arm/src/efm32/efm32_i2c.c:
##########
@@ -528,48 +528,24 @@ static useconds_t efm32_i2c_tousecs(int msgc, struct i2c_msg_s *msgs)
#ifndef CONFIG_I2C_POLLED
static inline int efm32_i2c_sem_waitdone(struct efm32_i2c_priv_s *priv)
{
- struct timespec abstime;
int ret;
do
{
- /* Get the current time */
-
- clock_gettime(CLOCK_REALTIME, &abstime);
-
- /* Calculate a time in the future */
-
-#if CONFIG_EFM32_I2CTIMEOSEC > 0
- abstime.tv_sec += CONFIG_EFM32_I2CTIMEOSEC;
-#endif
-
- /* Add a value proportional to the number of bytes in the transfer */
-
-#ifdef CONFIG_EFM32_I2C_DYNTIMEO
- abstime.tv_nsec += 1000 * efm32_i2c_tousecs(priv->msgc, priv->msgv);
- if (abstime.tv_nsec >= 1000 * 1000 * 1000)
- {
- abstime.tv_sec++;
- abstime.tv_nsec -= 1000 * 1000 * 1000;
- }
-
-#elif CONFIG_EFM32_I2CTIMEOMS > 0
- abstime.tv_nsec += CONFIG_EFM32_I2CTIMEOMS * 1000 * 1000;
- if (abstime.tv_nsec >= 1000 * 1000 * 1000)
- {
- abstime.tv_sec++;
- abstime.tv_nsec -= 1000 * 1000 * 1000;
- }
-#endif
-
/* Enable I2C interrupts */
efm32_i2c_putreg(priv, EFM32_I2C_IEN_OFFSET, I2C_IF_NACK | I2C_IF_ACK |
I2C_IF_MSTOP | I2C_IF_RXDATAV | I2C_IF_ERRORS);
/* Wait until either the transfer is complete or the timeout expires */
- ret = nxsem_timedwait_uninterruptible(&priv->sem_isr, &abstime);
+#ifdef CONFIG_EFM32_I2C_DYNTIMEO
+ ret = nxsem_tickwait_uninterruptible(&priv->sem_isr,
+ USEC2TICK(efm32_i2c_tousecs(priv->msgc, priv->msgv)));
+#else
+ ret = nxsem_tickwait_uninterruptible(&priv->sem_isr,
+ CONFIG_EFM32_I2CTIMEOTICKS);
Review Comment:
```suggestion
ret = nxsem_tickwait_uninterruptible(&priv->sem_isr,
CONFIG_EFM32_I2CTIMEOTICKS);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6226: Replace nxsem_timedwait_uninterruptible with nxsem_tickwait_uninterruptible
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6226:
URL: https://github.com/apache/incubator-nuttx/pull/6226#discussion_r871238116
##########
sched/semaphore/sem_tickwait.c:
##########
@@ -165,19 +145,24 @@ int nxsem_tickwait(FAR sem_t *sem, clock_t start, uint32_t delay)
*
****************************************************************************/
-int nxsem_tickwait_uninterruptible(FAR sem_t *sem, clock_t start,
- uint32_t delay)
+int nxsem_tickwait_uninterruptible(FAR sem_t *sem, uint32_t delay)
{
- int ret;
+ clock_t end = clock_systime_ticks() + delay;
- do
+ for (; ; )
{
/* Take the semaphore (perhaps waiting) */
- ret = nxsem_tickwait(sem, start, delay);
+ int ret = nxsem_tickwait(sem, delay);
+ if (ret != -EINTR)
+ {
+ return ret;
+ }
+
+ delay = end - clock_systime_ticks();
+ if ((int32_t)delay < 0)
+ {
+ delay = 0;
Review Comment:
Why not just simply return `-ETIMEDOUT` here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6226: Replace nxsem_timedwait_uninterruptible with nxsem_tickwait_uninterruptible
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6226:
URL: https://github.com/apache/incubator-nuttx/pull/6226#discussion_r872973586
##########
arch/xtensa/src/esp32/esp32_i2c.c:
##########
@@ -723,31 +723,10 @@ static void esp32_i2c_reset_fsmc(struct esp32_i2c_priv_s *priv)
#ifndef CONFIG_I2C_POLLED
static int esp32_i2c_sem_waitdone(struct esp32_i2c_priv_s *priv)
{
- int ret;
- struct timespec abstime;
-
- /* Get the current absolute time and adds a offset as timeout */
-
- clock_gettime(CLOCK_REALTIME, &abstime);
-
-#if CONFIG_ESP32_I2CTIMEOSEC > 0
- abstime.tv_sec += CONFIG_ESP32_I2CTIMEOSEC;
-#endif
-
-#if CONFIG_ESP32_I2CTIMEOMS > 0
- abstime.tv_nsec += CONFIG_ESP32_I2CTIMEOMS * NSEC_PER_MSEC;
- if (abstime.tv_nsec >= 1000 * NSEC_PER_MSEC)
- {
- abstime.tv_sec++;
- abstime.tv_nsec -= 1000 * NSEC_PER_MSEC;
- }
-#endif
-
/* Wait on ISR semaphore */
- ret = nxsem_timedwait_uninterruptible(&priv->sem_isr, &abstime);
-
- return ret;
+ return nxsem_tickwait_uninterruptible(&priv->sem_isr,
+ SEC2TICK(CONFIG_ESP32_I2CTIMEOSEC) + MSEC2TICK(CONFIG_ESP32_I2CTIMEOMS));
Review Comment:
Lets add
```
#define ESP32_I2CTIMEOTICKS \
(SEC2TICK(CONFIG_ESP32_I2CTIMEOSEC) + MSEC2TICK(CONFIG_ESP32_I2CTIMEOMS))
```
and use it here
##########
net/route/ramroute.h:
##########
@@ -200,19 +200,19 @@ void net_freeroute_ipv6(FAR struct net_route_ipv6_s *route);
void ramroute_ipv4_addlast(FAR struct net_route_ipv4_entry_s *entry,
FAR struct net_route_ipv4_queue_s *list);
FAR struct net_route_ipv4_entry_s *
- ramroute_ipv4_remfirst(struct net_route_ipv4_queue_s *list);
+ramroute_ipv4_remfirst(struct net_route_ipv4_queue_s *list);
FAR struct net_route_ipv4_entry_s *
- ramroute_ipv4_remafter(FAR struct net_route_ipv4_entry_s *entry,
+ramroute_ipv4_remafter(FAR struct net_route_ipv4_entry_s *entry,
FAR struct net_route_ipv4_queue_s *list);
Review Comment:
```suggestion
ramroute_ipv4_remafter(FAR struct net_route_ipv4_entry_s *entry,
FAR struct net_route_ipv4_queue_s *list);
```
##########
net/route/ramroute.h:
##########
@@ -200,19 +200,19 @@ void net_freeroute_ipv6(FAR struct net_route_ipv6_s *route);
void ramroute_ipv4_addlast(FAR struct net_route_ipv4_entry_s *entry,
FAR struct net_route_ipv4_queue_s *list);
FAR struct net_route_ipv4_entry_s *
- ramroute_ipv4_remfirst(struct net_route_ipv4_queue_s *list);
+ramroute_ipv4_remfirst(struct net_route_ipv4_queue_s *list);
FAR struct net_route_ipv4_entry_s *
- ramroute_ipv4_remafter(FAR struct net_route_ipv4_entry_s *entry,
+ramroute_ipv4_remafter(FAR struct net_route_ipv4_entry_s *entry,
FAR struct net_route_ipv4_queue_s *list);
#endif
#ifdef CONFIG_ROUTE_IPv6_RAMROUTE
void ramroute_ipv6_addlast(FAR struct net_route_ipv6_entry_s *entry,
FAR struct net_route_ipv6_queue_s *list);
FAR struct net_route_ipv6_entry_s *
- ramroute_ipv6_remfirst(struct net_route_ipv6_queue_s *list);
+ramroute_ipv6_remfirst(struct net_route_ipv6_queue_s *list);
FAR struct net_route_ipv6_entry_s *
- ramroute_ipv6_remafter(FAR struct net_route_ipv6_entry_s *entry,
+ramroute_ipv6_remafter(FAR struct net_route_ipv6_entry_s *entry,
struct net_route_ipv6_queue_s *list);
Review Comment:
```suggestion
ramroute_ipv6_remafter(FAR struct net_route_ipv6_entry_s *entry,
FAR struct net_route_ipv6_queue_s *list);
```
##########
sched/semaphore/sem_tickwait.c:
##########
@@ -165,19 +145,24 @@ int nxsem_tickwait(FAR sem_t *sem, clock_t start, uint32_t delay)
*
****************************************************************************/
-int nxsem_tickwait_uninterruptible(FAR sem_t *sem, clock_t start,
- uint32_t delay)
+int nxsem_tickwait_uninterruptible(FAR sem_t *sem, uint32_t delay)
{
- int ret;
+ clock_t end = clock_systime_ticks() + delay;
- do
+ for (; ; )
{
/* Take the semaphore (perhaps waiting) */
- ret = nxsem_tickwait(sem, start, delay);
+ int ret = nxsem_tickwait(sem, delay);
+ if (ret != -EINTR)
+ {
+ return ret;
Review Comment:
let's add `break` and move `return ret;` to the end of the function scope
##########
net/route/ramroute.h:
##########
@@ -200,19 +200,19 @@ void net_freeroute_ipv6(FAR struct net_route_ipv6_s *route);
void ramroute_ipv4_addlast(FAR struct net_route_ipv4_entry_s *entry,
FAR struct net_route_ipv4_queue_s *list);
FAR struct net_route_ipv4_entry_s *
- ramroute_ipv4_remfirst(struct net_route_ipv4_queue_s *list);
+ramroute_ipv4_remfirst(struct net_route_ipv4_queue_s *list);
FAR struct net_route_ipv4_entry_s *
- ramroute_ipv4_remafter(FAR struct net_route_ipv4_entry_s *entry,
+ramroute_ipv4_remafter(FAR struct net_route_ipv4_entry_s *entry,
FAR struct net_route_ipv4_queue_s *list);
#endif
#ifdef CONFIG_ROUTE_IPv6_RAMROUTE
void ramroute_ipv6_addlast(FAR struct net_route_ipv6_entry_s *entry,
FAR struct net_route_ipv6_queue_s *list);
FAR struct net_route_ipv6_entry_s *
- ramroute_ipv6_remfirst(struct net_route_ipv6_queue_s *list);
+ramroute_ipv6_remfirst(struct net_route_ipv6_queue_s *list);
Review Comment:
```suggestion
ramroute_ipv6_remfirst(FAR struct net_route_ipv6_queue_s *list);
```
##########
arch/risc-v/src/esp32c3/esp32c3_i2c.c:
##########
@@ -793,27 +793,9 @@ static void esp32c3_i2c_reset_fsmc(struct esp32c3_i2c_priv_s *priv)
#ifndef CONFIG_I2C_POLLED
static int esp32c3_i2c_sem_waitdone(struct esp32c3_i2c_priv_s *priv)
{
- int ret;
- struct timespec abstime;
-
- clock_gettime(CLOCK_REALTIME, &abstime);
-
-#if CONFIG_ESP32C3_I2CTIMEOSEC > 0
- abstime.tv_sec += CONFIG_ESP32C3_I2CTIMEOSEC;
-#endif
-
-#if CONFIG_ESP32C3_I2CTIMEOMS > 0
- abstime.tv_nsec += CONFIG_ESP32C3_I2CTIMEOMS * NSEC_PER_MSEC;
- if (abstime.tv_nsec >= 1000 * NSEC_PER_MSEC)
- {
- abstime.tv_sec++;
- abstime.tv_nsec -= 1000 * NSEC_PER_MSEC;
- }
-#endif
-
- ret = nxsem_timedwait_uninterruptible(&priv->sem_isr, &abstime);
-
- return ret;
+ return nxsem_tickwait_uninterruptible(&priv->sem_isr,
+ SEC2TICK(CONFIG_ESP32C3_I2CTIMEOSEC) +
+ MSEC2TICK(CONFIG_ESP32C3_I2CTIMEOMS));
Review Comment:
Lets add
```
#define ESP32C3_I2CTIMEOTICKS \
(SEC2TICK(CONFIG_ESP32C3_I2CTIMEOSEC) + MSEC2TICK(CONFIG_ESP32C3_I2CTIMEOMS))
```
and use it here
##########
net/route/ramroute.h:
##########
@@ -200,19 +200,19 @@ void net_freeroute_ipv6(FAR struct net_route_ipv6_s *route);
void ramroute_ipv4_addlast(FAR struct net_route_ipv4_entry_s *entry,
FAR struct net_route_ipv4_queue_s *list);
FAR struct net_route_ipv4_entry_s *
- ramroute_ipv4_remfirst(struct net_route_ipv4_queue_s *list);
+ramroute_ipv4_remfirst(struct net_route_ipv4_queue_s *list);
Review Comment:
```suggestion
ramroute_ipv4_remfirst(FAR struct net_route_ipv4_queue_s *list);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6226: Replace nxsem_timedwait_uninterruptible with nxsem_tickwait_uninterruptible
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6226:
URL: https://github.com/apache/incubator-nuttx/pull/6226#discussion_r872853501
##########
tools/nxstyle.c:
##########
@@ -748,7 +751,7 @@ static bool white_list(const char *ident, int lineno)
pptr++)
{
len = strlen(str);
- if (len2 >= len && strncmp(ident + len2 - len, str, len) == 0)
+ if (len2 >= len && strncmp(ident + len2 - len, str, len) == 0)
Review Comment:
```suggestion
if (len2 >= len && strncmp(ident + len2 - len, str, len) == 0)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6226: Replace nxsem_timedwait_uninterruptible with nxsem_tickwait_uninterruptible
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6226:
URL: https://github.com/apache/incubator-nuttx/pull/6226#discussion_r872981511
##########
arch/risc-v/src/esp32c3/esp32c3_i2c.c:
##########
@@ -793,27 +793,9 @@ static void esp32c3_i2c_reset_fsmc(struct esp32c3_i2c_priv_s *priv)
#ifndef CONFIG_I2C_POLLED
static int esp32c3_i2c_sem_waitdone(struct esp32c3_i2c_priv_s *priv)
{
- int ret;
- struct timespec abstime;
-
- clock_gettime(CLOCK_REALTIME, &abstime);
-
-#if CONFIG_ESP32C3_I2CTIMEOSEC > 0
- abstime.tv_sec += CONFIG_ESP32C3_I2CTIMEOSEC;
-#endif
-
-#if CONFIG_ESP32C3_I2CTIMEOMS > 0
- abstime.tv_nsec += CONFIG_ESP32C3_I2CTIMEOMS * NSEC_PER_MSEC;
- if (abstime.tv_nsec >= 1000 * NSEC_PER_MSEC)
- {
- abstime.tv_sec++;
- abstime.tv_nsec -= 1000 * NSEC_PER_MSEC;
- }
-#endif
-
- ret = nxsem_timedwait_uninterruptible(&priv->sem_isr, &abstime);
-
- return ret;
+ return nxsem_tickwait_uninterruptible(&priv->sem_isr,
+ SEC2TICK(CONFIG_ESP32C3_I2CTIMEOSEC) +
+ MSEC2TICK(CONFIG_ESP32C3_I2CTIMEOMS));
Review Comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6226: Replace nxsem_timedwait_uninterruptible with nxsem_tickwait_uninterruptible
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6226:
URL: https://github.com/apache/incubator-nuttx/pull/6226#discussion_r872981750
##########
sched/semaphore/sem_tickwait.c:
##########
@@ -165,19 +145,24 @@ int nxsem_tickwait(FAR sem_t *sem, clock_t start, uint32_t delay)
*
****************************************************************************/
-int nxsem_tickwait_uninterruptible(FAR sem_t *sem, clock_t start,
- uint32_t delay)
+int nxsem_tickwait_uninterruptible(FAR sem_t *sem, uint32_t delay)
{
- int ret;
+ clock_t end = clock_systime_ticks() + delay;
- do
+ for (; ; )
{
/* Take the semaphore (perhaps waiting) */
- ret = nxsem_tickwait(sem, start, delay);
+ int ret = nxsem_tickwait(sem, delay);
+ if (ret != -EINTR)
+ {
+ return ret;
Review Comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #6226: Replace nxsem_timedwait_uninterruptible with nxsem_tickwait_uninterruptible
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #6226:
URL: https://github.com/apache/incubator-nuttx/pull/6226#issuecomment-1126874538
@pkarashchenko could you merge this PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org