On Fri, Feb 22, 2019 at 10:27 AM Matthias Kaehlcke <mka@xxxxxxxxxxxx> 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@xxxxxxxxxxxx> If it matters, LGTM. Reviewed-by: Rajat Jain <rajatja@xxxxxxxxxx> > > --- > > 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@xxxxxxxxxxxx>
