public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: "Marek Behún" <kabel@kernel.org>
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 V2 1/2] net: phy: Make phy_interface_is_sgmii a switch statement
Date: Fri, 14 Apr 2023 10:38:52 +0200	[thread overview]
Message-ID: <20230414103852.38705065@dellmb> (raw)
In-Reply-To: <20230414042433.3436425-2-nm@ti.com>

On Thu, 13 Apr 2023 23:24:32 -0500
Nishanth Menon <nm@ti.com> 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>
> ---
> Changes Since v1:
> * Switch update based on feedback from Marek
> 
> V1: https://lore.kernel.org/r/20230413180713.2922524-2-nm@ti.com
> 
>  include/phy.h | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/include/phy.h b/include/phy.h
> index a837fed72352..51dadcf14478 100644
> --- a/include/phy.h
> +++ b/include/phy.h
> @@ -373,8 +373,15 @@ 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:
> +		return 0;
> +	}
>  }
>  
>  bool phy_interface_is_ncsi(void);

Reviewed-by: Marek Behún <kabel@kernel.org>

Please also consider dropping this function, since it is used only in
one driver
  drivers/net/phy/dp83867.c
and that device does not actually support QUSGMII, USXGMII nor QSGMII.
It only supports SGMII.

With rgmii, the corresponding phy_interface_is_rgmii() function makes
sense, but for SGMII the four modes are entirely too different for this
to make sense.

Marek

  reply	other threads:[~2023-04-14  8:39 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-14  4:24 [PATCH V2 0/2] net: phy: Make phy_interface_is_sgmii|rgmii a switch statement Nishanth Menon
2023-04-14  4:24 ` [PATCH V2 1/2] net: phy: Make phy_interface_is_sgmii " Nishanth Menon
2023-04-14  8:38   ` Marek Behún [this message]
2023-04-14 11:22   ` Marek Vasut
2023-04-14  4:24 ` [PATCH V2 2/2] net: phy: Make phy_interface_is_rgmii " Nishanth Menon
2023-04-14  8:39   ` Marek Behún
2023-04-14 11:21   ` Marek Vasut

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230414103852.38705065@dellmb \
    --to=kabel@kernel.org \
    --cc=marek.vasut+renesas@mailbox.org \
    --cc=michal.simek@amd.com \
    --cc=nm@ti.com \
    --cc=rfried.dev@gmail.com \
    --cc=s-vadapalli@ti.com \
    --cc=trini@konsulko.com \
    --cc=u-boot@lists.denx.de \
    --cc=vigneshr@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox