[PATCH net-next 0/3] nixge: fixed-link support

[PATCH net-next 0/3] nixge: fixed-link support

[Moritz Fischer]

Hi,

this series adds support for using nixge with fixed-link nodes,
as well as an early attempt to support nixge as a subdevice of
another device.

This series goes on top of: https://lkml.org/lkml/2018/8/28/1011

Patch 1: Adds of based fixed-link support, and hopefully isn't too
         controversial barring grave mistakes.

Patch 2: Is an attempt at making sure that nixge works as a subdevice
         of another (pci) device without a PHY. Note that same as Patch
         3 the actual platform data might still change since the parent
         device driver is still under development. So this is more of an
         RFC, too.

Patch 3: Is more of an RFC at this point since the PCI parent device is
         still under development, but required to run with IOMMU
         support. Feedback on this one would be appreciated.

Thanks for your input,

Moritz

Moritz Fischer (3):
  net: nixge: Add support for fixed-link subnodes
  net: nixge: Add support for having nixge as subdevice
  net: nixge: Use sysdev instead of ndev->dev.parent for DMA

 drivers/net/ethernet/ni/nixge.c     | 187 ++++++++++++++++++++++------
 include/linux/platform_data/nixge.h |  19 +++
 2 files changed, 165 insertions(+), 41 deletions(-)
 create mode 100644 include/linux/platform_data/nixge.h

-- 
2.18.0

[PATCH net-next 1/3] net: nixge: Add support for fixed-link subnodes

[Moritz Fischer]

Add support for fixed-link cases where no MDIO is
actually required to run the device.
In that case no MDIO bus is instantiated since the
actual registers are not available in hardware.

Signed-off-by: Moritz Fischer <mdf@kernel.org>
---
 drivers/net/ethernet/ni/nixge.c | 72 ++++++++++++++++++++++++---------
 1 file changed, 53 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/ni/nixge.c b/drivers/net/ethernet/ni/nixge.c
index 74cf52e3fb09..670249313ff0 100644
--- a/drivers/net/ethernet/ni/nixge.c
+++ b/drivers/net/ethernet/ni/nixge.c
@@ -1189,9 +1189,36 @@ static int nixge_mdio_write(struct mii_bus *bus, int phy_id, int reg, u16 val)
 	return err;
 }
 
+static int nixge_of_get_phy(struct nixge_priv *priv, struct device_node *np)
+{
+	priv->phy_mode = of_get_phy_mode(np);
+	if (priv->phy_mode < 0) {
+		dev_err(priv->dev, "not find \"phy-mode\" property\n");
+		return -EINVAL;
+	}
+
+	if (of_phy_is_fixed_link(np)) {
+		if (of_phy_register_fixed_link(np) < 0) {
+			dev_err(priv->dev, "broken fixed link spec\n");
+			return -EINVAL;
+		}
+
+		priv->phy_node = of_node_get(np);
+	} else {
+		priv->phy_node = of_parse_phandle(np, "phy-handle", 0);
+		if (!priv->phy_node) {
+			dev_err(priv->dev, "not find \"phy-handle\" property\n");
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
 static int nixge_mdio_setup(struct nixge_priv *priv, struct device_node *np)
 {
 	struct mii_bus *bus;
+	int err;
 
 	bus = devm_mdiobus_alloc(priv->dev);
 	if (!bus)
@@ -1230,6 +1257,7 @@ static int nixge_probe(struct platform_device *pdev)
 	struct nixge_priv *priv;
 	struct net_device *ndev;
 	struct resource *dmares;
+	struct device_node *np;
 	const u8 *mac_addr;
 	int err;
 
@@ -1237,6 +1265,8 @@ static int nixge_probe(struct platform_device *pdev)
 	if (!ndev)
 		return -ENOMEM;
 
+	np = pdev->dev.of_node;
+
 	platform_set_drvdata(pdev, ndev);
 	SET_NETDEV_DEV(ndev, &pdev->dev);
 
@@ -1286,24 +1316,19 @@ static int nixge_probe(struct platform_device *pdev)
 	priv->coalesce_count_rx = XAXIDMA_DFT_RX_THRESHOLD;
 	priv->coalesce_count_tx = XAXIDMA_DFT_TX_THRESHOLD;
 
-	err = nixge_mdio_setup(priv, pdev->dev.of_node);
-	if (err) {
-		netdev_err(ndev, "error registering mdio bus");
-		goto free_netdev;
-	}
-
-	priv->phy_mode = of_get_phy_mode(pdev->dev.of_node);
-	if (priv->phy_mode < 0) {
-		netdev_err(ndev, "not find \"phy-mode\" property\n");
-		err = -EINVAL;
-		goto unregister_mdio;
+	if (np) {
+		err = nixge_of_get_phy(priv, np);
+		if (err)
+			goto free_netdev;
 	}
 
-	priv->phy_node = of_parse_phandle(pdev->dev.of_node, "phy-handle", 0);
-	if (!priv->phy_node) {
-		netdev_err(ndev, "not find \"phy-handle\" property\n");
-		err = -EINVAL;
-		goto unregister_mdio;
+	/* only if it's not a fixed link, do we care about MDIO at all */
+	if (priv->phy_node && !of_phy_is_fixed_link(np)) {
+		err = nixge_mdio_setup(priv, np);
+		if (err) {
+			dev_err(&pdev->dev, "error registering mdio bus");
+			goto free_phy;
+		}
 	}
 
 	err = register_netdev(priv->ndev);
@@ -1315,8 +1340,13 @@ static int nixge_probe(struct platform_device *pdev)
 	return 0;
 
 unregister_mdio:
-	mdiobus_unregister(priv->mii_bus);
-
+	if (priv->mii_bus)
+		mdiobus_unregister(priv->mii_bus);
+free_phy:
+	if (np && of_phy_is_fixed_link(np)) {
+		of_phy_deregister_fixed_link(np);
+		of_node_put(np);
+	}
 free_netdev:
 	free_netdev(ndev);
 
@@ -1330,7 +1360,11 @@ static int nixge_remove(struct platform_device *pdev)
 
 	unregister_netdev(ndev);
 
-	mdiobus_unregister(priv->mii_bus);
+	if (priv->mii_bus)
+		mdiobus_unregister(priv->mii_bus);
+
+	if (np && of_phy_is_fixed_link(np))
+		of_phy_deregister_fixed_link(np);
 
 	free_netdev(ndev);
 
-- 
2.18.0

[PATCH net-next 2/3] net: nixge: Add support for having nixge as subdevice

[Moritz Fischer]

Add support for instantiating nixge as subdevice using
fixed-link and platform data to configure it.

Signed-off-by: Moritz Fischer <mdf@kernel.org>
---

Hi,

not this patch is still in the early stages,
and as the rest of this series goes on top of [1].

The actual platform data might still change since
the parent device driver is still under development.

Thanks for your time,

Moritz


[1] https://lkml.org/lkml/2018/8/28/1011

---
 drivers/net/ethernet/ni/nixge.c     | 71 ++++++++++++++++++++++++++---
 include/linux/platform_data/nixge.h | 19 ++++++++
 2 files changed, 83 insertions(+), 7 deletions(-)
 create mode 100644 include/linux/platform_data/nixge.h

diff --git a/drivers/net/ethernet/ni/nixge.c b/drivers/net/ethernet/ni/nixge.c
index 670249313ff0..fd8e5b02c459 100644
--- a/drivers/net/ethernet/ni/nixge.c
+++ b/drivers/net/ethernet/ni/nixge.c
@@ -7,6 +7,8 @@
 #include <linux/etherdevice.h>
 #include <linux/module.h>
 #include <linux/netdevice.h>
+#include <linux/phy_fixed.h>
+#include <linux/platform_data/nixge.h>
 #include <linux/of_address.h>
 #include <linux/of_mdio.h>
 #include <linux/of_net.h>
@@ -167,6 +169,7 @@ struct nixge_priv {
 	/* Connection to PHY device */
 	struct device_node *phy_node;
 	phy_interface_t		phy_mode;
+	struct phy_device *phydev;
 
 	int link;
 	unsigned int speed;
@@ -859,15 +862,25 @@ static void nixge_dma_err_handler(unsigned long data)
 static int nixge_open(struct net_device *ndev)
 {
 	struct nixge_priv *priv = netdev_priv(ndev);
-	struct phy_device *phy;
+	struct phy_device *phy = NULL;
 	int ret;
 
 	nixge_device_reset(ndev);
 
-	phy = of_phy_connect(ndev, priv->phy_node,
-			     &nixge_handle_link_change, 0, priv->phy_mode);
-	if (!phy)
-		return -ENODEV;
+	if (priv->dev->of_node) {
+		phy = of_phy_connect(ndev, priv->phy_node,
+				     &nixge_handle_link_change, 0,
+				     priv->phy_mode);
+		if (!phy)
+			return -ENODEV;
+	} else if (priv->phydev) {
+		ret = phy_connect_direct(ndev, priv->phydev,
+					 &nixge_handle_link_change,
+					 priv->phy_mode);
+		if (ret)
+			return ret;
+		phy = priv->phydev;
+	}
 
 	phy_start(phy);
 
@@ -1215,10 +1228,41 @@ static int nixge_of_get_phy(struct nixge_priv *priv, struct device_node *np)
 	return 0;
 }
 
+static int nixge_pdata_get_phy(struct nixge_priv *priv,
+			       struct nixge_platform_data *pdata)
+{
+	struct phy_device *phy = NULL;
+
+	if (!pdata)
+		return -EINVAL;
+
+	if (pdata && pdata->phy_interface == PHY_INTERFACE_MODE_NA) {
+		struct fixed_phy_status fphy_status = {
+			.link = 1,
+			.duplex = pdata->phy_duplex,
+			.speed = pdata->phy_speed,
+			.pause = 0,
+			.asym_pause = 0,
+		};
+
+		/* TODO: Pull out GPIO from pdata */
+		phy = fixed_phy_register(PHY_POLL, &fphy_status, -1,
+					 NULL);
+		if (IS_ERR_OR_NULL(phy)) {
+			dev_err(priv->dev,
+				"failed to register fixed PHY device\n");
+			return -ENODEV;
+		}
+	}
+	priv->phy_mode = pdata->phy_interface;
+	priv->phydev = phy;
+
+	return 0;
+}
+
 static int nixge_mdio_setup(struct nixge_priv *priv, struct device_node *np)
 {
 	struct mii_bus *bus;
-	int err;
 
 	bus = devm_mdiobus_alloc(priv->dev);
 	if (!bus)
@@ -1254,6 +1298,7 @@ static void *nixge_get_nvmem_address(struct device *dev)
 
 static int nixge_probe(struct platform_device *pdev)
 {
+	struct nixge_platform_data *pdata = NULL;
 	struct nixge_priv *priv;
 	struct net_device *ndev;
 	struct resource *dmares;
@@ -1320,10 +1365,16 @@ static int nixge_probe(struct platform_device *pdev)
 		err = nixge_of_get_phy(priv, np);
 		if (err)
 			goto free_netdev;
+	} else {
+		pdata = dev_get_platdata(&pdev->dev);
+		err = nixge_pdata_get_phy(priv, pdata);
+		if (err)
+			goto free_netdev;
 	}
 
 	/* only if it's not a fixed link, do we care about MDIO at all */
-	if (priv->phy_node && !of_phy_is_fixed_link(np)) {
+	if ((priv->phy_node && !of_phy_is_fixed_link(np)) ||
+	    (pdata && pdata->phy_interface != PHY_INTERFACE_MODE_NA)) {
 		err = nixge_mdio_setup(priv, np);
 		if (err) {
 			dev_err(&pdev->dev, "error registering mdio bus");
@@ -1347,6 +1398,9 @@ static int nixge_probe(struct platform_device *pdev)
 		of_phy_deregister_fixed_link(np);
 		of_node_put(np);
 	}
+
+	if (priv->phydev && phy_is_pseudo_fixed_link(priv->phydev))
+		fixed_phy_unregister(priv->phydev);
 free_netdev:
 	free_netdev(ndev);
 
@@ -1357,6 +1411,7 @@ static int nixge_remove(struct platform_device *pdev)
 {
 	struct net_device *ndev = platform_get_drvdata(pdev);
 	struct nixge_priv *priv = netdev_priv(ndev);
+	struct device_node *np = pdev->dev.of_node;
 
 	unregister_netdev(ndev);
 
@@ -1365,6 +1420,8 @@ static int nixge_remove(struct platform_device *pdev)
 
 	if (np && of_phy_is_fixed_link(np))
 		of_phy_deregister_fixed_link(np);
+	else if (priv->phydev && phy_is_pseudo_fixed_link(priv->phydev))
+		fixed_phy_unregister(priv->phydev);
 
 	free_netdev(ndev);
 
diff --git a/include/linux/platform_data/nixge.h b/include/linux/platform_data/nixge.h
new file mode 100644
index 000000000000..aa5dd5760074
--- /dev/null
+++ b/include/linux/platform_data/nixge.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (c) 2018 National Instruments Corp.
+ *
+ * Author: Moritz Fischer <mdf@kernel.org>
+ */
+
+#ifndef __NIXGE_PDATA_H__
+#define __NIXGE_PDATA_H__
+
+#include <linux/phy.h>
+
+struct nixge_platform_data {
+	phy_interface_t phy_interface;
+	int phy_speed;
+	int phy_duplex;
+};
+
+#endif /* __NIXGE_PDATA_H__ */
+
-- 
2.18.0

[PATCH net-next 3/3] net: nixge: Use sysdev instead of ndev->dev.parent for DMA

[Moritz Fischer]

Use sysdev instead of ndev->dev.parent for DMA in order to
enable usecases where an IOMMU might get in the way otherwise.
In the PCIe wrapper case on x86 the IOMMU group is not inherited
by the platform device that is created as a subdevice of the
PCI wrapper driver.

Signed-off-by: Moritz Fischer <mdf@kernel.org>
---

Hi,

not this patch is still in the early stages,
and as the rest of this series goes on top of [1].

The actual parent device might still change since
the parent device driver is still under development.

If there are clever suggestions on how to generalize
the assignment of sysdev, I'm all ears.

Thanks for your time,

Moritz


[1] https://lkml.org/lkml/2018/8/28/1011

---
 drivers/net/ethernet/ni/nixge.c | 50 +++++++++++++++++++++------------
 1 file changed, 32 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/ni/nixge.c b/drivers/net/ethernet/ni/nixge.c
index fd8e5b02c459..25fb1642d558 100644
--- a/drivers/net/ethernet/ni/nixge.c
+++ b/drivers/net/ethernet/ni/nixge.c
@@ -16,6 +16,7 @@
 #include <linux/of_irq.h>
 #include <linux/skbuff.h>
 #include <linux/phy.h>
+#include <linux/pci.h>
 #include <linux/mii.h>
 #include <linux/nvmem-consumer.h>
 #include <linux/ethtool.h>
@@ -165,6 +166,7 @@ struct nixge_priv {
 	struct net_device *ndev;
 	struct napi_struct napi;
 	struct device *dev;
+	struct device *sysdev;
 
 	/* Connection to PHY device */
 	struct device_node *phy_node;
@@ -250,7 +252,7 @@ static void nixge_hw_dma_bd_release(struct net_device *ndev)
 		phys_addr = nixge_hw_dma_bd_get_addr(&priv->rx_bd_v[i],
 						     phys);
 
-		dma_unmap_single(ndev->dev.parent, phys_addr,
+		dma_unmap_single(priv->sysdev, phys_addr,
 				 NIXGE_MAX_JUMBO_FRAME_SIZE,
 				 DMA_FROM_DEVICE);
 
@@ -261,16 +263,16 @@ static void nixge_hw_dma_bd_release(struct net_device *ndev)
 	}
 
 	if (priv->rx_bd_v)
-		dma_free_coherent(ndev->dev.parent,
+		dma_free_coherent(priv->sysdev,
 				  sizeof(*priv->rx_bd_v) * RX_BD_NUM,
 				  priv->rx_bd_v,
 				  priv->rx_bd_p);
 
 	if (priv->tx_skb)
-		devm_kfree(ndev->dev.parent, priv->tx_skb);
+		devm_kfree(priv->sysdev, priv->tx_skb);
 
 	if (priv->tx_bd_v)
-		dma_free_coherent(ndev->dev.parent,
+		dma_free_coherent(priv->sysdev,
 				  sizeof(*priv->tx_bd_v) * TX_BD_NUM,
 				  priv->tx_bd_v,
 				  priv->tx_bd_p);
@@ -290,19 +292,19 @@ static int nixge_hw_dma_bd_init(struct net_device *ndev)
 	priv->rx_bd_ci = 0;
 
 	/* Allocate the Tx and Rx buffer descriptors. */
-	priv->tx_bd_v = dma_zalloc_coherent(ndev->dev.parent,
+	priv->tx_bd_v = dma_zalloc_coherent(priv->sysdev,
 					    sizeof(*priv->tx_bd_v) * TX_BD_NUM,
 					    &priv->tx_bd_p, GFP_KERNEL);
 	if (!priv->tx_bd_v)
 		goto out;
 
-	priv->tx_skb = devm_kcalloc(ndev->dev.parent,
+	priv->tx_skb = devm_kcalloc(priv->sysdev,
 				    TX_BD_NUM, sizeof(*priv->tx_skb),
 				    GFP_KERNEL);
 	if (!priv->tx_skb)
 		goto out;
 
-	priv->rx_bd_v = dma_zalloc_coherent(ndev->dev.parent,
+	priv->rx_bd_v = dma_zalloc_coherent(priv->sysdev,
 					    sizeof(*priv->rx_bd_v) * RX_BD_NUM,
 					    &priv->rx_bd_p, GFP_KERNEL);
 	if (!priv->rx_bd_v)
@@ -327,7 +329,7 @@ static int nixge_hw_dma_bd_init(struct net_device *ndev)
 			goto out;
 
 		nixge_hw_dma_bd_set_offset(&priv->rx_bd_v[i], skb);
-		phys = dma_map_single(ndev->dev.parent, skb->data,
+		phys = dma_map_single(priv->sysdev, skb->data,
 				      NIXGE_MAX_JUMBO_FRAME_SIZE,
 				      DMA_FROM_DEVICE);
 
@@ -438,10 +440,10 @@ static void nixge_tx_skb_unmap(struct nixge_priv *priv,
 {
 	if (tx_skb->mapping) {
 		if (tx_skb->mapped_as_page)
-			dma_unmap_page(priv->ndev->dev.parent, tx_skb->mapping,
+			dma_unmap_page(priv->sysdev, tx_skb->mapping,
 				       tx_skb->size, DMA_TO_DEVICE);
 		else
-			dma_unmap_single(priv->ndev->dev.parent,
+			dma_unmap_single(priv->sysdev,
 					 tx_skb->mapping,
 					 tx_skb->size, DMA_TO_DEVICE);
 		tx_skb->mapping = 0;
@@ -519,9 +521,9 @@ static int nixge_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 		return NETDEV_TX_OK;
 	}
 
-	cur_phys = dma_map_single(ndev->dev.parent, skb->data,
+	cur_phys = dma_map_single(priv->sysdev, skb->data,
 				  skb_headlen(skb), DMA_TO_DEVICE);
-	if (dma_mapping_error(ndev->dev.parent, cur_phys))
+	if (dma_mapping_error(priv->sysdev, cur_phys))
 		goto drop;
 	nixge_hw_dma_bd_set_phys(cur_p, cur_phys);
 
@@ -539,10 +541,10 @@ static int nixge_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 		tx_skb = &priv->tx_skb[priv->tx_bd_tail];
 		frag = &skb_shinfo(skb)->frags[ii];
 
-		cur_phys = skb_frag_dma_map(ndev->dev.parent, frag, 0,
+		cur_phys = skb_frag_dma_map(priv->sysdev, frag, 0,
 					    skb_frag_size(frag),
 					    DMA_TO_DEVICE);
-		if (dma_mapping_error(ndev->dev.parent, cur_phys))
+		if (dma_mapping_error(priv->sysdev, cur_phys))
 			goto frag_err;
 		nixge_hw_dma_bd_set_phys(cur_p, cur_phys);
 
@@ -579,7 +581,7 @@ static int nixge_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 		cur_p = &priv->tx_bd_v[priv->tx_bd_tail];
 		cur_p->status = 0;
 	}
-	dma_unmap_single(priv->ndev->dev.parent,
+	dma_unmap_single(priv->sysdev,
 			 tx_skb->mapping,
 			 tx_skb->size, DMA_TO_DEVICE);
 drop:
@@ -611,7 +613,7 @@ static int nixge_recv(struct net_device *ndev, int budget)
 		if (length > NIXGE_MAX_JUMBO_FRAME_SIZE)
 			length = NIXGE_MAX_JUMBO_FRAME_SIZE;
 
-		dma_unmap_single(ndev->dev.parent,
+		dma_unmap_single(priv->sysdev,
 				 nixge_hw_dma_bd_get_addr(cur_p, phys),
 				 NIXGE_MAX_JUMBO_FRAME_SIZE,
 				 DMA_FROM_DEVICE);
@@ -636,10 +638,10 @@ static int nixge_recv(struct net_device *ndev, int budget)
 		if (!new_skb)
 			return packets;
 
-		cur_phys = dma_map_single(ndev->dev.parent, new_skb->data,
+		cur_phys = dma_map_single(priv->sysdev, new_skb->data,
 					  NIXGE_MAX_JUMBO_FRAME_SIZE,
 					  DMA_FROM_DEVICE);
-		if (dma_mapping_error(ndev->dev.parent, cur_phys)) {
+		if (dma_mapping_error(priv->sysdev, cur_phys)) {
 			/* FIXME: bail out and clean up */
 			netdev_err(ndev, "Failed to map ...\n");
 		}
@@ -1303,6 +1305,7 @@ static int nixge_probe(struct platform_device *pdev)
 	struct net_device *ndev;
 	struct resource *dmares;
 	struct device_node *np;
+	struct device *sysdev;
 	const u8 *mac_addr;
 	int err;
 
@@ -1331,9 +1334,20 @@ static int nixge_probe(struct platform_device *pdev)
 		eth_hw_addr_random(ndev);
 	}
 
+	sysdev = &pdev->dev;
+#ifdef CONFIG_PCI
+	/* if this is a subdevice of a PCI device we'll have to
+	 * make sure we'll use parent instead of our own platform
+	 * device to make sure  the DMA API if we use an IOMMU
+	 */
+	if (sysdev->parent && sysdev->parent->bus == &pci_bus_type)
+		sysdev = sysdev->parent;
+#endif
+
 	priv = netdev_priv(ndev);
 	priv->ndev = ndev;
 	priv->dev = &pdev->dev;
+	priv->sysdev = sysdev;
 
 	netif_napi_add(ndev, &priv->napi, nixge_poll, NAPI_POLL_WEIGHT);
 
-- 
2.18.0

Re: [PATCH net-next 2/3] net: nixge: Add support for having nixge as subdevice

[Moritz Fischer]

On Wed, Aug 29, 2018 at 5:49 PM Moritz Fischer <mdf@kernel.org> wrote:
>
> Add support for instantiating nixge as subdevice using
> fixed-link and platform data to configure it.
>
> Signed-off-by: Moritz Fischer <mdf@kernel.org>
> ---
>
> Hi,
>
> not this patch is still in the early stages,
> and as the rest of this series goes on top of [1].
>
> The actual platform data might still change since
> the parent device driver is still under development.
>
> Thanks for your time,
>
> Moritz
>
>
> [1] https://lkml.org/lkml/2018/8/28/1011
>
> ---
>  drivers/net/ethernet/ni/nixge.c     | 71 ++++++++++++++++++++++++++---
>  include/linux/platform_data/nixge.h | 19 ++++++++
>  2 files changed, 83 insertions(+), 7 deletions(-)
>  create mode 100644 include/linux/platform_data/nixge.h
>
> diff --git a/drivers/net/ethernet/ni/nixge.c b/drivers/net/ethernet/ni/nixge.c
> index 670249313ff0..fd8e5b02c459 100644
> --- a/drivers/net/ethernet/ni/nixge.c
> +++ b/drivers/net/ethernet/ni/nixge.c
> @@ -7,6 +7,8 @@
>  #include <linux/etherdevice.h>
>  #include <linux/module.h>
>  #include <linux/netdevice.h>
> +#include <linux/phy_fixed.h>
> +#include <linux/platform_data/nixge.h>
>  #include <linux/of_address.h>
>  #include <linux/of_mdio.h>
>  #include <linux/of_net.h>
> @@ -167,6 +169,7 @@ struct nixge_priv {
>         /* Connection to PHY device */
>         struct device_node *phy_node;
>         phy_interface_t         phy_mode;
> +       struct phy_device *phydev;
>
>         int link;
>         unsigned int speed;
> @@ -859,15 +862,25 @@ static void nixge_dma_err_handler(unsigned long data)
>  static int nixge_open(struct net_device *ndev)
>  {
>         struct nixge_priv *priv = netdev_priv(ndev);
> -       struct phy_device *phy;
> +       struct phy_device *phy = NULL;
>         int ret;
>
>         nixge_device_reset(ndev);
>
> -       phy = of_phy_connect(ndev, priv->phy_node,
> -                            &nixge_handle_link_change, 0, priv->phy_mode);
> -       if (!phy)
> -               return -ENODEV;
> +       if (priv->dev->of_node) {
> +               phy = of_phy_connect(ndev, priv->phy_node,
> +                                    &nixge_handle_link_change, 0,
> +                                    priv->phy_mode);
> +               if (!phy)
> +                       return -ENODEV;
> +       } else if (priv->phydev) {
> +               ret = phy_connect_direct(ndev, priv->phydev,
> +                                        &nixge_handle_link_change,
> +                                        priv->phy_mode);
> +               if (ret)
> +                       return ret;
> +               phy = priv->phydev;
> +       }
>
>         phy_start(phy);
>
> @@ -1215,10 +1228,41 @@ static int nixge_of_get_phy(struct nixge_priv *priv, struct device_node *np)
>         return 0;
>  }
>
> +static int nixge_pdata_get_phy(struct nixge_priv *priv,
> +                              struct nixge_platform_data *pdata)
> +{
> +       struct phy_device *phy = NULL;
> +
> +       if (!pdata)
> +               return -EINVAL;
> +
> +       if (pdata && pdata->phy_interface == PHY_INTERFACE_MODE_NA) {
> +               struct fixed_phy_status fphy_status = {
> +                       .link = 1,
> +                       .duplex = pdata->phy_duplex,
> +                       .speed = pdata->phy_speed,
> +                       .pause = 0,
> +                       .asym_pause = 0,
> +               };
> +
> +               /* TODO: Pull out GPIO from pdata */
> +               phy = fixed_phy_register(PHY_POLL, &fphy_status, -1,
> +                                        NULL);
> +               if (IS_ERR_OR_NULL(phy)) {
> +                       dev_err(priv->dev,
> +                               "failed to register fixed PHY device\n");
> +                       return -ENODEV;
> +               }
> +       }
> +       priv->phy_mode = pdata->phy_interface;
> +       priv->phydev = phy;
> +
> +       return 0;
> +}
> +
>  static int nixge_mdio_setup(struct nixge_priv *priv, struct device_node *np)
>  {
>         struct mii_bus *bus;
> -       int err;
>
>         bus = devm_mdiobus_alloc(priv->dev);
>         if (!bus)
> @@ -1254,6 +1298,7 @@ static void *nixge_get_nvmem_address(struct device *dev)
>
>  static int nixge_probe(struct platform_device *pdev)
>  {
> +       struct nixge_platform_data *pdata = NULL;
>         struct nixge_priv *priv;
>         struct net_device *ndev;
>         struct resource *dmares;
> @@ -1320,10 +1365,16 @@ static int nixge_probe(struct platform_device *pdev)
>                 err = nixge_of_get_phy(priv, np);
>                 if (err)
>                         goto free_netdev;
> +       } else {
> +               pdata = dev_get_platdata(&pdev->dev);
> +               err = nixge_pdata_get_phy(priv, pdata);
> +               if (err)
> +                       goto free_netdev;
>         }
>
>         /* only if it's not a fixed link, do we care about MDIO at all */
> -       if (priv->phy_node && !of_phy_is_fixed_link(np)) {
> +       if ((priv->phy_node && !of_phy_is_fixed_link(np)) ||
> +           (pdata && pdata->phy_interface != PHY_INTERFACE_MODE_NA)) {

Must've messed up the rebase. Missing a parents. I'll resubmit this
one. Sorry for the noise.
>                 err = nixge_mdio_setup(priv, np);
>                 if (err) {
>                         dev_err(&pdev->dev, "error registering mdio bus");
> @@ -1347,6 +1398,9 @@ static int nixge_probe(struct platform_device *pdev)
>                 of_phy_deregister_fixed_link(np);
>                 of_node_put(np);
>         }
> +
> +       if (priv->phydev && phy_is_pseudo_fixed_link(priv->phydev))
> +               fixed_phy_unregister(priv->phydev);
>  free_netdev:
>         free_netdev(ndev);
>
> @@ -1357,6 +1411,7 @@ static int nixge_remove(struct platform_device *pdev)
>  {
>         struct net_device *ndev = platform_get_drvdata(pdev);
>         struct nixge_priv *priv = netdev_priv(ndev);
> +       struct device_node *np = pdev->dev.of_node;
>
>         unregister_netdev(ndev);
>
> @@ -1365,6 +1420,8 @@ static int nixge_remove(struct platform_device *pdev)
>
>         if (np && of_phy_is_fixed_link(np))
>                 of_phy_deregister_fixed_link(np);
> +       else if (priv->phydev && phy_is_pseudo_fixed_link(priv->phydev))
> +               fixed_phy_unregister(priv->phydev);
>
>         free_netdev(ndev);
>
> diff --git a/include/linux/platform_data/nixge.h b/include/linux/platform_data/nixge.h
> new file mode 100644
> index 000000000000..aa5dd5760074
> --- /dev/null
> +++ b/include/linux/platform_data/nixge.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (c) 2018 National Instruments Corp.
> + *
> + * Author: Moritz Fischer <mdf@kernel.org>
> + */
> +
> +#ifndef __NIXGE_PDATA_H__
> +#define __NIXGE_PDATA_H__
> +
> +#include <linux/phy.h>
> +
> +struct nixge_platform_data {
> +       phy_interface_t phy_interface;
> +       int phy_speed;
> +       int phy_duplex;
> +};
> +
> +#endif /* __NIXGE_PDATA_H__ */
> +
> --
> 2.18.0
>

Thanks,
Moritz

Re: [PATCH net-next 1/3] net: nixge: Add support for fixed-link subnodes

[Andrew Lunn]

On Wed, Aug 29, 2018 at 05:40:44PM -0700, Moritz Fischer wrote:
> Add support for fixed-link cases where no MDIO is
> actually required to run the device.
> In that case no MDIO bus is instantiated since the
> actual registers are not available in hardware.

Hi Moritz

There are a few different use cases here:

The hardware is missing MDIO - You need fixed-link.

The hardware has MDIO, but you don't have a PHY connected on it, and
use fixed link.

The hardware has MDIO, and it is used e.g. for an Ethernet switch, or
a PHY for another Ethernet interface. Plus you need fixed link.

The binding typically looks like:

&fec1 {
        phy-mode = "rmii";
        pinctrl-names = "default";
        pinctrl-0 = <&pinctrl_fec1>;
        status = "okay";

        fixed-link {
                speed = <100>;
                full-duplex;
        };

        mdio1: mdio {
                #address-cells = <1>;
                #size-cells = <0>;
                status = "okay";

                switch0: switch0@0 {
                        compatible = "marvell,mv88e6085";
                        pinctrl-names = "default";
                        pinctrl-0 = <&pinctrl_switch>;
                        reg = <0>;
                        eeprom-length = <512>;
                        interrupt-parent = <&gpio3>;

It is important you have the mdio subnode, with PHYs and switches as
children. The driver currently gets this wrong, it uses
pdev->dev.of_node.

So the first patch should be to extend this behaviour. Look for a
child node called mdio. If it exists, call nixge_mdio_setup() passing
that child. Otherwise continue using pdev->dev.of_node, so you don't
break backwards compatibility.

Then a patch adding support for fixed-link. If the mdio child node
exists, you still need to register the MDIO bus. If there is no child
node, but there is a fixed-link, skip registering the mdio bus with
pdev->dev.of_node.

	Andrew

Re: [PATCH net-next 2/3] net: nixge: Add support for having nixge as subdevice

[Andrew Lunn]

On Wed, Aug 29, 2018 at 05:40:45PM -0700, Moritz Fischer wrote:
> Add support for instantiating nixge as subdevice using
> fixed-link and platform data to configure it.
> 
> Signed-off-by: Moritz Fischer <mdf@kernel.org>
> ---
> 
> Hi,
> 
> not this patch is still in the early stages,
> and as the rest of this series goes on top of [1].
> 
> The actual platform data might still change since
> the parent device driver is still under development.

Hi Moritz

Could you tell us more about the parent device. I'm guessing PCIe.  Is
it x86 so no device tree? Are there cases where it does not have a PHY
connected? What is connected instead? SFP? A switch? Can there be
multiple PHYs on the MDIO bus?

Answering these questions will help decide how best to structure this.

	 Andrew

Re: [PATCH net-next 2/3] net: nixge: Add support for having nixge as subdevice

[Moritz Fischer]

Hi Andrew,

On Wed, Aug 29, 2018 at 8:11 PM, Andrew Lunn <andrew@lunn.ch> wrote:

> Could you tell us more about the parent device. I'm guessing PCIe.  Is
> it x86 so no device tree? Are there cases where it does not have a PHY
> connected? What is connected instead? SFP? A switch? Can there be
> multiple PHYs on the MDIO bus?

The device is part of a larger FPGA design. One use case that I was trying
to support with this patch is PCIe with x86 (hopefully on it's own PF...)
Since the whole design isn't completely done, these are the use cases I
see upcoming and current:

ARM(64):
a) DT: PHY over MDIO (current use case), fixed-link with GPIO (coming)
b) DT: SFP (potentially coming)

x86:
a) no PHY (coming)-> fixed-link with GPIO
b) SFP (potentially), PHY over MDIO (potentially)

Thanks for your help,

Moritz

Re: [PATCH net-next 1/3] net: nixge: Add support for fixed-link subnodes

[Moritz Fischer]

Hi Andrew,

On Wed, Aug 29, 2018 at 8:04 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> On Wed, Aug 29, 2018 at 05:40:44PM -0700, Moritz Fischer wrote:
>> Add support for fixed-link cases where no MDIO is
>> actually required to run the device.
>> In that case no MDIO bus is instantiated since the
>> actual registers are not available in hardware.
>
> Hi Moritz
>
> There are a few different use cases here:
>
> The hardware is missing MDIO - You need fixed-link.

Agreed.
>
> The hardware has MDIO, but you don't have a PHY connected on it, and
> use fixed link.

Since it's an FPGA design in that case we'd probably build the hardware without
MDIO to save resources.

> The hardware has MDIO, and it is used e.g. for an Ethernet switch, or
> a PHY for another Ethernet interface. Plus you need fixed link.
We haven't had that yet but I can see that happen.
>
> The binding typically looks like:
>
> &fec1 {
>         phy-mode = "rmii";
>         pinctrl-names = "default";
>         pinctrl-0 = <&pinctrl_fec1>;
>         status = "okay";
>
>         fixed-link {
>                 speed = <100>;
>                 full-duplex;
>         };
>
>         mdio1: mdio {
>                 #address-cells = <1>;
>                 #size-cells = <0>;
>                 status = "okay";
>
>                 switch0: switch0@0 {
>                         compatible = "marvell,mv88e6085";
>                         pinctrl-names = "default";
>                         pinctrl-0 = <&pinctrl_switch>;
>                         reg = <0>;
>                         eeprom-length = <512>;
>                         interrupt-parent = <&gpio3>;
>
> It is important you have the mdio subnode, with PHYs and switches as
> children. The driver currently gets this wrong, it uses
> pdev->dev.of_node.

Oh, whoops. Yeah I should look into that. Any good examples of drivers doing
it right? Is the one going with the DT snippet above a good example?
>
> So the first patch should be to extend this behaviour. Look for a
> child node called mdio. If it exists, call nixge_mdio_setup() passing
> that child. Otherwise continue using pdev->dev.of_node, so you don't
> break backwards compatibility.

Ok will do.
>
> Then a patch adding support for fixed-link. If the mdio child node
> exists, you still need to register the MDIO bus. If there is no child
> node, but there is a fixed-link, skip registering the mdio bus with
> pdev->dev.of_node.
>
>         Andrew

Thanks for your feedback, much appreciated!

Moritz

Re: [PATCH net-next 2/3] net: nixge: Add support for having nixge as subdevice

[Andrew Lunn]

On Thu, Aug 30, 2018 at 09:39:39AM -0700, Moritz Fischer wrote:
> Hi Andrew,
> 
> On Wed, Aug 29, 2018 at 8:11 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> 
> > Could you tell us more about the parent device. I'm guessing PCIe.  Is
> > it x86 so no device tree? Are there cases where it does not have a PHY
> > connected? What is connected instead? SFP? A switch? Can there be
> > multiple PHYs on the MDIO bus?
> 
> The device is part of a larger FPGA design. One use case that I was trying
> to support with this patch is PCIe with x86 (hopefully on it's own PF...)
> Since the whole design isn't completely done, these are the use cases I
> see upcoming and current:
> 
> ARM(64):
> a) DT: PHY over MDIO (current use case), fixed-link with GPIO (coming)
> b) DT: SFP (potentially coming)
> 
> x86:
> a) no PHY (coming)-> fixed-link with GPIO
> b) SFP (potentially), PHY over MDIO (potentially)

Hi Mortiz

For SFP, you need to convert this driver to use phylink. That will
also help you with fixed-link, since phylink will handle probing that
for you.

But this brings its own problems. phylink and sfp currently has no
support for platform devices. The SFP driver needs to know which i2c
bus to use, and which GPIOs are connected to the SFP. phylink parses
the device tree to find out if there is an SFP device, or a fixed
link, etc. I don't know of any conceptual reason why platform data
would not work, it just needs implementing.

There also does not appear to be any in kernel users of the device
tree binding. That gives you some flexibility in that you could think
about making non-backwards compatible changes in the binding. I would
definitely want to move PHYs into an mdio subnode.

I'm not aware of any x86 drivers using fixed link. What they generally
do is register the mdio bus using mdiobus_register() and then use
phy_find_first() to get a PHY. This works O.K. for PCs, laptops, and
PCIe cards where there is only one PHY on the bus. What you might be
able to do is always register the MDIO bus, and if you fail to find a
PHY, instantiate a fixed-link and use that instead.

Reality is, all the core work in this area has been pushed by the
embedded world, which is ARM and device tree. For Intel and Server
style networking, drivers tend to either ignore the Linux core code
and write there own PHY and MDIO bus drivers, or it is all done in
firmware.

     Andrew

Re: [PATCH net-next 1/3] net: nixge: Add support for fixed-link subnodes

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 5891 bytes --]

Hi Moritz,

I love your patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Moritz-Fischer/nixge-fixed-link-support/20180830-150857
config: i386-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

Note: the linux-review/Moritz-Fischer/nixge-fixed-link-support/20180830-150857 HEAD 300a42d41dc76f270bff67d414dc7fc127d3f17c builds fine.
      It only hurts bisectibility.

All errors (new ones prefixed by >>):

   drivers/net/ethernet/ni/nixge.c: In function 'nixge_mdio_setup':
   drivers/net/ethernet/ni/nixge.c:1221:6: warning: unused variable 'err' [-Wunused-variable]
     int err;
         ^~~
   drivers/net/ethernet/ni/nixge.c: In function 'nixge_remove':
>> drivers/net/ethernet/ni/nixge.c:1366:6: error: 'np' undeclared (first use in this function); did you mean 'up'?
     if (np && of_phy_is_fixed_link(np))
         ^~
         up
   drivers/net/ethernet/ni/nixge.c:1366:6: note: each undeclared identifier is reported only once for each function it appears in

vim +1366 drivers/net/ethernet/ni/nixge.c

  1217	
  1218	static int nixge_mdio_setup(struct nixge_priv *priv, struct device_node *np)
  1219	{
  1220		struct mii_bus *bus;
> 1221		int err;
  1222	
  1223		bus = devm_mdiobus_alloc(priv->dev);
  1224		if (!bus)
  1225			return -ENOMEM;
  1226	
  1227		snprintf(bus->id, MII_BUS_ID_SIZE, "%s-mii", dev_name(priv->dev));
  1228		bus->priv = priv;
  1229		bus->name = "nixge_mii_bus";
  1230		bus->read = nixge_mdio_read;
  1231		bus->write = nixge_mdio_write;
  1232		bus->parent = priv->dev;
  1233	
  1234		priv->mii_bus = bus;
  1235	
  1236		return of_mdiobus_register(bus, np);
  1237	}
  1238	
  1239	static void *nixge_get_nvmem_address(struct device *dev)
  1240	{
  1241		struct nvmem_cell *cell;
  1242		size_t cell_size;
  1243		char *mac;
  1244	
  1245		cell = nvmem_cell_get(dev, "address");
  1246		if (IS_ERR(cell))
  1247			return NULL;
  1248	
  1249		mac = nvmem_cell_read(cell, &cell_size);
  1250		nvmem_cell_put(cell);
  1251	
  1252		return mac;
  1253	}
  1254	
  1255	static int nixge_probe(struct platform_device *pdev)
  1256	{
  1257		struct nixge_priv *priv;
  1258		struct net_device *ndev;
  1259		struct resource *dmares;
  1260		struct device_node *np;
  1261		const u8 *mac_addr;
  1262		int err;
  1263	
  1264		ndev = alloc_etherdev(sizeof(*priv));
  1265		if (!ndev)
  1266			return -ENOMEM;
  1267	
  1268		np = pdev->dev.of_node;
  1269	
  1270		platform_set_drvdata(pdev, ndev);
  1271		SET_NETDEV_DEV(ndev, &pdev->dev);
  1272	
  1273		ndev->features = NETIF_F_SG;
  1274		ndev->netdev_ops = &nixge_netdev_ops;
  1275		ndev->ethtool_ops = &nixge_ethtool_ops;
  1276	
  1277		/* MTU range: 64 - 9000 */
  1278		ndev->min_mtu = 64;
  1279		ndev->max_mtu = NIXGE_JUMBO_MTU;
  1280	
  1281		mac_addr = nixge_get_nvmem_address(&pdev->dev);
  1282		if (mac_addr && is_valid_ether_addr(mac_addr)) {
  1283			ether_addr_copy(ndev->dev_addr, mac_addr);
  1284			kfree(mac_addr);
  1285		} else {
  1286			eth_hw_addr_random(ndev);
  1287		}
  1288	
  1289		priv = netdev_priv(ndev);
  1290		priv->ndev = ndev;
  1291		priv->dev = &pdev->dev;
  1292	
  1293		netif_napi_add(ndev, &priv->napi, nixge_poll, NAPI_POLL_WEIGHT);
  1294	
  1295		dmares = platform_get_resource(pdev, IORESOURCE_MEM, 0);
  1296		priv->dma_regs = devm_ioremap_resource(&pdev->dev, dmares);
  1297		if (IS_ERR(priv->dma_regs)) {
  1298			netdev_err(ndev, "failed to map dma regs\n");
  1299			return PTR_ERR(priv->dma_regs);
  1300		}
  1301		priv->ctrl_regs = priv->dma_regs + NIXGE_REG_CTRL_OFFSET;
  1302		__nixge_hw_set_mac_address(ndev);
  1303	
  1304		priv->tx_irq = platform_get_irq_byname(pdev, "tx");
  1305		if (priv->tx_irq < 0) {
  1306			netdev_err(ndev, "could not find 'tx' irq");
  1307			return priv->tx_irq;
  1308		}
  1309	
  1310		priv->rx_irq = platform_get_irq_byname(pdev, "rx");
  1311		if (priv->rx_irq < 0) {
  1312			netdev_err(ndev, "could not find 'rx' irq");
  1313			return priv->rx_irq;
  1314		}
  1315	
  1316		priv->coalesce_count_rx = XAXIDMA_DFT_RX_THRESHOLD;
  1317		priv->coalesce_count_tx = XAXIDMA_DFT_TX_THRESHOLD;
  1318	
  1319		if (np) {
  1320			err = nixge_of_get_phy(priv, np);
  1321			if (err)
  1322				goto free_netdev;
  1323		}
  1324	
  1325		/* only if it's not a fixed link, do we care about MDIO at all */
  1326		if (priv->phy_node && !of_phy_is_fixed_link(np)) {
  1327			err = nixge_mdio_setup(priv, np);
  1328			if (err) {
  1329				dev_err(&pdev->dev, "error registering mdio bus");
  1330				goto free_phy;
  1331			}
  1332		}
  1333	
  1334		err = register_netdev(priv->ndev);
  1335		if (err) {
  1336			netdev_err(ndev, "register_netdev() error (%i)\n", err);
  1337			goto unregister_mdio;
  1338		}
  1339	
  1340		return 0;
  1341	
  1342	unregister_mdio:
  1343		if (priv->mii_bus)
  1344			mdiobus_unregister(priv->mii_bus);
  1345	free_phy:
  1346		if (np && of_phy_is_fixed_link(np)) {
  1347			of_phy_deregister_fixed_link(np);
  1348			of_node_put(np);
  1349		}
  1350	free_netdev:
  1351		free_netdev(ndev);
  1352	
  1353		return err;
  1354	}
  1355	
  1356	static int nixge_remove(struct platform_device *pdev)
  1357	{
  1358		struct net_device *ndev = platform_get_drvdata(pdev);
  1359		struct nixge_priv *priv = netdev_priv(ndev);
  1360	
  1361		unregister_netdev(ndev);
  1362	
  1363		if (priv->mii_bus)
  1364			mdiobus_unregister(priv->mii_bus);
  1365	
> 1366		if (np && of_phy_is_fixed_link(np))
  1367			of_phy_deregister_fixed_link(np);
  1368	
  1369		free_netdev(ndev);
  1370	
  1371		return 0;
  1372	}
  1373	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 65144 bytes --]

Re: [PATCH net-next 1/3] net: nixge: Add support for fixed-link subnodes

[Andrew Lunn]

> > The hardware has MDIO, but you don't have a PHY connected on it, and
> > use fixed link.
> 
> Since it's an FPGA design in that case we'd probably build the hardware without
> MDIO to save resources.

You can save resources, but is it worth the complexity else where,
like in the software?

> > It is important you have the mdio subnode, with PHYs and switches as
> > children. The driver currently gets this wrong, it uses
> > pdev->dev.of_node.
> 
> Oh, whoops.

Yes, and i also missed it. I generally review all new network drivers
and look at their MDIO and PHY code.

> Any good examples of drivers doing it right? Is the one going with
> the DT snippet above a good example?

That comes from the Freescale fec_main.c. It only supports DT, and
always uses of_mdiobus_register. You need to be a bit more flexible
for when you don't have DT. I'm not sure there are good example of
this, since they either don't need this flexibility, or they get it
wrong :-(

      Andrew

Re: [PATCH net-next 1/3] net: nixge: Add support for fixed-link subnodes

[Moritz Fischer]

Andrew,

On Thu, Aug 30, 2018 at 10:54 AM, Andrew Lunn <andrew@lunn.ch> wrote:
>> > The hardware has MDIO, but you don't have a PHY connected on it, and
>> > use fixed link.
>>
>> Since it's an FPGA design in that case we'd probably build the hardware without
>> MDIO to save resources.
>
> You can save resources, but is it worth the complexity else where,
> like in the software?

Depends. For now I definitely have build versions that don't have MDIO regs
there. I might be able to chat with HW folks ...

>
>> > It is important you have the mdio subnode, with PHYs and switches as
>> > children. The driver currently gets this wrong, it uses
>> > pdev->dev.of_node.
>>
>> Oh, whoops.
>
> Yes, and i also missed it. I generally review all new network drivers
> and look at their MDIO and PHY code.

I had looked at macb as an example and there were a bunch of other cases
where there was no 'mdio' subnode.

>
>> Any good examples of drivers doing it right? Is the one going with
>> the DT snippet above a good example?
>
> That comes from the Freescale fec_main.c. It only supports DT, and
> always uses of_mdiobus_register. You need to be a bit more flexible
> for when you don't have DT. I'm not sure there are good example of
> this, since they either don't need this flexibility, or they get it
> wrong :-(

Alright, no problem. I'll take a stab at it. And come back with a v2 ;-)
Need to look at your response in the other patch.

Cheers,

Moritz