> > > +- vsc8531,led-1-mode : LED mode. Specify how the LED[1] should behave. > > > + Allowed values are define in > > > + int rc; > > > + > > > + if (!of_node) > > > + return -ENODEV; > > > + > > > + rc = of_property_read_u8(of_node, led, &led_mode); > > > + if ((rc == 0) && > > > + ((led_mode > 15) || (led_mode == 7) || (led_mode == 11))) { > > > + phydev_err(phydev, "DT %s invalid\n", led); > > > + return -EINVAL; > > > + } else if (rc == -EINVAL) { > > > + return default_mode; > > > + } > > > > I don't think you understood my comment about of_property_read_u8() > > not modifying &led_mode on error. Please read the comment again, and > > simplify this. > > > > If i understand your comment correctly Nope, you don't. rc = of_property_read_u8(of_node, led, &led_mode); If rc indicates an error, led_mode is not touched. Read the documentation for this function. So you can simplifiy the code: led_mode = default_mode; err = of_property_read_u8(of_node, led, &led_mode); if (!err && (led_mode > 15 || led_mode == 7) | (led_mode == 11) { phydev_err(phydev, "DT %s invalid\n", led); return -EINVAL; } return led_mode; Andrew -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html