All of lore.kernel.org
 help / color / mirror / Atom feed
* [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; 5+ 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] 5+ 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; 5+ 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] 5+ messages in thread

* Re: [PATCH 1/4] Bluetooth: hci_bcm: Disallow set_baudrate for BCM4354
@ 2019-11-25  2:46   ` kbuild test robot
  0 siblings, 0 replies; 5+ messages in thread
From: kbuild test robot @ 2019-11-25  2:46 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 2210 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(a)lists.01.org Intel Corporation

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

^ permalink raw reply	[flat|nested] 5+ 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; 5+ 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] 5+ 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
  0 siblings, 0 replies; 5+ 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] 5+ messages in thread

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

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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  2:46   ` kbuild test robot
2019-11-25 17:52 ` Abhishek Pandit-Subedi
  -- strict thread matches above, loose matches on Subject: below --
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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.