device tree binding documentation outdated

device tree binding documentation outdated

[Russell King - ARM Linux]

Can someone please point me at the current documentation for the
iMX6DL pinctrl bindings?

I mean the stuff which looks like this:

#define MX6QDL_PAD_ENET_MDC__ESAI_TX5_RX0           0x1e8 0x5b8 0x858 0x2 0x0

which clearly doesn't match:

Required properties for pin configuration node:
- fsl,pins: two integers array, represents a group of pins mux and config
  setting. The format is fsl,pins = <PIN_FUNC_ID CONFIG>, PIN_FUNC_ID is a
  pin working on a specific function, which consists of a tuple of
  <mux_reg conf_reg input_reg mux_val input_val>.  CONFIG is the pad setting
  value like pull-up on this pin.

I thought DT was supposed to make bringing up support on new hardware
easier, but without documentation, I'm frankly stuffed with the cubox-i,
because it's impossible to work out what the right pinctrl settings
should be from what seems to be an undocumented pile of random numbers
stuffed into DT.

Thanks.

device tree binding documentation outdated

[Fabio Estevam]

Hi Russell,

On Thu, Sep 26, 2013 at 4:51 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> Can someone please point me at the current documentation for the
> iMX6DL pinctrl bindings?

Best reference is the mx6dl reference manual (chapter 37 - IOMUXC):
http://cache.freescale.com/files/32bit/doc/ref_manual/IMX6SDLRM.pdf

> I mean the stuff which looks like this:
>
> #define MX6QDL_PAD_ENET_MDC__ESAI_TX5_RX0           0x1e8 0x5b8 0x858 0x2 0x0

Let me try to decode it:

0x1e8: Offset of IOMUXC_SW_MUX_CTL_PAD_ENET_MDC register

0x5b8: Offset of IOMUXC_SW_PAD_CTL_PAD_ENET_MDC register

0x2: This means that the MUX_MODE field selects the ESAI_TX5_RX0 function
in this pad

0x0: SELECT_INPUT not used for this pad

> which clearly doesn't match:
>
> Required properties for pin configuration node:
> - fsl,pins: two integers array, represents a group of pins mux and config
>   setting. The format is fsl,pins = <PIN_FUNC_ID CONFIG>, PIN_FUNC_ID is a
>   pin working on a specific function, which consists of a tuple of
>   <mux_reg conf_reg input_reg mux_val input_val>.  CONFIG is the pad setting
>   value like pull-up on this pin.
>
> I thought DT was supposed to make bringing up support on new hardware
> easier, but without documentation, I'm frankly stuffed with the cubox-i,
> because it's impossible to work out what the right pinctrl settings
> should be from what seems to be an undocumented pile of random numbers
> stuffed into DT.

Let me know if you need help with the iomux settings for cubox-i.

Regards,

Fabio Estevam

device tree binding documentation outdated

[Russell King - ARM Linux]

On Thu, Sep 26, 2013 at 05:29:47PM -0300, Fabio Estevam wrote:
> Hi Russell,
> 
> On Thu, Sep 26, 2013 at 4:51 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > Can someone please point me at the current documentation for the
> > iMX6DL pinctrl bindings?
> 
> Best reference is the mx6dl reference manual (chapter 37 - IOMUXC):
> http://cache.freescale.com/files/32bit/doc/ref_manual/IMX6SDLRM.pdf
> 
> > I mean the stuff which looks like this:
> >
> > #define MX6QDL_PAD_ENET_MDC__ESAI_TX5_RX0           0x1e8 0x5b8 0x858 0x2 0x0
> 
> Let me try to decode it:
> 
> 0x1e8: Offset of IOMUXC_SW_MUX_CTL_PAD_ENET_MDC register
> 
> 0x5b8: Offset of IOMUXC_SW_PAD_CTL_PAD_ENET_MDC register
> 
> 0x2: This means that the MUX_MODE field selects the ESAI_TX5_RX0 function
> in this pad
> 
> 0x0: SELECT_INPUT not used for this pad

Okay, that's relatively straight forward.

> Let me know if you need help with the iomux settings for cubox-i.

Yes please. :)

Here's Rabeeh's preliminary patches against Freescale's 3.0.35 BSP 4.1.0
kernel:

http://download.solid-run.com/pub/solidrun/c1/kernel/initial/

I think I've translated the bulk of them.  I have a number of problems
though.  One of the things I'd like to sort out is the ethernet.  Most
of these convert straight:

+       MX6DL_PAD_ENET_MDIO__ENET_MDIO,
+       MX6DL_PAD_ENET_MDC__ENET_MDC,
+       IOMUX_PAD(0x0650, 0x0268, 5, 0x0000, 0, MX6DL_ENET_PAD_CTRL_PD), /* KEY_ROW4 reset signal */
+       
+       MX6DL_PAD_DI0_PIN2__GPIO_4_18, /* Interrupt */
+       /* RMII */
+       IOMUX_PAD(0x05B4, 0x01E4, 1, 0x0828, 0, PAD_CTL_PKE | PAD_CTL_PUE  |
+                 PAD_CTL_PUS_100K_DOWN), /* MX6DL_PAD_ENET_CRS_DV__ENET_RX_EN */
+       IOMUX_PAD(0x05C8, 0x01F8, 1, 0x0818, 0, PAD_CTL_PKE | PAD_CTL_PUE  |
+                 PAD_CTL_PUS_100K_DOWN), /* MX6DL_PAD_ENET_RXD0__ENET_RDATA_0 */
+       IOMUX_PAD(0x05CC, 0x01FC, 1, 0x081C, 0, PAD_CTL_PKE | PAD_CTL_PUE  |
+                 PAD_CTL_PUS_100K_DOWN), /* MX6DL_PAD_ENET_RXD1__ENET_RDATA_1 */
+       MX6DL_PAD_ENET_TXD0__ENET_TDATA_0,
+       MX6DL_PAD_ENET_TXD1__ENET_TDATA_1,
+       MX6DL_PAD_ENET_TX_EN__ENET_TX_EN,
+       MX6DL_PAD_GPIO_16__ENET_ANATOP_ETHERNET_REF_OUT,
+       MX6DL_PAD_RGMII_TXC__ENET_RGMII_TXC,
+       MX6DL_PAD_RGMII_TD0__ENET_RGMII_TD0,
+       MX6DL_PAD_RGMII_TD1__ENET_RGMII_TD1,
+       MX6DL_PAD_RGMII_TD2__ENET_RGMII_TD2,
+       MX6DL_PAD_RGMII_TD3__ENET_RGMII_TD3,
+       MX6DL_PAD_RGMII_TX_CTL__ENET_RGMII_TX_CTL,
+       MX6DL_PAD_ENET_REF_CLK__ENET_TX_CLK,
+       MX6DL_PAD_RGMII_RXC__ENET_RGMII_RXC,
+       IOMUX_PAD(0x0694, 0x02AC, 1, 0x0818, 1, MX6DL_ENET_PAD_CTRL_PD),/*RGMII RD0*/
+       IOMUX_PAD(0x0698, 0x02B0, 1, 0x081C, 1, MX6DL_ENET_PAD_CTRL_PD),/*RGMII RD1*/
+       /* In RGMII mode RD2 should be '1' to disable the PLL OFF mode */
+       MX6DL_PAD_RGMII_RD2__ENET_RGMII_RD2,
+       MX6DL_PAD_RGMII_RD3__ENET_RGMII_RD3,
+       /* In RGMII mode RX_DV should be pulled down for reset strap */
+       IOMUX_PAD(0x06A4, 0x02BC, 1, 0x0828, 1, MX6DL_ENET_PAD_CTRL_PD),/*RGMII RXCTL*/

The ones which don't are those IOMUX_PAD() ones - what I have so far for
them (I'm still busy adding to the DT file for everything else) are
(in order):

        MX6QDL_PAD_ENET_MDIO__ENET_MDIO
        MX6QDL_PAD_ENET_MDC__ENET_MDC 
        MX6QDL_PAD_KEY_ROW4__GPIO4_IO15 -- ?
        MX6QDL_PAD_DI0_PIN2__GPIO4_IO18
        /* RMII */
        MX6QDL_PAD_ENET_CRS_DV__ENET_RX_EN ... extra stuff
        MX6QDL_PAD_ENET_RXD0__ENET_RX_DATA0 ... extra stuff
        MX6QDL_PAD_ENET_RXD1__ENET_RX_DATA1 ... extra stuff
        MX6QDL_PAD_ENET_TXD0__ENET_TX_DATA0
        MX6QDL_PAD_ENET_TXD1__ENET_TX_DATA1
        MX6QDL_PAD_ENET_TX_EN__ENET_TX_EN
        MX6QDL_PAD_GPIO_16__ENET_REF_CLK  -- ?
        MX6QDL_PAD_RGMII_TXC__RGMII_TXC
        MX6QDL_PAD_RGMII_TD0__RGMII_TD0
        MX6QDL_PAD_RGMII_TD1__RGMII_TD1
        MX6QDL_PAD_RGMII_TD2__RGMII_TD2
        MX6QDL_PAD_RGMII_TD3__RGMII_TD3
        MX6QDL_PAD_RGMII_TX_CTL__RGMII_TX_CTL
        MX6QDL_PAD_ENET_REF_CLK__ENET_TX_CLK
        MX6QDL_PAD_RGMII_RXC__RGMII_RXC
        MX6QDL_PAD_RGMII_RD0__RGMII_RD0 ... extra stuff
        MX6QDL_PAD_RGMII_RD1__RGMII_RD1 ... extra stuff
        /* In RGMII mode RD2 should be '1' to disable the PLL OFF mode */
        MX6QDL_PAD_RGMII_RD2__RGMII_RD2
        MX6QDL_PAD_RGMII_RD3__RGMII_RD3
        /* In RGMII mode RX_DV should be pulled down for reset strap */
        MX6QDL_PAD_RGMII_RX_CTL__RGMII_RX_CTL ... extra stuff

If I have to customize any of these settings, how do I do that?

Final question is - in imx6qdl.dtsi, I see for instance:

        MX6QDL_PAD_ENET_MDIO__ENET_MDIO       0x1b0b0

What does the final number refer to?

Thanks.

device tree binding documentation outdated

[Matt Sealey]

On Thu, Sep 26, 2013 at 3:59 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:

[snip]

I'm sure I saw Shawn Guo throw a patch here that added the new
bindings etc. - did they get missed? Or did I imagine it?

BTW I'll second Fabio's offer of help, if you need it. I'm itching to
get a CuBox-i (or more than one..)

> +       IOMUX_PAD(0x0694, 0x02AC, 1, 0x0818, 1, MX6DL_ENET_PAD_CTRL_PD),/*RGMII RD0*/
> +       IOMUX_PAD(0x0698, 0x02B0, 1, 0x081C, 1, MX6DL_ENET_PAD_CTRL_PD),/*RGMII RD1*/
> +       /* In RGMII mode RD2 should be '1' to disable the PLL OFF mode */
> +       MX6DL_PAD_RGMII_RD2__ENET_RGMII_RD2,
> +       MX6DL_PAD_RGMII_RD3__ENET_RGMII_RD3,
> +       /* In RGMII mode RX_DV should be pulled down for reset strap */
> +       IOMUX_PAD(0x06A4, 0x02BC, 1, 0x0828, 1, MX6DL_ENET_PAD_CTRL_PD),/*RGMII RXCTL*/
>
> The ones which don't are those IOMUX_PAD() ones - what I have so far for
> them (I'm still busy adding to the DT file for everything else) are
> (in order):
>
>         MX6QDL_PAD_ENET_MDIO__ENET_MDIO
>         MX6QDL_PAD_ENET_MDC__ENET_MDC
>         MX6QDL_PAD_KEY_ROW4__GPIO4_IO15 -- ?
>         MX6QDL_PAD_DI0_PIN2__GPIO4_IO18
>         /* RMII */
>         MX6QDL_PAD_ENET_CRS_DV__ENET_RX_EN ... extra stuff
>         MX6QDL_PAD_ENET_RXD0__ENET_RX_DATA0 ... extra stuff
>         MX6QDL_PAD_ENET_RXD1__ENET_RX_DATA1 ... extra stuff
>         MX6QDL_PAD_ENET_TXD0__ENET_TX_DATA0
>         MX6QDL_PAD_ENET_TXD1__ENET_TX_DATA1
>         MX6QDL_PAD_ENET_TX_EN__ENET_TX_EN
>         MX6QDL_PAD_GPIO_16__ENET_REF_CLK  -- ?
>         MX6QDL_PAD_RGMII_TXC__RGMII_TXC
>         MX6QDL_PAD_RGMII_TD0__RGMII_TD0
>         MX6QDL_PAD_RGMII_TD1__RGMII_TD1
>         MX6QDL_PAD_RGMII_TD2__RGMII_TD2
>         MX6QDL_PAD_RGMII_TD3__RGMII_TD3
>         MX6QDL_PAD_RGMII_TX_CTL__RGMII_TX_CTL
>         MX6QDL_PAD_ENET_REF_CLK__ENET_TX_CLK
>         MX6QDL_PAD_RGMII_RXC__RGMII_RXC
>         MX6QDL_PAD_RGMII_RD0__RGMII_RD0 ... extra stuff
>         MX6QDL_PAD_RGMII_RD1__RGMII_RD1 ... extra stuff
>         /* In RGMII mode RD2 should be '1' to disable the PLL OFF mode */
>         MX6QDL_PAD_RGMII_RD2__RGMII_RD2
>         MX6QDL_PAD_RGMII_RD3__RGMII_RD3
>         /* In RGMII mode RX_DV should be pulled down for reset strap */
>         MX6QDL_PAD_RGMII_RX_CTL__RGMII_RX_CTL ... extra stuff
>
> If I have to customize any of these settings, how do I do that?

Do not put them in the binding header, that's for sure. Just list the
6 values required and put a comment next to it.

> Final question is - in imx6qdl.dtsi, I see for instance:
>
>         MX6QDL_PAD_ENET_MDIO__ENET_MDIO       0x1b0b0
>
> What does the final number refer to?

The essential definition of fsl,pins is now something like

<MUX_CTL_PAD_offset> <PAD_CTL_PAD_offset> <SELECT_INPUT_offset>
<MUX_value> <SELECT_INPUT_value> <PAD_value>

Essentially the device tree must always have those 6 values, and you
ran into one where the first 5 are defined as a preprocessor macro in
most cases.

The end result is in the i.MX6 manual - the 5 values in the header are
as they are, and 0x1b0b0 is on page 2307 (section 36.4.309,
IOMUXC_SW_PAD_CTL_PAD_ENET_MDIO) of the manual.

It decodes as HYS(EN), PUS(100K_OHM_PU), PUE(PULL), PKE(EN), ODE(DIS),
DSE(40_OHM), SPEED(100MHZ), SRE(SLOW).

You can actually find a good description of the default pad settings
(they are these, exactly) on page 282 of the manual (in table 4-1) for
this pin.

You may note three stupidities here, the first only by grokking the
code in drivers/pinctrl/pinctrl-imx.c:

* The SION bit of the MUX_CTL_PAD_* register is actually kept in the
field for the PAD_CTL_PAD_* value and swapped back in
* Notice that the offsets are ordered mux, pad, input but the value
pairs are ordered as mux, input, pad...

And third, not stupid as "you can't trust the bootloader" and possibly
"the manual could be wrong"..

* Most of the macros are great for readability, but in the DT the pad
setting values - as above ..ENET_MDIO 0x1b0b0 - are chip defaults and
in the manual.. or they're correct in the bootloader anyway making
pinctrl definitions a pain in the ass for doing what you want to do
right now and an effective NOP once you complete it.

The Vybrid binding is more or less just as confusing, as it is define
as mux_off, input_off, mux_mode, input_mode, pad_ctl - the mux_ctl and
pad_ctl registers from i.MX were merged into one, hooray! But there
are two values to define the content of the mux_off register offset..

Thankfully, you ran into this.. I am vindicated in my objections to
the initial patches, which were summarily ignored. This is not why
preprocessing device trees is a good idea, in fact it is the very
definition of the worst possible usage for it. It doesn't make writing
device trees easier - it took me longer to port pinctrl in the tree I
had to the new binding than it did to write the ENTIRE initial tree
with the old pinctrl binding in the first place. Rather than copy
pasting numbers out of the manual, I've ended up searching for those
numbers in a header, cross-referencing the values to ensure they're
correct and meeting my needs, copy pasting the preprocessor define
out, or rewriting 6 individual values anyway because I want a
non-common usage.

The preprocessor macros here just serve to abstract and therefore
obfuscate what is going on. They make the tree more readable at first
glance but they don't help people add new definitions, even with a
binding document (if it existed). It is difficult to cross-reference
the manual and the bindings as the pin namings aren't the same (so no
searching the PDF for those definitions).

I'd rather see patches to rework this to get rid of the preprocessing
completely, and direct people to the manuals instead, comment the
trees like they were before, and instead bind and document any quirks
- there are a couple of pins which must have other IP blocks involved
(IOMUXC_GPRx for example) which aren't covered by the binding except
with some weirdo hacks. Perhaps even spend a couple days trying to
figure out a better way of describing those pins to the pinctrl
subsystem than gluing chicken bits into existing fields like the
hardware did.

It's halfway in place, and the old system is still documented maybe
because it is still in use somewhere... I desperately want to go in
and just fix everything for i.MX5/6 right now but a raft of personal
problems and a severe lack of internet connection at home aren't
making it easy, unfortunately. That, and it would be HUGE churn.

-- 
Matt Sealey <neko@bakuhatsu.net>

device tree binding documentation outdated

[Russell King - ARM Linux]

On Thu, Sep 26, 2013 at 06:10:21PM -0500, Matt Sealey wrote:
> Thankfully, you ran into this.. I am vindicated in my objections to
> the initial patches, which were summarily ignored. This is not why
> preprocessing device trees is a good idea, in fact it is the very
> definition of the worst possible usage for it. It doesn't make writing
> device trees easier - it took me longer to port pinctrl in the tree I
> had to the new binding than it did to write the ENTIRE initial tree
> with the old pinctrl binding in the first place.

Yes.  I've been at DT for over six hours now, and all I've been able to
sort out is getting the two USB and SD cards working.  Honestly, I feel
totally worn out now through the amount of cross correlation that I'm
having to do to ensure I'm getting this stuff correct.

As Rabeeh uses the old macros from the header file, I'm having to look
those up.  Then I'm having to find the corresponding entry in the
-pinfunc.h header file.  Then I'm having to check whether the group
of pinctrl settings are correct - and if they are, use them.  If they
aren't, then wonder what to do about it.

I've taken the time to convert three directly (the sdhci card detect
and write protect signals) and the OTG ID signal.  The SPDIF I've
listed in the dts file I just grabbed from the imx6dl.dtsi file (the
one in there is paired with spdif in.)

There's the HDMI, ethernet and SPDIF out properly still to go.

I haven't sorted out a rootfs for this yet, so it fails trying to find
that.  (Rabeeh's kernel contained an initramfs buildroot image - it
might be useful to find some way to load that as a separate image -
does appended DT support that?)

Anyway, attached are the DTS and current boot log.
-------------- next part --------------
/dts-v1/;

#include "imx6dl.dtsi"

/ {
	model = "Solid-Run Cubox-i DL/Solo Board";
	compatible = "solid-run,cubox-i", "fsl,imx6dl";

	regulators {
		compatible = "simple-bus";

		reg_3p3v: 3p3v {
			compatible = "regulator-fixed";
			regulator-name = "3P3V";
			regulator-min-microvolt = <3300000>;
			regulator-max-microvolt = <3300000>;
			regulator-always-on;
		};

		reg_usb_otg_vbus: usb_otg_vbus {
			compatible = "regulator-fixed";
			regulator-name = "usb_otg_vbus";
			regulator-min-microvolt = <5000000>;
			regulator-max-microvolt = <5000000>;
			gpio = <&gpio3 22 0>;
			enable-active-high;
		};

		reg_usb_h1_vbus: usb_h1_vbus {
			compatible = "regulator-fixed";
			regulator-name = "usb_h1_vbus";
			regulator-min-microvolt = <5000000>;
			regulator-max-microvolt = <5000000>;
			gpio = <&gpio1 0 0>;
			enable-active-high;
		};
	};
};

&can1 {
	pinctrl-names = "default";
	pinctrl-0 = <&pinctrl_flexcan1_2>;
	status = "okay";
};

&i2c1 {
	pinctrl-names = "default";
	pinctrl-0 = <&pinctrl_i2c1_1>;

	/*
	 * Not fitted on developer board
	status = "okay";

	rtc: pcf8523 at 68 {
		compatible = "nxp,pcf8523";
		reg = <0x68>;
	};
	 */
};

&i2c2 {
	pinctrl-names = "default";
	pinctrl-0 = <&pinctrl_i2c2_2>;
	status = "okay";

	/* HDMI @0x50 */
};

&iomuxc {
	pinctrl_spdif_2_out_only: spdifgrp-3 {
		fsl,pins = <
			MX6QDL_PAD_GPIO_17__SPDIF_OUT 0x1b0b0
		>;
	};

	usbhc2 {
		pinctrl_usdhc2_cd_wp: usdhc2_cd_wp {
			fsl,pins = <
				MX6QDL_PAD_GPIO_4__SD2_CD_B 0x1f059
				MX6QDL_PAD_GPIO_2__SD2_WP 0x1f059
			>;
		};
	};

	usbotg {
		pinctrl_usbotg_cubox: usbotggrp-3 {
			/*
			 * Originally pinctrl_usbotg_2, but we want it to
			 * be pulled down for fixed host connection.
			 */
			fsl,pins = <
				MX6QDL_PAD_GPIO_1__USB_OTG_ID 0x13059
			>;
		};
	};
};

&uart1 {
	pinctrl-names = "default";
	pinctrl-0 = <&pinctrl_uart1_1>;
	status = "okay";
};

&usbotg {
	vbus-supply = <&reg_usb_otg_vbus>;
	pinctrl-names = "default";
	pinctrl-0 = <&pinctrl_usbotg_cubox>;
	status = "okay";
};

&usbh1 {
	vbus-supply = <&reg_usb_h1_vbus>;
	status = "okay";
};

&usdhc2 {
	pinctrl-names = "default";
	pinctrl-0 = <&pinctrl_usdhc2_2 &pinctrl_usdhc2_cd_wp>;
	vmmc-supply = <&reg_3p3v>;
	cd-gpios = <&gpio1 4 0>;
	/*
	 * Cubox-i dev board doesn't have this wired
	 * but comments suggest real hardware does.
	wp-gpios = <&gpio1 2 0>;
	 */
	status = "okay";
};

/* experimental stuff */

/* USDHC1 - Connected to BRCM Wifi/BT/FM */
&usdhc1 {
	pinctrl-names = "default";
	pinctrl-0 = <&pinctrl_usdhc1_2>;
	status = "okay";
};

/* SPDIF out */

/* CAN1 */
/* CCM */

/* ECSPI2 */

/* I2C1 */
/* I2C2 - HDMI DDC */

/* I2C3 */
&i2c3 {
	pinctrl-names = "default";
	pinctrl-0 = <&pinctrl_i2c3_1>;
	status = "okay";
};

/* PWM */

/* UART2 */

/* USB OC pin */

/* USDHC2 */

/* USDHC3 */
&usdhc3 {
	pinctrl-names = "default";
	pinctrl-0 = <&pinctrl_usdhc3_1>;
	status = "disabled";
};

/* USDHC4 */
&usdhc4 {
	pinctrl-names = "default";
	pinctrl-0 = <&pinctrl_usdhc4_1>;
	status = "disabled";
};

#if 0
&fec {
	pintctrl-names = "default";
	pinctrl-0 = <&pinctrl_enet_1>;
	phy-mode = "rgmii";
	status = "okay";
};
#endif

-------------- next part --------------
Uncompressing Linux... done, booting the kernel.
Booting Linux on physical CPU 0x0
Linux version 3.12.0-rc1+ (rmk at rmk-PC.arm.linux.org.uk) (gcc version 4.5.4 (GCC) ) #7 SMP Thu Sep 26 19:28:35 BST 2013
CPU: ARMv7 Processor [412fc09a] revision 10 (ARMv7), cr=10c53c7d
CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing instruction cache
Machine: Freescale i.MX6 Quad/DualLite (Device Tree), model: Solid-Run Cubox-i DL/Solo Board
Memory policy: ECC disabled, Data cache writealloc
PERCPU: Embedded 7 pages/cpu @80cc0000 s7488 r8192 d12992 u32768
Built 1 zonelists in Zone order, mobility grouping on.  Total pages: 130048
Kernel command line: console=ttymxc0,115200 video=mxcfb0:dev=hdmi,1920x1080M at 60,if=RGB24,bpp=32 rootdelay=3
PID hash table entries: 2048 (order: 1, 8192 bytes)
Dentry cache hash table entries: 65536 (order: 6, 262144 bytes)
Inode-cache hash table entries: 32768 (order: 5, 131072 bytes)
Memory: 510764K/524288K available (6007K kernel code, 287K rwdata, 1864K rodata, 323K init, 375K bss, 13524K reserved)
Virtual kernel memory layout:
    vector  : 0xffff0000 - 0xffff1000   (   4 kB)
    fixmap  : 0xfff00000 - 0xfffe0000   ( 896 kB)
    vmalloc : 0xa0800000 - 0xff000000   (1512 MB)
    lowmem  : 0x80000000 - 0xa0000000   ( 512 MB)
    modules : 0x7f000000 - 0x80000000   (  16 MB)
      .text : 0x80008000 - 0x807b7ec8   (7872 kB)
      .init : 0x807b8000 - 0x80808d40   ( 324 kB)
      .data : 0x8080a000 - 0x80851d20   ( 288 kB)
       .bss : 0x80851d28 - 0x808afa6c   ( 376 kB)
SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=1, Nodes=1
Hierarchical RCU implementation.
        RCU restricting CPUs from NR_CPUS=4 to nr_cpu_ids=1.
NR_IRQS:16 nr_irqs:16 16
L310 cache controller enabled
l2x0: 16 ways, CACHE_ID 0x410000c8, AUX_CTRL 0x32050000, Cache size: 512 kB
sched_clock: 32 bits at 66MHz, resolution 15ns, wraps every 65075ms
CPU identified as i.MX6DL, silicon rev 1.1
Console: colour dummy device 80x30
Calibrating delay loop... 1581.05 BogoMIPS (lpj=7905280)
pid_max: default: 32768 minimum: 301
Mount-cache hash table entries: 512
CPU: Testing write buffer coherency: ok
CPU0: thread -1, cpu 0, socket 0, mpidr 80000000
Setting up static identity map for 0x805e37f8 - 0x805e3868
Brought up 1 CPUs
SMP: Total of 1 processors activated.
CPU: All CPU(s) started in SVC mode.
devtmpfs: initialized
VFP support v0.3: implementor 41 architecture 3 part 30 variant 9 rev 4
pinctrl core: initialized pinctrl subsystem
regulator-dummy: no parameters
NET: Registered protocol family 16
DMA: preallocated 256 KiB pool for atomic coherent allocations
syscon 20c8000.anatop: regmap [mem 0x020c8000-0x020c8fff] registered
vdd1p1: 800 <--> 1375 mV at 1100 mV 
vdd3p0: 2800 <--> 3150 mV at 3000 mV 
vdd2p5: 2000 <--> 2750 mV at 2400 mV 
cpu: 725 <--> 1450 mV at 1150 mV 
vddpu: 725 <--> 1450 mV 
vddsoc: 725 <--> 1450 mV at 1200 mV 
syscon 20e0000.iomuxc-gpr: regmap [mem 0x020e0000-0x020e0037] registered
syscon 21bc000.ocotp: regmap [mem 0x021bc000-0x021bffff] registered
hw-breakpoint: found 5 (+1 reserved) breakpoint and 1 watchpoint registers.
hw-breakpoint: maximum watchpoint size is 4 bytes.
imx6dl-pinctrl 20e0000.iomuxc: no groups defined in /soc/aips-bus at 02000000/iomuxc at 020e0000/spdifgrp-3
imx6dl-pinctrl 20e0000.iomuxc: initialized IMX pinctrl driver
bio: create slab <bio-0> at 0
mxs-dma 110000.dma-apbh: initialized
3P3V: 3300 mV 
usb_otg_vbus: 5000 mV 
usb_h1_vbus: 5000 mV 
SCSI subsystem initialized
usbcore: registered new interface driver usbfs
usbcore: registered new interface driver hub
usbcore: registered new device driver usb
i2c i2c-1: IMX I2C adapter registered
i2c i2c-2: IMX I2C adapter registered
Linux video capture interface: v2.00
pps_core: LinuxPPS API ver. 1 registered
pps_core: Software ver. 5.3.6 - Copyright 2005-2007 Rodolfo Giometti <giometti@linux.it>
PTP clock support registered
Advanced Linux Sound Architecture Driver Initialized.
Switched to clocksource mxc_timer1
cfg80211: Calling CRDA to update world regulatory domain
NET: Registered protocol family 2
TCP established hash table entries: 4096 (order: 3, 32768 bytes)
TCP bind hash table entries: 4096 (order: 3, 32768 bytes)
TCP: Hash tables configured (established 4096 bind 4096)
TCP: reno registered
UDP hash table entries: 256 (order: 1, 8192 bytes)
UDP-Lite hash table entries: 256 (order: 1, 8192 bytes)
NET: Registered protocol family 1
RPC: Registered named UNIX socket transport module.
RPC: Registered udp transport module.
RPC: Registered tcp transport module.
RPC: Registered tcp NFSv4.1 backchannel transport module.
hw perfevents: enabled with ARMv7 Cortex-A9 PMU driver, 7 counters available
VFS: Disk quotas dquot_6.5.2
Dquot-cache hash table entries: 1024 (order 0, 4096 bytes)
NFS: Registering the id_resolver key type
Key type id_resolver registered
Key type id_legacy registered
jffs2: version 2.2. (NAND) ? 2001-2006 Red Hat, Inc.
fuse init (API version 7.22)
msgmni has been set to 997
io scheduler noop registered
io scheduler deadline registered
io scheduler cfq registered (default)
imx-weim 21b8000.weim: Driver registered.
imx-sdma 20ec000.sdma: initialized
Serial: IMX driver
2020000.serial: ttymxc0 at MMIO 0x2020000 (irq = 58, base_baud = 5000000) is a IMX
console [ttymxc0] enabled
serial: Freescale lpuart driver
[drm] Initialized drm 1.1.0 20060810
brd: module loaded
loop: module loaded
ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver
ehci-mxc: Freescale On-Chip EHCI Host driver
usbcore: registered new interface driver usb-storage
imx_usb 2184000.usb: usbmisc init failed, ret=-517
platform 2184000.usb: Driver imx_usb requests probe deferral
imx_usb 2184200.usb: usbmisc init failed, ret=-517
platform 2184200.usb: Driver imx_usb requests probe deferral
mousedev: PS/2 mouse device common for all mice
snvs_rtc 20cc034.snvs-rtc-lp: rtc core: registered 20cc034.snvs-rtc-lp as rtc0
i2c /dev entries driver
imx2-wdt 20bc000.wdog: IMX2+ Watchdog Timer enabled. timeout=60s (nowayout=0)
sdhci: Secure Digital Host Controller Interface driver
sdhci: Copyright(c) Pierre Ossman
sdhci-pltfm: SDHCI platform and OF driver helper
mmc0: no vqmmc regulator found
mmc0: no vmmc regulator found
mmc0: SDHCI controller on 2190000.usdhc [2190000.usdhc] using ADMA
mmc1: no vqmmc regulator found
mmc1: SDHCI controller on 2194000.usdhc [2194000.usdhc] using ADMA
usbcore: registered new interface driver usbhid
usbhid: USB HID core driver
[drm] Supports vblank timestamp caching Rev 1 (10.10.2010).
[drm] No driver support for vblank timestamp query.
[drm] Initialized imx-drm 1.0.0 20120507 on minor 0
imx-ipuv3 2400000.ipu: IPUv3H probed
imx-ipuv3-crtc imx-ipuv3-crtc.0: adding crtc failed with -16.
imx-ipuv3-crtc: probe of imx-ipuv3-crtc.0 failed with error -16
imx-ipuv3-crtc imx-ipuv3-crtc.1: adding crtc failed with -16.
imx-ipuv3-crtc: probe of imx-ipuv3-crtc.1 failed with error -16
TCP: cubic registered
NET: Registered protocol family 10
sit: IPv6 over IPv4 tunneling driver
NET: Registered protocol family 17
Key type dns_resolver registered
ci_hdrc ci_hdrc.0: doesn't support gadget
ci_hdrc ci_hdrc.0: EHCI Host Controller
ci_hdrc ci_hdrc.0: new USB bus registered, assigned bus number 1
ci_hdrc ci_hdrc.0: USB 2.0 started, EHCI 1.00
hub 1-0:1.0: USB hub found
hub 1-0:1.0: 1 port detected
ci_hdrc ci_hdrc.1: doesn't support gadget
ci_hdrc ci_hdrc.1: EHCI Host Controller
ci_hdrc ci_hdrc.1: new USB bus registered, assigned bus number 2
ci_hdrc ci_hdrc.1: USB 2.0 started, EHCI 1.00
hub 2-0:1.0: USB hub found
hub 2-0:1.0: 1 port detected
snvs_rtc 20cc034.snvs-rtc-lp: setting system clock to 1970-01-01 00:00:00 UTC (0)
imx-drm imx-drm: No connectors reported connected with modes
[drm] Cannot find any crtc or sizes - going 1024x768
Console: switching to colour frame buffer device 128x48
imx-drm imx-drm: fb0:  frame buffer device
imx-drm imx-drm: registered panic notifier
ALSA device list:
  No soundcards found.
Waiting 3 sec before mounting root device...
mmc1: host does not support reading read-only switch. assuming write-enable.
mmc1: new high speed SDHC card at address 0002
mmcblk0: mmc1:0002 00000 3.70 GiB 
 mmcblk0: p1
usb 2-1: new full-speed USB device number 2 using ci_hdrc
usb-storage 2-1:1.0: USB Mass Storage device detected
usb-storage 2-1:1.0: Quirks match for vid 1370 pid 6828: 20
scsi0 : usb-storage 2-1:1.0
scsi 0:0:0:0: Direct-Access     SWISSBIT Victorinox       1.89 PQ: 0 ANSI: 2
ready
sd 0:0:0:0: [sda] 126720 512-byte logical blocks: (64.8 MB/61.8 MiB)
sd 0:0:0:0: [sda] Write Protect is off
sd 0:0:0:0: [sda] No Caching mode page found
sd 0:0:0:0: [sda] Assuming drive cache: write through
sd 0:0:0:0: [sda] No Caching mode page found
sd 0:0:0:0: [sda] Assuming drive cache: write through
 sda:
sd 0:0:0:0: [sda] No Caching mode page found
sd 0:0:0:0: [sda] Assuming drive cache: write through
sd 0:0:0:0: [sda] Attached SCSI removable disk
VFS: Cannot open root device "(null)" or unknown-block(0,0): error -6
Please append a correct "root=" boot option; here are the available partitions:
b300         3880960 mmcblk0  driver: mmcblk
  b301          130048 mmcblk0p1 00000000-01
0800           63360 sda  driver: sd
Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(0,0)
drm_kms_helper: panic occurred, switching back to text console

device tree binding documentation outdated

[Matt Sealey]

Neat.

So the pushback I had on my initial objections was that having huge
lists of numbers is daunting and doesn't help novice device tree
writers add pin controls to the device tree. There is one benefit that
comes from it; update one preprocessor definition and all device trees
referencing it magically get new values.

An example:

https://git.linaro.org/gitweb?p=people/shawnguo/linux-2.6.git;a=commit;h=538bcbe251d621aa19c46babafd01ede8fb6ddde

One fix, every device tree picks it up.. so using preprocessor macros
really works. Congrats...

.... except this pin definition is referenced ONCE in a common include
for all i.MX6 (Q, D, L) platforms.

So the amount of work required actually could have been "patch one
line in the imx6qdl.dtsi" instead. Net benefit: zero. All the values
are in the manual, too.. and the manual is publicly available.

Add this to making the whole device tree source bigger than it needs
to be, introducing a magic 'save the preprocessor output for
validation' step for most people who debug this stuff for a living,
and a 3-extra-step round trip from the manual to create new trees from
cross reference to dirt old BSP source code (which, let's face it, is
exactly where most people doing an i.MX device tree are getting their
code from).

It is better than the original fsl,pins = <arbitrary_index> <pad
settings | (SION lsl 16)>, I will give you that.

I would propose we ditch pre-processing, and go back to raw values and
commenting the DT in this instance. There are other, much more noble,
uses for the preprocessing functionality.

Would anyone be too angry if I swapped the values to a logical order?


On Thu, Sep 26, 2013 at 6:29 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Thu, Sep 26, 2013 at 06:10:21PM -0500, Matt Sealey wrote:
>> Thankfully, you ran into this.. I am vindicated in my objections to
>> the initial patches, which were summarily ignored. This is not why
>> preprocessing device trees is a good idea, in fact it is the very
>> definition of the worst possible usage for it. It doesn't make writing
>> device trees easier - it took me longer to port pinctrl in the tree I
>> had to the new binding than it did to write the ENTIRE initial tree
>> with the old pinctrl binding in the first place.
>
> Yes.  I've been at DT for over six hours now, and all I've been able to
> sort out is getting the two USB and SD cards working.  Honestly, I feel
> totally worn out now through the amount of cross correlation that I'm
> having to do to ensure I'm getting this stuff correct.
>
> As Rabeeh uses the old macros from the header file, I'm having to look
> those up.  Then I'm having to find the corresponding entry in the
> -pinfunc.h header file.  Then I'm having to check whether the group
> of pinctrl settings are correct - and if they are, use them.  If they
> aren't, then wonder what to do about it.
>
> I've taken the time to convert three directly (the sdhci card detect
> and write protect signals) and the OTG ID signal.  The SPDIF I've
> listed in the dts file I just grabbed from the imx6dl.dtsi file (the
> one in there is paired with spdif in.)
>
> There's the HDMI, ethernet and SPDIF out properly still to go.
>
> I haven't sorted out a rootfs for this yet, so it fails trying to find
> that.  (Rabeeh's kernel contained an initramfs buildroot image - it
> might be useful to find some way to load that as a separate image -
> does appended DT support that?)
>
> Anyway, attached are the DTS and current boot log.
>
> /dts-v1/;
>
> #include "imx6dl.dtsi"
>
> / {
>         model = "Solid-Run Cubox-i DL/Solo Board";
>         compatible = "solid-run,cubox-i", "fsl,imx6dl";
>
>         regulators {
>                 compatible = "simple-bus";
>
>                 reg_3p3v: 3p3v {
>                         compatible = "regulator-fixed";
>                         regulator-name = "3P3V";
>                         regulator-min-microvolt = <3300000>;
>                         regulator-max-microvolt = <3300000>;
>                         regulator-always-on;
>                 };
>
>                 reg_usb_otg_vbus: usb_otg_vbus {
>                         compatible = "regulator-fixed";
>                         regulator-name = "usb_otg_vbus";
>                         regulator-min-microvolt = <5000000>;
>                         regulator-max-microvolt = <5000000>;
>                         gpio = <&gpio3 22 0>;
>                         enable-active-high;
>                 };
>
>                 reg_usb_h1_vbus: usb_h1_vbus {
>                         compatible = "regulator-fixed";
>                         regulator-name = "usb_h1_vbus";
>                         regulator-min-microvolt = <5000000>;
>                         regulator-max-microvolt = <5000000>;
>                         gpio = <&gpio1 0 0>;
>                         enable-active-high;
>                 };
>         };
> };
>
> &can1 {
>         pinctrl-names = "default";
>         pinctrl-0 = <&pinctrl_flexcan1_2>;
>         status = "okay";
> };
>
> &i2c1 {
>         pinctrl-names = "default";
>         pinctrl-0 = <&pinctrl_i2c1_1>;
>
>         /*
>          * Not fitted on developer board
>         status = "okay";
>
>         rtc: pcf8523 at 68 {
>                 compatible = "nxp,pcf8523";
>                 reg = <0x68>;
>         };
>          */
> };
>
> &i2c2 {
>         pinctrl-names = "default";
>         pinctrl-0 = <&pinctrl_i2c2_2>;
>         status = "okay";
>
>         /* HDMI @0x50 */
> };
>
> &iomuxc {
>         pinctrl_spdif_2_out_only: spdifgrp-3 {
>                 fsl,pins = <
>                         MX6QDL_PAD_GPIO_17__SPDIF_OUT 0x1b0b0
>                 >;
>         };
>
>         usbhc2 {
>                 pinctrl_usdhc2_cd_wp: usdhc2_cd_wp {
>                         fsl,pins = <
>                                 MX6QDL_PAD_GPIO_4__SD2_CD_B 0x1f059
>                                 MX6QDL_PAD_GPIO_2__SD2_WP 0x1f059
>                         >;
>                 };
>         };
>
>         usbotg {
>                 pinctrl_usbotg_cubox: usbotggrp-3 {
>                         /*
>                          * Originally pinctrl_usbotg_2, but we want it to
>                          * be pulled down for fixed host connection.
>                          */
>                         fsl,pins = <
>                                 MX6QDL_PAD_GPIO_1__USB_OTG_ID 0x13059
>                         >;
>                 };
>         };
> };
>
> &uart1 {
>         pinctrl-names = "default";
>         pinctrl-0 = <&pinctrl_uart1_1>;
>         status = "okay";
> };
>
> &usbotg {
>         vbus-supply = <&reg_usb_otg_vbus>;
>         pinctrl-names = "default";
>         pinctrl-0 = <&pinctrl_usbotg_cubox>;
>         status = "okay";
> };
>
> &usbh1 {
>         vbus-supply = <&reg_usb_h1_vbus>;
>         status = "okay";
> };
>
> &usdhc2 {
>         pinctrl-names = "default";
>         pinctrl-0 = <&pinctrl_usdhc2_2 &pinctrl_usdhc2_cd_wp>;
>         vmmc-supply = <&reg_3p3v>;
>         cd-gpios = <&gpio1 4 0>;
>         /*
>          * Cubox-i dev board doesn't have this wired
>          * but comments suggest real hardware does.
>         wp-gpios = <&gpio1 2 0>;
>          */
>         status = "okay";
> };
>
> /* experimental stuff */
>
> /* USDHC1 - Connected to BRCM Wifi/BT/FM */
> &usdhc1 {
>         pinctrl-names = "default";
>         pinctrl-0 = <&pinctrl_usdhc1_2>;
>         status = "okay";
> };
>
> /* SPDIF out */
>
> /* CAN1 */
> /* CCM */
>
> /* ECSPI2 */
>
> /* I2C1 */
> /* I2C2 - HDMI DDC */
>
> /* I2C3 */
> &i2c3 {
>         pinctrl-names = "default";
>         pinctrl-0 = <&pinctrl_i2c3_1>;
>         status = "okay";
> };
>
> /* PWM */
>
> /* UART2 */
>
> /* USB OC pin */
>
> /* USDHC2 */
>
> /* USDHC3 */
> &usdhc3 {
>         pinctrl-names = "default";
>         pinctrl-0 = <&pinctrl_usdhc3_1>;
>         status = "disabled";
> };
>
> /* USDHC4 */
> &usdhc4 {
>         pinctrl-names = "default";
>         pinctrl-0 = <&pinctrl_usdhc4_1>;
>         status = "disabled";
> };
>
> #if 0
> &fec {
>         pintctrl-names = "default";
>         pinctrl-0 = <&pinctrl_enet_1>;
>         phy-mode = "rgmii";
>         status = "okay";
> };
> #endif
>
>
> Uncompressing Linux... done, booting the kernel.
> Booting Linux on physical CPU 0x0
> Linux version 3.12.0-rc1+ (rmk at rmk-PC.arm.linux.org.uk) (gcc version 4.5.4 (GCC) ) #7 SMP Thu Sep 26 19:28:35 BST 2013
> CPU: ARMv7 Processor [412fc09a] revision 10 (ARMv7), cr=10c53c7d
> CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing instruction cache
> Machine: Freescale i.MX6 Quad/DualLite (Device Tree), model: Solid-Run Cubox-i DL/Solo Board
> Memory policy: ECC disabled, Data cache writealloc
> PERCPU: Embedded 7 pages/cpu @80cc0000 s7488 r8192 d12992 u32768
> Built 1 zonelists in Zone order, mobility grouping on.  Total pages: 130048
> Kernel command line: console=ttymxc0,115200 video=mxcfb0:dev=hdmi,1920x1080M at 60,if=RGB24,bpp=32 rootdelay=3
> PID hash table entries: 2048 (order: 1, 8192 bytes)
> Dentry cache hash table entries: 65536 (order: 6, 262144 bytes)
> Inode-cache hash table entries: 32768 (order: 5, 131072 bytes)
> Memory: 510764K/524288K available (6007K kernel code, 287K rwdata, 1864K rodata, 323K init, 375K bss, 13524K reserved)
> Virtual kernel memory layout:
>     vector  : 0xffff0000 - 0xffff1000   (   4 kB)
>     fixmap  : 0xfff00000 - 0xfffe0000   ( 896 kB)
>     vmalloc : 0xa0800000 - 0xff000000   (1512 MB)
>     lowmem  : 0x80000000 - 0xa0000000   ( 512 MB)
>     modules : 0x7f000000 - 0x80000000   (  16 MB)
>       .text : 0x80008000 - 0x807b7ec8   (7872 kB)
>       .init : 0x807b8000 - 0x80808d40   ( 324 kB)
>       .data : 0x8080a000 - 0x80851d20   ( 288 kB)
>        .bss : 0x80851d28 - 0x808afa6c   ( 376 kB)
> SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=1, Nodes=1
> Hierarchical RCU implementation.
>         RCU restricting CPUs from NR_CPUS=4 to nr_cpu_ids=1.
> NR_IRQS:16 nr_irqs:16 16
> L310 cache controller enabled
> l2x0: 16 ways, CACHE_ID 0x410000c8, AUX_CTRL 0x32050000, Cache size: 512 kB
> sched_clock: 32 bits at 66MHz, resolution 15ns, wraps every 65075ms
> CPU identified as i.MX6DL, silicon rev 1.1
> Console: colour dummy device 80x30
> Calibrating delay loop... 1581.05 BogoMIPS (lpj=7905280)
> pid_max: default: 32768 minimum: 301
> Mount-cache hash table entries: 512
> CPU: Testing write buffer coherency: ok
> CPU0: thread -1, cpu 0, socket 0, mpidr 80000000
> Setting up static identity map for 0x805e37f8 - 0x805e3868
> Brought up 1 CPUs
> SMP: Total of 1 processors activated.
> CPU: All CPU(s) started in SVC mode.
> devtmpfs: initialized
> VFP support v0.3: implementor 41 architecture 3 part 30 variant 9 rev 4
> pinctrl core: initialized pinctrl subsystem
> regulator-dummy: no parameters
> NET: Registered protocol family 16
> DMA: preallocated 256 KiB pool for atomic coherent allocations
> syscon 20c8000.anatop: regmap [mem 0x020c8000-0x020c8fff] registered
> vdd1p1: 800 <--> 1375 mV at 1100 mV
> vdd3p0: 2800 <--> 3150 mV at 3000 mV
> vdd2p5: 2000 <--> 2750 mV at 2400 mV
> cpu: 725 <--> 1450 mV at 1150 mV
> vddpu: 725 <--> 1450 mV
> vddsoc: 725 <--> 1450 mV at 1200 mV
> syscon 20e0000.iomuxc-gpr: regmap [mem 0x020e0000-0x020e0037] registered
> syscon 21bc000.ocotp: regmap [mem 0x021bc000-0x021bffff] registered
> hw-breakpoint: found 5 (+1 reserved) breakpoint and 1 watchpoint registers.
> hw-breakpoint: maximum watchpoint size is 4 bytes.
> imx6dl-pinctrl 20e0000.iomuxc: no groups defined in /soc/aips-bus at 02000000/iomuxc at 020e0000/spdifgrp-3
> imx6dl-pinctrl 20e0000.iomuxc: initialized IMX pinctrl driver
> bio: create slab <bio-0> at 0
> mxs-dma 110000.dma-apbh: initialized
> 3P3V: 3300 mV
> usb_otg_vbus: 5000 mV
> usb_h1_vbus: 5000 mV
> SCSI subsystem initialized
> usbcore: registered new interface driver usbfs
> usbcore: registered new interface driver hub
> usbcore: registered new device driver usb
> i2c i2c-1: IMX I2C adapter registered
> i2c i2c-2: IMX I2C adapter registered
> Linux video capture interface: v2.00
> pps_core: LinuxPPS API ver. 1 registered
> pps_core: Software ver. 5.3.6 - Copyright 2005-2007 Rodolfo Giometti <giometti@linux.it>
> PTP clock support registered
> Advanced Linux Sound Architecture Driver Initialized.
> Switched to clocksource mxc_timer1
> cfg80211: Calling CRDA to update world regulatory domain
> NET: Registered protocol family 2
> TCP established hash table entries: 4096 (order: 3, 32768 bytes)
> TCP bind hash table entries: 4096 (order: 3, 32768 bytes)
> TCP: Hash tables configured (established 4096 bind 4096)
> TCP: reno registered
> UDP hash table entries: 256 (order: 1, 8192 bytes)
> UDP-Lite hash table entries: 256 (order: 1, 8192 bytes)
> NET: Registered protocol family 1
> RPC: Registered named UNIX socket transport module.
> RPC: Registered udp transport module.
> RPC: Registered tcp transport module.
> RPC: Registered tcp NFSv4.1 backchannel transport module.
> hw perfevents: enabled with ARMv7 Cortex-A9 PMU driver, 7 counters available
> VFS: Disk quotas dquot_6.5.2
> Dquot-cache hash table entries: 1024 (order 0, 4096 bytes)
> NFS: Registering the id_resolver key type
> Key type id_resolver registered
> Key type id_legacy registered
> jffs2: version 2.2. (NAND) ? 2001-2006 Red Hat, Inc.
> fuse init (API version 7.22)
> msgmni has been set to 997
> io scheduler noop registered
> io scheduler deadline registered
> io scheduler cfq registered (default)
> imx-weim 21b8000.weim: Driver registered.
> imx-sdma 20ec000.sdma: initialized
> Serial: IMX driver
> 2020000.serial: ttymxc0 at MMIO 0x2020000 (irq = 58, base_baud = 5000000) is a IMX
> console [ttymxc0] enabled
> serial: Freescale lpuart driver
> [drm] Initialized drm 1.1.0 20060810
> brd: module loaded
> loop: module loaded
> ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver
> ehci-mxc: Freescale On-Chip EHCI Host driver
> usbcore: registered new interface driver usb-storage
> imx_usb 2184000.usb: usbmisc init failed, ret=-517
> platform 2184000.usb: Driver imx_usb requests probe deferral
> imx_usb 2184200.usb: usbmisc init failed, ret=-517
> platform 2184200.usb: Driver imx_usb requests probe deferral
> mousedev: PS/2 mouse device common for all mice
> snvs_rtc 20cc034.snvs-rtc-lp: rtc core: registered 20cc034.snvs-rtc-lp as rtc0
> i2c /dev entries driver
> imx2-wdt 20bc000.wdog: IMX2+ Watchdog Timer enabled. timeout=60s (nowayout=0)
> sdhci: Secure Digital Host Controller Interface driver
> sdhci: Copyright(c) Pierre Ossman
> sdhci-pltfm: SDHCI platform and OF driver helper
> mmc0: no vqmmc regulator found
> mmc0: no vmmc regulator found
> mmc0: SDHCI controller on 2190000.usdhc [2190000.usdhc] using ADMA
> mmc1: no vqmmc regulator found
> mmc1: SDHCI controller on 2194000.usdhc [2194000.usdhc] using ADMA
> usbcore: registered new interface driver usbhid
> usbhid: USB HID core driver
> [drm] Supports vblank timestamp caching Rev 1 (10.10.2010).
> [drm] No driver support for vblank timestamp query.
> [drm] Initialized imx-drm 1.0.0 20120507 on minor 0
> imx-ipuv3 2400000.ipu: IPUv3H probed
> imx-ipuv3-crtc imx-ipuv3-crtc.0: adding crtc failed with -16.
> imx-ipuv3-crtc: probe of imx-ipuv3-crtc.0 failed with error -16
> imx-ipuv3-crtc imx-ipuv3-crtc.1: adding crtc failed with -16.
> imx-ipuv3-crtc: probe of imx-ipuv3-crtc.1 failed with error -16
> TCP: cubic registered
> NET: Registered protocol family 10
> sit: IPv6 over IPv4 tunneling driver
> NET: Registered protocol family 17
> Key type dns_resolver registered
> ci_hdrc ci_hdrc.0: doesn't support gadget
> ci_hdrc ci_hdrc.0: EHCI Host Controller
> ci_hdrc ci_hdrc.0: new USB bus registered, assigned bus number 1
> ci_hdrc ci_hdrc.0: USB 2.0 started, EHCI 1.00
> hub 1-0:1.0: USB hub found
> hub 1-0:1.0: 1 port detected
> ci_hdrc ci_hdrc.1: doesn't support gadget
> ci_hdrc ci_hdrc.1: EHCI Host Controller
> ci_hdrc ci_hdrc.1: new USB bus registered, assigned bus number 2
> ci_hdrc ci_hdrc.1: USB 2.0 started, EHCI 1.00
> hub 2-0:1.0: USB hub found
> hub 2-0:1.0: 1 port detected
> snvs_rtc 20cc034.snvs-rtc-lp: setting system clock to 1970-01-01 00:00:00 UTC (0)
> imx-drm imx-drm: No connectors reported connected with modes
> [drm] Cannot find any crtc or sizes - going 1024x768
> Console: switching to colour frame buffer device 128x48
> imx-drm imx-drm: fb0:  frame buffer device
> imx-drm imx-drm: registered panic notifier
> ALSA device list:
>   No soundcards found.
> Waiting 3 sec before mounting root device...
> mmc1: host does not support reading read-only switch. assuming write-enable.
> mmc1: new high speed SDHC card at address 0002
> mmcblk0: mmc1:0002 00000 3.70 GiB
>  mmcblk0: p1
> usb 2-1: new full-speed USB device number 2 using ci_hdrc
> usb-storage 2-1:1.0: USB Mass Storage device detected
> usb-storage 2-1:1.0: Quirks match for vid 1370 pid 6828: 20
> scsi0 : usb-storage 2-1:1.0
> scsi 0:0:0:0: Direct-Access     SWISSBIT Victorinox       1.89 PQ: 0 ANSI: 2
> ready
> sd 0:0:0:0: [sda] 126720 512-byte logical blocks: (64.8 MB/61.8 MiB)
> sd 0:0:0:0: [sda] Write Protect is off
> sd 0:0:0:0: [sda] No Caching mode page found
> sd 0:0:0:0: [sda] Assuming drive cache: write through
> sd 0:0:0:0: [sda] No Caching mode page found
> sd 0:0:0:0: [sda] Assuming drive cache: write through
>  sda:
> sd 0:0:0:0: [sda] No Caching mode page found
> sd 0:0:0:0: [sda] Assuming drive cache: write through
> sd 0:0:0:0: [sda] Attached SCSI removable disk
> VFS: Cannot open root device "(null)" or unknown-block(0,0): error -6
> Please append a correct "root=" boot option; here are the available partitions:
> b300         3880960 mmcblk0  driver: mmcblk
>   b301          130048 mmcblk0p1 00000000-01
> 0800           63360 sda  driver: sd
> Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(0,0)
> drm_kms_helper: panic occurred, switching back to text console
>
>

device tree binding documentation outdated

[Jason Cooper]

Russell,

On Fri, Sep 27, 2013 at 12:29:07AM +0100, Russell King - ARM Linux wrote:
> I haven't sorted out a rootfs for this yet, so it fails trying to find
> that.  (Rabeeh's kernel contained an initramfs buildroot image - it
> might be useful to find some way to load that as a separate image -
> does appended DT support that?)

yes, point to it in your config, kbuild will build the initramfs in.
Appending the dtb and running mkimage has been working fine for me.

That's how I get to /sys/kernel/debug/pinctrl :)

thx,

Jason.

device tree binding documentation outdated

[Russell King - ARM Linux]

On Fri, Sep 27, 2013 at 09:15:54AM -0400, Jason Cooper wrote:
> Russell,
> 
> On Fri, Sep 27, 2013 at 12:29:07AM +0100, Russell King - ARM Linux wrote:
> > I haven't sorted out a rootfs for this yet, so it fails trying to find
> > that.  (Rabeeh's kernel contained an initramfs buildroot image - it
> > might be useful to find some way to load that as a separate image -
> > does appended DT support that?)
> 
> yes, point to it in your config, kbuild will build the initramfs in.
> Appending the dtb and running mkimage has been working fine for me.

Yes... if I could rip the initramfs out of Rabeeh's kernel.

Having sorted out the beer problem (pintctrl-names), I now have ethernet
sort of working, but not quite.  It produces data on my LAN, but that
data is not correct.

Here's an extract from the interesting bits that I captured using another
machine (other bytes are zeros):

	0x0000:  ffff ffff ffff d063 0000 0000 0800 4500
	0x0010:  0240 0000 0000 0800 4500 0240 0000 4000
	0x0020:  4011 38ae 0000 0000 4000 4011 38ae 0043
	0x0030:  0200 ffff ffff 0044 0043 022c ffff ffff
	0x0040:  0600 5fe9 bf2c 0000 0101 0600 5fe9 bfd8
	0x0050:  0002 0000 0000 0000 0000 0002 0000 0000
	0x0060:  0000 0000 0000 0000 0000 0000 0000 0000
	0x0070:  0000 0000 0000 0000 d063 0000 0000 0000
	0x0080:  0000 d063 0000 0000 0000 0000 0000 0000
...
	0x01d0:  0000 0000 0000 0000 0000 0000 0000 6382
	0x01e0:  0000 0000 0000 0000 0382 5363 3501 0137
	0x01f0:  0801 0306 5363 3501 0137 0000 0006 0c0f
	0x0200:  111a 28ff 0000 0000 0c0f 111a 0000 0000

That data has a pattern to it.  It's supposed to be a DHCP packet, which
would be something like (this is from the same board but booting Rabeeh's
kernel instead):

	0x0000:  ffff ffff ffff d063 0000 0000 0800 4500
	0x0010:  0240 0000 4000 4011 38ae 0000 0000 ffff
	0x0020:  ffff 0044 0043 022c 0000 0101 0600 5e9a
	0x0030:  dac5 0000 0000 0000 0000 0000 0000 0000
	0x0040:  0000 0000 0000 d063 0000 0000 0000 0000
	0x0050:  0000 0000 0000 0000 0000 0000 0000 0000
...
	0x0110:  0000 0000 0000 6382 5363 3501 0137 0801
	0x0120:  0306 0c0f 111a 28ff 0000 0000 0000 0000

Now, there's a pattern to the corruption - let me rearrange them slightly:

ff ff ff ff ff ff d0 63

00 00 00 00 08 00 45 00 02 40
00 00 00 00 08 00 45 00 02 40

00 00 40 00 40 11 38 ae 00 00
00 00 40 00 40 11 38 ae

00 43 02 00 ff ff ff ff 00 44
00 43 02 2c ff ff ff ff 06 00
5f e9 bf 2c 00 00 01 01 06 00
5f e9 bf d8

00 02 00 00 00 00 00 00 00 00
00 02 00 00 00 00 00 00 00 00

00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00
d0 63 00 00 00 00 00 00 00 00
d0 63 00 00 00 00 00 00 00 00
00 00 00 00
...

00 00 00 00 
00 00 00 00 00 00 00 00 00 00
63 82 00 00 00 00 00 00 00 00
03 82 53 63 35 01 01 37 08 01
03 06 53 63 35 01 01 37 00 00
00 06 0c 0f 11 1a 28 ff 00 00
00 00 0c 0f 11 1a 00 00 00 00

That's the same order of bytes as in the original packet.  Notice how there
are identical bytes between ones below and above, and notice how the repeated
groups of bytes are always 10 bytes apart.  No, it's not giving any errors
in the interface which I ran tcpdump on!

I'm not yet sure what could cause this...

device tree binding documentation outdated

[Russell King - ARM Linux]

On Fri, Sep 27, 2013 at 06:05:52PM +0100, Russell King - ARM Linux wrote:
> On Fri, Sep 27, 2013 at 09:15:54AM -0400, Jason Cooper wrote:
> > Russell,
> > 
> > On Fri, Sep 27, 2013 at 12:29:07AM +0100, Russell King - ARM Linux wrote:
> > > I haven't sorted out a rootfs for this yet, so it fails trying to find
> > > that.  (Rabeeh's kernel contained an initramfs buildroot image - it
> > > might be useful to find some way to load that as a separate image -
> > > does appended DT support that?)
> > 
> > yes, point to it in your config, kbuild will build the initramfs in.
> > Appending the dtb and running mkimage has been working fine for me.
> 
> Yes... if I could rip the initramfs out of Rabeeh's kernel.
> 
> Having sorted out the beer problem (pintctrl-names), I now have ethernet
> sort of working, but not quite.  It produces data on my LAN, but that
> data is not correct.
> 
> Here's an extract from the interesting bits that I captured using another
> machine (other bytes are zeros):
> 
> 	0x0000:  ffff ffff ffff d063 0000 0000 0800 4500
> 	0x0010:  0240 0000 0000 0800 4500 0240 0000 4000
> 	0x0020:  4011 38ae 0000 0000 4000 4011 38ae 0043
> 	0x0030:  0200 ffff ffff 0044 0043 022c ffff ffff
> 	0x0040:  0600 5fe9 bf2c 0000 0101 0600 5fe9 bfd8
> 	0x0050:  0002 0000 0000 0000 0000 0002 0000 0000
> 	0x0060:  0000 0000 0000 0000 0000 0000 0000 0000
> 	0x0070:  0000 0000 0000 0000 d063 0000 0000 0000
> 	0x0080:  0000 d063 0000 0000 0000 0000 0000 0000
> ...
> 	0x01d0:  0000 0000 0000 0000 0000 0000 0000 6382
> 	0x01e0:  0000 0000 0000 0000 0382 5363 3501 0137
> 	0x01f0:  0801 0306 5363 3501 0137 0000 0006 0c0f
> 	0x0200:  111a 28ff 0000 0000 0c0f 111a 0000 0000
> 
> That data has a pattern to it.  It's supposed to be a DHCP packet, which
> would be something like (this is from the same board but booting Rabeeh's
> kernel instead):
> 
> 	0x0000:  ffff ffff ffff d063 0000 0000 0800 4500
> 	0x0010:  0240 0000 4000 4011 38ae 0000 0000 ffff
> 	0x0020:  ffff 0044 0043 022c 0000 0101 0600 5e9a
> 	0x0030:  dac5 0000 0000 0000 0000 0000 0000 0000
> 	0x0040:  0000 0000 0000 d063 0000 0000 0000 0000
> 	0x0050:  0000 0000 0000 0000 0000 0000 0000 0000
> ...
> 	0x0110:  0000 0000 0000 6382 5363 3501 0137 0801
> 	0x0120:  0306 0c0f 111a 28ff 0000 0000 0000 0000
> 
> Now, there's a pattern to the corruption - let me rearrange them slightly:
> 
> ff ff ff ff ff ff d0 63
> 
> 00 00 00 00 08 00 45 00 02 40
> 00 00 00 00 08 00 45 00 02 40
> 
> 00 00 40 00 40 11 38 ae 00 00
> 00 00 40 00 40 11 38 ae
> 
> 00 43 02 00 ff ff ff ff 00 44
> 00 43 02 2c ff ff ff ff 06 00
> 5f e9 bf 2c 00 00 01 01 06 00
> 5f e9 bf d8
> 
> 00 02 00 00 00 00 00 00 00 00
> 00 02 00 00 00 00 00 00 00 00
> 
> 00 00 00 00 00 00 00 00 00 00
> 00 00 00 00 00 00 00 00 00 00
> d0 63 00 00 00 00 00 00 00 00
> d0 63 00 00 00 00 00 00 00 00
> 00 00 00 00
> ...
> 
> 00 00 00 00 
> 00 00 00 00 00 00 00 00 00 00
> 63 82 00 00 00 00 00 00 00 00
> 03 82 53 63 35 01 01 37 08 01
> 03 06 53 63 35 01 01 37 00 00
> 00 06 0c 0f 11 1a 28 ff 00 00
> 00 00 0c 0f 11 1a 00 00 00 00
> 
> That's the same order of bytes as in the original packet.  Notice how there
> are identical bytes between ones below and above, and notice how the repeated
> groups of bytes are always 10 bytes apart.  No, it's not giving any errors
> in the interface which I ran tcpdump on!
> 
> I'm not yet sure what could cause this...

An interesting data point.  Connect it to a 100mbit switch and it works.
gigabit and it behaves as above.

So, the question is: does anyone have gigabit networking working on imx6
with recent DT based kernels?

device tree binding documentation outdated

[Fabio Estevam]

On Fri, Sep 27, 2013 at 3:31 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Fri, Sep 27, 2013 at 06:05:52PM +0100, Russell King - ARM Linux wrote:

> An interesting data point.  Connect it to a 100mbit switch and it works.
> gigabit and it behaves as above.
>
> So, the question is: does anyone have gigabit networking working on imx6
> with recent DT based kernels?

Yes, we have Gigabit ethernet working on mx6sabrelite, wandboard,
sabresd boards.

Does the 3.0.35 kernel work with gigabit on cubox-i? Could this issue
be related to the ar8035 phy fixup code?

device tree binding documentation outdated

[Matt Sealey]

On Fri, Sep 27, 2013 at 1:52 PM, Fabio Estevam <festevam@gmail.com> wrote:
> On Fri, Sep 27, 2013 at 3:31 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
>> On Fri, Sep 27, 2013 at 06:05:52PM +0100, Russell King - ARM Linux wrote:
>
>> An interesting data point.  Connect it to a 100mbit switch and it works.
>> gigabit and it behaves as above.
>>
>> So, the question is: does anyone have gigabit networking working on imx6
>> with recent DT based kernels?
>
> Yes, we have Gigabit ethernet working on mx6sabrelite, wandboard,
> sabresd boards.
>
> Does the 3.0.35 kernel work with gigabit on cubox-i? Could this issue
> be related to the ar8035 phy fixup code?

The AR8035 has a PHY ID of 0x004dd072 and the code in the i.MX6
platform stuff specifies 0x004dd074 - so if it's really an 8035 then
it's not being run..

That said... it would still need to be set up the same way if it was
supplying a 125MHz clock.

Why isn't this in some PHY driver code and why isn't this being marked
as required by some kind of PHY node property anyway? Platform init
code is NOT the place to screw with this stuff... it wouldn't work if
phylib & drivers were modules anyway, right?

-- 
Matt

device tree binding documentation outdated

[Russell King - ARM Linux]

On Fri, Sep 27, 2013 at 03:16:48PM -0500, Matt Sealey wrote:
> On Fri, Sep 27, 2013 at 1:52 PM, Fabio Estevam <festevam@gmail.com> wrote:
> > On Fri, Sep 27, 2013 at 3:31 PM, Russell King - ARM Linux
> > <linux@arm.linux.org.uk> wrote:
> >> On Fri, Sep 27, 2013 at 06:05:52PM +0100, Russell King - ARM Linux wrote:
> >
> >> An interesting data point.  Connect it to a 100mbit switch and it works.
> >> gigabit and it behaves as above.
> >>
> >> So, the question is: does anyone have gigabit networking working on imx6
> >> with recent DT based kernels?
> >
> > Yes, we have Gigabit ethernet working on mx6sabrelite, wandboard,
> > sabresd boards.
> >
> > Does the 3.0.35 kernel work with gigabit on cubox-i? Could this issue
> > be related to the ar8035 phy fixup code?
> 
> The AR8035 has a PHY ID of 0x004dd072 and the code in the i.MX6
> platform stuff specifies 0x004dd074 - so if it's really an 8035 then
> it's not being run..

I've had this in my kernel since earlier today, after a previous comment
about where to add this fixup.  You may notice that I said earlier
in this thread - around 1pm UK time - that I'd already confirmed that
*this* fixup function *is* being run, and I even quoted this code in
that email... :)

As for "if it's really" I really have read the numbers off the top of
the chip and it "really" is an AR8035.  I wouldn't have the data sheet
and been probing its pads directly.

The phy is mounted between the MX6 (the big chip) and the HDMI connector.
It's crystal is immediately beside it.

At this point I'm going to say: please do not make the mistake of
thinking I'm just a software engineer.  I'm not.  I'm a hardware
engineer who still designs stuff from time to time.

diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
index 85a1b51..4da6c68 100644
--- a/arch/arm/mach-imx/mach-imx6q.c
+++ b/arch/arm/mach-imx/mach-imx6q.c
@@ -164,6 +164,46 @@ static int ar8031_phy_fixup(struct phy_device *dev)
 
 #define PHY_ID_AR8031	0x004dd074
 
+static int ar8035_phy_fixup(struct phy_device *dev)
+{
+	u16 val;
+
+	/* Ar803x phy SmartEEE feature cause link status generates glitch,
+	 * which cause ethernet link down/up issue, so disable SmartEEE
+	 */
+	phy_write(dev, 0xd, 0x3);
+	phy_write(dev, 0xe, 0x805d);
+	phy_write(dev, 0xd, 0x4003);
+
+	val = phy_read(dev, 0xe);
+	phy_write(dev, 0xe, val & ~(1 << 8));
+
+	/* To enable AR8031 output a 125MHz clk from CLK_25M */
+	phy_write(dev, 0xd, 0x7);
+	phy_write(dev, 0xe, 0x8016);
+	phy_write(dev, 0xd, 0x4007);
+
+	val = phy_read(dev, 0xe);
+	val &= 0xffe3;
+	val |= 0x18;
+	phy_write(dev, 0xe, val);
+
+	/* introduce tx clock delay */
+	phy_write(dev, 0x1d, 0x5);
+	val = phy_read(dev, 0x1e);
+	val |= 0x0100;
+	phy_write(dev, 0x1e, val);
+
+	/*check phy power*/
+	val = phy_read(dev, 0x0);
+	if (val & BMCR_PDOWN)
+		phy_write(dev, 0x0, val & ~BMCR_PDOWN);
+
+	return 0;
+}
+
+#define PHY_ID_AR8035 0x004dd072
+
 static void __init imx6q_enet_phy_init(void)
 {
 	if (IS_BUILTIN(CONFIG_PHYLIB)) {
@@ -173,6 +213,8 @@ static void __init imx6q_enet_phy_init(void)
 				ksz9031rn_phy_fixup);
 		phy_register_fixup_for_uid(PHY_ID_AR8031, 0xffffffff,
 				ar8031_phy_fixup);
+		phy_register_fixup_for_uid(PHY_ID_AR8035, 0xffffffef,
+				ar8035_phy_fixup);
 	}
 }
 

device tree binding documentation outdated

[Fabio Estevam]

On Thu, Sep 26, 2013 at 5:59 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:

> Here's Rabeeh's preliminary patches against Freescale's 3.0.35 BSP 4.1.0
> kernel:
>
> http://download.solid-run.com/pub/solidrun/c1/kernel/initial/

Ok, that helps. I assume they are using the AR8035 in RGMII mode.

We also need to provide the AR8035 reset (GPIO4_15, but could not see
from their code which pad that corresponds to) via 'phy-reset-gpios'
in the dts file.

>
> I think I've translated the bulk of them.  I have a number of problems
> though.  One of the things I'd like to sort out is the ethernet.  Most
> of these convert straight:
>
> +       MX6DL_PAD_ENET_MDIO__ENET_MDIO,
> +       MX6DL_PAD_ENET_MDC__ENET_MDC,
> +       IOMUX_PAD(0x0650, 0x0268, 5, 0x0000, 0, MX6DL_ENET_PAD_CTRL_PD), /* KEY_ROW4 reset signal */
> +
> +       MX6DL_PAD_DI0_PIN2__GPIO_4_18, /* Interrupt */
> +       /* RMII */
> +       IOMUX_PAD(0x05B4, 0x01E4, 1, 0x0828, 0, PAD_CTL_PKE | PAD_CTL_PUE  |
> +                 PAD_CTL_PUS_100K_DOWN), /* MX6DL_PAD_ENET_CRS_DV__ENET_RX_EN */
> +       IOMUX_PAD(0x05C8, 0x01F8, 1, 0x0818, 0, PAD_CTL_PKE | PAD_CTL_PUE  |
> +                 PAD_CTL_PUS_100K_DOWN), /* MX6DL_PAD_ENET_RXD0__ENET_RDATA_0 */
> +       IOMUX_PAD(0x05CC, 0x01FC, 1, 0x081C, 0, PAD_CTL_PKE | PAD_CTL_PUE  |
> +                 PAD_CTL_PUS_100K_DOWN), /* MX6DL_PAD_ENET_RXD1__ENET_RDATA_1 */
> +       MX6DL_PAD_ENET_TXD0__ENET_TDATA_0,
> +       MX6DL_PAD_ENET_TXD1__ENET_TDATA_1,
> +       MX6DL_PAD_ENET_TX_EN__ENET_TX_EN,
> +       MX6DL_PAD_GPIO_16__ENET_ANATOP_ETHERNET_REF_OUT,
> +       MX6DL_PAD_RGMII_TXC__ENET_RGMII_TXC,
> +       MX6DL_PAD_RGMII_TD0__ENET_RGMII_TD0,
> +       MX6DL_PAD_RGMII_TD1__ENET_RGMII_TD1,
> +       MX6DL_PAD_RGMII_TD2__ENET_RGMII_TD2,
> +       MX6DL_PAD_RGMII_TD3__ENET_RGMII_TD3,
> +       MX6DL_PAD_RGMII_TX_CTL__ENET_RGMII_TX_CTL,
> +       MX6DL_PAD_ENET_REF_CLK__ENET_TX_CLK,
> +       MX6DL_PAD_RGMII_RXC__ENET_RGMII_RXC,
> +       IOMUX_PAD(0x0694, 0x02AC, 1, 0x0818, 1, MX6DL_ENET_PAD_CTRL_PD),/*RGMII RD0*/
> +       IOMUX_PAD(0x0698, 0x02B0, 1, 0x081C, 1, MX6DL_ENET_PAD_CTRL_PD),/*RGMII RD1*/
> +       /* In RGMII mode RD2 should be '1' to disable the PLL OFF mode */
> +       MX6DL_PAD_RGMII_RD2__ENET_RGMII_RD2,
> +       MX6DL_PAD_RGMII_RD3__ENET_RGMII_RD3,
> +       /* In RGMII mode RX_DV should be pulled down for reset strap */
> +       IOMUX_PAD(0x06A4, 0x02BC, 1, 0x0828, 1, MX6DL_ENET_PAD_CTRL_PD),/*RGMII RXCTL*/
>
> The ones which don't are those IOMUX_PAD() ones - what I have so far for
> them (I'm still busy adding to the DT file for everything else) are
> (in order):
>
>         MX6QDL_PAD_ENET_MDIO__ENET_MDIO
>         MX6QDL_PAD_ENET_MDC__ENET_MDC
>         MX6QDL_PAD_KEY_ROW4__GPIO4_IO15 -- ?
>         MX6QDL_PAD_DI0_PIN2__GPIO4_IO18
>         /* RMII */
>         MX6QDL_PAD_ENET_CRS_DV__ENET_RX_EN ... extra stuff
>         MX6QDL_PAD_ENET_RXD0__ENET_RX_DATA0 ... extra stuff
>         MX6QDL_PAD_ENET_RXD1__ENET_RX_DATA1 ... extra stuff
>         MX6QDL_PAD_ENET_TXD0__ENET_TX_DATA0
>         MX6QDL_PAD_ENET_TXD1__ENET_TX_DATA1
>         MX6QDL_PAD_ENET_TX_EN__ENET_TX_EN
>         MX6QDL_PAD_GPIO_16__ENET_REF_CLK  -- ?
>         MX6QDL_PAD_RGMII_TXC__RGMII_TXC
>         MX6QDL_PAD_RGMII_TD0__RGMII_TD0
>         MX6QDL_PAD_RGMII_TD1__RGMII_TD1
>         MX6QDL_PAD_RGMII_TD2__RGMII_TD2
>         MX6QDL_PAD_RGMII_TD3__RGMII_TD3
>         MX6QDL_PAD_RGMII_TX_CTL__RGMII_TX_CTL
>         MX6QDL_PAD_ENET_REF_CLK__ENET_TX_CLK
>         MX6QDL_PAD_RGMII_RXC__RGMII_RXC
>         MX6QDL_PAD_RGMII_RD0__RGMII_RD0 ... extra stuff
>         MX6QDL_PAD_RGMII_RD1__RGMII_RD1 ... extra stuff
>         /* In RGMII mode RD2 should be '1' to disable the PLL OFF mode */
>         MX6QDL_PAD_RGMII_RD2__RGMII_RD2
>         MX6QDL_PAD_RGMII_RD3__RGMII_RD3
>         /* In RGMII mode RX_DV should be pulled down for reset strap */
>         MX6QDL_PAD_RGMII_RX_CTL__RGMII_RX_CTL ... extra stuff
>
> If I have to customize any of these settings, how do I do that?

Looks like the ethernet pads used in this board are the same as in
pinctrl_enet_1, but the pad settings may differ.

In order to customize it, you could create a pinctrl_enet_1_cuboxi
inside its dts file.

The configuration is explained at (CONFIG bits definition):
Documentation/devicetree/bindings/pinctrl/fsl,imx6dl-pinctrl.txt

> Final question is - in imx6qdl.dtsi, I see for instance:
>
>         MX6QDL_PAD_ENET_MDIO__ENET_MDIO       0x1b0b0
>
> What does the final number refer to?

The final number represents the pad settings of this pad (will map to
the register IOMUXC_SW_PAD_CTL_PAD_ENET_MDIO)

The meaning of each bit is also described at
Documentation/devicetree/bindings/pinctrl/fsl,imx6dl-pinctrl.txt

0x1b0b0 is the default value of the IOMUXC_SW_PAD_CTL_PAD_ENET_MDIO
register (section 37.4.363 of the mx6dl reference manual).

Regards,

Fabio Estevam

device tree binding documentation outdated

[Russell King - ARM Linux]

On Thu, Sep 26, 2013 at 08:25:49PM -0300, Fabio Estevam wrote:
> On Thu, Sep 26, 2013 at 5:59 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> 
> > Here's Rabeeh's preliminary patches against Freescale's 3.0.35 BSP 4.1.0
> > kernel:
> >
> > http://download.solid-run.com/pub/solidrun/c1/kernel/initial/
> 
> Ok, that helps. I assume they are using the AR8035 in RGMII mode.

It appears so.

> We also need to provide the AR8035 reset (GPIO4_15, but could not see
> from their code which pad that corresponds to) via 'phy-reset-gpios'
> in the dts file.

Okay, I've added this fixup function:

static int ar8035_phy_fixup(struct phy_device *dev)
{
	u16 val;
printk("ar8035 found\n");
	/* Ar803x phy SmartEEE feature cause link status generates glitch,
	 * which cause ethernet link down/up issue, so disable SmartEEE
	 */
	phy_write(dev, 0xd, 0x3);
	phy_write(dev, 0xe, 0x805d);
	phy_write(dev, 0xd, 0x4003);

	val = phy_read(dev, 0xe);
	phy_write(dev, 0xe, val & ~(1 << 8));

	/* To enable AR8031 output a 125MHz clk from CLK_25M */
	phy_write(dev, 0xd, 0x7);
	phy_write(dev, 0xe, 0x8016);
	phy_write(dev, 0xd, 0x4007);

	val = phy_read(dev, 0xe);
	val &= 0xffe3;
	val |= 0x18;
	phy_write(dev, 0xe, val);

	/* introduce tx clock delay */
	phy_write(dev, 0x1d, 0x5);
	val = phy_read(dev, 0x1e);
	val |= 0x0100;
	phy_write(dev, 0x1e, val);

	/*check phy power*/
	val = phy_read(dev, 0x0);
	if (val & BMCR_PDOWN)
		phy_write(dev, 0x0, val & ~BMCR_PDOWN);

	return 0;
}

#define PHY_ID_AR8035 0x004dd072

static void __init imx6q_enet_phy_init(void)
{
	if (IS_BUILTIN(CONFIG_PHYLIB)) {
...
		phy_register_fixup_for_uid(PHY_ID_AR8035, 0xffffffef,
				ar8035_phy_fixup);
	}
}

and in the dts file:

&iomuxc {
	enet {
		pinctrl_enet_1_cuboxi: enetgrp-4 {
			fsl,pins = <
				MX6QDL_PAD_ENET_MDIO__ENET_MDIO 	0x1b0b0
				MX6QDL_PAD_ENET_MDC__ENET_MDC		0x1b0b0
				MX6QDL_PAD_KEY_ROW4__GPIO4_IO15 	0x130b0
				MX6QDL_PAD_DI0_PIN2__GPIO4_IO18 	0x80000000
				/* RMII */
				MX6QDL_PAD_ENET_CRS_DV__ENET_RX_EN	0x03000
				MX6QDL_PAD_ENET_RXD0__ENET_RX_DATA0	0x03000
				MX6QDL_PAD_ENET_RXD1__ENET_RX_DATA1	0x03000
				MX6QDL_PAD_ENET_TXD0__ENET_TX_DATA0	0x80000000
				MX6QDL_PAD_ENET_TXD1__ENET_TX_DATA1	0x80000000
				MX6QDL_PAD_ENET_TX_EN__ENET_TX_EN	0x80000000
				//MX6QDL_PAD_GPIO_16__ENET_REF_CLK               Output on AR8035
				MX6QDL_PAD_RGMII_TXC__RGMII_TXC 	0x1b0b0
				MX6QDL_PAD_RGMII_TD0__RGMII_TD0 	0x1b0b0
				MX6QDL_PAD_RGMII_TD1__RGMII_TD1 	0x1b0b0
				MX6QDL_PAD_RGMII_TD2__RGMII_TD2 	0x1b0b0
				MX6QDL_PAD_RGMII_TD3__RGMII_TD3 	0x1b0b0
				MX6QDL_PAD_RGMII_TX_CTL__RGMII_TX_CTL	0x1b0b0
				MX6QDL_PAD_ENET_REF_CLK__ENET_TX_CLK	0x1b0b0
				MX6QDL_PAD_RGMII_RXC__RGMII_RXC 	0x1b0b0
				MX6QDL_PAD_RGMII_RD0__RGMII_RD0 	0x130b0
				MX6QDL_PAD_RGMII_RD1__RGMII_RD1 	0x130b0
				/* In RGMII mode RD2 should be '1' to disable the PLL OFF mode */
				MX6QDL_PAD_RGMII_RD2__RGMII_RD2 	0x130b0
				MX6QDL_PAD_RGMII_RD3__RGMII_RD3 	0x130b0
				/* In RGMII mode RX_DV should be pulled down for reset strap */
				MX6QDL_PAD_RGMII_RX_CTL__RGMII_RX_CTL	0x130b0
			>;
		};
	};
};

&fec {
	pintctrl-names = "default";
	pinctrl-0 = <&pinctrl_enet_1_cuboxi>;
	phy-mode = "rgmii";
	phy-reset-duration = <2>;
	phy-reset-gpios = <&gpio4 15 0>;
	status = "okay";
};

I get my "ar8035 found" so the fixup is working fine.  I also see the
ethernet link ok lamp go out and come back on, so I think the reset is
working.  However, I see no traffic from it - I'm booting with ip=dhcp
as the test at the moment.

I notice Rabeeh also does this:

+       imx6q_add_anatop_thermal_imx(1, &mx6q_c1_anatop_thermal_data);
+       /* Set GPR1, bit 21 to 1 */
+       mxc_iomux_set_gpr_register(1, 21, 1, 1);

Bit 21 appears to be IMX6Q_GPR1_ENET_CLK_SEL_MASK /
IMX6Q_GPR1_ENET_CLK_SEL_ANATOP, which I see imx6q_1588_init() is doing.
This happens some time before the phy fixup function is called, so I
think that's covered.

There's also this which follows the above:

+       /* Set enet clock to 50MHz RMII */
+       enet = clk_get_sys("enet.0", NULL);
+       if (IS_ERR(enet))
+               pr_err("Unable to get enet.0 clock\n");
+       else {
+               clk_prepare(enet);
+               clk_set_rate(enet, 50000000);
+               clk_enable(enet);
+       }

I'm not sure at the moment which clock that refers to, or whether that's
necessary only for the AR8030 (which is used in RMII mode).  The enable
bit seems to be CCM_CCGR1 CG5_OFFSET.  Even when I work out how that
relates to the mass of clock initialisations in
arch/arm/mach-imx/clk-imx6q.c, it's virtually impossible to translate
that into a DT <&clks N> representation because of this:

enum mx6q_clks {
        dummy, ckil, ckih, osc, pll2_pfd0_352m, pll2_pfd1_594m, pll2_pfd2_396m,
        pll3_pfd0_720m, pll3_pfd1_540m, pll3_pfd2_508m, pll3_pfd3_454m,
        pll2_198m, pll3_120m, pll3_80m, pll3_60m, twd, step, pll1_sw,
        periph_pre, periph2_pre, periph_clk2_sel, periph2_clk2_sel, axi_sel,

Is this sane?  Is it sane to expect people to sit and count these fscking
things?  No.  Add some comments, mark every 10 or so, or something like
that.  How this is at the moment, it's completely inpenetrable.

Given this, I'd much rather write board support in C rather than use this
disgusting DT crap which just seems to be designed to make it extremely
difficult to bring boards up.  I'm beginning to believe that DT is a
means to lock-in people to various vendors who understand this junk.

device tree binding documentation outdated

[Shawn Guo]

On Fri, Sep 27, 2013 at 01:13:57PM +0100, Russell King - ARM Linux wrote:
> There's also this which follows the above:
> 
> +       /* Set enet clock to 50MHz RMII */
> +       enet = clk_get_sys("enet.0", NULL);
> +       if (IS_ERR(enet))
> +               pr_err("Unable to get enet.0 clock\n");

I can not find clock lookup for "enet.0" in FSL 3.0.35 BSP 4.1.0, so
I'm not sure how this works at all.  Can you confirm that by running
Rabeeh's kernel to see if you get that error message?

> +       else {
> +               clk_prepare(enet);
> +               clk_set_rate(enet, 50000000);
> +               clk_enable(enet);
> +       }
> 
> I'm not sure at the moment which clock that refers to, or whether that's
> necessary only for the AR8030 (which is used in RMII mode).  The enable
> bit seems to be CCM_CCGR1 CG5_OFFSET.

There are two clocks ENET related, 'enet' (117) and 'enet_ref' (190).
The enet is fec internal working clock, and enet_ref is provided by
imx6q CCM (Clock Controller Module) on pad ENET_REF_CLK which can
generally be used to clock the external PHY.

The CCM_CCGR1_CG5 is clock 'enet', while I think the clock in above
code should be 'enet_ref'.  It seems you need to set up
MX6QDL_PAD_GPIO_16__ENET_REF_CLK to use the clock though.

> Even when I work out how that
> relates to the mass of clock initialisations in
> arch/arm/mach-imx/clk-imx6q.c, it's virtually impossible to translate
> that into a DT <&clks N> representation because of this:
> 
> enum mx6q_clks {
>         dummy, ckil, ckih, osc, pll2_pfd0_352m, pll2_pfd1_594m, pll2_pfd2_396m,
>         pll3_pfd0_720m, pll3_pfd1_540m, pll3_pfd2_508m, pll3_pfd3_454m,
>         pll2_198m, pll3_120m, pll3_80m, pll3_60m, twd, step, pll1_sw,
>         periph_pre, periph2_pre, periph_clk2_sel, periph2_clk2_sel, axi_sel,

The imx6q clock was created before DTC macro support is available, so we
have to cross check Documentation/devicetree/bindings/clock/imx6q-clock.txt
to interpret the clocks.  The imx6sl clock driver below is an example to
see how DTC macro helps there.

  arch/arm/mach-imx/clk-imx6sl.c
  include/dt-bindings/clock/imx6sl-clock.h

Shawn

device tree binding documentation outdated

[Russell King - ARM Linux]

On Fri, Sep 27, 2013 at 09:26:02PM +0800, Shawn Guo wrote:
> On Fri, Sep 27, 2013 at 01:13:57PM +0100, Russell King - ARM Linux wrote:
> > There's also this which follows the above:
> > 
> > +       /* Set enet clock to 50MHz RMII */
> > +       enet = clk_get_sys("enet.0", NULL);
> > +       if (IS_ERR(enet))
> > +               pr_err("Unable to get enet.0 clock\n");
> 
> I can not find clock lookup for "enet.0" in FSL 3.0.35 BSP 4.1.0, so
> I'm not sure how this works at all.  Can you confirm that by running
> Rabeeh's kernel to see if you get that error message?

Here . Trying to find clock enet.0 (null)
Returning clk_lookup at 824f9854
Here . Trying to find clock (null) esai_clk
Returning clk_lookup at 824f95ac
Here . Trying to find clock (null) pll3_pfd_508M
Returning clk_lookup at 824f919c
Here . Trying to find clock (null) asrc_clk
Returning clk_lookup at 824f96ec

So it seems to be able to get that clock.  I'm wondering if it's based
on BSP 4.0.0 which does have that clock, rather than 4.1.0.

> > +       else {
> > +               clk_prepare(enet);
> > +               clk_set_rate(enet, 50000000);
> > +               clk_enable(enet);
> > +       }
> > 
> > I'm not sure at the moment which clock that refers to, or whether that's
> > necessary only for the AR8030 (which is used in RMII mode).  The enable
> > bit seems to be CCM_CCGR1 CG5_OFFSET.
> 
> There are two clocks ENET related, 'enet' (117) and 'enet_ref' (190).
> The enet is fec internal working clock, and enet_ref is provided by
> imx6q CCM (Clock Controller Module) on pad ENET_REF_CLK which can
> generally be used to clock the external PHY.

In this case, the phy provides the clock to the IMX6.

> The CCM_CCGR1_CG5 is clock 'enet', while I think the clock in above
> code should be 'enet_ref'.  It seems you need to set up
> MX6QDL_PAD_GPIO_16__ENET_REF_CLK to use the clock though.

Okay, and it seems Rabeeh is setting the enet clock (117) to 50MHz.
It defaults to 66MHz for me in 3.12-rc1, but I've hacked clk-imx6q.c
to call clk_set_rate(clk[enet], 50000000).

I've changed the enet_ref clock pinctrl settings to:

        /* ENET_REF_CLK is an output on AR8035 - SION must be set */
        MX6QDL_PAD_GPIO_16__ENET_REF_CLK        0xc0000000
(which expands to: 0x214 0x5e4 0x80c 0x2 0x0 0xc0000000)

which in Rabeeh's version corresponds to:

        MX6DL_PAD_GPIO_16__ENET_ANATOP_ETHERNET_REF_OUT,
(which expands to: IOMUX_PAD(0x05E4, 0x0214, 0x12, 0x080C, 0, NO_PAD_CTRL))

I've been through all the pinctrl settings for this now, taking them
back to the hex numbers and comparing.  I'm quite certain I have the
pinctrl settings correct now.

Probing the AR8035, I see that it's generating its 125M clock, as per
the fixup function, so at least that's working.

I've just enabled the debug in pinctrl-imx.c, and I get this:

imx6dl-pinctrl 20e0000.iomuxc: group(3): enetgrp-4
imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_RESERVE0: 1 0x0001b0b0
imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_RESERVE1: 1 0x0001b0b0
imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_RESERVE2: 5 0x000130b0
imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_RESERVE3: 5 0x80000000
imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_RESERVE4: 1 0x00003000
imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_RESERVE5: 1 0x00003000
imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_RESERVE6: 1 0x00003000
imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_RESERVE7: 1 0x80000000
imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_RESERVE8: 1 0x80000000
imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_RESERVE9: 1 0x80000000
imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_RESERVE10: 18 0x80000000
imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_RESERVE11: 1 0x80000000
imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_RESERVE12: 1 0x0001b0b0
imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_RESERVE13: 1 0x0001b0b0
imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_RESERVE14: 1 0x0001b0b0
imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_RESERVE15: 1 0x0001b0b0
imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_RESERVE16: 1 0x0001b0b0
imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_RESERVE17: 1 0x0000a0b1
imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_RESERVE18: 1 0x0001b0b0
imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_CSI0_DAT10: 1 0x000130b0
imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_CSI0_DAT11: 1 0x000130b0
imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_CSI0_DAT12: 1 0x0001b0b0
imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_CSI0_DAT13: 1 0x0001b0b0
imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_CSI0_DAT14: 1 0x000130b0
...
brd: module loaded
loop: module loaded
imx6dl-pinctrl 20e0000.iomuxc: maps: function enet group enetgrp-4 num 19
ar8035_phy_fixup()
libphy: fec_enet_mii_bus: probed
fec 2188000.ethernet eth0: registered PHC device 0

So where is the pinctrl settings applied - or more to the point, why
aren't they being applied?  This would probably explain why ethernet
doesn't work - I mean, if the pinctrl settings are never written to
the registers...

device tree binding documentation outdated

[Russell King - ARM Linux]

On Fri, Sep 27, 2013 at 04:19:00PM +0100, Russell King - ARM Linux wrote:
> brd: module loaded
> loop: module loaded
> imx6dl-pinctrl 20e0000.iomuxc: maps: function enet group enetgrp-4 num 19
> ar8035_phy_fixup()
> libphy: fec_enet_mii_bus: probed
> fec 2188000.ethernet eth0: registered PHC device 0
> 
> So where is the pinctrl settings applied - or more to the point, why
> aren't they being applied?  This would probably explain why ethernet
> doesn't work - I mean, if the pinctrl settings are never written to
> the registers...

I found that.  A typo in the DT file.  pintctrl-names.  Clearly beer
is required.

(And that's also a good argument why we need a schema language to
verify DT bindings against.)

device tree binding documentation outdated

[Matt Sealey]

Russell,

There is a chicken bit in the IOMUXC GPR1 (IOMUXC+0x4 bit 21, it's on
page 1927 of the i.MX6DQRM Rev 1 04/2013) which turns on or off the
ENET_REF_CLK loopback. You could need to clear it to make the pad an
input to the i.MX6 rather than an output to the PHY.. while it may
seem like this removes the need for the SION bit in the pin you want,
that's actually also required otherwise the pad mux latch 'sees' the
data but the input latch behind it doesn't.

There is a great diagram somewhere in the manual (Figure 28-1, right
at the top of the GPIO docs) - SION *forces* data (it's the input_on
signal in the diagram IIRC) to the gpio data register behind the
IOMUX, but the logic behind every muxed pad is almost the same across
the SoC.

In essence, the pad control setting needs to be correct (SION set) and
that chicken bit correctly cleared, or the clock input never goes into
the MAC, because the MAC isn't connected directly to the pad, it's
connected to the input latch. The only logic block that can 'really'
read the pad latch is the GPIO unit.

Note that you MAY have to cycle the clock or reset the PHY or MAC
*after* setting the pin up, otherwise the MAC won't get the clock..
it's really a laborious process which isn't described properly in the
DT (all it needs is a property to define where it's clock comes from
and Do The Right Thing (tm) at driver init.. because this is
infuriatingly common on i.MX6 designs, but not so common that everyone
needs it)


On Fri, Sep 27, 2013 at 10:19 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Fri, Sep 27, 2013 at 09:26:02PM +0800, Shawn Guo wrote:
>> On Fri, Sep 27, 2013 at 01:13:57PM +0100, Russell King - ARM Linux wrote:
>> > There's also this which follows the above:
>> >
>> > +       /* Set enet clock to 50MHz RMII */
>> > +       enet = clk_get_sys("enet.0", NULL);
>> > +       if (IS_ERR(enet))
>> > +               pr_err("Unable to get enet.0 clock\n");
>>
>> I can not find clock lookup for "enet.0" in FSL 3.0.35 BSP 4.1.0, so
>> I'm not sure how this works at all.  Can you confirm that by running
>> Rabeeh's kernel to see if you get that error message?
>
> Here . Trying to find clock enet.0 (null)
> Returning clk_lookup at 824f9854
> Here . Trying to find clock (null) esai_clk
> Returning clk_lookup at 824f95ac
> Here . Trying to find clock (null) pll3_pfd_508M
> Returning clk_lookup at 824f919c
> Here . Trying to find clock (null) asrc_clk
> Returning clk_lookup at 824f96ec
>
> So it seems to be able to get that clock.  I'm wondering if it's based
> on BSP 4.0.0 which does have that clock, rather than 4.1.0.
>
>> > +       else {
>> > +               clk_prepare(enet);
>> > +               clk_set_rate(enet, 50000000);
>> > +               clk_enable(enet);
>> > +       }
>> >
>> > I'm not sure at the moment which clock that refers to, or whether that's
>> > necessary only for the AR8030 (which is used in RMII mode).  The enable
>> > bit seems to be CCM_CCGR1 CG5_OFFSET.
>>
>> There are two clocks ENET related, 'enet' (117) and 'enet_ref' (190).
>> The enet is fec internal working clock, and enet_ref is provided by
>> imx6q CCM (Clock Controller Module) on pad ENET_REF_CLK which can
>> generally be used to clock the external PHY.
>
> In this case, the phy provides the clock to the IMX6.
>
>> The CCM_CCGR1_CG5 is clock 'enet', while I think the clock in above
>> code should be 'enet_ref'.  It seems you need to set up
>> MX6QDL_PAD_GPIO_16__ENET_REF_CLK to use the clock though.
>
> Okay, and it seems Rabeeh is setting the enet clock (117) to 50MHz.
> It defaults to 66MHz for me in 3.12-rc1, but I've hacked clk-imx6q.c
> to call clk_set_rate(clk[enet], 50000000).
>
> I've changed the enet_ref clock pinctrl settings to:
>
>         /* ENET_REF_CLK is an output on AR8035 - SION must be set */
>         MX6QDL_PAD_GPIO_16__ENET_REF_CLK        0xc0000000
> (which expands to: 0x214 0x5e4 0x80c 0x2 0x0 0xc0000000)
>
> which in Rabeeh's version corresponds to:
>
>         MX6DL_PAD_GPIO_16__ENET_ANATOP_ETHERNET_REF_OUT,
> (which expands to: IOMUX_PAD(0x05E4, 0x0214, 0x12, 0x080C, 0, NO_PAD_CTRL))
>
> I've been through all the pinctrl settings for this now, taking them
> back to the hex numbers and comparing.  I'm quite certain I have the
> pinctrl settings correct now.
>
> Probing the AR8035, I see that it's generating its 125M clock, as per
> the fixup function, so at least that's working.
>
> I've just enabled the debug in pinctrl-imx.c, and I get this:
>
> imx6dl-pinctrl 20e0000.iomuxc: group(3): enetgrp-4
> imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_RESERVE0: 1 0x0001b0b0
> imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_RESERVE1: 1 0x0001b0b0
> imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_RESERVE2: 5 0x000130b0
> imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_RESERVE3: 5 0x80000000
> imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_RESERVE4: 1 0x00003000
> imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_RESERVE5: 1 0x00003000
> imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_RESERVE6: 1 0x00003000
> imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_RESERVE7: 1 0x80000000
> imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_RESERVE8: 1 0x80000000
> imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_RESERVE9: 1 0x80000000
> imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_RESERVE10: 18 0x80000000
> imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_RESERVE11: 1 0x80000000
> imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_RESERVE12: 1 0x0001b0b0
> imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_RESERVE13: 1 0x0001b0b0
> imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_RESERVE14: 1 0x0001b0b0
> imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_RESERVE15: 1 0x0001b0b0
> imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_RESERVE16: 1 0x0001b0b0
> imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_RESERVE17: 1 0x0000a0b1
> imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_RESERVE18: 1 0x0001b0b0
> imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_CSI0_DAT10: 1 0x000130b0
> imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_CSI0_DAT11: 1 0x000130b0
> imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_CSI0_DAT12: 1 0x0001b0b0
> imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_CSI0_DAT13: 1 0x0001b0b0
> imx6dl-pinctrl 20e0000.iomuxc: MX6DL_PAD_CSI0_DAT14: 1 0x000130b0
> ...
> brd: module loaded
> loop: module loaded
> imx6dl-pinctrl 20e0000.iomuxc: maps: function enet group enetgrp-4 num 19
> ar8035_phy_fixup()
> libphy: fec_enet_mii_bus: probed
> fec 2188000.ethernet eth0: registered PHC device 0
>
> So where is the pinctrl settings applied - or more to the point, why
> aren't they being applied?  This would probably explain why ethernet
> doesn't work - I mean, if the pinctrl settings are never written to
> the registers...
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

device tree binding documentation outdated

[Russell King - ARM Linux]

On Fri, Sep 27, 2013 at 11:52:52AM -0500, Matt Sealey wrote:
> Russell,
> 
> There is a chicken bit in the IOMUXC GPR1 (IOMUXC+0x4 bit 21, it's on
> page 1927 of the i.MX6DQRM Rev 1 04/2013) which turns on or off the
> ENET_REF_CLK loopback. You could need to clear it to make the pad an
> input to the i.MX6 rather than an output to the PHY.. while it may
> seem like this removes the need for the SION bit in the pin you want,
> that's actually also required otherwise the pad mux latch 'sees' the
> data but the input latch behind it doesn't.
> 
> There is a great diagram somewhere in the manual (Figure 28-1, right
> at the top of the GPIO docs) - SION *forces* data (it's the input_on
> signal in the diagram IIRC) to the gpio data register behind the
> IOMUX, but the logic behind every muxed pad is almost the same across
> the SoC.

The manual which Fabio pointed me at (iMX 6Solo/6DualLite - IMX6SDLRM.pdf)
doesn't correspond - figure 28-1 is something entirely different.

> In essence, the pad control setting needs to be correct (SION set) and
> that chicken bit correctly cleared, or the clock input never goes into
> the MAC, because the MAC isn't connected directly to the pad, it's
> connected to the input latch. The only logic block that can 'really'
> read the pad latch is the GPIO unit.
> 
> Note that you MAY have to cycle the clock or reset the PHY or MAC
> *after* setting the pin up, otherwise the MAC won't get the clock..
> it's really a laborious process which isn't described properly in the
> DT (all it needs is a property to define where it's clock comes from
> and Do The Right Thing (tm) at driver init.. because this is
> infuriatingly common on i.MX6 designs, but not so common that everyone
> needs it)

Okay, but from what I'm working from (which works) the order is:

- Set IOMUXC GPR1 bit 21.
- Set ipg/ahb ethernet clock to 50MHz
- IOMUX settings applied, including setting the reference clock SION bit
- Reset lowered, wait 2ms, reset raised
- imx6_init_fec(fec_data_rgmii) called

After that is the time that the phy is configured to supply the 125MHz
reference clock into the IMX6.

That order seems to be happening with my 3.12-rc1 kernel, although a
little more spread out.

Thanks.

device tree binding documentation outdated

[Matt Sealey]

On Fri, Sep 27, 2013 at 12:49 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Fri, Sep 27, 2013 at 11:52:52AM -0500, Matt Sealey wrote:
>> Russell,
>>
>> There is a chicken bit in the IOMUXC GPR1 (IOMUXC+0x4 bit 21, it's on
>> page 1927 of the i.MX6DQRM Rev 1 04/2013) which turns on or off the
>> ENET_REF_CLK loopback. You could need to clear it to make the pad an
>> input to the i.MX6 rather than an output to the PHY.. while it may
>> seem like this removes the need for the SION bit in the pin you want,
>> that's actually also required otherwise the pad mux latch 'sees' the
>> data but the input latch behind it doesn't.
>>
>> There is a great diagram somewhere in the manual (Figure 28-1, right
>> at the top of the GPIO docs) - SION *forces* data (it's the input_on
>> signal in the diagram IIRC) to the gpio data register behind the
>> IOMUX, but the logic behind every muxed pad is almost the same across
>> the SoC.
>
> The manual which Fabio pointed me at (iMX 6Solo/6DualLite - IMX6SDLRM.pdf)
> doesn't correspond - figure 28-1 is something entirely different.

I use the Quad/Dual (not Lite) manual for reference, most of it is
identical.. I keep forgetting there is another one, sorry.

It's at the top of the GPIO section, though (figure 29-1 in your manual).

> Okay, but from what I'm working from (which works) the order is:
>
> - Set IOMUXC GPR1 bit 21.

Just to clarify: set or clear? It should be clear if the PHY provides
the clock to the MAC, and set if the MAC provides the clock to the
PHY. There can be only one, Highlander!

> - Set ipg/ahb ethernet clock to 50MHz

Hmm.. this might be a touch low, but it's not the issue here. The way
I recall this working, the IP has a clock (IPG, AHB as above) and
there is a MAC clock too which you're generating from DIV_SELECT in
the ENET_PLL. They don't have to be the same..

> - IOMUX settings applied, including setting the reference clock SION bit

Which definitely implies the clock is input to the MAC...

> - Reset lowered, wait 2ms, reset raised
> - imx6_init_fec(fec_data_rgmii) called
>
> After that is the time that the phy is configured to supply the 125MHz
> reference clock into the IMX6.
>
> That order seems to be happening with my 3.12-rc1 kernel, although a
> little more spread out.

If the GPR1 bit isn't set right what you're getting is different
clocks when they should be sharing the same one... extra edges from
the internal ref clock (from ENET_PLL by default) might add a couple
extra bytes here and there if the PHY sends on edges it's clock and
the MAC receives on edges on the internal clock.

In 3.12-ish I see mach-imx/mach-imx6q.c:imx6q_1588_init() is setting
the bit, which means the reference clock is coming from
ENET_PLL[DIV_SELECT] and not the pad.

If you set it to 1, it sets pad loopback which means you'd output a
125MHz clock towards a pad you marked as input to the i.MX6 - the
IOMUX block will twiddle some internal flops from clock -> output
block but since it's not hooked to the pad, nothing will come of it.
But the MAC will use it to latch data over MDIO.

None of the boards in the tree right now seem to use an external PHY
clock at all, which is why they all work; they provide the PHY clock
from the i.MX6Q to an RGMII PHY (this is the lowest power way to do
it!).

This is wrong for you... but I have seen a couple that do it this way.
There was some terrible confusion with older MX6 tapeouts where this
bit wasn't documented and using RMII was a complete afterthought...
the manual reflects it now but I am fairly sure there is a quirk that
needs addressing.

In theory the i.MX6SL EVK has RMII but I am also fairly sure the
SoloLite doesn't have the option to use anything but RMII and either
fortuitous software or chicken-bit-does-nothing-here design is helping
it out.

I'm gonna go look at the manual some more.. for reference, this is
also a great design guide - IMX6QD6SDLHDG.pdf (it's on the same site
as you got the RM). Chapter 12 is comprehensive in it's discussion of
i.MX6QD[L] and RMII ethernet evil.

Does anyone at Freescale want to donate a SabreSD for Dual or Quad
(not-lite) and an i.MX6SL EVK this way? I may be able to scrounge up a
couple CuBox-i units but I think supporting the reference designs is
worth doing. Also I kind of want/need JTAG access and I am sure
SolidRun didn't even go so far as putting pads on the PCB...

-- 
Matt <neko@bakuhatsu.net>

device tree binding documentation outdated

[Russell King - ARM Linux]

I've got good news... finally!

On Fri, Sep 27, 2013 at 01:33:35PM -0500, Matt Sealey wrote:
> On Fri, Sep 27, 2013 at 12:49 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > Okay, but from what I'm working from (which works) the order is:
> >
> > - Set IOMUXC GPR1 bit 21.
> 
> Just to clarify: set or clear? It should be clear if the PHY provides
> the clock to the MAC, and set if the MAC provides the clock to the
> PHY. There can be only one, Highlander!

Well, the patch tells me:

+       /* Set GPR1, bit 21 to 1 */
+       mxc_iomux_set_gpr_register(1, 21, 1, 1);

> > - Set ipg/ahb ethernet clock to 50MHz
> 
> Hmm.. this might be a touch low, but it's not the issue here. The way
> I recall this working, the IP has a clock (IPG, AHB as above) and
> there is a MAC clock too which you're generating from DIV_SELECT in
> the ENET_PLL. They don't have to be the same..

Actually, this is the issue.  I now have doubts that the patches which
Rabeeh gave us early-developers correspond with the kernel binary he
rushed to us on Wednesday (he wasn't expecting them to be delivered soo
quickly.)

Rabeeh says in the wiki that his patches should be applied on top of
3.0.35 BSP 4.1.0, and the patches contain this:

+       /* Set enet clock to 50MHz RMII */
+       enet = clk_get_sys("enet.0", NULL);
+       if (IS_ERR(enet))
+               pr_err("Unable to get enet.0 clock\n");
+       else {
+               clk_prepare(enet);
+               clk_set_rate(enet, 50000000);
+               clk_enable(enet);
+       }

As was pointed out earlier, there is no device specific clock named
"enet.0" in BSP 4.1.0, but there is in BSP 4.0.0.  However, Rabeeh's
kernel doesn't complain with "Unable to get enet.0 clock", so I just
assumed that there's something that was missed from the diff which
allows that to work.

The real answer is: don't believe what you see in the patch!  This
will fail on BSP 4.1.0.  Removing my hack to do this in my kernel...
gets the ethernet working at both gigabit and 100mbit speeds!

Now, I'm not going to say that anything with regard to that GPR bit
is correct - but what I have now is something which does indeed work.
I did hang a scope on that signal (my scope probes have small tips
which can get on the solder pads up to the AR8035) and it didn't
show signs of any contention.

At this point, now that I have it working, I'm happy, and I think I'm
going to find some beer to satisfy that "pintctrl-names" mistake :)

device tree binding documentation outdated

[Matt Sealey]

On Fri, Sep 27, 2013 at 2:05 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> I've got good news... finally!
>
> On Fri, Sep 27, 2013 at 01:33:35PM -0500, Matt Sealey wrote:
>> On Fri, Sep 27, 2013 at 12:49 PM, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>> > Okay, but from what I'm working from (which works) the order is:
>> >
>> > - Set IOMUXC GPR1 bit 21.
>>
>> Just to clarify: set or clear? It should be clear if the PHY provides
>> the clock to the MAC, and set if the MAC provides the clock to the
>> PHY. There can be only one, Highlander!
>
> Well, the patch tells me:
>
> +       /* Set GPR1, bit 21 to 1 */
> +       mxc_iomux_set_gpr_register(1, 21, 1, 1);

But you're setting the MAC clock up, and the MAC clock always goes out
over RGMII_TXC and the PHY needs to use it.. so it works.

But that means setting up SION bits and setting ENET_REF_CLK up for
input is absolutely pointless..

This means the clock is being generated by the ethernet PLL

>> > - Set ipg/ahb ethernet clock to 50MHz
>>
>> Hmm.. this might be a touch low, but it's not the issue here. The way
>> I recall this working, the IP has a clock (IPG, AHB as above) and
>> there is a MAC clock too which you're generating from DIV_SELECT in
>> the ENET_PLL. They don't have to be the same..
>
> Actually, this is the issue.  I now have doubts that the patches which
> Rabeeh gave us early-developers correspond with the kernel binary he
> rushed to us on Wednesday (he wasn't expecting them to be delivered soo
> quickly.)
>
> Rabeeh says in the wiki that his patches should be applied on top of
> 3.0.35 BSP 4.1.0, and the patches contain this:
>
> +       /* Set enet clock to 50MHz RMII */

Note, RMII, not RGMII !!!!

Can you tell us what board you got:

http://cubox-i.com/table/

Subtle difference which I know I just faceplanted on in my previous
mail.. Same concept though... but if you have a DualLite then you
don't HAVE gigabit ethernet... a 125MHz clock is ridiculous, and Fabio
gave you the wrong manual.

If it's a Dual or Quad, then you do have gigabit... and the below applies..

> The real answer is: don't believe what you see in the patch!  This
> will fail on BSP 4.1.0.  Removing my hack to do this in my kernel...
> gets the ethernet working at both gigabit and 100mbit speeds!
>
> Now, I'm not going to say that anything with regard to that GPR bit
> is correct - but what I have now is something which does indeed work.
> I did hang a scope on that signal (my scope probes have small tips
> which can get on the solder pads up to the AR8035) and it didn't
> show signs of any contention.

I'm glad you have a scope..

It's not contention at all - the clocks never see each other on the
same pins. But there would be TWO 125MHz clocks being generated. One
at the MAC - which outputs to the PHY via TXC. And also you're
instructing the PHY to generate a 125MHz clock which should be input
to the MAC.. and it would use this for RXC too, and possibly for
internal clocking when it reads data accompanied TXC being clocked.
Screwy..

Usually you have an XTALIN on the PHY, and a REF_CLK out. There are
two main configurations;

* XTALIN connected to an oscillator, generating REF_CLK which is sent to the MAC
* XTALIN is connected to the MAC, REF_CLK is useless

The difference would be

* In the top situation, your PHY is generating the clock for the
entire system - the i.MX6 takes it and uses it drive logic and to
clock receive and sends it back out on TXC to transmit data (GPR1[21]
= 0)
* In the bottom situation, the i.MX6 generates a clock, the PHY uses
it to drive it's logic AND generate other clocks, and the i.MX6 clock
is looped back through the pad to the MAC to drive it's logic and
clock transmit data (GPR1[21] = 1)

And if you're driving it how I think you are what you're doing is
seeing XTALIN to the PHY from an external clock, the PHY is generating
ENET_REF_CLK to the i.MX6, but the i.MX6 is ignoring it and driving
it's logic and clocking data from it's internal reference clock.

Given the boards aren't released and the kernel you got isn't public,
I can't even look at the mess... is there any way you can convince
these guys to chop out a tiny section of the schematic vis a vis PHY
connection and strapping to the i.MX6, hopefully with some indicator
data direction? It's REALLY not possible, however you look at it, to
develop a device tree pinmuxing spec for a board without the
schematic. Who knows what horse shit is in the code they sent (if it's
based on the Freescale BSP, there could be upwards of 3 or 4 weird
reference-board-specific hacks littered around that don't apply to
their custom design and you shouldn't copy-paste them in)

The question is here, really, does the PHY have it's own crystal
input, and is it meant to provide the 125MHz clock, and do we want to
drive the MAC with it? Or does it get it's clock from the i.MX6?

--
Matt

device tree binding documentation outdated

[Matt Sealey]

On Fri, Sep 27, 2013 at 2:41 PM, Matt Sealey <neko@bakuhatsu.net> wrote:
> On Fri, Sep 27, 2013 at 2:05 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
>> I've got good news... finally!
>>
> The difference would be
>
> * In the top situation, your PHY is generating the clock for the
> entire system - the i.MX6 takes it and uses it drive logic and to
> clock receive and sends it back out on TXC to transmit data (GPR1[21]
> = 0)
> * In the bottom situation, the i.MX6 generates a clock, the PHY uses
> it to drive it's logic AND generate other clocks, and the i.MX6 clock
> is looped back through the pad to the MAC to drive it's logic and
> clock transmit data (GPR1[21] = 1)

Ignore that. It's not totally wrong but it's weird. This is really
difficult to put into words...

GPR1[21] = 0 i.MX6 receives a clock from the PHY. You need to generate
a clock at the PHY as input to the MAC. The MAC won't run until the
PHY is out of reset and the clock is generated. You may need SION set
for RMII depending on the pin...
GPR1[21] = 1 implies we generate a clock from i.MX6 TO the PHY. The
clock is looped back through the pad to the MAC. SION is irrelevant.

Still want to know who's clock it is and where it should be going....

device tree binding documentation outdated

[Russell King - ARM Linux]

On Fri, Sep 27, 2013 at 02:48:16PM -0500, Matt Sealey wrote:
> On Fri, Sep 27, 2013 at 2:41 PM, Matt Sealey <neko@bakuhatsu.net> wrote:
> > On Fri, Sep 27, 2013 at 2:05 PM, Russell King - ARM Linux
> > <linux@arm.linux.org.uk> wrote:
> >> I've got good news... finally!
> >>
> > The difference would be
> >
> > * In the top situation, your PHY is generating the clock for the
> > entire system - the i.MX6 takes it and uses it drive logic and to
> > clock receive and sends it back out on TXC to transmit data (GPR1[21]
> > = 0)
> > * In the bottom situation, the i.MX6 generates a clock, the PHY uses
> > it to drive it's logic AND generate other clocks, and the i.MX6 clock
> > is looped back through the pad to the MAC to drive it's logic and
> > clock transmit data (GPR1[21] = 1)
> 
> Ignore that. It's not totally wrong but it's weird. This is really
> difficult to put into words...
> 
> GPR1[21] = 0 i.MX6 receives a clock from the PHY. You need to generate
> a clock at the PHY as input to the MAC. The MAC won't run until the
> PHY is out of reset and the clock is generated. You may need SION set
> for RMII depending on the pin...
> GPR1[21] = 1 implies we generate a clock from i.MX6 TO the PHY. The
> clock is looped back through the pad to the MAC. SION is irrelevant.
> 
> Still want to know who's clock it is and where it should be going....

As I said, when Rabeeh is around, I will ask questions about this.
I'm fairly certain that we don't need _all_ those ethernet pinmux
settings (we just need the RGMII ones, and can omit the MII ones.)

Having hacked on this solidly since about 5pm yesterday, I'm not going
to spend very much time on this until later next week, or I can talk to
Rabeeh.

device tree binding documentation outdated

[Russell King - ARM Linux]

On Fri, Sep 27, 2013 at 09:21:10PM +0100, Russell King - ARM Linux wrote:
> On Fri, Sep 27, 2013 at 02:48:16PM -0500, Matt Sealey wrote:
> > Ignore that. It's not totally wrong but it's weird. This is really
> > difficult to put into words...
> > 
> > GPR1[21] = 0 i.MX6 receives a clock from the PHY. You need to generate
> > a clock at the PHY as input to the MAC. The MAC won't run until the
> > PHY is out of reset and the clock is generated. You may need SION set
> > for RMII depending on the pin...
> > GPR1[21] = 1 implies we generate a clock from i.MX6 TO the PHY. The
> > clock is looped back through the pad to the MAC. SION is irrelevant.
> > 
> > Still want to know who's clock it is and where it should be going....
> 
> As I said, when Rabeeh is around, I will ask questions about this.
> I'm fairly certain that we don't need _all_ those ethernet pinmux
> settings (we just need the RGMII ones, and can omit the MII ones.)
> 
> Having hacked on this solidly since about 5pm yesterday, I'm not going
> to spend very much time on this until later next week, or I can talk to
> Rabeeh.

Okay, the answer is that yes, GPR1 bit 21 should be clear on this version
of the hardware, but there's plans to have the IMX6 generate the clock
and remove the crystal for the AR8035.

Now, here's the question for the IMX6 maintainers: how do I avoid having
GPR1 bit 21 set - it's unconditionally set by imx6q_1588_init() in
arch/arm/mach-imx/mach-imx6q.c.  Moreover, how do I control this from
DT?

device tree binding documentation outdated

[Shawn Guo]

On Sat, Sep 28, 2013 at 09:38:59AM +0100, Russell King - ARM Linux wrote:
> Okay, the answer is that yes, GPR1 bit 21 should be clear on this version
> of the hardware, but there's plans to have the IMX6 generate the clock
> and remove the crystal for the AR8035.

Be careful, as mentioned in the reply to Matt, in that case, IMX6 can
only output the clock on pad GPIO_16, and the clock has to be externally
routed back to IMX6 on pad ENET_REF_CLK, so that ENET can get reference
clock for RGMII mode.

> Now, here's the question for the IMX6 maintainers: how do I avoid having
> GPR1 bit 21 set - it's unconditionally set by imx6q_1588_init() in
> arch/arm/mach-imx/mach-imx6q.c.  Moreover, how do I control this from
> DT?

GPR1[21] should be cleared only when there is a clock input on pad
GPIO_16 from external phy or oscillator.  Other than that, GPR1[21]
should always be set to loop the ENET_PLL clock back to ENET though
pad GPIO_16, for at least getting PTP (IEEE 1588) work, even in RGMII
mode.

So far, we haven't got any user with that setup (external phy or
oscillator generates clock to pad GPIO_16).  When we do, we can add a
device tree property for telling that.

Shawn

device tree binding documentation outdated

[Duan Fugang-B38611]

From:  Shawn Guo <shawn.guo@linaro.org>
Data: Sunday, September 29, 2013 2:13 PM

> To: Russell King - ARM Linux
> Cc: devicetree at vger.kernel.org; Fabio Estevam; linux-arm-
> kernel at lists.infradead.org; Matt Sealey
> Subject: Re: device tree binding documentation outdated
> 
> On Sat, Sep 28, 2013 at 09:38:59AM +0100, Russell King - ARM Linux wrote:
> > Okay, the answer is that yes, GPR1 bit 21 should be clear on this
> > version of the hardware, but there's plans to have the IMX6 generate
> > the clock and remove the crystal for the AR8035.
> 
> Be careful, as mentioned in the reply to Matt, in that case, IMX6 can only
> output the clock on pad GPIO_16, and the clock has to be externally routed
> back to IMX6 on pad ENET_REF_CLK, so that ENET can get reference clock for
> RGMII mode.
> 
> > Now, here's the question for the IMX6 maintainers: how do I avoid
> > having
> > GPR1 bit 21 set - it's unconditionally set by imx6q_1588_init() in
> > arch/arm/mach-imx/mach-imx6q.c.  Moreover, how do I control this from
> > DT?
> 
> GPR1[21] should be cleared only when there is a clock input on pad
> GPIO_16 from external phy or oscillator.  Other than that, GPR1[21] should
> always be set to loop the ENET_PLL clock back to ENET though pad GPIO_16,
> for at least getting PTP (IEEE 1588) work, even in RGMII mode.
> 
> So far, we haven't got any user with that setup (external phy or
> oscillator generates clock to pad GPIO_16).  When we do, we can add a
> device tree property for telling that.

Shawn, for RGMII mode, RGMII tx_clk cannot get external clock via GPIO_16, only by ENET_REF_CLK PAD.
	 For RMII mode, enet refrenece clock can get external clock via GPIO_16 and RGMII_TX_CTL PADs.

Thanks,
Andy

device tree binding documentation outdated

[Shawn Guo]

On Sun, Sep 29, 2013 at 06:23:39AM +0000, Duan Fugang-B38611 wrote:
> Shawn, for RGMII mode, RGMII tx_clk cannot get external clock via GPIO_16, only by ENET_REF_CLK PAD.
> 	 For RMII mode, enet refrenece clock can get external clock via GPIO_16 and RGMII_TX_CTL PADs.

Yes, that's my understanding too.  Please review the following message
to see if everything stands correct there.

http://thread.gmane.org/gmane.linux.drivers.devicetree/46115/focus=269646

Shawn

device tree binding documentation outdated

[Duan Fugang-B38611]

From: Shawn Guo [mailto:shawn.guo at linaro.org]
Sent: Sunday, September 29, 2013 2:35 PM

> Yes, that's my understanding too.  Please review the following message to
> see if everything stands correct there.
> 
> http://thread.gmane.org/gmane.linux.drivers.devicetree/46115/focus=269646
> 
Correct.

Thanks,
Andy

device tree binding documentation outdated

[Russell King - ARM Linux]

On Sun, Sep 29, 2013 at 02:13:05PM +0800, Shawn Guo wrote:
> On Sat, Sep 28, 2013 at 09:38:59AM +0100, Russell King - ARM Linux wrote:
> > Okay, the answer is that yes, GPR1 bit 21 should be clear on this version
> > of the hardware, but there's plans to have the IMX6 generate the clock
> > and remove the crystal for the AR8035.
> 
> Be careful, as mentioned in the reply to Matt, in that case, IMX6 can
> only output the clock on pad GPIO_16, and the clock has to be externally
> routed back to IMX6 on pad ENET_REF_CLK, so that ENET can get reference
> clock for RGMII mode.

Let me repeat that in case it was unclear.

The AR8035 on the current boards has a 25MHz crystal, and the AR8035 is
*currently* required to be programmed to generate a 125MHz clock back to
the IMX6 for the transmit timings.

In future, and on the real cubox-i, this crystal will not be present, and
it will be required that the IMX6 generates the 25MHz clock for the
AR8035.

So, for the _existing_ hardware, it is _required_ that the AR8035
generates the transmit clock, and this is fed in on GPIO 16 as
ENET_REF_CLK.  This means that SION for this *must* be set and
GPR1 bit 21 *must* be clear.

So:

> > Now, here's the question for the IMX6 maintainers: how do I avoid having
> > GPR1 bit 21 set - it's unconditionally set by imx6q_1588_init() in
> > arch/arm/mach-imx/mach-imx6q.c.  Moreover, how do I control this from
> > DT?
> 
> GPR1[21] should be cleared only when there is a clock input on pad
> GPIO_16 from external phy or oscillator.  Other than that, GPR1[21]
> should always be set to loop the ENET_PLL clock back to ENET though
> pad GPIO_16, for at least getting PTP (IEEE 1588) work, even in RGMII
> mode.
> 
> So far, we haven't got any user with that setup (external phy or
> oscillator generates clock to pad GPIO_16).  When we do, we can add a
> device tree property for telling that.

We need a property for this if we wish to support the carrier-1 development
board that Solid-run sent out to a load of developers last week.  If not,
I guess those guys like Olof will find that they won't have properly
configured ethernet support on their boards they've received in mainline
kernels.

I think that as a fair number of key kernel developers now have this
board with this requirement, we need to have this DT property provided
even though the final cubox-i will not require it.

device tree binding documentation outdated

[Russell King - ARM Linux]

On Wed, Oct 02, 2013 at 08:33:16PM +0100, Russell King - ARM Linux wrote:
> On Sun, Sep 29, 2013 at 02:13:05PM +0800, Shawn Guo wrote:
> > On Sat, Sep 28, 2013 at 09:38:59AM +0100, Russell King - ARM Linux wrote:
> > > Okay, the answer is that yes, GPR1 bit 21 should be clear on this version
> > > of the hardware, but there's plans to have the IMX6 generate the clock
> > > and remove the crystal for the AR8035.
> > 
> > Be careful, as mentioned in the reply to Matt, in that case, IMX6 can
> > only output the clock on pad GPIO_16, and the clock has to be externally
> > routed back to IMX6 on pad ENET_REF_CLK, so that ENET can get reference
> > clock for RGMII mode.
> 
> Let me repeat that in case it was unclear.
> 
> The AR8035 on the current boards has a 25MHz crystal, and the AR8035 is
> *currently* required to be programmed to generate a 125MHz clock back to
> the IMX6 for the transmit timings.
> 
> In future, and on the real cubox-i, this crystal will not be present, and
> it will be required that the IMX6 generates the 25MHz clock for the
> AR8035.
> 
> So, for the _existing_ hardware, it is _required_ that the AR8035
> generates the transmit clock, and this is fed in on GPIO 16 as
> ENET_REF_CLK.  This means that SION for this *must* be set and
> GPR1 bit 21 *must* be clear.

Okay, I think I'm getting to the bottom of this.

RGMII mode.

In current hardware, the phy has its own 25MHz crystal.  This provides
the phy with its clocks, and the phy itself contains a PLL, which it
uses to generate 125MHz.

The phy can be (and is) configured to generate this 125MHz clock on
the CLK_25M pin, which is connected to the IMX6 ENET_REF_CLK pad (V22
ball).

Hence, we have in DT:
	MX6QDL_PAD_ENET_REF_CLK__ENET_TX_CLK

So far, so good. 


On current hardware, GPIO 16 is not connected to anything except some
0402 resistor pads where the resistor is not fitted.  On future hardware,
the above crystal will be removed, and the appropriate 0402 resistor
placed to supply a clock provided on GPIO 16 to the phy.

Hence, to support both hardware configurations transparently, GPIO 16
should be configured to generate a 25MHz clock for use by the phy.

This apparantly is done by configuring the PLL on the IMX6 to 25MHz and
getting it to output it on this pin, and that is done in the freescale
BSP by this pin control setting:

	MX6DL_PAD_GPIO_16__ENET_ANATOP_ETHERNET_REF_OUT

Now, if you look up that in the 4.1.0 BSP:

#define MX6DL_PAD_GPIO_16__ENET_ANATOP_ETHERNET_REF_OUT                        \
		IOMUX_PAD(0x05E4, 0x0214, 0x12, 0x080C, 0, NO_PAD_CTRL)

and that says to write 0x12 to 0x20e0000 + 0x214 - the
IOMUXC_SW_MUX_CTL_PAD_GPIO16 register.  This name clearly implies that
the IMX6 is _outputting_ the reference clock.  The register values here
selects MUX_MODE 2 (ENET_REF_CLK) _with_ SION set.  There is no
configuration in this BSP for GPIO 16 in mux mode 2 without SION set.

However, because we want this pad to produce a clock from the internal
PLL, GPR1 bit 21 has to be set.  Remember - the ENET_TX_CLK is configured
to come from the ENET_REF_CLK pad, not GPIO 16.

So, I contend that it _is_ valid to have both GPIO 16 SION set and GPR1
bit 21 set.

All in all, we don't want to clear GPR1 bit 21 ever on the Cubox-i SOM.

Whether SION should be set or not is another - separate - issue.  All
we really care about is getting GPIO 16 providing a 25MHz clock.

device tree binding documentation outdated

[Fabio Estevam]

On Wed, Oct 2, 2013 at 8:49 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:

> This apparantly is done by configuring the PLL on the IMX6 to 25MHz and
> getting it to output it on this pin, and that is done in the freescale
> BSP by this pin control setting:
>
>         MX6DL_PAD_GPIO_16__ENET_ANATOP_ETHERNET_REF_OUT
>
> Now, if you look up that in the 4.1.0 BSP:
>
> #define MX6DL_PAD_GPIO_16__ENET_ANATOP_ETHERNET_REF_OUT                        \
>                 IOMUX_PAD(0x05E4, 0x0214, 0x12, 0x080C, 0, NO_PAD_CTRL)
>
> and that says to write 0x12 to 0x20e0000 + 0x214 - the
> IOMUXC_SW_MUX_CTL_PAD_GPIO16 register.  This name clearly implies that
> the IMX6 is _outputting_ the reference clock.  The register values here
> selects MUX_MODE 2 (ENET_REF_CLK) _with_ SION set.  There is no
> configuration in this BSP for GPIO 16 in mux mode 2 without SION set.

SION bit is set for this pad in mainline as well:

MX6QDL_PAD_GPIO_16__ENET_REF_CLK      0x4001b0a8

(bit 30 is set, which indicates SION).

device tree binding documentation outdated

[Shawn Guo]

On Thu, Oct 03, 2013 at 12:49:11AM +0100, Russell King - ARM Linux wrote:
> RGMII mode.
> 
> In current hardware, the phy has its own 25MHz crystal.  This provides
> the phy with its clocks, and the phy itself contains a PLL, which it
> uses to generate 125MHz.
> 
> The phy can be (and is) configured to generate this 125MHz clock on
> the CLK_25M pin, which is connected to the IMX6 ENET_REF_CLK pad (V22
> ball).
> 
> Hence, we have in DT:
> 	MX6QDL_PAD_ENET_REF_CLK__ENET_TX_CLK
> 
> So far, so good. 
> 
> 
> On current hardware, GPIO 16 is not connected to anything except some
> 0402 resistor pads where the resistor is not fitted.  On future hardware,
> the above crystal will be removed, and the appropriate 0402 resistor
> placed to supply a clock provided on GPIO 16 to the phy.
> 
> Hence, to support both hardware configurations transparently, GPIO 16
> should be configured to generate a 25MHz clock for use by the phy.

On AR8035 XTLI pin?  Here is what I read from AR8035 data sheet.

XTLI (Analog input signal): Crystal oscillator input. Requires a 27 pF
capacitor to GND.  Support external 25 MHz, 1.2V swing clock input
through this pin.

XTLO (Analog output signal): Crystal oscillator output; 27 pF to GND.

Will AR8035 just work if we provide a digital 25Mhz clock (from GPIO_16)
on its XTLI pin?

Shawn

> This apparantly is done by configuring the PLL on the IMX6 to 25MHz and
> getting it to output it on this pin, and that is done in the freescale
> BSP by this pin control setting:
> 
> 	MX6DL_PAD_GPIO_16__ENET_ANATOP_ETHERNET_REF_OUT
> 
> Now, if you look up that in the 4.1.0 BSP:
> 
> #define MX6DL_PAD_GPIO_16__ENET_ANATOP_ETHERNET_REF_OUT                        \
> 		IOMUX_PAD(0x05E4, 0x0214, 0x12, 0x080C, 0, NO_PAD_CTRL)
> 
> and that says to write 0x12 to 0x20e0000 + 0x214 - the
> IOMUXC_SW_MUX_CTL_PAD_GPIO16 register.  This name clearly implies that
> the IMX6 is _outputting_ the reference clock.  The register values here
> selects MUX_MODE 2 (ENET_REF_CLK) _with_ SION set.  There is no
> configuration in this BSP for GPIO 16 in mux mode 2 without SION set.
> 
> However, because we want this pad to produce a clock from the internal
> PLL, GPR1 bit 21 has to be set.  Remember - the ENET_TX_CLK is configured
> to come from the ENET_REF_CLK pad, not GPIO 16.
> 
> So, I contend that it _is_ valid to have both GPIO 16 SION set and GPR1
> bit 21 set.
> 
> All in all, we don't want to clear GPR1 bit 21 ever on the Cubox-i SOM.
> 
> Whether SION should be set or not is another - separate - issue.  All
> we really care about is getting GPIO 16 providing a 25MHz clock.

device tree binding documentation outdated

[Russell King - ARM Linux]

On Fri, Oct 04, 2013 at 11:45:39PM +0800, Shawn Guo wrote:
> On Thu, Oct 03, 2013 at 12:49:11AM +0100, Russell King - ARM Linux wrote:
> > RGMII mode.
> > 
> > In current hardware, the phy has its own 25MHz crystal.  This provides
> > the phy with its clocks, and the phy itself contains a PLL, which it
> > uses to generate 125MHz.
> > 
> > The phy can be (and is) configured to generate this 125MHz clock on
> > the CLK_25M pin, which is connected to the IMX6 ENET_REF_CLK pad (V22
> > ball).
> > 
> > Hence, we have in DT:
> > 	MX6QDL_PAD_ENET_REF_CLK__ENET_TX_CLK
> > 
> > So far, so good. 
> > 
> > 
> > On current hardware, GPIO 16 is not connected to anything except some
> > 0402 resistor pads where the resistor is not fitted.  On future hardware,
> > the above crystal will be removed, and the appropriate 0402 resistor
> > placed to supply a clock provided on GPIO 16 to the phy.
> > 
> > Hence, to support both hardware configurations transparently, GPIO 16
> > should be configured to generate a 25MHz clock for use by the phy.
> 
> On AR8035 XTLI pin?  Here is what I read from AR8035 data sheet.
> 
> XTLI (Analog input signal): Crystal oscillator input. Requires a 27 pF
> capacitor to GND.  Support external 25 MHz, 1.2V swing clock input
> through this pin.
> 
> XTLO (Analog output signal): Crystal oscillator output; 27 pF to GND.
> 
> Will AR8035 just work if we provide a digital 25Mhz clock (from GPIO_16)
> on its XTLI pin?

Well, "Support external 25 MHz, 1.2V swing clock input through this pin."
suggests the answer is yes.  The characterists suggest that if the supplied
clock is within -0.3 to 0.15V for the low level, and 0.8 to 1.5V for the
high level, it should work.

device tree binding documentation outdated

[Shawn Guo]

On Fri, Sep 27, 2013 at 02:48:16PM -0500, Matt Sealey wrote:
> GPR1[21] = 0 i.MX6 receives a clock from the PHY. You need to generate
> a clock at the PHY as input to the MAC. The MAC won't run until the
> PHY is out of reset and the clock is generated. You may need SION set
> for RMII depending on the pin...
> GPR1[21] = 1 implies we generate a clock from i.MX6 TO the PHY. The
> clock is looped back through the pad to the MAC. SION is irrelevant.
> 
> Still want to know who's clock it is and where it should be going....

There are some fuzzy around the imx6 ENET reference clock.  I was not
completely understanding it until I had a talk with the designer.

The following is a simplified and modified 'Table 23-1. ENET External
Signals' from reference manual ENET chapter.  I renamed a few things
(marked with ^^^)to avoid confusion.

Signal           Description                             Mode        Pad 
-------------------------------------------------------------------------------------
ENET_REF_CLK     In RMII mode, this signal is the        RMII        GPIO_16
                 reference clock for receive, transmit,
                 and the control interface.

ENET_TX_REF_CLK  Input clock, which provides a timing    MII/RGMII   PAD_ENET_REF_CLK
        ^^^^     reference for TX_EN, TX_DATA[3:0],         ^^^^^^   ^^^^
                 and TX_ER.

There are two ENET reference clocks.  One is ENET_REF_CLK which is used
by RMII and PTP (Precision Time Protocol, IEEE 1588) sampling, and the
other is ENET_TX_REF_CLK which is used by MII and RGMII.

Clock ENET_REF_CLK can only comes from pad GPIO_16 (RGMII_TX_CTL option
is ignored in this discussion) with either external PHY/oscillator clock
input on the pad (GPR1[21] = 0), or internal ENET_PLL output on the pad
and loop back to ENET_REF_CLK (GPR1[21] = 1) through the pad.

Clock ENET_TX_REF_CLK can only comes from pad PAD_ENET_REF_CLK, which
can only receives a clock input from external source.  This external
source can be RGMII PHY, oscillator or pad GPIO_16.  That said, GPR1[21]
and ENET_PLL do not have any implication on pad PAD_ENET_REF_CLK but
only GPIO_16.  If a board design wants to have i.MX6Q/DL provide 125MHz
clock to RGMII PHY, it has to do something the following digram shows.

                         i.MX6Q/DL
        -------------------------+
                                 |
             ENET_PLL            |             +-----------+
                 |               |             |           |
                 +-----> GPIO_16 |------+----->| RGMII PHY |
                            /    |      |      |           |
            PTP <----------/     |      |      +-----------+
                                 |      |
                PAD_ENET_REF_CLK |<-----+
                 +---/           |
                 |               |
                 v               |
         ENET_TX_REF_CLK         |
                                 |
        -------------------------+

The pad GPIO_16 has to be externally routed to PAD_ENET_REF_CLK, so that
the ENET can get the RGMII reference clock.  There are related
discussions about that as below.

https://community.freescale.com/message/330757#330757
https://community.freescale.com/message/350327#350327

Hope it makes the thing a little bit clear.

Shawn

device tree binding documentation outdated

[Russell King - ARM Linux]

On Fri, Sep 27, 2013 at 02:41:53PM -0500, Matt Sealey wrote:
> On Fri, Sep 27, 2013 at 2:05 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > Well, the patch tells me:
> >
> > +       /* Set GPR1, bit 21 to 1 */
> > +       mxc_iomux_set_gpr_register(1, 21, 1, 1);
> 
> But you're setting the MAC clock up, and the MAC clock always goes out
> over RGMII_TXC and the PHY needs to use it.. so it works.
> 
> But that means setting up SION bits and setting ENET_REF_CLK up for
> input is absolutely pointless..
> 
> This means the clock is being generated by the ethernet PLL

Interesting.  Looking in the patches, there's a need to disable some
modes on the phy - i wonder if this is why.  As I said, I think there
is some scope for experimentation here.  I think that even Rabeeh only
sorted this out internally within solid-run during the last week, so
it could be that some of these settings are sub-optimal.

When I next see Rabeeh around, I'll talk to him about it.

> >> > - Set ipg/ahb ethernet clock to 50MHz
> >>
> >> Hmm.. this might be a touch low, but it's not the issue here. The way
> >> I recall this working, the IP has a clock (IPG, AHB as above) and
> >> there is a MAC clock too which you're generating from DIV_SELECT in
> >> the ENET_PLL. They don't have to be the same..
> >
> > Actually, this is the issue.  I now have doubts that the patches which
> > Rabeeh gave us early-developers correspond with the kernel binary he
> > rushed to us on Wednesday (he wasn't expecting them to be delivered soo
> > quickly.)
> >
> > Rabeeh says in the wiki that his patches should be applied on top of
> > 3.0.35 BSP 4.1.0, and the patches contain this:
> >
> > +       /* Set enet clock to 50MHz RMII */
> 
> Note, RMII, not RGMII !!!!
> 
> Can you tell us what board you got:
> 
> http://cubox-i.com/table/
> 
> Subtle difference which I know I just faceplanted on in my previous
> mail.. Same concept though... but if you have a DualLite then you
> don't HAVE gigabit ethernet... a 125MHz clock is ridiculous, and Fabio
> gave you the wrong manual.
> 
> If it's a Dual or Quad, then you do have gigabit... and the below applies..

Okay, this is where things get a ltitle complicated.  The pre-release
dev hardware is not the final cubox hardware - it's a development
platform, which is currently fitted with a IMX6 Solo with an AR8035
phy.  The IXM6 Solo is gigabit capable, as is the AR8035.

There's support in Rabeeh's patches for an AR8030, which is a fast
ethernet phy (no gigabit).

I suspect that the production Cubox-i hardware will have an AR8030
fitted to the lower end models, but an AR8035 to the higher end,
while the dev hardware is a mix of low-end and high-end.

If you look at my photo of the dev hardware on google+, you'll notice
its quite different from the 2x2x2 form factor of the final product:

https://plus.google.com/103895730806848715870/posts/GMnPj1w8CGZ

> The question is here, really, does the PHY have it's own crystal
> input, ...

Yes it does.

device tree binding documentation outdated

[Fabio Estevam]

Matt,

On Fri, Sep 27, 2013 at 4:41 PM, Matt Sealey <neko@bakuhatsu.net> wrote:
> Subtle difference which I know I just faceplanted on in my previous
> mail.. Same concept though... but if you have a DualLite then you
> don't HAVE gigabit ethernet... a 125MHz clock is ridiculous, and Fabio
> gave you the wrong manual.

What you say here is not correct.

quad and dual: have Gigabit ethernet
dual-lite and solo: have Gigabit ethernet
solo-lite: don't have Gigabit ethernet

Anyway, good news that Russell managed to get ethernet running on cubox-i.

I will probably get access to a cubox-i next week :-)

device tree binding documentation outdated

[Linus Walleij]

On Thu, Sep 26, 2013 at 10:29 PM, Fabio Estevam <festevam@gmail.com> wrote:
> On Thu, Sep 26, 2013 at 4:51 PM, Russell King - ARM Linux

>> I mean the stuff which looks like this:
>>
>> #define MX6QDL_PAD_ENET_MDC__ESAI_TX5_RX0           0x1e8 0x5b8 0x858 0x2 0x0
>
> Let me try to decode it:
>
> 0x1e8: Offset of IOMUXC_SW_MUX_CTL_PAD_ENET_MDC register
>
> 0x5b8: Offset of IOMUXC_SW_PAD_CTL_PAD_ENET_MDC register
>
> 0x2: This means that the MUX_MODE field selects the ESAI_TX5_RX0 function
> in this pad
>
> 0x0: SELECT_INPUT not used for this pad

This I guess predates the inctroduction of defines into the DTC,
but we can do a lot better now.

Can someone from the i.MX camp take a sweep and convert this
to defines in some include/dt-bindings/imx.h or similar file so
this thing becomes readable?

Yours,
Linus Walleij

device tree binding documentation outdated

[Shawn Guo]

On Thu, Sep 26, 2013 at 11:35:43PM +0200, Linus Walleij wrote:
> On Thu, Sep 26, 2013 at 10:29 PM, Fabio Estevam <festevam@gmail.com> wrote:
> > On Thu, Sep 26, 2013 at 4:51 PM, Russell King - ARM Linux
> 
> >> I mean the stuff which looks like this:
> >>
> >> #define MX6QDL_PAD_ENET_MDC__ESAI_TX5_RX0           0x1e8 0x5b8 0x858 0x2 0x0
> >
> > Let me try to decode it:
> >
> > 0x1e8: Offset of IOMUXC_SW_MUX_CTL_PAD_ENET_MDC register
> >
> > 0x5b8: Offset of IOMUXC_SW_PAD_CTL_PAD_ENET_MDC register
> >
> > 0x2: This means that the MUX_MODE field selects the ESAI_TX5_RX0 function
> > in this pad
> >
> > 0x0: SELECT_INPUT not used for this pad
> 
> This I guess predates the inctroduction of defines into the DTC,
> but we can do a lot better now.
> 
> Can someone from the i.MX camp take a sweep and convert this
> to defines in some include/dt-bindings/imx.h or similar file so
> this thing becomes readable?

We call those macros like MX6QDL_PAD_ENET_MDC__ESAI_TX5_RX0 in
imx6q-pinfunc.h pin function ID.  It's all about choosing a pinmux
function (ESAI_TX5_RX0) for given pad (MX6QDL_PAD_ENET_MDC), and
that's all what users care and need to read from the macro.  The
decoding of the macro is the register details that users do not care in
general.  But fsl,pinctrl bindings doc documented these details anyway.

Shawn

device tree binding documentation outdated

[Russell King - ARM Linux]

On Fri, Sep 27, 2013 at 10:51:17AM +0800, Shawn Guo wrote:
> On Thu, Sep 26, 2013 at 11:35:43PM +0200, Linus Walleij wrote:
> > On Thu, Sep 26, 2013 at 10:29 PM, Fabio Estevam <festevam@gmail.com> wrote:
> > > On Thu, Sep 26, 2013 at 4:51 PM, Russell King - ARM Linux
> > 
> > >> I mean the stuff which looks like this:
> > >>
> > >> #define MX6QDL_PAD_ENET_MDC__ESAI_TX5_RX0           0x1e8 0x5b8 0x858 0x2 0x0
> > >
> > > Let me try to decode it:
> > >
> > > 0x1e8: Offset of IOMUXC_SW_MUX_CTL_PAD_ENET_MDC register
> > >
> > > 0x5b8: Offset of IOMUXC_SW_PAD_CTL_PAD_ENET_MDC register
> > >
> > > 0x2: This means that the MUX_MODE field selects the ESAI_TX5_RX0 function
> > > in this pad
> > >
> > > 0x0: SELECT_INPUT not used for this pad
> > 
> > This I guess predates the inctroduction of defines into the DTC,
> > but we can do a lot better now.
> > 
> > Can someone from the i.MX camp take a sweep and convert this
> > to defines in some include/dt-bindings/imx.h or similar file so
> > this thing becomes readable?
> 
> We call those macros like MX6QDL_PAD_ENET_MDC__ESAI_TX5_RX0 in
> imx6q-pinfunc.h pin function ID.  It's all about choosing a pinmux
> function (ESAI_TX5_RX0) for given pad (MX6QDL_PAD_ENET_MDC), and
> that's all what users care and need to read from the macro.  The
> decoding of the macro is the register details that users do not care in
> general.  But fsl,pinctrl bindings doc documented these details anyway.

No Shawn, they don't.

To me, encountering this for the first time, the documentation is
_wrong_ no two ways about it - as I said in my initial email.  Please
fix it, or delete it.  Don't leave it in its current crap state.  Bad
or misleading documentation is worse than no documentation.

I am a "user" of this crap, and it is _very_ confusing that the
documentation is wrong, and the fact that I sent my initial email in
this thread is proof that your statement above is wrong.

For instance, "two integers array" is a lie, plain and simple.  Yes, it
may be true that what users care about is a macro and a number but that
is not a "two integers array".  It would be helpful here to explain that
PIN_FUNC refers to a macro in the imx*-pinfunc.h header file, which
expands to four or five integers.

Bit 31 (or even bit 30) in the config number are not documented
either.

All the numbers come into play when you're trying to port a platform,
because when you encounter something like this:

+       IOMUX_PAD(0x05E0, 0x0210, 3, 0x0790, 1, PAD_CTL_PKE | PAD_CTL_PUE | \
+               PAD_CTL_PUS_100K_DOWN | PAD_CTL_SPEED_LOW |     \
+               PAD_CTL_DSE_80ohm | PAD_CTL_SRE_FAST | PAD_CTL_HYS),

you need to be able to find out which of these C macros that corresponds
to for the pinfunc.h mess.

As it is, sorting out the pinmuxing on IMX is a nightmare - I've so far
spent almost 8 hours on this problem trying to work out what the right
way to describe this stuff is in DT, and its far from what I'd call fun.
And I've still more to do on this.

Please fix the documentation.

device tree binding documentation outdated

[Shawn Guo]

On Fri, Sep 27, 2013 at 09:45:04AM +0100, Russell King - ARM Linux wrote:
> To me, encountering this for the first time, the documentation is
> _wrong_ no two ways about it - as I said in my initial email.  Please
> fix it, or delete it.  Don't leave it in its current crap state.  Bad
> or misleading documentation is worse than no documentation.

Ok, I will fix it.

> I am a "user" of this crap, and it is _very_ confusing that the
> documentation is wrong, and the fact that I sent my initial email in
> this thread is proof that your statement above is wrong.
> 
> For instance, "two integers array" is a lie, plain and simple.  Yes, it
> may be true that what users care about is a macro and a number but that
> is not a "two integers array".

Indeed.  I forgot killing this "two integers array" thing while I was
updating the document to expands PIN_FUNC_ID as <mux_reg conf_reg
input_reg mux_val input_val>.

> It would be helpful here to explain that
> PIN_FUNC refers to a macro in the imx*-pinfunc.h header file, which
> expands to four or five integers.

Ok.

> Bit 31 (or even bit 30) in the config number are not documented
> either.

They are documented in fsl,imx-pinctrl.txt.

> All the numbers come into play when you're trying to port a platform,
> because when you encounter something like this:
> 
> +       IOMUX_PAD(0x05E0, 0x0210, 3, 0x0790, 1,

These numbers have been taken care of by the macros in imx*-pinfunc.h.
You only need to find the corresponding macro for it from the header.

>                 PAD_CTL_PKE | PAD_CTL_PUE | \
> +               PAD_CTL_PUS_100K_DOWN | PAD_CTL_SPEED_LOW |     \
> +               PAD_CTL_DSE_80ohm | PAD_CTL_SRE_FAST | PAD_CTL_HYS),

The CONFIG number has nothing to do with imx*-pinfunc.h.  You will need
to translate the setting here into the last number of fsl,pins entry.

> you need to be able to find out which of these C macros that corresponds
> to for the pinfunc.h mess.
> 
> As it is, sorting out the pinmuxing on IMX is a nightmare - I've so far
> spent almost 8 hours on this problem trying to work out what the right
> way to describe this stuff is in DT, and its far from what I'd call fun.
> And I've still more to do on this.
> 
> Please fix the documentation.

Yes, will do.

Shawn

device tree binding documentation outdated

[Jason Cooper]

Hi Shawn,

On Fri, Sep 27, 2013 at 08:28:16PM +0800, Shawn Guo wrote:
> On Fri, Sep 27, 2013 at 09:45:04AM +0100, Russell King - ARM Linux wrote:
... 
> >                 PAD_CTL_PKE | PAD_CTL_PUE | \
> > +               PAD_CTL_PUS_100K_DOWN | PAD_CTL_SPEED_LOW |     \
> > +               PAD_CTL_DSE_80ohm | PAD_CTL_SRE_FAST | PAD_CTL_HYS),
> 
> The CONFIG number has nothing to do with imx*-pinfunc.h.  You will need
> to translate the setting here into the last number of fsl,pins entry.

The number entered into the dts{i} file still seems 'magic' to me.
Perhaps these should also be defined in imx*-pinfunc.h?  So an entry in
the dts file:

  esdhc1 {
          pinctrl_esdhc1: esdhc1grp {
                  fsl,pins = <
                          MX50_PAD_SD1_CMD__SD1_CMD      MX50_CFG_ESDHC
                          MX50_PAD_SD1_CLK__SD1_CLK      MX50_CFG_ESDHC
                          MX50_PAD_SD1_D0__SD1_D0        MX50_CFG_ESDHC
                          MX50_PAD_SD1_D1__SD1_D1        MX50_CFG_ESDHC
                          MX50_PAD_SD1_D2__SD1_D2        MX50_CFG_ESDHC
                          MX50_PAD_SD1_D3__SD1_D3        MX50_CFG_ESDHC
                          MX50_PAD_EIM_CRE__GPIO_1_27    0x0000
                          MX50_PAD_ECSPI2_SS0__GPIO_4_19 MX50_CFG_ESDHC
                  >;
          };
  };

And in imx50-pinfunc.h:

/* taken from the enum in fsl's BSP */

#define PAD_CTL_SRE_SLOW		0x0 << 0
#define PAD_CTL_SRE_FAST		0x1 << 0

#define PAD_CTL_DRV_LOW			0x0 << 1
#define PAD_CTL_DRV_MEDIUM		0x1 << 1
#define PAD_CTL_DRV_HIGH		0x2 << 1
#define PAD_CTL_DRV_MAX			0x3 << 1

#define PAD_CTL_ODE_OPENDRAIN_NONE	0x0 << 3
#define PAD_CTL_ODE_OPENDRAIN_ENABLE	0x1 << 3

#define PAD_CTL_100K_PD			0x0 << 4
#define PAD_CTL_360K_PD			0x0 << 4
#define PAD_CTL_47K_PU			0x1 << 4
#define PAD_CTL_75k_PU			0x1 << 4
#define PAD_CTL_100K_PU			0x2 << 4
#define PAD_CTL_22K_PU			0x3 << 4

#define PAD_CTL_PUE_KEEPER		0x0 << 6
#define PAD_CTL_PUE_PULL		0x1 << 6

#define PAD_CTL_PKE_NONE		0x0 << 7
#define PAD_CTL_PKE_ENABLE		0x1 << 7

#define PAD_CTL_HYS_NONE		0x0 << 8
#define PAD_CTL_HYS_ENABLE		0x1 << 8

#define PAD_CTL_DDR_INPUT_CMOS		0x0 << 9
#define PAD_CTL_DDR_INPUT_DDR		0x1 << 9

#define PAD_CTL_DRV_VOT_LOW		0x0 << 13
#define PAD_CTL_DRV_VOT_HIGH		0x1 << 13


#define MX50_CFG_ESDHC (PAD_CTL_DRV_HIGH | PAD_CTL_47K_PU | \
			PAD_CTL_PUE_PULL | PAD_CTL_PKE_ENABLE)

Or, we could@least have the PAD_CTL_* defined in one file for
everyone to use and individual boards could do the MX50_CFG_* on their
own.

I'm currently slogging through the exact same issues for adding imx50
(SanDisk Connect Wireless media Drive).  I thought I was just thick.
It's a relief to know I'm not the only one having trouble with this.

thx,

Jason.

device tree binding documentation outdated

[Russell King - ARM Linux]

On Fri, Sep 27, 2013 at 09:12:25AM -0400, Jason Cooper wrote:
> I'm currently slogging through the exact same issues for adding imx50
> (SanDisk Connect Wireless media Drive).  I thought I was just thick.
> It's a relief to know I'm not the only one having trouble with this.

I'm coming to the conclusion that it's almost to the point of "vendor
lockin" - you have to find someone who understands this stuff to take
your hardware description and turn it into a working DT description.

Still slogging away at the nonfunctional ethernet and still have no
idea why it's not working... just going through the pinctrl settings
again, this time decoding them right down to the values on both the
DT and the working code and comparing.

I think I'll be bringing up what a disaster this DT stuff is in
Edinburgh.

device tree binding documentation outdated

[Linus Walleij]

On Fri, Sep 27, 2013 at 3:21 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:

> I think I'll be bringing up what a disaster this DT stuff is in
> Edinburgh.

I think this topic suggested by Wolfram is spot on:
http://lists.linuxfoundation.org/pipermail/ksummit-2013-discuss/2013-July/000110.html

Yours,
Linus Walleij

device tree binding documentation outdated

[Jason Cooper]

On Fri, Sep 27, 2013 at 02:21:07PM +0100, Russell King - ARM Linux wrote:
> On Fri, Sep 27, 2013 at 09:12:25AM -0400, Jason Cooper wrote:
> > I'm currently slogging through the exact same issues for adding imx50
> > (SanDisk Connect Wireless media Drive).  I thought I was just thick.
> > It's a relief to know I'm not the only one having trouble with this.
> 
> I'm coming to the conclusion that it's almost to the point of "vendor
> lockin" - you have to find someone who understands this stuff to take
> your hardware description and turn it into a working DT description.

I don't think it's vendor lock-in, I think Freescale's pinctrl/gpio IP
block is just an over-complicated pain in the ass that doesn't lend
itself to DT well.

thx,

Jason.

device tree binding documentation outdated

[Matt Sealey]

On Fri, Sep 27, 2013 at 8:31 AM, Jason Cooper <jason@lakedaemon.net> wrote:
> On Fri, Sep 27, 2013 at 02:21:07PM +0100, Russell King - ARM Linux wrote:
>> On Fri, Sep 27, 2013 at 09:12:25AM -0400, Jason Cooper wrote:
>> > I'm currently slogging through the exact same issues for adding imx50
>> > (SanDisk Connect Wireless media Drive).  I thought I was just thick.
>> > It's a relief to know I'm not the only one having trouble with this.
>>
>> I'm coming to the conclusion that it's almost to the point of "vendor
>> lockin" - you have to find someone who understands this stuff to take
>> your hardware description and turn it into a working DT description.
>
> I don't think it's vendor lock-in, I think Freescale's pinctrl/gpio IP
> block is just an over-complicated pain in the ass that doesn't lend
> itself to DT well.


Me either - device trees aren't vendor lock-in but they ARE a huge
reminder of how silicon vendors prioritize software development - the
chip works. The i.MX IOMUX block lends itself VERY well to device
trees.

The problem with vendors is when they ship their chips and docs from
America or Israel or so on, do verification in the UK or Mexico or
India, hand it to a software team in some far away place like China or
India and expect everything to just come out all nice and shiny. The
problem compounds itself when the docs are bad, but in this instance,
Freescale usually give out REALLY good documentation.

The issue with this particular block is there have been 5 different
ways to "do it", originally it was just a list of addresses and values
in an array, then it got turned into a packed 64-bit magic value which
was decoded (possibly to reduce stack usage as a function call,
although it was always inlined...) and then device trees as an
enumerated index into a hardcoded array in Linux, and a pad settings
value on top, and now back to addresses and values again. The number
of possibilities for porting the thing is immense.

The only way to do it from the manual - or Freescale's IOMUX Tool,
which is pretty awesome if a little crashy and for Windows - is the 6
value tuple way.

As an example, this is what I get for the SabreLite Rev A, exported as CSV

~~
Chip,Package,Board,Board Rev.,Design Version
i.MX6DQ,BGA,Sabre_Lite,Rev. A,2

Ball,Name,PowerGroup,Instance.Signal,Instance-Alias.Signal-Alias,Comment,Register,Value,
Register,Value,Register,Value
A22,SD2_DATA0,NVCC_SD2,audmux.AUD4_RXD,audmux.AUD4_RXD,,0x020E0054,0x00000003,0x020E0368,0x0001B0B0,0x020E07B4,0x00000000
B22,SD2_DATA3,NVCC_SD2,audmux.AUD4_TXC,audmux.AUD4_TXC,,0x020E035C,0x00000003,0x020E0744,0x0001B0B0,0x020E07C4,0x00000001
A23,SD2_DATA2,NVCC_SD2,audmux.AUD4_TXD,audmux.AUD4_TXD,,0x020E0050,0x00000003,0x020E0364,0x0001B0B0,0x020E07B8,0x00000000
E20,SD2_DATA1,NVCC_SD2,audmux.AUD4_TXFS,audmux.AUD4_TXFS,,0x020E004C,0x00000003,0x020E0360,0x0001B0B0,0x020E07C8,0x00000000
~~

I used the last pin in my example previously in the list. Note the
0x04c, 0x360 addresses and 0x1b0b0 pad setting (setting the register
to 0x0 is an instruction to the fsl,pins parsing code to ignore it..
hence the need for better macros)

It would be incredibly easy to parse the CSV (or XML or..) or
cross-check the output source code (since it will give you source
code, if a little obtuse and over excited code) and generate the
preprocessor headers or even fsl,pins output for any particular board.
The Name field, Instance prefix, Signal suffix, and register values
and even the comment from the board designer (if they used the IOMUX
tool and if they didn't, bop on the nose with a newspaper..) right
into a device tree.

So, the design tools exist, they're just not being used, or they're
being misused, or supplemental parsers to extract and generate the
files don't exist (Shawn, we talked about the docs being generated
from the RTL and some ability to export them..)

A schema would help, too..

device tree binding documentation outdated

[Russell King - ARM Linux]

On Fri, Sep 27, 2013 at 11:33:19AM -0500, Matt Sealey wrote:
> On Fri, Sep 27, 2013 at 8:31 AM, Jason Cooper <jason@lakedaemon.net> wrote:
> > On Fri, Sep 27, 2013 at 02:21:07PM +0100, Russell King - ARM Linux wrote:
> >> On Fri, Sep 27, 2013 at 09:12:25AM -0400, Jason Cooper wrote:
> >> > I'm currently slogging through the exact same issues for adding imx50
> >> > (SanDisk Connect Wireless media Drive).  I thought I was just thick.
> >> > It's a relief to know I'm not the only one having trouble with this.
> >>
> >> I'm coming to the conclusion that it's almost to the point of "vendor
> >> lockin" - you have to find someone who understands this stuff to take
> >> your hardware description and turn it into a working DT description.
> >
> > I don't think it's vendor lock-in, I think Freescale's pinctrl/gpio IP
> > block is just an over-complicated pain in the ass that doesn't lend
> > itself to DT well.
> 
> 
> Me either - device trees aren't vendor lock-in but they ARE a huge
> reminder of how silicon vendors prioritize software development - the
> chip works. The i.MX IOMUX block lends itself VERY well to device
> trees.

The "vendor lock-in" that I'm talking about here is the lock-in to
the people who created the DT mess in the first place, *not* the
silicon vendor.

If it's extremely difficult to understand how to put together the
proper DT description because it's just a load of numbers, and the
knowledge is limited to a few developers, then that makes it really
difficult for 3rd parties to create their own device tree files.

There should be an effort to make writing a DT description as simple
and easy as possible, and part of that means ensuring that the
documentation helps people to understand how to do that, and certainly
doesn't add additional needless to confusion.

device tree binding documentation outdated

[Arnaud Patard (Rtp)]

Russell King - ARM Linux <linux@arm.linux.org.uk> writes:

> On Fri, Sep 27, 2013 at 09:12:25AM -0400, Jason Cooper wrote:
>> I'm currently slogging through the exact same issues for adding imx50
>> (SanDisk Connect Wireless media Drive).  I thought I was just thick.
>> It's a relief to know I'm not the only one having trouble with this.
>
> I'm coming to the conclusion that it's almost to the point of "vendor
> lockin" - you have to find someone who understands this stuff to take
> your hardware description and turn it into a working DT description.

I don't know imx6 vendor code, but at least on imx5x with mainline, half
of the job is easy. For instance, in C, there was a tab containing pad
settings, like that:
static iomux_v3_cfg_t mx51efika_pads[] = {
...
MX51_PAD_EIM_A16__GPIO2_10,
...

and now, it's more :

fsl,pins = <
...
MX51_PAD_EIM_A16__GPIO2_10  0x85
...

The real problem is more about finding the 0x85 value, which is not
easy. Of course, if the code did not use this kind of (imx-specific)
stuff, it's not suprising that it's hard to convert it.

>
> Still slogging away at the nonfunctional ethernet and still have no
> idea why it's not working... just going through the pinctrl settings
> again, this time decoding them right down to the values on both the
> DT and the working code and comparing.

There may be also some missing code on the driver side (not related to
pinctrl). It's already the case for some stuff on imx5x side.

my 2 cents,
Arnaud

device tree binding documentation outdated

[Matt Sealey]

Shawn, Russell,

Okay I have a proposal that might make this much easier. Why not
redefine the preprocessor macros as follows (one used as an example)

/* MUX_CTL definitions */
#define SION    0x10
#define ALT0    0x0
#define ALT5   0x5
#define GPIO ALT5

#define SD2_DAT1_MUX 0x04c
#define SD2_DAT1_INP 0x000
#define SD2_DAT1_CTL 0x360
#define SD2_DAT1__SD2_DATA1 0x0 ALT0
#define SD2_DAT1__SD2_DATA1_PAD 0x1b0b0
#define SD2_DAT1__GPIO1_IO14 0x0 ALT5
#define SD2_DAT1__GPIO1_IO14_PAD 0x1b0b0

#define IOMUX_R(name, function, mux_r, inp_r, pad_r, mux_v, inp_v, pad_v) \
                         mux_r pad_r inp_r mux_v inp_v pad_v

#define IOMUX_V(name, function, mux_v, inp_v, pad_v) \
                        name##_MUX name##_CTL name##_INP mux_v inp_v pad_v

#define IOMUX_P(name, function, pad_v) \
                        name##_MUX name##_CTL name##_INP
name##__##function pad_v

#define IOMUX_F(name, function)  \
                        name##_MUX name##_CTL name##_INP
name##__##function name##_##function##_PAD

Then you can have, in your lists;

fsl,pins = <  IOMUX_R(SD2_DAT1, SD2_DATA1, 0x04c, 0x000, 0x360, 0x0,
0x0, 0x1b0b0)    // long form, define pad name, function name,
register offsets, data. function is ignored.
          //         IOMUX_V(SD2_DAT1, SD2_GPIO1_IO14, 0x0, 0x15,
0x1b0b0)                          // short form, define pad name,
function name, data. function is ignored. We didn't use a prebaked ALT
define since we want to set the SION bit.
         //         IOMUX_P(SD2_DAT1, SD2_GPIO1_IO14, 0x1b0b0)
                               // short form, define pad name,
function name, pad settings.
         //         IOMUX_F(SD2_DAT1, SD2_DATA1)
                                              // beginner form, use a
prebaked value for all of the above. function is used to generate the
above.


Which gives the best of all worlds - you know the pad name, and
function type at every step. At the point someone is twiddling with
pad settings that aren't preprovided (or the defaults..) I do not
believe that it is worthwhile to "hide" the three data values. For the
uses where a custom board pin name wants to be used, SD2_DAT1,
SABRESD_WIFI_RST can be used in the IOMUX_V definition to give it some
meaning, and it will be ignored. In the worst case boards get lists of
definitions at the top of their .dts which define pin/pad names and
functions.

I did figure out that the headers are included explicitly by the
files, so in fact MX6QDL_PAD_BLAH_PREFIX_SD2_DAT1__SD2_DATA1 is
totally long-form and pointless. As above, fixed.

Also I noticed that none of the headers have guards on them, some of
the dtsi/dts don't include them and assume lower levels do. That's not
a great assumption to make.

This way I don't think I need to reorder the fsl,pins numbering at all
(although it is driving me nuts), and the macro can take into account
the weird order by normalizing it in the macro definition (as above).

I'll be throwing a patch for this out on the weekend. Can we wait on
the docs until we can define this properly? I want to ask for
'approval' first if only because rehashing all these files and
cross-checking preprocessed output (BTW.. I'll make a patch for
keeping pre-processed dts output) to compare results will take a VERY
long time for all these boards, and I won't submit a patch until it's
done.. maybe I'll start with Babbage since nobody uses it, and then
commit the Efika MX tree (finally) with the rehash then move to MX53
(I only have an old Quickstart) and MX6 for Cubox, Utilite, maybe
SabreSD and SabreLite (== Steev will you be able to test this? Which
i.MX boards do you have floating@your place?)

device tree binding documentation outdated

[Russell King - ARM Linux]

Here's another instance:

Documentation/devicetree/bindings/net/fsl-fec.txt:

Required properties:
- phy-mode : String, operation mode of the PHY interface.
  Supported values are: "mii", "gmii", "sgmii", "tbi", "rmii",
  "rgmii", "rgmii-id", "rgmii-rxid", "rgmii-txid", "rtbi", "smii".

Is this really required?  Maybe someone can point to where the code
makes use of this, because I'm damned if I can find it:

$ grep phy-mode drivers/net/ include/ net/ -r
drivers/net/ethernet/marvell/mvneta.c:              dev_err(&pdev->dev, "incorrect phy-mode\n");
drivers/net/ethernet/nxp/lpc_eth.c:                                            phy-mode", NULL);

"local-mac-address" in the optional properties also falls into this
category: only a small handful of network drivers make use of this,
and fec is not one of them.

What's the use of documentation if its inaccurate.

device tree binding documentation outdated

[Sascha Hauer]

On Fri, Sep 27, 2013 at 10:49:02AM +0100, Russell King - ARM Linux wrote:
> Here's another instance:
> 
> Documentation/devicetree/bindings/net/fsl-fec.txt:
> 
> Required properties:
> - phy-mode : String, operation mode of the PHY interface.
>   Supported values are: "mii", "gmii", "sgmii", "tbi", "rmii",
>   "rgmii", "rgmii-id", "rgmii-rxid", "rgmii-txid", "rtbi", "smii".
> 
> Is this really required?  Maybe someone can point to where the code
> makes use of this, because I'm damned if I can find it:
> 
> $ grep phy-mode drivers/net/ include/ net/ -r
> drivers/net/ethernet/marvell/mvneta.c:              dev_err(&pdev->dev, "incorrect phy-mode\n");
> drivers/net/ethernet/nxp/lpc_eth.c:                                            phy-mode", NULL);

See of_get_phy_mode() implemented in drivers/net/of_net.c and used in
the FEC driver.

> 
> "local-mac-address" in the optional properties also falls into this
> category: only a small handful of network drivers make use of this,
> and fec is not one of them.

of_get_mac_address(), also in drivers/net/of_net.c

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |