[PATCH v4 0/2] Bluetooth: btnxpuart: Add GPIO mechanism to

[PATCH v4 0/2] Bluetooth: btnxpuart: Add GPIO mechanism to

[Neeraj Sanjay Kale]

This patch series introduces a new optional device tree property
device-wakeup-gpios, which specifies the GPIO connected to BT_WAKE_IN pin of
the NXP chipset.

If this property is defined, the BTNXPUART driver uses this GPIO to put
the BT controller into sleep or wake it up, instead of the default UART
break mechanism.

The vendor command parameters HCI_NXP_WAKEUP_METHOD are configured to
use GPIO method instead of UART-break signal, to inform the chip about
this new mechanism.

Once power save feature is enabled, the driver puts the chip into power
save state by driving the GPIO high, and wakes it up by driving the GPIO
low.

Signed-off-by: Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>

Neeraj Sanjay Kale (2):
  dt-bindings: net: bluetooth: nxp: Add support for power save feature
    using GPIO
  Bluetooth: btnxpuart: Add GPIO support to power save feature

 .../net/bluetooth/nxp,88w8987-bt.yaml         |  8 +++
 drivers/bluetooth/btnxpuart.c                 | 57 +++++++++++++++++--
 2 files changed, 59 insertions(+), 6 deletions(-)

-- 
2.25.1

[PATCH v4 1/2] dt-bindings: net: bluetooth: nxp: Add support for power save feature using GPIO

[Neeraj Sanjay Kale]

This adds a new optional device tree property device-wakeup-gpios, which specifies
the GPIO connected to BT_WAKE_IN pin of the NXP chipset.

If this property is defined, the driver will use this GPIO for driving chip
into sleep/wakeup state, else use the UART break signal by default.

Signed-off-by: Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
---
v2: Rephrase description and use "-gpios". (Krzysztof, Rob)
v3: Use device-wakeup-gpios instead of h2c-ps-gpios. (Krzysztof)
v4: Add Reviewed-by (Rob)
---
 .../devicetree/bindings/net/bluetooth/nxp,88w8987-bt.yaml | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987-bt.yaml b/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987-bt.yaml
index 37a65badb448..0a2d7baf5db3 100644
--- a/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987-bt.yaml
+++ b/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987-bt.yaml
@@ -34,6 +34,12 @@ properties:
   firmware-name:
     maxItems: 1
 
+  device-wakeup-gpios:
+    maxItems: 1
+    description:
+      Host-To-Chip power save mechanism is driven by this GPIO
+      connected to BT_WAKE_IN pin of the NXP chipset.
+
 required:
   - compatible
 
@@ -41,10 +47,12 @@ additionalProperties: false
 
 examples:
   - |
+    #include <dt-bindings/gpio/gpio.h>
     serial {
         bluetooth {
             compatible = "nxp,88w8987-bt";
             fw-init-baudrate = <3000000>;
             firmware-name = "uartuart8987_bt_v0.bin";
+            device-wakeup-gpios = <&gpio 11 GPIO_ACTIVE_HIGH>;
         };
     };
-- 
2.25.1

[PATCH v4 2/2] Bluetooth: btnxpuart: Add GPIO support to power save feature

[Neeraj Sanjay Kale]

This adds support for driving the chip into sleep or wakeup with a GPIO.

If the device tree property device-wakeup-gpios is defined, the driver
utilizes this GPIO for controlling the chip's power save state, else it
uses the default UART-break method.

Signed-off-by: Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>
---
v3: Remove GPIO toggling in ps_init(). (Shenwei Wang)
v4: Use devm_gpiod_get_optional() instead of devm_gpiod_get()
---
 drivers/bluetooth/btnxpuart.c | 57 +++++++++++++++++++++++++++++++----
 1 file changed, 51 insertions(+), 6 deletions(-)

diff --git a/drivers/bluetooth/btnxpuart.c b/drivers/bluetooth/btnxpuart.c
index 2b8a07c745c9..83665f0b96c5 100644
--- a/drivers/bluetooth/btnxpuart.c
+++ b/drivers/bluetooth/btnxpuart.c
@@ -16,6 +16,7 @@
 #include <linux/crc8.h>
 #include <linux/crc32.h>
 #include <linux/string_helpers.h>
+#include <linux/gpio/consumer.h>
 
 #include <net/bluetooth/bluetooth.h>
 #include <net/bluetooth/hci_core.h>
@@ -82,6 +83,7 @@
 #define WAKEUP_METHOD_BREAK     1
 #define WAKEUP_METHOD_EXT_BREAK 2
 #define WAKEUP_METHOD_RTS       3
+#define WAKEUP_METHOD_GPIO      4
 #define WAKEUP_METHOD_INVALID   0xff
 
 /* power save mode status */
@@ -135,6 +137,7 @@ struct ps_data {
 	bool  driver_sent_cmd;
 	u16   h2c_ps_interval;
 	u16   c2h_ps_interval;
+	struct gpio_desc *h2c_ps_gpio;
 	struct hci_dev *hdev;
 	struct work_struct work;
 	struct timer_list ps_timer;
@@ -365,7 +368,7 @@ 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;
+	int status = 0;
 
 	if (psdata->ps_state == ps_state ||
 	    !test_bit(BTNXPUART_SERDEV_OPEN, &nxpdev->tx_state))
@@ -373,6 +376,14 @@ static void ps_control(struct hci_dev *hdev, u8 ps_state)
 
 	mutex_lock(&psdata->ps_lock);
 	switch (psdata->cur_h2c_wakeupmode) {
+	case WAKEUP_METHOD_GPIO:
+		if (ps_state == PS_STATE_AWAKE)
+			gpiod_set_value_cansleep(psdata->h2c_ps_gpio, 0);
+		else
+			gpiod_set_value_cansleep(psdata->h2c_ps_gpio, 1);
+		bt_dev_dbg(hdev, "Set h2c_ps_gpio: %s",
+			   str_high_low(ps_state == PS_STATE_SLEEP));
+		break;
 	case WAKEUP_METHOD_DTR:
 		if (ps_state == PS_STATE_AWAKE)
 			status = serdev_device_set_tiocm(nxpdev->serdev, TIOCM_DTR, 0);
@@ -422,15 +433,29 @@ static void ps_timeout_func(struct timer_list *t)
 	}
 }
 
-static void ps_setup(struct hci_dev *hdev)
+static int ps_setup(struct hci_dev *hdev)
 {
 	struct btnxpuart_dev *nxpdev = hci_get_drvdata(hdev);
+	struct serdev_device *serdev = nxpdev->serdev;
 	struct ps_data *psdata = &nxpdev->psdata;
 
+	psdata->h2c_ps_gpio = devm_gpiod_get_optional(&serdev->dev, "device-wakeup",
+						      GPIOD_OUT_LOW);
+	if (IS_ERR(psdata->h2c_ps_gpio)) {
+		bt_dev_err(hdev, "Error fetching device-wakeup-gpios: %ld"
+			   PTR_ERR(psdata->h2c_ps_gpio));
+		return PTR_ERR(psdata->h2c_ps_gpio);
+	}
+
+	if (!psdata->h2c_ps_gpio)
+		psdata->h2c_wakeup_gpio = 0xff;
+
 	psdata->hdev = hdev;
 	INIT_WORK(&psdata->work, ps_work_func);
 	mutex_init(&psdata->ps_lock);
 	timer_setup(&psdata->ps_timer, ps_timeout_func, 0);
+
+	return 0;
 }
 
 static bool ps_wakeup(struct btnxpuart_dev *nxpdev)
@@ -516,6 +541,9 @@ static int send_wakeup_method_cmd(struct hci_dev *hdev, void *data)
 	pcmd.c2h_wakeupmode = psdata->c2h_wakeupmode;
 	pcmd.c2h_wakeup_gpio = psdata->c2h_wakeup_gpio;
 	switch (psdata->h2c_wakeupmode) {
+	case WAKEUP_METHOD_GPIO:
+		pcmd.h2c_wakeupmode = BT_CTRL_WAKEUP_METHOD_GPIO;
+		break;
 	case WAKEUP_METHOD_DTR:
 		pcmd.h2c_wakeupmode = BT_CTRL_WAKEUP_METHOD_DSR;
 		break;
@@ -550,6 +578,7 @@ static void ps_init(struct hci_dev *hdev)
 {
 	struct btnxpuart_dev *nxpdev = hci_get_drvdata(hdev);
 	struct ps_data *psdata = &nxpdev->psdata;
+	u8 default_h2c_wakeup_mode = DEFAULT_H2C_WAKEUP_MODE;
 
 	serdev_device_set_tiocm(nxpdev->serdev, 0, TIOCM_RTS);
 	usleep_range(5000, 10000);
@@ -561,8 +590,17 @@ static void ps_init(struct hci_dev *hdev)
 	psdata->c2h_wakeup_gpio = 0xff;
 
 	psdata->cur_h2c_wakeupmode = WAKEUP_METHOD_INVALID;
+	if (psdata->h2c_ps_gpio)
+		default_h2c_wakeup_mode = WAKEUP_METHOD_GPIO;
+
 	psdata->h2c_ps_interval = PS_DEFAULT_TIMEOUT_PERIOD_MS;
-	switch (DEFAULT_H2C_WAKEUP_MODE) {
+
+	switch (default_h2c_wakeup_mode) {
+	case WAKEUP_METHOD_GPIO:
+		psdata->h2c_wakeupmode = WAKEUP_METHOD_GPIO;
+		gpiod_set_value_cansleep(psdata->h2c_ps_gpio, 0);
+		usleep_range(5000, 10000);
+		break;
 	case WAKEUP_METHOD_DTR:
 		psdata->h2c_wakeupmode = WAKEUP_METHOD_DTR;
 		serdev_device_set_tiocm(nxpdev->serdev, 0, TIOCM_DTR);
@@ -1279,6 +1317,9 @@ static int nxp_enqueue(struct hci_dev *hdev, struct sk_buff *skb)
 				psdata->c2h_wakeup_gpio = wakeup_parm.c2h_wakeup_gpio;
 				psdata->h2c_wakeup_gpio = wakeup_parm.h2c_wakeup_gpio;
 				switch (wakeup_parm.h2c_wakeupmode) {
+				case BT_CTRL_WAKEUP_METHOD_GPIO:
+					psdata->h2c_wakeupmode = WAKEUP_METHOD_GPIO;
+					break;
 				case BT_CTRL_WAKEUP_METHOD_DSR:
 					psdata->h2c_wakeupmode = WAKEUP_METHOD_DTR;
 					break;
@@ -1509,13 +1550,17 @@ 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");
-		hci_free_dev(hdev);
-		return -ENODEV;
+		goto probe_fail;
 	}
 
-	ps_setup(hdev);
+	if (ps_setup(hdev))
+		goto probe_fail;
 
 	return 0;
+
+probe_fail:
+	hci_free_dev(hdev);
+	return -ENODEV;
 }
 
 static void nxp_serdev_remove(struct serdev_device *serdev)
-- 
2.25.1

Re: [PATCH v4 2/2] Bluetooth: btnxpuart: Add GPIO support to power save feature

[kernel test robot]

Hi Neeraj,

kernel test robot noticed the following build errors:

[auto build test ERROR on bluetooth/master]
[also build test ERROR on bluetooth-next/master robh/for-next linus/master v6.12-rc2 next-20241004]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Neeraj-Sanjay-Kale/dt-bindings-net-bluetooth-nxp-Add-support-for-power-save-feature-using-GPIO/20241007-211315
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth.git master
patch link:    https://lore.kernel.org/r/20241007131316.626690-3-neeraj.sanjaykale%40nxp.com
patch subject: [PATCH v4 2/2] Bluetooth: btnxpuart: Add GPIO support to power save feature
config: x86_64-buildonly-randconfig-003-20241008 (https://download.01.org/0day-ci/archive/20241008/202410081424.BaGORAHw-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241008/202410081424.BaGORAHw-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410081424.BaGORAHw-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/bluetooth/btnxpuart.c:445:7: error: expected ')'
     445 |                            PTR_ERR(psdata->h2c_ps_gpio));
         |                            ^
   drivers/bluetooth/btnxpuart.c:444:3: note: to match this '('
     444 |                 bt_dev_err(hdev, "Error fetching device-wakeup-gpios: %ld"
         |                 ^
   include/net/bluetooth/bluetooth.h:278:2: note: expanded from macro 'bt_dev_err'
     278 |         BT_ERR("%s: " fmt, bt_dev_name(hdev), ##__VA_ARGS__)
         |         ^
   include/net/bluetooth/bluetooth.h:263:32: note: expanded from macro 'BT_ERR'
     263 | #define BT_ERR(fmt, ...)        bt_err(fmt "\n", ##__VA_ARGS__)
         |                                       ^
   1 error generated.


vim +445 drivers/bluetooth/btnxpuart.c

   434	
   435	static int ps_setup(struct hci_dev *hdev)
   436	{
   437		struct btnxpuart_dev *nxpdev = hci_get_drvdata(hdev);
   438		struct serdev_device *serdev = nxpdev->serdev;
   439		struct ps_data *psdata = &nxpdev->psdata;
   440	
   441		psdata->h2c_ps_gpio = devm_gpiod_get_optional(&serdev->dev, "device-wakeup",
   442							      GPIOD_OUT_LOW);
   443		if (IS_ERR(psdata->h2c_ps_gpio)) {
   444			bt_dev_err(hdev, "Error fetching device-wakeup-gpios: %ld"
 > 445				   PTR_ERR(psdata->h2c_ps_gpio));
   446			return PTR_ERR(psdata->h2c_ps_gpio);
   447		}
   448	
   449		if (!psdata->h2c_ps_gpio)
   450			psdata->h2c_wakeup_gpio = 0xff;
   451	
   452		psdata->hdev = hdev;
   453		INIT_WORK(&psdata->work, ps_work_func);
   454		mutex_init(&psdata->ps_lock);
   455		timer_setup(&psdata->ps_timer, ps_timeout_func, 0);
   456	
   457		return 0;
   458	}
   459	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v4 2/2] Bluetooth: btnxpuart: Add GPIO support to power save feature

[kernel test robot]

Hi Neeraj,

kernel test robot noticed the following build errors:

[auto build test ERROR on bluetooth/master]
[also build test ERROR on bluetooth-next/master robh/for-next linus/master v6.12-rc2 next-20241004]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Neeraj-Sanjay-Kale/dt-bindings-net-bluetooth-nxp-Add-support-for-power-save-feature-using-GPIO/20241007-211315
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth.git master
patch link:    https://lore.kernel.org/r/20241007131316.626690-3-neeraj.sanjaykale%40nxp.com
patch subject: [PATCH v4 2/2] Bluetooth: btnxpuart: Add GPIO support to power save feature
config: m68k-allmodconfig (https://download.01.org/0day-ci/archive/20241008/202410081516.Mt35mvJk-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241008/202410081516.Mt35mvJk-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410081516.Mt35mvJk-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

   In file included from drivers/bluetooth/btnxpuart.c:21:
   drivers/bluetooth/btnxpuart.c: In function 'ps_setup':
>> drivers/bluetooth/btnxpuart.c:445:28: error: expected ')' before 'PTR_ERR'
     445 |                            PTR_ERR(psdata->h2c_ps_gpio));
         |                            ^~~~~~~
   include/net/bluetooth/bluetooth.h:263:40: note: in definition of macro 'BT_ERR'
     263 | #define BT_ERR(fmt, ...)        bt_err(fmt "\n", ##__VA_ARGS__)
         |                                        ^~~
   drivers/bluetooth/btnxpuart.c:444:17: note: in expansion of macro 'bt_dev_err'
     444 |                 bt_dev_err(hdev, "Error fetching device-wakeup-gpios: %ld"
         |                 ^~~~~~~~~~
   include/net/bluetooth/bluetooth.h:263:39: note: to match this '('
     263 | #define BT_ERR(fmt, ...)        bt_err(fmt "\n", ##__VA_ARGS__)
         |                                       ^
   include/net/bluetooth/bluetooth.h:278:9: note: in expansion of macro 'BT_ERR'
     278 |         BT_ERR("%s: " fmt, bt_dev_name(hdev), ##__VA_ARGS__)
         |         ^~~~~~
   drivers/bluetooth/btnxpuart.c:444:17: note: in expansion of macro 'bt_dev_err'
     444 |                 bt_dev_err(hdev, "Error fetching device-wakeup-gpios: %ld"
         |                 ^~~~~~~~~~
>> include/net/bluetooth/bluetooth.h:278:16: warning: format '%s' expects a matching 'char *' argument [-Wformat=]
     278 |         BT_ERR("%s: " fmt, bt_dev_name(hdev), ##__VA_ARGS__)
         |                ^~~~~~
   include/net/bluetooth/bluetooth.h:263:40: note: in definition of macro 'BT_ERR'
     263 | #define BT_ERR(fmt, ...)        bt_err(fmt "\n", ##__VA_ARGS__)
         |                                        ^~~
   drivers/bluetooth/btnxpuart.c:444:17: note: in expansion of macro 'bt_dev_err'
     444 |                 bt_dev_err(hdev, "Error fetching device-wakeup-gpios: %ld"
         |                 ^~~~~~~~~~
   include/net/bluetooth/bluetooth.h:278:18: note: format string is defined here
     278 |         BT_ERR("%s: " fmt, bt_dev_name(hdev), ##__VA_ARGS__)
         |                 ~^
         |                  |
         |                  char *
>> include/net/bluetooth/bluetooth.h:278:16: warning: format '%ld' expects a matching 'long int' argument [-Wformat=]
     278 |         BT_ERR("%s: " fmt, bt_dev_name(hdev), ##__VA_ARGS__)
         |                ^~~~~~
   include/net/bluetooth/bluetooth.h:263:40: note: in definition of macro 'BT_ERR'
     263 | #define BT_ERR(fmt, ...)        bt_err(fmt "\n", ##__VA_ARGS__)
         |                                        ^~~
   drivers/bluetooth/btnxpuart.c:444:17: note: in expansion of macro 'bt_dev_err'
     444 |                 bt_dev_err(hdev, "Error fetching device-wakeup-gpios: %ld"
         |                 ^~~~~~~~~~

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for GET_FREE_REGION
   Depends on [n]: SPARSEMEM [=n]
   Selected by [m]:
   - RESOURCE_KUNIT_TEST [=m] && RUNTIME_TESTING_MENU [=y] && KUNIT [=m]


vim +445 drivers/bluetooth/btnxpuart.c

   434	
   435	static int ps_setup(struct hci_dev *hdev)
   436	{
   437		struct btnxpuart_dev *nxpdev = hci_get_drvdata(hdev);
   438		struct serdev_device *serdev = nxpdev->serdev;
   439		struct ps_data *psdata = &nxpdev->psdata;
   440	
   441		psdata->h2c_ps_gpio = devm_gpiod_get_optional(&serdev->dev, "device-wakeup",
   442							      GPIOD_OUT_LOW);
   443		if (IS_ERR(psdata->h2c_ps_gpio)) {
   444			bt_dev_err(hdev, "Error fetching device-wakeup-gpios: %ld"
 > 445				   PTR_ERR(psdata->h2c_ps_gpio));
   446			return PTR_ERR(psdata->h2c_ps_gpio);
   447		}
   448	
   449		if (!psdata->h2c_ps_gpio)
   450			psdata->h2c_wakeup_gpio = 0xff;
   451	
   452		psdata->hdev = hdev;
   453		INIT_WORK(&psdata->work, ps_work_func);
   454		mutex_init(&psdata->ps_lock);
   455		timer_setup(&psdata->ps_timer, ps_timeout_func, 0);
   456	
   457		return 0;
   458	}
   459	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki