netdev - Re: [PATCH net 1/4] rtase: Refactor the rtase_check_mac_version

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2fac05ba-7766-4586-8676-e30f09cd2d09@lunn.ch>
Date: Thu, 14 Nov 2024 16:38:08 +0100
From: Andrew Lunn <andrew@...n.ch>
To: Justin Lai <justinlai0215@...ltek.com>
Cc: kuba@...nel.org, davem@...emloft.net, edumazet@...gle.com,
	pabeni@...hat.com, andrew+netdev@...n.ch,
	linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
	horms@...nel.org, pkshih@...ltek.com, larry.chiu@...ltek.com
Subject: Re: [PATCH net 1/4] rtase: Refactor the
 rtase_check_mac_version_valid() function

On Thu, Nov 14, 2024 at 07:14:40PM +0800, Justin Lai wrote:
> 1. Sets tp->hw_ver.
> 2. Changes the return type from bool to int.
> 
> Signed-off-by: Justin Lai <justinlai0215@...ltek.com>
> ---
>  drivers/net/ethernet/realtek/rtase/rtase.h    |  2 ++
>  .../net/ethernet/realtek/rtase/rtase_main.c   | 21 +++++++++++--------
>  2 files changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/ethernet/realtek/rtase/rtase.h b/drivers/net/ethernet/realtek/rtase/rtase.h
> index 583c33930f88..547c71937b01 100644
> --- a/drivers/net/ethernet/realtek/rtase/rtase.h
> +++ b/drivers/net/ethernet/realtek/rtase/rtase.h
> @@ -327,6 +327,8 @@ struct rtase_private {
>  	u16 int_nums;
>  	u16 tx_int_mit;
>  	u16 rx_int_mit;
> +
> +	u32 hw_ver;
>  };
>  
>  #define RTASE_LSO_64K 64000
> diff --git a/drivers/net/ethernet/realtek/rtase/rtase_main.c b/drivers/net/ethernet/realtek/rtase/rtase_main.c
> index f8777b7663d3..33808afd588d 100644
> --- a/drivers/net/ethernet/realtek/rtase/rtase_main.c
> +++ b/drivers/net/ethernet/realtek/rtase/rtase_main.c
> @@ -1972,20 +1972,21 @@ static void rtase_init_software_variable(struct pci_dev *pdev,
>  	tp->dev->max_mtu = RTASE_MAX_JUMBO_SIZE;
>  }
>  
> -static bool rtase_check_mac_version_valid(struct rtase_private *tp)
> +static int rtase_check_mac_version_valid(struct rtase_private *tp)
>  {
> -	u32 hw_ver = rtase_r32(tp, RTASE_TX_CONFIG_0) & RTASE_HW_VER_MASK;
> -	bool known_ver = false;
> +	int ret = -ENODEV;
>  
> -	switch (hw_ver) {
> +	tp->hw_ver = rtase_r32(tp, RTASE_TX_CONFIG_0) & RTASE_HW_VER_MASK;
> +
> +	switch (tp->hw_ver) {
>  	case 0x00800000:
>  	case 0x04000000:
>  	case 0x04800000:

Since these magic numbers are being used in more places, please add
some #define with sensible names.

> -	if (!rtase_check_mac_version_valid(tp))
> -		return dev_err_probe(&pdev->dev, -ENODEV,
> -				     "unknown chip version, contact rtase maintainers (see MAINTAINERS file)\n");
> +	ret = rtase_check_mac_version_valid(tp);
> +	if (ret != 0) {
> +		dev_err(&pdev->dev,
> +			"unknown chip version, contact rtase maintainers (see MAINTAINERS file)\n");
> +	}

Since you are changing this, maybe include the hw_ver?

	Andrew