| Message ID | 20240201151747.7524-4-ansuelsmth@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: phy: Introduce PHY Package concept | expand |
Quoting Christian Marangi (2024-02-01 16:17:29) > +/** > + * of_phy_package_join - join a common PHY group in PHY package > + * @phydev: target phy_device struct > + * @priv_size: if non-zero allocate this amount of bytes for private data > + * > + * This is a variant of phy_package_join for PHY package defined in DT. > + * > + * The parent node of the @phydev is checked as a valid PHY package node > + * structure (by matching the node name "ethernet-phy-package") and the > + * base_addr for the PHY package is passed to phy_package_join. > + * > + * With this configuration the shared struct will also have the np value > + * filled to use additional DT defined properties in PHY specific > + * probe_once and config_init_once PHY package OPs. > + * > + * Returns < 1 on error, 0 on success. Esp. calling phy_package_join() So, < 0 on error ? > +int of_phy_package_join(struct phy_device *phydev, size_t priv_size) > +{ > + struct device_node *node = phydev->mdio.dev.of_node; > + struct device_node *package_node; > + u32 base_addr; > + int ret; > + > + if (!node) > + return -EINVAL; > + > + package_node = of_get_parent(node); Is the node put on package leave? > + if (!package_node) > + return -EINVAL; > + > + if (!of_node_name_eq(package_node, "ethernet-phy-package") of_put_node? + below. > + return -EINVAL; > + > + if (of_property_read_u32(package_node, "reg", &base_addr)) > + return -EINVAL; > + > + ret = phy_package_join(phydev, base_addr, priv_size); > + if (ret) > + return ret; > + > + phydev->shared->np = package_node; Just looked quickly, looks like ->np is uninitialized in the !of join case. > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(of_phy_package_join);
On Thu, Feb 01, 2024 at 05:40:28PM +0100, Antoine Tenart wrote: > Quoting Christian Marangi (2024-02-01 16:17:29) > > +/** > > + * of_phy_package_join - join a common PHY group in PHY package > > + * @phydev: target phy_device struct > > + * @priv_size: if non-zero allocate this amount of bytes for private data > > + * > > + * This is a variant of phy_package_join for PHY package defined in DT. > > + * > > + * The parent node of the @phydev is checked as a valid PHY package node > > + * structure (by matching the node name "ethernet-phy-package") and the > > + * base_addr for the PHY package is passed to phy_package_join. > > + * > > + * With this configuration the shared struct will also have the np value > > + * filled to use additional DT defined properties in PHY specific > > + * probe_once and config_init_once PHY package OPs. > > + * > > + * Returns < 1 on error, 0 on success. Esp. calling phy_package_join() > > So, < 0 on error ? > > > +int of_phy_package_join(struct phy_device *phydev, size_t priv_size) > > +{ > > + struct device_node *node = phydev->mdio.dev.of_node; > > + struct device_node *package_node; > > + u32 base_addr; > > + int ret; > > + > > + if (!node) > > + return -EINVAL; > > + > > + package_node = of_get_parent(node); > > Is the node put on package leave? > Didn't read the get_parent was incrementing the refcount... Will update the leave accordingly! > > + if (!package_node) > > + return -EINVAL; > > + > > + if (!of_node_name_eq(package_node, "ethernet-phy-package") > > of_put_node? + below. > > > + return -EINVAL; > > + > > + if (of_property_read_u32(package_node, "reg", &base_addr)) > > + return -EINVAL; > > + > > + ret = phy_package_join(phydev, base_addr, priv_size); > > + if (ret) > > + return ret; > > + > > + phydev->shared->np = package_node; > > Just looked quickly, looks like ->np is uninitialized in the !of join > case. > Correct, I will update the non of variant to inizialize ->np to NULL. (in theory we alloc every struct as zero so it should not be a problem but you are right with being safe on this.) Thanks! > > + > > + return 0; > > +} > > +EXPORT_SYMBOL_GPL(of_phy_package_join);
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index 52828d1c64f7..50874192e807 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -1730,6 +1730,56 @@ int phy_package_join(struct phy_device *phydev, int base_addr, size_t priv_size) } EXPORT_SYMBOL_GPL(phy_package_join); +/** + * of_phy_package_join - join a common PHY group in PHY package + * @phydev: target phy_device struct + * @priv_size: if non-zero allocate this amount of bytes for private data + * + * This is a variant of phy_package_join for PHY package defined in DT. + * + * The parent node of the @phydev is checked as a valid PHY package node + * structure (by matching the node name "ethernet-phy-package") and the + * base_addr for the PHY package is passed to phy_package_join. + * + * With this configuration the shared struct will also have the np value + * filled to use additional DT defined properties in PHY specific + * probe_once and config_init_once PHY package OPs. + * + * Returns < 1 on error, 0 on success. Esp. calling phy_package_join() + * with the same cookie but a different priv_size is an error. Or a parent + * node is not detected or is not valid or doesn't match the expected node + * name for PHY package. + */ +int of_phy_package_join(struct phy_device *phydev, size_t priv_size) +{ + struct device_node *node = phydev->mdio.dev.of_node; + struct device_node *package_node; + u32 base_addr; + int ret; + + if (!node) + return -EINVAL; + + package_node = of_get_parent(node); + if (!package_node) + return -EINVAL; + + if (!of_node_name_eq(package_node, "ethernet-phy-package")) + return -EINVAL; + + if (of_property_read_u32(package_node, "reg", &base_addr)) + return -EINVAL; + + ret = phy_package_join(phydev, base_addr, priv_size); + if (ret) + return ret; + + phydev->shared->np = package_node; + + return 0; +} +EXPORT_SYMBOL_GPL(of_phy_package_join); + /** * phy_package_leave - leave a common PHY group * @phydev: target phy_device struct @@ -1798,6 +1848,40 @@ int devm_phy_package_join(struct device *dev, struct phy_device *phydev, } EXPORT_SYMBOL_GPL(devm_phy_package_join); +/** + * devm_of_phy_package_join - resource managed of_phy_package_join() + * @dev: device that is registering this PHY package + * @phydev: target phy_device struct + * @priv_size: if non-zero allocate this amount of bytes for private data + * + * Managed of_phy_package_join(). Shared storage fetched by this function, + * phy_package_leave() is automatically called on driver detach. See + * of_phy_package_join() for more information. + */ +int devm_of_phy_package_join(struct device *dev, struct phy_device *phydev, + size_t priv_size) +{ + struct phy_device **ptr; + int ret; + + ptr = devres_alloc(devm_phy_package_leave, sizeof(*ptr), + GFP_KERNEL); + if (!ptr) + return -ENOMEM; + + ret = of_phy_package_join(phydev, priv_size); + + if (!ret) { + *ptr = phydev; + devres_add(dev, ptr); + } else { + devres_free(ptr); + } + + return ret; +} +EXPORT_SYMBOL_GPL(devm_of_phy_package_join); + /** * phy_detach - detach a PHY device from its network device * @phydev: target phy_device struct diff --git a/include/linux/phy.h b/include/linux/phy.h index a66f07d3f5f4..2aed925e6c23 100644 --- a/include/linux/phy.h +++ b/include/linux/phy.h @@ -329,6 +329,7 @@ struct mdio_bus_stats { * struct phy_package_shared - Shared information in PHY packages * @base_addr: Base PHY address of PHY package used to combine PHYs * in one package and for offset calculation of phy_package_read/write + * @np: Pointer to the Device Node if PHY package defined in DT * @refcnt: Number of PHYs connected to this shared data * @flags: Initialization of PHY package * @priv_size: Size of the shared private data @priv @@ -340,6 +341,8 @@ struct mdio_bus_stats { */ struct phy_package_shared { u8 base_addr; + /* With PHY package defined in DT this points to the PHY package node */ + struct device_node *np; refcount_t refcnt; unsigned long flags; size_t priv_size; @@ -1999,9 +2002,12 @@ int phy_ethtool_set_link_ksettings(struct net_device *ndev, const struct ethtool_link_ksettings *cmd); int phy_ethtool_nway_reset(struct net_device *ndev); int phy_package_join(struct phy_device *phydev, int base_addr, size_t priv_size); +int of_phy_package_join(struct phy_device *phydev, size_t priv_size); void phy_package_leave(struct phy_device *phydev); int devm_phy_package_join(struct device *dev, struct phy_device *phydev, int base_addr, size_t priv_size); +int devm_of_phy_package_join(struct device *dev, struct phy_device *phydev, + size_t priv_size); int __init mdio_bus_init(void); void mdio_bus_exit(void);
Add devm/of_phy_package_join helper to join PHYs in a PHY package. These are variant of the manual phy_package_join with the difference that these will use DT nodes to derive the base_addr instead of manually passing an hardcoded value. An additional value is added in phy_package_shared, "np" to reference the PHY package node pointer in specific PHY driver probe_once and config_init_once functions to make use of additional specific properties defined in the PHY package node in DT. The np value is filled only with of_phy_package_join if a valid PHY package node is found. A valid PHY package node must have the node name set to "ethernet-phy-package". Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> --- drivers/net/phy/phy_device.c | 84 ++++++++++++++++++++++++++++++++++++ include/linux/phy.h | 6 +++ 2 files changed, 90 insertions(+)