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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
