On Mon, 2 May 2022 at 22:01, Dmitry Baryshkov
<dmitry.baryshkov@xxxxxxxxxx> wrote:
>
> 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.
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
