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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170207131223.GE31560@lunn.ch>
Date:   Tue, 7 Feb 2017 14:12:23 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Raju Lakkaraju <Raju.Lakkaraju@...rosemi.com>
Cc:     netdev@...r.kernel.org, devicetree@...r.kernel.org,
        f.fainelli@...il.com, Allan.Nielsen@...rosemi.com
Subject: Re: [PATCH v2 net-next] net: phy: Add LED mode driver for Microsemi
 PHYs.

> > > +- 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