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?
> +
> + 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)
> + temp |= val << shift;
> + rc = phy_write(phydev, reg, temp);
> +
> + return rc < 0 ? rc : 0;
You could have;
return phy_write(phydev, reg, temp);
> +}
> +
> static int kszphy_config_init(struct phy_device *phydev)
> {
> return 0;
> }
>
> +static int kszphy_config_init_led8041(struct phy_device *phydev)
> +{
> + /* single led control, register 0x1e bits 15..14 */
> + return kszphy_setup_led(phydev, 0x1e, 14);
> +}
> +
> static int ksz8021_config_init(struct phy_device *phydev)
> {
> - int rc;
> const u16 val = KSZPHY_OMSO_B_CAST_OFF | KSZPHY_OMSO_RMII_OVERRIDE;
> + int rc;
> +
> + rc = kszphy_setup_led(phydev, 0x1f, 4);
> + if (rc)
> + dev_err(&phydev->dev, "failed to set led mode\n");
> +
> phy_write(phydev, MII_KSZPHY_OMSO, val);
> rc = ksz_config_flags(phydev);
> return rc < 0 ? rc : 0;
> @@ -166,6 +203,10 @@ static int ks8051_config_init(struct phy_device *phydev)
> {
> int rc;
>
> + rc = kszphy_setup_led(phydev, 0x1f, 4);
> + if (rc)
> + dev_err(&phydev->dev, "failed to set led mode\n");
> +
> rc = ksz_config_flags(phydev);
> return rc < 0 ? rc : 0;
> }
> @@ -327,7 +368,7 @@ static struct phy_driver ksphy_driver[] = {
> .features = (PHY_BASIC_FEATURES | SUPPORTED_Pause
> | SUPPORTED_Asym_Pause),
> .flags = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
> - .config_init = kszphy_config_init,
> + .config_init = kszphy_config_init_led8041,
> .config_aneg = genphy_config_aneg,
> .read_status = genphy_read_status,
> .ack_interrupt = kszphy_ack_interrupt,
> @@ -342,7 +383,7 @@ static struct phy_driver ksphy_driver[] = {
> .features = PHY_BASIC_FEATURES |
> SUPPORTED_Pause | SUPPORTED_Asym_Pause,
> .flags = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
> - .config_init = kszphy_config_init,
> + .config_init = kszphy_config_init_led8041,
> .config_aneg = genphy_config_aneg,
> .read_status = genphy_read_status,
> .ack_interrupt = kszphy_ack_interrupt,
> @@ -371,7 +412,7 @@ static struct phy_driver ksphy_driver[] = {
> .phy_id_mask = 0x00ffffff,
> .features = (PHY_BASIC_FEATURES | SUPPORTED_Pause),
> .flags = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
> - .config_init = kszphy_config_init,
> + .config_init = kszphy_config_init_led8041,
> .config_aneg = genphy_config_aneg,
> .read_status = genphy_read_status,
> .ack_interrupt = kszphy_ack_interrupt,
> --
> 1.8.5.3
>
--
Florian
--
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
