[U-Boot] [PATCH V4] ARM: mx6: Add support for Kosagi Novena

Sean Cross
2014-10-06T15:35:57Z

Hi Marek, Nikolay,

I thought I responded, but it might have gotten lost. I'm sorry about that.

I've taken a closer look, and responded below.

On 24/9/2014 5:40 PM, Marek Vasut wrote:
> On Wednesday, September 24, 2014 at 04:46:42 AM, Nikolay Dimitrov wrote:
>> Hi Marek,
>>
>> Following are some comments about FEC Ethernet:
>>
>> On 09/23/2014 01:18 PM, Marek Vasut wrote:
>>> +#define ENET_PAD_CTRL \
>>> + (PAD_CTL_PKE | PAD_CTL_PUE | \
>>> + PAD_CTL_PUS_100K_UP | PAD_CTL_SPEED_MED | \
>>> + PAD_CTL_DSE_40ohm | PAD_CTL_HYS)
>>> +
>> PAD_CTL_SPEED_MED falls on reserved bits (7-6).
>>
>> Special note: PAD_CTL_DSE_40ohm is defined as 0b110, which is documented
>> as "37/27 ohm @2.5V". I didn't saw any notes on the PHY spec or the
>> Novena schematic about the routing guidelines of the RGMII data lines,
>> but I can extrapolate that if the 125 MHz reference clock is routed as
>> 50-ohm line (this one is documented in the datasheet), then the data
>> lines are probably routed in a similar way. So the DSE value should be
>> 0b100, which is "57/43 ohm @ 2.5V", which in turn is defined in the
>> headers as PAD_CTL_DSE_60ohm (this is either a bug in the header, or I'm
>> reading an updated FSL PDF).
> Sean, can you comment on this ?

The pads probably don't support setting speed, as they're a high-speed
RG-MII pads are high-speed anyway. PAD_CTL_SPEED_MED can be removed.

In a pre-release version of the FSL PDF I have, 0b110 is indeed
documented as 40 ohm. You're correct in that it should be 50 ohm, which
the same version of the document doesn't have. The closes is 48 ohm,
which is 0b101. The older doc says 0b100 is 60 ohm, and there are no
distinctions about voltages as there are in Rev. 1 of the pdf, which is
probably what you're looking at.

>>> +static void novena_spl_setup_iomux_enet(void)
>>> +{
>>> + imx_iomux_v3_setup_multiple_pads(enet_pads1, ARRAY_SIZE(enet_pads1));
>>> + imx_iomux_v3_setup_multiple_pads(enet_pads2, ARRAY_SIZE(enet_pads2));
>>> +
>>> + gpio_direction_output(IMX_GPIO_NR(3, 23), 0);
>>> + gpio_direction_output(IMX_GPIO_NR(6, 30), 1);
>>> + gpio_direction_output(IMX_GPIO_NR(6, 25), 1);
>>> + gpio_direction_output(IMX_GPIO_NR(6, 27), 1);
>>> + gpio_direction_output(IMX_GPIO_NR(6, 28), 1);
>>> + gpio_direction_output(IMX_GPIO_NR(6, 29), 1);
>>> + gpio_direction_output(IMX_GPIO_NR(6, 24), 1);
>>> +}
>> I think that setting the iomuxes immediately one after the other doesn't
>> achieve the intended goal. After the 2nd iomux setup, the pads are
>> connected to the FEC, not to the GPIOs.
>>
>> There's one more issue here - when you get the PHY out of reset, you'll
>> have to both de-assert the RESET line while keeping the strapping
>> signals stable so the PHY can read them, but at the same time the PHY RX
>> pins are becoming outputs and driving the same lines, which is not good.
>> I'm giving a proposal how to fix this in the end.
>>
>> > +int board_early_init_f(void)
>> > +{
>> > +#if defined(CONFIG_VIDEO_IPUV3)
>> > + setup_display();
>> > +#endif
>> > +
>> > + /* Bring Ethernet PHY out of reset. */
>> > + gpio_set_value(IMX_GPIO_NR(3, 23), 1);
>> > +
>> > + return 0;
>> > +}
>>
>> Getting the PHY out of reset at this point causes unpredictable reset
>> timing - e.g. you can't guarantee how much time the chip was held in
>> reset. The PHY datasheet is quite unclear about this reset timing, the
>> only mentioned time is min. 10ms after power-on, and nothing about RESET
>> assertion/de-assertion. I can throw a wild guess that the reset pulse
>> should have similar duration, e.g. 10ms.
>>
>> Here's my proposal how to fix both (line driving conflict and reset
>> timing) issues:
>>
>> #define ENET_PHY_CFG_PC \
>> (PAD_CTL_HYS | PAD_CTL_PUS_22K_UP | PAD_CTL_PUE | PAD_CTL_PKE)
>>
>> static iomux_v3_cfg_t enet_pads1[] = {
>> MX6_PAD_ENET_MDIO__ENET_MDIO | MUX_PAD_CTRL(ENET_PAD_CTRL),
>> MX6_PAD_ENET_MDC__ENET_MDC | MUX_PAD_CTRL(ENET_PAD_CTRL),
>>
>> MX6_PAD_RGMII_TXC__RGMII_TXC | MUX_PAD_CTRL(ENET_PAD_CTRL),
>> MX6_PAD_RGMII_TD0__RGMII_TD0 | MUX_PAD_CTRL(ENET_PAD_CTRL),
>> MX6_PAD_RGMII_TD1__RGMII_TD1 | MUX_PAD_CTRL(ENET_PAD_CTRL),
>> MX6_PAD_RGMII_TD2__RGMII_TD2 | MUX_PAD_CTRL(ENET_PAD_CTRL),
>> MX6_PAD_RGMII_TD3__RGMII_TD3 | MUX_PAD_CTRL(ENET_PAD_CTRL),
>> MX6_PAD_RGMII_TX_CTL__RGMII_TX_CTL| MUX_PAD_CTRL(ENET_PAD_CTRL),
>> MX6_PAD_ENET_REF_CLK__ENET_TX_CLK | MUX_PAD_CTRL(ENET_PAD_CTRL),
>>
>> /* pin 35, PHY_AD2 */
>> MX6_PAD_RGMII_RXC__GPIO6_IO30 | MUX_PAD_CTRL(ENET_PHY_CFG_PC),
>> /* pin 32, MODE0 */
>> MX6_PAD_RGMII_RD0__GPIO6_IO25 | MUX_PAD_CTRL(ENET_PHY_CFG_PC),
>> /* pin 31, MODE1 */
>> MX6_PAD_RGMII_RD1__GPIO6_IO27 | MUX_PAD_CTRL(ENET_PHY_CFG_PC),
>> /* pin 28, MODE2 */
>> MX6_PAD_RGMII_RD2__GPIO6_IO28 | MUX_PAD_CTRL(ENET_PHY_CFG_PC),
>> /* pin 27, MODE3 */
>> MX6_PAD_RGMII_RD3__GPIO6_IO29 | MUX_PAD_CTRL(ENET_PHY_CFG_PC),
>> /* pin 33, CLK125_EN */
>> MX6_PAD_RGMII_RX_CTL__GPIO6_IO24| MUX_PAD_CTRL(ENET_PHY_CFG_PC),
>>
>> /* PHY nRST */
>> MX6_PAD_EIM_D23__GPIO3_IO23 | MUX_PAD_CTRL(NO_PAD_CTRL),
>> };
>>
>> static void novena_spl_setup_iomux_enet(void)
>> {
>> imx_iomux_v3_setup_multiple_pads(enet_pads1, ARRAY_SIZE(enet_pads1));
>>
>> /* Assert Ethernet PHY nRST */
>> gpio_direction_output(IMX_GPIO_NR(3, 23), 0);
>>
>> /* Using imx6 internal pull-ups to drive PHY config pins during PHY
>> reset */
>> gpio_direction_input(IMX_GPIO_NR(6, 30)); /* PHY_AD2 = 1 */
>> gpio_direction_input(IMX_GPIO_NR(6, 25)); /* MODE0 = 1 */
>> gpio_direction_input(IMX_GPIO_NR(6, 27)); /* MODE1 = 1 */
>> gpio_direction_input(IMX_GPIO_NR(6, 28)); /* MODE2 = 1 */
>> gpio_direction_input(IMX_GPIO_NR(6, 29)); /* MODE3 = 1 */
>> gpio_direction_input(IMX_GPIO_NR(6, 24)); /* CLK125_EN = 1 */
>>
>> /* Interpreting fig.8 from the PHY datasheet */
>> mdelay(10);
>>
>> /* De-assert Ethernet PHY nRST */
>> gpio_set_value(IMX_GPIO_NR(3, 23), 1);
>>
>> /* After PHY is configured, we can finally connect our FEC */
>> imx_iomux_v3_setup_multiple_pads(enet_pads2, ARRAY_SIZE(enet_pads2));
>>
>> /* PHY datasheet recommends on p.53 to wait at least 100us before using
>> MII, so we enforce this delay here */
>> udelay(100);
>> }
>>
>> And I would ditch the PHY un-reset code from board_early_init_f().
> Again, this is a question for the hardware guys. Also, it would be nice if you
> could prepare a patch, so you get the credit.
The manual says to wait at least 10ms between the supply voltage
stabilizing and deasserting reset. I think a delay of 10ms is
unnecessarily long, because VGEN5 (which generates the 2.5V line) comes
on as soon as the PMIC is stable. I think it would make more sense to
remove the mdelay(10), then reconfigure the iomux to enet_pads2 after
the 100us delay.

Sean