zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.45k stars 6.4k forks source link

Update H4 driver to add support of Low power mode for CYW43xxx #41970

Open npal-cy opened 2 years ago

npal-cy commented 2 years ago

Is your enhancement proposal related to a problem? Please describe. I am working on developing HCI extension driver for CYW43xxx. One feature which need to be added is Support low power mode for CYW43xx. The current version of H4 does not support this. There are two extra lines BT_DEV_WAKE and BT_HOST_WAKE:

  1. BT_DEV_WAKE - Bluetooth device wake-up signal: Signal from the host to the CYW4343X indicating that the host requires attention.
  2. BT_HOST_WAKE - Host wake-up signal. Signal from the CYW4343X to the host indicating that the CYW4343X requires attention.

I can handle BT_HOST_WAKE from CY43xxx HCI extension driver (set/release pm_constraint by BT_HOST_WAKE gpio event), but can't handle BT_DEV_WAKE. H4 driver should assert/deassert BT_DEV_WAKE before/after HCI UART TX operation.

Describe the solution you'd like one of possible solution can be adding callback function in bt_hci_driver structure and notify BT_HCI_EVENT_TRANSFER_DATA_START/ BT_HCI_EVENT_TRANSFER_DATA_COMPLETED events to callback, which will be implemented in Vendor HCI extension driver. Details below:

>> add bt_hci_driver_event in hci_driver.h

enum bt_hci_driver_event {

    /* This event indicates start of the HCI data transfer.
     *
     * This event does not return Event parameter.
     */
    BT_HCI_EVENT_TRANSFER_DATA_START       = 0,

    /* This event indicates completion of the HCI data transfer 
     *
     * This event does not return Event parameter.
     */ 
    BT_HCI_EVENT_TRANSFER_DATA_COMPLETED   = 1,

};

>> Add HCI callback function to bt_hci_driver structure.
>> default extension callback function will be locate in H4 as __weak (similar to bt_hci_transport_setup )
struct bt_hci_driver {
    ....
    /** 
     *  @brief HCI driver callback
     *
     * Vendors can use this callback to implement extansionfor BT HCI 
     * driver.  
     * 
     * @param event - HCI driver Event (refer to bt_hci_driver_event) 
     * 
     * @param event_param - Event parameter. It is pointer to void,
     *                      and should be casted to event-specific type.
     * 
     * Refer to bt_hci_driver_event enumeration for full list of 
     * events and detailed description each of them.
     *  
     * @return 0 on success, negative error value on failure
    int (*callback)(bt_hci_driver_event event, void* event_param);
};

>> update h4.c with indicates data transfer events to HCI extension driver: 

static int h4_send(struct net_buf *buf)
{
    BT_DBG("buf %p type %u len %u", buf, bt_buf_get_type(buf), buf->len);
    int err = 0;

#if defined(CONFIG_BT_HCI_EVENT_TRANSFER_DATA_NOTIFY)
    if (bt_dev.drv->callback) {
        err = bt_dev.drv->callback(BT_HCI_EVENT_TRANSFER_DATA_START, NULL);
        if (err) {
            return err;
        }
    }
#endif /* CONFIG_BT_HCI_EVENT_TRANSFER_DATA_NOTIFY */

    net_buf_put(&tx.fifo, buf);
    uart_irq_tx_enable(h4_dev);

    return err;
}

static inline void process_tx(void)
{
....
....
done:
    tx.type = H4_NONE;
    net_buf_unref(tx.buf);
    tx.buf = net_buf_get(&tx.fifo, K_NO_WAIT);
    if (!tx.buf) {
        uart_irq_tx_disable(h4_dev);
#if defined(CONFIG_BT_HCI_EVENT_TRANSFER_DATA_NOTIFY)
        if (bt_dev.drv->callback) {
            err = bt_dev.drv->callback(BT_HCI_EVENT_TRANSFER_DATA_COMPLETED, NULL);
            if (err) {
                return err;
            }
        }
#endif /* CONFIG_BT_HCI_EVENT_TRANSFER_DATA_NOTIFY */
...
}
zephyrbot commented 7 months ago

Hi @jhedberg,

This issue, marked as an Enhancement, was opened a while ago and did not get any traction. It was just assigned to you based on the labels. If you don't consider yourself the right person to address this issue, please re-assing it to the right person.

Please take a moment to review if the issue is still relevant to the project. If it is, please provide feedback and direction on how to move forward. If it is not, has already been addressed, is a duplicate, or is no longer relevant, please close it with a short comment explaining the reason.

@npal-cy you are also encouraged to help moving this issue forward by providing additional information and confirming this request/issue is still relevant to you.

Thanks!

ifyall commented 7 months ago

@npal-cy and I are still interested in this. I would like to hear from @jhedberg and the bluetooth subsystem team to understand if we are going the right direction or need to consider a different solution.

Ian

jhedberg commented 7 months ago

@ifyall @npal-cy have you considered just creating a custom driver for CYW43xxx? If code duplication is a concern, there's also #61221 which is intended to look into possibilities of factoring our common parts of H:4 handling (my initial feeling is that H:4 is so simple, that there's not much to be saved by trying to share code, but I guess we'll see if/when someone submits a draft PR).