linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] Bluetooth: btusb: add QCA6174A compatible properties
@ 2019-02-22  0:34 Brian Norris
  2019-02-22  0:34 ` [PATCH 2/3] dt-bindings: net: btusb: add QCA6174A IDs Brian Norris
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Brian Norris @ 2019-02-22  0:34 UTC (permalink / raw)
  To: Heiko Stuebner, Marcel Holtmann, Johan Hedberg
  Cc: Rob Herring, Enric Balletbo i Serra, Douglas Anderson,
	devicetree, linux-arm-kernel, linux-rockchip, linux-bluetooth,
	linux-kernel, Matthias Kaehlcke, Rajat Jain, Brian Norris

We may need to specify a GPIO wake pin for this device, so add a
compatible property for it.

There are at least to USB PID/VID variations of this chip: one with a
Lite-On ID and one with an Atheros ID.

Signed-off-by: Brian Norris <briannorris@chromium.org>
---
 drivers/bluetooth/btusb.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 470ee68555d9..380e6f38c607 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -2862,6 +2862,8 @@ static irqreturn_t btusb_oob_wake_handler(int irq, void *priv)
 
 static const struct of_device_id btusb_match_table[] = {
 	{ .compatible = "usb1286,204e" },
+	{ .compatible = "usb0cf3,e300" }, /* QCA6174A */
+	{ .compatible = "usb04ca,301a" }, /* QCA6174A (Lite-On) */
 	{ }
 };
 MODULE_DEVICE_TABLE(of, btusb_match_table);
-- 
2.21.0.rc0.258.g878e2cd30e-goog


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

* [PATCH 2/3] dt-bindings: net: btusb: add QCA6174A IDs
  2019-02-22  0:34 [PATCH 1/3] Bluetooth: btusb: add QCA6174A compatible properties Brian Norris
@ 2019-02-22  0:34 ` Brian Norris
  2019-02-22 18:05   ` Matthias Kaehlcke
  2019-02-22 21:57   ` Robin Murphy
  2019-02-22  0:34 ` [PATCH 3/3] arm64: dts: rockchip: move QCA6174A wakeup pin into its USB node Brian Norris
  2019-02-22 18:04 ` [PATCH 1/3] Bluetooth: btusb: add QCA6174A compatible properties Matthias Kaehlcke
  2 siblings, 2 replies; 9+ messages in thread
From: Brian Norris @ 2019-02-22  0:34 UTC (permalink / raw)
  To: Heiko Stuebner, Marcel Holtmann, Johan Hedberg
  Cc: Rob Herring, Enric Balletbo i Serra, Douglas Anderson,
	devicetree, linux-arm-kernel, linux-rockchip, linux-bluetooth,
	linux-kernel, Matthias Kaehlcke, Rajat Jain, Brian Norris

There are two USB PID/VID variations I've seen for this chip, and I want
to utilize the 'interrupts' property defined here already.

Signed-off-by: Brian Norris <briannorris@chromium.org>
---
 Documentation/devicetree/bindings/net/btusb.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/btusb.txt b/Documentation/devicetree/bindings/net/btusb.txt
index 37d67926dd6d..43c96c3380a6 100644
--- a/Documentation/devicetree/bindings/net/btusb.txt
+++ b/Documentation/devicetree/bindings/net/btusb.txt
@@ -9,6 +9,9 @@ Required properties:
 		 (more may be added later) are:
 
 		  "usb1286,204e" (Marvell 8997)
+		  "usb0cf3,e300" (Qualcomm QCA6174A)
+		  "usb04ca,301a" (Qualcomm QCA6174A (Lite-On))
+
 
 Also, vendors that use btusb may have device additional properties, e.g:
 Documentation/devicetree/bindings/net/marvell-bt-8xxx.txt
-- 
2.21.0.rc0.258.g878e2cd30e-goog


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

* [PATCH 3/3] arm64: dts: rockchip: move QCA6174A wakeup pin into its USB node
  2019-02-22  0:34 [PATCH 1/3] Bluetooth: btusb: add QCA6174A compatible properties Brian Norris
  2019-02-22  0:34 ` [PATCH 2/3] dt-bindings: net: btusb: add QCA6174A IDs Brian Norris
@ 2019-02-22  0:34 ` Brian Norris
  2019-02-22 18:27   ` Matthias Kaehlcke
  2019-02-22 18:04 ` [PATCH 1/3] Bluetooth: btusb: add QCA6174A compatible properties Matthias Kaehlcke
  2 siblings, 1 reply; 9+ messages in thread
From: Brian Norris @ 2019-02-22  0:34 UTC (permalink / raw)
  To: Heiko Stuebner, Marcel Holtmann, Johan Hedberg
  Cc: Rob Herring, Enric Balletbo i Serra, Douglas Anderson,
	devicetree, linux-arm-kernel, linux-rockchip, linux-bluetooth,
	linux-kernel, Matthias Kaehlcke, Rajat Jain, Brian Norris

Currently, we don't coordinate BT USB activity with our handling of the
BT out-of-band wake pin, and instead just use gpio-keys. That causes
problems because we have no way of distinguishing wake activity due to a
BT device (e.g., mouse) vs. the BT controller (e.g., re-configuring wake
mask before suspend). This can cause spurious wake events just because
we, for instance, try to reconfigure the host controller's event mask
before suspending.

We can avoid these synchronization problems by handling the BT wake pin
directly in the btusb driver -- for all activity up until BT controller
suspend(), we simply listen to normal USB activity (e.g., to know the
difference between device and host activity); once we're really ready to
suspend the host controller, there should be no more host activity, and
only *then* do we unmask the GPIO interrupt.

This is already supported by btusb; we just need to describe the wake
pin in the right node.

We list 2 compatible properties, since both PID/VID pairs show up on
Scarlet devices, and they're both essentially identical QCA6174A-based
modules.

Also note that the polarity was wrong before: Qualcomm implemented WAKE
as active high, not active low. We only got away with this because
gpio-keys always reconfigured us as bi-directional edge-triggered.

Finally, we have an external pull-up and a level-shifter on this line
(we didn't notice Qualcomm's polarity in the initial design), so we
can't do pull-down. Switch to pull-none.

Signed-off-by: Brian Norris <briannorris@chromium.org>
---
This patch is also required to make this stable, but since it's not
really tied to the device tree, and it's an existing bug, I sent it
separately:

  https://lore.kernel.org/patchwork/patch/1044896/
  Subject: Bluetooth: btusb: request wake pin with NOAUTOEN

 .../dts/rockchip/rk3399-gru-chromebook.dtsi   | 13 ++++++
 .../boot/dts/rockchip/rk3399-gru-scarlet.dtsi | 46 ++++++++++++-------
 arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi  | 13 ------
 3 files changed, 42 insertions(+), 30 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru-chromebook.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-gru-chromebook.dtsi
index c400be64170e..931640e9aed4 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-gru-chromebook.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-gru-chromebook.dtsi
@@ -200,6 +200,19 @@
 		pinctrl-0 = <&bl_en>;
 		pwm-delay-us = <10000>;
 	};
+
+	gpio_keys: gpio-keys {
+		compatible = "gpio-keys";
+		pinctrl-names = "default";
+		pinctrl-0 = <&bt_host_wake_l>;
+
+		wake_on_bt: wake-on-bt {
+			label = "Wake-on-Bluetooth";
+			gpios = <&gpio0 3 GPIO_ACTIVE_LOW>;
+			linux,code = <KEY_WAKEUP>;
+			wakeup-source;
+		};
+	};
 };
 
 &ppvar_bigcpu {
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi
index fc50b3ef758c..3e2196c08473 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi
@@ -175,6 +175,21 @@
 		pinctrl-0 = <&dmic_en>;
 		wakeup-delay-ms = <250>;
 	};
+
+	gpio_keys: gpio-keys {
+		compatible = "gpio-keys";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pen_eject_odl>;
+
+		pen-insert {
+			label = "Pen Insert";
+			/* Insert = low, eject = high */
+			gpios = <&gpio1 1 GPIO_ACTIVE_LOW>;
+			linux,code = <SW_PEN_INSERTED>;
+			linux,input-type = <EV_SW>;
+			wakeup-source;
+		};
+	};
 };
 
 /* pp900_s0 aliases */
@@ -328,20 +343,6 @@ camera: &i2c7 {
 		<400000000>;
 };
 
-&gpio_keys {
-	pinctrl-names = "default";
-	pinctrl-0 = <&bt_host_wake_l>, <&pen_eject_odl>;
-
-	pen-insert {
-		label = "Pen Insert";
-		/* Insert = low, eject = high */
-		gpios = <&gpio1 1 GPIO_ACTIVE_LOW>;
-		linux,code = <SW_PEN_INSERTED>;
-		linux,input-type = <EV_SW>;
-		wakeup-source;
-	};
-};
-
 &i2c_tunnel {
 	google,remote-bus = <0>;
 };
@@ -437,8 +438,19 @@ camera: &i2c7 {
 	status = "okay";
 };
 
-&wake_on_bt {
-	gpios = <&gpio1 2 GPIO_ACTIVE_LOW>;
+&usb_host0_ohci {
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	qca_bt: bt@1 {
+		compatible = "usb0cf3,e300", "usb04ca,301a";
+		reg = <1>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&bt_host_wake_l>;
+		interrupt-parent = <&gpio1>;
+		interrupts = <2 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-names = "wakeup";
+	};
 };
 
 /* PINCTRL OVERRIDES */
@@ -455,7 +467,7 @@ camera: &i2c7 {
 };
 
 &bt_host_wake_l {
-	rockchip,pins = <1 2 RK_FUNC_GPIO &pcfg_pull_up>;
+	rockchip,pins = <1 2 RK_FUNC_GPIO &pcfg_pull_none>;
 };
 
 &ec_ap_int_l {
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
index ea607a601a86..da03fa9c5662 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
@@ -269,19 +269,6 @@
 		#clock-cells = <0>;
 	};
 
-	gpio_keys: gpio-keys {
-		compatible = "gpio-keys";
-		pinctrl-names = "default";
-		pinctrl-0 = <&bt_host_wake_l>;
-
-		wake_on_bt: wake-on-bt {
-			label = "Wake-on-Bluetooth";
-			gpios = <&gpio0 3 GPIO_ACTIVE_LOW>;
-			linux,code = <KEY_WAKEUP>;
-			wakeup-source;
-		};
-	};
-
 	max98357a: max98357a {
 		compatible = "maxim,max98357a";
 		pinctrl-names = "default";
-- 
2.21.0.rc0.258.g878e2cd30e-goog


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

* Re: [PATCH 1/3] Bluetooth: btusb: add QCA6174A compatible properties
  2019-02-22  0:34 [PATCH 1/3] Bluetooth: btusb: add QCA6174A compatible properties Brian Norris
  2019-02-22  0:34 ` [PATCH 2/3] dt-bindings: net: btusb: add QCA6174A IDs Brian Norris
  2019-02-22  0:34 ` [PATCH 3/3] arm64: dts: rockchip: move QCA6174A wakeup pin into its USB node Brian Norris
@ 2019-02-22 18:04 ` Matthias Kaehlcke
  2 siblings, 0 replies; 9+ messages in thread
From: Matthias Kaehlcke @ 2019-02-22 18:04 UTC (permalink / raw)
  To: Brian Norris
  Cc: Heiko Stuebner, Marcel Holtmann, Johan Hedberg, Rob Herring,
	Enric Balletbo i Serra, Douglas Anderson, devicetree,
	linux-arm-kernel, linux-rockchip, linux-bluetooth, linux-kernel,
	Rajat Jain

On Thu, Feb 21, 2019 at 04:34:01PM -0800, Brian Norris wrote:
> We may need to specify a GPIO wake pin for this device, so add a
> compatible property for it.
> 
> There are at least to USB PID/VID variations of this chip: one with a
> Lite-On ID and one with an Atheros ID.
> 
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
>  drivers/bluetooth/btusb.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 470ee68555d9..380e6f38c607 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -2862,6 +2862,8 @@ static irqreturn_t btusb_oob_wake_handler(int irq, void *priv)
>  
>  static const struct of_device_id btusb_match_table[] = {
>  	{ .compatible = "usb1286,204e" },
> +	{ .compatible = "usb0cf3,e300" }, /* QCA6174A */
> +	{ .compatible = "usb04ca,301a" }, /* QCA6174A (Lite-On) */
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(of, btusb_match_table);

Reviewed-by: Matthias Kaehlcke <mka@chromium.org>

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

* Re: [PATCH 2/3] dt-bindings: net: btusb: add QCA6174A IDs
  2019-02-22  0:34 ` [PATCH 2/3] dt-bindings: net: btusb: add QCA6174A IDs Brian Norris
@ 2019-02-22 18:05   ` Matthias Kaehlcke
  2019-02-22 21:57   ` Robin Murphy
  1 sibling, 0 replies; 9+ messages in thread
From: Matthias Kaehlcke @ 2019-02-22 18:05 UTC (permalink / raw)
  To: Brian Norris
  Cc: Heiko Stuebner, Marcel Holtmann, Johan Hedberg, Rob Herring,
	Enric Balletbo i Serra, Douglas Anderson, devicetree,
	linux-arm-kernel, linux-rockchip, linux-bluetooth, linux-kernel,
	Rajat Jain

On Thu, Feb 21, 2019 at 04:34:02PM -0800, Brian Norris wrote:
> There are two USB PID/VID variations I've seen for this chip, and I want
> to utilize the 'interrupts' property defined here already.
> 
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
>  Documentation/devicetree/bindings/net/btusb.txt | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/btusb.txt b/Documentation/devicetree/bindings/net/btusb.txt
> index 37d67926dd6d..43c96c3380a6 100644
> --- a/Documentation/devicetree/bindings/net/btusb.txt
> +++ b/Documentation/devicetree/bindings/net/btusb.txt
> @@ -9,6 +9,9 @@ Required properties:
>  		 (more may be added later) are:
>  
>  		  "usb1286,204e" (Marvell 8997)
> +		  "usb0cf3,e300" (Qualcomm QCA6174A)
> +		  "usb04ca,301a" (Qualcomm QCA6174A (Lite-On))
> +
>  
>  Also, vendors that use btusb may have device additional properties, e.g:
>  Documentation/devicetree/bindings/net/marvell-bt-8xxx.txt

Reviewed-by: Matthias Kaehlcke <mka@chromium.org>

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

* Re: [PATCH 3/3] arm64: dts: rockchip: move QCA6174A wakeup pin into its USB node
  2019-02-22  0:34 ` [PATCH 3/3] arm64: dts: rockchip: move QCA6174A wakeup pin into its USB node Brian Norris
@ 2019-02-22 18:27   ` Matthias Kaehlcke
  2019-02-22 23:16     ` Rajat Jain
  0 siblings, 1 reply; 9+ messages in thread
From: Matthias Kaehlcke @ 2019-02-22 18:27 UTC (permalink / raw)
  To: Brian Norris
  Cc: Heiko Stuebner, Marcel Holtmann, Johan Hedberg, Rob Herring,
	Enric Balletbo i Serra, Douglas Anderson, devicetree,
	linux-arm-kernel, linux-rockchip, linux-bluetooth, linux-kernel,
	Rajat Jain

On Thu, Feb 21, 2019 at 04:34:03PM -0800, Brian Norris wrote:
> Currently, we don't coordinate BT USB activity with our handling of the
> BT out-of-band wake pin, and instead just use gpio-keys. That causes
> problems because we have no way of distinguishing wake activity due to a
> BT device (e.g., mouse) vs. the BT controller (e.g., re-configuring wake
> mask before suspend). This can cause spurious wake events just because
> we, for instance, try to reconfigure the host controller's event mask
> before suspending.
> 
> We can avoid these synchronization problems by handling the BT wake pin
> directly in the btusb driver -- for all activity up until BT controller
> suspend(), we simply listen to normal USB activity (e.g., to know the
> difference between device and host activity); once we're really ready to
> suspend the host controller, there should be no more host activity, and
> only *then* do we unmask the GPIO interrupt.
> 
> This is already supported by btusb; we just need to describe the wake
> pin in the right node.
> 
> We list 2 compatible properties, since both PID/VID pairs show up on
> Scarlet devices, and they're both essentially identical QCA6174A-based
> modules.
> 
> Also note that the polarity was wrong before: Qualcomm implemented WAKE
> as active high, not active low. We only got away with this because
> gpio-keys always reconfigured us as bi-directional edge-triggered.
> 
> Finally, we have an external pull-up and a level-shifter on this line
> (we didn't notice Qualcomm's polarity in the initial design), so we
> can't do pull-down. Switch to pull-none.
> 
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
> This patch is also required to make this stable, but since it's not
> really tied to the device tree, and it's an existing bug, I sent it
> separately:
> 
>   https://lore.kernel.org/patchwork/patch/1044896/
>   Subject: Bluetooth: btusb: request wake pin with NOAUTOEN
> 
>  .../dts/rockchip/rk3399-gru-chromebook.dtsi   | 13 ++++++
>  .../boot/dts/rockchip/rk3399-gru-scarlet.dtsi | 46 ++++++++++++-------
>  arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi  | 13 ------
>  3 files changed, 42 insertions(+), 30 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru-chromebook.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-gru-chromebook.dtsi
> index c400be64170e..931640e9aed4 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-gru-chromebook.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-gru-chromebook.dtsi
> @@ -200,6 +200,19 @@
>  		pinctrl-0 = <&bl_en>;
>  		pwm-delay-us = <10000>;
>  	};
> +
> +	gpio_keys: gpio-keys {
> +		compatible = "gpio-keys";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&bt_host_wake_l>;
> +
> +		wake_on_bt: wake-on-bt {
> +			label = "Wake-on-Bluetooth";
> +			gpios = <&gpio0 3 GPIO_ACTIVE_LOW>;
> +			linux,code = <KEY_WAKEUP>;
> +			wakeup-source;
> +		};
> +	};
>  };
>  
>  &ppvar_bigcpu {
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi
> index fc50b3ef758c..3e2196c08473 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi
> @@ -175,6 +175,21 @@
>  		pinctrl-0 = <&dmic_en>;
>  		wakeup-delay-ms = <250>;
>  	};
> +
> +	gpio_keys: gpio-keys {
> +		compatible = "gpio-keys";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pen_eject_odl>;
> +
> +		pen-insert {
> +			label = "Pen Insert";
> +			/* Insert = low, eject = high */
> +			gpios = <&gpio1 1 GPIO_ACTIVE_LOW>;
> +			linux,code = <SW_PEN_INSERTED>;
> +			linux,input-type = <EV_SW>;
> +			wakeup-source;
> +		};
> +	};
>  };
>  
>  /* pp900_s0 aliases */
> @@ -328,20 +343,6 @@ camera: &i2c7 {
>  		<400000000>;
>  };
>  
> -&gpio_keys {
> -	pinctrl-names = "default";
> -	pinctrl-0 = <&bt_host_wake_l>, <&pen_eject_odl>;
> -
> -	pen-insert {
> -		label = "Pen Insert";
> -		/* Insert = low, eject = high */
> -		gpios = <&gpio1 1 GPIO_ACTIVE_LOW>;
> -		linux,code = <SW_PEN_INSERTED>;
> -		linux,input-type = <EV_SW>;
> -		wakeup-source;
> -	};
> -};
> -
>  &i2c_tunnel {
>  	google,remote-bus = <0>;
>  };
> @@ -437,8 +438,19 @@ camera: &i2c7 {
>  	status = "okay";
>  };
>  
> -&wake_on_bt {
> -	gpios = <&gpio1 2 GPIO_ACTIVE_LOW>;
> +&usb_host0_ohci {
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +
> +	qca_bt: bt@1 {
> +		compatible = "usb0cf3,e300", "usb04ca,301a";
> +		reg = <1>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&bt_host_wake_l>;
> +		interrupt-parent = <&gpio1>;
> +		interrupts = <2 IRQ_TYPE_LEVEL_HIGH>;
> +		interrupt-names = "wakeup";
> +	};
>  };

I didn't know it was possible to configure (certain) USB devices
through the DT, neat!

>  
>  /* PINCTRL OVERRIDES */
> @@ -455,7 +467,7 @@ camera: &i2c7 {
>  };
>  
>  &bt_host_wake_l {
> -	rockchip,pins = <1 2 RK_FUNC_GPIO &pcfg_pull_up>;
> +	rockchip,pins = <1 2 RK_FUNC_GPIO &pcfg_pull_none>;
>  };
>  
>  &ec_ap_int_l {
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
> index ea607a601a86..da03fa9c5662 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
> @@ -269,19 +269,6 @@
>  		#clock-cells = <0>;
>  	};
>  
> -	gpio_keys: gpio-keys {
> -		compatible = "gpio-keys";
> -		pinctrl-names = "default";
> -		pinctrl-0 = <&bt_host_wake_l>;
> -
> -		wake_on_bt: wake-on-bt {
> -			label = "Wake-on-Bluetooth";
> -			gpios = <&gpio0 3 GPIO_ACTIVE_LOW>;
> -			linux,code = <KEY_WAKEUP>;
> -			wakeup-source;
> -		};
> -	};
> -
>  	max98357a: max98357a {
>  		compatible = "maxim,max98357a";
>  		pinctrl-names = "default";

Reviewed-by: Matthias Kaehlcke <mka@chromium.org>

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

* Re: [PATCH 2/3] dt-bindings: net: btusb: add QCA6174A IDs
  2019-02-22  0:34 ` [PATCH 2/3] dt-bindings: net: btusb: add QCA6174A IDs Brian Norris
  2019-02-22 18:05   ` Matthias Kaehlcke
@ 2019-02-22 21:57   ` Robin Murphy
  2019-02-22 22:10     ` Brian Norris
  1 sibling, 1 reply; 9+ messages in thread
From: Robin Murphy @ 2019-02-22 21:57 UTC (permalink / raw)
  To: Brian Norris, Heiko Stuebner, Marcel Holtmann, Johan Hedberg
  Cc: devicetree, linux-bluetooth, Douglas Anderson, linux-kernel,
	linux-rockchip, Rob Herring, Enric Balletbo i Serra, Rajat Jain,
	Matthias Kaehlcke, linux-arm-kernel

On 2019-02-22 12:34 am, Brian Norris wrote:
> There are two USB PID/VID variations I've seen for this chip, and I want
> to utilize the 'interrupts' property defined here already.
> 
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
>   Documentation/devicetree/bindings/net/btusb.txt | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/btusb.txt b/Documentation/devicetree/bindings/net/btusb.txt
> index 37d67926dd6d..43c96c3380a6 100644
> --- a/Documentation/devicetree/bindings/net/btusb.txt
> +++ b/Documentation/devicetree/bindings/net/btusb.txt
> @@ -9,6 +9,9 @@ Required properties:
>   		 (more may be added later) are:
>   
>   		  "usb1286,204e" (Marvell 8997)
> +		  "usb0cf3,e300" (Qualcomm QCA6174A)
> +		  "usb04ca,301a" (Qualcomm QCA6174A (Lite-On))

Nit: the USB device binding states that those leading zeroes in the VIDs 
should be suppressed.

Otherwise, thanks for the tip-off - I hadn't realised that OOB interrupt 
support was fully implemented already. I'll have to give this a go for 
the Realtek module in one of my TV boxes :)

Robin.

> +
>   
>   Also, vendors that use btusb may have device additional properties, e.g:
>   Documentation/devicetree/bindings/net/marvell-bt-8xxx.txt
> 

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

* Re: [PATCH 2/3] dt-bindings: net: btusb: add QCA6174A IDs
  2019-02-22 21:57   ` Robin Murphy
@ 2019-02-22 22:10     ` Brian Norris
  0 siblings, 0 replies; 9+ messages in thread
From: Brian Norris @ 2019-02-22 22:10 UTC (permalink / raw)
  To: Robin Murphy
  Cc: Heiko Stuebner, Marcel Holtmann, Johan Hedberg, devicetree,
	Linux Bluetooth mailing list, Douglas Anderson, Linux Kernel,
	open list:ARM/Rockchip SoC...,
	Rob Herring, Enric Balletbo i Serra, Rajat Jain,
	Matthias Kaehlcke, linux-arm-kernel

On Fri, Feb 22, 2019 at 1:57 PM Robin Murphy <robin.murphy@arm.com> wrote:
> On 2019-02-22 12:34 am, Brian Norris wrote:
> > +               "usb0cf3,e300" (Qualcomm QCA6174A)
> > +               "usb04ca,301a" (Qualcomm QCA6174A (Lite-On))
>
> Nit: the USB device binding states that those leading zeroes in the VIDs
> should be suppressed.

Ah, thanks. Didn't notice that part. I'll remove the zeroes and resend.

Brian

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

* Re: [PATCH 3/3] arm64: dts: rockchip: move QCA6174A wakeup pin into its USB node
  2019-02-22 18:27   ` Matthias Kaehlcke
@ 2019-02-22 23:16     ` Rajat Jain
  0 siblings, 0 replies; 9+ messages in thread
From: Rajat Jain @ 2019-02-22 23:16 UTC (permalink / raw)
  To: Matthias Kaehlcke
  Cc: Brian Norris, Heiko Stuebner, Marcel Holtmann, Johan Hedberg,
	Rob Herring, Enric Balletbo i Serra, Douglas Anderson,
	devicetree, linux-arm-kernel, linux-rockchip, Bluez mailing list,
	Linux Kernel Mailing List

On Fri, Feb 22, 2019 at 10:27 AM Matthias Kaehlcke <mka@chromium.org> wrote:
>
> On Thu, Feb 21, 2019 at 04:34:03PM -0800, Brian Norris wrote:
> > Currently, we don't coordinate BT USB activity with our handling of the
> > BT out-of-band wake pin, and instead just use gpio-keys. That causes
> > problems because we have no way of distinguishing wake activity due to a
> > BT device (e.g., mouse) vs. the BT controller (e.g., re-configuring wake
> > mask before suspend). This can cause spurious wake events just because
> > we, for instance, try to reconfigure the host controller's event mask
> > before suspending.
> >
> > We can avoid these synchronization problems by handling the BT wake pin
> > directly in the btusb driver -- for all activity up until BT controller
> > suspend(), we simply listen to normal USB activity (e.g., to know the
> > difference between device and host activity); once we're really ready to
> > suspend the host controller, there should be no more host activity, and
> > only *then* do we unmask the GPIO interrupt.
> >
> > This is already supported by btusb; we just need to describe the wake
> > pin in the right node.
> >
> > We list 2 compatible properties, since both PID/VID pairs show up on
> > Scarlet devices, and they're both essentially identical QCA6174A-based
> > modules.
> >
> > Also note that the polarity was wrong before: Qualcomm implemented WAKE
> > as active high, not active low. We only got away with this because
> > gpio-keys always reconfigured us as bi-directional edge-triggered.
> >
> > Finally, we have an external pull-up and a level-shifter on this line
> > (we didn't notice Qualcomm's polarity in the initial design), so we
> > can't do pull-down. Switch to pull-none.
> >
> > Signed-off-by: Brian Norris <briannorris@chromium.org>

If it matters, LGTM.

Reviewed-by: Rajat Jain <rajatja@google.com>

> > ---
> > This patch is also required to make this stable, but since it's not
> > really tied to the device tree, and it's an existing bug, I sent it
> > separately:
> >
> >   https://lore.kernel.org/patchwork/patch/1044896/
> >   Subject: Bluetooth: btusb: request wake pin with NOAUTOEN
> >
> >  .../dts/rockchip/rk3399-gru-chromebook.dtsi   | 13 ++++++
> >  .../boot/dts/rockchip/rk3399-gru-scarlet.dtsi | 46 ++++++++++++-------
> >  arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi  | 13 ------
> >  3 files changed, 42 insertions(+), 30 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru-chromebook.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-gru-chromebook.dtsi
> > index c400be64170e..931640e9aed4 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3399-gru-chromebook.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk3399-gru-chromebook.dtsi
> > @@ -200,6 +200,19 @@
> >               pinctrl-0 = <&bl_en>;
> >               pwm-delay-us = <10000>;
> >       };
> > +
> > +     gpio_keys: gpio-keys {
> > +             compatible = "gpio-keys";
> > +             pinctrl-names = "default";
> > +             pinctrl-0 = <&bt_host_wake_l>;
> > +
> > +             wake_on_bt: wake-on-bt {
> > +                     label = "Wake-on-Bluetooth";
> > +                     gpios = <&gpio0 3 GPIO_ACTIVE_LOW>;
> > +                     linux,code = <KEY_WAKEUP>;
> > +                     wakeup-source;
> > +             };
> > +     };
> >  };
> >
> >  &ppvar_bigcpu {
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi
> > index fc50b3ef758c..3e2196c08473 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi
> > @@ -175,6 +175,21 @@
> >               pinctrl-0 = <&dmic_en>;
> >               wakeup-delay-ms = <250>;
> >       };
> > +
> > +     gpio_keys: gpio-keys {
> > +             compatible = "gpio-keys";
> > +             pinctrl-names = "default";
> > +             pinctrl-0 = <&pen_eject_odl>;
> > +
> > +             pen-insert {
> > +                     label = "Pen Insert";
> > +                     /* Insert = low, eject = high */
> > +                     gpios = <&gpio1 1 GPIO_ACTIVE_LOW>;
> > +                     linux,code = <SW_PEN_INSERTED>;
> > +                     linux,input-type = <EV_SW>;
> > +                     wakeup-source;
> > +             };
> > +     };
> >  };
> >
> >  /* pp900_s0 aliases */
> > @@ -328,20 +343,6 @@ camera: &i2c7 {
> >               <400000000>;
> >  };
> >
> > -&gpio_keys {
> > -     pinctrl-names = "default";
> > -     pinctrl-0 = <&bt_host_wake_l>, <&pen_eject_odl>;
> > -
> > -     pen-insert {
> > -             label = "Pen Insert";
> > -             /* Insert = low, eject = high */
> > -             gpios = <&gpio1 1 GPIO_ACTIVE_LOW>;
> > -             linux,code = <SW_PEN_INSERTED>;
> > -             linux,input-type = <EV_SW>;
> > -             wakeup-source;
> > -     };
> > -};
> > -
> >  &i2c_tunnel {
> >       google,remote-bus = <0>;
> >  };
> > @@ -437,8 +438,19 @@ camera: &i2c7 {
> >       status = "okay";
> >  };
> >
> > -&wake_on_bt {
> > -     gpios = <&gpio1 2 GPIO_ACTIVE_LOW>;
> > +&usb_host0_ohci {
> > +     #address-cells = <1>;
> > +     #size-cells = <0>;
> > +
> > +     qca_bt: bt@1 {
> > +             compatible = "usb0cf3,e300", "usb04ca,301a";
> > +             reg = <1>;
> > +             pinctrl-names = "default";
> > +             pinctrl-0 = <&bt_host_wake_l>;
> > +             interrupt-parent = <&gpio1>;
> > +             interrupts = <2 IRQ_TYPE_LEVEL_HIGH>;
> > +             interrupt-names = "wakeup";
> > +     };
> >  };
>
> I didn't know it was possible to configure (certain) USB devices
> through the DT, neat!
>
> >
> >  /* PINCTRL OVERRIDES */
> > @@ -455,7 +467,7 @@ camera: &i2c7 {
> >  };
> >
> >  &bt_host_wake_l {
> > -     rockchip,pins = <1 2 RK_FUNC_GPIO &pcfg_pull_up>;
> > +     rockchip,pins = <1 2 RK_FUNC_GPIO &pcfg_pull_none>;
> >  };
> >
> >  &ec_ap_int_l {
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
> > index ea607a601a86..da03fa9c5662 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
> > @@ -269,19 +269,6 @@
> >               #clock-cells = <0>;
> >       };
> >
> > -     gpio_keys: gpio-keys {
> > -             compatible = "gpio-keys";
> > -             pinctrl-names = "default";
> > -             pinctrl-0 = <&bt_host_wake_l>;
> > -
> > -             wake_on_bt: wake-on-bt {
> > -                     label = "Wake-on-Bluetooth";
> > -                     gpios = <&gpio0 3 GPIO_ACTIVE_LOW>;
> > -                     linux,code = <KEY_WAKEUP>;
> > -                     wakeup-source;
> > -             };
> > -     };
> > -
> >       max98357a: max98357a {
> >               compatible = "maxim,max98357a";
> >               pinctrl-names = "default";
>
> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>

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

end of thread, other threads:[~2019-02-22 23:17 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-22  0:34 [PATCH 1/3] Bluetooth: btusb: add QCA6174A compatible properties Brian Norris
2019-02-22  0:34 ` [PATCH 2/3] dt-bindings: net: btusb: add QCA6174A IDs Brian Norris
2019-02-22 18:05   ` Matthias Kaehlcke
2019-02-22 21:57   ` Robin Murphy
2019-02-22 22:10     ` Brian Norris
2019-02-22  0:34 ` [PATCH 3/3] arm64: dts: rockchip: move QCA6174A wakeup pin into its USB node Brian Norris
2019-02-22 18:27   ` Matthias Kaehlcke
2019-02-22 23:16     ` Rajat Jain
2019-02-22 18:04 ` [PATCH 1/3] Bluetooth: btusb: add QCA6174A compatible properties Matthias Kaehlcke

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).