On 26/02/14 22:20, Florian Fainelli wrote:
Hi,
2014-02-26 3:48 GMT-08:00 Ben Dooks <ben.dooks@xxxxxxxxxxxxxxx>:
[snip]
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 5a8993b..0c9e434 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -148,15 +148,52 @@ static int ks8737_config_intr(struct phy_device *phydev)
return rc < 0 ? rc : 0;
}
+static int kszphy_setup_led(struct phy_device *phydev,
+ unsigned int reg, unsigned int shift)
+{
+
+ struct device *dev = &phydev->dev;
+ struct device_node *of_node = dev->of_node;
+ int rc, temp;
+ u32 val;
+
+ if (!of_node && dev->parent->of_node)
+ of_node = dev->parent->of_node;
+
+ if (of_property_read_u32(of_node, "micrel,led-mode", &val))
+ return 0;
This breaks non-OF configuration because of_read_property_read_u32()
will return -ENOSYS, so you skip the LED register configuration
entirely, is that intended?
Yes, it is only here for OF case. I should however check if
of_node is available for the !OF case.
+
+ temp = phy_read(phydev, reg);
+ if (temp < 0)
+ return temp;
+
+ temp &= 3 << shift;
The compiler cannot verify that we are not overflowing, you might want
to make sure that shift <= 14 (just in case)
Ok, should I add a WARN_ON(shift > 14) ?
+ temp |= val << shift;
+ rc = phy_write(phydev, reg, temp);
+
+ return rc < 0 ? rc : 0;
You could have;
return phy_write(phydev, reg, temp);
Thanks, will change to doing that.
--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
--
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
