On Fri, 4 Sep 2020 22:21:57 +0800 Landen Chao wrote:
> +static bool
> +mt7530_phy_mode_supported(struct dsa_switch *ds, int port,
> +                       const struct phylink_link_state *state)
>  {
>       struct mt7530_priv *priv = ds->priv;
> -     u32 mcr_cur, mcr_new;
>  
>       switch (port) {
>       case 0: /* Internal phy */
> @@ -1363,33 +1364,114 @@ static void mt7530_phylink_mac_config(struct 
> dsa_switch *ds, int port,
>       case 3:
>       case 4:
>               if (state->interface != PHY_INTERFACE_MODE_GMII)
> -                     return;
> +                     goto unsupported;

return false;

Jumping to a label which does nothing but returns makes the code less
readable.

>               break;
>       case 5: /* 2nd cpu port with phy of port 0 or 4 / external phy */
> -             if (priv->p5_interface == state->interface)
> -                     break;
>               if (!phy_interface_mode_is_rgmii(state->interface) &&
>                   state->interface != PHY_INTERFACE_MODE_MII &&
>                   state->interface != PHY_INTERFACE_MODE_GMII)
> -                     return;
> +                     goto unsupported;
> +             break;
> +     case 6: /* 1st cpu port */
> +             if (state->interface != PHY_INTERFACE_MODE_RGMII &&
> +                 state->interface != PHY_INTERFACE_MODE_TRGMII)
> +                     goto unsupported;
> +             break;
> +     default:
> +             dev_err(priv->dev, "%s: unsupported port: %i\n", __func__,
> +                     port);
> +             goto unsupported;
> +     }
> +
> +     return true;
> +
> +unsupported:
> +     return false;
> +}

> +static void
> +mt753x_phylink_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
> +                       const struct phylink_link_state *state)
> +{
> +     struct mt7530_priv *priv = ds->priv;
> +     u32 mcr_cur, mcr_new;
> +
> +     if (!mt753x_phy_mode_supported(ds, port, state))
> +             goto unsupported;
> +
> +     switch (port) {
> +     case 0: /* Internal phy */
> +     case 1:
> +     case 2:
> +     case 3:
> +     case 4:

case 0 ... 4:

> +             if (state->interface != PHY_INTERFACE_MODE_GMII)
> +                     goto unsupported;
> +             break;
> +     case 5: /* 2nd cpu port with phy of port 0 or 4 / external phy */
> +             if (priv->p5_interface == state->interface)
> +                     break;

> +static void
> +mt753x_phylink_validate(struct dsa_switch *ds, int port,
> +                     unsigned long *supported,
> +                     struct phylink_link_state *state)
> +{
> +     struct mt7530_priv *priv = ds->priv;
>       __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };

Please keep the variables longest to shortest (reverse xmas tree).

Reply via email to