lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAA8EJpq=3iK9S99j_ez7F-G1X+e9PdEyn6vGZmxKjEpnEs_W4g@mail.gmail.com>
Date: Tue, 22 Oct 2024 16:47:25 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Janaki Ramaiah Thota <quic_janathot@...cinc.com>
Cc: Bjorn Andersson <andersson@...nel.org>, Konrad Dybcio <konradybcio@...nel.org>, 
	Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	quic_mohamull@...cinc.com, quic_hbandi@...cinc.com, 
	linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] arm64: dts: qcom: qcs6490-rb3gen2: enable Bluetooth

On Tue, 22 Oct 2024 at 13:06, Janaki Ramaiah Thota
<quic_janathot@...cinc.com> wrote:
>
>
>
> On 10/10/2024 6:09 PM, Dmitry Baryshkov wrote:
> > On Thu, Oct 10, 2024 at 04:21:07PM GMT, Janaki Ramaiah Thota wrote:
> >> Add Bluetooth and UART7 support for qcs6490-rb3gen2.
> >>
> >> Signed-off-by: Janaki Ramaiah Thota <quic_janathot@...cinc.com>
> >> ---
> >>   arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 122 ++++++++++++++++++-
> >>   1 file changed, 121 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> >> index 0d45662b8028..c0bc44be7dd4 100644
> >> --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> >> +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> >> @@ -1,6 +1,6 @@
> >>   // SPDX-License-Identifier: BSD-3-Clause
> >>   /*
> >> - * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
> >> + * Copyright (c) 2023-2024 Qualcomm Innovation Center, Inc. All rights reserved.
> >>    */
> >>
> >>   /dts-v1/;
> >> @@ -32,6 +32,8 @@
> >>
> >>      aliases {
> >>              serial0 = &uart5;
> >> +            bluetooth0 = &bluetooth;
> >> +            serial1 = &uart7;
> >>      };
> >>
> >>      chosen {
> >> @@ -688,6 +690,39 @@
> >>      status = "okay";
> >>   };
> >>
> >
> > Please review the file carefully and find the right spot for these
> > parts.
> >
>
> Thanks for the review, corrected in [PATCH v3].
>
> >> +&qup_uart7_cts {
> >> +    /*
> >> +     * Configure a bias-bus-hold on CTS to lower power
> >> +     * usage when Bluetooth is turned off. Bus hold will
> >> +     * maintain a low power state regardless of whether
> >> +     * the Bluetooth module drives the pin in either
> >> +     * direction or leaves the pin fully unpowered.
> >> +     */
> >> +    bias-bus-hold;
> >> +};
> >> +
> >> +&qup_uart7_rts {
> >> +    /* We'll drive RTS, so no pull */
> >> +    drive-strength = <2>;
> >> +    bias-disable;
> >> +};
> >> +
> >> +&qup_uart7_rx {
> >> +    /*
> >> +     * Configure a pull-up on RX. This is needed to avoid
> >> +     * garbage data when the TX pin of the Bluetooth module is
> >> +     * in tri-state (module powered off or not driving the
> >> +     * signal yet).
> >> +     */
> >> +    bias-pull-up;
> >> +};
> >> +
> >> +&qup_uart7_tx {
> >> +    /* We'll drive TX, so no pull */
> >> +    drive-strength = <2>;
> >> +    bias-disable;
> >> +};
> >> +
> >>   &qupv3_id_0 {
> >>      status = "okay";
> >>   };
> >> @@ -719,12 +754,97 @@
> >>   &tlmm {
> >>      gpio-reserved-ranges = <32 2>, /* ADSP */
> >>                             <48 4>; /* NFC */
> >> +    bt_en: bt-en-state {
> >> +            pins = "gpio85";
> >> +            function = "gpio";
> >> +            output-low;
> >> +            bias-disable;
> >> +    };
> >> +
> >> +    qup_uart7_sleep_cts: qup-uart7-sleep-cts-state {
> >> +            pins = "gpio28";
> >> +            function = "gpio";
> >> +            /*
> >> +             * Configure a bias-bus-hold on CTS to lower power
> >> +             * usage when Bluetooth is turned off. Bus hold will
> >> +             * maintain a low power state regardless of whether
> >> +             * the Bluetooth module drives the pin in either
> >> +             * direction or leaves the pin fully unpowered.
> >> +             */
> >> +            bias-bus-hold;
> >> +    };
> >> +
> >> +    qup_uart7_sleep_rts: qup-uart7-sleep-rts-state {
> >> +            pins = "gpio29";
> >> +            function = "gpio";
> >> +            /*
> >> +             * Configure pull-down on RTS. As RTS is active low
> >> +             * signal, pull it low to indicate the BT SoC that it
> >> +             * can wakeup the system anytime from suspend state by
> >> +             * pulling RX low (by sending wakeup bytes).
> >> +             */
> >> +            bias-pull-down;
> >> +    };
> >> +
> >> +    qup_uart7_sleep_rx: qup-uart7-sleep-rx-state {
> >> +            pins = "gpio31";
> >> +            function = "gpio";
> >> +            /*
> >> +             * Configure a pull-up on RX. This is needed to avoid
> >> +             * garbage data when the TX pin of the Bluetooth module
> >> +             * is floating which may cause spurious wakeups.
> >> +             */
> >> +            bias-pull-up;
> >> +    };
> >> +
> >> +    qup_uart7_sleep_tx: qup-uart7-sleep-tx-state {
> >> +            pins = "gpio30";
> >> +            function = "gpio";
> >> +            /*
> >> +             * Configure pull-up on TX when it isn't actively driven
> >> +             * to prevent BT SoC from receiving garbage during sleep.
> >> +             */
> >> +            bias-pull-up;
> >> +    };
> >> +
> >> +    sw_ctrl: sw-ctrl-state {
> >> +            pins = "gpio86";
> >> +            function = "gpio";
> >> +            bias-pull-down;
> >> +    };
> >>   };
> >>
> >>   &uart5 {
> >>      status = "okay";
> >>   };
> >>
> >> +&uart7 {
> >> +    status = "okay";
> >> +    /delete-property/interrupts;
> >> +    interrupts-extended = <&intc GIC_SPI 608 IRQ_TYPE_LEVEL_HIGH>,
> >> +                            <&tlmm 31 IRQ_TYPE_EDGE_FALLING>;
> >
> > Why? Is it a UART interrupt? Or a BT-related interrupt?
> >
>
> it is HS UART interrupt for BT.

I don't see it being handled by the UART driver. It if is a BT
interrupt, it should go to the BT node.

>
> >> +    pinctrl-names = "default", "sleep";
> >> +    pinctrl-1 = <&qup_uart7_sleep_cts>, <&qup_uart7_sleep_rts>,
> >> +                    <&qup_uart7_sleep_tx>, <&qup_uart7_sleep_rx>;
> >> +
> >> +    bluetooth: bluetooth {
> >> +            compatible = "qcom,wcn6750-bt";
> >
> > Please use pwrseq and define the PMU unit.
> >
>
> we are in process of migrating to pwrseq, mean while can we merge this
> change ?

No, why? Please implement the recommended way to handle the hardware, pwrseq.

>
> >> +            pinctrl-names = "default";
> >> +            pinctrl-0 = <&bt_en>, <&sw_ctrl>;
> >> +            enable-gpios = <&tlmm 85 GPIO_ACTIVE_HIGH>;
> >> +            swctrl-gpios = <&tlmm 86 GPIO_ACTIVE_HIGH>;
> >> +            vddaon-supply = <&vreg_s7b_0p972>;
> >> +            vddbtcxmx-supply = <&vreg_s7b_0p972>;
> >> +            vddrfacmn-supply = <&vreg_s7b_0p972>;
> >> +            vddrfa0p8-supply = <&vreg_s7b_0p972>;
> >> +            vddrfa1p7-supply = <&vreg_s1b_1p872>;
> >> +            vddrfa1p2-supply = <&vreg_s8b_1p272>;
> >> +            vddrfa2p2-supply = <&vreg_s1c_2p19>;
> >> +            vddasd-supply = <&vreg_l11c_2p8>;
> >> +            max-speed = <3200000>;
> >> +    };
> >> +};
> >> +
> >>   &usb_1 {
> >>      status = "okay";
> >>   };
> >> --
> >> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> >> a Linux Foundation Collaborative Project
> >>
> >
>
> --
> Thanks
> Janakiram
>


-- 
With best wishes
Dmitry

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ