[PATCH] Bluetooth: hci_bcm: Fix empty regulator supplies for Intel Macs

[PATCH] Bluetooth: hci_bcm: Fix empty regulator supplies for Intel Macs

[Chen-Yu Tsai]

From: Chen-Yu Tsai <wens@csie.org>

The code path for Macs goes through bcm_apple_get_resources(), which
skips over the code that sets up the regulator supplies. As a result,
the call to regulator_bulk_enable() / regulator_bulk_disable() results
in a NULL pointer dereference.

This was reported on the kernel.org Bugzilla, bug 202963.

Unbreak Broadcom Bluetooth support on Intel Macs by checking if the
supplies were set up before enabling or disabling them.

The same does not need to be done for the clocks, as the common clock
framework API checks for NULL pointers.

Fixes: 75d11676dccb ("Bluetooth: hci_bcm: Add support for regulator supplies")
Cc: <stable@vger.kernel.org> # 5.0.x
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---

I do not own a Mac, so this needs to be tested by someone else.

---
 drivers/bluetooth/hci_bcm.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index ddbe518c3e5b..b5d31d583d60 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -228,9 +228,15 @@ static int bcm_gpio_set_power(struct bcm_device *dev, bool powered)
 	int err;
 
 	if (powered && !dev->res_enabled) {
-		err = regulator_bulk_enable(BCM_NUM_SUPPLIES, dev->supplies);
-		if (err)
-			return err;
+		/* Intel Macs use bcm_apple_get_resources() and don't
+		 * have regulator supplies configured.
+		 */
+		if (dev->supplies[0].supply) {
+			err = regulator_bulk_enable(BCM_NUM_SUPPLIES,
+						    dev->supplies);
+			if (err)
+				return err;
+		}
 
 		/* LPO clock needs to be 32.768 kHz */
 		err = clk_set_rate(dev->lpo_clk, 32768);
@@ -259,7 +265,13 @@ static int bcm_gpio_set_power(struct bcm_device *dev, bool powered)
 	if (!powered && dev->res_enabled) {
 		clk_disable_unprepare(dev->txco_clk);
 		clk_disable_unprepare(dev->lpo_clk);
-		regulator_bulk_disable(BCM_NUM_SUPPLIES, dev->supplies);
+
+		/* Intel Macs use bcm_apple_get_resources() and don't
+		 * have regulator supplies configured.
+		 */
+		if (dev->supplies[0].supply)
+			regulator_bulk_disable(BCM_NUM_SUPPLIES,
+					       dev->supplies);
 	}
 
 	/* wait for device to power on and come out of reset */
-- 
2.20.1

Re: [PATCH] Bluetooth: hci_bcm: Fix empty regulator supplies for Intel Macs

[Marcel Holtmann]

Hi Chen-Yu,

> The code path for Macs goes through bcm_apple_get_resources(), which
> skips over the code that sets up the regulator supplies. As a result,
> the call to regulator_bulk_enable() / regulator_bulk_disable() results
> in a NULL pointer dereference.
> 
> This was reported on the kernel.org Bugzilla, bug 202963.
> 
> Unbreak Broadcom Bluetooth support on Intel Macs by checking if the
> supplies were set up before enabling or disabling them.
> 
> The same does not need to be done for the clocks, as the common clock
> framework API checks for NULL pointers.
> 
> Fixes: 75d11676dccb ("Bluetooth: hci_bcm: Add support for regulator supplies")
> Cc: <stable@vger.kernel.org> # 5.0.x
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
> 
> I do not own a Mac, so this needs to be tested by someone else.
> 
> ---
> drivers/bluetooth/hci_bcm.c | 20 ++++++++++++++++----
> 1 file changed, 16 insertions(+), 4 deletions(-)

patch has been applied to bluetooth-next tree.

Regards

Marcel

Re: [PATCH] Bluetooth: hci_bcm: Fix empty regulator supplies for Intel Macs

[Dick Marinus]

This fixes the issue for hanging at modprobe intel-lpss-pci on my
macbook pro 13,3 2016.

Error in dmesg:

BUG: unable to handle kernel NULL pointer dereference at 0000000000000090

Thanks a lot!

Tested-by: Dick Marinus <dick@mrns.nl>
> From: Chen-Yu Tsai <wens@csie.org>
> 
> The code path for Macs goes through bcm_apple_get_resources(), which
> skips over the code that sets up the regulator supplies. As a result,
> the call to regulator_bulk_enable() / regulator_bulk_disable() results
> in a NULL pointer dereference.
> 
> This was reported on the kernel.org Bugzilla, bug 202963.
> 
> Unbreak Broadcom Bluetooth support on Intel Macs by checking if the
> supplies were set up before enabling or disabling them.
> 
> The same does not need to be done for the clocks, as the common clock
> framework API checks for NULL pointers.
> 
> Fixes: 75d11676dccb ("Bluetooth: hci_bcm: Add support for regulator supplies")
> Cc: <stable@vger.kernel.org> # 5.0.x
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
> 
> I do not own a Mac, so this needs to be tested by someone else.
> 
> ---
>  drivers/bluetooth/hci_bcm.c | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
> index ddbe518c3e5b..b5d31d583d60 100644
> --- a/drivers/bluetooth/hci_bcm.c
> +++ b/drivers/bluetooth/hci_bcm.c
> @@ -228,9 +228,15 @@ static int bcm_gpio_set_power(struct bcm_device *dev, bool powered)
>  	int err;
>  
>  	if (powered && !dev->res_enabled) {
> -		err = regulator_bulk_enable(BCM_NUM_SUPPLIES, dev->supplies);
> -		if (err)
> -			return err;
> +		/* Intel Macs use bcm_apple_get_resources() and don't
> +		 * have regulator supplies configured.
> +		 */
> +		if (dev->supplies[0].supply) {
> +			err = regulator_bulk_enable(BCM_NUM_SUPPLIES,
> +						    dev->supplies);
> +			if (err)
> +				return err;
> +		}
>  
>  		/* LPO clock needs to be 32.768 kHz */
>  		err = clk_set_rate(dev->lpo_clk, 32768);
> @@ -259,7 +265,13 @@ static int bcm_gpio_set_power(struct bcm_device *dev, bool powered)
>  	if (!powered && dev->res_enabled) {
>  		clk_disable_unprepare(dev->txco_clk);
>  		clk_disable_unprepare(dev->lpo_clk);
> -		regulator_bulk_disable(BCM_NUM_SUPPLIES, dev->supplies);
> +
> +		/* Intel Macs use bcm_apple_get_resources() and don't
> +		 * have regulator supplies configured.
> +		 */
> +		if (dev->supplies[0].supply)
> +			regulator_bulk_disable(BCM_NUM_SUPPLIES,
> +					       dev->supplies);
>  	}
>  
>  	/* wait for device to power on and come out of reset */
> -- 
> 2.20.1