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

[Nikolay Dimitrov]

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).

> +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().

Please tell me what you think.

Kind regards,
Nikolay