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@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx ...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
