[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f7a1c4de-1865-533e-c0cb-944bfdc19052@linux.intel.com>
Date: Tue, 7 Mar 2023 13:29:00 +0200 (EET)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Neeraj Sanjay Kale <neeraj.sanjaykale@....com>
cc: davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
pabeni@...hat.com, robh+dt@...nel.org,
krzysztof.kozlowski+dt@...aro.org, marcel@...tmann.org,
johan.hedberg@...il.com, luiz.dentz@...il.com,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Jiri Slaby <jirislaby@...nel.org>, alok.a.tiwari@...cle.com,
hdanton@...a.com, leon@...nel.org, Netdev <netdev@...r.kernel.org>,
devicetree@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>,
linux-bluetooth@...r.kernel.org,
linux-serial <linux-serial@...r.kernel.org>,
amitkumar.karwar@....com, rohit.fule@....com, sherry.sun@....com
Subject: Re: [PATCH v7 3/3] Bluetooth: NXP: Add protocol support for NXP
Bluetooth chipsets
On Mon, 6 Mar 2023, Neeraj Sanjay Kale wrote:
> This adds a driver based on serdev driver for the NXP BT serial protocol
> based on running H:4, which can enable the built-in Bluetooth device
> inside an NXP BT chip.
>
> This driver has Power Save feature that will put the chip into sleep state
> whenever there is no activity for 2000ms, and will be woken up when any
> activity is to be initiated over UART.
>
> This driver enables the power save feature by default by sending the vendor
> specific commands to the chip during setup.
>
> During setup, the driver checks if a FW is already running on the chip
> by waiting for the bootloader signature, and downloads device specific FW
> file into the chip over UART if bootloader signature is received..
>
> Signed-off-by: Neeraj Sanjay Kale <neeraj.sanjaykale@....com>
> ---
> v2: Removed conf file support and added static data for each chip based
> on compatibility devices mentioned in DT bindings. Handled potential
> memory leaks and null pointer dereference issues, simplified FW download
> feature, handled byte-order and few cosmetic changes. (Ilpo Järvinen,
> Alok Tiwari, Hillf Danton)
> v3: Added conf file support necessary to support different vendor modules,
> moved .h file contents to .c, cosmetic changes. (Luiz Augusto von Dentz,
> Rob Herring, Leon Romanovsky)
> v4: Removed conf file support, optimized driver data, add logic to select
> FW name based on chip signature (Greg KH, Ilpo Järvinen, Sherry Sun)
> v5: Replaced bt_dev_info() with bt_dev_dbg(), handled user-space cmd
> parsing in nxp_enqueue() in a better way. (Greg KH, Luiz Augusto von Dentz)
> v6: Add support for fw-init-baudrate parameter from device tree,
> modified logic to detect FW download is needed or FW is running. (Greg
> KH, Sherry Sun)
> v7: Renamed variables, improved FW download functions, include ps_data
> into btnxpuart_dev. (Ilpo Järvinen)
> ---
> MAINTAINERS | 1 +
> drivers/bluetooth/Kconfig | 11 +
> drivers/bluetooth/Makefile | 1 +
> drivers/bluetooth/btnxpuart.c | 1309 +++++++++++++++++++++++++++++++++
> 4 files changed, 1322 insertions(+)
> create mode 100644 drivers/bluetooth/btnxpuart.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 030ec6fe89df..fdb9b0788c89 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -22840,6 +22840,7 @@ M: Amitkumar Karwar <amitkumar.karwar@....com>
> M: Neeraj Kale <neeraj.sanjaykale@....com>
> S: Maintained
> F: Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987-bt.yaml
> +F: drivers/bluetooth/btnxpuart.c
>
> THE REST
> M: Linus Torvalds <torvalds@...ux-foundation.org>
> diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
> index 5a1a7bec3c42..359a4833e31f 100644
> --- a/drivers/bluetooth/Kconfig
> +++ b/drivers/bluetooth/Kconfig
> @@ -465,4 +465,15 @@ config BT_VIRTIO
> Say Y here to compile support for HCI over Virtio into the
> kernel or say M to compile as a module.
>
> +config BT_NXPUART
> + tristate "NXP protocol support"
> + depends on SERIAL_DEV_BUS
select CRC32 since you're using it now.
> + help
> + NXP is serial driver required for NXP Bluetooth
> + devices with UART interface.
> +
> + Say Y here to compile support for NXP Bluetooth UART device into
> + the kernel, or say M here to compile as a module (btnxpuart).
> +
> +
> endmenu
> +static void ps_control(struct hci_dev *hdev, u8 ps_state)
> +{
> + struct btnxpuart_dev *nxpdev = hci_get_drvdata(hdev);
> + struct ps_data *psdata = &nxpdev->psdata;
> + int status;
> +
> + if (psdata->ps_state == ps_state ||
> + !test_bit(BTNXPUART_SERDEV_OPEN, &nxpdev->tx_state))
> + return;
> +
> + switch (psdata->cur_h2c_wakeupmode) {
> + case WAKEUP_METHOD_DTR:
> + if (ps_state == PS_STATE_AWAKE)
> + status = serdev_device_set_tiocm(nxpdev->serdev, TIOCM_DTR, 0);
> + else
> + status = serdev_device_set_tiocm(nxpdev->serdev, 0, TIOCM_DTR);
> + break;
> + case WAKEUP_METHOD_BREAK:
> + default:
> + if (ps_state == PS_STATE_AWAKE)
> + status = serdev_device_break_ctl(nxpdev->serdev, 0);
> + else
> + status = serdev_device_break_ctl(nxpdev->serdev, -1);
> + bt_dev_dbg(hdev, "Set UART break: %s, status=%d",
> + str_on_off(ps_state == PS_STATE_SLEEP), status);
Add the #include for str_on_off too.
> +/* for legacy chipsets with V1 bootloader */
> +static int nxp_recv_fw_req_v1(struct hci_dev *hdev, struct sk_buff *skb)
> +{
> + struct btnxpuart_dev *nxpdev = hci_get_drvdata(hdev);
> + struct btnxpuart_data *nxp_data = nxpdev->nxp_data;
> + struct v1_data_req *req;
> + u32 requested_len;
> +
> + if (test_bit(BTNXPUART_CHECK_BOOT_SIGNATURE, &nxpdev->tx_state)) {
> + clear_bit(BTNXPUART_CHECK_BOOT_SIGNATURE, &nxpdev->tx_state);
> + wake_up_interruptible(&nxpdev->check_boot_sign_wait_q);
> + goto ret;
> + }
> +
> + if (!is_fw_downloading(nxpdev))
> + goto ret;
That BTNXPUART_CHECK_BOOT_SIGNATURE check above is also the same in 3
callsites of is_fw_downloading() so too should be moved into a common
helper (there was 4th call into is_fw_downloading() so make another
help for these 3 users).
--
i.
Powered by blists - more mailing lists