RE: [PATCH][v3] ARM: imx: pinctrl-imx: imx7d: add support for iomuxc lpsr

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux