| Message ID | 20180628194755.28504-2-grygorii.strashko@ti.com |
|---|---|
| State | Superseded |
| Delegated to: | Joe Hershberger |
| Headers | show |
| Series | net: phy: prevent uclass_eth device "node" field overwriting | expand |
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
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);
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(+)