Messages in this thread | | | Date | Thu, 22 Apr 2021 20:40:11 +0530 | From | rojay@codeauro ... | Subject | Re: [PATCH] arm64: dts: sc7280: Add qspi, qupv3_0 and qupv3_1 nodes |
| |
Hi Doug,
On 2021-03-12 03:24, Doug Anderson wrote: > Hi, > > On Wed, Mar 10, 2021 at 7:41 PM Roja Rani Yarubandi > <rojay@codeaurora.org> wrote: >> >> +&qspi_cs0 { >> + pinconf { >> + pins = "gpio15"; >> + bias-disable; >> + }; > > The "pinconf" / "pinmux" subnode shouldn't be used for new SoCs. See: > > http://lore.kernel.org/r/CAD=FV=UY_AFRrAY0tef5jP698LEng6oN652LcX3B4nG=aWh0gA@mail.gmail.com > > ...same feedback for this whole patch. >
Ok, will do the changes.
>> + qup_spi0_default: qup-spi0-default { >> + pinmux { >> + pins = "gpio0", "gpio1", >> + "gpio2", "gpio3"; >> + function = "qup00"; >> + }; >> + }; > > Please split these SPI nodes as per the thread above, like: > > tlmm: pinctrl@... { > qup_spi0_data_clk: qup-spi0-data-clk { > pins = "gpio0", "gpio1", "gpio2"; > function = "qup0"; > }; > > qup_spi0_cs: qup-spi0-cs { > pins = "gpio3"; > function = "qup0"; > }; > > qup_spi0_cs_gpio: qup-spi0-cs-gpio { > pins = "gpio3"; > function = "gpio"; > }; > }; > > >> + qup_uart0_default: qup-uart0-default { >> + pinmux { >> + pins = "gpio0", "gpio1", >> + "gpio2", "gpio3"; >> + function = "qup00"; >> + }; >> + }; > > I suspect things would actually be cleaner if we broke the uart lines > up since the boards tend to have to adjust pulls differently for each > line. With the new "no pinconf / pinmux" world it's pretty clean. It's > obviously up to Bjorn, but if it were me I'd request this in the SoC > file: > > qup_uart0_cts: qup-uart0-cts { > pins = "..."; > function = "qup00"; > }; > > qup_uart0_rts: qup-uart0-rts { > pins = "..."; > function = "qup00"; > }; > > qup_uart0_rx: qup-uart0-rx { > pins = "..."; > function = "qup00"; > }; > > qup_uart0_tx: qup-uart0-tx { > pins = "..."; > function = "qup00"; > }; > > ...and now when the board file wants to adjust the pulls they can just > reference each one: > > /* > * Comments about why the UART0 pulls make sense. > * Blah blah blah. > */ > > &qup_uart0_cts { > bias-pull-down; > }; > > &qup_uart0_rts { > drive-strength = <2>; > bias-disable; > }; > > &qup_uart0_rx { > bias-pull-up; > }; > > &qup_uart0_tx { > drive-strength = <2>; > bias-disable; > }; > > >> + qspi: spi@88dc000 { > > I believe the qspi node is sorted incorrectly. When I apply this to > the top of the Qualcomm tree it shows up after the "apps_smmu: > iommu@15000000" node. However: > > 0x088dc000 < 0x15000000 > > ...which means it should be _before_. >
Sure, will move before apps_smmu
Thanks, Roja > -Doug
| |