Hi, On 27/06/17 10:41, Maxime Ripard wrote: > On Tue, Jun 27, 2017 at 10:02:45AM +0100, Andre Przywara wrote: >> Hi, >> >> (CC:ing some people from that Rockchip dmwac series) >> >> On 27/06/17 09:21, Corentin Labbe wrote: >>> On Tue, Jun 27, 2017 at 04:11:21PM +0800, Chen-Yu Tsai wrote: >>>> On Tue, Jun 27, 2017 at 4:05 PM, Corentin Labbe >>>> <clabbe.montjoie@xxxxxxxxx> wrote: >>>>> On Mon, Jun 26, 2017 at 01:18:23AM +0100, André Przywara wrote: >>>>>> On 31/05/17 08:18, Corentin Labbe wrote: >>>>>>> The dwmac-sun8i is a heavy hacked version of stmmac hardware by >>>>>>> allwinner. >>>>>>> In fact the only common part is the descriptor management and the first >>>>>>> register function. >>>>>> >>>>>> Hi, >>>>>> >>>>>> I know I am a bit late with this, but while adapting the U-Boot driver >>>>>> to the new binding I was wondering about the internal PHY detection: >>>>>> >>>>>> >>>>>> So here you seem to deduce the usage of the internal PHY by the PHY >>>>>> interface specified in the DT (MII = internal, RGMII = external). >>>>>> I think I raised this question before, but isn't it perfectly legal for >>>>>> a board to use MII with an external PHY even on those SoCs that feature >>>>>> an internal PHY? >>>>>> On the first glance that does not make too much sense, but apart from >>>>>> not being the correct binding to describe all of the SoCs features I see >>>>>> two scenarios: >>>>>> 1) A board vendor might choose to not use the internal PHY because it >>>>>> has bugs, lacks features (configurability) or has other issues. For >>>>>> instance I have heard reports that the internal PHY makes the SoC go >>>>>> rather hot, possibly limiting the CPU frequency. By using an external >>>>>> MII PHY (which are still cheaper than RGMII PHYs) this can be avoided. >>>>>> 2) A PHY does not necessarily need to be directly connected to >>>>>> magnetics. Indeed quite some boards use (RG)MII to connect to a switch >>>>>> IC or some other network circuitry, for instance fibre connectors. >>>>>> >>>>>> 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" compatible >>>>>> 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 followup patch >>>>>> 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 it >>>> >>>> Can you provide a link? >>> >>> https://lkml.org/lkml/2017/6/23/479 >>> >>>> >>>> I'm not a fan of using phy-mode for this. There's no guarantee what >>>> mode the internal PHY uses. That's what phy-mode is for. >> >> I can understand Chen-Yu's concerns, but ... >> >>> For each soc the internal PHY mode is know and setted in emac_variant/internal_phy >>> 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 the usage >> of the internal PHY would be nice, I am bit tempted to use this easier >> 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>; reg = <1>; clocks = <&ccu CLK_BUS_EPHY>; resets = <&ccu RST_BUS_EPHY>; }; }; }; And then move the internal-PHY setup code into a separate PHY driver? That looks like the architecturally best solution to me, but is probably also a bit involved since it would require a separate PHY driver. Or can we make it simpler, but still use this binding? Cheers, Andre. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
