Date: Fri, 28 Jul 2017 09:39:36 +0200
From: Maxime Ripard <maxime.ripard@free-electrons.com>
To: Corentin Labbe <clabbe.montjoie@gmail.com>
Cc: Florian Fainelli <f.fainelli@gmail.com>, mark.rutland@arm.com,
        Andrew Lunn <andrew@lunn.ch>, hwg@rock-chips.com,
        huangtao@rock-chips.com, heiko@sntech.de, arnd@arndb.de,
        devicetree@vger.kernel.org, catalin.marinas@arm.com,
        davem@davemloft.net, will.deacon@arm.com, linux@armlinux.org.uk,
        linux-kernel@vger.kernel.org, linux-rockchip@lists.infradead.org,
        robh+dt@kernel.org, David Wu <david.wu@rock-chips.com>, olof@lixom.net,
        peppe.cavallaro@st.com, netdev@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org, alexandre.torgue@st.com
Subject: Re: [PATCH v2 05/11] net: stmmac: dwmac-rk: Add internal phy support
Message-ID: <20170728073936.jwgrq2z2nd3dqrti@flea.home>
References: <1501160156-30328-1-git-send-email-david.wu@rock-chips.com>
 <1501160540-30662-1-git-send-email-david.wu@rock-chips.com>
 <20170727134834.GD18666@lunn.ch>
 <65ae1747-4dff-b1d8-8aa3-684fc8c7809a@gmail.com>
 <20170727173152.GA16362@Red>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha1;
        protocol="application/pgp-signature"; boundary="mb5t67vs5o3drojx"
Content-Disposition: inline
In-Reply-To: <20170727173152.GA16362@Red>
User-Agent: NeoMutt/20170714 (1.8.3)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3971
Lines: 94


--mb5t67vs5o3drojx
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Thu, Jul 27, 2017 at 07:31:52PM +0200, Corentin Labbe wrote:
> On Thu, Jul 27, 2017 at 09:54:01AM -0700, Florian Fainelli wrote:
> > On 07/27/2017 06:48 AM, Andrew Lunn wrote:
> > > On Thu, Jul 27, 2017 at 09:02:16PM +0800, David Wu wrote:
> > >> To make internal phy work, need to configure the phy_clock,
> > >> phy cru_reset and related registers.
> > >>
> > >> Signed-off-by: David Wu <david.wu@rock-chips.com>
> > >> ---
> > >> changes in v2:
> > >>  - Use the standard "phy-mode" property for internal phy. (Florian)
> > >=20
> > > I think we need to discuss this. This PHY appears to be on an MDIO
> > > bus, it uses a standard PHY driver, and it appears to be using an RMII
> > > interface. So it is just an ordinary PHY.
> >=20
> > First, the fact that the internal PHY also appears through MDIO is
> > orthogonal to the fact that it is internal or external. Plenty of
> > designs have internal PHYs exposed through MDIO because that is
> > convenient. What matters though is how the data/clock lines are wired
> > internally, which is what "phy-mode" describes.
> >=20
> > >=20
> > > Internal is supposed to be something which is not ordinary, does not
> > > use one of the standard phy modes, needs something special to make it
> > > work.
> > >=20
> > > Florain, it appears to be your suggestion to use internal. What do you
> > > say?
> >=20
> > phy-mode =3D "internal" really means that it is not a standard MII vari=
ant
> > to connect the data/clock lines between the Ethernet MAC and the PHY,
> > and this can happen in some designs (although quite unlikely). So from
> > there we could do several things depending on the requirements:
> >=20
> > - if you can have your Ethernet MAC driver perform the necessary
> > configuration *after* you have been able to bind the PHY device with its
> > PHY driver, then the PHY driver should have PHY_IS_INTERNAL in its
> > flags, and you can use phy_is_internal() from PHYLIB to tell you that
> > and we could imagine using: phy-mode =3D "rmii" because that would not =
too
> > much of a stretch
> >=20
> > - if you need knowledge about this PHY connection type prior to binding
> > the PHY device and its driver (that is, before of_phy_connect()) we
> > could add a boolean property e.g: "phy-is-internal" that allows us to
> > know that, or we can have a new phy-mode value, e.g: "internal-rmii"
> > which describes that, either way would probably be fine, but the former
> > scales better
>=20
> We have the same problem on Allwinner SoCs for dwmac-sun8i, we need
> to set a syscon for chossing between internal/external PHY.
>
> Having this phy-is-internal would be very helpfull. (adding
> internal-xmii will add too many flags in our case)

In our case, we'll always have a phy node, so we can have a compatible
that will give you the same information.

Maxime

--=20
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

--mb5t67vs5o3drojx
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIcBAEBAgAGBQJZeuo3AAoJEBx+YmzsjxAght0P/AuvCotYX0025UWZZda9LYAD
HveAFwH6Q2WjDt8iNE9nVW9OUGX30FrPCYnSHSoLha2RQKisFIrjbDUFmFWV/eWZ
HqujLkqWoXJvW8cOaoCqBPHW9FiZFe09Q9dkVpR0JHL8gmhVeCD43T24YMavVlx9
fgkGZy/XKbLUmYHG/iFwIqrHjwOGghaeuit2XuMW4vcfH1xzjp0LCkpfVJ7u4a7l
QJZAY6AVidJ2wFiS8pPtkjHyAnG/z1S4CEaiSmXgf2v859j7ZMshr4PeYEE16qMd
mY/jjuYDNV7XjbeL3J8kmDugNRMLFgAs340qeDbnES0OSPQ+oSwdhAN7TzM5hYk7
/x+xZ+tEcCjgZE6vqkOTlIZvRxIJpyIntBJsPc+7nHENTaJ28doBE3XvBxJsh75u
dsiFxGyDZLbz/KHC8m6jidtNWrmwWeGdSIp3oQdIPdpaxhvGxHAyDaMKs7qE9MFh
sxSoVJEpxawQunapseMeMUooENn9uAFTHRPkD+DyZaeJ0lGsXOBIELFCxOcvrrKZ
dXvio4bxSJeqwgcN3cUqGn+WF2dP2zUo63PIi6h7vgrBlD0fEf3DNtcQBW5BQHs9
JBXAQo4eGAvCJbi1+ACllwuF6mGJWLCH6CQ+siQX2iTP4l/Xm45ZmmbGqlm8m0zt
IJzVAkOHq7WrCO7borhp
=T/3W
-----END PGP SIGNATURE-----

--mb5t67vs5o3drojx--