> +static int dp83640_led_brightness_set(struct phy_device *phydev, u8 index, > + enum led_brightness brightness) > +{ > + int val; > + > + if (index > DP83640_SPDLED_IDX) > + return -EINVAL; > + > + phy_write(phydev, PAGESEL, 0); > + > + val = phy_read(phydev, LEDCR) & ~DP83640_LED_DIS(index); > + val |= DP83640_LED_DRV(index); > + if (brightness) > + val |= DP83640_LED_VAL(index); > + else > + val &= ~DP83640_LED_VAL(index); > + phy_write(phydev, LEDCR, val); I don't understand this driver too well, but should this be using ext_read() and ext_write(). I'm also woundering if it would be good to implement the .read_page and .write_page members in phy_driver and then use phy_write_paged() and phy_write_paged() and phy_modify_paged(), which should do better locking. Andrew