* [PATCH] arm64: dts: qcom: sm8450: add uart20 node
@ 2022-05-01 19:54 Dmitry Baryshkov
2022-05-02 5:01 ` Vinod Koul
2022-05-02 17:59 ` Konrad Dybcio
0 siblings, 2 replies; 6+ messages in thread
From: Dmitry Baryshkov @ 2022-05-01 19:54 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Rob Herring, Krzysztof Kozlowski
Cc: linux-arm-msm, devicetree, Vinod Koul
Add device tree node for uart20, which is typically used for Bluetooth attachment.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
arch/arm64/boot/dts/qcom/sm8450.dtsi | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
index 4fcb6e2b096b..8b9d9c2cd02c 100644
--- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
@@ -996,6 +996,19 @@ spi20: spi@894000 {
status = "disabled";
};
+ uart20: serial@894000 {
+ compatible = "qcom,geni-uart";
+ reg = <0 0x00894000 0 0x4000>;
+ clock-names = "se";
+ clocks = <&gcc GCC_QUPV3_WRAP2_S5_CLK>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&qup_uart20_default>;
+ interrupts = <GIC_SPI 587 IRQ_TYPE_LEVEL_HIGH>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+ };
+
i2c21: i2c@898000 {
compatible = "qcom,geni-i2c";
reg = <0x0 0x00898000 0x0 0x4000>;
@@ -2757,6 +2770,15 @@ qup_uart7_tx: qup-uart7-tx {
drive-strength = <2>;
bias-disable;
};
+
+ qup_uart20_default: qup-uart20-default {
+ mux {
+ pins = "gpio76", "gpio77",
+ "gpio78", "gpio79";
+ function = "qup20";
+ };
+ };
+
};
apps_smmu: iommu@15000000 {
--
2.35.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] arm64: dts: qcom: sm8450: add uart20 node
2022-05-01 19:54 [PATCH] arm64: dts: qcom: sm8450: add uart20 node Dmitry Baryshkov
@ 2022-05-02 5:01 ` Vinod Koul
2022-05-02 17:59 ` Konrad Dybcio
1 sibling, 0 replies; 6+ messages in thread
From: Vinod Koul @ 2022-05-02 5:01 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Andy Gross, Bjorn Andersson, Rob Herring, Krzysztof Kozlowski,
linux-arm-msm, devicetree
On 01-05-22, 22:54, Dmitry Baryshkov wrote:
> Add device tree node for uart20, which is typically used for Bluetooth attachment.
Reviewed-by: Vinod Koul <vkoul@kernel.org>
--
~Vinod
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] arm64: dts: qcom: sm8450: add uart20 node
2022-05-01 19:54 [PATCH] arm64: dts: qcom: sm8450: add uart20 node Dmitry Baryshkov
2022-05-02 5:01 ` Vinod Koul
@ 2022-05-02 17:59 ` Konrad Dybcio
2022-05-02 19:01 ` Dmitry Baryshkov
1 sibling, 1 reply; 6+ messages in thread
From: Konrad Dybcio @ 2022-05-02 17:59 UTC (permalink / raw)
To: Dmitry Baryshkov, Andy Gross, Bjorn Andersson, Rob Herring,
Krzysztof Kozlowski
Cc: linux-arm-msm, devicetree, Vinod Koul
On 1.05.2022 21:54, Dmitry Baryshkov wrote:
> Add device tree node for uart20, which is typically used for Bluetooth attachment.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Reviewed-by: Vinod Koul <vkoul@kernel.org>
> ---
> arch/arm64/boot/dts/qcom/sm8450.dtsi | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> index 4fcb6e2b096b..8b9d9c2cd02c 100644
> --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> @@ -996,6 +996,19 @@ spi20: spi@894000 {
> status = "disabled";
> };
>
> + uart20: serial@894000 {
I think it should come before SPI alphabetically?
> + compatible = "qcom,geni-uart";
> + reg = <0 0x00894000 0 0x4000>;
> + clock-names = "se";
> + clocks = <&gcc GCC_QUPV3_WRAP2_S5_CLK>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&qup_uart20_default>;
No sleep state?
> + interrupts = <GIC_SPI 587 IRQ_TYPE_LEVEL_HIGH>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + status = "disabled";
> + };
> +
> i2c21: i2c@898000 {
> compatible = "qcom,geni-i2c";
> reg = <0x0 0x00898000 0x0 0x4000>;
> @@ -2757,6 +2770,15 @@ qup_uart7_tx: qup-uart7-tx {
> drive-strength = <2>;
> bias-disable;
> };
> +
> + qup_uart20_default: qup-uart20-default {
> + mux {
Please drop the unnecessary mux{} here.
> + pins = "gpio76", "gpio77",
> + "gpio78", "gpio79";
I think these could fit into a single 100-char-long line>?
> + function = "qup20";
Are there no default properties for this setup? I think boards that don't use standard Qualcomm connectivity setups (like Bluetooth on this specific UART) are rather scarce and it'd be more convenient to keep a standard setting here and override it where need be instead of copy-pasting the same thing over and over in 95-100% of the boards.
Konrad
> + };
> + };
> +
> };
>
> apps_smmu: iommu@15000000 {
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] arm64: dts: qcom: sm8450: add uart20 node
2022-05-02 17:59 ` Konrad Dybcio
@ 2022-05-02 19:01 ` Dmitry Baryshkov
2022-05-02 19:49 ` Dmitry Baryshkov
0 siblings, 1 reply; 6+ messages in thread
From: Dmitry Baryshkov @ 2022-05-02 19:01 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Andy Gross, Bjorn Andersson, Rob Herring, Krzysztof Kozlowski,
linux-arm-msm, devicetree, Vinod Koul
On Mon, 2 May 2022 at 20:59, Konrad Dybcio <konrad.dybcio@somainline.org> wrote:
>
>
>
> On 1.05.2022 21:54, Dmitry Baryshkov wrote:
> > Add device tree node for uart20, which is typically used for Bluetooth attachment.
> >
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > Reviewed-by: Vinod Koul <vkoul@kernel.org>
> > ---
> > arch/arm64/boot/dts/qcom/sm8450.dtsi | 22 ++++++++++++++++++++++
> > 1 file changed, 22 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> > index 4fcb6e2b096b..8b9d9c2cd02c 100644
> > --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> > @@ -996,6 +996,19 @@ spi20: spi@894000 {
> > status = "disabled";
> > };
> >
> > + uart20: serial@894000 {
> I think it should come before SPI alphabetically?
Argh. I sorted it using the label!
>
> > + compatible = "qcom,geni-uart";
> > + reg = <0 0x00894000 0 0x4000>;
> > + clock-names = "se";
> > + clocks = <&gcc GCC_QUPV3_WRAP2_S5_CLK>;
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&qup_uart20_default>;
> No sleep state?
No, uarts do not provide a sleep state.
>
> > + interrupts = <GIC_SPI 587 IRQ_TYPE_LEVEL_HIGH>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + status = "disabled";
> > + };
> > +
> > i2c21: i2c@898000 {
> > compatible = "qcom,geni-i2c";
> > reg = <0x0 0x00898000 0x0 0x4000>;
> > @@ -2757,6 +2770,15 @@ qup_uart7_tx: qup-uart7-tx {
> > drive-strength = <2>;
> > bias-disable;
> > };
> > +
> > + qup_uart20_default: qup-uart20-default {
> > + mux {
> Please drop the unnecessary mux{} here.
Ack.
>
> > + pins = "gpio76", "gpio77",
> > + "gpio78", "gpio79";
> I think these could fit into a single 100-char-long line>?
I'll check.
>
> > + function = "qup20";
> Are there no default properties for this setup? I think boards that don't use standard Qualcomm connectivity setups (like Bluetooth on this specific UART) are rather scarce and it'd be more convenient to keep a standard setting here and override it where need be instead of copy-pasting the same thing over and over in 95-100% of the boards.
I see your point. Let's do this.
>
> Konrad
>
> > + };
> > + };
> > +
> > };
> >
> > apps_smmu: iommu@15000000 {
> >
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] arm64: dts: qcom: sm8450: add uart20 node
2022-05-02 19:01 ` Dmitry Baryshkov
@ 2022-05-02 19:49 ` Dmitry Baryshkov
2022-05-02 21:18 ` Konrad Dybcio
0 siblings, 1 reply; 6+ messages in thread
From: Dmitry Baryshkov @ 2022-05-02 19:49 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Andy Gross, Bjorn Andersson, Rob Herring, Krzysztof Kozlowski,
linux-arm-msm, devicetree, Vinod Koul
On Mon, 2 May 2022 at 22:01, Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On Mon, 2 May 2022 at 20:59, Konrad Dybcio <konrad.dybcio@somainline.org> wrote:
> >
> >
> >
> > On 1.05.2022 21:54, Dmitry Baryshkov wrote:
> > > Add device tree node for uart20, which is typically used for Bluetooth attachment.
> > >
> > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > Reviewed-by: Vinod Koul <vkoul@kernel.org>
> > > ---
> > > arch/arm64/boot/dts/qcom/sm8450.dtsi | 22 ++++++++++++++++++++++
> > > 1 file changed, 22 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> > > index 4fcb6e2b096b..8b9d9c2cd02c 100644
> > > --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> > > @@ -996,6 +996,19 @@ spi20: spi@894000 {
> > > status = "disabled";
> > > };
> > >
> > > + uart20: serial@894000 {
> > I think it should come before SPI alphabetically?
>
> Argh. I sorted it using the label!
>
> >
> > > + compatible = "qcom,geni-uart";
> > > + reg = <0 0x00894000 0 0x4000>;
> > > + clock-names = "se";
> > > + clocks = <&gcc GCC_QUPV3_WRAP2_S5_CLK>;
> > > + pinctrl-names = "default";
> > > + pinctrl-0 = <&qup_uart20_default>;
> > No sleep state?
>
> No, uarts do not provide a sleep state.
I've checked other dts. Usually the sleep state is provided by the
board dts rather than the SoC's dtsi.
>
> >
> > > + interrupts = <GIC_SPI 587 IRQ_TYPE_LEVEL_HIGH>;
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > + status = "disabled";
> > > + };
> > > +
> > > i2c21: i2c@898000 {
> > > compatible = "qcom,geni-i2c";
> > > reg = <0x0 0x00898000 0x0 0x4000>;
> > > @@ -2757,6 +2770,15 @@ qup_uart7_tx: qup-uart7-tx {
> > > drive-strength = <2>;
> > > bias-disable;
> > > };
> > > +
> > > + qup_uart20_default: qup-uart20-default {
> > > + mux {
> > Please drop the unnecessary mux{} here.
>
> Ack.
>
> >
> > > + pins = "gpio76", "gpio77",
> > > + "gpio78", "gpio79";
> > I think these could fit into a single 100-char-long line>?
>
> I'll check.
>
> >
> > > + function = "qup20";
> > Are there no default properties for this setup? I think boards that don't use standard Qualcomm connectivity setups (like Bluetooth on this specific UART) are rather scarce and it'd be more convenient to keep a standard setting here and override it where need be instead of copy-pasting the same thing over and over in 95-100% of the boards.
>
> I see your point. Let's do this.
Well, comparing with other SoC dtsi shows that most of them declare
pins&functions in the dtsi and leave bias/ details to the board.dts
(despite code duplication). So let's follow that approach.
>
> >
> > Konrad
> >
> > > + };
> > > + };
> > > +
> > > };
> > >
> > > apps_smmu: iommu@15000000 {
> > >
>
>
>
> --
> With best wishes
> Dmitry
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] arm64: dts: qcom: sm8450: add uart20 node
2022-05-02 19:49 ` Dmitry Baryshkov
@ 2022-05-02 21:18 ` Konrad Dybcio
0 siblings, 0 replies; 6+ messages in thread
From: Konrad Dybcio @ 2022-05-02 21:18 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Andy Gross, Bjorn Andersson, Rob Herring, Krzysztof Kozlowski,
linux-arm-msm, devicetree, Vinod Koul
On 2.05.2022 21:49, Dmitry Baryshkov wrote:
> On Mon, 2 May 2022 at 22:01, Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> wrote:
>>
>> On Mon, 2 May 2022 at 20:59, Konrad Dybcio <konrad.dybcio@somainline.org> wrote:
>>>
>>>
>>>
>>> On 1.05.2022 21:54, Dmitry Baryshkov wrote:
>>>> Add device tree node for uart20, which is typically used for Bluetooth attachment.
>>>>
>>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>>> Reviewed-by: Vinod Koul <vkoul@kernel.org>
>>>> ---
>>>> arch/arm64/boot/dts/qcom/sm8450.dtsi | 22 ++++++++++++++++++++++
>>>> 1 file changed, 22 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
>>>> index 4fcb6e2b096b..8b9d9c2cd02c 100644
>>>> --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
>>>> +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
>>>> @@ -996,6 +996,19 @@ spi20: spi@894000 {
>>>> status = "disabled";
>>>> };
>>>>
>>>> + uart20: serial@894000 {
>>> I think it should come before SPI alphabetically?
>>
>> Argh. I sorted it using the label!
>>
>>>
>>>> + compatible = "qcom,geni-uart";
>>>> + reg = <0 0x00894000 0 0x4000>;
>>>> + clock-names = "se";
>>>> + clocks = <&gcc GCC_QUPV3_WRAP2_S5_CLK>;
>>>> + pinctrl-names = "default";
>>>> + pinctrl-0 = <&qup_uart20_default>;
>>> No sleep state?
>>
>> No, uarts do not provide a sleep state.
>
> I've checked other dts. Usually the sleep state is provided by the
> board dts rather than the SoC's dtsi.
Weird. Perhaps we could consider centralizing that, since it's common(-ish) in the end?
>
>>
>>>
>>>> + interrupts = <GIC_SPI 587 IRQ_TYPE_LEVEL_HIGH>;
>>>> + #address-cells = <1>;
>>>> + #size-cells = <0>;
>>>> + status = "disabled";
>>>> + };
>>>> +
>>>> i2c21: i2c@898000 {
>>>> compatible = "qcom,geni-i2c";
>>>> reg = <0x0 0x00898000 0x0 0x4000>;
>>>> @@ -2757,6 +2770,15 @@ qup_uart7_tx: qup-uart7-tx {
>>>> drive-strength = <2>;
>>>> bias-disable;
>>>> };
>>>> +
>>>> + qup_uart20_default: qup-uart20-default {
>>>> + mux {
>>> Please drop the unnecessary mux{} here.
>>
>> Ack.
>>
>>>
>>>> + pins = "gpio76", "gpio77",
>>>> + "gpio78", "gpio79";
>>> I think these could fit into a single 100-char-long line>?
>>
>> I'll check.
>>
>>>
>>>> + function = "qup20";
>>> Are there no default properties for this setup? I think boards that don't use standard Qualcomm connectivity setups (like Bluetooth on this specific UART) are rather scarce and it'd be more convenient to keep a standard setting here and override it where need be instead of copy-pasting the same thing over and over in 95-100% of the boards.
>>
>> I see your point. Let's do this.
>
> Well, comparing with other SoC dtsi shows that most of them declare
> pins&functions in the dtsi and leave bias/ details to the board.dts
> (despite code duplication). So let's follow that approach.
Eh, you're right, but I'd still argue it's really a thing that used to be more relevant in the past though, especially as newer devices seem to get closer and closer to Qualcomm reference designs.. Maybe Bjorn could give some input on this matter?
Konrad
>
>>
>>>
>>> Konrad
>>>
>>>> + };
>>>> + };
>>>> +
>>>> };
>>>>
>>>> apps_smmu: iommu@15000000 {
>>>>
>>
>>
>>
>> --
>> With best wishes
>> Dmitry
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-05-02 21:18 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-05-01 19:54 [PATCH] arm64: dts: qcom: sm8450: add uart20 node Dmitry Baryshkov
2022-05-02 5:01 ` Vinod Koul
2022-05-02 17:59 ` Konrad Dybcio
2022-05-02 19:01 ` Dmitry Baryshkov
2022-05-02 19:49 ` Dmitry Baryshkov
2022-05-02 21:18 ` Konrad Dybcio
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox