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 2021/12/21 19:58:20 UTC

[GitHub] [incubator-nuttx] acassis opened a new pull request #5056: Add WiFi/BLE Coexistence support

acassis opened a new pull request #5056:
URL: https://github.com/apache/incubator-nuttx/pull/5056


   ## Summary
   Add WiFi/BLE Coexistence support
   ## Impact
   Users will be to use WiFi and BLE at same time
   ## Testing
   ESP32-Devkitc
   


-- 
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] gustavonihei commented on a change in pull request #5056: Add WiFi/BLE Coexistence support

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5056:
URL: https://github.com/apache/incubator-nuttx/pull/5056#discussion_r774598903



##########
File path: arch/xtensa/src/esp32/esp32_wifi_adapter.c
##########
@@ -3990,7 +3913,11 @@ static int32_t esp_coex_wifi_request(uint32_t event, uint32_t latency,
 
 static int32_t esp_coex_wifi_release(uint32_t event)
 {
-  return 0;
+#if defined(CONFIG_SW_COEXIST_ENABLE)

Review comment:
       ```suggestion
   #if defined(CONFIG_ESP32_WIFI_BT_COEXIST)
   ```
   Wrong config name




-- 
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 #5056: Add WiFi/BLE Coexistence support

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


   


-- 
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] gustavonihei commented on a change in pull request #5056: Add WiFi/BLE Coexistence support

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5056:
URL: https://github.com/apache/incubator-nuttx/pull/5056#discussion_r774737719



##########
File path: arch/xtensa/src/esp32/esp32_wireless.h
##########
@@ -0,0 +1,106 @@
+/****************************************************************************
+ * arch/xtensa/src/esp32/esp32_wireless.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __ARCH_XTENSA_SRC_ESP32_ESP32_WIRELESS_H
+#define __ARCH_XTENSA_SRC_ESP32_ESP32_WIRELESS_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+

Review comment:
       It is important to document here that the following definitions are required by the library.
   Just in case someone accidentally removes them since they are not referenced anywhere in the codebase.




-- 
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] gustavonihei commented on a change in pull request #5056: Add WiFi/BLE Coexistence support

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5056:
URL: https://github.com/apache/incubator-nuttx/pull/5056#discussion_r773465590



##########
File path: arch/xtensa/src/esp32/esp32_wireless.c
##########
@@ -0,0 +1,279 @@
+/****************************************************************************
+ * arch/xtensa/src/esp32/esp32_wireless.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#include <nuttx/kmalloc.h>
+
+#include <debug.h>
+#include <assert.h>
+
+#include "xtensa.h"
+#include "hardware/esp32_soc.h"
+#include "hardware/esp32_dport.h"
+#include "esp_phy_init.h"
+#include "phy_init_data.h"
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+static inline void phy_digital_regs_store(void);
+static inline void phy_digital_regs_load(void);
+
+/****************************************************************************
+ * Extern Functions declaration
+ ****************************************************************************/
+
+#ifdef CONFIG_ESP32_BLE
+extern void coex_bt_high_prio(void);
+extern void phy_wakeup_init(void);
+extern void phy_close_rf(void);
+#endif
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/* Wi-Fi sleep private data */
+
+static uint32_t g_phy_clk_en_cnt;
+
+/* Reference count of enabling PHY */
+
+static uint8_t g_phy_access_ref;
+
+/* Memory to store PHY digital registers */
+
+static uint32_t *g_phy_digital_regs_mem = NULL;
+
+/* Indicate PHY is calibrated or not */
+
+static bool g_is_phy_calibrated = false;
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: phy_digital_regs_store
+ *
+ * Description:
+ *    Store  PHY digital registers.
+ *
+ ****************************************************************************/
+
+static inline void phy_digital_regs_store(void)
+{
+  if (g_phy_digital_regs_mem == NULL)
+    {
+      g_phy_digital_regs_mem = (uint32_t *)
+                    kmm_malloc(SOC_PHY_DIG_REGS_MEM_SIZE);
+    }
+
+  DEBUGASSERT(g_phy_digital_regs_mem != NULL);
+
+  phy_dig_reg_backup(true, g_phy_digital_regs_mem);
+}
+
+/****************************************************************************
+ * Name: phy_digital_regs_load
+ *
+ * Description:
+ *   Load  PHY digital registers.
+ *
+ ****************************************************************************/
+
+static inline void phy_digital_regs_load(void)
+{
+  if (g_phy_digital_regs_mem != NULL)
+    {
+      phy_dig_reg_backup(false, g_phy_digital_regs_mem);
+    }
+}
+
+/****************************************************************************
+ * Name: esp32_phy_enable_clock
+ *
+ * Description:
+ *   Enable PHY hardware clock
+ *
+ * Input Parameters:
+ *   None
+ *
+ * Returned Value:
+ *   None
+ *
+ ****************************************************************************/
+
+void esp32_phy_enable_clock(void)
+{
+  irqstate_t flags;
+
+  flags = enter_critical_section();
+
+  if (g_phy_clk_en_cnt == 0)
+    {
+      modifyreg32(DPORT_WIFI_CLK_EN_REG, 0,
+                  DPORT_WIFI_CLK_WIFI_BT_COMMON_M);
+    }
+
+  g_phy_clk_en_cnt++;
+
+  leave_critical_section(flags);
+}
+
+/****************************************************************************
+ * Name: esp32_phy_disable_clock
+ *
+ * Description:
+ *   Disable PHY hardware clock
+ *
+ * Input Parameters:
+ *   None
+ *
+ * Returned Value:
+ *   None
+ *
+ ****************************************************************************/
+
+void esp32_phy_disable_clock(void)
+{
+  irqstate_t flags;
+
+  flags = enter_critical_section();
+
+  if (g_phy_clk_en_cnt > 0)
+    {
+      g_phy_clk_en_cnt--;
+      if (g_phy_clk_en_cnt == 0)
+        {
+          modifyreg32(DPORT_WIFI_CLK_EN_REG,
+                      DPORT_WIFI_CLK_WIFI_BT_COMMON_M,
+                      0);
+        }
+    }
+
+  leave_critical_section(flags);
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: esp32_phy_disable
+ *
+ * Description:
+ *   Deinitialize PHY hardware
+ *
+ * Input Parameters:
+ *   None
+ *
+ * Returned Value:
+ *   None
+ *
+ ****************************************************************************/
+
+void esp32_phy_disable(void)
+{
+  irqstate_t flags;
+  flags = enter_critical_section();
+
+  g_phy_access_ref--;
+
+  if (g_phy_access_ref == 0)
+    {
+      /* Disable PHY and RF. */
+
+      phy_close_rf();
+
+      /* Disable Wi-Fi/BT common peripheral clock.
+       * Do not disable clock for hardware RNG.
+       */
+
+      esp32_phy_disable_clock();
+    }
+
+  leave_critical_section(flags);
+}
+
+/****************************************************************************
+ * Name: esp32_phy_enable
+ *
+ * Description:
+ *   Initialize PHY hardware
+ *
+ * Input Parameters:
+ *   None
+ *
+ * Returned Value:
+ *   None
+ *
+ ****************************************************************************/
+
+void esp32_phy_enable(void)
+{
+  static bool debug = false;
+  irqstate_t flags;
+  esp_phy_calibration_data_t *cal_data;
+  char *phy_version = get_phy_version_str();
+  if (debug == false)
+    {
+      debug = true;
+      wlinfo("phy_version %s\n", phy_version);
+    }
+
+  cal_data = kmm_zalloc(sizeof(esp_phy_calibration_data_t));
+  if (!cal_data)
+    {
+      wlerr("ERROR: Failed to kmm_zalloc");
+      DEBUGASSERT(0);
+      return;

Review comment:
       ```suggestion
         abort();
   ```
   If it doesn't make sense to continue with the code execution in this scenario, better call abort at once.




-- 
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] gustavonihei commented on a change in pull request #5056: Add WiFi/BLE Coexistence support

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5056:
URL: https://github.com/apache/incubator-nuttx/pull/5056#discussion_r773463998



##########
File path: arch/xtensa/src/esp32/esp32_wireless.c
##########
@@ -0,0 +1,279 @@
+/****************************************************************************
+ * arch/xtensa/src/esp32/esp32_wireless.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#include <nuttx/kmalloc.h>
+
+#include <debug.h>
+#include <assert.h>
+
+#include "xtensa.h"
+#include "hardware/esp32_soc.h"
+#include "hardware/esp32_dport.h"
+#include "esp_phy_init.h"
+#include "phy_init_data.h"
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+static inline void phy_digital_regs_store(void);
+static inline void phy_digital_regs_load(void);
+
+/****************************************************************************
+ * Extern Functions declaration
+ ****************************************************************************/
+
+#ifdef CONFIG_ESP32_BLE
+extern void coex_bt_high_prio(void);
+extern void phy_wakeup_init(void);
+extern void phy_close_rf(void);
+#endif
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/* Wi-Fi sleep private data */
+
+static uint32_t g_phy_clk_en_cnt;
+
+/* Reference count of enabling PHY */
+
+static uint8_t g_phy_access_ref;
+
+/* Memory to store PHY digital registers */
+
+static uint32_t *g_phy_digital_regs_mem = NULL;
+
+/* Indicate PHY is calibrated or not */
+
+static bool g_is_phy_calibrated = false;
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: phy_digital_regs_store
+ *
+ * Description:
+ *    Store  PHY digital registers.
+ *
+ ****************************************************************************/
+
+static inline void phy_digital_regs_store(void)
+{
+  if (g_phy_digital_regs_mem == NULL)
+    {
+      g_phy_digital_regs_mem = (uint32_t *)
+                    kmm_malloc(SOC_PHY_DIG_REGS_MEM_SIZE);
+    }
+
+  DEBUGASSERT(g_phy_digital_regs_mem != NULL);
+
+  phy_dig_reg_backup(true, g_phy_digital_regs_mem);
+}
+
+/****************************************************************************
+ * Name: phy_digital_regs_load
+ *
+ * Description:
+ *   Load  PHY digital registers.
+ *
+ ****************************************************************************/
+
+static inline void phy_digital_regs_load(void)
+{
+  if (g_phy_digital_regs_mem != NULL)
+    {
+      phy_dig_reg_backup(false, g_phy_digital_regs_mem);
+    }
+}
+
+/****************************************************************************
+ * Name: esp32_phy_enable_clock
+ *
+ * Description:
+ *   Enable PHY hardware clock
+ *
+ * Input Parameters:
+ *   None
+ *
+ * Returned Value:
+ *   None
+ *
+ ****************************************************************************/
+
+void esp32_phy_enable_clock(void)
+{
+  irqstate_t flags;
+
+  flags = enter_critical_section();
+
+  if (g_phy_clk_en_cnt == 0)
+    {
+      modifyreg32(DPORT_WIFI_CLK_EN_REG, 0,
+                  DPORT_WIFI_CLK_WIFI_BT_COMMON_M);
+    }
+
+  g_phy_clk_en_cnt++;
+
+  leave_critical_section(flags);
+}
+
+/****************************************************************************
+ * Name: esp32_phy_disable_clock
+ *
+ * Description:
+ *   Disable PHY hardware clock
+ *
+ * Input Parameters:
+ *   None
+ *
+ * Returned Value:
+ *   None
+ *
+ ****************************************************************************/
+
+void esp32_phy_disable_clock(void)
+{
+  irqstate_t flags;
+
+  flags = enter_critical_section();
+
+  if (g_phy_clk_en_cnt > 0)
+    {
+      g_phy_clk_en_cnt--;
+      if (g_phy_clk_en_cnt == 0)
+        {
+          modifyreg32(DPORT_WIFI_CLK_EN_REG,
+                      DPORT_WIFI_CLK_WIFI_BT_COMMON_M,
+                      0);
+        }
+    }
+
+  leave_critical_section(flags);
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: esp32_phy_disable
+ *
+ * Description:
+ *   Deinitialize PHY hardware
+ *
+ * Input Parameters:
+ *   None
+ *
+ * Returned Value:
+ *   None
+ *
+ ****************************************************************************/
+
+void esp32_phy_disable(void)
+{
+  irqstate_t flags;
+  flags = enter_critical_section();
+
+  g_phy_access_ref--;
+
+  if (g_phy_access_ref == 0)
+    {
+      /* Disable PHY and RF. */
+
+      phy_close_rf();
+
+      /* Disable Wi-Fi/BT common peripheral clock.
+       * Do not disable clock for hardware RNG.
+       */
+
+      esp32_phy_disable_clock();
+    }
+
+  leave_critical_section(flags);
+}
+
+/****************************************************************************
+ * Name: esp32_phy_enable
+ *
+ * Description:
+ *   Initialize PHY hardware
+ *
+ * Input Parameters:
+ *   None
+ *
+ * Returned Value:
+ *   None
+ *
+ ****************************************************************************/
+
+void esp32_phy_enable(void)
+{
+  static bool debug = false;
+  irqstate_t flags;
+  esp_phy_calibration_data_t *cal_data;
+  char *phy_version = get_phy_version_str();
+  if (debug == false)
+    {
+      debug = true;
+      wlinfo("phy_version %s\n", phy_version);
+    }

Review comment:
       ```suggestion
     if (debug == false)
       {
         char *phy_version = get_phy_version_str();
         wlinfo("phy_version %s\n", phy_version);
         debug = true;
       }
   ```
   `phy_version` scope may be restricted to the `if` block.




-- 
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] acassis commented on a change in pull request #5056: Add WiFi/BLE Coexistence support

Posted by GitBox <gi...@apache.org>.
acassis commented on a change in pull request #5056:
URL: https://github.com/apache/incubator-nuttx/pull/5056#discussion_r774745624



##########
File path: arch/xtensa/src/esp32/esp32_wireless.h
##########
@@ -0,0 +1,106 @@
+/****************************************************************************
+ * arch/xtensa/src/esp32/esp32_wireless.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __ARCH_XTENSA_SRC_ESP32_ESP32_WIRELESS_H
+#define __ARCH_XTENSA_SRC_ESP32_ESP32_WIRELESS_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define CONFIG_ESP32_SUPPORT_MULTIPLE_PHY_INIT_DATA_BIN 0

Review comment:
       Unfortunately the current IDF header files are old and doesn't have this new definition. Let keep it for now.




-- 
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] gustavonihei commented on a change in pull request #5056: Add WiFi/BLE Coexistence support

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5056:
URL: https://github.com/apache/incubator-nuttx/pull/5056#discussion_r774600330



##########
File path: arch/xtensa/src/esp32/esp32_wifi_adapter.c
##########
@@ -350,6 +358,7 @@ static int wifi_coex_set_schm_curr_phase_idx(int idx);
 static int wifi_coex_get_schm_curr_phase_idx(void);
 
 extern void coex_bt_high_prio(void);
+extern int coex_wifi_release(uint32_t event);

Review comment:
       `coex_wifi_release` is already declared in `esp_coexist_internal.h`, so no need to declare it again 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] gustavonihei commented on a change in pull request #5056: Add WiFi/BLE Coexistence support

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5056:
URL: https://github.com/apache/incubator-nuttx/pull/5056#discussion_r773462685



##########
File path: arch/xtensa/src/esp32/esp32_wireless.c
##########
@@ -0,0 +1,279 @@
+/****************************************************************************
+ * arch/xtensa/src/esp32/esp32_wireless.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#include <nuttx/kmalloc.h>
+
+#include <debug.h>
+#include <assert.h>
+
+#include "xtensa.h"
+#include "hardware/esp32_soc.h"
+#include "hardware/esp32_dport.h"
+#include "esp_phy_init.h"
+#include "phy_init_data.h"
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+static inline void phy_digital_regs_store(void);
+static inline void phy_digital_regs_load(void);
+
+/****************************************************************************
+ * Extern Functions declaration
+ ****************************************************************************/
+
+#ifdef CONFIG_ESP32_BLE
+extern void coex_bt_high_prio(void);
+extern void phy_wakeup_init(void);
+extern void phy_close_rf(void);
+#endif
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/* Wi-Fi sleep private data */
+
+static uint32_t g_phy_clk_en_cnt;
+
+/* Reference count of enabling PHY */
+
+static uint8_t g_phy_access_ref;
+
+/* Memory to store PHY digital registers */
+
+static uint32_t *g_phy_digital_regs_mem = NULL;
+
+/* Indicate PHY is calibrated or not */
+
+static bool g_is_phy_calibrated = false;
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: phy_digital_regs_store
+ *
+ * Description:
+ *    Store  PHY digital registers.
+ *
+ ****************************************************************************/
+
+static inline void phy_digital_regs_store(void)
+{
+  if (g_phy_digital_regs_mem == NULL)
+    {
+      g_phy_digital_regs_mem = (uint32_t *)
+                    kmm_malloc(SOC_PHY_DIG_REGS_MEM_SIZE);
+    }
+
+  DEBUGASSERT(g_phy_digital_regs_mem != NULL);
+
+  phy_dig_reg_backup(true, g_phy_digital_regs_mem);
+}
+
+/****************************************************************************
+ * Name: phy_digital_regs_load
+ *
+ * Description:
+ *   Load  PHY digital registers.
+ *
+ ****************************************************************************/
+
+static inline void phy_digital_regs_load(void)
+{
+  if (g_phy_digital_regs_mem != NULL)
+    {
+      phy_dig_reg_backup(false, g_phy_digital_regs_mem);
+    }
+}
+
+/****************************************************************************
+ * Name: esp32_phy_enable_clock
+ *
+ * Description:
+ *   Enable PHY hardware clock
+ *
+ * Input Parameters:
+ *   None
+ *
+ * Returned Value:
+ *   None
+ *
+ ****************************************************************************/
+
+void esp32_phy_enable_clock(void)
+{
+  irqstate_t flags;
+
+  flags = enter_critical_section();
+
+  if (g_phy_clk_en_cnt == 0)
+    {
+      modifyreg32(DPORT_WIFI_CLK_EN_REG, 0,
+                  DPORT_WIFI_CLK_WIFI_BT_COMMON_M);
+    }
+
+  g_phy_clk_en_cnt++;
+
+  leave_critical_section(flags);
+}
+
+/****************************************************************************
+ * Name: esp32_phy_disable_clock
+ *
+ * Description:
+ *   Disable PHY hardware clock
+ *
+ * Input Parameters:
+ *   None
+ *
+ * Returned Value:
+ *   None
+ *
+ ****************************************************************************/
+
+void esp32_phy_disable_clock(void)
+{
+  irqstate_t flags;
+
+  flags = enter_critical_section();
+
+  if (g_phy_clk_en_cnt > 0)
+    {
+      g_phy_clk_en_cnt--;
+      if (g_phy_clk_en_cnt == 0)
+        {
+          modifyreg32(DPORT_WIFI_CLK_EN_REG,
+                      DPORT_WIFI_CLK_WIFI_BT_COMMON_M,
+                      0);
+        }
+    }
+
+  leave_critical_section(flags);
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: esp32_phy_disable
+ *
+ * Description:
+ *   Deinitialize PHY hardware
+ *
+ * Input Parameters:
+ *   None
+ *
+ * Returned Value:
+ *   None
+ *
+ ****************************************************************************/
+
+void esp32_phy_disable(void)
+{
+  irqstate_t flags;
+  flags = enter_critical_section();
+
+  g_phy_access_ref--;
+
+  if (g_phy_access_ref == 0)
+    {
+      /* Disable PHY and RF. */
+
+      phy_close_rf();
+
+      /* Disable Wi-Fi/BT common peripheral clock.
+       * Do not disable clock for hardware RNG.
+       */
+
+      esp32_phy_disable_clock();
+    }
+
+  leave_critical_section(flags);
+}
+
+/****************************************************************************
+ * Name: esp32_phy_enable
+ *
+ * Description:
+ *   Initialize PHY hardware
+ *
+ * Input Parameters:
+ *   None
+ *
+ * Returned Value:
+ *   None
+ *
+ ****************************************************************************/
+
+void esp32_phy_enable(void)
+{
+  static bool debug = false;
+  irqstate_t flags;
+  esp_phy_calibration_data_t *cal_data;
+  char *phy_version = get_phy_version_str();
+  if (debug == false)
+    {
+      debug = true;
+      wlinfo("phy_version %s\n", phy_version);
+    }
+
+  cal_data = kmm_zalloc(sizeof(esp_phy_calibration_data_t));
+  if (!cal_data)
+    {
+      wlerr("ERROR: Failed to kmm_zalloc");

Review comment:
       ```suggestion
         wlerr("ERROR: Failed to allocate PHY calibration data buffer.");
   ```
   Error message should provide some context information to ease debug.




-- 
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] gustavonihei commented on a change in pull request #5056: Add WiFi/BLE Coexistence support

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5056:
URL: https://github.com/apache/incubator-nuttx/pull/5056#discussion_r773461087



##########
File path: arch/xtensa/src/esp32/esp32_wifi_adapter.h
##########
@@ -84,6 +102,10 @@ typedef void (*wifi_evt_cb_t)(void *p);
 
 typedef void (*wifi_txdone_cb_t)(uint8_t *data, uint16_t *len, bool status);
 
+#define COEX_ADAPTER_FUNCS_TIME_BLOCKING      0xffffffff
+
+/* extern coex_adapter_funcs_t g_coex_adapter_funcs; */

Review comment:
       ```suggestion
   ```
   Commented lines may be 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] gustavonihei commented on a change in pull request #5056: Add WiFi/BLE Coexistence support

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5056:
URL: https://github.com/apache/incubator-nuttx/pull/5056#discussion_r773467950



##########
File path: boards/xtensa/esp32/esp32-devkitc/src/esp32_bringup.c
##########
@@ -223,6 +223,15 @@ int esp32_bringup(void)
     }
 #endif
 
+#ifdef CONFIG_ESP32_WIFI_BT_COEXIST
+  ret = esp32_wifi_bt_coexist_init();
+  if (ret)
+    {
+      syslog(LOG_ERR, "ERROR: Failed to initialize Wi-Fi and BT \
+                       coexistence support\n");

Review comment:
       ```suggestion
         syslog(LOG_ERR, "ERROR: Failed to initialize Wi-Fi and BT "
                "coexistence support\n");
   ```
   All those whitespaces from the beginning of line 231 would be output since they are part of the string.




-- 
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] gustavonihei commented on a change in pull request #5056: Add WiFi/BLE Coexistence support

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5056:
URL: https://github.com/apache/incubator-nuttx/pull/5056#discussion_r774601010



##########
File path: arch/xtensa/src/esp32/esp32_wireless.h
##########
@@ -0,0 +1,78 @@
+/****************************************************************************
+ * arch/xtensa/src/esp32/esp32_wireless.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __ARCH_XTENSA_SRC_ESP32_ESP32_WIRELESS_H
+#define __ARCH_XTENSA_SRC_ESP32_ESP32_WIRELESS_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define CONFIG_ESP32_SUPPORT_MULTIPLE_PHY_INIT_DATA_BIN 0
+#define CONFIG_MAC_BB_PD                                0
+#define SOC_COEX_HW_PTI                                 0
+
+/****************************************************************************
+ * Public Function Prototypes
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: esp32_phy_enable
+ *
+ * Description:
+ *   Initialize PHY hardware
+ *
+ * Input Parameters:
+ *   None
+ *
+ * Returned Value:
+ *   None
+ *
+ ****************************************************************************/
+
+void esp32_phy_enable(void);
+
+/****************************************************************************
+ * Name: esp32_phy_disable
+ *
+ * Description:
+ *   Deinitialize PHY hardware
+ *
+ * Input Parameters:
+ *   None
+ *
+ * Returned Value:
+ *   None
+ *
+ ****************************************************************************/
+
+void esp32_phy_disable(void);
+
+void esp32_phy_enable_clock(void);
+
+void esp32_phy_disable_clock(void);

Review comment:
       Missing documentation on both functions




-- 
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] gustavonihei commented on a change in pull request #5056: Add WiFi/BLE Coexistence support

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5056:
URL: https://github.com/apache/incubator-nuttx/pull/5056#discussion_r774599694



##########
File path: arch/xtensa/src/esp32/esp32_wifi_adapter.c
##########
@@ -3926,7 +3829,13 @@ static void wifi_coex_deinit(void)
 
 static int wifi_coex_enable(void)
 {
+#if defined(CONFIG_ESP32_WIFI_BT_COEXIST)
+  int ret;
+  ret = coex_enable();
+  return ret;

Review comment:
       ```suggestion
     return coex_enable();
   ```
   nit: This may be simplified




-- 
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] gustavonihei commented on a change in pull request #5056: Add WiFi/BLE Coexistence support

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5056:
URL: https://github.com/apache/incubator-nuttx/pull/5056#discussion_r774737036



##########
File path: arch/xtensa/src/esp32/esp32_wireless.h
##########
@@ -0,0 +1,106 @@
+/****************************************************************************
+ * arch/xtensa/src/esp32/esp32_wireless.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __ARCH_XTENSA_SRC_ESP32_ESP32_WIRELESS_H
+#define __ARCH_XTENSA_SRC_ESP32_ESP32_WIRELESS_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define CONFIG_ESP32_SUPPORT_MULTIPLE_PHY_INIT_DATA_BIN 0

Review comment:
       ```suggestion
   #define CONFIG_ESP_PHY_MULTIPLE_INIT_DATA_BIN 0
   ```
   IDF marks `CONFIG_ESP32_SUPPORT_MULTIPLE_PHY_INIT_DATA_BIN` as deprecated option, so it is better to use the updated config for making the driver future-proof.




-- 
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] gustavonihei commented on a change in pull request #5056: Add WiFi/BLE Coexistence support

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5056:
URL: https://github.com/apache/incubator-nuttx/pull/5056#discussion_r773467950



##########
File path: boards/xtensa/esp32/esp32-devkitc/src/esp32_bringup.c
##########
@@ -223,6 +223,15 @@ int esp32_bringup(void)
     }
 #endif
 
+#ifdef CONFIG_ESP32_WIFI_BT_COEXIST
+  ret = esp32_wifi_bt_coexist_init();
+  if (ret)
+    {
+      syslog(LOG_ERR, "ERROR: Failed to initialize Wi-Fi and BT \
+                       coexistence support\n");

Review comment:
       ```suggestion
         syslog(LOG_ERR, "ERROR: Failed to initialize Wi-Fi and BT "
                "coexistence support\n");
   ```
   All those whitespaces from line 231 would be output since they are part of the string.




-- 
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] gustavonihei commented on a change in pull request #5056: Add WiFi/BLE Coexistence support

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5056:
URL: https://github.com/apache/incubator-nuttx/pull/5056#discussion_r773466360



##########
File path: arch/xtensa/src/esp32/hardware/esp32_soc.h
##########
@@ -804,6 +804,10 @@ extern int rom_i2c_writereg(int block, int block_id, int reg_add,
 
 #define INVALID_MMU_VAL             0x100
 
+/*  phy registers and memory size */
+
+#define SOC_PHY_DIG_REGS_MEM_SIZE (21*4)

Review comment:
       ```suggestion
   #define SOC_PHY_DIG_REGS_MEM_SIZE   (21*4)
   ```
   nit: adjust alignment




-- 
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] gustavonihei commented on a change in pull request #5056: Add WiFi/BLE Coexistence support

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5056:
URL: https://github.com/apache/incubator-nuttx/pull/5056#discussion_r774739194



##########
File path: arch/xtensa/src/esp32/esp32_wifi_adapter.c
##########
@@ -1165,6 +1188,70 @@ static int32_t esp_semphr_give(void *semphr)
   return osi_errno_trans(ret);
 }
 
+/****************************************************************************
+ * Name: esp_semphr_take_from_isr
+ *
+ * Description:
+ *   Try to take semaphore from within an interrupt service routine.
+ *
+ * Input Parameters:
+ *   semphr - Semaphore data pointer
+ *
+ * Returned Value:
+ *   True if success or false if fail
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_ESP32_WIFI_BT_COEXIST

Review comment:
       ```suggestion
   #ifdef CONFIG_ESP32_WIFI_BT_COEXIST
   
   /****************************************************************************
    * Name: esp_semphr_take_from_isr
    *
    * Description:
    *   Try to take semaphore from within an interrupt service routine.
    *
    * Input Parameters:
    *   semphr - Semaphore data pointer
    *
    * Returned Value:
    *   True if success or false if fail
    *
    ****************************************************************************/
   
   ```
   Since this `ifdef` wraps more than just a single function, it is better to move it up so that the documentation for the first function is not left out.




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