[PATCH v1] Bluetooth: btintel_pcie: Simplify MAC access request/release

[PATCH v1] Bluetooth: btintel_pcie: Simplify MAC access request/release

[Kiran K]

Drop the STOP_MAC_ACCESS_DIS and XTAL_CLK_REQ bit manipulations from
btintel_pcie_get_mac_access() and btintel_pcie_release_mac_access().
These bits are no longer required to be toggled by the host driver for
MAC access on the supported parts; the controller manages them
internally.

Also fix the idempotency check in btintel_pcie_get_mac_access(): only
assert MAC_ACCESS_REQ if it is not already set, instead of keying the
read-modify-write off the MAC_ACCESS_STS bit (which reflects grant,
not request, state).

Remove the now-unused STOP_MAC_ACCESS_DIS and XTAL_CLK_REQ register
defines from btintel_pcie.h.

Signed-off-by: Kiran K <kiran.k@intel.com>
---
 drivers/bluetooth/btintel_pcie.c | 10 +---------
 drivers/bluetooth/btintel_pcie.h |  3 ---
 2 files changed, 1 insertion(+), 12 deletions(-)

diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
index fda474406003..53a4ad80b871 100644
--- a/drivers/bluetooth/btintel_pcie.c
+++ b/drivers/bluetooth/btintel_pcie.c
@@ -594,9 +594,7 @@ static int btintel_pcie_get_mac_access(struct btintel_pcie_data *data)
 
 	reg = btintel_pcie_rd_reg32(data, BTINTEL_PCIE_CSR_FUNC_CTRL_REG);
 
-	reg |= BTINTEL_PCIE_CSR_FUNC_CTRL_STOP_MAC_ACCESS_DIS;
-	reg |= BTINTEL_PCIE_CSR_FUNC_CTRL_XTAL_CLK_REQ;
-	if ((reg & BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_STS) == 0)
+	if (!(reg & BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_REQ))
 		reg |= BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_REQ;
 
 	btintel_pcie_wr_reg32(data, BTINTEL_PCIE_CSR_FUNC_CTRL_REG, reg);
@@ -622,12 +620,6 @@ static void btintel_pcie_release_mac_access(struct btintel_pcie_data *data)
 	if (reg & BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_REQ)
 		reg &= ~BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_REQ;
 
-	if (reg & BTINTEL_PCIE_CSR_FUNC_CTRL_STOP_MAC_ACCESS_DIS)
-		reg &= ~BTINTEL_PCIE_CSR_FUNC_CTRL_STOP_MAC_ACCESS_DIS;
-
-	if (reg & BTINTEL_PCIE_CSR_FUNC_CTRL_XTAL_CLK_REQ)
-		reg &= ~BTINTEL_PCIE_CSR_FUNC_CTRL_XTAL_CLK_REQ;
-
 	btintel_pcie_wr_reg32(data, BTINTEL_PCIE_CSR_FUNC_CTRL_REG, reg);
 }
 
diff --git a/drivers/bluetooth/btintel_pcie.h b/drivers/bluetooth/btintel_pcie.h
index 2db85f71b2f8..7fc8c46ed689 100644
--- a/drivers/bluetooth/btintel_pcie.h
+++ b/drivers/bluetooth/btintel_pcie.h
@@ -34,9 +34,6 @@
 #define BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_STS	(BIT(20))
 
 #define BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_REQ	(BIT(21))
-/* Stop MAC Access disconnection request */
-#define BTINTEL_PCIE_CSR_FUNC_CTRL_STOP_MAC_ACCESS_DIS	(BIT(22))
-#define BTINTEL_PCIE_CSR_FUNC_CTRL_XTAL_CLK_REQ		(BIT(23))
 
 #define BTINTEL_PCIE_CSR_FUNC_CTRL_BUS_MASTER_STS	(BIT(28))
 #define BTINTEL_PCIE_CSR_FUNC_CTRL_BUS_MASTER_DISCON	(BIT(29))
-- 
2.53.0

RE: [v1] Bluetooth: btintel_pcie: Simplify MAC access request/release

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 882 bytes --]

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=1092797

---Test result---

Test Summary:
CheckPatch                    PASS      0.94 seconds
GitLint                       PASS      0.33 seconds
SubjectPrefix                 PASS      0.13 seconds
BuildKernel                   PASS      26.55 seconds
CheckAllWarning               PASS      29.35 seconds
CheckSparse                   PASS      27.82 seconds
BuildKernel32                 PASS      25.85 seconds
TestRunnerSetup               PASS      570.86 seconds
IncrementalBuild              PASS      25.18 seconds



https://github.com/bluez/bluetooth-next/pull/167

---
Regards,
Linux Bluetooth

Re: [PATCH v1] Bluetooth: btintel_pcie: Simplify MAC access request/release

[Paul Menzel]

Dear Kiran,


Thank yuo for your patch.

Am 11.05.26 um 15:39 schrieb Kiran K:
> Drop the STOP_MAC_ACCESS_DIS and XTAL_CLK_REQ bit manipulations from
> btintel_pcie_get_mac_access() and btintel_pcie_release_mac_access().
> These bits are no longer required to be toggled by the host driver for
> MAC access on the supported parts; the controller manages them
> internally.

Does the “no longer” refer to some controller firmware update? Please 
elaborate. (I personally prefer that as much as possible is done outside 
the firmware.) Also, maybe be explicit, and mention, if having the code 
has any practical downsides.

> Also fix the idempotency check in btintel_pcie_get_mac_access(): only
> assert MAC_ACCESS_REQ if it is not already set, instead of keying the
> read-modify-write off the MAC_ACCESS_STS bit (which reflects grant,
> not request, state).

*Also* is a good indicator to make something a separate commit. Any 
reason to not do it?

> Remove the now-unused STOP_MAC_ACCESS_DIS and XTAL_CLK_REQ register
> defines from btintel_pcie.h.
> 
> Signed-off-by: Kiran K <kiran.k@intel.com>

No Fixes: tag? At least the second issue sounds like something to backport.

> ---
>   drivers/bluetooth/btintel_pcie.c | 10 +---------
>   drivers/bluetooth/btintel_pcie.h |  3 ---
>   2 files changed, 1 insertion(+), 12 deletions(-)
> 
> diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
> index fda474406003..53a4ad80b871 100644
> --- a/drivers/bluetooth/btintel_pcie.c
> +++ b/drivers/bluetooth/btintel_pcie.c
> @@ -594,9 +594,7 @@ static int btintel_pcie_get_mac_access(struct btintel_pcie_data *data)
>   
>   	reg = btintel_pcie_rd_reg32(data, BTINTEL_PCIE_CSR_FUNC_CTRL_REG);
>   
> -	reg |= BTINTEL_PCIE_CSR_FUNC_CTRL_STOP_MAC_ACCESS_DIS;
> -	reg |= BTINTEL_PCIE_CSR_FUNC_CTRL_XTAL_CLK_REQ;
> -	if ((reg & BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_STS) == 0)
> +	if (!(reg & BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_REQ))
>   		reg |= BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_REQ;
>   
>   	btintel_pcie_wr_reg32(data, BTINTEL_PCIE_CSR_FUNC_CTRL_REG, reg);
> @@ -622,12 +620,6 @@ static void btintel_pcie_release_mac_access(struct btintel_pcie_data *data)
>   	if (reg & BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_REQ)
>   		reg &= ~BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_REQ;
>   
> -	if (reg & BTINTEL_PCIE_CSR_FUNC_CTRL_STOP_MAC_ACCESS_DIS)
> -		reg &= ~BTINTEL_PCIE_CSR_FUNC_CTRL_STOP_MAC_ACCESS_DIS;
> -
> -	if (reg & BTINTEL_PCIE_CSR_FUNC_CTRL_XTAL_CLK_REQ)
> -		reg &= ~BTINTEL_PCIE_CSR_FUNC_CTRL_XTAL_CLK_REQ;
> -
>   	btintel_pcie_wr_reg32(data, BTINTEL_PCIE_CSR_FUNC_CTRL_REG, reg);
>   }
>   
> diff --git a/drivers/bluetooth/btintel_pcie.h b/drivers/bluetooth/btintel_pcie.h
> index 2db85f71b2f8..7fc8c46ed689 100644
> --- a/drivers/bluetooth/btintel_pcie.h
> +++ b/drivers/bluetooth/btintel_pcie.h
> @@ -34,9 +34,6 @@
>   #define BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_STS	(BIT(20))
>   
>   #define BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_REQ	(BIT(21))
> -/* Stop MAC Access disconnection request */
> -#define BTINTEL_PCIE_CSR_FUNC_CTRL_STOP_MAC_ACCESS_DIS	(BIT(22))
> -#define BTINTEL_PCIE_CSR_FUNC_CTRL_XTAL_CLK_REQ		(BIT(23))
>   
>   #define BTINTEL_PCIE_CSR_FUNC_CTRL_BUS_MASTER_STS	(BIT(28))
>   #define BTINTEL_PCIE_CSR_FUNC_CTRL_BUS_MASTER_DISCON	(BIT(29))

The diff looks good.


Kind regards,

Paul

RE: [PATCH v1] Bluetooth: btintel_pcie: Simplify MAC access request/release

[K, Kiran]

Hi Paul,

Thanks for the comments.

>Subject: Re: [PATCH v1] Bluetooth: btintel_pcie: Simplify MAC access
>request/release
>
>Dear Kiran,
>
>
>Thank yuo for your patch.
>
>Am 11.05.26 um 15:39 schrieb Kiran K:
>> Drop the STOP_MAC_ACCESS_DIS and XTAL_CLK_REQ bit manipulations from
>> btintel_pcie_get_mac_access() and btintel_pcie_release_mac_access().
>> These bits are no longer required to be toggled by the host driver for
>> MAC access on the supported parts; the controller manages them
>> internally.
>
>Does the “no longer” refer to some controller firmware update? Please
>elaborate. (I personally prefer that as much as possible is done outside the
>firmware.) Also, maybe be explicit, and mention, if having the code has any
>practical downsides.

No, the “no longer” does not refer to any controller firmware update. The changes are relevant only to the driver. The driver now correctly programs a few registers before accessing the debug-related registers. Previously, it was programming these bits incorrectly, which prevented access to the debug registers during stress tests and caused device coredump failures.
>
>> Also fix the idempotency check in btintel_pcie_get_mac_access(): only
>> assert MAC_ACCESS_REQ if it is not already set, instead of keying the
>> read-modify-write off the MAC_ACCESS_STS bit (which reflects grant,
>> not request, state).
>
>*Also* is a good indicator to make something a separate commit. Any reason to
>not do it?
I kept the fix in the same patch because btintel_pcie_release_mac_access() is meant to undo what btintel_pcie_get_mac_access() does. Splitting them would break the pairing of get/release operations, so I felt it was logically one change.
>
>> Remove the now-unused STOP_MAC_ACCESS_DIS and XTAL_CLK_REQ register
>> defines from btintel_pcie.h.
>>
>> Signed-off-by: Kiran K <kiran.k@intel.com>
>
>No Fixes: tag? At least the second issue sounds like something to backport.
I will add appropriate Fixes: tags and send a v2.
>
>> ---
>>   drivers/bluetooth/btintel_pcie.c | 10 +---------
>>   drivers/bluetooth/btintel_pcie.h |  3 ---
>>   2 files changed, 1 insertion(+), 12 deletions(-)
>>
>> diff --git a/drivers/bluetooth/btintel_pcie.c
>> b/drivers/bluetooth/btintel_pcie.c
>> index fda474406003..53a4ad80b871 100644
>> --- a/drivers/bluetooth/btintel_pcie.c
>> +++ b/drivers/bluetooth/btintel_pcie.c
>> @@ -594,9 +594,7 @@ static int btintel_pcie_get_mac_access(struct
>> btintel_pcie_data *data)
>>
>>   	reg = btintel_pcie_rd_reg32(data, BTINTEL_PCIE_CSR_FUNC_CTRL_REG);
>>
>> -	reg |= BTINTEL_PCIE_CSR_FUNC_CTRL_STOP_MAC_ACCESS_DIS;
>> -	reg |= BTINTEL_PCIE_CSR_FUNC_CTRL_XTAL_CLK_REQ;
>> -	if ((reg & BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_STS) == 0)
>> +	if (!(reg & BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_REQ))
>>   		reg |= BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_REQ;
>>
>>   	btintel_pcie_wr_reg32(data, BTINTEL_PCIE_CSR_FUNC_CTRL_REG, reg);
>> @@ -622,12 +620,6 @@ static void btintel_pcie_release_mac_access(struct
>btintel_pcie_data *data)
>>   	if (reg & BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_REQ)
>>   		reg &= ~BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_REQ;
>>
>> -	if (reg & BTINTEL_PCIE_CSR_FUNC_CTRL_STOP_MAC_ACCESS_DIS)
>> -		reg &=
>~BTINTEL_PCIE_CSR_FUNC_CTRL_STOP_MAC_ACCESS_DIS;
>> -
>> -	if (reg & BTINTEL_PCIE_CSR_FUNC_CTRL_XTAL_CLK_REQ)
>> -		reg &= ~BTINTEL_PCIE_CSR_FUNC_CTRL_XTAL_CLK_REQ;
>> -
>>   	btintel_pcie_wr_reg32(data, BTINTEL_PCIE_CSR_FUNC_CTRL_REG, reg);
>>   }
>>
>> diff --git a/drivers/bluetooth/btintel_pcie.h
>> b/drivers/bluetooth/btintel_pcie.h
>> index 2db85f71b2f8..7fc8c46ed689 100644
>> --- a/drivers/bluetooth/btintel_pcie.h
>> +++ b/drivers/bluetooth/btintel_pcie.h
>> @@ -34,9 +34,6 @@
>>   #define BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_STS	(BIT(20))
>>
>>   #define BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_REQ	(BIT(21))
>> -/* Stop MAC Access disconnection request */
>> -#define BTINTEL_PCIE_CSR_FUNC_CTRL_STOP_MAC_ACCESS_DIS
>	(BIT(22))
>> -#define BTINTEL_PCIE_CSR_FUNC_CTRL_XTAL_CLK_REQ
>	(BIT(23))
>>
>>   #define BTINTEL_PCIE_CSR_FUNC_CTRL_BUS_MASTER_STS	(BIT(28))
>>   #define BTINTEL_PCIE_CSR_FUNC_CTRL_BUS_MASTER_DISCON
>	(BIT(29))
>
>The diff looks good.
>
>
>Kind regards,
>
>Paul

Thanks,
Kiran

Re: [PATCH v1] Bluetooth: btintel_pcie: Simplify MAC access request/release

[Luiz Augusto von Dentz]

Hi Kiran,

On Mon, May 11, 2026 at 9:26 AM Kiran K <kiran.k@intel.com> wrote:
>
> Drop the STOP_MAC_ACCESS_DIS and XTAL_CLK_REQ bit manipulations from
> btintel_pcie_get_mac_access() and btintel_pcie_release_mac_access().
> These bits are no longer required to be toggled by the host driver for
> MAC access on the supported parts; the controller manages them
> internally.
>
> Also fix the idempotency check in btintel_pcie_get_mac_access(): only
> assert MAC_ACCESS_REQ if it is not already set, instead of keying the
> read-modify-write off the MAC_ACCESS_STS bit (which reflects grant,
> not request, state).
>
> Remove the now-unused STOP_MAC_ACCESS_DIS and XTAL_CLK_REQ register
> defines from btintel_pcie.h.
>
> Signed-off-by: Kiran K <kiran.k@intel.com>
> ---
>  drivers/bluetooth/btintel_pcie.c | 10 +---------
>  drivers/bluetooth/btintel_pcie.h |  3 ---
>  2 files changed, 1 insertion(+), 12 deletions(-)
>
> diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
> index fda474406003..53a4ad80b871 100644
> --- a/drivers/bluetooth/btintel_pcie.c
> +++ b/drivers/bluetooth/btintel_pcie.c
> @@ -594,9 +594,7 @@ static int btintel_pcie_get_mac_access(struct btintel_pcie_data *data)
>
>         reg = btintel_pcie_rd_reg32(data, BTINTEL_PCIE_CSR_FUNC_CTRL_REG);
>
> -       reg |= BTINTEL_PCIE_CSR_FUNC_CTRL_STOP_MAC_ACCESS_DIS;
> -       reg |= BTINTEL_PCIE_CSR_FUNC_CTRL_XTAL_CLK_REQ;
> -       if ((reg & BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_STS) == 0)
> +       if (!(reg & BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_REQ))
>                 reg |= BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_REQ;
>
>         btintel_pcie_wr_reg32(data, BTINTEL_PCIE_CSR_FUNC_CTRL_REG, reg);
> @@ -622,12 +620,6 @@ static void btintel_pcie_release_mac_access(struct btintel_pcie_data *data)
>         if (reg & BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_REQ)
>                 reg &= ~BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_REQ;
>
> -       if (reg & BTINTEL_PCIE_CSR_FUNC_CTRL_STOP_MAC_ACCESS_DIS)
> -               reg &= ~BTINTEL_PCIE_CSR_FUNC_CTRL_STOP_MAC_ACCESS_DIS;
> -
> -       if (reg & BTINTEL_PCIE_CSR_FUNC_CTRL_XTAL_CLK_REQ)
> -               reg &= ~BTINTEL_PCIE_CSR_FUNC_CTRL_XTAL_CLK_REQ;
> -
>         btintel_pcie_wr_reg32(data, BTINTEL_PCIE_CSR_FUNC_CTRL_REG, reg);

Seem valid to me:

https://sashiko.dev/#/patchset/20260511133932.1217624-1-kiran.k%40intel.com

We could check; if no change is necessary, we can bail out early.

>  }
>
> diff --git a/drivers/bluetooth/btintel_pcie.h b/drivers/bluetooth/btintel_pcie.h
> index 2db85f71b2f8..7fc8c46ed689 100644
> --- a/drivers/bluetooth/btintel_pcie.h
> +++ b/drivers/bluetooth/btintel_pcie.h
> @@ -34,9 +34,6 @@
>  #define BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_STS      (BIT(20))
>
>  #define BTINTEL_PCIE_CSR_FUNC_CTRL_MAC_ACCESS_REQ      (BIT(21))
> -/* Stop MAC Access disconnection request */
> -#define BTINTEL_PCIE_CSR_FUNC_CTRL_STOP_MAC_ACCESS_DIS (BIT(22))
> -#define BTINTEL_PCIE_CSR_FUNC_CTRL_XTAL_CLK_REQ                (BIT(23))
>
>  #define BTINTEL_PCIE_CSR_FUNC_CTRL_BUS_MASTER_STS      (BIT(28))
>  #define BTINTEL_PCIE_CSR_FUNC_CTRL_BUS_MASTER_DISCON   (BIT(29))
> --
> 2.53.0
>
>


-- 
Luiz Augusto von Dentz