linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] Bluetooth: hci_bcm: Additional changes for BCM4354 support
@ 2019-11-06  0:29 Abhishek Pandit-Subedi
  2019-11-06  0:29 ` [PATCH 1/4] Bluetooth: hci_bcm: Disallow set_baudrate for BCM4354 Abhishek Pandit-Subedi
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Abhishek Pandit-Subedi @ 2019-11-06  0:29 UTC (permalink / raw)
  To: Marcel Holtmann, Johan Hedberg
  Cc: linux-bluetooth, Abhishek Pandit-Subedi, devicetree,
	David S. Miller, netdev, linux-kernel, Rob Herring, Ondrej Jirman,
	Mark Rutland, Chen-Yu Tsai


While adding support for the BCM4354, I discovered a few more things
that weren't working as they should have.

First, we disallow serdev from setting the baudrate on BCM4354. Serdev
sets the oper_speed first before calling hu->setup() in
hci_uart_setup(). On the BCM4354, this results in bcm_setup() failing
when the hci reset times out.

Next, we add support for setting the PCM parameters, which consists of
a pair of vendor specific opcodes to set the pcm parameters. The
documentation for these params are available in the brcm_patchram_plus
package (i.e. https://github.com/balena-os/brcm_patchram_plus). This is
necessary for PCM to work properly.

All changes were tested with rk3288-veyron-minnie.dts.



Abhishek Pandit-Subedi (4):
  Bluetooth: hci_bcm: Disallow set_baudrate for BCM4354
  Bluetooth: btbcm: Support pcm configuration
  Bluetooth: hci_bcm: Support pcm params in dts
  dt-bindings: net: bluetooth: update broadcom-bluetooth

 .../bindings/net/broadcom-bluetooth.txt       |  4 ++
 drivers/bluetooth/btbcm.c                     | 27 ++++++++++
 drivers/bluetooth/btbcm.h                     | 12 +++++
 drivers/bluetooth/hci_bcm.c                   | 52 ++++++++++++++++++-
 4 files changed, 94 insertions(+), 1 deletion(-)

-- 
2.24.0.rc1.363.gb1bccd3e3d-goog


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/4] Bluetooth: hci_bcm: Disallow set_baudrate for BCM4354
  2019-11-06  0:29 [PATCH 0/4] Bluetooth: hci_bcm: Additional changes for BCM4354 support Abhishek Pandit-Subedi
@ 2019-11-06  0:29 ` Abhishek Pandit-Subedi
  2019-11-06  0:29 ` [PATCH 2/4] Bluetooth: btbcm: Support pcm configuration Abhishek Pandit-Subedi
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 10+ messages in thread
From: Abhishek Pandit-Subedi @ 2019-11-06  0:29 UTC (permalink / raw)
  To: Marcel Holtmann, Johan Hedberg
  Cc: linux-bluetooth, Abhishek Pandit-Subedi, linux-kernel

Without updating the patchram, the BCM4354 does not support a higher
operating speed. The normal bcm_setup follows the correct order
(init_speed, patchram and then oper_speed) but the serdev driver will
set the operating speed before calling the hu->setup function. Thus,
for the BCM4354, disallow setting the operating speed before patchram.

Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
---

 drivers/bluetooth/hci_bcm.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index 0f851c0dde7f..4fe66e52927d 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -1167,7 +1167,7 @@ static int bcm_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static const struct hci_uart_proto bcm_proto = {
+static struct hci_uart_proto bcm_proto = {
 	.id		= HCI_UART_BCM,
 	.name		= "Broadcom",
 	.manufacturer	= 15,
@@ -1371,6 +1371,22 @@ static struct platform_driver bcm_driver = {
 	},
 };
 
+static int bcm_check_disallow_set_baudrate(struct serdev_device *serdev)
+{
+	const char *compatible = of_get_property(serdev->dev.of_node,
+						 "compatible", NULL);
+
+	if (compatible) {
+		/* BCM4354 can't run at full speed before patchram. Disallow
+		 * externally setting operating speed.
+		 */
+		if (!strcmp(compatible, "brcm,bcm43540-bt"))
+			return 1;
+	}
+
+	return 0;
+}
+
 static int bcm_serdev_probe(struct serdev_device *serdev)
 {
 	struct bcm_device *bcmdev;
@@ -1408,6 +1424,9 @@ static int bcm_serdev_probe(struct serdev_device *serdev)
 	if (err)
 		dev_err(&serdev->dev, "Failed to power down\n");
 
+	if (bcm_check_disallow_set_baudrate(serdev))
+		bcm_proto.set_baudrate = NULL;
+
 	return hci_uart_register_device(&bcmdev->serdev_hu, &bcm_proto);
 }
 
-- 
2.24.0.rc1.363.gb1bccd3e3d-goog


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/4] Bluetooth: btbcm: Support pcm configuration
  2019-11-06  0:29 [PATCH 0/4] Bluetooth: hci_bcm: Additional changes for BCM4354 support Abhishek Pandit-Subedi
  2019-11-06  0:29 ` [PATCH 1/4] Bluetooth: hci_bcm: Disallow set_baudrate for BCM4354 Abhishek Pandit-Subedi
@ 2019-11-06  0:29 ` Abhishek Pandit-Subedi
  2019-11-06  0:29 ` [PATCH 3/4] Bluetooth: hci_bcm: Support pcm params in dts Abhishek Pandit-Subedi
  2019-11-06  0:29 ` [PATCH 4/4] dt-bindings: net: bluetooth: update broadcom-bluetooth Abhishek Pandit-Subedi
  3 siblings, 0 replies; 10+ messages in thread
From: Abhishek Pandit-Subedi @ 2019-11-06  0:29 UTC (permalink / raw)
  To: Marcel Holtmann, Johan Hedberg
  Cc: linux-bluetooth, Abhishek Pandit-Subedi, linux-kernel

Add BCM vendor specific commands to configure PCM.

Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
---

 drivers/bluetooth/btbcm.c | 27 +++++++++++++++++++++++++++
 drivers/bluetooth/btbcm.h | 12 ++++++++++++
 2 files changed, 39 insertions(+)

diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c
index 2d2e6d862068..1b6afb11844a 100644
--- a/drivers/bluetooth/btbcm.c
+++ b/drivers/bluetooth/btbcm.c
@@ -105,6 +105,33 @@ int btbcm_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr)
 }
 EXPORT_SYMBOL_GPL(btbcm_set_bdaddr);
 
+int btbcm_set_pcm_params(struct hci_dev *hdev,
+			 const struct bcm_set_pcm_int_params *int_params,
+			 const struct bcm_set_pcm_format_params *format_params)
+{
+	struct sk_buff *skb;
+	int err;
+
+	skb = __hci_cmd_sync(hdev, 0xfc1c, 5, int_params, HCI_INIT_TIMEOUT);
+	if (IS_ERR(skb)) {
+		err = PTR_ERR(skb);
+		bt_dev_err(hdev, "BCM: Set PCM int params failed (%d)", err);
+		return err;
+	}
+	kfree_skb(skb);
+
+	skb = __hci_cmd_sync(hdev, 0xfc1e, 5, int_params, HCI_INIT_TIMEOUT);
+	if (IS_ERR(skb)) {
+		err = PTR_ERR(skb);
+		bt_dev_err(hdev, "BCM: Set PCM data params failed (%d)", err);
+		return err;
+	}
+	kfree_skb(skb);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(btbcm_set_pcm_params);
+
 int btbcm_patchram(struct hci_dev *hdev, const struct firmware *fw)
 {
 	const struct hci_command_hdr *cmd;
diff --git a/drivers/bluetooth/btbcm.h b/drivers/bluetooth/btbcm.h
index d204be8a84bf..4cc6769f6bfb 100644
--- a/drivers/bluetooth/btbcm.h
+++ b/drivers/bluetooth/btbcm.h
@@ -49,11 +49,16 @@ struct bcm_set_pcm_format_params {
 	__u8 right_justify;
 } __packed;
 
+#define BCM_PCM_PARAMS_COUNT 10
+
 #if IS_ENABLED(CONFIG_BT_BCM)
 
 int btbcm_check_bdaddr(struct hci_dev *hdev);
 int btbcm_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr);
 int btbcm_patchram(struct hci_dev *hdev, const struct firmware *fw);
+int btbcm_set_pcm_params(struct hci_dev *hdev,
+			 const struct bcm_set_pcm_int_params *int_params,
+			 const struct bcm_set_pcm_format_params *format_params);
 
 int btbcm_setup_patchram(struct hci_dev *hdev);
 int btbcm_setup_apple(struct hci_dev *hdev);
@@ -74,6 +79,13 @@ static inline int btbcm_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr)
 	return -EOPNOTSUPP;
 }
 
+int btbcm_set_pcm_params(struct hci_dev *hdev,
+			 const struct bcm_set_pcm_int_params *int_params,
+			 const struct bcm_set_pcm_format_params *format_params)
+{
+	return -EOPNOTSUPP;
+}
+
 static inline int btbcm_patchram(struct hci_dev *hdev, const struct firmware *fw)
 {
 	return -EOPNOTSUPP;
-- 
2.24.0.rc1.363.gb1bccd3e3d-goog


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/4] Bluetooth: hci_bcm: Support pcm params in dts
  2019-11-06  0:29 [PATCH 0/4] Bluetooth: hci_bcm: Additional changes for BCM4354 support Abhishek Pandit-Subedi
  2019-11-06  0:29 ` [PATCH 1/4] Bluetooth: hci_bcm: Disallow set_baudrate for BCM4354 Abhishek Pandit-Subedi
  2019-11-06  0:29 ` [PATCH 2/4] Bluetooth: btbcm: Support pcm configuration Abhishek Pandit-Subedi
@ 2019-11-06  0:29 ` Abhishek Pandit-Subedi
  2019-11-06  0:29 ` [PATCH 4/4] dt-bindings: net: bluetooth: update broadcom-bluetooth Abhishek Pandit-Subedi
  3 siblings, 0 replies; 10+ messages in thread
From: Abhishek Pandit-Subedi @ 2019-11-06  0:29 UTC (permalink / raw)
  To: Marcel Holtmann, Johan Hedberg
  Cc: linux-bluetooth, Abhishek Pandit-Subedi, linux-kernel

BCM chips may require configuration of PCM to operate correctly and
there is a vendor specific HCI command to do this. Add support in the
hci_bcm driver to parse this from devicetree and configure the chip.

Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
---

 drivers/bluetooth/hci_bcm.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index 4fe66e52927d..e94908a7e407 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -79,6 +79,7 @@
  * @hu: pointer to HCI UART controller struct,
  *	used to disable flow control during runtime suspend and system sleep
  * @is_suspended: whether flow control is currently disabled
+ * @pcm_params: Bytestring of pcm int and format params.
  */
 struct bcm_device {
 	/* Must be the first member, hci_serdev.c expects this. */
@@ -112,6 +113,9 @@ struct bcm_device {
 	struct hci_uart		*hu;
 	bool			is_suspended;
 #endif
+
+	bool			has_pcm_params;
+	u8			pcm_params[BCM_PCM_PARAMS_COUNT];
 };
 
 /* generic bcm uart resources */
@@ -529,6 +533,8 @@ static int bcm_setup(struct hci_uart *hu)
 	const struct firmware *fw;
 	unsigned int speed;
 	int err;
+	struct bcm_set_pcm_int_params int_params;
+	struct bcm_set_pcm_format_params format_params;
 
 	bt_dev_dbg(hu->hdev, "hu %p", hu);
 
@@ -576,6 +582,23 @@ static int bcm_setup(struct hci_uart *hu)
 			host_set_baudrate(hu, speed);
 	}
 
+	/* PCM parameters if any*/
+	if (bcm->dev && bcm->dev->has_pcm_params) {
+		memcpy(&int_params, &(bcm->dev->pcm_params[0]),
+		       sizeof(int_params));
+		memcpy(&format_params, &(bcm->dev->pcm_params[5]),
+		       sizeof(format_params));
+
+		err = btbcm_set_pcm_params(hu->hdev, &int_params,
+					   &format_params);
+
+		if (err) {
+			bt_dev_info(hu->hdev, "BCM: Set pcm params failed (%d)",
+				    err);
+		}
+
+	}
+
 finalize:
 	release_firmware(fw);
 
@@ -1112,7 +1135,15 @@ static int bcm_acpi_probe(struct bcm_device *dev)
 
 static int bcm_of_probe(struct bcm_device *bdev)
 {
+	int plen;
+
 	device_property_read_u32(bdev->dev, "max-speed", &bdev->oper_speed);
+	plen = device_property_read_u8_array(bdev->dev, "pcm-parameters",
+					     bdev->pcm_params,
+					     BCM_PCM_PARAMS_COUNT);
+	if (plen == 0)
+		bdev->has_pcm_params = true;
+
 	return 0;
 }
 
-- 
2.24.0.rc1.363.gb1bccd3e3d-goog


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/4] dt-bindings: net: bluetooth: update broadcom-bluetooth
  2019-11-06  0:29 [PATCH 0/4] Bluetooth: hci_bcm: Additional changes for BCM4354 support Abhishek Pandit-Subedi
                   ` (2 preceding siblings ...)
  2019-11-06  0:29 ` [PATCH 3/4] Bluetooth: hci_bcm: Support pcm params in dts Abhishek Pandit-Subedi
@ 2019-11-06  0:29 ` Abhishek Pandit-Subedi
  2019-11-07  0:46   ` Rob Herring
  2019-11-07 14:09   ` Marcel Holtmann
  3 siblings, 2 replies; 10+ messages in thread
From: Abhishek Pandit-Subedi @ 2019-11-06  0:29 UTC (permalink / raw)
  To: Marcel Holtmann, Johan Hedberg
  Cc: linux-bluetooth, Abhishek Pandit-Subedi, devicetree,
	David S. Miller, netdev, linux-kernel, Rob Herring, Ondrej Jirman,
	Mark Rutland, Chen-Yu Tsai

Add documentation for pcm-parameters.

Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>

---

 Documentation/devicetree/bindings/net/broadcom-bluetooth.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt b/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt
index c749dc297624..ae60277b5569 100644
--- a/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt
+++ b/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt
@@ -29,6 +29,9 @@ Optional properties:
    - "lpo": external low power 32.768 kHz clock
  - vbat-supply: phandle to regulator supply for VBAT
  - vddio-supply: phandle to regulator supply for VDDIO
+ - pcm-parameters: When set, will configure PCM parameters on the device. The
+   contents should be a 10-byte array corresponding to the pcm params (see
+   btbcm.h for more information).
 
 
 Example:
@@ -40,5 +43,6 @@ Example:
        bluetooth {
                compatible = "brcm,bcm43438-bt";
                max-speed = <921600>;
+               pcm-parameters = [1 2 0 1 1 0 0 0 0 0];
        };
 };
-- 
2.24.0.rc1.363.gb1bccd3e3d-goog


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 4/4] dt-bindings: net: bluetooth: update broadcom-bluetooth
  2019-11-06  0:29 ` [PATCH 4/4] dt-bindings: net: bluetooth: update broadcom-bluetooth Abhishek Pandit-Subedi
@ 2019-11-07  0:46   ` Rob Herring
  2019-11-07 14:09   ` Marcel Holtmann
  1 sibling, 0 replies; 10+ messages in thread
From: Rob Herring @ 2019-11-07  0:46 UTC (permalink / raw)
  To: Abhishek Pandit-Subedi
  Cc: Marcel Holtmann, Johan Hedberg, linux-bluetooth, devicetree,
	David S. Miller, netdev, linux-kernel, Ondrej Jirman,
	Mark Rutland, Chen-Yu Tsai

On Tue, Nov 05, 2019 at 04:29:23PM -0800, Abhishek Pandit-Subedi wrote:
> Add documentation for pcm-parameters.
> 
> Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> 
> ---
> 
>  Documentation/devicetree/bindings/net/broadcom-bluetooth.txt | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt b/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt
> index c749dc297624..ae60277b5569 100644
> --- a/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt
> +++ b/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt
> @@ -29,6 +29,9 @@ Optional properties:
>     - "lpo": external low power 32.768 kHz clock
>   - vbat-supply: phandle to regulator supply for VBAT
>   - vddio-supply: phandle to regulator supply for VDDIO
> + - pcm-parameters: When set, will configure PCM parameters on the device. The
> +   contents should be a 10-byte array corresponding to the pcm params (see
> +   btbcm.h for more information).

Needs a vendor prefix.

>  
>  
>  Example:
> @@ -40,5 +43,6 @@ Example:
>         bluetooth {
>                 compatible = "brcm,bcm43438-bt";
>                 max-speed = <921600>;
> +               pcm-parameters = [1 2 0 1 1 0 0 0 0 0];
>         };
>  };
> -- 
> 2.24.0.rc1.363.gb1bccd3e3d-goog
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 4/4] dt-bindings: net: bluetooth: update broadcom-bluetooth
  2019-11-06  0:29 ` [PATCH 4/4] dt-bindings: net: bluetooth: update broadcom-bluetooth Abhishek Pandit-Subedi
  2019-11-07  0:46   ` Rob Herring
@ 2019-11-07 14:09   ` Marcel Holtmann
  1 sibling, 0 replies; 10+ messages in thread
From: Marcel Holtmann @ 2019-11-07 14:09 UTC (permalink / raw)
  To: Abhishek Pandit-Subedi
  Cc: Johan Hedberg, Bluez mailing list, devicetree, David S. Miller,
	netdev, lkml, Rob Herring, Ondrej Jirman, Mark Rutland,
	Chen-Yu Tsai

Hi Abhishek,

> Add documentation for pcm-parameters.
> 
> Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> 
> ---
> 
> Documentation/devicetree/bindings/net/broadcom-bluetooth.txt | 4 ++++
> 1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt b/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt
> index c749dc297624..ae60277b5569 100644
> --- a/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt
> +++ b/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt
> @@ -29,6 +29,9 @@ Optional properties:
>    - "lpo": external low power 32.768 kHz clock
>  - vbat-supply: phandle to regulator supply for VBAT
>  - vddio-supply: phandle to regulator supply for VDDIO
> + - pcm-parameters: When set, will configure PCM parameters on the device. The
> +   contents should be a 10-byte array corresponding to the pcm params (see
> +   btbcm.h for more information).
> 
> 
> Example:
> @@ -40,5 +43,6 @@ Example:
>        bluetooth {
>                compatible = "brcm,bcm43438-bt";
>                max-speed = <921600>;
> +               pcm-parameters = [1 2 0 1 1 0 0 0 0 0];
>        };
> };

I think about 1-2 years there have been a discussion on how to represent these values in a DT. I prefer we split these into separate values so it becomes usable by other drivers / vendors as well. In addition, maybe we start to focus on the values that differ from the default.

Regards

Marcel


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/4] Bluetooth: hci_bcm: Disallow set_baudrate for BCM4354
@ 2019-11-23 10:01 Marcel Holtmann
  2019-11-25  2:46 ` kbuild test robot
  2019-11-25 17:52 ` Abhishek Pandit-Subedi
  0 siblings, 2 replies; 10+ messages in thread
From: Marcel Holtmann @ 2019-11-23 10:01 UTC (permalink / raw)
  To: linux-bluetooth

From: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>

Without updating the patchram, the BCM4354 does not support a higher
operating speed. The normal bcm_setup follows the correct order
(init_speed, patchram and then oper_speed) but the serdev driver will
set the operating speed before calling the hu->setup function. Thus,
for the BCM4354, don't set the operating speed before patchram.

Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
---
 drivers/bluetooth/hci_bcm.c | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index d2a6a4afdbbb..d48044276895 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -47,6 +47,14 @@
 
 #define BCM_NUM_SUPPLIES 2
 
+/**
+ * struct bcm_device_data - device specific data
+ * @no_early_set_baudrate: Disallow set baudrate before driver setup()
+ */
+struct bcm_device_data {
+	bool	no_early_set_baudrate;
+};
+
 /**
  * struct bcm_device - device driver resources
  * @serdev_hu: HCI UART controller struct
@@ -79,6 +87,7 @@
  * @hu: pointer to HCI UART controller struct,
  *	used to disable flow control during runtime suspend and system sleep
  * @is_suspended: whether flow control is currently disabled
+ * @no_early_set_baudrate: don't set_baudrate before setup()
  */
 struct bcm_device {
 	/* Must be the first member, hci_serdev.c expects this. */
@@ -112,6 +121,7 @@ struct bcm_device {
 	struct hci_uart		*hu;
 	bool			is_suspended;
 #endif
+	bool			no_early_set_baudrate;
 };
 
 /* generic bcm uart resources */
@@ -447,7 +457,13 @@ static int bcm_open(struct hci_uart *hu)
 	if (bcm->dev) {
 		hci_uart_set_flow_control(hu, true);
 		hu->init_speed = bcm->dev->init_speed;
-		hu->oper_speed = bcm->dev->oper_speed;
+
+		/* If oper_speed is set, ldisc/serdev will set the baudrate
+		 * before calling setup()
+		 */
+		if (!bcm->dev->no_early_set_baudrate)
+			hu->oper_speed = bcm->dev->oper_speed;
+
 		err = bcm_gpio_set_power(bcm->dev, true);
 		hci_uart_set_flow_control(hu, false);
 		if (err)
@@ -565,6 +581,8 @@ static int bcm_setup(struct hci_uart *hu)
 	/* Operational speed if any */
 	if (hu->oper_speed)
 		speed = hu->oper_speed;
+	else if (bcm->dev && bcm->dev->oper_speed)
+		speed = bcm->dev->oper_speed;
 	else if (hu->proto->oper_speed)
 		speed = hu->proto->oper_speed;
 	else
@@ -1374,6 +1392,7 @@ static struct platform_driver bcm_driver = {
 static int bcm_serdev_probe(struct serdev_device *serdev)
 {
 	struct bcm_device *bcmdev;
+	const struct bcm_device_data *data;
 	int err;
 
 	bcmdev = devm_kzalloc(&serdev->dev, sizeof(*bcmdev), GFP_KERNEL);
@@ -1408,6 +1427,10 @@ static int bcm_serdev_probe(struct serdev_device *serdev)
 	if (err)
 		dev_err(&serdev->dev, "Failed to power down\n");
 
+	data = device_get_match_data(bcmdev->dev);
+	if (data)
+		bcmdev->no_early_set_baudrate = data->no_early_set_baudrate;
+
 	return hci_uart_register_device(&bcmdev->serdev_hu, &bcm_proto);
 }
 
@@ -1424,7 +1447,7 @@ static const struct of_device_id bcm_bluetooth_of_match[] = {
 	{ .compatible = "brcm,bcm4345c5" },
 	{ .compatible = "brcm,bcm4330-bt" },
 	{ .compatible = "brcm,bcm43438-bt" },
-	{ .compatible = "brcm,bcm43540-bt" },
+	{ .compatible = "brcm,bcm43540-bt", .data = &bcm4354_device_data },
 	{ .compatible = "brcm,bcm4335a0" },
 	{ },
 };
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/4] Bluetooth: hci_bcm: Disallow set_baudrate for BCM4354
  2019-11-23 10:01 [PATCH 1/4] Bluetooth: hci_bcm: Disallow set_baudrate for BCM4354 Marcel Holtmann
@ 2019-11-25  2:46 ` kbuild test robot
  2019-11-25 17:52 ` Abhishek Pandit-Subedi
  1 sibling, 0 replies; 10+ messages in thread
From: kbuild test robot @ 2019-11-25  2:46 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: kbuild-all, linux-bluetooth

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

Hi Marcel,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on next-20191122]
[cannot apply to bluetooth-next/master bluetooth/master net-next/master v5.4-rc8 v5.4-rc7 v5.4-rc6 v5.4-rc8]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Marcel-Holtmann/Bluetooth-hci_bcm-Disallow-set_baudrate-for-BCM4354/20191125-034727
base:    b9d3d01405061bb42358fe53f824e894a1922ced
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gcc (GCC) 7.4.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.4.0 make.cross ARCH=sh 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/bluetooth/hci_bcm.c:1450:47: error: 'bcm4354_device_data' undeclared here (not in a function); did you mean 'bcm_device_data'?
     { .compatible = "brcm,bcm43540-bt", .data = &bcm4354_device_data },
                                                  ^~~~~~~~~~~~~~~~~~~
                                                  bcm_device_data

vim +1450 drivers/bluetooth/hci_bcm.c

  1443	
  1444	#ifdef CONFIG_OF
  1445	static const struct of_device_id bcm_bluetooth_of_match[] = {
  1446		{ .compatible = "brcm,bcm20702a1" },
  1447		{ .compatible = "brcm,bcm4345c5" },
  1448		{ .compatible = "brcm,bcm4330-bt" },
  1449		{ .compatible = "brcm,bcm43438-bt" },
> 1450		{ .compatible = "brcm,bcm43540-bt", .data = &bcm4354_device_data },
  1451		{ .compatible = "brcm,bcm4335a0" },
  1452		{ },
  1453	};
  1454	MODULE_DEVICE_TABLE(of, bcm_bluetooth_of_match);
  1455	#endif
  1456	

---
0-DAY kernel test infrastructure                 Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 52822 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/4] Bluetooth: hci_bcm: Disallow set_baudrate for BCM4354
  2019-11-23 10:01 [PATCH 1/4] Bluetooth: hci_bcm: Disallow set_baudrate for BCM4354 Marcel Holtmann
  2019-11-25  2:46 ` kbuild test robot
@ 2019-11-25 17:52 ` Abhishek Pandit-Subedi
  1 sibling, 0 replies; 10+ messages in thread
From: Abhishek Pandit-Subedi @ 2019-11-25 17:52 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth

On Sat, Nov 23, 2019 at 2:01 AM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> From: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
>
> Without updating the patchram, the BCM4354 does not support a higher
> operating speed. The normal bcm_setup follows the correct order
> (init_speed, patchram and then oper_speed) but the serdev driver will
> set the operating speed before calling the hu->setup function. Thus,
> for the BCM4354, don't set the operating speed before patchram.
>
> Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
> ---
>  drivers/bluetooth/hci_bcm.c | 27 +++++++++++++++++++++++++--
>  1 file changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
> index d2a6a4afdbbb..d48044276895 100644
> --- a/drivers/bluetooth/hci_bcm.c
> +++ b/drivers/bluetooth/hci_bcm.c
> @@ -47,6 +47,14 @@
>
>  #define BCM_NUM_SUPPLIES 2
>
> +/**
> + * struct bcm_device_data - device specific data
> + * @no_early_set_baudrate: Disallow set baudrate before driver setup()
> + */
> +struct bcm_device_data {
> +       bool    no_early_set_baudrate;
> +};
> +
>  /**
>   * struct bcm_device - device driver resources
>   * @serdev_hu: HCI UART controller struct
> @@ -79,6 +87,7 @@
>   * @hu: pointer to HCI UART controller struct,
>   *     used to disable flow control during runtime suspend and system sleep
>   * @is_suspended: whether flow control is currently disabled
> + * @no_early_set_baudrate: don't set_baudrate before setup()
>   */
>  struct bcm_device {
>         /* Must be the first member, hci_serdev.c expects this. */
> @@ -112,6 +121,7 @@ struct bcm_device {
>         struct hci_uart         *hu;
>         bool                    is_suspended;
>  #endif
> +       bool                    no_early_set_baudrate;
>  };
>
>  /* generic bcm uart resources */
> @@ -447,7 +457,13 @@ static int bcm_open(struct hci_uart *hu)
>         if (bcm->dev) {
>                 hci_uart_set_flow_control(hu, true);
>                 hu->init_speed = bcm->dev->init_speed;
> -               hu->oper_speed = bcm->dev->oper_speed;
> +
> +               /* If oper_speed is set, ldisc/serdev will set the baudrate
> +                * before calling setup()
> +                */
> +               if (!bcm->dev->no_early_set_baudrate)
> +                       hu->oper_speed = bcm->dev->oper_speed;
> +
>                 err = bcm_gpio_set_power(bcm->dev, true);
>                 hci_uart_set_flow_control(hu, false);
>                 if (err)
> @@ -565,6 +581,8 @@ static int bcm_setup(struct hci_uart *hu)
>         /* Operational speed if any */
>         if (hu->oper_speed)
>                 speed = hu->oper_speed;
> +       else if (bcm->dev && bcm->dev->oper_speed)
> +               speed = bcm->dev->oper_speed;
>         else if (hu->proto->oper_speed)
>                 speed = hu->proto->oper_speed;
>         else
> @@ -1374,6 +1392,7 @@ static struct platform_driver bcm_driver = {
>  static int bcm_serdev_probe(struct serdev_device *serdev)
>  {
>         struct bcm_device *bcmdev;
> +       const struct bcm_device_data *data;
>         int err;
>
>         bcmdev = devm_kzalloc(&serdev->dev, sizeof(*bcmdev), GFP_KERNEL);
> @@ -1408,6 +1427,10 @@ static int bcm_serdev_probe(struct serdev_device *serdev)
>         if (err)
>                 dev_err(&serdev->dev, "Failed to power down\n");
>
> +       data = device_get_match_data(bcmdev->dev);
> +       if (data)
> +               bcmdev->no_early_set_baudrate = data->no_early_set_baudrate;
> +
>         return hci_uart_register_device(&bcmdev->serdev_hu, &bcm_proto);
>  }
>
> @@ -1424,7 +1447,7 @@ static const struct of_device_id bcm_bluetooth_of_match[] = {
>         { .compatible = "brcm,bcm4345c5" },
>         { .compatible = "brcm,bcm4330-bt" },
>         { .compatible = "brcm,bcm43438-bt" },
> -       { .compatible = "brcm,bcm43540-bt" },
> +       { .compatible = "brcm,bcm43540-bt", .data = &bcm4354_device_data },
>         { .compatible = "brcm,bcm4335a0" },
>         { },
>  };
> --
> 2.23.0
>

Looks good except missing `bcm4354_device_data` as noted by kbuild test robot.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2019-11-25 17:52 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-11-06  0:29 [PATCH 0/4] Bluetooth: hci_bcm: Additional changes for BCM4354 support Abhishek Pandit-Subedi
2019-11-06  0:29 ` [PATCH 1/4] Bluetooth: hci_bcm: Disallow set_baudrate for BCM4354 Abhishek Pandit-Subedi
2019-11-06  0:29 ` [PATCH 2/4] Bluetooth: btbcm: Support pcm configuration Abhishek Pandit-Subedi
2019-11-06  0:29 ` [PATCH 3/4] Bluetooth: hci_bcm: Support pcm params in dts Abhishek Pandit-Subedi
2019-11-06  0:29 ` [PATCH 4/4] dt-bindings: net: bluetooth: update broadcom-bluetooth Abhishek Pandit-Subedi
2019-11-07  0:46   ` Rob Herring
2019-11-07 14:09   ` Marcel Holtmann
  -- strict thread matches above, loose matches on Subject: below --
2019-11-23 10:01 [PATCH 1/4] Bluetooth: hci_bcm: Disallow set_baudrate for BCM4354 Marcel Holtmann
2019-11-25  2:46 ` kbuild test robot
2019-11-25 17:52 ` Abhishek Pandit-Subedi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).