From: rajpat@codeaurora.org
To: Matthias Kaehlcke <mka@chromium.org>
Cc: Andy Gross <agross@kernel.org>,
Bjorn Andersson <bjorn.andersson@linaro.org>,
Rob Herring <robh+dt@kernel.org>,
linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org,
devicetree@vger.kernel.org, rnayak@codeaurora.org,
saiprakash.ranjan@codeaurora.org, msavaliy@qti.qualcomm.com,
skakit@codeaurora.org, Roja Rani Yarubandi <rojay@codeaurora.org>
Subject: Re: [PATCH V4 2/4] arm64: dts: sc7280: Add QUPv3 wrapper_0 nodes
Date: Wed, 11 Aug 2021 17:43:48 +0530 [thread overview]
Message-ID: <bdfd39def9a11104c043090d920ef4df@codeaurora.org> (raw)
In-Reply-To: <YP7cmkayoajJ+1yj@google.com>
On 2021-07-26 21:32, Matthias Kaehlcke wrote:
> On Mon, Jul 26, 2021 at 07:10:45PM +0530, Rajesh Patil wrote:
>> From: Roja Rani Yarubandi <rojay@codeaurora.org>
>>
>> Add QUPv3 wrapper_0 DT nodes for SC7280 SoC.
>>
>> Signed-off-by: Roja Rani Yarubandi <rojay@codeaurora.org>
>> Signed-off-by: Rajesh Patil <rajpat@codeaurora.org>
>> ---
>> Changes in V4:
>> - As per Bjorn's comment, added QUP Wrapper_0 nodes
>> other than debug-uart node
>> - Dropped interconnect votes for wrapper_0 node
>>
>> Changes in V3:
>> - Broken the huge V2 patch into 3 smaller patches.
>> 1. QSPI DT nodes
>> 2. QUP wrapper_0 DT nodes
>> 3. QUP wrapper_1 DT nodes
>>
>> Changes in V2:
>> - As per Doug's comments removed pinmux/pinconf subnodes.
>> - As per Doug's comments split of SPI, UART nodes has been done.
>> - Moved QSPI node before aps_smmu as per the order.
>>
>> arch/arm64/boot/dts/qcom/sc7280-idp.dts | 84 ++++
>> arch/arm64/boot/dts/qcom/sc7280.dtsi | 720
>> ++++++++++++++++++++++++++++++++
>> 2 files changed, 804 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dts
>> b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
>> index b0bfd8e..f63cf51 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dts
>> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
>> @@ -358,6 +358,16 @@
>> vdda18-supply = <&vreg_l1c_1p8>;
>> };
>>
>> +&uart7 {
>> + status = "okay";
>> +
>> + /delete-property/interrupts;
>> + interrupts-extended = <&intc GIC_SPI 608 IRQ_TYPE_LEVEL_HIGH>,
>> + <&tlmm 31 IRQ_TYPE_EDGE_FALLING>;
>> + pinctrl-names = "default", "sleep";
>> + pinctrl-1 = <&qup_uart7_sleep_cts>, <&qup_uart7_sleep_rts>,
>> <&qup_uart7_sleep_tx>, <&qup_uart7_sleep_rx>;
>> +};
>> +
>> /* PINCTRL - additions to nodes defined in sc7280.dtsi */
>>
>> &qspi_cs0 {
>> @@ -428,3 +438,77 @@
>> bias-pull-up;
>> };
>> };
>> +&qup_uart7_cts {
>> + /*
>> + * Configure a pull-down on CTS to match the pull of
>> + * the Bluetooth module.
>> + */
>> + bias-pull-down;
>> +};
>> +
>> +&qup_uart7_rts {
>> + /* We'll drive RTS, so no pull */
>> + drive-strength = <2>;
>> + bias-disable;
>> +};
>> +
>> +&qup_uart7_tx {
>> + /* We'll drive TX, 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;
>> +};
>> +
>> +&tlmm {
>> + qup_uart7_sleep_cts: qup-uart7-sleep-cts {
>> + pins = "gpio28";
>> + function = "gpio";
>> + /*
>> + * Configure a pull-down on CTS to match the pull of
>> + * the Bluetooth module.
>> + */
>> + bias-pull-down;
>> + };
>> +
>> + qup_uart7_sleep_rts: qup-uart7-sleep-rts {
>> + 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_tx: qup-uart7-sleep-tx {
>> + 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;
>> + };
>>
>> + qup_uart7_sleep_rx: qup-uart7-sleep-rx {
>> + 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;
>> + };
>> +};
>
> How the patches of this series are split strikes me as a bit odd.
> Supposedly
> this patch adds the QUPv3 wrapper_0 DT nodes for the SC7280, however
> the
> above is the pin configuration for the Bluetooth UART of the SC7280 IDP
> board.
> I don't see a good reason why that should be part of this patch. It
> should be
> a separate change whose subject indicates that it configures the
> Bluetooth UART
> of the SC7280 IDP.
>
Okay will split this up.
> Without this conflation of SoC and board DT it would seem perfectly
> reasonable
> to squash this patch and '[4/4] arm64: dts: sc7280: Add QUPv3 wrapper_1
> nodes'
> into a single one, they are essentially doing the same thing, I see no
> need to
> have different patches for the wrapper 0 and 1 nodes.
Previously when QUP wrapper 0 and wrapper 1 nodes were added in single
patch, we faced some git issues as the patch was huge. Hence we split it
up.
https://partnerissuetracker.corp.google.com/issues/177045897#comment12
next prev parent reply other threads:[~2021-08-11 12:14 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-26 13:40 [PATCH V4 0/4] Add QSPI and QUPv3 DT nodes for SC7280 SoC Rajesh Patil
2021-07-26 13:40 ` [PATCH V4 1/4] arm64: dts: sc7280: Add QSPI node Rajesh Patil
2021-07-26 16:07 ` Matthias Kaehlcke
2021-08-11 12:14 ` rajpat
2021-07-26 13:40 ` [PATCH V4 2/4] arm64: dts: sc7280: Add QUPv3 wrapper_0 nodes Rajesh Patil
2021-07-26 16:02 ` Matthias Kaehlcke
2021-08-11 12:13 ` rajpat [this message]
2021-08-11 14:52 ` Matthias Kaehlcke
2021-08-12 7:13 ` rajpat
2021-08-12 12:49 ` Matthias Kaehlcke
2021-07-27 19:20 ` Stephen Boyd
2021-08-11 12:17 ` rajpat
2021-07-26 13:40 ` [PATCH V4 3/4] arm64: dts: sc7280: Update QUPv3 Debug UART DT node Rajesh Patil
2021-07-26 15:42 ` Matthias Kaehlcke
2021-08-11 12:04 ` rajpat
2021-07-26 13:40 ` [PATCH V4 4/4] arm64: dts: sc7280: Add QUPv3 wrapper_1 nodes Rajesh Patil
2021-07-27 0:14 ` [PATCH V4 0/4] Add QSPI and QUPv3 DT nodes for SC7280 SoC Stephen Boyd
2021-08-11 12:14 ` rajpat
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=bdfd39def9a11104c043090d920ef4df@codeaurora.org \
--to=rajpat@codeaurora.org \
--cc=agross@kernel.org \
--cc=bjorn.andersson@linaro.org \
--cc=devicetree@vger.kernel.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mka@chromium.org \
--cc=msavaliy@qti.qualcomm.com \
--cc=rnayak@codeaurora.org \
--cc=robh+dt@kernel.org \
--cc=rojay@codeaurora.org \
--cc=saiprakash.ranjan@codeaurora.org \
--cc=skakit@codeaurora.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).