Re: [PATCH] arm64: dts: qcom: sm8450: add uart20 node

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 2 May 2022 at 20:59, Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxx> 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@xxxxxxxxxx>
> > Reviewed-by: Vinod Koul <vkoul@xxxxxxxxxx>
> > ---
> >  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




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux