Re: [PATCH 01/10] dt-bindings: pinctrl: Add bindings for Actions S900 SoC
From: Manivannan Sadhasivam
Date: Mon Feb 19 2018 - 12:14:06 EST
Hi Andreas,
On Sat, Feb 17, 2018 at 10:03:53PM +0100, Andreas Färber wrote:
> Am 17.02.2018 um 21:44 schrieb Manivannan Sadhasivam:
> > Add pinctrl bindings for Actions Semi S900 SoC
> >
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>
> > ---
> > .../bindings/pinctrl/actions,s900-pinctrl.txt | 171 +++++++++++++++++++++
> > 1 file changed, 171 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/pinctrl/actions,s900-pinctrl.txt
> >
> > diff --git a/Documentation/devicetree/bindings/pinctrl/actions,s900-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/actions,s900-pinctrl.txt
> > new file mode 100644
> > index 000000000000..ee7d3ecd9bd9
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pinctrl/actions,s900-pinctrl.txt
> > @@ -0,0 +1,171 @@
> > +Actions Semi S900 Pin Controller
>
> So you are targeting to have one bindings file per SoC?
>
Yes. Similar to what Qcom drivers are doing! Having a single
binding will look messy if we add support for other
OWL family SoC's like S500, S700...
> > +
> > +This binding describes the pin controller found in the S900 SoC.
> > +
> > +Required Properties:
> > +
> > +- compatible: Should be "actions,s900-pinctrl"
> > +- reg: Should contain the register base address and size of
> > + the pin controller.
> > +- clocks: phandle of the clock feeding the pin controller
> > +
> > +Please refer to pinctrl-bindings.txt in this directory for details of the
> > +common pinctrl bindings used by client devices, including the meaning of the
> > +phrase "pin configuration node".
> > +
> > +The pin configuration nodes act as a container for an arbitrary number of
> > +subnodes. Each of these subnodes represents some desired configuration for a
> > +pin, a group, or a list of pins or groups. This configuration can include the
> > +mux function to select on those group(s), and various pin configuration
> > +parameters, such as pull-up, drive strength, etc.
> > +
> > +PIN CONFIGURATION NODES:
> > +
> > +The name of each subnode is not important; all subnodes should be enumerated
> > +and processed purely based on their content.
> > +
> > +Each subnode only affects those parameters that are explicitly listed. In
> > +other words, a subnode that lists a mux function but no pin configuration
> > +parameters implies no information about any pin configuration parameters.
> > +Similarly, a pin subnode that describes a pullup parameter implies no
> > +information about e.g. the mux function.
> > +
> > +Pinmux functions are available only for the pin groups while pinconf
> > +parameters are available for both pin groups and individual pins.
> > +
> > +The following generic properties as defined in pinctrl-bindings.txt are valid
> > +to specify in a pin configuration subnode:
> > +
> > +Required Properties:
> > +
> > +- pins: An array of strings, each string containing the name of a pin.
> > + These pins are used for selecting the pull control and schmitt
> > + trigger parameters. The following are the list of pins
> > + available:
> > +
> > + ETH_TXD0, ETH_TXD1, ETH_TXEN, ETH_RXER, ETH_CRS_DV,
> > + ETH_RXD1, ETH_RXD0, ETH_REF_CLK, ETH_MDC, ETH_MDIO,
> > + SIRQ0, SIRQ1, SIRQ2, I2S_D0, I2S_BCLK0, I2S_LRCLK0,
> > + I2S_MCLK0, I2S_D1, I2S_BCLK1, I2S_LRCLK1, I2S_MCLK1,
> > + PCM1_IN, PCM1_CLK, PCM1_SYNC, PCM1_OUT, ERAM_A5,
> > + ERAM_A6, ERAM_A7, ERAM_A8, ERAM_A9, ERAM_A10, ERAM_A11,
> > + LVDS_OEP, LVDS_OEN, LVDS_ODP, LVDS_ODN, LVDS_OCP,
> > + LVDS_OCN, LVDS_OBP, LVDS_OBN, LVDS_OAP, LVDS_OAN,
> > + LVDS_EEP, LVDS_EEN, LVDS_EDP, LVDS_EDN, LVDS_ECP,
> > + LVDS_ECN, LVDS_EBP, LVDS_EBN, LVDS_EAP, LVDS_EAN,
> > + SD0_D0, SD0_D1, SD0_D2, SD0_D3, SD1_D0, SD1_D1,
> > + SD1_D2, SD1_D3, SD0_CMD, SD0_CLK, SD1_CMD, SD1_CLK,
> > + SPI0_SCLK, SPI0_SS, SPI0_MISO, SPI0_MOSI, UART0_RX,
> > + UART0_TX, UART2_RX, UART2_TX, UART2_RTSB, UART2_CTSB,
> > + UART3_RX, UART3_TX, UART3_RTSB, UART3_CTSB, UART4_RX,
> > + UART4_TX, I2C0_SCLK, I2C0_SDATA, I2C1_SCLK, I2C1_SDATA,
> > + I2C2_SCLK, I2C2_SDATA, CSI0_DN0, CSI0_DP0, CSI0_DN1,
> > + CSI0_DP1, CSI0_CN, CSI0_CP, CSI0_DN2, CSI0_DP2, CSI0_DN3,
> > + CSI0_DP3, DSI_DP3, DSI_DN3, DSI_DP1, DSI_DN1, DSI_CP,
> > + DSI_CN, DSI_DP0, DSI_DN0, DSI_DP2, DSI_DN2, SENSOR0_PCLK,
> > + CSI1_DN0,CSI1_DP0,CSI1_DN1, CSI1_DP1, CSI1_CN, CSI1_CP,
> > + SENSOR0_CKOUT, NAND0_D0, NAND0_D1, NAND0_D2, NAND0_D3,
> > + NAND0_D4, NAND0_D5, NAND0_D6, NAND0_D7, NAND0_DQS,
> > + NAND0_DQSN, NAND0_ALE, NAND0_CLE, NAND0_CEB0, NAND0_CEB1,
> > + NAND0_CEB2, NAND0_CEB3, NAND1_D0, NAND1_D1, NAND1_D2,
> > + NAND1_D3, NAND1_D4, NAND1_D5, NAND1_D6, NAND1_D7, NAND1_DQS,
> > + NAND1_DQSN, NAND1_ALE, NAND1_CLE, NAND1_CEB0, NAND1_CEB1,
> > + NAND1_CEB2, NAND1_CEB3, SGPIO0, SGPIO1, SGPIO2, SGPIO3
>
> Should be lower-case?
>
Okay.
> > +
> > +- groups: An array of strings, each string containing the name of a pin
> > + group. These pin groups are used for selecting the pinmux
> > + functions. The following are the list of pin groups available:
> > +
> > + mfp0_22, mfp0_21_20_eth_mdc, mfp0_21_20_eth_mdio,
> > + mfp0_19_sirq0, mfp0_19_sirq1, mfp0_18_16_eth_txd0,
> > + mfp0_18_16_eth_txd1, mfp0_15_13_eth_txen, mfp0_15_13_eth_rxer,
> > + mfp0_12_11, mfp0_10_8_eth_rxd1, mfp0_10_8_eth_rxd0, mfp0_7_6,
> > + mfp0_5_i2s_d0, mfp0_5_i2s_d1, mfp0_4_3, mfp0_2_i2s0,
> > + mfp0_2_i2s1, mfp0_1_0_pcm1_in_out, mfp0_1_0_pcm1_clk,
> > + mfp0_1_0_pcm1_sync, mfp1_31_29_eram_a5, mfp1_31_29_eram_a6,
> > + mfp1_31_29_eram_a7, mfp1_28_26_eram_a8, mfp1_28_26_eram_a9,
> > + mfp1_28_26_eram_a10, mfp1_25_23, mfp1_22_lvds_oep_odn,
> > + mfp1_22_lvds_ocp_obn, mfp1_22_lvds_oap_oan, mfp1_21_lvds_e,
> > + mfp1_5_4, mfp1_3_1_spi0_ss, mfp1_3_1_spi0_miso, mfp2_23,
> > + mfp2_22, mfp2_21, mfp2_20, mfp2_19_17, mfp2_16_14, mfp2_13_11,
> > + mfp2_10_9, mfp2_8_7, mfp2_6_5, mfp2_4_3, mfp2_2_0, mfp3_27,
> > + mfp3_21_19, mfp3_18_16, mfp3_15, mfp3_14, mfp3_13,
> > + mfp3_12_dsi_dp3_dn1, mfp3_12_dsi_cp_dn0, mfp3_12_dsi_dp2_dn2,
> > + mfp3_11, mfp3_10_nand1_ceb3, mfp3_10_nand1_ceb0, mfp3_9,
> > + mfp3_8
> > +
> > + These pin groups are used for selecting the drive strength
> > + parameters.
> > +
> > + drv0_31_30, drv0_29_28, drv0_27_26, drv0_25_24, drv0_23_22,
> > + drv0_21_20, drv0_19_18, drv0_17_16, drv0_15_14, drv0_13_12,
> > + drv0_11_10, drv0_9_8, drv0_7_6, drv0_5_4, drv0_3_2, drv0_1_0,
> > + drv1_29_28, drv1_27_26, drv1_25_24, drv1_23_22, drv1_21_20,
> > + drv1_19_18, drv1_17_16, drv1_15_14, drv1_13_12, drv1_11_10,
> > + drv1_9_8, drv1_7_6, drv1_5_4, drv2_31_30, drv2_29_28,
> > + drv2_27_26, drv2_21_20
> > +
> > + These pin groups are used for selecting the slew rate
> > + parameters.
> > +
> > + sr0_15, sr0_14, sr0_13, sr0_12, sr0_11, sr0_10, sr0_9, sr0_8,
> > + sr0_7, sr0_6, sr0_5, sr0_4, sr0_3, sr0_2, sr0_1, sr0_0,
> > + sr1_25, sr1_24, sr1_23, sr1_22, sr1_21, sr1_20, sr1_19,
> > + sr1_18, sr2_31, sr2_30, sr2_29, sr2_25
> > +
> > +- function: An array of strings, each string containing the name of the
> > + pinmux functions. These functions can only be selected by
> > + the corresponding pin groups. The following are the list of
> > + pinmux functions available:
> > +
> > + eram, eth_rmii, eth_smii, spi0, spi1, spi2, spi3, sens0,
> > + uart0, uart1, uart2, uart3, uart4, uart5, uart6, i2s0, i2s1,
> > + pcm0, pcm1, jtag, pwm0, pwm1, pwm2, pwm3, pwm4, pwm5, sd0,
> > + sd1, sd2, sd3, i2c0, i2c1, i2c2, i2c3, i2c4, i2c5, lvds,
> > + usb30, usb20, gpu, mipi_csi0, mipi_csi1, mipi_dsi, nand0,
> > + nand1, spdif, sirq0, sirq1, sirq2
> > +
> > +Optional Properties:
> > +
> > +- bias-bus-hold: No arguments. The specified pins should retain the previous
> > + state value.
> > +- bias-high-impedance: No arguments. The specified pins should be configured
> > + as high impedance.
> > +- bias-pull-down: No arguments. The specified pins should be configued as
>
> configured
>
Ack.
> > + pull down.
> > +- bias-pull-up: No arguments. The specified pins should be configued as
>
> ditto
>
Ack.
> > + pull up.
> > +- input-schmitt-enable: No arguments: Enable schmitt trigger for the specified
> > + pins
> > +- input-schmitt-disable: No arguments: Disable schmitt trigger for the specified
> > + pins
> > +- slew-rate: Integer. Sets slew rate for the specified pins.
> > + Valid values are:
> > + <0> - Slow
> > + <1> - Fast
> > +- drive-strength: Integer. Selects the drive strength for the specified
> > + pins.
> > + Valid values are:
> > + <2> - mA
> > + <4> - mA
> > + <8> - mA
> > + <12> - mA
>
> This looks weird and should get a better explanation: <2> - 2 mA?
> Or "... drive strength in mA for ..." and drop "- mA"?
>
How about,
drive-strength: Integer. Selects the drive strength for the specified
pins in mA.
Valid values are:
2
4
8
12
Thanks,
Mani
> > +
> > +Example:
> > +
> > + pinctrl: pinctrl@e01b0000 {
> > + compatible = "actions,s900-pinctrl";
> > + reg = <0x0 0xe01b0000 0x0 0x1000>;
> > + clocks = <&cmu CLK_GPIO>;
> > +
> > + uart2-default: uart2-default {
> > + pinmux {
> > + groups = "mfp1_22_lvds_oep_odn";
> > + function = "uart2";
> > + };
> > + pinconf {
> > + groups = "drv1_27_26";
> > + drive-strength = <12>;
> > + };
> > + };
> > + };
>
> Regards,
> Andreas
>
> --
> SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Felix Imendörffer, Jane Smithard, Graham Norton
> HRB 21284 (AG Nürnberg)