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/06/25 05:52:51 UTC

[GitHub] [incubator-nuttx] anchao opened a new pull request, #6518: wireless/bcm43xxx: add dynamic kso control support

anchao opened a new pull request, #6518:
URL: https://github.com/apache/incubator-nuttx/pull/6518

   ## Summary
   
   wireless/bcm43xxx: add dynamic kso control support
   
   1. PMU control by KeepSdioOn(KSO) if firmware support save restore
   2. Remove watchdog timer
   
   Signed-off-by: chao.an <an...@xiaomi.com>
   
   ## Impact
   
   N/A
   
   ## Testing
   
   bcm43013 lowpower  4mA -> 500uA
   


-- 
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] anchao commented on a diff in pull request #6518: wireless/bcm43xxx: add dynamic kso control support

Posted by GitBox <gi...@apache.org>.
anchao commented on code in PR #6518:
URL: https://github.com/apache/incubator-nuttx/pull/6518#discussion_r906660136


##########
drivers/wireless/ieee80211/bcm43xxx/bcmf_sdio.c:
##########
@@ -132,23 +131,96 @@ static struct bcmf_sdio_frame
 int bcmf_oob_irq(FAR void *arg)
 {
   FAR struct bcmf_sdio_dev_s *sbus = (FAR struct bcmf_sdio_dev_s *)arg;
+  int semcount;
 
   if (sbus->ready)
     {
       /* Signal bmcf thread */
 
       sbus->irq_pending = true;
-      nxsem_post(&sbus->thread_signal);
+
+      nxsem_get_value(&sbus->thread_signal, &semcount);
+      if (semcount < 1)
+        {
+          nxsem_post(&sbus->thread_signal);
+        }
     }
 
   return OK;
 }
 
-int bcmf_sdio_bus_sleep(FAR struct bcmf_sdio_dev_s *sbus, bool sleep)
+int bcmf_sdio_kso_enable(FAR struct bcmf_sdio_dev_s *sbus, bool enable)
 {
-  int ret;
+  uint8_t value;
   int loops;
+  int ret;
+
+  if (!sbus->ready)
+    {
+      return -EPERM;
+    }
+
+  if (sbus->kso_enable == enable)
+    {
+      return OK;
+    }
+
+  if (enable)
+    {
+      loops = 200;
+      while (--loops > 0)
+        {
+          ret = bcmf_write_reg(sbus, 1, SBSDIO_FUNC1_SLEEPCSR,
+                               SBSDIO_FUNC1_SLEEPCSR_KSO_MASK |
+                               SBSDIO_FUNC1_SLEEPCSR_DEVON_MASK);
+          if (ret != OK)
+            {
+              wlerr("HT Avail request failed %d\n", ret);
+              return ret;
+            }
+
+          nxsig_usleep(100 * 1000);
+          ret = bcmf_read_reg(sbus, 1, SBSDIO_FUNC1_SLEEPCSR, &value);
+          if (ret != OK)
+            {
+              return ret;
+            }
+
+          if (value & (SBSDIO_FUNC1_SLEEPCSR_KSO_MASK |
+                       SBSDIO_FUNC1_SLEEPCSR_DEVON_MASK))

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 #6518: wireless/bcm43xxx: add dynamic kso control support

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6518:
URL: https://github.com/apache/incubator-nuttx/pull/6518#discussion_r906661477


##########
drivers/wireless/ieee80211/bcm43xxx/bcmf_sdio.c:
##########
@@ -892,23 +962,44 @@ int bcmf_sdio_thread(int argc, char **argv)
 
   while (true)
     {
-      /* Wait for event (device interrupt, user request or waitdog timer) */
+      /* Check if RX/TX frames are available */
 
-      ret = nxsem_wait_uninterruptible(&sbus->thread_signal);
-      if (ret < 0)
+      if ((sbus->intstatus & I_HMB_FRAME_IND) == 0 &&
+          (sbus->tx_queue.tail == NULL) &&
+          !sbus->irq_pending)
         {
-          wlerr("Error while waiting for semaphore\n");
-          break;
-        }
+          /* Wait for event (device interrupt or user request) */
+
+          if (timeout == UINT_MAX)
+            {
+              ret = nxsem_wait_uninterruptible(&sbus->thread_signal);
+            }
+          else
+            {
+              ret = nxsem_tickwait_uninterruptible(&sbus->thread_signal,
+                                                   timeout);
+            }
 
-      /* Restart the waitdog timer */
+          if (ret == -ETIMEDOUT)
+            {
+              /* Turn off clock request. */
+
+              timeout = UINT_MAX;
+              bcmf_sdio_bus_lowpower(sbus, true);
+              continue;

Review Comment:
   oops... sorry, missed where the scope ends. I thought that it will drop to the end of while, but it is end of `if ((sbus->intstatus & I_HMB_FRAME_IND) == 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 #6518: wireless/bcm43xxx: add dynamic kso control support

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6518:
URL: https://github.com/apache/incubator-nuttx/pull/6518#discussion_r906642299


##########
drivers/wireless/ieee80211/bcm43xxx/bcmf_sdio.c:
##########
@@ -132,23 +131,96 @@ static struct bcmf_sdio_frame
 int bcmf_oob_irq(FAR void *arg)
 {
   FAR struct bcmf_sdio_dev_s *sbus = (FAR struct bcmf_sdio_dev_s *)arg;
+  int semcount;
 
   if (sbus->ready)
     {
       /* Signal bmcf thread */
 
       sbus->irq_pending = true;
-      nxsem_post(&sbus->thread_signal);
+
+      nxsem_get_value(&sbus->thread_signal, &semcount);
+      if (semcount < 0)

Review Comment:
   <= ? otherwise you may lose the event



##########
drivers/wireless/ieee80211/bcm43xxx/bcmf_sdpcm.c:
##########
@@ -470,7 +471,11 @@ int bcmf_sdpcm_queue_frame(FAR struct bcmf_dev_s *priv,
 
   /* Notify bcmf thread tx frame is ready */
 
-  nxsem_post(&sbus->thread_signal);
+  nxsem_get_value(&sbus->thread_signal, &semcount);
+  if (semcount < 0)

Review Comment:
   ditto



-- 
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] anchao commented on a diff in pull request #6518: wireless/bcm43xxx: add dynamic kso control support

Posted by GitBox <gi...@apache.org>.
anchao commented on code in PR #6518:
URL: https://github.com/apache/incubator-nuttx/pull/6518#discussion_r906643925


##########
drivers/wireless/ieee80211/bcm43xxx/bcmf_sdio.c:
##########
@@ -132,23 +131,96 @@ static struct bcmf_sdio_frame
 int bcmf_oob_irq(FAR void *arg)
 {
   FAR struct bcmf_sdio_dev_s *sbus = (FAR struct bcmf_sdio_dev_s *)arg;
+  int semcount;
 
   if (sbus->ready)
     {
       /* Signal bmcf thread */
 
       sbus->irq_pending = true;
-      nxsem_post(&sbus->thread_signal);
+
+      nxsem_get_value(&sbus->thread_signal, &semcount);
+      if (semcount < 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 a diff in pull request #6518: wireless/bcm43xxx: add dynamic kso control support

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6518:
URL: https://github.com/apache/incubator-nuttx/pull/6518#discussion_r906655476


##########
drivers/wireless/ieee80211/bcm43xxx/bcmf_sdio.c:
##########
@@ -892,23 +962,44 @@ int bcmf_sdio_thread(int argc, char **argv)
 
   while (true)
     {
-      /* Wait for event (device interrupt, user request or waitdog timer) */
+      /* Check if RX/TX frames are available */
 
-      ret = nxsem_wait_uninterruptible(&sbus->thread_signal);
-      if (ret < 0)
+      if ((sbus->intstatus & I_HMB_FRAME_IND) == 0 &&
+          (sbus->tx_queue.tail == NULL) &&
+          !sbus->irq_pending)
         {
-          wlerr("Error while waiting for semaphore\n");
-          break;
-        }
+          /* Wait for event (device interrupt or user request) */
+
+          if (timeout == UINT_MAX)
+            {
+              ret = nxsem_wait_uninterruptible(&sbus->thread_signal);
+            }
+          else
+            {
+              ret = nxsem_tickwait_uninterruptible(&sbus->thread_signal,
+                                                   timeout);
+            }
 
-      /* Restart the waitdog timer */
+          if (ret == -ETIMEDOUT)
+            {
+              /* Turn off clock request. */
+
+              timeout = UINT_MAX;
+              bcmf_sdio_bus_lowpower(sbus, true);
+              continue;

Review Comment:
   No need.
   ```suggestion
   ```



##########
drivers/wireless/ieee80211/bcm43xxx/bcmf_sdio.c:
##########
@@ -132,23 +131,96 @@ static struct bcmf_sdio_frame
 int bcmf_oob_irq(FAR void *arg)
 {
   FAR struct bcmf_sdio_dev_s *sbus = (FAR struct bcmf_sdio_dev_s *)arg;
+  int semcount;
 
   if (sbus->ready)
     {
       /* Signal bmcf thread */
 
       sbus->irq_pending = true;
-      nxsem_post(&sbus->thread_signal);
+
+      nxsem_get_value(&sbus->thread_signal, &semcount);
+      if (semcount < 1)
+        {
+          nxsem_post(&sbus->thread_signal);
+        }
     }
 
   return OK;
 }
 
-int bcmf_sdio_bus_sleep(FAR struct bcmf_sdio_dev_s *sbus, bool sleep)
+int bcmf_sdio_kso_enable(FAR struct bcmf_sdio_dev_s *sbus, bool enable)
 {
-  int ret;
+  uint8_t value;
   int loops;
+  int ret;
+
+  if (!sbus->ready)
+    {
+      return -EPERM;
+    }
+
+  if (sbus->kso_enable == enable)
+    {
+      return OK;
+    }
+
+  if (enable)
+    {
+      loops = 200;
+      while (--loops > 0)
+        {
+          ret = bcmf_write_reg(sbus, 1, SBSDIO_FUNC1_SLEEPCSR,
+                               SBSDIO_FUNC1_SLEEPCSR_KSO_MASK |
+                               SBSDIO_FUNC1_SLEEPCSR_DEVON_MASK);
+          if (ret != OK)
+            {
+              wlerr("HT Avail request failed %d\n", ret);
+              return ret;
+            }
+
+          nxsig_usleep(100 * 1000);
+          ret = bcmf_read_reg(sbus, 1, SBSDIO_FUNC1_SLEEPCSR, &value);
+          if (ret != OK)
+            {
+              return ret;
+            }
+
+          if (value & (SBSDIO_FUNC1_SLEEPCSR_KSO_MASK |
+                       SBSDIO_FUNC1_SLEEPCSR_DEVON_MASK))

Review Comment:
   ```suggestion
             if (value & (SBSDIO_FUNC1_SLEEPCSR_KSO_MASK |
                          SBSDIO_FUNC1_SLEEPCSR_DEVON_MASK) != 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 merged pull request #6518: wireless/bcm43xxx: add dynamic kso control support

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #6518:
URL: https://github.com/apache/incubator-nuttx/pull/6518


-- 
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] anchao commented on a diff in pull request #6518: wireless/bcm43xxx: add dynamic kso control support

Posted by GitBox <gi...@apache.org>.
anchao commented on code in PR #6518:
URL: https://github.com/apache/incubator-nuttx/pull/6518#discussion_r906660308


##########
drivers/wireless/ieee80211/bcm43xxx/bcmf_sdio.c:
##########
@@ -892,23 +962,44 @@ int bcmf_sdio_thread(int argc, char **argv)
 
   while (true)
     {
-      /* Wait for event (device interrupt, user request or waitdog timer) */
+      /* Check if RX/TX frames are available */
 
-      ret = nxsem_wait_uninterruptible(&sbus->thread_signal);
-      if (ret < 0)
+      if ((sbus->intstatus & I_HMB_FRAME_IND) == 0 &&
+          (sbus->tx_queue.tail == NULL) &&
+          !sbus->irq_pending)
         {
-          wlerr("Error while waiting for semaphore\n");
-          break;
-        }
+          /* Wait for event (device interrupt or user request) */
+
+          if (timeout == UINT_MAX)
+            {
+              ret = nxsem_wait_uninterruptible(&sbus->thread_signal);
+            }
+          else
+            {
+              ret = nxsem_tickwait_uninterruptible(&sbus->thread_signal,
+                                                   timeout);
+            }
 
-      /* Restart the waitdog timer */
+          if (ret == -ETIMEDOUT)
+            {
+              /* Turn off clock request. */
+
+              timeout = UINT_MAX;
+              bcmf_sdio_bus_lowpower(sbus, true);
+              continue;

Review Comment:
   why? it is unnecessary to ty other condition further if enter lowpoer mode 



-- 
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