* [PATCH v2 1/3] net: stmmac: allow drivers to explicitly select PHY device @ 2025-05-27 17:55 James Hilliard 2025-05-27 17:55 ` [PATCH v2 2/3] net: stmmac: dwmac-sun8i: Allow runtime AC200/AC300 phy selection James Hilliard ` (2 more replies) 0 siblings, 3 replies; 31+ messages in thread From: James Hilliard @ 2025-05-27 17:55 UTC (permalink / raw) To: netdev Cc: linux-sunxi, James Hilliard, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Russell King, Russell King (Oracle), Furong Xu, Kunihiko Hayashi, linux-stm32, linux-arm-kernel, linux-kernel Some devices like the Allwinner H616 need the ability to select a phy in cases where multiple PHY's may be present in a device tree due to needing the ability to support multiple SoC variants with runtime PHY selection. Signed-off-by: James Hilliard <james.hilliard1@gmail.com> --- .../net/ethernet/stmicro/stmmac/stmmac_main.c | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index 59d07d0d3369..949c4a8a1456 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -1210,17 +1210,25 @@ static int stmmac_init_phy(struct net_device *dev) */ if (!phy_fwnode || IS_ERR(phy_fwnode)) { int addr = priv->plat->phy_addr; - struct phy_device *phydev; + struct phy_device *phydev = NULL; - if (addr < 0) { - netdev_err(priv->dev, "no phy found\n"); - return -ENODEV; + if (priv->plat->phy_node) { + phy_fwnode = of_fwnode_handle(priv->plat->phy_node); + phydev = fwnode_phy_find_device(phy_fwnode); + fwnode_handle_put(phy_fwnode); } - phydev = mdiobus_get_phy(priv->mii, addr); if (!phydev) { - netdev_err(priv->dev, "no phy at addr %d\n", addr); - return -ENODEV; + if (addr < 0) { + netdev_err(priv->dev, "no phy found\n"); + return -ENODEV; + } + + phydev = mdiobus_get_phy(priv->mii, addr); + if (!phydev) { + netdev_err(priv->dev, "no phy at addr %d\n", addr); + return -ENODEV; + } } ret = phylink_connect_phy(priv->phylink, phydev); -- 2.34.1 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v2 2/3] net: stmmac: dwmac-sun8i: Allow runtime AC200/AC300 phy selection 2025-05-27 17:55 [PATCH v2 1/3] net: stmmac: allow drivers to explicitly select PHY device James Hilliard @ 2025-05-27 17:55 ` James Hilliard 2025-05-27 17:55 ` [PATCH v2 3/3] dt-bindings: net: sun8i-emac: Add AC300 EMAC1 nvmem " James Hilliard 2025-05-27 19:14 ` [PATCH v2 1/3] net: stmmac: allow drivers to explicitly select PHY device Andrew Lunn 2 siblings, 0 replies; 31+ messages in thread From: James Hilliard @ 2025-05-27 17:55 UTC (permalink / raw) To: netdev Cc: linux-sunxi, James Hilliard, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland, Maxime Coquelin, Alexandre Torgue, Yinggang Gu, Huacai Chen, Yanteng Si, Russell King (Oracle), Uwe Kleine-König, Jinjie Ruan, Paul Kocialkowski, linux-arm-kernel, linux-stm32, linux-kernel The Allwinner H616 ships with two different copackaged phy variants, in order to determine the phy being used we need to read an efuse and then select the appropriate PHY based on the AC300 bit. By defining an emac node without a phy-handle we can override the default PHY selection logic in stmmac by passing a specific phy_node selected based on the ac200 and ac300 names in a phys list. This allows us to have a device tree that defines both PHY variants even though only one will actually end up being used at runtime based on the ac300 nvmem efuse bit. Signed-off-by: James Hilliard <james.hilliard1@gmail.com> --- .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c index 6c7e8655a7eb..50d37876fabf 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c @@ -11,6 +11,7 @@ #include <linux/mdio-mux.h> #include <linux/mfd/syscon.h> #include <linux/module.h> +#include <linux/nvmem-consumer.h> #include <linux/of.h> #include <linux/of_mdio.h> #include <linux/of_net.h> @@ -280,6 +281,8 @@ static const struct emac_variant emac_variant_h6 = { #define SYSCON_ETCS_EXT_GMII 0x1 #define SYSCON_ETCS_INT_GMII 0x2 +#define AC300_KEY BIT(8) /* 1: AC300 PHY, 0: AC200 PHY */ + /* sun8i_dwmac_dma_reset() - reset the EMAC * Called from stmmac via stmmac_dma_ops->reset */ @@ -1149,6 +1152,35 @@ static struct regmap *sun8i_dwmac_get_syscon_from_dev(struct device_node *node) return regmap; } +/* H616 SoCs can contain either an AC200 PHY (needs i2c init) or an AC300 + * PHY (no i2c). The silicon variant is flagged by the AC300_KEY efuse. + */ +static int sun8i_dwmac_get_ac300_phy(struct device *dev, + struct plat_stmmacenet_data *plat_dat) +{ + u16 val; + + /* If the nvmem cell is absent, use normal phy selection. */ + if (nvmem_cell_read_u16(dev, "ac300", &val)) + return 0; + + const char *phy_name = (val & AC300_KEY) ? "ac300" : "ac200"; + int index = of_property_match_string(dev->of_node, "phy-names", + phy_name); + if (index < 0) { + dev_err(dev, "PHY name not found in device tree\n"); + return -EINVAL; + } + + plat_dat->phy_node = of_parse_phandle(dev->of_node, "phys", index); + if (!plat_dat->phy_node) { + dev_err(dev, "Failed to get PHY node from phys property\n"); + return -EINVAL; + } + + return 0; +} + static int sun8i_dwmac_probe(struct platform_device *pdev) { struct plat_stmmacenet_data *plat_dat; @@ -1222,6 +1254,10 @@ static int sun8i_dwmac_probe(struct platform_device *pdev) if (IS_ERR(plat_dat)) return PTR_ERR(plat_dat); + ret = sun8i_dwmac_get_ac300_phy(dev, plat_dat); + if (ret) + return ret; + /* platform data specifying hardware features and callbacks. * hardware features were copied from Allwinner drivers. */ -- 2.34.1 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v2 3/3] dt-bindings: net: sun8i-emac: Add AC300 EMAC1 nvmem phy selection 2025-05-27 17:55 [PATCH v2 1/3] net: stmmac: allow drivers to explicitly select PHY device James Hilliard 2025-05-27 17:55 ` [PATCH v2 2/3] net: stmmac: dwmac-sun8i: Allow runtime AC200/AC300 phy selection James Hilliard @ 2025-05-27 17:55 ` James Hilliard 2025-05-27 19:14 ` [PATCH v2 1/3] net: stmmac: allow drivers to explicitly select PHY device Andrew Lunn 2 siblings, 0 replies; 31+ messages in thread From: James Hilliard @ 2025-05-27 17:55 UTC (permalink / raw) To: netdev Cc: linux-sunxi, James Hilliard, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland, Maxime Ripard, Alexandre Torgue, Giuseppe Cavallaro, Jose Abreu, devicetree, linux-arm-kernel, linux-kernel The Allwinner H616 EMAC1 can be connected to an copackaged AC200 or AC300 PHY depending upon the variant. Add a new allwinner,sun50i-h616-emac1 compatible and example, support for the allwinner,sun50i-h616-emac1 EMAC1 MAC will be added later on. Add nvmem-cells and nvmem-cell-names properties for the ac300 efuse based phy selection. Signed-off-by: James Hilliard <james.hilliard1@gmail.com> --- Changes v1 -> v2: - fix dt_binding_check --- .../net/allwinner,sun8i-a83t-emac.yaml | 75 ++++++++++++++++++- .../bindings/net/ethernet-controller.yaml | 26 +++++-- .../devicetree/bindings/net/snps,dwmac.yaml | 2 + 3 files changed, 94 insertions(+), 9 deletions(-) diff --git a/Documentation/devicetree/bindings/net/allwinner,sun8i-a83t-emac.yaml b/Documentation/devicetree/bindings/net/allwinner,sun8i-a83t-emac.yaml index 7fe0352dff0f..3a8c31dd9ae7 100644 --- a/Documentation/devicetree/bindings/net/allwinner,sun8i-a83t-emac.yaml +++ b/Documentation/devicetree/bindings/net/allwinner,sun8i-a83t-emac.yaml @@ -18,6 +18,7 @@ properties: - const: allwinner,sun8i-r40-gmac - const: allwinner,sun8i-v3s-emac - const: allwinner,sun50i-a64-emac + - const: allwinner,sun50i-h616-emac1 - items: - enum: - allwinner,sun20i-d1-emac @@ -58,7 +59,6 @@ required: - clock-names - resets - reset-names - - phy-handle - phy-mode - syscon @@ -73,6 +73,7 @@ allOf: - allwinner,sun8i-h3-emac - allwinner,sun8i-v3s-emac - allwinner,sun50i-a64-emac + - allwinner,sun50i-h616-emac1 then: properties: @@ -189,6 +190,42 @@ allOf: - mdio-parent-bus - mdio@1 + - if: + not: + properties: + compatible: + contains: + enum: + - allwinner,sun50i-h616-emac1 + + then: + required: + - phy-handle + + - if: + properties: + compatible: + contains: + enum: + - allwinner,sun50i-h616-emac1 + + then: + properties: + nvmem-cells: true + + nvmem-cell-names: true + + phys: + maxItems: 2 + + phy-names: + items: + - const: ac200 + - const: ac300 + + mdio: + $ref: mdio.yaml# + unevaluatedProperties: false examples: @@ -321,4 +358,40 @@ examples: }; }; + - | + ethernet@5030000 { + compatible = "allwinner,sun50i-h616-emac1"; + syscon = <&syscon>; + reg = <0x05030000 0x10000>; + interrupts = <0 15 4>; + interrupt-names = "macirq"; + resets = <&ccu 31>; + reset-names = "stmmaceth"; + clocks = <&ccu 83>; + clock-names = "stmmaceth"; + phys = <&ac200_rmii_phy>, <&ac300_rmii_phy>; + phy-names = "ac200", "ac300"; + phy-mode = "rgmii"; + nvmem-cells = <&ephy_acx00>; + nvmem-cell-names = "ac300"; + + mdio { + compatible = "snps,dwmac-mdio"; + #address-cells = <1>; + #size-cells = <0>; + + ac300_rmii_phy: ac300-ethernet-phy@0 { + #phy-cells = <0>; + compatible = "ethernet-phy-ieee802.3-c22"; + reg = <0>; + }; + + ac200_rmii_phy: ac200-ethernet-phy@1 { + #phy-cells = <0>; + compatible = "ethernet-phy-ieee802.3-c22"; + reg = <1>; + }; + }; + }; + ... diff --git a/Documentation/devicetree/bindings/net/ethernet-controller.yaml b/Documentation/devicetree/bindings/net/ethernet-controller.yaml index a2d4c626f659..710e651851e5 100644 --- a/Documentation/devicetree/bindings/net/ethernet-controller.yaml +++ b/Documentation/devicetree/bindings/net/ethernet-controller.yaml @@ -45,14 +45,6 @@ properties: description: Specifies maximum speed in Mbit/s supported by the device. - nvmem-cells: - maxItems: 1 - description: - Reference to an nvmem node for the MAC address - - nvmem-cell-names: - const: mac-address - phy-connection-type: description: Specifies interface type between the Ethernet device and a physical @@ -260,6 +252,24 @@ dependencies: pcs-handle-names: [pcs-handle] allOf: + - if: + not: + properties: + compatible: + contains: + enum: + - allwinner,sun50i-h616-emac1 + + then: + properties: + nvmem-cells: + maxItems: 1 + description: + Reference to an nvmem node for the MAC address + + nvmem-cell-names: + const: mac-address + - if: properties: phy-mode: diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml index 78b3030dc56d..a6dfed00c48f 100644 --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml @@ -56,6 +56,7 @@ properties: - allwinner,sun8i-r40-gmac - allwinner,sun8i-v3s-emac - allwinner,sun50i-a64-emac + - allwinner,sun50i-h616-emac1 - amlogic,meson6-dwmac - amlogic,meson8b-dwmac - amlogic,meson8m2-dwmac @@ -620,6 +621,7 @@ allOf: - allwinner,sun8i-r40-gmac - allwinner,sun8i-v3s-emac - allwinner,sun50i-a64-emac + - allwinner,sun50i-h616-emac1 - loongson,ls2k-dwmac - loongson,ls7a-dwmac - ingenic,jz4775-mac -- 2.34.1 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH v2 1/3] net: stmmac: allow drivers to explicitly select PHY device 2025-05-27 17:55 [PATCH v2 1/3] net: stmmac: allow drivers to explicitly select PHY device James Hilliard 2025-05-27 17:55 ` [PATCH v2 2/3] net: stmmac: dwmac-sun8i: Allow runtime AC200/AC300 phy selection James Hilliard 2025-05-27 17:55 ` [PATCH v2 3/3] dt-bindings: net: sun8i-emac: Add AC300 EMAC1 nvmem " James Hilliard @ 2025-05-27 19:14 ` Andrew Lunn 2025-05-27 19:21 ` James Hilliard 2 siblings, 1 reply; 31+ messages in thread From: Andrew Lunn @ 2025-05-27 19:14 UTC (permalink / raw) To: James Hilliard Cc: netdev, linux-sunxi, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Russell King, Russell King (Oracle), Furong Xu, Kunihiko Hayashi, linux-stm32, linux-arm-kernel, linux-kernel On Tue, May 27, 2025 at 11:55:54AM -0600, James Hilliard wrote: > Some devices like the Allwinner H616 need the ability to select a phy > in cases where multiple PHY's may be present in a device tree due to > needing the ability to support multiple SoC variants with runtime > PHY selection. I'm not convinced about this yet. As far as i see, it is different variants of the H616. They should have different compatibles, since they are not actually compatible, and you should have different DT descriptions. So you don't need runtime PHY selection. Andrew ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 1/3] net: stmmac: allow drivers to explicitly select PHY device 2025-05-27 19:14 ` [PATCH v2 1/3] net: stmmac: allow drivers to explicitly select PHY device Andrew Lunn @ 2025-05-27 19:21 ` James Hilliard 2025-05-27 20:01 ` Andrew Lunn 0 siblings, 1 reply; 31+ messages in thread From: James Hilliard @ 2025-05-27 19:21 UTC (permalink / raw) To: Andrew Lunn Cc: netdev, linux-sunxi, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Russell King, Russell King (Oracle), Furong Xu, Kunihiko Hayashi, linux-stm32, linux-arm-kernel, linux-kernel On Tue, May 27, 2025 at 1:14 PM Andrew Lunn <andrew@lunn.ch> wrote: > > On Tue, May 27, 2025 at 11:55:54AM -0600, James Hilliard wrote: > > Some devices like the Allwinner H616 need the ability to select a phy > > in cases where multiple PHY's may be present in a device tree due to > > needing the ability to support multiple SoC variants with runtime > > PHY selection. > > I'm not convinced about this yet. As far as i see, it is different > variants of the H616. They should have different compatibles, since > they are not actually compatible, and you should have different DT > descriptions. So you don't need runtime PHY selection. Different compatibles for what specifically? I mean the PHY compatibles are just the generic "ethernet-phy-ieee802.3-c22" compatibles. > > Andrew ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 1/3] net: stmmac: allow drivers to explicitly select PHY device 2025-05-27 19:21 ` James Hilliard @ 2025-05-27 20:01 ` Andrew Lunn 2025-05-27 20:16 ` James Hilliard 2025-05-30 23:46 ` James Hilliard 0 siblings, 2 replies; 31+ messages in thread From: Andrew Lunn @ 2025-05-27 20:01 UTC (permalink / raw) To: James Hilliard Cc: netdev, linux-sunxi, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Russell King, Russell King (Oracle), Furong Xu, Kunihiko Hayashi, linux-stm32, linux-arm-kernel, linux-kernel On Tue, May 27, 2025 at 01:21:21PM -0600, James Hilliard wrote: > On Tue, May 27, 2025 at 1:14 PM Andrew Lunn <andrew@lunn.ch> wrote: > > > > On Tue, May 27, 2025 at 11:55:54AM -0600, James Hilliard wrote: > > > Some devices like the Allwinner H616 need the ability to select a phy > > > in cases where multiple PHY's may be present in a device tree due to > > > needing the ability to support multiple SoC variants with runtime > > > PHY selection. > > > > I'm not convinced about this yet. As far as i see, it is different > > variants of the H616. They should have different compatibles, since > > they are not actually compatible, and you should have different DT > > descriptions. So you don't need runtime PHY selection. > > Different compatibles for what specifically? I mean the PHY compatibles > are just the generic "ethernet-phy-ieee802.3-c22" compatibles. You at least have a different MTD devices, exporting different clocks/PWM/Reset controllers. That should have different compatibles, since they are not compatible. You then need phandles to these different clocks/PWM/Reset controllers, and for one of the PHYs you need a phandle to the I2C bus, so the PHY driver can do the initialisation. So i think in the end you know what PHY you have on the board, so there is no need to do runtime detection. What you might want however is to validate the MTD device compatible against the fuse and return -ENODEV if the compatible is wrong for the fuse. Andrew ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 1/3] net: stmmac: allow drivers to explicitly select PHY device 2025-05-27 20:01 ` Andrew Lunn @ 2025-05-27 20:16 ` James Hilliard 2025-05-27 20:30 ` Andrew Lunn 2025-05-30 23:46 ` James Hilliard 1 sibling, 1 reply; 31+ messages in thread From: James Hilliard @ 2025-05-27 20:16 UTC (permalink / raw) To: Andrew Lunn Cc: netdev, linux-sunxi, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Russell King, Russell King (Oracle), Furong Xu, Kunihiko Hayashi, linux-stm32, linux-arm-kernel, linux-kernel On Tue, May 27, 2025 at 2:02 PM Andrew Lunn <andrew@lunn.ch> wrote: > > On Tue, May 27, 2025 at 01:21:21PM -0600, James Hilliard wrote: > > On Tue, May 27, 2025 at 1:14 PM Andrew Lunn <andrew@lunn.ch> wrote: > > > > > > On Tue, May 27, 2025 at 11:55:54AM -0600, James Hilliard wrote: > > > > Some devices like the Allwinner H616 need the ability to select a phy > > > > in cases where multiple PHY's may be present in a device tree due to > > > > needing the ability to support multiple SoC variants with runtime > > > > PHY selection. > > > > > > I'm not convinced about this yet. As far as i see, it is different > > > variants of the H616. They should have different compatibles, since > > > they are not actually compatible, and you should have different DT > > > descriptions. So you don't need runtime PHY selection. > > > > Different compatibles for what specifically? I mean the PHY compatibles > > are just the generic "ethernet-phy-ieee802.3-c22" compatibles. > > You at least have a different MTD devices, exporting different > clocks/PWM/Reset controllers. I assume you mean MFD not MTD devices here. > That should have different compatibles, > since they are not compatible. I agree with that for the MFD devices, but we still need a way to choose the correct one at runtime otherwise initialization won't succeed AFAIU. > You then need phandles to these > different clocks/PWM/Reset controllers, and for one of the PHYs you > need a phandle to the I2C bus, so the PHY driver can do the > initialisation. Well this would be an indirect reference to the i2c bus right? I mean the phy would reference a reset controller which would in turn reference the I2C bus right? > So i think in the end you know what PHY you have on > the board, so there is no need to do runtime detection. But we still need to somehow runtime select the correct phy which in turn references the phandle to the correct reset controller right? > What you might want however is to validate the MTD device compatible > against the fuse and return -ENODEV if the compatible is wrong for the > fuse. Sure, that may make sense to do as well, but I still don't see how that impacts the need to runtime select the PHY which is configured for the correct MFD. ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 1/3] net: stmmac: allow drivers to explicitly select PHY device 2025-05-27 20:16 ` James Hilliard @ 2025-05-27 20:30 ` Andrew Lunn 2025-05-27 20:37 ` James Hilliard 0 siblings, 1 reply; 31+ messages in thread From: Andrew Lunn @ 2025-05-27 20:30 UTC (permalink / raw) To: James Hilliard Cc: netdev, linux-sunxi, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Russell King, Russell King (Oracle), Furong Xu, Kunihiko Hayashi, linux-stm32, linux-arm-kernel, linux-kernel > Sure, that may make sense to do as well, but I still don't see > how that impacts the need to runtime select the PHY which > is configured for the correct MFD. If you know what variant you have, you only include the one PHY you actually have, and phy-handle points to it, just as normal. No runtime selection. Andrew ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 1/3] net: stmmac: allow drivers to explicitly select PHY device 2025-05-27 20:30 ` Andrew Lunn @ 2025-05-27 20:37 ` James Hilliard 2025-05-27 21:48 ` Andrew Lunn 2025-05-28 7:53 ` Russell King (Oracle) 0 siblings, 2 replies; 31+ messages in thread From: James Hilliard @ 2025-05-27 20:37 UTC (permalink / raw) To: Andrew Lunn Cc: netdev, linux-sunxi, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Russell King, Russell King (Oracle), Furong Xu, Kunihiko Hayashi, linux-stm32, linux-arm-kernel, linux-kernel On Tue, May 27, 2025 at 2:30 PM Andrew Lunn <andrew@lunn.ch> wrote: > > > Sure, that may make sense to do as well, but I still don't see > > how that impacts the need to runtime select the PHY which > > is configured for the correct MFD. > > If you know what variant you have, you only include the one PHY you > actually have, and phy-handle points to it, just as normal. No runtime > selection. Oh, so here's the issue, we have both PHY variants, older hardware generally has AC200 PHY's while newer ships AC300 PHY's, but when I surveyed our deployed hardware using these boards many systems of similar age would randomly mix AC200 and AC300 PHY's. It appears there was a fairly long transition period where both variants were being shipped. ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 1/3] net: stmmac: allow drivers to explicitly select PHY device 2025-05-27 20:37 ` James Hilliard @ 2025-05-27 21:48 ` Andrew Lunn 2025-05-27 22:47 ` James Hilliard 2025-05-28 7:53 ` Russell King (Oracle) 1 sibling, 1 reply; 31+ messages in thread From: Andrew Lunn @ 2025-05-27 21:48 UTC (permalink / raw) To: James Hilliard Cc: netdev, linux-sunxi, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Russell King, Russell King (Oracle), Furong Xu, Kunihiko Hayashi, linux-stm32, linux-arm-kernel, linux-kernel On Tue, May 27, 2025 at 02:37:03PM -0600, James Hilliard wrote: > On Tue, May 27, 2025 at 2:30 PM Andrew Lunn <andrew@lunn.ch> wrote: > > > > > Sure, that may make sense to do as well, but I still don't see > > > how that impacts the need to runtime select the PHY which > > > is configured for the correct MFD. > > > > If you know what variant you have, you only include the one PHY you > > actually have, and phy-handle points to it, just as normal. No runtime > > selection. > > Oh, so here's the issue, we have both PHY variants, older hardware > generally has AC200 PHY's while newer ships AC300 PHY's, but > when I surveyed our deployed hardware using these boards many > systems of similar age would randomly mix AC200 and AC300 PHY's. Are they pin compatible? But i assume none of these boards .dts files are actually in mainline? So they need to go through review, and are likely to be horribly broken and need fixing? So you can fix up the PHY node as part of the cleanup. Andrew ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 1/3] net: stmmac: allow drivers to explicitly select PHY device 2025-05-27 21:48 ` Andrew Lunn @ 2025-05-27 22:47 ` James Hilliard 0 siblings, 0 replies; 31+ messages in thread From: James Hilliard @ 2025-05-27 22:47 UTC (permalink / raw) To: Andrew Lunn Cc: netdev, linux-sunxi, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Russell King, Russell King (Oracle), Furong Xu, Kunihiko Hayashi, linux-stm32, linux-arm-kernel, linux-kernel On Tue, May 27, 2025 at 3:48 PM Andrew Lunn <andrew@lunn.ch> wrote: > > On Tue, May 27, 2025 at 02:37:03PM -0600, James Hilliard wrote: > > On Tue, May 27, 2025 at 2:30 PM Andrew Lunn <andrew@lunn.ch> wrote: > > > > > > > Sure, that may make sense to do as well, but I still don't see > > > > how that impacts the need to runtime select the PHY which > > > > is configured for the correct MFD. > > > > > > If you know what variant you have, you only include the one PHY you > > > actually have, and phy-handle points to it, just as normal. No runtime > > > selection. > > > > Oh, so here's the issue, we have both PHY variants, older hardware > > generally has AC200 PHY's while newer ships AC300 PHY's, but > > when I surveyed our deployed hardware using these boards many > > systems of similar age would randomly mix AC200 and AC300 PHY's. > > Are they pin compatible? From my understanding they are entirely pin compatible. > But i assume none of these boards .dts files are actually in mainline? > So they need to go through review, and are likely to be horribly > broken and need fixing? So you can fix up the PHY node as part of the > cleanup. The specific board I'm working with is not in mainline, however there are boards in mainline that will have the exact same issue. They simply do not currently have any hardline ethernet support in mainline at the moment and have to rely on wifi for internet connectivity unless using out of tree patches. ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 1/3] net: stmmac: allow drivers to explicitly select PHY device 2025-05-27 20:37 ` James Hilliard 2025-05-27 21:48 ` Andrew Lunn @ 2025-05-28 7:53 ` Russell King (Oracle) 2025-05-28 11:57 ` James Hilliard 1 sibling, 1 reply; 31+ messages in thread From: Russell King (Oracle) @ 2025-05-28 7:53 UTC (permalink / raw) To: James Hilliard Cc: Andrew Lunn, netdev, linux-sunxi, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Furong Xu, Kunihiko Hayashi, linux-stm32, linux-arm-kernel, linux-kernel On Tue, May 27, 2025 at 02:37:03PM -0600, James Hilliard wrote: > On Tue, May 27, 2025 at 2:30 PM Andrew Lunn <andrew@lunn.ch> wrote: > > > > > Sure, that may make sense to do as well, but I still don't see > > > how that impacts the need to runtime select the PHY which > > > is configured for the correct MFD. > > > > If you know what variant you have, you only include the one PHY you > > actually have, and phy-handle points to it, just as normal. No runtime > > selection. > > Oh, so here's the issue, we have both PHY variants, older hardware > generally has AC200 PHY's while newer ships AC300 PHY's, but > when I surveyed our deployed hardware using these boards many > systems of similar age would randomly mix AC200 and AC300 PHY's. > > It appears there was a fairly long transition period where both variants > were being shipped. Given that DT is supposed to describe the hardware that is being run on, it should _describe_ _the_ _hardware_ that the kernel is being run on. That means not enumerating all possibilities in DT and then having magic in the kernel to select the right variant. That means having a correct description in DT for the kernel to use. I don't think that abusing "phys" is a good idea. It's quite normal for the boot loader to fix up the device tree according to the hardware - for example, adding the actual memory location and sizes that are present, adding reserved memory regions, etc. I don't see why you couldn't detect the differences and have the boot loader patch the device tree appropriately. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last! ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 1/3] net: stmmac: allow drivers to explicitly select PHY device 2025-05-28 7:53 ` Russell King (Oracle) @ 2025-05-28 11:57 ` James Hilliard 2025-05-28 13:24 ` Andrew Lunn 0 siblings, 1 reply; 31+ messages in thread From: James Hilliard @ 2025-05-28 11:57 UTC (permalink / raw) To: Russell King (Oracle) Cc: Andrew Lunn, netdev, linux-sunxi, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Furong Xu, Kunihiko Hayashi, linux-stm32, linux-arm-kernel, linux-kernel On Wed, May 28, 2025 at 1:53 AM Russell King (Oracle) <linux@armlinux.org.uk> wrote: > > On Tue, May 27, 2025 at 02:37:03PM -0600, James Hilliard wrote: > > On Tue, May 27, 2025 at 2:30 PM Andrew Lunn <andrew@lunn.ch> wrote: > > > > > > > Sure, that may make sense to do as well, but I still don't see > > > > how that impacts the need to runtime select the PHY which > > > > is configured for the correct MFD. > > > > > > If you know what variant you have, you only include the one PHY you > > > actually have, and phy-handle points to it, just as normal. No runtime > > > selection. > > > > Oh, so here's the issue, we have both PHY variants, older hardware > > generally has AC200 PHY's while newer ships AC300 PHY's, but > > when I surveyed our deployed hardware using these boards many > > systems of similar age would randomly mix AC200 and AC300 PHY's. > > > > It appears there was a fairly long transition period where both variants > > were being shipped. > > Given that DT is supposed to describe the hardware that is being run on, > it should _describe_ _the_ _hardware_ that the kernel is being run on. > > That means not enumerating all possibilities in DT and then having magic > in the kernel to select the right variant. That means having a correct > description in DT for the kernel to use. The approach I'm using is IMO quite similar to say other hardware variant runtime detection DT features like this: https://github.com/torvalds/linux/commit/157ce8f381efe264933e9366db828d845bade3a1 There's already a good bit of mdio autodetection code like that which scans mdio buses for PHY ID's in stmmac. To me this is just allowing for device specific autodetection logic, it's not like we don't already have a good bit of generic PHY auto detection code in the kernel already. > I don't think that abusing "phys" is a good idea. > > It's quite normal for the boot loader to fix up the device tree > according to the hardware - for example, adding the actual memory > location and sizes that are present, adding reserved memory regions, > etc. I don't see why you couldn't detect the differences and have > the boot loader patch the device tree appropriately. I mean, sure, that's technically possible, it just seems like it's not the best fit option here since there seems to be no real reason this sort of autodetection can't be handled in the kernel itself. ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 1/3] net: stmmac: allow drivers to explicitly select PHY device 2025-05-28 11:57 ` James Hilliard @ 2025-05-28 13:24 ` Andrew Lunn 2025-05-28 14:11 ` Chen-Yu Tsai 0 siblings, 1 reply; 31+ messages in thread From: Andrew Lunn @ 2025-05-28 13:24 UTC (permalink / raw) To: James Hilliard Cc: Russell King (Oracle), netdev, linux-sunxi, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Furong Xu, Kunihiko Hayashi, linux-stm32, linux-arm-kernel, linux-kernel On Wed, May 28, 2025 at 05:57:38AM -0600, James Hilliard wrote: > On Wed, May 28, 2025 at 1:53 AM Russell King (Oracle) > <linux@armlinux.org.uk> wrote: > > > > On Tue, May 27, 2025 at 02:37:03PM -0600, James Hilliard wrote: > > > On Tue, May 27, 2025 at 2:30 PM Andrew Lunn <andrew@lunn.ch> wrote: > > > > > > > > > Sure, that may make sense to do as well, but I still don't see > > > > > how that impacts the need to runtime select the PHY which > > > > > is configured for the correct MFD. > > > > > > > > If you know what variant you have, you only include the one PHY you > > > > actually have, and phy-handle points to it, just as normal. No runtime > > > > selection. > > > > > > Oh, so here's the issue, we have both PHY variants, older hardware > > > generally has AC200 PHY's while newer ships AC300 PHY's, but > > > when I surveyed our deployed hardware using these boards many > > > systems of similar age would randomly mix AC200 and AC300 PHY's. > > > > > > It appears there was a fairly long transition period where both variants > > > were being shipped. > > > > Given that DT is supposed to describe the hardware that is being run on, > > it should _describe_ _the_ _hardware_ that the kernel is being run on. > > > > That means not enumerating all possibilities in DT and then having magic > > in the kernel to select the right variant. That means having a correct > > description in DT for the kernel to use. > > The approach I'm using is IMO quite similar to say other hardware > variant runtime detection DT features like this: > https://github.com/torvalds/linux/commit/157ce8f381efe264933e9366db828d845bade3a1 That is for things link a HAT on a RPi. It is something which is easy to replace, and is expected to be replaced. You are talking about some form of chiplet like component within the SoC package. It is not easy to replace, and not expected to be replaced. Different uses cases altogether. What i think we will end up with is the base SoC .dtsi file, and two additional .dtsi files describing the two PHY variants. Andrew ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 1/3] net: stmmac: allow drivers to explicitly select PHY device 2025-05-28 13:24 ` Andrew Lunn @ 2025-05-28 14:11 ` Chen-Yu Tsai 2025-05-28 17:25 ` James Hilliard 0 siblings, 1 reply; 31+ messages in thread From: Chen-Yu Tsai @ 2025-05-28 14:11 UTC (permalink / raw) To: Andrew Lunn Cc: James Hilliard, Russell King (Oracle), netdev, linux-sunxi, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Furong Xu, Kunihiko Hayashi, linux-stm32, linux-arm-kernel, linux-kernel On Wed, May 28, 2025 at 9:25 PM Andrew Lunn <andrew@lunn.ch> wrote: > > On Wed, May 28, 2025 at 05:57:38AM -0600, James Hilliard wrote: > > On Wed, May 28, 2025 at 1:53 AM Russell King (Oracle) > > <linux@armlinux.org.uk> wrote: > > > > > > On Tue, May 27, 2025 at 02:37:03PM -0600, James Hilliard wrote: > > > > On Tue, May 27, 2025 at 2:30 PM Andrew Lunn <andrew@lunn.ch> wrote: > > > > > > > > > > > Sure, that may make sense to do as well, but I still don't see > > > > > > how that impacts the need to runtime select the PHY which > > > > > > is configured for the correct MFD. > > > > > > > > > > If you know what variant you have, you only include the one PHY you > > > > > actually have, and phy-handle points to it, just as normal. No runtime > > > > > selection. > > > > > > > > Oh, so here's the issue, we have both PHY variants, older hardware > > > > generally has AC200 PHY's while newer ships AC300 PHY's, but > > > > when I surveyed our deployed hardware using these boards many > > > > systems of similar age would randomly mix AC200 and AC300 PHY's. > > > > > > > > It appears there was a fairly long transition period where both variants > > > > were being shipped. > > > > > > Given that DT is supposed to describe the hardware that is being run on, > > > it should _describe_ _the_ _hardware_ that the kernel is being run on. > > > > > > That means not enumerating all possibilities in DT and then having magic > > > in the kernel to select the right variant. That means having a correct > > > description in DT for the kernel to use. > > > > The approach I'm using is IMO quite similar to say other hardware > > variant runtime detection DT features like this: > > https://github.com/torvalds/linux/commit/157ce8f381efe264933e9366db828d845bade3a1 > > That is for things link a HAT on a RPi. It is something which is easy > to replace, and is expected to be replaced. Actually it's for second sourced components that are modules _within_ the device (a tablet or a laptop) that get swapped in at the factory. Definitely not something easy to replace and not expected to be replaced by the end user. The other thing is that there are no distinguishing identifiers for a device tree match for the swap-in variants at the board / device level. Though I do have something that does DT fixups in the kernel for IDs passed over by the firmware. There are other reasons for this arrangement, one being that the firmware is not easily upgradable. ChenYu > You are talking about some form of chiplet like component within the > SoC package. It is not easy to replace, and not expected to be > replaced. > > Different uses cases altogether. > > What i think we will end up with is the base SoC .dtsi file, and two > additional .dtsi files describing the two PHY variants. > > Andrew > ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 1/3] net: stmmac: allow drivers to explicitly select PHY device 2025-05-28 14:11 ` Chen-Yu Tsai @ 2025-05-28 17:25 ` James Hilliard 2025-05-28 18:34 ` Russell King (Oracle) 0 siblings, 1 reply; 31+ messages in thread From: James Hilliard @ 2025-05-28 17:25 UTC (permalink / raw) To: wens, Andrew Lunn Cc: Russell King (Oracle), netdev, linux-sunxi, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Furong Xu, Kunihiko Hayashi, linux-stm32, linux-arm-kernel, linux-kernel On Wed, May 28, 2025 at 8:12 AM Chen-Yu Tsai <wens@csie.org> wrote: > > On Wed, May 28, 2025 at 9:25 PM Andrew Lunn <andrew@lunn.ch> wrote: > > > > On Wed, May 28, 2025 at 05:57:38AM -0600, James Hilliard wrote: > > > On Wed, May 28, 2025 at 1:53 AM Russell King (Oracle) > > > <linux@armlinux.org.uk> wrote: > > > > > > > > On Tue, May 27, 2025 at 02:37:03PM -0600, James Hilliard wrote: > > > > > On Tue, May 27, 2025 at 2:30 PM Andrew Lunn <andrew@lunn.ch> wrote: > > > > > > > > > > > > > Sure, that may make sense to do as well, but I still don't see > > > > > > > how that impacts the need to runtime select the PHY which > > > > > > > is configured for the correct MFD. > > > > > > > > > > > > If you know what variant you have, you only include the one PHY you > > > > > > actually have, and phy-handle points to it, just as normal. No runtime > > > > > > selection. > > > > > > > > > > Oh, so here's the issue, we have both PHY variants, older hardware > > > > > generally has AC200 PHY's while newer ships AC300 PHY's, but > > > > > when I surveyed our deployed hardware using these boards many > > > > > systems of similar age would randomly mix AC200 and AC300 PHY's. > > > > > > > > > > It appears there was a fairly long transition period where both variants > > > > > were being shipped. > > > > > > > > Given that DT is supposed to describe the hardware that is being run on, > > > > it should _describe_ _the_ _hardware_ that the kernel is being run on. > > > > > > > > That means not enumerating all possibilities in DT and then having magic > > > > in the kernel to select the right variant. That means having a correct > > > > description in DT for the kernel to use. > > > > > > The approach I'm using is IMO quite similar to say other hardware > > > variant runtime detection DT features like this: > > > https://github.com/torvalds/linux/commit/157ce8f381efe264933e9366db828d845bade3a1 > > > > That is for things link a HAT on a RPi. It is something which is easy > > to replace, and is expected to be replaced. > > Actually it's for second sourced components that are modules _within_ > the device (a tablet or a laptop) that get swapped in at the factory. > Definitely not something easy to replace and not expected to be replaced > by the end user. Yeah, to me it seems like the PHY situation is similar, it's not replaceable due to being copackaged, it seems the vendor just switched over to a second source for the PHY partway through the production run without distinguishing different SoC variants with new model numbers. Keep in mind stmmac itself implements mdio PHY scanning already, which is a form of runtime PHY autodetection, so I don't really see how doing nvmem/efuse based PHY autodetection is all that different from that as both are forms of PHY runtime autodetection. https://github.com/torvalds/linux/blob/v6.15/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c#L646-L673 > The other thing is that there are no distinguishing identifiers for a > device tree match for the swap-in variants at the board / device level. > Though I do have something that does DT fixups in the kernel for IDs > passed over by the firmware. There are other reasons for this arrangement, > one being that the firmware is not easily upgradable. > > ChenYu > > > You are talking about some form of chiplet like component within the > > SoC package. It is not easy to replace, and not expected to be > > replaced. > > > > Different uses cases altogether. > > > > What i think we will end up with is the base SoC .dtsi file, and two > > additional .dtsi files describing the two PHY variants. I think having a single PHY .dtsi for both here is ideal if we can do runtime detection, since it's difficult to know which potential variants of PHY various devices shipped with, in our case we have access to enough hardware to determine that we have both variants but for other devices most developers will likely only have access to one PHY variant even if hardware at various points in time may have shipped with both variants. IMO without runtime detection this will likely be a major footgun for anyone trying to add ethernet support to H616 boards as they will have difficulty testing on hardware they don't have. If we provide a single .dtsi that implements the autodetection correctly then they will simply be able to include that and the hardware will then likely function on both variants automatically by including the single .dtsi file. Requiring the bootloader to modify the DT adds a good bit of complexity and will be difficult for most developers to test as many will not have access to hardware with both PHY variants. ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 1/3] net: stmmac: allow drivers to explicitly select PHY device 2025-05-28 17:25 ` James Hilliard @ 2025-05-28 18:34 ` Russell King (Oracle) 2025-05-28 19:10 ` James Hilliard 0 siblings, 1 reply; 31+ messages in thread From: Russell King (Oracle) @ 2025-05-28 18:34 UTC (permalink / raw) To: James Hilliard Cc: wens, Andrew Lunn, netdev, linux-sunxi, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Furong Xu, Kunihiko Hayashi, linux-stm32, linux-arm-kernel, linux-kernel On Wed, May 28, 2025 at 11:25:20AM -0600, James Hilliard wrote: > On Wed, May 28, 2025 at 8:12 AM Chen-Yu Tsai <wens@csie.org> wrote: > > > > On Wed, May 28, 2025 at 9:25 PM Andrew Lunn <andrew@lunn.ch> wrote: > > > > > > On Wed, May 28, 2025 at 05:57:38AM -0600, James Hilliard wrote: > > > > On Wed, May 28, 2025 at 1:53 AM Russell King (Oracle) > > > > <linux@armlinux.org.uk> wrote: > > > > > > > > > > On Tue, May 27, 2025 at 02:37:03PM -0600, James Hilliard wrote: > > > > > > On Tue, May 27, 2025 at 2:30 PM Andrew Lunn <andrew@lunn.ch> wrote: > > > > > > > > > > > > > > > Sure, that may make sense to do as well, but I still don't see > > > > > > > > how that impacts the need to runtime select the PHY which > > > > > > > > is configured for the correct MFD. > > > > > > > > > > > > > > If you know what variant you have, you only include the one PHY you > > > > > > > actually have, and phy-handle points to it, just as normal. No runtime > > > > > > > selection. > > > > > > > > > > > > Oh, so here's the issue, we have both PHY variants, older hardware > > > > > > generally has AC200 PHY's while newer ships AC300 PHY's, but > > > > > > when I surveyed our deployed hardware using these boards many > > > > > > systems of similar age would randomly mix AC200 and AC300 PHY's. > > > > > > > > > > > > It appears there was a fairly long transition period where both variants > > > > > > were being shipped. > > > > > > > > > > Given that DT is supposed to describe the hardware that is being run on, > > > > > it should _describe_ _the_ _hardware_ that the kernel is being run on. > > > > > > > > > > That means not enumerating all possibilities in DT and then having magic > > > > > in the kernel to select the right variant. That means having a correct > > > > > description in DT for the kernel to use. > > > > > > > > The approach I'm using is IMO quite similar to say other hardware > > > > variant runtime detection DT features like this: > > > > https://github.com/torvalds/linux/commit/157ce8f381efe264933e9366db828d845bade3a1 > > > > > > That is for things link a HAT on a RPi. It is something which is easy > > > to replace, and is expected to be replaced. > > > > Actually it's for second sourced components that are modules _within_ > > the device (a tablet or a laptop) that get swapped in at the factory. > > Definitely not something easy to replace and not expected to be replaced > > by the end user. > > Yeah, to me it seems like the PHY situation is similar, it's not replaceable > due to being copackaged, it seems the vendor just switched over to a > second source for the PHY partway through the production run without > distinguishing different SoC variants with new model numbers. > > Keep in mind stmmac itself implements mdio PHY scanning already, > which is a form of runtime PHY autodetection, so I don't really see > how doing nvmem/efuse based PHY autodetection is all that different > from that as both are forms of PHY runtime autodetection. What is different is using "phys" and "phy-names" which historically has never been used for ethernet PHYs. These have been used for serdes PHYs (e.g. multi-protocol PHYs that support PCIe, SATA, and ethernet protocols but do not provide ethernet PHY capability). Historically, "phys" and "phy-names" have been the domain of drivers/phy and not drivers/net/phy. drivers/net/phy PHYs have been described using "phy-handle". So, you're deviating from the common usage pattern, and I'm not sure whether that has been made clear to the DT maintainers that that is what is going on in this patch series. As for the PHY scanning is a driver implementation issue; it doesn't have any effect on device tree, it doesn't "abuse" DT properties to do so. The PHY scanning is likely historical, probably from times where the stmmac platform data was provided by board files (thus having the first detected PHY made things simpler.) Therefore, I don't think using it as a justification for more "autodetection" stands up. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last! ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 1/3] net: stmmac: allow drivers to explicitly select PHY device 2025-05-28 18:34 ` Russell King (Oracle) @ 2025-05-28 19:10 ` James Hilliard 2025-05-28 19:26 ` Andrew Lunn 0 siblings, 1 reply; 31+ messages in thread From: James Hilliard @ 2025-05-28 19:10 UTC (permalink / raw) To: Russell King (Oracle) Cc: wens, Andrew Lunn, netdev, linux-sunxi, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Furong Xu, Kunihiko Hayashi, linux-stm32, linux-arm-kernel, linux-kernel On Wed, May 28, 2025 at 12:34 PM Russell King (Oracle) <linux@armlinux.org.uk> wrote: > > On Wed, May 28, 2025 at 11:25:20AM -0600, James Hilliard wrote: > > On Wed, May 28, 2025 at 8:12 AM Chen-Yu Tsai <wens@csie.org> wrote: > > > > > > On Wed, May 28, 2025 at 9:25 PM Andrew Lunn <andrew@lunn.ch> wrote: > > > > > > > > On Wed, May 28, 2025 at 05:57:38AM -0600, James Hilliard wrote: > > > > > On Wed, May 28, 2025 at 1:53 AM Russell King (Oracle) > > > > > <linux@armlinux.org.uk> wrote: > > > > > > > > > > > > On Tue, May 27, 2025 at 02:37:03PM -0600, James Hilliard wrote: > > > > > > > On Tue, May 27, 2025 at 2:30 PM Andrew Lunn <andrew@lunn.ch> wrote: > > > > > > > > > > > > > > > > > Sure, that may make sense to do as well, but I still don't see > > > > > > > > > how that impacts the need to runtime select the PHY which > > > > > > > > > is configured for the correct MFD. > > > > > > > > > > > > > > > > If you know what variant you have, you only include the one PHY you > > > > > > > > actually have, and phy-handle points to it, just as normal. No runtime > > > > > > > > selection. > > > > > > > > > > > > > > Oh, so here's the issue, we have both PHY variants, older hardware > > > > > > > generally has AC200 PHY's while newer ships AC300 PHY's, but > > > > > > > when I surveyed our deployed hardware using these boards many > > > > > > > systems of similar age would randomly mix AC200 and AC300 PHY's. > > > > > > > > > > > > > > It appears there was a fairly long transition period where both variants > > > > > > > were being shipped. > > > > > > > > > > > > Given that DT is supposed to describe the hardware that is being run on, > > > > > > it should _describe_ _the_ _hardware_ that the kernel is being run on. > > > > > > > > > > > > That means not enumerating all possibilities in DT and then having magic > > > > > > in the kernel to select the right variant. That means having a correct > > > > > > description in DT for the kernel to use. > > > > > > > > > > The approach I'm using is IMO quite similar to say other hardware > > > > > variant runtime detection DT features like this: > > > > > https://github.com/torvalds/linux/commit/157ce8f381efe264933e9366db828d845bade3a1 > > > > > > > > That is for things link a HAT on a RPi. It is something which is easy > > > > to replace, and is expected to be replaced. > > > > > > Actually it's for second sourced components that are modules _within_ > > > the device (a tablet or a laptop) that get swapped in at the factory. > > > Definitely not something easy to replace and not expected to be replaced > > > by the end user. > > > > Yeah, to me it seems like the PHY situation is similar, it's not replaceable > > due to being copackaged, it seems the vendor just switched over to a > > second source for the PHY partway through the production run without > > distinguishing different SoC variants with new model numbers. > > > > Keep in mind stmmac itself implements mdio PHY scanning already, > > which is a form of runtime PHY autodetection, so I don't really see > > how doing nvmem/efuse based PHY autodetection is all that different > > from that as both are forms of PHY runtime autodetection. > > What is different is using "phys" and "phy-names" which historically > has never been used for ethernet PHYs. These have been used for serdes > PHYs (e.g. multi-protocol PHYs that support PCIe, SATA, and ethernet > protocols but do not provide ethernet PHY capability). Hmm, yeah, I had copied the convention used here https://github.com/torvalds/linux/blob/v6.15/arch/arm64/boot/dts/ti/k3-j784s4-evm-quad-port-eth-exp1.dtso#L42-L43 > Historically, "phys" and "phy-names" have been the domain of > drivers/phy and not drivers/net/phy. drivers/net/phy PHYs have > been described using "phy-handle". Yeah, I noticed it wasn't commonly used for ethernet PHYs, but I didn't see any other way to define multiple named "phy-handle"s for ethernet PHYs so I tried to use a similar style to serdes. > So, you're deviating from the common usage pattern, and I'm not sure > whether that has been made clear to the DT maintainers that that is > what is going on in this patch series. Ah, I thought it was fairly clear in the patch descriptions/example that this was a non-standard situation due to unusual hardware. > As for the PHY scanning is a driver implementation issue; it doesn't > have any effect on device tree, it doesn't "abuse" DT properties to > do so. I was just pointing that out as an example of runtime autodetection being something the kernel supports. To me it seems using existing conventions like "phys" and "phy-names" is the least invasive way to define phy's that need to be chosen at runtime. > The PHY scanning is likely historical, probably from times > where the stmmac platform data was provided by board files (thus > having the first detected PHY made things simpler.) I think a lot of ethernet drivers use phy_find_first() for phy scanning as well so it's not limited to just stmmac AFAIU. > Therefore, I > don't think using it as a justification for more "autodetection" > stands up. If anything I think the i2c OF component probe functionality is a much clearer example of precedent for DT integrated runtime hardware detection since the purpose of that(supporting second source components within the same device tree) is effectively the same use case as this efuse based runtime hardware detection. ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 1/3] net: stmmac: allow drivers to explicitly select PHY device 2025-05-28 19:10 ` James Hilliard @ 2025-05-28 19:26 ` Andrew Lunn 2025-05-28 19:45 ` James Hilliard 0 siblings, 1 reply; 31+ messages in thread From: Andrew Lunn @ 2025-05-28 19:26 UTC (permalink / raw) To: James Hilliard Cc: Russell King (Oracle), wens, netdev, linux-sunxi, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Furong Xu, Kunihiko Hayashi, linux-stm32, linux-arm-kernel, linux-kernel > I think a lot of ethernet drivers use phy_find_first() for phy scanning > as well so it's not limited to just stmmac AFAIU. You need to differentiate by time. It has become a lot less used in the last decade. DT describes the PHY, so there is no need to hunt around for it. The only real use case now a days is USB dongles, which don't have DT, and maybe PCIe devices without ACPI support. I suggest you give up pushing this. You have two Maintainers saying no to this, so it is very unlikely you are going to succeed. Andrew ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 1/3] net: stmmac: allow drivers to explicitly select PHY device 2025-05-28 19:26 ` Andrew Lunn @ 2025-05-28 19:45 ` James Hilliard 2025-05-28 21:05 ` Andrew Lunn 0 siblings, 1 reply; 31+ messages in thread From: James Hilliard @ 2025-05-28 19:45 UTC (permalink / raw) To: Andrew Lunn Cc: Russell King (Oracle), wens, netdev, linux-sunxi, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Furong Xu, Kunihiko Hayashi, linux-stm32, linux-arm-kernel, linux-kernel On Wed, May 28, 2025 at 1:27 PM Andrew Lunn <andrew@lunn.ch> wrote: > > > I think a lot of ethernet drivers use phy_find_first() for phy scanning > > as well so it's not limited to just stmmac AFAIU. > > You need to differentiate by time. It has become a lot less used in > the last decade. DT describes the PHY, so there is no need to hunt > around for it. The only real use case now a days is USB dongles, which > don't have DT, and maybe PCIe devices without ACPI support. I mean, hardware probing features for this sort of use case have been getting added outside the network subsystem so I'm not sure what the issue with this is as those use cases don't appear to be meaningfully different. > I suggest you give up pushing this. You have two Maintainers saying no > to this, so it is very unlikely you are going to succeed. So what should I be doing instead? It's not clear to me what the issue with this approach is as it appears to be a rather non-invasive change. > I personally don't like depending on the bootloader. I often replace > the bootloader, because it is a crippled version that does not let me > TFTP boot, does not have flash enabled for saving configuration, and i > want to use barebox etc. I think it is much better when Linux drives > the hardware, not the bootloader. As you said earlier we don't want to rely on the bootloader(which I agree with), so it's unclear how we should support SoC's that require runtime autodetection like this in the kernel. ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 1/3] net: stmmac: allow drivers to explicitly select PHY device 2025-05-28 19:45 ` James Hilliard @ 2025-05-28 21:05 ` Andrew Lunn 2025-05-28 21:14 ` James Hilliard 0 siblings, 1 reply; 31+ messages in thread From: Andrew Lunn @ 2025-05-28 21:05 UTC (permalink / raw) To: James Hilliard Cc: Russell King (Oracle), wens, netdev, linux-sunxi, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Furong Xu, Kunihiko Hayashi, linux-stm32, linux-arm-kernel, linux-kernel On Wed, May 28, 2025 at 01:45:40PM -0600, James Hilliard wrote: > On Wed, May 28, 2025 at 1:27 PM Andrew Lunn <andrew@lunn.ch> wrote: > > > > > I think a lot of ethernet drivers use phy_find_first() for phy scanning > > > as well so it's not limited to just stmmac AFAIU. > > > > You need to differentiate by time. It has become a lot less used in > > the last decade. DT describes the PHY, so there is no need to hunt > > around for it. The only real use case now a days is USB dongles, which > > don't have DT, and maybe PCIe devices without ACPI support. > > I mean, hardware probing features for this sort of use case have been > getting added outside the network subsystem so I'm not sure what the > issue with this is as those use cases don't appear to be meaningfully > different. > > > I suggest you give up pushing this. You have two Maintainers saying no > > to this, so it is very unlikely you are going to succeed. > > So what should I be doing instead? Describe the one PHY which actually exists in device tree for the board, and point to it using phy-handle. No runtime detection, just correctly describe the hardware. Do you have examples of boards where the SoC variant changed during the boards production life? Andrew ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 1/3] net: stmmac: allow drivers to explicitly select PHY device 2025-05-28 21:05 ` Andrew Lunn @ 2025-05-28 21:14 ` James Hilliard 2025-05-28 21:29 ` Andrew Lunn 0 siblings, 1 reply; 31+ messages in thread From: James Hilliard @ 2025-05-28 21:14 UTC (permalink / raw) To: Andrew Lunn Cc: Russell King (Oracle), wens, netdev, linux-sunxi, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Furong Xu, Kunihiko Hayashi, linux-stm32, linux-arm-kernel, linux-kernel On Wed, May 28, 2025 at 3:05 PM Andrew Lunn <andrew@lunn.ch> wrote: > > On Wed, May 28, 2025 at 01:45:40PM -0600, James Hilliard wrote: > > On Wed, May 28, 2025 at 1:27 PM Andrew Lunn <andrew@lunn.ch> wrote: > > > > > > > I think a lot of ethernet drivers use phy_find_first() for phy scanning > > > > as well so it's not limited to just stmmac AFAIU. > > > > > > You need to differentiate by time. It has become a lot less used in > > > the last decade. DT describes the PHY, so there is no need to hunt > > > around for it. The only real use case now a days is USB dongles, which > > > don't have DT, and maybe PCIe devices without ACPI support. > > > > I mean, hardware probing features for this sort of use case have been > > getting added outside the network subsystem so I'm not sure what the > > issue with this is as those use cases don't appear to be meaningfully > > different. > > > > > I suggest you give up pushing this. You have two Maintainers saying no > > > to this, so it is very unlikely you are going to succeed. > > > > So what should I be doing instead? > > Describe the one PHY which actually exists in device tree for the > board, and point to it using phy-handle. No runtime detection, just > correctly describe the hardware. But the boards randomly contain SoC's with different PHY's so we have to support both variants. > Do you have examples of boards where the SoC variant changed during > the boards production life? Yes, the boards I'm working for example, but this is likely an issue for other boards as well(vendor BSP auto detects PHY variants): https://www.zeusbtc.com/ASIC-Miner-Repair/Parts-Tools-Details.asp?ID=1139 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 1/3] net: stmmac: allow drivers to explicitly select PHY device 2025-05-28 21:14 ` James Hilliard @ 2025-05-28 21:29 ` Andrew Lunn 2025-05-28 21:45 ` James Hilliard 0 siblings, 1 reply; 31+ messages in thread From: Andrew Lunn @ 2025-05-28 21:29 UTC (permalink / raw) To: James Hilliard Cc: Russell King (Oracle), wens, netdev, linux-sunxi, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Furong Xu, Kunihiko Hayashi, linux-stm32, linux-arm-kernel, linux-kernel > > Describe the one PHY which actually exists in device tree for the > > board, and point to it using phy-handle. No runtime detection, just > > correctly describe the hardware. > > But the boards randomly contain SoC's with different PHY's so we > have to support both variants. You have two .dts files, resulting in two .dtb files, which are 95% identical, but import a different .dtsi file for the PHY. You can test if the correct .dtb blob is used by checking the fuse. If it is wrong, you can give a hint what .dtb should be used. Or, as Russell suggested, you give the bootloader both .dtb blobs, and it can pick the correct one to pass to the kernel. Or the bootloader can patch the .dtb blob to make it fit the hardware. > > Do you have examples of boards where the SoC variant changed during > > the boards production life? > > Yes, the boards I'm working for example, but this is likely an issue for > other boards as well(vendor BSP auto detects PHY variants): > https://www.zeusbtc.com/ASIC-Miner-Repair/Parts-Tools-Details.asp?ID=1139 Mainline generally does not care what vendors do, because they often do horrible things. Which is O.K, it is open source, they can do what they want in their fork of the kernel. But for Mainline, we expect a high level of quality, and a uniform way of doing things. This can also act as push back on SoC vendors, for doing silly things like changing the PHY within a SoC without changing its name/number. Andrew ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 1/3] net: stmmac: allow drivers to explicitly select PHY device 2025-05-28 21:29 ` Andrew Lunn @ 2025-05-28 21:45 ` James Hilliard 2025-05-28 23:47 ` Andrew Lunn 0 siblings, 1 reply; 31+ messages in thread From: James Hilliard @ 2025-05-28 21:45 UTC (permalink / raw) To: Andrew Lunn Cc: Russell King (Oracle), wens, netdev, linux-sunxi, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Furong Xu, Kunihiko Hayashi, linux-stm32, linux-arm-kernel, linux-kernel On Wed, May 28, 2025 at 3:29 PM Andrew Lunn <andrew@lunn.ch> wrote: > > > > Describe the one PHY which actually exists in device tree for the > > > board, and point to it using phy-handle. No runtime detection, just > > > correctly describe the hardware. > > > > But the boards randomly contain SoC's with different PHY's so we > > have to support both variants. > > You have two .dts files, resulting in two .dtb files, which are 95% > identical, but import a different .dtsi file for the PHY. > > You can test if the correct .dtb blob is used by checking the fuse. If > it is wrong, you can give a hint what .dtb should be used. How is this better than just choosing the correct PHY based on the efuse? > Or, as Russell suggested, you give the bootloader both .dtb blobs, and > it can pick the correct one to pass to the kernel. Or the bootloader > can patch the .dtb blob to make it fit the hardware. This is what I'm really trying to avoid since it requires special handling in the bootloader and therefore will result in a lot of broken systems since most people doing ports to H616 based boards will only ever test against one PHY variant. > > > Do you have examples of boards where the SoC variant changed during > > > the boards production life? > > > > Yes, the boards I'm working for example, but this is likely an issue for > > other boards as well(vendor BSP auto detects PHY variants): > > https://www.zeusbtc.com/ASIC-Miner-Repair/Parts-Tools-Details.asp?ID=1139 > > Mainline generally does not care what vendors do, because they often > do horrible things. Which is O.K, it is open source, they can do what > they want in their fork of the kernel. That's not really true IMO, mainline implements all sorts of workarounds for various vendor hardware quicks/weirdness. > But for Mainline, we expect a high level of quality, and a uniform way > of doing things. Sure, and I'm trying to do that here rather than do some super hacky unmaintainable bootloader based device tree selector. > This can also act as push back on SoC vendors, for doing silly things > like changing the PHY within a SoC without changing its name/number. It won't here, because Allwinner doesn't care about non-BSP kernels. ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 1/3] net: stmmac: allow drivers to explicitly select PHY device 2025-05-28 21:45 ` James Hilliard @ 2025-05-28 23:47 ` Andrew Lunn 2025-05-29 0:31 ` James Hilliard 0 siblings, 1 reply; 31+ messages in thread From: Andrew Lunn @ 2025-05-28 23:47 UTC (permalink / raw) To: James Hilliard Cc: Russell King (Oracle), wens, netdev, linux-sunxi, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Furong Xu, Kunihiko Hayashi, linux-stm32, linux-arm-kernel, linux-kernel > > Or, as Russell suggested, you give the bootloader both .dtb blobs, and > > it can pick the correct one to pass to the kernel. Or the bootloader > > can patch the .dtb blob to make it fit the hardware. > > This is what I'm really trying to avoid since it requires special > handling in the bootloader and therefore will result in a lot of broken > systems since most people doing ports to H616 based boards will only > ever test against one PHY variant. Which in some ways is good. They will then issue four letter words at Allwinner, and go find a better SoC vendor. > > > > Do you have examples of boards where the SoC variant changed during > > > > the boards production life? > > > > > > Yes, the boards I'm working for example, but this is likely an issue for > > > other boards as well(vendor BSP auto detects PHY variants): > > > https://www.zeusbtc.com/ASIC-Miner-Repair/Parts-Tools-Details.asp?ID=1139 > > > > Mainline generally does not care what vendors do, because they often > > do horrible things. Which is O.K, it is open source, they can do what > > they want in their fork of the kernel. > > That's not really true IMO, mainline implements all sorts of workarounds > for various vendor hardware quicks/weirdness. > > > But for Mainline, we expect a high level of quality, and a uniform way > > of doing things. > > Sure, and I'm trying to do that here rather than do some super hacky > unmaintainable bootloader based device tree selector. > > > This can also act as push back on SoC vendors, for doing silly things > > like changing the PHY within a SoC without changing its name/number. > > It won't here, because Allwinner doesn't care about non-BSP kernels. It can be indirect pressure. There are some OEMs which care about Mainline. They will do their due diligence, find that user report Mainline if flaky on these devices, and go find a different vendor. There will be some OEM which get burnt by this mess, and when they come to their second generation device, they will switch vendor and tell the old vendor why. It could well be Allwinner can support their bottom line without caring about Mainline, so really don't care. But Mainline can help point OEMs away from them to those which are more Mainline friendly. We also need to think about this as a two way street. What does this SoC bring to Mainline? Why should Mainline care about it? It has some major design issues, do we want to say that is O.K? Do we want other vendors to think we are O.K. with bad designs? Worse still, this is stmmac, which lots of vendors already abuse in lots of different ways. Russell has put in a lot of effort recently to clean up some of that abuse, and we are pushing back hard on new abusers. If you can hide this mess away in the bootloader, it just looks like a regular device, we are likely to accept it. If you try to do something different to the normal for PHYs, we are very likely to reject it. Andrew ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 1/3] net: stmmac: allow drivers to explicitly select PHY device 2025-05-28 23:47 ` Andrew Lunn @ 2025-05-29 0:31 ` James Hilliard 0 siblings, 0 replies; 31+ messages in thread From: James Hilliard @ 2025-05-29 0:31 UTC (permalink / raw) To: Andrew Lunn Cc: Russell King (Oracle), wens, netdev, linux-sunxi, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Furong Xu, Kunihiko Hayashi, linux-stm32, linux-arm-kernel, linux-kernel On Wed, May 28, 2025 at 5:47 PM Andrew Lunn <andrew@lunn.ch> wrote: > > > > Or, as Russell suggested, you give the bootloader both .dtb blobs, and > > > it can pick the correct one to pass to the kernel. Or the bootloader > > > can patch the .dtb blob to make it fit the hardware. > > > > This is what I'm really trying to avoid since it requires special > > handling in the bootloader and therefore will result in a lot of broken > > systems since most people doing ports to H616 based boards will only > > ever test against one PHY variant. > > Which in some ways is good. They will then issue four letter words at > Allwinner, and go find a better SoC vendor. No, they won't, because the vendor that actually buys from Allwinner also uses the BSP and doesn't care as long as the BSP works. > > > > > > Do you have examples of boards where the SoC variant changed during > > > > > the boards production life? > > > > > > > > Yes, the boards I'm working for example, but this is likely an issue for > > > > other boards as well(vendor BSP auto detects PHY variants): > > > > https://www.zeusbtc.com/ASIC-Miner-Repair/Parts-Tools-Details.asp?ID=1139 > > > > > > Mainline generally does not care what vendors do, because they often > > > do horrible things. Which is O.K, it is open source, they can do what > > > they want in their fork of the kernel. > > > > That's not really true IMO, mainline implements all sorts of workarounds > > for various vendor hardware quicks/weirdness. > > > > > But for Mainline, we expect a high level of quality, and a uniform way > > > of doing things. > > > > Sure, and I'm trying to do that here rather than do some super hacky > > unmaintainable bootloader based device tree selector. > > > > > This can also act as push back on SoC vendors, for doing silly things > > > like changing the PHY within a SoC without changing its name/number. > > > > It won't here, because Allwinner doesn't care about non-BSP kernels. > > It can be indirect pressure. There are some OEMs which care about > Mainline. They will do their due diligence, find that user report > Mainline if flaky on these devices, and go find a different > vendor. There are zero OEMs in my industry that provide hardware with any mainline support at all. The vendors are outright hostile to 3rd party firmware and put significant effort into preventing 3rd party firmware. The OEMs will view the lack of mainline support as a bonus unfortunately as they like everything locked down. > There will be some OEM which get burnt by this mess, and when > they come to their second generation device, they will switch vendor > and tell the old vendor why. It could well be Allwinner can support > their bottom line without caring about Mainline, so really don't > care. But Mainline can help point OEMs away from them to those which > are more Mainline friendly. The OEMs in my industry will not change due to a lack of mainline support, they have no interest in mainline support, they only care that their products work well enough to sell, and since there is little competition > We also need to think about this as a two way street. What does this > SoC bring to Mainline? Why should Mainline care about it? The H616 is used in a number of development boards(orangepi and such) and TV boxes, having decent mainline support at least allows downstream integrators to have a chance at improving the situation regardless of vendor cooperation, maybe the SoC vendor will eventually care if people are actually using mainline kernels more. > It has some > major design issues, do we want to say that is O.K? Do we want other > vendors to think we are O.K. with bad designs? I mean, it's a second source issue for the PHY, less of an outright design issue and more likely was just some cost optimization. If we excluded all vendors with bad designs then Linux would have far less hardware support. > Worse still, this is > stmmac, which lots of vendors already abuse in lots of different > ways. Russell has put in a lot of effort recently to clean up some of > that abuse, and we are pushing back hard on new abusers. > > If you can hide this mess away in the bootloader, it just looks like a > regular device, we are likely to accept it. If you try to do something > different to the normal for PHYs, we are very likely to reject it. I'm confused, the kernel isn't the bootloader so how can it be accepted by the kernel if the implementation is not in the kernel? Linux supports plenty of weird hardware so I really don't see why having a quirk for this specific board is such a problem as long as the quirk doesn't introduce maintainability issues. ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 1/3] net: stmmac: allow drivers to explicitly select PHY device 2025-05-27 20:01 ` Andrew Lunn 2025-05-27 20:16 ` James Hilliard @ 2025-05-30 23:46 ` James Hilliard 2025-05-30 23:56 ` Florian Fainelli 1 sibling, 1 reply; 31+ messages in thread From: James Hilliard @ 2025-05-30 23:46 UTC (permalink / raw) To: Andrew Lunn Cc: netdev, linux-sunxi, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Russell King, Russell King (Oracle), Furong Xu, Kunihiko Hayashi, linux-stm32, linux-arm-kernel, linux-kernel On Tue, May 27, 2025 at 2:02 PM Andrew Lunn <andrew@lunn.ch> wrote: > > On Tue, May 27, 2025 at 01:21:21PM -0600, James Hilliard wrote: > > On Tue, May 27, 2025 at 1:14 PM Andrew Lunn <andrew@lunn.ch> wrote: > > > > > > On Tue, May 27, 2025 at 11:55:54AM -0600, James Hilliard wrote: > > > > Some devices like the Allwinner H616 need the ability to select a phy > > > > in cases where multiple PHY's may be present in a device tree due to > > > > needing the ability to support multiple SoC variants with runtime > > > > PHY selection. > > > > > > I'm not convinced about this yet. As far as i see, it is different > > > variants of the H616. They should have different compatibles, since > > > they are not actually compatible, and you should have different DT > > > descriptions. So you don't need runtime PHY selection. > > > > Different compatibles for what specifically? I mean the PHY compatibles > > are just the generic "ethernet-phy-ieee802.3-c22" compatibles. > > You at least have a different MTD devices, exporting different > clocks/PWM/Reset controllers. That should have different compatibles, > since they are not compatible. You then need phandles to these > different clocks/PWM/Reset controllers, and for one of the PHYs you > need a phandle to the I2C bus, so the PHY driver can do the > initialisation. So i think in the end you know what PHY you have on > the board, so there is no need to do runtime detection. Hmm, thinking about this again, maybe it makes sense to just do the runtime detection in the MFD driver entirely, as it turns out the AC300 initialization sequence is largely a subset of the AC200 initialization sequence(AC300 would just not need any i2c part of the initialization sequence). So if we use the same MFD driver which internally does autodetection then we can avoid the need for selecting separate PHY's entirely. This at least is largely how the vendor BSP driver works at the moment. Would this approach make sense? > What you might want however is to validate the MTD device compatible > against the fuse and return -ENODEV if the compatible is wrong for the > fuse. > > Andrew ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 1/3] net: stmmac: allow drivers to explicitly select PHY device 2025-05-30 23:46 ` James Hilliard @ 2025-05-30 23:56 ` Florian Fainelli 2025-05-31 0:02 ` James Hilliard 0 siblings, 1 reply; 31+ messages in thread From: Florian Fainelli @ 2025-05-30 23:56 UTC (permalink / raw) To: James Hilliard, Andrew Lunn Cc: netdev, linux-sunxi, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Russell King, Russell King (Oracle), Furong Xu, Kunihiko Hayashi, linux-stm32, linux-arm-kernel, linux-kernel On 5/30/25 16:46, James Hilliard wrote: > On Tue, May 27, 2025 at 2:02 PM Andrew Lunn <andrew@lunn.ch> wrote: >> >> On Tue, May 27, 2025 at 01:21:21PM -0600, James Hilliard wrote: >>> On Tue, May 27, 2025 at 1:14 PM Andrew Lunn <andrew@lunn.ch> wrote: >>>> >>>> On Tue, May 27, 2025 at 11:55:54AM -0600, James Hilliard wrote: >>>>> Some devices like the Allwinner H616 need the ability to select a phy >>>>> in cases where multiple PHY's may be present in a device tree due to >>>>> needing the ability to support multiple SoC variants with runtime >>>>> PHY selection. >>>> >>>> I'm not convinced about this yet. As far as i see, it is different >>>> variants of the H616. They should have different compatibles, since >>>> they are not actually compatible, and you should have different DT >>>> descriptions. So you don't need runtime PHY selection. >>> >>> Different compatibles for what specifically? I mean the PHY compatibles >>> are just the generic "ethernet-phy-ieee802.3-c22" compatibles. >> >> You at least have a different MTD devices, exporting different >> clocks/PWM/Reset controllers. That should have different compatibles, >> since they are not compatible. You then need phandles to these >> different clocks/PWM/Reset controllers, and for one of the PHYs you >> need a phandle to the I2C bus, so the PHY driver can do the >> initialisation. So i think in the end you know what PHY you have on >> the board, so there is no need to do runtime detection. > > Hmm, thinking about this again, maybe it makes sense to just > do the runtime detection in the MFD driver entirely, as it turns > out the AC300 initialization sequence is largely a subset of the > AC200 initialization sequence(AC300 would just not need any > i2c part of the initialization sequence). So if we use the same > MFD driver which internally does autodetection then we can > avoid the need for selecting separate PHY's entirely. This at > least is largely how the vendor BSP driver works at the moment. > > Would this approach make sense? This has likely been discussed, but cannot you move the guts of patch #2 into u-boot or the boot loader being used and have it patch the PHY Device Tree node's "reg" property accordingly before handing out the DTB to the kernel? Another way to address what you want to do is to remove the "reg" property from the Ethernet PHY node and just let of_mdiobus_register() automatically scan, you have the advantage of having the addresses consecutive so this won't dramatically increase the boot time... I do that on the boards I suppose that have a removable mezzanine card that includes a PHY address whose address is dictated by straps so we don't want to guess, we let the kernel auto detect instead. -- Florian ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 1/3] net: stmmac: allow drivers to explicitly select PHY device 2025-05-30 23:56 ` Florian Fainelli @ 2025-05-31 0:02 ` James Hilliard 2025-05-31 0:24 ` Florian Fainelli 0 siblings, 1 reply; 31+ messages in thread From: James Hilliard @ 2025-05-31 0:02 UTC (permalink / raw) To: Florian Fainelli Cc: Andrew Lunn, netdev, linux-sunxi, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Russell King, Russell King (Oracle), Furong Xu, Kunihiko Hayashi, linux-stm32, linux-arm-kernel, linux-kernel On Fri, May 30, 2025 at 5:56 PM Florian Fainelli <f.fainelli@gmail.com> wrote: > > On 5/30/25 16:46, James Hilliard wrote: > > On Tue, May 27, 2025 at 2:02 PM Andrew Lunn <andrew@lunn.ch> wrote: > >> > >> On Tue, May 27, 2025 at 01:21:21PM -0600, James Hilliard wrote: > >>> On Tue, May 27, 2025 at 1:14 PM Andrew Lunn <andrew@lunn.ch> wrote: > >>>> > >>>> On Tue, May 27, 2025 at 11:55:54AM -0600, James Hilliard wrote: > >>>>> Some devices like the Allwinner H616 need the ability to select a phy > >>>>> in cases where multiple PHY's may be present in a device tree due to > >>>>> needing the ability to support multiple SoC variants with runtime > >>>>> PHY selection. > >>>> > >>>> I'm not convinced about this yet. As far as i see, it is different > >>>> variants of the H616. They should have different compatibles, since > >>>> they are not actually compatible, and you should have different DT > >>>> descriptions. So you don't need runtime PHY selection. > >>> > >>> Different compatibles for what specifically? I mean the PHY compatibles > >>> are just the generic "ethernet-phy-ieee802.3-c22" compatibles. > >> > >> You at least have a different MTD devices, exporting different > >> clocks/PWM/Reset controllers. That should have different compatibles, > >> since they are not compatible. You then need phandles to these > >> different clocks/PWM/Reset controllers, and for one of the PHYs you > >> need a phandle to the I2C bus, so the PHY driver can do the > >> initialisation. So i think in the end you know what PHY you have on > >> the board, so there is no need to do runtime detection. > > > > Hmm, thinking about this again, maybe it makes sense to just > > do the runtime detection in the MFD driver entirely, as it turns > > out the AC300 initialization sequence is largely a subset of the > > AC200 initialization sequence(AC300 would just not need any > > i2c part of the initialization sequence). So if we use the same > > MFD driver which internally does autodetection then we can > > avoid the need for selecting separate PHY's entirely. This at > > least is largely how the vendor BSP driver works at the moment. > > > > Would this approach make sense? > > This has likely been discussed, but cannot you move the guts of patch #2 > into u-boot or the boot loader being used and have it patch the PHY > Device Tree node's "reg" property accordingly before handing out the DTB > to the kernel? No, that's not really the issue, the "reg" property can actually be the same for both the AC200 and AC300 phy's, both support using address 0, the AC200 additionally supports address 1. In my example they are different simply so that they don't conflict in the device tree. The actual issue is that they have differing initialization sequences and won't appear in mdio bus scans until after the initialization is complete. > Another way to address what you want to do is to remove the "reg" > property from the Ethernet PHY node and just let of_mdiobus_register() > automatically scan, you have the advantage of having the addresses > consecutive so this won't dramatically increase the boot time... I do > that on the boards I suppose that have a removable mezzanine card that > includes a PHY address whose address is dictated by straps so we don't > want to guess, we let the kernel auto detect instead. Yeah, I noticed this, but it doesn't really help since it's not the address that's incompatible but the reset sequence, I'm having trouble finding examples for mdio based reset drivers in the kernel however. ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 1/3] net: stmmac: allow drivers to explicitly select PHY device 2025-05-31 0:02 ` James Hilliard @ 2025-05-31 0:24 ` Florian Fainelli 2025-05-31 0:49 ` James Hilliard 0 siblings, 1 reply; 31+ messages in thread From: Florian Fainelli @ 2025-05-31 0:24 UTC (permalink / raw) To: James Hilliard Cc: Andrew Lunn, netdev, linux-sunxi, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Russell King, Russell King (Oracle), Furong Xu, Kunihiko Hayashi, linux-stm32, linux-arm-kernel, linux-kernel On 5/30/25 17:02, James Hilliard wrote: > On Fri, May 30, 2025 at 5:56 PM Florian Fainelli <f.fainelli@gmail.com> wrote: >> >> On 5/30/25 16:46, James Hilliard wrote: >>> On Tue, May 27, 2025 at 2:02 PM Andrew Lunn <andrew@lunn.ch> wrote: >>>> >>>> On Tue, May 27, 2025 at 01:21:21PM -0600, James Hilliard wrote: >>>>> On Tue, May 27, 2025 at 1:14 PM Andrew Lunn <andrew@lunn.ch> wrote: >>>>>> >>>>>> On Tue, May 27, 2025 at 11:55:54AM -0600, James Hilliard wrote: >>>>>>> Some devices like the Allwinner H616 need the ability to select a phy >>>>>>> in cases where multiple PHY's may be present in a device tree due to >>>>>>> needing the ability to support multiple SoC variants with runtime >>>>>>> PHY selection. >>>>>> >>>>>> I'm not convinced about this yet. As far as i see, it is different >>>>>> variants of the H616. They should have different compatibles, since >>>>>> they are not actually compatible, and you should have different DT >>>>>> descriptions. So you don't need runtime PHY selection. >>>>> >>>>> Different compatibles for what specifically? I mean the PHY compatibles >>>>> are just the generic "ethernet-phy-ieee802.3-c22" compatibles. >>>> >>>> You at least have a different MTD devices, exporting different >>>> clocks/PWM/Reset controllers. That should have different compatibles, >>>> since they are not compatible. You then need phandles to these >>>> different clocks/PWM/Reset controllers, and for one of the PHYs you >>>> need a phandle to the I2C bus, so the PHY driver can do the >>>> initialisation. So i think in the end you know what PHY you have on >>>> the board, so there is no need to do runtime detection. >>> >>> Hmm, thinking about this again, maybe it makes sense to just >>> do the runtime detection in the MFD driver entirely, as it turns >>> out the AC300 initialization sequence is largely a subset of the >>> AC200 initialization sequence(AC300 would just not need any >>> i2c part of the initialization sequence). So if we use the same >>> MFD driver which internally does autodetection then we can >>> avoid the need for selecting separate PHY's entirely. This at >>> least is largely how the vendor BSP driver works at the moment. >>> >>> Would this approach make sense? >> >> This has likely been discussed, but cannot you move the guts of patch #2 >> into u-boot or the boot loader being used and have it patch the PHY >> Device Tree node's "reg" property accordingly before handing out the DTB >> to the kernel? > > No, that's not really the issue, the "reg" property can actually be > the same for both the AC200 and AC300 phy's, both support using > address 0, the AC200 additionally supports address 1. In my example > they are different simply so that they don't conflict in the device tree. > > The actual issue is that they have differing initialization sequences and > won't appear in mdio bus scans until after the initialization is complete. > >> Another way to address what you want to do is to remove the "reg" >> property from the Ethernet PHY node and just let of_mdiobus_register() >> automatically scan, you have the advantage of having the addresses >> consecutive so this won't dramatically increase the boot time... I do >> that on the boards I suppose that have a removable mezzanine card that >> includes a PHY address whose address is dictated by straps so we don't >> want to guess, we let the kernel auto detect instead. > > Yeah, I noticed this, but it doesn't really help since it's not the address > that's incompatible but the reset sequence, I'm having trouble finding > examples for mdio based reset drivers in the kernel however. Fair enough, but it seems like we need to dig up a bit more here on that topic. There is an opportunity for a MDIO driver to implement a "pre-scan" reset by filling in a mdio_bus::reset callback and there you can do various things to ensure that your Ethernet PHY will be responsive. You can see an example under drivers/net/mdio/mdio-bcm-unimac.c to address a deficiency of certain Ethernet PHYs. Through Device Tree you can use the standard properties "reset-gpios", "reset-assert-us", "reset-deassert-us" to implement a basic reset sequence on a per-PHY basis, there are other properties that apply to the MDIO bus/controller specifically that are also documented. How does it currently work given that your example Device Tree uses: compatible = "ethernet-phy-ieee802.3-c22" this will still require the OF MDIO bus layer to read the PHYSID1/PHYSID2 registers in order to match your PHY device with its driver. You indicated that the PHYs "won't appear in mdio bus scan" unless that sequence is implemented. How would they currently respond given the example? If you can involve the boot loader, you can create a compatible string for your PHY of the form: compatible = "ethernet-phy-idae02.5090" that includes the PHY OUI, and that will tell the OF MDIO bus code to bind the PHY device with the driver specified in the compatible string without reading the PHYSID1/PHYSID2 registers. Since you can detect the boards variants, you could do that. It then becomes highly desirable to have a "dedicated" (as opposed to using the "Generic PHY") driver that within the .probe function can take care of putting the PHY in a working state. -- Florian ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 1/3] net: stmmac: allow drivers to explicitly select PHY device 2025-05-31 0:24 ` Florian Fainelli @ 2025-05-31 0:49 ` James Hilliard 0 siblings, 0 replies; 31+ messages in thread From: James Hilliard @ 2025-05-31 0:49 UTC (permalink / raw) To: Florian Fainelli Cc: Andrew Lunn, netdev, linux-sunxi, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Russell King, Russell King (Oracle), Furong Xu, Kunihiko Hayashi, linux-stm32, linux-arm-kernel, linux-kernel On Fri, May 30, 2025 at 6:24 PM Florian Fainelli <f.fainelli@gmail.com> wrote: > > On 5/30/25 17:02, James Hilliard wrote: > > On Fri, May 30, 2025 at 5:56 PM Florian Fainelli <f.fainelli@gmail.com> wrote: > >> > >> On 5/30/25 16:46, James Hilliard wrote: > >>> On Tue, May 27, 2025 at 2:02 PM Andrew Lunn <andrew@lunn.ch> wrote: > >>>> > >>>> On Tue, May 27, 2025 at 01:21:21PM -0600, James Hilliard wrote: > >>>>> On Tue, May 27, 2025 at 1:14 PM Andrew Lunn <andrew@lunn.ch> wrote: > >>>>>> > >>>>>> On Tue, May 27, 2025 at 11:55:54AM -0600, James Hilliard wrote: > >>>>>>> Some devices like the Allwinner H616 need the ability to select a phy > >>>>>>> in cases where multiple PHY's may be present in a device tree due to > >>>>>>> needing the ability to support multiple SoC variants with runtime > >>>>>>> PHY selection. > >>>>>> > >>>>>> I'm not convinced about this yet. As far as i see, it is different > >>>>>> variants of the H616. They should have different compatibles, since > >>>>>> they are not actually compatible, and you should have different DT > >>>>>> descriptions. So you don't need runtime PHY selection. > >>>>> > >>>>> Different compatibles for what specifically? I mean the PHY compatibles > >>>>> are just the generic "ethernet-phy-ieee802.3-c22" compatibles. > >>>> > >>>> You at least have a different MTD devices, exporting different > >>>> clocks/PWM/Reset controllers. That should have different compatibles, > >>>> since they are not compatible. You then need phandles to these > >>>> different clocks/PWM/Reset controllers, and for one of the PHYs you > >>>> need a phandle to the I2C bus, so the PHY driver can do the > >>>> initialisation. So i think in the end you know what PHY you have on > >>>> the board, so there is no need to do runtime detection. > >>> > >>> Hmm, thinking about this again, maybe it makes sense to just > >>> do the runtime detection in the MFD driver entirely, as it turns > >>> out the AC300 initialization sequence is largely a subset of the > >>> AC200 initialization sequence(AC300 would just not need any > >>> i2c part of the initialization sequence). So if we use the same > >>> MFD driver which internally does autodetection then we can > >>> avoid the need for selecting separate PHY's entirely. This at > >>> least is largely how the vendor BSP driver works at the moment. > >>> > >>> Would this approach make sense? > >> > >> This has likely been discussed, but cannot you move the guts of patch #2 > >> into u-boot or the boot loader being used and have it patch the PHY > >> Device Tree node's "reg" property accordingly before handing out the DTB > >> to the kernel? > > > > No, that's not really the issue, the "reg" property can actually be > > the same for both the AC200 and AC300 phy's, both support using > > address 0, the AC200 additionally supports address 1. In my example > > they are different simply so that they don't conflict in the device tree. > > > > The actual issue is that they have differing initialization sequences and > > won't appear in mdio bus scans until after the initialization is complete. > > >> Another way to address what you want to do is to remove the "reg" > >> property from the Ethernet PHY node and just let of_mdiobus_register() > >> automatically scan, you have the advantage of having the addresses > >> consecutive so this won't dramatically increase the boot time... I do > >> that on the boards I suppose that have a removable mezzanine card that > >> includes a PHY address whose address is dictated by straps so we don't > >> want to guess, we let the kernel auto detect instead. > > > > Yeah, I noticed this, but it doesn't really help since it's not the address > > that's incompatible but the reset sequence, I'm having trouble finding > > examples for mdio based reset drivers in the kernel however. > > Fair enough, but it seems like we need to dig up a bit more here on that > topic. There is an opportunity for a MDIO driver to implement a > "pre-scan" reset by filling in a mdio_bus::reset callback and there you > can do various things to ensure that your Ethernet PHY will be > responsive. You can see an example under > drivers/net/mdio/mdio-bcm-unimac.c to address a deficiency of certain > Ethernet PHYs. So if I need to do custom stuff to make the generic PHY's addresses on the mdio bus live would I replace the generic "snps,dwmac-mdio" compatible with a custom compatible maybe? > Through Device Tree you can use the standard properties "reset-gpios", > "reset-assert-us", "reset-deassert-us" to implement a basic reset > sequence on a per-PHY basis, there are other properties that apply to > the MDIO bus/controller specifically that are also documented. The mdio initialization sequence for both PHY's is custom from my understanding so presumably we can't use the generic "reset-gpios" and such. > How does it currently work given that your example Device Tree uses: > > compatible = "ethernet-phy-ieee802.3-c22" > > this will still require the OF MDIO bus layer to read the > PHYSID1/PHYSID2 registers in order to match your PHY device with its > driver. You indicated that the PHYs "won't appear in mdio bus scan" > unless that sequence is implemented. How would they currently respond > given the example? In my example it's not actually doing the initialization part yet, that's all being done in some super hacky u-boot code. My assumption was that we need different generic phy nodes to differentiate the resets but I suppose that could all be done elsewhere in whichever driver implements the initialization sequence. > If you can involve the boot loader, you can create a compatible string > for your PHY of the form: > > compatible = "ethernet-phy-idae02.5090" > > that includes the PHY OUI, and that will tell the OF MDIO bus code to > bind the PHY device with the driver specified in the compatible string > without reading the PHYSID1/PHYSID2 registers. Since you can detect the > boards variants, you could do that. The address 0 and 1 PHY OUI's are the same for the AC200/AC300, the AC300 PHY however has a different PHY OUI for address 0x10 which is effectively used in place of the i2c initialization sequence in the AC200. Note this 0x10 address is not usable for normal operations, it's essentially only used to activate the main mdio address 0 used for normal operations. > It then becomes highly desirable to have a "dedicated" (as opposed to > using the "Generic PHY") driver that within the .probe function can take > care of putting the PHY in a working state. > -- > Florian ^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2025-05-31 0:51 UTC | newest] Thread overview: 31+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-05-27 17:55 [PATCH v2 1/3] net: stmmac: allow drivers to explicitly select PHY device James Hilliard 2025-05-27 17:55 ` [PATCH v2 2/3] net: stmmac: dwmac-sun8i: Allow runtime AC200/AC300 phy selection James Hilliard 2025-05-27 17:55 ` [PATCH v2 3/3] dt-bindings: net: sun8i-emac: Add AC300 EMAC1 nvmem " James Hilliard 2025-05-27 19:14 ` [PATCH v2 1/3] net: stmmac: allow drivers to explicitly select PHY device Andrew Lunn 2025-05-27 19:21 ` James Hilliard 2025-05-27 20:01 ` Andrew Lunn 2025-05-27 20:16 ` James Hilliard 2025-05-27 20:30 ` Andrew Lunn 2025-05-27 20:37 ` James Hilliard 2025-05-27 21:48 ` Andrew Lunn 2025-05-27 22:47 ` James Hilliard 2025-05-28 7:53 ` Russell King (Oracle) 2025-05-28 11:57 ` James Hilliard 2025-05-28 13:24 ` Andrew Lunn 2025-05-28 14:11 ` Chen-Yu Tsai 2025-05-28 17:25 ` James Hilliard 2025-05-28 18:34 ` Russell King (Oracle) 2025-05-28 19:10 ` James Hilliard 2025-05-28 19:26 ` Andrew Lunn 2025-05-28 19:45 ` James Hilliard 2025-05-28 21:05 ` Andrew Lunn 2025-05-28 21:14 ` James Hilliard 2025-05-28 21:29 ` Andrew Lunn 2025-05-28 21:45 ` James Hilliard 2025-05-28 23:47 ` Andrew Lunn 2025-05-29 0:31 ` James Hilliard 2025-05-30 23:46 ` James Hilliard 2025-05-30 23:56 ` Florian Fainelli 2025-05-31 0:02 ` James Hilliard 2025-05-31 0:24 ` Florian Fainelli 2025-05-31 0:49 ` James Hilliard
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).