| Message ID | 1344637896-14267-2-git-send-email-timur@freescale.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Kumar Gala |
| Headers | show |
On Aug 10, 2012, at 5:31 PM, Timur Tabi wrote: > diff --git a/arch/powerpc/platforms/85xx/corenet_ds.c b/arch/powerpc/platforms/85xx/corenet_ds.c > index 925b028..a79fc79 100644 > --- a/arch/powerpc/platforms/85xx/corenet_ds.c > +++ b/arch/powerpc/platforms/85xx/corenet_ds.c > @@ -106,6 +106,13 @@ static const struct of_device_id of_device_ids[] __devinitconst = { > { > .name = "handles", > }, > + { > + /* > + * Warning: this entry might need to be located before those > + * for the Fman Ethernet nodes. > + */ > + .compatible = "mdio-mux", > + }, > {} > }; Under what condition would that be the case? - k
Kumar Gala wrote: >> > + { >> > + /* >> > + * Warning: this entry might need to be located before those >> > + * for the Fman Ethernet nodes. >> > + */ >> > + .compatible = "mdio-mux", >> > + }, >> > {} >> > }; > Under what condition would that be the case? We had this discussion already. In my tests, I had to locate this entry before these entries: { .compatible = "fsl,dpaa" }, { .compatible = "fsl,srio", }, otherwise, the mdio-mux code would not prepare the mdio mus in time, and there would be initialization failures. Now maybe this goes away with -EPROBE_DEFER, or maybe it doesn't. But until we push the DPAA drivers upstream, we won't know.
On 08/14/2012 04:48 PM, Timur Tabi wrote: > Kumar Gala wrote: >>>> + { >>>> + /* >>>> + * Warning: this entry might need to be located before those >>>> + * for the Fman Ethernet nodes. >>>> + */ >>>> + .compatible = "mdio-mux", >>>> + }, >>>> {} >>>> }; >> Under what condition would that be the case? > > We had this discussion already. I think that was internally, and not on this specific comment wording. I don't think that code comment adequately explains things. > otherwise, the mdio-mux code would not prepare the mdio mus in time, and > there would be initialization failures. Now maybe this goes away with > -EPROBE_DEFER, or maybe it doesn't. But until we push the DPAA drivers > upstream, we won't know. Do you know if it's theoretically supposed to be fixed and just can't test it, or are you unsure of whether it's even supposed to work? I don't think we should be relying on the order of this list to determine probe order. For one thing, it won't work if the drivers register after you create the platform devices (e.g. they're modules). -Scott
Scott Wood wrote: > I think that was internally, and not on this specific comment wording. > I don't think that code comment adequately explains things. I don't really have any more insight to add. >> otherwise, the mdio-mux code would not prepare the mdio mus in time, and >> there would be initialization failures. Now maybe this goes away with >> -EPROBE_DEFER, or maybe it doesn't. But until we push the DPAA drivers >> upstream, we won't know. > > Do you know if it's theoretically supposed to be fixed and just can't > test it, or are you unsure of whether it's even supposed to work? I'm not sure of anything. For one thing, we don't implement EPROBE_DEFER in the DPAA drivers, so we'd probably have to fix that before anything. And then, I'm just guessing that's the solution. > I don't think we should be relying on the order of this list to > determine probe order. For one thing, it won't work if the drivers > register after you create the platform devices (e.g. they're modules). I agree we should not be relying on the order, but I don't know what to do. EPROBE_DEFER was designed to handle this situation, so my recommendation is to worry about it later. I can beef up the comment to talk about that, if you want.
On 08/14/2012 04:58 PM, Timur Tabi wrote: > Scott Wood wrote: > >> I think that was internally, and not on this specific comment wording. >> I don't think that code comment adequately explains things. > > I don't really have any more insight to add. My point (at least, this part of it) was that more of the insight you've already provided should be moved from e-mail discussion to the code comment. >>> otherwise, the mdio-mux code would not prepare the mdio mus in time, and >>> there would be initialization failures. Now maybe this goes away with >>> -EPROBE_DEFER, or maybe it doesn't. But until we push the DPAA drivers >>> upstream, we won't know. >> >> Do you know if it's theoretically supposed to be fixed and just can't >> test it, or are you unsure of whether it's even supposed to work? > > I'm not sure of anything. For one thing, we don't implement EPROBE_DEFER > in the DPAA drivers, so we'd probably have to fix that before anything. > And then, I'm just guessing that's the solution. I feel confident saying it is the solution, at least until it is demonstrated otherwise. >> I don't think we should be relying on the order of this list to >> determine probe order. For one thing, it won't work if the drivers >> register after you create the platform devices (e.g. they're modules). > > I agree we should not be relying on the order, but I don't know what to > do. EPROBE_DEFER was designed to handle this situation, so my > recommendation is to worry about it later. I can beef up the comment to > talk about that, if you want. If the DPAA driver doesn't implement it when it's submitted, it's a bug in the DPAA driver and we should insist it be fixed. I don't think we should at all entertain the notion that careful device id list ordering is even a potential solution. If anything, I'd make the ordering be "wrong" to force that code path to be tested -- though ideally there would be a more systematic approach to such testing, that doesn't require inefficiency during normal boot. -Scott
Scott Wood wrote: > > If anything, I'd make the ordering be "wrong" to force that code path to > be tested -- though ideally there would be a more systematic approach to > such testing, that doesn't require inefficiency during normal boot. I can't force the ordering to be wrong, because it's the only entry in the list. The DPAA entries are not there yet. This is what I have now: static const struct of_device_id of_device_ids[] __devinitconst = { { .compatible = "simple-bus" }, { .compatible = "fsl,srio", }, { .compatible = "fsl,p4080-pcie", }, { .compatible = "fsl,qoriq-pcie-v2.2", }, { .compatible = "fsl,qoriq-pcie-v2.3", }, { .compatible = "fsl,qoriq-pcie-v2.4", }, /* The following two are for the Freescale hypervisor */ { .name = "hypervisor", }, { .name = "handles", }, { /* * Warning: this entry might need to be located before those * for the Fman Ethernet nodes, although using EPROBE_DEFER * in the DPAA drivers could fix that. */ .compatible = "mdio-mux", }, {} };
On 08/14/2012 06:08 PM, Timur Tabi wrote: > Scott Wood wrote: >> >> If anything, I'd make the ordering be "wrong" to force that code path to >> be tested -- though ideally there would be a more systematic approach to >> such testing, that doesn't require inefficiency during normal boot. > > I can't force the ordering to be wrong, because it's the only entry in the > list. The DPAA entries are not there yet. Right, I mean once the DPAA entries are added. > This is what I have now: > > static const struct of_device_id of_device_ids[] __devinitconst = { > { > .compatible = "simple-bus" > }, > { > .compatible = "fsl,srio", > }, > { > .compatible = "fsl,p4080-pcie", > }, > { > .compatible = "fsl,qoriq-pcie-v2.2", > }, > { > .compatible = "fsl,qoriq-pcie-v2.3", > }, > { > .compatible = "fsl,qoriq-pcie-v2.4", > }, > /* The following two are for the Freescale hypervisor */ > { > .name = "hypervisor", > }, > { > .name = "handles", > }, > { > /* > * Warning: this entry might need to be located before those > * for the Fman Ethernet nodes, although using EPROBE_DEFER > * in the DPAA drivers could fix that. > */ > .compatible = "mdio-mux", > }, I'd either say nothing here or say only "The ethernet driver should use EPROBE_DEFER to ensure that the mdio-mux is probed first". Don't give whoever submits the DPAA ethernet driver the idea that relying on list order is an acceptable solution. -Scott
diff --git a/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi b/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi index 8d35d2c..1ce5056 100644 --- a/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi +++ b/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi @@ -347,4 +347,16 @@ /include/ "qoriq-usb2-mph-0.dtsi" /include/ "qoriq-usb2-dr-0.dtsi" /include/ "qoriq-sec4.0-0.dtsi" +/include/ "qoriq-fman-0.dtsi" +/include/ "qoriq-fman-0-1g-0.dtsi" +/include/ "qoriq-fman-0-1g-1.dtsi" +/include/ "qoriq-fman-0-1g-2.dtsi" +/include/ "qoriq-fman-0-1g-3.dtsi" +/include/ "qoriq-fman-0-10g-0.dtsi" +/include/ "qoriq-fman-1.dtsi" +/include/ "qoriq-fman-1-1g-0.dtsi" +/include/ "qoriq-fman-1-1g-1.dtsi" +/include/ "qoriq-fman-1-1g-2.dtsi" +/include/ "qoriq-fman-1-1g-3.dtsi" +/include/ "qoriq-fman-1-10g-0.dtsi" }; diff --git a/arch/powerpc/boot/dts/fsl/p4080si-pre.dtsi b/arch/powerpc/boot/dts/fsl/p4080si-pre.dtsi index b9556ee..1be6360 100644 --- a/arch/powerpc/boot/dts/fsl/p4080si-pre.dtsi +++ b/arch/powerpc/boot/dts/fsl/p4080si-pre.dtsi @@ -69,6 +69,9 @@ rtic_c = &rtic_c; rtic_d = &rtic_d; sec_mon = &sec_mon; + + fman0 = &fman0; + fman1 = &fman1; }; cpus { diff --git a/arch/powerpc/boot/dts/p4080ds.dts b/arch/powerpc/boot/dts/p4080ds.dts index 3e20460..68ae3bb 100644 --- a/arch/powerpc/boot/dts/p4080ds.dts +++ b/arch/powerpc/boot/dts/p4080ds.dts @@ -41,6 +41,44 @@ #size-cells = <2>; interrupt-parent = <&mpic>; + aliases { + ethernet0 = &fm1_gb0; // FM1@DTSEC1 + ethernet1 = &fm1_gb1; // FM1@DTSEC2 + ethernet2 = &fm1_gb2; // FM1@DTSEC3 + ethernet3 = &fm1_gb3; // FM1@DTSEC4 + ethernet4 = &fm1_10g; // FM1@TGEC1 + ethernet5 = &fm2_gb0; // FM2@DTSEC1 + ethernet6 = &fm2_gb1; // FM2@DTSEC2 + ethernet7 = &fm2_gb2; // FM2@DTSEC3 + ethernet8 = &fm2_gb3; // FM2@DTSEC4 + ethernet9 = &fm2_10g; // FM2@TGEC1 + + phy_rgmii = &phy_rgmii; + + phy_sgmii_slot3_1c = &phy_sgmii_slot3_1c; + phy_sgmii_slot3_1d = &phy_sgmii_slot3_1d; + phy_sgmii_slot3_1e = &phy_sgmii_slot3_1e; + phy_sgmii_slot3_1f = &phy_sgmii_slot3_1f; + + phy_sgmii_slot4_1c = &phy_sgmii_slot4_1c; + phy_sgmii_slot4_1d = &phy_sgmii_slot4_1d; + phy_sgmii_slot4_1e = &phy_sgmii_slot4_1e; + phy_sgmii_slot4_1f = &phy_sgmii_slot4_1f; + + phy_sgmii_slot5_1c = &phy_sgmii_slot5_1c; + phy_sgmii_slot5_1d = &phy_sgmii_slot5_1d; + phy_sgmii_slot5_1e = &phy_sgmii_slot5_1e; + phy_sgmii_slot5_1f = &phy_sgmii_slot5_1f; + + // Used by U-Boot to find the mdio-mux nodes + emi1_rgmii = &emi1_rgmii; + emi1_slot3 = &emi1_slot3; + emi1_slot4 = &emi1_slot4; + emi1_slot5 = &emi1_slot5; + emi2_slot4 = &emi2_slot4; + emi2_slot5 = &emi2_slot5; + }; + memory { device_type = "memory"; }; @@ -106,6 +144,117 @@ dr_mode = "host"; phy_type = "ulpi"; }; + + fman0: fman@400000 { + fm1_gb0: ethernet@e0000 { + tbi-handle = <&tbi0>; + phy-connection-type = "sgmii"; + }; + + mdio0: mdio@e1120 { + tbi0: tbi-phy@8 { + reg = <0x8>; + device_type = "tbi-phy"; + }; + }; + + fm1_gb1: ethernet@e2000 { + tbi-handle = <&tbi1>; + phy-connection-type = "sgmii"; + }; + + mdio@e3120 { + tbi1: tbi-phy@8 { + reg = <8>; + device_type = "tbi-phy"; + }; + }; + + fm1_gb2: ethernet@e4000 { + tbi-handle = <&tbi2>; + phy-connection-type = "sgmii"; + }; + + mdio@e5120 { + tbi2: tbi-phy@8 { + reg = <8>; + device_type = "tbi-phy"; + }; + }; + + fm1_gb3: ethernet@e6000 { + tbi-handle = <&tbi3>; + phy-connection-type = "sgmii"; + }; + + mdio@e7120 { + tbi3: tbi-phy@8 { + reg = <8>; + device_type = "tbi-phy"; + }; + }; + + fm1_10g: ethernet@f0000 { + phy-handle = <&phy_xgmii_slot5>; + phy-connection-type = "xgmii"; + }; + }; + + fman1: fman@500000 { + fm2_gb0: ethernet@e0000 { + tbi-handle = <&tbi5>; + phy-connection-type = "sgmii"; + }; + + mdio@e1120 { + tbi5: tbi-phy@8 { + reg = <8>; + device_type = "tbi-phy"; + }; + }; + + fm2_gb1: ethernet@e2000 { + tbi-handle = <&tbi6>; + phy-connection-type = "sgmii"; + }; + + mdio@e3120 { + tbi6: tbi-phy@8 { + reg = <8>; + device_type = "tbi-phy"; + }; + }; + + fm2_gb2: ethernet@e4000 { + tbi-handle = <&tbi7>; + phy-connection-type = "sgmii"; + }; + + mdio@e5120 { + tbi7: tbi-phy@8 { + reg = <8>; + device_type = "tbi-phy"; + }; + }; + + fm2_gb3: ethernet@e6000 { + tbi-handle = <&tbi8>; + phy-connection-type = "sgmii"; + }; + + mdio@e7120 { + tbi8: tbi-phy@8 { + reg = <8>; + device_type = "tbi-phy"; + }; + }; + + fm2_10g: ethernet@f0000 { + phy-handle = <&phy_xgmii_slot4>; + phy-connection-type = "xgmii"; + }; + }; + }; rio: rapidio@ffe0c0000 { @@ -182,6 +331,164 @@ }; }; + fsl,dpaa { + compatible = "fsl,p4080-dpaa", "fsl,dpaa"; + + ethernet@0 { + compatible = "fsl,p4080-dpa-ethernet", "fsl,dpa-ethernet"; + fsl,fman-mac = <&fm1_gb0>; + }; + ethernet@1 { + compatible = "fsl,p4080-dpa-ethernet", "fsl,dpa-ethernet"; + fsl,fman-mac = <&fm1_gb1>; + }; + ethernet@2 { + compatible = "fsl,p4080-dpa-ethernet", "fsl,dpa-ethernet"; + fsl,fman-mac = <&fm1_gb2>; + }; + ethernet@3 { + compatible = "fsl,p4080-dpa-ethernet", "fsl,dpa-ethernet"; + fsl,fman-mac = <&fm1_gb3>; + }; + ethernet@4 { + compatible = "fsl,p4080-dpa-ethernet", "fsl,dpa-ethernet"; + fsl,fman-mac = <&fm1_10g>; + }; + ethernet@5 { + compatible = "fsl,p4080-dpa-ethernet", "fsl,dpa-ethernet"; + fsl,fman-mac = <&fm2_gb0>; + }; + ethernet@6 { + compatible = "fsl,p4080-dpa-ethernet", "fsl,dpa-ethernet"; + fsl,fman-mac = <&fm2_gb1>; + }; + ethernet@7 { + compatible = "fsl,p4080-dpa-ethernet", "fsl,dpa-ethernet"; + fsl,fman-mac = <&fm2_gb2>; + }; + ethernet@8 { + compatible = "fsl,p4080-dpa-ethernet", "fsl,dpa-ethernet"; + fsl,fman-mac = <&fm2_gb3>; + }; + ethernet@9 { + compatible = "fsl,p4080-dpa-ethernet", "fsl,dpa-ethernet"; + fsl,fman-mac = <&fm2_10g>; + }; + }; + + mdio-mux-emi1 { + compatible = "mdio-mux-gpio", "mdio-mux"; + // The 'reg' property of the MDIO nodes is written to the + // GPIOs as a single number, but the driver parses the number + // in little-endian order, so these GPIO pins need to be in + // little-endian order. + gpios = <&gpio0 1 0>, <&gpio0 0 0>; + mdio-parent-bus = <&mdio0>; + #address-cells = <1>; + #size-cells = <0>; + + emi1_rgmii: mdio@0 { // RGMII + status = "disabled"; + reg = <0>; + #address-cells = <1>; + #size-cells = <0>; + + phy_rgmii: ethernet-phy@0 { + reg = <0x0>; + }; + }; + + emi1_slot3: mdio@2 { // Slot 3 SGMII + status = "disabled"; + reg = <2>; + #address-cells = <1>; + #size-cells = <0>; + + phy_sgmii_slot3_1c: ethernet-phy@1c { + reg = <0x1c>; + }; + phy_sgmii_slot3_1d: ethernet-phy@1d { + reg = <0x1d>; + }; + phy_sgmii_slot3_1e: ethernet-phy@1e { + reg = <0x1e>; + }; + phy_sgmii_slot3_1f: ethernet-phy@1f { + reg = <0x1f>; + }; + }; + + emi1_slot4: mdio@1 { // Slot 4 SGMII + status = "disabled"; + reg = <1>; + #address-cells = <1>; + #size-cells = <0>; + + phy_sgmii_slot4_1c: ethernet-phy@1c { + reg = <0x1c>; + }; + phy_sgmii_slot4_1d: ethernet-phy@1d { + reg = <0x1d>; + }; + phy_sgmii_slot4_1e: ethernet-phy@1e { + reg = <0x1e>; + }; + phy_sgmii_slot4_1f: ethernet-phy@1f { + reg = <0x1f>; + }; + }; + + emi1_slot5: mdio@3 { // Slot 5 SGMII + status = "disabled"; + reg = <3>; + #address-cells = <1>; + #size-cells = <0>; + + phy_sgmii_slot5_1c: ethernet-phy@1c { + reg = <0x1c>; + }; + phy_sgmii_slot5_1d: ethernet-phy@1d { + reg = <0x1d>; + }; + phy_sgmii_slot5_1e: ethernet-phy@1e { + reg = <0x1e>; + }; + phy_sgmii_slot5_1f: ethernet-phy@1f { + reg = <0x1f>; + }; + }; + }; + + mdio-mux-emi2 { + compatible = "mdio-mux-gpio", "mdio-mux"; + // The mdio-mux-gpio driver wants GPIOs in little-endian order + gpios = <&gpio0 3 0>, <&gpio0 2 0>; + mdio-parent-bus = <&xmdio0>; + #address-cells = <1>; + #size-cells = <0>; + + emi2_slot4: mdio@1 { // Slot 4 XAUI (FM2) + status = "disabled"; + reg = <1>; + #address-cells = <1>; + #size-cells = <0>; + + phy_xgmii_slot4: ethernet-phy@0 { + reg = <0>; + }; + }; + + emi2_slot5: mdio@3 { // Slot 5 XAUI (FM1) + status = "disabled"; + reg = <3>; + #address-cells = <1>; + #size-cells = <0>; + + phy_xgmii_slot5: ethernet-phy@4 { + reg = <4>; + }; + }; + }; }; /include/ "fsl/p4080si-post.dtsi" diff --git a/arch/powerpc/platforms/85xx/Kconfig b/arch/powerpc/platforms/85xx/Kconfig index 159c01e..591ec5e 100644 --- a/arch/powerpc/platforms/85xx/Kconfig +++ b/arch/powerpc/platforms/85xx/Kconfig @@ -235,6 +235,9 @@ config P4080_DS select GPIO_MPC8XXX select HAS_RAPIDIO select PPC_EPAPR_HV_PIC + select MPC8xxx_GPIO if PHYLIB + select MDIO_BUS_MUX if PHYLIB + select MDIO_BUS_MUX_GPIO if PHYLIB help This option enables support for the P4080 DS board diff --git a/arch/powerpc/platforms/85xx/corenet_ds.c b/arch/powerpc/platforms/85xx/corenet_ds.c index 925b028..a79fc79 100644 --- a/arch/powerpc/platforms/85xx/corenet_ds.c +++ b/arch/powerpc/platforms/85xx/corenet_ds.c @@ -106,6 +106,13 @@ static const struct of_device_id of_device_ids[] __devinitconst = { { .name = "handles", }, + { + /* + * Warning: this entry might need to be located before those + * for the Fman Ethernet nodes. + */ + .compatible = "mdio-mux", + }, {} };
The Freescale P4080 SOC contains two Fman Ethernet controllers, each of which contains four 1G Ethernet MACs and one 10G Ethernet MAC. The SerDes protocol determines how the MACs are routed to various RGMII, SGMII, or XGMII PHYs on the board, so the P4080DS reference board provides a MDIO bus multiplexing feature that is controlled via GPIOs. This patch adds the basic Ethernet, MDIO, mdio-mux, and PHY nodes necessary to get MDIO bus muxing working, using Cavium's mdio-mux-gpio driver. The remaining DPAA nodes (Qman, Bman, etc), as well as the DPAA device drivers, are not included. Signed-off-by: Timur Tabi <timur@freescale.com> --- arch/powerpc/boot/dts/fsl/p4080si-post.dtsi | 12 + arch/powerpc/boot/dts/fsl/p4080si-pre.dtsi | 3 + arch/powerpc/boot/dts/p4080ds.dts | 307 +++++++++++++++++++++++++++ arch/powerpc/platforms/85xx/Kconfig | 3 + arch/powerpc/platforms/85xx/corenet_ds.c | 7 + 5 files changed, 332 insertions(+), 0 deletions(-)