On Mon, Jun 08, 2020 at 06:02:07PM +0200, Heiko Stuebner wrote: > +static int vsc8531_probe(struct phy_device *phydev) > +{ > + struct vsc8531_private *vsc8531; > + int rate_magic; > + u32 default_mode[2] = {VSC8531_LINK_1000_ACTIVITY, > + VSC8531_LINK_100_ACTIVITY}; > + > + rate_magic = vsc85xx_edge_rate_magic_get(phydev); > + if (rate_magic < 0) > + return rate_magic; > + > + vsc8531 = devm_kzalloc(&phydev->mdio.dev, sizeof(*vsc8531), GFP_KERNEL); > + if (!vsc8531) > + return -ENOMEM; > + > + phydev->priv = vsc8531; > + > + vsc8531->rate_magic = rate_magic; > + vsc8531->nleds = 2; > + vsc8531->supp_led_modes = VSC85XX_SUPP_LED_MODES; > + vsc8531->hw_stats = vsc85xx_hw_stats; > + vsc8531->nstats = ARRAY_SIZE(vsc85xx_hw_stats); > + vsc8531->stats = devm_kcalloc(&phydev->mdio.dev, vsc8531->nstats, > + sizeof(u64), GFP_KERNEL); > + if (!vsc8531->stats) > + return -ENOMEM; > + > + vsc8531_dt_clkout_rate_get(phydev); > + > + return vsc85xx_dt_led_modes_get(phydev, default_mode); > +} Hi Heiko The clock change itself looks O.K. Maybe we want to standardize on the name of the DT property, since it could be shared across all PHYs which have a clock output? Could you add another patch first which refactors the _probe() functions. There is a lot of repeated code which could be put into a helper. Thanks Andrew