[GitHub] wes3 commented on a change in pull request #655: Support for APOLLO 2 and emspi HCI transport

[GitBox <gi...@apache.org>]

wes3 commented on a change in pull request #655: Support for APOLLO 2 and emspi HCI transport
URL: https://github.com/apache/mynewt-core/pull/655#discussion_r150285687
 
 

 ##########
 File path: net/nimble/transport/emspi/src/ble_hci_emspi.c
 ##########
 @@ -0,0 +1,935 @@
+/*
+ * 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.
+ */
+
+#include <assert.h>
+#include <string.h>
+#include <stdio.h>
+#include <errno.h>
+#include <stdint.h>
+#include "sysinit/sysinit.h"
+#include "syscfg/syscfg.h"
+#include "os/os_cputime.h"
+#include "bsp/bsp.h"
+#include "os/os.h"
+#include "mem/mem.h"
+#include "hal/hal_gpio.h"
+#include "hal/hal_spi.h"
+
+/* BLE */
+#include "nimble/ble.h"
+#include "nimble/nimble_opt.h"
+#include "nimble/hci_common.h"
+#include "nimble/ble_hci_trans.h"
+
+#include "transport/emspi/ble_hci_emspi.h"
+
+#include "am_mcu_apollo.h"
+
+/***
+ * NOTES:
+ * The emspi HCI transport doesn't use event buffer priorities.  All incoming
+ * and outgoing events use buffers from the same pool.
+ *
+ */
+
+#define BLE_HCI_EMSPI_PKT_EVT_COUNT         \
+    (MYNEWT_VAL(BLE_HCI_EVT_HI_BUF_COUNT) + \
+     MYNEWT_VAL(BLE_HCI_EVT_LO_BUF_COUNT))
+
+#define BLE_HCI_EMSPI_PKT_NONE          0x00
+#define BLE_HCI_EMSPI_PKT_CMD           0x01
+#define BLE_HCI_EMSPI_PKT_ACL           0x02
+#define BLE_HCI_EMSPI_PKT_EVT           0x04
+
+#define BLE_HCI_EMSPI_CTLR_STATUS_OK    0xc0
+#define BLE_HCI_EMSPI_OP_TX             0x42
+#define BLE_HCI_EMSPI_OP_RX             0x81
+
+/**
+ * A packet to be sent over the UART.  This can be a command, an event, or ACL
+ * data.
+ */
+struct ble_hci_emspi_pkt {
+    STAILQ_ENTRY(ble_hci_emspi_pkt) next;
+    void *data;
+    uint8_t type;
+};
+STAILQ_HEAD(, ble_hci_emspi_pkt) ble_hci_emspi_tx_q;
+
+static os_event_fn ble_hci_emspi_event_txrx;
+
+static struct os_event ble_hci_emspi_ev_txrx = {
+    .ev_cb = ble_hci_emspi_event_txrx,
+};
+
+static struct os_eventq ble_hci_emspi_evq;
+static struct os_task ble_hci_emspi_task;
+static os_stack_t ble_hci_emspi_stack[MYNEWT_VAL(BLE_HCI_EMSPI_STACK_SIZE)];
+
+static ble_hci_trans_rx_cmd_fn *ble_hci_emspi_rx_cmd_cb;
+static void *ble_hci_emspi_rx_cmd_arg;
+
+static ble_hci_trans_rx_acl_fn *ble_hci_emspi_rx_acl_cb;
+static void *ble_hci_emspi_rx_acl_arg;
+
+static struct os_mempool ble_hci_emspi_evt_hi_pool;
+static void *ble_hci_emspi_evt_hi_buf;
+static struct os_mempool ble_hci_emspi_evt_lo_pool;
+static void *ble_hci_emspi_evt_lo_buf;
+
+static struct os_mempool ble_hci_emspi_cmd_pool;
+static void *ble_hci_emspi_cmd_buf;
+
+static struct os_mempool ble_hci_emspi_pkt_pool;
+static void *ble_hci_emspi_pkt_buf;
+
+static struct os_mbuf_pool ble_hci_emspi_acl_mbuf_pool;
+static struct os_mempool ble_hci_emspi_acl_pool;
+static void *ble_hci_emspi_acl_buf;
+
+static void
+ble_hci_emspi_rdy_isr(void *arg)
+{
+    os_eventq_put(&ble_hci_emspi_evq, &ble_hci_emspi_ev_txrx);
+}
+
+static void
+ble_hci_emspi_initiate_write(void)
+{
+    hal_gpio_irq_disable(MYNEWT_VAL(BLE_HCI_EMSPI_RDY_PIN));
+
+    /* Assert slave select. */
+    hal_gpio_write(MYNEWT_VAL(BLE_HCI_EMSPI_SS_PIN), 0);
+
+    /* Wait for controller to indicate ready-to-receive. */
+    while (!hal_gpio_read(MYNEWT_VAL(BLE_HCI_EMSPI_RDY_PIN))) { }
+}
+
+static void
+ble_hci_emspi_terminate_write(void)
+{
+    const uint64_t rdy_mask =
+        AM_HAL_GPIO_BIT(MYNEWT_VAL(BLE_HCI_EMSPI_RDY_PIN));
+    os_sr_t sr;
+
+    am_hal_gpio_int_clear(rdy_mask);
+
+    /* Deassert slave select. */
+    hal_gpio_write(MYNEWT_VAL(BLE_HCI_EMSPI_SS_PIN), 1);
+
+    OS_ENTER_CRITICAL(sr);
+    hal_gpio_irq_enable(MYNEWT_VAL(BLE_HCI_EMSPI_RDY_PIN));
+
+    if (hal_gpio_read(MYNEWT_VAL(BLE_HCI_EMSPI_RDY_PIN))) {
+        am_hal_gpio_int_set(rdy_mask);
 
 Review comment:
   In defense of Chris, when we were discussing this I had mentioned that this would be one way to do it without realizing at the time that there was no generic way to set the interrupt pending flag (meaning no hal to do it). Given that the interrupt only posts an event I think your suggestion is a good one Andrzej; just post the event.
   
   I do not understand why hal_gpio_irq_enabled() would need to clear the interrupt before it is enabled however. There would be no harm in that interrupt firing off (waste some time maybe). Personally, (just my opinion), I would prefer a hal that did not automatically clear a pending interrupt flag in the enable routine. The hal already provides a way to clear the interrupt. I can see cases where you would not want hal_gpio_irq_enabled() to automatically clear a pending interrupt. I just feel it is more flexible to have separate routines for enabling interrupts and clearing them (seems a bit more flexible to me). This way the developer can do what they want (clear it or not prior to enabling).

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services