[U-Boot] [PATCH 076/080] phy: Fix style violations

Joe Hershberger joe.hershberger at ni.com
Tue Dec 5 20:17:37 UTC 2017


On Fri, Sep 29, 2017 at 7:52 AM, Mario Six <mario.six at gdsys.cc> wrote:
> Fix some style violations in the generic PHY management code.
>
> Signed-off-by: Mario Six <mario.six at gdsys.cc>
> ---
>  drivers/net/phy/phy.c | 82 ++++++++++++++++++++++++++++++---------------------
>  1 file changed, 49 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index 5be51d73ce..0050a70075 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -27,7 +27,7 @@ DECLARE_GLOBAL_DATA_PTR;
>  /* Generic PHY support and helper functions */
>
>  /**
> - * genphy_config_advert - sanitize and advertise auto-negotation parameters
> + * genphy_config_advert - sanitize and advertise auto-negotiation parameters
>   * @phydev: target phy_device struct
>   *
>   * Description: Writes MII_ADVERTISE with the appropriate values,
> @@ -117,7 +117,6 @@ static int genphy_config_advert(struct phy_device *phydev)
>         return changed;
>  }
>
> -
>  /**
>   * genphy_setup_forced - configures/forces speed/duplex from @phydev
>   * @phydev: target phy_device struct
> @@ -130,14 +129,15 @@ static int genphy_setup_forced(struct phy_device *phydev)
>         int err;
>         int ctl = BMCR_ANRESTART;
>
> -       phydev->pause = phydev->asym_pause = 0;
> +       phydev->pause = 0;
> +       phydev->asym_pause = 0;
>
> -       if (SPEED_1000 == phydev->speed)
> +       if (phydev->speed == SPEED_1000)
>                 ctl |= BMCR_SPEED1000;
> -       else if (SPEED_100 == phydev->speed)
> +       else if (phydev->speed == SPEED_100)
>                 ctl |= BMCR_SPEED100;
>
> -       if (DUPLEX_FULL == phydev->duplex)
> +       if (phydev->duplex == DUPLEX_FULL)
>                 ctl |= BMCR_FULLDPLX;
>
>         err = phy_write(phydev, MDIO_DEVAD_NONE, MII_BMCR, ctl);
> @@ -145,7 +145,6 @@ static int genphy_setup_forced(struct phy_device *phydev)
>         return err;
>  }
>
> -
>  /**
>   * genphy_restart_aneg - Enable and Restart Autonegotiation
>   * @phydev: target phy_device struct
> @@ -169,7 +168,6 @@ int genphy_restart_aneg(struct phy_device *phydev)
>         return ctl;
>  }
>
> -
>  /**
>   * genphy_config_aneg - restart auto-negotiation or write BMCR
>   * @phydev: target phy_device struct
> @@ -182,7 +180,7 @@ int genphy_config_aneg(struct phy_device *phydev)
>  {
>         int result;
>
> -       if (AUTONEG_ENABLE != phydev->autoneg)
> +       if (phydev->autoneg != AUTONEG_ENABLE)
>                 return genphy_setup_forced(phydev);
>
>         result = genphy_config_advert(phydev);
> @@ -192,7 +190,8 @@ int genphy_config_aneg(struct phy_device *phydev)
>
>         if (result == 0) {
>                 /* Advertisment hasn't changed, but maybe aneg was never on to

Shouldn't this text start on the line after "/*" ?

> -                * begin with?  Or maybe phy was isolated? */
> +                * begin with?  Or maybe phy was isolated?
> +                */
>                 int ctl = phy_read(phydev, MDIO_DEVAD_NONE, MII_BMCR);
>
>                 if (ctl < 0)
> @@ -203,7 +202,8 @@ int genphy_config_aneg(struct phy_device *phydev)
>         }
>
>         /* Only restart aneg if we are advertising something different

Same here.

> -        * than we were before.  */
> +        * than we were before.
> +        */
>         if (result > 0)
>                 result = genphy_restart_aneg(phydev);
>
> @@ -240,7 +240,7 @@ int genphy_update_link(struct phy_device *phydev)
>                 int i = 0;
>
>                 printf("%s Waiting for PHY auto negotiation to complete",
> -                       phydev->dev->name);
> +                      phydev->dev->name);
>                 while (!(mii_reg & BMSR_ANEGCOMPLETE)) {
>                         /*
>                          * Timeout reached ?
> @@ -305,7 +305,8 @@ int genphy_parse_link(struct phy_device *phydev)
>                          */
>                         gblpa = phy_read(phydev, MDIO_DEVAD_NONE, MII_STAT1000);
>                         if (gblpa < 0) {
> -                               debug("Could not read MII_STAT1000. Ignoring gigabit capability\n");
> +                               debug("Could not read MII_STAT1000. ");
> +                               debug("Ignoring gigabit capability\n");
>                                 gblpa = 0;
>                         }
>                         gblpa &= phy_read(phydev,
> @@ -338,8 +339,9 @@ int genphy_parse_link(struct phy_device *phydev)
>                         if (lpa & LPA_100FULL)
>                                 phydev->duplex = DUPLEX_FULL;
>
> -               } else if (lpa & LPA_10FULL)
> +               } else if (lpa & LPA_10FULL) {
>                         phydev->duplex = DUPLEX_FULL;
> +               }
>
>                 /*
>                  * Extended status may indicate that the PHY supports
> @@ -574,7 +576,9 @@ static int phy_probe(struct phy_device *phydev)
>  {
>         int err = 0;
>
> -       phydev->advertising = phydev->supported = phydev->drv->features;
> +       phydev->advertising = phydev->drv->features;
> +       phydev->supported = phydev->drv->features;
> +
>         phydev->mmds = phydev->drv->mmds;
>
>         if (phydev->drv->probe)
> @@ -594,7 +598,7 @@ static struct phy_driver *generic_for_interface(phy_interface_t interface)
>  }
>
>  static struct phy_driver *get_phy_driver(struct phy_device *phydev,
> -                               phy_interface_t interface)
> +                                        phy_interface_t interface)
>  {
>         struct list_head *entry;
>         int phy_id = phydev->phy_id;
> @@ -617,11 +621,12 @@ static struct phy_device *phy_device_create(struct mii_dev *bus, int addr,
>         struct phy_device *dev;
>
>         /* We allocate the device, and initialize the

Same here.

> -        * default values */
> +        * default values
> +        */
>         dev = malloc(sizeof(*dev));
>         if (!dev) {
>                 printf("Failed to allocate PHY device for %s:%d\n",
> -                       bus->name, addr);
> +                      bus->name, addr);
>                 return NULL;
>         }
>
> @@ -660,7 +665,8 @@ int __weak get_phy_id(struct mii_dev *bus, int addr, int devad, u32 *phy_id)
>         int phy_reg;
>
>         /* Grab the bits from PHYIR1, and put them

Same here.


> -        * in the upper half */
> +        * in the upper half
> +        */
>         phy_reg = bus->read(bus, addr, devad, MII_PHYSID1);
>
>         if (phy_reg < 0)
> @@ -680,9 +686,11 @@ int __weak get_phy_id(struct mii_dev *bus, int addr, int devad, u32 *phy_id)
>  }
>
>  static struct phy_device *create_phy_by_mask(struct mii_dev *bus,
> -               unsigned phy_mask, int devad, phy_interface_t interface)
> +                                            uint phy_mask, int devad,
> +                                            phy_interface_t interface)
>  {
>         u32 phy_id = 0xffffffff;
> +
>         while (phy_mask) {
>                 int addr = ffs(phy_mask) - 1;
>                 int r = get_phy_id(bus, addr, devad, &phy_id);
> @@ -695,11 +703,13 @@ static struct phy_device *create_phy_by_mask(struct mii_dev *bus,
>  }
>
>  static struct phy_device *search_for_existing_phy(struct mii_dev *bus,
> -               unsigned phy_mask, phy_interface_t interface)
> +                                                 uint phy_mask,
> +                                                 phy_interface_t interface)
>  {
>         /* If we have one, return the existing device, with new interface */
>         while (phy_mask) {
>                 int addr = ffs(phy_mask) - 1;
> +
>                 if (bus->phymap[addr]) {
>                         bus->phymap[addr]->interface = interface;
>                         return bus->phymap[addr];
> @@ -710,7 +720,8 @@ static struct phy_device *search_for_existing_phy(struct mii_dev *bus,
>  }
>
>  static struct phy_device *get_phy_device_by_mask(struct mii_dev *bus,
> -               unsigned phy_mask, phy_interface_t interface)
> +                                                uint phy_mask,
> +                                                phy_interface_t interface)
>  {
>         int i;
>         struct phy_device *phydev;
> @@ -722,7 +733,7 @@ static struct phy_device *get_phy_device_by_mask(struct mii_dev *bus,
>         /* Otherwise we have to try Clause 45 */
>         for (i = 0; i < 5; i++) {
>                 phydev = create_phy_by_mask(bus, phy_mask,
> -                               i ? i : MDIO_DEVAD_NONE, interface);
> +                                           i ? i : MDIO_DEVAD_NONE, interface);
>                 if (IS_ERR(phydev))
>                         return NULL;
>                 if (phydev)
> @@ -732,6 +743,7 @@ static struct phy_device *get_phy_device_by_mask(struct mii_dev *bus,
>         debug("\n%s PHY: ", bus->name);
>         while (phy_mask) {
>                 int addr = ffs(phy_mask) - 1;
> +
>                 debug("%d ", addr);
>                 phy_mask &= ~(1 << addr);
>         }
> @@ -741,7 +753,8 @@ static struct phy_device *get_phy_device_by_mask(struct mii_dev *bus,
>  }
>
>  /**
> - * get_phy_device - reads the specified PHY device and returns its @phy_device struct
> + * get_phy_device - reads the specified PHY device and returns its
> + *                  @phy_device struct
>   * @bus: the target MII bus
>   * @addr: PHY address on the MII bus
>   *
> @@ -820,15 +833,15 @@ int miiphy_reset(const char *devname, unsigned char addr)
>         return phy_reset(phydev);
>  }
>
> -struct phy_device *phy_find_by_mask(struct mii_dev *bus, unsigned phy_mask,
> -               phy_interface_t interface)
> +struct phy_device *phy_find_by_mask(struct mii_dev *bus, uint phy_mask,
> +                                     phy_interface_t interface)
>  {
>         /* Reset the bus */
>         if (bus->reset) {
>                 bus->reset(bus);
>
>                 /* Wait 15ms to make sure the PHY has come out of hard reset */
> -               udelay(15000);
> +               mdelay(15);
>         }
>
>         return get_phy_device_by_mask(bus, phy_mask, interface);
> @@ -844,8 +857,8 @@ void phy_connect_dev(struct phy_device *phydev, struct eth_device *dev)
>         phy_reset(phydev);
>         if (phydev->dev && phydev->dev != dev) {
>                 printf("%s:%d is connected to %s.  Reconnecting to %s\n",
> -                               phydev->bus->name, phydev->addr,
> -                               phydev->dev->name, dev->name);
> +                      phydev->bus->name, phydev->addr,
> +                      phydev->dev->name, dev->name);
>         }
>         phydev->dev = dev;
>         debug("%s connected to %s\n", dev->name, phydev->drv->name);
> @@ -853,20 +866,23 @@ void phy_connect_dev(struct phy_device *phydev, struct eth_device *dev)
>
>  #ifdef CONFIG_DM_ETH
>  struct phy_device *phy_connect(struct mii_dev *bus, int addr,
> -               struct udevice *dev, phy_interface_t interface)
> +                              struct udevice *dev,
> +                              phy_interface_t interface)
>  #else
>  struct phy_device *phy_connect(struct mii_dev *bus, int addr,
> -               struct eth_device *dev, phy_interface_t interface)
> +                              struct eth_device *dev,
> +                              phy_interface_t interface)
>  #endif
>  {
>         struct phy_device *phydev = NULL;
>  #ifdef CONFIG_PHY_FIXED
>         int sn;
>         const char *name;
> +
>         sn = fdt_first_subnode(gd->fdt_blob, dev_of_offset(dev));
>         while (sn > 0) {
>                 name = fdt_get_name(gd->fdt_blob, sn, NULL);
> -               if (name != NULL && strcmp(name, "fixed-link") == 0) {
> +               if (name && strcmp(name, "fixed-link") == 0) {
>                         phydev = phy_device_create(bus,
>                                                    sn, PHY_FIXED_ID, interface);
>                         break;
> @@ -874,7 +890,7 @@ struct phy_device *phy_connect(struct mii_dev *bus, int addr,
>                 sn = fdt_next_subnode(gd->fdt_blob, sn);
>         }
>  #endif
> -       if (phydev == NULL)
> +       if (!phydev)
>                 phydev = phy_find_by_mask(bus, 1 << addr, interface);
>
>         if (phydev)
> --
> 2.11.0
>


More information about the U-Boot mailing list