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); -- Ansuel