Re: [PATCH 1/2] net: phy: Make phy_interface_is_sgmii a switch statement

From: "Marek Behún" <marek.behun@nic.cz>
To: Nishanth Menon <nm@ti.com>
Cc: Marek Vasut <marek.vasut+renesas@mailbox.org>,
	Tom Rini <trini@konsulko.com>,
	Siddharth Vadapalli <s-vadapalli@ti.com>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Michal Simek <michal.simek@amd.com>,
	Ramon Fried <rfried.dev@gmail.com>,
	u-boot@lists.denx.de
Subject: Re: [PATCH 1/2] net: phy: Make phy_interface_is_sgmii a switch statement
Date: Thu, 13 Apr 2023 20:56:02 +0200	[thread overview]
Message-ID: <20230413185602.3c4ysaggyxujuvug@kandell> (raw)
In-Reply-To: <20230413180713.2922524-2-nm@ti.com>

On Thu, Apr 13, 2023 at 01:07:12PM -0500, Nishanth Menon wrote:
> Recent commit 75d28899e3e9 ("net: phy: Synchronize PHY interface modes
> with Linux") reordered the enum definitions. This caused the range of
> enums that this api was checking to go bad.
> 
> While it is possible for the phy drivers to practically use the enum's
> directly, drivers such as dp83867 use this helper to manage the
> configuration of the phy correctly.
> 
> Reported-by: Tom Rini <trini@konsulko.com>
> Signed-off-by: Nishanth Menon <nm@ti.com>
> ---
>  include/phy.h | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/include/phy.h b/include/phy.h
> index a837fed72352..1c4dc23bc5ba 100644
> --- a/include/phy.h
> +++ b/include/phy.h
> @@ -373,8 +373,16 @@ static inline bool phy_interface_is_rgmii(struct phy_device *phydev)
>   */
>  static inline bool phy_interface_is_sgmii(struct phy_device *phydev)
>  {
> -	return phydev->interface >= PHY_INTERFACE_MODE_SGMII &&
> -		phydev->interface <= PHY_INTERFACE_MODE_QSGMII;
> +	switch (phydev->interface) {
> +	case PHY_INTERFACE_MODE_SGMII:
> +	case PHY_INTERFACE_MODE_QUSGMII:
> +	case PHY_INTERFACE_MODE_USXGMII:
> +	case PHY_INTERFACE_MODE_QSGMII:
> +		return 1;
> +	default:
> +		fallthrough;

Why not just put the return 0; statement here instead of fallthrough and
drop it from after the switch statement?

> +	}
> +	return 0;
>  }