On Wed, Dec 06, 2023 at 01:44:17AM +0000, Daniel Golle wrote: > +struct phylink_pcs *mtk_pcs_lynxi_select_pcs(struct device_node *np, phy_interface_t mode) > +{ > + struct platform_device *pdev; > + struct mtk_pcs_lynxi *mpcs; > + > + if (!np) > + return NULL; > + > + if (!of_device_is_available(np)) > + return ERR_PTR(-ENODEV); > + > + if (!of_match_node(mtk_pcs_lynxi_of_match, np)) > + return ERR_PTR(-EINVAL); > + > + pdev = of_find_device_by_node(np); > + if (!pdev || !platform_get_drvdata(pdev)) { > + if (pdev) > + put_device(&pdev->dev); > + return ERR_PTR(-EPROBE_DEFER); > + } > + > + mpcs = platform_get_drvdata(pdev); > + put_device(&pdev->dev); > + > + return &mpcs->pcs; > +} > +EXPORT_SYMBOL(mtk_pcs_lynxi_select_pcs); If you're going to play games like this, then you must mark the driver with .suppress_bind_attrs = true to remove the bind/unbind attributes in userspace that could wreak havoc with the above - because there is _nothing_ that guarantees that the memory you're returning from this function will remain intact. Basically, it's racy. Also, I'm not sure I approve of using the "select_pcs" suffix (I haven't spotted _where_ you use this, but returning EPROBE_DEFER to phylink's mac_select_pcs() method doesn't do anything to defer any probe, so that's an entirely misleading error code. If we are going to have device drivers for PCS, then we need to seriously think about how we look up PCS and return the phylink_pcs pointer - and also how we handle the PCS device going away. None of that should be coded into _any_ PCS driver. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!