RE: [PATCH 2/2] Bluetooth: btnxpuart: implement powerup sequence.

[Sherry Sun]

> -----Original Message-----
> From: Catalin Popescu <catalin.popescu@xxxxxxxxxxxxxxxxxxxx>
> Sent: Friday, October 4, 2024 7:36 PM
> To: Amitkumar Karwar <amitkumar.karwar@xxxxxxx>; Neeraj Sanjay Kale
> <neeraj.sanjaykale@xxxxxxx>; marcel@xxxxxxxxxxxx;
> luiz.dentz@xxxxxxxxx; robh@xxxxxxxxxx; krzk+dt@xxxxxxxxxx;
> conor+dt@xxxxxxxxxx; p.zabel@xxxxxxxxxxxxxx
> Cc: linux-bluetooth@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; m.felsch@xxxxxxxxxxxxxx; bsp-
> development.geo@xxxxxxxxxxxxxxxxxxxx; Catalin Popescu
> <catalin.popescu@xxxxxxxxxxxxxxxxxxxx>
> Subject: [PATCH 2/2] Bluetooth: btnxpuart: implement powerup sequence.
> 
> NXP bluetooth chip shares power supply and reset gpio with a WLAN chip.
> Add support for power supply and reset and enforce powerup
> sequence:
> 1. apply power supply
> 2. deassert reset/powerdown

Hi Catalin,

For NXP WIFI/BT chip, WIFI and BT share the one PDn pin, which means that both wifi and BT controller will be powered on and off at the same time. So you control this pin in the BT driver probe and remove function will directly break the wifi function I think.

> 
> Signed-off-by: Catalin Popescu <catalin.popescu@xxxxxxxxxxxxxxxxxxxx>
> ---
>  drivers/bluetooth/btnxpuart.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/bluetooth/btnxpuart.c b/drivers/bluetooth/btnxpuart.c
> index 4f493be763b8..e58e7ac7999f 100644
> --- a/drivers/bluetooth/btnxpuart.c
> +++ b/drivers/bluetooth/btnxpuart.c
> @@ -16,6 +16,8 @@
>  #include <linux/crc8.h>
>  #include <linux/crc32.h>
>  #include <linux/string_helpers.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/reset.h>
> 
>  #include <net/bluetooth/bluetooth.h>
>  #include <net/bluetooth/hci_core.h>
> @@ -188,6 +190,7 @@ struct btnxpuart_dev {
> 
>  	struct ps_data psdata;
>  	struct btnxpuart_data *nxp_data;
> +	struct reset_control *pdn;
>  };
> 
>  #define NXP_V1_FW_REQ_PKT	0xa5
> @@ -1458,6 +1461,7 @@ static int nxp_serdev_probe(struct serdev_device
> *serdev)  {
>  	struct hci_dev *hdev;
>  	struct btnxpuart_dev *nxpdev;
> +	int err;
> 
>  	nxpdev = devm_kzalloc(&serdev->dev, sizeof(*nxpdev), GFP_KERNEL);
>  	if (!nxpdev)
> @@ -1485,6 +1489,16 @@ static int nxp_serdev_probe(struct serdev_device
> *serdev)
> 
>  	crc8_populate_msb(crc8_table, POLYNOMIAL8);
> 
> +	nxpdev->pdn = devm_reset_control_get_optional_shared(&serdev-
> >dev, NULL);
> +	if (IS_ERR(nxpdev->pdn))
> +		return PTR_ERR(nxpdev->pdn);
> +
> +	err = devm_regulator_get_enable(&serdev->dev, "vcc");
> +	if (err) {
> +		dev_err(&serdev->dev, "Failed to enable vcc regulator\n");
> +		return err;
> +	}
> +

Now in NXP local repo, the PDn pin has been controlled by the corresponding wifi PCIe/SDIO controller, so we won't add the vcc-supply and reset-gpios properties for the BT driver, seems the code here will return err if no vcc-supply and reset-gpios properties provided, which will break the BT driver probe.

Best Regards
Sherry

>  	/* Initialize and register HCI device */
>  	hdev = hci_alloc_dev();
>  	if (!hdev) {
> @@ -1492,6 +1506,8 @@ static int nxp_serdev_probe(struct serdev_device
> *serdev)
>  		return -ENOMEM;
>  	}
> 
> +	reset_control_deassert(nxpdev->pdn);
> +
>  	nxpdev->hdev = hdev;
> 
>  	hdev->bus = HCI_UART;
> @@ -1509,6 +1525,7 @@ static int nxp_serdev_probe(struct serdev_device
> *serdev)
> 
>  	if (hci_register_dev(hdev) < 0) {
>  		dev_err(&serdev->dev, "Can't register HCI device\n");
> +		reset_control_assert(nxpdev->pdn);
>  		hci_free_dev(hdev);
>  		return -ENODEV;
>  	}
> @@ -1540,6 +1557,7 @@ static void nxp_serdev_remove(struct
> serdev_device *serdev)
>  	}
>  	ps_cleanup(nxpdev);
>  	hci_unregister_dev(hdev);
> +	reset_control_assert(nxpdev->pdn);
>  	hci_free_dev(hdev);
>  }
> 
> --
> 2.34.1
>