Hi All, I will sent a new patch series including documentation for pad group id encoding in Documentation/devicetree/bindings/pinctrl/fsl,imx7d-pinctrl.txt Thanks for your comments :) - Adrian > -----Original Message----- > From: Zhi Li [mailto:lznuaa@xxxxxxxxx] > Sent: Thursday, July 16, 2015 11:22 AM > To: Shawn Guo > Cc: Alonso Lazcano Adrian-B38018; devicetree@xxxxxxxxxxxxxxx; Li Frank- > B20596; Garg Nitin-B37173; linus.walleij@xxxxxxxxxx; linux- > gpio@xxxxxxxxxxxxxxx; robh+dt@xxxxxxxxxx; shawn.guo@xxxxxxxxxx; Huang > Yongcai-B20788; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; Gong Yibin-B38343 > Subject: Re: [PATCH][v3] ARM: imx: pinctrl-imx: imx7d: add support for iomuxc > lpsr > > On Thu, Jul 16, 2015 at 10:50 AM, Shawn Guo <shawnguo@xxxxxxxxxx> wrote: > > Adrian, > > > > Please configure your email client to use bottom-posting rather than > > top-posting. > > > > On Wed, Jul 15, 2015 at 03:55:59PM +0000, Alonso Adrian wrote: > >> [Adrian] IOMUXC and IOMUXC-LPSR are in different base address; each > controller provides SW_PAD_CTL (CONF_REG) and SW_MUX_CTL (MUX_REG) > but only IOMUXC provides SELECT_INPUT registers for pad daisy chain > configuration, so pads from IOMUXC-LPSR that need daisy chain settings need > to access the IOMUXC register space address to access SELECT_INPUT. > >> > > > > Thanks for the explanation. > > > >> [Adrian] One disadvantage that we found if we created two driver instances > for IOMUXC and IOMUXC-LPSR controllers is that in the device tree machine > files "end-users" could be creating pinctrl nodes mixing pads from different > IOMUXC controllers resulting that pad that doesn't belong to that domain > controller it will not be configured properly. > >> > >> For example a valid pad configuration for I2C1 could use pads as shown > below: > >> &iomuxc { > >> pinctrl_i2c1_1: i2c1grp-1 { > >> fsl,pins = < > >> MX7D_PAD_GPIO1_IO04__I2C1_SCL /* IOMUXC-LPSR domain > */ > >> MX7D_PAD_I2C1_SDA__I2C1_SDA /* IOMUXC domain */ > >> >; > >> }; > >> }; > >> > >> By extending pinctrl-imx to support both controllers the above > >> configuration is supported, Otherwise using two driver instances "end- > users" will need to do something like: > >> > >> &iomuxc { > >> pinctrl_i2c1_1: i2c1grp-1 { > >> fsl,pins = < > >> MX7D_PAD_I2C1_SDA__I2C1_SDA /* IOMUXC domain */ > >> >; > >> }; > >> }; > >> > >> &iomuxc_lpsr { > >> pinctrl_i2c1_1: i2c1grp-1 { > >> fsl,pins = < > >> MX7D_PAD_GPIO1_IO04__I2C1_SCL /* IOMUXC-LPSR domain > */ > >> >; > >> }; > >> }; > >> > >> And this is something that we will like to avoid so "end-user" don't have to > worry about and configure pads as they are used to. > >> > > > > This is exactly how hardware is designed, and having device tree > > describe how hardware is laid out is the right thing to do. In that > > case, when people check the device tree source with a i.MX7D Reference > > Manual on hands, they can easily understand how hardware is > > represented in device tree. > > > > Also, your example with pins for a single device across IOMUXC and > > IOMUXC-LPSR should be the rare case except GPIO pins. > > Although cross IOMUX and IOMUXC-LPSR is rare, it will be headache if there > are one boards. > > Some SD card, ENET have reset\wp\cd pin. > > pinctrl_usdhc1: usdhc1grp { > fsl,pins = < > MX7D_PAD_SD1_CMD__SD1_CMD 0x59 > MX7D_PAD_SD1_CLK__SD1_CLK 0x19 > MX7D_PAD_SD1_DATA0__SD1_DATA0 0x59 > MX7D_PAD_SD1_DATA1__SD1_DATA1 0x59 > MX7D_PAD_SD1_DATA2__SD1_DATA2 0x59 > MX7D_PAD_SD1_DATA3__SD1_DATA3 0x59 > MX7D_PAD_SD1_CD_B__GPIO5_IO0 > 0x59 /* CD */ > MX7D_PAD_SD1_WP__GPIO5_IO1 > 0x59 /* WP */ > MX7D_PAD_SD1_RESET_B__GPIO5_IO2 > 0x59 /* vmmc */ > >; > > Customer may choose below pin to IOMUX-lPSR. > > MX7D_PAD_SD1_CD_B__GPIO5_IO0 > MX7D_PAD_SD1_WP__GPIO5_IO1 > MX7D_PAD_SD1_RESET_B__GPIO5_IO2 > > FEC have Phy Reset GPIO, which possibly choose IOMUX-lPSR. > > > > > For your I2C1 > > example, a sane hardware design would chose one group among the > > following instead of the one you put above. > > > > MX7D_PAD_I2C1_SCL__I2C1_SCL > > MX7D_PAD_I2C1_SDA__I2C1_SDA > > > > MX7D_PAD_UART1_RX_DATA__I2C1_SCL > > MX7D_PAD_UART1_TX_DATA__I2C1_SDA > > > > MX7D_PAD_GPIO1_IO04__I2C1_SCL > > MX7D_PAD_GPIO1_IO05__I2C1_SDA > > > > So this is not really a problem for us to implement two IOMUXC instances. > > The only problem we need to solve is the select input register access > > for IOMUXC-LPSR driver. I have some idea for that, i.e. honestly > > represent how hardware is laid out. I will give more details on that > > tomorrow. > > It is very easy to make mistake. > According to pin NAME macro, it is not easy to distinguish between IOMUX and > IOMUX-LPSR. > > Internal kernel tree use two pinctrl instance. > > It will be simple if we think IOMUX and IOMUX-LPSR together, and one module > have two group registers. > > best regards > Frank Li > > > > >> [Adrian] For the artificial encoding of pad group id's for > >> IOMUXC-LPSR, I think there is no other way to do it (limited > >> imagination here :P); Pad group id's are extracted from MUX_REG offset > address (pad_id = mux_reg / 4) but when combining both IOMUXC and > IOMUXC-LPSR Pad group ID's overlap so end up encoding the pad_id for > IOMUXC-LPSR to resolve the proper pad group ID. > > > > One big missing piece from your patch is the update to bindings > > document > > Documentation/devicetree/bindings/pinctrl/fsl,imx-pinctrl.txt. Have > > you thought about how you will document all these magics and hacks you > have done here? > > > > Shawn ��.n��������+%������w��{.n����z�{��ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
