[U-Boot] [PATCH 0/5] net: phy: prevent uclass_eth device "node" field overwriting

This series prevents the UCLASS_ETH device "node" field overwriting by some network drivers when Ethernet PHYs are linked to UCLASS_ETH device using "phy-handle" DT property and when Ethernet PHY driver needs to read some additional information from DT (like dp83867).
It fixes following cases:
- network drivers priv->phydev = phy_connect(priv->bus, priv->phyaddr, dev, priv->interface); <-- phydev is connected to dev which is UCLASS_ETH device
if (priv->phy_of_handle > 0) dev_set_of_offset(priv->phydev->dev, priv->phy_of_handle); <-- phydev->dev->node is overwritten by phy-handle DT node
- PHY driver in .config() callback int node = dev_of_offset(dev); <-- PHY driver uses overwritten dev->node const void *fdt = gd->fdt_blob;
if (fdtdec_get_bool(fdt, node, "property")) ...
As result, UCLASS_ETH device can't be used any more for DT accessing.
It adds new field ofnode node to struct phy_device and updates TI CPSW and zynq_gem drivers to use it.
zynq_gem.c, xilinx_phy.c changes only build tested.
Dependency: This series has dependency from https://patchwork.ozlabs.org/cover/936370/ due to possible merge conflicts
PS: Not sure if any other Net drivers need to be updated, at least I've not found any.
Grygorii Strashko (5): net: phy: add ofnode node to struct phy_device net: phy: dp83867: switch to use phy_get_ofnode() net: phy: xilinx: switch to use phy_get_ofnode() drivers: net: cpsw: fix phy dt node setting drivers: net: zynq_gem: fix phy dt node setting
drivers/net/cpsw.c | 2 +- drivers/net/phy/ti.c | 7 +++++-- drivers/net/phy/xilinx_phy.c | 10 ++++++---- drivers/net/zynq_gem.c | 2 +- include/phy.h | 13 +++++++++++++ 5 files changed, 26 insertions(+), 8 deletions(-)

Now the UCLASS_ETH device "node" field is owerwritten by some network drivers in case of Ethernet PHYs which are linked to UCLASS_ETH device using "phy-handle" DT property and when Ethernet PHY driver needs to read some additional information from DT. In such cases following happens (in general):
- network drivers priv->phydev = phy_connect(priv->bus, priv->phyaddr, dev, priv->interface); <-- phydev is connected to dev which is UCLASS_ETH device
if (priv->phy_of_handle > 0) dev_set_of_offset(priv->phydev->dev, priv->phy_of_handle); <-- phydev->dev->node is overwritten by phy-handle DT node
- PHY driver in .config() callback int node = dev_of_offset(dev); <-- PHY driver uses overwritten dev->node const void *fdt = gd->fdt_blob;
if (fdtdec_get_bool(fdt, node, "property")) ...
As result, UCLASS_ETH device can't be used any more for DT accessing.
This patch adds additional ofnode node field to struct phy_device which can be set explicitly by network drivers and used by PHY drivers, so overwriting can be avoided. Also add helper function phy_get_ofnode() which will check and return phy_device->node or dev_ofnode(phydev->dev) for backward compatibility with existing drivers.
Signed-off-by: Grygorii Strashko grygorii.strashko@ti.com --- include/phy.h | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/include/phy.h b/include/phy.h index 7c3fc5c..0575ea8 100644 --- a/include/phy.h +++ b/include/phy.h @@ -9,6 +9,7 @@ #ifndef _PHY_H #define _PHY_H
+#include <dm.h> #include <linux/list.h> #include <linux/mii.h> #include <linux/ethtool.h> @@ -165,6 +166,7 @@ struct phy_device {
#ifdef CONFIG_DM_ETH struct udevice *dev; + ofnode node; #else struct eth_device *dev; #endif @@ -235,11 +237,22 @@ void phy_connect_dev(struct phy_device *phydev, struct udevice *dev); struct phy_device *phy_connect(struct mii_dev *bus, int addr, struct udevice *dev, phy_interface_t interface); +static inline ofnode phy_get_ofnode(struct phy_device *phydev) +{ + if (ofnode_valid(phydev->node)) + return phydev->node; + else + return dev_ofnode(phydev->dev); +} #else void phy_connect_dev(struct phy_device *phydev, struct eth_device *dev); struct phy_device *phy_connect(struct mii_dev *bus, int addr, struct eth_device *dev, phy_interface_t interface); +static inline ofnode phy_get_ofnode(struct phy_device *phydev) +{ + return ofnode_null(); +} #endif int phy_startup(struct phy_device *phydev); int phy_config(struct phy_device *phydev);

On Thu, Jun 28, 2018 at 2:47 PM, Grygorii Strashko grygorii.strashko@ti.com wrote:
Now the UCLASS_ETH device "node" field is owerwritten by some network drivers in case of Ethernet PHYs which are linked to UCLASS_ETH device using "phy-handle" DT property and when Ethernet PHY driver needs to read some additional information from DT. In such cases following happens (in general):
network drivers priv->phydev = phy_connect(priv->bus, priv->phyaddr, dev, priv->interface); <-- phydev is connected to dev which is UCLASS_ETH device
if (priv->phy_of_handle > 0) dev_set_of_offset(priv->phydev->dev, priv->phy_of_handle); <-- phydev->dev->node is overwritten by phy-handle DT node
PHY driver in .config() callback int node = dev_of_offset(dev); <-- PHY driver uses overwritten dev->node const void *fdt = gd->fdt_blob;
if (fdtdec_get_bool(fdt, node, "property")) ...
As result, UCLASS_ETH device can't be used any more for DT accessing.
This patch adds additional ofnode node field to struct phy_device which can be set explicitly by network drivers and used by PHY drivers, so overwriting can be avoided. Also add helper function phy_get_ofnode() which will check and return phy_device->node or dev_ofnode(phydev->dev) for backward compatibility with existing drivers.
Signed-off-by: Grygorii Strashko grygorii.strashko@ti.com
Acked-by: Joe Hershberger joe.hershberger@ni.com

On 07/02/2018 04:10 PM, Joe Hershberger wrote:
On Thu, Jun 28, 2018 at 2:47 PM, Grygorii Strashko grygorii.strashko@ti.com wrote:
Now the UCLASS_ETH device "node" field is owerwritten by some network drivers in case of Ethernet PHYs which are linked to UCLASS_ETH device using "phy-handle" DT property and when Ethernet PHY driver needs to read some additional information from DT. In such cases following happens (in general):
network drivers priv->phydev = phy_connect(priv->bus, priv->phyaddr, dev, priv->interface); <-- phydev is connected to dev which is UCLASS_ETH device
if (priv->phy_of_handle > 0) dev_set_of_offset(priv->phydev->dev, priv->phy_of_handle); <-- phydev->dev->node is overwritten by phy-handle DT node
PHY driver in .config() callback int node = dev_of_offset(dev); <-- PHY driver uses overwritten dev->node const void *fdt = gd->fdt_blob;
if (fdtdec_get_bool(fdt, node, "property")) ...
As result, UCLASS_ETH device can't be used any more for DT accessing.
This patch adds additional ofnode node field to struct phy_device which can be set explicitly by network drivers and used by PHY drivers, so overwriting can be avoided. Also add helper function phy_get_ofnode() which will check and return phy_device->node or dev_ofnode(phydev->dev) for backward compatibility with existing drivers.
Signed-off-by: Grygorii Strashko grygorii.strashko@ti.com
Acked-by: Joe Hershberger joe.hershberger@ni.com
I will resend v2 of this, as we found a bug - ofnode field need to be explicitly initialized in phy_device_create() for !CONFIG_OF_LIVE

Use PHY API phy_get_ofnode() helper to get PHY DT node.
Signed-off-by: Grygorii Strashko grygorii.strashko@ti.com --- drivers/net/phy/ti.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/net/phy/ti.c b/drivers/net/phy/ti.c index 98c36ab..d4a7e39 100644 --- a/drivers/net/phy/ti.c +++ b/drivers/net/phy/ti.c @@ -172,8 +172,11 @@ void phy_write_mmd_indirect(struct phy_device *phydev, int prtad, static int dp83867_of_init(struct phy_device *phydev) { struct dp83867_private *dp83867 = phydev->priv; - struct udevice *dev = phydev->dev; - ofnode node = dev_ofnode(dev); + ofnode node; + + node = phy_get_ofnode(phydev); + if (!ofnode_valid(node)) + return -EINVAL;
if (ofnode_read_bool(node, "ti,max-output-impedance")) dp83867->io_impedance = DP83867_IO_MUX_CFG_IO_IMPEDANCE_MAX;

On Thu, Jun 28, 2018 at 2:47 PM, Grygorii Strashko grygorii.strashko@ti.com wrote:
Use PHY API phy_get_ofnode() helper to get PHY DT node.
Signed-off-by: Grygorii Strashko grygorii.strashko@ti.com
Acked-by: Joe Hershberger joe.hershberger@ni.com

Use PHY API phy_get_ofnode() helper to get PHY DT node.
Signed-off-by: Grygorii Strashko grygorii.strashko@ti.com --- drivers/net/phy/xilinx_phy.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/net/phy/xilinx_phy.c b/drivers/net/phy/xilinx_phy.c index 004cfcf..3aa8891 100644 --- a/drivers/net/phy/xilinx_phy.c +++ b/drivers/net/phy/xilinx_phy.c @@ -10,8 +10,6 @@ #include <phy.h> #include <dm.h>
-DECLARE_GLOBAL_DATA_PTR; - #define MII_PHY_STATUS_SPD_MASK 0x0C00 #define MII_PHY_STATUS_FULLDUPLEX 0x1000 #define MII_PHY_STATUS_1000 0x0800 @@ -101,10 +99,14 @@ static int xilinxphy_startup(struct phy_device *phydev) static int xilinxphy_of_init(struct phy_device *phydev) { u32 phytype; + ofnode node;
debug("%s\n", __func__); - phytype = fdtdec_get_int(gd->fdt_blob, dev_of_offset(phydev->dev), - "xlnx,phy-type", -1); + node = phy_get_ofnode(phydev); + if (!ofnode_valid(node)) + return -EINVAL; + + phytype = ofnode_read_u32_default(node, "xlnx,phy-type", -1); if (phytype == XAE_PHY_TYPE_1000BASE_X) phydev->flags |= XAE_PHY_TYPE_1000BASE_X;

On Thu, Jun 28, 2018 at 2:47 PM, Grygorii Strashko grygorii.strashko@ti.com wrote:
Use PHY API phy_get_ofnode() helper to get PHY DT node.
Signed-off-by: Grygorii Strashko grygorii.strashko@ti.com
Acked-by: Joe Hershberger joe.hershberger@ni.com

Now CPSW driver will overwrite UCLASS_ETH node when PHY is connected and configured which is not correct. Use struct phydev->node instead.
Signed-off-by: Grygorii Strashko grygorii.strashko@ti.com --- drivers/net/cpsw.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/cpsw.c b/drivers/net/cpsw.c index 9919d39..c31695e 100644 --- a/drivers/net/cpsw.c +++ b/drivers/net/cpsw.c @@ -999,7 +999,7 @@ static int cpsw_phy_init(struct cpsw_priv *priv, struct cpsw_slave *slave)
#ifdef CONFIG_DM_ETH if (slave->data->phy_of_handle) - dev_set_of_offset(phydev->dev, slave->data->phy_of_handle); + phydev->node = offset_to_ofnode(slave->data->phy_of_handle); #endif
priv->phydev = phydev;

On Thu, Jun 28, 2018 at 2:47 PM, Grygorii Strashko grygorii.strashko@ti.com wrote:
Now CPSW driver will overwrite UCLASS_ETH node when PHY is connected and configured which is not correct. Use struct phydev->node instead.
Signed-off-by: Grygorii Strashko grygorii.strashko@ti.com
Acked-by: Joe Hershberger joe.hershberger@ni.com

Now zynq_gem driver will overwrite UCLASS_ETH node when PHY is connected and configured which is not correct. Use struct phydev->node instead.
Signed-off-by: Grygorii Strashko grygorii.strashko@ti.com --- drivers/net/zynq_gem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c index a817f2e..56d76e3 100644 --- a/drivers/net/zynq_gem.c +++ b/drivers/net/zynq_gem.c @@ -350,7 +350,7 @@ static int zynq_phy_init(struct udevice *dev) priv->phydev->advertising = priv->phydev->supported;
if (priv->phy_of_handle > 0) - dev_set_of_offset(priv->phydev->dev, priv->phy_of_handle); + priv->phydev->node = offset_to_ofnode(priv->phy_of_handle);
return phy_config(priv->phydev); }

On Thu, Jun 28, 2018 at 2:47 PM, Grygorii Strashko grygorii.strashko@ti.com wrote:
Now zynq_gem driver will overwrite UCLASS_ETH node when PHY is connected and configured which is not correct. Use struct phydev->node instead.
Signed-off-by: Grygorii Strashko grygorii.strashko@ti.com
Acked-by: Joe Hershberger joe.hershberger@ni.com
participants (2)
-
Grygorii Strashko
-
Joe Hershberger