Hi Wells Lu, thanks for your patch! On Wed, Oct 27, 2021 at 10:55 AM Wells Lu <wellslutw@xxxxxxxxx> wrote: > + properties: > + pins: > + description: | > + Define pins which are used by pinctrl node's client device. > + > + It consists of one or more integers which represents the config > + setting for corresponding pin. Please use macro SPPCTL_IOPAD to > + define the integers for pins. > + > + The first argument of the macro is pin number, the second is pin > + type, the third is type of GPIO, the last is default output state > + of GPIO. > + $ref: /schemas/types.yaml#/definitions/uint32-array > + > + function: > + description: | > + Define pin-function which is used by pinctrl node's client device. > + The name should be one of string in the following enumeration. > + $ref: "/schemas/types.yaml#/definitions/string" > + enum: [ SPI_FLASH, SPI_FLASH_4BIT, SPI_NAND, CARD0_EMMC, SD_CARD, > + UA0, FPGA_IFX, HDMI_TX, LCDIF, USB0_OTG, USB1_OTG ] > + > + groups: > + description: | > + Define pin-group in a specified pin-function. > + The name should be one of string in the following enumeration. > + $ref: "/schemas/types.yaml#/definitions/string" > + enum: [ SPI_FLASH1, SPI_FLASH2, SPI_FLASH_4BIT1, SPI_FLASH_4BIT2, > + SPI_NAND, CARD0_EMMC, SD_CARD, UA0, FPGA_IFX, HDMI_TX1, > + HDMI_TX2, HDMI_TX3, LCDIF, USB0_OTG, USB1_OTG ] Is it possible to use Documentation/devicetree/bindings/pinctrl/pinmux-node.yaml for this like other drivers do? > + zero_func: > + description: | > + Disabled pins which are not used by pinctrl node's client device. > + $ref: /schemas/types.yaml#/definitions/uint32-array I have never seen this before. Can't you just use pin control hogs for this so the pin controller just take care of these pins? > + allOf: > + - if: > + properties: > + function: > + enum: > + - SPI_FLASH > + then: > + properties: > + groups: > + enum: > + - SPI_FLASH1 > + - SPI_FLASH2 > + - if: > + properties: > + function: > + enum: > + - SPI_FLASH_4BIT > + then: > + properties: > + groups: > + enum: > + - SPI_FLASH_4BIT1 > + - SPI_FLASH_4BIT2 > + - if: > + properties: > + function: > + enum: > + - SPI_NAND > + then: > + properties: > + groups: > + enum: > + - SPI_NAND > + - if: > + properties: > + function: > + enum: > + - CARD0_EMMC > + then: > + properties: > + groups: > + enum: > + - CARD0_EMMC > + - if: > + properties: > + function: > + enum: > + - SD_CARD > + then: > + properties: > + groups: > + enum: > + - SD_CARD > + - if: > + properties: > + function: > + enum: > + - UA0 > + then: > + properties: > + groups: > + enum: > + - UA0 > + - if: > + properties: > + function: > + enum: > + - FPGA_IFX > + then: > + properties: > + groups: > + enum: > + - FPGA_IFX > + - if: > + properties: > + function: > + enum: > + - HDMI_TX > + then: > + properties: > + groups: > + enum: > + - HDMI_TX1 > + - HDMI_TX2 > + - HDMI_TX3 > + - if: > + properties: > + function: > + enum: > + - LCDIF > + then: > + properties: > + groups: > + enum: > + - LCDIF > + - if: > + properties: > + function: > + enum: > + - USB0_OTG > + then: > + properties: > + groups: > + enum: > + - USB0_OTG > + - if: > + properties: > + function: > + enum: > + - USB1_OTG > + then: > + properties: > + groups: > + enum: > + - USB1_OTG This looks complex to me, I need feedback from bindings people on this. > + pins_uart0: pins_uart0 { > + function = "UA0"; > + groups = "UA0"; > + }; > + > + pins_uart1: pins_uart1 { > + pins = < > + SPPCTL_IOPAD(11,SPPCTL_PCTL_G_PMUX,MUXF_UA1_TX,0) > + SPPCTL_IOPAD(10,SPPCTL_PCTL_G_PMUX,MUXF_UA1_RX,0) > + SPPCTL_IOPAD(7,SPPCTL_PCTL_G_GPIO,0,SPPCTL_PCTL_L_OUT) > + >; > + }; This first looks like two ways to do the same thing? UART0 uses strings for group + function and uart1 control individual pins. Is it possible to just do it one way? I think the pins = <...> scheme includes also multiplexing settings and then it should be named pinmux = <...>: Please read Documentation/devicetree/bindings/pinctrl/pinmux-node.yaml closely. Yours, Linus Walleij