On 02/12/2022 01:50, Doug Anderson wrote: > Hi, > > On Fri, Nov 18, 2022 at 8:42 AM Krzysztof Kozlowski > <krzysztof.kozlowski@xxxxxxxxxx> wrote: >> >> DT schema expects TLMM pin configuration nodes to be named with >> '-state' suffix and their optional children with '-pins' suffix. >> >> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> >> >> --- >> >> Cc: Doug Anderson <dianders@xxxxxxxxxxxx> >> >> Tested on Qualcomm RB3. Please kndly test a bit more on other devices. >> This should not have an functional impact. > > Added Rob Clark and Fritz who are the only people I know that still do > anything with cheza. > > >> - wcd_intr_default: wcd_intr_default { >> + wcd_intr_default: wcd-intr-default-state { >> pins = <54>; > > Not new to your patch, but I'm surprised it truly works to use an > integer for a pin? How does it know that 54 is an integer and not a > string??? Maybe it passes the checks as phandle... Anyway, I'll fix it in separate patch, assuming this is GPIO54. On schematics I see CODEC_INT1_N GPIO_54. > > >> &qup_uart3_default { >> - pinmux { >> - pins = "gpio41", "gpio42", "gpio43", "gpio44"; >> + cts-rts-pins { >> + pins = "gpio41", "gpio42"; >> function = "qup3"; >> }; >> }; > > FWIW, I would have expected that the SoC dtsi file would get a "4-pin" > definition (similar to what you did with qup_uart6_4pin) and then we'd > use that here. Sure. > > >> qup_uart6_4pin: qup-uart6-4pin-state { >> - >> - cts-pins { >> + qup_uart6_4pin_cts: cts-pins { >> pins = "gpio45"; >> function = "qup6"; >> - bias-pull-down; > > After your patch, where is the above bias set for cheza, db845c, > oneplus, shift-axolotl, ...? > > >> }; >> >> - rts-tx-pins { >> + qup_uart6_4pin_rts_tx: rts-tx-pins { >> pins = "gpio46", "gpio47"; >> function = "qup6"; >> - drive-strength = <2>; >> - bias-disable; > > After your patch, where is the above bias / drive-strength set? They don't use 4-pin setup. If they use, I would assume they will override the entries just like sdm850 boards (where I override it to set these). Alternatively I can keep it in DTSI, but it is not really property of the SoC. > > >> }; >> >> - rx-pins { >> + qup_uart6_4pin_rx: rx-pins { >> pins = "gpio48"; >> function = "qup6"; >> - bias-pull-up; > > After your patch, where is the above bias set? Best regards, Krzysztof