On Tue, Mar 31, 2020 at 10:44:59AM +0200, David Jander wrote:

I have checked with the datasheet of the AR8035, and AFAICS, what the code
does is this:

 - Disable the SmartEEE feature of the phy. The comment in the code implies
   that for some reason it doesn't work, but the reason itself is not given.
   Anyway, disabling SmartEEE should IMHO opinion be controlled by a DT
   setting. There is no reason to believe this problem is specific to the
   i.MX6. Besides, it is a feature of the phy, so it seems logical to expose
   that via the DT. Once that is done, it has no place here.

 - Set the external clock output to 125MHz. This is needed because the i.MX6
   needs a 125MHz reference clock input. But it is not a requirement to use
   this output. It is perfectly fine and possible to design a board that uses
   an external oscillator for this. It is also possible that an i.MX6 design
   has such a phy connected to a MAC behind a switch or some other interface.
   Independent of i.MX6 this setting can also be necessary for other hardware
   designs, based on different SoC's. In summary, this is a feature of the
   specific hardware design at hand, and has nothing to do with the i.MX6
   specifically. This should definitely be exposed through the DT and not be
   here.

 - Enable TXC delay. To clarify, the RGMII specification version 1 specified
   that the RXC and TXC traces should be routed long enough to introduce a
   certain delay to the clock signal, or the delay should be introduced via
   other means. In a later version of the spec, a provision was given for MAC
   or PHY devices to generate this delay internally. The i.MX6 MAC interface
   is unable to generate the required delay internally, so it has to be taken
   care of either by the board layout, or by the PHY device. This is the
   crucial point: The amount of delay set by the PHY delay register depends on
   the board layout. It should NEVER be hard-coded in SoC setup code. The
   correct way is to specify it in the DT. Needless to say that this too,
   isn't i.MX6-specific.

Let's say this is simple to do, shall we?

So, if I disable the call to ar8031_phy_fixup() from ar8035_phy_fixup(),
and add the following to the imx6qdl-sr-som.dtsi fragment:

&fec {
...
        phy-handle = <&phy>;

        mdio {
                #address-cells = <1>;
                #size-cells = <0>;

                phy: ethernet-phy@0 {
                        reg = <0>;
                        qca,clk-out-frequency = <125000000>;
                };
        };
};

Note that phy-mode is already RGMII-ID.  This should work, right?

The link still comes up, which is good, but the PHY registers for
the clock output are wrong.

MMD 3 register 0x8016 contains 0xb282, not 0xb29a which it has
_with_ the quirk - and thus the above clock frequency stated in
DT is not being selected.  Forcing this register to the right
value restores networking.

Yes, the PHY driver is being used:

Qualcomm Atheros AR8035 2188000.ethernet-1:00: attached PHY driver [Qualcomm Atheros AR8035] (mii_bus:phy_addr=2188000.ethernet-1:00, irq=POLL)

So that's not the problem.

Adding some debug shows that the phy_device that is being used is
the correct one:

Qualcomm Atheros AR8035 2188000.ethernet-1:00: node=/soc/aips-bus@2100000/ethernet@2188000/mdio/ethernet-phy@0

and it is correctly parsing the clk-out-frequency property:

Qualcomm Atheros AR8035 2188000.ethernet-1:00: cof=0 125000000

When we get to attaching the PHY however:

Qualcomm Atheros AR8035 2188000.ethernet-1:00: clk_25m_mask=0004 clk_25m_reg=0000

which is just wrong.  That's because:

                if (at803x_match_phy_id(phydev, ATH8030_PHY_ID) ||
                    at803x_match_phy_id(phydev, ATH8035_PHY_ID)) {
                        priv->clk_25m_reg &= ~AT8035_CLK_OUT_MASK;
                        priv->clk_25m_mask &= ~AT8035_CLK_OUT_MASK;
                }

is patently untested - those "~" should not be there.  These masks
are one-bits-set for the values that comprise the fields, not
zero-bits-set.

So, I see a patch series is going to be necessary to fix the cockup(s)
in the PHY driver before we can do anything with DT files.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up