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

2014-10-08 Thread Marek Vasut
On Thursday, October 09, 2014 at 02:50:31 AM, Nikolay Dimitrov wrote:
> Hi Marek,
> 
> On 10/06/2014 05:00 PM, Marek Vasut wrote:
> > On Monday, October 06, 2014 at 02:50:17 PM, Nikolay Dimitrov wrote:
> >> Hi Marek, Sean,
> >> 
> >> Do you have any comments on the PHY signal driving conflict and reset
> >> timing issue (see below)?
> > 
> > I don't, I sense expert knowledge from you. Now we need to wait for the
> > authors.
> > 
> > I spent this weekend showing Novena board off at a local conference and
> > somehow it became the center of attention without much help ;-) I see
> > Novena is like the epitome of awesomeness :)
> 
> Well, after signing NDAs with our own blood for years, everyone seems to
> be happy to see a product that's just designed to be hacked (and to hack
> other gadgets with it, of course).

Yes, I agree the Novena has extreme potential there.

Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2014-10-08 Thread Nikolay Dimitrov

Hi Marek,

On 10/06/2014 05:00 PM, Marek Vasut wrote:

On Monday, October 06, 2014 at 02:50:17 PM, Nikolay Dimitrov wrote:

Hi Marek, Sean,

Do you have any comments on the PHY signal driving conflict and reset
timing issue (see below)?


I don't, I sense expert knowledge from you. Now we need to wait for the authors.

I spent this weekend showing Novena board off at a local conference and somehow
it became the center of attention without much help ;-) I see Novena is like the
epitome of awesomeness :)


Well, after signing NDAs with our own blood for years, everyone seems to 
be happy to see a product that's just designed to be hacked (and to hack 
other gadgets with it, of course).


Regards,
Nikolay
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2014-10-06 Thread Marek Vasut
On Monday, October 06, 2014 at 05:35:57 PM, Sean Cross wrote:
> 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.

OK, let me first post a V5 of the patch and then base all the changes on that, 
all right ?

Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2014-10-06 Thread Sean Cross

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 */
  

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

2014-10-06 Thread Marek Vasut
On Monday, October 06, 2014 at 02:50:17 PM, Nikolay Dimitrov wrote:
> Hi Marek, Sean,
> 
> Do you have any comments on the PHY signal driving conflict and reset
> timing issue (see below)?

I don't, I sense expert knowledge from you. Now we need to wait for the authors.

I spent this weekend showing Novena board off at a local conference and somehow 
it became the center of attention without much help ;-) I see Novena is like the
epitome of awesomeness :)

Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2014-10-06 Thread Nikolay Dimitrov

Hi Marek, Sean,

Do you have any comments on the PHY signal driving conflict and reset 
timing issue (see below)?


On 09/24/2014 05:46 AM, Nikolay Dimitrov wrote:

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.

...


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);
}


Kind regards,
Nikolay
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2014-09-27 Thread Nikolay Dimitrov

Hi Marek,

Here are my last findings:

On 09/23/2014 01:18 PM, Marek Vasut wrote:
> +static void novena_spl_setup_iomux_sdhc(void)
> +{
> +  imx_iomux_v3_setup_multiple_pads(usdhc2_pads, ARRAY_SIZE(usdhc2_pads));
> +  imx_iomux_v3_setup_multiple_pads(usdhc3_pads, ARRAY_SIZE(usdhc3_pads));
> +
> +  /* Micro SD card detect */
> +  gpio_direction_input(IMX_GPIO_NR(7, 0));
> +
> +  /* Big SD card detect */
> +  gpio_direction_input(IMX_GPIO_NR(1, 4));
> +}

I think that you mentioned elsewhere in the source, that microSD card 
doesn't have CD. Also, GPIO7_IO00 is not connected to anything.



> +#define NOVENA_USB_HUB_RESET  IMX_GPIO_NR(7, 12)

Can you please double-check this one, it seems that GPIO7_IO12 is 
actually connected as PCIE_PWRON on the schematic.



> +/*
> + * USB
> + */
> +#ifdef CONFIG_USB_EHCI_MX6
> +int board_ehci_hcd_init(int port)
> +{
> +  /* Reset USB hub */
> +  if (port == 1) {
> +  gpio_set_value(NOVENA_USB_HUB_RESET, 0);
> +  mdelay(2);
> +  gpio_set_value(NOVENA_USB_HUB_RESET, 1);
> +  }
> +  return 0;
> +}
> +#endif

Can you verify this - USB hubs seems to be reset not by this pin, but 
via RESETBMCU.



> +#define NOVENA_AUDIO_PWRONIMX_GPIO_NR(5, 17)

This signal is not connected to the audio power switch, because R30A is 
marked as DNP. The audio power switch seems to be controlled by 
KEY_ROW1, which is not configured by uboot, I think.



> +#define NOVENA_HDMI_GHOST_HPD IMX_GPIO_NR(5, 4)
...
> +static void novena_spl_setup_iomux_video(void)
> +{
> +  imx_iomux_v3_setup_multiple_pads(hdmi_pads, ARRAY_SIZE(hdmi_pads));
> +  gpio_direction_input(NOVENA_HDMI_GHOST_HPD);
> +}

Is there any added value of having this pin initialized? It should be 
already configured as input on reset.



> + * Environment is on MMC, starting at offset 64KiB from start of the 
card.

...
> +#define CONFIG_ENV_OFFSET (512 * 1024)

Comments and code say different things about the ENV offset - which one 
is the correct value?



I don't have any more comments on v4. Sorry it took so long, but it 
takes ages to cross-check the code, schematic and datasheets, you 
know... Thanks for your patience. Please tell me if I can help more.


Kind regards,
Nikolay
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2014-09-27 Thread Marek Vasut
On Saturday, September 27, 2014 at 06:46:24 PM, Sean Cross wrote:
> I asked Bunnie about this, and the routing is 50 ohms.
> 
> On 24 September, 2014 5:40:59 pm GMT+08:00, 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).

Fixed.

[...]

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

Good point, moved the second mux below the GPIO settings.

But what about the reset timing?

Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2014-09-27 Thread Sean Cross
I asked Bunnie about this, and the routing is 50 ohms. 

On 24 September, 2014 5:40:59 pm GMT+08:00, 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 ?
>
>> > +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 i

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

2014-09-24 Thread Marek Vasut
On Wednesday, September 24, 2014 at 01:03:11 PM, Nikolay Dimitrov wrote:
> Hi Marek,
> 
> On 9/24/2014 12:37 PM, Marek Vasut wrote:
> > Also, can you please review the entire thing so I don't have to retest
> > and resend the patch every day with one minor modification each time?
> > Thanks Best regards, Marek Vasut
> 
> Sorry about that :(. I definitely understand. Unfortunately it just
> happens that I have limited hours to help mostly during the night, so I
> decided to send immediately my findings instead of keeping them for
> myself for days/weeks. I'll definitely consider your request for my next
> comments.

Thanks, please put together one consistent email so I can address all the stuff 
in bulk.

Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2014-09-24 Thread Nikolay Dimitrov

Hi Marek,

On 9/24/2014 12:37 PM, Marek Vasut wrote:
Also, can you please review the entire thing so I don't have to retest 
and resend the patch every day with one minor modification each time? 
Thanks Best regards, Marek Vasut 


Sorry about that :(. I definitely understand. Unfortunately it just 
happens that I have limited hours to help mostly during the night, so I 
decided to send immediately my findings instead of keeping them for 
myself for days/weeks. I'll definitely consider your request for my next 
comments.


Kind regards,
Nikolay

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2014-09-24 Thread Marek Vasut
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 ?

> > +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 */

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

2014-09-24 Thread Marek Vasut
On Wednesday, September 24, 2014 at 01:57:50 AM, Nikolay Dimitrov wrote:
> Hi Marek,
> 
> Some comments about SPI:
> 
> On 09/23/2014 01:18 PM, Marek Vasut wrote:
> > +/*
> > + * SPI
> > + */
> > +#ifdef CONFIG_MXC_SPI
> > +static iomux_v3_cfg_t ecspi1_pads[] = {
> > +   /* SS1 */
> > +   MX6_PAD_EIM_D19__GPIO3_IO19   | MUX_PAD_CTRL(SPI_PAD_CTRL),
> > +   MX6_PAD_EIM_D17__ECSPI1_MISO | MUX_PAD_CTRL(SPI_PAD_CTRL),
> > +   MX6_PAD_EIM_D18__ECSPI1_MOSI | MUX_PAD_CTRL(SPI_PAD_CTRL),
> > +   MX6_PAD_EIM_D16__ECSPI1_SCLK | MUX_PAD_CTRL(SPI_PAD_CTRL),
> > +};
> > +
> > +static void novena_spl_setup_iomux_spi(void)
> > +{
> > +   imx_iomux_v3_setup_multiple_pads(ecspi1_pads, ARRAY_SIZE(ecspi1_pads));
> > +   gpio_direction_output(CONFIG_SF_DEFAULT_CS, 1);
> > +}
> > +#else
> > +static void novena_spl_setup_iomux_spi(void) {}
> > +#endif
> 
> I checked the schematic and didn't saw any usage of ECSPI1. In addition,
> these pads (EIM D16/17/18/19) are used by other interfaces. Can you
> please also double-check this?

Good find, I'll zap this part. Sean, why was this in the original Novena U-Boot 
source?

Also, can you please review the entire thing so I don't have to retest and 
resend the patch every day with one minor modification each time?

Thanks

Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2014-09-23 Thread 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))

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

2014-09-23 Thread Nikolay Dimitrov

Hi Marek,

Some comments about SPI:

On 09/23/2014 01:18 PM, Marek Vasut wrote:

+/*
+ * SPI
+ */
+#ifdef CONFIG_MXC_SPI
+static iomux_v3_cfg_t ecspi1_pads[] = {
+   /* SS1 */
+   MX6_PAD_EIM_D19__GPIO3_IO19   | MUX_PAD_CTRL(SPI_PAD_CTRL),
+   MX6_PAD_EIM_D17__ECSPI1_MISO | MUX_PAD_CTRL(SPI_PAD_CTRL),
+   MX6_PAD_EIM_D18__ECSPI1_MOSI | MUX_PAD_CTRL(SPI_PAD_CTRL),
+   MX6_PAD_EIM_D16__ECSPI1_SCLK | MUX_PAD_CTRL(SPI_PAD_CTRL),
+};
+
+static void novena_spl_setup_iomux_spi(void)
+{
+   imx_iomux_v3_setup_multiple_pads(ecspi1_pads, ARRAY_SIZE(ecspi1_pads));
+   gpio_direction_output(CONFIG_SF_DEFAULT_CS, 1);
+}
+#else
+static void novena_spl_setup_iomux_spi(void) {}
+#endif


I checked the schematic and didn't saw any usage of ECSPI1. In addition, 
these pads (EIM D16/17/18/19) are used by other interfaces. Can you 
please also double-check this?


Kind regards,
Nikolay
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot