Re: [PATCH v6 05/21] net-next: stmmac: Add dwmac-sun8i
From: Maxime Ripard <hidden>
Date: 2017-06-27 16:00:16
Also in:
linux-arm-kernel, linux-devicetree, lkml
On Tue, Jun 27, 2017 at 11:33:56AM +0100, Andre Przywara wrote:
Hi, On 27/06/17 11:23, Icenowy Zheng wrote:quoted
于 2017年6月27日 GMT+08:00 下午6:15:58, Andre Przywara [off-list ref] 写到:quoted
Hi, On 27/06/17 10:41, Maxime Ripard wrote:quoted
On Tue, Jun 27, 2017 at 10:02:45AM +0100, Andre Przywara wrote:quoted
Hi, (CC:ing some people from that Rockchip dmwac series) On 27/06/17 09:21, Corentin Labbe wrote:quoted
On Tue, Jun 27, 2017 at 04:11:21PM +0800, Chen-Yu Tsai wrote:quoted
On Tue, Jun 27, 2017 at 4:05 PM, Corentin Labbe [off-list ref] wrote:quoted
On Mon, Jun 26, 2017 at 01:18:23AM +0100, André Przywara wrote:quoted
On 31/05/17 08:18, Corentin Labbe wrote:quoted
The dwmac-sun8i is a heavy hacked version of stmmac hardware by allwinner. In fact the only common part is the descriptor management andthe firstquoted
quoted
quoted
quoted
quoted
quoted
quoted
register function.Hi, I know I am a bit late with this, but while adapting the U-Bootdriverquoted
quoted
quoted
quoted
quoted
quoted
to the new binding I was wondering about the internal PHYdetection:quoted
quoted
quoted
quoted
quoted
quoted
So here you seem to deduce the usage of the internal PHY by thePHYquoted
quoted
quoted
quoted
quoted
quoted
interface specified in the DT (MII = internal, RGMII =external).quoted
quoted
quoted
quoted
quoted
quoted
I think I raised this question before, but isn't it perfectlylegal forquoted
quoted
quoted
quoted
quoted
quoted
a board to use MII with an external PHY even on those SoCs thatfeaturequoted
quoted
quoted
quoted
quoted
quoted
an internal PHY? On the first glance that does not make too much sense, but apartfromquoted
quoted
quoted
quoted
quoted
quoted
not being the correct binding to describe all of the SoCsfeatures I seequoted
quoted
quoted
quoted
quoted
quoted
two scenarios: 1) A board vendor might choose to not use the internal PHYbecause itquoted
quoted
quoted
quoted
quoted
quoted
has bugs, lacks features (configurability) or has other issues.Forquoted
quoted
quoted
quoted
quoted
quoted
instance I have heard reports that the internal PHY makes theSoC goquoted
quoted
quoted
quoted
quoted
quoted
rather hot, possibly limiting the CPU frequency. By using anexternalquoted
quoted
quoted
quoted
quoted
quoted
MII PHY (which are still cheaper than RGMII PHYs) this can beavoided.quoted
quoted
quoted
quoted
quoted
quoted
2) A PHY does not necessarily need to be directly connected to magnetics. Indeed quite some boards use (RG)MII to connect to aswitchquoted
quoted
quoted
quoted
quoted
quoted
IC or some other network circuitry, for instance fibreconnectors.quoted
quoted
quoted
quoted
quoted
quoted
So I was wondering if we would need an explicit: allwinner,use-internal-phy; boolean DT property to signal the usage of the internal PHY? Alternatively we could go with the negative version: allwinner,disable-internal-phy; Or what about introducing a new "allwinner,internal-mii-phy"compatiblequoted
quoted
quoted
quoted
quoted
quoted
string for the *PHY* node and use that? I just want to avoid that we introduce a binding that causes us headaches later. I think we can still fix this with a followuppatchquoted
quoted
quoted
quoted
quoted
quoted
before the driver and its binding hit a release kernel. Cheers, Andre.I just see some patch, where "phy-mode = internal" is valid. I will try to find a way to use itCan you provide a link?https://lkml.org/lkml/2017/6/23/479quoted
I'm not a fan of using phy-mode for this. There's no guaranteewhatquoted
quoted
quoted
quoted
mode the internal PHY uses. That's what phy-mode is for.I can understand Chen-Yu's concerns, but ...quoted
For each soc the internal PHY mode is know and setted inemac_variant/internal_phyquoted
quoted
quoted
So its not a problem.that is true as well, at least for now. So while I agree that having a separate property to indicate theusagequoted
quoted
of the internal PHY would be nice, I am bit tempted to use thiseasierquoted
quoted
approach and piggy back on the existing phy-mode property.We're trying to fix an issue that works for now too. If we want to consider future weird cases, then we must consider all of them. And the phy mode changing is definitely not really far fetched. I agree with Chen-Yu, and I really feel like the compatible solution you suggested would cover both your concerns, and ours.So something like this? emac: emac@1c30000 { compatible = "allwinner,sun8i-h3-emac"; ... phy-mode = "mii"; phy-handle = <&int_mii_phy>; ... mdio: mdio { #address-cells = <1>; #size-cells = <0>; int_mii_phy: ethernet-phy@1 { compatible = "allwinner,sun8i-h3-ephy"; syscon = <&syscon>;The MAC still needs to set some bits of syscon register.Yes, the syscon property needs also to be in the MAC node, that was meant to be somewhere in the second "..." ;-) But now since Chen-Yu mentioned that we need to set up the PHY *first* to make it actually discoverable via MDIO, I wonder if we could change this to: 1) have the DT as described here 2) Let the dwmac-sun8i driver peek into the node referenced by phy-handle and check the compatible string there. 3) If that matches some allwinner internal PHY name, it sets up the PHY to make it respond when the MDIO driver queries its bus.
That would be quite easy to do, and would be my implementation of choice yes.
Or can a PHY driver set itself up (since we have clocks and resets properties in there) *before* the MDIO bus gets scanned? Chen-Yu's comment in the other mail hints at that this is not easily possible.
Yeah, that's not easily doable, it would require your driver to probe before your device has showed up, which is not quite what the driver model is made like. Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org For more options, visit https://groups.google.com/d/optout.
Attachments
- signature.asc [application/pgp-signature] 801 bytes