Re: [PATCH v2 net-next] net: phy: Add LED mode driver for Microsemi PHYs.

[Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>]

> > > +- 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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html