2023-03-16 00:15:34

by Kiran K

[permalink] [raw]
Subject: [PATCH v2 1/2] Bluetooth: btintel: Add support to reset bluetooth via ACPI DSM

New Intel platforms supports reset of Bluetooth device via ACPI DSM
methods. The legacy reset mechanism via GPIO will be deprecated in
future. This patch checks the platform support for reset methods and if
supported uses the same instead of legacy GPIO toggling method.

ACPI firmware supports two types of reset method based on NIC card.
(Discrete or Integrated).

1. VSEC Type - Vendor Specific Extended Capability. Here BT_EN and
BT_IF_SELECT lines are driven by a register in PCH cluster. This
interface is supported on discrete BT solution.

2. WDISABLE2 - In this soluton, W_DISABLE2 pin in M.2 is connected to
physical GPIO from PCH. The DSM interface shall toggle this to recover
from error.

Signed-off-by: Kiran K <[email protected]>
Reported-by: kernel test robot <[email protected]>
---
changes in v2:
Fix compilation error when compiled for ARCH=arc
Details here:
https://patchwork.kernel.org/project/bluetooth/patch/[email protected]/

drivers/bluetooth/btintel.c | 121 ++++++++++++++++++++++++++++++++++++
drivers/bluetooth/btintel.h | 2 +
drivers/bluetooth/btusb.c | 16 +++++
3 files changed, 139 insertions(+)

diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index af774688f1c0..c4b02ac42106 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -10,6 +10,7 @@
#include <linux/firmware.h>
#include <linux/regmap.h>
#include <linux/acpi.h>
+#include <acpi/acpi_bus.h>
#include <asm/unaligned.h>

#include <net/bluetooth/bluetooth.h>
@@ -43,6 +44,15 @@ struct cmd_write_boot_params {
u8 fw_build_yy;
} __packed;

+enum {
+ DSM_SET_WDISABLE2_DELAY = 1,
+ DSM_SET_RESET_METHOD = 3,
+};
+
+static const guid_t btintel_guid_dsm =
+ GUID_INIT(0xaa10f4e0, 0x81ac, 0x4233,
+ 0xab, 0xf6, 0x3b, 0x2a, 0xc5, 0x0e, 0x28, 0xd9);
+
int btintel_check_bdaddr(struct hci_dev *hdev)
{
struct hci_rp_read_bd_addr *bda;
@@ -2379,6 +2389,116 @@ static void btintel_set_ppag(struct hci_dev *hdev, struct intel_version_tlv *ver
kfree_skb(skb);
}

+static int btintel_acpi_reset_method(struct hci_dev *hdev)
+{
+ int ret = 0;
+ acpi_status status;
+ union acpi_object *p, *ref;
+ struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
+
+ status = acpi_evaluate_object(ACPI_HANDLE(GET_HCIDEV_DEV(hdev)), "_PRR", NULL, &buffer);
+ if (ACPI_FAILURE(status)) {
+ bt_dev_err(hdev, "Failed to run _PRR method");
+ ret = -ENODEV;
+ return ret;
+ }
+ p = buffer.pointer;
+
+ if (p->package.count != 1 || p->type != ACPI_TYPE_PACKAGE) {
+ bt_dev_err(hdev, "Invalid arguments");
+ ret = -EINVAL;
+ goto exit_on_error;
+ }
+
+ ref = &p->package.elements[0];
+ if (ref->type != ACPI_TYPE_LOCAL_REFERENCE) {
+ bt_dev_err(hdev, "Invalid object type: 0x%x", ref->type);
+ ret = -EINVAL;
+ goto exit_on_error;
+ }
+
+ status = acpi_evaluate_object(ref->reference.handle, "_RST", NULL, NULL);
+ if (ACPI_FAILURE(status)) {
+ bt_dev_err(hdev, "Failed to run_RST method");
+ ret = -ENODEV;
+ goto exit_on_error;
+ }
+
+exit_on_error:
+ kfree(buffer.pointer);
+ return ret;
+}
+
+static void btintel_set_dsm_reset_method(struct hci_dev *hdev,
+ struct intel_version_tlv *ver_tlv)
+{
+ struct btintel_data *data = hci_get_priv(hdev);
+ acpi_handle handle = ACPI_HANDLE(GET_HCIDEV_DEV(hdev));
+ u8 reset_payload[4] = {0x01, 0x00, 0x01, 0x00};
+ union acpi_object *obj, argv4;
+ enum {
+ RESET_TYPE_WDISABLE2,
+ RESET_TYPE_VSEC
+ };
+
+ handle = ACPI_HANDLE(GET_HCIDEV_DEV(hdev));
+
+ if (!handle) {
+ bt_dev_dbg(hdev, "No support for bluetooth device in ACPI firmware");
+ return;
+ }
+
+ if (!acpi_has_method(handle, "_PRR")) {
+ bt_dev_err(hdev, "No support for _PRR ACPI method");
+ return;
+ }
+
+ switch (ver_tlv->cnvi_top & 0xfff) {
+ case 0x910: /* GalePeak2 */
+ reset_payload[2] = RESET_TYPE_VSEC;
+ break;
+ default:
+ /* WDISABLE2 is the default reset method */
+ reset_payload[2] = RESET_TYPE_WDISABLE2;
+
+ if (!acpi_check_dsm(handle, &btintel_guid_dsm, 0,
+ BIT(DSM_SET_WDISABLE2_DELAY))) {
+ bt_dev_err(hdev, "No dsm support to set reset delay");
+ return;
+ }
+ argv4.integer.type = ACPI_TYPE_INTEGER;
+ /* delay required to toggle BT power */
+ argv4.integer.value = 160;
+ obj = acpi_evaluate_dsm(handle, &btintel_guid_dsm, 0,
+ DSM_SET_WDISABLE2_DELAY, &argv4);
+ if (!obj) {
+ bt_dev_err(hdev, "Failed to call dsm to set reset delay");
+ return;
+ }
+ ACPI_FREE(obj);
+ }
+
+ bt_dev_info(hdev, "DSM reset method type: 0x%02x", reset_payload[2]);
+
+ if (!acpi_check_dsm(handle, &btintel_guid_dsm, 0,
+ DSM_SET_RESET_METHOD)) {
+ bt_dev_warn(hdev, "No support for dsm to set reset method");
+ return;
+ }
+ argv4.buffer.type = ACPI_TYPE_BUFFER;
+ argv4.buffer.length = sizeof(reset_payload);
+ argv4.buffer.pointer = reset_payload;
+
+ obj = acpi_evaluate_dsm(handle, &btintel_guid_dsm, 0,
+ DSM_SET_RESET_METHOD, &argv4);
+ if (!obj) {
+ bt_dev_err(hdev, "Failed to call dsm to set reset method");
+ return;
+ }
+ ACPI_FREE(obj);
+ data->acpi_reset_method = btintel_acpi_reset_method;
+}
+
static int btintel_bootloader_setup_tlv(struct hci_dev *hdev,
struct intel_version_tlv *ver)
{
@@ -2691,6 +2811,7 @@ static int btintel_setup_combined(struct hci_dev *hdev)
/* Setup MSFT Extension support */
btintel_set_msft_opcode(hdev,
INTEL_HW_VARIANT(ver_tlv.cnvi_bt));
+ btintel_set_dsm_reset_method(hdev, &ver_tlv);

err = btintel_bootloader_setup_tlv(hdev, &ver_tlv);
break;
diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
index 8fdb65b66315..8e7c51a78327 100644
--- a/drivers/bluetooth/btintel.h
+++ b/drivers/bluetooth/btintel.h
@@ -159,12 +159,14 @@ enum {
INTEL_BROKEN_SHUTDOWN_LED,
INTEL_ROM_LEGACY,
INTEL_ROM_LEGACY_NO_WBS_SUPPORT,
+ INTEL_ACPI_RESET_ACTIVE,

__INTEL_NUM_FLAGS,
};

struct btintel_data {
DECLARE_BITMAP(flags, __INTEL_NUM_FLAGS);
+ int (*acpi_reset_method)(struct hci_dev *hdev);
};

#define btintel_set_flag(hdev, nr) \
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 7382b021f3df..bfee12b6ab57 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -838,10 +838,26 @@ static void btusb_intel_cmd_timeout(struct hci_dev *hdev)
{
struct btusb_data *data = hci_get_drvdata(hdev);
struct gpio_desc *reset_gpio = data->reset_gpio;
+ struct btintel_data *intel_data = hci_get_priv(hdev);

if (++data->cmd_timeout_cnt < 5)
return;

+ if (intel_data->acpi_reset_method) {
+ if (test_and_set_bit(INTEL_ACPI_RESET_ACTIVE, intel_data->flags)) {
+ bt_dev_err(hdev, "acpi: last reset failed ? Not resetting again");
+ return;
+ }
+
+ bt_dev_err(hdev, "Initiating acpi reset method");
+ /* If ACPI reset method fails, lets try with legacy GPIO
+ * toggling
+ */
+ if (!intel_data->acpi_reset_method(hdev)) {
+ return;
+ }
+ }
+
if (!reset_gpio) {
btusb_reset(hdev);
return;
--
2.17.1



2023-03-16 00:15:35

by Kiran K

[permalink] [raw]
Subject: [PATCH v2 2/2] ACPI: utils: acpi_evaluate_dsm_typed - fix redefinition error

acpi_evaluate_dsm_typed function needs to be gaurded with CONFIG_ACPI to
avoid redefintion error when stub is also enabled.

In file included from ../drivers/bluetooth/btintel.c:13:
../include/acpi/acpi_bus.h:57:1: error: redefinition of 'acpi_evaluate_dsm_typed'
57 | acpi_evaluate_dsm_typed(acpi_handle handle, const guid_t *guid,..
| ^~~~~~~~~~~~~~~~~~~~~~~
In file included from ../drivers/bluetooth/btintel.c:12:
../include/linux/acpi.h:967:34: note: previous definition of
'acpi_evaluate_dsm_typed' with type 'union acpi_object *(void *,
const guid_t *, u64, u64, union acpi_object *, acpi_object_type)'
{aka 'union acpi_object *(void *, const guid_t *, long long unsigned int,
long long unsigned int, union acpi_object *, unsigned int)'}
967 | static inline union acpi_object
*acpi_evaluate_dsm_typed(acpi_handle handle,

Fixes: 1b94ad7ccc21 ("ACPI: utils: Add acpi_evaluate_dsm_typed() and acpi_check_dsm() stubs")
Signed-off-by: Kiran K <[email protected]>
---

changes:
Fix compilation error when compiled for ARCH=arc
Details here:
https://patchwork.kernel.org/project/bluetooth/patch/[email protected]/


include/acpi/acpi_bus.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index e44be31115a6..fc131b4aee4e 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -52,7 +52,7 @@ bool acpi_dock_match(acpi_handle handle);
bool acpi_check_dsm(acpi_handle handle, const guid_t *guid, u64 rev, u64 funcs);
union acpi_object *acpi_evaluate_dsm(acpi_handle handle, const guid_t *guid,
u64 rev, u64 func, union acpi_object *argv4);
-
+#ifdef CONFIG_ACPI
static inline union acpi_object *
acpi_evaluate_dsm_typed(acpi_handle handle, const guid_t *guid, u64 rev,
u64 func, union acpi_object *argv4,
@@ -68,6 +68,7 @@ acpi_evaluate_dsm_typed(acpi_handle handle, const guid_t *guid, u64 rev,

return obj;
}
+#endif

#define ACPI_INIT_DSM_ARGV4(cnt, eles) \
{ \
--
2.17.1


2023-03-16 01:18:43

by bluez.test.bot

[permalink] [raw]
Subject: RE: [v2,1/2] Bluetooth: btintel: Add support to reset bluetooth via ACPI DSM

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=730552

---Test result---

Test Summary:
CheckPatch PASS 1.70 seconds
GitLint FAIL 0.87 seconds
SubjectPrefix FAIL 0.68 seconds
BuildKernel PASS 32.89 seconds
CheckAllWarning PASS 35.15 seconds
CheckSparse PASS 40.32 seconds
CheckSmatch PASS 108.87 seconds
BuildKernel32 PASS 31.38 seconds
TestRunnerSetup PASS 451.42 seconds
TestRunner_l2cap-tester PASS 17.34 seconds
TestRunner_iso-tester PASS 17.89 seconds
TestRunner_bnep-tester PASS 5.66 seconds
TestRunner_mgmt-tester PASS 113.92 seconds
TestRunner_rfcomm-tester PASS 9.12 seconds
TestRunner_sco-tester PASS 8.46 seconds
TestRunner_ioctl-tester PASS 9.87 seconds
TestRunner_mesh-tester PASS 7.34 seconds
TestRunner_smp-tester PASS 8.30 seconds
TestRunner_userchan-tester PASS 6.07 seconds
IncrementalBuild PASS 35.80 seconds

Details
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[v2,1/2] Bluetooth: btintel: Add support to reset bluetooth via ACPI DSM

WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
25: B1 Line exceeds max length (94>80): "https://patchwork.kernel.org/project/bluetooth/patch/[email protected]/"
[v2,2/2] ACPI: utils: acpi_evaluate_dsm_typed - fix redefinition error

WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
7: B1 Line exceeds max length (81>80): "../include/acpi/acpi_bus.h:57:1: error: redefinition of 'acpi_evaluate_dsm_typed'"
25: B1 Line exceeds max length (94>80): "https://patchwork.kernel.org/project/bluetooth/patch/[email protected]/"
##############################
Test: SubjectPrefix - FAIL
Desc: Check subject contains "Bluetooth" prefix
Output:
"Bluetooth: " prefix is not specified in the subject


---
Regards,
Linux Bluetooth

2023-03-16 05:22:27

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] ACPI: utils: acpi_evaluate_dsm_typed - fix redefinition error

Hi Kiran,

On Wed, Mar 15, 2023 at 5:32 PM Kiran K <[email protected]> wrote:
>
> acpi_evaluate_dsm_typed function needs to be gaurded with CONFIG_ACPI to
> avoid redefintion error when stub is also enabled.
>
> In file included from ../drivers/bluetooth/btintel.c:13:
> ../include/acpi/acpi_bus.h:57:1: error: redefinition of 'acpi_evaluate_dsm_typed'
> 57 | acpi_evaluate_dsm_typed(acpi_handle handle, const guid_t *guid,..
> | ^~~~~~~~~~~~~~~~~~~~~~~
> In file included from ../drivers/bluetooth/btintel.c:12:
> ../include/linux/acpi.h:967:34: note: previous definition of
> 'acpi_evaluate_dsm_typed' with type 'union acpi_object *(void *,
> const guid_t *, u64, u64, union acpi_object *, acpi_object_type)'
> {aka 'union acpi_object *(void *, const guid_t *, long long unsigned int,
> long long unsigned int, union acpi_object *, unsigned int)'}
> 967 | static inline union acpi_object
> *acpi_evaluate_dsm_typed(acpi_handle handle,
>
> Fixes: 1b94ad7ccc21 ("ACPI: utils: Add acpi_evaluate_dsm_typed() and acpi_check_dsm() stubs")

This should probably be sent to the ACPI list/maintainer, but first
make sure there isn't already a fix which would mean we have to update
bluetooth-next to stop this kind of error.

> Signed-off-by: Kiran K <[email protected]>
> ---
>
> changes:
> Fix compilation error when compiled for ARCH=arc
> Details here:
> https://patchwork.kernel.org/project/bluetooth/patch/[email protected]/
>
>
> include/acpi/acpi_bus.h | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index e44be31115a6..fc131b4aee4e 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -52,7 +52,7 @@ bool acpi_dock_match(acpi_handle handle);
> bool acpi_check_dsm(acpi_handle handle, const guid_t *guid, u64 rev, u64 funcs);
> union acpi_object *acpi_evaluate_dsm(acpi_handle handle, const guid_t *guid,
> u64 rev, u64 func, union acpi_object *argv4);
> -
> +#ifdef CONFIG_ACPI
> static inline union acpi_object *
> acpi_evaluate_dsm_typed(acpi_handle handle, const guid_t *guid, u64 rev,
> u64 func, union acpi_object *argv4,
> @@ -68,6 +68,7 @@ acpi_evaluate_dsm_typed(acpi_handle handle, const guid_t *guid, u64 rev,
>
> return obj;
> }
> +#endif
>
> #define ACPI_INIT_DSM_ARGV4(cnt, eles) \
> { \
> --
> 2.17.1
>


--
Luiz Augusto von Dentz

2023-03-16 07:29:14

by Kiran K

[permalink] [raw]
Subject: RE: [PATCH v2 2/2] ACPI: utils: acpi_evaluate_dsm_typed - fix redefinition error

Hi Luiz,

> -----Original Message-----
> From: Luiz Augusto von Dentz <[email protected]>
> Sent: Thursday, March 16, 2023 10:52 AM
> To: K, Kiran <[email protected]>
> Cc: [email protected]; Srivatsa, Ravishankar
> <[email protected]>; Tumkur Narayan, Chethan
> <[email protected]>
> Subject: Re: [PATCH v2 2/2] ACPI: utils: acpi_evaluate_dsm_typed - fix
> redefinition error
>
> Hi Kiran,
>
> On Wed, Mar 15, 2023 at 5:32 PM Kiran K <[email protected]> wrote:
> >
> > acpi_evaluate_dsm_typed function needs to be gaurded with CONFIG_ACPI
> > to avoid redefintion error when stub is also enabled.
> >
> > In file included from ../drivers/bluetooth/btintel.c:13:
> > ../include/acpi/acpi_bus.h:57:1: error: redefinition of
> 'acpi_evaluate_dsm_typed'
> > 57 | acpi_evaluate_dsm_typed(acpi_handle handle, const guid_t *guid,..
> > | ^~~~~~~~~~~~~~~~~~~~~~~
> > In file included from ../drivers/bluetooth/btintel.c:12:
> > ../include/linux/acpi.h:967:34: note: previous definition of
> > 'acpi_evaluate_dsm_typed' with type 'union acpi_object *(void *, const
> > guid_t *, u64, u64, union acpi_object *, acpi_object_type)'
> > {aka 'union acpi_object *(void *, const guid_t *, long long unsigned
> > int, long long unsigned int, union acpi_object *, unsigned int)'}
> > 967 | static inline union acpi_object
> > *acpi_evaluate_dsm_typed(acpi_handle handle,
> >
> > Fixes: 1b94ad7ccc21 ("ACPI: utils: Add acpi_evaluate_dsm_typed() and
> > acpi_check_dsm() stubs")
>
> This should probably be sent to the ACPI list/maintainer, but first make sure
> there isn't already a fix which would mean we have to update bluetooth-next
> to stop this kind of error.
>
Ok. It looks this issue is present in linux-pm tree. I have pushed the patch over there. I will let you know once its merged so that you can rebase bluetooth-next. Thanks

Regards,
Kiran