diff mbox series

[next,v2,2/2] bluetooth: btnxpuart: implement powerup sequence

Message ID 20250227105718.1692639-2-catalin.popescu@leica-geosystems.com (mailing list archive)
State New
Headers show
Series [next,v2,1/2] dt-bindings: net: bluetooth: nxp: add support for supply and reset | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success
tedd_an/SubjectPrefix fail "Bluetooth: " prefix is not specified in the subject

Commit Message

POPESCU Catalin Feb. 27, 2025, 10:57 a.m. UTC 
NXP bluetooth chip shares power supply and reset gpio with a WLAN
chip. Add support for power supply and reset and enforce powerup
sequence:
- apply power supply
- deassert reset/powerdown

Signed-off-by: Catalin Popescu <catalin.popescu@leica-geosystems.com>
Reviewed-by: Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>
---
v2:
- rebase on linux-next tag next-20250227
- add reviewed-by
---
 drivers/bluetooth/btnxpuart.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

Comments

Loic Poulain March 6, 2025, 3:29 p.m. UTC | #1 
Hi,

On Thu, 27 Feb 2025 at 11:57, Catalin Popescu
<catalin.popescu@leica-geosystems.com> wrote:
>
> NXP bluetooth chip shares power supply and reset gpio with a WLAN
> chip. Add support for power supply and reset and enforce powerup
> sequence:
> - apply power supply
> - deassert reset/powerdown
>
> Signed-off-by: Catalin Popescu <catalin.popescu@leica-geosystems.com>
> Reviewed-by: Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>
> ---
> v2:
> - rebase on linux-next tag next-20250227
> - add reviewed-by
[...]
> @@ -1522,6 +1525,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)
> @@ -1549,6 +1553,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);

Maybe devm_reset_control_get_optional_shared_deasserted could be
useful here instead?




> +       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;
> +       }
> +
>         /* Initialize and register HCI device */
>         hdev = hci_alloc_dev();
>         if (!hdev) {
> @@ -1556,6 +1570,8 @@ static int nxp_serdev_probe(struct serdev_device *serdev)
>                 return -ENOMEM;
>         }
>
> +       reset_control_deassert(nxpdev->pdn);
> +
>         nxpdev->hdev = hdev;
>
>         hdev->bus = HCI_UART;
> @@ -1583,6 +1599,7 @@ static int nxp_serdev_probe(struct serdev_device *serdev)
>         return 0;
>
>  probe_fail:
> +       reset_control_assert(nxpdev->pdn);
>         hci_free_dev(hdev);
>         return -ENODEV;
>  }
> @@ -1609,6 +1626,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.43.0
>
POPESCU Catalin March 6, 2025, 3:56 p.m. UTC | #2 
On 06/03/2025 16:29, Loic Poulain wrote:
> This email is not from Hexagon’s Office 365 instance. Please be careful while clicking links, opening attachments, or replying to this email.
>
>
> Hi,
Hi Loic
>
> On Thu, 27 Feb 2025 at 11:57, Catalin Popescu
> <catalin.popescu@leica-geosystems.com> wrote:
>> NXP bluetooth chip shares power supply and reset gpio with a WLAN
>> chip. Add support for power supply and reset and enforce powerup
>> sequence:
>> - apply power supply
>> - deassert reset/powerdown
>>
>> Signed-off-by: Catalin Popescu <catalin.popescu@leica-geosystems.com>
>> Reviewed-by: Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>
>> ---
>> v2:
>> - rebase on linux-next tag next-20250227
>> - add reviewed-by
> [...]
>> @@ -1522,6 +1525,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)
>> @@ -1549,6 +1553,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);
> Maybe devm_reset_control_get_optional_shared_deasserted could be
> useful here instead?
This is a new API introduced in 6.13 and my kernel is not uptodate 
enough to test it (6.12).
Moreover, it would break the powerup sequence as we first need to apply 
power then deassert the reset.
>
>
>
>> +       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;
>> +       }
>> +
>>          /* Initialize and register HCI device */
>>          hdev = hci_alloc_dev();
>>          if (!hdev) {
>> @@ -1556,6 +1570,8 @@ static int nxp_serdev_probe(struct serdev_device *serdev)
>>                  return -ENOMEM;
>>          }
>>
>> +       reset_control_deassert(nxpdev->pdn);
>> +
>>          nxpdev->hdev = hdev;
>>
>>          hdev->bus = HCI_UART;
>> @@ -1583,6 +1599,7 @@ static int nxp_serdev_probe(struct serdev_device *serdev)
>>          return 0;
>>
>>   probe_fail:
>> +       reset_control_assert(nxpdev->pdn);
>>          hci_free_dev(hdev);
>>          return -ENODEV;
>>   }
>> @@ -1609,6 +1626,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.43.0
>>
diff mbox series

Patch

diff --git a/drivers/bluetooth/btnxpuart.c b/drivers/bluetooth/btnxpuart.c
index 4f2f429c9800..81d1c9b97146 100644
--- a/drivers/bluetooth/btnxpuart.c
+++ b/drivers/bluetooth/btnxpuart.c
@@ -17,6 +17,8 @@ 
 #include <linux/crc32.h>
 #include <linux/string_helpers.h>
 #include <linux/gpio/consumer.h>
+#include <linux/regulator/consumer.h>
+#include <linux/reset.h>
 
 #include <net/bluetooth/bluetooth.h>
 #include <net/bluetooth/hci_core.h>
@@ -191,6 +193,7 @@  struct btnxpuart_dev {
 
 	struct ps_data psdata;
 	struct btnxpuart_data *nxp_data;
+	struct reset_control *pdn;
 };
 
 #define NXP_V1_FW_REQ_PKT	0xa5
@@ -1522,6 +1525,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)
@@ -1549,6 +1553,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;
+	}
+
 	/* Initialize and register HCI device */
 	hdev = hci_alloc_dev();
 	if (!hdev) {
@@ -1556,6 +1570,8 @@  static int nxp_serdev_probe(struct serdev_device *serdev)
 		return -ENOMEM;
 	}
 
+	reset_control_deassert(nxpdev->pdn);
+
 	nxpdev->hdev = hdev;
 
 	hdev->bus = HCI_UART;
@@ -1583,6 +1599,7 @@  static int nxp_serdev_probe(struct serdev_device *serdev)
 	return 0;
 
 probe_fail:
+	reset_control_assert(nxpdev->pdn);
 	hci_free_dev(hdev);
 	return -ENODEV;
 }
@@ -1609,6 +1626,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);
 }