* [PATCH 1/3] net: ethernet: ixp4xx: Add DT bindings @ 2021-04-19 22:51 Linus Walleij 2021-04-19 22:51 ` [PATCH 2/3] net: ethernet: ixp4xx: Support device tree probing Linus Walleij ` (2 more replies) 0 siblings, 3 replies; 14+ messages in thread From: Linus Walleij @ 2021-04-19 22:51 UTC (permalink / raw) To: netdev, David S . Miller, Jakub Kicinski Cc: Linus Walleij, Zoltan HERPAI, Raylynn Knight, devicetree This adds device tree bindings for the IXP4xx ethernet controller with optional MDIO bridge. Cc: Zoltan HERPAI <wigyori@uid0.hu> Cc: Raylynn Knight <rayknight@me.com> Cc: devicetree@vger.kernel.org Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- ChangeLog v1->v2: - Add schema for the (optional) embedded MDIO bus inside the ethernet controller in an "mdio" node instead of just letting the code randomly populate and present it to the operating system. - Reference the standard schemas for ethernet controller and MDIO buses. - Add intel,npe to indentify the NPE unit used with each ethernet adapter. --- .../bindings/net/intel,ixp4xx-ethernet.yaml | 80 +++++++++++++++++++ 1 file changed, 80 insertions(+) create mode 100644 Documentation/devicetree/bindings/net/intel,ixp4xx-ethernet.yaml diff --git a/Documentation/devicetree/bindings/net/intel,ixp4xx-ethernet.yaml b/Documentation/devicetree/bindings/net/intel,ixp4xx-ethernet.yaml new file mode 100644 index 000000000000..55ef6ff7d171 --- /dev/null +++ b/Documentation/devicetree/bindings/net/intel,ixp4xx-ethernet.yaml @@ -0,0 +1,80 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +# Copyright 2018 Linaro Ltd. +%YAML 1.2 +--- +$id: "http://devicetree.org/schemas/net/intel,ixp4xx-ethernet.yaml#" +$schema: "http://devicetree.org/meta-schemas/core.yaml#" + +title: Intel IXP4xx ethernet + +allOf: + - $ref: "ethernet-controller.yaml#" + +maintainers: + - Linus Walleij <linus.walleij@linaro.org> + +description: | + The Intel IXP4xx ethernet makes use of the IXP4xx NPE (Network + Processing Engine) and the IXP4xx Queue Mangager to process + the ethernet frames. It can optionally contain an MDIO bus to + talk to PHYs. + +properties: + compatible: + const: intel,ixp4xx-ethernet + + reg: + maxItems: 1 + description: Ethernet MMIO address range + + queue-rx: + $ref: '/schemas/types.yaml#/definitions/phandle-array' + maxItems: 1 + description: phandle to the RX queue on the NPE + + queue-txready: + $ref: '/schemas/types.yaml#/definitions/phandle-array' + maxItems: 1 + description: phandle to the TX READY queue on the NPE + + phy-handle: true + + intel,npe: + $ref: /schemas/types.yaml#/definitions/uint32 + enum: [0, 1, 2, 3] + description: which NPE (Network Processing Engine) this ethernet + instance is using + + mdio: + type: object + $ref: "mdio.yaml#" + description: optional node for embedded MDIO controller + +required: + - compatible + - reg + - queue-rx + - queue-txready + +additionalProperties: false + +examples: + - | + ethernet@c8009000 { + compatible = "intel,ixp4xx-ethernet"; + reg = <0xc8009000 0x1000>; + status = "disabled"; + queue-rx = <&qmgr 3>; + queue-txready = <&qmgr 20>; + intel,npe = <1>; + phy-handle = <&phy1>; + + mdio { + #address-cells = <1>; + #size-cells = <0>; + phy1: phy@1 { + #phy-cells = <0>; + reg = <1>; + }; + }; + }; -- 2.29.2 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/3] net: ethernet: ixp4xx: Support device tree probing 2021-04-19 22:51 [PATCH 1/3] net: ethernet: ixp4xx: Add DT bindings Linus Walleij @ 2021-04-19 22:51 ` Linus Walleij 2021-04-20 1:35 ` Andrew Lunn 2021-04-19 22:51 ` [PATCH 3/3] net: ethernet: ixp4xx: Use OF MDIO bus registration Linus Walleij 2021-04-20 1:26 ` [PATCH 1/3] net: ethernet: ixp4xx: Add DT bindings Andrew Lunn 2 siblings, 1 reply; 14+ messages in thread From: Linus Walleij @ 2021-04-19 22:51 UTC (permalink / raw) To: netdev, David S . Miller, Jakub Kicinski Cc: Linus Walleij, Zoltan HERPAI, Raylynn Knight This adds device tree probing to the IXP4xx ethernet driver. Add a platform data bool to tell us whether to register an MDIO bus for the device or not, as well as the corresponding NPE. We need to drop the memory region request as part of this since the OF core will request the memory for the device. Cc: Zoltan HERPAI <wigyori@uid0.hu> Cc: Raylynn Knight <rayknight@me.com> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- ChangeLog v1->v2: - Do not rely on earlier attempt for identifying which ports to use: we split the support clearly in a DT way and a playform data way. - Isolate the DT code better. --- drivers/net/ethernet/xscale/ixp4xx_eth.c | 202 ++++++++++++++++------- include/linux/platform_data/eth_ixp4xx.h | 2 + 2 files changed, 148 insertions(+), 56 deletions(-) diff --git a/drivers/net/ethernet/xscale/ixp4xx_eth.c b/drivers/net/ethernet/xscale/ixp4xx_eth.c index 9defaa21a1a9..758f820068b5 100644 --- a/drivers/net/ethernet/xscale/ixp4xx_eth.c +++ b/drivers/net/ethernet/xscale/ixp4xx_eth.c @@ -28,6 +28,7 @@ #include <linux/kernel.h> #include <linux/net_tstamp.h> #include <linux/of.h> +#include <linux/of_mdio.h> #include <linux/phy.h> #include <linux/platform_data/eth_ixp4xx.h> #include <linux/platform_device.h> @@ -165,7 +166,6 @@ struct eth_regs { }; struct port { - struct resource *mem_res; struct eth_regs __iomem *regs; struct npe *npe; struct net_device *netdev; @@ -1358,19 +1358,132 @@ static const struct net_device_ops ixp4xx_netdev_ops = { .ndo_validate_addr = eth_validate_addr, }; +#ifdef CONFIG_OF +static struct eth_plat_info *ixp4xx_of_get_platdata(struct device *dev) +{ + struct device_node *np = dev->of_node; + struct of_phandle_args queue_spec; + struct eth_plat_info *plat; + struct device_node *phy_np; + struct device_node *mdio_np; + u32 val; + int ret; + + plat = devm_kzalloc(dev, sizeof(*plat), GFP_KERNEL); + if (!plat) + return NULL; + + ret = of_property_read_u32(np, "intel,npe", &val); + if (ret) { + dev_err(dev, "no NPE engine specified\n"); + return NULL; + } + /* NPE ID 0x00, 0x10, 0x20... */ + plat->npe = (val << 4); + + phy_np = of_parse_phandle(np, "phy-handle", 0); + if (phy_np) { + ret = of_property_read_u32(phy_np, "reg", &val); + if (ret) { + dev_err(dev, "cannot find phy reg\n"); + return NULL; + } + of_node_put(phy_np); + } else { + dev_err(dev, "cannot find phy instance\n"); + val = 0; + } + plat->phy = val; + + /* Check if this device has an MDIO bus */ + mdio_np = of_get_child_by_name(np, "mdio"); + if (mdio_np) { + plat->has_mdio = true; + of_node_put(mdio_np); + } + + /* Get the rx queue as a resource from queue manager */ + ret = of_parse_phandle_with_fixed_args(np, "queue-rx", 1, 0, + &queue_spec); + if (ret) { + dev_err(dev, "no rx queue phandle\n"); + return NULL; + } + plat->rxq = queue_spec.args[0]; + + /* Get the txready queue as resource from queue manager */ + ret = of_parse_phandle_with_fixed_args(np, "queue-txready", 1, 0, + &queue_spec); + if (ret) { + dev_err(dev, "no txready queue phandle\n"); + return NULL; + } + plat->txreadyq = queue_spec.args[0]; + + return plat; +} +#else +static struct eth_plat_info *ixp4xx_of_get_platdata(struct device *dev) +{ + return NULL; +} +#endif + static int ixp4xx_eth_probe(struct platform_device *pdev) { char phy_id[MII_BUS_ID_SIZE + 3]; struct phy_device *phydev = NULL; struct device *dev = &pdev->dev; + struct device_node *np = dev->of_node; struct eth_plat_info *plat; - resource_size_t regs_phys; struct net_device *ndev; struct resource *res; struct port *port; int err; - plat = dev_get_platdata(dev); + if (np) { + plat = ixp4xx_of_get_platdata(dev); + if (!plat) + return -ENODEV; + } else { + plat = dev_get_platdata(dev); + if (!plat) + return -ENODEV; + plat->npe = pdev->id; + switch (plat->npe) { + case IXP4XX_ETH_NPEA: + /* If the MDIO bus is not up yet, defer probe */ + break; + case IXP4XX_ETH_NPEB: + /* On all except IXP43x, NPE-B is used for the MDIO bus. + * If there is no NPE-B in the feature set, bail out, + * else we have the MDIO bus here. + */ + if (!cpu_is_ixp43x()) { + if (!(ixp4xx_read_feature_bits() & + IXP4XX_FEATURE_NPEB_ETH0)) + return -ENODEV; + /* Else register the MDIO bus on NPE-B */ + plat->has_mdio = true; + } + break; + case IXP4XX_ETH_NPEC: + /* IXP43x lacks NPE-B and uses NPE-C for the MDIO bus + * access, if there is no NPE-C, no bus, nothing works, + * so bail out. + */ + if (cpu_is_ixp43x()) { + if (!(ixp4xx_read_feature_bits() & + IXP4XX_FEATURE_NPEC_ETH)) + return -ENODEV; + /* Else register the MDIO bus on NPE-B */ + plat->has_mdio = true; + } + break; + default: + return -ENODEV; + } + } if (!(ndev = devm_alloc_etherdev(dev, sizeof(struct port)))) return -ENOMEM; @@ -1378,59 +1491,29 @@ static int ixp4xx_eth_probe(struct platform_device *pdev) SET_NETDEV_DEV(ndev, dev); port = netdev_priv(ndev); port->netdev = ndev; - port->id = pdev->id; + port->id = plat->npe; /* Get the port resource and remap */ res = platform_get_resource(pdev, IORESOURCE_MEM, 0); if (!res) return -ENODEV; - regs_phys = res->start; port->regs = devm_ioremap_resource(dev, res); if (IS_ERR(port->regs)) return PTR_ERR(port->regs); - switch (port->id) { - case IXP4XX_ETH_NPEA: - /* If the MDIO bus is not up yet, defer probe */ - if (!mdio_bus) - return -EPROBE_DEFER; - break; - case IXP4XX_ETH_NPEB: - /* - * On all except IXP43x, NPE-B is used for the MDIO bus. - * If there is no NPE-B in the feature set, bail out, else - * register the MDIO bus. - */ - if (!cpu_is_ixp43x()) { - if (!(ixp4xx_read_feature_bits() & - IXP4XX_FEATURE_NPEB_ETH0)) - return -ENODEV; - /* Else register the MDIO bus on NPE-B */ - if ((err = ixp4xx_mdio_register(port->regs))) - return err; - } - if (!mdio_bus) - return -EPROBE_DEFER; - break; - case IXP4XX_ETH_NPEC: - /* - * IXP43x lacks NPE-B and uses NPE-C for the MDIO bus access, - * of there is no NPE-C, no bus, nothing works, so bail out. - */ - if (cpu_is_ixp43x()) { - if (!(ixp4xx_read_feature_bits() & - IXP4XX_FEATURE_NPEC_ETH)) - return -ENODEV; - /* Else register the MDIO bus on NPE-C */ - if ((err = ixp4xx_mdio_register(port->regs))) - return err; + /* Register the MDIO bus if we have it */ + if (plat->has_mdio) { + err = ixp4xx_mdio_register(port->regs); + if (err) { + dev_err(dev, "failed to register MDIO bus\n"); + return err; } - if (!mdio_bus) - return -EPROBE_DEFER; - break; - default: - return -ENODEV; } + /* If the instance with the MDIO bus has not yet appeared, + * defer probing until it gets probed. + */ + if (!mdio_bus) + return -EPROBE_DEFER; ndev->netdev_ops = &ixp4xx_netdev_ops; ndev->ethtool_ops = &ixp4xx_ethtool_ops; @@ -1444,12 +1527,6 @@ static int ixp4xx_eth_probe(struct platform_device *pdev) if (!(port->npe = npe_request(NPE_ID(port->id)))) return -EIO; - port->mem_res = request_mem_region(regs_phys, REGS_SIZE, ndev->name); - if (!port->mem_res) { - err = -EBUSY; - goto err_npe_rel; - } - port->plat = plat; npe_port_tab[NPE_ID(port->id)] = port; memcpy(ndev->dev_addr, plat->hwaddr, ETH_ALEN); @@ -1463,11 +1540,17 @@ static int ixp4xx_eth_probe(struct platform_device *pdev) udelay(50); snprintf(phy_id, MII_BUS_ID_SIZE + 3, PHY_ID_FMT, - mdio_bus->id, plat->phy); + mdio_bus->id, plat->phy); phydev = phy_connect(ndev, phy_id, &ixp4xx_adjust_link, PHY_INTERFACE_MODE_MII); + if (!phydev) { + err = -ENODEV; + dev_err(dev, "no phydev\n"); + goto err_free_mem; + } if (IS_ERR(phydev)) { err = PTR_ERR(phydev); + dev_err(dev, "could not connect phydev (%d)\n", err); goto err_free_mem; } @@ -1485,8 +1568,6 @@ static int ixp4xx_eth_probe(struct platform_device *pdev) phy_disconnect(phydev); err_free_mem: npe_port_tab[NPE_ID(port->id)] = NULL; - release_resource(port->mem_res); -err_npe_rel: npe_release(port->npe); return err; } @@ -1502,12 +1583,21 @@ static int ixp4xx_eth_remove(struct platform_device *pdev) ixp4xx_mdio_remove(); npe_port_tab[NPE_ID(port->id)] = NULL; npe_release(port->npe); - release_resource(port->mem_res); return 0; } +static const struct of_device_id ixp4xx_eth_of_match[] = { + { + .compatible = "intel,ixp4xx-ethernet", + }, + { }, +}; + static struct platform_driver ixp4xx_eth_driver = { - .driver.name = DRV_NAME, + .driver = { + .name = DRV_NAME, + .of_match_table = of_match_ptr(ixp4xx_eth_of_match), + }, .probe = ixp4xx_eth_probe, .remove = ixp4xx_eth_remove, }; diff --git a/include/linux/platform_data/eth_ixp4xx.h b/include/linux/platform_data/eth_ixp4xx.h index 6f652ea0c6ae..114b0940729f 100644 --- a/include/linux/platform_data/eth_ixp4xx.h +++ b/include/linux/platform_data/eth_ixp4xx.h @@ -14,6 +14,8 @@ struct eth_plat_info { u8 rxq; /* configurable, currently 0 - 31 only */ u8 txreadyq; u8 hwaddr[6]; + u8 npe; /* NPE instance used by this interface */ + bool has_mdio; /* If this instance has an MDIO bus */ }; #endif -- 2.29.2 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] net: ethernet: ixp4xx: Support device tree probing 2021-04-19 22:51 ` [PATCH 2/3] net: ethernet: ixp4xx: Support device tree probing Linus Walleij @ 2021-04-20 1:35 ` Andrew Lunn 2021-04-20 8:38 ` Linus Walleij 0 siblings, 1 reply; 14+ messages in thread From: Andrew Lunn @ 2021-04-20 1:35 UTC (permalink / raw) To: Linus Walleij Cc: netdev, David S . Miller, Jakub Kicinski, Zoltan HERPAI, Raylynn Knight > + phy_np = of_parse_phandle(np, "phy-handle", 0); > + if (phy_np) { > + ret = of_property_read_u32(phy_np, "reg", &val); > + if (ret) { > + dev_err(dev, "cannot find phy reg\n"); > + return NULL; > + } > + of_node_put(phy_np); > + } else { > + dev_err(dev, "cannot find phy instance\n"); > + val = 0; > + } > + plat->phy = val; phy-handle can point to a PHY on any bus. You should not assume it is the devices own bus. Once you have phy_np call of_phy_find_device() which gives you the actual phy device. Please don't let the limitations of the current platform data limit you from implementing this correctly. Andrew ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] net: ethernet: ixp4xx: Support device tree probing 2021-04-20 1:35 ` Andrew Lunn @ 2021-04-20 8:38 ` Linus Walleij 0 siblings, 0 replies; 14+ messages in thread From: Linus Walleij @ 2021-04-20 8:38 UTC (permalink / raw) To: Andrew Lunn Cc: netdev, David S . Miller, Jakub Kicinski, Zoltan HERPAI, Raylynn Knight On Tue, Apr 20, 2021 at 3:35 AM Andrew Lunn <andrew@lunn.ch> wrote: > > + phy_np = of_parse_phandle(np, "phy-handle", 0); > > + if (phy_np) { > > + ret = of_property_read_u32(phy_np, "reg", &val); > > + if (ret) { > > + dev_err(dev, "cannot find phy reg\n"); > > + return NULL; > > + } > > + of_node_put(phy_np); > > + } else { > > + dev_err(dev, "cannot find phy instance\n"); > > + val = 0; > > + } > > + plat->phy = val; > > phy-handle can point to a PHY on any bus. You should not assume it is > the devices own bus. Once you have phy_np call of_phy_find_device() > which gives you the actual phy device. Please don't let the > limitations of the current platform data limit you from implementing > this correctly. In patch 3/3 I migrate to of_phy_get_and_connect() which I assume fixes this, I just wanted to split up the work, but do you prefer that I simply squash patches 2 & 3 into one? Yours, Linus Walleij ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/3] net: ethernet: ixp4xx: Use OF MDIO bus registration 2021-04-19 22:51 [PATCH 1/3] net: ethernet: ixp4xx: Add DT bindings Linus Walleij 2021-04-19 22:51 ` [PATCH 2/3] net: ethernet: ixp4xx: Support device tree probing Linus Walleij @ 2021-04-19 22:51 ` Linus Walleij 2021-04-19 22:54 ` Linus Walleij 2021-04-20 1:47 ` Andrew Lunn 2021-04-20 1:26 ` [PATCH 1/3] net: ethernet: ixp4xx: Add DT bindings Andrew Lunn 2 siblings, 2 replies; 14+ messages in thread From: Linus Walleij @ 2021-04-19 22:51 UTC (permalink / raw) To: netdev, David S . Miller, Jakub Kicinski Cc: Linus Walleij, Zoltan HERPAI, Raylynn Knight This augments the IXP4xx to use the OF MDIO bus code when registering the MDIO bus and when looking up the PHY for the ethernet network device. Cc: Zoltan HERPAI <wigyori@uid0.hu> Cc: Raylynn Knight <rayknight@me.com> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- ChangeLog v1->v2: - New patch making use of more OF infrastructure. --- drivers/net/ethernet/xscale/Kconfig | 1 + drivers/net/ethernet/xscale/ixp4xx_eth.c | 34 +++++++++--------------- 2 files changed, 14 insertions(+), 21 deletions(-) diff --git a/drivers/net/ethernet/xscale/Kconfig b/drivers/net/ethernet/xscale/Kconfig index 7b83a6e5d894..468ffe3d1707 100644 --- a/drivers/net/ethernet/xscale/Kconfig +++ b/drivers/net/ethernet/xscale/Kconfig @@ -22,6 +22,7 @@ config IXP4XX_ETH tristate "Intel IXP4xx Ethernet support" depends on ARM && ARCH_IXP4XX && IXP4XX_NPE && IXP4XX_QMGR select PHYLIB + select OF_MDIO if OF select NET_PTP_CLASSIFY help Say Y here if you want to use built-in Ethernet ports diff --git a/drivers/net/ethernet/xscale/ixp4xx_eth.c b/drivers/net/ethernet/xscale/ixp4xx_eth.c index 758f820068b5..1e1779b53f22 100644 --- a/drivers/net/ethernet/xscale/ixp4xx_eth.c +++ b/drivers/net/ethernet/xscale/ixp4xx_eth.c @@ -250,6 +250,7 @@ static inline void memcpy_swab32(u32 *dest, u32 *src, int cnt) static DEFINE_SPINLOCK(mdio_lock); static struct eth_regs __iomem *mdio_regs; /* mdio command and status only */ static struct mii_bus *mdio_bus; +static struct device_node *mdio_bus_np; static int ports_open; static struct port *npe_port_tab[MAX_NPES]; static struct dma_pool *dma_pool; @@ -533,7 +534,8 @@ static int ixp4xx_mdio_register(struct eth_regs __iomem *regs) mdio_bus->write = &ixp4xx_mdio_write; snprintf(mdio_bus->id, MII_BUS_ID_SIZE, "ixp4xx-eth-0"); - if ((err = mdiobus_register(mdio_bus))) + err = of_mdiobus_register(mdio_bus, mdio_bus_np); + if (err) mdiobus_free(mdio_bus); return err; } @@ -1364,7 +1366,6 @@ static struct eth_plat_info *ixp4xx_of_get_platdata(struct device *dev) struct device_node *np = dev->of_node; struct of_phandle_args queue_spec; struct eth_plat_info *plat; - struct device_node *phy_np; struct device_node *mdio_np; u32 val; int ret; @@ -1381,25 +1382,12 @@ static struct eth_plat_info *ixp4xx_of_get_platdata(struct device *dev) /* NPE ID 0x00, 0x10, 0x20... */ plat->npe = (val << 4); - phy_np = of_parse_phandle(np, "phy-handle", 0); - if (phy_np) { - ret = of_property_read_u32(phy_np, "reg", &val); - if (ret) { - dev_err(dev, "cannot find phy reg\n"); - return NULL; - } - of_node_put(phy_np); - } else { - dev_err(dev, "cannot find phy instance\n"); - val = 0; - } - plat->phy = val; - /* Check if this device has an MDIO bus */ mdio_np = of_get_child_by_name(np, "mdio"); if (mdio_np) { plat->has_mdio = true; - of_node_put(mdio_np); + mdio_bus_np = mdio_np; + /* DO NOT put the mdio_np, it will be used */ } /* Get the rx queue as a resource from queue manager */ @@ -1539,10 +1527,14 @@ static int ixp4xx_eth_probe(struct platform_device *pdev) __raw_writel(DEFAULT_CORE_CNTRL, &port->regs->core_control); udelay(50); - snprintf(phy_id, MII_BUS_ID_SIZE + 3, PHY_ID_FMT, - mdio_bus->id, plat->phy); - phydev = phy_connect(ndev, phy_id, &ixp4xx_adjust_link, - PHY_INTERFACE_MODE_MII); + if (np) { + phydev = of_phy_get_and_connect(ndev, np, ixp4xx_adjust_link); + } else { + snprintf(phy_id, MII_BUS_ID_SIZE + 3, PHY_ID_FMT, + mdio_bus->id, plat->phy); + phydev = phy_connect(ndev, phy_id, &ixp4xx_adjust_link, + PHY_INTERFACE_MODE_MII); + } if (!phydev) { err = -ENODEV; dev_err(dev, "no phydev\n"); -- 2.29.2 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] net: ethernet: ixp4xx: Use OF MDIO bus registration 2021-04-19 22:51 ` [PATCH 3/3] net: ethernet: ixp4xx: Use OF MDIO bus registration Linus Walleij @ 2021-04-19 22:54 ` Linus Walleij 2021-04-20 1:47 ` Andrew Lunn 1 sibling, 0 replies; 14+ messages in thread From: Linus Walleij @ 2021-04-19 22:54 UTC (permalink / raw) To: netdev, David S . Miller, Jakub Kicinski Cc: Zoltan HERPAI, Raylynn Knight, Andrew Lunn, Heiner Kallweit, Russell King On Tue, Apr 20, 2021 at 12:51 AM Linus Walleij <linus.walleij@linaro.org> wrote: > This augments the IXP4xx to use the OF MDIO bus code when > registering the MDIO bus and when looking up the PHY > for the ethernet network device. > > Cc: Zoltan HERPAI <wigyori@uid0.hu> > Cc: Raylynn Knight <rayknight@me.com> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > ChangeLog v1->v2: > - New patch making use of more OF infrastructure. Realized after sending that the MDIO people wanna look at this too so adding them to CC, tell me if you want me to resend the patch. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] net: ethernet: ixp4xx: Use OF MDIO bus registration 2021-04-19 22:51 ` [PATCH 3/3] net: ethernet: ixp4xx: Use OF MDIO bus registration Linus Walleij 2021-04-19 22:54 ` Linus Walleij @ 2021-04-20 1:47 ` Andrew Lunn 2021-04-20 8:44 ` Linus Walleij 1 sibling, 1 reply; 14+ messages in thread From: Andrew Lunn @ 2021-04-20 1:47 UTC (permalink / raw) To: Linus Walleij Cc: netdev, David S . Miller, Jakub Kicinski, Zoltan HERPAI, Raylynn Knight > @@ -1381,25 +1382,12 @@ static struct eth_plat_info *ixp4xx_of_get_platdata(struct device *dev) > /* NPE ID 0x00, 0x10, 0x20... */ > plat->npe = (val << 4); > > - phy_np = of_parse_phandle(np, "phy-handle", 0); > - if (phy_np) { > - ret = of_property_read_u32(phy_np, "reg", &val); > - if (ret) { > - dev_err(dev, "cannot find phy reg\n"); > - return NULL; > - } > - of_node_put(phy_np); > - } else { > - dev_err(dev, "cannot find phy instance\n"); > - val = 0; > - } > - plat->phy = val; > - Isn't this code you just added in the previous patch? > /* Check if this device has an MDIO bus */ > mdio_np = of_get_child_by_name(np, "mdio"); > if (mdio_np) { > plat->has_mdio = true; > - of_node_put(mdio_np); > + mdio_bus_np = mdio_np; > + /* DO NOT put the mdio_np, it will be used */ > } > > /* Get the rx queue as a resource from queue manager */ > @@ -1539,10 +1527,14 @@ static int ixp4xx_eth_probe(struct platform_device *pdev) > __raw_writel(DEFAULT_CORE_CNTRL, &port->regs->core_control); > udelay(50); > > - snprintf(phy_id, MII_BUS_ID_SIZE + 3, PHY_ID_FMT, > - mdio_bus->id, plat->phy); > - phydev = phy_connect(ndev, phy_id, &ixp4xx_adjust_link, > - PHY_INTERFACE_MODE_MII); > + if (np) { > + phydev = of_phy_get_and_connect(ndev, np, ixp4xx_adjust_link); > + } else { > + snprintf(phy_id, MII_BUS_ID_SIZE + 3, PHY_ID_FMT, > + mdio_bus->id, plat->phy); mdiobus_get_phy() and phy_connect_direct() might be better. Andrew ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] net: ethernet: ixp4xx: Use OF MDIO bus registration 2021-04-20 1:47 ` Andrew Lunn @ 2021-04-20 8:44 ` Linus Walleij 2021-04-20 12:53 ` Andrew Lunn 0 siblings, 1 reply; 14+ messages in thread From: Linus Walleij @ 2021-04-20 8:44 UTC (permalink / raw) To: Andrew Lunn Cc: netdev, David S . Miller, Jakub Kicinski, Zoltan HERPAI, Raylynn Knight On Tue, Apr 20, 2021 at 3:47 AM Andrew Lunn <andrew@lunn.ch> wrote: > > @@ -1381,25 +1382,12 @@ static struct eth_plat_info *ixp4xx_of_get_platdata(struct device *dev) > > /* NPE ID 0x00, 0x10, 0x20... */ > > plat->npe = (val << 4); > > > > - phy_np = of_parse_phandle(np, "phy-handle", 0); > > - if (phy_np) { > > - ret = of_property_read_u32(phy_np, "reg", &val); > > - if (ret) { > > - dev_err(dev, "cannot find phy reg\n"); > > - return NULL; > > - } > > - of_node_put(phy_np); > > - } else { > > - dev_err(dev, "cannot find phy instance\n"); > > - val = 0; > > - } > > - plat->phy = val; > > - > > Isn't this code you just added in the previous patch? Yep. It's by the token of "one technical step per patch" but I suggested that maybe you prefer to take two technical steps in one big patch, then we can just squash patches 2 & 3. I'll fix it as you want it just tell me how :) > > - snprintf(phy_id, MII_BUS_ID_SIZE + 3, PHY_ID_FMT, > > - mdio_bus->id, plat->phy); > > - phydev = phy_connect(ndev, phy_id, &ixp4xx_adjust_link, > > - PHY_INTERFACE_MODE_MII); > > + if (np) { > > + phydev = of_phy_get_and_connect(ndev, np, ixp4xx_adjust_link); > > + } else { > > + snprintf(phy_id, MII_BUS_ID_SIZE + 3, PHY_ID_FMT, > > + mdio_bus->id, plat->phy); > > mdiobus_get_phy() and phy_connect_direct() might be better. Do you mean for the legacy code path (else clause), or the new code path with of_phy_get_and_connect() or both? I tried not to change the legacy code in order to not introduce regressions, so if I change that I suppose it should be a separate patch. On the other hand this driver has not been much maintained the recent years so we might need to be a bit rough when bringing it into shape. (After migrating all of IXP4xx to device tree a lot of the legacy code will eventually be deleted.) Yours, Linus Walleij ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] net: ethernet: ixp4xx: Use OF MDIO bus registration 2021-04-20 8:44 ` Linus Walleij @ 2021-04-20 12:53 ` Andrew Lunn 0 siblings, 0 replies; 14+ messages in thread From: Andrew Lunn @ 2021-04-20 12:53 UTC (permalink / raw) To: Linus Walleij Cc: netdev, David S . Miller, Jakub Kicinski, Zoltan HERPAI, Raylynn Knight On Tue, Apr 20, 2021 at 10:44:16AM +0200, Linus Walleij wrote: > On Tue, Apr 20, 2021 at 3:47 AM Andrew Lunn <andrew@lunn.ch> wrote: > > > > @@ -1381,25 +1382,12 @@ static struct eth_plat_info *ixp4xx_of_get_platdata(struct device *dev) > > > /* NPE ID 0x00, 0x10, 0x20... */ > > > plat->npe = (val << 4); > > > > > > - phy_np = of_parse_phandle(np, "phy-handle", 0); > > > - if (phy_np) { > > > - ret = of_property_read_u32(phy_np, "reg", &val); > > > - if (ret) { > > > - dev_err(dev, "cannot find phy reg\n"); > > > - return NULL; > > > - } > > > - of_node_put(phy_np); > > > - } else { > > > - dev_err(dev, "cannot find phy instance\n"); > > > - val = 0; > > > - } > > > - plat->phy = val; > > > - > > > > Isn't this code you just added in the previous patch? > > Yep. It's by the token of "one technical step per patch" I don't actually seeing it being a step, since it is actually broken and of_phy_get_and_connect() does pretty much everything it should do, which is what you replace it with. It is a long time since i converted a platform_data driver to DT. But i remember trying to fill in the platform_data structure from DT was often the wrong way to do it. They contain different data, and you cannot easily map one to the other. So you need to make bigger changes to the probe function. You have two intermingled code paths, one dealing with platform_data, and the other using DT. I've not looked in detail, but i guess my first step would be, cleanly register the MDIO bus. Second step would be to register the PHY. And it might need some refactoring patches just to make it easier to understand. > > > - snprintf(phy_id, MII_BUS_ID_SIZE + 3, PHY_ID_FMT, > > > - mdio_bus->id, plat->phy); > > > - phydev = phy_connect(ndev, phy_id, &ixp4xx_adjust_link, > > > - PHY_INTERFACE_MODE_MII); > > > + if (np) { > > > + phydev = of_phy_get_and_connect(ndev, np, ixp4xx_adjust_link); > > > + } else { > > > + snprintf(phy_id, MII_BUS_ID_SIZE + 3, PHY_ID_FMT, > > > + mdio_bus->id, plat->phy); > > > > mdiobus_get_phy() and phy_connect_direct() might be better. > > Do you mean for the legacy code path (else clause), or the > new code path with of_phy_get_and_connect() or both? > > I tried not to change the legacy code in order to not introduce > regressions, so if I change that I suppose it should be a > separate patch. Yes, the legacy code. You don't often see this string parsing method. And since you have the bus, and the index, you can directly go to the PHY avoiding it all. A separate patch would be better. Andrew ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] net: ethernet: ixp4xx: Add DT bindings 2021-04-19 22:51 [PATCH 1/3] net: ethernet: ixp4xx: Add DT bindings Linus Walleij 2021-04-19 22:51 ` [PATCH 2/3] net: ethernet: ixp4xx: Support device tree probing Linus Walleij 2021-04-19 22:51 ` [PATCH 3/3] net: ethernet: ixp4xx: Use OF MDIO bus registration Linus Walleij @ 2021-04-20 1:26 ` Andrew Lunn 2021-04-22 15:39 ` Linus Walleij 2 siblings, 1 reply; 14+ messages in thread From: Andrew Lunn @ 2021-04-20 1:26 UTC (permalink / raw) To: Linus Walleij Cc: netdev, David S . Miller, Jakub Kicinski, Zoltan HERPAI, Raylynn Knight, devicetree > + mdio { > + #address-cells = <1>; > + #size-cells = <0>; > + phy1: phy@1 { > + #phy-cells = <0>; Hi Linus phy-cells is not part of the Ethernet PHY binding. Andrew ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] net: ethernet: ixp4xx: Add DT bindings 2021-04-20 1:26 ` [PATCH 1/3] net: ethernet: ixp4xx: Add DT bindings Andrew Lunn @ 2021-04-22 15:39 ` Linus Walleij 2021-04-22 15:56 ` Andrew Lunn 0 siblings, 1 reply; 14+ messages in thread From: Linus Walleij @ 2021-04-22 15:39 UTC (permalink / raw) To: Andrew Lunn, Rob Herring Cc: netdev, David S . Miller, Jakub Kicinski, Zoltan HERPAI, Raylynn Knight, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS On Tue, Apr 20, 2021 at 3:26 AM Andrew Lunn <andrew@lunn.ch> wrote: > > + mdio { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + phy1: phy@1 { > > + #phy-cells = <0>; > > Hi Linus > > phy-cells is not part of the Ethernet PHY binding. Nevertheless: CHECK Documentation/devicetree/bindings/net/intel,ixp4xx-ethernet.example.dt.yaml /var/linus/linux-nomadik/build-ixp4/Documentation/devicetree/bindings/net/intel,ixp4xx-ethernet.example.dt.yaml: phy@1: '#phy-cells' is a required property From schema: /home/linus/.local/lib/python3.9/site-packages/dtschema/schemas/phy/phy-provider.yaml It has been hardcoded as required into the dtschema python package. Looks like this: properties: $nodename: pattern: "^(|usb-|usb2-|usb3-|pci-|pcie-|sata-)phy(@[0-9a-f,]+)*$" "#phy-cells": true phy-supply: true required: - "#phy-cells" additionalProperties: true If this is wrong I bet Rob needs to hear about it. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] net: ethernet: ixp4xx: Add DT bindings 2021-04-22 15:39 ` Linus Walleij @ 2021-04-22 15:56 ` Andrew Lunn 2021-04-22 17:33 ` Linus Walleij 0 siblings, 1 reply; 14+ messages in thread From: Andrew Lunn @ 2021-04-22 15:56 UTC (permalink / raw) To: Linus Walleij Cc: Rob Herring, netdev, David S . Miller, Jakub Kicinski, Zoltan HERPAI, Raylynn Knight, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS On Thu, Apr 22, 2021 at 05:39:07PM +0200, Linus Walleij wrote: > On Tue, Apr 20, 2021 at 3:26 AM Andrew Lunn <andrew@lunn.ch> wrote: > > > > + mdio { > > > + #address-cells = <1>; > > > + #size-cells = <0>; > > > + phy1: phy@1 { > > > + #phy-cells = <0>; > > > > Hi Linus > > > > phy-cells is not part of the Ethernet PHY binding. > > Nevertheless: > > CHECK Documentation/devicetree/bindings/net/intel,ixp4xx-ethernet.example.dt.yaml > /var/linus/linux-nomadik/build-ixp4/Documentation/devicetree/bindings/net/intel,ixp4xx-ethernet.example.dt.yaml: > phy@1: '#phy-cells' is a required property > From schema: > /home/linus/.local/lib/python3.9/site-packages/dtschema/schemas/phy/phy-provider.yaml > > It has been hardcoded as required into the dtschema python package. > Looks like this: > > properties: > $nodename: > pattern: "^(|usb-|usb2-|usb3-|pci-|pcie-|sata-)phy(@[0-9a-f,]+)*$" > > "#phy-cells": true > > phy-supply: true > > required: > - "#phy-cells" > > additionalProperties: true > > If this is wrong I bet Rob needs to hear about it. That is the wrong sort of PHY. That is a generic PHY, not a PHY, aka Ethernet PHY. Maybe you need to change the label to ethernet-phy ? Andrew ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] net: ethernet: ixp4xx: Add DT bindings 2021-04-22 15:56 ` Andrew Lunn @ 2021-04-22 17:33 ` Linus Walleij 2021-04-22 20:13 ` Andrew Lunn 0 siblings, 1 reply; 14+ messages in thread From: Linus Walleij @ 2021-04-22 17:33 UTC (permalink / raw) To: Andrew Lunn Cc: Rob Herring, netdev, David S . Miller, Jakub Kicinski, Zoltan HERPAI, Raylynn Knight, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS On Thu, Apr 22, 2021 at 5:56 PM Andrew Lunn <andrew@lunn.ch> wrote: > On Thu, Apr 22, 2021 at 05:39:07PM +0200, Linus Walleij wrote: > > On Tue, Apr 20, 2021 at 3:26 AM Andrew Lunn <andrew@lunn.ch> wrote: > > > > > > + mdio { > > > > + #address-cells = <1>; > > > > + #size-cells = <0>; > > > > + phy1: phy@1 { > > > > + #phy-cells = <0>; > > > > > > Hi Linus > > > > > > phy-cells is not part of the Ethernet PHY binding. > > > > Nevertheless: > > > > CHECK Documentation/devicetree/bindings/net/intel,ixp4xx-ethernet.example.dt.yaml > > /var/linus/linux-nomadik/build-ixp4/Documentation/devicetree/bindings/net/intel,ixp4xx-ethernet.example.dt.yaml: > > phy@1: '#phy-cells' is a required property > > From schema: > > /home/linus/.local/lib/python3.9/site-packages/dtschema/schemas/phy/phy-provider.yaml > > > > It has been hardcoded as required into the dtschema python package. > > Looks like this: > > > > properties: > > $nodename: > > pattern: "^(|usb-|usb2-|usb3-|pci-|pcie-|sata-)phy(@[0-9a-f,]+)*$" > > > > "#phy-cells": true > > > > phy-supply: true > > > > required: > > - "#phy-cells" > > > > additionalProperties: true > > > > If this is wrong I bet Rob needs to hear about it. > > That is the wrong sort of PHY. That is a generic PHY, not a PHY, aka > Ethernet PHY. It is a bit confusing :D not to mention that the term "phy" or "physical interface" as I suppose it is meant to be understood is a bit ambiguous to begin with. > Maybe you need to change the label to ethernet-phy ? Yeah that works, I'll do that. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] net: ethernet: ixp4xx: Add DT bindings 2021-04-22 17:33 ` Linus Walleij @ 2021-04-22 20:13 ` Andrew Lunn 0 siblings, 0 replies; 14+ messages in thread From: Andrew Lunn @ 2021-04-22 20:13 UTC (permalink / raw) To: Linus Walleij Cc: Rob Herring, netdev, David S . Miller, Jakub Kicinski, Zoltan HERPAI, Raylynn Knight, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS > It is a bit confusing :D not to mention that the term > "phy" or "physical interface" as I suppose it is meant to > be understood is a bit ambiguous to begin with. Yes. For a long time, phy meant Ethernet PHY. And then the generic PHY framework came along, and decided to reuse the phy prefix, rather than genphy, or something unique. And now we have confusion. Andrew ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2021-04-22 20:13 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-04-19 22:51 [PATCH 1/3] net: ethernet: ixp4xx: Add DT bindings Linus Walleij 2021-04-19 22:51 ` [PATCH 2/3] net: ethernet: ixp4xx: Support device tree probing Linus Walleij 2021-04-20 1:35 ` Andrew Lunn 2021-04-20 8:38 ` Linus Walleij 2021-04-19 22:51 ` [PATCH 3/3] net: ethernet: ixp4xx: Use OF MDIO bus registration Linus Walleij 2021-04-19 22:54 ` Linus Walleij 2021-04-20 1:47 ` Andrew Lunn 2021-04-20 8:44 ` Linus Walleij 2021-04-20 12:53 ` Andrew Lunn 2021-04-20 1:26 ` [PATCH 1/3] net: ethernet: ixp4xx: Add DT bindings Andrew Lunn 2021-04-22 15:39 ` Linus Walleij 2021-04-22 15:56 ` Andrew Lunn 2021-04-22 17:33 ` Linus Walleij 2021-04-22 20:13 ` Andrew Lunn
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.